Skip to content

ci: run CI on stacked PRs, gated on real stack membership - #339

Merged
jeremy-wayland merged 3 commits into
fix/mcp-protocol-negotiationfrom
ci/stacked-pr-ci
Aug 14, 2026
Merged

jeremy-wayland merged 3 commits into
fix/mcp-protocol-negotiationfrom
ci/stacked-pr-ci

Conversation

@sgathrid

@sgathrid sgathrid commented Aug 11, 2026

Copy link
Copy Markdown
Member

TL;DR

Stacked PRs target a topic branch, not main. The old pull_request.branches allowlist therefore skipped CI entirely for stacks (see #337: zero runs until it was linked / this gate landed).

This PR:

  1. Opens the pull_request trigger (no base-branch filter).
  2. Adds a gate job + scripts/ci_gate.py that re-applies policy:
    • trunk base (main / migration / release/**) → run
    • member of a GitHub stack → run
    • random PR into someone’s topic branch → skip
  3. Fails open if the stack GraphQL call errors (prefer a green/red signal over silent skip).
Stack #338 · base #337 → this (top)
Head ci/stacked-pr-ci (b88e4b6)
Base fix/mcp-protocol-negotiation (#337)
Required for product? No — optional policy. Ship #324+#337 without this if you prefer.

Why this exists

Situation Old CI With this PR
PR → main Runs Runs (gate admits trunk, no stack API)
Stacked PR → parent topic branch No workflow Runs if pullRequest.stack is set
PR opened, not yet stack-linked No workflow Still no (until linked) — gate can’t invent membership
One-off PR → random topic branch No workflow Still skipped

Note: GitHub may already match some stack workflows against the stack trunk for branches: filters. This PR does not rely on that: policy is explicit in-repo. The important fix is “linked stacked PR gets CI.”


How the gate works

pull_request event
       │
       ▼
  gate job (always)
       │
       ├─ trunk base? ──────────────► run: true  (no GraphQL)
       ├─ stack API says stacked? ──► run: true
       ├─ GraphQL/IO error? ────────► run: true  (fail open + warning)
       └─ else ─────────────────────► run: false (skip rust/composable/…)
Piece Role
.github/workflows/ci.yml Unfiltered pull_request:; gate → conditional jobs
scripts/ci_gate.py Pure decision function + --selftest (12 cases)
TRUNK_PATTERNS Must stay in sync with former allowlist by hand
Permissions pull-requests: read enough for stack field (verified on a real run)

Non-goals

  • Does not change push triggers (main / migration only).
  • Does not merge or bypass branch protection.
  • Does not fix release.yml / docs.yml firing on stack trunk (pre-existing once stacks match trunk) — separate ops call if codesign cost is a concern.

How to review

  1. ci.yml — trigger comment + gate job wiring (needs.gate.outputs.run).
  2. ci_gate.pydecide() + selftest table; fail-open path.
  3. Optional: confirm a stacked PR log shows CI gate: run=true with real GraphQL data (not only the fail-open branch).

Validate

python3 scripts/ci_gate.py --selftest
# On a stacked PR Actions run: gate job green; rust/composable run when run=true

Merge

Order PR Required?
1–2 #324#337 Yes (product / protocol)
3 This (#339) Optional — merge if you want explicit stacked-PR CI policy

Safe to close and rely on native stack-trunk matching if you want less CI surface; documented tradeoff, not a default.

Known flake (unrelated)

ensure_graph_succeeds_directly_when_update_produces_a_graph can hit Text file busy (os error 26) on Linux CI. Pre-existing on main; this PR does not touch graphify. Rerun failed jobs if it trips.

sgathrid and others added 3 commits August 12, 2026 08:22
`on.pull_request.branches` filters on a PR's *base*, and a stacked PR targets
its parent topic branch. The allowlist `[main,
worktree-rust-migration-v0.4.0, 'release/**']` therefore matched no stacked PR
at all: #337 (base `chore/rmcp-3x-264`) drew zero Actions runs, so a stacked
change was reviewed and merged with no verification behind it. `ci.yml` has no
`workflow_dispatch`, so it could not even be triggered by hand.

Widening the allowlist to topic-branch globs would admit every PR aimed at
anyone's branch, which is not the intent. Instead the trigger is unfiltered and
the policy moves into a `gate` job backed by `scripts/ci_gate.py`:

* `push` — already filtered by the trigger. Run.
* `pull_request` into a `TRUNK_PATTERNS` branch. Run, decided before any API
  call, so the path to `main` gains no new dependency.
* `pull_request` belonging to a GitHub stack, whatever it targets. Run.
* Any other `pull_request` — a one-off aimed at a topic branch. Skip.

Stack membership is real data, not inferred from branch names: the
`PullRequestStack` GraphQL API exposes `pullRequest.stack { number size }`,
which returns a stack for #337 and #324 (stack #338, positions 2 and 1) and
`null` for non-stacked PRs such as #320 and #318.

`--selftest` covers all twelve decision-table cases and runs as the gate's
first step, including the `push` path that could otherwise break `main` and a
check that `release/*` does not match across a slash. Actions globs are not
`fnmatch` — `*` stops at `/` while `**` crosses it — so the matcher is
implemented rather than delegated.

Unverifiable locally, and deliberately loud about it: whether an Actions
installation token can read `PullRequestStack` is a different question from
whether the logic is right. The local checks used a personal token. So the gate
prints the raw GraphQL response including any `errors` array, never fails its
own step, and fails *open* — an unreadable response runs CI with a `::warning::`
saying the stacked-PR-only policy is not in effect, rather than silently
skipping verification. This PR's own run is the test of that access.

No CHANGELOG entry: CI plumbing, per `.agents/AGENTS.md:90`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The comment described `TRUNK_PATTERNS` as mirroring the old
`on.pull_request.branches` allowlist "plus `on.push.branches`", but the two
differ: the push filter has no `release/**`. Reading it as a union would suggest
a push to `release/*` consults the list, when the trigger never fires for one at
all — pushes are filtered before the gate runs, and `decide()` admits any non-PR
event outright.

Also states plainly that nothing enforces the correspondence with the YAML, so
the sync is by hand.

Comments only; no behavior change. `--selftest` 12/12, `actionlint` clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jeremy-wayland
jeremy-wayland merged commit a9df024 into main Aug 14, 2026
45 checks passed
@jeremy-wayland
jeremy-wayland deleted the ci/stacked-pr-ci branch September 4, 2026 19:19
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.

3 participants