Next release

.claude/skills/database-patterns/SKILL.md

@@ -34,6 +34,16 @@ Before implementing any feature that reads or writes the `Devices` table, audit
 
 ---
 
+## Device Identity: `devMac` (today's PK) vs `devGUID` (the intended durable identity)
+
+`devMac STRING(50) PRIMARY KEY NOT NULL COLLATE NOCASE` (`server/db/schema/app.sql`) is still the literal SQL primary key. `devGUID TEXT` (indexed via `idx_dev_guid`) is a plain column today, but per the maintainer it's the intended long-term durable identity, since MAC has known limits as an identifier that `devGUID` doesn't share (privacy MAC randomization on iOS/Android/Windows, virtualized/containerized interfaces sharing one physical MAC, multi-homed devices presenting several). `devGUID` already backs device-history grouping (`server/models/device_history_instance.py`) and workflow trigger lookups (`server/workflows/triggers.py`).
+
+This is a gradual, in-progress migration, not a flag day. New code should resolve device identity from an already-fetched device row (which carries both `devMac` and `devGUID`) rather than assuming either field is *the* identifier, so it doesn't need rework as the migration progresses.
+
+**One thing that will never migrate, regardless of how far the PK change goes:** `Plugins_Objects.objectPrimaryId`, `CurrentScan.scanMac`, and `Events.eveMac` are permanently MAC-keyed. A plugin discovers a device by scanning the network, so it can only ever report a MAC address, never an app-internal `devGUID` NetAlertX hasn't assigned yet at scan time. This isn't a migration gap to eventually close; it's a structural ceiling on what network-originated data can ever identify a device by.
+
+---
+
 ## `*Source` Fields — Attribution System
 
 The `FIELD_SOURCE_MAP` in `server/db/authoritative_handler.py` defines 10 fields that carry write attribution via paired `*Source` columns:

.claude/skills/pr-analysis/SKILL.md

@@ -47,7 +47,7 @@ For each comment, determine:
 
 1. **Identify all actionable comments** before touching any file.
 2. **Load relevant skills** to understand conventions that apply.
-3. **Prepare a plan** — list each file and the exact change required.
+3. **Prepare a plan** — list each file and the exact change required. If a comment calls for new logic (a new check, helper, or condition), search for an existing equivalent first - the whole file being edited, not just the section in question, plus sibling pages/the Python backend - and extract/reuse it rather than planning a parallel implementation (see `code-standards`' DRY Principle section).
 4. **Make changes one comment at a time** — keep commits focused.
 5. **Run targeted tests** after each change (`testing-workflow` skill).
 6. **Reply** only after the commit is pushed. Include the short SHA.

.claude/skills/prd-writing/SKILL.md

@@ -9,6 +9,8 @@ description: Read before writing a PRD, design doc, or feature proposal. Covers
 
 Triggered by: "write a PRD", "draft a design doc", "spec out this feature", "create a PRD for X". Reserve this for changes where getting the design wrong is expensive to unwind — new cross-cutting mechanisms, schema changes, anything touching multiple subsystems. A one-file bug fix doesn't need this process.
 
+**A UI feature that needs to survive a page reload, coordinate across tabs, or react to a backend push/poll is a cross-cutting mechanism even when it looks like "just a small UI feature."** It's easy to start implementing straight away because the visible surface (a badge, an icon) looks trivial - but the hard part is always the state-propagation mechanism underneath, and that mechanism usually has hidden dependencies on other code that already touches the same data. A real case: a "settings still applying" indicator went through three full implementation-and-break cycles (a cookie with a guessed timeout, a new PHP endpoint duplicating existing logic, a `localStorage`/polling redesign with its own bugs) before anyone traced every existing consumer of `app_state.json` and found that `front/js/sse_manager.js`'s `handleStateUpdate()` already pushed the exact signal needed to every open page. Step 4 ("trace every downstream consumer") below would have caught this on attempt #1 if it had been applied before writing any code, not after three failures.
+
 ## Core principle: a PRD is a claim-verification exercise, not a writing exercise
 
 Every sentence that asserts something about how the code currently works must be checked against the actual code before it goes in — not written from memory, not inferred from a plugin's name or reputation, not assumed because it sounds plausible. Two failure patterns to watch for:
@@ -26,7 +28,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).
+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.

.claude/skills/scan-pipeline/SKILL.md

