Skip to content

Commit efb9d22

Browse files
claudeclaude
committed
fix(ci): a step's if: decides whether it runs, so it belongs to its identity
Review round 9, P1. `RawStep` and the digest never read `if:`, and the consequence was larger than the report: `if: false` on `run: pnpm gate layering` left the digest byte-identical, so the step was still approved by SHAPE, the lane was still credited with the gate, and `pnpm check:gate-manifest` reported ok. Verified end to end before fixing — a planted lane whose only gate step could never execute produced zero findings. A one-word edit silently disabling a gate is the exact failure #1429 exists to prevent, so this was the manifest failing at its stated purpose. `if:` is now part of `stepExtras`, which means a conditional step is no longer waved through by shape at all: it must be written into the baseline, where the condition is visible and any later change to it moves the digest. Eleven real gate steps are conditional (`always()`, the `failure()` envelope recorders, a device-availability guard, and the Apple cache-miss build), so the baseline grows 81 -> 90 entries and each now records its condition. Regressions added for both of round 8/9's proofs, since the previous commit argued non-vacuity by hand instead of checking one in: `if: false` on a gate step must fail, and a GATE_ACTIONS body swapped for a no-op must stop crediting its callers. NOT fixed here, and flagged rather than papered over: a conditional step still CREDITS its lane. Refusing that credit is not free — it unowns 21 checks including `unit`, `coverage` and both Swift runners — because `if: always()` is safe while `if: cache-hit != 'true'` genuinely does not run on a hit. Whether a cached build counts as running its check is a design question, not a defect. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01SkS4S8XXrfkJ8TD1VBKkvJ
1 parent 122b21c commit efb9d22

3 files changed

Lines changed: 153 additions & 60 deletions

File tree

‎scripts/gate/audit.test.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,8 @@ jobs:
8989
steps:
9090
${steps}`;
9191

92+
const GATE_ACTION = '.github/actions/setup-apple-runner-build/action.yml';
93+
9294
const kinds = (model: Model) => [...new Set(audit(model, baseline).map((f) => f.assertion))].sort();
9395

9496
test('the live tree is green — every planted failure below is a real difference', () => {
@@ -166,6 +168,21 @@ runs:
166168
}),
167169
['bypass', 'surface'],
168170
],
171+
[
172+
'r9: `if: false` cannot silently disable a gate step',
173+
() =>
174+
plantWorkflow(`name: Planted
175+
on:
176+
pull_request:
177+
jobs:
178+
planted:
179+
steps:
180+
- name: Run the layering gate
181+
if: false
182+
run: pnpm gate layering
183+
`),
184+
['bypass'],
185+
],
169186
[
170187
'r6: an undeclared third-party action',
171188
() =>
@@ -278,6 +295,25 @@ test('editing the body behind a listed step is rejected, and its entry goes iner
278295
assert.ok(found.some((f) => f.assertion === 'inert'));
279296
});
280297

298+
test('a gate-valued action must be proven to run its gate, not trusted to', () => {
299+
// Round 8: listing an action in GATE_ACTIONS credited every caller with whatever id it
300+
// passed. Swapping the body for a no-op left the credit intact. The commit that fixed it
301+
// proved non-vacuity by hand; this is that proof, checked in.
302+
const withBody = (run: string) =>
303+
audit(
304+
{
305+
...base,
306+
gateActionBodies: { ...base.gateActionBodies, [GATE_ACTION]: { run, boundTo: 'gate' } },
307+
},
308+
baseline,
309+
).filter((failure) => failure.assertion === 'gate');
310+
311+
assert.deepEqual(withBody('pnpm gate "$INPUT_GATE"'), [], 'the real body satisfies the contract');
312+
const noop = withBody('true');
313+
assert.equal(noop.length, 1, 'a no-op body must not credit its callers');
314+
assert.match(noop[0]?.message ?? '', /does not invoke/);
315+
});
316+
281317
test('a baseline entry describing no live step is inert', () => {
282318
const invented = [
283319
...baseline,

0 commit comments

Comments
 (0)