551 lines
28 KiB
Markdown
551 lines
28 KiB
Markdown
# 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.
|
|
|
|
**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:
|
|
|
|
```zig
|
|
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:
|
|
|
|
```zig
|
|
// 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`:
|
|
|
|
```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
|
|
`NameError`s, 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.
|