Skip to content

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

Merged
wenzowski merged 2 commits into
mainfrom
claude/ci-gate-cost-bundle-bush41
Aug 30, 2026
Merged

wenzowski merged 2 commits into
mainfrom
claude/ci-gate-cost-bundle-bush41

Conversation

@wenzowski

Copy link
Copy Markdown
Contributor

Closes CLOUD-1140

Why

ci is the entire critical path of a CI run. Measured on run 33244045030 (green):

layer cost share
the ci job, end to end 1983s 99.5% of the whole run (1992s)
mise run ci inside it 1832s 92.4% of the job
test:bats inside that ~1520s ~83%

perf finished at 9.2 minutes and idled for 24 more. Moving the suite off the shared runner is 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.

This is CLOUD-398's slice 1. That issue closed Done having shipped slice 2 only, leaving three of its acceptance clauses unmet verbatim.

Why the worker knob cannot deliver it instead: CLOUD-386 swept it at the runner's real width and every direction is worse — 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 robs one chain to pay another. A second runner is the experiment the sweep could not contain, and point D already measured what it needs: the suite alone with the box to itself is 623s against 829s contended.

Why it lands now, when it didn't yesterday

This is a replay of PR #742 (bafefee), which was closed unlanded against exactly one blocker.

ci-local-parity property 5 requires a new pull_request job to be named in $CI_REQUIRED_CHECKS, both directions — a job added and not listed is silently unrequired. So bats joins the roster. That made tests/checks-green.bats:102 fail:

[[ "$output" == *"with no run at all: ci, perf, final"* ]]

a deliberate literal in a file policy/shell-retirement.rego governs, whose V-SHELL-RULE-EDITED declares one route (R-PORT-AND-RETIRE) with no override and no bypass_env.

#758 retired that suite and its program, so the roster now reaches the decision as a flag rather than being pinned by a bats literal. The two commits cherry-picked onto the new main with no conflicts, and:

$ git diff --name-only origin/main...HEAD -- 'tests/*.bats' 'mise-tasks/*.sh'
$

No governed file is edited. That empty output is the shared acceptance clause of this row and CLOUD-1143 — "adding a name to $CI_REQUIRED_CHECKS no longer requires editing any .bats file, demonstrated by CLOUD-1140 landing on top of this without one."

The false green this closes

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 would be green on that gate while CI silently stopped running it — the profiled-step-not-in-check class, one layer up where the gate cannot see.

policy/ci-suite-lane.rego is that gate: it refuses a step carved out by HK_SKIP_STEPS that no job runs. This is why the change is 753 lines rather than two.

What's here

file change
.github/workflows/ci.yml ci gains HK_SKIP_STEPS: test:bats; new bats job with a measured timeout-minutes; final gains bats in needs:
policy/ci-suite-lane.rego new — the carve-out gate
crates/batten/tests/ci_suite_lane.rs new — its compiled-binary tier
batten.toml the [[rule]] row registering the module and its [[verdict]] rows
policy/bats-invocation.rego derives the job from mise run test:bats instead of hard-coding jobs.ci
crates/batten/tests/bats_invocation.rs the fixture gains the run half it needs once the job is derived
mise.toml bats in CI_REQUIRED_CHECKS; test:bats invoked from verify:gated's body

The verify:gated detail is load-bearing. test:bats goes in the task body, not depends: taplo reflows a long depends array to multi-line and three suites read it with awk '/^depends = /{print; exit}'. Putting it in depends also double-runs the suite, since depends entries run in parallel with the body.

Verification

  • mise run ci-local-parity — green, all 17 properties, including property 5 both directions with the new job.
  • mise run verify — running; the bats tier is the one that matters here, since it must now pass with the grown roster.
  • The wall-clock and runner-seconds deltas will be reported from this PR's own run, whichever way they fall. A negative result closes the question rather than being dropped.

Generated by Claude Code

@linear-code

linear-code Bot commented Aug 30, 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.

BLOCKED on CLOUD-1143: the roster change obliges an edit shell-retirement refuses — 2026-08-29

