CodeIssuesPull RequestsActionsSecurityInsights
✨ AI
More
Settings

fix(ci): two false greens — a job that ran nothing, and a failing line that didn't fail #5594

MergedXSccantynz wants to mergefix/ci-no-steps-executedmainopened 1d ago
ccantynzcommented 1d ago

Both found by checking whether docs/AUDIT-CI-RUNNER-ISOLATION.md was still accurate rather than assuming. It was — it recorded both two days ago (R8 and R9) and nothing had acted on them.

1. A job that executed nothing reported success

A step with no run: is recorded "skipped", because v1 doesn't support uses:. Nothing counted how many steps actually ran. So a job whose steps were all uses: produced zero failures, anyFailed stayed false, the run went green — and passingCheckNames credits a required CI check from a workflow_runs row with status = 'success', so the merge gate believed it too.

This lands hardest on the person we most want to succeed: a team arriving from GitHub, whose imported workflows are mostly uses: actions/checkout@v4 and friends. They'd have seen a green tick over a run that never compiled a line.

A green that means nothing is worse than no CI, because no CI is at least honest about what it knows.

Fixed at the job level, not the step level — deliberately. A uses: step beside real run: steps must still let the real ones execute and prove something. Only a job that declared steps and ran none fails. steps.length > 0 is equally deliberate: a genuinely empty job is a different thing from one whose every step was silently ignored, and conflating them turns the guard into noise.

The log now names the unsupported steps and says what to do, because a red build that doesn't say which steps it couldn't run sends someone hunting through YAML.

2. A failing line in a multi-line run: didn't fail the step

Steps ran under bash -c. Without -e, execution continues past a failing line and the exit code is the last command's — so a build that failed in the middle reported success. GitHub Actions uses bash -e for exactly this reason.

Same defect the Vapron session found in its own gate driver yesterday (if $step 2>&1 | tail -25; then echo OK — a pipeline's status is its last command's, and tail always succeeds). Different mechanism, identical consequence: the exit code being reported is not the exit code that matters.

Behaviour change, stated plainly: a workflow that relied on a failing line being ignored will now go red. That's correct — it was already failing, we just weren't saying so. Blast radius is small today because workflow-exec-policy restricts execution to trusted accounts.

Also

Updates the audit doc: records R8 as fixed, and amends its claim that a skipped check never satisfies a required check — as of 2026-08-30 there's exactly one exception (AI Review skipped for provider unavailability), which my own change introduced and would otherwise have quietly made that paragraph false.

Verified by mutation: restoring the original status line makes a job that ran nothing pass again, and fails the suite.

ccantynzAI Reviewcommented 1d ago

AI Triage

(no summary)

Priority: medium Risk area: mixed

Suggested labels: (no label suggestions) Suggested reviewers: (no reviewer suggestions)

Suggestions only — nothing has been applied. The PR author stays in control.

gluecron[bot]🤖 botAI Reviewcommented 1d ago

AI review unavailable

The platform's AI balance is exhausted, so AI generation is temporarily unavailable. Nothing was lost. You can queue this as a repair for the internal agent from the repository's Health page, or try again once the balance is restored. The PR is otherwise unchanged.

Cross-repo impact

See what breaks downstream if this PR merges.

Analyze →
⮌ Merged

This pull request was merged into main.

c comment · e edit title · m merge · a approve · r request changes · ? shortcuts