Skip to content

fix(captured)!: select the record the key is the subject of, not one that mentions it - #842

Draft
wenzowski wants to merge 3 commits into
mainfrom
cloud-1387-carry
Draft

fix(captured)!: select the record the key is the subject of, not one that mentions it#842
wenzowski wants to merge 3 commits into
mainfrom
cloud-1387-carry

Conversation

@wenzowski

Copy link
Copy Markdown
Contributor

DO NOT LAND AS-IS. This is carried work. The code is written and verified; what is missing is an ordering that cannot be recovered retroactively — see Why this cannot land below. Refs CLOUD-1387.

The defect

captured::reduce selected 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: "Batten", sorted later and was never consulted. claim-before-code therefore 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-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, so this can only ever narrow what answers — raise-only in house-style §8's sense, and no landed batten.toml changes verdict underneath a consumer.
  • The path is the row's, so no tracker field name reaches crates/batten and rule 1 stays paid.
  • find_in_filtered is the one walk both entry points share, so the ordering keeps a single authority.

Verification

  • mise run test:cargo4210/4210 with BATTEN_STEP_RECEIPT_BYPASS=1
  • batten check --rule claim-before-code — exit 0 with the fix, exit 2 without it
  • The discriminating case searches a nonce until the citing document genuinely sorts ahead of the subject by digest, and asserts the search succeeded — handle order is digest order, so a case that wrote both and hoped would pass under the old code whenever the coin landed the other way.

Why this cannot land

mise run config-lint against origin/main:

batten.toml:rule[claim-before-code].captured rule-predicate-changed

lint.rs:42-48 requires evidence from two places written at different moments: a Weakens: 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 a Weakens: pair — so the groom reads Groom::Silent and refuses. Adding it now and re-minting is what batten.toml:342 calls "the exact shape §8 refuses."

The gate is also flagging a narrowingbatten.toml:833 calls 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::hook read ambient git state — it set neither a state home nor GIT_CEILING_DIRECTORIES, twelve lines above hook_in, which sets both and says why. A repo() fixture has no .git, so discovery walked up into the real checkout and completion.unlanded pre-empted the advisory the case asserted. That case passed or failed on whether the developer had pushed.
  • The break was measured, not predicted. mise run semver refused with constructible_struct_adds_field: CaptureQuery is pub, 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

