Skip to content

perf(ci): give the shell suite its own runner, and gate the carve-out that lets it - #742

Closed
wenzowski wants to merge 2 commits into
mainfrom
claude/cloud-1140-suite-own-runner
Closed

wenzowski wants to merge 2 commits into
mainfrom
claude/cloud-1140-suite-own-runner

Conversation

@wenzowski

Copy link
Copy Markdown
Contributor

Closes CLOUD-1140.

What this is

CLOUD-398 was closed Done on #606, which landed slice 2 only — the four pure-Rust jobs made absent on a diff they cannot judge. Slice 1, moving the shell suite off the shared runner, was fully refined and never implemented. Three of that issue's acceptance clauses are unmet verbatim:

  • "The union of the CI jobs' hk steps is the whole gate, proven by a gate rather than by reading, and shown able to fail."
  • "A step carrying no lane, or two, is red before CI."
  • "ci wall clock is measurably lower, and the runner-seconds delta is reported rather than assumed."

This is that slice.

The arithmetic moved by 6x

CLOUD-398 costed the split on an 857s gate with a 697s suite and concluded ~−167s, −20% — real but thin against a metered runner. Measured 2026-08-29 on run 33244045030 (green):

layer cost share
whole CI run 1992s —
the ci job 1983s 99.5% — perf finished at 9.2 min and idled for 24 more
mise run ci inside it 1832s 92.4% of the job
setup, checkout, disk reclaim, caches, teardown 151s 7.6%
test:bats inside mise run ci ~1520s ~83%

ci is the entire critical path. CLOUD-386 swept the worker count at the runner's real width and found every direction worse, because each point redistributes workers inside one saturated 2-core box. Its own point D measured the number this needs: the suite alone with the box to itself is 623s, against 829s contended. A second runner is the experiment that sweep could not contain, and it turns the gate's wall clock from a sum of two chains into a max — paid twice per land lap, since verify runs the same gate locally.

Why HK_SKIP_STEPS, and not the two obvious alternatives

  • A profile cannot express it. hk resolves multiple profiles on a step with AND and the CLI value supplants the config list, so carving one step out needs the whole lane partition CLOUD-398 costed at ~160s of duplication and explicitly sequenced after this.
  • A second mise task would put a name in the workflow that verify does not run — ci-local-parity property 3. So test:bats is named in verify:gated's depends instead, beside the ci/ci:quick pair that has overlapped there for as long as the quick tier has existed. Naming it in a comment would satisfy the grep and nothing else, which is the shape that gate's own header condemns.

The false green this ships its own gate for

hook-profile-check reads hk check --all --plan and asserts every slow-tagged step is included there. It cannot see what a workflow job passes — which is why it reads .claude/hooks/git-hook.sh's invocation line for its other half.

Measured: HK_SKIP_STEPS=test:bats takes that plan from 58 steps to 57 with reason env_exclude, while the gate's own reading stays green. So the suite could be carved out of the ci job and run by no job at all, with every gate in this repository green over a suite that never executed.

ci-suite-lane refuses exactly that direction. The opposite mistake — a name matching no step — runs the step in both jobs, which shows up in the bill and needs no gate because it has a symptom.

A Rego module rather than a shell gate because shell-retirement governs mise-tasks/** and tests/**/*.bats on the added side as well as the edited one (delta.added, mutant row shell-added-unchecked), so the engine is the only route a new predicate has. Hence the MUTANT-EXEMPT row and the compiled-binary second tier.

bats-invocation had to move with it

Its install_args clause asserted rush was installed in the ci job. That was correct exactly while the ci job ran the suite; after the move it would have been green over a bats job with no rush — the same false green one layer over. The job is now derived from which one invokes mise run test:bats, so it follows the suite wherever it goes next. Its fixture changed too: the old one carried an install list and no run, so with the job derived it could no longer reach the assertion it exists for.

Files

