Skip to content

test(#787): a tripwire under the one-fine-worker premise, with its limits measured - #836

Merged
djhenry merged 6 commits into
mainfrom
fix-787-one-worker
Aug 1, 2026
Merged

test(#787): a tripwire under the one-fine-worker premise, with its limits measured#836
djhenry merged 6 commits into
mainfrom
fix-787-one-worker

Conversation

@djhenry

@djhenry djhenry commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Addresses #787.

Round 4. Round-3 review returned five must-fix. All five are done. Four of them were false sentences I had written about code that was already right — the same defect class as rounds 1 and 2 — and the fifth was a real hole in the reach control's own input.


The premise, and what actually goes wrong

NavStatus.local_planner_dead is latched for the life of a fine worker. The agent is told it is session-scoped. Those are the same span only because exactly one LocalPlanner is constructed per client process. That premise is true today. Four tracked sentences rest on it and nothing went red when it stopped being true.

  1. crates/eqoxide-ipc/src/lib.rs — the NavStatus::local_planner_dead field doc, sentence 1
  2. crates/eqoxide-ipc/src/lib.rs — the same field doc, sentence 2
  3. crates/eqoxide-http/src/observe.rs — the nav_local_planner_dead publication comment
  4. docs/http-api.md### nav_local_planner_dead — fine-planner liveness, session-scoped

Mechanism, and why the others were rejected

Chosen: a source scan over the whole treewalker::tests::exactly_one_production_fine_worker_is_built_in_the_tree_787. Exactly one unmarked Walker::new( site (in ActionLoop::new), exactly one unmarked LocalPlanner::spawn( site (inside Walker::new). Its failure message enumerates all four sentences and says the session-scoped name has become wrong. That message is the deliverable; the scan is the trigger.

Rejected: a process-global runtime counter. cfg(test) is per-compilation: when eqoxide-nav is built as a dependency of eqoxide-net, cfg(test) is FALSE for it, so a #[cfg(not(test))] trip is live inside eqoxide-net's own suite — which builds two ActionLoops, hence two Walkers, in one process. Measured: I built the probe and it fired there while eqoxide-nav's own suite was unaffected.

Rejected: a FineWorkerSlot token. Changes Walker::new's signature, forcing an edit to the in-flight action_loop.rs, and needs a pub test escape hatch that degrades it back to a source convention.


Round-4 must-fixes

M1 — the member list was printed and never checked

Review broke the per-member reach control from its own input, in both directions:

  • workspace_members_787 armed on buf.ends_with("members"), and default-members ends with members. Adding default-members = ["tools"] collapsed the list to one member, took 29 .rs files dark with a planted production construction among them, and the guard passed while printing workspace members checked = 1.
  • A # comment containing a quoted string inside the members array was swept in as a phantom 14th member and turned a fully intact tree RED.

Fixed as recommended, and the third part is the one that matters: anchor the key (a trimmed line must start with members), strip comments first, and assert the parsed list equal to the set of directories that actually contain a Cargo.toml. The manifest lists 13; the filesystem yields the same 13 (tools plus twelve crates/*).

All rows below were run by me on a copy of this branch with no .git — the builder shape, where the per-member control is the only reach control in force. Verdict GREEN = guard reported ok.

# tree manifest result
M0 intact pristine GREENwalked 172 … workspace members checked = 13
M1 walk skips eqoxide-ui, unmarked production Walker::new( planted inside it pristine REDworkspace member `crates/eqoxide-ui` contributed ZERO files; Walked 143 file(s). Members checked: 13.
M2 same planted violation default-members = ["tools"] added after members RED, still Members checked: 13
M2b same planted violation default-members = ["tools"] added before members RED, still Members checked: 13
M3 intact members reflowed multi-line with # the "tools" crate must stay first — see #787 inside the array GREEN, workspace members checked = 13
M4a a crate dir on disk (crates/eqoxide-zzz787/Cargo.toml) the manifest does not list pristine REDParsed 13 …; on disk 14 …
M4b intact "crates/eqoxide-renderer" deleted from members REDParsed 12 …; on disk 13 …
M8 everything restored pristine GREEN, walked 172 … members checked = 13

M4a/M4b are the positive control the review asked for: the assertion binds in both directions.

The stripper is itself a scanner, so it carries its own reach control — an inline probe over a = "x # not-a-comment" # yes-a-comment\nb = 'y # also-not', asserting the quoted # survives in both string forms, that the real comment is gone, and that the line structure is unchanged. Both failure directions were mutated:

# mutation of strip_toml_comments_787 result
M5 made a no-op (return text.to_string()) RED — "removed NOTHING from a probe that contains a real comment — it is a no-op"
M6 made naive (line.split('#').next(), quote-unaware) RED — "corrupted a quoted value … Got: "a = \"x \nb = 'y ""

And the strongest row, which is why the assertion and not just the anchor is the fix. With the anchor deliberately removed (key.trim_end().ends_with("members") — the round-3 behaviour) and default-members present and the violation planted:

# mutation result
M7 anchor removed + default-members + planted violation REDParsed 1 ({"tools"}); on disk 13 (…)

The parser still collapses to one member, exactly as review measured — and the tree is red anyway, because the list is now compared to something. That is the difference between a printed number and a control.

M2 — "every row below was RUN" was false; the two rows have now been run

Not scoped, not deleted — run, and one of them turned out to be wrong:

row what was run result
let ctor = LocalPlanner::spawn; then ctor() unmarked production fn second_fine_worker_787() added to walker.rs GREEN — survives exactly as the row claimed. Row now says "run, not reasoned, since round 4"
a Default/Clone impl for Walker impl Default for Walker whose body is a struct literal filling local_planner REDfound 2 site(s). The row's "NOT caught" was wrong. To be a fine worker the literal has to fill local_planner, and the plain spelling of that is LocalPlanner::spawn(, which is the second needle
same the same impl reaching the worker via let make = LocalPlanner::spawn; … make() GREEN — so it evades only through the fn-pointer row, not on its own

Both Default plants were text-level: the guard is a text scan, and a full Walker struct literal does not compile outside its own module. That limitation is stated in the row. The blanket sentence is replaced by per-row provenance, so the claim is checkable one row at a time — which is the form the false one took.

M3 — the degraded-mode floor was wrong: 16 of 172, ≤156 dark, 90.7%

eqoxide-nav carries two named anchors (walker.rs and planner.rs) and one representative cannot be both. So the minimum satisfying corpus is 13 member representatives + 1 (nav's second anchor) + 2 outside any member (src/app.rs, tests/walker_sim.rs) = 16 of 172. Up to 156 — 90.7% — could be dark with every control green. My round-3 figures (15 / 157 / 91%) counted eqoxide-nav once. Corrected in the rustdoc; the figures did not appear in docs/http-api.md.

M4 — the --nocapture parenthetical contradicted its own code

Removed. The disclosure is written to the process's real stderr handle precisely so that no flag is needed; --nocapture would be required only for the println! form, which is the form this deliberately does not use. Confirmed again in this round's builder log of a passing run with no --nocapture:

#787 guard: reach control = PER WORKSPACE MEMBER only (weaker); walked 172 .rs file(s); git index available = false (0 tracked); workspace members checked = 13

M5 — "a re-export can only be spelled pub use" was false

It is contradicted by the code's own pub(crate) / pub(super) / pub(in path) stripping and by row A2c. The row and the code comment now say: every re-export leads with a visibility, pub is only the shortest of them, which is why the parenthesised forms are stripped rather than assumed away.


Standing from earlier rounds

The walk-only corpus was measured and declined; review confirmed the decision. git ls-files '*.rs' and the filesystem walk produce the same 172 files, identical by name (comm empty both directions, LC_ALL=C) on two trees. So the corpus is already the walk — union(index, walk) with equal sets — and dropping the index would delete the one per-FILE reach control the merge gate runs, leaving the 16-of-172 floor in force everywhere instead of only where git is absent. A difference in checking, not in what is scanned.

The marker narrowing stays withdrawn. A marked second production construction in any file, original untouched, leaves the guard green; removing the marker goes RED naming the file.

Retraction, standing from round 2. My round-1 body claimed the guard "would have caught the historical in-process-relogin shape #774 hardened against". That is measured and false. It is the one shape the guard structurally cannot see.

The honest name for this instrument: a TRIPWIRE, not a pin

The premise is a RUNTIME count — one worker per process. The guard is a TEXT count — one construction SITE written.

An in-process relogin re-enters an existing construction path. It adds zero new textual sites. Move the production construction into a helper in action_loop.rs and call that helper twice: one unmarked site in the whole tree, in the expected file, both asserts satisfied, two fine workers per process, guard green.

Mutation table (earlier rounds, still standing)

RED = guard failed. SURVIVING/GREEN = guard reported ok.

Reach controls (#778: a positive control is not enough)

# mutation result
BASE shipped text, unmutated SURVIVING
R1–R5 planted unmarked Walker::new( in src/app.rs, action_loop.rs, command/nav.rs, tests/walker_sim.rs, http/observe.rs RED (all five)
B3a walk skips crates/eqoxide-http, nothing planted REDthe round-1 form passed here at 152 files
B3b walk skips that crate and a site is planted inside it RED
C4 git present + walk skips a member RED (index equality)
C1 no git + walk skips a member RED (per-member)
C2 no git, tree intact — the builder case SURVIVING
C3 no git + planted unmarked site RED
C5 git succeeds but lists nothing RED
C6 workspace member list unparseable RED
M2a (r3) no git + walk skips tools (the 13th member) RED
M2b (r3) same, with the round-2 contains('/') predicate SURVIVING

Positive controls and closed holes

# mutation result
M6 unmarked second LocalPlanner::spawn() in planner.rs RED
M9 marker stripped from one existing test site RED ("matched only 6 … at least 7 expected")
A2a–A2d use / pub use / pub(crate) useas X, for both types RED (A2b was GREEN in round 1)
A5a–A5c comment interposed in the path, block and line form, both types RED (A5a was GREEN in round 1)

Not caught — measured survivors, not assumed

# mutation result
M1a marked second production construction, any file, original intact SURVIVING
A1 let mk = Walker::new; then mk(..) — fn-pointer binding SURVIVING
A6 <Walker>::new(..) — qualified path SURVIVING
A3 macro_rules! body carrying the marker, invoked twice unmarked SURVIVING
A8 one textual site executed twice (the relogin shape) SURVIVING — subject mismatch, not a hole
A9 (round 4) LocalPlanner::spawn reached through a fn-pointer binding GREEN

False REDs — the harmless direction, disclosed

# mutation result
F1 the needle inside a /* … */ block comment RED (false)
F2 the needle inside a string literal RED (false)

Both follow from combining the raw and comment-blanked passes by MAX, which is what keeps two constructions on one line counting as two.

Orthogonality — attacked where the claim does not constrain

# mutation result
S1 planted site that carries the marker SURVIVING (documented escape)
S2 the #787 paragraph in docs/http-api.md reworded to say something false SURVIVING
S3 LocalPlanner::is_dead() forced to false — behaviour, no site touched SURVIVING

Every mutation script asserts count(old) == 1 before writing; three attempts across the rounds asserted out rather than producing a silent false result, and were re-run. Restores are cp -p/rsync + touch, never git checkout, and every round-4 run above was preceded by a touch so cargo could not hand back a stale binary.

Workspace suite over the final pushed text

cargo test --workspace --locked --no-fail-fast, remote builder, commit dec5b14.

figure predicate value
compile sentinel Finished `test` profile Finished `test` profile [unoptimized + debuginfo] target(s) in 15m 04s
targets that reported ^running [0-9]+ tests?$ = 55, ^test result: = 55 55 / 55, no lost binary
non-canonical result lines ^test result: not matching ^test result: (ok|FAILED)\. 0
empty targets ^running 0 tests$ 14 (cross-checked: ^test result: ok\. 0 passed; 0 failed; 0 ignored = 14)
totals, four terms sum over result lines vs sum of headers 1839 passed + 0 failed + 47 ignored + 0 filtered out = 1886 = header sum 1886

Zero failures; ^error = 0; ^warning = 0. 55 targets = 42 unit binaries (Running (unittests|tests) = 42) + 13 doc-test targets (^ Doc-tests = 13).

One measured wrinkle worth stating rather than smoothing over: the anchored predicate ^ *Running returns 41, not 42, because at one point a cargo Running line is glued to the tail of a test's output line — test the_extracted_plan_emits_the_pre_741_command_stream ... ok Running tests/zone_pass_routing.rs (…). The log path is branch-unique and was deleted before this run, and stdout and stderr were merged into it, so an interleave is the expected cause — though I did not prove no other writer, so treat the cause as reasoned and the line as measured. It changed no figure above: headers and result lines are both anchored, both count 55, and the header sum equals the four-term total.

Completion was required on both signals: ps -p showed the process gone AND the log ends with a complete test result: line. Finished `test` profile is the compile sentinel, not run completion. The header regex is tests? — singular; running 1 test is singular and a plural-only regex fakes a lost binary. The empty-target predicate is anchored: unanchored, 0 passed; 0 failed; 0 ignored matches 20 passed; and 10 passed; as substrings and reports 16.

scripts/check-no-local-detail.sh and scripts/check-wrapped-literals.py both pass; the latter independently reports 172 tracked .rs files, the same corpus size the guard prints.

NOT verified, and not claimed

MEASURED:

  • every mutation row above, including the SURVIVING ones, and every round-4 row M0–M8, M4a/M4b, M5–M7, A9
  • the runtime-counter argument — the #[cfg(not(test))] probe firing inside eqoxide-net's suite
  • index vs walk compared by name on two trees: 172 = 172, both differences empty
  • the degraded mode on the real builder — the disclosure line is in this PR's workspace log, in a passing run with no --nocapture
  • rbuild's rsync exclusions, and that none of them holds a tracked .rs
  • CI's actions/checkout@v4
  • the relogin shape leaving the guard green (A8), and the marker hole being file-independent (M1a)
  • that the manifest's 13 members and the 13 directories containing a Cargo.toml are the same set

REASONED, not measured:

  • That the plainly-written second site is the shape a second worker would most likely arrive in. A judgement about how people edit code, and the entire cost/benefit case for a tripwire rests on it.
  • That no current code path reaches LocalPlanner::spawn other than through Walker::new. The guard checks text, not reachability.
  • That 16-of-172 is the true minimum under the degraded control. Derived from the assertions by hand; no run has exercised a corpus at that floor.
  • That stream interleaving, not a second writer, produced the glued Running line in the suite log.
  • That strip_toml_comments_787 is correct for TOML forms its probe does not cover — multi-line basic strings (""") and literal strings (''') in particular. The probe covers single-line " and ' only.

NOT verified at all:

  • No live client run. Structural change, nothing observable in-game.
  • The guard has never fired in anger — every RED here is planted.
  • No single-file miss has been exercised under the no-git path; by construction the degraded control cannot see one.

Known evasions, stated plainly

In the guard's rustdoc as a per-row table marked caught / caught-since-round-N / not-caught, and summarised for agents in docs/http-api.md: a fn-pointer binding (for either type), an angle-bracket qualified path, a marker-carrying macro_rules! body, any marked line in any file, and — the one that matters — the same single site executed twice.

Addresses #787 rather than Closes: what this delivers is a cheap alarm on the likely edit plus four sentences that no longer overstate themselves. The strongest form of the check is not buildable here, and I would rather leave the issue open with an accurate instrument under it than close it behind a confident label.

… rests on

`NavStatus.local_planner_dead` is latched and WORKER-scoped; the agent is
told it is SESSION-scoped. Those are the same span only while exactly one
fine steering worker is built per client process — a premise that was true,
verified twice, and held up by nothing but prose. Four sentences rest on it
directly and none of them would have gone red.

Adds `exactly_one_production_fine_worker_is_built_in_the_tree_787`: a
whole-tree scan asserting exactly one unmarked `Walker::new(` site (in
`ActionLoop::new`) and exactly one unmarked `LocalPlanner::spawn(` site
(inside `Walker::new`). Its failure message enumerates the four sentences
and says the session-scoped name has become wrong.

The four sentences now name the guard instead of saying nothing pins them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQVEpaaKeXsZcW9VT2roeV
@djhenry

djhenry commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

Independent adversarial review — PR #836

Reviewed at b2ed5a9 in a detached worktree of the PR head (merge base 0497f6b). All builds/tests
on the remote builder. Every mutation planted at a call site, reverted from a cp -p copy and
verified by md5sum; no git checkout / git restore / git stash at any point. Both trees end
clean.

Verdict: BLOCKING. The code does what the PR says it does, and the PR's disclosure discipline is
better than most. But the load-bearing argument — that the residual holes push into a false RED,
which is the safe direction — is falsified by measurement. I planted eight production fine-worker
constructions the guard cannot see, in one run, and the guard reported ok. One of them is the exact
decay scenario #787 was filed about.


BLOCKING

B1 — Eight production construction forms are INVISIBLE to the guard. Measured, one green run.

crates/eqoxide-nav/src/walker.rs:3206 (the guard), :3262 (the alias check), :3135 (flatten_787).

I planted all eight simultaneously and ran
cargo test -p eqoxide-nav --lib exactly_one_production_fine_worker:

test walker::tests::exactly_one_production_fine_worker_is_built_in_the_tree_787 ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 254 filtered out; finished in 2.86s

Simultaneity is the proof: any one of them becoming visible would have pushed the count to 2 and gone
RED, so a single green run discharges all eight independently.

Positive control, same tree, immediately after reverting — a plain unmarked site appended to
src/app.rs:

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 254 filtered out; finished in 3.10s
  crates/eqoxide-net/src/action_loop.rs:626: let walker = eqoxide_nav::walker::Walker::new(
  src/app.rs:3472: let _w = eqoxide_nav::walker::Walker::new(nav, world, col, intent, view, za);

So the guard is live, its reach does cover repo-root src/, and the eight below are genuine misses.

# planted form verdict
A1 function-pointer binding: let ctor = eqoxide_nav::walker::Walker::new; then ctor(..) SURVIVING
A7 same for LocalPlanner::spawn SURVIVING
A2 pub use re-export rename: pub use eqoxide_nav::walker::Walker as FineWorker; + FineWorker::new(..) SURVIVING
A5 comment interposed in the path: Walker /* … */::new(..) SURVIVING
A6 qualified path: <eqoxide_nav::walker::Walker>::new(..) SURVIVING
A3 macro body carrying the marker, invoked from production (disclosed; now measured) SURVIVING
A4 a second production construction in action_loop.rs carrying the marker SURVIVING
A8 one textual site, called twice (see B2) SURVIVING

Three of these deserve naming individually:

A1 is a measured survivor against a different pin in this repo already (#828 round 3). The name is
written followed by ;, never by (, and it is not an as-rename — it passes through both of the
guard's checks. It is the single cheapest way to build a second worker invisibly.

A2 is a one-line defect, not a philosophical limit. walker.rs:3262 tests
code.starts_with("use "). A re-export can only be spelled pub use, which does not start with
use . Measured contrast on the same tree, back to back:

  • use eqoxide_nav::walker::Walker as RevPlainAlias;RED at walker.rs:3286
    (#787: a use statement RENAMES Walker or LocalPlanner …)
  • pub use eqoxide_nav::walker::Walker as FineWorker;GREEN

The rustdoc says the plain form is "closed" and that only "a rename in an intermediate module that
then re-exports under a third name" is open. The measured hole is far cheaper than that: a single
pub use line, no intermediate module. Fix: strip a leading visibility (pub, pub(crate), …)
before the starts_with("use ") test.

A5 falsifies the specific evasion M7 advertises as closed. flatten_787 strips whitespace but not
comments, so Walker /* x */ ::new( — valid Rust, and the natural shape of a reflowed call that
picked up an explanatory comment — never forms the needle. The PR sells reflow-resistance as a
distinguishing feature over a line-based scan; it is reflow-resistant only when nothing lands in the
gap.

A4 shows the marker is an unpinned opt-out. Nothing asserts where markers may appear or how many
there may be — marked >= 6 (walker.rs:3296) is a floor only. A // #787-NOT-PRODUCTION pasted
onto a production line disarms the guard for that site forever and nothing reds. S1 disclosed this for
tests/; I measured it in a production file (crates/eqoxide-net/src/action_loop.rs), which is
the case that matters. Consider asserting the marker set by exact path:line list, or at minimum
that no marked site lives outside a mod tests file.

B2 — The guard's subject is not the premise's subject, and #787's own decay scenario slips through

#787 is explicit about the failure it exists to catch: "The day an in-process relogin lands — which
is the exact shape #774 hardened Walker::new against — those four sentences become false, and
nothing in the tree goes red."

An in-process relogin re-enters an existing construction path. It adds zero new textual sites.

A8, measured: I moved the production construction into a helper in the same file and called that
helper twice — the shape a relogin would actually take. One unmarked Walker::new( site in the whole
tree, in the expected file, so both the count assert and the location assert pass. Two fine workers
per process. Guard: ok.

The premise is "exactly one fine steering worker is constructed per client process" — a runtime
count. The guard measures "exactly one construction site is written" — a text count. Those are
not the same claim, and they diverge precisely on the relogin path.

The PR body lists under REASONED-not-measured: "That the guard would have caught the historical
in-process-relogin shape #774 hardened against. Plausible, untested."
That is now measured, and for
the most natural implementation the plausible direction is false.

I accept the PR's case against shape (b) — E1's cfg(test)-is-per-compilation finding is real and
well argued. The problem is not that a source scan was chosen; it is that the tracked prose now claims
the premise is pinned when what is pinned is a proxy that misses the named scenario.

B3 — Reach control: a 20-file subtree can vanish with all three controls green

This is the #778 lesson turned back on the guard. I ran it rather than reading it.

Mutation: make the walk skip crates/eqoxide-http/, and plant a plain unmarked production
Walker::new( inside it (crates/eqoxide-http/src/observe.rs — the author's own R5 probe location).

REV836 SCANNED=152
test walker::tests::exactly_one_production_fine_worker_is_built_in_the_tree_787 ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 254 filtered out

152 ≥ the 150 floor (walker.rs:3222), all five named anchors (:3229) still present, planted site
invisible, guard green. 20 files — 11.6% of the corpus — silently uncovered.

Is the 150 floor doing any work, or is it decorative? It is not decorative, but it is thin.
Measured per-crate .rs counts at this commit (total 172):

eqoxide-ui 29   eqoxide-renderer 25   eqoxide-http 20   eqoxide-core 18
eqoxide-protocol 14   eqoxide-command 14   src 13   tests 10
eqoxide-net 9   eqoxide-nav 9   tools 3   ipc 3   crash 3   telemetry 1   assets 1

Anchors sit in eqoxide-net, eqoxide-nav, src, tests. So the floor does fire independently
for a skip of eqoxide-ui (→143) or eqoxide-renderer (→147). It does not fire for
eqoxide-http (→152, measured), eqoxide-core (→154), eqoxide-protocol (→158),
eqoxide-command (→158), or any combination totalling ≤22 files. Worst case: 22 files / 12.8% of
the tree invisible with every control green.

Suggested fix, cheap and exact: assert files.len() equals a pinned constant (the guard already
knows the number is 172 and the repo already has two independent producers of it), or build the
corpus from git ls-files so the count and the corpus have the same source. An equality is a real
reach control; a floor 13% below the true value is a tolerance band.

B4 — False completeness claims in tracked files

Four of the five prose edits overstate what was built. On a source-scan pin, an unqualified "every" /
"anywhere" / "ever" is a defect on sight, and these are now falsified by measurement, not by argument.

  1. docs/http-api.md:1030-1032 — the agent-facing one, and the worst.
    "a whole-tree source scan that fails, naming this paragraph, if a second fine-worker
    construction site is ever added
    (nav: local_planner_dead's "session-scoped" name rests on "exactly one fine worker per process", which nothing pins #787)."
    No qualifier, no hedge, no pointer to the evasion list.
    A1/A2/A5/A6 each add a second construction site and the scan does not fail. This is the sentence an
    agent reads.
  2. crates/eqoxide-ipc/src/lib.rs:1619 "a whole-tree source scan that fails … if a second
    unmarked Walker::new or LocalPlanner::spawn construction site appears anywhere."

    "unmarked" helps; "anywhere" is still false — A1/A2/A5/A6 are unmarked, are construction sites, and
    are not seen. The hedge that follows names 2 evasions; there are now at least 8.
  3. crates/eqoxide-nav/src/walker.rs:318 "which fails and names the four 'session-scoped'
    sentences the day that stops being true."
    False for A8, the relogin shape, which is the
    scenario this comment is about.
  4. crates/eqoxide-http/src/observe.rs:1119 "a premise pinned by …" Same overstatement,
    weaker context.
  5. crates/eqoxide-nav/src/walker.rs:3196 — the asymmetry argument itself. "Every one of those
    makes the guard MISS a second worker. The Source-scan call-site pins prove a call is written, not reached (#721 A2b, #773 E1b/E2, #791 round 1) #799 evasions that make a written call unreachable …
    push this guard the other way — into a false RED — which is the safe direction."
    The disclosed
    Direction-B list is incomplete (4 named, ≥8 measured) and the balance claim is falsified: the
    residual is dominated by Direction-B misses, not by false REDs. The PR's central defence of "a
    source scan is defensible here" rests on this sentence.

The honest form of all five is roughly: "a source scan that catches a plainly-written second
construction site; it does not catch a fn-pointer binding, a pub use rename, an interposed comment,
a qualified path, a macro-wrapped marked site, a marked production site, or an existing site executed
twice — see the guard's rustdoc."

B5 — Closes #787 should be Addresses #787

#787's bar: "This issue exists so the premise does not evaporate when it stops being true." Its named
evaporation path is the in-process relogin, and B2 measures that the guard stays green on it. Add the
six undisclosed written-site evasions (B1) and the 12.8% reach tolerance (B3), and the guard is a
useful tripwire, not a pin.

Recommended: retitle to Addresses #787, keep the issue open with the measured evasion table, or fix
B1's cheap items (pub use, fn-pointer binding, comment-stripping in flatten_787) and B3's count
equality and re-review. The prose in B4 must be corrected either way — that part is not optional,
because it is what a future agent will read as truth.


NON-BLOCKING

  • N1 walker.rs:3296 asserts marked >= 6 while its own message says "expected the 4 Walker
    fixtures in this file and the 3 LocalPlanner fixtures"
    — i.e. 7. Harmless today (stripping a
    marker also trips the count assert, per M9), but the message states an assertion that is not made.
  • N2 The failure message's item 4 quotes the heading as
    ### nav_local_planner_dead — fine-planner liveness, session-scoped; the actual line
    (docs/http-api.md:1017) is ### `nav_local_planner_dead` — … with backticks. Copy-pasting the
    quoted string into a grep finds nothing.
  • N3 The new test-name citations in crates/eqoxide-ipc/src/lib.rs and
    crates/eqoxide-http/src/observe.rs sit outside the citation-scan corpus (steering.rs,
    walker.rs, collision.rs, tests/walker_sim.rs), so renaming the test rots them silently. There
    is precedent (the 766 test is cited the same way), so this is not new — worth one sentence.
  • N4 The scan walks the filesystem, not the git index. An untracked scratch .rs containing the
    needle is a false RED. Safe direction, undisclosed.

What I could NOT break (verified, PR claims that hold)

  • The guard is green at HEAD and runs in CI (cargo test --workspace --locked).
  • Positive control reaches repo-root src/ — plain unmarked site → RED with the verbatim
    four-sentence message quoted in the PR body. R2 reproduced.
  • The plain use … as X; rename is caught — RED at walker.rs:3286. M8 reproduced.
  • S3 reproduced exactly. Deleting nav.nav_state.lock().unwrap().local_planner_dead = false; from
    Walker::new and running the whole nav lib:
    test result: FAILED. 238 passed; 1 failed; 16 ignored — the single failure is
    a_new_walker_does_not_inherit_a_previous_workers_death_766, and
    exactly_one_production_fine_worker_is_built_in_the_tree_787 is ok. The orthogonality claim is
    correct and neither test masks the other.
  • The 172-file claim. git ls-files '*.rs' | wc -l = 172 at the PR head and at the merge
    base; scripts/check-wrapped-literals.py independently reports "172 tracked .rs files"; the
    builder's rsync excludes target/, .git, .claude/, assets/, and the walk skips target/ and
    dot-dirs, so the scanned set and the tracked set do coincide here.
  • scripts/check-no-local-detail.sh → OK. scripts/check-wrapped-literals.py → OK.
  • The _cited registration is correct. NOT_PRODUCTION's rustdoc cites the test in backticks on a
    /// line, doc_citations picks it up, the test is a #[test] in walker.rs, so guard_entries
    requires it in _cited — and it is there. (four_sentences has one underscore, below the ≥3
    threshold, so it needs no entry.)
  • The four quoted sentences exist verbatim at the four cited locations (modulo N2's backticks).
  • Walker derives neither Default nor Clone — evasion 3 in the rustdoc is accurate today.
  • The E1 argument against shape (b)cfg(test) being per-compilation — is sound and is the
    right reason to reject a runtime counter here. I did not re-run E1.
  • No live E2E was run, correctly. This is a test-only change with nothing observable in-game; the
    mutation-checked suite is the ceiling. Stating that rather than skipping silently.

Five-figure log standard — full workspace run

test --workspace --locked --no-fail-fast, stdout and stderr captured to separate files, process
exit 0.

  1. Compile sentinel (stderr): Finished `test` profile [unoptimized + debuginfo] target(s) in 19m 55s.
    This is the COMPILE sentinel only. Run completion confirmed separately: process exit 0, and the
    final stdout line is a complete canonical test result: line terminated by a newline (checked with
    od -c). Zero test result: FAILED lines.
  2. ^running [0-9]+ tests?$ headers = 55; ^test result: lines = 55 — equal (lost-binary check
    passes). Matched with tests?; this log contains 2 singular running 1 test headers, which a
    plural-only regex would have dropped. Corroborated by the target announce lines on stderr: 42
    Running + 13 Doc-tests = 55.
  3. Non-canonical test result: lines: 0. Predicate: every ^test result: line also matches
    ^test result: (ok|FAILED)\. [0-9]+ passed; [0-9]+ failed; [0-9]+ ignored; [0-9]+ measured; [0-9]+ filtered out; finished in [0-9.]+s$.
  4. Targets with nothing to run: 14, anchored on the full triple
    ^test result: ok\. 0 passed; 0 failed; 0 ignored. Second predicate ^running 0 tests$ also 14,
    and by name it is the same 14 targets (enumerated below).
  5. Aggregate: passed=1831, failed=0, ignored=47, filtered=0
    1831 + 0 + 47 + 0 = 1878, and the sum of the running N tests headers = 1878. Equal.
    (measured=0, not a term in the identity.)

All five reproduce the PR body's figures exactly.


The 14-vs-16 empty-target disagreement — SETTLED, and it is a predicate artifact

Two reviewers previously reported 14 and 16 on the same origin/main. Counts could not settle
it, so I enumerated by name. Targets were paired to result blocks by index: cargo runs test binaries
sequentially, so the n-th Running/Doc-tests line on stderr corresponds to the n-th
running N tests / test result: pair on stdout.

The mechanism: 0 passed; is a SUBSTRING of 20 passed; and 10 passed;.

predicate count
^test result: ok\. 0 passed; 0 failed; 0 ignored (anchored) 14
^running 0 tests$ 14identical set by name
0 passed; 0 failed; 0 ignored (unanchored) 16
[^0-9]0 passed; (left-guarded, any ignored) 18
0 passed (bare) 20

14 is the true figure. The 14 (both predicates, same set):

unittests src/main.rs                (eqoxide)
unittests src/bin/render_model.rs
unittests src/bin/crash_probe.rs
unittests src/diagnose_glb.rs        (tools)
unittests src/validate_glb.rs        (tools)
Doc-tests eqoxide
Doc-tests eqoxide_assets
Doc-tests eqoxide_command
Doc-tests eqoxide_crash
Doc-tests eqoxide_nav
Doc-tests eqoxide_protocol
Doc-tests eqoxide_renderer
Doc-tests eqoxide_telemetry
Doc-tests eqoxide_ui

16 − 14 = the two substring false hits, both of which actually ran tests:

tests/shadow_caster_selection.rs   test result: ok. 20 passed; 0 failed; 0 ignored; …
tests/shadow_shader.rs             test result: ok. 10 passed; 0 failed; 0 ignored; …

18 − 14 = four genuinely ignored-only targets (0 passed but nonzero ignored), which the
anchored triple correctly excludes:

tests/asset_sync_live.rs      0 passed; 0 failed; 1 ignored
tests/water_capability.rs     0 passed; 0 failed; 5 ignored
Doc-tests eqoxide_http        0 passed; 0 failed; 2 ignored
Doc-tests eqoxide_net         0 passed; 0 failed; 1 ignored

20 = 18 + the 2 substring hits. So the "different trees" hypothesis was correctly refuted, and the
correct replacement is: the number depends entirely on whether the predicate is line-anchored and
whether it guards the digit to the left of 0 passed.
Anyone quoting an empty-target count must
quote the predicate with it. Recommend the repo standardise on ^running 0 tests$, which cannot be
fooled by a substring.


Method / hygiene

  • Detached worktree at the PR head; the author's branch worktree untouched.
  • Every mutation at a call site, never in a callee body; wrap mutations run, not only deletions
    (A8 is a wrap; A5/A6 are rewrites; A4 is an annotation).
  • Universals attacked in both directions: forcing the condition → RED (positive control, use-rename);
    changing what the claim does not constrain → SURVIVING (S3 reproduced).
  • All five mutated files restored from cp -p copies and verified by md5sum; the review worktree and
    the shared checkout both end with an empty git status --porcelain.

… misses

Round-1 review of #836 measured the guard's argument false, not its code.
I argued the residual holes push toward a false RED (safe); the review
planted eight second-worker constructions at once and the guard reported
`ok` — the residual is dominated by misses the scan cannot see.

Closes the two cheap holes:

  * `pub use …::Walker as W;` — the alias check tested `starts_with("use ")`
    and a re-export can only be spelled `pub use`, so the form that actually
    matters was green. Visibility is now stripped first (`pub`, `pub(crate)`,
    `pub(in …)`).
  * `Walker /* x */ ::new(` — whitespace-only flattening never forms the
    needle. Every file is now scanned twice, raw and with comments blanked,
    combined by MAX per line so two constructions on one line still count
    as two.

Fixes the reach control. `files.len() >= 150` was a tolerance band: review
measured a 20-file crate (11.6%) vanishing under it with every other control
green and a planted production site inside it invisible. The corpus is now
`git ls-files` unioned with the walk, and the walk is asserted EQUAL to the
index; an empty corpus fails rather than passes.

Corrects the four dependent sentences, which is the agent-facing half. They
said "pinned", "ever", "the day that stops being true". A source scan counts
SITES; the premise is about WORKERS. One site executed twice — the in-process
relogin shape #787 is filed about — adds zero text and leaves the guard green.
That was measured. The sentences now say tripwire, name the uncaught forms,
and tell the reader to read "session-scoped" as accurate today and cheaply
checked, not as guaranteed.

The rustdoc evasion table is now measured row by row against the shipped
code, including the rows that say NOT caught, and including two false-RED
directions (needle in a block comment, needle in a string literal).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQVEpaaKeXsZcW9VT2roeV
… workers

The round-2 reach fix made the git index the corpus and hard-failed when
`git ls-files` did not run. The remote builder that compiles and runs this
workspace receives an rsync'd copy of the worktree with NO `.git`, so the
whole suite went red there with a message about corpus enumeration — a
missing index dressed up as a finding about fine workers, which is the same
class of dishonesty #787 is about.

The index is now optional and the guard states which control was in force:

  * git present (developer/agent worktree) — walk asserted EQUAL to the
    index, per FILE;
  * git absent (builder) — every workspace member read from the root
    `Cargo.toml` must contribute at least one walked file, per MEMBER.

The per-member control is weaker and the rustdoc and the failure text both
say so ("git index available: false"). It still catches the failure that was
actually measured, because that failure was a whole crate directory dropping
out of the walk, not a single file.

Measured, all on the shipped code:

  BASE  git present, tree intact                     SURVIVING
  C2    no git, tree intact (the builder case)       SURVIVING
  C1    no git + walk skips crates/eqoxide-http      RED (member control)
  C3    no git + planted unmarked site               RED
  C4    git present + walk skips a member            RED (index equality)
  C5    git succeeds but lists nothing               RED
  C6    workspace member list unparseable            RED

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQVEpaaKeXsZcW9VT2roeV
djhenry added a commit that referenced this pull request Aug 1, 2026
…ot drop a zone silently (#835)

`open_corpus_zone` becomes the single owner of the corpus prologue: open the zone, load the
`.glb`, build collision, load the `.wtr`, and close the zone on every failure path — so a corpus
loop can no longer drop a zone without the rollup recording it. Five corpora move onto it (the
four `*_blast_radius` loops in `tests/walker_sim.rs` and `water_grid_budget_measurement`).

The honesty argument, unchanged from #807: these corpora end by printing a headline figure that
is read as being about the zone list the test names. A zone that fell out at a bare `continue`
was simply absent from the denominator, so the printed number claimed a coverage it did not
have. The prologue now ends with `is_complete()` and an `over N/M zones` breakdown.

Closes #807.

## Review

Reviewed adversarially across three rounds. Round 1 raised five findings; the orchestrator ruled
four MUST and one out of scope (an assert reorder, now tracked). Round 2 was APPROVE with two
findings marked non-blocking — and those two were taken anyway, because both were false
sentences in the very paragraph this PR exists to make accurate. Round 3 was three exact sentence
swaps plus one word, verified at source by the orchestrator rather than by a third reviewer
dispatch.

What the doc now says, and did not before:

* The guarantee is narrowed to what is true: every `Err` return is preceded by a `skip`/`add`
  that closes the zone. The `Ok` tail is deliberately the caller's obligation, and a named test
  pins that. An earlier draft claimed no path through the function could leave a zone open —
  that was false, and it was deleted rather than qualified.
* A disclosure bullet states plainly that CI does not exercise this function body at all. Every
  test that pins the three drop paths injects closures via `open_corpus_zone_with`, so a mutation
  of the path construction or the `cell` pass-through would survive the whole suite. Closing that
  needs an asset fixture CI does not have.
* The completeness claim about corpora existing "once" is gone; one owner describes five corpora,
  not six.

## Known remaining, tracked not hidden

Ten further zone-drop sites across five loops in `collision.rs` are untouched, four of them
silent. They are enumerated per-line in **#839**, and the doc points there rather than restating
a tally that would drift. #839's first draft undercounted them as seven — this PR's reviewer
caught it, and the issue was corrected in public with the reason: the scan pattern used to answer
"which drops are silent" could by construction only match the bare form, and was then reused to
answer "how many drop sites are there". Same shape as #778 and #836-B3.

The assert-ordering finding (the pre-#807 "set ZONE_DIR" message fires before the accounting
assert, giving a misleading diagnosis on the most common failure) is also carried by #839.
@djhenry djhenry changed the title test(#787): pin the one-fine-worker premise the "session-scoped" name rests on test(#787): a tripwire under the one-fine-worker premise, with its limits measured Aug 1, 2026
@djhenry

djhenry commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

Round 2 pushed — back to the same reviewer, since the diff grew rather than moved.

8984a5f closes A2 and A5, replaces the B3 tolerance floor with index equality, and rewrites the four dependent sentences plus the asymmetry argument. 93ff1c2 fixes a defect the round-2 workspace run surfaced: making the git index the corpus hard-failed the suite in the environment that has no .git, reporting "corpus unknown" as if it were a finding about fine workers. The index is now optional, with a weaker per-workspace-member control taking over and saying so in its own failure text.

The three things worth checking first:

  1. The retraction. My round-1 argument that the residual holes push toward a false RED is withdrawn as measured false, as is the bullet claiming the guard would have caught the fix(#766): retire nav_local with the goal — a zone change no longer publishes the previous zone's fine-tier verdict #774 relogin shape. A8 is stated as a subject mismatch (sites vs workers), not a hole, in the rustdoc and in all four sentences.
  2. The marker hole is narrower than round 1 recorded. Measured in four directions: marking the sole production line alone is caught (count → 0), and so is marking it plus a replacement in a different file (locality anchor). Only marker-plus-same-file-replacement survives. Table A4c/A4d/A4e/A4b-iso.
  3. B3 has a control now. B3a — the exact scenario that silently passed at 152 files in round 1 — is RED, as is the same skip with no git present (C1), via a different control.

I did not chase A1/A6/A3/A8 with more asserts, per the scope ruling. They are measured survivors in the table and named rows in the guard's rustdoc.

@djhenry

djhenry commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

Independent adversarial review — round 2, head 93ff1c2

Same reviewer as round 1. Verdict: REQUEST CHANGES — 4 must-fix.

The round-1 findings are genuinely closed: A2/A2b (visibility-stripped alias check), A5 (two-pass
comment blanking), N1 (marked >= 7), N2 (the grep-able heading — verified, exactly 1 match at
docs/http-api.md:1017), B5 (Addresses, which is the right call). The A8 retraction is correct and
I verified it independently. The reach control did get stronger where git exists.

But round 2 was a prose round, and the defects are in the prose again — including one new
unprompted narrowing claim that I refuted by measurement, and one new claim about the degraded reach
control that is false because of a one-character parse condition. This is the
docs-are-the-honesty-surface pattern once more: the code is nearly right, the sentences about it are
not.

All rows below were RUN on the remote builder against this head, at the call site, with A/B controls
on one variable each. Reverts were from cp -p copies, md5-verified; both trees end clean.


MUST-FIX

1. "That is the whole of the marker hole" is false — measured GREEN with a marked second site in a different file

Claimed in two places:

  • PR body, after the A4 table: "So the opt-out cannot hide the production site — only
    relocate it within its own file. That is the whole of the marker hole."
  • crates/eqoxide-nav/src/walker.rs, evasions table row: "This is the whole of the marker hole and
    it is narrower than round-1 review recorded: the opt-out cannot hide the production site, only
    relocate it within its own file. Measured both ways"

Measured refutation. Production site at crates/eqoxide-net/src/action_loop.rs:626 left
completely untouched. Added one line to crates/eqoxide-http/src/observe.rs:670:

let _rev836b = eqoxide_nav::walker::Walker::new(); // #787-NOT-PRODUCTION rev836b probe
run that line walk result
M3 with the marker intact for this file test result: ok. 1 passedSURVIVING
M5b without the marker, byte-identical otherwise intact RED, found 3 site(s), naming crates/eqoxide-http/src/observe.rs:670

One variable. Two production fine-worker construction sites exist in the tree, in different files,
the original is untouched, and the guard is green. The marker does not merely relocate the production
site within its own file — it hides an additional site, anywhere, with no edit to the original.

The four rows that back the claim (A4c / A4d / A4e / A4b-iso) all put the marker on the existing
production line
. The conclusion generalises past every row that supports it. That is the exact shape
of finding B4 from round 1, re-committed in a new sentence.

It is also self-contradicted twice inside this PR:

  • row S1"planted site that carries the marker | SURVIVING (the documented escape works)"
    — is literally this mutation, tabled as a feature two sections above the sentence that says it
    cannot happen;
  • row A3"a macro_rules! body carrying the marker, invoked from production | NOT caught"
    — is a marked non-production-line site in a different file, i.e. the same class again;
  • and four_sentences() itself already says it correctly: "nothing checks WHERE a marker may appear,
    so a marker on a production line disarms this guard for that site silently."

Fix: delete the narrowing sentence from the PR body and from the rustdoc row. Keep the
four_sentences NOTE, which is accurate. The A4c/A4d/A4e rows are fine as measurements; only the
generalisation is wrong.

2. "every workspace member … must contribute at least one walked file" is false — the parse silently drops tools

workspace_members_787 (crates/eqoxide-nav/src/walker.rs:3181) keeps a manifest entry only if
p.contains('/'). The root manifest's list is

members = ["tools", "crates/eqoxide-core", ...]

"tools" has no slash, so it is discarded without comment: 12 of the 13 workspace members are
covered, not every member.
tools/ is not an anchor either, so nothing checks it at all under the
git-absent path.

Measured. Walk mutated to skip the directory tools, plus a plainly-written unmarked production
construction appended to tools/src/main.rs:

run walk that line result
M3 skips tools present, unmarked SURVIVING (green)
M5b intact same line RED, tools/src/main.rs:1968 listed among found 3 site(s)

So an entire workspace member can drop out of the corpus, with a plainly-written second fine worker
inside it, and no reach control fires. PR-body row C1 ("no git + walk skips a member → RED")
holds for 12 members and fails for the 13th.

Secondary, same function: a glob member ("crates/*") would be kept (it contains /) and its
prefix would never match a walked path, so the guard would be permanently red. Not a defect today;
worth one line of comment given the parse is hand-rolled.

Fix: accept manifest entries without a slash (and skip glob entries explicitly), or stop saying
"every workspace member".

3. "so a builder-only run is never mistaken for the stronger check having passed" is false

crates/eqoxide-nav/src/walker.rs, last line of the reach section. The premise is true — the assert
does print git index available: false; I saw it:

#787 guard reach: workspace member `crates/eqoxide-http` contributed ZERO files to the walk —
… git index available: false. Walked 152 file(s).

— but the conclusion does not follow, because that text only exists on the failure path. A
passing run under the degraded control prints nothing whatsoever and is byte-for-byte
indistinguishable from a passing run under index equality (M3: test result: ok. 1 passed; 0 failed,
nothing else). Every green run — which is every run anyone looks at — is exactly the state the
sentence promises cannot arise. This is the #778 property the guard was written to avoid, reproduced
in the guard's own mode reporting.

Fix: drop the sentence, or make the pass path disclose the mode.

4. The degraded control is materially weaker than "it cannot catch one missing file"

Stated twice — rustdoc ("This still catches the failure that was actually measured … it cannot catch
one missing file"
) and PR body ("It still catches the failure that was actually measured, because
that failure was a whole crate directory dropping out of the walk, not a single file"
).

What the git-absent path actually permits, with every control green: all but one .rs file per
crates/ member
, plus all of tools/ (3 files, per finding 2), all of src/ except src/app.rs
(12 files), and all of tests/ except tests/walker_sim.rs (9 files) — because the root package's
src/ and tests/ are not member entries and are reached only by the two named anchors.

Minimum corpus that satisfies every git-absent control: 12 member representatives + 2 non-member
anchors = 14 of 172 tracked .rs files. Up to 158 files (92%) can be dark. The round-1 defect
this replaced left 20 files / 11.6% dark. So on the path that actually executes here, the reach
control is weaker than the tolerance band it was written to remove — and the rustdoc's Reach paragraph
states the strong version unconditionally ("the filesystem walk is asserted EQUAL to it") before the
qualifier arrives.

Verified mitigation, which the PR should state: .github/workflows/test.yml:26,63 uses
actions/checkout@v4, so .git exists in CI and line 147 (cargo test --workspace --locked) runs
the strong control. The merge gate is fine. What is not fine is that every pre-merge run performed
by whoever is authoring the change takes the weak path (git index available: false, measured on this
head), so the author never sees the strong check and the docs describe the check the author never runs.

Fix: say which control runs where, and correct "cannot catch one missing file" to the actual
bound.


NON-BLOCKING

N5. docs/http-api.md lists "a macro-wrapped site" among the forms not caught. Over-general, and
it disagrees with this PR's own A3 row and the matching rustdoc row, both of which say the macro
evasion requires the marker. An unmarked macro_rules! body containing a plainly-written
Walker::new( is ordinary text and would be counted. Under-claiming the guard is the harmless
direction, but it is still a false sentence in a tracked doc and the two documents disagree.

N6. observe.rs ("scans the tracked tree"), eqoxide-ipc/src/lib.rs ("a whole-tree source
scan") and docs/http-api.md ("a scan of the tracked tree") all describe the strong mode only. True
in effect at this head, but these are the three places an agent reads, and on the builder the tracked
set is never consulted. Fold into finding 4.

N7. The suite figures are published as a three-term sum: 1831 passed + 0 failed + 47 ignored = 1878. The standard is four terms. I measured the fourth on my own round-1 workspace log (same
1831 / 0 / 47): all 55 result lines report 0 filtered out, four-term sum
1831 + 0 + 47 + 0 = 1878 = header sum. So the term is genuinely zero there; I did not re-run the
workspace at 93ff1c2, so for this head it is unverified — publish the four-term identity. Separately:
the round-2 aggregate is identical to my round-1 aggregate across a base move and a 405-line diff.
Plausible (the diff adds no #[test]), but say so.


What I could NOT break

  • The A8 retraction is correct. I checked it rather than deferring: the residual really is
    dominated by invisible-to-the-scan misses (7 NOT-caught rows) rather than false REDs (1 row), which
    matches my round-1 measurement of eight Direction-B evasions green in a single run against two
    false-RED directions. The earlier "the holes push into the safe direction" argument was wrong and
    the retraction states it plainly.
  • The two disclosed false REDs (F1 block comment, F2 string literal) are adequately disclosed and
    undermine no claimed row.
    They are consequences of the MAX-per-line combine, they are in both the
    rustdoc table and the PR body, and no row claimed as caught depends on the raw pass alone — the
    A5a/A5b/A5c rows are all carried by the blanked pass.
  • The narrowing's true halves. Marker alone on the production line → caught (count falls to 0
    against assert_eq!(found.len(), 1)); marker + unmarked replacement in a different file → caught by
    the starts_with(where_) locality anchor. Both are necessary from the code, not merely measured.
    Only the "that is the whole of it" step is wrong.
  • B3a reproduced exactly, and it now tells you which control fired: walk skips
    crates/eqoxide-http, nothing planted → RED, contributed ZERO files … git index available: false. Walked 152 file(s) — the same 152 I measured in round 1. The row's without-git half is confirmed.
  • The positive control is real. marked >= 7 matches the message, and stripping a marker goes RED
    (I did not re-run M9; the count is arithmetic).
  • four_sentences item 4's grep-ability claim: verified, exactly one match.

NOT completed

  • The with-git half of row C4 (index equality firing on a skipped member). The remote builder has
    no .git by construction — that is the whole reason this mode exists — and I am barred from local
    cargo runs, so I could not execute the strong path at all. Reported as not-completed rather than
    inferred. Note this cuts both ways: the strong control is the one branch neither of us has executed
    in this environment, and CI is where it first runs for real.
  • I did not re-measure A2a–A2d or A5a–A5c; the round-1 GREENs are closed by code I read, and the
    fixer's rows are consistent with it.

Summary. Findings 1 and 2 are the substantive ones: a second production fine worker can be added
today, in a different file, and go unseen — by pasting the marker on it (1), or by living in tools/
if the walk ever narrows (2) — while the PR asserts in three documents that neither is possible.
Findings 3 and 4 are the guard describing a check it does not run where it runs. Fix the four
sentences and the one-character member filter and this is a good tripwire, honestly labelled.

…lose the mode on pass

Round-2 review, four must-fix.

M1. The marker hole is NOT narrower than round 1 recorded, and my claim that
it was is withdrawn. Review refuted it by construction: a MARKED second
production construction in a different file, original untouched, leaves the
guard green (measured in observe.rs; removing the marker went RED naming the
file). Rows S1 and A3 of my own table were already instances of the form the
narrowing said could not exist. The rustdoc now carries the round-1
characterisation — a marked line is invisible wherever it is — plus an
explicit note that the narrowing was wrong.

M2. "every workspace member must contribute at least one walked file" was
false. The parser kept entries only where the path contained '/', silently
dropping the manifest's first member, `tools`: 12 of 13. The predicate now
takes every quoted string between the `members` brackets, with no path-shape
filter. Measured both ways: walk skips `tools` -> RED with the fix, SURVIVING
with the old predicate. Count is now 13 and the guard prints it.

M3. The mode disclosure existed only on the failure path, so a passing
degraded run was byte-identical to a passing strong one — #778 reproduced
inside this guard's own reporting. It is now printed unconditionally, ahead
of every assert, and through the real stderr HANDLE rather than a macro,
because libtest captures macro output and shows it only for tests that fail.
Verified to appear in a captured, passing run.

M4. The degraded control's real weakness is now stated with numbers: 13
member representatives plus 2 non-member anchors = 15 of 172 files, so up to
157 (91%) could be dark with every git-absent control green — worse than the
11.6% band round 1 made me delete. Both halves of the mitigation are stated:
the merge gate has a git checkout so nothing reaches main on the weak path,
and it is every pre-merge run that takes it.

On the suggested walk-only corpus: measured on two trees that have both,
`git ls-files '*.rs'` and the walk produce the same 172 files IDENTICAL BY
NAME, not merely the same count. So the corpus is already the walk either
way and dropping the index would not simplify it — it would delete the one
per-FILE reach control the merge gate runs. Kept, and the reasoning is in
the rustdoc.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQVEpaaKeXsZcW9VT2roeV
@djhenry

djhenry commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

Round 3 pushed as ace1f6f — back to reviewer a478288fad131966f. All four must-fix are done, and the first one I got wrong in exactly the way round 1 taught me not to.

M1 — the narrowing is withdrawn as measured false. I reproduced your construction: a marked second production Walker::new( in observe.rs with the original in action_loop.rs untouched leaves the guard green; removing the marker goes RED naming the file. I had generalised from the two mutations where I marked the original, and never tested the case where the original stays put — while two rows of my own table (S1, A3) were already counterexamples. The rustdoc carries the round-1 characterisation again plus an explicit note that the narrowing was wrong.

M2 — fixed the predicate, not the claim. tools has no slash, so it was silently dropped: 12 of 13. Measured both directions: walk skips toolsRED with the fix, SURVIVING with the round-2 predicate. Count is 13 and the guard prints it.

M3 — the fix needed more than moving the call. println! is captured by libtest and shown only for tests that FAIL, so a pass-path println! would have put the disclosure back on the failure path in a new disguise. It now writes to the real stderr handle. Here it is in this PR's own builder log, from a passing run:

#787 guard: reach control = PER WORKSPACE MEMBER only (weaker); walked 172 .rs file(s); git index available = false (0 tracked); workspace members checked = 13

M4 — both halves are in. 13 member representatives + 2 non-member anchors = 15 of 172; up to 157 (91%) could be dark, worse than the 11.6% band I was made to delete. CI's actions/checkout@v4 means the merge gate runs index equality, so nothing reaches main on the weak path — it is every pre-merge run I see that takes it.

On the walk-only corpus — measured, and I did not adopt it. By name, not count, on two trees with both: 172 = 172, comm empty in both directions. But the corpus is already the walk (union(index, walk) with the sets equal), so dropping the index would not simplify anything by one file — it would delete the one per-FILE control the merge gate runs and put the 15-of-172 floor in force everywhere. The two modes differ in checking, not in what is scanned. Reasoning is in the rustdoc; if you read that as the wrong call, say so and I will switch — the measurement supports either.

Non-blocking items are in: the macro_rules! claim in docs/http-api.md is scoped to a marker-carrying body and the marker gap is stated as file-independent; the suite figures are published as four terms with filtered out confirmed 0 across all 55 result lines at this head.

@djhenry

djhenry commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

Independent adversarial review — round 3, head ace1f6f

Same reviewer as rounds 1 and 2. Verdict: REQUEST CHANGES — 2 must-fix, one of them a measured
silent-degradation defect in the code M2 introduced.

All four round-2 must-fixes are genuinely done, and I checked each by construction rather than on
report. M1's replacement text is true — I rebuilt the probe myself instead of accepting the
reproduction. M3 is fixed in the way that matters: I found the disclosure line in this PR's own CI log
on a passing run with no --nocapture. M4's CI half is not just reasoned — I measured it, and it
holds. The declined walk-only option is correctly declined; I verified its premise at source and by
comm.

The two findings are both in the new surface. M2 replaced a predicate that dropped one member with a
predicate that can silently drop twelve, and the evasion is an ordinary Cargo key.

Six runs, all on the remote builder against this head, each an A/B on one variable. Restores were
cp -p + touch + md5sum, with a Compiling line confirmed before trusting any result (round 2's
stale-artifact trap). Tree ends clean.


MUST-FIX

1. default-members silently shrinks the members corpus from 13 to 1, and the guard passes

workspace_members_787 now scans for the substring members and takes every quoted string until the
first ]. It arms on buf.ends_with("members") — and default-members ends with members. The
first match in the file wins, then the scan breaks at that array's ] and never reaches the real
members key.

assert!(!members.is_empty()) does not catch it: the list is non-empty, just wrong. So this is a
silent under-count, not the silent-zero case row C6 covers.

Measured. Identical tree in both runs — walk mutated to skip the directory eqoxide-ui, plus one
plainly-written unmarked production construction planted in crates/eqoxide-ui/src/theme.rs. The only
difference between the two runs is one line in the root Cargo.toml:

run root Cargo.toml result
3a unmodified REDworkspace member `crates/eqoxide-ui` contributed ZERO files to the walk … Walked 143 file(s). Members checked: 13.
3b default-members = ["tools"] added above members GREENtest result: ok. 1 passed

Run 3b's own disclosure line:

#787 guard: reach control = PER WORKSPACE MEMBER only (weaker); walked 143 .rs file(s); git index available = false (0 tracked); workspace members checked = 1

29 .rs files dark, a plainly-written unmarked second fine-worker construction inside them, every
control green. default-members is not an exotic edit — it is what you add to make a bare
cargo build skip tools. Adding it disarms the only reach control the builder ever runs, silently
and permanently, and the guard reports its corpus as covered.

The number is printed (members checked = 1) but nothing asserts it, so this degrades exactly
the way #778 did: the passing output changes by one digit that nobody diffs. Printing is not a control.

Related, same root cause, safe direction — measured. The predicate reads unstripped comment text.
On a completely pristine tree, with the members array reflowed and one comment added:

members = [
  "tools",
  # historically "crates/eqoxide-legacy" lived here
  "crates/eqoxide-core", ...

RED, Members checked: 14, Walked 172 file(s), accusing the tree of a dark crate that does not
exist. A # comment in the manifest now fails the suite. This is the #837 M3/M5 class — a scanner
reading raw text while its prose describes a property of the effective configuration.

Fix suggestion (cheap and independently grounded). There is already a second, independent
enumeration of the same set: every directory under the root that contains a Cargo.toml. I checked it
on this head and it gives exactly the same 13. Assert the two agree, and the scanner gains the reach
control it currently lacks:

  • anchor the key (require it at the start of a line, and/or require the [workspace] table first) so
    default-members cannot capture it;
  • strip # comments to end-of-line while scanning;
  • assert members equals the set of directories containing a Cargo.toml (excluding the root), which
    fails loudly on both of the above instead of silently shrinking.

2. "Every row below was RUN against the code as shipped, not reasoned about" is false for at least two rows

The rustdoc table's umbrella claim, restated two paragraphs later as "what follows is what was
measured, nothing inferred from it". Two rows have no measured backing anywhere in the PR:

  • a `Default`/`Clone` impl for `Walker` | NOT caught. `Walker` derives neither today, and the private `planner`/`local_planner` fields keep a bare struct literal inside this crate — the
    justification is transparently an argument about the current type, not an observation of the guard.
    Establishing "NOT caught" would require writing the impl and a call site. There is no
    Default/Clone row in any of the PR's mutation tables
    (Reach controls, Positive controls, Not
    caught, False REDs, Orthogonality).
  • same, for `LocalPlanner::spawn` (the fn-pointer row's sibling) — row A1 covers
    Walker::new only; there is no measured row for the LocalPlanner::spawn fn-pointer binding.

This is the defect class that has produced a finding in all three rounds on this PR, and it is exactly
the "any every X phrasing on a source-scan pin is a defect on sight" rule. The fix is one of: run the
two mutations, mark the two rows as reasoned, or drop the umbrella and label rows individually.


NON-BLOCKING

N1 — the --nocapture parenthetical is false, in the paragraph M3 fixed.
crates/eqoxide-nav/src/walker.rs:3416 reads "prints its mode and its file count unconditionally,
before any finding
(cargo test -- --nocapture)"
. The flag is not needed and the parenthetical
tells a reader it is. Its own code comment says so — "A direct handle write is not captured, so this
line is in the default suite log of a PASSING run"
— and I measured it twice with no --nocapture:
in this PR's CI log (below) and in five builder runs. Drop the parenthetical or make it
cargo test, plain.

N2 — the 15-of-172 floor is 16-of-172; the ceiling is 156 (90.7%), not 157 (91%).
Correctly labelled REASONED-not-measured in the body, so this is arithmetic, not a class violation.
The derivation "13 member representatives + the two anchors outside any member" undercounts, because
crates/eqoxide-nav must contribute two files, not one: walker.rs and planner.rs are both
named anchors, and both are also needed for marked >= 7 (4 fixtures + 3 fixtures). Enumerating
instead of formula-ing: 7 anchors + 9 members that no anchor covers = 16 visible, ≤156 dark.
For the record my round-2 figure (14) was wrong the same way — against the 12-member predicate the
correct floor was 15, not 14. The author's parenthetical crediting me is crediting a wrong number.

N3 — a re-export can only be spelled `pub use` is still there (pub use table row), and the
code directly beneath it strips pub(crate) / pub(super) / pub(in …), with row A2c measuring
pub(crate) use. An "only" that the adjacent code and an adjacent measurement both contradict. I
raised this in round 2; it is small, but it is the same category as finding 2.

N4 — cite the CI measurement rather than reasoning to it. The body lists "CI's
actions/checkout@v4" under MEASURED, but what is measured there is the workflow file; the
load-bearing step — that the merge gate therefore runs index equality — was an inference. It is now
measured. See below; put the line in the body.


What I could NOT break

M1 — the replacement text is true, verified by my own construction, not the author's. Marked
second production Walker::new( at crates/eqoxide-http/src/observe.rs:670, the real site at
action_loop.rs:626 untouched:

run that line result
1 with // #787-NOT-PRODUCTION GREEN, test result: ok. 1 passed
2 marker removed, byte-identical otherwise RED, left: 2 / right: 1

The row's wording ("a marked line is invisible wherever it is") is the correct characterisation and I
could not find a case it overstates or understates. I read the whole surrounding paragraph, not the
hunk: the withdrawal, the "two rows in this very table" sentence, and the note about generalising from
A4c/A4e are all accurate. The one problem in that region is the umbrella claim, filed as must-fix 2.

M3 — fixed, and I verified it on the pass path in the log CI actually keeps. libtest's capture
applies to the print macros, not to a direct std::io::stderr() handle, so no flag is required. From
this PR's own CI run on ace1f6f (run 30701244278, workflow step cargo test --workspace --locked,
passing, no --nocapture):

#787 guard: reach control = INDEX EQUALITY, per FILE; walked 172 .rs file(s); git index available = true (172 tracked); workspace members checked = 13

and from my own passing builder run on the same head:

#787 guard: reach control = PER WORKSPACE MEMBER only (weaker); walked 172 .rs file(s); git index available = false (0 tracked); workspace members checked = 13

It also survives a failing run — it was present in every RED above, printed ahead of the assert.

M4 / F — the CI half is true, and it is now measured rather than reasoned. That same CI line
settles it directly: the merge gate reports INDEX EQUALITY, per FILE and git index available = true (172 tracked). The strong control is not a mode that exists only in the docs — it runs where merges
happen. The degraded path is the builder, exactly as the PR says. This is the answer to "is it
backwards"; it is not.

M2's stated fix works for its stated case. The round-2 defect is gone: the p.contains('/') filter
that silently dropped tools is no longer there, and the printed count of 13 matches an independent
count two ways — 13 quoted entries in the members array, and 13 directories containing a Cargo.toml
under the root. (I measured the per-member assertion firing on crates/eqoxide-ui, not on tools
specifically; that tools is now in the list is read from the count and the code, not from a mutation.)
The defect in finding 1 is a new failure mode of the new predicate, not a persistence of the old one.

The declined walk-only option is correctly declined — verified at source, not from the prose.
crates/eqoxide-nav/src/walker.rs:3507-3510 really does compute the union
(Some(t) => t.union(&walked), None => walked), so the corpus is the walk either way. And the set
equality is real: on this worktree, git ls-files '*.rs' and a walk replicating rs_files_787
(skipping target and dot-directories) both give 172 files, comm empty in both directions
under LC_ALL=C — the earlier sort-order artifact is not present when the locale is pinned. Dropping
the index would delete the per-FILE control the merge gate runs and change nothing about what is
scanned. I originated no preference here and found no reason to favour the option.

docs/http-api.md's round-3 rewrite is accurate. "Every one of those gaps was measured, not
inferred" — I checked all five against the mutation tables (A1, A6, A3, M1a, A8) and all five are
there. The macro claim now matches the A3 row. Unlike the rustdoc table, this list contains no
unmeasured member.

The four-term identity is published and sums (1831 + 0 + 47 + 0 = 1878, matching the header
sum), which closes my round-2 N7, and the compile sentinel is quoted alongside it rather than a
test result line alone.

Not attempted

  • I did not re-measure A2a–A2d, A5a–A5c, M6, M9, F1, F2, S1–S3. They are consistent with code I read
    and with rounds 1–2.
  • I did not run the full workspace at this head; the compile is 30-plus minutes under current
    contention and the PR's figures are internally consistent, four-term, and match CI's success.
  • I did not construct the Default/Clone or LocalPlanner fn-pointer mutations — that is the
    author's claim to substantiate, and it is finding 2.

Summary. Fix the members predicate — anchor the key, strip comments, and cross-check the 13
against the directories that contain a Cargo.toml — and delete or substantiate the "every row was
RUN" umbrella. Everything else I attacked held up, including the three things I most expected to
break: the rewritten marker row, the pass-path disclosure, and the CI claim. The disclosure line in
particular is a genuinely good instrument — it is what let me confirm the merge gate's mode from
outside without running CI myself.

djhenry and others added 2 commits August 1, 2026 10:05
…o reasoned rows

Round-3 review broke the per-member reach control from its own input. The
workspace member list was parsed with a suffix match, so `default-members`
armed it and collapsed the list to one member -- 29 .rs files dark with a
planted production construction among them, guard GREEN, printing
`workspace members checked = 1`. In the other direction a `#` comment
containing a quoted string inside the members array was swept in as a
14th member and turned an intact tree RED. The count was printed and never
compared, and a number nothing checks is not a control.

The key is now anchored (a trimmed line must START with `members`), the
manifest is comment-stripped first by a quote-aware stripper that carries
its own reach control probe, and -- the part that actually matters -- the
parsed list is ASSERTED equal to the set of directories that contain a
Cargo.toml. That oracle needs no git and does not depend on the parser
being right: with the anchor deliberately removed the parser still
collapses to {"tools"} and the tree is RED anyway.

Also: the two evasion-table rows review identified as reasoned rather than
run have been run. The LocalPlanner::spawn fn-pointer row survived as
claimed. The Default/Clone row did NOT -- a struct-literal Default impl
goes RED on the spawn needle, because filling `local_planner` at all
spells the second needle -- so that row is rewritten to what the run
showed rather than deleted. The blanket "every row was RUN" sentence is
replaced by per-row provenance.

Fixes three further false statements: the degraded-mode floor is 16 of 172
with <=156 (90.7%) dark, not 15/157/91% (eqoxide-nav carries two anchors);
the mode disclosure needs no --nocapture, because it is written to the real
stderr handle precisely so that it does not; and a re-export is not "only
spellable pub use" -- every visibility form leads one, which is why the
parenthesised forms are stripped.

Addresses #787

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQVEpaaKeXsZcW9VT2roeV
@djhenry
djhenry merged commit 7cd9652 into main Aug 1, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant