Lane 2: first-run setup's own state, and a backup read-back (#728) #754
Loading…
Reference in a new issue
No description provided.
Delete branch "build/lane2-setup-server"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Lane 2 of the v4.5.0 redesign (#728), the second PR of its series: the server side of PUB-8, setting up a new server (
IS-251–IS-258, #439, D68, D70). Backend only; the wizard's pages are a later PR.The happy surprise: six of the seven steps already had somewhere to write
Going through them one at a time, the wizard turns out to be a path through endpoints the product already has, not a new settings system:
IS-251)IS-252)PUT /api/admin/backup/config,POST /api/admin/backup/run,GET /api/admin/backup/logsIS-253)GET /api/admin/settings/secrets-healthalready answers the questionIS-254)PUT /api/admin/settings/retentionIS-255)PUT /api/admin/settings/bot, thenwhisper/testandllm/test— Instance health's own tests, which is what PUB-8's header asked for, "not a second set (#439)"IS-256)PUT /api/admin/settings/bot,POST /api/admin/settings/bot/regenerate-key,GET /api/admin/settings/bot/pingregenerate-keyalready returns the key exactly once, and its docstring already says so — which is the drawing's "Made for you. Put it in the bot's environment; it is not shown again", word for word. The drawing got that one right.So no step writes settings by a second path. A wizard with its own write routes would be a second set of validation, audit and encryption rules to keep in step with the first, and they would drift.
What is actually new: the state
Nothing recorded that an instance had been set up at all.
GET /api/setup/state,POST /api/setup/steps/{step}andPOST /api/setup/finish, all admin-only, keepdone,skippedandcompleted_at.Two behaviours are the feature rather than plumbing, and each has a test:
finishnever refuses over a skipped step. D70: an unfinished setup reminds, and never blocks.An unknown step or outcome is a 400 with this project's
{"detail": "..."}rather than FastAPI's list-shaped 422 —outcomeis validated in the service for exactly that reason.No migration.
setup_stateis a key in the existingapp_settingsJSON store.The backup read-back (
IS-252)POST /api/admin/backup/verifytakes the newest successful backup, or one named bylog_id, and runspg_restore --listover the archive:This reads the archive; it does not restore it — recorded as a departure on #728 before it was built. A real restore needs a scratch database, the time to load a full dump into it, and an answer for what happens when the dump is large; that belongs with the backup feature, not inside a first-run wizard. So the step's wording says "read it back" and claims no more than it does.
It still catches the failures that actually happen: a missing file, a truncated or corrupt archive, an archive written by a newer
pg_dumpthan this server'spg_restorecan read (#429's problem from the other side), a timeout, andpg_restoreitself being absent — each averified: falsewith one sentence, never a 500.pg_restoreis in the backend image, at/usr/bin/pg_restore; the PGDG client package bundles it withpg_dump.Nothing puts the archive path or
pg_restore's raw stderr anywhere a non-admin can read.A real limitation, filed rather than papered over: #753
A backup that has been shipped off-host reports that it has no local copy to read. That is not a bug in this endpoint:
run_scheduled_backupcomputes the local path and then overwritesbackup_logs.destinationwith the redacted rclone spec, so the dump's filename is gone from the row even though the file is still on disk — the code says so two lines above.The consequence is the wrong way round: verification works on instances that keep backups only locally, and not on the ones that took the trouble to ship them off-host, which is the configuration #414 exists to encourage. Filed as #753 rather than fixed here, because it is a defect in the backup feature and this endpoint behaves correctly in its presence.
The other recorded departure: the key step shows no key
IS-253. The drawing showsQB_SECRET_KEY=2f9c…a41ewith a Copy button; this PR adds no endpoint that returns any part of it, and the wizard will not draw one.GET /api/admin/settings/bot's own docstring states the rule the whole settings layer follows — "Secrets are never returned — only presence flags" — andSECRET_KEYis the key every other secret is encrypted with. It also buys the operator nothing: they put it in the server's environment themselves. The step's information comes fromGET /api/admin/settings/secrets-health, which #428 and #430 built for precisely the question "is every encrypted setting readable with this key", after a restore onto a host with a different one produced a system that looked restored and was not.Checks
ruff format --checkandruff checkoverwebapp/backend/ scripts/ webapp/frontend/e2e/at the pinned 0.4.4: clean. Full backend suite 3875 passed, 13 skipped; the 26 new tests cover admin-only on all four endpoints (401 and 403), the replace-on-re-record behaviour in both directions, finish-with-skips, and everyverified: falsebranch includingpg_restorebeing absent.🤖 Generated with Claude Code
Lane 2 PR 2 (PUB-8, IS-251-258): the wizard's seven steps drive endpoints that already exist (Bot Settings, Backups, Retention, the AI test buttons), so the only new state is whether a server has been through setup at all. - app/services/settings_service.py: KEY_SETUP_STATE (unencrypted — no secrets), the SetupState dataclass, and get/record/finish_setup helpers. Re-recording a step moves it between done/skipped rather than duplicating it, so the skipped list can empty (D70). finish() never inspects skipped — D70: reminds, never blocks. - app/routers/setup.py (new): GET /api/setup/state, POST /api/setup/steps/{step}, POST /api/setup/finish — all require_admin. An unknown step or outcome is a 400 via ValueError from the service, not a 500 or a silent no-op. Writes nothing but this state; each step's actual settings still go through its own existing admin endpoint. - app/main.py: mounts the new router at /api beside admin. - app/routers/admin.py: POST /admin/backup/verify (IS-252) — reads the newest successful backup (or one by log_id) with `pg_restore --list` rather than restoring it (a recorded departure from the drawing). Handles a missing/truncated archive, an off-host-shipped backup whose local path isn't recoverable from backup_logs.destination once rclone has redacted it (#414), and an image without pg_restore, all as `verified: false` with a one-sentence `detail` — never a 500, never raw stderr or the real path in the response detail. - tests/test_setup.py (new), tests/test_admin.py: admin-only enforcement, the replace-on-re-record and never-blocks-on-finish behaviours, the unknown-step 400, and the read-back's missing/truncated/off-host/ pg_restore-absent paths. Does not touch the key step's SECRET_KEY display, the AI test buttons, or frontend — all explicitly out of scope per the lane-2 contract. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>