file change
.github/workflows/ci.yml ci job sets HK_SKIP_STEPS: test:bats; new bats job; final needs [ci, bats, perf]
mise.toml bats in CI_REQUIRED_CHECKS; test:bats in verify:gated's depends
policy/ci-suite-lane.rego new module, 7 load-time cases
policy/bats-invocation.rego the install clause derives its job
batten.toml the rule row and two [[verdict]] rows with routes
crates/batten/tests/ci_suite_lane.rs compiled-binary tier, 7 cases

Verification

Green locally before pushing: policy-test (22 bundles, 254 cases), batten-check (exit 0 over this tree), actionlint, ci-tools-check, timeout-check (39 jobs), ci-local-parity (all 17 properties), and the full hk fast tier at commit time.

The measurement CI itself has to return is the one no local run can give: the ci and bats jobs' own durations. The wall-clock delta and the runner-seconds delta both get reported on this PR, whichever way they fall — a negative result closes CLOUD-398's deferred question rather than being dropped.

Not in this change

  • The lane partition over all ~58 steps. CLOUD-398 costed it at ~160s of duplication and sequenced it after this; without it the ci job still runs the cheap steps the bats job's preamble also pays for. Judgement call recorded on CLOUD-1140 §2.
  • Runner width. All 38 runs-on: in this repository are ubuntu-latest = 2 cores, and the 2026-08-11 sweep measured the same suite at 210.7s (2 workers) vs 100.3s (4) — a 2.1x from cores alone. That is a billing decision, not a code one. Judgement call recorded on CLOUD-1140 §2.
  • Re-deriving the ci job's timeout-minutes: 87. Its p95=1730s is about a job that no longer runs the suite, so it is now over-declared. It needs 25 runs of the new shape before timeout-drift can answer; CLOUD-1140's acceptance carries it.

Generated by Claude Code

… that lets it

CLOUD-398 was closed Done on PR #606, which landed slice 2 only — the four
pure-Rust jobs made absent on a diff they cannot judge. Slice 1, moving the
shell suite off the shared runner, was fully refined and never implemented, and
three of that issue's acceptance clauses are unmet verbatim.

The arithmetic that made it look marginal has moved by 6x. CLOUD-398 costed the
split on an 857s gate with a 697s suite and concluded ~-167s. Measured on run
33244045030: the `ci` job is 1983s of a 1992s run — the entire critical path,
with `perf` finishing at 9.2 min and idling for 24 more — `mise run ci` is 1832s
of that, and `test:bats` ~83% of that again. CLOUD-386 swept the worker count at
the runner's real width and found every direction worse, because each point
redistributes workers inside one saturated 2-core box; its own point D measured
the suite alone with the box to itself at 623s against 829s contended. A second
runner is the experiment that sweep could not contain, and it turns the gate's
wall clock from a sum of two chains into a max.

`HK_SKIP_STEPS` rather than a profile or a second task, and each alternative
fails for its own reason. A profile cannot express it: hk resolves multiple
profiles on a step with AND and the CLI value supplants the config list, so
carving one step out needs the whole lane partition CLOUD-398 costed at ~160s of
duplication and explicitly sequenced after this. A second mise task would put a
name in the workflow that `verify` does not run, which is `ci-local-parity`
property 3 — so `test:bats` is named in `verify:gated`'s depends instead, beside
the `ci`/`ci:quick` pair that has overlapped there for as long as the quick tier
has existed.

THE FALSE GREEN THIS SHIPS ITS OWN GATE FOR. `hook-profile-check` reads `hk
check --all --plan` and asserts every slow-tagged step is included there; it
cannot see what a workflow job passes, which is why it reads the git hook's
invocation line for its other half. Measured: `HK_SKIP_STEPS=test:bats` takes
that plan from 58 steps to 57 with reason `env_exclude`, while the gate's own
reading stays green — so the suite could be carved out of the `ci` job and run
by no job at all, with every gate in this repository green over a suite that
never executed. `ci-suite-lane` refuses exactly that direction. The opposite
mistake, a name matching no step, runs the step in both jobs and needs no gate
because it has a symptom.

