Files
manage/openspec/changes/service-credential-tester/tasks.md
T
Developer c4f68b4938 spec(service-credential-tester): add tasks (2 slices, each <=400 lines)
S1 backend: TestResult + test_callable + translate_connection_error + POST
/api/services/test + 7 per-type routines + tests. S2 frontend: type + API fn +
hook + shared ServiceTestPanel wired into CreateServiceDialog + Settings.tsx
ServiceConfigEditor (per design source-finding). Each slice leaves pytest/npm
build/npm lint green.
2026-07-09 22:25:27 +00:00

28 KiB
Raw Blame History

SDD Tasks: Service Credential Tester

Change: service-credential-tester Phase: tasks Date: 2026-07-09

Review Workload Forecast

Field Value
Estimated changed lines ~550700 (sum of two implementation slices)
400-line budget risk LowMedium
Chained PRs recommended Yes
Suggested split PR 1: backend endpoint + TestResult/test_callable/translate_connection_error + 7 per-type routines + tests → PR 2: frontend type + API fn + hook + ServiceTestPanel + wire into both surfaces + tests
Delivery strategy auto-chain
Chain strategy stacked-to-main
Decision needed before apply: No
Chained PRs recommended: Yes
Chain strategy: stacked-to-main
400-line budget risk: LowMedium

Each slice individually lands under the 400-line review budget. Slices are ordered S1 → S2; S1 is independently shippable (backend endpoint works, frontend has no Test UI until S2). Per openspec/config.yaml rules, each slice leaves npm run build (tsc -b + vite build), npm run lint, and backend pytest green.


Slice ordering rationale (critical)

Slice 1 builds the backend endpoint + per-type test routines. After S1:

  • POST /api/services/test validates input (reuses _validate_input), dispatches to the definition's test_callable, and returns {ok, detail, evidence} — no persistence.
  • Each of 7 service types (qbittorrent, prometheus, alertmanager, jellyfin, authentik, ssh_tasks, nextcloud) has a test_connection routine; backups has test_callable = None.
  • TestResult dataclass + translate_connection_error shared helper live in integrations/base.py.
  • All backend tests are green: per-type mocked routines, endpoint dispatch, validation-first, no-persistence.
  • The frontend has NO Test UI yet — the endpoint is callable via API only. That's fine; S2 adds the UI.

Slice 2 adds the frontend Test UI. A shared ServiceTestPanel component (Test button + result pill + Save-anyway override + field-edit-clears-result) is wired into BOTH CreateServiceDialog (in ServicesPage.tsx) AND ServiceConfigEditor (in Settings.tsx — per the design source finding, NOT ServicePage.tsx). The panel is identical in both surfaces.

This ordering ensures the backend is proven (endpoint dispatches correctly, validation rejects malformed input, no-persistence holds) before any frontend churn.


Source-finding corrections (read before applying)

The design (§0) surfaced deviations from the proposal/spec. Tasks below incorporate these corrections:

Spec/proposal claim Actual source reality Correction in tasks
CT-115: "the edit dialog on ServicePage.tsx" ServicePage.tsx has no edit dialog. Service editing lives in Settings.tsx::ServiceConfigEditor (master-detail panel, lines ~14351596). Task 2.6 targets Settings.tsx::ServiceConfigEditor, NOT ServicePage.tsx.
CT-109: "reuses the test_machine_ssh connect flow" test_machine_ssh (settings.py:122180) is a router endpoint, not a reusable function. But build_ssh_client in task_runner.py builds the client from a ServiceRecord. Task 1.8 uses build_ssh_client(store, ServiceRecord) + .connect(), then applies the same error-translation patterns inline. It does NOT import or call test_machine_ssh.
Proposal: "construct a QbittorrentClient from the config + decrypted secrets" QbittorrentClient.__init__ takes positional strings (base_url, username, password, timeout), not a config dict. Task 1.4 extracts fields explicitly from config/secrets dicts.
Proposal mentions jellyseerr as a distinct type Active registry has no jellyseerr entry (merged into Jellyfin by services-as-hub-ia). No jellyseerr test routine. The 7 remote types are: qbittorrent, prometheus, alertmanager, jellyfin, authentik, ssh_tasks, nextcloud.

