fix(captured)!: select the record the key is the subject of, not one that mentions it - #842
fix(captured)!: select the record the key is the subject of, not one that mentions it#842wenzowski wants to merge 3 commits into
Conversation
`the_read_shaped_gh_calls_are_allowed` asserted a full allow over `gh pr create --draft --fill`, but that command is also subject to `pr-names-an-issue` — a `requires_key` row whose evidence is the command, the branch name, and the subjects on `origin/main..HEAD`. So the case was asking about the developer's branch rather than about any `gh` lifecycle row, and `main` itself — no key, nothing ahead — is red. It passed in CI only because a PR branch satisfies the other row by construction. The file already carried the right mechanism one case up: `assert_no_gh_lifecycle_refusal`, whose doc says it is "weaker than a full allow on purpose, and only for commands where a SECOND row legitimately fires". `gh pr ready` uses it for exactly this reason. `gh pr create` is the same shape and is moved onto it. The coverage is narrowed rather than dropped: the four `gh` lifecycle rows are still asserted not to refuse the command. Refs: CLOUD-1384
…that mentions it `captured::reduce` picked a capture by byte containment over the whole response and took the first match in handle order. So a `[[rule.captured]]` row did not read "the record for CLOUD-N" — it read whichever stored response happened to contain that string, with a digest deciding between them. Measured over this repository's own store: 14 captures contained `CLOUD-1188`; the one read carried no `project` node and answered `false`; the `get_issue` for that row, carrying `project`, sorted later and was never consulted. `claim-before-code` therefore refused a filed, Done row as unfiled, and the count went 0 -> 1 from ordinary board reads with no tree or config change — a false deny that reading things causes. The module's own header anticipates the neighbouring failure — "digest order says nothing about time" — and picks a predicate stable under a later read to survive it. That reasoning is sound and does not help here: this is not a stale answer about the right document, it is a confident answer about the wrong one, and no predicate survives that. The engine already had the right selector. `capture find --key-at` resolves a response whose scalar at a consumer-named path equals the key, most recent first in the log's append order. `[[rule.captured]]` now takes an optional `key_at` and resolves through it, so there is one authority on what "the capture for this key" means instead of two. Absent, `key_at` keeps containment: this field can only narrow what answers, so it is raise-only in house-style §8's sense and no landed row changes verdict underneath a consumer. The path is the row's, so no tracker field name reaches `crates/batten` and rule 1 stays paid. BREAKING CHANGE: `CaptureQuery` gains a `key_at` field, which breaks anyone constructing one with struct-literal syntax. The break is on the LIBRARY surface only and was measured rather than predicted — `mise run semver` refused the branch with `constructible_struct_adds_field`. The CONSUMER surface does not break: the field is optional and an absent one keeps the old selection, so no landed `batten.toml` changes verdict. The one route that would avoid it, `#[non_exhaustive]`, is itself a break. `find_in_filtered` is the one walk both entry points share, so the ordering keeps a single authority; it is infallible by construction and the public wrappers supply the `Ok` their signatures promise. Refs: CLOUD-1387 Admits: 881e29a69acb906fd45e2d2f6da9f1a4f2c07b0ed727fce642a4edb07873e857 Admits-rule: protected-mutation Admits-verdict: path write refused Admits-subject: batten.toml Admits-head: 12a4d16 Admits-epoch: bae4ca980f7ca899d94baa814ec7ef5a5d8c654d50b490c895e7747a4e0a5099 Admits-author: alec@wenzowski.com Admits-prev: a6e22913ed3e183812f454250f5623214f55a63438912fd5c77fb3fae0730073 Admits-answer-lost: `claim-before-code` stays wrongly red. It refuses this repository today from a payload that was never about CLOUD-1188 — 14 captures contain the literal, the one read has no `project` node — so `verify` refuses every tree in this container and CLOUD-1384's committed fix cannot land. Without the row edit the engine change is inert: the new selector is only reached by a row that declares the path. Admits-answer-precondition: The surface cannot express this change: `key_at` is a field on a `[[rule.captured]]` row, and a rule row exists only in the committed config — there is no verb that edits one, and no consumer of Batten could add it any other way. The write is a five-line addition to one existing row plus its comment, lands in the same diff as the engine change that reads it, and is exactly what a reviewer needs to see to judge CLOUD-1387. Admits-answer-rejected-route: `config read first` does not apply: I read the row and the module it feeds before editing, and the diagnosis came from that read plus the store. `patch run first` does not apply either: there is no patch to apply — this is a new field this session is adding, so nothing exists to run. The third alternative, `batten capture prune`, I rejected on the merits rather than as unavailable: deleting the captures that outrank the right one is curating evidence to turn a gate green, which is the shape a gate exists to refuse, and it would leave the defect live for the next read.
…ixture Two cases for CLOUD-1387, and the second is the defect arriving in this suite rather than in the engine. `a_mentioning_document_does_not_answer_for_the_key` is the compiled tier the row owes: a store holding the record the key is the subject of and a second that merely cites it, asserting the reduction answers from the first. Handle order is digest order, so "the citing document sorts first" is a property of the bytes and not of the writing order — a case that wrote both and hoped would pass under the old code whenever the coin landed the other way. So a nonce is searched until the citing document genuinely sorts ahead, and the search itself is asserted: the case establishes its own premise instead of assuming it. `stop_posture::hook` set neither a state home nor a git ceiling, twelve lines above `hook_in`, which sets both and says why. A `repo()` fixture carries no `.git`, so git discovery walked UP, and `CARGO_TARGET_TMPDIR` sits under the real checkout — so `completion.unlanded` read the developer's own unlanded commits and pre-empted the advisory the case was asserting. `a_stranded_finding_is_pointed_at_and_the_turn_still_ends` therefore passed or failed on whether the person running it had pushed. Both additions are narrowings, so a case that passed under the ambient version now passes for a reason it actually establishes. Refs: CLOUD-1387
CLOUD-1387 `captured` selects a capture by byte containment, so any document that merely MENTIONS the key and sorts earlier by digest answers for it — `claim-before-code` is refusing this repository from the wrong payload
Why
let Some(node) = parsed
.iter()
.find(|(text, node)| text.contains(&row.key) && node.is_some())Containment over the raw response bytes, first match in handle order. So the row does not read "the captured record for CLOUD-1188" — it reads whichever stored response happens to contain that string and sorts first by digest. An issue body citing the key, a search result listing it, a comment quoting it: all are equally eligible, and the one that wins is decided by a SHA. Measured 2026-09-03, this container's store, for the key
So This is not the failure mode the module guarded against, and the difference is the point. The engine already has the correct selector and this path does not use it. Severity: this is a false deny that a read causes. It flipped 0 → 1 during one session, from ordinary board reads that mentioned the key — no config change, no tree change. Any consumer can turn any Prior art searched: CLOUD-1263 and its sibling (both Backlog) record that Refinement — Ready (select by subject, not by mention) Refinement gate: Definition of Ready & Done. This body carries only specializations.
The claims object — authoritative for §6 and §8. {
"source_of_truth": "crates/batten/src/captured.rs",
"gate": { "task": "batten-check", "exits": [0, 2] },
"commit_type": "fix!",
"blockers": [],
"tests": [
{
"file": "crates/batten/tests/it/captured_facts.rs",
"mutation": "mentioning-document-answers-for-the-key"
}
]
}The slug names the discriminating case. A store carrying two records — one whose subject is the key, one that merely cites it, with the citing one sorting first — and the assertion that the reduction answers from the subject. Today that case is red; a fix that only reordered the store would leave it red for a different reason, which is what makes it discriminating rather than coverage. The consumer surface does not break, and the two are different objects: The alternative that would have avoided it — marking Acceptance
|
|
Warning Review limit reachedNext included review available in 36 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 selected for processing (9)
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 |
|
❌ The last analysis has failed. |
|
SonarCloud reports "The last analysis has failed" on That is the analysis itself erroring, not a quality finding, and it gates nothing here. Per I have not re-run it: this PR is a deliberate carry (see the first line of the description) and cannot land until CLOUD-1387's One thing worth reading properly if this comes up on a PR that is landing: the same memory records that trunk being red does not make your failure trunk's. Read Generated by Claude Code |
`captured::reduce` selects a row's capture with `text.contains(&row.key)` — the FIRST stored response whose bytes merely MENTION the key rather than the one the key is the subject of. So any response quoting a row shadows that row's real payload, and `reduce = "present"` over `node = "project"` answers false for a row that is on a project. `batten capture show <handle> --raw` prints `"project":"Batten"` for the real capture; the gate cannot reach it. THE SHADOWING RESPONSE WAS THE FILING OF A BUG ABOUT THIS DEFECT, whose body names the keyed row while describing something else. Filing the report triggers the thing reported, which is an instance no fixture would invent and the reason it is written down here rather than summarised. It is CLOUD-1387 and PR #842 carries the fix, so this waiver is dated and should be removed by whoever lands it. Porting that change instead would mean duplicating a nine-file breaking change already open, which `claim-not-raced` exists to stop. WHY A WAIVER AND NOT THE TWO ALTERNATIVES. The verdict declares no `override` route, so `batten override spend` refuses it — an assumption to the contrary is corrected here rather than left standing. `capture prune` was reachable and is worse: an id nothing captured reads as could-not-look, so pruning 275 records would make the gate ABSTAIN rather than pass. Deleting evidence until a gate stops deciding is the failure this repository's rules exist to prevent, and `prune` being a declared verb does not change what the act is. The other half of this was a real defect and is FIXED rather than waived, one commit back: the `get_issue` projection omitted `project` entirely, so the rule was unsatisfiable by construction on every clone. That is what keeps this waiver narrow — with the field carried, only the selection is still wrong. Weakens: waiver-added waiver[claim-before-code] Weakens: rule-predicate-changed rule[claim-not-raced].check Weakens: rule-predicate-changed rule[claim-not-raced].glob Refs: CLOUD-1170 Admits: 80e576355accbb764fc0673e3013747faadb0fae79a2fefe36a46f0fcc714a91 Admits-rule: protected-mutation Admits-verdict: path write refused Admits-subject: batten.toml Admits-head: 39c76cb Admits-epoch: 0859537fc6f82ebee62e75ecb8af07ec7c779d9f506b427356a1762dc3496c12 Admits-author: alec@wenzowski.com Admits-prev: c145a0a484d97603ee976f7756f19fa91b3f2667d39ff335562527a088c8683c Admits-answer-lost: PR #793 cannot land. `claim-before-code` refuses on every clone that has read the keyed row, the verdict declares no override route, and the alternative reachable without this file is `capture prune`, which makes the gate ABSTAIN rather than pass — deleting 275 records until a gate stops deciding. Admits-answer-precondition: A `[[waiver]]` lives in the committed authority and nowhere else — that is the design, since a waiver a branch could add locally would be the undesigned hatch the waiver surface replaced. This one exempts `claim-before-code` for the CLOUD-1387 selection defect, with a reason and a date, so the exemption is reviewable and lapses on its own. Admits-answer-rejected-route: `config read first` is what this IS: the exemption is committed config, reviewed as config, which is exactly what a waiver is for. `patch run first` does not apply — nothing here is malformed. Repairing the selection defect instead would mean duplicating PR #842's nine-file breaking change, which `claim-not-raced` exists to stop.
`captured::reduce` selects a row's capture with `text.contains(&row.key)` — the FIRST stored response whose bytes merely MENTION the key rather than the one the key is the subject of. So any response quoting a row shadows that row's real payload, and `reduce = "present"` over `node = "project"` answers false for a row that is on a project. `batten capture show <handle> --raw` prints `"project":"Batten"` for the real capture; the gate cannot reach it. THE SHADOWING RESPONSE WAS THE FILING OF A BUG ABOUT THIS DEFECT, whose body names the keyed row while describing something else. Filing the report triggers the thing reported, which is an instance no fixture would invent and the reason it is written down here rather than summarised. It is CLOUD-1387 and PR #842 carries the fix, so this waiver is dated and should be removed by whoever lands it. Porting that change instead would mean duplicating a nine-file breaking change already open, which `claim-not-raced` exists to stop. WHY A WAIVER AND NOT THE TWO ALTERNATIVES. The verdict declares no `override` route, so `batten override spend` refuses it — an assumption to the contrary is corrected here rather than left standing. `capture prune` was reachable and is worse: an id nothing captured reads as could-not-look, so pruning 275 records would make the gate ABSTAIN rather than pass. Deleting evidence until a gate stops deciding is the failure this repository's rules exist to prevent, and `prune` being a declared verb does not change what the act is. The other half of this was a real defect and is FIXED rather than waived, one commit back: the `get_issue` projection omitted `project` entirely, so the rule was unsatisfiable by construction on every clone. That is what keeps this waiver narrow — with the field carried, only the selection is still wrong. Weakens: waiver-added waiver[claim-before-code] Weakens: rule-predicate-changed rule[claim-not-raced].check Weakens: rule-predicate-changed rule[claim-not-raced].glob Refs: CLOUD-1170 Admits: 80e576355accbb764fc0673e3013747faadb0fae79a2fefe36a46f0fcc714a91 Admits-rule: protected-mutation Admits-verdict: path write refused Admits-subject: batten.toml Admits-head: 39c76cb Admits-epoch: 0859537fc6f82ebee62e75ecb8af07ec7c779d9f506b427356a1762dc3496c12 Admits-author: alec@wenzowski.com Admits-prev: c145a0a484d97603ee976f7756f19fa91b3f2667d39ff335562527a088c8683c Admits-answer-lost: PR #793 cannot land. `claim-before-code` refuses on every clone that has read the keyed row, the verdict declares no override route, and the alternative reachable without this file is `capture prune`, which makes the gate ABSTAIN rather than pass — deleting 275 records until a gate stops deciding. Admits-answer-precondition: A `[[waiver]]` lives in the committed authority and nowhere else — that is the design, since a waiver a branch could add locally would be the undesigned hatch the waiver surface replaced. This one exempts `claim-before-code` for the CLOUD-1387 selection defect, with a reason and a date, so the exemption is reviewable and lapses on its own. Admits-answer-rejected-route: `config read first` is what this IS: the exemption is committed config, reviewed as config, which is exactly what a waiver is for. `patch run first` does not apply — nothing here is malformed. Repairing the selection defect instead would mean duplicating PR #842's nine-file breaking change, which `claim-not-raced` exists to stop.
The defect
captured::reduceselected a capture by byte containment over the whole response and took the first match in handle order. So a[[rule.captured]]row did not read "the record for CLOUD-N" — it read whichever stored response happened to contain that string, with a digest deciding between them.Measured over this repository's own store: 14 captures contained
CLOUD-1188; the one read carried noprojectnode and answeredfalse; theget_issuefor that row, carryingproject: "Batten", sorted later and was never consulted.claim-before-codetherefore refused a filed, Done row as unfiled.The module's own header anticipates the neighbouring failure — "digest order says nothing about time" — and picks a predicate stable under a later read to survive it. Sound, and it does not help: this is not a stale answer about the right document, it is a confident answer about the wrong one, and no choice of predicate survives that.
The verdict drifts with unrelated reads, in both directions. It went 0 → 1 from ordinary board reads with no tree or config change; hours later, with the same pre-fix containment config, it read 0 again because more issues had been read in between. So this produces a false green as well as a false deny — and the false green is the direction that hides a real finding.
The fix
The engine already had the right selector:
capture find --key-atresolves a response whose scalar at a consumer-named path equals the key, most recent first in the log's append order.[[rule.captured]]now takes an optionalkey_atand resolves through it, so there is one authority on what "the capture for this key" means instead of two.key_atkeeps containment, so this can only ever narrow what answers — raise-only in house-style §8's sense, and no landedbatten.tomlchanges verdict underneath a consumer.crates/battenand rule 1 stays paid.find_in_filteredis the one walk both entry points share, so the ordering keeps a single authority.Verification
mise run test:cargo— 4210/4210 withBATTEN_STEP_RECEIPT_BYPASS=1batten check --rule claim-before-code— exit 0 with the fix, exit 2 without itWhy this cannot land
mise run config-lintagainstorigin/main:lint.rs:42-48requires evidence from two places written at different moments: aWeakens:commit trailer and the groomed body that named the same pair before the work started, copied into the claim receipt at mint time. This row's receipt was minted before the fix, but its body did not name aWeakens:pair — so the groom readsGroom::Silentand refuses. Adding it now and re-minting is whatbatten.toml:342calls "the exact shape §8 refuses."The gate is also flagging a narrowing —
batten.toml:833calls the check "a byte comparison with no ranking", so it cannot tell direction. That is a candidate follow-up, not an argument for overriding it here.To land: groom CLOUD-1387's
Weakens:clause first, re-mint the claim, then cherry-pick these commits onto a fresh branch with a matching trailer. The code needs no changes.Also in the diff
stop_posture::hookread ambient git state — it set neither a state home norGIT_CEILING_DIRECTORIES, twelve lines abovehook_in, which sets both and says why. Arepo()fixture has no.git, so discovery walked up into the real checkout andcompletion.unlandedpre-empted the advisory the case asserted. That case passed or failed on whether the developer had pushed.mise run semverrefused withconstructible_struct_adds_field:CaptureQueryispub, so adding a field breaks struct-literal construction. Library surface only; the consumer surface does not break, and#[non_exhaustive]is itself a break, so there is no patch-compatible route.The first commit here is a duplicate of PR #841's, already merged to
main.Generated by Claude Code