A module rather than a shell gate because `shell-retirement` governs
`mise-tasks/**` and `tests/**/*.bats` on the added side as well as the edited
one, so the engine is the only route a new predicate has.

`bats-invocation` had to move with it. Its `install_args` clause asserted `rush`
was installed in the `ci` job, which was correct exactly while that job ran the
suite; after the move it would have been green over a `bats` job with no rush —
the same false green one layer over. The job is derived from which one invokes
the suite now, and the fixture carries the invocation as well as the list,
because one with only the list could no longer reach the assertion it exists for.

Refs: CLOUD-1140
@linear-code

linear-code Bot commented Aug 29, 2026 •

Copy link
Copy Markdown
CLOUD-1140 CLOUD-398's slice 1 never landed: the shell suite is still 83% of a 30-minute `ci` job it shares a runner with, and the split is now worth 6x what it was costed at

Why

CLOUD-398 was closed Done on PR #606, which landed slice 2 only — the four pure-Rust jobs made absent on a diff they cannot judge. Slice 1, moving the shell suite off the shared runner, was fully refined and never implemented. Three of that issue's acceptance clauses are unmet verbatim:

  • "The union of the CI jobs' hk steps is the whole gate, proven by a gate rather than by reading, and shown able to fail."
  • "A step carrying no lane, or two, is red before CI."
  • "ci wall clock is measurably lower, and the runner-seconds delta is reported rather than assumed."

This row is that slice. It is filed rather than reopening CLOUD-398, which is released.

**The arithmetic that made it marginal has moved by 6x. **CLOUD-398 costed the split on an 857s gate with a 697s suite and concluded ~-167s, -20% — real but thin against a metered runner. Measured 2026-08-29 on run 33244045030 (green):

layer cost share
the ci job, end to end 1983s 99.5% of the whole CI run (1992s)
mise run ci inside it 1832s 92.4% of the job
setup, checkout, disk reclaim, caches, teardown 151s 7.6%
test:bats inside mise run ci ~1520s ~83% (mise.toml:1547, 829/998s local; 1435.7/1670.9s on the runner)

ci is the entire critical path — perf finished at 9.2 min and idled for 24 more. The split is now worth roughly -1100s of wall clock, about -60%, and it is paid twice per land lap, because verify runs the same gate locally before every push.

**Why the worker knob cannot deliver this instead. **CLOUD-386 swept it at the runner's real width and every direction is worse (mise.toml:1544): 4 workers costs the gate 173s, giving bats the box costs 50%, capping nextest costs 20%. Every point in that sweep redistributes workers inside one saturated 2-core box, so each one robs one chain to pay another. A second runner is the experiment that sweep could not contain, and point D already measured the number it needs: the suite alone with the box to itself is 623s, against 829s contended.

What CLOUD-398 got right and this must not re-derive. Its correction of 2026-08-21 stands: the lane partition is an optimisation of the split, not its enabling mechanism. The suite does not need a lane — it needs to not be in the other job. [tasks."test:bats"] is already a standalone task with its own receipt, selector and count assertion.

**The false green this must close, and it is the reason this is not a two-line change. **hook-profile-check judges hk check --all --plan and asserts every slow-tagged step is included there. It never reads what a workflow job passes. So a ci job that skips the suite is green on that gate while CI silently stops running it — exactly the profiled-step-not-in-check class the gate exists for, one layer up where it cannot see. The gate reads .claude/hooks/git-hook.sh's invocation line for the same reason; it must read the workflow's too.

