Skip to content

ci: run on pull requests whose base is not main - #382

Merged
vianneybacoup merged 2 commits into
mainfrom
ci/run-on-stacked-pull-requests
Sep 9, 2026
Merged

ci: run on pull requests whose base is not main#382
vianneybacoup merged 2 commits into
mainfrom
ci/run-on-stacked-pull-requests

Conversation

@vianneybacoup

Copy link
Copy Markdown
Collaborator

Problem

CI.yml and helm-integration.yml both declared:

pull_request:
  branches:
    - main

A stacked PR targets its parent PR's branch, not main, so it matches nothing. Observed on the current stack:

PR Base Checks
#379 main 8, all green
#380 fix/sso-cli-flow-coverage labeler only

Why it matters less than it looks — and still matters

Nothing untested reaches main. Merging a stacked PR pushes commits onto the parent's head branch, which fires synchronize and re-runs the parent's full CI on the combined result.

The cost is when you learn. You review and merge the stacked PR blind, and a failure surfaces afterwards on a PR that was green, with the two changes now mixed together — which is precisely the separation stacking exists to buy.

(main is unprotected, so no required check blocks any of this today. This is discipline, not a gate.)

Change

Drop the base filter on pull_request; keep it on push so a branch push doesn't double-run alongside its own PR.

Volume barely moves — every Dependabot PR already targets main and already runs. The only additions are stacked PRs, i.e. the ones currently uncovered. concurrency with cancel-in-progress: true is already configured, and for a pull_request event github.ref is refs/pull/<n>/merge, so stacked PRs don't cancel each other.

helm-integration is widened too. It's the heavier job, so this one is a judgement call — the alternative is leaving it on workflow_dispatch for stacked PRs, which means a stacked PR touching helm/ is silently skipped unless somebody remembers. Say the word and I'll drop that half.

Rollout

This does not retroactively help #380 — a pull_request run resolves its workflow from the PR's merge commit, so #380 picks the new triggers up once its branch carries this change. After merging here: rebase #379 onto main, then rebase #380 onto fix/sso-cli-flow-coverage, and #380 gets the full suite.

🤖 Generated with Claude Code

Both workflows filtered on `pull_request: branches: [main]`, so a stacked PR
— one targeting its parent PR's branch rather than main — got no CI at all.
PR #380 landed with only `labeler` green while #379, its parent, had all
eight checks.

Nothing untested actually reached main: merging a stacked PR pushes to the
parent's head branch, which re-runs the parent's CI on the combined result.
But the feedback arrives after the merge, attached to a PR that was green,
which is the wrong time and the wrong place to discover a failure.

Drop the base filter on `pull_request` and keep it on `push`, so a branch
push does not double-run alongside its own PR. Volume barely moves: every
Dependabot PR already targets main, so the only additions are stacked PRs,
which are exactly the ones missing coverage. `concurrency` with
`cancel-in-progress` already collapses superseded runs, and for a
`pull_request` event `github.ref` is `refs/pull/<n>/merge`, so stacked PRs
do not cancel each other.

helm-integration is widened too: it is the heavier job, but leaving it
filtered means a stacked PR touching the charts is silently skipped, and
relying on someone remembering `workflow_dispatch` is how that gets missed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 9, 2026 10:09
@github-actions github-actions Bot added the ci Continuous delivery label Sep 9, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is a straightforward workflow trigger adjustment that aligns with the stated goal of running CI on stacked PRs and introduces no apparent functional or security risks.

Pull request overview

This PR updates the GitHub Actions workflow triggers so CI runs for pull requests regardless of the PR’s base branch, ensuring stacked PRs (which target a parent branch rather than main) still receive full CI coverage.

Changes:

  • Removed the pull_request.branches: [main] base-branch filter from .github/workflows/CI.yml.
  • Removed the same base-branch filter from .github/workflows/helm-integration.yml.
  • Added brief inline comments documenting why the base-branch filter is intentionally omitted.
File summaries
File Description
.github/workflows/CI.yml Ensures the main CI workflow runs for stacked PRs by triggering on all pull_request base branches.
.github/workflows/helm-integration.yml Ensures Helm integration tests also run for stacked PRs by triggering on all pull_request base branches.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

The rationale lives in the previous commit message and in the PR.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@vianneybacoup
vianneybacoup deployed to helm-integration September 9, 2026 10:41 — with GitHub Actions Active
@vianneybacoup
vianneybacoup merged commit 2784503 into main Sep 9, 2026
8 checks passed
@vianneybacoup
vianneybacoup deleted the ci/run-on-stacked-pull-requests branch September 9, 2026 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Continuous delivery

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants