From a3646d959b8ddeccf2a808c082d0b227318459f8 Mon Sep 17 00:00:00 2001 From: Sidney Gathrid Date: Tue, 11 Aug 2026 10:53:14 -0400 Subject: [PATCH 1/3] ci: run CI on stacked PRs, gated on real stack membership MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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) --- .github/workflows/ci.yml | 70 +++++++++++++- scripts/ci_gate.py | 201 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 270 insertions(+), 1 deletion(-) create mode 100644 scripts/ci_gate.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f25476a2..4a02545b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3,11 +3,73 @@ name: CI on: push: branches: [main, worktree-rust-migration-v0.4.0] + # Deliberately unfiltered. `branches:` filters on a PR's *base*, and a stacked + # PR targets its parent topic branch — so the old allowlist + # ([main, worktree-rust-migration-v0.4.0, 'release/**']) excluded every + # stacked PR from CI entirely, leaving stacked changes reviewed with no + # verification behind them. The policy moved into the `gate` job below, which + # mirrors that allowlist and additionally admits PRs that belong to a GitHub + # stack. `scripts/ci_gate.py` owns the decision; TRUNK_PATTERNS there must + # stay in sync with `on.push.branches` above. pull_request: - branches: [main, worktree-rust-migration-v0.4.0, 'release/**'] jobs: + # Decides whether the rest of the workflow runs. Trunk-targeted PRs are + # admitted without consulting the stack API at all, so the critical path to + # `main` gains no new dependency; only topic-branch PRs are asked whether + # they are stacked. + gate: + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: read + outputs: + run: ${{ steps.decide.outputs.run }} + steps: + - uses: actions/checkout@v7 + + - name: Check the gate's own decision table + run: python3 scripts/ci_gate.py --selftest + + # `PullRequestStack` is a new API and whether an Actions installation + # token can read it is exactly what this step establishes. The raw + # response is printed — `errors` array included — so a single run answers + # that, and the step never fails: an unreadable response has to fail open + # in `ci_gate.py`, not block CI here. + - name: Query stack membership + if: github.event_name == 'pull_request' + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + OWNER: ${{ github.repository_owner }} + NAME: ${{ github.event.repository.name }} + NUMBER: ${{ github.event.pull_request.number }} + run: | + # shellcheck disable=SC2016 # $owner/$name/$number are GraphQL + # variables and must reach the API unexpanded; the shell must not + # touch them. Real values are bound by the -F flags below. + gh api graphql \ + -f query='query($owner:String!,$name:String!,$number:Int!){repository(owner:$owner,name:$name){pullRequest(number:$number){number baseRefName stack{number size}}}}' \ + -F owner="$OWNER" -F name="$NAME" -F number="$NUMBER" \ + > stack.json 2> stack.err || true + echo "--- raw GraphQL response ---" + cat stack.json || true + echo "--- stderr ---" + cat stack.err || true + + - name: Decide + id: decide + env: + EVENT_NAME: ${{ github.event_name }} + BASE_REF: ${{ github.event.pull_request.base.ref }} + run: | + python3 scripts/ci_gate.py \ + --event-name "$EVENT_NAME" \ + --base-ref "$BASE_REF" \ + --stack-response stack.json + rust: + needs: gate + if: needs.gate.outputs.run == 'true' runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 @@ -75,6 +137,8 @@ jobs: | grep -q '"topos_evaluate_code"' composable: + needs: gate + if: needs.gate.outputs.run == 'true' runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 @@ -142,6 +206,8 @@ jobs: fi wheel: + needs: gate + if: needs.gate.outputs.run == 'true' runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 @@ -174,6 +240,8 @@ jobs: args: --release --bindings bin --manifest-path topos/mcp/Cargo.toml --out dist extension: + needs: gate + if: needs.gate.outputs.run == 'true' runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 diff --git a/scripts/ci_gate.py b/scripts/ci_gate.py new file mode 100644 index 00000000..9911ec75 --- /dev/null +++ b/scripts/ci_gate.py @@ -0,0 +1,201 @@ +#!/usr/bin/env python3 +"""Decide whether CI should run for the current event. + +`ci.yml` used to restrict `pull_request` to `main`, the migration branch, and +`release/**`. A stacked PR targets its parent topic branch instead, so it +matched nothing and got no CI at all — the review window for a stacked change +had no verification behind it. + +The trigger is therefore unfiltered and this script re-applies the policy: + +* `push` — the trigger's own `branches` list already filtered it. Run. +* `pull_request` into a branch on `TRUNK_PATTERNS`. Run. (Unchanged behavior; + decided before any API call, so trunk PRs never depend on the stack query.) +* `pull_request` that is part of a GitHub stack, whatever it targets. Run. +* Any other `pull_request` — a one-off PR aimed at someone's topic branch. + Skip. This is the case the unfiltered trigger would otherwise let in. + +Stack membership comes from the `PullRequestStack` GraphQL API, queried by the +workflow and handed to us as a file so this stays pure and testable. If that +response is missing, malformed, or carries an `errors` array, we **fail open** +and run: a permissions or schema problem must not silently skip verification. +That path is loud on purpose — see `--selftest` and the `::warning::` below. +""" + +from __future__ import annotations + +import argparse +import json +import os +import re +import sys +from pathlib import Path + +# Must mirror `on.pull_request.branches` as it stood before the filter was +# removed, plus `on.push.branches`. Keep the two in sync: this list is now the +# only thing restricting which non-stacked PRs get CI. +TRUNK_PATTERNS = ("main", "worktree-rust-migration-v0.4.0", "release/**") + + +def matches_branch(ref: str, pattern: str) -> bool: + """Match a ref against one GitHub Actions branch pattern. + + Actions globs are not `fnmatch`: `*` stops at `/` while `**` crosses it, + so `release/*` must not match `release/a/b` but `release/**` must. Plain + `fnmatch` maps both to `.*` and would quietly over-match. + """ + out: list[str] = [] + i = 0 + while i < len(pattern): + if pattern.startswith("**", i): + out.append(".*") + i += 2 + elif pattern[i] == "*": + out.append("[^/]*") + i += 1 + else: + out.append(re.escape(pattern[i])) + i += 1 + return re.fullmatch("".join(out), ref) is not None + + +def is_trunk(ref: str) -> bool: + return any(matches_branch(ref, p) for p in TRUNK_PATTERNS) + + +def stack_number(response: str) -> int | None: + """Pull the stack number out of a raw GraphQL response. + + Raises on anything unexpected so the caller can fail open rather than read + a malformed payload as "not stacked". + """ + payload = json.loads(response) + if payload.get("errors"): + raise ValueError(f"GraphQL errors: {payload['errors']}") + pr = payload["data"]["repository"]["pullRequest"] + stack = pr.get("stack") + if stack is None: + return None + return int(stack["number"]) + + +def decide(event_name: str, base_ref: str, response: str | None) -> tuple[bool, str]: + """Return ``(run, reason)``. Never raises; unexpected input fails open.""" + if event_name != "pull_request": + return True, f"event is {event_name!r}, not a pull request" + if is_trunk(base_ref): + return True, f"PR targets trunk branch {base_ref!r}" + if not response or not response.strip(): + return True, ( + "::warning::FAIL-OPEN: no stack response available, so stack " + "membership is unknown and CI is running anyway. The " + "stacked-PR-only policy is NOT in effect for this run." + ) + try: + number = stack_number(response) + except (ValueError, KeyError, TypeError, json.JSONDecodeError) as exc: + return True, ( + f"::warning::FAIL-OPEN: could not read stack membership ({exc}), " + "so CI is running anyway. The stacked-PR-only policy is NOT in " + "effect for this run." + ) + if number is None: + return False, ( + f"PR targets {base_ref!r}, which is neither a trunk branch nor " + "part of a stack" + ) + return True, f"PR is part of stack #{number}" + + +SELFTEST_CASES: tuple[tuple[str, str, str | None, bool], ...] = ( + # Pushes are pre-filtered by the trigger and must never be gated off; this + # is the one path that can break the default branch. + ("push", "main", None, True), + ("workflow_dispatch", "", None, True), + # Trunk PRs keep the old behavior with no dependency on the stack API. + ("pull_request", "main", None, True), + ("pull_request", "release/v0.4.4", None, True), + ("pull_request", "release/a/b", None, True), + ("pull_request", "worktree-rust-migration-v0.4.0", None, True), + # A stacked PR into a topic branch: the case that had no CI before. + ( + "pull_request", + "chore/rmcp-3x-264", + '{"data":{"repository":{"pullRequest":{"stack":{"number":338}}}}}', + True, + ), + # A one-off PR into a topic branch: the case we intend to skip. + ( + "pull_request", + "someones/topic", + '{"data":{"repository":{"pullRequest":{"stack":null}}}}', + False, + ), + # Fail-open paths: absent, malformed, and error responses all run CI. + ("pull_request", "someones/topic", None, True), + ("pull_request", "someones/topic", "not json", True), + ( + "pull_request", + "someones/topic", + '{"errors":[{"message":"Field \'stack\' doesn\'t exist"}]}', + True, + ), + ("pull_request", "someones/topic", '{"data":{"repository":null}}', True), +) + + +def selftest() -> int: + failures = 0 + for event_name, base_ref, response, want in SELFTEST_CASES: + got, reason = decide(event_name, base_ref, response) + status = "ok" if got == want else "FAIL" + if got != want: + failures += 1 + print(f" [{status}] {event_name} -> {base_ref!r}: run={got} ({reason})") + # `release/*` must not cross a slash, or the mirror of the Actions filter + # would be wrong in the permissive direction. + if matches_branch("release/a/b", "release/*"): + print(" [FAIL] 'release/*' matched across a slash") + failures += 1 + if failures: + print(f"selftest FAILED ({failures} case(s))", file=sys.stderr) + return 1 + print(f"selftest passed ({len(SELFTEST_CASES)} cases)") + return 0 + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--event-name", default=os.environ.get("GITHUB_EVENT_NAME", "")) + parser.add_argument("--base-ref", default="") + parser.add_argument( + "--stack-response", + type=Path, + help="File holding the raw GraphQL response for this PR's stack.", + ) + parser.add_argument( + "--selftest", + action="store_true", + help="Check the decision table without touching the network.", + ) + args = parser.parse_args(argv) + + if args.selftest: + return selftest() + + response: str | None = None + if args.stack_response and args.stack_response.is_file(): + response = args.stack_response.read_text(encoding="utf-8") + + run, reason = decide(args.event_name, args.base_ref, response) + print(f"CI gate: run={str(run).lower()} — {reason}") + + output = os.environ.get("GITHUB_OUTPUT") + if output: + with open(output, "a", encoding="utf-8") as handle: + handle.write(f"run={str(run).lower()}\n") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) From 484ac77aae6327867bff3770daa43b497f44e529 Mon Sep 17 00:00:00 2001 From: Sidney Gathrid Date: Tue, 11 Aug 2026 11:00:56 -0400 Subject: [PATCH 2/3] docs(ci): correct how TRUNK_PATTERNS relates to the trigger filters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .github/workflows/ci.yml | 6 ++++-- scripts/ci_gate.py | 12 +++++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4a02545b..a3995e56 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,8 +9,10 @@ on: # stacked PR from CI entirely, leaving stacked changes reviewed with no # verification behind them. The policy moved into the `gate` job below, which # mirrors that allowlist and additionally admits PRs that belong to a GitHub - # stack. `scripts/ci_gate.py` owns the decision; TRUNK_PATTERNS there must - # stay in sync with `on.push.branches` above. + # stack. `scripts/ci_gate.py` owns the decision; its TRUNK_PATTERNS mirrors + # the removed allowlist and must be kept in sync by hand. `on.push.branches` + # above stays narrower on purpose — pushes are filtered by the trigger and + # never reach that list. pull_request: jobs: diff --git a/scripts/ci_gate.py b/scripts/ci_gate.py index 9911ec75..b3ed8167 100644 --- a/scripts/ci_gate.py +++ b/scripts/ci_gate.py @@ -31,9 +31,15 @@ import sys from pathlib import Path -# Must mirror `on.pull_request.branches` as it stood before the filter was -# removed, plus `on.push.branches`. Keep the two in sync: this list is now the -# only thing restricting which non-stacked PRs get CI. +# Mirrors the `on.pull_request.branches` allowlist as it stood before that +# filter was removed. This list is now the only thing restricting which +# non-stacked PRs get CI, and nothing enforces the correspondence, so keep it in +# sync by hand. +# +# Note this is *not* a union with `on.push.branches`, which is deliberately +# narrower (no `release/**`). Pushes never consult this list: the trigger has +# already filtered them by the time the gate runs, so `decide()` admits any +# non-PR event outright. TRUNK_PATTERNS = ("main", "worktree-rust-migration-v0.4.0", "release/**") From c43ffb1b0bfac285a2d2125eb7dc3b3ef1536efe Mon Sep 17 00:00:00 2001 From: jeremy-wayland Date: Wed, 12 Aug 2026 08:11:55 -0700 Subject: [PATCH 3/3] remove rust migration branch --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a3995e56..a160039e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,7 +2,7 @@ name: CI on: push: - branches: [main, worktree-rust-migration-v0.4.0] + branches: [main] # Deliberately unfiltered. `branches:` filters on a PR's *base*, and a stacked # PR targets its parent topic branch — so the old allowlist # ([main, worktree-rust-migration-v0.4.0, 'release/**']) excluded every