Refinement — Ready (move the suite to its own job, and gate that the two jobs still cover the whole gate)

  • **Source of truth (§1). **hk.pkl's check plan (hk check --all --plan --json) for what the gate contains, and .github/workflows/ci.yml's run: scalars for what each job invokes. The result is the ci and bats jobs' own step timings before and after, from the run history. None of those is a number a human types.
  • Mechanism as a computable predicate (§2). One assertion over committed text: the set of gate steps the ci job runs, unioned with the step the bats job runs, is the whole check plan — no step in neither. Decides from the plan JSON and the workflow YAML alone; no network, no run history.
  • Deliberately not in scope (§2). The lane partition over all ~58 steps — CLOUD-398 costed it at ~160s of duplication and explicitly sequenced it after this. Runner width, which is a billing question and not a code one. The top-of-distribution suite costs in bench/suites/RESULTS.md. Widening ci-slow-needed's inert list.
  • **Effect (§3). **read — workflow text and gate configuration. No new verb, no change to the derived read-only allowlist.
  • Output and exit (§5). Pointer-only: a step name and the job that does or does not run it. Never a step's command line and never its output.
  • **Commit / bump (§6). **perf(ci) — no bump. Nothing under crates/.
  • **Test obligation (§7). **tests/hook-profile-check.bats gains the workflow-side case, shown able to fail by a ci job that skips a step no other job runs. tests/ci-local-parity.bats already covers properties 3, 5 and 6 by construction, so the new job's presence in $CI_REQUIRED_CHECKS and in final's needs: is gated without new cases.
  • Blockers (§8). None. relatedTo CLOUD-398 (the slice this completes, and the design), CLOUD-386 (the worker sweep that establishes there is nothing left inside one box), CLOUD-439 (the rejected cheaper alternative), CLOUD-509 (hook-profile-check's subject), CLOUD-886 (suite-select, which narrows what the new job runs).

Acceptance

  • The suite runs as its own required job; the ci job no longer runs it.
  • A gate proves the two jobs' union is the whole check plan, and is shown able to fail by a step in neither.
  • verify still runs test:bats, asserted rather than assumed — ci-local-parity property 3 requires every task a workflow names to be one verify runs.
  • The new job carries a timeout-minutes budget derived from a measurement, not grandfathered.
  • The ci job's own timeout-minutes: 87 budget is re-derived from mise run timeout-drift once the new shape has run, rather than left stating a p95 the job no longer has.
  • The wall-clock delta and the runner-seconds delta are both reported, whichever way they fall. A negative result closes the question rather than being dropped.

CLOUD-398 The hk gate's two chains contend for four cores, so parallelising inside it returns a third of what the same work returns alone

Why

hk.pkl runs cargo-fmt → cargo-clippy → test → batten-check concurrently with shfmt → shellcheck → test:bats. One runner, four cores, both chains competing.

CLOUD-386 measured what that costs. The same bats suite, alone on a 4-core box, ran 72.73s; inside the gate on CI (run 31535645426) it ran 192.59s. That reading is wrong, and CLOUD-439 retired it before this row was refined — corrected in the body 2026-08-21 rather than left in a comment, because a headline number nobody reads past is the whole case. The two figures came from different machines: 72.73s local, 192.59s on ubuntu-latest. Measured on one box, same commit, same suite, in-gate vs alone is 90.26s → 100.3s. Contention costs about 1.11×, not 2.6×. The retirement is committed, in the test:bats comment in mise.toml, three lines below the ~19s capping figure this row's refinement quotes from the same paragraph. The gap this issue was sizing is a slower runner.

This is the issue CLOUD-386 explicitly listed as out of scope, and I had separately argued against it. That argument was: a split converts max(cargo, bats) into max(cargo, install + bats), so once bats is fast it buys nothing and costs a metered runner. The reasoning was sound and the input was wrong — it assumed bats-under-the-gate would be ~70s. And then the reversal was wrong too, for the reason above: the 192.59s it turned on is a different machine's number, so the original argument was closer to right than the reversal that overturned it. What a separate runner removes is ~10s of contention plus the serialisation with the cargo chain, which is the -167s wall clock costed above and nothing on the bill. Both the reversal and its correction stay here rather than being quietly re-filed, because the original rejection is written down in CLOUD-386 and someone will find it.

Both preconditions have cleared, and the cheap option was tried and rejected — 2026-08-20

This issue said it should not be pulled before two things. Both are now Done, so the hold no longer applies.

CLOUD-439 was recorded here as a blocker and as "a cheaper thing to try first": cap hk's own --jobs, or bats', so peak demand stops being roughly double the cores. It did not merely measure that — it rejected it. Capping hk to one slot per chain cost ~19s, because the ~23 sub-second gates stop overlapping the two long chains. The measurement is recorded in mise.toml's test:bats comment, together with the worker sweep that found 4 to be the optimum in both directions on a 4-core box. So the cheap lever was pulled and made things worse; the blockedBy relation is cleared and CLOUD-439 stays relatedTo as the evidence.

CLOUD-393's laps-per-landed-PR arithmetic also exists now, which was the second hold.

The arithmetic has moved, by a lot — measured 2026-08-20

The case above was built on a 242s gate. On run 32395938706:

  • mise run ci — 857s
  • of which the hk step test:bats — 697s, 81%
  • the ci job end to end — 15m03s, and it is the CI critical path

The suite is 2,300 cases. The tuning that chose 4 workers was measured at 1,039.

What a split is actually worth, on the corrected 1.11× rather than the 2.6× above: moving the suite to its own runner leaves ci at ~160s and the suite at ~687s, since the suite does not get meaningfully faster — it just stops being added to the other 160s.

  • wall clock ~857s → ~690s, about -167s, -20%, less whatever the second runner queues for
  • billed minutes ~850s against ~857s — no saving at all, one runner's work split across two

So slice 1 is a wall-clock change and not a cost one, and it needs deciding on that basis. The 697s is a slow 4-core runner executing 2,300 cases; the levers on it are the suite's size (CLOUD-312's retirements) and sharding, neither of which is a lane partition.

