Files
nxdns/specs/milestone-33.md
T
mokhtar cc23c97218
Gates / frontend (push) Successful in 1m34s
Gates / test (push) Successful in 2m3s
Gates / test-aarch64 (push) Failing after 3h13m33s
Gates / package (push) Successful in 5m20s
Gates / container (push) Successful in 15s
CI / gates (push) Failing after 6h30m45s
milestone 33: contract closure — samples, file-authority enumeration, dead code, bundle ceiling
2026-08-22 23:31:37 +02:00

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 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.

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.

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).