Repository navigation
fix: doctor mediator, the defect-ledger bypass it uncovered, and the suite's largest remaining case - #837
Conversation
CLOUD-1349 The mediator ran 16 versions behind the tree it was judging, and nothing reported it — a stale engine reads exactly like a working one
§1 SubjectThe absence of any comparison between the The §2 The measurementMeasured 2026-09-02 in a Claude Code web container, over one full session landing CLOUD-1321.
The engine mediated every tool call in that session against a rule table 16 versions old, and nothing anywhere said so. §3 What that silently disabled, measured by the moment it stoppedThe tell is the step change at the install, not an argument. Across roughly six hours the session issued dozens of
All three shapes had been issued repeatedly before the install and had passed. The rules were in §4 Why this is the worst shape in the model rather than an ordinary bug
A stale mediator defeats all of it at once, one layer beneath where any of it can look. The config is correct, the modules are correct, the tests are green, and the process actually adjudicating calls is enforcing a different, older contract. Silence from the hook is the documented signal that it IS mediating (
§5 Why the existing sensors did not catch it
Refinement — Ready (the engine states which contract it is enforcing) Refinement gate: Definition of Ready & Done. This body carries only specializations. {
"source_of_truth": "batten --version reported 0.0.121 while the workspace was 0.0.137, /tmp/session-start-batten.log did not exist so mise.toml:1987 never ran, and CARGO_PKG_VERSION appears in eight source files of which doctor.rs is not one",
"gate": { "task": "doctor", "exits": [0, 1] },
"commit_type": "fix",
"blockers": [],
"tests": [
{ "file": "policy/harness-wiring.rego", "mutation": "stray-unread" },
{ "file": "policy/harness-wiring.rego", "mutation": "stale-unguarded" }
]
}
Acceptance
§6 Cost of not doing itMeasured, this session: every mediated call for ~6 hours adjudicated against a 16-version-old table, three rule classes provably unenforced, and the discovery arriving only because an unrelated task reached for a subcommand that did not exist. A session that never happened to do that would have finished believing it had been mediated throughout — and would have been right about the silence and wrong about what it meant. Found while landing CLOUD-1321 (PR #828). CLOUD-1186 Narrowing a run skips the engine-side defect-ledger gate, so giving `enforce` a rule selector would create a one-token route around a gate deliberately placed where a rule table cannot lower it
Why
That is a YAGNI argument plus a posture, and
So Why this needs to land before the campaign's spawning gatesThe retirement campaign ports gates into Today a spawning gate ported into a rule row cannot be invoked by name at all — which is exactly the narrowing Refinement — Ready (decouple the skip from the narrowing, on the spawning surface) Refinement gate: Definition of Ready & Done. This body carries only specializations.
Acceptance
Found while pressure-testing a proposal to give both gate verbs an object: the mechanical change was a line, and the coupling it would have inherited was a security regression. CLOUD-1358 Two `cli.rs` cases are 644s of the Windows suite's 1693s — 38% — and they set an asymptotic floor no sharding can pass
Why Measured on the
Two cases are 644.0s — 38% of the suite. The distribution is even more extreme than CLOUD-1223's: 25% of summed CPU is 2 cases (0.1%), 50% is 62 cases (1.6%). Both are the shape CLOUD-1223 named and this pair escaped. Each reads the committed CLOUD-1223's closing acceptance bullet is refuted for these two. It reads: "The four The estimate that deferred this was wrong by 18x, and recording that is the point. These two were examined during PR #819 and declined as "~18s each, worth ~36s" — a local 4-core reading — with the trade stated as "saves ~9s each and costs the cross-rule sort-order property". The real figure is 644s. They set the suite's asymptotic floor, which kills the sharding lever. A partitioned suite cannot finish faster than its slowest single case: 324.85s. Refinement — Ready
Acceptance
Generated by Claude Code CLOUD-1209 `shell_write_advisory` runs the engine against the live tree, so four cases are decided by whether the developer's branch holds a claim receipt — green in CI, red on the desk
Why Four of the ten cases in
Measured, and the captured cause is not the one a reading of the diff suggests. The engine answers the test's
The consequence is a verdict that depends on the developer's session rather than on the code. These four pass iff the current branch happens to carry a claim receipt: green in CI, where a fresh clone has no receipt and the rule finds nothing to key on, and red for any local session that has not run
Root cause. The file's own header explains why it exists as a compiled-binary tier rather than a Refinement — Ready
Test obligation The four cases pass on a branch with no claim receipt, and are shown able to fail by an assertion whose premise the fixture creates rather than inherits. Commit / bump (§6): Blockers (§8): none. Acceptance
|
|
Warning Review limit reachedNext included review available in 47 minutes. View limit detailsLimit details: You’ve used the included review currently available. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Free Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (15)
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Essentials by visiting https://app.coderabbit.ai/settings/billing. Comment |
wenzowski
changed the title
doctor mediator — decide WHICH engine the registrations reachdoctor mediator, the defect-ledger bypass it uncovered, and the suite's largest remaining case
|
HANDOFF — this PR's branch is 7 commits behind the finished work. Do not read
That ref carries everything the body above describes. This PR's own tip ( To take this over: reset the PR branch to State when I stopped, all verified locally, nothing assumed:
One thing a taker should not repeat. Every lap so far failed on a different real defect — Measurements behind the perf claims are on CLOUD-1358, including two refutations of that row's own analysis (the Windows job is CPU-bound rather than floor-bound, so sharding would help). Generated by Claude Code |
|
Handoff pointer updated: the preserved ref is now What the rebase resolved, because it was a real collision rather than a textual one. Another session fixed the same Verified after resolving: Laps since the last comment, none of which were gate failures:
Nothing in that list is a reason to re-run without reading — but of the five laps so far, three ended for reasons that require no code change at all, which is worth knowing before assuming the branch is unhealthy. Generated by Claude Code |
|
Preserved ref moved again: What changed since the last pointer:
Every one resolved by keeping both verbs in sorted order; nothing displaced from either side. That five files need touching for one new sub-verb is the design working — each is a deliberate registration with a written reason — not friction. The two generated files are why this comment exists rather than just a new SHA. I resolved those by taking a side, purely to finish the rebase. Regenerating afterwards showed the golden schema I had picked asserted a surface of 45 commands where the binary emits 46 —
Also worth knowing before re-running: one lap died on the disk floor with Generated by Claude Code |
…reach `doctor hooks` answers whether the registrations reach an engine. Nothing answered WHICH one, and the gap between those two questions is where a session spends six hours believing it is mediated: silence from the hook is the documented sign it IS mediating, so an engine enforcing an old rule table and one enforcing the committed table produce identical evidence. Measured three times in one container on 2026-09-02, `doctor hooks` reporting `0 unwired` throughout. A SUB-VERB, NOT A FOURTH CHECK IN THE BARE REPORT. This landed once inside `diagnose()` and `verify` refused it: `this_repository_is_healthy` went red because `land` had rebuilt `target/release/batten` while the install was an hour old. The check was telling the truth — and whether a container's install is current is a property of the WORLD, while bare `doctor` answers a property of the COMMIT, so folding it in made a commit gate answer on install recency. `.claude/rules/toolchain.md` records that defect for `lock-check` and its remedy was the same split. House style §2 already specifies `doctor <SUB>`. CONTENT, NOT THE VERSION STRING, correcting CLOUD-1349's own §2: `--version` read 0.0.137 on both sides while the installed binary refused the tree's own batten.toml over a key that landed in the base commit. A config surface moves without a version bump, and on a fast-forward-only trunk that is the ordinary case rather than an edge one. Nothing is spawned. `doctor` is `Effect::Read` and the agent allowlist is `filter(effect == read)` with no second list, so running a program a wiring file names would put config-supplied code behind a row any consumer agent may call. Four states: the two could-not-look causes are named variants rather than a string, so `Unresolvable` and `Unbuilt` are kept apart by the type, and neither folds into clean. Refs: CLOUD-1349
…ts old placement Ten cases over the compiled binary. The load-bearing ones: `a_mediator_built_from_this_tree_passes` is the anti-vacuity mirror — identical bytes, same fixture shape, same PATH — because a check that refused unconditionally would satisfy the stale case exactly as well. `equal_length_binaries_that_differ_are_still_refused` pins that the length compare is a short-circuit and not the predicate. Two builds of one source at the same length is the ordinary case for a changed constant, which is also the drift hardest to notice by eye. `the_bare_report_is_unchanged_by_this_sub_verb` is the regression this verb exists as a sub-verb to avoid. An earlier revision put the comparison in `diagnose()`'s check list and `this_repository_is_healthy` went red whenever a rebuild had outpaced the install — a world-property deciding a commit gate. The two could-not-look causes are asserted apart: nothing to compare AGAINST and nothing to compare WITH have different remedies, so one reason id for both would send the reader to the wrong place. Fixtures execute nothing, so neither planted file needs to be a real program — which is what keeps the whole tier at 0.2s. Refs: CLOUD-1349
Completions, the parent's man page and a new `batten-doctor-mediator.1`, all generated by `mise run schema`, `completions` and `man`. No hand edits — `the_committed_pages_are_the_ones_the_binary_emits` is what would catch one. Refs: CLOUD-1349
…e row set Both literals are hand-maintained and that is the design: a command joining the agent allowlist is a safety-critical edit, so it costs a deliberate line and a written reason rather than arriving in a generated diff. `verify` refused the branch until both were written. The reason here is stricter than its sibling's. `doctor mediator` resolves a program name and then deliberately does NOT run it, comparing that file's bytes against the artifact this tree builds. Running what a wiring file names would put config-supplied code behind a row on this very list — CLOUD-170's invariant, and why `on_path` stats rather than executes. Refs: CLOUD-1349
It compares the INSTALL against the BUILD, never the build against the SOURCE. When `target/release/batten` is itself behind the tree, both sides are equally stale, they agree, and the verdict is `mediator ok`. Measured one command apart while this very row was in flight: `land` rebased onto a `main` carrying a new `[[rule.review]]` key, the engine refused the tree's own batten.toml with `unknown field`, and `batten doctor mediator` answered `mediator ok`. Both binaries were the same pre-rebase build. Fourth staleness occurrence in one container that day, and the first this check missed. Stated because a check answering a narrower question than its name suggests IS the failure this repository is organised against — a dead gate and a clean tree are byte-identical on the decision surface, so shipping the verb while it reads as answering more than it does would reproduce the defect one layer up. What is bought stands: the original measured failure, an image-baked or hand-installed binary against a tree that builds a different one, which ran unnoticed for six hours. `two_equally_stale_binaries_agree_and_this_reports_current` pins it as known behaviour rather than leaving it in prose, so a later change cannot widen or narrow the bound with nothing going red. Closing the remainder needs a build-freshness predicate that is still a read; mtime-against-sources is the obvious candidate and is not obviously sound, since a rebase touches files cargo would not rebuild from — it trades this false negative for a false positive. A separate predicate with its own design, not a tightening of this one. Refs: CLOUD-1349
The gate states its own rule: a new leaf verb is `PointerOnly` unless there is a reason it is not, and the reason is the field, so it is written rather than assumed. Pointer-only here for a sharper reason than its siblings. This verb's whole subject is two absolute paths and two digests — the shape most likely to leak one into output. It emits neither: the verdict is a stable token, and the digest is deliberately withheld because it is stable per content but varies per machine, so printing one would defeat §6's byte-stability while telling the reader nothing actionable. The remedy is `mise run install:local`; the verdict says whether to run it. This tier runs the verb and inspects what it actually wrote, so rule 4 is enforced over the bytes rather than declared by the author about their own code. Refs: CLOUD-1349
The emitted surface grew a command, so the golden moves with it. The diff is exactly two additions and nothing else — the command's own row, and its entry in the emitted read-only allowlist — which is the snapshot earning its keep: a surface change that touched anything further would show here. Refs: CLOUD-1349
`a_turn_that_strands_nothing_is_silent` ran through the unisolated `hook` helper, so a real session's findings could reach a fixture asserting nothing is said. The hazard is stated verbatim 300 lines above it, on `hook_in`: "an ambient one would let a real session's findings decide a fixture's verdict." NOT A FLAKE, and re-running would have been the wrong call. Measured 2026-09-03: red inside a `land` lap, green on the next isolated run of the same commit. `unlanded` reports once per HEAD sha and every lap rebases to a fresh one, so the session's own unlanded work is unreported at exactly the moment the lap runs the suite and reported at every other moment. It would fail every lap and pass every re-run — red for the author who is landing, green for everyone else, which is the class CLOUD-1209 already carries for `shell_write_advisory`. Silence is what makes this case the exposed one: every sibling asserts a specific advisory and an extra one cannot make those pass, while any advisory at all fails this. Widens the PR by one case. Stated rather than slipped in: it is not caused by this diff, but it blocks its landing on every lap, the fix reuses the isolating helper already in the file, and calling it infrastructure noise would have been false. Refs: CLOUD-1349, CLOUD-1209
CLOUD-1186 predicted this regression in writing, and CLOUD-1358 shipped it a day later without reading the row. The engine-side gates were skipped on `only.is_empty()` alone. That is a sound convenience on `check` — a caller asking about one row is not asking about the budget or the ledger. CLOUD-1358 then gave `enforce` a `--rule` selector, and the same branch made `batten enforce --rule <id>` a ONE-TOKEN WAY TO SKIP THE DEFECT LEDGER on the verb that runs user-declared commands. The ledger gate is engine-side rather than a `[[rule]]` row precisely so a branch cannot lower it by editing a rule table; the narrowing restored that lowering by another route. CLOUD-1186's own words, from before the selector existed: "a reasonable UX convenience on `check` ... and a security hole on `enforce`", and it names landing this decoupling as the precondition for narrowing `enforce` at all. The skip is now surface-dependent. `check` narrowed is byte-identical to before. `enforce` narrowed still evaluates the ledger. THE BUDGET KEEPS THE SKIP ON BOTH, and the split is the reason `ledger_findings` exists: a budget measures declared instruction files — a property of the tree the caller did not ask about — while the ledger is a claim about this branch's own conduct. Only one of the two is the security property. Four cases, and the mirror earned its keep before it guarded anything: it caught that my first fixture would not parse, because it exercises a path the existing suite already proves works. Shown able to fail by restoring the shipped code — exactly 1 of 27 reddens, the narrowed-check case included in the 26 that do not, so it discriminates the regression and nothing else. BREAKING CHANGE: `batten enforce --rule <id>` now reports defect-ledger findings it previously skipped, so a branch with a tampered ledger that exited 0 under a narrowed enforce now exits 2. That is the defect, not a new rule. Refs: CLOUD-1186, CLOUD-1358
…the family `the_committed_repo_config_gates_a_repository` ran `enforce` over the whole ~103 row committed ruleset to assert a single pointer, `crates/** no-conflict-markers`. Its own comment already said the other rows could not affect that: the asserted stdout is "byte-identical whether the other committed rules are present, absent, misspelled, mis-globbed, or switched off". MEASURED ON THE RUNNER RATHER THAN INFERRED. On the 2-core Windows job it is 436.42s — 24% of that suite's 1810.1s of CPU and its single largest item, with the next slowest at 28.28s. Locally the same case is 21.5s; the 20x is the contention signature the two cases CLOUD-1358 narrowed showed, not a different workload. Narrowed to `--rule no-conflict-markers`: 21.526s -> 0.153s here, assertion unchanged. That suite is CPU-bound rather than floor-bound — wall tracks CPU/2 to within 0.4% on both readings taken — so removing 436s of CPU should be ~218s of wall off every `rust.yml` run. Shown able to fail by pointing `--rule` at a DIFFERENT real committed row rather than by deleting one: the case reddens, so it is asserting this rule's finding and not merely that something was emitted. Refs: CLOUD-1358
…lence Supersedes the previous commit's one-case fix, whose reasoning was wrong and was disproved by the next landing lap. That commit isolated `a_turn_that_strands_nothing_is_silent` and argued only a silence-asserting case could be exposed, since an EXTRA advisory cannot falsify a case asserting a specific one. The next lap failed `a_stranded_finding_is_pointed_at_and_the_turn_still_ends`. The engine emits AT MOST ONE nudge, ranked — two on a turn is how a channel stops being read — so an ambient finding does not add to the expected advisory, it DISPLACES it. Every case in the file is exposed, and the fix belongs at the helper rather than at whichever case failed most recently. `hook` now delegates to `hook_in`, whose own comment already stated the hazard: "an ambient one would let a real session's findings decide a fixture's verdict." The exposure is worst exactly while its author is landing. `unlanded` reports once per HEAD sha and every lap rebases to a fresh one, so a case is red on the lap and green on the re-run — the shape that gets called a flake and re-run until it passes. It is not one, and neither reading of it was. Refs: CLOUD-1349, CLOUD-1209
…ision The rebase resolved `completions/*` by taking one side, which is a choice rather than a derivation — `main` added `doctor session` while this branch added `doctor mediator`, so neither side's generated file carried both. Regenerated with `mise run completions`; all three shells changed. `man/` and the schemas needed no regeneration, so both sub-verbs' pages had already merged cleanly. Refs: CLOUD-1349
…b-verbs The rebase resolved this file by taking one side, so it asserted a surface of 45 commands where the binary emits 46 — `main`'s `doctor session` was absent. That is a claim about what the binary emits, settled only by running it. Regenerated: the diff adds `doctor session` to the command surface and to the read-only allowlist, and displaces nothing from this branch. CI would have caught it; the point of regenerating rather than trusting the resolution is that here is cheaper than there. Refs: CLOUD-1349
f2a5d3e to
4a3106c
Compare
|
❌ The last analysis has failed. |
|
/fast-forward |
1 participant
Closes CLOUD-1349
Closes CLOUD-1186
DO-NOT-CLOSE CLOUD-1358 — already In Review via #834; this adds a third case to the same mechanism, not a new row.
DO-NOT-CLOSE CLOUD-1209 — one case isolated here; that row's own subject is
shell_write_advisory's four, untouched.Three rows, one CI matrix.
CLOUD-1186 — a bypass I shipped in #834, predicted in writing beforehand
This is the important one. CLOUD-1358 gave
enforcea--ruleselector withouttouching the engine-gate branch, which still skipped engine-side gates on
only.is_empty()alone. So from that merge until this commit,batten enforce --rule <id>was a one-token way to skip the defect-ledger gate on the verb thatruns user-declared commands. That gate is engine-side rather than a
[[rule]]row precisely so a branch cannot lower it by editing a rule table; the narrowing
restored the lowering by another route.
CLOUD-1186 was open, groomed, and said so in its title. I did not search for it
before adding the flag.
The skip is now surface-dependent —
checknarrowed is byte-identical to before,enforcenarrowed still evaluates the ledger. The budget keeps the skip onboth: it measures declared instruction files, a property of the tree the caller
did not ask about, while the ledger is a claim about this branch's conduct. Only
one is the security property.
Four cases over the compiled binary, covering §7's (a)/(b)/(c) plus an
anti-vacuity mirror asserting the same fixture does fail an unnarrowed
check— without it, (b) passes whenever the fixture is broken, which it immediately
caught. Shown able to fail by restoring the shipped code: exactly 1 of 27
reddens, narrowed-
checkamong the 26 that do not.Two divergences from §2 are recorded on the row: the selector stays a flag
rather than moving to the object position (that respelling is CLOUD-1184's, and
is a second breaking surface change in two days), and the budget keeps its skip.
CLOUD-1349 — which engine the registrations reach
doctor hooksanswers whether the registrations reach an engine; nothing answeredwhich. Measured four times in one container: a binary that cannot parse the
tree's own
batten.tomlwhiledoctor hooksreports0 unwired.Two corrections to the row's §2, both recorded on it: it cannot spawn (that would
put config-supplied code behind a row on the
filter(effect == read)allowlist),and version equality does not discriminate — measured
0.0.137on both sideswhile the binary refused the tree's config. So the comparison is over content.
A sub-verb, not a check in the bare report. It landed once inside
diagnose()and
verifyrefused it:this_repository_is_healthywent red whenever a rebuildoutpaced the install — a world-property deciding a commit gate, the
lock-checkdefect
.claude/rules/toolchain.mdrecords.the_bare_report_is_unchanged_by_this_sub_verbpins it.
Its blind spot is stated and pinned, not shipped silently: it compares the
install to the build, never the build to the source, so two equally stale binaries
agree and it reports
ok. Measured one command after the engine refused the tree'sconfig.
two_equally_stale_binaries_agree_and_this_reports_currentfixes that asknown behaviour.
The suite's largest remaining case — 436s
the_committed_repo_config_gates_a_repositoryran ~103 rules to assert onepointer; its own comment said the others could not affect the result. On the
2-core Windows job it is 436.42s — 24% of that suite's 1810.1s CPU, next
slowest 28.28s. Narrowed: 21.526s → 0.153s locally, assertion unchanged, shown
able to fail by pointing
--ruleat a different real row.Where CI actually stands, measured
rust.ymlmedian 30.0m → ~18.5m (four successful runs, Sep 2–3) — that is#819's cache fix.
The Windows suite itself went 849.9s → 908.5s: #834 removed 644s of CPU and
other work added ~760s the same day. Two corrections to #834's stated analysis:
that suite is CPU-bound, not floor-bound (wall tracks CPU/2 to within 0.4% on
both readings, so sharding would help), and the pole is a single case rather
than a cluster.
Verification
Local suite 4223/4223.
verifyreachedfast-forward-greenon the prior lap.🤖 Generated with Claude Code
https://claude.ai/code/session_01CTqon6Q8TXmhGpdtQeaT6V