docs: unwrap hand-wrapped prose repo-wide
Gates / frontend (push) Successful in 1m2s
Gates / test (push) Successful in 1m38s
Gates / package (push) Successful in 5m5s
Gates / test-aarch64 (push) Successful in 6m30s
Gates / container (push) Successful in 15s
CI / gates (push) Successful in 13m30s

This commit is contained in:
2026-08-15 16:27:36 +02:00
parent 50b8fd5c61
commit 5b3d1cd65c
48 changed files with 2691 additions and 11699 deletions
+71 -370
View File
@@ -1,95 +1,35 @@
# Milestone 18: collapse the duplicated infrastructure
Goal: the copy-paste infrastructure from `TECH_DEBT.md` Theme 2 becomes
shared code — the listener core first (the audit's only high finding, four
hand-synced copies of the most invariant-heavy concurrency code in the
repository, drift already live), then the repository memory-safety
choreography, the web CRUD shells, the transport scaffolding, the name
normalization, the line iterator, and the frontend class constants. After
this milestone, a fix in any of these families lands once.
Goal: the copy-paste infrastructure from `TECH_DEBT.md` Theme 2 becomes shared code — the listener core first (the audit's only high finding, four hand-synced copies of the most invariant-heavy concurrency code in the repository, drift already live), then the repository memory-safety choreography, the web CRUD shells, the transport scaffolding, the name normalization, the line iterator, and the frontend class constants. After this milestone, a fix in any of these families lands once.
**PROVISIONAL.** Written before milestones 15-17 were built. m16 touches the
listeners (DoH receiveHead race, stats export), the manager (refresh_lock)
and the metrics tests; m17 touches pool.zig and the handlers. Re-verify
every line reference and re-diff the copies before each session starts.
**PROVISIONAL.** Written before milestones 15-17 were built. m16 touches the listeners (DoH receiveHead race, stats export), the manager (refresh_lock) and the metrics tests; m17 touches pool.zig and the handlers. Re-verify every line reference and re-diff the copies before each session starts.
## Rulings (binding)
### 1. One listener core
`tcp_server.zig`, `dot_server.zig`, `doh_server.zig` and `web/server.zig`
each carry private copies of: the `State`/`ConnState`/`Stop`/`Claim` enums,
`retry_delay`, `bump`, the slot pool with `claim`/`finish` (the same
three-step close dance), `beginShutdown` (identical body, different log
text), `serve` with the cancel-protection dance, `acceptLoop` with the same
error mapping, `decideClaim` in two shapes plus `firstFree`, and — in the
three DNS listeners — the byte-identical `Outcome`/`Result`/`race`/`expire`
select harness. `readPrefix`/`readBody`/`writeReply` are byte-identical
between tcp and dot. The predicted failure mode already fired once: the
milestone-10 review hand-ported the `handshook` TLS-context-leak fix from
dot to doh. Six unit tests are duplicated between tcp and dot, three more
between doh and web.
`tcp_server.zig`, `dot_server.zig`, `doh_server.zig` and `web/server.zig` each carry private copies of: the `State`/`ConnState`/`Stop`/`Claim` enums, `retry_delay`, `bump`, the slot pool with `claim`/`finish` (the same three-step close dance), `beginShutdown` (identical body, different log text), `serve` with the cancel-protection dance, `acceptLoop` with the same error mapping, `decideClaim` in two shapes plus `firstFree`, and — in the three DNS listeners — the byte-identical `Outcome`/`Result`/`race`/`expire` select harness. `readPrefix`/`readBody`/`writeReply` are byte-identical between tcp and dot. The predicted failure mode already fired once: the milestone-10 review hand-ported the `handshook` TLS-context-leak fix from dot to doh. Six unit tests are duplicated between tcp and dot, three more between doh and web.
New file `src/server/listener.zig`:
- The four enums, `retry_delay`, `bump` — plain shared declarations.
- The race harness: `Outcome`, `Result`, `race`, `expire` — one copy,
generic over the raced function (the existing `comptime f: anytype` shape
is already generic; it only needs to move).
- The framing helpers `readPrefix`, `readBody`, `writeReply` (tcp/dot
consumers; doh speaks HTTP).
- `pub fn Core(comptime Cfg: type) type` generating: the conns array with
`Cfg.ConnPayload` per slot, mutex, `run_state`, `shutdown_begun`,
`claim`/`finish`/`beginShutdown`/`decideClaim`/`firstFree`, the accept
loop, and the `serve` skeleton with the cancel-protection dance. `Cfg`
supplies exactly four things: the per-slot payload type (`ConnPayload`
the payloads genuinely differ per listener, TLS context and buffers),
the per-connection serve function (`serveConn`), the read/write buffer
sizes, and the at-capacity behavior (close for the DNS listeners; the
web listener's `refuse` 503 for web).
- The Core does **not** own the TLS lifecycle. Certificate pin and
release, TLS-context ownership, the handshake, close_notify, and
cleanup after a late race completion form one ordered sequence
(doh_server.zig:294-330, dot_server.zig:288-323) and stay inside each
listener's `serveConn`. What is shared is a helper,
`listener.handshakeStage`, that owns the `handshook` exactly-once flag
and its late-success cleanup contract — the exact defect the
milestone-10 review hand-ported — with a doc comment stating the
caller's required ordering (pin → handshake via the helper → serve →
TLS close → release, cleanup on every early exit). Both TLS listeners
adopt it; the leak class closes there, not in the Core.
- The race harness: `Outcome`, `Result`, `race`, `expire` — one copy, generic over the raced function (the existing `comptime f: anytype` shape is already generic; it only needs to move).
- The framing helpers `readPrefix`, `readBody`, `writeReply` (tcp/dot consumers; doh speaks HTTP).
- `pub fn Core(comptime Cfg: type) type` generating: the conns array with `Cfg.ConnPayload` per slot, mutex, `run_state`, `shutdown_begun`, `claim`/`finish`/`beginShutdown`/`decideClaim`/`firstFree`, the accept loop, and the `serve` skeleton with the cancel-protection dance. `Cfg` supplies exactly four things: the per-slot payload type (`ConnPayload` — the payloads genuinely differ per listener, TLS context and buffers), the per-connection serve function (`serveConn`), the read/write buffer sizes, and the at-capacity behavior (close for the DNS listeners; the web listener's `refuse` 503 for web).
- The Core does **not** own the TLS lifecycle. Certificate pin and release, TLS-context ownership, the handshake, close_notify, and cleanup after a late race completion form one ordered sequence (doh_server.zig:294-330, dot_server.zig:288-323) and stay inside each listener's `serveConn`. What is shared is a helper, `listener.handshakeStage`, that owns the `handshook` exactly-once flag and its late-success cleanup contract — the exact defect the milestone-10 review hand-ported — with a doc comment stating the caller's required ordering (pin → handshake via the helper → serve → TLS close → release, cleanup on every early exit). Both TLS listeners adopt it; the leak class closes there, not in the Core.
- The shared unit tests move here; the per-file duplicates are deleted.
Unifications the extraction forces, all sanctioned:
- Counter name: `connections` everywhere. tcp and web rename `accepted`.
This renames the m16-added metric family
`nxdns_tcp_server_accepted_total``nxdns_tcp_server_connections_total`
— greenfield, allowed; update the metrics name test and the docs
reference. Core stats are one shared struct; `tls_handshake_failures`,
`bad_requests`, `requests` stay listener-specific beside it, and the
exported snapshots stay flat so /metrics output is unchanged except the
tcp rename.
- `deinit` shape: all four store the allocator at `listen` (dot's
milestone-10 deviation becomes the rule) and take `(self, io)`.
- The dead `pub fn serve` at doh_server.zig:687-707 (documented as
intentional in specs/milestone-10.md:286-289) is deleted; note the
update beside that spec line.
- Counter name: `connections` everywhere. tcp and web rename `accepted`. This renames the m16-added metric family `nxdns_tcp_server_accepted_total``nxdns_tcp_server_connections_total` — greenfield, allowed; update the metrics name test and the docs reference. Core stats are one shared struct; `tls_handshake_failures`, `bad_requests`, `requests` stay listener-specific beside it, and the exported snapshots stay flat so /metrics output is unchanged except the tcp rename.
- `deinit` shape: all four store the allocator at `listen` (dot's milestone-10 deviation becomes the rule) and take `(self, io)`.
- The dead `pub fn serve` at doh_server.zig:687-707 (documented as intentional in specs/milestone-10.md:286-289) is deleted; note the update beside that spec line.
The per-listener files keep what genuinely differs: the connection payload
(TLS context and buffers), the serve-one-connection logic, DoH's HTTP
handling, and the web listener's request router. Every existing listener
integration test must pass unchanged apart from renamed counters.
The per-listener files keep what genuinely differs: the connection payload (TLS context and buffers), the serve-one-connection logic, DoH's HTTP handling, and the web listener's request router. Every existing listener integration test must pass unchanged apart from renamed counters.
### 2. The repository list choreography exists once
The `prepare → ArrayList → errdefer out.deinit → errdefer freeX →
columnTextAlloc → append` shape is hand-rolled 19 times across the seven
repository files, with the load-bearing errdefer ordering repeated 18 times
and 26 hand-written `freeX` loops. The milestone-4 spec's own reference
sample (specs/milestone-4.md:1263-1272) declares the two errdefers in the
**reverse, use-after-free order** — every implementation silently corrected
it; the next repo copied from the spec is a landmine.
The `prepare → ArrayList → errdefer out.deinit → errdefer freeX → columnTextAlloc → append` shape is hand-rolled 19 times across the seven repository files, with the load-bearing errdefer ordering repeated 18 times and 26 hand-written `freeX` loops. The milestone-4 spec's own reference sample (specs/milestone-4.md:1263-1272) declares the two errdefers in the **reverse, use-after-free order** — every implementation silently corrected it; the next repo copied from the spec is a landmine.
`src/storage/repositories/crud.zig` (today: `execStrict` only) gains:
@@ -105,38 +45,16 @@ pub fn listRows(
pub fn freeRows(comptime Row: type, gpa: Allocator, items: []const Row) void
```
`listRows` owns the errdefer choreography — written once, with the ordering
comment. `freeRows` frees every `[]const u8` **and every `?[]const u8`**
field by comptime reflection — `SourceRow.checksum` is an allocated
optional slice (sources_repo.zig:91-104) and its current destructor frees
the non-null payload explicitly; a shallow slice-only reflection leaks it.
Any other owning field shape is a `@compileError`, so a future row cannot
silently leak. The allocation-failure tests must keep a case with a
non-null checksum.
The per-row `readRow` functions stay in the repos and keep their per-column
errdefers. All 19 list functions become one-line delegations; the `freeX`
wrappers stay as thin `pub` shims where callers use them, or are deleted
where `freeRows` is called directly. Every `checkAllAllocationFailures`
test stays and must pass — they now exercise the shared helper from 20
angles.
`listRows` owns the errdefer choreography — written once, with the ordering comment. `freeRows` frees every `[]const u8` **and every `?[]const u8`** field by comptime reflection — `SourceRow.checksum` is an allocated optional slice (sources_repo.zig:91-104) and its current destructor frees the non-null payload explicitly; a shallow slice-only reflection leaks it. Any other owning field shape is a `@compileError`, so a future row cannot silently leak. The allocation-failure tests must keep a case with a non-null checksum. The per-row `readRow` functions stay in the repos and keep their per-column errdefers. All 19 list functions become one-line delegations; the `freeX` wrappers stay as thin `pub` shims where callers use them, or are deleted where `freeRows` is called directly. Every `checkAllAllocationFailures` test stays and must pass — they now exercise the shared helper from 20 angles.
Correct the milestone-4 sample in place and note the correction in that
spec.
Correct the milestone-4 sample in place and note the correction in that spec.
### 3. The web CRUD shells are generated; the decisions stay hand-written
Seven handler files repeat the 4-line `configDb` switch 40 times in three
forms, and five of them repeat identical `list`/`get`/`remove` shells. The
four reload flavors (plain; local.zig's publish-under-lock; blocklists'
pruneFiles; groups' read-back-under-lock) are genuinely different and are
**not** unified.
Seven handler files repeat the 4-line `configDb` switch 40 times in three forms, and five of them repeat identical `list`/`get`/`remove` shells. The four reload flavors (plain; local.zig's publish-under-lock; blocklists' pruneFiles; groups' read-back-under-lock) are genuinely different and are **not** unified.
- `mutations.zig` gains `pub fn requireConfigDb(state: *server.WebState)
error{NoConfigDb}!*db.Db`. The 40 switch sites become one-line `catch`
arms producing the same three response forms (the Failure payload is the
same constant text everywhere).
- `mutations.zig` gains a comptime resource descriptor generating only the
identical trio:
- `mutations.zig` gains `pub fn requireConfigDb(state: *server.WebState) error{NoConfigDb}!*db.Db`. The 40 switch sites become one-line `catch` arms producing the same three response forms (the Failure payload is the same constant text everywhere).
- `mutations.zig` gains a comptime resource descriptor generating only the identical trio:
```zig
// desc is an anonymous struct literal; `anytype` is not legal as a
@@ -150,184 +68,67 @@ pruneFiles; groups' read-back-under-lock) are genuinely different and are
// []const u8 ("upstreams", the JSON list key).
```
yielding `list`/`get`/`remove` handlers in the exact current shape.
Adopted by upstreams, groups, blocklists, clients, rules and the two
local.zig sets where the shell matches; `settings.zig` (get + applyPut
only) does not adopt. All `applyCreate`/`applyUpdate`/`applyDelete`
bodies and the per-resource decision functions (countEnabledExcept,
updateLocked/deleteLocked, pruneFiles, applyReplacePrefixes, publish,
RuleView, applyPut) stay hand-written.
yielding `list`/`get`/`remove` handlers in the exact current shape. Adopted by upstreams, groups, blocklists, clients, rules and the two local.zig sets where the shell matches; `settings.zig` (get + applyPut only) does not adopt. All `applyCreate`/`applyUpdate`/`applyDelete` bodies and the per-resource decision functions (countEnabledExcept, updateLocked/deleteLocked, pruneFiles, applyReplacePrefixes, publish, RuleView, applyPut) stay hand-written.
Acceptance is a grep: the 4-line switch appears zero times outside
`mutations.zig`.
Acceptance is a grep: the 4-line switch appears zero times outside `mutations.zig`.
### 4. The transport scaffolding is shared, and DoH gets its missing unwrap
- The byte-identical `Outcome` + `expire` + select-race body in
`pool.zig:253-274/310-317` and `forward_client.zig:195-215/260-267`
moves to `transport.zig` as one generic exchange-race helper (the raced
function, the budget and one comment word are the only differences
today). `manager.zig`'s `fetchWithin` may adopt it if it generalizes
without contortion; not required.
- `mapPhase` (duplicated byte-for-byte, dot_client.zig:283 /
forward_client.zig:293) moves to `transport.zig` as `pub`.
- The cancel-protected close helpers (four near-copies:
forward_client.zig:281/287, dot_client.zig:271/277) become one
`pub fn closeBlocked(io: std.Io, target: anytype) void` in transport.zig
(`target.close(io)` under swapped protection). The inline copies in
logging.zig and metrics.zig are out of scope — they are not transport
code.
- `doh_client.zig` gains the stashed-cause unwrap the other two transports
carry (`concreteRead`/`concreteWrite` precedent, dot_client.zig:303-318):
today its `mapError` never reads the http client's stashed cause behind
`ReadFailed`/`WriteFailed`, so rare mid-exchange local errors (e.g.
ENOBUFS) are recorded as peer faults against a healthy upstream — a
stated spec-invariant violation, fixed once for DoT and left in DoH.
Port the unwrap and the corresponding stub-based tests.
- `sendFailure`/`receiveFailure` stay per-file: the unwrapping genuinely
differs by stream type.
- The byte-identical `Outcome` + `expire` + select-race body in `pool.zig:253-274/310-317` and `forward_client.zig:195-215/260-267` moves to `transport.zig` as one generic exchange-race helper (the raced function, the budget and one comment word are the only differences today). `manager.zig`'s `fetchWithin` may adopt it if it generalizes without contortion; not required.
- `mapPhase` (duplicated byte-for-byte, dot_client.zig:283 / forward_client.zig:293) moves to `transport.zig` as `pub`.
- The cancel-protected close helpers (four near-copies: forward_client.zig:281/287, dot_client.zig:271/277) become one `pub fn closeBlocked(io: std.Io, target: anytype) void` in transport.zig (`target.close(io)` under swapped protection). The inline copies in logging.zig and metrics.zig are out of scope — they are not transport code.
- `doh_client.zig` gains the stashed-cause unwrap the other two transports carry (`concreteRead`/`concreteWrite` precedent, dot_client.zig:303-318): today its `mapError` never reads the http client's stashed cause behind `ReadFailed`/`WriteFailed`, so rare mid-exchange local errors (e.g. ENOBUFS) are recorded as peer faults against a healthy upstream — a stated spec-invariant violation, fixed once for DoT and left in DoH. Port the unwrap and the corresponding stub-based tests.
- `sendFailure`/`receiveFailure` stay per-file: the unwrapping genuinely differs by stream type.
### 5. `normalizeName` lives beside `fromText`
The copies in `records.zig:197` and `forward_zones.zig:129` are
byte-identical (only doc comments differ). Move the function to
`src/dns/name.zig` as `pub fn normalizeText(text: []const u8, buf:
*[types.max_name_len]u8) error{BadName}![]const u8`, next to `fromText`
(both callers already import the module; it stays pure — no allocation, no
Io). Both files delete their copies and their private `NameError`.
The copies in `records.zig:197` and `forward_zones.zig:129` are byte-identical (only doc comments differ). Move the function to `src/dns/name.zig` as `pub fn normalizeText(text: []const u8, buf: *[types.max_name_len]u8) error{BadName}![]const u8`, next to `fromText` (both callers already import the module; it stays pure — no allocation, no Io). Both files delete their copies and their private `NameError`.
The other three variants are **not** unified — their policies differ on
purpose (rules.zig:195 rejects controls and space but skips `fromText`
because patterns hold `*`; compiler.zig:155 adds a two-label minimum and
counter-based reporting; dns_cache.zig:59 validates nothing because its
input is asserted). Each of the three gains a one-line comment pointing at
`name.normalizeText` and naming its own policy difference, so the next
reader knows the divergence is intentional.
The other three variants are **not** unified — their policies differ on purpose (rules.zig:195 rejects controls and space but skips `fromText` because patterns hold `*`; compiler.zig:155 adds a two-label minimum and counter-based reporting; dns_cache.zig:59 validates nothing because its input is asserted). Each of the three gains a one-line comment pointing at `name.normalizeText` and naming its own policy difference, so the next reader knows the divergence is intentional.
### 6. One bounded-line iterator
The subtle `takeDelimiter`/`StreamTooLong`/`discardDelimiterInclusive` loop
exists twice (`compiler.zig:64-87`, `manager.zig collectSample:1423-1441`),
both correct, both carrying the infinite-loop-hazard comment and a
regression test. Their seven behavioral differences (termination, counting,
trimming, filtering, exit style, error set, arm order) all live *outside*
the hazardous core. New shared iterator in `src/filter/parsers.zig`:
The subtle `takeDelimiter`/`StreamTooLong`/`discardDelimiterInclusive` loop exists twice (`compiler.zig:64-87`, `manager.zig collectSample:1423-1441`), both correct, both carrying the infinite-loop-hazard comment and a regression test. Their seven behavioral differences (termination, counting, trimming, filtering, exit style, error set, arm order) all live *outside* the hazardous core. New shared iterator in `src/filter/parsers.zig`:
```zig
pub const LineEvent = union(enum) { line: []const u8, long_line };
pub fn nextBoundedLine(r: *std.Io.Reader, max_len: usize) error{ReadFailed}!?LineEvent
```
encapsulating the take/discard dance (including the
EndOfStream-during-discard arm and the large-reader-buffer length check).
The limit is a **parameter** — `parsers.zig` must not import `compiler.zig`
(compiler imports parsers, so that is a cycle, and parsers.zig is a
standalone std-only fuzz-module root); both callers pass
`compiler.max_line_len` from their side. `compile` keeps its `long_lines` counting
and `\r` handling on top; `collectSample` keeps its trim/filter/count-limit
on top. Both regression tests must still pass; the m15 compiler fuzz target
now exercises the shared core.
encapsulating the take/discard dance (including the EndOfStream-during-discard arm and the large-reader-buffer length check). The limit is a **parameter** — `parsers.zig` must not import `compiler.zig` (compiler imports parsers, so that is a cycle, and parsers.zig is a standalone std-only fuzz-module root); both callers pass `compiler.max_line_len` from their side. `compile` keeps its `long_lines` counting and `\r` handling on top; `collectSample` keeps its trim/filter/count-limit on top. Both regression tests must still pass; the m15 compiler fuzz target now exercises the shared core.
### 7. `cli.zig` stops re-spelling app policy
- The DoH buffer sizes: `app.zig:89-90` names
`doh_request_buf_len = 1024` / `doh_transfer_buf_len = 4096` (private);
`cli.zig:885-886` re-spells them as bare literals, so changing the
constants would leave `nxdns check` probing different buffers than
`nxdns run` uses — undermining the probe's stated purpose. The constants
move to `doh_client.zig` as `pub const default_request_buf_len` /
`default_transfer_buf_len`; both `app.zig` and `cli.zig` use them. No
deeper unification of `probeUpstreams` vs `Upstreams.build` — the
one-at-a-time probe and the slab build are intentionally different
shapes.
- The ZON failure channels: `check` prints the multi-line `zon_diag`
rendering inline in a single `FAIL` line (cli.zig:721-729), embedding
newlines mid-record and contradicting the one-line-per-problem promise
(docs/reference/configuration.md:332); `import` routes through
`reportParseFailure` (config/import.zig:167 — currently **private**).
Make `reportParseFailure` `pub`; `check` builds a local
`validate.Diagnostics`, feeds the parse failure through it, and renders
each problem as its own `FAIL` line like its other diagnostics.
Acceptance: a config with a multi-line ZON error yields one `FAIL` line
per parser message from both `check` and `import`.
- The DoH buffer sizes: `app.zig:89-90` names `doh_request_buf_len = 1024` / `doh_transfer_buf_len = 4096` (private); `cli.zig:885-886` re-spells them as bare literals, so changing the constants would leave `nxdns check` probing different buffers than `nxdns run` uses — undermining the probe's stated purpose. The constants move to `doh_client.zig` as `pub const default_request_buf_len` / `default_transfer_buf_len`; both `app.zig` and `cli.zig` use them. No deeper unification of `probeUpstreams` vs `Upstreams.build` — the one-at-a-time probe and the slab build are intentionally different shapes.
- The ZON failure channels: `check` prints the multi-line `zon_diag` rendering inline in a single `FAIL` line (cli.zig:721-729), embedding newlines mid-record and contradicting the one-line-per-problem promise (docs/reference/configuration.md:332); `import` routes through `reportParseFailure` (config/import.zig:167 — currently **private**). Make `reportParseFailure` `pub`; `check` builds a local `validate.Diagnostics`, feeds the parse failure through it, and renders each problem as its own `FAIL` line like its other diagnostics. Acceptance: a config with a multi-line ZON error yields one `FAIL` line per parser message from both `check` and `import`.
### 8. `build.zig` wires a test suite once
The host (lines 47-69) and aarch64 (150-179) test suites repeat twelve
wiring lines; the aarch64 triple is re-spelled at :152 instead of read from
`cross_targets` (:10-13). Extract `fn addTestSuite(b, target, optimize,
options, web_assets) *Step.Compile` mirroring the existing `addExecutable`
helper; the aarch64 call resolves its target from `cross_targets[1]` (or a
named constant both use). The two blocks' three real differences
(`linkage = .static`, `skip_foreign_checks`) stay at the call sites. The
m15 fuzz-suite wiring repeats the same shape three times — fold those calls
into the helper too if the anonymous-import needs line up; otherwise leave
them and say so.
The host (lines 47-69) and aarch64 (150-179) test suites repeat twelve wiring lines; the aarch64 triple is re-spelled at :152 instead of read from `cross_targets` (:10-13). Extract `fn addTestSuite(b, target, optimize, options, web_assets) *Step.Compile` mirroring the existing `addExecutable` helper; the aarch64 call resolves its target from `cross_targets[1]` (or a named constant both use). The two blocks' three real differences (`linkage = .static`, `skip_foreign_checks`) stay at the call sites. The m15 fuzz-suite wiring repeats the same shape three times — fold those calls into the helper too if the anonymous-import needs line up; otherwise leave them and say so.
### 9. One Smith encoder
`sliceInput` is byte-identical in `blocklist_fuzz.zig:160` and
`corpus.zig:123` (blocklist_fuzz cannot import corpus.zig — corpus imports
the `dns` module its build target lacks; that is why it was copied). New
dependency-free `tests/fuzz/smith_encode.zig` holding `sliceInput` and its
length self-test; `corpus.zig` and `blocklist_fuzz.zig` import it by
relative path (each fuzz module compiles its own copy — source-level dedup
is the goal). `pairInput` stays in blocklist_fuzz, `sliceIntInput` stays in
corpus — both build on the shared one. The m15 fuzz files
(`compiler_fuzz.zig`, `http_util_fuzz.zig`) adopt it where they inlined
the idiom.
`sliceInput` is byte-identical in `blocklist_fuzz.zig:160` and `corpus.zig:123` (blocklist_fuzz cannot import corpus.zig — corpus imports the `dns` module its build target lacks; that is why it was copied). New dependency-free `tests/fuzz/smith_encode.zig` holding `sliceInput` and its length self-test; `corpus.zig` and `blocklist_fuzz.zig` import it by relative path (each fuzz module compiles its own copy — source-level dedup is the goal). `pairInput` stays in blocklist_fuzz, `sliceIntInput` stays in corpus — both build on the shared one. The m15 fuzz files (`compiler_fuzz.zig`, `http_util_fuzz.zig`) adopt it where they inlined the idiom.
### 10. Frontend: one class vocabulary, shared form plumbing
15 of 30 non-test `.tsx` files re-declare Tailwind class constants in two
naming conventions; the primary-button literal appears at 7 sites, the
input literal at 6, the focus-visible fragment 39 times across 17 files —
and the two inputs that *dropped* the focus ring (PrefixesEditor.tsx:13,
GroupsPage.tsx:17, the only unfocusable inputs in the app) silently violate
the milestone-9 accessibility floor. The project's own precedent
(InlineError was hoisted during milestone 9) is house style left
unapplied.
15 of 30 non-test `.tsx` files re-declare Tailwind class constants in two naming conventions; the primary-button literal appears at 7 sites, the input literal at 6, the focus-visible fragment 39 times across 17 files — and the two inputs that *dropped* the focus ring (PrefixesEditor.tsx:13, GroupsPage.tsx:17, the only unfocusable inputs in the app) silently violate the milestone-9 accessibility floor. The project's own precedent (InlineError was hoisted during milestone 9) is house style left unapplied.
- New `web/src/ui/classes.ts` exporting the named constants (camelCase, one
convention): `inputClass`, `buttonClass`, `primaryButtonClass`,
`thClass`, `tdClass`, `formCardClass`, `tableWrapClass`,
`retryButtonClass` — taken verbatim from the current majority literals
(all carrying the focus ring). All 15 declaring files adopt; the two
ring-less inputs are fixed by adoption. Acceptance is a grep: the three
top literals appear only in `ui/classes.ts`, and every
input/button/select in `web/src` carries the focus-visible fragment (the
one sanctioned variant is PauseWidget's negative offset).
- New `web/src/ui/useCrudForm.ts`: the mutation trio + `FormState` +
`openForm`/`onSubmit`/`onDelete` plumbing that RecordsTab and ZonesTab
share byte-for-byte, parameterized over the entity and its three
mutation factories. Both tabs adopt it; their field JSX and tables stay
local. Other pages may adopt where the shape fits; none is forced.
- The shadowing `InlineError` in DashboardPage.tsx:36: extend
`lib/InlineError.tsx` with an optional `onRetry` prop rendering the
retry button; DashboardPage imports it and the local copy is deleted.
- New `web/src/ui/classes.ts` exporting the named constants (camelCase, one convention): `inputClass`, `buttonClass`, `primaryButtonClass`, `thClass`, `tdClass`, `formCardClass`, `tableWrapClass`, `retryButtonClass` — taken verbatim from the current majority literals (all carrying the focus ring). All 15 declaring files adopt; the two ring-less inputs are fixed by adoption. Acceptance is a grep: the three top literals appear only in `ui/classes.ts`, and every input/button/select in `web/src` carries the focus-visible fragment (the one sanctioned variant is PauseWidget's negative offset).
- New `web/src/ui/useCrudForm.ts`: the mutation trio + `FormState` + `openForm`/`onSubmit`/`onDelete` plumbing that RecordsTab and ZonesTab share byte-for-byte, parameterized over the entity and its three mutation factories. Both tabs adopt it; their field JSX and tables stay local. Other pages may adopt where the shape fits; none is forced.
- The shadowing `InlineError` in DashboardPage.tsx:36: extend `lib/InlineError.tsx` with an optional `onRetry` prop rendering the retry button; DashboardPage imports it and the local copy is deleted.
## Sessions
S1-S7 run in parallel; no two sessions write the same file. Interfaces
fixed here: S4 makes the two DoH buffer constants `pub` in
`doh_client.zig`; S5 consumes them from `cli.zig`/`app.zig` — wait for S4's
commit of that one declaration or agree the exact names above and build
against them.
S1-S7 run in parallel; no two sessions write the same file. Interfaces fixed here: S4 makes the two DoH buffer constants `pub` in `doh_client.zig`; S5 consumes them from `cli.zig`/`app.zig` — wait for S4's commit of that one declaration or agree the exact names above and build against them.
### Session S1: listener core
Owns `src/server/listener.zig` (new), `src/server/tcp_server.zig`,
`src/server/dot_server.zig`, `src/server/doh_server.zig`,
`src/web/server.zig`, `src/web/metrics.zig`, and the listener integration
tests (`tcp_server_integration_test.zig`,
`udp_server_integration_test.zig`, `src/web/server_integration_test.zig`,
`resolver_integration_test.zig`, `phase7_integration_test.zig` as needed).
Ruling 1.
Owns `src/server/listener.zig` (new), `src/server/tcp_server.zig`, `src/server/dot_server.zig`, `src/server/doh_server.zig`, `src/web/server.zig`, `src/web/metrics.zig`, and the listener integration tests (`tcp_server_integration_test.zig`, `udp_server_integration_test.zig`, `src/web/server_integration_test.zig`, `resolver_integration_test.zig`, `phase7_integration_test.zig` as needed). Ruling 1.
### Session S2: storage
Owns `src/storage/repositories/*` and the milestone-4 spec correction.
Ruling 2.
Owns `src/storage/repositories/*` and the milestone-4 spec correction. Ruling 2.
### Session S3: web handlers
@@ -335,19 +136,11 @@ Owns `src/web/handlers/*`, `src/web/web_integration_test.zig`. Ruling 3.
### Session S4: transport
Owns `src/upstream/transport.zig`, `src/upstream/pool.zig`,
`src/upstream/dot_client.zig`, `src/upstream/doh_client.zig`,
`src/local/forward_client.zig`. Ruling 4, and the `pub` constants half of
ruling 7.
Owns `src/upstream/transport.zig`, `src/upstream/pool.zig`, `src/upstream/dot_client.zig`, `src/upstream/doh_client.zig`, `src/local/forward_client.zig`. Ruling 4, and the `pub` constants half of ruling 7.
### Session S5: names, lines, cli
Owns `src/dns/name.zig`, `src/local/records.zig`,
`src/local/forward_zones.zig`, `src/filter/rules.zig`,
`src/filter/compiler.zig`, `src/filter/manager.zig`,
`src/filter/parsers.zig`, `src/cache/dns_cache.zig` (comment only),
`src/cli.zig`, `src/config/import.zig`, `src/app.zig`. Rulings 5, 6, and
the consumer half of 7.
Owns `src/dns/name.zig`, `src/local/records.zig`, `src/local/forward_zones.zig`, `src/filter/rules.zig`, `src/filter/compiler.zig`, `src/filter/manager.zig`, `src/filter/parsers.zig`, `src/cache/dns_cache.zig` (comment only), `src/cli.zig`, `src/config/import.zig`, `src/app.zig`. Rulings 5, 6, and the consumer half of 7.
### Session S6: build and fuzz
@@ -359,19 +152,13 @@ Owns `web/src/**`. Ruling 10.
### Orchestrator
Strikes the closed findings in `TECH_DEBT.md`; records the tcp metric
rename in the docs reference; updates the milestone-10 deviation note
(ruling 1) and the milestone-4 sample (ruling 2).
Strikes the closed findings in `TECH_DEBT.md`; records the tcp metric rename in the docs reference; updates the milestone-10 deviation note (ruling 1) and the milestone-4 sample (ruling 2).
## Module layout
New files: `src/server/listener.zig`, `tests/fuzz/smith_encode.zig`,
`web/src/ui/classes.ts`, `web/src/ui/useCrudForm.ts`.
New files: `src/server/listener.zig`, `tests/fuzz/smith_encode.zig`, `web/src/ui/classes.ts`, `web/src/ui/useCrudForm.ts`.
Deleted surface: the four per-listener copies of the shared machinery, the
dead `doh_server.serve`, the two `normalizeName` copies and their
`NameError`s, the duplicated listener unit tests, the DashboardPage
`InlineError`.
Deleted surface: the four per-listener copies of the shared machinery, the dead `doh_server.serve`, the two `normalizeName` copies and their `NameError`s, the duplicated listener unit tests, the DashboardPage `InlineError`.
## Acceptance (milestone complete)
@@ -408,143 +195,57 @@ dead `doh_server.serve`, the two `normalizeName` copies and their
## Recorded (implementation)
Accepted deviations and findings from the built milestone. Each was
reviewed and accepted at integration; the rulings above stand except as
recorded here.
Accepted deviations and findings from the built milestone. Each was reviewed and accepted at integration; the rulings above stand except as recorded here.
### Ruling 1 (S1)
- `Cfg` supplies more than the four listed members: `Owner`,
`ConnPayload`, `serveConn`, `read_buffer_len`, `write_buffer_len`,
plus `log` (the owner's `std.log` scope) and `name`, because the
accept loop and shutdown log and the text had to stay byte-identical.
Two optional decls: `refuse` (absent means close the stream; the web
listener supplies its 503) and `initPayload`/`deinitPayload`, which
exist only for the web arena's create-in-listen / destroy-in-deinit
lifecycle.
- The read/write staging buffers live in the core's `Conn`, not in
`ConnPayload` — all four listeners have exactly one of each and
differ only in size. The web listener's `recv_buf`/`send_buf` are now
`read_buf`/`write_buf`.
- Stats layout: core counters live at `server.core.stats.*`
(`listener.CoreStats`); listener-specific counters stay on the owner
at `server.stats.*`. `tcp_server.Stats` is an alias of `CoreStats`.
Exported snapshots stay flat, so /metrics output is unchanged except
the tcp rename. `idle_timeouts` sits in `CoreStats` per the ruling,
which gives the web listener a counter it never bumps; it exports no
family, so nothing is visible.
- The docs-reference update for the rename has no target:
`nxdns_tcp_server_accepted_total` appears in no file under docs/ or
web/. Only the metrics name test changed.
- 18 duplicated unit tests were deleted (6 tcp, 6 dot, 3 doh, 3 web)
and replaced by 6 shared claim-rule tests in `listener.zig`.
`web/server_integration_test.zig`'s `withServer` now returns a local
plain-`u64` `Counters` struct instead of `server.Stats`.
- File-ownership breach: S1 edited two files owned by other sessions,
both mechanical fallout of the sanctioned `deinit` shape change —
three call sites in `src/app.zig` (S5) and one line in
`src/web/web_integration_test.zig` (S3). Both owners reviewed and
kept the edits. S1 also added the required `src/tests.zig` import
line for the new file.
- `Cfg` supplies more than the four listed members: `Owner`, `ConnPayload`, `serveConn`, `read_buffer_len`, `write_buffer_len`, plus `log` (the owner's `std.log` scope) and `name`, because the accept loop and shutdown log and the text had to stay byte-identical. Two optional decls: `refuse` (absent means close the stream; the web listener supplies its 503) and `initPayload`/`deinitPayload`, which exist only for the web arena's create-in-listen / destroy-in-deinit lifecycle.
- The read/write staging buffers live in the core's `Conn`, not in `ConnPayload` — all four listeners have exactly one of each and differ only in size. The web listener's `recv_buf`/`send_buf` are now `read_buf`/`write_buf`.
- Stats layout: core counters live at `server.core.stats.*` (`listener.CoreStats`); listener-specific counters stay on the owner at `server.stats.*`. `tcp_server.Stats` is an alias of `CoreStats`. Exported snapshots stay flat, so /metrics output is unchanged except the tcp rename. `idle_timeouts` sits in `CoreStats` per the ruling, which gives the web listener a counter it never bumps; it exports no family, so nothing is visible.
- The docs-reference update for the rename has no target: `nxdns_tcp_server_accepted_total` appears in no file under docs/ or web/. Only the metrics name test changed.
- 18 duplicated unit tests were deleted (6 tcp, 6 dot, 3 doh, 3 web) and replaced by 6 shared claim-rule tests in `listener.zig`. `web/server_integration_test.zig`'s `withServer` now returns a local plain-`u64` `Counters` struct instead of `server.Stats`.
- File-ownership breach: S1 edited two files owned by other sessions, both mechanical fallout of the sanctioned `deinit` shape change — three call sites in `src/app.zig` (S5) and one line in `src/web/web_integration_test.zig` (S3). Both owners reviewed and kept the edits. S1 also added the required `src/tests.zig` import line for the new file.
### Ruling 2 (S2)
- `listRowsBound(comptime Row, database, gpa, comptime sql, args:
anytype, comptime readRow)` exists beside `listRows` for the bound
queries; `freeRow` exists beside `freeRows`. Unknown owning field
shapes are a `@compileError` as ruled. The milestone-4 sample
correction was made by S2, not the orchestrator.
- `listRowsBound(comptime Row, database, gpa, comptime sql, args: anytype, comptime readRow)` exists beside `listRows` for the bound queries; `freeRow` exists beside `freeRows`. Unknown owning field shapes are a `@compileError` as ruled. The milestone-4 sample correction was made by S2, not the orchestrator.
### Ruling 3 (S3)
- `plural` is a separate descriptor member: the "listing X" log context
differs from the JSON envelope key for three resources
(`local_records`, `client_prefixes`, `forward_zones`), so deriving one
from the other would change three log strings.
- `view` is an optional descriptor member: rules and local records map
rows through `RuleView`/`RecordView`; without it neither could adopt
without changing its response body. When absent, the row serializes
as-is with no copy.
- `remove` has two comptime-detected arities: three handlers' decision
functions read rows and take an `Allocator` before the id, three do
not. The generator validates the full signature of whichever shape it
finds.
- `plural` is a separate descriptor member: the "listing X" log context differs from the JSON envelope key for three resources (`local_records`, `client_prefixes`, `forward_zones`), so deriving one from the other would change three log strings.
- `view` is an optional descriptor member: rules and local records map rows through `RuleView`/`RecordView`; without it neither could adopt without changing its response body. When absent, the row serializes as-is with no copy.
- `remove` has two comptime-detected arities: three handlers' decision functions read rows and take an `Allocator` before the id, three do not. The generator validates the full signature of whichever shape it finds.
### Ruling 4 (S4)
- The ruling's line numbers were stale after m17 (pool.zig's race sites
were at 184-206 and 306-335).
- `closeBlocked` branches at comptime on the close method's parameter
count: `tls_client.TlsStream.close()` takes no `Io` (it owns the one
it was built with), while the other three closes take `io`.
- "The stashed-cause accessor" is three accessors, one per collapse
point: the send phase reads `req.connection.?.stream_writer.err`,
`receiveHead` reads `Connection.getReadError()`, and the body read
consults `Response.bodyErr()` first (HTTP framing faults) then the
connection.
- `Connection.getReadError` can panic (it reads `stream_reader.err.?`).
The unwrap guards the plain-connection no-cause case; the TLS case
relies on std's documented contract that a cause exists after
`error.ReadFailed`.
- `raceWithin` requires the raced function's return type to be exactly
`ExchangeError!T` at comptime. Stricter than the ruling asked; it is
what keeps every failure path inside the race group. Consequence:
`manager.zig`'s `fetchWithin` (optional per the ruling) cannot adopt
it as written — its raced function has a different error set.
- The acceptance line "`fn expire(` production copies are gone" is
scoped to ruling 4's transport files. `filter/manager.zig`,
`storage/logger.zig` and `server/listener.zig` keep their own — none
is transport code.
- The ruling's line numbers were stale after m17 (pool.zig's race sites were at 184-206 and 306-335).
- `closeBlocked` branches at comptime on the close method's parameter count: `tls_client.TlsStream.close()` takes no `Io` (it owns the one it was built with), while the other three closes take `io`.
- "The stashed-cause accessor" is three accessors, one per collapse point: the send phase reads `req.connection.?.stream_writer.err`, `receiveHead` reads `Connection.getReadError()`, and the body read consults `Response.bodyErr()` first (HTTP framing faults) then the connection.
- `Connection.getReadError` can panic (it reads `stream_reader.err.?`). The unwrap guards the plain-connection no-cause case; the TLS case relies on std's documented contract that a cause exists after `error.ReadFailed`.
- `raceWithin` requires the raced function's return type to be exactly `ExchangeError!T` at comptime. Stricter than the ruling asked; it is what keeps every failure path inside the race group. Consequence: `manager.zig`'s `fetchWithin` (optional per the ruling) cannot adopt it as written — its raced function has a different error set.
- The acceptance line "`fn expire(` production copies are gone" is scoped to ruling 4's transport files. `filter/manager.zig`, `storage/logger.zig` and `server/listener.zig` keep their own — none is transport code.
### Rulings 5, 6, 7 (S5, S4)
- `nextBoundedLine` returns `.long_line` from the
EndOfStream-during-discard arm, and the next call returns `null`. The
two originals disagreed there (`compile` counted then broke;
`collectSample` returned without counting); this shape preserves both
behaviors — `compile` counts exactly as before, `collectSample`
ignores the event and sees `null`. Safe because
`discardDelimiterInclusive` drains the stream before reporting
`EndOfStream`.
- The length check runs on the raw line for both callers, as the ruling
placed it. One input changes classification: a line of exactly
`max_line_len + 1` bytes ending in `\r` was compiled before and now
counts as `long_lines`. Accepted as the intended reading.
- `check`'s per-message `FAIL` lines render import's path constant
(`FAIL config: <line:col: message>`), not the file name. `checkImpl`
prints the file path immediately above and `check` examines one
source per run, and this keeps `check` and `import` rendering the
same failure identically — the acceptance criterion.
- `nextBoundedLine` returns `.long_line` from the EndOfStream-during-discard arm, and the next call returns `null`. The two originals disagreed there (`compile` counted then broke; `collectSample` returned without counting); this shape preserves both behaviors — `compile` counts exactly as before, `collectSample` ignores the event and sees `null`. Safe because `discardDelimiterInclusive` drains the stream before reporting `EndOfStream`.
- The length check runs on the raw line for both callers, as the ruling placed it. One input changes classification: a line of exactly `max_line_len + 1` bytes ending in `\r` was compiled before and now counts as `long_lines`. Accepted as the intended reading.
- `check`'s per-message `FAIL` lines render import's path constant (`FAIL config: <line:col: message>`), not the file name. `checkImpl` prints the file path immediately above and `check` examines one source per run, and this keeps `check` and `import` rendering the same failure identically — the acceptance criterion.
### Rulings 8, 9 (S6)
- The fuzz-suite wiring did not fold into `addTestSuite` (the ruling's
own fallback): it lives in a separate `addFuzzSuite` helper, with a
shared `sourceModule` helper. The aarch64 target resolves from
`cross_targets[1]` behind a comptime prefix guard.
- `pairInput` moved into `smith_encode.zig` rather than staying in
blocklist_fuzz: m15's fuzz files had made it a two-copy duplicate,
which is the condition ruling 9 exists to remove.
- The fuzz-suite wiring did not fold into `addTestSuite` (the ruling's own fallback): it lives in a separate `addFuzzSuite` helper, with a shared `sourceModule` helper. The aarch64 target resolves from `cross_targets[1]` behind a comptime prefix guard.
- `pairInput` moved into `smith_encode.zig` rather than staying in blocklist_fuzz: m15's fuzz files had made it a two-copy duplicate, which is the condition ruling 9 exists to remove.
### Ruling 10 (S7)
- `ui/classes.ts` exports 17 constants, not the 8 listed — the extra
ones are the focus-ring fragment and compositions the 15 adopting
files needed to drop their literals without re-spelling anything.
- Seven ring-less focusable controls were fixed by adoption, not the
two the ruling named. The acceptance grep ("every input/button/select
carries the focus-visible fragment") was taken as the invariant over
the anti-requirement's count of two, which undercounted.
- `ui/classes.ts` exports 17 constants, not the 8 listed — the extra ones are the focus-ring fragment and compositions the 15 adopting files needed to drop their literals without re-spelling anything.
- Seven ring-less focusable controls were fixed by adoption, not the two the ruling named. The acceptance grep ("every input/button/select carries the focus-visible fragment") was taken as the invariant over the anti-requirement's count of two, which undercounted.
## Anti-requirements
- No behavioral changes: this milestone moves code. The only sanctioned
behavior deltas are the tcp counter rename, DoH's cause unwrap, the two
restored focus rings, and the `check` ZON rendering — each named above.
- No behavioral changes: this milestone moves code. The only sanctioned behavior deltas are the tcp counter rename, DoH's cause unwrap, the two restored focus rings, and the `check` ZON rendering — each named above.
- No unification of the three intentionally-divergent normalize policies.
- No unification of the four reload flavors.
- No `web/src/ui/` component library beyond `classes.ts` and
`useCrudForm.ts` — no button/table/dialog components in this milestone.
- No listener behavior additions (timeouts, keepalive) — m16 finished
those.
- No `web/src/ui/` component library beyond `classes.ts` and `useCrudForm.ts` — no button/table/dialog components in this milestone.
- No listener behavior additions (timeouts, keepalive) — m16 finished those.
- No renaming of exported metric families beyond the one tcp rename.