Skip to content

ci: tolerate skipped build in ci-pass gate#2190

Open
just-jeb wants to merge 1 commit intomasterfrom
fix/ci-pass-tolerate-skipped-build
Open

ci: tolerate skipped build in ci-pass gate#2190
just-jeb wants to merge 1 commit intomasterfrom
fix/ci-pass-tolerate-skipped-build

Conversation

@just-jeb
Copy link
Copy Markdown
Owner

PR Checklist

  • Tests for the changes have been added (for bug fixes / features) — N/A, CI workflow change
  • Docs have been added / updated (for bug fixes / features) — N/A

PR Type

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[x] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Every push to master whose head commit is the release bot's ci(release): publish ends with a red ✗ on the workflow run. Recent examples: run 24958214443, run 24908348004.

Root cause is in .github/workflows/ci.yml:

  • The build job has if: github.event_name == 'workflow_dispatch' || (!contains(github.event.head_commit.message, 'ci(release)')) — it deliberately skips on the publish bot's commits to prevent infinite publish loops.
  • The ci-pass step then fails because it treats needs.build.result != "success" as failure, with no allowance for skipped. Note the asymmetry: integration is already allowed to be skipped (when no tests are discovered), but build is not.

The actual publish step has already run and pushed package versions before this red run begins, so impact is purely cosmetic noise on the master status, but it's misleading and tarnishes the branch-protection signal.

Issue Number: N/A

What is the new behavior?

Replace the per-job string comparisons with GitHub's idiomatic pattern:

- if: contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled')
  run: exit 1

Behavior matrix:

Scenario build.result integration.result Before After
Normal master/PR run, tests pass success success pass pass
Master/PR with no discovered tests success skipped pass pass
Real build break failure skipped fail fail
Integration test failure success failure fail fail
Cancelled run cancelled skipped/cancelled fail fail
ci(release): publish commit (build's if: skips it) skipped skipped fail (bug) pass

Why skipped is safe to tolerate for build: in GitHub Actions, skipped only occurs when the job's own if: evaluates false. build has no needs:, so it can't cascade-skip. A genuine build problem still produces failure or cancelled, both of which still trip the guard.

Side benefits:

  • Removes the duplicated per-job checks.
  • Symmetrises treatment of build and integration.
  • New needed jobs added to ci-pass later inherit the rule with no edit.
  • if: always() on the job is preserved (load-bearing per .github/AGENTS.md: branch protection needs the check to always report).

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

The alternative — adding the same ci(release) if: predicate from build onto ci-pass — would also fix the symptom, but it makes ci-pass itself skip on release commits, which can break required-status-check configurations in branch protection. Keeping ci-pass always-running and tolerating skipped results is the lighter touch.

The ci-pass step treated needs.build.result != "success" as failure,
which made every release-bot push fail: build's own if: skips on
ci(release) commits to prevent publish loops, so build.result is
"skipped" by design on those runs.

Replace the per-job result checks with the standard contains pattern
over needs.*.result. Failure and cancelled still fail the gate;
skipped is tolerated symmetrically for build and integration. New
needed jobs added later inherit the rule without further edits.
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