12 KiB
Milestone 33: contract closure
Redesign step 6 of specs/ui-redesign.md (build-sequence step 6): the closure sweep. Remove what the redesign obsoleted, close the contract-sample gaps, add the cross-surface acceptance tests, wire a byte-budget gate, and fix the stale doc references. No behavior changes, no new dependencies, no new endpoints. This milestone ends the redesign; a release cut follows it.
Grounded in a full-tree inventory (2026-08-22); the m32 deletion left almost nothing orphaned, so the sweep is small and the acceptance tests are the substance.
Sessions
S1 (Zig + contract surfaces + docs) and S2 (admin sweep + link tests + byte budget) run in parallel — no shared files. S1 owns admin/src/lib/contractSamples.gen.ts (regeneration) and nothing else under admin/; S2 does not touch that file. Neither session runs the admin typecheck as its own gate — tsc -b reads the whole admin tree and writes .tsbuildinfo, so it cannot run against a tree the other session is editing. The orchestrator runs tsc, vitest, and the full build once after both sessions land (see milestone acceptance).
Session S1: Zig closure, contract samples, file-authority enumeration, docs
S1.1 Contract-sample gaps
contract_sample_walk (web_integration_test.zig:3707) leaves exactly seven successful single-resource GETs unsampled: /api/groups/{id}, /api/blocklists/{id}, /api/rules/{id}, /api/local-records/{id}, /api/forward-zones/{id}, /api/clients/{id}, /api/upstreams/{id} (/api/queries/{id}, /api/diagnostics/{id}, /api/groups/{id}/sources are already sampled). Add the seven, each inserted directly after the walk's existing create of that resource (clients have no POST — place the client sample after the existing client list sample) — seedConfig and the walk already supply every needed row (client and upstream from seedConfig, group 1 from schema creation, the rest created mid-walk); do not add new seed state, which would shift unrelated goldens. Use the existing ts_type conventions so writeSampleImports stays correct. Excluded with stated reasons in a comment beside the walk: /metrics (Prometheus text, not a JSON contract), /api/openapi.yaml (served verbatim, drift-tested elsewhere), /api/queries/live (SSE stream, not byte-sampleable). Regenerate admin/src/lib/contractSamples.gen.ts with the documented command. S1 does not run the admin typecheck (see §Sessions); if a new sample exposes a server/types mismatch, that is a server bug for S1 to fix — types.ts belongs to S2.
S1.2 File-authority enumeration
Replace the 4-route spot check (fileModeClasses, web_integration_test.zig:1177-1210) with an enumeration: the test iterates every route whose policy == .config_write from router.routes (22 today) and asserts each returns the 403 managed-file body in file mode. A valid body cannot be built generically per method — the existing contract table already carries a valid concrete target and body for every route and is drift-checked against router.routes; reuse those per-route cases (or extend that table with what the 403 walk needs) so the 403 is provably the router's, not a 400. The test asserts its case count equals the table's config_write count, so a future config_write route cannot ship unenumerated. The one per-handler exception (clients.zig declared-row read) keeps its existing dedicated test.
S1.3 Zig visibility sweep
Un-pub the symbols with no external references (inventory list: api_limiter.isLoopback, http_util.decodeInPlace/queryPairs, router.formatAllow, server.sessionAuth/bucketLimit, static.acceptsGzip/etagMatches/diskRelativePath, stats.periodParam, health.queryHistoryState/diskState/diagnosticsUnavailable, live.writeEvent, and the apply* families in handlers/{blocklists,clients,groups,local,settings}.zig). Known corrections: auth.applyLogin is referenced from settings.zig tests — it stays pub; mutations.checkClientIp has no production caller at all (only an in-file test) — delete the function and its test, don't just un-export dead code. In-file tests keep access; verify every symbol before touching it and report any other inventory errors.
S1.4 OpenAPI + docs
- The
Provenanceschema (openapi.yaml:2169) is referenced by no path, and there is no honest place to wire it:/api/queries/{id}already refsQueryDetail, and/api/queries/liveis an SSE byte stream whose response schema must staytype: string— a$refthere would falsely document the response as one JSON object. Delete the schema. Before deleting, compare its field documentation againstQueryDetailand the live endpoint's description; fold any information that exists only inProvenanceinto the live endpoint's prose description (which is where the SSE event payload is documented). Drift tests stay green. - Stale page references:
docs/tutorial/first-run.md:214("Blocklists page" → the Protection page's Sources tab),docs/reference/configuration.md:150("query-log page"/"live view" → Activity history/live),docs/reference/api.md:109("Query log page" → the Activity surface),docs/how-to/upgrade.md:345("settings page" → the System page).docs/explanation/performance-and-testing.md:63stays — historical anecdote about a page that existed then.
S1.5 Acceptance (S1)
zig build testand-Dintegrationgreen; contract byte-compare green; sample count grew by exactly seven.- The enumeration test covers all
config_writeroutes and pins the count. - openapi drift + docs drift green.
Session S2: admin sweep, cross-surface link tests, byte budget, a11y pins
S2.1 Dead-code removals
api.tsupdateRule(:187) deleted — no edit affordance exists and none is being added; the server route stays (API completeness is a server contract, the admin client only carries what the UI uses).types.tsGroupSources(:399) deleted.queryKeysstays exported — it has real external consumers (features/pause/protection.ts, plus tests/fixtures inPauseControl.test.tsx,SystemPage.test.tsx,features/clients/testFixtures.tsx). No change.admin/src/features/queries/renamed toadmin/src/features/provenance/— no page lives there since m29; the four helper modules (provenanceCopy, qtype, querySummary, provenanceFixture) keep their names, importers updated, and the two prose references tofeatures/queries/...inadmin/src/lib/types.tscomments updated too (the grep gate covers them). Pure rename, no logic edits.
S2.2 Cross-surface link acceptance tests
Most emitters are already pinned: ActivityDetailPage.test.tsx (RelatedActions domain/client bounds and half-bounded fallback), OverviewPage.test.tsx (both stat-tile links), ClientDetailPage.test.tsx (24 h link), HealthStrip.test.tsx and DiagnosticDetailPage.test.tsx (configuration links). Do not duplicate any of them. Add only the two genuinely missing pieces:
- Unit tests directly on
relatedBoundsanddiagnosticsBounds(features/activity/relatedLinks.ts) pinning the bound arithmetic in isolation (origin-bound fallback ±300 s per bound) — today it is only pinned through component renders. Half-open inclusion is a server-side property; do not try to unit-test it here (that module performs no inclusion check). - One agreement test: an object like
{ mode: "history", domain, ...relatedBounds(origin) }passed tovalidateActivitySearch(features/activity/search.ts) round-trips to the same applied filter — the emittedsince/untilare accepted as safe integers and land as the applied window. This is the admin half of the server'ssince <= ts < untilcontract test (queries_repo.zig:1592). No router mount needed. - The session report maps each emitter to the test that pins it (existing or new).
LiveActivity's recovered-row detail link intentionally carries the live origin (mode=live, no bounds) — changing it to a bounded history link would be a behavior change and is out of scope for this milestone.
S2.3 Byte budget
New admin/scripts/assert-bundle-size.mjs, wired into npm run build before stamp-dist (a failed size check must not leave a fresh .src-hash beside an oversized bundle that a later Zig build would accept as valid): sums admin/dist/assets/* and fails above the budget. No workflow edit — the CI and release jobs already run npm run build, so the gate rides along. Budget: 800,000 bytes (current total 708,352 — ~13% headroom). One number, total bytes, no per-chunk budgets, no gzip modeling — the gate exists to catch an accidental dependency or asset landing in the bundle, not to micro-manage chunks. The script prints the total and the top five chunks on failure.
S2.4 Accessibility pins
No new tooling (no-new-deps). Verify the spec's explicit requirements are pinned and add only what is missing: donut SVGs aria-hidden + focusable="false" with the visible legend and visually hidden table as the accessible surface (m30 tests — cite or add), the empty-window text, and the activity surface's existing role/aria coverage (m29 — cite). The session report lists, for each spec accessibility clause (ui-redesign.md:60, :287), the test that pins it.
S2.5 Step-6 coverage ledger
ui-redesign step 6 also names query-log-recreation and active-event-recovery acceptance coverage. Both have substantial existing tests; do not write new ones unless the ledger finds a named behavior with no pin. The session report maps each step-6 acceptance clause to its test (file + test name), same format as the accessibility ledger — that map is what lets the milestone claim step 6 closed.
S2.6 Acceptance (S2)
- vitest, oxlint, prettier clean;
npm run buildgreen including the new size gate (tsc and the Zig dist build run post-merge by the orchestrator — see §Sessions). git grep -n "features/queries"empty outside CHANGELOG/spec history.- The S2.2 tests pass; the round-trip agreement test exists.
- CHANGELOG.md Unreleased entry for the milestone (S2 owns it): closure sweep, new acceptance tests, bundle-size gate.
File ownership
S1: src/**, docs/**, src/web/openapi.yaml, admin/src/lib/contractSamples.gen.ts (regeneration only). S2: admin/** except contractSamples.gen.ts, plus CHANGELOG.md. No workflow files change. Parallel — the sets are disjoint.
Anti-requirements
- No behavior changes; no new endpoints; no new dependencies; no a11y tooling.
- No per-chunk or gzip budgets; one total-bytes number.
- No admin affordances added to justify keeping dead client code (the rule-edit UI is out of scope).
- No rewriting of existing passing tests unrelated to this spec's tasks (the spec-required removals —
fileModeClasses, thecheckClientIptest, theProvenancedrift assertion — and the rename's import updates are the whole allowance).
Implementation notes (post-build sync)
- The
apply*sweep also coveredrules.zig,upstreams.zig,auth.applyLogout, andpause.apply— the S1.3 list missed them; all verified reference-free.certs.applyReloadstayspub(external test caller). http_util.decodeInPlace/queryPairsstaypub:tests/fuzz/http_util_fuzz.zigimports the module (build.zig:154) — the inventory only scannedsrc/.- The drift guard's negative control (
NullableObject) was retargeted atQueryDetailplus an addedidfield rather than deleted, keeping the proof that the guard sees through a$ref. configuration.mdhad two stale "query-log page" occurrences in the same section; both fixed.fileModeClassesbecamefileModeConfigWrites; it replays thecontracttable's real target+body perconfig_writeroute and asserts case count ==router.routescount (22 both sides).- S2.4/S2.5 found every accessibility and step-6 clause already pinned; the only new tests are the 18 assertions in
relatedLinks.test.ts(all mutation-checked).
Acceptance (milestone complete)
- Both sessions' gates green; post-merge the orchestrator runs
(cd admin && npm run typecheck), the full admin gate set,zig build -Dadmin-dist=admin/dist, andzig build test -Dintegration— all green. - The step-6 coverage ledger (S2.5) and accessibility ledger (S2.4) are complete.
- CHANGELOG updated. This closes specs/ui-redesign.md's build sequence; the release cut follows as its own step (owner-approved).