Goal: remove the dead surface, unify the re-hardcoded constants, close the small silent failures, and fix the frontend state hazards — the residue of TECH_DEBT.md Theme 8 plus the remaining lows, after the behavioral fixes (m16), the contract work (m17) and the duplication refactors (m18) have landed.

PROVISIONAL. This spec was written before milestones 16-18 were built. It assumes: m17 shipped BADVERS (so edns.extendedRcode has a production caller), m17 built the upstream editor (so the queries.ts upstream factories have consumers and are not deleted here), and m18 shipped the frontend ui/ extraction and deleted doh_server's dead serve(). Before starting this milestone, re-verify every line reference and update this spec where the earlier milestones moved things.

Rulings (binding)

1. The dead ECS parse surface is deleted

src/dns/edns.zig: delete parseEcs (:111), Ecs (:98-105), EcsError (:107), ecs_family_ipv4/ecs_family_ipv6 (:95-96), and their in-file tests (:371-:469 block, nine tests). stripEcs filters by option code without decoding, forward mode keys on raw bytes — no production path decodes ECS.

Trap the audit missed: tests/fuzz/dns_fuzz.zig:86 calls edns.parseEcs inside a fuzz target that the default test step builds. Delete that line in the same change or the build breaks.

extendedRcode (:272) is kept: milestone 17 gave it the BADVERS caller. If m17 diverged and BADVERS was not built, delete it too and note the divergence here.

Precedent: ecsPayload was built and removed within milestone 7 when it lost its only caller (specs/milestone-7.md:361). Git history keeps the code.

2. `writeSettings` returns an error union

src/web/handlers/settings.zig:349 returns ?db.Error, so every exit is a value return and the errdefer at :354 never fires — it implies protection it cannot provide. The real safety is two explicit tx.rollback() calls (:358, :363 — the audit said three; it is two).

Change the return type to db.Error!void. The errdefer becomes live (db.Tx.rollback at storage/db.zig:814 is idempotent and documented safe in an errdefer), and the two manual rollback-then-return arms collapse into plain try/error returns. The one caller (applyPut, :327) switches from if (writeSettings(...)) |err| to catch; its hash-free-on-failure behavior is unchanged.

3. The logger's field widths become the single source

src/storage/logger.zig:44-48 holds four file-private constants (max_domain_len = 253, max_client_len = 45, max_reason_len = 32, max_upstream_len = 64). Make all four pub. Then:

src/server/handler.zig:67 deletes its local max_reason_len = 32; the comptime guard at :69-75 checks logger.max_reason_len, so the guard finally proves what its comment claims.
src/server/handler.zig:79 deletes its local max_ip_text = 45 and uses logger.max_client_len (the :82 max_resolver_text derivation follows).
src/server/clients.zig:32 likewise.
src/web/handlers/queries.zig:27-30 — the fifth copy the audit missed — adopts logger.max_domain_len for the domain width. Its max_client_len is 64, not 45; the query log's client column is written by the logger and can never exceed 45 bytes, so it adopts logger.max_client_len too. If a test proves a wider value ever reaches that field, stop and record the divergence instead of forcing it.

Acceptance is a grep: the literals 253, 45 and 32 appear in these roles only in logger.zig.

4. `track()` reports fullness; one mutex acquisition per query

src/server/handler.zig:255-258 takes the tracker mutex twice per query: tracker.track(io, from) and then tracker.snapshotStats(io) — a full struct copy — solely to mirror dropped_full into the handler's atomic tracker_full (:150-153).

Change Tracker.track/trackAt (src/server/clients.zig:89-97) to return u64: the current dropped_full, read under the mutex they already hold. The handler stores that return value. snapshotStats stays for /metrics.

5. Log truncation gets a marker and a counter

src/platform/logging.zig:285-288: mw.print(format, args) catch {} on a fixed 2048-byte writer, then the partial buffer is used with no marker — the module's own never-both-discarded-and-silent invariant, violated in its own kitchen. On error.WriteFailed: overwrite the final three bytes of the buffered message with ... (the safe_url.zig marker, src/safe_url.zig:11-14) and increment a new state.stats.lines_truncated, sibling to lines_deduped (:295). One test: a message over 2048 bytes ends in ... and bumps the counter by one.

6. TLS errors are classified by name, not by name prefix

src/filter/fetcher.zig:169-170 and src/upstream/doh_client.zig:160-161 classify TLS failures with startsWith(@errorName(err), "Tls") / "Certificate" — a std rename silently downgrades TLS failures into generic buckets. Verified reachable set from std.http.Client (0.16.0) today: exactly error.TlsInitializationFailed and error.CertificateBundleLoadFailure. But this milestone lands after milestone 18, whose ruling 4 makes the DoH path unwrap the client's stashed read cause — and that cause set includes the record-layer members (TlsAlert, TlsBadRecordMac, TlsDecodeError, ...) from std.crypto.tls.Client. A two-member switch written against today's surface would downgrade those to generic receive failures.

In both files: replace the prefix match with an exact switch → error.TlsFailed naming the two top-level members plus, on whichever paths m18's cause unwrap surfaces, the Tls* members of the unwrapped read-cause set — enumerated at implementation time from the pinned std sources against the m18 code as landed, not from this spec's list. The else falls through to the existing per-phase mapping. Rewrite both tests (fetcher.zig:311-313, doh_client.zig:261-263): they currently assert on CertificateExpired, which nothing can produce, and omit CertificateBundleLoadFailure; the new tests use only names from the verified reachable set, including a stashed record-layer cause on the unwrapped path.

If m18 already extracted a shared mapError into transport.zig, apply this ruling to the shared copy instead and note it here.

7. The transcribed mbedTLS values get an init-time check

src/platform/tls_server.zig:501-511 hand-transcribes four config enums and six error codes from the pinned Mbed TLS 3.6.7 headers with no drift guard — unlike the sizes and alignment, which the shim reports. A renumbered close_notify after a version bump would silently invert truncation detection (:286, :351).

src/platform/mbedtls_shim.c gains ten getters in the existing shape (int nx_const_ssl_transport_stream(void) { return MBEDTLS_SSL_TRANSPORT_STREAM; } etc. — the needed headers are already included). tls_server.zig verifies all ten against the transcribed constants next to the existing alignment assert at :78, and the :657 test block asserts them too. Cheap insurance, one-time cost.

8. The reload lock invariant is written down

Fourteen call sites in four handler files (groups, rules, clients, blocklists — the audit said five handlers; it is four files, fourteen sites) call mutations.reload after releasing config_lock. That is safe only because the production reload_fn re-reads the database under the manager's own writer lock — a cross-module fact stated nowhere. local.zig is the deliberate opposite (publish under the lock; the ordering rationale lives at local.zig:80-85).

Extend the doc comment on mutations.reload (src/web/handlers/mutations.zig:122-125): the reload_fn contract is that it re-reads all state from the database itself; it must never accept pre-read rows (contrast swapLocalTables, which is the pre-read shape and is why local.zig holds the lock). A future signature change to reload that adds row-passing breaks fourteen call sites' correctness — the comment must say exactly that. Documentation only; no locking change.

9. `web_dev_dir` moves into WebState

src/app.zig:605-609 holds the --web-dev directory in a module-level mutable global because the fallback handler "has no closure" — but serveWebDev (:613-620) receives *WebState and discards it. Add dev_dir: []const u8 = "" to WebState (src/web/server.zig, beside version), set it at the composition root, read it in serveWebDev, delete the global and its apologia comment.

10. The private key PEM is wiped

src/server/cert_store.zig:326 frees the key PEM without zeroization, and readPem (:345-362) uses readFileAllocOptions, whose grow-as-you-read can leave intermediate copies in freed pages — a wipe at the call site cannot reach them. Split the key path: a readKeyPem that reads through a single fixed max_pem_bytes + 1 buffer (no reallocation), copies to an exact-size allocation, and std.crypto.secureZeros the big buffer before returning; the caller wipes the returned slice before gpa.free. The idiom precedent is auth.zig:228. The certificate PEM path stays as-is — it is public material. Runs at boot and on real renewals only (:311-312 stats gate the poll).

11. Header-array overflow asserts