**Disposition: #742 is closed unlanded and the branch **claude/cloud-1140-suite-own-runner is kept at bafefee. The split is written and locally green on everything but the one case below, so it lands unchanged once CLOUD-1143 clears. Nothing is discarded and no gate was weakened to get past it.

The change is built and pushed (#742, bafefee). verify fails on exactly one case, and it is not in the change's substance.

ci-local-parity property 5 requires a new pull_request job to be named in $CI_REQUIRED_CHECKS — both directions, because a job added and not listed is silently unrequired. So bats joins the roster. tests/checks-green.bats then fails at line 102:

[[ "$output" == *"with no run at all: ci, perf, final"* ]]

That suite reads the real roster out of mise.toml [env] deliberately (its header: "a copy of it that could disagree with the one landing uses"), and line 102 is a deliberate literal whose own comment says "a roster change must edit this line — and that is the sensor working, not drift." It is the sensor working. It is also tests/**/*.bats, which policy/shell-retirement.rego governs.

There is no route through it. V-SHELL-RULE-EDITED declares one route, R-PORT-AND-RETIRE, with no override and no bypass_env. The single admitted edit is only_drops_a_retired_reference, whose predicate is explicit that "neither is adding a line" — changing a literal is a removal plus an addition, so it is refused twice over. Read off the module rather than inferred.

What the rule says to do about it. .claude/rules/toolchain.md: "A plan that says 'this row edits foo.sh' has not found a blocked row — it has found a row whose §1 is written in the wrong shape. Re-scope it to a retirement or leave the file alone." So this row's §1 is wrong: it needs to include retiring checks-green onto the engine, program and suite together.

The size of that, measured rather than guessed. checks-green is named in 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. Retiring it is a campaign rather than a sub-task of this row, and getting it wrong wedges every future PR instead of failing loudly. No row for it exists on the board today.

Three alternatives were ruled out before concluding this, each on its own reason rather than by preference:

  • Leave bats out of the roster. ci-local-parity property 5 refuses it, and land would fast-forward without waiting for the suite — 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 unchanged. Refused as laundering: that set means absence is a legitimate reading, and this job always runs. It 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, so still line 102.

What is green. Everything else the change touches: 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. The other three verify failures were mine and are fixed in bafefee — a taplo reflow of verify:gated's depends that three suites read as a single line, and a bats-invocation fixture left vacuous once its job became derived.

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). blockedBy CLOUD-1143 — the roster cannot grow while tests/checks-green.bats pins it literally and may not be edited. This clause is a correction: the row was refined claiming no blockers and had one, which was discovered by walking into it at verify rather than at refinement time. 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.

The four suites this row set aside, ranked by what retiring them costs — 2026-08-29

§2 defers "the top-of-distribution suite costs in bench/suites/RESULTS.md", correctly: this row moves the suite, it does not shrink it. 1244.6s of serial cost is unchanged by the split. That deferral is now measured, so the follow-on is not a fresh investigation.

Also milestoned Stage 5 in the same edit — this row was filed with none, so no milestone-scoped sweep could return it, and it sits on the ci critical path. CLOUD-1124's class.

**The top four are 44.1% of serial suite cost in 4 of 147 files: **derived-check 210.8s / 16.9%, land-lock 145.1s / 11.7%, ci-wait 102.5s / 8.2%, land 91.0s / 7.3%.

No row on the board owns the retirement of any of them. Every ticket naming them treats them as instruments that keep running, readers to be repointed, or flake defects already fixed — never as §1 subjects to delete. Searched across derived-check, land-lock, land.sh, ci-wait, checks-green and the wave rows (CLOUD-843, CLOUD-910).

Ranked by governed siblings dragged in, which is the real cost

rank program suite cost governed siblings verdict
1 ci-wait 102.5s 0 SELF-CONTAINED
2 derived-check 210.8s 1 (man-pages.sh) CLOSES
3 land-lock 145.1s 3 BLOCKED
4 land 91.0s 6 BLOCKED, worst

ci-wait is the leaf and the cheapest thing here. Nothing resolves it by path except its own suite. Its one live caller is mise-tasks/land.sh:1838 — mise run ci-wait, a task name, which is exactly the shape 42cbee1 records as why the claim gate's port landed cleanly: "its only callers named it by task name, so mise.toml could answer for it with no frozen file touched at all." It is not in $MUTANT_GATES and carries no #MUTANT row, so there is no gate to drop. Two deleted paths, two ledger arms, zero governed sibling edits.

