docs(service-registry): lock decisions (cascade delete, required key, SSH runner model)
- §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).
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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`.
|
||||
|
||||
Reference in New Issue
Block a user