diff --git a/.claude/skills/prd-writing/SKILL.md b/.claude/skills/prd-writing/SKILL.md index 1cca925d8..08a539fea 100644 --- a/.claude/skills/prd-writing/SKILL.md +++ b/.claude/skills/prd-writing/SKILL.md @@ -26,7 +26,7 @@ Both are plausible, well-written, and wrong. Reading the code first catches both 4. **For every mechanism, trace every downstream consumer — not just the first one you find.** The single highest-value question before calling a design complete: "where else does this exact same check or logic get independently re-derived?" In a codebase without one source of truth for a concept (e.g. "is this record currently active" computed by three different queries in three different files), patching the first occurrence and stopping is the most common way a design ships with a hidden, silent gap. Grep for the pattern, not just the function you already know about. 5. **Record rejected alternatives with the reasoning, not just the chosen design.** Give it its own subsection (`### Rejected: X`). Without this, a future reader — or your own future self — re-proposes the rejected idea because the "why not" only ever existed in a conversation, not in the document. 6. **Force every open question to an explicit decision**, even if the decision is "accept as-is for v1, revisit if feedback says otherwise." An open question left unresolved in a PRD gets silently decided by whoever implements it — usually differently than anyone intended. -7. **Write the test plan as part of the PRD, not after.** Concrete test cases — naming real functions/queries, not "add tests for X" — force you to notice design gaps you'd otherwise miss; the moment you try to write "assert Y happens" and realize the current design can't produce Y is often the first time the gap becomes visible. Check the repo for an existing test pattern for this shape of change before inventing a new one (e.g. a prior presence-logic bug fixed via `test/db_test_helpers.py` fixtures is the template for the next one, not a reason to build new test infrastructure). When execution actually starts, write that test code before the implementation, run it against the pre-fix code, and confirm it fails for the right reason (a real assertion mismatch, not a collection/import error) before writing a line of the fix. A test that never failed red can't be trusted to have caught anything - it's usually the first sign the fixture doesn't actually distinguish broken from fixed behavior (a real case: a single-device DB fixture passed against both the buggy and the fixed code, because nothing in it could tell "this row matches" from "any row matches" - see `scan-pipeline` Gotcha 8). Writing the test first also tends to surface implementation code that's awkward to exercise in isolation - treat that as a refactor signal, not friction to route around. +7. **Write the test plan as part of the PRD, not after.** Concrete test cases — naming real functions/queries, not "add tests for X" — force you to notice design gaps you'd otherwise miss; the moment you try to write "assert Y happens" and realize the current design can't produce Y is often the first time the gap becomes visible. Check the repo for an existing test pattern for this shape of change before inventing a new one (e.g. a prior presence-logic bug fixed via `test/db_test_helpers.py` fixtures is the template for the next one, not a reason to build new test infrastructure). When execution actually starts, write that test code before the implementation, run it against the pre-fix code, and confirm it fails for the right reason before writing a line of the fix. What "the right reason" means depends on what's being tested: a regression test against existing behavior must fail with a real behavioral assertion mismatch, not a collection/import error - a test that never failed red that way can't be trusted to have caught anything, and is usually the first sign the fixture doesn't actually distinguish broken from fixed behavior (a real case: a single-device DB fixture passed against both the buggy and the fixed code, because nothing in it could tell "this row matches" from "any row matches" - see `scan-pipeline` Gotcha 8). A test for a brand-new contract (a function/interface that doesn't exist yet) legitimately fails with a missing-interface error instead (`AttributeError`, `ImportError`) before it's written - that's the expected red for that case, not a sign the test is wrong. Writing the test first also tends to surface implementation code that's awkward to exercise in isolation - treat that as a refactor signal, not friction to route around. 8. **Ask explicitly whether validating this needs real end-to-end infrastructure** (a new or modified plugin, a UI click-through) or whether synthetic unit-level fixtures suffice — don't assume either way. Check whether the functions under test take a DB connection/dict/list as a parameter (testable in isolation, no real plugin needed) or require a real file on disk (harder to fake, may need one). 9. **Ask explicitly whether this feature/mechanism should be exposed via the API, and give a recommendation, not just flag it as an open question.** This codebase already has three real API surfaces to consider extending rather than inventing a fourth: REST (`server/api_server/*_endpoint.py`), GraphQL (`graphql_endpoint.py`), and a Prometheus `/metrics` endpoint (`prometheus_endpoint.py`). Skipping this question doesn't mean "no API access needed"; it means the answer gets silently decided later by whoever first wants to query the new data externally, usually as its own separate feature request re-litigating a design this PRD already had full context to settle. Not everything needs exposure: purely internal/diagnostic state with no plausible external consumer doesn't, but say so explicitly, with the reason, rather than leaving it unaddressed. 10. **Check performance against the real schema and real scale, not assumptions.** For every new or changed query: does it use an existing index, or add an unindexed lookup, a new join, or a correlated subquery? Grep `CREATE INDEX` for *every* index on the tables involved, not just the first one you find — a column can have both a plain index and a separate expression index (e.g. `idx_eve_mac_date_type ON Events(eveMac, ...)` alongside `idx_eve_lower_mac_date_type ON Events(LOWER(eveMac), ...)`), and missing the second one produces a wrong verdict. Then weigh cost by how often the query runs (once is nothing; every few minutes forever is a standing cost) and by real scale — **production users run 10,000+ devices**, not a homelab handful. A `CurrentScan` with 2-5 rows per device (one per contributing plugin) is routinely 20,000-50,000+ rows in one cycle; reason about that number, not a smaller hopeful one. A correlated `EXISTS`/subquery re-evaluated per outer row is fine *if* the correlated column is indexed — e.g. `current_scan_presence_condition()` (`server/scan/presence.py`) is exactly this shape against `CurrentScan.scanMac`, covered by `idx_currentscan_scanmac`. The real risk is an unindexed correlated lookup: an accidental self-join scanning the full inner table per outer row, which looks fine and passes tests at small scale but isn't at production scale. Check with `EXPLAIN QUERY PLAN` at a realistic row count, built against the *complete* real index set (copy every `CREATE INDEX` for the table, or run it against an actual `app.db`) rather than a hand-picked subset — a partial index set produces a misleading plan in either direction, not just "looks worse than it is." If it comes back unindexed for real, a `GROUP BY` aggregate is the usual fix. diff --git a/.gemini/skills/prd-writing/SKILL.md b/.gemini/skills/prd-writing/SKILL.md index 8d56a7389..41fdb1896 100644 --- a/.gemini/skills/prd-writing/SKILL.md +++ b/.gemini/skills/prd-writing/SKILL.md @@ -26,7 +26,7 @@ Both are plausible, well-written, and wrong. Reading the code first catches both 4. **For every mechanism, trace every downstream consumer — not just the first one you find.** The single highest-value question before calling a design complete: "where else does this exact same check or logic get independently re-derived?" In a codebase without one source of truth for a concept (e.g. "is this record currently active" computed by three different queries in three different files), patching the first occurrence and stopping is the most common way a design ships with a hidden, silent gap. Grep for the pattern, not just the function you already know about. 5. **Record rejected alternatives with the reasoning, not just the chosen design.** Give it its own subsection (`### Rejected: X`). Without this, a future reader — or your own future self — re-proposes the rejected idea because the "why not" only ever existed in a conversation, not in the document. 6. **Force every open question to an explicit decision**, even if the decision is "accept as-is for v1, revisit if feedback says otherwise." An open question left unresolved in a PRD gets silently decided by whoever implements it — usually differently than anyone intended. -7. **Write the test plan as part of the PRD, not after.** Concrete test cases — naming real functions/queries, not "add tests for X" — force you to notice design gaps you'd otherwise miss; the moment you try to write "assert Y happens" and realize the current design can't produce Y is often the first time the gap becomes visible. Check the repo for an existing test pattern for this shape of change before inventing a new one (e.g. a prior presence-logic bug fixed via `test/db_test_helpers.py` fixtures is the template for the next one, not a reason to build new test infrastructure). When execution actually starts, write that test code before the implementation, run it against the pre-fix code, and confirm it fails for the right reason (a real assertion mismatch, not a collection/import error) before writing a line of the fix. A test that never failed red can't be trusted to have caught anything - it's usually the first sign the fixture doesn't actually distinguish broken from fixed behavior (a real case: a single-device DB fixture passed against both the buggy and the fixed code, because nothing in it could tell "this row matches" from "any row matches" - see `scan-pipeline` Gotcha 8). Writing the test first also tends to surface implementation code that's awkward to exercise in isolation - treat that as a refactor signal, not friction to route around. +7. **Write the test plan as part of the PRD, not after.** Concrete test cases — naming real functions/queries, not "add tests for X" — force you to notice design gaps you'd otherwise miss; the moment you try to write "assert Y happens" and realize the current design can't produce Y is often the first time the gap becomes visible. Check the repo for an existing test pattern for this shape of change before inventing a new one (e.g. a prior presence-logic bug fixed via `test/db_test_helpers.py` fixtures is the template for the next one, not a reason to build new test infrastructure). When execution actually starts, write that test code before the implementation, run it against the pre-fix code, and confirm it fails for the right reason before writing a line of the fix. What "the right reason" means depends on what's being tested: a regression test against existing behavior must fail with a real behavioral assertion mismatch, not a collection/import error - a test that never failed red that way can't be trusted to have caught anything, and is usually the first sign the fixture doesn't actually distinguish broken from fixed behavior (a real case: a single-device DB fixture passed against both the buggy and the fixed code, because nothing in it could tell "this row matches" from "any row matches" - see `scan-pipeline` Gotcha 8). A test for a brand-new contract (a function/interface that doesn't exist yet) legitimately fails with a missing-interface error instead (`AttributeError`, `ImportError`) before it's written - that's the expected red for that case, not a sign the test is wrong. Writing the test first also tends to surface implementation code that's awkward to exercise in isolation - treat that as a refactor signal, not friction to route around. 8. **Ask explicitly whether validating this needs real end-to-end infrastructure** (a new or modified plugin, a UI click-through) or whether synthetic unit-level fixtures suffice — don't assume either way. Check whether the functions under test take a DB connection/dict/list as a parameter (testable in isolation, no real plugin needed) or require a real file on disk (harder to fake, may need one). 9. **Ask explicitly whether this feature/mechanism should be exposed via the API, and give a recommendation, not just flag it as an open question.** This codebase already has three real API surfaces to consider extending rather than inventing a fourth: REST (`server/api_server/*_endpoint.py`), GraphQL (`graphql_endpoint.py`), and a Prometheus `/metrics` endpoint (`prometheus_endpoint.py`). Skipping this question doesn't mean "no API access needed"; it means the answer gets silently decided later by whoever first wants to query the new data externally, usually as its own separate feature request re-litigating a design this PRD already had full context to settle. Not everything needs exposure: purely internal/diagnostic state with no plausible external consumer doesn't, but say so explicitly, with the reason, rather than leaving it unaddressed. 10. **Check performance against the real schema and real scale, not assumptions.** For every new or changed query: does it use an existing index, or add an unindexed lookup, a new join, or a correlated subquery? Grep `CREATE INDEX` for *every* index on the tables involved, not just the first one you find — a column can have both a plain index and a separate expression index (e.g. `idx_eve_mac_date_type ON Events(eveMac, ...)` alongside `idx_eve_lower_mac_date_type ON Events(LOWER(eveMac), ...)`), and missing the second one produces a wrong verdict. Then weigh cost by how often the query runs (once is nothing; every few minutes forever is a standing cost) and by real scale — **production users run 10,000+ devices**, not a homelab handful. A `CurrentScan` with 2-5 rows per device (one per contributing plugin) is routinely 20,000-50,000+ rows in one cycle; reason about that number, not a smaller hopeful one. A correlated `EXISTS`/subquery re-evaluated per outer row is fine *if* the correlated column is indexed — e.g. `current_scan_presence_condition()` (`server/scan/presence.py`) is exactly this shape against `CurrentScan.scanMac`, covered by `idx_currentscan_scanmac`. The real risk is an unindexed correlated lookup: an accidental self-join scanning the full inner table per outer row, which looks fine and passes tests at small scale but isn't at production scale. Check with `EXPLAIN QUERY PLAN` at a realistic row count, built against the *complete* real index set (copy every `CREATE INDEX` for the table, or run it against an actual `app.db`) rather than a hand-picked subset — a partial index set produces a misleading plan in either direction, not just "looks worse than it is." If it comes back unindexed for real, a `GROUP BY` aggregate is the usual fix. diff --git a/.github/skills/prd-writing/SKILL.md b/.github/skills/prd-writing/SKILL.md index 18c6d6ba8..c0fbdbe76 100644 --- a/.github/skills/prd-writing/SKILL.md +++ b/.github/skills/prd-writing/SKILL.md @@ -26,7 +26,7 @@ Both are plausible, well-written, and wrong. Reading the code first catches both 4. **For every mechanism, trace every downstream consumer — not just the first one you find.** The single highest-value question before calling a design complete: "where else does this exact same check or logic get independently re-derived?" In a codebase without one source of truth for a concept (e.g. "is this record currently active" computed by three different queries in three different files), patching the first occurrence and stopping is the most common way a design ships with a hidden, silent gap. Grep for the pattern, not just the function you already know about. 5. **Record rejected alternatives with the reasoning, not just the chosen design.** Give it its own subsection (`### Rejected: X`). Without this, a future reader — or your own future self — re-proposes the rejected idea because the "why not" only ever existed in a conversation, not in the document. 6. **Force every open question to an explicit decision**, even if the decision is "accept as-is for v1, revisit if feedback says otherwise." An open question left unresolved in a PRD gets silently decided by whoever implements it — usually differently than anyone intended. -7. **Write the test plan as part of the PRD, not after.** Concrete test cases — naming real functions/queries, not "add tests for X" — force you to notice design gaps you'd otherwise miss; the moment you try to write "assert Y happens" and realize the current design can't produce Y is often the first time the gap becomes visible. Check the repo for an existing test pattern for this shape of change before inventing a new one (e.g. a prior presence-logic bug fixed via `test/db_test_helpers.py` fixtures is the template for the next one, not a reason to build new test infrastructure). When execution actually starts, write that test code before the implementation, run it against the pre-fix code, and confirm it fails for the right reason (a real assertion mismatch, not a collection/import error) before writing a line of the fix. A test that never failed red can't be trusted to have caught anything - it's usually the first sign the fixture doesn't actually distinguish broken from fixed behavior (a real case: a single-device DB fixture passed against both the buggy and the fixed code, because nothing in it could tell "this row matches" from "any row matches" - see `scan-pipeline` Gotcha 8). Writing the test first also tends to surface implementation code that's awkward to exercise in isolation - treat that as a refactor signal, not friction to route around. +7. **Write the test plan as part of the PRD, not after.** Concrete test cases — naming real functions/queries, not "add tests for X" — force you to notice design gaps you'd otherwise miss; the moment you try to write "assert Y happens" and realize the current design can't produce Y is often the first time the gap becomes visible. Check the repo for an existing test pattern for this shape of change before inventing a new one (e.g. a prior presence-logic bug fixed via `test/db_test_helpers.py` fixtures is the template for the next one, not a reason to build new test infrastructure). When execution actually starts, write that test code before the implementation, run it against the pre-fix code, and confirm it fails for the right reason before writing a line of the fix. What "the right reason" means depends on what's being tested: a regression test against existing behavior must fail with a real behavioral assertion mismatch, not a collection/import error - a test that never failed red that way can't be trusted to have caught anything, and is usually the first sign the fixture doesn't actually distinguish broken from fixed behavior (a real case: a single-device DB fixture passed against both the buggy and the fixed code, because nothing in it could tell "this row matches" from "any row matches" - see `scan-pipeline` Gotcha 8). A test for a brand-new contract (a function/interface that doesn't exist yet) legitimately fails with a missing-interface error instead (`AttributeError`, `ImportError`) before it's written - that's the expected red for that case, not a sign the test is wrong. Writing the test first also tends to surface implementation code that's awkward to exercise in isolation - treat that as a refactor signal, not friction to route around. 8. **Ask explicitly whether validating this needs real end-to-end infrastructure** (a new or modified plugin, a UI click-through) or whether synthetic unit-level fixtures suffice — don't assume either way. Check whether the functions under test take a DB connection/dict/list as a parameter (testable in isolation, no real plugin needed) or require a real file on disk (harder to fake, may need one). 9. **Ask explicitly whether this feature/mechanism should be exposed via the API, and give a recommendation, not just flag it as an open question.** This codebase already has three real API surfaces to consider extending rather than inventing a fourth: REST (`server/api_server/*_endpoint.py`), GraphQL (`graphql_endpoint.py`), and a Prometheus `/metrics` endpoint (`prometheus_endpoint.py`). Skipping this question doesn't mean "no API access needed"; it means the answer gets silently decided later by whoever first wants to query the new data externally, usually as its own separate feature request re-litigating a design this PRD already had full context to settle. Not everything needs exposure: purely internal/diagnostic state with no plausible external consumer doesn't, but say so explicitly, with the reason, rather than leaving it unaddressed. 10. **Check performance against the real schema and real scale, not assumptions.** For every new or changed query: does it use an existing index, or add an unindexed lookup, a new join, or a correlated subquery? Grep `CREATE INDEX` for *every* index on the tables involved, not just the first one you find — a column can have both a plain index and a separate expression index (e.g. `idx_eve_mac_date_type ON Events(eveMac, ...)` alongside `idx_eve_lower_mac_date_type ON Events(LOWER(eveMac), ...)`), and missing the second one produces a wrong verdict. Then weigh cost by how often the query runs (once is nothing; every few minutes forever is a standing cost) and by real scale — **production users run 10,000+ devices**, not a homelab handful. A `CurrentScan` with 2-5 rows per device (one per contributing plugin) is routinely 20,000-50,000+ rows in one cycle; reason about that number, not a smaller hopeful one. A correlated `EXISTS`/subquery re-evaluated per outer row is fine *if* the correlated column is indexed — e.g. `current_scan_presence_condition()` (`server/scan/presence.py`) is exactly this shape against `CurrentScan.scanMac`, covered by `idx_currentscan_scanmac`. The real risk is an unindexed correlated lookup: an accidental self-join scanning the full inner table per outer row, which looks fine and passes tests at small scale but isn't at production scale. Check with `EXPLAIN QUERY PLAN` at a realistic row count, built against the *complete* real index set (copy every `CREATE INDEX` for the table, or run it against an actual `app.db`) rather than a hand-picked subset — a partial index set produces a misleading plan in either direction, not just "looks worse than it is." If it comes back unindexed for real, a `GROUP BY` aggregate is the usual fix. diff --git a/README.md b/README.md index a93d8609b..e2bac24c3 100755 --- a/README.md +++ b/README.md @@ -40,7 +40,7 @@ Use NetAlertX to spot shadow IT, unauthorized hardware, IPAM drift, and other ch ## Quick Start > [!WARNING] -> **Important:** If upgrading an older installation read the [Migration guide](https://docs.netalertx.com/MIGRATION/?h=migrat#12-migration-from-netalertx-v25524) for detailed instructions. +> **Important:** If upgrading an older installation read the [Migration guide](https://docs.netalertx.com/MIGRATION/) for detailed instructions - it lists each migration scenario by version, so pick the one matching your installed version. Start NetAlertX in seconds with Docker: diff --git a/front/js/devices-table.js b/front/js/devices-table.js index bf58b2ad0..55253c00e 100644 --- a/front/js/devices-table.js +++ b/front/js/devices-table.js @@ -496,6 +496,11 @@ function initializeDatatable (status) { } }, // Dates + /** + * Renders the First Connection / Last Offline column cells: an empty + * cellData renders as a blank cell, otherwise as cellData localized + * into the user's configured timezone/locale. + */ {targets: [mapIndx(COL.devFirstConnection), mapIndx(COL.devLastConnection)], 'createdCell': function (td, cellData, rowData, row, col) { // devFirstConnection/devLastConnection are DB NOT NULL with no default, diff --git a/server/plugins/freebox/freebox.py b/server/plugins/freebox/freebox.py index 14a84c0a9..34f153b3f 100755 --- a/server/plugins/freebox/freebox.py +++ b/server/plugins/freebox/freebox.py @@ -84,18 +84,12 @@ def map_device_type(type: str): def select_l3_entries_for_presence(host): """ - Decide which l3connectivities entries represent this host being present - this cycle. - - Normally one entry per currently-reachable L3 address (today's existing - behavior, e.g. both IPv4 and IPv6 reachable at once). When the host is - still active but none of its L3 addresses answer as reachable right now, - falls back to a single best-effort entry instead of reporting nothing - - a transient L3 reachability drop on every address must not be read as - "device gone" when the host-level `active` flag (the Freebox's own - traffic-based presence signal, independent of L3) says otherwise. See - GitHub issue #1828. Returns an empty list only when the host itself is - not active, which still correctly represents a genuinely absent device. + Select which l3connectivities entries represent presence for a host this + cycle: every currently-reachable entry if at least one exists; otherwise, + if the host itself is active, a single best-effort entry (preferring one + Freebox still marks active even though unreachable, else the first + entry, else an empty dict if there are no L3 entries at all); otherwise + (host not active) an empty list. """ l3 = host.get("l3connectivities") if not isinstance(l3, list): @@ -112,8 +106,15 @@ def select_l3_entries_for_presence(host): mylog("verbose", [f"[{pluginName}] Host active but no reachable L3 address - using fallback IP"]) if l3: - return [l3[0]] - return [{"addr": "0.0.0.0", "last_time_reachable": None}] + # Each l3connectivities entry has its own "active" flag, independent + # of "reachable" - prefer one Freebox still considers active over an + # arbitrary stale entry; fall back to the first entry if none are. + return [next((e for e in l3 if e.get("active")), l3[0])] + + # No L3 data at all for this host this cycle - still assert presence + # (primaryId/MAC alone is enough), but don't fabricate an address or + # timestamp. main() leaves secondaryId/watched4 blank for an empty dict. + return [{}] async def get_device_data(api_version: int, api_address: str, api_port: int): @@ -197,17 +198,23 @@ def main(): if mac == '(unknown)': continue for ip in select_l3_entries_for_presence(host): - plugin_objects.add_object( - primaryId=mac, - secondaryId=ip.get("addr", "0.0.0.0"), - watched1=host.get("primary_name", "(unknown)"), - watched2=host.get("vendor_name", "(unknown)"), - watched3=map_device_type(host.get("host_type", "")), + if "last_time_reachable" in ip: # .get(..., 0) alone isn't enough: the Freebox API can return this # key present but explicitly null, and dict.get()'s default only # applies when the key is absent, not when its value is None - # `or 0` catches both, avoiding a TypeError from fromtimestamp(None). - watched4=datetime.fromtimestamp(ip.get("last_time_reachable") or 0, tz=dt_timezone.utc).strftime(DATETIME_PATTERN), + watched4 = datetime.fromtimestamp(ip.get("last_time_reachable") or 0, tz=dt_timezone.utc).strftime(DATETIME_PATTERN) + else: + # select_l3_entries_for_presence()'s no-L3-data fallback ({}) - + # leave blank rather than fabricating an epoch-zero timestamp. + watched4 = "" + plugin_objects.add_object( + primaryId=mac, + secondaryId=ip.get("addr", ""), + watched1=host.get("primary_name", "(unknown)"), + watched2=host.get("vendor_name", "(unknown)"), + watched3=map_device_type(host.get("host_type", "")), + watched4=watched4, extra="", foreignKey=mac, ) diff --git a/test/plugins/test_freebox.py b/test/plugins/test_freebox.py index a5180e02b..59ee9dff2 100644 --- a/test/plugins/test_freebox.py +++ b/test/plugins/test_freebox.py @@ -41,8 +41,11 @@ with patch("helper.get_setting_value", return_value="UTC"), \ # Shared helpers # --------------------------------------------------------------------------- -def _l3(addr="192.168.1.10", reachable=True, last_time_reachable=1700000000): - return {"addr": addr, "reachable": reachable, "last_time_reachable": last_time_reachable} +def _l3(addr="192.168.1.10", reachable=True, active=None, last_time_reachable=1700000000): + entry = {"addr": addr, "reachable": reachable, "last_time_reachable": last_time_reachable} + if active is not None: + entry["active"] = active + return entry def _host(mac="aa:bb:cc:dd:ee:01", active=True, l3connectivities=None, @@ -103,22 +106,44 @@ class TestSelectL3EntriesForPresence: result = freebox.select_l3_entries_for_presence(host) assert [e["addr"] for e in result] == ["10.0.0.1"] - def test_sentinel_ip_when_active_but_no_l3_entries_at_all(self): + def test_prefers_active_entry_among_unreachable_when_available(self): + """Each l3connectivities entry has its own 'active' flag, independent + of 'reachable' (per the Freebox API's LanHostL3Connectivity schema) - + when nothing is reachable, an entry Freebox still marks active is a + better guess than an arbitrary stale one. The active entry is placed + second on purpose, so a naive "just take the first one" fallback + would fail this test.""" + l3_entries = [_l3("10.0.0.1", reachable=False, active=False), + _l3("10.0.0.2", reachable=False, active=True)] + host = _host(active=True, l3connectivities=l3_entries) + result = freebox.select_l3_entries_for_presence(host) + assert [e["addr"] for e in result] == ["10.0.0.2"] + + def test_falls_back_to_first_entry_when_none_are_active_either(self): + l3_entries = [_l3("10.0.0.1", reachable=False), _l3("10.0.0.2", reachable=False)] + host = _host(active=True, l3connectivities=l3_entries) + result = freebox.select_l3_entries_for_presence(host) + assert [e["addr"] for e in result] == ["10.0.0.1"] + + def test_empty_entry_when_active_but_no_l3_entries_at_all(self): + """No fabricated '0.0.0.0' address or epoch-zero timestamp when there's + genuinely no L3 data - an empty dict lets main() leave secondaryId/ + watched4 blank instead of writing misleading placeholder values.""" host = _host(active=True, l3connectivities=[]) result = freebox.select_l3_entries_for_presence(host) - assert [e["addr"] for e in result] == ["0.0.0.0"] + assert result == [{}] - def test_sentinel_ip_when_l3connectivities_missing_entirely(self): + def test_empty_entry_when_l3connectivities_missing_entirely(self): host = _host(active=True) # l3connectivities key omitted entirely assert "l3connectivities" not in host result = freebox.select_l3_entries_for_presence(host) - assert [e["addr"] for e in result] == ["0.0.0.0"] + assert result == [{}] - def test_non_list_l3connectivities_treated_as_absent(self): + def test_empty_entry_when_l3connectivities_not_a_list(self): host = _host(active=True) host["l3connectivities"] = "not-a-list" result = freebox.select_l3_entries_for_presence(host) - assert [e["addr"] for e in result] == ["0.0.0.0"] + assert result == [{}] # =========================================================================== @@ -171,6 +196,26 @@ class TestMainHostLoop: assert result == 0 assert mock_po.add_object.call_count == 0 + def test_active_host_no_l3_entries_emits_blank_ip_and_timestamp(self): + """A host with active=True but no l3connectivities at all still gets + a presence row (primaryId/MAC alone is enough to assert presence), + but must not fabricate a '0.0.0.0' address or an epoch-zero + 'last seen' timestamp - both would be misleading for data we don't + actually have.""" + hosts = [_host(mac="aa:bb:cc:dd:ee:05", active=True, l3connectivities=[])] + mock_po = MagicMock() + + with self._patch_settings(), \ + patch.object(freebox, "get_device_data", AsyncMock(return_value=(None, hosts))), \ + patch.object(freebox, "plugin_objects", mock_po): + result = freebox.main() + + assert result == 0 + assert mock_po.add_object.call_count == 1 + call = mock_po.add_object.call_args_list[0] + assert call.kwargs["secondaryId"] == "" + assert call.kwargs["watched4"] == "" + def test_reachable_host_unchanged(self): """Regression guard: the common/working case (at least one reachable L3 address) must be unaffected by this fix."""