What the split breaks, concretely

All satisfiable, all load-bearing:

  • ci-local-parity property 3 — every mise run <task> in a run: scalar must appear inside the [tasks.verify] block. A new task must be reachable from verify.
  • ci-local-parity property 5, both directions — $CI_REQUIRED_CHECKS must gain the new job name, matching the job's name: exactly.
  • final's needs: — property 6 asserts the fan-in covers its whole set, so adding the job is gated automatically. This part is the design working.
  • ci-tools-check — the new job needs its own single-line install_args, every name a [tools] key. ci.yml documents why the list must stay on one line.
  • timeout-check — a new job needs a budget comment. Write it measured, not grandfathered; CLOUD-352 owns the conversion of the other 30.
  • checks-green — a new required check that is ever skipped is exit 3, i.e. a hang. Absent is fine (zizmor precedent); skipped is not. This constrains slice 2 below and is the reason it uses a workflow-level filter rather than a job-level condition.

Mechanism, decided — a lane partition with a totality gate

Correction, 2026-08-21: the lane partition is not required for the cheap job, and the section below over-specifies it. The reasoning that led here was that hk profiles are additive, so a subset cannot be selected. That holds for selecting the slow half alone. It does not hold for the cheap half, which is already one flag: --profile '!slow', the exact form .claude/hooks/git-hook:77 has been using at pre-commit all along.

The partition already exists in hk.pkl. Exactly six steps carry profiles = List("slow") — token-bench-check, sbom-check, test:bats, cargo-clippy, test, batten-check — and they are the expensive ones. So a cheap always-running job costs a task and a flag, not a tagging pass over every step in gate.

What the lane work still buys, stated so it is a trade rather than a discard. Without it there is no way to say "slow only", so the path-gated full job re-runs the cheap steps: about 160s of the 857s gate, paid twice on a diff that touches code. The lane partition removes that duplication and nothing else. It is an optimisation of the split, not the enabling mechanism, and it should be sequenced accordingly.

A constraint on the shape, from a gate rather than from taste. hook-profile-check refuses a tier that check stops running — its own words are that a tier missing from check means CI is not running it at all. So [tasks.hooks] cannot become --profile '!slow'. The shape that satisfies it is an always-running cheap job beside the existing full-gate job, with the path filter on the full one.

