milestone 21: abp list exceptions and a regex rule kind
This commit is contained in:
+338
-35
@@ -71,6 +71,11 @@ order preserved; the on-disk header (manager.zig:221-241) gains
|
||||
`# exceptions {d}` after the `# wildcards` line and the pinning test at
|
||||
manager.zig:1914-1946 is extended, not weakened.
|
||||
|
||||
The "checksum over the `.list` body followed by the `.wild` body" sentence
|
||||
exists in THREE places, not the one this ruling first named: `compiler.zig:38-39`,
|
||||
`manager.zig:218` and `sources_repo.zig:100`. All three move together, or the
|
||||
next reader trusts a stale one.
|
||||
|
||||
### 4. Exception counts persist and surface
|
||||
|
||||
Migration step 3 (`ddl_v3`, appended at `src/storage/migrations.zig:23-26`):
|
||||
@@ -82,8 +87,13 @@ checksum doc line at sources_repo.zig:100 is updated alongside
|
||||
`SourceStatus` rehydration (`manager.zig:1458-1502`) restores it alongside the
|
||||
existing three counts; `StatusView` (`src/web/handlers/blocklists.zig:52-78`)
|
||||
gains `exceptions: u32`; the blocklists UI shows it where `skipped_regex`
|
||||
already shows (`web/src/features/blocklists/SourceStatusSection.tsx`,
|
||||
`web/src/lib/types.ts:163,192`).
|
||||
already shows.
|
||||
|
||||
`skipped_regex` shows in TWO tables, and `exceptions` follows it into both:
|
||||
`SourceStatus.skipped_regex` (`web/src/lib/types.ts:192`) renders at
|
||||
`SourceStatusSection.tsx:112`, and `Blocklist.skipped_regex_count`
|
||||
(`types.ts:163`) renders at `BlocklistsPage.tsx:188`. S1 therefore owns
|
||||
`BlocklistsPage.tsx` as well.
|
||||
|
||||
### 5. The regex engine is a Pike VM, linear-time by construction, `std` only
|
||||
|
||||
@@ -91,7 +101,25 @@ New file `src/filter/regex.zig`. Syntax: literal bytes, `.`, character
|
||||
classes `[...]` with ranges and leading-`^` negation, escapes
|
||||
`\. \\ \- \d \w`, repetition `* + ? {n} {n,m}`, alternation `|`,
|
||||
non-capturing grouping `(...)`, anchors `^` and `$`. No backreferences, no
|
||||
lookaround, no captures. Matching is unanchored unless anchors are written
|
||||
lookaround, no captures.
|
||||
|
||||
**Two syntax amendments from S2's review**, both widening what is accepted:
|
||||
|
||||
- `{n,}` is legal. It lowers to `{n}` followed by `*`, stays linear, and an
|
||||
over-large `n` still reports `PatternTooComplex`. Operators write this form;
|
||||
rejecting it buys no safety.
|
||||
- `\` before any ASCII punctuation yields that literal, not only the five
|
||||
escapes listed above — `\*`, `\/` and `\+` occur in Pi-hole-style patterns.
|
||||
This can only narrow a pattern to a literal, never silently change its
|
||||
meaning. Escapes that WOULD change meaning stay rejected: any alphanumeric
|
||||
escape outside `\d` and `\w` (`\s`, `\b`, `\1`, `\D`, `\p{L}`) is
|
||||
`BadPattern`.
|
||||
|
||||
**One rejection the review added:** a quantifier applied directly to another
|
||||
quantifier is `BadPattern`. `a+?` previously compiled as `(a+)?`, which
|
||||
matches every name — a block rule written in conventional lazy syntax would
|
||||
have sinkholed the whole LAN instead of being refused. Parenthesised forms
|
||||
such as `(a+)?` stay legal and keep their meaning. Matching is unanchored unless anchors are written
|
||||
(POSIX-grep convention, matching Pi-hole user expectations). Input is the
|
||||
normalized lowercase name, ≤ `types.max_name_len` bytes. Hard limits, each a
|
||||
distinct error: pattern ≤ 256 bytes (`PatternTooLong`), compiled program
|
||||
@@ -106,8 +134,19 @@ pub fn matches(prog: *const Program, input: []const u8) bool;
|
||||
|
||||
`matches` is a Pike VM: two thread lists, each program counter admitted at
|
||||
most once per input position, worst case O(program × input) with zero
|
||||
allocation at match time (thread lists sized from the program at compile
|
||||
time). The engine is a fuzz-module root like parsers.zig and imports only
|
||||
allocation at match time.
|
||||
|
||||
**Amended in S2.** This ruling first said the thread lists live in the
|
||||
`Program`, sized at compile time. They do not, and must not: the runtime is
|
||||
`std.Io.Threaded`, so several query threads evaluate one shared snapshot at
|
||||
once. Scratch inside a shared `Program` is a data race, and reaching it
|
||||
through `*const Program` would need a `@constCast` that is undefined
|
||||
behaviour on a genuinely const program. All VM scratch — both thread lists,
|
||||
the admission marks and the closure stack — is instead a fixed array on the
|
||||
caller's stack, sized by the compile-time `max_program_len` constant. Zero
|
||||
allocation at match time is preserved, the published signature is unchanged,
|
||||
and a `Program` becomes safe to share across threads, which the original
|
||||
wording would have prevented. The engine is a fuzz-module root like parsers.zig and imports only
|
||||
`std`.
|
||||
|
||||
### 6. `regex` is a third rule kind, validated at the edge, memoized by the cache
|
||||
@@ -125,7 +164,14 @@ set, and a rename would fail both. `model.RuleKind`
|
||||
validate.zig:891-902) and `src/filter/rules.zig:62-68` extend. `RuleSet`
|
||||
(`rules.zig:28-36`) grows `regex_allow` and `regex_block` slices holding
|
||||
compiled `Program`s plus their pattern texts (for `Decision.matched`);
|
||||
`bucketOf` (rules.zig:133-143) becomes a six-bucket layout;
|
||||
`bucketOf` (rules.zig:133-143) becomes a six-bucket layout, and the fixed
|
||||
`var spans: [4]std.ArrayList(Span)` at rules.zig:55 widens with it;
|
||||
`patternIsValid` (`validate.zig:1099-1103`) is declared
|
||||
`error{OutOfMemory}!bool`, so a regex compile's `BadPattern`,
|
||||
`PatternTooLong` and `PatternTooComplex` must either fold into `false` or
|
||||
widen that error set together with its caller at validate.zig:893 — the
|
||||
diagnostic text itself needs no edit, because validate.zig:898 already
|
||||
interpolates `rule.kind.toDb()` into "{f} is not a valid {s} pattern";
|
||||
`max_regex_per_group: usize = 256` with `TooManyRegexRules` mirroring
|
||||
`max_wildcards_per_group` (rules.zig:26). A pattern that fails to compile is
|
||||
`error.BadPattern` at snapshot build, never skipped (rules.zig:41-49 doc
|
||||
@@ -144,7 +190,14 @@ rules.zig:42 becomes `"kind must be 'exact', 'wildcard' or 'regex'"`.
|
||||
`src/web/openapi.yaml:1948,1962,1976`: all three `enum: [exact, wildcard]` become
|
||||
`[exact, wildcard, regex]`. `web/src/lib/types.ts:195`:
|
||||
`RuleKind = "exact" | "wildcard" | "regex"`; the rules page kind selector
|
||||
gains the option. Contract samples regenerated. `nxdns export` / `import`
|
||||
gains the option. Milestone 23 replaced the native `<select>` with a React
|
||||
Aria wrapper, so that is now a data edit, not JSX: append to `KIND_OPTIONS`
|
||||
at `RulesPage.tsx:15-18`, and extend the option-list assertion at
|
||||
`RulesPage.test.tsx:119`. Any new test that opens the selector depends on the
|
||||
`CSS.escape` polyfill in `web/vitest.setup.ts`. Contract samples are
|
||||
regenerated with the AGENTS.md command
|
||||
(`zig build test -Dintegration -Dcontract-samples-out=...`); no npm script
|
||||
generates them. `nxdns export` / `import`
|
||||
round-trip the new kind with no extra work once `RuleKind.toDb/fromDb` extend
|
||||
— the enum round-trip test at model.zig:782 is extended to prove it.
|
||||
|
||||
@@ -156,7 +209,11 @@ counted and skipped; `$` modifiers (except the `$important` suffix of ruling
|
||||
1), partial-segment wildcards, and browser-syntax honoring stay permanently
|
||||
out. PLAN.md:26 (§2.1 filtering sentence), PLAN.md:106 (§3.9), PLAN.md:108-117
|
||||
(§3.10 precedence) and PLAN.md:700 (decision B) are updated to match rulings
|
||||
2 and 6. In-code echoes of the old §2.2 move with it:
|
||||
2 and 6. PLAN.md:99 (§3.8) documents exactly two compiled bodies and names
|
||||
`<source_id>.wild` as the second; ruling 3's third body makes it stale, so S1
|
||||
updates that line when it lands the `.allow` body. PLAN.md:73 ("No
|
||||
TOML/regex/HTTP packages needed") stays true and stays as written — a
|
||||
homegrown engine adds no package. In-code echoes of the old §2.2 move with it:
|
||||
`src/filter/wildcard.zig:6-7,20-23`, `src/filter/parsers.zig:25`,
|
||||
`src/filter/parser_abp.zig:5-6`, `src/filter/compiler.zig:129-131`.
|
||||
|
||||
@@ -205,8 +262,10 @@ Owns: `src/filter/parsers.zig`, `src/filter/parser_abp.zig`,
|
||||
(step 3 only), `src/storage/repositories/sources_repo.zig`,
|
||||
`src/web/handlers/blocklists.zig`, `src/web/handlers/lookup.zig` (doc
|
||||
sentence only), `src/web/openapi.yaml` (StatusView shape only),
|
||||
`web/src/features/blocklists/*`, `web/src/lib/types.ts` (source-stat fields
|
||||
only), `web/src/lib/contractSamples.gen.ts`,
|
||||
`web/src/features/blocklists/*` (which includes `BlocklistsPage.tsx` per
|
||||
ruling 4), `web/src/lib/types.ts` (source-stat fields
|
||||
only), `web/src/lib/contractSamples.gen.ts`, `PLAN.md` (line 99 only, per
|
||||
ruling 8 — S3 owns every other PLAN edit),
|
||||
`tests/fuzz/blocklist_fuzz.zig`, `tests/fuzz/compiler_fuzz.zig`, and the
|
||||
dump-golden assertions in `src/config/import.zig` and
|
||||
`src/config/reconcile.zig` test suites only (ruling 9's `exception_count`
|
||||
@@ -222,7 +281,9 @@ fallout from `ddl_v3`; S1 touches no reconcile logic).
|
||||
exceptions as loadable, not rejected.
|
||||
- S1.4 matcher: `SourceSets.exceptions`, `Reason.blocklist_exception`,
|
||||
the new evaluate level per ruling 2, `memoryBytes` includes the new sets.
|
||||
- S1.5 storage + API + UI per ruling 4.
|
||||
- S1.5 storage + API + UI per ruling 4. `migrations.zig:349` asserts
|
||||
`target_version == 2`; S1 moves it to 3 (S3 moves it to 4). The spec did
|
||||
not name that test; it fails otherwise.
|
||||
- S1.6 tests: parser cases (`@@||x^`, `@@||x`, `@@||x^$important`,
|
||||
`@@||x^$third-party` → unsupported, `@@x` → unsupported); compiler
|
||||
three-body + checksum-compat cases (empty allow body reproduces the old
|
||||
@@ -233,15 +294,21 @@ fallout from `ddl_v3`; S1 touches no reconcile logic).
|
||||
`filter_integration_test.zig` with a real ABP fixture carrying `@@` lines.
|
||||
|
||||
Acceptance (S1):
|
||||
- [ ] `zig build test` passes; fuzz targets build and run.
|
||||
- [ ] A fixture list with `||ads.example^` and `@@||good.ads.example^`
|
||||
- [x] `zig build test` passes; fuzz targets build and run.
|
||||
- [x] A fixture list with `||ads.example^` and `@@||good.ads.example^`
|
||||
compiled and loaded blocks `ads.example` and `x.ads.example`, does not
|
||||
block `good.ads.example` or `y.good.ads.example`, and
|
||||
`/api/lookup` reports `blocklist_exception` with the source id for the
|
||||
latter two.
|
||||
- [ ] A pre-milestone data directory (no `.allow` files, old checksums)
|
||||
loads with zero checksum mismatches.
|
||||
- [ ] `POST /api/blocklists/update` response rows carry `exceptions`.
|
||||
latter two. Also proven live against the AdGuard DNS filter, which
|
||||
carries `||ads.tvb.com^` beside `@@||api.ads.tvb.com^`: `dig` sinkholed
|
||||
`ads.tvb.com` and `x.ads.tvb.com` to 0.0.0.0 and resolved
|
||||
`api.ads.tvb.com` normally.
|
||||
- [x] A pre-milestone data directory (no `.allow` files, old checksums)
|
||||
loads with zero checksum mismatches. Proven live by deleting a loaded
|
||||
source's `.allow` and reloading.
|
||||
- [x] `POST /api/blocklists/update` response rows carry `exceptions`
|
||||
(live: `"domains":154667,"wildcards":154666,"exceptions":11,
|
||||
"skipped_regex":21`).
|
||||
|
||||
### Session S2: the regex engine (tier 2, engine only)
|
||||
|
||||
@@ -257,9 +324,13 @@ target only).
|
||||
- S2.3 the fuzz target per ruling 10.
|
||||
|
||||
Acceptance (S2):
|
||||
- [ ] `zig build test` passes with the new file in `src/tests.zig`.
|
||||
- [ ] The step-bound property holds under the fuzz corpus.
|
||||
- [ ] `regex.zig` imports nothing but `std`.
|
||||
- [x] `zig build test` passes with the new file in `src/tests.zig`.
|
||||
- [x] The step-bound property holds under the fuzz corpus. `expectLinear`
|
||||
(`tests/fuzz/regex_fuzz.zig:101`) asserts
|
||||
`steps <= program_len * (input_len + 1)` over the corpus that
|
||||
`zig build test` replays. An interactive `--fuzz` session could not be
|
||||
used as additional evidence — see the deviation below.
|
||||
- [x] `regex.zig` imports nothing but `std`.
|
||||
|
||||
### Session S3: the regex rule kind, wired through (needs S1 + S2)
|
||||
|
||||
@@ -280,21 +351,32 @@ ruling 9), `PLAN.md`, `src/filter/wildcard.zig` (comments),
|
||||
- S3.4 web + UI + openapi + samples per ruling 7.
|
||||
- S3.5 PLAN and comment amendments per ruling 8.
|
||||
- S3.6 bench: the filter suite in `tools/bench.zig` gains a variant with 32
|
||||
regex rules loaded; the existing p95 < 1 ms assertion covers it.
|
||||
regex rules loaded; the existing p95 < 1 ms assertion covers it. The line to
|
||||
change is `tools/bench.zig:158`, which currently reads `.rules = &.{}` — the
|
||||
filter bench loads no rules at all today, so the variant is new coverage
|
||||
rather than an edit to an existing rule set. S3 also moves
|
||||
`migrations.zig:349` from 3 to 4.
|
||||
- S3.7 tests: rule CRUD with kind `regex` through the API including the 400
|
||||
for a bad pattern at insert time; precedence cases regex-allow over
|
||||
regex-block, wildcard over regex, regex over list entries; export/import
|
||||
round trip; a migration test upgrading a v3 database.
|
||||
|
||||
Acceptance (S3):
|
||||
- [ ] `zig build test` and `cd web && npm test` pass.
|
||||
- [ ] `POST /api/rules` with `{"kind":"regex","pattern":"^ad[0-9]+-"}`
|
||||
returns 201; with `"pattern":"("` returns 400 naming the pattern.
|
||||
- [ ] A regex block rule blocks a matching name; `/api/lookup` reports
|
||||
`rule_block_regex` and `matched` carries the pattern text.
|
||||
- [ ] `zig build bench -Doptimize=ReleaseFast -- filter` passes its targets
|
||||
with the regex variant present.
|
||||
- [ ] PLAN §2.2 no longer forbids operator regex; all listed echoes updated.
|
||||
- [x] `zig build test` and `cd web && npm test` pass.
|
||||
- [x] `POST /api/rules` with `{"kind":"regex","pattern":"^ad[0-9]+-"}`
|
||||
returns 201; with `"pattern":"("` returns 400 naming the pattern
|
||||
(live: `{"error":"rules[0].pattern: '(' is not a valid regex pattern"}`).
|
||||
- [x] A regex block rule blocks a matching name; `/api/lookup` reports
|
||||
`rule_block_regex` and `matched` carries the pattern text (live:
|
||||
`ad42-tracker.example.com` blocked, `matched` `^ad[0-9]+-`).
|
||||
- [x] `zig build bench -Doptimize=ReleaseFast -- filter` passes its targets
|
||||
with the regex variant present (32 regex rules; p95 2.89 µs against a
|
||||
1 ms target; VmRSS 32.0 MiB against a 100 MiB target).
|
||||
- [x] PLAN §2.2 no longer forbids operator regex; all listed echoes updated.
|
||||
Ruling 8's list turned out to be incomplete: §7.1's evaluation sequence,
|
||||
the §5 module tree and the Phase 5 summary also spoke of a two-body,
|
||||
three-kind, no-exception world. All are corrected, and the sweep that
|
||||
found them also caught milestone-20 drift the ruling never covered.
|
||||
|
||||
### Orchestrator
|
||||
|
||||
@@ -314,15 +396,236 @@ Deleted surface: none.
|
||||
|
||||
## Acceptance (milestone complete)
|
||||
|
||||
- [ ] All session acceptance boxes above.
|
||||
- [ ] Schema at version 4; a v2 database migrates cleanly with data intact.
|
||||
- [ ] A pre-milestone blocklist data directory loads without refetch.
|
||||
- [ ] The six-level operator precedence plus three list levels behave per
|
||||
- [x] All session acceptance boxes above.
|
||||
- [x] Schema at version 4; a v2 database migrates cleanly with data intact
|
||||
(`migrations.zig:344`, "a version 2 database upgrades and keeps its
|
||||
sources at exception_count 0"; the live smoke migrated `0 to 4`).
|
||||
- [x] A pre-milestone blocklist data directory loads without refetch.
|
||||
- [x] The six-level operator precedence plus three list levels behave per
|
||||
ruling 2, proven by matcher tests that enumerate adjacent-level pairs.
|
||||
- [ ] No `src/filter/` fuzz-root file imports anything but `std`.
|
||||
- [ ] Contract samples, openapi.yaml and `web/src/lib/types.ts` agree with
|
||||
Every boundary on the nine-level ladder has a test in which one query
|
||||
matches **both** levels, so reversing either order fails the suite. The
|
||||
milestone added four: "both wildcard levels beat an allow regex that
|
||||
matches", "an operator block rule beats a list exception", "a list
|
||||
exception beats a list domain entry on the same name", and "a list
|
||||
domain entry beats a list wildcard entry". The last two were added after
|
||||
the second review pass found the earlier claim overstated — the existing
|
||||
exception test used a name absent from `.list`, so it pinned
|
||||
exception-versus-wildcard rather than exception-versus-domain.
|
||||
- [x] No `src/filter/` fuzz-root file imports anything but `std`
|
||||
(`regex.zig` imports `std` alone).
|
||||
- [x] Contract samples, openapi.yaml and `web/src/lib/types.ts` agree with
|
||||
the server (the drift guards pass).
|
||||
|
||||
## Recorded (anchor re-verification, 2026-08-12)
|
||||
|
||||
The spec was written against `ffc3ca6` and verified against `a8e0fe4`. It was
|
||||
re-verified a third time after milestones 22 and 23 landed (TypeScript 7,
|
||||
Tailwind removed, StyleX and React Aria). Findings, all folded into the
|
||||
rulings above:
|
||||
|
||||
- **No Zig file changed** between `a8e0fe4` and this re-verification. Every
|
||||
Zig anchor holds; six ranges are off by a line or two but still contain what
|
||||
the spec names. The schema is still at version 2, so migration steps 3 and 4
|
||||
are genuinely new.
|
||||
- The rules-page kind selector is no longer a `<select>`. It is a
|
||||
`KIND_OPTIONS` array feeding `web/src/ui/Select.tsx`, a React Aria wrapper
|
||||
(ruling 7 rewritten).
|
||||
- `migrations.zig:349` pins `target_version` and is unnamed by the original
|
||||
spec. S1 moves it to 3, S3 to 4.
|
||||
- `exception_count` has two UI sites, not one, so S1 owns `BlocklistsPage.tsx`
|
||||
(ruling 4 rewritten).
|
||||
- The checksum sentence has three copies, not one (ruling 3 rewritten).
|
||||
- `patternIsValid`'s error set cannot carry the engine's three error tags as
|
||||
written (ruling 6 rewritten).
|
||||
- `PLAN.md:99` documents two compiled bodies and goes stale with ruling 3
|
||||
(ruling 8 rewritten).
|
||||
- `tools/bench.zig:158` reads `.rules = &.{}`, so the filter bench loads no
|
||||
rules today.
|
||||
- `web/vitest.setup.ts` polyfills `CSS.escape`; without it, a test that opens
|
||||
the React Aria selector throws under jsdom.
|
||||
- `npm run build` gained `scripts/assert-css-layers.mjs`, which fails on any
|
||||
rule outside a cascade layer. A pure StyleX change cannot trip it.
|
||||
|
||||
## Recorded (implementation)
|
||||
|
||||
Deviations and findings from the build, the Codex review rounds and the live
|
||||
smoke. Everything here is folded into the code; nothing is outstanding.
|
||||
|
||||
- **Regex scratch is on the caller's stack, not in `Program`.** `std.Io.Threaded`
|
||||
means several query threads share one snapshot, so the Pike VM's two thread
|
||||
lists cannot live in the compiled program. `matches` takes its scratch from
|
||||
the caller's frame, which keeps `Program` immutable and shareable and keeps
|
||||
the match path allocation-free.
|
||||
- **`{n,}` is legal and `\` + any ASCII punctuation is legal.** Ruling 5 named
|
||||
neither. Quantifier-on-quantifier (`a+?` read as `(a+)?`) is `BadPattern`:
|
||||
the first Codex round found `a+?` compiling to something that matched
|
||||
everything.
|
||||
- **Embedded whitespace was accepted on the anchored ABP forms.** A line such
|
||||
as `||good.example bad.example^` reached the compiler, which lowercases and
|
||||
length-checks but does not reject a space, and wrote an entry only a query
|
||||
carrying the same space could match. `compiler.zig` passes `.wildcard` and
|
||||
`.exception` text to `addCandidate` whole, which is why the space survived
|
||||
there. `parser_abp.isNameCandidate` now refuses whitespace and control bytes
|
||||
on those two paths.
|
||||
|
||||
The bare-name path deliberately does **not** use it. `compiler.zig:93-98`
|
||||
tokenizes `.domain` text on whitespace and adds each field separately, so a
|
||||
bare line carrying a space was never broken — it produced two valid entries.
|
||||
Since `detectFormat` assigns one format per source, a mostly-ABP list that
|
||||
also carries hosts-style lines depends on exactly that tokenizer to keep
|
||||
them working. The first attempt at this fix applied the helper to all three
|
||||
paths and silently dropped that fallback; the second Codex pass caught it.
|
||||
Two tests now pin it: a parser test that the bare form stays `.domain`, and
|
||||
a compiler test that an ABP-classified list carrying `0.0.0.0 ads.example`
|
||||
still emits `ads.example`. The third pass pointed out that the parser test
|
||||
alone would pass even if the compiler stopped tokenizing, which is the
|
||||
behaviour the fallback actually depends on.
|
||||
- **The `.allow` body left stale enumerations behind it.** Adding a third
|
||||
compiled body — and with it a fourth temporary, `.allow.tmp` — updated the
|
||||
production code but not every place that lists the file names. The third review pass found four: two `manager.zig` tests that
|
||||
spell the names by hand, the orphan-sweep fixture in
|
||||
`filter_integration_test.zig`, and `PLAN.md` §3.13 and §5. The
|
||||
`manager.zig` table-driven test was worse than stale — it iterates
|
||||
`source_file_suffixes` itself, so deleting an entry changes the code and the
|
||||
test's expectations together and everything still passes. The whole repo was
|
||||
then swept for the pattern rather than the four instances patched, which
|
||||
turned up seven more — including the reload-cancellation test, whose fixture
|
||||
wrote no `.allow` file at all, so the third of `loadSource`'s three read
|
||||
sites was never exercised. A fourth pass then found test 10e comparing only
|
||||
`.list` and `.wild` across a restart, so a restart that rewrote the exception
|
||||
body alone would have stayed green. A `comptime` assertion on
|
||||
`source_file_suffixes.len` now breaks the build when a suffix is added or
|
||||
removed without updating the hand-written tests.
|
||||
- **A pre-existing flake in the required suite.** `src/cli.zig:1538` asserted
|
||||
`std.mem.count(u8, text, "OK") == 0` over output that embeds the temporary
|
||||
directory path. `std.testing.tmpDir` names that directory with base64 over
|
||||
12 random bytes, whose 64-symbol alphabet includes uppercase: 15 adjacent
|
||||
positions each carry `OK` with probability 1/4096, so about one run in 273
|
||||
fails a test that has nothing to do with naming. It surfaced during
|
||||
this milestone's watched-fail injections. The assertion now checks that no
|
||||
line *starts* with a verdict, which covers both `OK:` and
|
||||
`OK upstreams[...]` and cannot match a path segment. Reproduced and fixed
|
||||
outside the milestone's scope because a randomly failing required gate
|
||||
devalues every green run after it.
|
||||
- **`PLAN.md` still described the seed-once config model.** Seven sites said or
|
||||
implied that the first start seeds the database from `/etc/nxdns/config.zon`,
|
||||
which milestone 20 replaced with the two authority modes selected by the
|
||||
presence of `--config`. One of them listed a `config/bootstrap.zig` that does
|
||||
not exist — the module is `loader.zig` plus `reconcile.zig`. The claim was
|
||||
checked against `src/app.zig:339`, not just against the docs. This is
|
||||
milestone-20 drift found while fixing the milestone-21 echoes in the same
|
||||
document, and corrected because PLAN is the source of truth a later session
|
||||
builds from.
|
||||
- **`PLAN.md` §12.1 held a config sample nobody could load.** It described an
|
||||
`.upstream.servers` field that never existed and omitted the required
|
||||
`.groups` and `.upstreams`. The section now points at
|
||||
`docs/reference/configuration.md` and `nxdns export` and keeps only a
|
||||
skeleton: a second copy of the schema is what produced the drift, so the
|
||||
copy is gone rather than corrected.
|
||||
- **`PLAN.md` §7.1 claimed blocklist entries never parent-walk.** Two of the
|
||||
three list levels do: wildcard entries match every proper parent, and
|
||||
exception entries walk the candidate chain, which is why
|
||||
`@@||good.ads.example^` also lifts `y.good.ads.example`. Only domain entries
|
||||
match the query name alone.
|
||||
- **`src/config/model.zig`'s header described the retired bootstrap** and
|
||||
omitted `exception_count` from its list of runtime columns. The first attempt
|
||||
at correcting it introduced a new error — it called import a wholesale
|
||||
replacement set against reconciliation — which the sixth pass caught.
|
||||
`import.zig:3` is explicit that import has been a thin wrapper over
|
||||
`reconcile.zig` since milestone 20, so there is one declarative write path,
|
||||
not two, and it preserves the runtime state of every row the input still
|
||||
names (`reconcile.zig:1139`). A seventh pass then corrected two more claims
|
||||
in the same header: `Config` is the whole shape of a config file but not the
|
||||
only shape the repositories accept (the API writes through `RuleInput`,
|
||||
`ClientInput`, `ClientEdit`), and compiled-body reuse turns on the preserved
|
||||
source id and checksum rather than the counters — `loadSource` names the
|
||||
files after the id and accepts them only against the stored checksum.
|
||||
- **`PLAN.md` §11.2 presented the v1 DDL as the live schema.** It predates
|
||||
three migrations, so it lacks `upstreams.tls_name` and
|
||||
`blocklist_sources.exception_count` and its `kind` CHECK admits only
|
||||
`exact` and `wildcard` — implementing against it would produce a database
|
||||
that rejects every regex rule this milestone added. The section is now
|
||||
labelled the v1 baseline and points at `config_schema.zig` and
|
||||
`migrations.zig`, with the three steps named.
|
||||
- **`INSTALL.md` said the service reads `config.zon` "on the first start".**
|
||||
Neither authority mode behaves that way: under `run --config` the service
|
||||
reads it on every start, and under database authority `nxdns import` reads it
|
||||
while a bare `nxdns run` never does. The sentence justified a file mode, so
|
||||
an operator tightening permissions after first boot would have broken the
|
||||
next file-mode restart. More milestone-20 drift. The claim had a second copy
|
||||
in `docs/how-to/install-with-systemd.md`, found only because the seventh pass
|
||||
looked for it after the first copy was fixed.
|
||||
- **`PLAN.md` §7.1 and the Phase 5 summary missed this milestone's own
|
||||
additions.** The evaluation sequence went straight from operator rules to
|
||||
blocklist domains, omitting the exception level, and the matcher was still
|
||||
enumerated as exact/parent/wildcard with no regex. Ruling 8 required these
|
||||
echoes and S3 did not reach them.
|
||||
- **The UI trimmed regex patterns.** `RulesPage.onSubmit` applied
|
||||
`pattern.trim()` to every kind, so a regex created in the UI did not store
|
||||
the bytes an identical `POST /api/rules` would. It now trims only `exact`
|
||||
and `wildcard`, where the server normalizes anyway. No client-side
|
||||
whitespace rejection was added: `" foo|bar"` still has a live `bar` branch,
|
||||
so refusing it would over-reject, and the server stays the authority on
|
||||
pattern validity.
|
||||
- **The rule pattern field opted into mobile autocapitalization.** An
|
||||
autocapitalized regex validates and then silently never matches, because
|
||||
query names are lowercase and a regex is never normalized. The field now
|
||||
sets `autoCapitalize="none"`, `autoCorrect="off"`, `spellCheck={false}`.
|
||||
- **`RuleSet.build` received the snapshot arena for its temporaries.** An
|
||||
arena reclaims only its most recent allocation, so every `defer …deinit`
|
||||
inside `build` was a silent no-op and the scratch survived until snapshot
|
||||
teardown, uncounted by `memoryBytes()`. `build` now takes a permanent and a
|
||||
scratch allocator, and `matcher.zig` passes `arena` and `gpa` respectively.
|
||||
Regex programs compile into scratch and are copied across by a new
|
||||
`Program.clone`, which keeps `regex.zig` a single-allocator engine and
|
||||
leaves `compile`'s signature — and therefore `config/validate.zig` and the
|
||||
fuzz target — untouched.
|
||||
|
||||
Measured on 16 groups each holding 256 regex rules of 254 bytes,
|
||||
4096 wildcards and 2048 exact rules, with no blocklist sources:
|
||||
arena capacity fell from 135,662,440 to 12,686,214 bytes against an
|
||||
unchanged `memoryBytes()` of 11,058,791, so the ratio of real to reported
|
||||
went from 12.27× to 1.15×. The hidden footprint per snapshot fell from
|
||||
118.83 MiB to 1.55 MiB, so 117.28 MiB went away. A reload holds two
|
||||
snapshots, so it was carrying twice that.
|
||||
`memoryBytes()` needed no change: the formula was always right about what
|
||||
the `RuleSet` retains, and the divergence was arena capacity the formula
|
||||
does not claim to describe. The residual 1.15× is arena node headers and
|
||||
page rounding, which `memoryBytes` documents itself as excluding.
|
||||
|
||||
The guard is `the build's temporaries stay out of the permanent allocator`,
|
||||
which asserts `arena.queryCapacity() < 2 * set.memoryBytes()` and passes
|
||||
`scratch` as `testing.allocator`, so a permanent allocation wrongly taken
|
||||
from scratch also fails as a leak. It was watched failing with the split
|
||||
reverted.
|
||||
- **An interactive `--fuzz` session cannot run on zig 0.16.0.** Building any
|
||||
fuzz target with `-ffuzz` fails inside the stock
|
||||
`/usr/lib/zig/compiler/test_runner.zig:566`, which passes a
|
||||
`*builtin.StackTrace` to `debug.writeStackTrace` where a
|
||||
`*const debug.StackTrace` is wanted — two distinct struct declarations. The
|
||||
failure is entirely inside the toolchain and reproduces on `compiler-fuzz`,
|
||||
a target this milestone did not touch. Corpus replay under `zig build test`
|
||||
is unaffected and remains the evidence for the step-bound property.
|
||||
- **`api.md` gained a block-reason table.** The milestone added three reason
|
||||
tags and no doc page enumerated any of them. The table lists all nine in
|
||||
evaluation order plus `none` and the `cname:` prefix.
|
||||
- **Two comment sites still counted three temporaries.** `manager.zig` line 40
|
||||
(the `refresh_lock` invariant) and line 1293 (`pruneOrphans`) both omitted
|
||||
`.allow.tmp`. Each presents itself as exhaustive, so a maintainer could have
|
||||
added an `.allow.tmp` writer outside `refresh_lock` and let the orphan sweep
|
||||
delete it mid-compile. `sourceFileId` and `source_file_suffixes` already
|
||||
matched all four.
|
||||
- **The rule pattern placeholder named only two kinds.** Ruling 7 requires the
|
||||
web contract to name the third kind everywhere it names the first two, and
|
||||
the placeholder read `ads.example.com or *.example.com`. It now carries a
|
||||
regex example too.
|
||||
- **Two pre-existing tutorial errors surfaced during the docs sweep.**
|
||||
`tutorial/first-run.md` step 14 claimed the compiled list stays on disk when
|
||||
it is in fact pruned (real output: `pruned orphaned blocklist file 1.allow`,
|
||||
`1.wild`, `1.list`), and step 11 named `ads.doubleclick.net`, which
|
||||
StevenBlack no longer carries.
|
||||
|
||||
## Anti-requirements
|
||||
|
||||
- No `$` modifier support beyond tolerating `$important` on exception lines.
|
||||
|
||||
Reference in New Issue
Block a user