Move to openspec/changes/archive/2026-07-09-per-instance-hook-scoping/ (R100 renames preserved). 9 artifacts. Canonical openspec/specs/ service-instance-scoping/ remains. Resolves multi-instance wrong-data bug (hooks now scope by instance.id; instance switcher re-scopes). Carry-overs: fetchBackupDashboard untouched (design decision 5); subquery scoping for runs/alerts (schema asymmetry).
14 KiB
SDD Verify Report: Per-Instance Hook Scoping
Change: per-instance-hook-scoping
Phase: verify
Date: 2026-07-09
Mode: Read-only adversarial audit (no source edits, no git push)
Implementing commit: 3bc7ce5 (HEAD; single slice, 12 files, +268 / −73)
Executive summary
| Gate | Command | Result |
|---|---|---|
| Backend tests | cd backend && PYTHONPATH=src python3 -m pytest -q |
PASS — 368 passed |
| Backend lint | cd backend && PYTHONPATH=src python3 -m ruff check src tests |
PASS — All checks passed |
| Frontend build | cd frontend && npm run build |
PASS — exit 0 |
| Frontend lint | cd frontend && npm run lint |
PASS — 0 errors, 1 pre-existing warning (unrelated) |
| Frontend tests | cd frontend && npx vitest run |
PASS — 46 files, 165 tests passed |
Implementation verdict: All 21 spec requirements (PI-101..PI-121) are satisfied against source, with one deliberate, design-documented scope refinement (the fetchBackupDashboard / get_backup_dashboard variant was intentionally excluded per design decision 5 — see PI-109/PI-110 notes). The implementation matches the 6 design decisions. Backend filter SQL is parameterized (no injection risk).
BLOCKING process finding: The implementation is committed and correct, but tasks.md checkboxes were never updated — 0/17 are checked and no apply-progress.md artifact exists. Per the SDD verify contract, unchecked implementation tasks are CRITICAL archive blockers regardless of code state. The work is functionally done; the checkbox/progress hygiene is the gap. This must be reconciled (checkbox reconciliation via apply-progress) before archive.
Spec coverage (PI-101 .. PI-121)
| Req | Verdict | Evidence |
|---|---|---|
| PI-101 useAlertmanagerAlerts(serviceId) | ✅ PASS | useObservability.ts: queryKey: ["observability","alerts", serviceId ?? ""], queryFn: () => fetchAlertmanagerAlerts(serviceId). serviceId in key + fetch. |
| PI-102 useAlertmanagerStatus(serviceId) | ✅ PASS | queryKey: ["observability","alertmanager-status", serviceId ?? ""], arrow queryFn captures serviceId. |
| PI-103 usePrometheusStatus(serviceId) | ✅ PASS | queryKey: ["observability","prometheus-status", serviceId ?? ""], arrow queryFn. |
| PI-104 useBackupJobs(serviceId) | ✅ PASS | useBackups.ts: queryKey: ["backups","jobs", serviceId ?? ""]. |
| PI-105 useBackupRuns(jobId,status,serviceId) | ✅ PASS | queryKey: ["backups","runs", jobId, status, serviceId ?? ""]. |
| PI-106 useBackupAlerts(jobId,ack,sev,serviceId) | ✅ PASS | queryKey includes serviceId ?? "" as last element. |
| PI-107 usePrometheusTargets + useMonitoringMachines unchanged | ✅ PASS | Neither gains serviceId; both still global, direct queryFn refs. MetricsTab keeps usePrometheusTargets() global. |
PI-108 observability fetch fns pass ?service_id |
✅ PASS | client.ts: fetchAlertmanagerAlerts/Status + fetchPrometheusStatus each (serviceId?) → serviceId ? { service_id: serviceId } : undefined. |
PI-109 backup fetch fns pass ?service_id |
✅ PASS* | backups.ts: fetchBackupJobs, fetchBackupRuns (3rd arg), fetchBackupAlerts (4th arg) append service_id via conditional spread. fetchBackupDashboard deliberately NOT modified (design decision 5). *Spec text PI-109 lists fetchBackupDashboard; design/tasks override this — see Findings. |
| PI-110 backup endpoints accept service_id | ✅ PASS* | routers/backups.py: get_backup_jobs, get_backup_runs, get_backup_alerts each gain service_id: str | None = None and thread it into the store. get_backup_dashboard is in routers/dashboard.py and intentionally untouched (design decision 5). *Spec PI-110 lists it; design overrides. |
| PI-111 SettingsStore filter by service_id | ✅ PASS | settings_store.py: list_backup_jobs direct WHERE service_id = ?; list_backup_runs + list_backup_alerts use job_id IN (SELECT id FROM backup_jobs WHERE service_id = ?) subquery. Truthy-check if service_id: → None AND "" skip filter (backward-compat). Parameterized — no injection. |
| PI-112 Alertmanager/Prometheus already accept service_id | ✅ PASS (zero backend change, confirmed) | monitoring.py: get_alertmanager_alerts/status + get_prometheus_status already service_id: str | None = None → resolve_service_record(store, "<type>", service_id). No change needed. |
| PI-113 AlertsTab passes instance.id | ✅ PASS | AlertsTab.tsx: useAlertmanagerAlerts(instance.id) + useAlertmanagerStatus(instance.id). No void instance;. TODO removed; docstring updated. |
| PI-114 MetricsTab passes instance.id | ✅ PASS | MetricsTab.tsx: usePrometheusStatus(instance.id). usePrometheusTargets() stays global. TODO removed. |
| PI-115 JobsTab passes instance.id | ✅ PASS | JobsTab.tsx: useBackupJobs(instance.id), useBackupRuns(undefined, undefined, instance.id), useBackupAlerts(undefined, false, undefined, instance.id). TODO removed. |
| PI-116 instance switcher re-scopes | ✅ PASS | ServicePage.tsx: sibling switcher onClick={() => navigate(...)} → URL param change → instance useMemo recomputes → <TabComponent instance={instance} /> new prop → queryKey change → refetch. Falls out naturally. |
| PI-117 dashboard widgets unaffected | ✅ PASS | Widgets resolve via useWidgetData (separate path). Grep confirms no widget/component imports any of the 6 modified hooks. BackupDashboardWidget uses useBackupDashboard (unchanged) — its test green. Widget tests unchanged & passing. |
| PI-118 all new params optional | ✅ PASS | Every new param is optional with a default preserving today's behavior. Non-regression callers verified: backup_poller.py (list_backup_jobs(), list_backup_alerts(acknowledged=False)), domain/dashboard.py (list_backup_jobs(), list_backup_runs(job_id=...,limit=...)), build_backup_dashboard_summary(store) all call with no service_id → return all. |
| PI-119 backend tests + lint green | ✅ PASS | TestBackupServiceScoping covers jobs/runs/alerts filtered + unfiltered (incl. empty-string) + endpoint threading. 368 pytest pass; ruff clean. |
| PI-120 frontend typecheck/build/lint | ✅ PASS | npm run build exit 0; npm run lint 0 errors (1 pre-existing warning in WidgetConfigDialog.tsx, unrelated). |
| PI-121 hook + tab tests cover wiring | ✅ PASS (1 quality note) | AlertsTab.test.tsx asserts vi.mocked(useAlertmanagerAlerts).toHaveBeenCalledWith("am-1") — strong assertion. Hook test useBackups.test.ts exists covering all 6 hooks. Quality note: the hook-test "different keys" assertion (expect(a).not.toBe(b) on renderHook result refs) is weak/tautology-prone — see Findings. |
Coverage: 21/21 requirements satisfied against source.
Adversarial checks
- Any tab still calling a scoped hook without instance.id? No. All three tabs pass
instance.id; no remainingvoid instance;anywhere inservice-tabs/. - Instance switcher actually re-scopes? Yes —
navigate(...)re-derivesinstancefrom URL → queryKey changes → refetch (PI-116). - Dashboard widget regression? None.
fetchBackupDashboard/useBackupDashboard/get_backup_dashboard/build_backup_dashboard_summaryall untouched;BackupDashboardWidgettest green. - Subquery SQL injection? Safe — both subqueries use
?placeholders withparams.append(service_id). No string interpolation of user input. - Cache-key collision? All 6 modified hooks include
serviceId ?? ""as last queryKey element (verified per hook). Single stable key for undefined; distinct key per instance.
Structured status & actionContext findings
- Native status engine reports
state: blocked,taskProgress 0/17,applyProgress: missing,nextRecommended: "domain specs are missing or partial", withblockedReasonsciting missing domain specs + legacy flat spec present without domain specs. - These blockers are artifact-hygiene, not implementation gaps. The code is committed and all gates are green. The substantive blockers to archive are:
tasks.mdcheckboxes: 0/17 checked (all still- [ ]). Implementation is done; checkboxes were not reconciled.- No
apply-progress.mdartifact exists. - No domain
specs/deltas — a flatspec.mdis present without domain spec packages (same pattern the status engine flags). This is a sync-stage concern (sync reconciles flat→domain), not a verify blocker; verify is satisfied.
actionContext.mode: repo-local,allowedEditRoots: ["/home/user/manage"]— implementation files are all within allowed roots. Ownership proven.
Strict TDD
Strict TDD is not active in openspec/config.yaml for this workflow (no strict-TDD directive present; this verify run was not gated on a TDD cycle-evidence table). N/A.
Review workload / PR boundary
tasks.mdforecast: single PR, ~257 lines, low budget risk, no chained PRs.- Actual: single commit
3bc7ce5, +268/−73, 12 files — matches the single-slice forecast. No scope creep. No chained-PR boundary violation.
Findings
Blocking
- BLOCKER-1 (CRITICAL — archive blocker; process hygiene, NOT code):
tasks.mdcheckboxes unreconciled — 0/17 checked, noapply-progress.md. The implementation for all 17 sub-tasks is present and verified, buttasks.mdstill shows every task as- [ ](e.g.- [ ] **1.1 Add service_id filter to list_backup_jobs**…- [ ] **1.17 Verify Slice 1 …**). Per the SDD verify contract, unchecked implementation tasks are CRITICAL archive blockers. The native status engine correctly reflects this (0/17). Exact unchecked lines (all 17):- [ ] **1.1 Add \service_id` filter to `list_backup_jobs`**`- [ ] **1.2 Add \service_id` filter to `list_backup_runs` (subquery)**`- [ ] **1.3 Add \service_id` filter to `list_backup_alerts` (subquery)**`- [ ] **1.4 Add backend tests for backup service scoping**- [ ] **1.5 Add \service_id` query param to backup endpoints**`- [ ] **1.6 Add backend test for endpoint threading**- [ ] **1.7 Confirm Alertmanager/Prometheus endpoints need no change**- [ ] **1.8 Add \serviceId` param to observability fetch functions**`- [ ] **1.9 Add \serviceId` param to backup fetch functions (NOT Dashboard)**`- [ ] **1.10 Add \serviceId` to `useObservability` hooks**`- [ ] **1.11 Add \serviceId` to `useBackups` hooks**`- [ ] **1.12 Wire \instance.id` into AlertsTab**`- [ ] **1.13 Wire \instance.id` into MetricsTab**`- [ ] **1.14 Wire \instance.id` into JobsTab**`- [ ] **1.15 Add hook test for per-instance queryKey**- [ ] **1.16 Add tab test for \instance.id` wiring**`- [ ] **1.17 Verify Slice 1 (build + lint + test green)**- Resolution (apply/sync phase, not verify): create
apply-progress.mdrecording the work done in3bc7ce5, then flip the 17 checkboxes to[x]. This is a stale-checkbox reconciliation explicitly permitted by the archive-exception clause once apply-progress proves the work.
Non-blocking
- NB-1 (spec↔design wording mismatch — amend spec): Spec PI-109 and PI-110 literally list
fetchBackupDashboard/get_backup_dashboardas in-scope for a new service param. Design decision 5 and tasks 1.5/1.9 deliberately exclude them (dashboard widget path, PI-117 risk). The implementation correctly follows the design. Recommend amendingspec.mdPI-109/PI-110 to drop the dashboard variant so spec and design agree. No functional defect. - NB-2 (weak assertion quality, PI-121 hook test):
useBackups.test.ts"different keys for different serviceIds" assertsexpect(a).not.toBe(b)on tworenderHookresult objects. This is trivially true (eachrenderHookreturns a new reference) and does not actually assert thequeryKeydiffers — the test would pass even if queryKeys collided. The hook wiring is correct in source (manually verified), and the tab test (AlertsTab.test.tsxtoHaveBeenCalledWith("am-1")) is strong, so PI-121 is satisfied. Recommend strengthening the hook test to inspectqueryClient.getQueryCache().getAll()keys or assertqueryFn(mockedfetchBackupJobs) is invoked with the distinct serviceId. - NB-3 (project-map staleness): Several
.pi-map.mdartifacts still describe the pre-change signatures (e.g.list_backup_jobs(self)withoutservice_id,fetchBackupJobs(), router docs without service_id). These are generated map docs, not source. Cosmetic; reconcile viaproject_map_patchwhen convenient. Not part of this change's source scope.
Commands run (exact)
| Command | Result |
|---|---|
cd backend && PYTHONPATH=src python3 -m pytest -q |
368 passed, 2 warnings |
cd backend && PYTHONPATH=src python3 -m pytest tests/test_backups.py -q -k "ServiceScoping or endpoint_threads or filtered or unfiltered" |
6 passed, 2 deselected |
cd backend && PYTHONPATH=src python3 -m ruff check src tests |
All checks passed! |
cd frontend && npm run build |
exit 0 (tsc -b + vite build) |
cd frontend && npm run lint |
0 errors, 1 warning (pre-existing, WidgetConfigDialog.tsx) |
cd frontend && npx vitest run |
46 files, 165 tests passed |
Changed files (commit 3bc7ce5)
backend/src/media_library_viewer_api/routers/backups.pybackend/src/media_library_viewer_api/services/settings_store.pybackend/tests/test_backups.pyfrontend/src/api/backups.tsfrontend/src/api/client.tsfrontend/src/hooks/useBackups.tsfrontend/src/hooks/useObservability.tsfrontend/src/hooks/__tests__/useBackups.test.ts(new)frontend/src/pages/service-tabs/AlertsTab.tsxfrontend/src/pages/service-tabs/MetricsTab.tsxfrontend/src/pages/service-tabs/JobsTab.tsxfrontend/src/pages/service-tabs/__tests__/AlertsTab.test.tsx(updated)
Conclusion
The per-instance-hook-scoping change is functionally complete and correct: all 21 requirements pass against source, all 5 gates are green, non-regression is confirmed, and the subquery SQL is injection-safe. The only blocker is process hygiene — tasks.md checkboxes are stale (0/17) and apply-progress.md is missing. Reconcile those (write apply-progress, flip checkboxes) to clear the archive gate. No code changes are required.