The concrete case that surfaced this, measured 2026-08-21. ci.yml carries no paths: filter at all — its trigger is pull_request: types: [...] and nothing more — so slice 2 took the four Rust jobs off a non-Rust diff while leaving the slowest job on every diff. A change touching only .serena/memories/** pays the whole 857s gate, of which test:bats is 697s.

That 697s is provably unable to change its verdict on such a diff: every bats suite that touches memories builds its subject under a temp fixture rather than reading the real tree — memories-check, module-map-check, fanout-guard and ready-guard all construct it under $ROOT. The gates that genuinely can be moved by a memories diff are memories-check, module-map-check, attribution-check and rules-drift, all sub-second shell steps in the untagged tier, and all of them sit behind the 697s today.

Two questions the implementation must answer rather than assume. Whether sbom-check, token-bench-check and batten-check can be moved by a diff that touches no code — batten-check evaluates ratchets over the tree, so it is the one most likely to break the clean split. And the paths list needs a gate over its own honesty, the rust-paths-check shape, or this reproduces slice 2's one-sided mistake with the main gate as the blast radius instead of four jobs.

The profiles question this issue was waiting on. hk 1.54's semantics are that an untagged step always runs and a tagged step runs only when its profile is enabled, so --profile shell means "everything plus shell", not "only shell". .claude/hooks/git-hook already relies on the negative form, --profile '!slow', and hk.pkl states that a step declaring no profile therefore always runs.

Two options were on the table and both were rejected: tagging every step makes the pre-commit hook's behaviour depend on a flag, and --step/--skip-step lists are two hand-maintained lists — the second-authority shape non-negotiable 6 and ci-local-parity's own header condemn.

The answer is the first option plus the mechanism that makes its objection safe, which is this repo's standard move rather than a new one:

  • Every step in the gate mapping carries exactly one lane profile. slow stays what it already is — an orthogonal tier, not a lane — so the pre-commit tier behaviour is unchanged.
  • A gate asserts the partition is total and disjoint: every step in gate carries exactly one lane, so a step cannot land in neither half and vanish.
  • The two failure modes are not the ones stated above, measured in a throwaway repo 2026-08-21. hk 1.54 resolves multiple profiles on a step with AND, and --profile repeats rather than taking a comma list. So a step carrying two lanes runs in neither job — that is the one that vanishes silently — while an untagged step runs in both and merely wastes a runner. Disjointness is the safety assertion; totality is the waste one.
  • A second assertion pins that the pre-commit invocation names every declared lane. That is what converts "the hook's behaviour depends on a flag" from an unguarded hazard into a gated one — adding a lane cannot silently switch steps off.

hk.pkl remains the single authority; the per-job lists are derived from it, never maintained beside it. This is the only option that satisfies this issue's own first acceptance clause — the union proven by a gate rather than by reading.

Slice 2 — the pure-Rust jobs are absent, never skipped — LANDED 2026-08-21, PR #606

Folded in here rather than filed separately, because splitting it would reproduce the two-authorities problem this issue is about.

windows, cross, darwin-link and semver decide a pure function of the Rust tree. Measured 2026-08-20: they are 27.8 of 43.7 billed minutes per CI run, 64% — windows alone is 50.6%, since windows-latest bills at 2×. Their verdict cannot change unless crates/**, Cargo.toml, Cargo.lock or rust-toolchain* changed, and 19 of the last 50 commits on main touched those paths.

The shape is fixed by the checks-green constraint above: a workflow-level paths: filter on a separate workflow file, the zizmor.yml precedent, so no check run is created at all. Those job names then join CI_ABSENT_OK_CHECKS beside the entries zizmor and the test.yml jobs already hold. A job-level if: is the forbidden form — it mints a skipped conclusion, CI_ANSWERED_CONCLUSIONS does not contain skipped, and land waits forever.

The filter ships with a gate over its own honesty, the shape CLOUD-224 set: cases asserting the jobs are selected for a crates/** change and for a Cargo.lock-only change, and are not for a Markdown-only change. The negative case is the one that regresses silently.

Refinement — Ready (a lane partition proven total by a gate, plus an absent-not-skipped path filter)

  • Source of truth (§1). hk.pkl's gate mapping is the single authority for which step is in which lane; per-step timings from the ci job log, before and after, are the authority for the result. Neither is a number a human types.
  • Mechanism as a computable predicate (§2). Two assertions over committed text: every step in gate carries exactly one lane profile (total and disjoint), and the pre-commit invocation names every declared lane. Both decide from hk.pkl and the hook script alone — no network, no run history.
  • Deliberately not in scope (§2). Anything that changes what the gate proves. Every step still runs on every PR that can be affected by it; only which runner runs it changes, and slice 2 removes a job only for diffs whose verdict it could not alter.
  • Effect (§3). read — gate configuration and workflow text. No new verb, no change to the derived read-only allowlist.
  • Output and exit (§5). Pointer-only: the step names that carry no lane or more than one, and the job names a filter selected. Never step output.
  • Commit / bump (§6). perf(ci) — no bump.
  • Test obligation (§7). tests/ci-local-parity.bats covers properties 3, 5 and 6 by construction. Two new obligations it cannot see: the lane partition is total and disjoint, shown able to fail by a step with no lane and by a step with two; and the path filter selects for crates/** and for Cargo.lock, and does not select for a Markdown-only diff. The Markdown-only case is the one that regresses silently, so it is asserted explicitly rather than inferred from a timing.
  • Blockers (§8). None. blockedBy CLOUD-439 (closed) — its measurement rejected the cheaper alternative, which is what unblocks this. relatedTo CLOUD-386, CLOUD-352, CLOUD-393, CLOUD-509, CLOUD-662.

Acceptance

  • The union of the CI jobs' hk steps is the whole gate, proven by a gate rather than by reading, and shown able to fail.
  • A step carrying no lane, or two, is red before CI.
  • ci wall clock is measurably lower, and the runner-seconds delta is reported rather than assumed.
  • A Markdown-only push reports the four Rust jobs as absent, not skipped, and land converges on it.
  • A Cargo.lock-only push runs all four.
  • The decision to accept the runner-seconds delta is recorded against CLOUD-393's laps-per-landed-PR number, not against an intuition about CI cost.

Review in Linear

@coderabbitai

coderabbitai Bot commented Aug 29, 2026 •

Copy link
Copy Markdown

Review Change Stack

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Free

Run ID: b3a51713-88a9-4a59-bc98-3d4576d13d46

📥 Commits

Reviewing files that changed from the base of the PR and between 2f1be52 and bafefee.

📒 Files selected for processing (7)
  • .github/workflows/ci.yml
  • batten.toml
  • crates/batten/tests/bats_invocation.rs
  • crates/batten/tests/ci_suite_lane.rs
  • mise.toml
  • policy/bats-invocation.rego
  • policy/ci-suite-lane.rego

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The CI workflow now runs test:bats in a dedicated job and includes that job in final validation. Gated verification runs the Bats suite explicitly. The new ci-suite-lane policy checks that skipped tasks run in another workflow job and reports uncovered or unreadable workflows. Bats invocation discovery now uses the job that runs the suite. Integration tests cover the committed workflow and focused fixtures.

Merge Risk: ⚪ Minimal · up to bafef

This PR moves the shell suite to its own runner and adds safeguards for lane coverage and invocation tracking; no actionable merge-blocking risk remains after normal checks and review.

Warning

Git: CodeRabbit could not clone the repository, so clone-backed analysis was skipped and this review may be incomplete. Verify repository clone access, such as SSH credentials, before requesting another full review. If clone access is intentionally unavailable, use path_filters to narrow the review scope.


Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/settings/billing.

Comment @coderabbitai help to get the list of available commands.

…re's job

Three of the four failures a full `verify` found, all of them mine and none in
the change's substance.

`verify:gated`'s `depends` array was the wrong home for `test:bats`. Adding an
eighth element pushed the line past taplo's width, `mise run fmt` exploded it to
a multi-line array, and three suites read that key with
`awk '/^depends = /{print; exit}'` — one line, so every substring assertion over
it failed at once. The array is restored and the call moved into the body, which
is a better home on its own merits: `depends` entries run in PARALLEL, so an
entry there would have started a second full suite run concurrently with the `ci`
leg that already runs it through hk, where the body call lands after the gate set
has finished and answers from the step receipt in ~0.3s. It is still named where
`ci-local-parity` property 3 reads, since that gate's block covers both the
`verify` and `verify:gated` tables.

`bats-invocation`'s Rust fixture carried an install list and no invocation, which
was fine while the install clause read `jobs.ci.steps` by name and is not now
that it derives the job from `mise run test:bats`. Left alone the derivation was
empty, the clause vacuous, and the case that exists to catch a missing parallel
backend passed over a workflow with none. The fixture carries both halves now.

Refs: CLOUD-1140
@sonarqubecloud

Copy link
Copy Markdown

❌ The last analysis has failed.

See analysis details on SonarQube Cloud

Copy link
Copy Markdown
Contributor Author

Closing unlanded. The branch claude/cloud-1140-suite-own-runner is kept at bafefee, and this reopens unchanged once CLOUD-1143 clears.

Why

verify is green on everything this change touches except one case, and that case is a correctly-working sensor that may not be edited.

  1. ci-local-parity property 5 requires a new pull_request job to be named in $CI_REQUIRED_CHECKS, in both directions — a job added and not listed is silently unrequired, and land would fast-forward without waiting for the suite. So bats joins the roster.
  2. tests/checks-green.bats reads the real roster out of mise.toml [env] rather than a fixture copy, deliberately — its header: "a copy of it that could disagree with the one landing uses." Line 102 asserts the derived output literally, and its own comment says "a roster change must edit this line — and that is the sensor working, not drift."
  3. That file is tests/**/*.bats, governed by policy/shell-retirement.rego. V-SHELL-RULE-EDITED declares one route, R-PORT-AND-RETIRE, with no override and no bypass_env. The one admitted edit, only_drops_a_retired_reference, is explicit that "neither is adding a line" — so changing a literal is refused twice over, as a removal and as an addition.

