[kyle-sexton]

No related issue: GitHub Support proposal, switch flips reach open PRs

Summary

ci-status's hold steps read their config (pr-pipeline.yaml, the trusted-actor list, the tracker config, merge-holds.mjs and its siblings) from a sparse checkout at github.event.pull_request.base.sha. A re-run replays the event payload, so that SHA stays stale: a policy switch merged to main later, such as merge.diff-check.path-holds or merge.trust-path-hold-apps, never reached an open pull request until a new event with a newer base arrived. #11072, #10919, #10973 and #10983 hit this.

Fix

• New step hold_ref ("Resolve the hold config's commit"), same if: as base_config. For a pull_request event whose base ref is the repository's default branch, it reads the branch tip with an unauthenticated git ls-remote https://github.com/$GITHUB_REPOSITORY refs/heads/<default>, run from $RUNNER_TEMP (outside any checkout) under GIT_TERMINAL_PROMPT=0 and a 60 s timeout. It keeps only the line whose ref is exactly refs/heads/<default> and requires 40 hex. Any other base, and a merge group, keep the event's base SHA. A failed or malformed read falls back to the base SHA with a ::warning:: naming the fallback. Every event value comes through env.
• base_config checks out ${{ steps.hold_ref.outputs.sha }}, still a 40-hex SHA, so actions/checkout still skips its API calls. Its if: also requires that output to be non-empty: with an empty ref, checkout falls back to the event's ref, the pull request's own merge commit. hold_ref exits 1 without an output when the event's base SHA is not 40 hex, so the checkout skips and the hold steps fail closed on an empty BASE_CONFIG, as they already do.
• The comment above the step and the five error messages from steps that read BASE_CONFIG now say the config is the default branch's tip at run time, or the base commit for another base. The safety argument stands: no pull request code is checked out, and a PR head cannot change main.
• No trigger, permission, token, concurrency or sparse-list change. The hold-floor hold_patterns block (chore(pr-pipeline): turn off the agent App trust-path and exempt-author path holds #11068) is untouched.

Verification

• scripts/ci-hold-human-gated.test.sh now pins the checkout's ref: to hold_ref and its non-empty guard. It also runs hold_ref's body against a stub git in eight cases: default-branch tip, another base, merge group, ls-remote failure, non-hex answer, tail-matching decoy ref, decoy plus exact ref, and a non-SHA base. Before the workflow change: PASS=56 FAIL=18. After: PASS=74 FAIL=0.
• All 20 scripts/*.test.sh scoped to this workflow pass, including the four whose message pins changed.
• actionlint clean; zizmor --offline no findings; scripts/check-lane-coverage.sh --check passes. hold_ref has no continue-on-error, so it needs no opt-out entry.

Related

• feat(pr-pipeline): turn path-based merge holds off behind one switch (owner Q15) #11058 built the path-holds read this makes reach open pull requests.
• chore(pr-pipeline): turn off the agent App trust-path and exempt-author path holds #11068 edits a different step of this workflow. This PR stays out of its hunks.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-30T02:51:18.584403Z	abcf39a	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

claude-security-review reviewed abcf39a74a9f28d5e3aa22cac6332f14a19f8895: 0 finding(s), 0 as inline comments.

No security issues found in this pull request.

I read the whole diff. It changes the ci-status hold steps in .github/workflows/pr-require-checks.yml (a trusted path) and four test scripts.

What the change does. A new hold_ref step picks the commit that the hold config is read from. For a pull_request event whose base is the default branch, it reads that branch's tip with git ls-remote. For any other base, and for a merge group, it keeps the event's base SHA. The base_config checkout now uses that output, and its if: skips the checkout when the output is empty.

Who can change the config source.

• The tip comes from https://github.com/$GITHUB_REPOSITORY, using the base repository's github.event.repository.default_branch. A fork's head or a pull request's own commit cannot reach it.
• Only a merge to the default branch moves it, and the old base SHA was already a commit on that branch. The trust level is unchanged, and the default branch's rulesets and required reviews still apply.
• The checkout keeps persist-credentials: false and checks out no pull request code.

Injection and parsing.

• Every event value (EVENT, BASE_REF, DEFAULT_BRANCH, BASE_SHA) enters the run: body through env. No ${{ }} expression sits inside the script.
• The awk filter keeps only the line whose ref is exactly refs/heads/<default>. The value must match ^[0-9a-f]{40}$ before it is written to $GITHUB_OUTPUT. A decoy ref or non-hex output cannot inject a ref or an extra output line, and the tests cover both cases.
• A base SHA that is not 40-hex exits 1 without writing an output. The checkout then skips and BASE_CONFIG stays empty, which the downstream hold steps already treat as a failure. That also stops the checkout falling back to the pull request's own merge commit.

Failure modes.

• If ls-remote fails or returns bad data, the step falls back to the event's base SHA with a warning. That is the behaviour before this change, so it is no weaker.
• The tip read at run time differs from the base commit only by commits already merged to the default branch. A looser policy reaching open pull requests would take a merge by a trusted actor.
• The reads of merge.trust-path-hold-apps, merge.agent-identity and the lanes App id still fail closed on an unreadable config. The remaining changes are message wording and test pins.

critical-findings: 0

Spend: no paid resources or billing settings change. The step adds one unauthenticated git ls-remote call per ci-status run.

[github-actions]

claude-review reviewed abcf39a74a9f28d5e3aa22cac6332f14a19f8895: 0 finding(s), 0 as inline comments.

No blocking or advisory findings. I read all 5 changed files in the diff and the PR context. I ran nothing, and the PR's pass counts (PASS=74 FAIL=0) are the author's claim, not something I confirmed.

What the change does. A new hold_ref step resolves the commit the hold config is checked out at (.github/workflows/pr-require-checks.yml). For a pull_request into the default branch it reads the tip of that branch with an unauthenticated git ls-remote. Any other base, and every merge group, keeps the event's base SHA. base_config now checks out steps.hold_ref.outputs.sha.

Checks that held up:

• Trust boundary: the tip of the default branch is not pull request code, because a pull request head cannot change it. The ref is still a 40-hex SHA, so actions/checkout still skips its API calls. No credential is persisted, and the trigger, permission and token surface is unchanged.
• Empty ref: base_config's if: requires a non-empty hold_ref output. That stops actions/checkout from falling back to the event's ref, which is the pull request's own merge commit. A non-SHA base makes hold_ref exit 1 with no output, so the checkout is skipped and every hold step fails closed on an empty BASE_CONFIG, as they already do.
• Expression injection: event values reach the run body only through env, and the new test asserts the body contains no ${{.
• Ref matching: awk matches $2 == "refs/heads/<default>" exactly, so a decoy such as refs/heads/x/refs/heads/main is rejected. The 40-hex check runs on the result.
• Failure handling: a failed ls-remote is not caught by || tip=, because there is no pipefail. It is still handled: tip is empty, so the hex check fails and the base SHA is used with a warning. A step that can fail cannot silently drop the hold, since !cancelled() lets the hold steps run after it.
• Tests: the new stub-git cases cover the default-branch tip, another base, a merge group, an ls-remote failure, a non-hex answer, a decoy ref, a decoy plus the exact ref, and a non-SHA base. The four message pins in ci-hold-pr-unblock-status.test.sh, ci-lanes-pr-hold.test.sh and ci-review-lanes.test.sh match the new workflow strings.

Not blocking:

• Several comments still call the config "the base commit's", for example .github/workflows/pr-require-checks.yml lines 4237, 4593, 4689, 4936, 5562 and 5667. They are now loosely worded for the default-branch case, but the PR's rewritten block comment states the rule and the comments are not wrong enough to block.
• The step keeps the name "Check out the base commit's hold config", and the test pins that name.

Cost: none. The change adds one git ls-remote call to an existing job and no billed resource.

No manifest items are hit.