derived-check is the largest prize and it closes. Nothing resolves it by path; hk.pkl:289 calls it by task name. Its one drag-in is not a caller but its own header — tests/derived-check.bats:2 declares # subject: mise-tasks/derived-check.sh mise-tasks/man-pages.sh, and SubjectFacts::died requires every declared subject to die ("a suite declaring two subjects of which one still stands has work left"). man-pages.sh is cheap: no suite, not in $MUTANT_GATES, and its only other reference is mise.toml:1356, ungoverned. Three deleted paths, three ledger arms, zero governed sibling edits. Independent of schema-check — no path reference, no task-name call, no shared subject header; the coupling is rhetorical only.

Its 210.8s is process-spawn tax, not compilation: ~77 cargo run -p batten invocations per gate run × 10 @test cases ≈ 770 dispatches.

land-lock and land **are blocked, and not negotiably. **mise-tasks/ci-lease-precondition.sh:176 fetches land-lock.sh's blob from main and execs it, branching on its exit code. Removing that block deletes lines naming no deleted path, so only_drops_a_retired_reference fails; repoints_at_the_declared_successor cannot help either, since it demands replace(was, gone, succ) == line byte-for-byte and no single-token substitution turns a fetch-write-chmod-exec block into a compiled-verb call. ci-lease-precondition.sh is itself non-retirable: $MUTANT_GATES-listed, own suite, fetched by path from main by five workflows. land adds ci-slow-needed.sh and ci-local-parity.sh:1086-1088, the latter literally opening and grepping land.sh's bytes as its own predicate. 42cbee1's ready-lint → graph-check shape, two levels deeper.

The ordering that matters to THIS row

CLOUD-1143 — this row's blocker — must repoint six checks-green readers, and ci-wait.sh:46 is one of them (verdict_of="$(dirname "$0")/checks-green.sh", by path). Retiring ci-wait first removes one of those six, so it shrinks this row's blocker rather than competing with it. land.sh and land-lock.sh are two more of the six and are blocked — which is why CLOUD-1143 ends up entrenching them as live bash with rewritten exit-branching.

A hazard CLOUD-1143 does not carry: changing checks-green's exit table (1/3 → stdout) breaks the exit-semantics assertions in tests/ci-wait.bats, tests/land.bats and tests/land-lock.bats. Each such edit is V-SHELL-RULE-EDITED with only R-PORT-AND-RETIRE as a route. That row names none of the three suites. Retiring ci-wait first removes a third of that hazard too.

Recommended sequence: retire ci-wait (102.5s, free), then derived-check + man-pages (210.8s, closes) — 313.3s, 25.2% of serial suite cost, from five deleted paths and no governed sibling edited. Both need new rows; neither exists. Leave land and land-lock until ci-lease-precondition has a route, and note CLOUD-880 — Done — already concluded "land stays bash", so that pair is a decision to reopen rather than work to schedule.

Review in Linear

@coderabbitai

coderabbitai Bot commented Aug 30, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 29 minutes.

View limit details

Limit details: You’ve used the included review currently available.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Free

Run ID: b9a0d244-fd72-4c8d-a24a-f75c1fc46720

📥 Commits

Reviewing files that changed from the base of the PR and between 2fe242e and e340f79.

📒 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

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.

… 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
…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
@wenzowski
wenzowski marked this pull request as ready for review August 30, 2026 15:32
@wenzowski
wenzowski force-pushed the claude/ci-gate-cost-bundle-bush41 branch from 48f6388 to e340f79 Compare August 30, 2026 15:32
@sonarqubecloud

Copy link
Copy Markdown

❌ The last analysis has failed.

See analysis details on SonarQube Cloud

@wenzowski

Copy link
Copy Markdown
Contributor Author

/fast-forward

@wenzowski
wenzowski merged commit e340f79 into main Aug 30, 2026
11 of 12 checks passed
@wenzowski
wenzowski deleted the claude/ci-gate-cost-bundle-bush41 branch August 30, 2026 15:54
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