@@ -58,6 +58,8 @@ This is the scan-pipeline-local half of a bigger attribution system — see `dat
 7. **`LatestEventsPerMAC` is intentionally `CurrentScan`-gated - correct for its one existing caller, a trap for a new one.** It `INNER JOIN`s `CurrentScan`, which is exactly right for the mainline "New Connections" query (every MAC it looks up is already known to be in `CurrentScan` this cycle, via its own `present_agg`). It is not a general-purpose "last event for any MAC" lookup. A caller that needs the last event for a MAC *not* in `CurrentScan` this cycle (e.g. a NIC-covered parent with no direct scan row of its own) gets no row at all from this view, regardless of that MAC's real event history - silently, not an error. `insert_events()`'s NIC-derived reconnect query reads `Events` directly instead (a correlated `ORDER BY eveDateTime DESC LIMIT 1`, covered by `idx_eve_mac_datetime_desc`), sidestepping the view entirely rather than trying to make it handle both shapes.
 8. **A correlated helper's `mac_column` argument silently binds to the helper's own inner row, not the caller's, whenever the helper's inner table already has a column of that name in scope - alias or no alias.** SQL resolves an unqualified name in the innermost enclosing scope first and only searches outward if nothing matches there; `nic_derived_presence_condition()`'s inner scan is `FROM Devices`, and `Devices` has a `devMac` column, so *any* bare `"devMac"` argument - not just one that happens to collide with an alias name - bound to the helper's own inner row. Every bare-`"devMac"` call site (both `Device Down` queries, `Disconnected`, `update_devLastConnection_from_CurrentScan()`) was affected: any NIC-covered device anywhere in `Devices` made every *other* absent device, including one with no NIC children at all, look NIC-derived-present, silently suppressing its real event or bumping its `devLastConnection`. (A qualified-but-colliding argument, e.g. `"nic_parent.devMac"` passed from a caller aliasing its own row `nic_parent` while the helper's own inner alias was also `nic_parent`, is the same root cause in a narrower form.) Fixed by requiring every caller to pass a qualified reference to *its own* table/alias (`"Devices.devMac"`, `"DevicesView.devMac"`) and having `nic_derived_presence_condition()` reject a bare `mac_column` outright, on top of the existing `presence_scan`/`nic_presence_parent`-collision guards. `current_scan_presence_condition()` doesn't need this: its inner scan is `FROM CurrentScan`, which has no `devMac` column, so a bare `"devMac"` has nothing to bind to inward and correctly falls back to the caller's row. Only caught by a test with two sibling devices in one DB where one should match and the other shouldn't - every single-device test passed regardless, because "any row" and "this row" are the same row when there's only one.
 
+7. **`CurrentScan.scanMac`/`Events.eveMac` are permanently MAC-keyed, independent of the devGUID-as-PK migration.** See `database-patterns`' "Device Identity" section - `devMac` is today's actual schema PK, `devGUID` is the intended long-term identity, but a plugin can only ever report a MAC from network discovery, never an app-internal `devGUID`. Don't design around these tables ever becoming devGUID-keyed.
+
 ## When to read this vs. other docs/skills
 
 - Writing or reviewing a plugin's `config.json`/data contract → `plugin-development`, `docs/PLUGINS_DEV*.md`. This skill covers what happens *after* a plugin's rows land in `CurrentScan`, not the authoring contract.

.gemini/skills/database-patterns/SKILL.md

@@ -34,6 +34,16 @@ Before implementing any feature that reads or writes the `Devices` table, audit
 
 ---
 
+## Device Identity: `devMac` (today's PK) vs `devGUID` (the intended durable identity)
+
+`devMac STRING(50) PRIMARY KEY NOT NULL COLLATE NOCASE` (`server/db/schema/app.sql`) is still the literal SQL primary key. `devGUID TEXT` (indexed via `idx_dev_guid`) is a plain column today, but per the maintainer it's the intended long-term durable identity, since MAC has known limits as an identifier that `devGUID` doesn't share (privacy MAC randomization on iOS/Android/Windows, virtualized/containerized interfaces sharing one physical MAC, multi-homed devices presenting several). `devGUID` already backs device-history grouping (`server/models/device_history_instance.py`) and workflow trigger lookups (`server/workflows/triggers.py`).
+
+This is a gradual, in-progress migration, not a flag day. New code should resolve device identity from an already-fetched device row (which carries both `devMac` and `devGUID`) rather than assuming either field is *the* identifier, so it doesn't need rework as the migration progresses.
+
+**One thing that will never migrate, regardless of how far the PK change goes:** `Plugins_Objects.objectPrimaryId`, `CurrentScan.scanMac`, and `Events.eveMac` are permanently MAC-keyed. A plugin discovers a device by scanning the network, so it can only ever report a MAC address, never an app-internal `devGUID` NetAlertX hasn't assigned yet at scan time. This isn't a migration gap to eventually close; it's a structural ceiling on what network-originated data can ever identify a device by.
+
+---
+
 ## `*Source` Fields — Attribution System
 
 The `FIELD_SOURCE_MAP` in `server/db/authoritative_handler.py` defines 10 fields that carry write attribution via paired `*Source` columns:

.gemini/skills/pr-analysis/SKILL.md

@@ -47,9 +47,9 @@ For each comment, determine:
 
 1. **Identify all actionable comments** before touching any file.
 2. **Load relevant skills** to understand conventions that apply.
-3. **Prepare a plan** — list each file and the exact change required.
-4. **Make changes one comment at a time** — keep commits focused.
-5. **Run targeted tests** after each change (`testing-workflow` skill).
+3. **Prepare a plan** — list each file and the exact change required. If a comment calls for new logic (a new check, helper, or condition), search for an existing equivalent first - the whole file being edited, not just the section in question, plus sibling pages/the Python backend - and extract/reuse it rather than planning a parallel implementation (see `code-standards`' DRY Principle section).
+4. **Make changes one comment at a time** — keep commits focused. For a comment claiming a bug: write the test that should catch it first, run it against the current (unfixed) code, and confirm it fails for the right reason before writing the fix - see `prd-writing`'s test-first step for why (a test that never failed red can't be trusted to have caught anything).
+5. **Rerun tests after each change** (`testing-workflow` skill) - confirm the new/updated test now passes, not just that nothing else broke.
 6. **Reply** only after the commit is pushed. Include the short SHA.
 
 ## Reply Guidelines