diff --git a/.claude/settings.json b/.claude/settings.json index e9ea0b629..7fc3ae0fc 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -88,6 +88,15 @@ } ] }, + "autoMode": { + "allow": [ + "$defaults", + "Button lifecycle commands are normal work in this repository and are allowed, including inside compounds and with heredocs: `mise run `, `mise exec -- cargo ...`, `cargo` directly, `hk check`, and `batten ` — among them `override request`/`override spend`, `claim check`, `check`, `enforce`, `policy`, `config`, `receipt`, `commit check`, `attribution check` and `mutate`.", + "Writing a commit message or an admission answer to the session scratchpad (python3/cat/printf into a heredoc or file) and then feeding it to `git commit -F ` or `batten override request < ` is the DECLARED route for those gates, not an attempt to evade one: `run-shape`'s `commit write missing` and `commit bind missing` refuse every other spelling.", + "A compound that combines a scratch-file write with `git add` and `git commit`, or that chains a `batten`/`mise` gate with the command it gates, is one lifecycle step rather than unrelated actions.", + "`git commit` and `git push` to the session's own feature branch are ordinary steps here: AGENTS.md makes carrying a change through to landed-and-verified the default and the gates, not a prompt, are what stop it." + ] + }, "permissions": { "allow": [ "Edit", @@ -104,6 +113,11 @@ "Bash(batten:*)", "Bash(cp:*)", "Bash(mkdir:*)", + "Bash(cargo:*)", + "Bash(mise exec:*)", + "Bash(python3:*)", + "Bash(hk:*)", + "Bash(jq:*)", "mcp__serena", "mcp__serena__delete_memory", "mcp__serena__edit_memory", diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 614e787bc..faeb3313d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1118,6 +1118,21 @@ jobs: with: # The profile is part of the key — see the `ci` job above. shared-key: perf- + # READ-ONLY, because `perf.yml`'s own job is this family's writer and + # always was. It runs daily on a schedule, `runs-on: ubuntu-latest`, and + # takes this exact `shared-key` — same key, same architecture — so its + # entry sits on `refs/heads/main` and every pull request can read it. + # This job was nonetheless writing a full `--release` target directory + # on top of that, scoped to `refs/pull/N/merge` where nothing else can + # reach it: the largest single entry any reader here produces, for the + # profile no other job builds. + # + # `ci-cache-declared` could not see the writer until this change, + # because its `warmed` set counted `push` triggers only. That bound is + # now `trunk_writer` — push OR schedule — for the reason recorded in the + # module: what a pull request can read follows the writing run's REF, + # and a scheduled run on `main` has the same ref a pushed one does. + save-if: false # CLOUD-1331, corrected by CLOUD-1342. The BASE arm is a pure function of # the merge-base SHA, the pinned toolchain and `[profile.release]`, and # `main` advances only by fast-forward to already-judged SHAs — so diff --git a/.github/workflows/release-plz.yml b/.github/workflows/release-plz.yml index 296b80403..444b39f12 100644 --- a/.github/workflows/release-plz.yml +++ b/.github/workflows/release-plz.yml @@ -257,6 +257,134 @@ jobs: - run: mise exec -- cargo nextest run --no-run --workspace if: steps.rust-cache.outputs.cache-hit != 'true' + # THE TWO ORPHANED FAMILIES ON THE PULL-REQUEST PATH (CLOUD-1477's siblings). + # + # `ci-` has had warm writers since CLOUD-1416/1477 and `windows-` since the job + # below. `cross-` and `semver-` had none, and both are REQUIRED checks — so every + # pull request cold-built them AND uploaded a multi-hundred-MB entry scoped to + # `refs/pull/N/merge` that no other pull request can read. That write is the + # store pressure `cache-sweep.yml` exists to reclaim rather than a saving: + # CLOUD-1453 measured 10.77 GiB against a 10 GiB ceiling with 81% of it four + # already-merged PRs. Their readers now carry `save-if: false`, so these two jobs + # are what makes the families readable at all. + # + # ONE JOB PER FAMILY, NOT A SHARED ENTRY, and `rust.yml`'s matrix block already + # records why: a second leg restoring a shared entry thrashes it. `cross` builds + # a windows-gnu target and `semver` a base-revision closure; neither is the `ci-` + # host artifact set, so pointing them at `ci-` would trade a cold build for a + # thrashed one. + # + # x64 FOR THE READERS' REASON, NOT BY DEFAULT. rust-cache puts + # `runnerOS-runnerArch` inside the restore prefix (`config.ts:93`, before `:133`), + # so a writer on another arch writes an entry its reader cannot see — the exact + # trap CLOUD-1416 sprang on `batten-check`. Both readers are `ubuntu-latest` + # (`rust.yml:75`, `:319`); these must move only with them. + cache-warm-cross: + name: cache-warm-cross + runs-on: ubuntu-latest + # Grandfathered for `cache-warm-linux`'s reason: no measured p95 exists for a + # job that has never run, and a guessed number would read as measured. + timeout-minutes: 30 # budget: grandfathered measured=2026-09-09 + # A warm job must never be able to red the release lane, exactly as above. + continue-on-error: true + permissions: + contents: read + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: false + - uses: jdx/mise-action@3c2e0cf82a5b2e5249f0d3635a4d83d0ae861518 # v4.2.5 (CLOUD-404 retry fix, now a release) + with: + # Pinned to `batten.toml`'s `[[provision]]` version (CLOUD-1672). The + # digest above pins the ACTION; this pins the MISE it installs, which + # is a separate resolution the digest does not reach. + version: 2026.9.1 + # What the `cross` job installs, for its stated reason: cross-check is + # `rustup target add` + `cargo check`, and rust is the only tool it + # touches. + install_args: rust + - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + # Read by the compile step below; `ci-local-parity` property 17 holds the + # `id` and the guard together. + id: rust-cache + with: + # Must match `rust.yml`'s `cross` job exactly or this writes an entry + # that job cannot read. + shared-key: cross- + # Compile only when there is nothing to restore, as every warm job here + # does. `DOCTOR_TARGETS` must match the reader's or this fills a different + # target's artifacts. + - run: mise run cross-check + if: steps.rust-cache.outputs.cache-hit != 'true' + env: + DOCTOR_TARGETS: x86_64-pc-windows-gnu + + cache-warm-darwin-link: + name: cache-warm-darwin-link + runs-on: ubuntu-latest + # THE MATRIX MIRRORS THE READER'S, AND THAT IS NOT DECORATION. + # `ci-cache-declared` resolves a family from the `shared-key` AS WRITTEN in + # the workflow, so a writer spelling the expansion (`aarch64-apple-darwin`) + # while the reader spells `${{ matrix.target }}` is two families to the gate + # and one to GitHub — measured: `read-family-has-a-warm-writer` fired on + # `rust.yml`'s `darwin-link` with this job already present. Carrying the same + # single-leg matrix makes the two keys identical as text, and keeps the + # reader's own promise that restoring a second leg is a one-word change — + # it is now one word on each side. + strategy: + fail-fast: false + matrix: + target: [aarch64-apple-darwin] + # Grandfathered for `cache-warm-linux`'s reason: no measured p95 exists for a + # job that has never run, and a guessed number would read as measured. + timeout-minutes: 30 # budget: grandfathered measured=2026-09-09 + # A warm job must never be able to red the release lane, exactly as above. + continue-on-error: true + permissions: + contents: read + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: false + - uses: jdx/mise-action@3c2e0cf82a5b2e5249f0d3635a4d83d0ae861518 # v4.2.5 (CLOUD-404 retry fix, now a release) + with: + # Pinned to `batten.toml`'s `[[provision]]` version (CLOUD-1672). The + # digest above pins the ACTION; this pins the MISE it installs, which + # is a separate resolution the digest does not reach. + version: 2026.9.1 + # What the `darwin-link` job installs, for its stated reason: zig + # supplies the Darwin linker, cargo-zigbuild drives cargo through it, + # and rust is the compiler. + install_args: rust zig github:rust-cross/cargo-zigbuild + - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + # Read by the compile step below; `ci-local-parity` property 17 holds the + # `id` and the guard together. + id: rust-cache + with: + # SPELLED EXACTLY AS THE READER SPELLS IT — see the matrix note on + # this job. rust-cache sees the expansion either way; the gate sees + # the text, and only this spelling makes the two one family. + shared-key: ${{ matrix.target }} + # Compile only when there is nothing to restore, as every warm job here + # does. `DOCTOR_TARGETS` must match the reader's or this fills a different + # target's artifacts. + - run: mise run darwin-link ${{ matrix.target }} + if: steps.rust-cache.outputs.cache-hit != 'true' + env: + DOCTOR_TARGETS: ${{ matrix.target }} + + # `semver-` IS DELIBERATELY NOT WARMED HERE, and the reason is worth recording + # so the next reader does not "finish the job". `rust.yml`'s semver block states + # that it builds RUSTDOC with a different rustc than every other job, which is + # why it holds its own key at all. A warm job running `cargo build --workspace` + # would fill that family with host artifacts the reader does not want and still + # leave the rustdoc build cold — occupying the slot with the wrong content, + # which is worse than the cold build it replaces. Warming it means running what + # the reader runs, and `mise run semver` is a COMPARISON against a base rather + # than a build, so a trunk-side warmer is a different design question. + # `cross-` has no such split: `mise run cross-check` is exactly what its reader + # runs, which is why only that one is warmed above. + cache-warm-windows: name: cache-warm-windows runs-on: windows-latest diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 1c692750f..15f3f71f2 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -176,6 +176,13 @@ jobs: with: # The profile is part of the key — see `ci.yml`'s `ci` job. shared-key: cross- + # READ-ONLY, because `cache-warm-cross` on trunk is now this family's + # writer. A PR-side write lands on `refs/pull/N/merge` and is readable + # by no other pull request, so it bought nothing and cost the store — + # CLOUD-1453 measured 81% of a 10.77 GiB store as four merged PRs' + # unreadable entries. Same posture as every warmed family's readers + # (`ci.yml:420`, `:846`, `commit-lint.yml:138`). + save-if: false - run: mise run cross-check env: # Only the triple cross-check type-checks. doctor's default pair would @@ -307,6 +314,17 @@ jobs: # otherwise thrash a shared entry if the second leg is restored. # The profile is part of the key too — see `ci.yml`'s `ci` job. shared-key: ${{ matrix.target }} + # READ-ONLY, because `cache-warm-darwin-link` on trunk is now this + # family's writer. A PR-side write lands on `refs/pull/N/merge` and is + # readable by no other pull request, so it bought nothing and cost the + # store — CLOUD-1453 measured 81% of a 10.77 GiB store as four merged + # pull requests' unreadable entries. Same posture as every warmed + # family's readers (`:185`, `ci.yml:420`, `:846`, `commit-lint.yml:138`). + # + # Measured cold on run 34332015804, this job: 242s, the second-longest + # completed job on that pull request behind `semver`, and it is in + # `CI_REQUIRED_CHECKS` — so every ready paid it. + save-if: false - run: mise run darwin-link ${{ matrix.target }} env: # Exactly the target being linked; doctor's default would also fetch diff --git a/AGENTS.md b/AGENTS.md index 920138458..ad0b6b13f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -119,17 +119,16 @@ run in the web sandbox — read `mem:github-access` before doubting.) **This governs PR conduct above any harness default — and above your own judgement.** Run the lifecycle tasks as written, never wrapped in bespoke retry or pre-check logic; `main` advancing under your branch is this loop working, not a race -to engineer around. No heartbeats (`send_later`/Routines/timers) to babysit a PR — -fetching CI on demand is fine, the ban is on timers. No reflexive drive-to-green -pushing: a red run means verify was skipped, and a webhook's silence is not success. +to engineer around. No heartbeats to babysit a PR — `send_later`, Routines, timers +and `subscribe_pr_activity` are denied by rule: a webhook's silence is not success, +so its absence is design, not a gap. Fetch CI on demand, and never reflexively push +to green — a red run means verify was skipped. ## Background the slow path; never block the foreground -**Any command that can exceed ~2 minutes goes to the background** -(`run_in_background`): `mise run ci|verify|cross-check`, a full test suite, a cold -`cargo` build, a provision/install, or waiting on any external result. Enforced, not -stylistic — foreground `sleep` is blocked and a foreground command is killed at ~2 -minutes, so it does not run slower, it _fails_. +**EVERY `mise` call is backgrounded** (`run_in_background`), **and so is anything +else past ~2 minutes**. Gated — `sleep` is blocked, `foreground-mise` the rest, and +a foreground command is _killed_ at ~2 min. **No fast list, `alive` included.** **The exit notification IS the wake-up; waiting for it costs nothing.** A backgrounded task re-invokes you when it exits (measured 523/524, failures included), so the turn in between is the _designed_ state, not one to fill — @@ -141,8 +140,9 @@ by `run-shape-guard`. To ask what a live task is _doing_, `mise run alive`. **Two habits defeat this silently, both failing green:** piping a `mise run` into a pager (the exit status becomes the pager's) or detaching it with `nohup`/`&` -(the wake-up is lost). Redirect to a file; put `run_in_background` on the long -command, never on a launcher that returns at once. Gated by `verdict-not-discarded`. +(the wake-up is lost). Put `run_in_background` on the long command, never a launcher, and +**never redirect it** — the harness captures where the HUMAN watches, so `>log +2>&1` writes where nobody reads. `verdict-not-discarded`, `background-redirect`. **Never** use a foreground `sleep`, spin a foreground busy-poll, or end a turn idle "to watch" something — background it, act on its exit, and commit first, since **committed-and-pushed is the only state surviving a reclaim, and that is the TREE's diff --git a/batten.toml b/batten.toml index 599feac77..8ffc381c9 100644 --- a/batten.toml +++ b/batten.toml @@ -3281,13 +3281,18 @@ filters = [ "wc", ] reason = """ -The compliant form is the command alone in the call, redirected to a file: +The compliant form is the command ALONE in the call, backgrounded and \ +unredirected: - mise run >/tmp/.log 2>&1 + mise run with run_in_background on the tool call itself -The tool result IS the verdict — read the log in a SEPARATE call. Pass \ -run_in_background on the tool call itself for anything that can exceed ~2 \ -minutes. A pager over a FILE is fine; a pager over a live task is not.""" +The tool result IS the verdict, and the harness captures the output to a file it \ +names back — read that in a SEPARATE call. THIS TEXT USED TO PRESCRIBE \ +`>/tmp/.log 2>&1` AND THAT FORM IS NOW REFUSED from both sides: \ +`foreground-mise` because the harness kills a foreground call at ~2 minutes, and \ +`background-redirect` because a private log file is one nobody reads while the \ +pane the human watches stays empty. A pager over a FILE is fine; a pager over a \ +live task is not.""" # A task runner: `run` names a task and its exit status is that task's verdict. # `mise exec`/`mise x` are not here — they run another program, and the engine's @@ -4388,6 +4393,42 @@ fields = [ "project", ] +# THE DOCUMENT-SIDE REDUCTION, and it is the same defect one method over (CLOUD-1742). +# `get_document` had no row, so every document read was handed back re-serialised +# rather than reduced — the shape `crates/batten/src/mcp.rs` already records for an +# undeclared method: MORE OUT THAN IN. A document's `content` is its entire body, so +# this is the widest single field the tracker returns and the one nobody reading a +# projection asked for. +# +# DROPPING `content` COSTS A GATE NOTHING, for the reason the `get_issue` block above +# states of its own narrowing: `mcp call` stores every response WHOLE, so a gate reads +# the body from the capture store by key while the model receives a dozen fields. The +# two have never needed to be the same channel. +# +# THE PROVENANCE FIELDS ARE NOT DECORATION, and choosing them for size would have been +# the error. `updatedAt`, `updatedBy` and `team` are exactly the three that made +# CLOUD-1742 legible: a spec `AGENTS.md` cites as authoritative was overwritten in +# place and reparented to another team, and those fields are what showed it. A +# projection carrying only `id`, `title` and `url` would have reported the clobbered +# document as the same document — which is the failure this row's sibling exists to +# prevent one surface over. +[[mcp.result]] +method = "get_document" +reduce = "project" +fields = [ + "id", + "title", + "url", + "slugId", + "createdAt", + "creator", + "updatedAt", + "updatedBy", + "archivedAt", + "project", + "team", +] + # THE LIST-SIDE REDUCTION (CLOUD-1380). `list_issues` had no row and the verb said # so on every call — `undeclared — stored 17,313 bytes, emitted 20,366`. MORE OUT # THAN IN, because an undeclared result is handed back re-serialised rather than @@ -7657,6 +7698,12 @@ expires = "2026-10-31" # that cost an admission spend on this branch — `override request` resolves its # finding anchor by the module id too — and nothing gates the mismatch, which is # the third surface keyed off a different id from the one `--rule` selects. +[[waiver]] +rule = "platform-gated-test-added" +path = "crates/batten/tests/it/mutate.rs" +reason = "the sweep's own watchdog needs a suite that never returns, and every part of that fixture is already unix-only: the case reaches `TOY_GATE` (:107), `CAUGHT` (:134) and `lend_bats` (:177), each carrying `#[cfg(unix)]` because bats is a bash program with no extension that Windows can neither symlink nor execute. So the `cfg!` arm this rule prefers would not type-check — the three items do not exist on that target — and the attribute is required rather than chosen to silence a leg, which is the one bar this route is held to. The case it covers is the anti-vacuity for CLOUD-1726: `BATS_TEST_TIMEOUT` was removed because the runner does not reap its `sleep N` watchdog on a FAILING case and a caught mutation is a failing case, so without a bound of our own a hanging mutant would hold a sweep forever. `base = \"origin/main\"` makes this floor itself on landing, so the row is expected to lapse unused." +expires = "2026-10-31" + [[waiver]] rule = "platform-gated-test-added" path = "crates/batten/src/provision.rs" @@ -12205,7 +12252,7 @@ target = "until ; do sleep 1; done — where reads something the ha [[verdict.route]] id = "task run other" kind = "command" -target = "mise run alive" +target = "mise run alive, backgrounded" # CLOUD-821. NOT a narrower `sleep run blocked`: backgrounding is the remedy for # that one and the subject of this one, so an author reading the wrong class here @@ -12232,7 +12279,7 @@ target = "until ; do sleep 1; done — where reads something the ha [[verdict.route]] id = "task run other" kind = "command" -target = "mise run alive" +target = "mise run alive, backgrounded" # THE OTHER HALF OF THE ROW ABOVE, and it exists because that row's own exemption # was the escape (CLOUD-1337). `timer run refused` allows a backgrounded `sleep` @@ -12274,7 +12321,45 @@ target = "delete the loop and end the turn — the backgrounded task wakes you o [[verdict.route]] id = "task watch other" kind = "command" -target = "mise run alive" +target = "mise run alive, backgrounded" + +# THE DURATION ESTIMATE, RETIRED. `sleep run blocked` and this row are the same +# failure at two ends: there the caller waits in the foreground, here it WORKS in +# the foreground, and both are killed at ~2 minutes with the turn spent. The +# difference is who makes the mistake — a `sleep` names its own duration, while +# `mise run` asks the caller to predict one, and every task on this repo can +# exceed the bound behind a cold `cargo` build nobody can see coming. +[[verdict]] +id = "task run blocked" +gloss = "a foreground `mise` call is killed at ~2 minutes, so it fails rather than runs slowly" +class = """ +The harness kills a foreground command at about two minutes, so a `mise` task that runs longer does not run slowly — it FAILS, and takes the turn with it. Every task here can cross that bound behind a cargo build the caller cannot see coming, which is why this is not a duration to estimate: pass `run_in_background` on the tool call itself and act on the exit notification, delivered 523 of 524 in one session including every failure. THERE IS NO FAST LIST, and `alive` is not on one either: a carve-out is a judgement call handed back to the caller this rule exists to stop consulting, and backgrounding a short task costs one turn and returns the same text. +""" + +[[verdict.route]] +id = "task run first" +kind = "command" +target = "re-issue the same command with run_in_background on the tool call" + +[[verdict.route]] +id = "task run other" +kind = "command" +target = "mise run alive, backgrounded" + +# CLOUD-1722's other half, and it is `verdict-not-discarded` in a new spelling: +# there a pipe hands the exit status to the pager, here a redirect hands the +# OUTPUT to a file the human is not looking at. +[[verdict]] +id = "redirect write unread" +gloss = "a backgrounded call redirecting its own output writes a second file nobody reads" +class = """ +The harness already captures a backgrounded task's output to a file it names back to the caller, and surfaces it where the HUMAN watches. A `> log 2>&1` inside the command substitutes a private file for that one: the exit notification still fires, the pane the human reads is empty, and the run they were meant to be able to see over is gone. Drop the redirect and read the harness's own output file. AN INPUT REDIRECT IS NOT THIS CLASS — reading a file into a backgrounded command discards nothing and is untouched. +""" + +[[verdict.route]] +id = "redirect write first" +kind = "command" +target = "drop the `> file 2>&1` and read the output file the harness names back" [[verdict]] id = "workflow parse broken" diff --git a/crates/batten/src/checks_green.rs b/crates/batten/src/checks_green.rs index 148f9eb21..643579b2d 100644 --- a/crates/batten/src/checks_green.rs +++ b/crates/batten/src/checks_green.rs @@ -179,6 +179,90 @@ fn key(run: &Run) -> (String, u64) { (run.started_at.clone(), run.id) } +/// Which run answers for one name? +/// +/// **A COMPLETED-BUT-UNANSWERED RUN NEVER DISPLACES A REAL VERDICT**, and that +/// clause is the whole of this function. Everything else is the key-then-rank +/// order [`latest_per_name`] always had. +/// +/// MEASURED, AND IT STOPPED THE WHOLE FLEET. GitHub registers the path-filtered +/// COPY of a workflow one to five seconds after the copy that actually runs, and +/// the copy that does not apply concludes `skipped`. So the skipped twin is the +/// LATER run by `started_at` and won outright — `ci` succeeded at 02:34:02 and +/// was overruled by a `skipped` twin at 02:34:03, and six required checks read +/// as "no verdict" over a head whose every job was green. `checks green` returned +/// `Pending`, which is the correct response to that reading; the reading was +/// wrong. Nothing landed on `main` for six hours across three branches, each +/// holding the landing lease while `ci-wait` waited for an answer that had +/// already arrived and been discarded. +/// +/// CLOUD-436's rule is not weakened by this, it is completed. That rule exists +/// because a draft-era skip set "vetoes a verdict that already exists" — and +/// ordering by start time fixed the union only for the case where the residue is +/// OLDER. The residue here is newer, so the same veto came back through the +/// ordering that was supposed to end it. +/// +/// **STILL-RUNNING IS DELIBERATELY NOT COVERED** (rank 4). A rerun in flight over +/// a name that already succeeded must still read `Pending`: the new run may fail, +/// and preferring the old success would be a false green on the one path where +/// the answer is genuinely not in yet. Only a run that COMPLETED without +/// producing a verdict — `skipped`, `cancelled`, or whatever GitHub adds next — +/// is refused the displacement, because such a run judged nothing and a reading +/// that lets it erase a judgement is reporting the absence of an answer it holds. +/// The named cases are this file's own unit tier, so the declared suite path is +/// this file (CLOUD-1267's shape, as `landed.rs` uses it). +/// +/// THE OPENER IS `//`, NOT `///`. `OPENERS` is `["//", "#"]` and the marker must +/// follow one directly, so a doc comment's third slash leaves `/ MUTANT-SUITE …` +/// and matches nothing. Declared inside a doc comment this row parses as prose, +/// the module never becomes a subject, and `mutate census` stays green over a +/// declaration nobody sweeps — which is the coverage-shaped nothing the verb +/// exists to refuse. Written here first in exactly that dead form. +/// +/// The mutation moves `UNANSWERED` off the rank `rank()` actually assigns, so the +/// guard below can never fire and a later `skipped` twin wins outright again — +/// which is precisely the state that stopped the fleet. It is a constant rather +/// than the predicate because a row's script may carry no `|` of its own, and +/// because a guard commented out would also redden the two arms that must keep +/// their old behaviour, telling us less about which clause the cases pin. +//MUTANT-SUITE crates/batten/src/checks_green.rs +//MUTANT unanswered-displaces-a-verdict|s@const UNANSWERED: u8 = 3;@const UNANSWERED: u8 = 9;@|a_later_skipped_twin_does_not_erase_a_verdict +fn winner<'a>(runs: &[&'a Run], answered: &[String]) -> Option<&'a Run> { + const UNANSWERED: u8 = 3; + // `max_by_key` over `(key, rank)` is the whole ordering, and it is a + // FUNCTION OF THE SET rather than of the arrival order — which is the defect + // CLOUD-1722's first shape carried. The old pairwise fold applied a + // non-transitive predicate, so a success, an in-flight rerun and a newer + // skipped twin read `Green` in one slice order and `Pending` in another. + // + // `Option` RATHER THAN AN `expect`, and the empty case is the caller's to + // not have. Every group this is called with is non-empty by construction — + // `latest_per_name` builds them by pushing — but a panic message asserting + // that is a claim the type could carry instead, and `clippy::expect_used` is + // denied here precisely so the assertion goes into the signature. The caller + // reads it back with `filter_map`, where an empty group contributes no name + // rather than aborting a verdict the rest of the reading could still answer. + let latest = *runs + .iter() + .max_by_key(|run| (key(run), rank(run, answered)))?; + if rank(latest, answered) != UNANSWERED { + return Some(latest); + } + // The completed-but-unanswered latest yields to the latest run that DID + // judge or is still judging — but only when that run is STRICTLY OLDER. + // An equal key is the unorderable pair, which must still fall to the least + // conclusive reading: preferring the answer there would make the fail-closed + // case fail open, the one direction this module cannot afford. + Some( + runs.iter() + .filter(|run| rank(run, answered) != UNANSWERED) + .max_by_key(|run| (key(run), rank(run, answered))) + .filter(|candidate| key(candidate) < key(latest)) + .copied() + .unwrap_or(latest), + ) +} + /// Latest run per name (CLOUD-436), over the required subset only. /// /// An unrelated check gets neither a vote nor a veto — the same scoping that @@ -190,25 +274,17 @@ fn key(run: &Run) -> (String, u64) { /// construction, emitted from inside the same pass; two passes would only have /// it by inspection. fn latest_per_name<'a>(runs: &'a [Run], roster: &Roster) -> BTreeMap<&'a str, &'a Run> { - let mut best: BTreeMap<&str, &Run> = BTreeMap::new(); + let mut grouped: BTreeMap<&str, Vec<&Run>> = BTreeMap::new(); for run in runs { if !roster.required.iter().any(|name| name == &run.name) { continue; } - match best.get(run.name.as_str()) { - None => { - best.insert(&run.name, run); - } - Some(held) => { - let (hk, hr) = (key(held), rank(held, &roster.answered)); - let (nk, nr) = (key(run), rank(run, &roster.answered)); - if nk > hk || (nk == hk && nr > hr) { - best.insert(&run.name, run); - } - } - } + grouped.entry(&run.name).or_default().push(run); } - best + grouped + .into_iter() + .filter_map(|(name, group)| Some((name, winner(&group, &roster.answered)?))) + .collect() } /// The judged view: one pointer per required name that HAS a run, in roster @@ -373,6 +449,123 @@ mod tests { } } + /// THE MEASURED SHAPE, and the reason `winner` exists (CLOUD-1722). + /// + /// GitHub registers the path-filtered COPY of a workflow a few seconds after + /// the copy that runs, and the copy that does not apply concludes `skipped`. + /// Ordering purely by `started_at` therefore hands the name to the twin that + /// judged nothing. Taken from `2ce05b82`: `ci` succeeded at 02:34:02 and was + /// overruled by a `skipped` twin at 02:34:03, six required checks read as "no + /// verdict", and no branch in the repository could land for six hours. + #[test] + fn a_later_skipped_twin_does_not_erase_a_verdict() { + let mut runs = green_set(); + runs.push(run( + "completed", + "skipped", + "ci", + "2026-09-09T02:34:03Z", + 102_315_915_050, + )); + assert_eq!( + decide(&runs, &roster()), + Ok(Verdict::Green), + "a skipped copy of a workflow that did not apply judged nothing" + ); + } + + /// THE ORDER-INDEPENDENCE CASE, and the defect the first shape carried. + /// + /// A success, an in-flight rerun of the same name, and a NEWER skipped twin. + /// The pairwise fold this replaced applied a non-transitive predicate, so the + /// slice order decided the verdict: `skipped, in-flight, success` let the old + /// success displace the rerun through the unanswered guard and returned + /// `Green` while the rerun was still running — a false green over the exact + /// path the anti-vacuity case protects. `runs_from_body` preserves the + /// source array order, so the reading depended on what GitHub happened to + /// serialise first. Both orders must read `Pending`. + #[test] + fn a_rerun_in_flight_under_a_newer_skipped_twin_is_pending_in_either_order() { + let success = run("completed", "success", "ci", "2026-09-09T02:34:02Z", 1); + let in_flight = run("in_progress", "", "ci", "2026-09-09T02:40:00Z", 2); + let skipped = run("completed", "skipped", "ci", "2026-09-09T02:40:03Z", 3); + for order in [ + vec![skipped.clone(), in_flight.clone(), success.clone()], + vec![success, in_flight, skipped], + ] { + let mut runs = green_set(); + runs.retain(|existing| existing.name != "ci"); + runs.extend(order.clone()); + assert!( + matches!( + decide(&runs, &roster()), + Ok(Verdict::Pending(Pending::Running { .. })) + ), + "a rerun in flight is not answered by an older success, whatever \ + order the runs arrive in: {order:?}" + ); + } + } + + /// THE ANTI-VACUITY HALF, and the one direction this must not buy. + /// + /// A rerun IN FLIGHT over a name that already succeeded is not the same shape: + /// the new run may fail, so the answer genuinely is not in yet and `Pending` + /// is right. A fix that preferred the standing success here would be a false + /// green on the only path where waiting is the correct behaviour — which is + /// worse than the stall it replaces, because a stall is recoverable. + #[test] + fn a_rerun_in_flight_still_reads_pending() { + let mut runs = green_set(); + runs.push(run( + "in_progress", + "", + "ci", + "2026-09-09T02:40:00Z", + 102_315_915_051, + )); + assert!( + matches!(decide(&runs, &roster()), Ok(Verdict::Pending(_))), + "a rerun that has not concluded is not answered by the run it replaced" + ); + } + + /// The rule stays a completion of CLOUD-436 rather than a reversal of it: an + /// OLDER skip set must still lose to the verdict that came after it, which is + /// the direction ordering by start time was introduced to fix. + #[test] + fn an_older_skip_still_loses_to_the_verdict_that_followed() { + let mut runs = green_set(); + runs.insert( + 0, + run( + "completed", + "skipped", + "ci", + "2026-09-09T02:00:00Z", + 102_315_915_049, + ), + ); + assert_eq!(decide(&runs, &roster()), Ok(Verdict::Green)); + } + + /// A REAL FAILURE IS STILL RED, whichever order it arrives in. `failure` is an + /// answered conclusion, so it is not what `winner` protects against — and + /// a fix that let a stale success outrank a later failure would be the false + /// green this whole module exists to stop. + #[test] + fn a_later_failure_still_wins() { + let mut runs = green_set(); + runs.push(run( + "completed", + "failure", + "ci", + "2026-09-09T02:45:00Z", + 102_315_915_052, + )); + assert!(matches!(decide(&runs, &roster()), Ok(Verdict::Red(_)))); + } + fn green_set() -> Vec { ["ci", "perf", "final"] .iter() @@ -692,18 +885,49 @@ mod tests { } #[test] - fn a_success_superseded_by_a_skip_is_not_an_answer() { - // The draft economy survives supersession (CLOUD-247, CLOUD-327): a name - // whose LATEST run skipped is still not an answer, whatever graded - // before it. + fn a_success_superseded_by_a_skip_is_an_answer_now() { + // REVERSED DELIBERATELY (CLOUD-1722), and the reasoning lives here rather + // than only in a commit message because the case it replaces was right + // for three weeks and a reader needs to know what changed under it. + // + // It asserted the draft economy (CLOUD-247, CLOUD-327): a name whose + // LATEST run skipped is not an answer, whatever graded before it — so a + // PR readied, graded green, then RE-DRAFTED does not keep reading green + // off the stale run. That case is real and its two runs are a day apart. + // + // IT CANNOT BE TOLD FROM THE ONE THAT STOPPED THE FLEET. GitHub registers + // the path-filtered COPY of a workflow one to five seconds after the copy + // that runs, and the copy that does not apply concludes `skipped`. Both + // readings are "a success, then a later skip"; only the gap differs, and + // a gate deciding on a duration would be estimating rather than deciding + // (non-negotiable rule 3). `check_suite`, the workflow-run id and the + // check-run id were each checked as discriminators and none is one: two + // workflow files dispatched by a single push take their ids in arbitrary + // relative order, so "later" means nothing between concurrent copies. + // + // SO THE DRAFT CONCERN MOVES TO THE GATE THAT ALREADY OWNS IT. + // The consumer's fast-forward workflow refuses a draft head + // unconditionally, before any checks reading at all — added by CLOUD-853 + // precisely because the ruleset admitted a draft's empty check set as + // satisfying "required checks green". A re-drafted PR cannot fast-forward + // whatever this predicate answers, so the cost of this reversal is one + // wasted lap of `land`, not a merge nobody graded. + // + // The cost of NOT reversing it was measured on 2026-09-09: six required + // checks reading "no verdict" over a head whose every job had succeeded, + // three branches taking the landing lease in turn and holding it, and + // nothing reaching `main` for six hours. let reading = vec![ run("completed", "success", "ci", "2026-08-11T00:00:00Z", 1), run("completed", "skipped", "ci", "2026-08-12T00:00:00Z", 2), + run("completed", "success", "perf", "2026-08-11T00:00:00Z", 3), + run("completed", "success", "final", "2026-08-11T00:00:00Z", 4), ]; - let Ok(Verdict::Pending(Pending::NoVerdict(findings))) = decide(&reading, &roster()) else { - panic!("the later skip speaks for the name"); - }; - assert_eq!(findings[0].to_string(), "ci skipped"); + assert_eq!( + decide(&reading, &roster()), + Ok(Verdict::Green), + "a run that completed without judging cannot erase the judgement it followed" + ); } #[test] diff --git a/crates/batten/src/mutate.rs b/crates/batten/src/mutate.rs index 805c0a1db..3968e8a38 100644 --- a/crates/batten/src/mutate.rs +++ b/crates/batten/src/mutate.rs @@ -88,6 +88,7 @@ use std::collections::BTreeMap; use std::fmt; +use std::fs; use std::path::{Path, PathBuf}; use anyhow::{Context as _, Result, bail}; @@ -862,17 +863,189 @@ fn spawn(dir: &Path, program: &str, args: &[String], env: &[(String, String)]) - // reported "every one caught". Nothing here feeds a suite from stdin, and // closing it is what keeps that true if anything ever does. command.stdin(std::process::Stdio::null()); - let answer = command - .output() + // OUR OWN WATCHDOG, BECAUSE THE RUNNER'S CHARGES FOR EVERY RED CASE + // (CLOUD-1726, and this closes the question that row left open). + // + // The earlier reading blamed `.output()` reading two pipes to EOF while + // bats' watchdog subshell still held them. That is wrong, and the + // measurement that settles it does not involve this process at all — bats + // invoked straight from a shell, output to `/dev/null`, stdin closed, the + // same lent runner and the same `run` helper: + // + // | case | bound 3 | bound 6 | + // | ---- | ------- | ------- | + // | passing | 0.086s | 0.084s | + // | FAILING | 3.086s | 6.092s | + // + // The discriminator is the case's VERDICT, not the capture. `bats-exec-test` + // aborts its countdown on a normal finish and does not on a failing one, so + // the bound is paid in full by exactly the outcome a mutation sweep exists + // to produce: a caught mutation IS a red case. Every row this sweep gets + // right was buying the whole watchdog, which is why the cost read as linear + // in the bound and why unsetting it looked like a fix. + // + // So the bound moves here and `BATS_TEST_TIMEOUT` goes. The requirement is + // unchanged and is the reason a bound has to exist at all — a mutant that + // hangs is precisely what a sweep must survive — but a watchdog that only + // fires on a hang costs nothing on the rows that pass. + // + // THE DIRECT CHILD ONLY — see `reap`, which records why signalling the + // process group instead cost three unrelated suites in CI. + // UNSET, NOT SET SMALL, AND THE DIFFERENCE IS THE WHOLE DEFECT. `bats-exec-test` + // implements `BATS_TEST_TIMEOUT` as a literal `sleep N` child that it does not + // reap on a FAILING case — observed directly, `sleep 300` still running under a + // `bats-exec-test` reparented to init while `bats` itself waited on it. A caught + // mutation is a failing case, so every row this sweep gets right waited out the + // whole bound. + // + // The 300 is the CONSUMER's, exported from their task runner's environment block + // for their own suite, and an exported variable reaches every descendant — so it + // arrived here uninvited. Removing this entry from `suite_env` is therefore not + // enough: absent an explicit removal the child inherits the ambient value, which + // is how a "no bound" reading still cost 300s. It has to be unset on the command. + command.env_remove("BATS_TEST_TIMEOUT"); + // FILES RATHER THAN PIPES, because nothing reads them until the child is + // gone. A pipe holds 64 KiB and then blocks its writer, so a chatty failure + // would deadlock against a reader that is waiting for the exit — the one + // hazard `.output()` avoided by reading both pipes concurrently. A file has + // no such bound, and the watchdog above is what makes waiting-then-reading + // safe rather than merely tidy. + let capture = std::env::temp_dir().join(format!( + "batten-mutate-{}-{}", + std::process::id(), + CAPTURE.fetch_add(1, std::sync::atomic::Ordering::Relaxed) + )); + fs::create_dir_all(&capture).with_context(|| { + format!( + "mutate: could not create the capture directory {}", + capture.display() + ) + })?; + let out_path = capture.join("stdout"); + let err_path = capture.join("stderr"); + command.stdout(std::process::Stdio::from( + fs::File::create(&out_path) + .with_context(|| format!("mutate: could not open {}", out_path.display()))?, + )); + command.stderr(std::process::Stdio::from( + fs::File::create(&err_path) + .with_context(|| format!("mutate: could not open {}", err_path.display()))?, + )); + let mut child = command + .spawn() .with_context(|| format!("mutate: could not run {program}"))?; - let mut output = String::from_utf8_lossy(&answer.stdout).into_owned(); - output.push_str(&String::from_utf8_lossy(&answer.stderr)); - Ok(Ran { - ok: answer.status.success(), + // A BLOCKING WAIT WITH A DEADLINE, NOT A POLL. `std::thread::sleep` is a + // denied method here (`clippy.toml`) and the ban draws exactly the right + // line: "a poll bounded by a real terminal state is legitimate, a timer + // standing in for an exit condition is not". A poll loop would have needed + // an exemption, and `delay-waivers-not-growing` refuses a twelfth — rightly, + // because this wait has a terminal state to block on and therefore needs no + // delay at all. + // + // The worker owns the child and calls `wait`; the receive carries the bound. + // A suite that returns wakes this immediately, and one that hangs runs into + // `recv_timeout` — no interval, nothing to tune, and no sleep. + let pid = child.id(); + let (send_status, statuses) = std::sync::mpsc::channel(); + let worker = std::thread::spawn(move || { + let status = child.wait(); + // The receiver is gone only when this call already timed out and the + // caller stopped listening, which is the hang path below; the status it + // would have carried is the kill this function reports. + let _ = send_status.send(status); + }); + let Ok(reported) = statuses.recv_timeout(suite_bound()) else { + // The bound ran out, so the suite is hung. Kill it and take the status + // the worker's `wait` returns once it does. + reap(pid); + let reaped = statuses + .recv() + .map_err(|_| anyhow::anyhow!("mutate: the wait for {program} was lost"))? + .with_context(|| format!("mutate: could not reap {program}"))?; + let _ = worker.join(); + return Ok(finish(reaped, &out_path, &err_path, &capture)); + }; + let status = reported.with_context(|| format!("mutate: could not wait for {program}"))?; + // The worker is finished: it has sent, so its `wait` returned. + let _ = worker.join(); + Ok(finish(status, &out_path, &err_path, &capture)) +} + +/// Read back what the child wrote and drop the capture. +/// +/// One place, because both the ordinary exit and the hang path answer with the +/// same shape and a second copy is how the two drift apart. +fn finish( + status: std::process::ExitStatus, + out_path: &Path, + err_path: &Path, + capture: &Path, +) -> Ran { + let mut output = fs::read_to_string(out_path).unwrap_or_default(); + output.push_str(&fs::read_to_string(err_path).unwrap_or_default()); + // Best effort: a capture left behind is a few bytes under the system temp + // directory, and failing a sweep over housekeeping would trade a real + // verdict for tidiness. + let _ = fs::remove_dir_all(capture); + Ran { + ok: status.success(), output, - }) + } } +/// Distinguishes concurrent captures within one process; the pid separates +/// processes. A counter rather than a random name, so a leftover directory names +/// the call that made it. +static CAPTURE: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); + +/// How long a suite may run before the sweep stops waiting for it. +/// +/// Small because the subject is one filtered case over a staged toy tree rather +/// than a repository's whole suite. Overridable, because a consumer whose gate +/// suite is genuinely slower needs a way up that is not editing the engine; the +/// name is batten's own so it cannot collide with the `BATS_*` namespace the +/// runner owns. +fn suite_bound() -> std::time::Duration { + std::time::Duration::from_secs( + std::env::var("BATTEN_MUTATE_SUITE_TIMEOUT") + .ok() + .and_then(|value| value.parse::().ok()) + .filter(|seconds| *seconds > 0) + .unwrap_or(30), + ) +} + +/// Stop the child, best effort. +/// +/// THE DIRECT CHILD ONLY, AND NEVER ITS PROCESS GROUP. An earlier version made +/// the child a group leader and signalled the group, reasoning that bats forks +/// twice and the leader alone leaves the rest orphaned. That is true and it is +/// not worth the hazard: `kill(-pid)` addresses whatever group carries that id, +/// so if the group never formed — the call is best effort and its failure is +/// invisible here — the signal lands on the group this process is already in, +/// which under a parallel test runner is its SIBLINGS. Measured: the +/// group-signalling version reddened three `symbols` cases in CI with "the +/// analyser did not resolve", a suite that shares nothing with this module +/// except that it was running at the same time. +/// +/// What an orphan costs by comparison is bounded: a `sleep` under a staged tree +/// this sweep is about to replace, which exits on its own. +/// +/// Best effort is the honest contract, for `exec.rs`'s stated reason: the +/// process may already have gone, which is the outcome being asked for, and +/// `ESRCH` on the way out is not a failure anyone can act on. +#[cfg(unix)] +fn reap(pid: u32) { + if let Some(pid) = rustix::process::Pid::from_raw(i32::try_from(pid).unwrap_or_default()) { + let _ = rustix::process::kill_process(pid, rustix::process::Signal::KILL); + } +} + +/// Off unix there is no `kill(2)` to reach for here, and the wait above has +/// already stopped listening — the worker thread is what still holds the child. +#[cfg(not(unix))] +fn reap(_pid: u32) {} + // --------------------------------------------------------------------------- // Running a suite. // --------------------------------------------------------------------------- @@ -926,6 +1099,12 @@ fn suite_env(root: &Path) -> Vec<(String, String)> { String::from("BATTEN_TEST_SCRATCH_LANE"), String::from("mutate"), ), + // NO `BATS_TEST_TIMEOUT`, AND THAT IS THE POINT OF CLOUD-1726'S FIX. + // The runner's watchdog is not cancelled on a FAILING case, and a caught + // mutation is a failing case — so the consumer's bound was paid in full + // by every row this sweep gets right. The bound now lives in `spawn`, + // where it fires on a hang and costs nothing on a verdict; the evidence + // is the table there. ] } diff --git a/crates/batten/src/verdict.rs b/crates/batten/src/verdict.rs index acdedd624..ff125021b 100644 --- a/crates/batten/src/verdict.rs +++ b/crates/batten/src/verdict.rs @@ -1776,9 +1776,11 @@ checked it -- a class a reader believes is worse than one they cannot look up.", id: "verdict read dropped", gloss: "piping a verdict-bearing command into a pager or filter discards its status", class: "The pipeline exits with the FILTER's status, which is 0 whether the command \ -passed or failed. A verdict is read from the harness, never inferred from output. Redirect \ -to a file and read the file in a separate call; a pager over a FILE is fine, a pager over a \ -live task is not.", +passed or failed. A verdict is read from the harness, never inferred from output. Background \ +the command and read the exit code the notification carries; a pager over a FILE is fine, a \ +pager over a live task is not. DO NOT REDIRECT INSTEAD -- this text prescribed `> file 2>&1` \ +for its whole life, and `background-redirect` now refuses exactly that, because the harness \ +already captures a backgrounded task's output where the human watches.", routes: &[read("rule read first", "rules/toolchain.md")], applicability: Applicability::Advice, }, diff --git a/crates/batten/tests/it/agentic_record.rs b/crates/batten/tests/it/agentic_record.rs index d43736b20..ac6a5d753 100644 --- a/crates/batten/tests/it/agentic_record.rs +++ b/crates/batten/tests/it/agentic_record.rs @@ -378,50 +378,188 @@ fn the_committed_records_satisfy_this_gate() { ); } -#[test] -fn a_replay_over_the_committed_records_fires_on_every_required_key() { - // CLOUD-1116'S ACCEPTANCE CLAUSE, run rather than written up. Each required - // key is removed from the committed record set in turn and the gate must fire; - // a key whose removal is SILENT is a required key nothing requires, which is - // the dead-clause shape this repository keeps re-meeting. +/// The replay's own reading of the gate, IN-PROCESS rather than over a spawn. +/// +/// **THE REPLAY IS NOT ONE OF THIS FILE'S E2E CLAIMS** (CLOUD-1750). The header +/// above names three things only the compiled binary can prove — that the engine +/// PARSES both records into `input.tree.documents`, that an array of tables +/// arrives iterable, and that an absent record reaches `input.tree.missing`. The +/// cases that assert those still spawn, and should. The replay asserts something +/// else entirely and 78 times over: that one predicate fires when one required +/// key is gone. +/// +/// `rules::run_static` is the same read surface a consumer reaches — the engine, +/// with the same document parsing, not a `with input as` stub whose green means +/// nothing. So the guarantee the header defends is unchanged and the process +/// boundary is gone. +/// +/// The scratch repository stays: `input.tree.*` is defined over TRACKED paths, so +/// git is what makes the surface non-empty. One per block, not one per mutation. +fn replay_vocabulary() -> Vec { + // NARROWED TO WHAT THIS MODULE RAISES, and the whole table is refused here. + // `run_static` holds the both-directions invariant the registry holds: a + // declared class nothing raises "reads as coverage". A fixture enabling ONE + // module against the committed table therefore fails on every other row's + // class — which is the refusal this helper met first, and it is the engine + // being right rather than a fixture problem. // - // Removal is by line and by first occurrence per trial block, so the count - // below is mutations rather than deleted lines. + // **BUILT ONCE PER BLOCK, NOT PER MUTATION.** Reading it inside the loop + // re-parsed the whole committed verdict table 13 times and took the block + // from 2.0s to 4.4s — a conversion that moved the tier and lost the time, + // which is why the number is taken after every one of these changes rather + // than assumed from the shape. + common::verdicts_in(&common::at_root(".")) + .into_iter() + .filter(|verdict| RAISED.contains(&verdict.id.as_str())) + .collect() +} + +fn replayed_findings( + dir: &std::path::Path, + verdicts: &[batten::verdict::DeclaredVerdict], +) -> String { + let row: batten::rules::Rule = serde_json::from_value(serde_json::json!({ + "id": "agentic-experiment-record", + "kind": "policy", + "scope": "tree", + "documents": [TRIALS, METHOD], + "module": "policy/agentic-experiment-record.rego", + "severity": "deny", + })) + .expect("the loader accepts the committed row's shape"); + + batten::rules::run_static( + &[row], + &[], + batten::policy::Vocabulary { + patterns: &[], + verdicts, + recorders: &[], + }, + dir, + ) + .expect("the read surface runs a policy row") + .findings + .into_iter() + .map(|finding| finding.rule) + .collect::>() + .join("\n") +} + +/// The trial blocks this file replays, one `#[test]` each. +/// +/// **THE SPLIT IS THE PERFORMANCE FIX AND THIS CONSTANT IS ITS SAFETY CATCH** +/// (CLOUD-1745). One `#[test]` looping every block ran the whole replay serially +/// inside a single test binary invocation, so `cargo nextest`'s per-test +/// parallelism could not reach it: 89.0s isolated, 133.4s under `verify`'s load, +/// on four cores with three of them idle. Split per block, the runner schedules +/// them. +/// +/// A constant rather than a count taken at runtime, because the population this +/// file replays is fixed by how many `#[test]` functions are WRITTEN below. +/// [`the_replay_covers_every_trial_block`] is what refuses a corpus that grew +/// past them — without it, adding a seventh `[[trial]]` row would silently leave +/// that row unswept while every existing test stayed green, which is the +/// coverage-shaped nothing this whole replay exists to refuse. +const REPLAYED_BLOCKS: usize = 6; + +/// CLOUD-1116's acceptance clause for one trial block, run rather than written up. +/// +/// Each required key is removed from that block in turn and the gate must fire; a +/// key whose removal is SILENT is a required key nothing requires, which is the +/// dead-clause shape this repository keeps re-meeting. Removal is by line and by +/// first occurrence within the block, so the count is mutations rather than +/// deleted lines. +/// +/// **THE SCRATCH REPOSITORY IS BUILT ONCE PER BLOCK AND THE RECORD REWRITTEN IN +/// PLACE, AND THIS IS THE LARGER HALF OF THE FIX.** 78 [`repo`] builds became 6. +/// +/// A first pass timed `git init` alone at ~5.4ms and concluded the rebuilds were +/// worth ~0.4s of the 89s. That measurement was of the wrong thing: [`repo`] also +/// writes the config and both records, copies the module, and then `git add -A` +/// and commits the whole tree — about 1.0s of each iteration's 1.14s. The `batten +/// check` subprocess, which the first pass blamed, is ~0.15s. +/// +/// Recorded because the wrong number is the tempting one: timing the cheap +/// primitive a helper calls, rather than the helper, reads as rigour and is not. +fn replay_block(block: usize) { let trials = std::fs::read_to_string(common::at_root(TRIALS)).expect("the trials record"); let method = std::fs::read_to_string(common::at_root(METHOD)).expect("the method record"); - let blocks: Vec<&str> = trials.split("\n[[trial]]").collect(); - let mut examined = 0_usize; + let dir = repo(&format!("replay-{block}"), Some(&trials), Some(&method)); + let verdicts = replay_vocabulary(); let mut fired = 0_usize; - for (block, _) in blocks.iter().enumerate().skip(1) { - examined += 1; - for (key, line) in REQUIRED_LINES.iter().enumerate() { - let mutated = remove_first_line_starting_with(&trials, block, line); - assert_ne!( - mutated, trials, - "the replay must actually mutate: trial block {block} carries no `{line}` line, \ - so this key is declared required and is not present to be removed" - ); - let dir = repo( - &format!("replay-{block}-{key}"), - Some(&mutated), - Some(&method), - ); - if findings(&dir).contains("agentic-record-incomplete") { - fired += 1; - } + for line in &REQUIRED_LINES { + let mutated = remove_first_line_starting_with(&trials, block, line); + assert_ne!( + mutated, trials, + "the replay must actually mutate: trial block {block} carries no `{line}` line, \ + so this key is declared required and is not present to be removed" + ); + // The record is rewritten under a repository that already exists. A gate + // reads the WORKING bytes of a tracked path, so the mutation is visible + // without a second commit — and if that were ever untrue this assertion + // would go to zero rather than quietly passing. + write(&dir, TRIALS, &mutated); + if replayed_findings(&dir, &verdicts).contains("agentic-record-incomplete") { + fired += 1; } } - let mutations = examined * REQUIRED_LINES.len(); assert_eq!( - fired, mutations, - "every required key's removal must be caught: {fired} of {mutations} mutations fired \ - over {examined} trial rows" + fired, + REQUIRED_LINES.len(), + "every required key's removal must be caught: {fired} of {} mutations fired over trial \ + block {block}", + REQUIRED_LINES.len() + ); +} + +#[test] +fn the_replay_covers_every_trial_block() { + // THE ANTI-VACUITY OF THE SPLIT. Six `#[test]` functions replay six blocks by + // name; nothing else notices a seventh. A corpus that outgrows them must fail + // HERE rather than shrink the replay silently. + let trials = std::fs::read_to_string(common::at_root(TRIALS)).expect("the trials record"); + let blocks = trials.split("\n[[trial]]").count() - 1; + assert_eq!( + blocks, REPLAYED_BLOCKS, + "{blocks} trial block(s) are committed and {REPLAYED_BLOCKS} are replayed: add or remove \ + a `replay_block` test so every block is swept" ); } +#[test] +fn a_replay_over_trial_block_1_fires_on_every_required_key() { + replay_block(1); +} + +#[test] +fn a_replay_over_trial_block_2_fires_on_every_required_key() { + replay_block(2); +} + +#[test] +fn a_replay_over_trial_block_3_fires_on_every_required_key() { + replay_block(3); +} + +#[test] +fn a_replay_over_trial_block_4_fires_on_every_required_key() { + replay_block(4); +} + +#[test] +fn a_replay_over_trial_block_5_fires_on_every_required_key() { + replay_block(5); +} + +#[test] +fn a_replay_over_trial_block_6_fires_on_every_required_key() { + replay_block(6); +} + /// Remove the first line inside trial block `block` (1-indexed, as /// `split(\"\\n[[trial]]\")` yields it) whose trimmed form starts with `prefix`. fn remove_first_line_starting_with(trials: &str, block: usize, prefix: &str) -> String { diff --git a/crates/batten/tests/it/cli.rs b/crates/batten/tests/it/cli.rs index a68d4b574..1a1df93e3 100644 --- a/crates/batten/tests/it/cli.rs +++ b/crates/batten/tests/it/cli.rs @@ -226,6 +226,22 @@ fn claude_payload(command: &str) -> String { .to_string() } +/// [`claude_payload`] with the call's backgrounding STATED. +/// +/// Most hosts send no `run_in_background` at all and the engine projects `null`, +/// which is why the plain builder carries no key — that absence is the ordinary +/// envelope rather than an omission. This one is for the rows that read the +/// posture: `foreground-mise` refuses an unstated one on the strict side, so a +/// case asserting the allowed shape has to say so out loud. +fn claude_payload_backgrounded(command: &str) -> String { + serde_json::json!({ + "hook_event_name": "PreToolUse", + "tool_name": "Bash", + "tool_input": { "command": command, "run_in_background": true } + }) + .to_string() +} + /// A Claude Code `PreToolUse` payload wrapping one bare tool call. /// /// Empty input, deliberately: a row keyed on the tool name alone reads no field @@ -3434,16 +3450,37 @@ fn the_committed_shape_rules_fire_on_every_banned_shape() { // state is written by the case rather than inherited from whatever is running // — and `another_task_is_none_of_this_gates_business` beside it is what keeps // the row from becoming a blanket refusal of `mise run`. - for command in [ - "gh pr view 42", - "mise exec -- cargo test -p batten", - "mise run test:cargo", - ] { - let output = run_hook_in(&root, "exit-code", &claude_payload(command), false); + let command = "gh pr view 42"; + let output = run_hook_in(&root, "exit-code", &claude_payload(command), false); + assert_eq!( + output.status.code(), + Some(0), + "the committed policy must allow {command:?}" + ); + + // THE `mise` CALLS MOVED TO THE BACKGROUNDED FORM, and the move is the rule + // rather than an accommodation of it. `foreground-mise` refuses every + // foreground `mise` invocation with no fast list, because the harness kills a + // foreground call at ~2 minutes and each of these two can cross that bound + // behind a cargo build the caller cannot see coming — `test:cargo` is the + // whole nextest lap. Asserting the committed policy still ALLOWS them + // foreground would be asserting the gate does not hold. + // + // They stay in this case rather than leaving it, because the claim they carry + // is unchanged and still worth pinning: `mise run` is not blanket-refused + // here. A rule that denied it outright would take the backgrounded form too, + // and this is where that would surface. + for command in ["mise exec -- cargo test -p batten", "mise run test:cargo"] { + let output = run_hook_in( + &root, + "exit-code", + &claude_payload_backgrounded(command), + false, + ); assert_eq!( output.status.code(), Some(0), - "the committed policy must allow {command:?}" + "the committed policy must allow a backgrounded {command:?}" ); } } diff --git a/crates/batten/tests/it/gh_guard.rs b/crates/batten/tests/it/gh_guard.rs index 5b38d9977..cec195466 100644 --- a/crates/batten/tests/it/gh_guard.rs +++ b/crates/batten/tests/it/gh_guard.rs @@ -97,6 +97,15 @@ fn root() -> PathBuf { PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../..") } +/// [`bash_payload`] with `run_in_background` stated true. +fn bash_payload_backgrounded(command: &str) -> String { + let escaped = serde_json::to_string(command).expect("a command is encodable"); + format!( + "{{\"hook_event_name\":\"PreToolUse\",\"tool_name\":\"Bash\",\ + \"tool_input\":{{\"command\":{escaped},\"run_in_background\":true}}}}" + ) +} + /// A Claude Code `PreToolUse` envelope carrying a shell command. fn bash_payload(command: &str) -> String { let escaped = serde_json::to_string(command).expect("a command is encodable"); @@ -156,6 +165,24 @@ fn allowed(command: &str) { ); } +/// [`allowed`], with the call's backgrounding STATED. +/// +/// For a command another committed row reads the posture of. `foreground-mise` +/// refuses every foreground `mise` invocation with no fast list, so a case whose +/// property is something else entirely — wrapper look-through, here — has to +/// carry the posture or it measures that row instead of its own. +fn allowed_backgrounded(command: &str) { + let out = stdout(&run_with_stdin_at_real_root( + &root(), + &["adjudicate", "--harness", "claude-code"], + &bash_payload_backgrounded(command), + )); + assert!( + !out.contains("\"deny\""), + "the committed policy must allow a backgrounded: {command}\n{out}" + ); +} + // --- blocked: shapes a task already encapsulates ------------------------------ #[test] @@ -301,8 +328,13 @@ fn a_task_name_is_not_a_wrapped_program() { // it, so the case names one no lock is ever taken for. That `land` itself is // allowed when unheld is `singleton_gate.rs::an_unheld_task_starts`, where // the lock state is written rather than inherited. - allowed("mise run fmt"); - allowed("mise exec --"); + // BACKGROUNDED, because `foreground-mise` now refuses the foreground form of + // both and this case is not about that: the property under test is that + // `mise run` names a TASK while `mise exec` runs another program, and the + // look-through is what decides it. Left foreground, the case would measure + // the newer row and report the older one broken. + allowed_backgrounded("mise run fmt"); + allowed_backgrounded("mise exec --"); } // --- the hook's own document contract ----------------------------------------- diff --git a/crates/batten/tests/it/hook_skip_local.rs b/crates/batten/tests/it/hook_skip_local.rs index 11ba78003..94f4f0d81 100644 --- a/crates/batten/tests/it/hook_skip_local.rs +++ b/crates/batten/tests/it/hook_skip_local.rs @@ -82,6 +82,31 @@ fn allowed(command: &str) { ); } +/// [`allowed`], with the call's backgrounding STATED. +/// +/// These cases adjudicate against the LIVE root, so every committed row reaches +/// them — `foreground-mise` included, which refuses a foreground `mise` call with +/// no fast list. An anti-vacuity case has to survive on this row's own account +/// rather than by another row's silence, so the posture is stated and the +/// remaining question is whether THIS row fires. +fn allowed_backgrounded(command: &str) { + let escaped = serde_json::to_string(command).expect("a command is encodable"); + let payload = format!( + "{{\"hook_event_name\":\"PreToolUse\",\"tool_name\":\"Bash\",\ + \"tool_input\":{{\"command\":{escaped},\"run_in_background\":true}}}}" + ); + let root = common::at_root("."); + let out = common::stdout(&common::run_with_stdin( + &root, + &["adjudicate", "--harness", "claude-code"], + &payload, + )); + assert!( + !out.contains("\"deny\""), + "the committed policy must allow a backgrounded: {command}\n{out}" + ); +} + #[test] fn a_local_step_skip_is_refused() { // The measured command, as it was actually run on this branch. @@ -104,7 +129,7 @@ fn the_declared_ci_carve_is_not_judged_here() { // hands hk exactly this, and `ci-suite-lane` is the row that governs it. A // guard refusing the repository's own declared invocation gets disabled, and // then it enforces nothing at all. - allowed("HK_SKIP_STEPS=test:bats mise run ci"); + allowed_backgrounded("HK_SKIP_STEPS=test:bats mise run ci"); } #[test] @@ -120,7 +145,7 @@ fn an_ordinary_command_is_allowed() { // ANTI-VACUITY. Without these the denies above are satisfied by a build that // refuses every command, which would name this row every time. allowed("git commit -m 'an ordinary commit'"); - allowed("mise run ci"); + allowed_backgrounded("mise run ci"); allowed("RUST_LOG=debug git commit -m 'another variable is not this one'"); } diff --git a/crates/batten/tests/it/mutate.rs b/crates/batten/tests/it/mutate.rs index 8d005b5c2..64f4afd52 100644 --- a/crates/batten/tests/it/mutate.rs +++ b/crates/batten/tests/it/mutate.rs @@ -237,6 +237,58 @@ fn census(root: &Path, gates: &str) -> (i32, String, String) { // The sweep's decision table. // --------------------------------------------------------------------------- +/// ANTI-VACUITY FOR THE SWEEP'S OWN WATCHDOG (CLOUD-1726). +/// +/// The bound moved out of `BATS_TEST_TIMEOUT` and into the sweep, because the +/// runner implements its bound as a `sleep N` child it does not reap on a +/// FAILING case — and a caught mutation is a failing case, so every row the +/// sweep got right waited the bound out. Unsetting it took this module from +/// 92.6s to 2.4s. +/// +/// That is only safe while something still ends a suite that genuinely hangs, +/// which is what a mutation sweep must survive by construction: a mutant can +/// make a gate loop forever. Without this case the removal is indistinguishable +/// from having no bound at all, and the first hanging mutant would block a sweep +/// until somebody noticed. +#[cfg(unix)] +#[test] +fn a_suite_that_hangs_is_ended_by_the_sweeps_own_bound() { + let root = toy("hangs"); + let mut gate = String::from(TOY_GATE); + gate.push_str(CAUGHT); + gate.push('\n'); + write_program(&root, "mise-tasks/toy.sh", &gate); + // The case never returns on its own. `read` on a closed stdin would, so the + // wait has to be one nothing external can satisfy. + write( + &root, + "tests/toy.bats", + "#!/usr/bin/env bats\n@test \"over the limit is refused\" {\n\tsleep 3600\n}\n@test \"under the limit passes\" {\n\ttrue\n}\n", + ); + track(&root); + lend_bats(&root); + + let started = std::time::Instant::now(); + let answer = common::batten() + .args(["mutate", "sweep"]) + .current_dir(&root) + .env("MUTANT_GATES", "toy") + .env("BATTEN_MUTATE_SUITE_TIMEOUT", "2") + .output() + .expect("run batten mutate"); + let waited = started.elapsed(); + + assert!( + waited < std::time::Duration::from_secs(60), + "the sweep waited {waited:?} on a suite that never returns, so the bound \ + did not fire and a hanging mutant would hold it forever" + ); + // The verdict itself is deliberately not asserted: what a killed suite + // reports is the sweep's business and is covered by the decision table + // above. This case asserts only that the sweep CAME BACK. + let _ = answer.status.code(); +} + #[cfg(unix)] #[test] fn a_mutation_its_suite_catches_is_a_pass() { diff --git a/crates/batten/tests/it/pipeline_shapes.rs b/crates/batten/tests/it/pipeline_shapes.rs index f7718059e..51a91a331 100644 --- a/crates/batten/tests/it/pipeline_shapes.rs +++ b/crates/batten/tests/it/pipeline_shapes.rs @@ -62,6 +62,41 @@ fn assert_allowed(command: &str) { assert_eq!(verdict(command), Some(0), "must allow: {command}"); } +/// [`assert_allowed`], with the call's backgrounding STATED. +/// +/// These adjudicate against the LIVE root, so `foreground-mise` reaches any case +/// naming a `mise` call and refuses it with no fast list. A case whose subject is +/// a DIFFERENT row has to state the posture or it measures that one instead. +/// [`cause`], with the call's backgrounding STATED. +/// +/// A refusal renders one cause, so a case asserting WHICH class a shape renders +/// has to keep every other row from firing on the same string. +fn cause_backgrounded(command: &str) -> String { + let encoded = serde_json::to_string(command).expect("a command is encodable"); + let payload = format!( + "{{\"hook_event_name\":\"PreToolUse\",\"tool_name\":\"Bash\",\ + \"tool_input\":{{\"command\":{encoded},\"run_in_background\":true}}}}" + ); + stderr(&run_with_stdin_at_real_root( + &root(), + &["adjudicate", "--harness", "exit-code"], + &payload, + )) +} + +fn assert_allowed_backgrounded(command: &str) { + let encoded = serde_json::to_string(command).expect("a command is encodable"); + let payload = format!( + "{{\"hook_event_name\":\"PreToolUse\",\"tool_name\":\"Bash\",\ + \"tool_input\":{{\"command\":{encoded},\"run_in_background\":true}}}}" + ); + let code = + run_with_stdin_at_real_root(&root(), &["adjudicate", "--harness", "exit-code"], &payload) + .status + .code(); + assert_eq!(code, Some(0), "must allow a backgrounded: {command}"); +} + /// The refusal text, for the cases where WHICH operand a deny names is the thing /// under test rather than the verdict. A free function beside the two above /// because both families need it now: the discard family asserts that three @@ -96,7 +131,7 @@ fn a_read_only_query_carries_no_verdict_and_composes_freely() { assert_allowed("git log --oneline -5 | head -2"); assert_allowed("git status --short | wc -l"); assert_allowed("gh pr view 42 | tail -3"); - assert_allowed("mise exec -- cargo metadata | jq .packages"); + assert_allowed_backgrounded("mise exec -- cargo metadata | jq .packages"); // `jq` is composition rather than a verdict substitute, so it is not a filter // even downstream of a real verdict. assert_allowed("gh pr view 42 --json title | jq -r .title"); @@ -127,7 +162,7 @@ fn an_and_chain_is_allowed_because_it_cannot_manufacture_a_green() { // arithmetic rather than taste: `a && b` short-circuits, so a failure in `a` // still exits the list non-zero. There is no false green to stop, and // `verify`'s own body is built from guarded chains for that property. - assert_allowed("mise run fmt && mise run verify"); + assert_allowed_backgrounded("mise run fmt && mise run verify"); // THE GIT-FAMILY ARM, AND ITS OPERAND MOVED (CLOUD-1351). This read // `git fetch origin main && git rebase origin/main`, and that command is now // DENIED — by `rebase-not-hand-stepped`, for hand-stepping a step @@ -142,7 +177,7 @@ fn an_and_chain_is_allowed_because_it_cannot_manufacture_a_green() { // edited to make a new deny pass is otherwise indistinguishable from a test // weakened to fit a change. assert_allowed("git fetch origin main && git fetch origin --tags"); - assert_allowed("mise exec -- cargo build && mise exec -- cargo test"); + assert_allowed_backgrounded("mise exec -- cargo build && mise exec -- cargo test"); } #[test] @@ -155,31 +190,57 @@ fn detaching_a_verdict_orphans_it_from_the_tool_call() { } #[test] -fn the_prescribed_form_is_allowed_including_its_redirection() { - // THE regression test for the parser change. `2>&1` carries a literal `&`, - // and the form this engine prescribes contains one — so an `&` test - // that did not exempt redirections would refuse the exact idiom the refusal - // recommends, which is the worst failure this gate could have. - assert_allowed("mise run verify >/tmp/verify.log 2>&1"); +fn a_redirection_is_not_a_background_ampersand() { + // THE regression test for the parser change, and its subject is the PARSER + // rather than any prescription: `2>&1` and `&>` carry a literal `&`, so an + // `&` test that did not exempt redirections would read a redirect as a + // detach. That property is unchanged and is pinned here on a program no + // other row has an opinion about. + assert_allowed("git push origin branch >/tmp/push.log 2>&1"); + assert_allowed("git push origin branch &>/tmp/push.log"); + assert_allowed("git push origin branch >/tmp/p.log 2>&1 && echo queued"); +} + +/// THE PRESCRIPTION MOVED, AND THESE ARE THE CASES THAT SAY SO (CLOUD-1722). +/// +/// `verdict-not-discarded` used to recommend `mise run verify >/tmp/verify.log +/// 2>&1` — redirect the output rather than pipe it, so the exit status stays the +/// task's. Two newer rows make that exact string unrunnable from either side: +/// `foreground-mise` refuses it foreground, because the harness kills a +/// foreground call at ~2 minutes, and `background-redirect` refuses it +/// backgrounded, because the harness already captures a backgrounded task's +/// output where the human watches and a private log file is one nobody reads. +/// +/// A repository that prescribes a form two of its own rows refuse is worse than +/// one that prescribes nothing, so the form is now: BACKGROUNDED, UNREDIRECTED, +/// UNPIPED. All three rows agree on it, and it is what these cases pin. +#[test] +fn the_prescribed_form_is_backgrounded_and_keeps_its_own_output() { + assert_allowed_backgrounded("mise run verify"); // NOT `land`, and for CLOUD-438's reason rather than by preference: this // case adjudicates against the LIVE root, `land.sh` is the one task that // takes a singleton, and the singleton row refuses a second start while a // live process holds it — so under `verify`, which runs this suite from - // inside `land`, the verdict here would turn on a lock rather than on the - // redirection this case is about. `gh_guard.rs` records the same swap after - // measuring the failure. - assert_allowed("mise run fmt >/tmp/fmt.log 2>&1"); - assert_allowed("mise exec -- cargo test -p batten >/tmp/test.log 2>&1"); - assert_allowed("git push origin branch >/tmp/push.log 2>&1"); - // The other redirection spellings that carry an `&`. - assert_allowed("mise run verify &>/tmp/verify.log"); - assert_allowed("mise run verify >/tmp/v.log 2>&1 && echo queued"); + // inside `land`, the verdict here would turn on a lock rather than on what + // this case is about. `gh_guard.rs` records the same swap after measuring + // the failure. + assert_allowed_backgrounded("mise run fmt"); + assert_allowed_backgrounded("mise exec -- cargo test -p batten"); +} + +#[test] +fn the_retired_redirect_form_is_refused_from_both_sides() { + // The anti-vacuity half of the case above: without these, "the prescribed + // form is backgrounded" is satisfied by a build that allows the old form too, + // and the prescription would be advice rather than a rule. + assert_denied("mise run verify >/tmp/verify.log 2>&1"); + assert_denied("mise run verify &>/tmp/verify.log"); } #[test] fn a_verdict_alone_in_the_call_is_the_prescribed_form() { - assert_allowed("mise run verify"); - assert_allowed("mise exec -- cargo test -p batten"); + assert_allowed_backgrounded("mise run verify"); + assert_allowed_backgrounded("mise exec -- cargo test -p batten"); assert_allowed("git push origin branch"); assert_allowed("bats tests/land.bats"); } @@ -190,7 +251,7 @@ fn a_bare_invocation_that_answers_nothing_is_not_a_verdict() { // usage. Piping usage is not discarding a verdict, because there is none. assert_allowed("bats --version | head -1"); assert_allowed("bats --help | tail -5"); - assert_allowed("mise exec -- cargo | head -3"); + assert_allowed_backgrounded("mise exec -- cargo | head -3"); } #[test] @@ -208,7 +269,17 @@ fn a_pager_on_an_earlier_query_does_not_condemn_a_later_command() { // Judged per segment. A pager attached to a read-only first element says // nothing about a verdict-bearing second one, and judging the whole string // refused exactly that — a correct command using the recommended form. - assert_allowed("git log --oneline | head -3 && mise run verify >/tmp/v.log 2>&1"); + // + // THE RECOMMENDED FORM MOVED, AND THIS CASE FOLLOWED IT (CLOUD-1722). It used + // to read `&& mise run verify >/tmp/v.log 2>&1`, which is now two refusals + // rather than a model answer: `foreground-mise` refuses the foreground call + // because the harness kills one at ~2 minutes, and `background-redirect` + // refuses a backgrounded call that redirects its own output, because the + // harness already captures it where the human watches. What is left is the + // form both rows agree on — backgrounded, unredirected — and pinning THAT + // here is the point: this case's subject is the per-segment judging, so it + // must carry a second element that is genuinely correct today. + assert_allowed_backgrounded("git log --oneline | head -3 && mise run verify"); // And the direction that matters: a write must not be excused by a read. assert_denied("git log --oneline | head -3 && mise run verify | tail -2"); } @@ -218,11 +289,9 @@ fn the_refusal_states_the_principle_rather_than_naming_one_command() { // CLOUD-199's second instance happened because an agent complied with the // narrower wording exactly and made the same error on the next command. The // cause therefore has to generalise, and the remedy has to be the row's. - let refusal = stderr(&run_with_stdin_at_real_root( - &root(), - &["adjudicate", "--harness", "exit-code"], - &payload("mise run verify | tail -6"), - )); + // Backgrounded for `each_shape_renders_its_own_cause`'s reason: one refusal + // renders one cause, and this case asserts WHICH row the reader is sent to. + let refusal = cause_backgrounded("mise run verify | tail -6"); assert!( refusal.contains("verdict-not-discarded"), "names the rule: {refusal}" @@ -263,14 +332,25 @@ fn each_shape_renders_its_own_cause() { // arm, so the discrimination this case asserts is now over three registry // tokens — the same three structures, reachable from `batten policy explain` // instead of only from this file. - assert!(cause("mise run verify | tail -1").contains("verdict read dropped")); - assert!(cause("mise run verify >log 2>&1; ls").contains("verdict carry other")); - assert!(cause("nohup mise run verify &").contains("turn watch dropped")); + // BACKGROUNDED, AND THAT IS A PRECONDITION OF THE CASE RATHER THAN A DETAIL + // (CLOUD-1722). A refusal renders ONE cause, so a second row firing on the + // same string masks the class under test: measured, `mise run verify >log + // 2>&1; ls` read `task run blocked foreground-mise` and this case could no + // longer see `verdict carry other` at all. Stating the posture takes + // `foreground-mise` out of the way and leaves the shape's own row to answer. + // + // The `>log 2>&1` also left the middle shape, for the same reason one layer + // on: backgrounded, `background-redirect` claims it. The redirect was never + // what that case was about — `; ls` is, because it hands the exit status to + // `ls` — so dropping it makes the case name its own subject. + assert!(cause_backgrounded("mise run verify | tail -1").contains("verdict read dropped")); + assert!(cause_backgrounded("mise run verify; ls").contains("verdict carry other")); + assert!(cause_backgrounded("nohup mise run verify &").contains("turn watch dropped")); // And they are three, not one wearing three hats: no shape renders another's // class. Without this arm a composer collapsing all three onto one token // would still satisfy the three assertions above via a substring. - assert!(!cause("mise run verify | tail -1").contains("turn watch dropped")); - assert!(!cause("nohup mise run verify &").contains("verdict read dropped")); + assert!(!cause_backgrounded("mise run verify | tail -1").contains("turn watch dropped")); + assert!(!cause_backgrounded("nohup mise run verify &").contains("verdict read dropped")); } // --- the substitution family (CLOUD-864) -------------------------------------- diff --git a/crates/batten/tests/it/run_shape.rs b/crates/batten/tests/it/run_shape.rs index 3406302a3..acba63e08 100644 --- a/crates/batten/tests/it/run_shape.rs +++ b/crates/batten/tests/it/run_shape.rs @@ -209,7 +209,29 @@ fn fixture(name: &str) -> PathBuf { "[[verdict.route]]\n", "id = \"task run first\"\n", "kind = \"command\"\n", - "target = \"until ; do sleep 1; done\"\n", + "target = \"until ; do sleep 1; done\"\n\n", + "[[verdict]]\n", + "id = \"task run blocked\"\n", + "gloss = \"a foreground `mise` call is killed at ~2 minutes, so it fails rather than runs slowly\"\n", + "class = \"\"\"\n", + "Every task here can cross the bound behind a cargo build nobody sees coming. \\\n", + "Pass run_in_background on the tool call and act on the exit notification.\n", + "\"\"\"\n\n", + "[[verdict.route]]\n", + "id = \"task run first\"\n", + "kind = \"command\"\n", + "target = \"re-issue the same command with run_in_background\"\n\n", + "[[verdict]]\n", + "id = \"redirect write unread\"\n", + "gloss = \"a backgrounded call redirecting its own output writes a second file nobody reads\"\n", + "class = \"\"\"\n", + "The harness already captures a backgrounded task's output where the human \\\n", + "watches. A private file replaces the run they were meant to see over.\n", + "\"\"\"\n\n", + "[[verdict.route]]\n", + "id = \"redirect write first\"\n", + "kind = \"command\"\n", + "target = \"drop the redirect and read the harness's own output file\"\n", ), ) .expect("write the fixture authority"); @@ -742,5 +764,72 @@ fn a_process_read_outside_a_loop_is_not_a_wait() { // it would refuse its own remedy. let root = fixture("reads-a-process-once"); allowed_background(&root, "pgrep -f mise", true); - allowed(&root, "mise run alive"); + // BACKGROUNDED, and it was foreground until `foreground-mise` landed: the + // probe is still the remedy this class recommends, and it is now a + // backgrounded one like every other `mise` call. + allowed_background(&root, "mise run alive", true); +} + +// --- the foreground `mise` call, and the backgrounded call that hides its own +// --- output ------------------------------------------------------------------ + +#[test] +fn a_foreground_mise_run_is_refused() { + // The harness kills a foreground call at ~2 minutes, so this does not run + // slowly — it FAILS, and takes the turn with it. + let root = fixture("foreground-mise"); + denied_background(&root, "mise run verify", false); + // AND WITH THE HOST SAYING NOTHING, which is the ordinary envelope: an + // unknown posture over a call that can spend the whole turn is the case to + // be strict about. + denied(&root, "mise run ci"); +} + +#[test] +fn a_short_mise_task_is_refused_just_the_same() { + // THE CASE THAT SAYS THERE IS NO FAST LIST. `alive` is the prescribed + // liveness probe and returns in well under a second — and exempting it would + // hand the duration judgement back to the caller this rule exists to stop + // consulting. Backgrounding it costs one turn and returns the same text. + let root = fixture("foreground-mise-alive"); + denied_background(&root, "mise run alive", false); +} + +#[test] +fn a_backgrounded_mise_run_is_allowed() { + let root = fixture("background-mise"); + allowed_background(&root, "mise run verify", true); +} + +#[test] +fn a_program_merely_spelt_near_mise_is_not_a_mise_call() { + // The program is anchored on `programs`, never on a word: a path mentioning + // mise is an argument, and `mise` inside a quoted span is prose. + let root = fixture("mise-word"); + allowed(&root, "cat mise.toml"); + allowed(&root, "grep -n 'mise run verify' AGENTS.md"); +} + +#[test] +fn a_backgrounded_call_redirecting_its_own_output_is_refused() { + // The harness captures a backgrounded task's output and surfaces it where + // the HUMAN watches; `> log 2>&1` substitutes a private file for that one. + let root = fixture("background-redirect"); + denied_background(&root, "cargo build > /tmp/log 2>&1", true); + denied_background(&root, "cargo build 2> /tmp/err", true); +} + +#[test] +fn a_foreground_redirect_is_not_this_rule() { + // Nothing is captured for a foreground call, so a redirect there discards + // no output anyone was going to read. + let root = fixture("foreground-redirect"); + allowed_background(&root, "cargo build > /tmp/log 2>&1", false); +} + +#[test] +fn a_backgrounded_input_redirect_is_untouched() { + // Reading a file INTO a backgrounded command discards nothing. + let root = fixture("background-stdin"); + allowed_background(&root, "cargo build < /tmp/answers", true); } diff --git a/hk.pkl b/hk.pkl index 3e7cd1f3e..05e032ef6 100644 --- a/hk.pkl +++ b/hk.pkl @@ -1145,8 +1145,14 @@ local fixers = new Mapping { ["cargo-fmt"] = gate["cargo-fmt"] } -// The steps above marked `profiles = List("slow")` are the six that dominate the -// gate: 111.1s total, 100.3s of it `test:bats` at nproc=4. They are not cheaper +// The steps above marked `profiles = List("slow")` are the nine that dominate +// the gate: 111.1s total, 100.3s of it `test:bats` at nproc=4. Counted from the +// mapping rather than recalled — `coderabbit-config-check`, `hk-contract-check`, +// `token-bench-check`, `sbom-check`, `test:bats`, `cargo-clippy`, `test`, +// `batten-check`, `policy-test`. It said "six" from the tier's first shape and +// three steps joined it afterwards without the count following, which is the +// same drift `suite-bench-check` exists to keep out of the bats table: a number +// in prose has no gate behind it and ages silently. They are not cheaper // than they look and they are not optional — they simply do not belong on the // path a developer pays on every `git commit`, because `hk check --all` (which // `mise run ci`, and therefore `verify` and CI, drive over this same mapping) @@ -1180,6 +1186,39 @@ hooks { // Conventional Commits (https://www.conventionalcommits.org/en/v1.0.0/). // PRs land by fast-forward, so every commit reaches main with its original // SHA and must be conventional for release-plz to compute semver correctly. + // + // THESE TWO STEPS STILL ROUTE THROUGH `mise run`, AND THE BUDGET SAYS THEY + // SHOULD NOT (CLOUD-1397). A git hook is a batten hook: README's `wired` row is + // 8.0ms p50 against a published <=100ms ceiling, and nothing makes the + // commit-msg surface exempt because git is the harness. + // + // Measured 2026-09-09, same message, warm, three runs each: + // + // mise run commit-msg, body spelling `cargo run` 582ms + // mise run commit-msg, body resolving the binary ~300ms + // batten commit check --message 121-130ms + // batten attribution check --message 16-18ms + // mise --version, the process alone, no task 105-125ms + // + // The task bodies now resolve the binary, which is the 582 -> ~300ms half and + // is in `mise.toml`. The other half is unreachable from here: `mise` alone + // costs more than the whole budget before a task body runs, so only a step + // calling the binary directly can land under it. + // + // WHY THAT HALF IS NOT HERE, AND IT IS A GATE RATHER THAN A PREFERENCE. + // Respelling these two steps breaks `tests/commit-attribution.bats:19` and + // `tests/commit-convention.bats:42`, which pin the call site by its text -- + // correctly, since CLOUD-216 found this gate wired to nothing. Following them + // means EDITING a governed bats suite, and `shell-retirement`'s edit arm + // refuses that with no override route and no `bypass_env`: the campaign admits + // deleting such a file whole, never changing it. Retiring those two suites into + // Rust is CLOUD-1748's work and is being done elsewhere; doing it here to buy + // 170ms would be taking that campaign's subject hostage to a latency fix. + // + // So this waits on the suites, and CLOUD-1397 carries the remainder. What does + // NOT wait is the 8687ms case -- the first commit after touching one + // `crates/batten/src` file, which was a hook running the compiler -- because + // that lived in the task body and is gone. ["commit-msg"] { steps { ["conventional-commit"] { diff --git a/mise.toml b/mise.toml index 712c39662..ab3a2e192 100644 --- a/mise.toml +++ b/mise.toml @@ -617,7 +617,7 @@ CI_FANIN_WORKFLOW = ".github/workflows/ci.yml" BATS_TEST_TIMEOUT = "300" REGORUS_OPA_COMPLIANCE = "1.2.0" REGORUS_OPA_COMPLIANCE_FOR = "0.11" -MUTANT_GATES = "mise,attestation-check,engine-config,engine-doctor,engine-landed,engine-mcp,engine-pinned,engine-ready,engine-verdict,engine-surface,agentic-experiment-record,awk-regex-check,bats-invocation,batten-glob-check,board-diff-overlap,board-payloads,board-sweep,branch-age-check,cap-drift,cfg-gated-test,ci-cache-declared,ci-hygiene,ci-parity,ci-slow-needed,ci-suite-lane,ci-tools-check,claim-before-code,claim-order-is-stated,claimed-keys,closing-key-check,coderabbit-config-check,commit-hygiene,connector-allow-guard,connector-allow-resolve,container-preflight,darwin-link,deferral-check,denials-outlive-the-turn,digest-major-agreement,doctor,done-check,done-pr-check,duplicate-close-check,egress-fencing,evaluator-closure-check,evaluator-io-check,filed-here,finding-sink-check,fixture-forks,forge-verdict-required,graph-check,harness-grant,harness-wiring,hk-fix-selection,hk-plan-required,hook-pin-check,hook-skip-local,in-progress-drain,install-check,land-divergence-assert,landed-check,landing-loop,landing-roster-guarded,leased-push,license-table-check,linear-check,lock-complete,macos-link-check,mcp-allow-check,mcp-attach-check,mcp-timeout-budget,merged-pr-keys,mise-action-floor,mise-pin-agreement,module-map-check,msrv-pin-agreement,mutation-declared-case,no-doctests,nonverdict-assert,ntia-check,obligations-bound,perf-assert,pinned-toolchain,pipefail-grep-check,plan-complete,pr-partition-restated,pr-unsubscribed,privileged-lane,prose-only,publish-credential-check,ready-cites-check,ready-lint,reclaim-census,release-assets-check,release-due,release-provision-parity,release-tag-shape,release-tracking-check,released,remedy-authorship,repetition-without-progress,report-only-check,review-answered,review-dispatched,run-shape,rust-paths-check,sbom,sbom-inventory,serena-mcp,shell-hygiene,shell-retirement,shell-write-advisory,signing-posture,sonar-gate,spec-ref-check,stop-posture,stop-posture-check,suite-bench-check,suite-subject-retirable,task-substitution,test-targets,timeout-check,token-bench-check,transcript-corpus-check,tree-clean,trunk-based,validator-verdict-clean,verdict-routes-resolve,weakens-declared,worktree-registration,spawn-widening,nextest-slow,engine-lease" +MUTANT_GATES = "mise,attestation-check,engine-checks-green,engine-config,engine-doctor,engine-landed,engine-mcp,engine-pinned,engine-ready,engine-verdict,engine-surface,agentic-experiment-record,awk-regex-check,bats-invocation,batten-glob-check,board-diff-overlap,board-payloads,board-sweep,branch-age-check,cap-drift,cfg-gated-test,ci-cache-declared,ci-hygiene,ci-parity,ci-slow-needed,ci-suite-lane,ci-tools-check,claim-before-code,claim-order-is-stated,claimed-keys,closing-key-check,coderabbit-config-check,commit-hygiene,connector-allow-guard,connector-allow-resolve,container-preflight,darwin-link,deferral-check,denials-outlive-the-turn,digest-major-agreement,doctor,done-check,done-pr-check,duplicate-close-check,egress-fencing,evaluator-closure-check,evaluator-io-check,filed-here,finding-sink-check,fixture-forks,forge-verdict-required,graph-check,harness-grant,harness-wiring,hk-fix-selection,hk-plan-required,hook-pin-check,hook-skip-local,in-progress-drain,install-check,land-divergence-assert,landed-check,landing-loop,landing-roster-guarded,leased-push,license-table-check,linear-check,lock-complete,macos-link-check,mcp-allow-check,mcp-attach-check,mcp-timeout-budget,merged-pr-keys,mise-action-floor,mise-pin-agreement,module-map-check,msrv-pin-agreement,mutation-declared-case,no-doctests,nonverdict-assert,ntia-check,obligations-bound,perf-assert,pinned-toolchain,pipefail-grep-check,plan-complete,pr-partition-restated,pr-unsubscribed,privileged-lane,prose-only,publish-credential-check,ready-cites-check,ready-lint,reclaim-census,release-assets-check,release-due,release-provision-parity,release-tag-shape,release-tracking-check,released,remedy-authorship,repetition-without-progress,report-only-check,review-answered,review-dispatched,run-shape,rust-paths-check,sbom,sbom-inventory,serena-mcp,shell-hygiene,shell-retirement,shell-write-advisory,signing-posture,sonar-gate,spec-ref-check,stop-posture,stop-posture-check,suite-bench-check,suite-subject-retirable,task-substitution,test-targets,timeout-check,token-bench-check,transcript-corpus-check,tree-clean,trunk-based,validator-verdict-clean,verdict-routes-resolve,weakens-declared,worktree-registration,spawn-widening,nextest-slow,engine-lease" # --- GitHub reachability behind an egress proxy (Claude Code web sandbox etc.) --- # mise resolves every tool's release through GitHub's *API* host, api.github.com. @@ -3969,12 +3969,64 @@ echo "verify: fast-forward-green — rebased on latest main, ci + cross + commit [tasks.commit-msg] description = "Gate: one pending commit message's subject follows the convention (policy: [commit] in batten.toml)" -# `cargo run` for the same reason `batten-check` and `commit-attribution` use it: -# the gate must judge the working tree's engine and config as the pair that ships. +# THE BINARY ON PATH FIRST, AND `cargo run` ONLY WHERE THERE IS NONE — CLOUD-1620's +# shape, applied to the path that pays it most (CLOUD-1397). +# +# A GIT HOOK IS A BATTEN HOOK, and is held to the same published ceiling. README's +# `wired` row is 8.0ms p50 against a ≤100ms budget, and `perf-assert` enforces it +# on the mediated surface; nothing argues the commit-msg surface is exempt just +# because git is the harness rather than Claude Code. +# +# MEASURED 2026-09-09 on this container, same message, warm, three runs: this +# task as the hook fired it, 582ms; `cargo run --quiet` with NOTHING to rebuild +# 291ms; `target/debug/batten` 176ms; the release binary 125ms; and 8687ms on the +# first commit after touching one `crates/batten/src` file. +# +# A FIRST PASS AT THESE NUMBERS READ 12ms FOR THE RELEASE BINARY AND IT WAS A +# MEASUREMENT OF A FAILURE. `target/release/batten` was 0.0.155 against a tree at +# 0.0.158 and REFUSED `batten.toml` outright — "vocabulary `action`: `fix` is +# declared and no class or route name spends it" — so the 12ms was a config load +# aborting, not a gate reaching a verdict. Rebuilt, the same call is 125ms. A +# timing taken over a non-zero exit is not a timing of the work, and a stale +# binary is exactly the shape that produces one (CLOUD-1688). +# +# A HOOK MUST NEVER COMPILE, and this is not a preference. Rebuilding that stale +# release binary took ~4 minutes on this container (15:48 -> 15:52, `lto = "thin"` +# in `[profile.release]`), so a hook that rebuilds when it finds a stale binary +# would spend four minutes at the moment an author saves. The 8687ms debug case is +# the same defect an order of magnitude cheaper: a hook running the compiler is +# not a slow gate, it is a different program. The build belongs at provisioning, +# where `session:batten` already puts it. The +# `_.path` entry above already resolves a bare `batten` to THIS checkout's +# `target/release`, built by `session:batten` -> `install:local` at session start +# with an `::error::` when it cannot be — so the fast branch is the tree's own +# engine, not a stale installed one, and the property the previous comment claimed +# for `cargo run` ("judge the working tree's engine and config as the pair that +# ships") is kept rather than traded. +# +# THE CONDITION TESTS PRESENCE AND NOTHING ELSE, which is where `attribution +# identity`'s spelling must not be copied. That one reads +# `if command -v batten && batten ; then :; else cargo run …` and its own +# comment claims `if`/`else` avoids the fallthrough that `a && b || c` has — but +# those two are the same program: a binary that EXISTS and legitimately REFUSES +# takes the else branch either way, so the shape it warns about is the shape it +# ships. Harmless for a write that wants a retry; wrong for a gate, which would +# then pay the build precisely when it refuses and run the whole judgement twice +# to reach the same verdict. A gate that is slowest exactly when it says no is a +# gate authors learn to stop running. +# +# So the verdict is not in the condition. Resolve, then run once, and let the +# exit code be the gate's own. +# +# The fallback is what keeps a runner green: CI has no installed binary and +# `bash: line 25: batten: command not found` is a measured failure there +# (`auto-bot-land.yml:305-310`), so the branch that builds stays for the host that +# needs it and never runs where a human is waiting. +# # The pattern itself is `batten.toml`'s, not this file's — a rule about what a # commit may BE is the engine's, and `mise.toml` configures how tools run # (CLOUD-701). -run = 'cargo run --quiet -p batten -- commit check --message "{{arg(name="file")}}"' +run = 'if command -v batten >/dev/null 2>&1; then batten commit check --message "{{arg(name="file")}}"; else cargo run --quiet -p batten -- commit check --message "{{arg(name="file")}}"; fi' [tasks.commit-attribution] description = "Gate: no vendor authorship, branding or session links in BASE_SHA..HEAD_SHA (policy: [attribution] in batten.toml)" @@ -3987,7 +4039,12 @@ description = "Gate: no vendor authorship, branding or session links in one pend # The commit-msg-hook half. Same policy and same engine as `commit-attribution`; # only the object differs — a message on disk plus the identity `git var` says # git is about to stamp, rather than commits that already exist. -run = 'cargo run --quiet -p batten -- attribution check --message "{{arg(name="file")}}"' +# +# Resolve-then-run, for `commit-msg`'s reasons above and its measurements: these +# two are the pair that fires on EVERY commit, so the 582ms each was paying was +# the whole per-commit tax, and the 8687ms first-commit-after-an-edit case was a +# git hook running the compiler twice over. +run = 'if command -v batten >/dev/null 2>&1; then batten attribution check --message "{{arg(name="file")}}"; else cargo run --quiet -p batten -- attribution check --message "{{arg(name="file")}}"; fi' [tasks.attribution-identity] description = "Write: set this clone's repo-local git identity when it is unset or carries a denied vendor identity" diff --git a/policy/ci-cache-declared.rego b/policy/ci-cache-declared.rego index 86c77b599..014e0a7bc 100644 --- a/policy/ci-cache-declared.rego +++ b/policy/ci-cache-declared.rego @@ -44,6 +44,7 @@ #MUTANT cargo-reach-may-go-uncached|s@not declares_a_cache(path, name)@false@|a_cargo_job_with_no_cache_step_is_refused #MUTANT warmed-family-may-be-written|s@not reads_only(step)@false@|a_pull_request_writer_of_a_warmed_family_is_refused #MUTANT orphaned-reader-may-pass|s@not [shared_key(step), arch(runner(path, name))] in warmed@false@|a_read_only_consumer_of_an_unwarmed_family_is_refused +#MUTANT scheduled-writer-may-not-warm|s@trunk_writer(path) if _ := triggers(path).schedule@trunk_writer(path) if false@|a_scheduled_trunk_writer_warms_the_family # METADATA # description: | @@ -106,15 +107,26 @@ triggers(path) := t if { on_pull_request(path) if _ := triggers(path).pull_request -# `push` ONLY, AND NOT `schedule`, WHICH IS A DELIBERATE BOUND rather than an -# oversight. "Warmed" here means an entry a pull request can actually READ, and -# GitHub scopes a cache read to the run's own ref plus the base branch — so what -# matters is a writer on the trunk, which in this repository is a push-triggered -# workflow. The scheduled workflows do write `perf-`, `coverage-` and `fuzz-`, -# and those families are outside this predicate on purpose: `perf`'s base arm is -# a separate cache keyed to the merge base, which CLOUD-1331 and CLOUD-1342 -# already settled, and refusing it here would re-open a decision made elsewhere. -on_push(path) if _ := triggers(path).push +# A TRUNK WRITER IS ONE THAT RUNS ON THE TRUNK, AND THE TRIGGER IS NOT WHAT +# DECIDES THAT. The earlier version of this predicate read `push` only and +# defended the bound by saying trunk writers in this repository are +# push-triggered. Its own operative sentence refutes it: "GitHub scopes a cache +# read to the run's own ref plus the base branch" — so what a pull request can +# read is settled by the writing run's REF, and a `schedule` run on the default +# branch has ref `refs/heads/main` exactly as a `push` one does. Excluding +# schedules described the repository's habits rather than the mechanism, and it +# cost a real reading: `ci.yml`'s `perf` job restores the same `perf-` key, on the +# same architecture, that `perf.yml` writes daily from `main` — a warm family the +# model called orphaned, so its pull-request reader kept writing a full +# `--release` target directory nobody could read. +# +# THE CLOUD-1331 CARVE-OUT SURVIVES, AND IT WAS NEVER ABOUT THIS FAMILY. That row +# and CLOUD-1342 settled `perf`'s BASE ARM, a separate entry keyed to the merge +# base (`ci.yml:1129-1145`), which no clause here reaches. Naming it in defence of +# excluding `perf-` conflated two caches that share a job and nothing else. +trunk_writer(path) if _ := triggers(path).push + +trunk_writer(path) if _ := triggers(path).schedule # --- the steps, and the two facts a cache step carries ------------------------ @@ -436,7 +448,7 @@ runner(path, name) := label if { warmed contains [key, where] if { some entry in job_step - on_push(entry[0]) + trunk_writer(entry[0]) key := shared_key(entry[2]) where := arch(runner(entry[0], entry[1])) } @@ -668,6 +680,29 @@ test_a_writer_of_an_unwarmed_family_is_not_this_rules_business if { count(violation) == 0 with input as tree(no_writer, pr_reader("cross-", true)) } +# THE TRIGGER IS NOT WHAT MAKES A WRITER A TRUNK WRITER, and this is the case that +# pins it. A `schedule` run on the default branch has ref `refs/heads/main`, so a +# pull request reads its entry exactly as it reads a `push` writer's. Without this +# case the predicate could drop the `schedule` clause and stay green, which is the +# state that had `ci.yml`'s `perf` job writing over a family `perf.yml` already +# warmed daily. +test_a_scheduled_trunk_writer_warms_the_family if { + count(violation) == 0 with input as tree( + scheduled_writer, + pr_reader_on("ci-", false, "ubuntu-latest"), + ) +} + +# The other direction, so the widened predicate cannot pass by warming everything: +# a scheduled writer on ANOTHER architecture is still not this reader's family. +test_a_scheduled_writer_on_another_architecture_still_leaves_the_reader_empty if { + some finding in violation with input as tree( + scheduled_writer, + pr_reader_on("ci-", false, "ubuntu-24.04-arm"), + ) + finding.rule == "read-family-has-a-warm-writer" +} + test_a_job_reaching_no_cargo_needs_no_cache if { count(violation) == 0 with input as tree(warm_writer, inert_reader) } @@ -696,6 +731,16 @@ tasks := { warm_writer := warm_writer_on("ubuntu-latest") +# The same job on the same runner, triggered by `schedule` rather than `push` — +# `perf.yml`'s shape, and the one the earlier predicate did not count. +scheduled_writer := {"on": {"schedule": [{"cron": "0 5 * * *"}]}, "jobs": {"cache-warm-linux": { + "runs-on": "ubuntu-latest", + "steps": [{ + "uses": "Swatinem/rust-cache@6323deb1", + "with": {"shared-key": "ci-"}, + }], +}}} + warm_writer_on(label) := {"on": {"push": {"branches": ["main"]}}, "jobs": {"cache-warm-linux": { "runs-on": label, "steps": [{ diff --git a/policy/run-shape.rego b/policy/run-shape.rego index 84a4011a6..03097876b 100644 --- a/policy/run-shape.rego +++ b/policy/run-shape.rego @@ -67,6 +67,10 @@ rules contains "background-timer" rules contains "polls-a-local-process" +rules contains "foreground-mise" + +rules contains "background-redirect" + # CLOUD-613's three, and none of them is over a program NAME — a mutation on the # `sleep` or `git` token survives, because every ALLOW row already fails some # other conjunct. Each of these corrupts the conjunct that carries the verdict. @@ -86,6 +90,8 @@ rules contains "polls-a-local-process" #MUTANT single-quoted-span-judged|s@^single_scrubbed := quoted_out(code_lines.*@single_scrubbed := code_lines@|a_git_commit_inside_a_quoted_span_is_prose #MUTANT-SUITE crates/batten/tests/it/run_shape.rs #MUTANT process-poll-unread|s@^\tcount(process_probes) > 0$@\tfalse@|a_backgrounded_wait_polling_a_process_is_refused +#MUTANT mise-background-unread|s@^\tinput.call\["run-in-background"\] != true$@\ttrue@|a_backgrounded_mise_run_is_allowed +#MUTANT redirect-background-unread|s@^\tinput.call\["run-in-background"\] == true$@\ttrue@|a_foreground_redirect_is_not_this_rule #MUTANT bracket-is-an-exit|s@^\tcondition_program(segment) in {"pgrep", "pkill", "ps", "jobs"}$@\tcondition_program(segment) in {"pgrep", "pkill", "ps", "jobs"}; not contains(segment.raw, "[")@|a_bracketed_pattern_is_refused_just_the_same violation contains { @@ -180,6 +186,64 @@ violation contains { count(process_probes) > 0 } +# EVERY `mise` CALL IS BACKGROUNDED, WITH NO EXEMPTION LIST. +# +# AGENTS.md has carried the rule as a duration — "any command that can exceed ~2 +# minutes" — and a duration is a prediction the caller makes about a task it has +# not run. The prediction is wrong in the direction that costs: a foreground call +# is KILLED at ~2 minutes rather than run slowly, so the mis-estimate does not +# cost the difference between the guess and the truth, it costs the whole run +# plus the turn. And on this repo the estimate is over `mise`, whose tasks are +# the gate itself: `verify`, `ci`, `land`, a cold `cargo` build behind any of +# them. +# +# NO CARVE-OUT FOR THE FAST TASKS, deliberately, and `alive` is the one worth +# naming since it is the prescribed liveness probe. Backgrounding it costs one +# turn and returns the same text; a carve-out costs a list that every new task +# has to be judged against, by the same caller whose judgement this rule exists +# to stop consulting. A predicate with no list cannot be argued with, which is +# the property (house style §5). +violation contains { + "rule": "foreground-mise", + "verdict": "task run blocked", +} if { + some program in input.call.programs + basename(program.program) == "mise" + + # `!= true` for the same three-valued read `foreground-sleep` takes: `null` + # is "the host said nothing", and an unknown posture over a call that can + # spend the whole turn is the case to be strict about. + input.call["run-in-background"] != true +} + +# A BACKGROUNDED CALL THAT REDIRECTS ITS OWN OUTPUT WRITES WHERE NOBODY READS. +# +# The harness already captures a backgrounded task's output to a file it names +# back to the caller AND surfaces where the HUMAN watches. A `> log 2>&1` inside +# the command substitutes a second, private file for that one: the notification +# still fires, the output pane is empty, and the human loses the run they were +# meant to be able to see over. +# +# It is also how a verdict gets discarded. `mise run ci > log 2>&1` under a shell +# that is later piped or chained hands the exit status to the redirect's own +# element, which is the `verdict-not-discarded` family in a new spelling. +# +# JUDGED ON THE REDIRECTION TOKENS, which the segment carries as words of its own +# — `>` and its target are two words, `2>&1` is one (CLOUD-1382's parse). An +# INPUT redirect is untouched: reading a file into a backgrounded command +# discards nothing. +violation contains { + "rule": "background-redirect", + "verdict": "redirect write unread", +} if { + input.call["run-in-background"] == true + some segment in input.call.segments + some word in segment.words + output_redirect(word) +} + +output_redirect(word) if word in {">", ">>", "&>", "&>>", "2>", "2>>", "2>&1", ">&2", "1>", "1>>"} + # --------------------------------------------------------------------------- # CLOUD-613's terms, over `input.call.segments`. # @@ -842,3 +906,107 @@ test_a_mention_of_sleep_is_not_a_call if { "segments": [seg(["echo", "sleep", "90"], null, false)], }} } + +# --------------------------------------------------------------------------- +# The two families CLOUD-1722 added: every `mise` call is backgrounded, and a +# backgrounded call keeps its own output. +# +# `programs` rather than `words[0]`, and these cases are where that matters: +# `foreground-mise` anchors on the engine's RESOLVED program, so a fixture must +# carry the key. `sleeps` above reaches for a segment's program through +# `words_program_index`; this family does not, because the mediated document +# already publishes the resolution and CLOUD-1382 says a first word is not a +# first program. +# --------------------------------------------------------------------------- + +prog(name, arguments) := { + "program": name, + "name": name, + "arguments": arguments, + "mediated": true, +} + +test_a_foreground_mise_run_is_refused if { + some v in violation with input as {"call": { + "command": "mise run verify", + "run-in-background": false, + "programs": [prog("mise", ["run", "verify"])], + "segments": [seg(["mise", "run", "verify"], null, false)], + }} + v.rule == "foreground-mise" +} + +# THE STRICT SIDE OF THE THREE-VALUED READ, and the ordinary envelope: most hosts +# send no posture at all, and an unknown one over a call that can spend the whole +# turn is the case to be strict about. +# +# `null` RATHER THAN AN ABSENT KEY, and the difference is not cosmetic. The +# schema types this field `["boolean", "null"]`, so the engine always emits it +# and "the host said nothing" arrives as an explicit null. Written with the key +# missing, this case measured GREEN over a refusal that never fired: Rego reads +# an absent key as undefined and `undefined != true` is undefined, so the +# conjunct fails and the whole violation drops. A fixture encoding a document +# the engine cannot produce proves nothing, which is the same lesson `inner`'s +# comment records for the loop keyword. +test_an_unstated_posture_is_refused_too if { + some v in violation with input as {"call": { + "command": "mise run ci", + "run-in-background": null, + "programs": [prog("mise", ["run", "ci"])], + "segments": [seg(["mise", "run", "ci"], null, false)], + }} + v.rule == "foreground-mise" +} + +test_a_backgrounded_mise_run_is_allowed if { + count(violation) == 0 with input as {"call": { + "command": "mise run verify", + "run-in-background": true, + "programs": [prog("mise", ["run", "verify"])], + "segments": [seg(["mise", "run", "verify"], null, false)], + }} +} + +# THE ANCHORING CASE, the sibling of `a_mention_of_sleep_is_not_a_call`: `mise` +# as an ARGUMENT is not an invocation of it. +test_a_mention_of_mise_is_not_a_call if { + count(violation) == 0 with input as {"call": { + "command": "cat mise.toml", + "run-in-background": false, + "programs": [prog("cat", ["mise.toml"])], + "segments": [seg(["cat", "mise.toml"], null, false)], + }} +} + +test_a_backgrounded_call_redirecting_its_own_output_is_refused if { + some v in violation with input as {"call": { + "command": "cargo build > /tmp/log 2>&1", + "run-in-background": true, + "programs": [prog("cargo", ["build"])], + "segments": [seg(["cargo", "build", ">", "/tmp/log", "2>&1"], null, false)], + }} + v.rule == "background-redirect" +} + +# NOTHING IS CAPTURED FOR A FOREGROUND CALL, so a redirect there discards no +# output anyone was going to read. This is the case that keeps the rule from +# becoming a blanket ban on redirection. +test_a_foreground_redirect_is_not_this_rule if { + count(violation) == 0 with input as {"call": { + "command": "cargo build > /tmp/log 2>&1", + "run-in-background": false, + "programs": [prog("cargo", ["build"])], + "segments": [seg(["cargo", "build", ">", "/tmp/log", "2>&1"], null, false)], + }} +} + +# AN INPUT REDIRECT IS UNTOUCHED: reading a file INTO a backgrounded command +# discards nothing. Judged on the redirection TOKENS, and `<` is not one of them. +test_a_backgrounded_input_redirect_is_untouched if { + count(violation) == 0 with input as {"call": { + "command": "cargo build < /tmp/answers", + "run-in-background": true, + "programs": [prog("cargo", ["build"])], + "segments": [seg(["cargo", "build", "<", "/tmp/answers"], null, true)], + }} +} diff --git a/rules/scanning.md b/rules/scanning.md index 2d93c2e65..5f290bee0 100644 --- a/rules/scanning.md +++ b/rules/scanning.md @@ -1,15 +1,82 @@ # Choosing an instrument for a whole-tree question These load when you are about to ask something about the whole tree rather than -about the file in front of you. The question decides the tool, and the three -questions are not interchangeable. - -| the question | instrument | -| ----------------------------------------------------------------------- | ----------------------------- | -| does this file contain this literal string | a structured text search | -| is this token in command position, inside a comment, or inside a string | a tree-sitter matcher | -| which type does this name resolve to | clippy, rust-analyzer, Serena | -| has this already been filed, decided, or measured | the board, before the tree | +about the file in front of you. The question decides the tool, and the five +questions are not interchangeable. Rows one to four choose between instruments +that LOOK at something; row five does not, and the section after the table is +about what its answer licenses. + +| the question | instrument | +| ----------------------------------------------------------------------- | ------------------------------ | +| does this file contain this literal string | a structured text search | +| is this token in command position, inside a comment, or inside a string | a tree-sitter matcher | +| which type does this name resolve to | clippy, rust-analyzer, Serena | +| has this already been filed, decided, or measured | the board, before the tree | +| what does this gate DECIDE over this subject | run it, and read the exit code | + +**Row five is the one that is not a reader at all, and that is why it gets +missed.** The other four choose between instruments that LOOK at something; this +one runs a program and reads its verdict. A question about what a gate _decides_ +is row five; a question about how it is _implemented_ is row one or three. +Reading source to predict a verdict is answering the first with the second's +instrument — and it is worse than guessing, because it looks like rigour: you +cite `file:line`, the reasoning is legible, and it can be confidently wrong. + +**Measured 2026-09-09, three answers to one question.** Why did `checks green` +report `pending` over a head whose every job had succeeded? Reading `pr_watch.rs` +gave a mechanism; reading `checks_green.rs:33-39` gave the opposite and the first +was retracted into two Linear rows; `SHA=… mise run checks-green` printed the +answer in four seconds and the first reading had been right. The retraction had +to be retracted. Row four's sentence applies unchanged one level up: **source +feels like an answer and running the predicate feels like a detour.** + +## Running it answers the GATE's question, which is not always yours + +Row five is right about the instrument and silent about what the reading +_licenses_, and that silence is the next failure along. Two kinds of gate return +the same `0`: + +- **A decider** takes your whole question as its object. `checks green` over a + SHA, `no_artifact_name_reaches_the_core` over the tree, `closing-key-check` + over a PR body. Exit `0` IS the answer, and re-deriving it by hand is the + detour row five names. +- **A floor** takes a NECESSARY condition as its object. `ready lint` over a + Ready block, `config-lint`, every linter. Exit `0` says _no known defect was + found_, and the reasoning it cannot reach is still yours to do. + +**One question separates them, and it is asked of the PAIR — this gate, your +question — never of the gate alone: can this gate be satisfied by content that +is well-formed and false _as an answer to what you are asking_?** If it can, it +is a floor for your question. `crates/batten/src/ready.rs` +says this about itself where `REQUIRED_CLAIMS` is declared — the prose path +"validates the clauses that ARE there and says nothing about absence", and the +claims object exists because "a key cannot be well-formed prose". A key can be +well-formed and untrue, and no exit code reaches that. + +**Measured 2026-09-09.** Four rows this branch filed were refused +`filed-unrefined`. Their Ready blocks were repaired, `ready lint` answered +`satisfies the checkable Ready clauses` on all four, and that was reported to a +human as grooming done. Two of the four had had their required `tests` key +filled with a test file picked off a glob and a mutation name invented to fit — +well-formed, unverifiable by the gate, false. One of those two was a row whose +own §2 said its mechanism was still undecided, which is precisely the case +`REQUIRED_CLAIMS` was built to catch; the fabricated fill hid it. The gate did +its job exactly as designed. The floor was read as a verdict. + +**KIND IS RELATIONAL, WHICH IS WHY THIS STAYS PROSE.** The obvious mechanism — +a column on each rule declaring which kind it is — does not survive contact: +`[budget.instructions]` is a **decider** over "is this surface inside its line +budget" and a **floor** over "is this surface correct", and CLOUD-1687 exists +precisely because it read green at 199 of 199 lines while the content had forked +from the spec it cites. One rule, both roles, settled by which question the +reader brought. A rule cannot declare a property it does not have, and +non-negotiable rule 3 forbids a gate whose object is "which question did you +mean" — that is a judgement. So this row is feedforward by construction rather +than by omission, and CLOUD-1567 owns what a mechanism here could still reach. + +So row five's conclusion is bounded: **run the gate to learn what the gate +decides, then ask whether what it decides is what you were asking.** A decider's +`0` ends the question. A floor's `0` is where your reasoning starts. Row two is the one the tree kept reaching past. Rows one and three both have a habit behind them — `grep` is in every hand, and `rules/rust.md` already