Neither mechanism is wrong. Together they mean the required-check roster cannot grow until checks-green is retired, which nothing in the tree says and which was found by walking into it.

Three ways around it, and why none was taken

  • Leave bats off the roster — property 5 refuses it, and it would be a false green on the one job this change exists to create.
  • Put bats in $CI_ABSENT_OK_CHECKS — it would be elided from line 102's list and the suite would pass untouched. That set means absence is a legitimate reading, and this job always runs, so this is tuning the gate to fit the test.
  • A matrix leg instead of a job — check names become ci (bats) and ci (gate), so ci leaves the roster. Still a roster change.

What happens instead

CLOUD-1143 is filed to retire checks-green onto the engine — measured at 37 tracked files, including land.sh, ci-wait.sh, land-lock.sh, land-divergence-assert.sh, abandon-matrix.sh, bot-issue.sh and five workflows. It is the one definition of "is this SHA green" and the whole landing path reads it, so it is a campaign rather than a sub-task of this PR; a defect there wedges every future PR instead of failing loudly. CLOUD-1140 is blockedBy it and back in Todo with its §8 corrected — it was refined claiming no blockers and had one.

What is green and stays on the branch

policy-test (22 bundles, 254 cases), batten-check over this tree, actionlint, ci-tools-check, timeout-check (39 jobs), ci-local-parity (all 17 properties), and the hk fast tier at commit time.

Two findings from this branch are worth keeping regardless of when it lands:

  • hook-profile-check cannot see a workflow-level carve-out. Measured: HK_SKIP_STEPS=test:bats takes hk check --all --plan from 58 steps to 57 with reason env_exclude, while that gate stays green. policy/ci-suite-lane.rego is the predicate for it.
  • bats-invocation was one commit from going stale silently. Its install_args clause asserted rush was installed in the ci job — true only while that job ran the suite. It derives the job from mise run test:bats now, so it follows the suite.

Refs: CLOUD-1140, CLOUD-1143


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant