Next release

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

@@ -0,0 +1,142 @@
+---
+name: database-patterns
+description: Read before designing a feature that writes to the Devices table, adding audit/history logging, or choosing between a SQLite trigger and a Python hook. Covers the full Devices write-path inventory, the FIELD_SOURCE_MAP *Source attribution system, and event-sourced vs snapshot audit logging tradeoffs.
+---
+
+# Database Patterns
+
+## Devices Table — Write-Path Inventory
+
+Before implementing any feature that reads or writes the `Devices` table, audit ALL write paths. The table is modified from many locations — missing one path is a correctness bug.
+
+**Known production write paths (as of 2026-07-04):**
+
+| File | Function | Fields written |
+|---|---|---|
+| `server/models/device_instance.py` | `setDeviceData()` | All user-editable fields |
+| `server/models/device_instance.py` | `updateField()` | Any single field (workflows) |
+| `server/models/device_instance.py` | `updateDeviceColumn()` | Any single column |
+| `server/models/device_instance.py` | `deleteDevices()` etc. | DELETE operations |
+| `server/scan/device_handling.py` | `update_devices_data_from_scan()` | Scan-derived fields |
+| `server/scan/device_handling.py` | `update_vendors_from_mac()` | `devVendor`, `devVendorSource` |
+| `server/scan/device_handling.py` | Name resolution block | `devName`, `devFQDN`, `*Source` |
+| `server/scan/device_handling.py` | `update_ipv4_ipv6()` | `devPrimaryIPv4`, `devPrimaryIPv6` |
+| `server/scan/device_handling.py` | `update_icons_and_types()` | `devIcon`, `devType` |
+| `server/scan/device_handling.py` | `update_presence_from_CurrentScan()` | `devPresentLastScan` |
+| `server/scan/device_handling.py` | `update_devLastConnection_from_CurrentScan()` | `devLastConnection` |
+| `server/scan/device_handling.py` | `update_devPresentLastScan_based_on_*()` | `devPresentLastScan` |
+| `server/db/authoritative_handler.py` | `enforce_source_on_user_update()` | `*Source` columns |
+| `server/db/authoritative_handler.py` | `lock_field()` / `unlock_field()` | `*Source` columns |
+| `server/models/notification_instance.py` | `clearPendingEmailFlag()` | `devLastNotification` |
+| `server/plugins/db_cleanup/script.py` | `cleanup_database()` | DELETE operations |
+
+**Key insight:** Most scan functions use `sql.executemany()` — there is no per-row Python state available. Python hooks before/after executemany require a pre-fetch+diff pattern that is expensive and error-prone.
+
+---
+
+## `*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:
+
+```python
+FIELD_SOURCE_MAP = {
+    "devMac":           "devMacSource",
+    "devName":          "devNameSource",
+    "devFQDN":          "devFQDNSource",
+    "devLastIP":        "devLastIPSource",
+    "devVendor":        "devVendorSource",
+    "devSSID":          "devSSIDSource",
+    "devParentMAC":     "devParentMACSource",
+    "devParentPort":    "devParentPortSource",
+    "devParentRelType": "devParentRelTypeSource",
+    "devVlan":          "devVlanSource",
+}
+```
+
+`*Source` values: `'USER'`, `'LOCKED'`, `'NEWDEV'`, or a plugin prefix (e.g., `'ARPSCAN'`, `'NSLOOKUP'`).
+
+These fields are updated **in the same transaction** as the primary field. A SQLite `AFTER UPDATE` trigger can read `NEW.devNameSource` to obtain correct attribution without any extra context-passing.
+
+**Attribution rules for features that need `changedBy`:**
+
+| Field category | Attribution |
+|---|---|
+| In `FIELD_SOURCE_MAP` | `COALESCE(NULLIF(NEW.<field>Source, ''), 'system')` |
+| User-only fields (`devGroup`, `devComments`, `devFavorite`, `devOwner`, `devLocation`, etc.) | `'user:api'` — only `setDeviceData()` writes these |
+| Auto-computed fields (`devIcon`, `devType`, `devPrimaryIPv4`, `devPrimaryIPv6`) | `'system'` |
+| `*Source` fields themselves | `'system'` |
+
+---
+
+## Cross-Cutting Concerns — Prefer SQLite Triggers Over Python Hooks
+
+When a feature needs to intercept **every write** to the `Devices` table (audit logging, computed columns, cascading logic), prefer a **SQLite `AFTER UPDATE` / `AFTER INSERT` trigger** over Python-layer hooks.
+
+**Why:**
+- Triggers catch all 14+ write paths automatically, including `executemany()` bulk updates
+- Zero modifications to existing write-path functions (DRY)
+- Self-healing: future write paths are automatically covered
+- Attribution is available via `NEW.*Source` fields (see above)
+
+**When Python hooks are still appropriate:**
+- The logic needs access to Python objects, settings, or services not available in SQL
+- The feature only fires from one or two known write paths
+- The logic is too complex to express in SQL (multi-table joins with app-layer business logic)
+
+### Trigger Performance Pattern
+
+```sql
+CREATE TRIGGER trg_example
+AFTER UPDATE ON Devices
+FOR EACH ROW
+-- Guard: short-circuit entire body when feature is disabled (zero cost)
+WHEN (SELECT CAST(setValue AS INTEGER) FROM Settings WHERE setKey = 'FEATURE_ENABLED') > 0
+BEGIN
+  -- Per-field conditional insert
+  INSERT INTO SomeTable (devGUID, column, oldVal, newVal, changedBy, ts)
+  SELECT NEW.devGUID, 'devName', OLD.devName, NEW.devName,
+         COALESCE(NULLIF(NEW.devNameSource, ''), 'system'),
+         datetime('now', 'utc')
+  WHERE OLD.devName IS NOT NEW.devName
+    AND instr(',' || (SELECT setValue FROM Settings WHERE setKey = 'TRACKED_FIELDS') || ',', ',devName,') > 0;
+  -- Repeat for each tracked field...
+END;
+```
+
+**Performance:** The Settings table is tiny (~100 rows) and stays in SQLite's page cache. Per-row Settings reads inside triggers are effectively in-memory lookups. The `WHEN` guard makes the disabled state zero-cost.
+
+---
+
+## Snapshot vs Event-Sourced Audit Logging
+
+When implementing change history, always use **event-sourced (per-field rows)** not **snapshots (full row copies)**.
+
+| | Event-sourced | Snapshot |
+|---|---|---|
+| Storage | Small — only changed fields | Large — all 40+ columns every mutation |
+| Filter by field | O(log n) via index | O(n) — must diff every adjacent pair |
+| Filter by source | O(log n) via index | Not possible without diffing |
+| `changedBy` attribution | Embedded at write time | Not available without extra context |
+| Retention calculation | Simple timestamp DELETE | Same, but much higher storage |
+
+At 1000 devices, 5-min scan interval, 14-day retention: snapshot storage ≈ 280 MB/day. Event-sourced storage for the same workload is typically <1 MB/day (most scans produce no tracked field changes).
+
+---
+
+## `DevicesHistory` Table — Reference Schema
+
+```sql
+CREATE TABLE IF NOT EXISTS DevicesHistory (
+    id           INTEGER PRIMARY KEY AUTOINCREMENT,
+    devGUID      TEXT NOT NULL,
+    timestamp    DATETIME DEFAULT CURRENT_TIMESTAMP,
+    changedBy    TEXT NOT NULL,
+    changedColumn TEXT NOT NULL,
+    oldValue     TEXT,
+    newValue     TEXT,
+    FOREIGN KEY (devGUID) REFERENCES Devices(devGUID) ON DELETE CASCADE
+);
+
+CREATE INDEX IF NOT EXISTS idx_devhist_guid_column ON DevicesHistory(devGUID, changedColumn);
+CREATE INDEX IF NOT EXISTS idx_devhist_timestamp   ON DevicesHistory(timestamp);
+```

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

@@ -0,0 +1,59 @@
+---
+name: prd-writing
+description: Read before writing a PRD, design doc, or feature proposal. Covers challenging the idea before designing it, verifying every claim against actual code (not memory or a plugin's name/category), tracing every downstream consumer of a new mechanism, evaluating performance impact against the schema/indexes that actually exist, recording rejected alternatives and open-issue decisions explicitly, and a dedicated final-check pass before calling it done.
+---
+
+# PRD Writing
+
+## When to use
+
+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.
+
+## 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 real, caught-in-review examples from this exact process:
+
+- A draft claimed "plugin X's rows are the cleanest case for a static presence-flag value" — reasonable-sounding, and wrong. Reading the actual script showed it already reports a live per-row state field and computes an equivalent boolean internally that was simply never wired up. The claim was never checked against the script, only against the plugin's category ("reservation-style").
+- A draft claimed "no changes needed here" for two queries when adding a new presence column, on the reasoning that they were already "inert" for the new case. True for one sub-case (a device that starts absent), false for the transition sub-case (a device going from online to newly-suppressed) — because those two queries used a different existence check than the one already patched. It was missed for a full turn, until a direct question ("does this handle the transition where a device *was* online?") forced a re-trace of the actual call graph.
+
+Both mistakes were plausible, well-written, and wrong. Neither would have survived actually reading the code first.
+
+## Process
+
+1. **Understand the current mechanism by reading the actual code before writing anything.** Cite `file:line` for every claim about current behavior. Delegate to an Explore/general-purpose agent for breadth if the surface area is large, but treat its findings as a starting point to spot-check, not a finished citation — verify anything load-bearing yourself before it goes in the PRD.
+2. **Challenge the idea before designing it.** If the user proposes a solution, ask: is this solving the right problem? Does it conflate unrelated concerns (see axis-separation, next)? Does a similar or previously-rejected mechanism already exist that this would collide with semantically? A naming near-collision with an existing field/concept that has different, incompatible semantics is a signal to stop and check precedence rules, not a coincidence to wave off.
+3. **Identify the independent axes.** A feature request that arrives as "option A and option B" is often two or three orthogonal concerns bundled together — e.g. "should this exist at all," "should it notify," and "should it assert presence" are three separate questions, not one. Cramming them into a single enum/flag produces combinations you can't express later (what if a plugin wants A+C but not B?). Give each axis its own mechanism.
+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 actually 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).
+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. **Evaluate performance impact against the schema that actually exists, not the schema you'd expect, and against real deployment scale, not an imagined one.** For every new or modified query: does it reuse an existing index, or does it add an unindexed lookup, a new join, or a correlated subquery? Check for real — don't assume a column is indexed just because it looks identity-like (`CurrentScan.scanMac` looked like exactly the kind of column that should have an index; grepping for `CREATE INDEX` showed this codebase never gave it one, and neither did the first draft of the design that needed it — since fixed, `idx_currentscan_scanmac` now exists in `server/db/db_upgrade.py:ensure_CurrentScan()`, so check whether a later PRD's problem is already mitigated before assuming it's new). Then multiply the per-query cost by two things: how often it runs (a full scan inside a loop that fires once is nothing; the same scan inside a cycle that reruns every few minutes forever is a standing cost, permanently), and the actual scale this project runs at — **known real production users run 10,000+ devices** (confirmed directly by the project owner, not a guess or an inference from `CLAUDE.md`'s "homelabs, MSPs, and NOCs" framing). At that scale, a `CurrentScan` populated at 2-5 rows per device (one per contributing plugin, the normal case) is routinely 20,000-50,000+ rows in a single cycle — treat that as the number to reason about, not a hypothetical upper bound reserved for some future large deployment. Concrete example from this process: implementing a new multi-source precedence rule as a correlated `EXISTS` subquery re-evaluated per candidate row reads as perfectly reasonable, passes every test at small scale, and is an accidental self-join with no index behind it at scale — the fix (add the missing index, express the aggregation as one `GROUP BY` pass instead of a per-row correlated check) had to be written into the PRD explicitly, or it would have shipped as a footgun that real 10k-device users would have hit, not a theoretical one.
+10. **Do a dedicated final-check pass, out loud, before calling it done.** Re-read the whole document end to end and specifically check:
+    - Did a correction made mid-document actually propagate everywhere it needed to (the Design subsection *and* Affected Files *and* Tests *and* any execution-plan summary)? A correction landing in one place and not its siblings is worse than never catching it, because now the document silently contradicts itself.
+    - Does every "this is the cleanest/simplest real case" claim still hold up if you actually re-read that specific piece of code right now, or was it asserted by pattern-matching a name/category? Re-verify, don't re-assert.
+    - Does anything render incorrectly as markdown — an unfenced ASCII diagram or code block will collapse into one line under lazy-paragraph-continuation, the same class of bug as a list missing its preceding blank line.
+    - Do any internal anchor links' slugs actually match their headings?
+    - Does the design still cleanly separate its axes, or did a later addition quietly re-conflate two concerns inside what's supposed to be a single-purpose mechanism (the same mistake step 3 exists to catch at the top level can reappear one level down inside an individual mechanism's own value set — e.g. a 3-value enum where two of the values are secretly independent booleans in a trenchcoat).
+11. **Leave a visible trail of corrections instead of silently rewriting.** When a review pass — yours or someone else's — finds something wrong, write "**Correction (caught in review):** ..." inline rather than quietly fixing the earlier text and moving on. This is what makes a PRD trustworthy to a second reader: they can see what was checked and what changed, not just receive a polished final answer with no visible seams.
+
+## Structure to follow
+
+- **Problem** — grounded in specific, cited current behavior, not a general complaint.
+- **Goals / Non-goals** — non-goals should name specific things that sound in-scope but aren't, each with a one-line reason.
+- **Design** — one subsection per independent axis/mechanism (step 3). Include a `### Rejected: X` subsection for any alternative seriously considered (step 5).
+- **Open issues** — each with an explicit recorded decision (step 6), not left dangling.
+- **Affected files** — concrete `file:function:line` references, not bare filenames.
+- **Backward compatibility** — explicit default values and why they preserve current behavior for existing consumers.
+- **Performance impact** — the baseline (what's unindexed/slow *today*, independent of this change), what the change adds that's negligible, what's genuinely new and worth mitigating, and concrete mitigations rather than a vague "should be fine" (step 9).
+- **Docs/skills to update** — anywhere this needs to be reflected outside the code itself (external docs, paired skill files, template files new authors copy from).
+- **Tests** — organized by mechanism, each case naming the real function/query it exercises and the concrete assertion (step 7), plus a manual verification checklist for anything that can't be unit-tested (including an `EXPLAIN QUERY PLAN` check at realistic scale if the Performance impact section found a genuine risk).
+- **(Optional) Execution plan** — phased, referencing the same file/function names used above rather than restating the design in vaguer terms.
+
+## Before starting: check for an existing architecture-reference skill
+
+If a skill already documents the subsystem the feature touches, load it before researching from scratch — don't re-derive call graphs or mechanism details that are already written down. If the feature touches a subsystem with no such skill, and understanding it required significant re-derivation from raw code, that's a signal to write one afterward so the next PRD in that area doesn't start from zero.
+
+## Where to save
+
+`.gemini/internal-docs/PRDs/<kebab-case-name>.md`, unless the user specifies otherwise. Mark the status line (`**Status:** Draft — pending review`) so it's clear this hasn't been approved yet, and keep the author line accurate about who actually made the calls (a design discussion with an assistant is not sole assistant authorship).