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.

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.

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

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.

src/storage/repositories/crud.zig (today: execStrict only) gains:

pub fn listRows(
    comptime Row: type,
    database: *db.Db,
    gpa: Allocator,
    comptime sql: []const u8,
    comptime readRow: fn (*db.Stmt, Allocator) db.Error!Row,
) db.Error!std.ArrayList(Row)

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.

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.

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:

// desc is an anonymous struct literal; `anytype` is not legal as a
// struct *field* type, so the descriptor arrives as `comptime desc:
// anytype` and is validated field-by-field at comptime (@hasField +
// @compileError on a missing or mistyped member).
pub fn Resource(comptime desc: anytype) type
// desc members: Row: type; list / get / remove: the repo fns (get and
// remove may be `null` for list-only resources); label: []const u8
// ("an upstream", used in respondFailure contexts); envelope:
// []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.

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.

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

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.

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.

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.

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.

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.

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.

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.

Session S2: storage

Owns src/storage/repositories/* and the milestone-4 spec correction. Ruling 2.

Session S3: web handlers

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.

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.

Session S6: build and fuzz

Owns build.zig, tests/fuzz/*. Rulings 8, 9.

Session S7: frontend

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

Module layout

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 NameErrors, the duplicated listener unit tests, the DashboardPage InlineError.

Acceptance (milestone complete)

All four listeners build on listener.Core; every listener integration test passes; /metrics output is byte-identical to before except nxdns_tcp_server_connections_total (name test updated).
The handshook invariant exists once, in listener.handshakeStage; grep finds no per-listener copy; both TLS listeners call it.
Grep: errdefer out.deinit appears only in crud.zig; all 20+ checkAllAllocationFailures tests pass; the milestone-4 sample shows the safe order.
Grep: the configDb switch appears zero times outside mutations.zig; all web integration tests pass; the four reload flavors are byte-unchanged.
pool.zig and forward_client.zig share the race helper; fn expire( production copies are gone (test-local copies may stay).
DoH's mapError unwraps stashed causes; the ported stub tests pass.
records.zig and forward_zones.zig call name.normalizeText; the three intentional variants carry their pointer comments.
Both line-iterator call sites use nextBoundedLine; both regression tests pass.
nxdns check renders a multi-line ZON failure as one FAIL line per message (test); the buffer constants exist once, pub, in doh_client.zig.
build.zig has one addTestSuite; the aarch64 triple is read from cross_targets; the qemu CI job passes.
sliceInput exists once; both fuzz corpora still decode (the length self-test runs from smith_encode.zig).
Grep: the three top class literals live only in ui/classes.ts; every input in web/src carries the focus-visible fragment; RecordsTab and ZonesTab use useCrudForm; DashboardPage imports the shared InlineError. npm run test, typecheck, lint green.
Full suite green: zig build test -Dintegration.

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.

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.

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.

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.

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.

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.

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.

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.

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 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 renaming of exported metric families beyond the one tcp rename.

28 KiB Raw Permalink Blame History

Milestone 18: collapse the duplicated infrastructure