src/web/http_util.zig:335 maps a too-long extra_headers onto error.OutOfMemory — a future eighth header would surface as mysterious OOM-labeled drops. Every one of the 62 call sites passes a compile-time count, maximum 3 (static.zig:174, +1 for content-type = 4 of 8). Replace the check with std.debug.assert(extra_headers.len + 1 <= headers.len); so the mistake fails loudly in tests.

12. Shipped `.gz` siblings are verified; orphans are rejected

tools/gen_web_assets.zig:71-77 embeds a dist-shipped .gz sibling with zero content checks, and an orphan .gz (no base file) is silently embedded as unreachable application/octet-stream bytes. Exposure is latent (no frontend compression plugin today) — close it while it is cheap:

Sibling: decompress with std.compress.flate.Decompress (container .gzip; the tool already uses the Compress side at :133) and byte-compare against the base file; std.process.fatal on mismatch, matching the tool's existing error style (:73-75).
Orphan: std.process.fatal naming the path, instead of indexing it.

Extend the embedded-dist test (src/web/static.zig:413-439) to decompress each .gz entry and compare with its base — it currently checks magic bytes and size only.

13. The Dockerfile arch default comes from the host

deploy/docker/Dockerfile:21: ${TARGETARCH:-amd64} — under the legacy builder TARGETARCH is empty, so a plain docker build on the Pi 5 packages the x86_64 binary and dies at docker run with exec-format, far from the mistake. Replace the default with a uname -m mapping resolved before the case:

RUN arch="${TARGETARCH:-}"; \
    if [ -z "$arch" ]; then case "$(uname -m)" in \
        x86_64) arch=amd64 ;; aarch64) arch=arm64 ;; \
        *) echo "unsupported build host $(uname -m); use buildx" >&2; exit 1 ;; \
    esac; fi; \
    case "$arch" in ...existing arms... esac

The existing unsupported-arch arm stays fail-fast.

14. Frontend: the settings registry is typed

web/src/features/settings/SettingsPage.tsx — SectionDef exists (:21) but FieldDef.key is string (:16), driving as Record<string, unknown> casts at :217, :226, :261. A typo'd key compiles and breaks the field. Make the pair generic:

interface FieldDef<S extends keyof Settings> {
    key: keyof Settings[S] & string;
    kind: "number" | "text" | "boolean" | readonly string[];
}
interface SectionDef<S extends keyof Settings> { section: S; title: string; fields: readonly FieldDef<S>[]; }
function defineSection<S extends keyof Settings>(def: SectionDef<S>) { return def; }

SECTIONS (:35-118, 12 sections) becomes a tuple of defineSection(...) calls, TLS_FIELDS (:27) becomes FieldDef<"doh_server" | "dot_server">[] compatible, and the three Record<string, unknown> casts are deleted. The per-branch value casts inside FieldRow (:141, :160, :174, :198) may stay — they are narrowing on kind, not drift holes. Acceptance: renaming a Settings field makes tsc fail on the registry.

15. Frontend: the settings baseline is frozen

Same file, :220 — buildSettingsPatch(data.settings, edited, ...) diffs the mount-time clone against live query data, so a background refetch makes out-of-band changes appear as user edits and Save silently reverts them. Add a baseline state initialized with the same structuredClone and reset in the same onSuccess (:234-241) where edited resets; the patch becomes buildSettingsPatch(baseline, edited, ...). settingsDiff.ts is unchanged.

16. Frontend: refresh status leaves the query cache

web/src/features/blocklists/BlocklistsPage.tsx:33 reads the refresh snapshot with non-subscribing getQueryData from a cache entry that has no queryFn, no subscriber, and the default 5-minute gcTime — after which the page claims no refresh ever ran. This is client UI state. New module web/src/features/blocklists/refreshStore.ts in the exact shape of settings/restartBanner.ts (module-level value + listener Set + useSyncExternalStore hook), holding SourceStatus[] | null. The mutation (blocklistsUpdateNowMutation, lib/queries.ts:156) writes the store instead of setQueryData; the page subscribes via the hook; the queryKeys.blocklistSources entry and its doc comment are deleted. Note the prefix-invalidation side effect disappears with it (invalidateBlocklistWorld invalidating ["blocklists"] used to mark the entry stale — harmless, since the entry could never refetch).

17. Frontend: one default-group helper

Four encodings of "group 1", two semantics (GroupsPage.tsx:14 named constant; PrefixesEditor.tsx:21 and LookupPage.tsx:133 byte-identical prefer-id-1 expressions; RulesPage.tsx:24 first-of-list — which, because the API orders by name (groups_repo.zig:148), preselects the alphabetically first group, not the default). New module web/src/lib/defaultGroup.ts:

export const DEFAULT_GROUP_ID = 1;
export function defaultGroupId(groups: readonly Group[]): number {
    return groups.find((g) => g.id === DEFAULT_GROUP_ID)?.id ?? groups[0]?.id ?? DEFAULT_GROUP_ID;
}

All four sites adopt it; RulesPage's preselect bug goes away with the adoption.

18. Frontend: the dashboard primes without throwing

web/src/routes.tsx:97-102 — the dashboard loader's Promise.all over four ensureQueryData calls means one failing endpoint blanks the whole page with RouteError on cold navigation, defeating the page's own per-widget degrade (DashboardPage.tsx:68-96, which uses useQuery precisely to allow it). Change only the dashboard loader to Promise.allSettled. The five other Promise.all loaders stay: their pages use useSuspenseQuery and have no granular fallback to preserve.

19. Frontend: the query-key table is complete

web/src/lib/queries.ts — eight raw literals, all in this file: seven ["lookup"] prefix-invalidations (:99, :125, :132, :159, :167, :189, :211) and one ["groups"] (:133). Add lookupAll: ["lookup"] as const to queryKeys, use it at the seven sites, and use queryKeys.groups at :133. Acceptance: no ["lookup"]/["groups"] literal outside the table.

20. Frontend: the form catch swallows only the expected class

web/src/features/blocklists/BlocklistForm.tsx:21-33 — the bare catch {} wraps both the await onSubmit and the reset setState calls; anything but the mutation rejection dies silently with no console trace. House precedent is auth/store.tsx:85-89 (swallow only the 401 it expects, rethrow the rest), pinned by two tests. Restructure: try { await onSubmit(...) } catch (error) { if (error instanceof ApiError) return; throw error; }, resets after the try. The page's <InlineError> keeps rendering the mutation error as today.

Carried in from milestone 17: the stale phase-comment sweep

Milestone 17 ruling 7 rewrote the "Phase 7/8" headers in three files; its repo-wide acceptance grep then found 24 more stale phase references in files its sessions did not own, recorded here for this milestone: safesearch.zig:58, forward_client.zig:7, dns_cache.zig:10/:42/:435, cli.zig:332, filter_integration_test.zig:1391, logger.zig:178/:303, retention.zig:7/:137/:151, disk_monitor.zig:7, querylog_schema.zig:71, db.zig:272, manager.zig:292/:613/:1145, and the six repository files (groups_repo.zig:7, rules_repo.zig:15, sources_repo.zig:9, upstreams_repo.zig:7, local_repo.zig:3, clients_repo.zig:12-13). Each comment is rewritten to state the as-built truth it gestures at (not deleted, unless it says nothing beyond the phase number). Line numbers are hints from m17-time HEAD; re-grep before acting. Acceptance: "Phase 7" and "Phase 8" appear in no source doc comment repo-wide; spec files and TECH_DEBT.md are exempt. The session owning each file in the plan below picks up its share; files owned by no session fall to S4.

Also carried from milestone 17 (ruling 4 residual): the login log line in src/web/handlers/auth.zig prints the socket peer, which reads as loopback for every login behind a trusted proxy; it should print request.client_addr.

Sessions

S1-S4 run in parallel; no two sessions write the same file.

Session S1: backend surface

Owns src/dns/edns.zig, tests/fuzz/dns_fuzz.zig, src/web/handlers/settings.zig, src/web/handlers/mutations.zig, src/web/http_util.zig, src/server/cert_store.zig, src/app.zig, src/web/server.zig. Rulings 1, 2, 8, 9, 10, 11.

Session S2: constants, counters, classification

Owns src/storage/logger.zig, src/server/handler.zig, src/server/clients.zig, src/web/handlers/queries.zig, src/platform/logging.zig, src/filter/fetcher.zig, src/upstream/doh_client.zig, src/platform/tls_server.zig, src/platform/mbedtls_shim.c. Rulings 3, 4, 5, 6, 7.

