Run CI on every pull request, not only those targeting master - #398
Merged
Merged
Conversation
`ci.yml` filtered `pull_request` to `branches: [master]`, so a pull request stacked on another branch triggered no run at all. The consequence was not a missing badge: such a PR reports `mergeStateStatus: CLEAN`, because the only check that does run is GitGuardian, and "clean" means "no required check failed" rather than "the suite ran". A stack accumulated unchecked changes until its root merged, at which point the first real run met several pull requests' worth of change at once. This was reached, not hypothesised. #397 merged into `claude/voucher-presence` with one check on it; #308 sat on that branch through a base sync, a conflict resolution and a test rewrite, showing CLEAN throughout. A manual `workflow_dispatch` on #308's branch was the first Bridge CI run it ever had, and it passed on macOS and Windows -- so the gap was coverage, not a latent failure. It could as easily have been the other way. The `push` trigger stays master-only: a branch worth checking has a pull request, and that is what triggers the run, so filtering the push side avoids a second run per branch rather than losing coverage. `ci.yml` is in the compatibility surface, so the surface and matrix are resealed: one hash changed, pin set unchanged at 212, `reseal.sh --verify` exit 0 read directly. Verified: `check-ci-workflow-consistency.mjs` passes; the compatibility gate passes; tools workspace 11 binaries, 53 passed, 0 failed. The workflow parses and keeps its 9 jobs, with `pull_request` now carrying only `types`. Not changed, and worth a separate decision: `dependency-security.yml` carries the same `branches: [master]` filter, and `docs/proposed-merge-gate-ci.md` proposes a merge-gate workflow with it too. Both inherit this gap. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
lamemustafa
added a commit
that referenced
this pull request
Sep 15, 2026
…ster (#401) `dependency-security.yml` carried the same `pull_request: branches: [master]` filter #398 removed from `ci.yml`, so a pull request stacked on another branch got no `cargo-audit` or frontend dependency audit at all. It still reported a green mergeable state, because nothing required had failed -- the same shape of false green, in the workflow whose whole job is to tell you about an advisory. The cost of the gap is different from `ci.yml`'s, which is why it is worth fixing separately rather than living with. A missing test run delays a signal about code the PR contains. A missing dependency audit delays a signal about a published advisory that may already apply, and the delay lasts until the stack root merges -- so the advisory surfaces attached to whichever PR happens to land first, not to the one that introduced the dependency. `push` stays master-only for the reason `ci.yml` gives: a branch worth auditing has a pull request, and that is what triggers the run. `dependency-security.yml` is in the compatibility surface, so surface and matrix are resealed: one hash changed, pin set unchanged at 212 (set-compared against master, not counted), and the recorded sha256 matches the file's bytes. Verified with every exit status read directly: `reseal.sh --verify` exit 0; `check-ci-workflow-consistency.mjs` exit 0; compatibility gate passes; tools workspace 11 binaries, 53 passed, 0 failed. The workflow parses and keeps its single `audit` job, with `pull_request` now carrying only `types`. Co-authored-by: t <dev@example.invalid> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ci.ymlfilteredpull_requesttobranches: [master], so a pull request stacked on another branch triggered no run at all.Why this was invisible
The failure mode is not a missing badge. Such a PR reports
mergeStateStatus: CLEAN, because the only check that does run is GitGuardian — so "clean" means no required check failed, never the suite ran. A stack accumulates unchecked changes until its root merges, at which point the first real run meets several PRs' worth of change at once.This was reached, not hypothesised
claude/voucher-presencewith one check on it, and that check was GitGuardian.CLEANthroughout.workflow_dispatchon Identify a voucher by the marker Bridge wrote into its narration #308's branch was the first Bridge CI run that branch had ever had.total_check_runswas 1 andgh run list --branchreturned nothing before it.That run passed, on macOS and Windows — so what was missing was coverage, not a latent failure. It could as easily have been the other way, which is the point.
The change
Drop the base-branch filter from
pull_request.pushstays master-only on purpose: a branch worth checking has a pull request, and that is what triggers the run, so filtering the push side avoids a second run per branch rather than losing coverage.Cost
Every stacked PR and every sync push to one now runs the full matrix. The existing
concurrencygroup is keyed ongithub.refwithcancel-in-progress: true, so repeated pushes to one PR supersede rather than queue. If the added spend is unwelcome, the narrower alternative is a base-branch allowlist — but it bakes in a naming convention and silently reopens this gap for any base that does not match it.Verification
check-ci-workflow-consistency.mjscompatibility_gate_passed:unknown_claims=11:evidenced_claims=0reseal.sh --verifyci.ymlis in the compatibility surface, so surface and matrix are resealed: one hash changed, pin set unchanged at 212. The workflow parses and keeps its 9 jobs, withpull_requestnow carrying onlytypes.This PR targets master, so it triggers CI under the old rule and is its own first test.
Left alone deliberately
dependency-security.ymlcarries the samebranches: [master]filter, anddocs/proposed-merge-gate-ci.mdproposes a merge-gate workflow with it too. Both inherit this gap; both are separate decisions with their own cost, so neither is changed here.🤖 Generated with Claude Code