`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
@linear-code

linear-code Bot commented Sep 3, 2026

Copy link
Copy Markdown
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

captured::reduce (crates/batten/src/captured.rs:124-127) picks the capture a [[rule.captured]] row reads like this:

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 claim-before-code declares (key = "CLOUD-1188", node = "project", reduce = "present"):

captures containing the literal CLOUD-1188 14
first three in handle order unparseable, skipped
the one actually readresponse-63e6dea4…, 14422 bytes has no project node → present = false
the get_issue for CLOUD-1188 itself — response-6fcfabda… project: "Batten" → would answer true, and is never consulted

So mise run batten-check reports 1 claim-before-code — verdict claim mint absent — over a row that is filed, Done, and in project Batten. batten capture find --tool get_issue --key-at id CLOUD-1188 returns exactly one record and it is the right one.

This is not the failure mode the module guarded against, and the difference is the point. policy/claim-before-code.rego's own header anticipates the time half — *"a key read twice is two records and *reduce answers from the first in HANDLE order, which is digest order and says nothing about time" — and chose a predicate that is stable under a later read to survive it. That reasoning is sound and does not help: this is not a stale answer about the right document, it is a confident answer about the wrong document. No choice of predicate survives reading a payload that was never about the row.

The engine already has the correct selector and this path does not use it. batten capture find --key-at <path> <value> selects a record where the key is the record's subject rather than a substring of it — that is exactly the discrimination missing here. Rule 1 is why reduce avoided a parsed field ("which member carries a key is a tracker's schema"), and the comment says so; but --key-at already solves that the right way, by having the consumer name the path.

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 captured gate red simply by reading something, and the remedy a reader would reach for (batten capture prune) is evidence-curation to get green, which is the shape a gate exists to refuse. It also makes verify red on main in this container, so it blocks landing.

Prior art searched: CLOUD-1263 and its sibling (both Backlog) record that captured binds one id to one literal key, so a runtime-varying key set is unspellable. That is a different limitation — expressiveness of the key — and neither says the key selects the wrong document.


Refinement — Ready (select by subject, not by mention)

Refinement gate: Definition of Ready & Done. This body carries only specializations.

  • Authority boundary (§1). crates/batten/src/captured.rs, and whatever [[rule.captured]] field the fix adds to crates/batten/src/config.rs and the generated schema.
  • Computable predicate (§2). With a store holding both a document that merely mentions the key and the record whose subject it is, reduce answers from the second, whichever sorts first by digest. Asserted over the compiled binary, not with input as.
  • Rule 1 stays intact (§2). The path is the CONSUMER's to name, as --key-at already has it, so no tracker schema reaches crates/batten. A fix that hardcoded id in the engine would trade this defect for a rule 1 violation.
  • Rule 6 is the live tension (§2). This adds a field to the config surface, which the house style says to keep narrow. The row must state why the narrower alternatives — leaving containment and documenting it, or pruning — are worse, rather than adding the field by default.
  • Backward compatibility (§2). A row declaring no path must keep its current meaning or be refused at load; it must not silently change verdict. Which of those two is chosen goes on this row.
  • Blockers (§8). None. relatedTo CLOUD-1188 (the row that landed this fact), CLOUD-1385 and CLOUD-1384 (the other two ambient-input defects found in the same session).

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 break is on the LIBRARY surface, and it was measured rather than predicted. **commit_type was fix when this row was written; mise run semver refused the branch with constructible_struct_adds_field, because CaptureQuery is pub and adding key_at breaks anyone constructing one with struct-literal syntax. That is the authority on the library half — ready-lint's own §6 comment says so — so the row is corrected to fix! rather than the gate worked around.

The consumer surface does not break, and the two are different objects: key_at is optional, an absent one keeps containment, and no landed batten.toml changes verdict. Below 0.1.0 the declared major collapses to a patch anyway, so nothing about the release changes; what changes is that the record is honest.

The alternative that would have avoided it — marking CaptureQuery #[non_exhaustive] — is itself a break, so there is no patch-compatible route to the field.

Acceptance

  • mise run batten-check is green on a tree whose store holds documents citing the declared key, with no capture pruned.
  • The compiled tier carries the two-record case above and it is shown red before the fix.
  • Rule 1 holds: no tracker field name is spelled in crates/batten.

Review in Linear

@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 36 minutes.

Check out review usage here.

View limit details

Limit 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.
You're only billed for reviews past your plan's rate limits ($0.25/file).

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Free

Run ID: 4d5acbcd-bf47-47a9-a085-0d5b29e6405e

📥 Commits

Reviewing files that changed from the base of the PR and between b0ff5fd and 5d20627.

📒 Files selected for processing (9)
  • batten.toml
  • crates/batten/src/capture.rs
  • crates/batten/src/captured.rs
  • crates/batten/src/facts.rs
  • crates/batten/tests/it/captured_facts.rs
  • crates/batten/tests/it/gh_guard.rs
  • crates/batten/tests/it/stop_posture.rs
  • schema/batten.local.schema.json
  • schema/batten.schema.json

Note

🎁 Summarized by CodeRabbit Free

Your 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 @coderabbitai help to get the list of available commands.

@sonarqubecloud

sonarqubecloud Bot commented Sep 3, 2026

Copy link
Copy Markdown

❌ The last analysis has failed.

See analysis details on SonarQube Cloud

Copy link
Copy Markdown
Contributor Author

SonarCloud reports "The last analysis has failed" on 5d20627f. Standing down on it rather than acting, with the reason rather than silently.

That is the analysis itself erroring, not a quality finding, and it gates nothing here. Per mem:workflow/sonar-scope, measured 2026-08-22: there is no SONAR_TOKEN and no Sonar step in any workflow — analysis arrives through the SonarCloud GitHub App's automatic analysis, so it is "not under this repository's control or observation" — and 5c510fa removed sonar-gate from both final and verify:gated, so nothing in the landing path reads it today. Restoring it is CLOUD-897's, and that row says it must not be restored unfixed.

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 Weakens: clause is groomed ahead of the work, so its CI state is not what is blocking anything. Whoever picks that up gets a fresh analysis from the push that carries the re-groomed commits.

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 details_url…&pullRequest=N is yours, …&branch=main is not — because a conclusion without it "is not a verdict about anything in particular."


Generated by Claude Code

wenzowski added a commit that referenced this pull request Sep 4, 2026
`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.
wenzowski added a commit that referenced this pull request Sep 4, 2026
`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.
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