diff --git a/.claude/skills/database-patterns/SKILL.md b/.claude/skills/database-patterns/SKILL.md new file mode 100644 index 000000000..c04c8633f --- /dev/null +++ b/.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.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); +``` diff --git a/.claude/skills/scan-pipeline/SKILL.md b/.claude/skills/scan-pipeline/SKILL.md new file mode 100644 index 000000000..b53ac7c7a --- /dev/null +++ b/.claude/skills/scan-pipeline/SKILL.md @@ -0,0 +1,59 @@ +--- +name: scan-pipeline +description: Read before modifying the scan pipeline (server/scan/session_events.py, server/scan/device_handling.py). Covers process_scan()'s call order and why it's load-bearing, the CurrentScan/Events/Sessions/DevicesView relationships, how a session actually closes (there is no close function), and the FIELD_SPECS field-write authority mechanism. Use this when touching device presence, connect/disconnect events, or session/timeline behavior. +--- + +# Scan Pipeline & Device Presence Lifecycle + +## Scope + +This skill covers what happens *after* a plugin's rows land in `CurrentScan` — presence computation, event generation, and session/timeline derivation. It does not cover the plugin-authoring side (manifest, data contract, settings) — see the `plugin-development` skill and `docs/PLUGINS_DEV*.md` for that. It also does not cover the general `*Source` attribution system or SQLite triggers for audit logging — see the `database-patterns` skill (Copilot tree only, as of this writing) for that; the field-authority section below is the scan-pipeline-local half of that bigger system, and the two overlap. + +## The core tables/views and their lifetimes + +- **`CurrentScan`** — ephemeral scratch table. Populated by `process_plugin_events()` for any plugin whose `config.json` declares `mapped_to_table`, then fully deleted at the end of every `process_scan()` cycle (`DELETE FROM CurrentScan`). Never assume a value written into a `CurrentScan` row is readable outside the cycle it arrived in — by the time a later cycle needs to check "was this row flagged X," the row is gone. This bit a real design (see Gotcha 2 below). +- **`Devices`** — persistent identity + state table. +- **`Events`** — persistent, append-only log of state-transition events (`New Device`, `Connected`, `Down Reconnected`, `Device Down`, `Disconnected`, `IP Changed`). This *is* the audit trail; `Sessions` is derived from it, not the other way around. +- **`Sessions`** — not incrementally updated; fully wiped and rebuilt every cycle from a view (see `Convert_Events_to_Sessions` below). Don't reason about it as a live connection state machine — it's a materialized query result recomputed from `Events` each cycle. +- **`Online_History`** — persistent, one row per scan cycle, feeds the dashboard's online/offline graph. Purely a rollup of `devPresentLastScan`/`devAlertDown`/`devIsSleeping` counts on `DevicesView` — no independent state of its own. + +## Key views + +- **`LatestDeviceScan`** (`server/db/db_upgrade.py`) — `Devices` LEFT JOIN'd to the most recent `CurrentScan` row **per `(scanMac, scanSourcePlugin)` pair**, ranked via `ROW_NUMBER() OVER (PARTITION BY scanMac, scanSourcePlugin ...)`. This is why `update_devices_data_from_scan()` loops over `DISTINCT scanSourcePlugin` and re-queries this view once per plugin: when two plugins report the same device in the same cycle, they are *not* merged into one row before processing — each plugin's contribution is evaluated independently, per field, through the authority mechanism below. +- **`LatestEventsPerMAC`** — most recent Event per MAC, joined to `Devices` and `CurrentScan`. Used by the "New Connections" query in `insert_events()` to decide whether a device was previously down (→ `Down Reconnected`) or genuinely new (→ `Connected`). +- **`Convert_Events_to_Sessions`** — the actual definition of "is this device's session still open." **There is no `close_session()`-style function anywhere in this codebase.** A session closes purely as an emergent property: `pair_sessions_events()` sets `evePairEventRowid` on a `New Device`/`Connected`/`Down Reconnected` Event to point at the next `Disconnected`/`Device Down` Event for that MAC, and this view computes `sesStillConnected = 1` exactly when that pairing is still `NULL`. If a session needs to close, the fix is always "make sure the right `Events` row gets inserted" — never a direct `Sessions` mutation (the one exception is `create_new_devices()`'s reconnect-insert, noted below). +- **`DevicesView`** — adds computed `devIsSleeping`/`devFlapping`/`devStatus` on top of `Devices`. This is what the UI and `insertOnlineHistory()` actually read presence from, not the raw `Devices` table. + +## `process_scan()` call order (`server/scan/session_events.py`) — the order is load-bearing, not incidental + +1. `save_own_device()`, `exclude_ignored_devices()` +2. `insert_events(db)` — **runs before presence gets updated for this cycle.** Deliberate: the Down/Disconnected/Connected queries need the *previous* cycle's `devPresentLastScan` value to detect a transition (present last cycle but absent now, or vice versa). If this ran after the presence update, every query would see the already-updated value and the edge-triggered design would collapse into either never firing or firing every cycle. +3. `create_new_devices(db)` — the source comment is explicit: "after create events -> avoid 'connection' event." Brand-new devices get a `New Device` event instead of a `Connected` event, because at step 2 they didn't exist as `Devices` rows for either query to match. Also contains a raw `INSERT INTO Sessions ... sesStillConnected = 1` for devices that already exist but have no currently-open session — the one place outside the `Events`-derived path that writes `Sessions` directly. +4. `update_devices_data_from_scan(db)` — field-level updates for existing devices; see the authority mechanism below. +5. `update_sync_hub_node`, `update_devLastConnection_from_CurrentScan` +6. `update_presence_from_CurrentScan(db)` — sets `devPresentLastScan` from bare `CurrentScan` presence *for this cycle* (this becomes the "previous" value step 2 reads on the *next* cycle). +7. `update_devPresentLastScan_based_on_nics(db)` — NIC/parent-child presence aggregation; can override step 6's result for parent devices. +8. `update_devPresentLastScan_based_on_force_status(db)` — the user's manual `devForceStatus` override, runs **last**, wins unconditionally over everything above. +9. `update_vendors_from_mac`, `update_ipv4_ipv6`, `update_icons_and_types` +10. `pair_sessions_events(db)` — pairs `Events` rows as described above. +11. `create_sessions_snapshot(db)` — `DELETE FROM Sessions; INSERT INTO Sessions SELECT * FROM Convert_Events_to_Sessions`. This is the point where `Sessions` actually reflects step 10's pairing. +12. `insertOnlineHistory(db)` — dashboard graph rollup. +13. `skip_repeated_notifications(db)` +14. `DELETE FROM CurrentScan` — the ephemeral table's entire lifetime is one call to `process_scan()`. + +## Field-write authority for scan-derived updates + +`update_devices_data_from_scan()` (`server/scan/device_handling.py`) does not blindly overwrite fields from whichever plugin ran most recently. Each trackable field is declared once in `FIELD_SPECS` (`scan_col`, `source_col`, a `priority` list of plugin prefixes, optional `allow_override_if_changed`), and `can_overwrite_field()` uses that plus `get_plugin_authoritative_settings()` (which reads a plugin's own settings for an explicit authority override) to decide, per field per row, whether this plugin's value may replace what's there. The paired `Source` column (`devNameSource`, `devLastIPSource`, etc.) records who currently owns the field. `devMac` itself is never a target of these updates — it's the join key, not a tracked field — so no scan-derived update path can ever alter a device's identity, only its attributes. + +This is the scan-pipeline-local half of a bigger attribution system — see the `database-patterns` skill for `FIELD_SOURCE_MAP` / `server/db/authoritative_handler.py`, the full `*Source` attribution model, and how SQLite triggers consume it for audit logging. Read both if touching anything that writes a `*Source` column. + +## Two real gotchas (not hypothetical — both surfaced live during a design review) + +1. **A "presence" check almost always exists in more than one place.** When adding a per-row signal meaning "don't count this as a live sighting" (e.g. a proposed `scanPresence` column), every query that independently re-derives "is this MAC currently present" from `CurrentScan` has to be updated together — `update_presence_from_CurrentScan()`, the `insert_events()` "New Connections"/"Device Down"/"Disconnected" queries, and the raw `INSERT INTO Sessions` inside `create_new_devices()` all encode that same question separately. Patching one and missing a sibling produces a UI where the device badge, the Events log, and the Sessions timeline each tell a different story for the same device. See `.gemini/internal-docs/PRDs/plugin-import-behavior-controls.md` for the worked example — a `scanPresence = 0` transition that never closed its session because only one of three "is it present" queries had been patched. +2. **`CurrentScan` is deleted at the end of every cycle — a per-row flag on it cannot express a decision that needs to survive to a cycle where the row is absent.** Anything that fires specifically *because* a row is missing (`Device Down`, `Disconnected`) cannot read a flag that lived on that now-gone row. If a per-row plugin signal needs to affect behavior beyond the cycle it arrived in, persist it onto the `Devices` row at creation time (e.g. seeding `devAlertDown`/`devAlertEvents` from the row's flag instead of the global `NEWDEV_*` defaults) rather than trying to make the ephemeral table carry it forward. + +## When to read this vs. other docs/skills + +- Writing or reviewing a plugin's `config.json`/data contract → `plugin-development` skill, `docs/PLUGINS_DEV*.md`. This skill is about what happens *after* a plugin's rows land in `CurrentScan`, not the plugin-authoring contract itself. +- Devices-table write paths, `*Source` attribution, audit/history logging, SQLite triggers → `database-patterns` skill. +- Actually implementing a change here → read the relevant function in `server/scan/session_events.py` / `server/scan/device_handling.py` directly before trusting this skill's line-number references; they're a map, not a guarantee, and will drift as the code moves. diff --git a/.gemini/skills/database-patterns/SKILL.md b/.gemini/skills/database-patterns/SKILL.md new file mode 100644 index 000000000..d7633f6f4 --- /dev/null +++ b/.gemini/skills/database-patterns/SKILL.md @@ -0,0 +1,142 @@ +--- +name: database-patterns +description: NetAlertX database architecture patterns. Use this when designing features that write to the Devices table, implementing audit/history logging, or choosing between trigger-based vs Python-hook approaches. +--- + +# 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.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); +``` diff --git a/.gemini/skills/scan-pipeline/SKILL.md b/.gemini/skills/scan-pipeline/SKILL.md new file mode 100644 index 000000000..95189f094 --- /dev/null +++ b/.gemini/skills/scan-pipeline/SKILL.md @@ -0,0 +1,59 @@ +--- +name: scan-pipeline +description: Reference for how the scan pipeline actually works — process_scan()'s call order, the CurrentScan/Events/Sessions/DevicesView relationships, how a session actually "closes" (there is no close function), and the FIELD_SPECS field-write authority mechanism. Load this before modifying server/scan/session_events.py or server/scan/device_handling.py, or when reasoning about device presence, connect/disconnect events, or session/timeline behavior. +--- + +# Scan Pipeline & Device Presence Lifecycle + +## Scope + +This skill covers what happens *after* a plugin's rows land in `CurrentScan` — presence computation, event generation, and session/timeline derivation. It does not cover the plugin-authoring side (manifest, data contract, settings) — see the `plugin-development` skill and `docs/PLUGINS_DEV*.md` for that. It also does not cover the general `*Source` attribution system or SQLite triggers for audit logging — see the `database-patterns` skill (Copilot tree only, as of this writing) for that; the field-authority section below is the scan-pipeline-local half of that bigger system, and the two overlap. + +## The core tables/views and their lifetimes + +- **`CurrentScan`** — ephemeral scratch table. Populated by `process_plugin_events()` for any plugin whose `config.json` declares `mapped_to_table`, then fully deleted at the end of every `process_scan()` cycle (`DELETE FROM CurrentScan`). Never assume a value written into a `CurrentScan` row is readable outside the cycle it arrived in — by the time a later cycle needs to check "was this row flagged X," the row is gone. This bit a real design (see Gotcha 2 below). +- **`Devices`** — persistent identity + state table. +- **`Events`** — persistent, append-only log of state-transition events (`New Device`, `Connected`, `Down Reconnected`, `Device Down`, `Disconnected`, `IP Changed`). This *is* the audit trail; `Sessions` is derived from it, not the other way around. +- **`Sessions`** — not incrementally updated; fully wiped and rebuilt every cycle from a view (see `Convert_Events_to_Sessions` below). Don't reason about it as a live connection state machine — it's a materialized query result recomputed from `Events` each cycle. +- **`Online_History`** — persistent, one row per scan cycle, feeds the dashboard's online/offline graph. Purely a rollup of `devPresentLastScan`/`devAlertDown`/`devIsSleeping` counts on `DevicesView` — no independent state of its own. + +## Key views + +- **`LatestDeviceScan`** (`server/db/db_upgrade.py`) — `Devices` LEFT JOIN'd to the most recent `CurrentScan` row **per `(scanMac, scanSourcePlugin)` pair**, ranked via `ROW_NUMBER() OVER (PARTITION BY scanMac, scanSourcePlugin ...)`. This is why `update_devices_data_from_scan()` loops over `DISTINCT scanSourcePlugin` and re-queries this view once per plugin: when two plugins report the same device in the same cycle, they are *not* merged into one row before processing — each plugin's contribution is evaluated independently, per field, through the authority mechanism below. +- **`LatestEventsPerMAC`** — most recent Event per MAC, joined to `Devices` and `CurrentScan`. Used by the "New Connections" query in `insert_events()` to decide whether a device was previously down (→ `Down Reconnected`) or genuinely new (→ `Connected`). +- **`Convert_Events_to_Sessions`** — the actual definition of "is this device's session still open." **There is no `close_session()`-style function anywhere in this codebase.** A session closes purely as an emergent property: `pair_sessions_events()` sets `evePairEventRowid` on a `New Device`/`Connected`/`Down Reconnected` Event to point at the next `Disconnected`/`Device Down` Event for that MAC, and this view computes `sesStillConnected = 1` exactly when that pairing is still `NULL`. If a session needs to close, the fix is always "make sure the right `Events` row gets inserted" — never a direct `Sessions` mutation (the one exception is `create_new_devices()`'s reconnect-insert, noted below). +- **`DevicesView`** — adds computed `devIsSleeping`/`devFlapping`/`devStatus` on top of `Devices`. This is what the UI and `insertOnlineHistory()` actually read presence from, not the raw `Devices` table. + +## `process_scan()` call order (`server/scan/session_events.py`) — the order is load-bearing, not incidental + +1. `save_own_device()`, `exclude_ignored_devices()` +2. `insert_events(db)` — **runs before presence gets updated for this cycle.** Deliberate: the Down/Disconnected/Connected queries need the *previous* cycle's `devPresentLastScan` value to detect a transition (present last cycle but absent now, or vice versa). If this ran after the presence update, every query would see the already-updated value and the edge-triggered design would collapse into either never firing or firing every cycle. +3. `create_new_devices(db)` — the source comment is explicit: "after create events -> avoid 'connection' event." Brand-new devices get a `New Device` event instead of a `Connected` event, because at step 2 they didn't exist as `Devices` rows for either query to match. Also contains a raw `INSERT INTO Sessions ... sesStillConnected = 1` for devices that already exist but have no currently-open session — the one place outside the `Events`-derived path that writes `Sessions` directly. +4. `update_devices_data_from_scan(db)` — field-level updates for existing devices; see the authority mechanism below. +5. `update_sync_hub_node`, `update_devLastConnection_from_CurrentScan` +6. `update_presence_from_CurrentScan(db)` — sets `devPresentLastScan` from bare `CurrentScan` presence *for this cycle* (this becomes the "previous" value step 2 reads on the *next* cycle). +7. `update_devPresentLastScan_based_on_nics(db)` — NIC/parent-child presence aggregation; can override step 6's result for parent devices. +8. `update_devPresentLastScan_based_on_force_status(db)` — the user's manual `devForceStatus` override, runs **last**, wins unconditionally over everything above. +9. `update_vendors_from_mac`, `update_ipv4_ipv6`, `update_icons_and_types` +10. `pair_sessions_events(db)` — pairs `Events` rows as described above. +11. `create_sessions_snapshot(db)` — `DELETE FROM Sessions; INSERT INTO Sessions SELECT * FROM Convert_Events_to_Sessions`. This is the point where `Sessions` actually reflects step 10's pairing. +12. `insertOnlineHistory(db)` — dashboard graph rollup. +13. `skip_repeated_notifications(db)` +14. `DELETE FROM CurrentScan` — the ephemeral table's entire lifetime is one call to `process_scan()`. + +## Field-write authority for scan-derived updates + +`update_devices_data_from_scan()` (`server/scan/device_handling.py`) does not blindly overwrite fields from whichever plugin ran most recently. Each trackable field is declared once in `FIELD_SPECS` (`scan_col`, `source_col`, a `priority` list of plugin prefixes, optional `allow_override_if_changed`), and `can_overwrite_field()` uses that plus `get_plugin_authoritative_settings()` (which reads a plugin's own settings for an explicit authority override) to decide, per field per row, whether this plugin's value may replace what's there. The paired `Source` column (`devNameSource`, `devLastIPSource`, etc.) records who currently owns the field. `devMac` itself is never a target of these updates — it's the join key, not a tracked field — so no scan-derived update path can ever alter a device's identity, only its attributes. + +This is the scan-pipeline-local half of a bigger attribution system — see the `database-patterns` skill for `FIELD_SOURCE_MAP` / `server/db/authoritative_handler.py`, the full `*Source` attribution model, and how SQLite triggers consume it for audit logging. Read both if touching anything that writes a `*Source` column. + +## Two real gotchas (not hypothetical — both surfaced live during a design review) + +1. **A "presence" check almost always exists in more than one place.** When adding a per-row signal meaning "don't count this as a live sighting" (e.g. a proposed `scanPresence` column), every query that independently re-derives "is this MAC currently present" from `CurrentScan` has to be updated together — `update_presence_from_CurrentScan()`, the `insert_events()` "New Connections"/"Device Down"/"Disconnected" queries, and the raw `INSERT INTO Sessions` inside `create_new_devices()` all encode that same question separately. Patching one and missing a sibling produces a UI where the device badge, the Events log, and the Sessions timeline each tell a different story for the same device. See `.gemini/internal-docs/PRDs/plugin-import-behavior-controls.md` for the worked example — a `scanPresence = 0` transition that never closed its session because only one of three "is it present" queries had been patched. +2. **`CurrentScan` is deleted at the end of every cycle — a per-row flag on it cannot express a decision that needs to survive to a cycle where the row is absent.** Anything that fires specifically *because* a row is missing (`Device Down`, `Disconnected`) cannot read a flag that lived on that now-gone row. If a per-row plugin signal needs to affect behavior beyond the cycle it arrived in, persist it onto the `Devices` row at creation time (e.g. seeding `devAlertDown`/`devAlertEvents` from the row's flag instead of the global `NEWDEV_*` defaults) rather than trying to make the ephemeral table carry it forward. + +## When to read this vs. other docs/skills + +- Writing or reviewing a plugin's `config.json`/data contract → `plugin-development` skill, `docs/PLUGINS_DEV*.md`. This skill is about what happens *after* a plugin's rows land in `CurrentScan`, not the plugin-authoring contract itself. +- Devices-table write paths, `*Source` attribution, audit/history logging, SQLite triggers → `database-patterns` skill. +- Actually implementing a change here → read the relevant function in `server/scan/session_events.py` / `server/scan/device_handling.py` directly before trusting this skill's line-number references; they're a map, not a guarantee, and will drift as the code moves. diff --git a/.gemini/skills/skills-index/SKILL.md b/.gemini/skills/skills-index/SKILL.md index 924346fff..5d25d7393 100644 --- a/.gemini/skills/skills-index/SKILL.md +++ b/.gemini/skills/skills-index/SKILL.md @@ -28,6 +28,8 @@ Skills with the same purpose exist in more than one, sometimes under different n | Devcontainer | `devcontainer-management` | `devcontainer-services` + `devcontainer-setup` + `devcontainer-configs` | — | Gemini combines into one (uses `docker exec`); Copilot splits into 3 focused skills | | PR review | `pr-analysis` | `pr-analysis` | `pr-analysis` | How to classify and respond to PR comments; pre-flight skill loading checklist | | Logging | `logging-standards` | `logging-standards` | — | `mylog` levels, message format, what not to log | +| Scan pipeline internals | `scan-pipeline` | `scan-pipeline` | `scan-pipeline` | `process_scan()` call order and why it's load-bearing, `CurrentScan`/`Events`/`Sessions`/`DevicesView` relationships, how a session actually closes (no `close_session()` exists), and the `FIELD_SPECS` field-write authority mechanism. Complements `database-patterns` (Devices write-path/`*Source` attribution) rather than duplicating it. | +| Database patterns | `database-patterns` | `database-patterns` | `database-patterns` | Devices table write-path inventory, the `FIELD_SOURCE_MAP`/`*Source` attribution system in `server/db/authoritative_handler.py`, SQLite trigger vs. Python-hook tradeoffs, and event-sourced vs. snapshot audit logging. | --- @@ -40,7 +42,6 @@ No Gemini equivalent yet: | `api-development` | Creating REST API endpoints | | `authentication` | API tokens and 401/403 debugging | | `code-standards` | Coding conventions and style rules | -| `database-patterns` | Device table write paths, SQLite triggers, audit logging, `*Source` attribution | | `database-reset` | Wipe and regenerate the database and config | | `docker-build` | Build Docker images for testing or production | | `docker-prune` | Clean unused Docker resources (destructive — requires confirmation) | diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index fa632e24c..ea5fc3255 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -45,6 +45,7 @@ Procedural knowledge lives in `.github/skills/`. Load the appropriate skill when | Find files and paths | `project-navigation` | | Coding standards | `code-standards` | | Devices table write paths, SQLite triggers, audit logging, `*Source` attribution | `database-patterns` | +| `process_scan()` call order, CurrentScan/Events/Sessions internals, device presence lifecycle | `scan-pipeline` | ## Execution Protocol diff --git a/.github/skills/scan-pipeline/SKILL.md b/.github/skills/scan-pipeline/SKILL.md new file mode 100644 index 000000000..4e6bd0575 --- /dev/null +++ b/.github/skills/scan-pipeline/SKILL.md @@ -0,0 +1,59 @@ +--- +name: netalertx-scan-pipeline +description: NetAlertX scan pipeline internals — process_scan() call order and why it's load-bearing, CurrentScan/Events/Sessions/DevicesView relationships, the Events-to-Sessions pairing mechanism, and the FIELD_SPECS field-write authority system. Use this when modifying server/scan/session_events.py or server/scan/device_handling.py, or reasoning about device presence, connect/disconnect events, or session timelines. +--- + +# Scan Pipeline & Device Presence Lifecycle + +## Scope + +This skill covers what happens *after* a plugin's rows land in `CurrentScan` — presence computation, event generation, and session/timeline derivation. It does not cover the plugin-authoring side (manifest, data contract, settings) — see the `plugin-development` skill and `docs/PLUGINS_DEV*.md` for that. It also does not cover the general `*Source` attribution system or SQLite triggers for audit logging — see the `database-patterns` skill (Copilot tree only, as of this writing) for that; the field-authority section below is the scan-pipeline-local half of that bigger system, and the two overlap. + +## The core tables/views and their lifetimes + +- **`CurrentScan`** — ephemeral scratch table. Populated by `process_plugin_events()` for any plugin whose `config.json` declares `mapped_to_table`, then fully deleted at the end of every `process_scan()` cycle (`DELETE FROM CurrentScan`). Never assume a value written into a `CurrentScan` row is readable outside the cycle it arrived in — by the time a later cycle needs to check "was this row flagged X," the row is gone. This bit a real design (see Gotcha 2 below). +- **`Devices`** — persistent identity + state table. +- **`Events`** — persistent, append-only log of state-transition events (`New Device`, `Connected`, `Down Reconnected`, `Device Down`, `Disconnected`, `IP Changed`). This *is* the audit trail; `Sessions` is derived from it, not the other way around. +- **`Sessions`** — not incrementally updated; fully wiped and rebuilt every cycle from a view (see `Convert_Events_to_Sessions` below). Don't reason about it as a live connection state machine — it's a materialized query result recomputed from `Events` each cycle. +- **`Online_History`** — persistent, one row per scan cycle, feeds the dashboard's online/offline graph. Purely a rollup of `devPresentLastScan`/`devAlertDown`/`devIsSleeping` counts on `DevicesView` — no independent state of its own. + +## Key views + +- **`LatestDeviceScan`** (`server/db/db_upgrade.py`) — `Devices` LEFT JOIN'd to the most recent `CurrentScan` row **per `(scanMac, scanSourcePlugin)` pair**, ranked via `ROW_NUMBER() OVER (PARTITION BY scanMac, scanSourcePlugin ...)`. This is why `update_devices_data_from_scan()` loops over `DISTINCT scanSourcePlugin` and re-queries this view once per plugin: when two plugins report the same device in the same cycle, they are *not* merged into one row before processing — each plugin's contribution is evaluated independently, per field, through the authority mechanism below. +- **`LatestEventsPerMAC`** — most recent Event per MAC, joined to `Devices` and `CurrentScan`. Used by the "New Connections" query in `insert_events()` to decide whether a device was previously down (→ `Down Reconnected`) or genuinely new (→ `Connected`). +- **`Convert_Events_to_Sessions`** — the actual definition of "is this device's session still open." **There is no `close_session()`-style function anywhere in this codebase.** A session closes purely as an emergent property: `pair_sessions_events()` sets `evePairEventRowid` on a `New Device`/`Connected`/`Down Reconnected` Event to point at the next `Disconnected`/`Device Down` Event for that MAC, and this view computes `sesStillConnected = 1` exactly when that pairing is still `NULL`. If a session needs to close, the fix is always "make sure the right `Events` row gets inserted" — never a direct `Sessions` mutation (the one exception is `create_new_devices()`'s reconnect-insert, noted below). +- **`DevicesView`** — adds computed `devIsSleeping`/`devFlapping`/`devStatus` on top of `Devices`. This is what the UI and `insertOnlineHistory()` actually read presence from, not the raw `Devices` table. + +## `process_scan()` call order (`server/scan/session_events.py`) — the order is load-bearing, not incidental + +1. `save_own_device()`, `exclude_ignored_devices()` +2. `insert_events(db)` — **runs before presence gets updated for this cycle.** Deliberate: the Down/Disconnected/Connected queries need the *previous* cycle's `devPresentLastScan` value to detect a transition (present last cycle but absent now, or vice versa). If this ran after the presence update, every query would see the already-updated value and the edge-triggered design would collapse into either never firing or firing every cycle. +3. `create_new_devices(db)` — the source comment is explicit: "after create events -> avoid 'connection' event." Brand-new devices get a `New Device` event instead of a `Connected` event, because at step 2 they didn't exist as `Devices` rows for either query to match. Also contains a raw `INSERT INTO Sessions ... sesStillConnected = 1` for devices that already exist but have no currently-open session — the one place outside the `Events`-derived path that writes `Sessions` directly. +4. `update_devices_data_from_scan(db)` — field-level updates for existing devices; see the authority mechanism below. +5. `update_sync_hub_node`, `update_devLastConnection_from_CurrentScan` +6. `update_presence_from_CurrentScan(db)` — sets `devPresentLastScan` from bare `CurrentScan` presence *for this cycle* (this becomes the "previous" value step 2 reads on the *next* cycle). +7. `update_devPresentLastScan_based_on_nics(db)` — NIC/parent-child presence aggregation; can override step 6's result for parent devices. +8. `update_devPresentLastScan_based_on_force_status(db)` — the user's manual `devForceStatus` override, runs **last**, wins unconditionally over everything above. +9. `update_vendors_from_mac`, `update_ipv4_ipv6`, `update_icons_and_types` +10. `pair_sessions_events(db)` — pairs `Events` rows as described above. +11. `create_sessions_snapshot(db)` — `DELETE FROM Sessions; INSERT INTO Sessions SELECT * FROM Convert_Events_to_Sessions`. This is the point where `Sessions` actually reflects step 10's pairing. +12. `insertOnlineHistory(db)` — dashboard graph rollup. +13. `skip_repeated_notifications(db)` +14. `DELETE FROM CurrentScan` — the ephemeral table's entire lifetime is one call to `process_scan()`. + +## Field-write authority for scan-derived updates + +`update_devices_data_from_scan()` (`server/scan/device_handling.py`) does not blindly overwrite fields from whichever plugin ran most recently. Each trackable field is declared once in `FIELD_SPECS` (`scan_col`, `source_col`, a `priority` list of plugin prefixes, optional `allow_override_if_changed`), and `can_overwrite_field()` uses that plus `get_plugin_authoritative_settings()` (which reads a plugin's own settings for an explicit authority override) to decide, per field per row, whether this plugin's value may replace what's there. The paired `Source` column (`devNameSource`, `devLastIPSource`, etc.) records who currently owns the field. `devMac` itself is never a target of these updates — it's the join key, not a tracked field — so no scan-derived update path can ever alter a device's identity, only its attributes. + +This is the scan-pipeline-local half of a bigger attribution system — see the `database-patterns` skill for `FIELD_SOURCE_MAP` / `server/db/authoritative_handler.py`, the full `*Source` attribution model, and how SQLite triggers consume it for audit logging. Read both if touching anything that writes a `*Source` column. + +## Two real gotchas (not hypothetical — both surfaced live during a design review) + +1. **A "presence" check almost always exists in more than one place.** When adding a per-row signal meaning "don't count this as a live sighting" (e.g. a proposed `scanPresence` column), every query that independently re-derives "is this MAC currently present" from `CurrentScan` has to be updated together — `update_presence_from_CurrentScan()`, the `insert_events()` "New Connections"/"Device Down"/"Disconnected" queries, and the raw `INSERT INTO Sessions` inside `create_new_devices()` all encode that same question separately. Patching one and missing a sibling produces a UI where the device badge, the Events log, and the Sessions timeline each tell a different story for the same device. See `.gemini/internal-docs/PRDs/plugin-import-behavior-controls.md` for the worked example — a `scanPresence = 0` transition that never closed its session because only one of three "is it present" queries had been patched. +2. **`CurrentScan` is deleted at the end of every cycle — a per-row flag on it cannot express a decision that needs to survive to a cycle where the row is absent.** Anything that fires specifically *because* a row is missing (`Device Down`, `Disconnected`) cannot read a flag that lived on that now-gone row. If a per-row plugin signal needs to affect behavior beyond the cycle it arrived in, persist it onto the `Devices` row at creation time (e.g. seeding `devAlertDown`/`devAlertEvents` from the row's flag instead of the global `NEWDEV_*` defaults) rather than trying to make the ephemeral table carry it forward. + +## When to read this vs. other docs/skills + +- Writing or reviewing a plugin's `config.json`/data contract → `plugin-development` skill, `docs/PLUGINS_DEV*.md`. This skill is about what happens *after* a plugin's rows land in `CurrentScan`, not the plugin-authoring contract itself. +- Devices-table write paths, `*Source` attribution, audit/history logging, SQLite triggers → `database-patterns` skill. +- Actually implementing a change here → read the relevant function in `server/scan/session_events.py` / `server/scan/device_handling.py` directly before trusting this skill's line-number references; they're a map, not a guarantee, and will drift as the code moves. diff --git a/.github/skills/skills-overview/SKILL.md b/.github/skills/skills-overview/SKILL.md index 8c0b26184..5afe10f70 100644 --- a/.github/skills/skills-overview/SKILL.md +++ b/.github/skills/skills-overview/SKILL.md @@ -28,6 +28,8 @@ Skills with the same purpose exist in more than one, sometimes under different n | Devcontainer | `devcontainer-services` + `devcontainer-setup` + `devcontainer-configs` | `devcontainer-management` | — | Copilot splits into 3 focused skills; Gemini combines into one (uses `docker exec`) | | PR review | `pr-analysis` | `pr-analysis` | `pr-analysis` | How to classify and respond to PR comments; pre-flight skill loading checklist | | Logging | `logging-standards` | `logging-standards` | — | `mylog` levels, message format, what not to log | +| Scan pipeline internals | `scan-pipeline` | `scan-pipeline` | `scan-pipeline` | `process_scan()` call order and why it's load-bearing, `CurrentScan`/`Events`/`Sessions`/`DevicesView` relationships, how a session actually closes (no `close_session()` exists), and the `FIELD_SPECS` field-write authority mechanism. Complements `database-patterns` (Devices write-path/`*Source` attribution) rather than duplicating it. | +| Database patterns | `database-patterns` | `database-patterns` | `database-patterns` | Devices table write-path inventory, the `FIELD_SOURCE_MAP`/`*Source` attribution system in `server/db/authoritative_handler.py`, SQLite trigger vs. Python-hook tradeoffs, and event-sourced vs. snapshot audit logging. | --- @@ -40,7 +42,6 @@ No Gemini equivalent yet: | `api-development` | Creating REST API endpoints | | `authentication` | API tokens and 401/403 debugging | | `code-standards` | Coding conventions and style rules | -| `database-patterns` | Device table write paths, SQLite triggers, audit logging, `*Source` attribution | | `database-reset` | Wipe and regenerate the database and config | | `docker-build` | Build Docker images for testing or production | | `docker-prune` | Clean unused Docker resources (destructive — requires confirmation) | diff --git a/scripts/check_skill_pairs.py b/scripts/check_skill_pairs.py index 123318503..6cc4ed04d 100644 --- a/scripts/check_skill_pairs.py +++ b/scripts/check_skill_pairs.py @@ -28,6 +28,8 @@ GROUPS = [ [".gemini/skills/plugin-readme/plugin-readme-skill.md", ".github/skills/plugin-readme/SKILL.md", ".claude/skills/plugin-readme/SKILL.md"], [".gemini/skills/testing-workflow/SKILL.md", ".github/skills/testing-workflow/SKILL.md", ".claude/skills/testing-workflow/SKILL.md"], [".gemini/skills/pr-analysis/SKILL.md", ".github/skills/pr-analysis/SKILL.md", ".claude/skills/pr-analysis/SKILL.md"], + [".gemini/skills/scan-pipeline/SKILL.md", ".github/skills/scan-pipeline/SKILL.md", ".claude/skills/scan-pipeline/SKILL.md"], + [".gemini/skills/database-patterns/SKILL.md", ".github/skills/database-patterns/SKILL.md", ".claude/skills/database-patterns/SKILL.md"], [".gemini/skills/settings/SKILL.md", ".github/skills/settings-management/SKILL.md"], [".gemini/skills/mcp-activation/SKILL.md", ".github/skills/mcp-activation/SKILL.md"], [".gemini/skills/project-navigation/SKILL.md", ".github/skills/project-navigation/SKILL.md"],