Gates / frontend (push) Successful in 1m22s
Gates / test (push) Successful in 1m54s
Gates / test-aarch64 (push) Successful in 7m57s
Gates / package (push) Successful in 5m29s
Gates / container (push) Successful in 10s
CI / gates (push) Successful in 32m51s
173 lines
21 KiB
Markdown
173 lines
21 KiB
Markdown
# Milestone 32: task-shaped configuration and file mode
|
|
|
|
Redesign step 5 of specs/ui-redesign.md (§Information architecture, §File mode, §API changes `/api/config/status`, §Deletions, build-sequence step 5). The six top-level configuration routes (Groups, Blocklists, Rules, Local DNS, Upstreams, Settings) become three task-shaped subpages — `/configuration/protection`, `/configuration/resolution`, `/configuration/system` — in both editable (database-mode) and read-only (file-mode) forms; the server gains `GET /api/config/status` with a server-owned `restart_pending`; the client-only restart banner and the standing read-only banner die; Clients gains its detail route. Old routes are replaced atomically, no aliases.
|
|
|
|
Codex design review round 1 folded in; corrections marked (D1..D14).
|
|
|
|
## Sessions
|
|
|
|
S1 (Zig endpoint + envelope change + the minimal admin data-layer migration that keeps tsc green), then S2 (admin: configuration restructure + file mode + shell), then S3 (admin: Clients redesign). Sequential — S2 and S3 both touch `routes.tsx` and the clients feature boundary.
|
|
|
|
---
|
|
|
|
## Session S1: `/api/config/status` and server-owned `restart_pending`
|
|
|
|
### S1.1 The endpoint
|
|
|
|
`GET /api/config/status`, new handler `src/web/handlers/config.zig`, route policy `.read`, auth `.session`. Route count 63 → 64 (bump the count test, routes.zig:159-161). Response:
|
|
|
|
```json
|
|
{"authority": "database" | "managed_file", "path": null | "/etc/nxdns/config.zon", "reconciled_at": null | 1755600000, "restart_pending": false}
|
|
```
|
|
|
|
- `authority`/`path`/`reconciled_at` read from the existing `WebState.authority` (server.zig:113-125) and `WebState.reconciled_at` (server.zig:126-130). `path` is null exactly when authority is `database`; `reconciled_at` is null in database mode (both already true of the state today).
|
|
- `restart_pending`: new `restart_pending: std.atomic.Value(bool) = .init(false)` on `WebState` beside `reconciled_at`, same "per-process state, never persisted" doc comment. It is process state: set by database-mode mutations that need a restart, cleared by nothing but process exit.
|
|
|
|
### S1.2 Setters
|
|
|
|
Exactly the mutations whose changes do not take effect live (the sites already emitting per-response `restart_required`):
|
|
|
|
- `src/web/handlers/upstreams.zig` create, update and remove.
|
|
- `src/web/handlers/settings.zig` `put` (settings.zig:521-529) — only when the patch touches a restart-required key. The server already owns that key set (`restart_required_keys`, settings.zig:68); a patch touching only `web.password` must not set the flag (password applies live, docs/reference/api.md:29). This replaces the admin's `patchRequiresRestart` (S2 deletes it) — the server becomes the one home of that decision.
|
|
- **(D4)** The flag is stored only *after* the database mutation commits — a validation failure or SQL error must leave it untouched.
|
|
|
|
File-mode requests never reach these handlers (router 403, router.zig:173-176), so the flag can only rise in database mode.
|
|
|
|
### S1.3 Envelope change
|
|
|
|
`GET /api/settings` loses its `authority` object (settings.zig:485-501 serialization, openapi.yaml). `/api/config/status` is the one home; two copies of the same fact drift. `restart_required` (the key list) stays on the settings envelope — field-level metadata about the settings form, not process state. Per-response `restart_required: true` on upstream views stays (harmless, view-local).
|
|
|
|
### S1.4 Surfaces and tests
|
|
|
|
- `src/web/openapi.yaml`: new path + `ConfigStatus` schema; `SettingsEnvelope` loses `authority`. Drift tests stay green (openapi.zig).
|
|
- `docs/reference/api.md`: new route row (docs_drift_test.zig enforces per-operation rows); settings envelope description updated.
|
|
- **(D10)** `contract_sample_walk` is a static table — add a `get_config_status` sample explicitly, typed `ConfigStatus`, so the new shape is byte-checked against a real response like every other endpoint.
|
|
- `src/web/web_integration_test.zig`: config-status view struct. **(D4, R2-1)** A transition matrix where **every setter is proved `false → true` in isolation** — a probe made while the flag is already `true` proves nothing. Isolation comes from a fresh server process per setter (the integration harness already boots servers per scenario; restarting between assertions is also the proof that restart clears the flag):
|
|
- database mode, fresh boot: `{"database", null, null, false}`;
|
|
- password-only settings patch → still `false` (re-authenticate after setting a non-empty password — that patch revokes sessions and turns authentication on);
|
|
- a **mixed** patch (password + a restart-required key) → `true` (**R3-2**: re-authenticate before the status assertion — the password half revokes sessions, exactly as in the password-only case);
|
|
- a non-password settings patch → `true`;
|
|
- upstream create → `true`; upstream update → `true` (fresh process, upstream seeded first while the flag is then reset by the restart); upstream delete → `true` (fresh process, second upstream seeded directly before the probe);
|
|
- a rejected mutation (invalid body → 4xx) → `false` preserved; a mutation failing at the SQL layer preserves `false` too — **(R3-3)** the settings write fault seam is reachable from in-file tests, so pin the committed-write ordering with it; no escape clause;
|
|
- file mode (the existing `--config` fixture): `{"managed_file", path, reconciled_at != null, false}`, and a config write still 403s.
|
|
- **(D1)** S1 owns the *minimal* admin migration that keeps the tree green, nothing styled: `admin/src/lib/types.ts` (`ConfigStatus` added, `SettingsEnvelope.authority` removed), `admin/src/lib/contractSamples.gen.ts` (regenerated via `zig build test -Dintegration -Dcontract-samples-out=...`), `admin/src/lib/api.ts` (`getConfigStatus`), `admin/src/lib/queries.ts` (`configStatusQuery` + `queryKeys.configStatus`; **(D5)** declared with `staleTime: 0`, `refetchOnWindowFocus: "always"`, and a 60 s `refetchInterval` — restart truth must not depend on the mutating tab), `admin/src/features/settings/authority.ts` (`useAuthority`/`useReadOnlyConfig` retarget to the config status query, same exported names and file path — S2 moves the file, S1 does not), and `authority.test.tsx` migrated to the new source. `cd admin && npx tsc --noEmit` and `npx vitest run` green at S1 exit.
|
|
- **(R2-2)** S1 also delivers the admin side of "Reload certificates" that S2's System page consumes, since S1 owns the contract surfaces: the `api.ts` wrapper for the existing `/api/certs/reload` endpoint, its result type in `types.ts`, and — if the endpoint has no contract sample today — the `contract_sample_walk` entry for it, matching how the walk handles the other POST actions.
|
|
|
|
### S1.5 Acceptance (S1)
|
|
|
|
- [ ] `zig build test` and `-Dintegration` green; route count test at 64; openapi + docs drift green; the D4 matrix passes.
|
|
- [ ] Admin tsc + vitest green after the S1-owned migration.
|
|
|
|
---
|
|
|
|
## Session S2: task-shaped configuration, file mode, shell
|
|
|
|
Everything below follows specs/ui-redesign.md §File mode verbatim where quoted; that section is binding.
|
|
|
|
### S2.1 Routes — created and deleted in one change
|
|
|
|
- `/configuration/protection`, `/configuration/resolution`, `/configuration/system` — three routes under the shell; **no `/configuration` landing route** (a bare `/configuration` falls to the router's not-found handling; build nothing).
|
|
- Deleted in the same edit: `/groups`, `/blocklists`, `/rules`, `/local-dns`, `/upstreams`, `/settings`. No aliases, no redirects.
|
|
- Loaders (fire-and-forget `ensureQueryData`, never awaited — the m30 pattern): protection → groups + blocklists + rules + clients (**D3**: the group detail's client count needs it); resolution → upstreams + localRecords + forwardZones; system → settings. Every configuration loader also starts `configStatusQuery`.
|
|
- Tab state is URL search (`?tab=`), validated with a per-page default. **(D11)** `?group=` on Protection: positive integer; an unknown or deleted id falls back to the first group and normalizes the URL with a `replace` navigation; changing `tab` preserves `group`; user-initiated tab and group changes `push` history entries so back/forward walks them. `ui/Tabs.tsx` is uncontrolled — add a controlled prop, not a second component.
|
|
|
|
### S2.2 The three pages
|
|
|
|
**Protection** (`admin/src/features/configuration/ProtectionPage.tsx` + feature dir), tabs `groups` (default) | `sources`:
|
|
|
|
- Groups tab is group-centred per §Protection: group list (master) with selected-group detail (`?group=<id>`): effective safe-search setting, assigned blocklist sources (the existing `GroupSourcesEditor` capability), rules scoped to the selected group, and a client count from `clientsQuery` linking to `/clients?group=<id>`. **(D3)** The `?group=` *filter* on the Clients page lands in S3; within this milestone the link is complete at milestone acceptance, and S2's test asserts only the href.
|
|
- Sources tab: the shared blocklist catalogue (list, add/edit/delete in database mode) and the "Update now" runtime action with immediate action feedback (started/failed). **(D7)** `SourceStatusSection` and `refreshStore` are deleted, per §Deletions — the ephemeral status readout is gone by ruling; refresh outcomes live in Diagnostics. Their tests are intentionally dead and listed as such in the session report.
|
|
- The existing feature components are raw material, not the deliverable: capabilities move, old page shells die. One home per capability; no re-export shims.
|
|
|
|
**Resolution** (`ResolutionPage.tsx`), tabs `upstreams` (default) | `records` | `zones`: the upstream pool (create/edit/toggle/delete), local records, forward zones — the existing capabilities rehomed.
|
|
|
|
**System** (`SystemPage.tsx`), no tabs: the settings sections (the `defineSection` registry survives), restart-required key annotations from the envelope's `restart_required` list, and **(D8)** the "Reload certificates" runtime action (§File mode preserves it): the existing `/api/certs/reload` call with result/error rendering, enabled in both authority modes, with tests proving both.
|
|
**(D13)** The derived `auth_enabled` value renders as derived authentication status sourced from `web.password` — it has no ZON key and must not be shown with an invented one.
|
|
|
|
### S2.3 File mode is a rendering, not a disabled form
|
|
|
|
Per §File mode, in file mode each page renders **definition lists for scalars and tables or cards for collections, with human labels and the exact ZON key shown secondarily** (e.g. `logging.retention_days`). Binding, verbatim: "No text inputs, no checkboxes, no Add/Edit/Delete, no disabled form shells, no simulated Save." A short page note says where edits happen (the config file path) and that a restart may be needed. Runtime actions (update blocklists now, reload certificates, pause/resume, login/logout, delete an observed undeclared client) stay ordinary enabled buttons.
|
|
|
|
Mechanically: each page selects a read-only rendering branch on resolved authority — not `<fieldset disabled>` (SettingsPage.tsx:385 dies), not per-control `disabled` props (the GroupsPage/RulesPage pattern dies). A small `DefinitionList` component in `ui/` (none exists).
|
|
|
|
**(D6)** Authority is three-state in the UI: `pending | resolved | failed`. A configuration page renders neither form nor definition list until the status query resolves (skeleton/pending presentation); a failed status query renders an explicit error state with Retry, never editable forms by default. `undefined` is not database mode — the current hook's fallback dies with it.
|
|
|
|
**(R2-3)** The lock is global, not page-local: `useReadOnlyConfig` (and every mutation affordance that consults it, including the interim Clients page) treats *anything other than resolved database authority* — pending, failed, or `managed_file` — as locked. And the failure must be visible outside configuration pages too: when the config status query is in a failed state, the shell renders a compact status-unavailable indicator (beside where the restart notice would sit), so a dead `/api/config/status` cannot silently hide file authority or a pending restart on Overview/Activity/Diagnostics/Clients. Pin both with tests. **(R3-4)** The gate covers configuration renderings and configuration mutations only: authority-independent runtime actions (update now, reload certificates, pause/resume, observed-client delete, login/logout) stay visible and enabled alongside a pending or failed status.
|
|
|
|
### S2.4 Shell: authority line, lock indicator, restart notice
|
|
|
|
- **Authority line** in the Configuration sub-navigation area, exactly the §File mode shape: `File-managed · /etc/nxdns/config.zon · loaded 19 Aug 2026, 08:42` — wording is "loaded", never a claim about current file contents; `reconciled_at`, browser-local. File mode only.
|
|
- **Compact lock indicator** as a small shared component beside affected controls elsewhere (S3's Clients edit affordances consume it).
|
|
- **`ReadOnlyConfigBanner` deleted** (AppShell.tsx:274). **`RestartBanner`, `restartBanner.ts` and `patchRequiresRestart` deleted** (AppShell.tsx:273, settings/restartBanner.ts, SettingsPage.tsx:15-19 and the raise sites) — replaced by a non-dismissable shell notice when `restart_pending` is true, driven by `configStatusQuery`. **(D5)** Coherence is the query options S1 set (staleTime 0, always-refetch-on-focus, 60 s interval) plus `configStatus` invalidation in the S2.5 mutations; a notice raised by another tab or API client appears within one interval, and a process restart clears it on the next fetch — the smoke verifies the restart half. Wording: pending changes need a restart; a browser refresh must not clear it (server state — pin with a remount test).
|
|
- `NAV_ITEMS` (AppShell.tsx:19-30): Overview, Activity, Clients, Diagnostics, then a labelled "Configuration" group containing Protection, Resolution, System — a labelled group, not a collapsible tree. The six old items leave.
|
|
|
|
### S2.5 Data layer
|
|
|
|
- **(D2)** `authority.ts` moves to `admin/src/features/configuration/authority.ts`; S2 updates **every** consumer's import, and for the three modules under `features/clients/**` this is an explicitly granted exception to S3's ownership: import-path-only edits, no behavioral change.
|
|
- Mutations that can set `restart_pending` (upstreams create/update/delete, settings put) invalidate `queryKeys.configStatus` in `onSuccess`.
|
|
|
|
### S2.6 Test migration and sweep
|
|
|
|
No-shrink rule: every behavior pinned by the old page tests is re-pinned in the new homes or listed as intentionally dead with its page (D7's status section is the known dead set). Old test files: GroupsPage, BlocklistsPage, BlocklistForm, RulesPage, LocalDnsPage, UpstreamsPage, SettingsPage, RestartBanner, authority (migrated in S1), AppShell. New pinned behaviors at minimum:
|
|
|
|
- Tab and group selection are URL state; back/forward restores them; group survives tab change; unknown group falls back with a replace (D11).
|
|
- **(D12)** File mode renders zero mutation controls: assert the configuration content area contains no `input`, `textarea`, `select`, `[role="combobox"]`, `[role="checkbox"]`, `[contenteditable]`, and no `button` outside an explicit allowlist (tab triggers, the named runtime actions, Retry). Definition lists show ZON keys.
|
|
- Authority pending shows no form; authority failure shows the error state, not editable forms (D6).
|
|
- The authority line renders in file mode only; the restart notice appears from config status and survives remount; deleted routes are gone.
|
|
- Loaders fire-and-forget.
|
|
|
|
### S2.7 Acceptance (S2)
|
|
|
|
- [ ] tsc, vitest, oxlint, prettier clean; `npm run build` and `zig build -Dadmin-dist=admin/dist` green.
|
|
- [ ] No import, route string or nav reference to `/groups`, `/blocklists`, `/rules`, `/local-dns`, `/upstreams`, `/settings` (API paths `/api/*` stay).
|
|
- [ ] S2.6 behaviors pinned.
|
|
|
|
---
|
|
|
|
## Session S3: Clients redesign
|
|
|
|
### S3.1 `/clients` and `/clients/$id`
|
|
|
|
Per §Clients: Clients keeps primary navigation because naming unknown devices is operations, not configuration.
|
|
|
|
- `/clients` list: identity-first — address, name with the *learned* marker (only place the marker appears; query tables keep unmarked `ClientName`), group, first/last seen. **(D3)** Implements the `?group=<id>` filter Protection links to (validated positive integer; unknown id shows an empty-filter state with a clear-filter action, not a crash). Edit/delete follow file-mode rules (declared clients read-only in file mode with the lock indicator; observed-client delete stays a runtime action).
|
|
- **(D9)** `/clients/$id` detail: `$id` is the **numeric row id** (the identifier `PUT /api/clients/{id}` already uses). Data comes from `clientsQuery` — the loader `ensureQueryData`s it, so a cold deep link fetches the list once; an id not in the loaded list renders an explicit missing-client state (deleted or never existed), no crash, no retry loop. Content: identity (name, learned/declared/edited provenance, first/last seen), effective policy (group, link to `/configuration/protection?group=`), and an Activity link `/activity?mode=history&client=<addr>&since=<now-24h>&until=<now>` with absolute bounds computed at click time. No new API.
|
|
- **Network assignments**: the prefix-assignment section renames from "Prefixes" to "Network assignments" (§Clients), stays on the list page below the table, existing editor semantics in database mode, definition rendering in file mode.
|
|
- **(D9, D14)** S3 owns `admin/src/features/clients/**`, `admin/src/routes.tsx` (the clients route edits), `CHANGELOG.md`, and — if a list-to-detail selector needs a home — a minimal additive touch of `admin/src/lib/queries.ts`. S3 does not touch `AppShell.tsx` (the nav item already exists and covers the child route).
|
|
|
|
### S3.2 Acceptance (S3)
|
|
|
|
- [ ] tsc, vitest, oxlint, prettier clean; builds green.
|
|
- [ ] Old ClientsPage tests migrated under the no-shrink rule; new: cold deep link to `/clients/$id` fetches and renders (test with an empty cache); missing id → missing-client state; activity link carries absolute bounds; `?group=` filters and the unknown-group state renders; learned marker appears on Clients pages only.
|
|
- [ ] **(R2-4)** File-mode Clients coverage, explicitly: no edit affordance for observed or declared clients; observed-client delete stays enabled (runtime action); declared-client delete is locked; Network assignments render with zero mutation controls; pending or failed authority exposes no *configuration* mutation control (the R2-3 lock) — observed-client delete, an authority-independent runtime action, stays enabled throughout (R3-4). The client-authority assertions currently living in the settings `authority.test.tsx` are re-pinned here. **(R3-1)** Placement: when S2 moves `authority.test.tsx` to `features/configuration/`, it moves the *clients-specific* assertions into `features/clients/ClientsPage.test.tsx` instead — a test-only exception to S3's ownership, adjusting mechanics only — so S3 owns every clients test it must rewrite and never edits `features/configuration/`.
|
|
|
|
---
|
|
|
|
## Smoke (orchestrator, after S3)
|
|
|
|
Real binary + built bundle, both modes:
|
|
|
|
1. Database mode (no `--config`): walk Protection/Resolution/System editable forms; create an upstream → restart notice appears and survives reload; restart the process → notice gone (D5). Screenshots: the three pages, the nav group, the restart notice, desktop + 390 px.
|
|
2. File mode (smoke config): the three pages render definition lists (no mutation controls), authority line shows path + loaded time, a config API write 403s, "Update now" and "Reload certificates" still work (D8). Screenshots: Protection and System in file mode.
|
|
3. Clients: list with learned marker, `?group=` filter from Protection, detail page via cold deep link, Clients→Activity bounds link. Screenshots.
|
|
4. Screenshots shown to the owner before any commit (standing rule).
|
|
|
|
## File ownership
|
|
|
|
S1: `src/web/**`, `docs/reference/api.md`, `admin/src/lib/types.ts`, `admin/src/lib/contractSamples.gen.ts`, `admin/src/lib/api.ts`, `admin/src/lib/queries.ts`, `admin/src/features/settings/authority.ts`, `admin/src/features/settings/authority.test.tsx` (D1). S2: `admin/src/**` except `features/clients/**` — with two narrow exceptions inside `features/clients/**`: import-path-only edits (D2), and the R3-1 test-only move of the clients-specific authority assertions into `features/clients/ClientsPage.test.tsx`. S3: `admin/src/features/clients/**`, `admin/src/routes.tsx`, minimal additive `admin/src/lib/queries.ts`, `CHANGELOG.md` (D9, D14). Sequential, so the shared files never see two writers at once.
|
|
|
|
## Anti-requirements
|
|
|
|
- No `/configuration` landing route; no route aliases or redirects for the six deleted routes.
|
|
- No disabled form shells, no simulated Save, no full-width authority banner (§File mode contract).
|
|
- No new server mutation endpoints; no restart endpoint; no persistence for `restart_pending`.
|
|
- No client-side authority guessing from 403s — authority comes from `/api/config/status` only, and unknown is never editable (D6).
|
|
- No Overview/Activity/Diagnostics changes; step 6 (contract closure) stays out.
|
|
- No new dependencies.
|
|
|
|
## Acceptance (milestone complete)
|
|
|
|
- [ ] All session gates green; `zig build test -Dintegration` green end-to-end (contract samples byte-compare included).
|
|
- [ ] Smoke walked in both modes with screenshots shown to the owner.
|
|
- [ ] CHANGELOG updated.
|