From 9459de5c07ccd78e189a042416a4d8b85f5264d0 Mon Sep 17 00:00:00 2001 From: Developer Date: Mon, 22 Jun 2026 11:14:01 +0000 Subject: [PATCH] docs(service-registry): lock decisions (cascade delete, required key, SSH runner model) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - §11 decisions: cascade-delete services with widgets; MANAGE_ENCRYPTION_KEY always required; SSH task runner is multi-instance with reusable tasks. - §12 SSH task runner model: instances absorb SSH task transport, tasks stay global/reusable with default_service_id, service_task_runs logs history, widget config { task_id, service_id? }. - tasks.md: add service_task_runs table + cascade-delete tests to Slice 1, SSH run-logging to Slice 2, follow-ups (Actions rebuild, machine unification). --- openspec/changes/service-registry/design.md | 87 ++++++++++++++++++--- openspec/changes/service-registry/tasks.md | 30 +++++-- 2 files changed, 99 insertions(+), 18 deletions(-) diff --git a/openspec/changes/service-registry/design.md b/openspec/changes/service-registry/design.md index 89a1486..ac99d3c 100644 --- a/openspec/changes/service-registry/design.md +++ b/openspec/changes/service-registry/design.md @@ -222,7 +222,7 @@ def decrypt_secrets(blob: dict[str, str]) -> dict[str, str]: ... | GET | `/api/services` | List service instances (no plaintext secrets; only "set" flags). | | POST | `/api/services` | Create instance (validates type, config, secret schema). | | PUT | `/api/services/{id}` | Update instance. | -| DELETE | `/api/services/{id}` | Delete instance (and refuse while widgets reference it, or cascade). | +| DELETE | `/api/services/{id}` | Delete instance; **cascade-deletes** widgets referencing it in the same transaction. | ### Widgets (unchanged paths, new semantics) @@ -341,13 +341,80 @@ Each slice keeps `pytest`, `ruff`, `npm run lint`, and `npm run build` green. Estimated total: ~2,000–2,400 changed lines across four PRs. -## 11. Open questions to resolve before apply +## 11. Decisions resolved -1. Should deleting a service that still has widgets block (return 409) or cascade-delete - the widgets? Recommend **block with 409** and require the user to remove widgets first. -2. Should `MANAGE_ENCRYPTION_KEY` have a development default (e.g. derived from a fixed - dev key when `AUTH_ENABLED=false`)? Recommend **no** — require it always to avoid - accidental plaintext in dev. -3. Does the SSH task runner service hold the SSH key reference, or does it reference a - machine? Recommend the service record holds `machine_id` (transport) + optional - task-scoped overrides; the saved-task registry stays unchanged. +1. **Deleting a service that still has widgets** → **cascade delete.** The store deletes + every `dashboard_widgets` row referencing the service inside the same transaction as + the service delete. Simple and safe in SQLite; no 409 pre-check. +2. **`MANAGE_ENCRYPTION_KEY` dev default** → **always required.** No fallback, even when + `AUTH_ENABLED=false`. Startup fails fast if it is missing or not a valid Fernet key. +3. **SSH task runner shape** → **multi-instance, reusable tasks, persisted run history.** + See §12 for the full model. + +## 12. SSH task runner model + +The SSH task runner is the most involved service type. Instances absorb the SSH task +execution role currently held by machines; tasks stay global and reusable; every +invocation is logged. + +### 12.1 Instances + +- `service_type = "ssh_tasks"`. +- Each instance is an SSH endpoint: `host`, `port`, `username`, `ssh_key_id`, optional + `passphrase`. Connection config lives on the service record; the SSH key itself stays + in the existing saved-key registry (referenced by `ssh_key_id`). +- Multi-instance by design ("home server", "media box", …). + +### 12.2 Tasks (global, reusable) + +- Saved tasks remain a **global** registry (`name`, `task_type` shell/python, `content`, + `enabled`). A task is **not** owned by an instance. +- Each task gains `default_service_id` (replaces the old `default_machine_id`) — the + instance it targets by default. At run time the caller may override the target + instance. +- A task can therefore run against any instance; the link is captured per-run. + +### 12.3 Run history (logs) + +A new `service_task_runs` table records every invocation: + +```sql +CREATE TABLE IF NOT EXISTS service_task_runs ( + id TEXT PRIMARY KEY, + task_id TEXT NOT NULL, + service_id TEXT NOT NULL, + status TEXT NOT NULL, -- success | failure | timeout | error + exit_status INTEGER, + duration_ms INTEGER, + stdout_tail TEXT, + stderr_tail TEXT, + error TEXT, + created_at INTEGER NOT NULL +); +CREATE INDEX IF NOT EXISTS idx_service_task_runs_service ON service_task_runs(service_id, created_at DESC); +CREATE INDEX IF NOT EXISTS idx_service_task_runs_task ON service_task_runs(task_id, created_at DESC); +``` + +- Populated by the SSH task adapter on every widget data fetch and by the Actions + runner on manual runs. +- Surfaced on the instance's service page as a log/history list, and on the task detail + as recent runs. +- Replaces the legacy `saved_task_runs` concept once the Actions page is rebuilt on + services (Slice 4 / a follow-up). + +### 12.4 SSH task widget + +Widget config for `ssh_tasks` becomes `{ task_id, service_id? }`: + +- If `service_id` is omitted, the task's `default_service_id` is used. +- The adapter loads the task, resolves the instance, runs it, appends a + `service_task_runs` row, and returns the trimmed stdout/stderr/exit status. + +### 12.5 Relationship to machines + +- The SSH task execution role moves **out of machines** into `ssh_tasks` instances. +- Machines **keep** their role for the File Browser and node_exporter monitoring + transport in this change, to avoid also reworking Files/Monitoring here. +- Practical consequence: an SSH host used for both files and tasks may be defined twice + (once as a machine, once as an ssh_tasks instance) during the transition. Unifying + machines under services is an explicit **follow-up change**, not part of this one. diff --git a/openspec/changes/service-registry/tasks.md b/openspec/changes/service-registry/tasks.md index 8f4beb4..6025cfc 100644 --- a/openspec/changes/service-registry/tasks.md +++ b/openspec/changes/service-registry/tasks.md @@ -43,7 +43,9 @@ Chain strategy: stacked-to-main - Files: `services/settings_store.py` (modify), `services/service_store.py` (new) - Lines: ~120 - Details: `services` table in `init_schema`; CRUD helpers; decrypt-on-read for - adapters; "set" flags for the API without plaintext. + adapters; "set" flags for the API without plaintext. **Cascade delete:** removing a + service deletes its widgets in the same transaction. Also add the + `service_task_runs` table (design §12.3) now so later slices can populate it. - [ ] **1.5 Add service Pydantic models + router** - Files: `models/services.py` (new), `routers/services.py` (new), `main.py` (modify) - Lines: ~110 @@ -55,9 +57,10 @@ Chain strategy: stacked-to-main - Details: Extend startup validation to require `MANAGE_ENCRYPTION_KEY`. - [ ] **1.7 Add backend tests** - Files: `backend/tests/test_services.py` (new) - - Lines: ~120 + - Lines: ~140 - Details: Registry contents, CRUD round-trip, secret encryption/decryption, - unknown service type → 422, missing encryption key → startup error. + unknown service type → 422, missing/invalid encryption key → startup error, + cascade-delete removes a service's widgets. - [ ] **1.8 Verify** - Run: `cd backend && .venv/bin/ruff check . && PYTHONPATH=src .venv/bin/python -m pytest` @@ -74,10 +77,11 @@ Chain strategy: stacked-to-main as `{service_type}.{kind}` during transition; drop `addon_id`. - [ ] **2.2 Refactor source adapters** - Files: `widgets/sources.py` (modify) - - Lines: ~140 + - Lines: ~160 - Details: Each adapter takes `(service: ServiceRecord, widget_kind, config)`. `SOURCE_ADAPTERS` keyed by `service_type`. Jellyfin/Grafana/Prometheus/SSH adapters - resolve connection from the service record. + resolve connection from the service record. The SSH adapter resolves the task + + instance, runs it, and **appends a `service_task_runs` row** (design §12.3). - [ ] **2.3 Retire old widget registry** - Files: `widgets/registry.py` (delete or hollow out), `widgets/__init__.py` - Lines: ~-60 @@ -150,7 +154,7 @@ Chain strategy: stacked-to-main - Files: `frontend/src/widgets/*` (modify) - Lines: ~120 - Details: Components read `widget_kind`; data shapes unchanged but sourced from the - service adapter. + service adapter. SSH task widget shows last run status from `service_task_runs`. - [ ] **4.3 Remove machine Jellyfin/Jellyseerr fields** - Files: `frontend/src/pages/Settings.tsx`, `frontend/src/types/index.ts` (modify) @@ -184,15 +188,25 @@ Chain strategy: stacked-to-main - [ ] **5.3 Manual dev-stack check** — `docker compose -f docker-compose.dev.yml up --build`: - Create a Grafana service from the UI; verify the dashboard link widget works. - Create a Jellyfin service; verify the activity widget resolves it. - - Delete a service with widgets → 409; remove widgets → delete succeeds. + - Delete a service with widgets → widgets are cascade-deleted and the service is gone. - Restart the stack; secrets remain usable (key stable). - Missing `MANAGE_ENCRYPTION_KEY` → backend refuses to start. + - SSH task runner: define two instances, run the same reusable task against each, + and see both runs in the instance's history log. ## Guards ```text -Decision needed before apply: Yes (resolve design §11 first) +Decision needed before apply: No (design §11 resolved) Chained PRs recommended: Yes Chain strategy: stacked-to-main 400-line budget risk: High ``` + +## Explicit follow-ups (out of scope for this change) + +- Rebuild the Actions page UI on top of services (global reusable tasks + + `default_service_id`), replacing the current machine-based saved-task runner. +- Unify machines under services so an SSH host is defined once (today machines still + own File Browser + node_exporter transport; see design §12.5). +- Key rotation / re-encrypt workflow for `MANAGE_ENCRYPTION_KEY`.