No proposal/spec scope change is required — the intent (per-type credential tester) holds. These corrections refine implementation details and the edit-surface target.


Slice 1: Backend endpoint + test_callable + per-type routines + tests

Goal: Add POST /api/services/test that validates unsaved service input, dispatches to a per-type test_callable, and returns {ok, detail, evidence} without persisting. Each of 7 service types gets a test_connection routine; backups has no test.

Satisfies: CT-101, CT-102, CT-103, CT-104, CT-105, CT-106, CT-107, CT-108, CT-109, CT-110, CT-111, CT-112, CT-113, CT-119.

  • 1.1 Add TestResult dataclass + TestCallable type alias + test_callable field on ServiceDefinition (CT-101, CT-103)

    • Files: backend/src/media_library_viewer_api/integrations/base.py (modify)
    • Lines: ~30
    • Dependencies: none
    • Details: Add @dataclass(frozen=True) class TestResult with fields ok: bool, detail: str, evidence: str | None = None. Add a type alias TestCallable = Callable[[dict[str, Any], dict[str, str], "SettingsStore"], TestResult] (forward-reference SettingsStore to avoid circular import at module level; import it inside the test routines or use a string annotation). Add test_callable: TestCallable | None = None to ServiceDefinition — placed LAST (after widget_kinds) so the dataclass field-ordering with a default works. Import Callable from typing. Update TYPE_CHECKING guard if needed for the SettingsStore forward reference.
  • 1.2 Add translate_connection_error shared helper (CT-104..CT-110)

    • Files: backend/src/media_library_viewer_api/integrations/base.py (modify, same file)
    • Lines: ~40
    • Dependencies: 1.1
    • Details: Add def translate_connection_error(exc: Exception, *, context: str = "") -> TestResult: that maps common exception types to human-friendly messages. Handle: requests.HTTPError with status 401/403 → auth message; requests.ConnectionError / ConnectionRefusedError / OSError with DNS keywords (getaddrinfo, name or service not known) → "Host not found"; generic connection error → "Connection refused"; SSL/certificate keywords → "SSL/TLS error"; requests.Timeout / TimeoutError / asyncio.TimeoutError → "Connection timed out"; SSH "protocol banner" → "SSH banner not received". Fallback: truncate message to 200 chars, prefix with context. The helper returns TestResult(ok=False, detail=...) always. Import requests and asyncio at the top of base.py (or inside the function to avoid circular imports — prefer top-level since base.py already may need requests for the type checks).
  • 1.3 Add test_connection for qbittorrent (CT-104)

    • Files: backend/src/media_library_viewer_api/integrations/qbittorrent.py (modify)
    • Lines: ~25
    • Dependencies: 1.1, 1.2
    • Details: Add def test_connection(config: dict[str, Any], secrets: dict[str, str], store: SettingsStore) -> TestResult: that constructs QbittorrentClient(config["base_url"], secrets["username"], secrets["password"], timeout=config.get("timeout_seconds", 10)), calls client.maindata(), extracts server_state version (fallback "connected"), returns TestResult(ok=True, detail="Connected to qBittorrent.", evidence=version). Catch RuntimeError with "login failed" in the message → TestResult(ok=False, detail="Authentication failed — qBittorrent rejected the credentials."). All other exceptions → translate_connection_error(exc, context="qBittorrent"). Import QbittorrentClient from clients.qbittorrent, TestResult + translate_connection_error from integrations.base. Wire test_callable=test_connection into the DEFINITION.
  • 1.4 Add test_connection for prometheus via Grafana gateway (CT-105)

    • Files: backend/src/media_library_viewer_api/integrations/prometheus.py (modify)
    • Lines: ~30
    • Dependencies: 1.1, 1.2
    • Details: Add def test_connection(config, secrets, store) -> TestResult: that issues POST {grafana_url}/api/ds/query with Authorization: Bearer {grafana_api_key}, body {"queries": [{"datasource": {"uid": datasource_uid, "type": "prometheus"}, "expr": "up", "format": "time_series", "intervalMs": 15000, "maxDataPoints": 1, "refId": "A"}], "from": "now-1m", "to": "now"}. Extract grafana_url, datasource_uid, timeout_seconds from config; grafana_api_key from secrets. Missing URL/key → TestResult(ok=False, detail="...") (no network). Success → TestResult(ok=True, detail="Grafana gateway reachable.", evidence="Gateway reachable; datasource responded."). Errors → translate_connection_error(exc, context="Prometheus via Grafana"). Mirror MetricSource._gateway_query request shape exactly. Wire test_callable=test_connection into the DEFINITION.
  • 1.5 Add test_connection for alertmanager (CT-106)

    • Files: backend/src/media_library_viewer_api/integrations/alertmanager.py (modify)
    • Lines: ~20
    • Dependencies: 1.1, 1.2
    • Details: Add def test_connection(config, secrets, store) -> TestResult: that GETs {base_url}/api/v2/status with optional Authorization: Bearer {api_key} (when secret present). Extract version from versionInfo.version (fallback "connected"). Success → TestResult(ok=True, detail="Connected to Alertmanager.", evidence=version). Errors → translate_connection_error(exc, context="Alertmanager"). Wire test_callable=test_connection into the DEFINITION.
  • 1.6 Add test_connection for jellyfin (CT-107)

    • Files: backend/src/media_library_viewer_api/integrations/jellyfin.py (modify)
    • Lines: ~20
    • Dependencies: 1.1, 1.2
    • Details: Add def test_connection(config, secrets, store) -> TestResult: that constructs JellyfinClient(config["base_url"], secrets["api_key"], timeout=config.get("timeout_seconds", 10)) and calls .users(). Success → TestResult(ok=True, detail="Connected to Jellyfin.", evidence=f"{len(users)} users"). Errors → translate_connection_error(exc, context="Jellyfin"). Import JellyfinClient from clients.jellyfin. Wire test_callable=test_connection into the DEFINITION.
  • 1.7 Add test_connection for authentik (CT-108)

    • Files: backend/src/media_library_viewer_api/integrations/authentik.py (modify)
    • Lines: ~20
    • Dependencies: 1.1, 1.2
    • Details: Add def test_connection(config, secrets, store) -> TestResult: that constructs AuthentikClient(base_url=config["base_url"], api_token=secrets["api_token"], timeout=config.get("timeout_seconds", 10)) and calls .users(page=1, page_size=1) — the lightest directory probe. Success → TestResult(ok=True, detail="Connected to Authentik.", evidence=...). Errors → translate_connection_error(exc, context="Authentik"). Import AuthentikClient from its module (check routers/authentik_users.py::_build_client for the exact import path + constructor signature). Wire test_callable=test_connection into the DEFINITION.
  • 1.8 Add test_connection for ssh_tasks reusing build_ssh_client (CT-109)

    • Files: backend/src/media_library_viewer_api/integrations/ssh_tasks.py (modify)
    • Lines: ~30
    • Dependencies: 1.1, 1.2
    • Details: Add def test_connection(config, secrets, store) -> TestResult: that constructs a ServiceRecord(id="", service_type="ssh_tasks", name="test", config=config, secrets=secrets, enabled=True), calls build_ssh_client(store, service) (from services.task_runner), then .connect() inside try/finally with .close(). Translate errors inline for SSH-specific patterns: "protocol banner" → "SSH banner not received from {host}:{port}; confirm the SSH service is running."; "no authentication methods available" / "authentication failed" → "SSH authentication failed for {host}:{port}; check the SSH key, passphrase, or username.". Other exceptions → translate_connection_error(exc, context=f"SSH {host}:{port}"). Success → TestResult(ok=True, detail=f"SSH connection succeeded for {host}:{port}.", evidence=f"Connected to {host}:{port}"). Import build_ssh_client from media_library_viewer_api.services.task_runner and ServiceRecord from media_library_viewer_api.widgets.sources (or define locally to avoid circular import — check). Do NOT import or call test_machine_ssh — it's a router endpoint, not a reusable function. Known-host recording is preserved automatically by RemoteSSHClient.connect(). Wire test_callable=test_connection into the DEFINITION.
  • 1.9 Add test_connection for nextcloud (CT-110)

    • Files: backend/src/media_library_viewer_api/integrations/nextcloud.py (modify)
    • Lines: ~15
    • Dependencies: 1.1, 1.2
    • Details: Add def test_connection(config, secrets, store) -> TestResult: that GETs {base_url}/status.php (unauthenticated — /status.php is public). Extract version from the JSON response (fallback "connected"). Success → TestResult(ok=True, detail="Connected to Nextcloud.", evidence=version). Errors → translate_connection_error(exc, context="Nextcloud"). Wire test_callable=test_connection into the DEFINITION.
  • 1.10 Confirm backups DEFINITION has test_callable = None (CT-111)

    • Files: backend/src/media_library_viewer_api/integrations/backups.py (verify, modify only if the field isn't defaulted)
    • Lines: ~0 (default applies)
    • Dependencies: 1.1
    • Details: backups should NOT gain a test_connection function. Its DEFINITION relies on the default test_callable=None from ServiceDefinition. Verify by reading the file; no code change expected unless the DEFINITION is constructed with explicit keyword args that omit test_callable (the default handles it). The endpoint returns {ok: true, detail: "No connection test for this service type"} automatically.
  • 1.11 Add POST /api/services/test endpoint (CT-101, CT-102, CT-103, CT-112, CT-113)

    • Files: backend/src/media_library_viewer_api/routers/services.py (modify)
    • Lines: ~30
    • Dependencies: 1.11.10
    • Details: Add @router.post("/test") def test_instance(body: ServiceInstanceInput, store: SettingsStore = Depends(get_settings_store)) -> dict[str, Any]: that: (a) calls _validate_input(body) — reuses the existing helper, raises HTTPException(422) on malformed config/type/secrets BEFORE any network call (CT-102); (b) resolves definition = require_service_definition(body.service_type); (c) if definition.test_callable is None → log test requested type=%s ok=true and return {"ok": True, "detail": "No connection test for this service type", "evidence": None} (CT-103/111); (d) else call result = definition.test_callable(body.config, body.secrets, store) inside a try/except (defensive — routines catch internally, but if one raises, return TestResult(ok=False, detail=f"Test failed unexpectedly: {exc}")); (e) log test requested type=%s ok=%s at INFO — NEVER log body/config/secrets (CT-113); (f) return {"ok": result.ok, "detail": result.detail, "evidence": result.evidence}. NO call to store.upsert_service, store.update_setting, or any persistence method (CT-112). Import require_service_definition from integrations.registry (already imported or adjacent).
  • 1.12 Add backend tests for translate_connection_error (CT-119)

    • Files: backend/tests/test_services.py (modify) or a new backend/tests/test_credential_tester.py (create)
    • Lines: ~40
    • Dependencies: 1.2
    • Details: Test the shared helper directly: (a) requests.HTTPError with a mock response .status_code = 401 → auth message; (b) requests.ConnectionError("getaddrinfo failed") → "Host not found"; (c) requests.Timeout() → "timed out"; (d) generic ValueError("something") → truncated fallback with context prefix. Assert ok=False and the detail string contains expected keywords.
  • 1.13 Add backend tests for per-type test_connection routines (CT-119)

    • Files: backend/tests/test_services.py (modify) or backend/tests/test_credential_tester.py (create/modify)
    • Lines: ~100
    • Dependencies: 1.31.9
    • Details: For each of the 7 remote types, add at least two tests: (a) SUCCESS — mock the client/request to return valid data, assert {ok: True, detail: ..., evidence: ...}; (b) FAILURE — mock the client/request to raise, assert {ok: False, detail: <readable message>, evidence: None}. Specific cases:
      • qbittorrent: success returns version; RuntimeError("qBittorrent login failed: Fails.") → detail mentions "Authentication failed".
      • prometheus: success returns gateway-reachable evidence; HTTP 401 → auth message.
      • alertmanager: success returns version; connection refused → unreachable message.
      • jellyfin: success returns user count; HTTP 401 → auth message.
      • authentik: success returns user count; connection error → unreachable.
      • ssh_tasks: mock build_ssh_client to return a mock client; .connect() raises "authentication failed" → SSH auth message; .connect() succeeds → connected evidence.
      • nextcloud: success returns version; 404 → connection error.
    • Use unittest.mock.patch to mock at the right boundary (the client constructor or requests.get/post).
  • 1.14 Add backend tests for the endpoint: dispatch, validation-first, no-persistence (CT-119)

    • Files: backend/tests/test_api.py (modify) or backend/tests/test_credential_tester.py (create/modify)
    • Lines: ~50
    • Dependencies: 1.11
    • Details: (a) DISPATCH: POST /api/services/test with a backups body → {ok: True, detail: "No connection test..."} and no network call. (b) VALIDATION-FIRST: body with schema-less base_url for qbittorrent → HTTP 422 (not 200); assert the test_callable was NOT called (mock it, assert call count 0). (c) NO-PERSISTENCE: call /test with a valid qbittorrent body (test_callable mocked to return ok), assert store.list_services() count is unchanged before/after. (d) NO-SECRET-LOGS: use caplog at INFO level, call /test with secrets: {"password": "hunter2"}, assert no log line contains "hunter2".
  • 1.15 Verify Slice 1 (pytest + ruff + frontend still builds)

    • Run: cd backend && PYTHONPATH=src python3 -m pytest -q && PYTHONPATH=src python3 -m ruff check src tests
    • Run: cd frontend && npm run build && npm run lint
    • Verify: all backend tests pass (per-type routines, endpoint dispatch, validation, no-persistence, no-secret-logs); ruff clean; frontend still builds + lints (no frontend change in S1, so this is a regression check only).
    • Risk flag: the test_callable field on ServiceDefinition is a new field with a default. Verify that existing DEFINITION construction sites (all 8 integration modules) still compile — they should, since the default is None, but a frozen dataclass with field-ordering can surprise if any DEFINITION passes positional args. Check for positional-arg construction (unlikely — the codebase uses keyword args).

Slice 1 total: ~300380 changed lines.


Slice 2: Frontend Test UI — API client + hook + shared panel + wire into both surfaces + tests

Goal: Add a "Test credentials" button + result pill + Create/Save gating to BOTH the add-service dialog (CreateServiceDialog in ServicesPage.tsx) and the edit panel (ServiceConfigEditor in Settings.tsx). A shared ServiceTestPanel component avoids duplication.

Satisfies: CT-114, CT-115, CT-116, CT-117, CT-118, CT-120, CT-121.

  • 2.1 Add ServiceTestResult type (CT-114)

    • Files: frontend/src/types/index.ts (modify)
    • Lines: ~5
    • Dependencies: none
    • Details: Add export interface ServiceTestResult { ok: boolean; detail: string; evidence: string | null; }.
  • 2.2 Add testServiceInstance API client function (CT-114)

    • Files: frontend/src/api/services.ts (modify)
    • Lines: ~5
    • Dependencies: 2.1
    • Details: Add export async function testServiceInstance(input: ServiceInstanceInput): Promise<ServiceTestResult> { return post<ServiceTestResult>("/api/services/test", input); }. Import ServiceTestResult from ../types. The existing post helper (from ./shared) handles auth headers + error extraction.
  • 2.3 Add useTestServiceInstance hook (CT-114)

    • Files: frontend/src/hooks/useServices.ts (modify)
    • Lines: ~8
    • Dependencies: 2.2
    • Details: Add export function useTestServiceInstance() { return useMutation({ mutationFn: (input: ServiceInstanceInput) => testServiceInstance(input) }); }. Import useMutation from @tanstack/react-query (already imported in the file). Import testServiceInstance from ../api/services. No cache invalidation needed — the test is a one-shot mutation.
  • 2.4 Create shared ServiceTestPanel component (CT-115, CT-116, CT-117, CT-118)

    • Files: frontend/src/components/ServiceTestPanel.tsx (NEW)
    • Lines: ~70
    • Dependencies: 2.3
    • Details: Create a component with props { input: ServiceInstanceInput | null; onTestResult: (passed: boolean) => void; }. Internal state: result: ServiceTestResult | null, saveAnyway: boolean. useEffect([input]) clears result and calls onTestResult(false) whenever input changes (new object reference on every field edit — CT-118). A "Test credentials" Button (variant outline, size sm) fires testService.mutateAsync(input); while pending shows "Testing…" and is disabled. On result: render an Alert — green/default variant with ✓ Connected — {evidence} on success, destructive variant with ✗ {detail} on failure. A "Save anyway (skip test)" checkbox toggles saveAnyway; when checked, calls onTestResult(true) regardless of test outcome (CT-117). The parent reads the gating signal via the onTestResult callback. Import useTestServiceInstance, Alert/AlertDescription from @/components/ui/alert, Button from @/components/ui/button.
  • 2.5 Wire ServiceTestPanel into CreateServiceDialog (CT-115, CT-117)

    • Files: frontend/src/pages/ServicesPage.tsx (modify)
    • Lines: ~20
    • Dependencies: 2.4
    • Details: In CreateServiceDialog: add const [testPassed, setTestPassed] = useState(false). Reset it in reset(). Build testInput from the current draft (null if no draft). Render <ServiceTestPanel input={testInput} onTestResult={setTestPassed} /> below the config/secret fields and above the footer. Update DialogFooter's confirmDisabled to include !testPassed (i.e. confirmDisabled={!draft.name.trim() || saveService.isPending || !testPassed}). Import ServiceTestPanel from ../components/ServiceTestPanel.
  • 2.6 Wire ServiceTestPanel into ServiceConfigEditor in Settings.tsx (CT-115, CT-117)

    • Files: frontend/src/pages/Settings.tsx (modify)
    • Lines: ~20
    • Dependencies: 2.4
    • Details: In ServiceConfigEditor (NOT ServicePage.tsx — per design source finding §0, service editing lives in Settings.tsx): add const [testPassed, setTestPassed] = useState(false). Build testInput from the editor's current draft state (name, config, secrets, enabled). Render <ServiceTestPanel input={testInput} onTestResult={setTestPassed} /> below the form fields. Gate the Save button on !testPassed (disabled until test passes or Save anyway is checked). Reset testPassed when switching instances (if the editor has an instance-switch effect). Import ServiceTestPanel.
    • Risk flag: the spec CT-115 says "ServicePage.tsx" — that is textual drift. The actual edit surface is Settings.tsx::ServiceConfigEditor. This task targets the correct file.
  • 2.7 Add frontend tests for ServiceTestPanel (CT-120)

    • Files: frontend/src/components/__tests__/ServiceTestPanel.test.tsx (NEW)
    • Lines: ~80
    • Dependencies: 2.4
    • Details: Test cases: (a) renders Test button; (b) click fires the mocked useTestServiceInstance mutation; (c) success result → green ✓ Connected pill with evidence text; (d) failure result → red pill with detail text; (e) "Save anyway" checkbox checked → onTestResult(true) called regardless of test state; (f) editing input (passing a new object) clears the result and calls onTestResult(false). Mock useTestServiceInstance via vi.mock("../../hooks/useServices", ...). Use @testing-library/react + @testing-library/user-event.
  • 2.8 Update existing dialog tests for gating (CT-120)

    • Files: frontend/src/pages/__tests__/ServicesPage.test.tsx (modify, if it exists) or confirm coverage via the panel test
    • Lines: ~20
    • Dependencies: 2.5
    • Details: If ServicesPage.test.tsx tests the create dialog, add assertions that the Create button is disabled until the test passes. Mock the test mutation to return {ok: true} and verify the button enables. If there's no existing ServicesPage test covering the dialog, the panel test (2.7) covers the gating behavior adequately — note the coverage decision.
  • 2.9 Verify Slice 2 (build + lint + test)

    • Run: cd frontend && npm run build && npm run lint && npx vitest run
    • Run: cd backend && PYTHONPATH=src python3 -m pytest -q (regression: Slice 1 tests still pass)
    • Verify: frontend typechecks + builds; lint 0 errors; vitest passes (new panel test + existing tests); backend still green.
    • Risk flag: ServiceConfigEditor in Settings.tsx is a large component (~160 lines). The wiring (state + panel render + button gating) must be surgical — do not refactor the editor. Add the testPassed state, the testInput build, the panel render, and the button disabled prop only.

Slice 2 total: ~250320 changed lines.


Integration verification (post-slice)

  • 3.1 Full backend test run

    • Run: cd backend && PYTHONPATH=src python3 -m pytest -q
    • Verify: all tests pass (per-type routines, endpoint, validation, no-persistence, no-secret-logs).
  • 3.2 Full frontend build + lint + test

    • Run: cd frontend && npm run build && npm run lint && npx vitest run
    • Verify: 0 errors; panel test covers button/pill/gating/field-clear.
  • 3.3 End-to-end dispatch check

    • Verify (by reading source or running a manual API call): POST /api/services/test with a backups body returns {ok: true, detail: "No connection test for this service type"}; with a qbittorrent body (mocked client) dispatches to the qbittorrent routine; with an unknown type returns 422.
  • 3.4 No-persistence check

    • Verify (by test in 1.14): calling /test does not create a service row.
  • 3.5 No-secret-logs check

    • Verify (by test in 1.14): no log line contains a secret value.

Risk flags summary

  1. (a) CT-115 spec drift — edit surface is Settings.tsx::ServiceConfigEditor, NOT ServicePage.tsx. Task 2.6 targets the correct file. The spec's literal text is stale; the design (§0) corrected it. Do not add a Test button to ServicePage.tsx (it's a read-only tabbed view with no edit form).

  2. (b) ssh_tasks test must NOT duplicate test_machine_ssh. test_machine_ssh (settings.py:122180) is a router endpoint, not a reusable function. Task 1.8 uses build_ssh_client(store, ServiceRecord) from task_runner.py + .connect(), then translates errors inline using the same message patterns. No import of test_machine_ssh.

  3. (c) The shared ServiceTestPanel must work identically in both surfaces. Tasks 2.5 + 2.6 wire the same component into CreateServiceDialog and ServiceConfigEditor. The panel's input prop is an object built from the parent's draft state; onTestResult is a callback the parent uses for gating. The panel owns the Test button, result pill, and Save-anyway checkbox; the parent owns the confirm button's disabled prop.

  4. (d) ServiceDefinition is a frozen dataclass. Adding test_callable with a default None as the LAST field works with dataclass field-ordering. Verify no DEFINITION is constructed with positional args (the codebase uses keyword args, so this should be safe). Task 1.15 checks this.

  5. (e) Circular import risk for SettingsStore in TestCallable. The type alias references SettingsStore (from services.settings_store). Use a string forward-reference ("SettingsStore") in the type alias to avoid importing settings_store into integrations/base.py at module level. The actual SettingsStore is passed at runtime by the endpoint handler. Task 1.1 handles this.

  6. (f) Settings.tsx::ServiceConfigEditor is a large component. Task 2.6 wiring must be surgical (add state + input build + panel render + button disabled prop). Do NOT refactor the editor or touch unrelated fields.