From 75a23a2f41af3b92ca65f87882e8a77f99cc52e4 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Thu, 17 Sep 2026 20:31:49 +0000 Subject: [PATCH 1/3] fix(tests): a fixture must not resolve the repository it runs inside MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `repo_with_config` builds a fixture with a config and no `.git`, and the spawn helpers set no `GIT_CEILING_DIRECTORIES`, so `git::git_dir` walked up out of `target/tmp/` and answered with this checkout's own `.git`. Measured: `.git/batten-sightings/` in the real repository, holding entries the suite wrote. What that buys is an ordering dependency across the whole binary. `refusal::first_sighting` keys a per-session store under `$GIT_DIR`, and a repeat renders SHORT — the class explanation and every remedy dropped. With one store behind every fixture, whether a case sees the long rendering or the short one is decided by which spawn reached that class first: across sibling cases, across targets sharing the checkout, and across whatever the developer's session already did. `the_deny_is_a_function_of_config_and_argv_not_the_ambient_environment` was the visible casualty. It compared a first sighting against a repeat and read the difference as the environment having changed the reason. Each run now gets its own fixture and so its own store, and the case asserts its baseline IS the first-sighting rendering — otherwise it compares two short lines and comes back green having checked almost nothing. `two_fixtures_refusing_alike_render_alike` is the discriminating arm: no environment varied at all, two fixtures, one refusal, identical stderr required. It reds on the tree without the ceiling — a long line against a short one — and passes with it, which is what makes the fix a mechanism rather than a repair. The ceiling is set in `common::batten` rather than per spawn site, which is CLOUD-619's rule for these variables: `advisory_drain.rs` set it by hand at three call sites and every suite that did not think of it inherited the defect. Refs: CLOUD-1830 --- crates/batten/tests/it/cli.rs | 132 +++++++++++++++++++-------- crates/batten/tests/it/common/mod.rs | 29 ++++++ 2 files changed, 123 insertions(+), 38 deletions(-) diff --git a/crates/batten/tests/it/cli.rs b/crates/batten/tests/it/cli.rs index 49c04902e..6f2c8891b 100644 --- a/crates/batten/tests/it/cli.rs +++ b/crates/batten/tests/it/cli.rs @@ -2927,33 +2927,73 @@ fn hook_denies_a_truncating_redirect_against_a_protected_path() { } } +/// One refusal of `rm guarded/thing`, from a fixture of its own. +/// +/// A FIXTURE PER RUN, AND THE REASON IS `first_sighting` (CLOUD-1830). A class +/// explains itself ONCE per session: the second firing renders the pointer line +/// alone, dropping the `— ` clause and every remedy (`hook.rs:4832`). The +/// store that decides it is keyed under `$GIT_DIR`, so two runs against one +/// fixture are a first sighting and a repeat — and comparing their stderr +/// measures which ran first, not what the caller varied. +/// +/// Each run therefore gets its own fixture and so its own store, which makes +/// every one of them a first sighting. `common::batten` sets +/// `GIT_CEILING_DIRECTORIES` for the other half of the same defect: without it a +/// fixture carrying no `.git` resolved the real repository's, and every case in +/// the binary shared one store with the checkout. +fn refuse_in(name: &str, extra: Option<(&str, &str)>) -> Output { + let dir = repo_with_protected_policy(name); + let payload = claude_payload("rm guarded/thing"); + let mut command = batten(); + command + .current_dir(&dir) + .args(["adjudicate", "--harness", "exit-code"]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + if let Some((key, value)) = extra { + command.env(key, value); + } + let mut child = command.spawn().expect("spawn batten hook"); + child + .stdin + .take() + .expect("piped stdin") + .write_all(payload.as_bytes()) + .expect("write payload"); + child.wait_with_output().expect("run batten hook") +} + #[test] fn the_deny_is_a_function_of_config_and_argv_not_the_ambient_environment() { // Acceptance (c): "a repeat attempt with the sandbox disabled is still // denied" — i.e. the verdict is computed from config plus argv, so nothing // ambient can turn it off. Asserted by varying the environment around an // identical payload and config and requiring byte-identical answers. - let dir = repo_with_protected_policy("protected-deterministic"); - let payload = claude_payload("rm guarded/thing"); - let baseline = run_hook_in(&dir, "exit-code", &payload, false); - // THE SAME BUILDER AS THE BASELINE, WHICH IS THE WHOLE ASSERTION (CLOUD-1821). - // `run_hook_in` spawns through `batten_at_real_root()` — `batten()` plus a - // state root of the suite's own — while this loop used bare `batten()`. So the - // varied runs read the AMBIENT state root and the baseline read an isolated - // one: the two sides differed in a way that has nothing to do with the - // variable under test, and the case compared two harnesses while claiming to - // compare two environments. - // - // It surfaced as a missing remedy tail. A refusal already seen in the ambient - // root is emitted in its short form (CLOUD-1286's ceiling), so the varied - // `stderr` lost "— a mutating verb was aimed at a path the config protects; …" - // on exactly the runs that had one. Green wherever the ambient root happened - // to be cold, red on the musl leg, and never about `BATTEN_SANDBOX` at all — - // `scratch_state_root`'s own doc names this as the defect it exists to close. // - // `HOME` stays in the list and now means something: with the state root - // pinned, a changed `HOME` must NOT move the verdict, which is the claim. It - // points at a scratch directory rather than `/tmp` so it belongs to this case + // WHAT THIS CASE COULD NOT SEE UNTIL CLOUD-1830. Every run used to share one + // sightings store, so the baseline was a first sighting and each varied run a + // repeat — and a repeat renders short. The comparison was therefore between + // two different RENDERINGS of the same verdict, which it read as the + // environment having changed the reason. It was green or red on scheduling: + // the musl leg's ordering reddened it, and that looked like a libc divergence + // until the same four runs, with the store cleared between them, came back + // byte-identical on both targets. + let baseline = refuse_in("protected-deterministic-baseline", None); + assert_eq!( + baseline.status.code(), + Some(2), + "the baseline must be the deny this case varies around: {}", + String::from_utf8_lossy(&baseline.stderr) + ); + assert!( + String::from_utf8_lossy(&baseline.stderr).contains('—'), + "and it must be the FIRST-SIGHTING rendering, or the comparison below is \ + between two short lines and asserts almost nothing: {}", + String::from_utf8_lossy(&baseline.stderr) + ); + + // `HOME` gets a scratch directory rather than `/tmp`: it belongs to this case // instead of to every process on the machine. let scratch_home = common::scratch("protected-deterministic-home"); let scratch_home = scratch_home.display().to_string(); @@ -2963,24 +3003,10 @@ fn the_deny_is_a_function_of_config_and_argv_not_the_ambient_environment() { ("NO_COLOR", "1"), ("HOME", scratch_home.as_str()), ] { - let mut command = common::batten_at_real_root(); - command - .current_dir(&dir) - .args(["adjudicate", "--harness", "exit-code"]) - .env_remove("BATTEN_HOOK_BYPASS") - .env_remove("BATTEN_GH_GUARD_BYPASS") - .env(key, value) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()); - let mut child = command.spawn().expect("spawn batten hook"); - child - .stdin - .take() - .expect("piped stdin") - .write_all(payload.as_bytes()) - .expect("write payload"); - let output = child.wait_with_output().expect("run batten hook"); + let output = refuse_in( + &format!("protected-deterministic-{}", key.to_lowercase()), + Some((key, value)), + ); assert_eq!( output.status.code(), baseline.status.code(), @@ -2993,6 +3019,36 @@ fn the_deny_is_a_function_of_config_and_argv_not_the_ambient_environment() { } } +// THE ARM THAT MAKES THE CEILING FIX A MECHANISM (CLOUD-1830). Two fixtures, no +// environment varied at all, one identical refusal: their stderr must match. +// +// It fails on the tree that has this defect. Without +// `GIT_CEILING_DIRECTORIES` a fixture carrying no `.git` resolves the enclosing +// repository's, both runs share one sightings store, the second is a repeat and +// renders short — so the assertion reds with a long line against a short one. +// With the ceiling set each fixture answers for itself and both are first +// sightings. +// +// SEPARATE FROM THE CASE ABOVE, deliberately: that one varies the environment and +// would still pass if both of its sides were short. This one varies NOTHING, so +// the only thing it can detect is the coupling — which is what makes it the +// discriminating half rather than a second copy. +#[test] +fn two_fixtures_refusing_alike_render_alike() { + let first = refuse_in("protected-alike-first", None); + let second = refuse_in("protected-alike-second", None); + assert_eq!( + first.status.code(), + second.status.code(), + "two fixtures, one refusal, two verdicts" + ); + assert_eq!( + first.stderr, second.stderr, + "two fixtures refusing alike must render alike; a difference here means \ + they are sharing a sightings store — see CLOUD-1830" + ); +} + #[test] fn the_committed_protected_paths_fire_on_a_mutating_verb() { // The same obligation the shape rows carry: every other protected-path test diff --git a/crates/batten/tests/it/common/mod.rs b/crates/batten/tests/it/common/mod.rs index 0ea0dea99..a30281ef3 100644 --- a/crates/batten/tests/it/common/mod.rs +++ b/crates/batten/tests/it/common/mod.rs @@ -272,6 +272,35 @@ pub(crate) fn batten() -> Command { command.env_remove(name); } command.env("BATTEN_BIN", env!("CARGO_BIN_EXE_batten")); + // A FIXTURE MUST NOT RESOLVE THE REPOSITORY IT IS RUNNING INSIDE (CLOUD-1830). + // + // Fixtures live under `target/tmp/`, which is INSIDE this checkout, and most + // of them carry no `.git` of their own — `repo_with_config` (cli.rs) builds + // one with a config and nothing else. `git::git_dir` therefore walked up and + // answered with the real repository's `.git`, so anything the engine files + // per-worktree went there: measured, `/home/user/batten/.git/batten-sightings` + // held entries written by the suite. + // + // THE COUPLING THAT BUYS, stated because a shared directory sounds harmless. + // `refusal::first_sighting` keys a per-session store under `$GIT_DIR` and a + // repeat renders SHORT — the class explanation and every remedy are dropped + // (`hook.rs:4832`). With one store behind every fixture, whether a case sees + // the long form or the short one is decided by which spawn in the binary + // reached that class first: across sibling cases, across targets sharing the + // checkout, and across whatever the developer's own session already did. + // `the_deny_is_a_function_of_config_and_argv_not_the_ambient_environment` was + // green or red on that ordering alone, and read as a musl divergence for a + // while because the musl leg simply scheduled differently. + // + // HERE RATHER THAN AT EACH SPAWN SITE, which is CLOUD-619's rule for exactly + // these variables: `advisory_drain.rs` set this by hand at three call sites + // and every suite that did not think of it inherited the defect. One place, + // so no suite has to remember. + // + // The real-root suites are unaffected: `batten_at_real_root` runs with the + // repository root as the working directory, which resolves without traversing + // past this ceiling. + command.env("GIT_CEILING_DIRECTORIES", target_tmp()); command } From 3fa1ecebfb2a8f2cc7dde5bef39bea3a5218a800 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Thu, 17 Sep 2026 20:32:06 +0000 Subject: [PATCH 2/3] feat(ci): restore the musl leg, with the defect it found repaired MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reinstates `f588a788`'s revert. `test:musl` executes the workspace suite against `x86_64-unknown-linux-musl` — the triple `install.sh` resolves for every Linux consumer, 113 downloads against 4 of the glibc build on v0.0.159 — so the binary almost everyone runs reaches the execution rung instead of stopping at a type-check. The leg was lifted rather than weakened because the defect it exposed made `verify` red, and `verify` gates every fix. That defect is repaired in the commit beside this one: fixtures were resolving this checkout's own `.git`, so one sightings store sat behind every case and a class explained itself once per binary rather than once per fixture. The leg did not diverge; it scheduled differently, which is all it took. Refs: CLOUD-1821 --- .github/workflows/release-plz.yml | 55 +++++++++++- .github/workflows/rust.yml | 136 ++++++++++++++++++++++++++++++ mise.toml | 56 +++++++++++- 3 files changed, 243 insertions(+), 4 deletions(-) diff --git a/.github/workflows/release-plz.yml b/.github/workflows/release-plz.yml index 60be27e7d..32727c719 100644 --- a/.github/workflows/release-plz.yml +++ b/.github/workflows/release-plz.yml @@ -319,6 +319,53 @@ jobs: env: DOCTOR_TARGETS: x86_64-pc-windows-gnu + # The `musl-` family's one writer, for `cache-warm-cross`'s reason rather than + # by analogy: `rust.yml`'s `musl` job reads that family `save-if: false`, so + # without a trunk-side writer the entry is never filled and every pull request + # pays a cold build of the workspace against a second target. `mise run + # test:musl` is exactly what the reader runs, which is the condition the + # `semver-` note below says a warm job must meet before it is worth having. + cache-warm-musl: + name: cache-warm-musl + 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-17 + # 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: + # The reader checks out the submodule for the reason its own comment + # gives — `crates/batten/tests` carries a suite about that path — so a + # writer without it fills a different build's artifacts. + submodules: true + 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 `musl` job installs, for its stated reason: `tests/cli.rs` + # materializes fixtures carrying this repository's `batten.toml`, whose + # rules spawn hk and jq. + install_args: rust hk aqua:jqlang/jq github:nextest-rs/nextest + - 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 `musl` job exactly or this writes an entry + # that job cannot read. + shared-key: musl- + # Compile only when there is nothing to restore, as every warm job here + # does. + - run: mise run test:musl + if: steps.rust-cache.outputs.cache-hit != 'true' + cache-warm-darwin-link: name: cache-warm-darwin-link runs-on: ubuntu-latest @@ -334,7 +381,13 @@ jobs: strategy: fail-fast: false matrix: - target: [aarch64-apple-darwin] + # BOTH LEGS, because the reader now has both. `rust.yml`'s `darwin-link` + # gained `x86_64-apple-darwin` (CLOUD-364) and this matrix did not follow + # it — `read-family-has-a-warm-writer` reported the orphan against + # `rust.yml:206` on the very next run, which is the gate doing exactly + # what the note above describes and the reason a family is resolved from + # the `shared-key` text rather than from its expansion. + target: [aarch64-apple-darwin, x86_64-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 diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 50db44e9a..a36567a71 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -189,6 +189,142 @@ jobs: # additionally pull the Darwin std, which darwin-link's own job owns. DOCTOR_TARGETS: x86_64-pc-windows-gnu + # THE RUNG ABOVE `cross`, AND THE ONLY ONE THAT COSTS NOTHING TO REACH + # (CLOUD-1821). `cross` type-checks `x86_64-unknown-linux-musl` and this + # EXECUTES it: a statically linked musl binary runs on a glibc host, so the + # suite runs on the ubuntu runner already paid for — no container, no emulator, + # no foreign runner. A type-check cannot see a linker problem and neither can + # see a behavioural one, which is exactly the gap between the two jobs. + # + # IT IS THE TRIPLE THAT MATTERS MOST. `install.sh` resolves this one for every + # Linux consumer — 113 downloads against 4 of the glibc build on v0.0.159 — so + # until this job existed the binary almost everyone runs was covered by the fact + # that it compiled, and the one almost nobody runs was the only one whose suite + # had ever executed. + # + # ITS OWN JOB, NOT A SECOND STEP IN `ci`. A musl failure folded into `ci` reads + # as a workspace failure; a distinct check-run name is what makes the libc the + # identified variable. Same reason `darwin-link` runs its legs under + # `fail-fast: false` rather than as a loop. + # + # `mise run test:musl`, NOT `mise exec -- cargo …`. This runner is + # `ubuntu-latest`, which `ci-local-parity` does not class as foreign, so + # property 3 applies in full and the task must be one `verify` runs by name. The + # `mise exec` spelling `windows` and `macos` use exists BECAUSE they are exempt; + # borrowing it here would give up a parity that is available. + musl: + name: musl + if: ${{ github.event.pull_request.draft == false }} + runs-on: ubuntu-latest + # GRANDFATHERED, AND HONESTLY SO: no run of this job exists, so there is no + # quantile to derive from and any number here would be invented. `mise run + # timeout-drift` re-derives it once the series is long enough to have a p95. + # `windows`'s comment below records what a ceiling taken from n=0 — or, worse, + # from n=1 — costs. + timeout-minutes: 30 # budget: grandfathered measured=2026-09-17 + # `actions: write` for one call only: cancelling THIS run when the landing + # lease does not authorise the branch (CLOUD-420). A job that could not cancel + # itself would have to fail, and a failed job concludes the run `failure` + # rather than `cancelled` — which reds `final` and makes `land` re-draft a + # healthy PR. + permissions: + contents: read + actions: write + steps: + # CLOUD-420: THE LANDING LEASE, ENFORCED WHERE THE MONEY IS SPENT. First + # step, before any checkout or toolchain install, so a run this branch is + # not authorised to make costs the rounding rather than a suite. Body + # fetched from `main`, never from this head, and `|| exit 0` on every line + # so a body that will not parse cannot red the first step of every job. + # Every justification lives on `run_lease_guard` and on the `cross` job + # above, which is the copy this one follows rather than re-argues. + - name: Landing lease precondition + env: + GH_TOKEN: ${{ github.token }} + GH_REPO: ${{ github.repository }} + LEASE_HEAD_REF: ${{ github.head_ref }} + # The HEAD sha, never `github.sha`: on a pull_request event that is the + # merge commit, whose tree is trunk's wherever this head did not touch + # it — and the staleness read asks about the head's own landing paths. + LEASE_HEAD_SHA: ${{ github.event.pull_request.head.sha }} + LEASE_RUN_ID: ${{ github.run_id }} + run: | + installer=$(gh api -H "Accept: application/vnd.github.raw" \ + "repos/$GH_REPO/contents/install.sh?ref=main") || exit 0 + printf '%s\n' "$installer" | \ + BATTEN_VERSION_FROM_REF=main BATTEN_INSTALL_DIR="$RUNNER_TEMP/batten-bin" \ + BATTEN_ALLOW_OFF_PATH=1 BATTEN_REQUIRE="lease guard" sh || exit 0 + # THE POLICY IS TRUNK'S TOO. This step runs BEFORE any checkout, so the + # directory the guard stands in is empty and `config::load` would find + # nothing — `[lease] landing_paths` reading as *no paths declared* is + # the silence the row was written to end. Fetched from `main` for the + # installer's own reason: a head must not pin the policy it is judged + # by. + mkdir -p "$RUNNER_TEMP/batten-config" || exit 0 + gh api -H "Accept: application/vnd.github.raw" \ + "repos/$GH_REPO/contents/batten.toml?ref=main" \ + >"$RUNNER_TEMP/batten-config/batten.toml" || exit 0 + "$RUNNER_TEMP/batten-bin/batten" --config-in "$RUNNER_TEMP/batten-config" \ + lease guard \ + "$LEASE_HEAD_SHA" "$LEASE_HEAD_REF" "$LEASE_RUN_ID" || exit 0 + + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + # The LIBC must be the only variable, which is this job's entire value. + # A missing `tests/bats` would be a second one — `crates/batten/tests` + # carries a suite about that path — and it would read as a musl finding, + # which is the misattribution this job exists to avoid. The same + # sentence, for the same reason, sits on `windows` and `macos`. + submodules: true + - 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 + # THE LIST EVERY LEG THAT RUNS THE SUITE INSTALLS, for the reason + # `windows` records rather than by copying: `tests/cli.rs` materializes + # fixtures carrying this repository's `batten.toml`, whose + # `no-conflict-markers` rule is `hk util check-merge-conflict`, so a + # fixture that loads that config cannot evaluate ANY rule without hk. + # `jq` is read at runtime by the mise tasks three more command rules + # invoke. Neither is about the libc, which is why both belong here. + install_args: rust hk aqua:jqlang/jq github:nextest-rs/nextest + - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + with: + # A DISTINCT FAMILY, because this target dir holds musl artifacts the + # `ci` family's glibc entry does not — sharing one would thrash rather + # than warm, which is `cross`'s and `semver`'s reason rather than an + # analogy. SHARED rather than keyed so a job in another workflow can + # fill it (CLOUD-840): without `shared-key` the job id enters the key + # and a warm job under any other name computes a different one. + shared-key: musl- + # READ-ONLY: `cache-warm-musl` on `main` is this family's one writer. A + # write from a `pull_request` run lands under `refs/pull/N/merge` where + # no other pull request can read it — CLOUD-1453 measured 81% of a + # 10.77 GiB store as exactly that. + save-if: false + # A JOB THAT DECLARES A TOOL MUST BE ABLE TO REACH IT, asserted before the + # suite rather than discovered inside it — `ci-tools-check` holds these + # names against `mise.toml`, which cannot know whether a runner resolves + # one. The alternative is measured on `windows`: three `tests/cli.rs` cases + # reporting an exit code about a missing tool while naming the rule they + # meant to test, each costing a round trip to attribute. + - name: Assert the declared tools resolve + shell: bash + run: | + set -euo pipefail + echo "PATH=$PATH" + for tool in hk jq cargo cargo-nextest; do + if ! resolved=$(command -v "$tool"); then + echo "::error::$tool is declared in install_args but does not resolve on PATH" >&2 + exit 1 + fi + echo "$tool -> $resolved" + done + hk --version + - run: mise run test:musl + # The macOS gate. `cargo check` never links, so it cannot see a dependency that # needs an Apple SDK; this LINKS a Darwin target, which is the only check with # no false negatives. It runs concurrently with ci/cross, and the Darwin diff --git a/mise.toml b/mise.toml index 23382d0f6..bd09d20ab 100644 --- a/mise.toml +++ b/mise.toml @@ -393,7 +393,7 @@ _.path = ["tests/bats/bin", "target/release"] # alone — the fan-in. This roster is what `ci-wait` WAITS ON before landing, and # waiting only on `final` would strand every landing on a check that fans in from # eighteen. Two sets, two purposes; neither derives the other. -CI_REQUIRED_CHECKS = "ci,batten-check,bats,cross,commit-lint,zizmor,darwin-link (aarch64-apple-darwin),darwin-link (x86_64-apple-darwin),semver,perf,windows,macos,final,action (ubuntu-latest),action (macos-latest),action (windows-latest),action-violation,action-deny,action-usage,action-internal (ubuntu-latest),action-internal (macos-latest),action-final" +CI_REQUIRED_CHECKS = "ci,batten-check,bats,cross,musl,commit-lint,zizmor,darwin-link (aarch64-apple-darwin),darwin-link (x86_64-apple-darwin),semver,perf,windows,macos,final,action (ubuntu-latest),action (macos-latest),action (windows-latest),action-violation,action-deny,action-usage,action-internal (ubuntu-latest),action-internal (macos-latest),action-final" # ─── THE LAP'S READY PHASE (CLOUD-1148) ─────────────────────────────────────── # @@ -519,7 +519,7 @@ LEASE_STOP_NOTE = "mise-tasks/reclaim-census.sh note x land-stopped" # that one success as the whole roster, and `land` posted /fast-forward into a # branch protection still listing the other six as expected. The bot was # rejected. A stall is recoverable; a false green is not (CLOUD-337). -CI_ABSENT_OK_CHECKS = "zizmor,cross,darwin-link (aarch64-apple-darwin),darwin-link (x86_64-apple-darwin),semver,windows,macos,action (ubuntu-latest),action (macos-latest),action (windows-latest),action-violation,action-deny,action-usage,action-internal (ubuntu-latest),action-internal (macos-latest),action-final" +CI_ABSENT_OK_CHECKS = "zizmor,cross,musl,darwin-link (aarch64-apple-darwin),darwin-link (x86_64-apple-darwin),semver,windows,macos,action (ubuntu-latest),action (macos-latest),action (windows-latest),action-violation,action-deny,action-usage,action-internal (ubuntu-latest),action-internal (macos-latest),action-final" # The conclusions that CONSTITUTE AN ANSWER about a required check, written once # and read by `checks-green` and by `land`'s `graded_runs` — for exactly the # reason CI_REQUIRED_CHECKS is written once (CLOUD-327), and against exactly the @@ -3318,6 +3318,48 @@ done ./mise-tasks/step-receipt.sh record cross-check || true ''' +[tasks."test:musl"] +description = "Run the workspace suite against x86_64-unknown-linux-musl — the triple install.sh resolves for every Linux consumer (CLOUD-1821)" +# The `rustup target add` below is not idempotent against a half-installed +# component; `doctor` is what makes it one — `cross-check`'s reason, one row up. +depends = ["doctor"] +# THE RUNG ABOVE `cross-check`, FOR THE ONE TARGET THAT REACHES IT FOR FREE. +# `cross-check` type-checks this triple and `darwin-link` links two more, but a +# type-check cannot see a linker problem and a link cannot see a behavioural one. +# EXECUTION closes both, and a statically linked musl binary runs on a glibc +# host — so this triple reaches the top rung on the runner and the laptop already +# paid for, with no container, no emulator and no foreign runner. +# +# IT IS ALSO THE TRIPLE THAT MATTERS MOST. `install.sh` resolves +# `x86_64-unknown-linux-musl` for every Linux consumer: 113 downloads of it +# against 4 of the glibc build on v0.0.159. Until this task existed, the binary +# almost everyone runs was covered by the fact that it compiled, and the one +# almost nobody runs was the only one whose suite had ever executed. +# +# WHY NOT A `--target` ON `test:cargo`. That task's single cargo statement is read +# by `ci-local-parity` property 16, which requires a foreign leg to spell +# `mise exec -- ` plus exactly it — so a target added there would make `windows` +# and `macos` run the LINUX musl suite. Two separable questions, two tasks: does +# the suite pass on this OS, and does it pass against this libc. +# +# NOT receipt-routed, unlike `test:cargo`, and the distinction is the KEY rather +# than the cost: a receipt keyed on the same sources, command and toolchain would +# be minted by whichever suite ran first and then answer for the other. Same +# reason `test:filter` and the four `hk` tiers above decline one. +run = ''' +# Guarded by hand because a task body does not run under `set -e` +# (rules/toolchain.md). Routed through target-ensure so a concurrent toolchain +# mutator queues on the lock instead of colliding inside rustup (CLOUD-220). +if ! mise run target-ensure x86_64-unknown-linux-musl; then + echo "::error:: test:musl: could not install the musl target, so its suite did not run." >&2 + exit 1 +fi +if ! cargo nextest run --workspace --target x86_64-unknown-linux-musl; then + echo "::error:: test:musl: the suite fails against x86_64-unknown-linux-musl — the triple install.sh resolves for every Linux consumer." >&2 + exit 1 +fi +''' + [tasks.hk-version] description = "Gate: hk's version is pinned identically in mise.toml and hk.pkl (amends URL). A bump to one without the other is the moment to revisit the hook config." shell = "bash -c" # uses grep -oE + cut; keep off /bin/sh for consistency @@ -4162,6 +4204,14 @@ shell = "bash -c" # completeness question it was named for is now `lock-complete`, a pure gate in # hk.pkl; currency runs on a schedule (.github/workflows/lock-currency.yml). # +# `test:musl` IS HERE BECAUSE ITS CI LEG IS NOT FOREIGN (CLOUD-1821). The job runs +# on `ubuntu-latest`, so `ci-local-parity` property 3 applies in full — every task +# CI runs is one `verify` runs — and the exemption `windows` and `macos` take does +# not reach it. That is the property working rather than a cost: this suite CAN be +# run locally, so a failure in it is one a free local run catches before a runner +# does. It costs no wall clock that `ci` does not already cost, since `depends` +# runs its entries in parallel. +# # `tree-clean` is here AND in the body, and the duplication is the design # (CLOUD-277). Here it fails a dirty tree in seconds rather than after the ~170s # the rest of this list costs. In the body it is load-bearing: `depends` runs @@ -4171,7 +4221,7 @@ shell = "bash -c" # window, the same shape `linear-check` already has in `verify` and # `ready-guard`. It is deliberately NOT an hk step: `pre-commit` runs over a # tree that is dirty by definition. -depends = ["tree-clean", "ci", "cross-check", "darwin-link", "zizmor", "semver"] +depends = ["tree-clean", "ci", "cross-check", "darwin-link", "test:musl", "zizmor", "semver"] run = ''' # A task body does not run under `set -e` (see rules/toolchain.md), so # every step whose failure changes the verdict is guarded by hand. From 2f5507af873ba67593606d338d9624c4f41b7ef9 Mon Sep 17 00:00:00 2001 From: Alec Wenzowski Date: Thu, 17 Sep 2026 21:11:21 +0000 Subject: [PATCH 3/3] fix(tests): the refusal cases own their repository, and so their store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `refusal::first_sighting` keys a per-session store under `$GIT_DIR`: a class explains itself once, and the next firing renders the pointer line alone, dropping the `— ` clause and every remedy (`hook.rs:4832`). `repo_with_config` builds a fixture with a config and no `.git`, so `git::git_dir` walked up out of `target/tmp/` and answered with this checkout's own — measured, `.git/batten-sightings/` in the real repository held entries the suite wrote. Every such fixture shared one store, so two runs of one refusal were a first sighting and a repeat, and `the_deny_is_a_function_of_config_and_argv_not_the_ambient_environment` compared their renderings and read the difference as the environment having changed the reason. Green or red on nextest's scheduling. The fixtures behind these cases now carry their own repository, so each owns its store and every refusal is a first sighting. A tree-wide `GIT_CEILING_DIRECTORIES` was tried first and is wrong, recorded because the next reader will reach for it: `acceptance_corpus`'s fixtures run `enforce`, which needs a repository, and they legitimately inherit the enclosing one — cutting that off reds them with "is not a git repository". `two_fixtures_refusing_alike_render_alike` is the discriminating arm: two fixtures, one refusal, no environment varied, identical stderr required AND both required to be the first-sighting rendering. That second half is load-bearing — without it two fixtures sharing a warm store are both short and equal, and the case stayed green against a tree with no per-fixture repository at all. Shown able to fail: with the repositories removed it reds naming the repeat. Refs: CLOUD-1830 --- crates/batten/tests/it/cli.rs | 67 ++++++++++++++++++++-------- crates/batten/tests/it/common/mod.rs | 29 ------------ 2 files changed, 49 insertions(+), 47 deletions(-) diff --git a/crates/batten/tests/it/cli.rs b/crates/batten/tests/it/cli.rs index 6f2c8891b..72d8de4b9 100644 --- a/crates/batten/tests/it/cli.rs +++ b/crates/batten/tests/it/cli.rs @@ -2850,10 +2850,11 @@ fn hook_honours_a_shape_rule_a_local_override_added() { } /// A fixture repo declaring the CLOUD-96 cross product: two verbs, one path. -fn repo_with_protected_policy(name: &str) -> PathBuf { - repo_with_config( - name, - r#"version = 1 +/// The protected-path policy these cases refuse against. +/// +/// Hoisted to a constant so `refuse_in` can build a fixture that owns its `.git` +/// without duplicating the rows — see that helper for why the repository matters. +const PROTECTED_POLICY: &str = r#"version = 1 protected = ["guarded/**"] [[verb]] @@ -2865,8 +2866,10 @@ redirect = "restore it with git" verb = ">" effect = "destructive" redirect = "append instead" -"#, - ) +"#; + +fn repo_with_protected_policy(name: &str) -> PathBuf { + repo_with_config(name, PROTECTED_POLICY) } #[test] @@ -2927,22 +2930,33 @@ fn hook_denies_a_truncating_redirect_against_a_protected_path() { } } -/// One refusal of `rm guarded/thing`, from a fixture of its own. +/// One refusal of `rm guarded/thing`, from a fixture that owns its own `.git`. /// -/// A FIXTURE PER RUN, AND THE REASON IS `first_sighting` (CLOUD-1830). A class +/// A REPOSITORY PER RUN, AND `first_sighting` IS THE REASON (CLOUD-1830). A class /// explains itself ONCE per session: the second firing renders the pointer line /// alone, dropping the `— ` clause and every remedy (`hook.rs:4832`). The -/// store that decides it is keyed under `$GIT_DIR`, so two runs against one -/// fixture are a first sighting and a repeat — and comparing their stderr -/// measures which ran first, not what the caller varied. -/// -/// Each run therefore gets its own fixture and so its own store, which makes -/// every one of them a first sighting. `common::batten` sets -/// `GIT_CEILING_DIRECTORIES` for the other half of the same defect: without it a -/// fixture carrying no `.git` resolved the real repository's, and every case in -/// the binary shared one store with the checkout. +/// store that decides it is keyed under `$GIT_DIR` (`refusal.rs:367`). +/// +/// `repo_with_config` builds a fixture with a config and NO `.git`, so +/// `git::git_dir` walked up out of `target/tmp/` and answered with this +/// checkout's own — measured, `.git/batten-sightings/` in the real repository +/// held entries the suite wrote. Every fixture therefore shared one store, so two +/// runs of one refusal were a first sighting and a repeat, and comparing their +/// stderr measured which ran first rather than what the caller varied. It was +/// green or red on nextest's scheduling, and read as a musl divergence for a +/// while because the musl leg simply scheduled differently. +/// +/// Giving the fixture its own repository is the narrow fix and the right one. A +/// tree-wide `GIT_CEILING_DIRECTORIES` was tried and is WRONG: fixtures like +/// `acceptance_corpus`'s run `enforce`, which needs a repository, and they +/// legitimately inherit the enclosing one — cutting that off reds them with +/// "is not a git repository". fn refuse_in(name: &str, extra: Option<(&str, &str)>) -> Output { - let dir = repo_with_protected_policy(name); + let dir = Fixture::new(name) + .config(PROTECTED_POLICY) + .git() + .base_commit() + .build(); let payload = claude_payload("rm guarded/thing"); let mut command = batten(); command @@ -3047,6 +3061,23 @@ fn two_fixtures_refusing_alike_render_alike() { "two fixtures refusing alike must render alike; a difference here means \ they are sharing a sightings store — see CLOUD-1830" ); + // AND BOTH MUST BE THE FIRST-SIGHTING RENDERING. Equality alone is not the + // property: two fixtures sharing a warm store are both REPEATS, both short, + // and equal — so the assertion above passes while the coupling it exists to + // catch is fully present. Measured: without this line the case stayed green + // against a tree with no per-fixture repository at all. + // + // A fixture that owns its `.git` owns its store, so its refusal is always a + // first sighting and always carries the class clause. That is the observable + // difference between owning one and borrowing the repository's. + for (which, output) in [("first", &first), ("second", &second)] { + let rendered = String::from_utf8_lossy(&output.stderr); + assert!( + rendered.contains('—'), + "the {which} fixture rendered a REPEAT, so it is reading a store some \ + other run already warmed — see CLOUD-1830: {rendered}" + ); + } } #[test] diff --git a/crates/batten/tests/it/common/mod.rs b/crates/batten/tests/it/common/mod.rs index a30281ef3..0ea0dea99 100644 --- a/crates/batten/tests/it/common/mod.rs +++ b/crates/batten/tests/it/common/mod.rs @@ -272,35 +272,6 @@ pub(crate) fn batten() -> Command { command.env_remove(name); } command.env("BATTEN_BIN", env!("CARGO_BIN_EXE_batten")); - // A FIXTURE MUST NOT RESOLVE THE REPOSITORY IT IS RUNNING INSIDE (CLOUD-1830). - // - // Fixtures live under `target/tmp/`, which is INSIDE this checkout, and most - // of them carry no `.git` of their own — `repo_with_config` (cli.rs) builds - // one with a config and nothing else. `git::git_dir` therefore walked up and - // answered with the real repository's `.git`, so anything the engine files - // per-worktree went there: measured, `/home/user/batten/.git/batten-sightings` - // held entries written by the suite. - // - // THE COUPLING THAT BUYS, stated because a shared directory sounds harmless. - // `refusal::first_sighting` keys a per-session store under `$GIT_DIR` and a - // repeat renders SHORT — the class explanation and every remedy are dropped - // (`hook.rs:4832`). With one store behind every fixture, whether a case sees - // the long form or the short one is decided by which spawn in the binary - // reached that class first: across sibling cases, across targets sharing the - // checkout, and across whatever the developer's own session already did. - // `the_deny_is_a_function_of_config_and_argv_not_the_ambient_environment` was - // green or red on that ordering alone, and read as a musl divergence for a - // while because the musl leg simply scheduled differently. - // - // HERE RATHER THAN AT EACH SPAWN SITE, which is CLOUD-619's rule for exactly - // these variables: `advisory_drain.rs` set this by hand at three call sites - // and every suite that did not think of it inherited the defect. One place, - // so no suite has to remember. - // - // The real-root suites are unaffected: `batten_at_real_root` runs with the - // repository root as the working directory, which resolves without traversing - // past this ceiling. - command.env("GIT_CEILING_DIRECTORIES", target_tmp()); command }