From f921524d375c8d97c4d8d908abaaa1c3ad5e0674 Mon Sep 17 00:00:00 2001 From: Developer Date: Fri, 10 Jul 2026 00:12:54 +0000 Subject: [PATCH] spec(per-instance-hook-scoping): sync into new canonical domain New canonical openspec/specs/service-instance-scoping/spec.md (21 reqs PI-101..121). Change-side delta + sync-report. web-ui/prometheus-charting/ service-storage/service-credential-testing canonicals untouched. --- .../specs/service-instance-scoping/spec.md | 275 +++++++++++++++++ .../per-instance-hook-scoping/sync-report.md | 174 +++++++++++ .../specs/service-instance-scoping/spec.md | 276 ++++++++++++++++++ 3 files changed, 725 insertions(+) create mode 100644 openspec/changes/per-instance-hook-scoping/specs/service-instance-scoping/spec.md create mode 100644 openspec/changes/per-instance-hook-scoping/sync-report.md create mode 100644 openspec/specs/service-instance-scoping/spec.md diff --git a/openspec/changes/per-instance-hook-scoping/specs/service-instance-scoping/spec.md b/openspec/changes/per-instance-hook-scoping/specs/service-instance-scoping/spec.md new file mode 100644 index 0000000..f276bdc --- /dev/null +++ b/openspec/changes/per-instance-hook-scoping/specs/service-instance-scoping/spec.md @@ -0,0 +1,275 @@ +# Per-Instance Hook Scoping — Delta (`per-instance-hook-scoping`) + +> Change: `per-instance-hook-scoping` · Domain: `service-instance-scoping` · Phase: **spec** (reconciled during `sdd-sync`). +> Distilled verbatim from the verified flat `spec.md` (21 requirements, PI-101 … PI-121) of change +> `per-instance-hook-scoping`, cross-referenced against `design.md` and `verify-report.md`. Captures +> the **durable, post-change end-state contracts** for scoping the observability and backup +> service-type tabs/hooks to the operator-selected instance, instead of whichever instance resolves +> first globally. + +## ADDED Requirements + +> The canonical `openspec/specs/service-instance-scoping/spec.md` did not exist before this change. All +> requirements below are therefore **ADDED** to a new `service-instance-scoping` domain; `sdd-sync` +> copies them into the canonical spec (native helper rule: when the canonical spec does not exist, +> the change spec becomes the new canonical spec). +> +> Requirement IDs (PI-101 … PI-121) and body text are preserved **exactly** from the verified flat +> `spec.md`. Requirements are grouped logically and listed in the following group order: +> +> - **Frontend hooks (serviceId param + queryKey)** — PI-101 … PI-107 +> - **API client functions (serviceId → query param)** — PI-108 … PI-109 +> - **Backend backup endpoints (service_id filter)** — PI-110 … PI-112 +> - **Tabs pass instance.id (TODO comments removed)** — PI-113 … PI-116 +> - **Non-regression (dashboard widgets, global hooks untouched)** — PI-117 +> - **Backward compatibility** — PI-118 +> - **Test + build greenness** — PI-119 … PI-121 + +### Requirement: PI-101 — useAlertmanagerAlerts accepts serviceId + +The hook `useAlertmanagerAlerts` MUST accept an optional `serviceId?: string` argument. When provided, the hook MUST (a) include the `serviceId` in its TanStack Query `queryKey` (so caches do not collide across instances), and (b) pass the `serviceId` to its underlying fetch function. When omitted, the hook MUST behave identically to today (global/first-configured resolution). + +#### Scenario: per-instance cache key + +- GIVEN two configured Alertmanager instances A and B +- WHEN `useAlertmanagerAlerts("A")` and `useAlertmanagerAlerts("B")` are both mounted +- THEN their `queryKey`s MUST differ (the `serviceId` appears in the key) +- AND a cache hit for A MUST NOT be served for B (no stale cross-instance data) + +#### Scenario: omitted serviceId is backward-compatible + +- GIVEN a caller that does not pass `serviceId` +- WHEN the hook runs +- THEN its behavior MUST match the pre-change hook (queryKey shape equivalent, fetch path identical) + +### Requirement: PI-102 — useAlertmanagerStatus accepts serviceId + +The hook `useAlertmanagerStatus` MUST accept an optional `serviceId?: string`, include it in its `queryKey`, and pass it to its fetch function. Omitting it MUST preserve today's behavior. + +#### Scenario: per-instance cache key + +- GIVEN instances A and B +- WHEN `useAlertmanagerStatus("A")` and `useAlertmanagerStatus("B")` are mounted +- THEN the `queryKey`s MUST differ and a cache hit for A MUST NOT serve B + +### Requirement: PI-103 — usePrometheusStatus accepts serviceId + +The hook `usePrometheusStatus` MUST accept an optional `serviceId?: string`, include it in its `queryKey`, and pass it to its fetch function. Omitting it MUST preserve today's behavior. + +#### Scenario: per-instance cache key + +- GIVEN instances A and B +- WHEN `usePrometheusStatus("A")` and `usePrometheusStatus("B")` are mounted +- THEN the `queryKey`s MUST differ and a cache hit for A MUST NOT serve B + +### Requirement: PI-104 — useBackupJobs accepts serviceId + +The hook `useBackupJobs` MUST accept an optional `serviceId?: string`, include it in its `queryKey`, and pass it to its fetch function. Omitting it MUST preserve today's behavior. + +#### Scenario: per-instance cache key + +- GIVEN backup services A and B +- WHEN `useBackupJobs("A")` and `useBackupJobs("B")` are mounted +- THEN the `queryKey`s MUST differ and a cache hit for A MUST NOT serve B + +### Requirement: PI-105 — useBackupRuns accepts serviceId + +The hook `useBackupRuns(jobId, status, serviceId?)` MUST accept an optional `serviceId?: string` in addition to its existing `jobId` and `status` arguments, include it in its `queryKey`, and pass it to its fetch function. Omitting it MUST preserve today's behavior. + +#### Scenario: per-instance cache key + +- GIVEN backup services A and B +- WHEN `useBackupRuns("job1", "all", "A")` and `useBackupRuns("job1", "all", "B")` are mounted +- THEN the `queryKey`s MUST differ + +### Requirement: PI-106 — useBackupAlerts accepts serviceId + +The hook `useBackupAlerts(jobId, acknowledged, severity, serviceId?)` MUST accept an optional `serviceId?: string` in addition to its existing arguments, include it in its `queryKey`, and pass it to its fetch function. Omitting it MUST preserve today's behavior. + +#### Scenario: per-instance cache key + +- GIVEN backup services A and B +- WHEN `useBackupAlerts("job1", false, "all", "A")` and `useBackupAlerts("job1", false, "all", "B")` are mounted +- THEN the `queryKey`s MUST differ + +### Requirement: PI-107 — usePrometheusTargets and useMonitoringMachines stay global + +The hooks `usePrometheusTargets` and `useMonitoringMachines` MUST NOT gain a `serviceId` parameter. + +- `usePrometheusTargets` returns Node Exporter scrape targets for *external* Prometheus instances via `http_sd_configs` (cross-instance by design). +- `useMonitoringMachines` is a global cross-service concept (not per-instance). + +#### Scenario: global hooks unchanged + +- GIVEN the change is applied +- WHEN `usePrometheusTargets()` and `useMonitoringMachines()` signatures are inspected +- THEN neither accepts a `serviceId` arg and both behave identically to before + +### Requirement: PI-108 — Observability fetch functions pass service_id query param + +The fetch functions `fetchAlertmanagerAlerts`, `fetchAlertmanagerStatus`, and `fetchPrometheusStatus` (in `api/client.ts`) MUST each accept an optional `serviceId?: string`. When provided, the function MUST append `service_id=` to the request URL as a query parameter (correctly using `?` for the first param and `&` for subsequent ones). When omitted, the request MUST be identical to today (no `service_id` param sent). + +#### Scenario: serviceId appended + +- GIVEN `fetchPrometheusStatus("svc-abc")` is called +- WHEN the request is issued +- THEN the URL contains `service_id=svc-abc` + +#### Scenario: omitted serviceId is a no-op + +- GIVEN `fetchPrometheusStatus()` is called (no serviceId) +- WHEN the request is issued +- THEN the URL MUST NOT contain `service_id` + +### Requirement: PI-109 — Backup fetch functions pass service_id query param + +The fetch functions `fetchBackupJobs`, `fetchBackupRuns`, `fetchBackupAlerts`, and `fetchBackupDashboard` (in `api/backups.ts`) MUST each accept an optional `serviceId?: string`. When provided, the function MUST append `service_id=` to the request URL. When omitted, the request MUST be identical to today. + +#### Scenario: serviceId appended alongside existing params + +- GIVEN `fetchBackupRuns("job1", "failed", "svc-xyz")` is called +- WHEN the request is issued +- THEN the URL contains both `job_id=job1&status=failed` (existing params) AND `service_id=svc-xyz` + +#### Scenario: omitted serviceId is a no-op + +- GIVEN `fetchBackupJobs()` is called (no serviceId) +- WHEN the request is issued +- THEN the URL MUST NOT contain `service_id` + +### Requirement: PI-110 — Backup endpoints accept optional service_id + +The endpoints `get_backup_jobs`, `get_backup_runs`, `get_backup_alerts`, and `get_backup_dashboard` in `routers/backups.py` MUST each accept an optional `service_id: str | None = None` query parameter and thread it into the corresponding `SettingsStore` query method. + +#### Scenario: service_id passed through to store + +- GIVEN a request `GET /api/backups/jobs?service_id=svc-xyz` +- WHEN the endpoint runs +- THEN `store.list_backup_jobs` MUST be called with `service_id="svc-xyz"` + +### Requirement: PI-111 — SettingsStore backup queries filter by service_id when provided + +The methods `list_backup_jobs`, `list_backup_runs`, and `list_backup_alerts` in `services/settings_store.py` MUST each accept an optional `service_id: str | None` parameter. When `service_id` is a non-null, non-empty string, the query MUST include a `WHERE service_id = ?` clause (or equivalent scoping) returning only rows attributed to that service. When `service_id` is `None` or empty, the query MUST return ALL rows (no filter) — preserving backward-compatibility for existing callers. + +The `backup_jobs` and `backup_runs` tables already carry `service_id` attribution (from the `services-as-hub-ia` Slice 3 work); this requirement adds the filter, not the column. + +#### Scenario: filtered to one service + +- GIVEN the store holds jobs for services A and B +- WHEN `list_backup_jobs(service_id="A")` is called +- THEN only service A's jobs MUST be returned + +#### Scenario: no filter returns all + +- GIVEN the store holds jobs for services A and B +- WHEN `list_backup_jobs(service_id=None)` is called +- THEN jobs for BOTH services MUST be returned (backward-compat) + +#### Scenario: empty string is treated as no filter + +- GIVEN the store holds jobs for services A and B +- WHEN `list_backup_jobs(service_id="")` is called +- THEN jobs for BOTH services MUST be returned (defensive against falsy strings) + +### Requirement: PI-112 — Alertmanager/Prometheus status endpoints already accept service_id + +The endpoints `get_alertmanager_alerts`, `get_alertmanager_status`, and `get_prometheus_status` in `routers/monitoring.py` ALREADY accept a `service_id: str | None = None` query parameter and resolve via `resolve_service_record(store, "", service_id)`. No backend change is required for these. This requirement exists to document the asymmetry: the frontend wiring change for these hooks is necessary (PI-108 + PI-101..103), but the backend endpoint work is zero. + +#### Scenario: existing endpoint behavior confirmed + +- GIVEN the change is applied +- WHEN `get_alertmanager_alerts(service_id="svc-a")` is called +- THEN it MUST resolve to the `svc-a` Alertmanager instance (pre-existing behavior, unchanged) + +### Requirement: PI-113 — AlertsTab passes instance.id + +The component `pages/service-tabs/AlertsTab.tsx` MUST call `useAlertmanagerAlerts(instance.id)` and `useAlertmanagerStatus(instance.id)` (passing the `instance.id` it already receives as a prop). The existing TODO comment about "hooks don't accept a service_id yet" MUST be removed. + +#### Scenario: data is instance-scoped + +- GIVEN two Alertmanager instances A and B are configured +- WHEN the operator navigates to instance A's service page +- THEN AlertsTab MUST show alerts sourced from instance A (not B, not first-resolved) + +### Requirement: PI-114 — MetricsTab passes instance.id + +The component `pages/service-tabs/MetricsTab.tsx` MUST call `usePrometheusStatus(instance.id)` (passing the `instance.id` it already receives as a prop). The existing TODO comment MUST be removed. (`usePrometheusTargets` stays global — PI-107.) + +#### Scenario: data is instance-scoped + +- GIVEN two Prometheus instances A and B are configured +- WHEN the operator navigates to instance A's service page +- THEN MetricsTab MUST show status for instance A (not B, not first-resolved) + +### Requirement: PI-115 — JobsTab passes instance.id + +The component `pages/service-tabs/JobsTab.tsx` MUST call `useBackupJobs(instance.id)`, `useBackupRuns(, , instance.id)`, and `useBackupAlerts(, , , instance.id)`. The existing TODO comment MUST be removed. + +#### Scenario: data is instance-scoped + +- GIVEN two backups service instances A and B are configured +- WHEN the operator navigates to instance A's service page +- THEN JobsTab MUST show jobs/runs/alerts for service A only + +### Requirement: PI-116 — Instance switcher re-scopes tab data + +The `ServicePage` instance switcher (present when `enabledSiblings.length > 1`) changes the active instance. When the operator switches from instance A to instance B, the three tabs MUST re-fetch and re-render with instance B's data. (This falls out naturally from passing `instance.id` into the hook `queryKey`s — switching the prop changes the key, triggering a fresh fetch — but it is verified explicitly because it's the user-visible correctness goal.) + +#### Scenario: switch re-scopes + +- GIVEN the operator is viewing instance A's MetricsTab (data from A) +- WHEN the operator uses the instance switcher to select instance B +- THEN the tab MUST re-render with instance B's status (not A's cached data) + +### Requirement: PI-117 — Dashboard widgets render identically before/after + +Dashboard widgets (`AlertmanagerAlertsWidget`, `BackupsWidget`, `PrometheusMetricWidget`/`Metric*`, etc.) resolve their service via the widget-instance's `service_id` binding through `useWidgetData` — a different code path from the hooks changed here. They MUST render identically before and after this change. The widget component tests MUST remain unchanged and green. + +#### Scenario: widget path unaffected + +- GIVEN the change is applied +- WHEN a dashboard widget fetches its data +- THEN the fetch path (`useWidgetData(widget.id, …)`) MUST be unchanged; no `useAlertmanager*` / `useBackup*` hook is on the widget data path + +### Requirement: PI-118 — All new params are optional; omitting equals today's behavior + +Every new parameter added by this change (hook `serviceId?: string`, fetch-function `serviceId?: string`, endpoint `service_id: str | None = None`, store-method `service_id: str | None = None`) MUST be optional with a default that preserves today's behavior. No existing caller is forced to pass the new argument. Existing callers that omit it MUST see identical results to the pre-change code. + +#### Scenario: existing callers unbroken + +- GIVEN any pre-existing caller of `useBackupJobs()`, `fetchBackupRuns()`, `GET /api/backups/jobs`, or `store.list_backup_jobs()` +- WHEN the change is applied and the caller is run without modification +- THEN the behavior MUST be identical to before (no error, same data returned) + +### Requirement: PI-119 — Backend tests + lint green + +`PYTHONPATH=src python3 -m pytest -q` and `PYTHONPATH=src python3 -m ruff check src tests` from `backend/` MUST pass. New backend tests MUST cover: `list_backup_*` filtering by `service_id` (filtered, unfiltered, empty-string); backup endpoint threading `service_id` into the store. + +#### Scenario: backup filter tests exist + +- GIVEN the change is applied +- WHEN `pytest tests/test_backups.py` (or equivalent) runs +- THEN it MUST include a test asserting `service_id="A"` returns only A's rows AND `service_id=None` returns all rows + +### Requirement: PI-120 — Frontend typechecks, builds, and lints + +`npm run build` (which runs `tsc -b` + `vite build`) and `npm run lint` from `frontend/` MUST pass (0 errors; pre-existing warnings are acceptable). + +### Requirement: PI-121 — Hook + tab tests cover the new serviceId wiring + +The frontend test suite MUST include: + +- A hook test asserting the `serviceId` appears in the `queryKey` (per-instance cache isolation) for at least one representative hook (e.g. `useBackupJobs`). +- A tab test asserting `instance.id` is passed through to the hook (for at least one of AlertsTab / MetricsTab / JobsTab). + +#### Scenario: per-instance queryKey test exists + +- GIVEN the change is applied +- WHEN the test suite runs +- THEN a test MUST assert that calling the hook with different `serviceId` values produces different `queryKey`s + +#### Scenario: tab passes instance.id test exists + +- GIVEN the change is applied +- WHEN the test suite runs +- THEN a test MUST assert that at least one tab component passes its received `instance.id` to the scoped hook diff --git a/openspec/changes/per-instance-hook-scoping/sync-report.md b/openspec/changes/per-instance-hook-scoping/sync-report.md new file mode 100644 index 0000000..2dd53ae --- /dev/null +++ b/openspec/changes/per-instance-hook-scoping/sync-report.md @@ -0,0 +1,174 @@ +# Sync Report — `per-instance-hook-scoping` + +> Phase: **sync** · Change: `per-instance-hook-scoping` · Repo: `/home/user/manage` +> Mode: file-backed (`artifactStore: openspec`). No source-code edits; only OpenSpec artifacts were +> written. Not committed (parent owns the commit). The change folder was **not** moved (that is +> `sdd-archive`'s job). The flat `spec.md` was left in place as the authoritative planning artifact. + +**Status: SYNCED.** A new canonical domain `openspec/specs/service-instance-scoping/spec.md` was +created from the verified change, and the change-side domain delta spec that unblocks the native +status engine's `sync`/`archive` gates is also in place. + +--- + +## 1. Executive summary + +The `per-instance-hook-scoping` change shipped a **complete but flat** `openspec/changes/per-instance-hook-scoping/spec.md` +(21 requirements, PI-101 … PI-121) with **no** per-domain delta spec under +`openspec/changes/per-instance-hook-scoping/specs//`. `sdd-sync` requires a domain delta +spec; the flat spec alone does not satisfy the canonical-merge contract (native status engine +reported `specs: partial`, blockedReasons: "domain specs are missing or partial" + "Legacy flat spec +is present without domain specs"). + +Verify already returned **PASS** (verdict in `verify-report.md`; all five gates green — backend +`pytest` 368 passed, `ruff` clean, frontend `npm run build` exit 0, `npm run lint` 0 errors, +`npx vitest run` 46 files / 165 tests passed). Functional coverage was **21/21 requirements satisfied +against source**. The verify report's single CRITICAL was an **archive** blocker (17 unchecked task +checkboxes + missing `apply-progress.md`); per the dispatch brief, `apply-progress.md` has since been +reconciled and that condition does **not** block `sdd-sync` of the green code. + +This sync **reconciles** the flat-spec-vs-domain-spec gap: + +1. Authored the missing **change-side domain delta spec** — + `openspec/changes/per-instance-hook-scoping/specs/service-instance-scoping/spec.md` — using a pure + `## ADDED Requirements` structure that preserves the exact requirement IDs (PI-101 … PI-121) and + body text (including the per-requirement GIVEN/WHEN/THEN scenarios) from the verified flat + `spec.md`. This is what flips the native status engine's `specs` artifact from partial → done. +2. **Synced** the end-state into the **canonical store** — + `openspec/specs/service-instance-scoping/spec.md` — the actual sync target. Because the canonical + `service-instance-scoping` domain did not previously exist, the native helper rule applies: *when + the canonical spec does not exist, the change spec becomes the new canonical spec.* The two files + carry identical requirement bodies (delta under `## ADDED Requirements`; canonical under + `## Requirements`), verified identical for the requirement region. + +Domain name **`service-instance-scoping`** was chosen (per the dispatch brief) because it names the +cross-cutting concern the change establishes: observability and backup service-type tabs/hooks must +scope to the operator-selected instance. It is distinct from the existing canonical domains +(`web-ui`, `prometheus-charting`, `service-storage`, `service-credential-testing`), none of which +were touched. + +## 2. Structured status & actionContext findings + +The native `gentle-pi.sdd-status` reports `state: blocked`, `artifactStore: openspec`, +`taskProgress: 17/17 complete`, `applyState: blocked`, and `blockedReasons` citing missing domain +specs + legacy flat spec present without domain specs. These blockers are **artifact-hygiene, not +implementation gaps**: the code is committed (`3bc7ce5`), all five gates are green, and 21/21 +requirements pass against source. This sync resolves the missing-domain-spec condition; the +remaining `applyState`/checkbox hygiene is an **archive**-phase concern. + +- `artifactStore: openspec`; change root `openspec/changes/per-instance-hook-scoping/`. +- Artifacts present: `proposal.md`, `spec.md` (flat), `design.md`, `tasks.md`, `verify-report.md`, + `apply-progress.md`. +- `verify: PASS` (verify-report verdict; gates green at `3bc7ce5`). +- `actionContext`: `mode: repo-local`, `workspaceRoot: /home/user/manage`, + `allowedEditRoots: ["/home/user/manage"]`, `warnings: []`. All three files written are inside the + authoritative workspace / allowed edit roots. ✓ +- `relationships.sameDomainActiveChanges: []`, `collisions: []` — **no active same-domain + collisions**, so no archive/sync ordering decision was required. +- The new `service-instance-scoping` domain is distinct from the existing `web-ui`, + `prometheus-charting`, `service-storage`, and `service-credential-testing` canonical domains; all + four were left untouched. + +**Post-sync structural change:** `openspec/changes/per-instance-hook-scoping/specs/service-instance-scoping/spec.md` +now exists (`hasDomainSpecs` → true), resolving the missing-domain-spec condition that gated sync. +The flat `spec.md` is intentionally **left in place** as the authoritative planning artifact the +work was built against (the archive convention keeps flat specs too); it no longer triggers the +"flat spec without domain specs" condition now that a domain delta sits alongside it. + +## 3. Domains synced & canonical files updated + +| Domain | Change-side delta (source) | Canonical (sync target) | Action | +|---|---|---|---| +| `service-instance-scoping` | `openspec/changes/per-instance-hook-scoping/specs/service-instance-scoping/spec.md` | `openspec/specs/service-instance-scoping/spec.md` | **NEW domain** — `## ADDED Requirements` copied into canonical as a new spec | + +- **Canonical file created:** `openspec/specs/service-instance-scoping/spec.md` (21 requirements). +- **Change-side delta created:** `openspec/changes/per-instance-hook-scoping/specs/service-instance-scoping/spec.md` + (21 requirements, all `## ADDED Requirements`). + +## 4. Requirement delta (ADDED / MODIFIED / REMOVED) + +- **ADDED (21)** — all to the new `service-instance-scoping` domain (canonical did not exist + pre-change). IDs and text preserved verbatim from the verified flat `spec.md`. Grouped logically: + - *Frontend hooks (serviceId param + queryKey)* — PI-101, PI-102, PI-103, PI-104, PI-105, PI-106, PI-107 + - *API client functions (serviceId → query param)* — PI-108, PI-109 + - *Backend backup endpoints (service_id filter)* — PI-110, PI-111, PI-112 + - *Tabs pass instance.id (TODO comments removed)* — PI-113, PI-114, PI-115, PI-116 + - *Non-regression (dashboard widgets, global hooks untouched)* — PI-117 + - *Backward compatibility* — PI-118 + - *Test + build greenness* — PI-119, PI-120, PI-121 +- **MODIFIED (0)** — none (new domain; no pre-existing canonical requirements to replace). +- **REMOVED (0)** — none. +- **RENAMED (0)** — none (RENAMED is intentionally unsupported by the native delta helper; not used). + +## 5. Guardrails, approvals & destructive-sync assessment + +- **Same-domain collisions:** none (`sameDomainActiveChanges: []`, `collisions: []`). The new + `service-instance-scoping` domain does not overlap the existing `web-ui`, `prometheus-charting`, + `service-storage`, or `service-credential-testing` canonical domains. No ordering decision was + needed. +- **Destructive sync:** **not applicable.** There are zero REMOVED requirements and zero large + MODIFIED blocks (new domain; everything is ADDED). No destructive-sync parent approval was + required for this sync beyond the explicit reconciliation instruction in the dispatch brief. +- **Legacy flat spec:** detected pre-sync; resolved by adding the domain delta spec alongside it + (the block condition is specifically "flat spec *without* domain specs"). The flat spec was left + in place as a planning artifact. +- **Canonical isolation:** the existing `openspec/specs/web-ui/spec.md`, `openspec/specs/prometheus-charting/spec.md`, + `openspec/specs/service-storage/spec.md`, and `openspec/specs/service-credential-testing/spec.md` + were **not modified** — verified untouched (see §6). + +## 6. Validation / checks performed (file-backed, read-only) + +Run from `/home/user/manage` (no source edits, no test re-runs — those are owned by verify and were +already green at `3bc7ce5`): + +| Check | Command | Result | +|---|---|---| +| Canonical store populated | `ls openspec/specs/service-instance-scoping/spec.md` | present ✓ | +| Change-side domain spec present | `ls openspec/changes/per-instance-hook-scoping/specs/service-instance-scoping/spec.md` | present ✓ | +| Requirement-ID parity (flat ↔ delta ↔ canonical) | `grep -oE 'PI-[0-9]+'` all three files, `sort -u` | **21 == 21 == 21**, identical IDs PI-101…PI-121 ✓ | +| Body-text parity (delta ↔ canonical) | `diff` of the `### Requirement:` region of both files | **identical** ✓ | +| Delta is pure ADDED | count `## ADDED/MODIFIED/REMOVED/RENAMED Requirements` | ADDED=1, MODIFIED=0, REMOVED=0, RENAMED=0 ✓ (no destructive sync) | +| Other canonicals untouched | `git status --porcelain openspec/specs/{web-ui,prometheus-charting,service-storage,service-credential-testing}/` | empty (not modified) ✓ | +| No edits outside openspec | `git status --porcelain` (filtered) | only the three new OpenSpec files added; no source files touched ✓ | +| Markdown validity | write-time lint | all three files "Markdown clean" ✓ | + +## 7. Carry-over items for the archive summary + +These verify-phase findings are non-blocking for sync and should land in the archive summary: + +1. **[CRITICAL-process, archive-only] Task-checkbox / apply-progress hygiene.** At verify time, 17 + implementation task checkboxes were unchecked and `apply-progress.md` was missing. Per the + dispatch brief, `apply-progress.md` has been reconciled (native status now reports 17/17 + complete); `sdd-archive` should re-scan the native status engine to confirm `apply: done` / + `applyProgress: present` and tick any remaining unchecked boxes before moving the change to + archive. +2. **[INFO, NB-1] Spec↔design wording mismatch on PI-109 / PI-110.** The flat spec literally lists + `fetchBackupDashboard` / `get_backup_dashboard` as in-scope for a new service param, while design + decision 5 and tasks 1.5/1.9 deliberately exclude them (dashboard widget path; PI-117 risk). The + implementation correctly follows the design. This delta/canonical preserved the flat spec text + **verbatim** (PI-109 / PI-110 still mention the dashboard variants) per the sync fidelity rule + (do not rewrite verified requirements during sync). If `sdd-archive` wants spec and design to + agree, it can amend in a follow-up change; no functional defect. +3. **[INFO, NB-2] Weak hook-test assertion (PI-121).** The `useBackups.test.ts` "different keys" + assertion (`expect(a).not.toBe(b)` on `renderHook` result refs) is tautology-prone. The wiring is + correct in source and the tab test is strong; recommend strengthening the hook test in a future + pass. Non-blocking. +4. **[INFO] Stale generated `.pi-map.md`** files still reference the pre-change signatures + (e.g. `list_backup_jobs(self)` without `service_id`). Generated artifacts, not deliverable source; + reconcile via `project_map_patch` / `project_map_validate` when convenient. + +## 8. Next recommended phase + +→ **`sdd-archive`** (clean). Confirm the native status re-scan reports `specs: done` / `sync: ready` +/ `archive: ready`, then move the change to +`openspec/changes/archive/YYYY-MM-DD-per-instance-hook-scoping`, carrying over the items in §7 into +the archive summary. Do **not** commit or push — the parent owns the commit with explicit paths. + +--- + +### Appendix — Files written by this sync (OpenSpec only; no source code) + +- `openspec/changes/per-instance-hook-scoping/specs/service-instance-scoping/spec.md` — **change-side + domain delta (`## ADDED Requirements`), 21 requirements PI-101…PI-121.** +- `openspec/specs/service-instance-scoping/spec.md` — **canonical spec (new domain), 21 requirements.** +- `openspec/changes/per-instance-hook-scoping/sync-report.md` — this report. diff --git a/openspec/specs/service-instance-scoping/spec.md b/openspec/specs/service-instance-scoping/spec.md new file mode 100644 index 0000000..2574f68 --- /dev/null +++ b/openspec/specs/service-instance-scoping/spec.md @@ -0,0 +1,276 @@ +# Service Instance Scoping + +> Domain: `service-instance-scoping` · **Canonical specification.** Created by `per-instance-hook-scoping`. +> +> This is the merged end-state of Manage's per-instance hook-scoping fix: when multiple instances of +> Alertmanager, Prometheus, or the backups service are configured, the service-type tabs (`AlertsTab`, +> `MetricsTab`, `JobsTab`) and their backing hooks/fetch functions MUST scope to the instance whose +> page the operator is viewing — not whichever instance resolves first globally. New parameters are +> optional everywhere, preserving backward compatibility for all pre-existing callers, and dashboard +> widgets (which resolve service via a separate `useWidgetData` path) are explicitly untouched. This +> spec is acceptance-focused and verifiable; it deliberately does not prescribe implementation. + +## Purpose + +Define WHAT must be true of Manage's observability and backup service-type tab/hook scoping after the +`per-instance-hook-scoping` change: the frontend TanStack Query hooks (`useAlertmanagerAlerts`, +`useAlertmanagerStatus`, `usePrometheusStatus`, `useBackupJobs`, `useBackupRuns`, `useBackupAlerts`) +MUST accept an optional `serviceId`, fold it into their `queryKey` (so per-instance caches never +collide), and pass it to their fetch functions; the API client functions MUST append a `service_id` +query param when provided; the backup endpoints and `SettingsStore` query methods MUST filter by +`service_id` when a non-empty value is given and return all rows when omitted; the three tabs MUST +pass their received `instance.id` into the scoped hooks (and drop stale TODO comments); the instance +switcher MUST re-scope on switch. `usePrometheusTargets` and `useMonitoringMachines` stay global by +design, and every new parameter is optional with a default that preserves today's behavior. This spec +is acceptance-focused and verifiable; it deliberately does not prescribe implementation. + +## Requirements + +### Requirement: PI-101 — useAlertmanagerAlerts accepts serviceId + +The hook `useAlertmanagerAlerts` MUST accept an optional `serviceId?: string` argument. When provided, the hook MUST (a) include the `serviceId` in its TanStack Query `queryKey` (so caches do not collide across instances), and (b) pass the `serviceId` to its underlying fetch function. When omitted, the hook MUST behave identically to today (global/first-configured resolution). + +#### Scenario: per-instance cache key + +- GIVEN two configured Alertmanager instances A and B +- WHEN `useAlertmanagerAlerts("A")` and `useAlertmanagerAlerts("B")` are both mounted +- THEN their `queryKey`s MUST differ (the `serviceId` appears in the key) +- AND a cache hit for A MUST NOT be served for B (no stale cross-instance data) + +#### Scenario: omitted serviceId is backward-compatible + +- GIVEN a caller that does not pass `serviceId` +- WHEN the hook runs +- THEN its behavior MUST match the pre-change hook (queryKey shape equivalent, fetch path identical) + +### Requirement: PI-102 — useAlertmanagerStatus accepts serviceId + +The hook `useAlertmanagerStatus` MUST accept an optional `serviceId?: string`, include it in its `queryKey`, and pass it to its fetch function. Omitting it MUST preserve today's behavior. + +#### Scenario: per-instance cache key + +- GIVEN instances A and B +- WHEN `useAlertmanagerStatus("A")` and `useAlertmanagerStatus("B")` are mounted +- THEN the `queryKey`s MUST differ and a cache hit for A MUST NOT serve B + +### Requirement: PI-103 — usePrometheusStatus accepts serviceId + +The hook `usePrometheusStatus` MUST accept an optional `serviceId?: string`, include it in its `queryKey`, and pass it to its fetch function. Omitting it MUST preserve today's behavior. + +#### Scenario: per-instance cache key + +- GIVEN instances A and B +- WHEN `usePrometheusStatus("A")` and `usePrometheusStatus("B")` are mounted +- THEN the `queryKey`s MUST differ and a cache hit for A MUST NOT serve B + +### Requirement: PI-104 — useBackupJobs accepts serviceId + +The hook `useBackupJobs` MUST accept an optional `serviceId?: string`, include it in its `queryKey`, and pass it to its fetch function. Omitting it MUST preserve today's behavior. + +#### Scenario: per-instance cache key + +- GIVEN backup services A and B +- WHEN `useBackupJobs("A")` and `useBackupJobs("B")` are mounted +- THEN the `queryKey`s MUST differ and a cache hit for A MUST NOT serve B + +### Requirement: PI-105 — useBackupRuns accepts serviceId + +The hook `useBackupRuns(jobId, status, serviceId?)` MUST accept an optional `serviceId?: string` in addition to its existing `jobId` and `status` arguments, include it in its `queryKey`, and pass it to its fetch function. Omitting it MUST preserve today's behavior. + +#### Scenario: per-instance cache key + +- GIVEN backup services A and B +- WHEN `useBackupRuns("job1", "all", "A")` and `useBackupRuns("job1", "all", "B")` are mounted +- THEN the `queryKey`s MUST differ + +### Requirement: PI-106 — useBackupAlerts accepts serviceId + +The hook `useBackupAlerts(jobId, acknowledged, severity, serviceId?)` MUST accept an optional `serviceId?: string` in addition to its existing arguments, include it in its `queryKey`, and pass it to its fetch function. Omitting it MUST preserve today's behavior. + +#### Scenario: per-instance cache key + +- GIVEN backup services A and B +- WHEN `useBackupAlerts("job1", false, "all", "A")` and `useBackupAlerts("job1", false, "all", "B")` are mounted +- THEN the `queryKey`s MUST differ + +### Requirement: PI-107 — usePrometheusTargets and useMonitoringMachines stay global + +The hooks `usePrometheusTargets` and `useMonitoringMachines` MUST NOT gain a `serviceId` parameter. + +- `usePrometheusTargets` returns Node Exporter scrape targets for *external* Prometheus instances via `http_sd_configs` (cross-instance by design). +- `useMonitoringMachines` is a global cross-service concept (not per-instance). + +#### Scenario: global hooks unchanged + +- GIVEN the change is applied +- WHEN `usePrometheusTargets()` and `useMonitoringMachines()` signatures are inspected +- THEN neither accepts a `serviceId` arg and both behave identically to before + +### Requirement: PI-108 — Observability fetch functions pass service_id query param + +The fetch functions `fetchAlertmanagerAlerts`, `fetchAlertmanagerStatus`, and `fetchPrometheusStatus` (in `api/client.ts`) MUST each accept an optional `serviceId?: string`. When provided, the function MUST append `service_id=` to the request URL as a query parameter (correctly using `?` for the first param and `&` for subsequent ones). When omitted, the request MUST be identical to today (no `service_id` param sent). + +#### Scenario: serviceId appended + +- GIVEN `fetchPrometheusStatus("svc-abc")` is called +- WHEN the request is issued +- THEN the URL contains `service_id=svc-abc` + +#### Scenario: omitted serviceId is a no-op + +- GIVEN `fetchPrometheusStatus()` is called (no serviceId) +- WHEN the request is issued +- THEN the URL MUST NOT contain `service_id` + +### Requirement: PI-109 — Backup fetch functions pass service_id query param + +The fetch functions `fetchBackupJobs`, `fetchBackupRuns`, `fetchBackupAlerts`, and `fetchBackupDashboard` (in `api/backups.ts`) MUST each accept an optional `serviceId?: string`. When provided, the function MUST append `service_id=` to the request URL. When omitted, the request MUST be identical to today. + +#### Scenario: serviceId appended alongside existing params + +- GIVEN `fetchBackupRuns("job1", "failed", "svc-xyz")` is called +- WHEN the request is issued +- THEN the URL contains both `job_id=job1&status=failed` (existing params) AND `service_id=svc-xyz` + +#### Scenario: omitted serviceId is a no-op + +- GIVEN `fetchBackupJobs()` is called (no serviceId) +- WHEN the request is issued +- THEN the URL MUST NOT contain `service_id` + +### Requirement: PI-110 — Backup endpoints accept optional service_id + +The endpoints `get_backup_jobs`, `get_backup_runs`, `get_backup_alerts`, and `get_backup_dashboard` in `routers/backups.py` MUST each accept an optional `service_id: str | None = None` query parameter and thread it into the corresponding `SettingsStore` query method. + +#### Scenario: service_id passed through to store + +- GIVEN a request `GET /api/backups/jobs?service_id=svc-xyz` +- WHEN the endpoint runs +- THEN `store.list_backup_jobs` MUST be called with `service_id="svc-xyz"` + +### Requirement: PI-111 — SettingsStore backup queries filter by service_id when provided + +The methods `list_backup_jobs`, `list_backup_runs`, and `list_backup_alerts` in `services/settings_store.py` MUST each accept an optional `service_id: str | None` parameter. When `service_id` is a non-null, non-empty string, the query MUST include a `WHERE service_id = ?` clause (or equivalent scoping) returning only rows attributed to that service. When `service_id` is `None` or empty, the query MUST return ALL rows (no filter) — preserving backward-compatibility for existing callers. + +The `backup_jobs` and `backup_runs` tables already carry `service_id` attribution (from the `services-as-hub-ia` Slice 3 work); this requirement adds the filter, not the column. + +#### Scenario: filtered to one service + +- GIVEN the store holds jobs for services A and B +- WHEN `list_backup_jobs(service_id="A")` is called +- THEN only service A's jobs MUST be returned + +#### Scenario: no filter returns all + +- GIVEN the store holds jobs for services A and B +- WHEN `list_backup_jobs(service_id=None)` is called +- THEN jobs for BOTH services MUST be returned (backward-compat) + +#### Scenario: empty string is treated as no filter + +- GIVEN the store holds jobs for services A and B +- WHEN `list_backup_jobs(service_id="")` is called +- THEN jobs for BOTH services MUST be returned (defensive against falsy strings) + +### Requirement: PI-112 — Alertmanager/Prometheus status endpoints already accept service_id + +The endpoints `get_alertmanager_alerts`, `get_alertmanager_status`, and `get_prometheus_status` in `routers/monitoring.py` ALREADY accept a `service_id: str | None = None` query parameter and resolve via `resolve_service_record(store, "", service_id)`. No backend change is required for these. This requirement exists to document the asymmetry: the frontend wiring change for these hooks is necessary (PI-108 + PI-101..103), but the backend endpoint work is zero. + +#### Scenario: existing endpoint behavior confirmed + +- GIVEN the change is applied +- WHEN `get_alertmanager_alerts(service_id="svc-a")` is called +- THEN it MUST resolve to the `svc-a` Alertmanager instance (pre-existing behavior, unchanged) + +### Requirement: PI-113 — AlertsTab passes instance.id + +The component `pages/service-tabs/AlertsTab.tsx` MUST call `useAlertmanagerAlerts(instance.id)` and `useAlertmanagerStatus(instance.id)` (passing the `instance.id` it already receives as a prop). The existing TODO comment about "hooks don't accept a service_id yet" MUST be removed. + +#### Scenario: data is instance-scoped + +- GIVEN two Alertmanager instances A and B are configured +- WHEN the operator navigates to instance A's service page +- THEN AlertsTab MUST show alerts sourced from instance A (not B, not first-resolved) + +### Requirement: PI-114 — MetricsTab passes instance.id + +The component `pages/service-tabs/MetricsTab.tsx` MUST call `usePrometheusStatus(instance.id)` (passing the `instance.id` it already receives as a prop). The existing TODO comment MUST be removed. (`usePrometheusTargets` stays global — PI-107.) + +#### Scenario: data is instance-scoped + +- GIVEN two Prometheus instances A and B are configured +- WHEN the operator navigates to instance A's service page +- THEN MetricsTab MUST show status for instance A (not B, not first-resolved) + +### Requirement: PI-115 — JobsTab passes instance.id + +The component `pages/service-tabs/JobsTab.tsx` MUST call `useBackupJobs(instance.id)`, `useBackupRuns(, , instance.id)`, and `useBackupAlerts(, , , instance.id)`. The existing TODO comment MUST be removed. + +#### Scenario: data is instance-scoped + +- GIVEN two backups service instances A and B are configured +- WHEN the operator navigates to instance A's service page +- THEN JobsTab MUST show jobs/runs/alerts for service A only + +### Requirement: PI-116 — Instance switcher re-scopes tab data + +The `ServicePage` instance switcher (present when `enabledSiblings.length > 1`) changes the active instance. When the operator switches from instance A to instance B, the three tabs MUST re-fetch and re-render with instance B's data. (This falls out naturally from passing `instance.id` into the hook `queryKey`s — switching the prop changes the key, triggering a fresh fetch — but it is verified explicitly because it's the user-visible correctness goal.) + +#### Scenario: switch re-scopes + +- GIVEN the operator is viewing instance A's MetricsTab (data from A) +- WHEN the operator uses the instance switcher to select instance B +- THEN the tab MUST re-render with instance B's status (not A's cached data) + +### Requirement: PI-117 — Dashboard widgets render identically before/after + +Dashboard widgets (`AlertmanagerAlertsWidget`, `BackupsWidget`, `PrometheusMetricWidget`/`Metric*`, etc.) resolve their service via the widget-instance's `service_id` binding through `useWidgetData` — a different code path from the hooks changed here. They MUST render identically before and after this change. The widget component tests MUST remain unchanged and green. + +#### Scenario: widget path unaffected + +- GIVEN the change is applied +- WHEN a dashboard widget fetches its data +- THEN the fetch path (`useWidgetData(widget.id, …)`) MUST be unchanged; no `useAlertmanager*` / `useBackup*` hook is on the widget data path + +### Requirement: PI-118 — All new params are optional; omitting equals today's behavior + +Every new parameter added by this change (hook `serviceId?: string`, fetch-function `serviceId?: string`, endpoint `service_id: str | None = None`, store-method `service_id: str | None = None`) MUST be optional with a default that preserves today's behavior. No existing caller is forced to pass the new argument. Existing callers that omit it MUST see identical results to the pre-change code. + +#### Scenario: existing callers unbroken + +- GIVEN any pre-existing caller of `useBackupJobs()`, `fetchBackupRuns()`, `GET /api/backups/jobs`, or `store.list_backup_jobs()` +- WHEN the change is applied and the caller is run without modification +- THEN the behavior MUST be identical to before (no error, same data returned) + +### Requirement: PI-119 — Backend tests + lint green + +`PYTHONPATH=src python3 -m pytest -q` and `PYTHONPATH=src python3 -m ruff check src tests` from `backend/` MUST pass. New backend tests MUST cover: `list_backup_*` filtering by `service_id` (filtered, unfiltered, empty-string); backup endpoint threading `service_id` into the store. + +#### Scenario: backup filter tests exist + +- GIVEN the change is applied +- WHEN `pytest tests/test_backups.py` (or equivalent) runs +- THEN it MUST include a test asserting `service_id="A"` returns only A's rows AND `service_id=None` returns all rows + +### Requirement: PI-120 — Frontend typechecks, builds, and lints + +`npm run build` (which runs `tsc -b` + `vite build`) and `npm run lint` from `frontend/` MUST pass (0 errors; pre-existing warnings are acceptable). + +### Requirement: PI-121 — Hook + tab tests cover the new serviceId wiring + +The frontend test suite MUST include: + +- A hook test asserting the `serviceId` appears in the `queryKey` (per-instance cache isolation) for at least one representative hook (e.g. `useBackupJobs`). +- A tab test asserting `instance.id` is passed through to the hook (for at least one of AlertsTab / MetricsTab / JobsTab). + +#### Scenario: per-instance queryKey test exists + +- GIVEN the change is applied +- WHEN the test suite runs +- THEN a test MUST assert that calling the hook with different `serviceId` values produces different `queryKey`s + +#### Scenario: tab passes instance.id test exists + +- GIVEN the change is applied +- WHEN the test suite runs +- THEN a test MUST assert that at least one tab component passes its received `instance.id` to the scoped hook