From 75947f80e1fc7799d254022331f2ad515b792285 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Wed, 2 Sep 2026 04:44:00 -0700 Subject: [PATCH] ci: close two version/CI gaps a green PR cannot show you MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TASK-100, two of its three parts. The third (strict: true, so a PR's checks must have run against current main) is a branch-protection change with a measured blast radius and is raised separately. 1. The version guard could not see a PR PAIR. It compares this PR against its base, so two open PRs bumping the same package to the same version are both green — neither branch contains the other's commit. Measured, not assumed: two branches both taking cli 0.1.30 to 0.1.31 while touching different files under cli/src merge with NO conflict, and the result is a single 0.1.31 holding both PRs' source. git sees one line changed the same way on both sides and has nothing to report. npm then carries a version mapping to an artifact neither PR alone produced — the exact defect of #979 and #1017. Older PR keeps the version, newer picks the next, so one author can always clear it alone. Only an ADDED version line counts: every stale branch carries an old package.json and proposes nothing by doing so. 2. Stacked PRs are under-gated, and it reads as a full green. Every guard here is `branches: [main]`, so none runs on a PR based on another feature branch. The three open stacked PRs (#1219, #1172, #1132) carry 4-5 checks each against ~12 on a main-based PR; absent from all three are the version guard, the stale-base guard and CodeQL. Nothing counts checks, so a short green looks like a clean one. The new guard therefore has no branches filter — a check scoped to main cannot see the PRs it exists to catch. Both gh calls in the pairs arm fail closed. An unchecked error yields an empty version list, which reads as "this PR bumps nothing" and passes: the guard at its most reassuring exactly when blind. Co-Authored-By: Claude Opus 5 --- .github/workflows/package-version-guard.yml | 62 +++++++++++++++++++++ .github/workflows/pr-base-guard.yml | 47 ++++++++++++++++ 2 files changed, 109 insertions(+) create mode 100644 .github/workflows/pr-base-guard.yml diff --git a/.github/workflows/package-version-guard.yml b/.github/workflows/package-version-guard.yml index b0f44f388..b0ae62aad 100644 --- a/.github/workflows/package-version-guard.yml +++ b/.github/workflows/package-version-guard.yml @@ -25,6 +25,7 @@ on: permissions: contents: read + pull-requests: read concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number }} @@ -121,3 +122,64 @@ jobs: done exit $fail + + # The check above compares this PR against the base. It cannot see a + # SECOND open PR bumping the same package to the same version, because + # neither branch contains the other's commit — and that pair merges + # CLEAN. Measured, not assumed: two branches both taking cli 0.1.30 to + # 0.1.31 while touching different files under cli/src merge with no + # conflict, and the result is one 0.1.31 holding both PRs' source. Each + # PR's guard was green the whole time. npm then has a version that maps + # to an artifact neither PR alone produced, which is the exact defect + # #979 and #1017 were. + # + # Older PR keeps the version, newer picks the next one — so it is always + # fixable by one author alone. Failing both would be a deadlock. + - name: No older open PR is taking this package to the same version + env: + GH_TOKEN: ${{ github.token }} + PR: ${{ github.event.pull_request.number }} + REPO: ${{ github.repository }} + shell: bash + run: | + set -euo pipefail + + # Every gh call is checked. An unchecked failure yields an empty + # version list, which reads as "this PR bumps nothing" and PASSES — + # the guard at its most reassuring exactly when it has gone blind. + # Same reason the `|| true` came off the fetch above. + proposed() { + local pr="$1" pkg="$2" patch + if ! patch=$(gh api "repos/$REPO/pulls/$pr/files" --paginate \ + --jq ".[] | select(.filename == \"$pkg/package.json\") | .patch"); then + echo "::error::gh api failed reading PR #$pr. Not passing on a check that could not run." >&2 + return 1 + fi + # Only an ADDED version line counts. A PR that merely carries an + # old package.json (every stale branch does) proposes nothing. + printf '%s\n' "$patch" | sed -n 's/^+.*"version": "\([^"]*\)".*/\1/p' | head -1 + } + + if ! older=$(gh api "repos/$REPO/pulls?state=open&per_page=100" --paginate \ + --jq ".[] | select(.number < $PR) | .number"); then + echo "::error::gh api failed listing open PRs. Not passing on a blind check." + exit 1 + fi + + fail=0 + for pkg in cli commonly-mcp; do + mine=$(proposed "$PR" "$pkg") + [ -z "$mine" ] && { echo "· $pkg: this PR proposes no version"; continue; } + echo "· $pkg: this PR proposes $mine" + for other in $older; do + theirs=$(proposed "$other" "$pkg") + [ "$theirs" = "$mine" ] || continue + echo "::error file=$pkg/package.json::$pkg $mine is already claimed by the older open PR #$other. Both bumps merge clean — git sees the same line changed the same way — so npm would carry one $mine built from both PRs' source. #$other keeps $mine; pick the next version here." + fail=1 + done + done + + if [ "$fail" = "0" ]; then + echo "✓ no older open PR claims these versions" + fi + exit $fail diff --git a/.github/workflows/pr-base-guard.yml b/.github/workflows/pr-base-guard.yml new file mode 100644 index 000000000..22c6cedcd --- /dev/null +++ b/.github/workflows/pr-base-guard.yml @@ -0,0 +1,47 @@ +name: PR Base Guard + +# Every other guard in this repo is declared `on: pull_request: branches: +# [main]`, so none of them runs on a PR whose base is another feature branch. +# Measured on the three open stacked PRs (#1219, #1172, #1132): 4-5 checks +# each, against ~12 on a main-based PR. Missing from all three are the version +# guard, the stale-base guard and the CodeQL analyze jobs. +# +# So the filter every guard uses to scope itself excludes precisely the PRs +# that are least gated, and the result reads as a full green rather than a +# short one. Nothing counts checks. +# +# A stacked base is also an auto-close dependency: when the parent merges and +# its branch is deleted, GitHub closes or silently retargets the child, and a +# retarget lands a diff that was never CI'd against main. +# +# Deliberately has NO branches filter. A guard that scopes itself to main +# cannot see the thing it is checking for. + +on: + pull_request: + types: [opened, synchronize, reopened, ready_for_review, edited] + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number }} + cancel-in-progress: true + +jobs: + base-is-main: + name: PR targets main + runs-on: ubuntu-latest + steps: + - name: The base branch must be main + env: + BASE: ${{ github.event.pull_request.base.ref }} + shell: bash + run: | + set -euo pipefail + if [ "$BASE" = "main" ]; then + echo "✓ base is main" + exit 0 + fi + echo "::error::This PR targets '$BASE', not main. Guards in this repo are scoped \`branches: [main]\`, so most of them never run here — a stacked PR shows a green that is short, not clean. Retarget to main and rebase; if the parent must land first, say so on the PR and land it, but do not merge this against an un-CI'd base." + exit 1