milestone 33: contract closure — samples, file-authority enumeration, dead code, bundle ceiling

specs/milestone-33.md

@@ -0,0 +1,104 @@
+# 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 `Provenance` schema (openapi.yaml:2169) is referenced by no path, and there is no honest place to wire it: `/api/queries/{id}` already refs `QueryDetail`, and `/api/queries/live` is an SSE byte stream whose response schema must stay `type: string` — a `$ref` there would falsely document the response as one JSON object. **Delete the schema.** Before deleting, compare its field documentation against `QueryDetail` and the live endpoint's description; fold any information that exists only in `Provenance` into 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:63` stays — historical anecdote about a page that existed then.
+
+### S1.5 Acceptance (S1)
+
+- [ ] `zig build test` and `-Dintegration` green; contract byte-compare green; sample count grew by exactly seven.
+- [ ] The enumeration test covers all `config_write` routes 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.ts` `updateRule` (: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.ts` `GroupSources` (:399) deleted.
+- `queryKeys` stays exported — it has real external consumers (`features/pause/protection.ts`, plus tests/fixtures in `PauseControl.test.tsx`, `SystemPage.test.tsx`, `features/clients/testFixtures.tsx`). No change.
+- `admin/src/features/queries/` renamed to `admin/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 to `features/queries/...` in `admin/src/lib/types.ts` comments 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 `relatedBounds` and `diagnosticsBounds` (`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 to `validateActivitySearch` (`features/activity/search.ts`) round-trips to the same applied filter — the emitted `since`/`until` are accepted as safe integers and land as the applied window. This is the admin half of the server's `since <= ts < until` contract 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 build` green 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`, the `checkClientIp` test, the `Provenance` drift assertion — and the rename's import updates are the whole allowance).
+
+## Implementation notes (post-build sync)
+
+- The `apply*` sweep also covered `rules.zig`, `upstreams.zig`, `auth.applyLogout`, and `pause.apply` — the S1.3 list missed them; all verified reference-free. `certs.applyReload` stays `pub` (external test caller).
+- `http_util.decodeInPlace`/`queryPairs` stay `pub`: `tests/fuzz/http_util_fuzz.zig` imports the module (build.zig:154) — the inventory only scanned `src/`.
+- The drift guard's negative control (`NullableObject`) was retargeted at `QueryDetail` plus an added `id` field rather than deleted, keeping the proof that the guard sees through a `$ref`.
+- `configuration.md` had two stale "query-log page" occurrences in the same section; both fixed.
+- `fileModeClasses` became `fileModeConfigWrites`; it replays the `contract` table's real target+body per `config_write` route and asserts case count == `router.routes` count (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`, and `zig 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).