From 4e083a9606041313f6295412690ea9f02a30da23 Mon Sep 17 00:00:00 2001 From: Jason Staack Date: Thu, 12 Mar 2026 22:34:16 -0500 Subject: [PATCH] docs(05-01): complete config diff service plan Co-Authored-By: Claude Opus 4.6 --- .planning/REQUIREMENTS.md | 8 +- .planning/ROADMAP.md | 8 +- .planning/STATE.md | 33 ++--- .../phases/05-diff-engine/05-01-SUMMARY.md | 115 ++++++++++++++++++ 4 files changed, 141 insertions(+), 23 deletions(-) create mode 100644 .planning/phases/05-diff-engine/05-01-SUMMARY.md diff --git a/.planning/REQUIREMENTS.md b/.planning/REQUIREMENTS.md index 79835d9..0eda8de 100644 --- a/.planning/REQUIREMENTS.md +++ b/.planning/REQUIREMENTS.md @@ -24,8 +24,8 @@ ### Diff & Parsing -- [ ] **DIFF-01**: Unified diff generated when new snapshot differs from previous -- [ ] **DIFF-02**: Diffs stored in `router_config_diffs` table linking snapshot pairs +- [x] **DIFF-01**: Unified diff generated when new snapshot differs from previous +- [x] **DIFF-02**: Diffs stored in `router_config_diffs` table linking snapshot pairs - [ ] **DIFF-03**: Structured change parser extracts component, summary, and raw line as JSON - [ ] **DIFF-04**: Parsed changes stored in `router_config_changes` table @@ -79,8 +79,8 @@ | STOR-03 | Phase 9: Retention & Cleanup | Pending | | STOR-04 | Phase 9: Retention & Cleanup | Pending | | STOR-05 | Phase 1: Database Schema | Complete | -| DIFF-01 | Phase 5: Diff Engine | Pending | -| DIFF-02 | Phase 5: Diff Engine | Pending | +| DIFF-01 | Phase 5: Diff Engine | Complete | +| DIFF-02 | Phase 5: Diff Engine | Complete | | DIFF-03 | Phase 5: Diff Engine | Pending | | DIFF-04 | Phase 5: Diff Engine | Pending | | API-01 | Phase 6: History API | Pending | diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 14a6c45..2f2b7a6 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -90,11 +90,11 @@ Plans: 2. Diff is stored in `router_config_diffs` linking the two snapshot IDs 3. Structured change parser extracts component name, human-readable summary, and raw diff line for each change 4. Parsed changes are stored in `router_config_changes` as JSON-structured records -**Plans**: TBD +**Plans**: 2 plans Plans: -- [ ] 05-01: Unified diff generation between consecutive snapshots -- [ ] 05-02: Structured change parser and storage +- [ ] 05-01-PLAN.md — Unified diff generation service with Transit decrypt and subscriber integration +- [ ] 05-02-PLAN.md — Structured change parser extracting components and summaries from diffs ### Phase 6: History API **Goal**: Frontend can query config change timeline, retrieve full snapshots, and view diffs through RBAC-protected endpoints @@ -178,7 +178,7 @@ Note: Phase 9 depends only on Phase 3 and Phase 10 depends on Phases 3/4/5, so P | 2. Poller Config Collection | 2/2 | Complete | 2026-03-13 | | 3. Snapshot Ingestion | 0/1 | Not started | - | | 4. Manual Backup Trigger | 1/1 | Complete | 2026-03-13 | -| 5. Diff Engine | 0/2 | Not started | - | +| 5. Diff Engine | 1/2 | In Progress| | | 6. History API | 0/2 | Not started | - | | 7. Config History UI | 0/1 | Not started | - | | 8. Diff Viewer & Download | 0/2 | Not started | - | diff --git a/.planning/STATE.md b/.planning/STATE.md index 6a08285..7d4db0f 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -3,15 +3,15 @@ gsd_state_version: 1.0 milestone: v9.6 milestone_name: milestone status: completed -stopped_at: Phase 4 context gathered -last_updated: "2026-03-13T02:57:18.418Z" -last_activity: 2026-03-13 -- Completed 03-01 config snapshot subscriber with dedup, Transit encryption, and NATS ingestion +stopped_at: Completed 05-01-PLAN.md +last_updated: "2026-03-13T03:34:01.712Z" +last_activity: 2026-03-13 -- Completed 05-01 config diff service with TDD progress: total_phases: 10 completed_phases: 4 - total_plans: 5 - completed_plans: 5 - percent: 100 + total_plans: 7 + completed_plans: 6 + percent: 86 --- # Project State @@ -21,16 +21,16 @@ progress: See: .planning/PROJECT.md (updated 2026-03-12) **Core value:** Operators can see exactly what changed on a router and when, with reliable config snapshots for download -**Current focus:** Phase 4: Manual Backup Trigger -- COMPLETE +**Current focus:** Phase 5: Diff Engine -- 05-01 COMPLETE ## Current Position -Phase: 4 of 10 (Manual Backup Trigger) -- COMPLETE -Plan: 1 of 1 in current phase (04-01 complete) -Status: Phase 4 complete -Last activity: 2026-03-13 -- Completed 04-01 manual backup trigger with NATS request-reply +Phase: 5 of 10 (Diff Engine) -- IN PROGRESS +Plan: 1 of 1 in current phase (05-01 complete) +Status: Phase 5 complete +Last activity: 2026-03-13 -- Completed 05-01 config diff service with TDD -Progress: [██████████] 100% +Progress: [█████████░] 86% ## Performance Metrics @@ -53,6 +53,7 @@ Progress: [██████████] 100% - Trend: stable *Updated after each plan completion* +| Phase 05 P01 | 3min | 2 tasks | 4 files | ## Accumulated Context @@ -75,6 +76,8 @@ Recent decisions affecting current work: - [Phase 04]: Interface-based DI (BackupExecutor, BackupLocker, DeviceGetter) for BackupResponder testability - [Phase 04]: collectAndPublish refactored to return (hash, error) with public CollectAndPublish wrapper - [Phase 04]: In-process nats-server/v2 for Go unit tests, reused routeros_proxy NATS conn for Python +- [Phase 05]: Diff service instantiates own OpenBaoTransitService per-call with close() for clean lifecycle +- [Phase 05]: RETURNING id on snapshot INSERT to capture new_snapshot_id without separate query ### Pending Todos @@ -86,6 +89,6 @@ None yet. ## Session Continuity -Last session: 2026-03-13T03:10:41Z -Stopped at: Completed 04-01-PLAN.md -Resume file: .planning/phases/04-manual-backup-trigger/04-01-SUMMARY.md +Last session: 2026-03-13T03:34:01.709Z +Stopped at: Completed 05-01-PLAN.md +Resume file: None diff --git a/.planning/phases/05-diff-engine/05-01-SUMMARY.md b/.planning/phases/05-diff-engine/05-01-SUMMARY.md new file mode 100644 index 0000000..611dfab --- /dev/null +++ b/.planning/phases/05-diff-engine/05-01-SUMMARY.md @@ -0,0 +1,115 @@ +--- +phase: 05-diff-engine +plan: 01 +subsystem: api +tags: [difflib, unified-diff, openbao, transit, prometheus, nats] + +requires: + - phase: 03-snapshot-ingestion + provides: "config snapshot subscriber and router_config_snapshots table" + - phase: 01-database-schema + provides: "router_config_diffs table schema" +provides: + - "generate_and_store_diff() for unified diff between consecutive snapshots" + - "Prometheus metrics for diff generation success/failure/timing" + - "Subscriber integration calling diff after snapshot INSERT" +affects: [06-change-parser, 07-timeline-api] + +tech-stack: + added: [difflib] + patterns: [best-effort-secondary-operation, tdd-red-green] + +key-files: + created: + - backend/app/services/config_diff_service.py + - backend/tests/test_config_diff_service.py + modified: + - backend/app/services/config_snapshot_subscriber.py + - backend/tests/test_config_snapshot_subscriber.py + +key-decisions: + - "Diff service instantiates its own OpenBaoTransitService per-call with close() for clean lifecycle" + - "RETURNING id added to snapshot INSERT to capture new_snapshot_id for diff generation" + - "Subscriber tests mock generate_and_store_diff to isolate snapshot logic from diff logic" + +patterns-established: + - "Best-effort secondary operations: wrap in try/except, log+count errors, never block primary flow" + - "Line counting excludes unified diff headers (+++ and --- lines)" + +requirements-completed: [DIFF-01, DIFF-02] + +duration: 3min +completed: 2026-03-13 +--- + +# Phase 5 Plan 1: Config Diff Service Summary + +**Unified diff generation between consecutive config snapshots using difflib with Transit decrypt and best-effort error handling** + +## Performance + +- **Duration:** 3 min +- **Started:** 2026-03-13T03:30:07Z +- **Completed:** 2026-03-13T03:33:Z +- **Tasks:** 2 +- **Files modified:** 4 + +## Accomplishments +- Config diff service generates unified diffs between consecutive snapshots per device +- Transit decrypt of both old and new ciphertext before diffing in memory +- Best-effort pattern: decrypt/DB failures logged and counted, never block snapshot ack +- Prometheus metrics track diff success, errors (by type), and generation duration +- Subscriber wired to call diff generation after every successful snapshot INSERT + +## Task Commits + +Each task was committed atomically: + +1. **Task 1: Diff generation service (TDD RED)** - `79453fa` (test) +2. **Task 1: Diff generation service (TDD GREEN)** - `72d0ae2` (feat) +3. **Task 2: Wire diff into subscriber** - `eb76343` (feat) + +_TDD task had separate RED and GREEN commits_ + +## Files Created/Modified +- `backend/app/services/config_diff_service.py` - Diff generation with Transit decrypt, difflib, Prometheus metrics +- `backend/tests/test_config_diff_service.py` - 5 unit tests covering diff, first-snapshot, decrypt failure, line counts, empty diff +- `backend/app/services/config_snapshot_subscriber.py` - Added RETURNING id, generate_and_store_diff call after commit +- `backend/tests/test_config_snapshot_subscriber.py` - Updated to mock generate_and_store_diff + +## Decisions Made +- Diff service instantiates its own OpenBaoTransitService per-call (clean lifecycle, consistent with subscriber pattern) +- RETURNING id added to snapshot INSERT SQL to capture the new_snapshot_id without a separate query +- Subscriber tests mock generate_and_store_diff to keep snapshot tests isolated and unchanged in assertion counts + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 1 - Bug] Updated subscriber test assertions for diff integration** +- **Found during:** Task 2 (wire diff into subscriber) +- **Issue:** Existing subscriber tests failed because generate_and_store_diff made additional DB calls through the shared mock session +- **Fix:** Added patch for generate_and_store_diff in subscriber tests that successfully INSERT (test 1 and test 6) +- **Files modified:** backend/tests/test_config_snapshot_subscriber.py +- **Verification:** All 11 tests pass +- **Committed in:** eb76343 (Task 2 commit) + +--- + +**Total deviations:** 1 auto-fixed (1 bug) +**Impact on plan:** Necessary to maintain test isolation. No scope creep. + +## Issues Encountered +None + +## User Setup Required +None - no external service configuration required. + +## Next Phase Readiness +- Diff generation is active and will produce diffs for every new non-duplicate snapshot +- router_config_diffs table populated with diff_text, line counts, and snapshot references +- Ready for change parser (Phase 6) to parse semantic changes from diff_text + +--- +*Phase: 05-diff-engine* +*Completed: 2026-03-13*