Session S3: frontend

Owns everything under web/src/. Rulings 14-20.

Session S4: tools and deploy

Owns tools/gen_web_assets.zig, src/web/static.zig, deploy/docker/Dockerfile. Rulings 12, 13.

Orchestrator

Re-verifies this spec's line references against post-m18 reality before S1-S4 start; strikes the closed findings in TECH_DEBT.md after.

Module layout

New files:

web/src/features/blocklists/refreshStore.ts — refresh snapshot store.
web/src/lib/defaultGroup.ts — default-group constant and helper.

Deleted surface: parseEcs/Ecs/EcsError/ecs_family_* (edns.zig), the web_dev_dir global (app.zig), the queryKeys.blocklistSources entry (queries.ts).

Acceptance (milestone complete)

parseEcs and its family are gone; zig build test compiles and passes (the dns_fuzz caller was removed in the same change).
writeSettings returns db.Error!void; the errdefer is the only rollback path; the settings PUT integration tests still pass. A fault test forces a failure after Tx.begin (a fault seam under builtin.is_test, the milestone-15 rotation-seam shape) and proves the errdefer rolled the transaction back: a subsequent Tx.begin on the same connection succeeds.
Grep proves the field widths exist once: 253/45/32 in their logger roles appear only in logger.zig.
One tracker-mutex acquisition per query: track returns dropped_full; /metrics output for the tracker family is unchanged.
An oversized log message ends in ... and increments lines_truncated; the new test pins both.
Both TLS mapError switches name exactly the two reachable errors; the rewritten tests use reachable names only.
The ten mbedTLS constants are verified at init against shim getters; a deliberately wrong transcription fails the :657 test block.
mutations.reload's doc comment states the re-read contract and the fourteen-call-site consequence.
WebState.dev_dir replaces the global; --web-dev serving still works (manual check with a dev dir).
The key PEM path reads through a fixed buffer and is wiped before free; no readFileAllocOptions remains on the key path.
respondBytes asserts instead of returning OutOfMemory.
gen_web_assets rejects a corrupted shipped sibling and an orphan .gz (both proven with a doctored dist in a temp dir); the embedded test decompresses and compares.
docker build (legacy builder, no buildx) on this host produces a runnable image; the Dockerfile has no bare :-amd64 default.
tsc fails when a Settings field named in SECTIONS is renamed (proven, then reverted).
The settings baseline freezes at mount: a simulated background refetch with out-of-band changes produces no phantom patch entries (frontend test).
The refresh snapshot survives past gcTime: a frontend test renders the status 6+ minutes of fake time after the mutation.
All four group-id sites import defaultGroup.ts; RulesPage preselects the id-1 group when present (frontend test with a group sorting before "default").
The dashboard renders per-widget errors on cold navigation with one endpoint failing (frontend test); the other five loaders are unchanged.
No ["lookup"] or ["groups"] literal outside queryKeys.
BlocklistForm rethrows a non-ApiError (frontend test, mirroring the logout pair).
Full suite green: zig build test -Dintegration, npm run test, npm run typecheck, npm run lint.

Recorded (implementation)

Accepted deviations and findings from the built milestone.

Ruling 1 (S1)

The edns.zig test block held six parseEcs tests, not nine. A seventh use lived inside "encodeOpt round-trips through record parse and parseOpt", which is not a parseEcs test; its last two lines now compare the found option's bytes directly.
extendedRcode stays, with a nuance the ruling's premise missed: the BADVERS production path uses the write-side splitRcode; extendedRcode is the read-side verifier whose callers are the two BADVERS wire-format tests (handler.zig, packet.zig). Deleting it would weaken those tests.
The write-fault seam for the rollback test has no exported control surface — the test lives in the same file and needs none.
The ruling-8 doc comment names local.zig's publish function instead of a line number, so it cannot go stale.

Rulings 3-7 (S2)

