Gates / test (push) Successful in 2m58s
Gates / frontend (push) Successful in 3m57s
Gates / test-aarch64 (push) Successful in 8m20s
Gates / package (push) Successful in 7m27s
Gates / container (push) Successful in 17s
CI / gates (push) Successful in 19m9s
643 lines
36 KiB
Markdown
643 lines
36 KiB
Markdown
# Milestone 25: client names learned over reverse DNS
|
||
|
||
Goal: the clients table names devices by itself. The tracker already
|
||
materialises every querying address into a `clients` row (PLAN §7.2,
|
||
`src/server/clients.zig`), but `name` stays NULL until the operator types one —
|
||
which defeats the point of auto-materialisation on a LAN whose router already
|
||
knows every DHCP hostname. This milestone asks the router: for each unnamed
|
||
client, nxdns builds the address's reverse name, matches it against the
|
||
operator's conditional forward zones (PLAN §6.5), sends one PTR query to the
|
||
zone's resolver, and stores the answer as a *learned* name — runtime state
|
||
beside `last_seen`, never configuration. The dashboard shows it; a hand-typed
|
||
name always wins; a reverse name no forward zone covers is never sent anywhere.
|
||
|
||
Design written 2026-08-14 against HEAD `c428bc2`, revised after a Codex review
|
||
of the first draft (all 19 findings accepted; the review's fingerprints are
|
||
called out inline where a first-draft claim was wrong).
|
||
|
||
## Implementation contract (read first)
|
||
|
||
- Read `AGENTS.md`, then this spec whole, before session work starts.
|
||
- **The v1 baseline is editable and there is no migration step** (milestone-24
|
||
ruling 1 stands unchanged). The two new columns are lines edited into
|
||
`config_schema.ddl_v1` plus the identical lines in PLAN §11.2, kept
|
||
byte-identical. A `ddl_v2` or a second `Step` is wrong and must be reverted.
|
||
A development database stamped version 1 before the edit fails its first
|
||
`SELECT` naming the columns; delete the scratch database.
|
||
- The new `src/local/reverse_name.zig` is pure: bytes in, bytes out, no
|
||
`std.Io`, no clock, no sockets. (The blanket claim "src/local/ is pure" from
|
||
the first draft was false — `forward_client.zig` does socket I/O by design;
|
||
the purity requirement here binds the *new* module and the lookup-table
|
||
modules it sits beside.) Everything that touches the network or the database
|
||
lives in `src/server/`.
|
||
- After the API shape change, regenerate
|
||
`web/src/lib/contractSamples.gen.ts` with the AGENTS.md command (never a
|
||
hand edit) and update `web/src/lib/types.ts` to match, in the same session.
|
||
- Verify stdlib claims against `../zig` at tag 0.16.0. `specs/research/` holds
|
||
verified notes. One verified here: randomness is on the `Io` interface —
|
||
`io.random(buffer)` (`std/Io.zig:2468`); `std.crypto.random` does not exist
|
||
in 0.16.0.
|
||
- No `std.log.err` in new code. PTR answers are untrusted bytes: never let one
|
||
reach a log line, a metric label, or the UI without passing `acceptHostname`
|
||
(ruling 4).
|
||
|
||
## Rulings (binding)
|
||
|
||
### 1. When: the tracker's flush pass resolves, bounded, serial, last
|
||
|
||
Resolution is a step appended to the tracker's existing flush pass
|
||
(`Tracker.flushOnce`, `src/server/clients.zig:164`), not a new loop and not a
|
||
query-path hook. Reasons: the pass already runs every `flush_interval_s` (60 s)
|
||
on a dedicated task with a dedicated `config.db` connection (app.zig:523-524,
|
||
:754), it already respects the disk-monitor gate, and a name can only be
|
||
learned for a row that exists — which the flush itself just wrote. A DNS query
|
||
must never wait on naming, and with this placement it structurally cannot.
|
||
|
||
**Order within one pass is fixed: drain-and-write, then prune (when due), then
|
||
resolve.** One `now_s` is read at the top of the pass and used by all three
|
||
steps. Resolving before pruning could spend PTR queries on rows the same pass
|
||
deletes; the fixed order makes that impossible, and a test proves a row past
|
||
the prune cutoff on a due pass causes no exchange.
|
||
|
||
Per pass, the resolver:
|
||
|
||
- selects at most `max_per_pass = 16` candidate rows (ruling 3 defines a
|
||
candidate), ordered by `name_attempt_after` ascending then ip;
|
||
- resolves them **serially** — in-flight is exactly 1. At 16 per 60 s pass the
|
||
LAN resolver sees at most ~0.27 queries/second from naming.
|
||
|
||
**Naming has its own read timeout**, `ptr_read_timeout` = 2 s on the `.awake`
|
||
clock, a `pub const` on the resolver — it does *not* inherit the handler's
|
||
`forward_read_timeout`, which the operator may have set as high as the
|
||
upstream budget. **The budget bounds the whole attempt, not one transport
|
||
leg**: `ForwardClient` hands its duration to the UDP receive and then again
|
||
to the TCP fallback, so a resolver that answers UDP late with TC=1 and then
|
||
stalls TCP would spend up to 2× the budget per attempt — 64 s per pass, not
|
||
32. The default `exchangeFn` therefore wraps the entire `ForwardClient`
|
||
exchange in `transport.raceWithin(io, ptr_read_timeout, ...)` (the same
|
||
mechanism `exchangeTcp` itself uses, forward_client.zig:195), so UDP,
|
||
fallback and all share one deadline. The worst case is 16 × 2 s = 32 s of
|
||
naming per pass, and that number is a bound, not an estimate. That is acceptable because the drain already ran first: a slow pass
|
||
delays the *next* pass's start (the run loop sleeps after `flushOnce`
|
||
returns), and one extra interval against a 512-slot pending table on a
|
||
household LAN drops nothing. The acceptance test for this: with a stub client
|
||
that times out on every exchange, the pass still drains its pending batch
|
||
before any exchange is attempted, and attempts stop at `max_per_pass`.
|
||
|
||
Retry cadence is split by outcome (first draft used one 24 h cadence for
|
||
everything, which left a freshly-declared zone, a recovered router, or a fixed
|
||
resolver unnamed for a day while forward-zone changes publish live):
|
||
|
||
- **definitive** outcomes (`answered`, `nxdomain`): next attempt after
|
||
`refresh_after_s = 86_400` — the daily refresh that tracks DHCP renames;
|
||
- **non-definitive** outcomes (`no_zone`, `failed`, `invalid`): next attempt
|
||
after `retry_after_s = 3_600` — an operator who declares the missing zone
|
||
or fixes the resolver sees names within the hour, and a dead resolver costs
|
||
at most 16 timeouts per hour, not per minute.
|
||
|
||
Both constants are `pub const` on the resolver; no config knob (household
|
||
scale, AGENTS.md: no generality nobody asked for).
|
||
|
||
### 2. Where: only through a declared forward zone, never the pool
|
||
|
||
The reverse name (`10.1.168.192.in-addr.arpa`, or the 32-nibble `ip6.arpa`
|
||
form) is matched against the live forward-zones table — the same
|
||
`forward_zones.Zones.match` the query path uses, bracketed by
|
||
`LocalTables.acquire`/`release` (`src/server/local_tables.zig:48`), so naming
|
||
always sees the generation the operator last published.
|
||
|
||
**The zone's `resolver` is copied by value under the shared lock, and the
|
||
handle is released before any socket work.** `match` returns a pointer into
|
||
the current table generation, and `swap` frees the old generation as soon as
|
||
it holds the exclusive lock — a handle held across a PTR exchange, or a
|
||
released handle whose `*const Zone` is still dereferenced, is a use-after-free
|
||
against a config reload. `validate.Resolver` is a plain value; the copy is the
|
||
whole fix. A regression test must place the `swap` **inside the hazard
|
||
window** — after the handle release, before the copied resolver is used. A
|
||
test that swaps before `runPass` evaluates its arguments exercises nothing
|
||
(the first implementation's test had exactly this defect: the stub's exchange
|
||
performed the swap, but the resolver had already been copied into the call's
|
||
arguments, so the test passed regardless of what production retained). The
|
||
required shape: two candidates in one pass, two generations whose zones
|
||
cover both but carry **different resolver ports**; the stub `exchangeFn`
|
||
performs the swap during the *first* attempt (freeing the old generation
|
||
under `testing.allocator`) and records each attempt's resolver. Assertions:
|
||
the first attempt's resolver still reads the old generation's port after the
|
||
swap (the copy, not freed memory), and the second attempt's resolver reads
|
||
the *new* generation's port (each attempt re-acquires; no table pointer or
|
||
handle is cached across attempts). With a production `runPass` that retains
|
||
`*const Zone` or the handle across the release, the first assertion reads
|
||
freed memory and the second reads the stale generation — either fails.
|
||
|
||
- Match found: one PTR query to that zone's resolver through
|
||
`forward_client.ForwardClient`, built per attempt as `Context.viaForwardZone`
|
||
builds one (handler.zig:411), with `ptr_read_timeout` (ruling 1).
|
||
- No match: **no query is sent to anyone** — not the pool, not any resolver.
|
||
The attempt counts under `no_zone` and retries per ruling 1.
|
||
|
||
Scope of the promise, stated precisely (the first draft's "never to a public
|
||
resolver" claimed more than the code enforces): what this milestone
|
||
guarantees is that a reverse name reaches **only the resolver of a zone the
|
||
operator declared**, and never the upstream pool. `parseResolver`
|
||
(validate.zig:274) accepts any IP literal, so an operator who declares
|
||
`168.192.in-addr.arpa → udp://8.8.8.8:53` has pointed their private reverse
|
||
space at a public resolver — that is their declaration, and nxdns does not
|
||
second-guess it, any more than it second-guesses the same zone for forward
|
||
queries. Likewise nxdns cannot stop a declared LAN resolver forwarding
|
||
onward. The docs (ruling 11) say this in one sentence.
|
||
|
||
The DNS cache is not consulted and not written: one PTR per client per day is
|
||
not worth a cache entry, and bypassing the cache keeps the resolver a plain
|
||
consumer of the exchange seam.
|
||
|
||
### 3. Precedence: the operator's name always wins; NXDOMAIN clears
|
||
|
||
Schema (ruling 5) keeps learned names in their own column, so precedence is a
|
||
display rule, not a write conflict — `name` is never written by this feature
|
||
and `learned_name` is never written by the operator.
|
||
|
||
A **candidate** row satisfies both of:
|
||
|
||
- `name IS NULL OR name = ''` — a row whose displayed name would come from
|
||
learning. `hand_edited` does not appear in the predicate (the first draft
|
||
had it; display precedence never consults it, so candidacy must not
|
||
either): a hand-edited row the operator grouped but did not name still
|
||
benefits, and a named row is skipped whatever its flag — its learned name
|
||
would never be shown.
|
||
- `name_attempt_after <= now_s` (ruling 6 gives the exact SQL).
|
||
|
||
Outcomes of an attempt, written through one repo call (ruling 6), each
|
||
setting `name_attempt_after` per ruling 1's cadence:
|
||
|
||
- **Answer with a valid PTR target**: store it in `learned_name`
|
||
(lowercased, no trailing dot), overwriting whatever learned name was there —
|
||
the router is the authority on its own zone, and a changed answer is a
|
||
renamed device, not a conflict. A test covers the overwrite: two attempts,
|
||
two different valid targets, second wins.
|
||
- **NXDOMAIN, or NOERROR with no matching PTR record** (NODATA): clear
|
||
`learned_name` to NULL. The router affirmatively says the address has no
|
||
name; keeping a stale one would show a device under its previous owner's
|
||
hostname after a lease change.
|
||
- **Everything else** — transport error, any RCODE other than NOERROR and
|
||
NXDOMAIN (REFUSED, FORMERR, SERVFAIL, unknown values), malformed reply,
|
||
invalid hostname (ruling 4): keep the stored `learned_name` as it stands,
|
||
count under `failed` (or `invalid`). An outage must not strip names from
|
||
the whole dashboard.
|
||
|
||
**"RCODE" here is the full 12-bit value**: the header's 4 bits extended by
|
||
the OPT record's upper 8 bits when the response carries one
|
||
(`packet.findOptRecord` locates it; `dns/edns.zig` decodes it). A response
|
||
whose header says NOERROR under a nonzero extended RCODE is a failure, not
|
||
an answer and not NODATA — classifying on the header nibble alone would
|
||
store or clear a name on what the resolver called an error. An OPT record
|
||
that fails to parse makes the whole reply malformed (`failed`, keep). The
|
||
first implementation had exactly this bug; the regression test is a
|
||
NOERROR header + OPT with a nonzero extended RCODE, asserting `failed`
|
||
counts and the stored name survives — it must fail with the classification
|
||
reverted to header-only.
|
||
|
||
**Staleness bound, stated honestly:** an address reassigned to a new device
|
||
immediately after a successful lookup shows the previous device's hostname for
|
||
up to `refresh_after_s` (24 h) — the next refresh then overwrites or clears
|
||
it. That bound is accepted: it matches the prune cadence's granularity, and
|
||
tightening it means more PTR traffic for a cosmetic lag. The docs state the
|
||
bound.
|
||
|
||
`upsertSeen`, `updateClient`, `pruneStale` and the reconcile engine are
|
||
untouched: a pruned row takes its learned name with it, a re-materialised
|
||
device is re-learned within a pass, and `updateClient` writing a name removes
|
||
the row from candidacy (the name predicate) without touching `learned_name`.
|
||
|
||
### 4. PTR answers are untrusted input: `acceptHostname` gates everything
|
||
|
||
The answer bytes come from whatever box the operator pointed a zone at, and
|
||
they land in a UI. Validation has three layers, all required:
|
||
|
||
1. **Transport**: `transport.validateResponse` (ID and question echo — that is
|
||
*all* it checks; the first draft leaned on it for more).
|
||
2. **Record selection**: walk `packet.answers`; the accepted record is the
|
||
**first** whose `rtype` is `.ptr`, whose class is `.in`, and whose owner
|
||
name equals the queried reverse name case-insensitively
|
||
(`name.eqlIgnoreCase`, name.zig:185). Records failing any of these are
|
||
skipped, not errors; an answer section with no accepted record under
|
||
RCODE NOERROR is NODATA (ruling 3 clears). Extra accepted records after
|
||
the first are ignored.
|
||
3. **Hostname shape**: the PTR target decodes via `record.rdataCname`
|
||
(record.zig:101 handles PTR) and formats via `name.formatText`; the text is
|
||
accepted only if every byte is in `[a-z0-9._-]` after ASCII-lowercasing
|
||
`A-Z`, labels are 1–63 bytes, the whole name is ≤ 253 bytes, no label
|
||
starts or ends with `-`, and there is no empty label (which also rejects a
|
||
trailing dot — `formatText` emits none, so one appearing is malformed).
|
||
Underscore is included because real DHCP hostnames carry it; nothing else
|
||
is. A failing target counts under `invalid` and keeps the stored name.
|
||
|
||
`acceptHostname` is a pure function in `src/local/reverse_name.zig`, tested
|
||
against: the empty string, a 254-byte name, a 64-byte label, `a b`, `a\x00b`,
|
||
`héllo`, `-x`, `x-.y`, `a..b`, `.a`, `a.` (trailing dot), and accepting
|
||
`nas-1.lan`, `my_printer.home`, `x`.
|
||
|
||
### 5. Schema: two runtime columns, not a reuse of `name`
|
||
|
||
`clients` gains, in `config_schema.ddl_v1` directly after `name`:
|
||
|
||
```sql
|
||
learned_name TEXT,
|
||
name_attempt_after INTEGER NOT NULL DEFAULT 0,
|
||
```
|
||
|
||
with the identical lines in PLAN §11.2. `name_attempt_after` is the epoch
|
||
second before which the resolver will not attempt this row again; 0 (the
|
||
default, and every pre-existing row) means "eligible now". Storing the *next*
|
||
attempt rather than the *last* one keeps the candidate predicate a plain
|
||
comparison (no addition that could overflow on a corrupt value, no special
|
||
case for "never attempted") and lets one column carry ruling 1's two cadences.
|
||
|
||
Reusing `name` with a provenance marker was rejected: `name` feeds
|
||
`listClients`, which feeds `nxdns export`, and a learned name in an export
|
||
would turn runtime state into configuration — it would churn export diffs as
|
||
leases move, and under file authority the next reconcile would fight it. With
|
||
separate columns:
|
||
|
||
- **Export/import untouched.** `listClients` selects `name` only
|
||
(clients_repo.zig:33-38); no code change, and a test proves an export before
|
||
and after a learned name lands is byte-identical.
|
||
- **Reconcile untouched.** The engine never touches runtime columns; the
|
||
learned columns ride the row exactly as `last_seen` does. Promotion
|
||
(declaring an observed address) keeps them; deletion of a declared row drops
|
||
them with the row, correctly.
|
||
- **File-authority mode needs no write rule.** Naming is a runtime write like
|
||
`upsertSeen`, permitted in both authority modes for the same reason. In
|
||
file mode the operator's *declared* name wins through the ordinary
|
||
precedence (a non-empty `name` removes the row from candidacy after the
|
||
reconcile writes it), and the learned name stays display-only runtime
|
||
state.
|
||
|
||
The baseline test in `src/storage/migrations.zig` ("a fresh database reaches
|
||
the baseline...") gains `columnExists` probes for both columns.
|
||
|
||
### 6. Repo surface: two calls, owned by the resolver
|
||
|
||
`src/storage/repositories/clients_repo.zig`, in the runtime section beside
|
||
`upsertSeen` / `pruneStale`:
|
||
|
||
- `resolveCandidates(database, out, now_s)` — fills a caller-supplied
|
||
fixed-capacity buffer (capacity `max_per_pass`, each slot
|
||
`logger.max_client_len` bytes — the bound every address text in this
|
||
program is sized by) and returns the filled slice or count. **No
|
||
allocation**: the candidate set is small by construction and the resolver
|
||
runs on the tracker's task, which owns no allocator today. The WHERE
|
||
clause, exactly:
|
||
|
||
```sql
|
||
(name IS NULL OR name = '') AND name_attempt_after <= ?1
|
||
ORDER BY name_attempt_after, ip LIMIT 16
|
||
```
|
||
|
||
Tests cover: `now_s` smaller than any cadence constant still selects
|
||
never-attempted rows (`DEFAULT 0`); the boundary `name_attempt_after =
|
||
now_s` selects; `now_s + 1` does not; an extreme stored value
|
||
(`i64` max) never selects and never traps.
|
||
- `noteNameOutcome(database, ip, outcome)` where the outcome carries
|
||
`attempt_after: i64` and one of: store `text`, clear, keep. One UPDATE:
|
||
always sets `name_attempt_after`; sets `learned_name` to the text on
|
||
store, to NULL on clear, leaves it alone on keep. **A row deleted between
|
||
selection and this call makes the UPDATE touch zero rows; that is a
|
||
no-op by design, not an error and not a counter** — the device left, and
|
||
nothing was learned about nothing.
|
||
|
||
`ClientRow` gains `learned_name: []const u8` (NULL reads as `""`, like
|
||
`name`); `readClientRow` and both SELECTs carry it. `name_attempt_after` is
|
||
**not** exposed on `ClientRow` or the API — it is scheduling state with no
|
||
operator meaning, and keeping it out keeps the contract surface one field.
|
||
`listClients` (export) is not widened — that is the point of ruling 5.
|
||
|
||
### 7. The resolver module: `src/server/client_names.zig`
|
||
|
||
New file. A `Resolver` struct owning:
|
||
|
||
- `stats: Stats` with the six **outcome** counters `attempted`, `answered`,
|
||
`nxdomain`, `no_zone`, `invalid`, `failed`, plus two counters *outside* the
|
||
outcome sum: `read_failures` (the candidate SELECT failed; the pass skips
|
||
naming) and `write_failures` (`noteNameOutcome` failed; the outcome was
|
||
still counted). Guarded by the tracker's mutex pattern, with a
|
||
`snapshotStats`. Invariant, asserted in a test: `attempted` equals
|
||
`answered + nxdomain + no_zone + invalid + failed`. Database failures log
|
||
at most one `warn` per pass each for reads and writes, tracker-style
|
||
(clients.zig:181-186); network failures are the counters' job and log
|
||
nothing new (the `forward_client` module's existing `debug`-level
|
||
diagnostics are exempt from this milestone's no-log rule — they predate it,
|
||
are debug-level, and suppressing them per-caller would fork the client).
|
||
- an **exchange seam**: production code cannot inject into `ForwardClient`
|
||
(it is a concrete struct the resolver constructs), so the seam sits above
|
||
it — a field
|
||
`exchangeFn: *const fn (io, validate.Resolver, query, response_buf) transport.ExchangeError![]u8`
|
||
defaulting to a function that builds a stack `ForwardClient` with
|
||
`ptr_read_timeout` and calls `exchange`. Tests replace the pointer; the
|
||
production default is itself covered by one test against a loopback UDP
|
||
socket if one already exists in the suite's patterns, otherwise by the live
|
||
smoke. The no-zone test runs the **production** pass code with a counting
|
||
stub and asserts the count stays 0.
|
||
- a `runPass(io, database, tables, now_s)`: select candidates (ruling 6), and
|
||
for each — build the reverse name (pure, ruling 8), acquire/match/**copy
|
||
resolver**/release (ruling 2), build the PTR query: header with a random id
|
||
filled from `io.random` (`std/Io.zig:2468` — *not* `std.crypto.random`,
|
||
which 0.16.0 does not have), RD set, one question, qtype `.ptr`, qclass
|
||
`.in`, encoded with `dns/header` + `dns/question.encode`; exchange through
|
||
the seam, classify per rulings 3–4, write per ruling 6. The id is
|
||
test-visible through the seam (the stub sees the query bytes), so no
|
||
determinism hook is needed beyond the seam itself.
|
||
|
||
Wiring: `Tracker.flushOnce` gains an optional `*client_names.Resolver`
|
||
parameter, invoked **after** the drain/write and prune steps (ruling 1's
|
||
order); `app.zig` constructs the Resolver beside the tracker (holding the
|
||
`*LocalTables` pointer) and passes it through `Tracker.run`'s arguments
|
||
(app.zig:754). The resolver runs on the tracker's task against `tracker_db` —
|
||
the dedicated-connection rule (clients.zig:127-131) is satisfied because it
|
||
is the *same* task, serial with the flush. A gated pass (disk critical) skips
|
||
naming too: naming writes.
|
||
|
||
### 8. The pure part: `src/local/reverse_name.zig`
|
||
|
||
New file, pure (no Io, no clock). Two functions plus their tests:
|
||
|
||
- `reverseName(addr: address.NetAddress, buf: *[max_reverse_len]u8) []const u8`
|
||
— `d.c.b.a.in-addr.arpa` for v4, the 32-nibble lowercase `ip6.arpa` form for
|
||
v6. `max_reverse_len` is a comptime bound (v6 form: 32 nibbles · 2 + 8 for
|
||
`ip6.arpa` = 72; compute, don't hand-wave). An IPv4-mapped v6 address was
|
||
already canonicalised to `ip4` by `NetAddress` (clients.zig test at :305
|
||
proves the tracker's addresses are canonical) — assert, don't re-handle.
|
||
- `acceptHostname(text: []const u8) bool` — ruling 4's gate, with ruling 4's
|
||
test table.
|
||
|
||
`src/platform/address.zig` types are plain values; importing them into
|
||
`local/` keeps the module pure. Register the file in `src/tests.zig` the way
|
||
its siblings are registered.
|
||
|
||
### 9. Failure visibility: one metrics group
|
||
|
||
The resolver must be reachable from `metrics.collect`, which reads
|
||
`server.WebState` — so `src/web/server.zig`'s `WebState` gains an optional
|
||
`client_names: ?*client_names.Resolver` collaborator, populated by `app.zig`
|
||
beside `tracker` (the first draft wired the sample and forgot the state; S2
|
||
owns `src/web/server.zig` for this field). `src/web/metrics.zig`: `Sample`
|
||
gains `client_names: ?client_names.Resolver.Stats`, collected beside the
|
||
tracker's (metrics.zig:191-193) and rendered as
|
||
`counterGroup(w, "nxdns_client_names_", ...)` beside `nxdns_clients_`
|
||
(metrics.zig:333-334). `counterGroup` renders every struct field, so all
|
||
eight counters (six outcomes + `read_failures` + `write_failures`) surface
|
||
without per-field code. The render test pins one line (e.g.
|
||
`nxdns_client_names_no_zone_total`).
|
||
|
||
### 10. API and UI: learned is visible and visibly different
|
||
|
||
- `GET /api/clients` rows carry `learned_name`
|
||
(`src/web/openapi.yaml` `required` + `properties` for the client schema;
|
||
contract samples regenerated; `web/src/lib/types.ts` client row type gains
|
||
`learned_name: string`). `name_attempt_after` stays internal (ruling 6).
|
||
- `web/src/features/clients/ClientsPage.tsx`: the name cell shows `name` when
|
||
non-empty; otherwise `learned_name` in the page's existing muted text style
|
||
with a visually-distinct treatment that marks it as learned (exact
|
||
affordance is the implementer's, but: no new colour system, reuse the muted
|
||
style the page already has, and the distinction must survive a screen
|
||
reader — an `aria-label` or visible suffix, not colour alone). An empty
|
||
both shows what it shows today.
|
||
- `ClientEditDialog.tsx` may pre-fill its name field with `learned_name` when
|
||
`name` is empty — adopting the learned name as a typed name is the natural
|
||
gesture — but saving still goes through the ordinary PUT and sets
|
||
`hand_edited` server-side as today. This applies in database-authority mode
|
||
only; in file mode client edits answer 403 as they do today, and the
|
||
file-mode story is ruling 5's (declared name wins via candidacy).
|
||
- Mocks in `ClientsPage.test.tsx` gain the field with distinct values;
|
||
rendering tests assert: a named row shows `name` and not `learned_name`; an
|
||
unnamed row shows `learned_name` with the learned affordance.
|
||
|
||
### 11. Docs
|
||
|
||
- `docs/reference/configuration.md`, clients section: a paragraph stating the
|
||
learned-name mechanism; that it requires a conditional forward zone
|
||
covering the LAN's reverse space (with the `168.192.in-addr.arpa` example);
|
||
that hand-typed names win; that learned names never appear in exports; that
|
||
a rename or lease change can display stale for up to a day (ruling 3's
|
||
bound). The cadence must be described as it is, not rounded up: each pass
|
||
attempts **at most 16 due rows**, and a row is due per ruling 1's two
|
||
cadences — "every unnamed device, once a minute" overstates both coverage
|
||
and rate and must not appear; and one sentence that PTR queries go to the declared zone's
|
||
resolver, wherever the operator pointed it (ruling 2's scope).
|
||
- The how-to/tutorial page that documents conditional forward zones gains the
|
||
reverse-zone example if it lacks one (locate it; do not guess its path).
|
||
- PLAN §7.2 gains one sentence: materialised clients are named by PTR through
|
||
the declared forward zones; learned names are runtime state.
|
||
- `docs/reference/api.md` only if it enumerates client row fields (check).
|
||
|
||
## Sessions
|
||
|
||
Three, strictly sequential: S1 → S2 → S3. S2 needs S1's columns and repo
|
||
calls; S2 owns the contract regeneration (the API shape changes when S1 widens
|
||
`ClientRow`, but the sample regen runs the integration suite, which S2's
|
||
wiring changes — one regen at the end of S2 avoids regenerating twice). S3
|
||
reads the fields S2's regenerated types carry.
|
||
|
||
### Session S1: pure module, schema, repo
|
||
|
||
Owns: `src/local/reverse_name.zig` (new), `src/tests.zig` (registration),
|
||
`src/storage/config_schema.zig`, `src/storage/migrations.zig` (test),
|
||
`src/storage/repositories/clients_repo.zig`, `PLAN.md` (§11.2 lines only).
|
||
|
||
- S1.1 ruling 8: `reverse_name.zig` with exhaustive tests (v4, v6, bounds;
|
||
`acceptHostname` accept/reject table from ruling 4).
|
||
- S1.2 ruling 5: the two DDL lines in both copies, baseline column probes.
|
||
- S1.3 ruling 6: repo calls, `ClientRow` widening, tests — including: the
|
||
candidate SQL's boundary and extreme-value cases; the ordering; NULL
|
||
round-trips as `""`; export is byte-stable across a `noteNameOutcome`;
|
||
`updateClient` leaves `learned_name` alone; the zero-row UPDATE no-op.
|
||
|
||
Acceptance (S1):
|
||
- [ ] `zig build test` passes; baseline test proves both columns exist.
|
||
- [ ] `reverseName` output for `192.168.1.10` is
|
||
`10.1.168.192.in-addr.arpa` and for `fd00::1` is the full 32-nibble
|
||
lowercase form; both are matched by a `forward_zones.Zones` built over
|
||
`168.192.in-addr.arpa` / a covering `ip6.arpa` zone in a test that ties
|
||
the two modules together.
|
||
- [ ] `acceptHostname` passes ruling 4's full accept/reject table, including
|
||
`a..b`, `.a` and the trailing-dot case.
|
||
- [ ] An export taken before and after `noteNameOutcome` on an observed row is
|
||
byte-identical.
|
||
|
||
### Session S2: resolver, wiring, metrics, API contract (needs S1)
|
||
|
||
Owns: `src/server/client_names.zig` (new), `src/tests.zig` (registration),
|
||
`src/server/clients.zig`, `src/app.zig`, `src/web/server.zig` (the
|
||
`client_names` field), `src/web/metrics.zig`, `src/web/openapi.yaml`,
|
||
`web/src/lib/types.ts`, `web/src/lib/contractSamples.gen.ts` (regenerated),
|
||
`web/src/features/clients/ClientsPage.test.tsx` (mock fields only).
|
||
|
||
- S2.1 ruling 7: the Resolver, the exchange seam, `flushOnce` hook (ruling 1
|
||
order), app and WebState wiring.
|
||
- S2.2 rulings 1–4: classification tests against a stub exchange. Tests must
|
||
cover: answered stores and lowercases; a second answered overwrites;
|
||
NXDOMAIN clears; NODATA clears; REFUSED and an unknown RCODE keep and count
|
||
`failed`; timeout keeps; invalid hostname keeps and counts `invalid`; a
|
||
record with wrong type, class, or owner name is skipped; no-zone sends
|
||
nothing (the stub's call count is the assertion) and counts;
|
||
`name_attempt_after` lands at `now + refresh_after_s` for definitive and
|
||
`now + retry_after_s` for non-definitive outcomes; the candidate cap holds;
|
||
a named row is never attempted; a gated pass attempts nothing; drain
|
||
precedes any exchange when every exchange times out (ruling 1); a row past
|
||
the prune cutoff on a due pass causes no exchange (ruling 1); the
|
||
two-generation swap test per ruling 2's exact shape (swap inside the
|
||
hazard window, both assertions); a NOERROR header under a nonzero OPT
|
||
extended RCODE counts `failed` and keeps the stored name (ruling 3); the
|
||
default `exchangeFn` bounds UDP plus the TCP fallback under one
|
||
`ptr_read_timeout` deadline (ruling 1) — a loopback resolver that answers
|
||
UDP with TC=1 late in the budget and then stalls TCP must fail the attempt
|
||
within one budget, not two (assert elapsed with margin; inject a shortened
|
||
budget locally if the default seam needs one).
|
||
- S2.3 ruling 9: WebState field, metrics sample, render pin.
|
||
- S2.4 ruling 10's API half: openapi.yaml, regen samples, types.ts, mock
|
||
fields.
|
||
|
||
Acceptance (S2):
|
||
- [ ] `zig build test` and `zig build test -Dintegration` pass; the
|
||
regenerated samples carry `learned_name` in the client row sample and
|
||
do not carry `name_attempt_after`.
|
||
- [ ] `cd web && npm run typecheck && npm test` pass with the widened types.
|
||
- [ ] The stats-sum invariant test passes (`attempted` = sum of the five
|
||
outcomes; `read_failures`/`write_failures` outside it).
|
||
- [ ] The no-zone test proves zero exchanges for an uncovered reverse name
|
||
through the production pass code.
|
||
- [ ] The concurrent-swap regression test passes (ruling 2).
|
||
|
||
### Session S3: UI and docs (needs S2)
|
||
|
||
Owns: `web/src/features/clients/ClientsPage.tsx`,
|
||
`web/src/features/clients/ClientEditDialog.tsx`,
|
||
`web/src/features/clients/ClientsPage.test.tsx` (rendering assertions),
|
||
`docs/reference/configuration.md`, `docs/reference/api.md` (if applicable),
|
||
the conditional-forwarding doc page, `PLAN.md` (§7.2 sentence only).
|
||
|
||
- S3.1 ruling 10's UI half.
|
||
- S3.2 ruling 11.
|
||
|
||
Acceptance (S3):
|
||
- [ ] `cd web && npm run typecheck && npm test && npm run lint && npm run
|
||
build` pass; rendering tests cover named-wins and learned-affordance.
|
||
- [ ] The docs name the reverse-zone prerequisite with a concrete
|
||
`in-addr.arpa` example, the one-day staleness bound, and the
|
||
resolver-is-the-operator's-choice sentence.
|
||
|
||
### Orchestrator
|
||
|
||
Verify each session's acceptance before starting the next. After S3, the full
|
||
gate set (`zig build test`, `zig build test -Dintegration`, `test-aarch64` if
|
||
qemu is present, `cd web && npm test`, `npm run assert-bundled`), then a live
|
||
smoke per the verify-against-the-real-network rule: a scratch server with a
|
||
forward zone `168.192.in-addr.arpa → udp://<router>:53` (or a local stub
|
||
resolver serving PTR), one real query from a LAN client, then wait one flush
|
||
interval and confirm the learned name in `GET /api/clients`, in the UI, and in
|
||
`nxdns_client_names_answered_total`. Then delete the zone and confirm the next
|
||
attempt counts `no_zone` with no upstream traffic (tcpdump or the pool
|
||
counters — pool queries must not move); with the hourly non-definitive
|
||
cadence, age `name_attempt_after` by SQL in the scratch database rather than
|
||
waiting. In database mode, hand-edit the name and confirm it wins and the row
|
||
stops being attempted. In file mode, confirm a file-declared name wins after
|
||
reconcile and the learned name remains display-only. Record deviations in
|
||
`## Recorded (implementation)`.
|
||
|
||
## Module layout
|
||
|
||
New files: `src/local/reverse_name.zig`, `src/server/client_names.zig`.
|
||
Deleted surface: none.
|
||
|
||
## File ownership
|
||
|
||
| File | Session |
|
||
| --- | --- |
|
||
| `src/local/reverse_name.zig` (new) | S1 |
|
||
| `src/storage/config_schema.zig`, `src/storage/migrations.zig` (test) | S1 |
|
||
| `src/storage/repositories/clients_repo.zig` | S1 |
|
||
| `PLAN.md` | S1 (§11.2 lines), then S3 (§7.2 sentence) — sequential, never concurrent |
|
||
| `src/tests.zig` | S1, then S2 — sequential |
|
||
| `src/server/client_names.zig` (new) | S2 |
|
||
| `src/server/clients.zig`, `src/app.zig`, `src/web/server.zig`, `src/web/metrics.zig` | S2 |
|
||
| `src/web/openapi.yaml`, `web/src/lib/types.ts`, `web/src/lib/contractSamples.gen.ts` | S2 |
|
||
| `web/src/features/clients/ClientsPage.test.tsx` | S2 (mock fields), then S3 (assertions) — sequential |
|
||
| `web/src/features/clients/ClientsPage.tsx`, `ClientEditDialog.tsx` | S3 |
|
||
| `docs/reference/configuration.md`, `docs/reference/api.md`, forwarding doc page | S3 |
|
||
|
||
## Acceptance (milestone complete)
|
||
|
||
- [ ] All session acceptance boxes.
|
||
- [ ] `config_schema.ddl_v1` and PLAN §11.2 byte-identical, both carrying the
|
||
two lines; `migrations.steps` still one step, `target_version` still 1.
|
||
- [ ] The live smoke: a real client's hostname appears without any operator
|
||
edit; removing the zone stops queries entirely (`no_zone` moves, pool
|
||
counters do not).
|
||
- [ ] `nxdns export` output is byte-identical before and after names are
|
||
learned.
|
||
- [ ] Database mode: hand-editing a name wins over the learned name
|
||
immediately and permanently (the row leaves candidacy). File mode: a
|
||
file-declared name wins after reconcile; the learned name is
|
||
display-only runtime state.
|
||
- [ ] `src/web/openapi.yaml` reviewed by hand against the changed client row
|
||
shape, reviewer says so in `## Recorded` (no automated guard covers
|
||
schema-to-response agreement — milestone-24 finding, still true).
|
||
|
||
## Anti-requirements
|
||
|
||
- No migration step, no `ddl_v2`, no runtime schema probing.
|
||
- No PTR query to the upstream pool, and none for a reverse name no declared
|
||
forward zone covers — under any fallback, ever. (The declared resolver
|
||
itself is the operator's choice; ruling 2 scopes the promise.)
|
||
- No reuse of `clients.name` for learned names, and no learned data in
|
||
`nxdns export` / `import` ZON or in reconcile semantics.
|
||
- No mDNS, NetBIOS, DHCP-lease-file parsing, or any second naming source.
|
||
PTR through declared zones is the mechanism; a router that serves no
|
||
reverse zone yields `no_zone` counts and an unnamed row, visibly.
|
||
- No per-query or query-path resolution: naming rides the flush pass only.
|
||
- No config knobs for cadence, caps, or timeouts; constants per ruling 1.
|
||
- No DNS-cache participation for PTR probes.
|
||
- No unbounded anything: candidates per pass, in-flight, per-attempt timeout,
|
||
name length, and retry cadence are all bounded above by rulings 1 and 4.
|
||
- No storing or displaying a PTR target that fails `acceptHostname` — not
|
||
even truncated or escaped.
|
||
- No new per-attempt log lines from this milestone's code; counters and the
|
||
metrics group are the surface. `forward_client.zig`'s existing debug
|
||
diagnostics stand as they are.
|
||
- No allocation on the naming path: candidate storage is fixed-capacity
|
||
(ruling 6).
|
||
|
||
## Recorded (implementation)
|
||
|
||
- `src/server/phase7_integration_test.zig` sits outside the ownership table
|
||
but gained a mechanical `, null` argument at every `flushOnce`/`run` call
|
||
site, following the new `names: ?*client_names.Resolver` parameter.
|
||
- Review: one external round found four defects (extended RCODE ignored the
|
||
OPT upper bits; `exchangeWithin` did not bound the UDP+TCP pair under one
|
||
deadline; the swap test asserted nothing observable; a docs cadence
|
||
sentence). All four were fixed; the re-review of
|
||
`src/server/client_names.zig` returned no findings.
|
||
- Every fix shipped with a test the author watched fail with the fix
|
||
reverted. For the swap test the hazard was injected (a pass-level resolver
|
||
cache that skips re-acquisition) and the second-port assertion failed; the
|
||
one-deadline test's bound is `budget + late/2` (550 ms) because the
|
||
unbounded path takes ~700 ms and a 2×-budget bound would pass it.
|
||
- `src/web/openapi.yaml` was reviewed by hand against `ClientRow`
|
||
(clients_repo.zig:312): the nine required fields match one-to-one and
|
||
`name_attempt_after` is absent from the response, as intended.
|
||
- Live smoke (scratch server, file mode, zone `127.in-addr.arpa` at a local
|
||
stub PTR resolver): one real query materialised the client; the flush pass
|
||
learned `smoke-host.lan` with no operator edit, visible in
|
||
`GET /api/clients` and `nxdns_client_names_answered_total`. Removing the
|
||
zone and re-arming `name_attempt_after` produced `no_zone` with zero
|
||
packets to the stub and no pool movement. The learned name survived a
|
||
restart's reconcile. A file-declared `name` won in the API after reconcile
|
||
with the learned name still present as display-only state, and the named
|
||
row left candidacy (`attempted` stayed 0 over a full flush interval). The
|
||
database-mode hand-edit path runs the same candidacy SQL
|
||
(`name IS NULL OR name = ''`) and is covered by the repo unit tests rather
|
||
than a second live run. The UI half is covered by the ClientsPage
|
||
rendering tests, not a live browser check.
|
||
- The first smoke attempt polled a stale pre-milestone binary out of
|
||
`zig-out/bin` — `zig build test` does not refresh the install step. Rebuild
|
||
before any live check.
|