Ownership amendment: src/web/metrics.zig (owned by no session) was granted to S2 mid-flight. Ruling 5's new counter raises the /metrics sample count 29 → 30 (nxdns_log_lines_truncated_total); the hard-coded assertion became fully derived (1 + dns_stat_fields.len + @typeInfo(LoggerCounters)... + @typeInfo(logging.Stats)...), so a new counter in any of those structs extends the exposition and the assertion together.
Ruling 6 lands asymmetrically, following the ruling body over the stale acceptance bullet: doh_client.zig names eleven members (the two collapsed top-level ones plus the nine Tls* members of the unwrapped read-cause set); fetcher.zig names only the two, because it has no cause unwrap and no record-layer member can reach it. Both files carry a comment pointing at the other.
Addition beyond the ruling: an exact switch does not catch a std rename — error.X in an expression names a member into existence, so a renamed member leaves the switch compiling and matching nothing. Comptime guards close this for real: doh_client.zig asserts every member of std.crypto.tls.Client.ReadError maps to TlsFailed, and both files assert the two collapsed names still exist in std.http.Client.RequestError.
The ruling's "stashed record-layer cause through the unwrap" test is not constructible: the stub connection is .plain on purpose, and a plain connection's stashed error type has no Tls* member. The record-layer set is instead tested by an inline for over the std error set against mapError — all nine members, not one sample.
Ruling 3 behavioral note: queries.zig's client width dropped 64 → 45 as directed; a ?client= filter of 46-64 bytes now returns 400 instead of matching nothing. src/filter/wildcard.zig's max_pattern_len = 253 is a rule-pattern limit, not a logger role, and stays.

Rulings 14-20 (S3)

Ruling 14: the three Record<string, unknown> consumption casts collapse into one documented cast inside sectionValues rather than zero — iterating the heterogeneous registry erases the per-section correlation TypeScript needs, and TlsListenerSettings (a named interface, no implicit index signature) blocks the assignment route with types.ts off-limits. Four casts before, one after.
Ruling 20: the catch body lives in an exported swallowMutationError helper because jsdom never fires unhandledrejection, so a rethrow from an async submit handler cannot be asserted directly; the helper is tested directly, mirroring how auth/store.tsx's logout pair is tested.
The refresh store notifies synchronously inside onSuccess, one flush before react-query's success dispatch; one pre-existing assertion moved from getByText to await findByText. The sub-frame transient was judged cheaper than batching through notifyManager.
lib/queries.ts imports features/blocklists/refreshStore.ts (lib → features) because the spec directs the mutation factory to write the store; recorded as sanctioned layering.
SourceStatusSection's prop is SourceStatus[] | null to match the store; BlocklistsPage tests clear the module-level store in beforeEach.
Ruling 18: post-m18 there are four other Promise.all loaders, not five.

Rulings 12, 13 and carry-ins (S4)

The undecodable-sibling message reports decompress.err orelse err, because std.Io.Reader collapses every decode failure into error.ReadFailed and stashes the real one.
The Dockerfile keeps its && chain instead of the spec sketch's ; separators, so any failed step still aborts the layer; the case arms are untouched.
The phase-comment sweep leaves PLAN.md:605/:609 alone — they are the section headings that define Phase 7 and Phase 8, not stale comments.
The orchestrator added the host-fallback sentence to docs/how-to/install-with-docker.md.

Residual (named, not fixed)

fetcher.zig hands the collapsed ReadFailed/WriteFailed to mapError with no cause unwrap, so a blocklist download canceled at shutdown returns error.ReceiveFailed instead of error.Canceled — the class m18 ruling 4 fixed for DoH. Consequence: a wrong source status and a spurious warn line at shutdown. Fixing it needs req/resp plumbing at four call sites; it deserves its own ruling.

Anti-requirements

No deletion of extendedRcode (m17 gave it a caller) and no deletion of the upstream mutation factories (m17 built their consumer).
No locking change in the reload handlers — ruling 8 is documentation.
No new frontend dependencies; the store is hand-rolled like restartBanner.
No compression plugin for the frontend build; ruling 12 guards the seam, it does not start using it.
No useInfiniteQuery migration and no QueryLogPage work — that was milestone 16.
No renaming of settings keys or API fields — typing the registry must not change the wire format.

27 KiB Raw Permalink Blame History

Milestone 19: hygiene sweep