fix(admin-merge-gate): scope internal gh calls to the gated command's --repo#1054
Merged
Conversation
… --repo _check_ci_status/_query_gh_checks (ci_status.py) and _gh_pr_head_branch (codex_warden_hooks.py) never passed gh a --repo flag, relying entirely on gh's cwd-based git-remote resolution. Gating a `gh pr merge --repo <other> --admin` command from a worktree tree whose ambient remote is a DIFFERENT repo silently checked the WRONG repo's CI/PR state -- confirmed live this session merging a sliamh11/deus-v2 PR from inside the sliamh11/Deus tree, where the gate resolved an unrelated same-numbered PR on the wrong repo and produced a false "CI is red" block. Add _extract_repo_flag (command_parse.py) to capture an explicit -R/--repo value already present in the gated command (global or subcommand-local position, last-occurrence-wins, gh's attached -Rowner/repo form included), and thread it through every gh call site (_check_ci_status, _gh_pr_head_branch, _pr_matches_worktree, run_admin_merge_gate, approve_admin_merge). Additive and backward-compatible: argv is byte-identical to today when no --repo/-R is present in the gated command. Verified via 3 real end-to-end hook invocations against live GitHub state (codex_warden_hooks.py run admin-merge-gate), not just unit tests: a same-repo error path, a cross-repo case reproducing the exact bug (unscoped gh resolves the wrong repo's same-numbered PR) and confirming the fix reads the correct repo's real data, and a happy/allow path with a real green PR. 294/294 tests pass (15 new). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
Thanks for your first PR, @sliamh11! 🎉 A few things to keep in mind:
Please allow 2–3 days for review. Feel free to ask questions in the comments. |
sliamh11
added a commit
that referenced
this pull request
Jul 17, 2026
🤖 I have created a release *beep* *boop* --- ## [1.27.0](v1.26.0...v1.27.0) (2026-07-17) ### Features * **deus-v2:** validate subscription billing through credential proxy (LIA-397) ([#1035](#1035)) ([38aa247](38aa247)) ### Bug Fixes * **admin-merge-gate:** allow admin-merge on plan-limited private repos ([#1055](#1055)) ([8d283a9](8d283a9)) * **admin-merge-gate:** exclude known-advisory checks from plan-limited fallback ([#1056](#1056)) ([8e67e6a](8e67e6a)) * **admin-merge-gate:** scope internal gh calls to the gated command's --repo ([#1054](#1054)) ([545b941](545b941)) * **code-search:** sweep orphaned rowids from vec/fts on reindex (LIA-368) ([#1042](#1042)) ([fad4885](fad4885)) * **ingress:** don't kill the OAuth refresh timer on a port-conflict retry (LIA-363) ([#1020](#1020)) ([0cf61dd](0cf61dd)) * **memory:** exclude orphaned entries from FTS query (LIA-370) ([#1040](#1040)) ([0645453](0645453)) * **memory:** re-embed nodes missing an embedding row (LIA-369) ([#1041](#1041)) ([f2855b0](f2855b0)) * **memory:** repair entries_fts write-path drift (LIA-370) ([#1043](#1043)) ([1414898](1414898)) * **runtime:** handle stream errors on tool-proxy req + container stdin (LIA-362, LIA-385) ([#1021](#1021)) ([df9df3f](df9df3f)) * **security:** fail-closed gh gate for external projects (LIA-361) ([#1013](#1013)) ([ee9b62e](ee9b62e)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_check_ci_status/_query_gh_checks(ci_status.py) and_gh_pr_head_branch(codex_warden_hooks.py) never passedghan explicit--repo, relying entirely ongh's cwd-based git-remote resolution.gh pr merge --repo <other-repo> --admin <n>command run from a worktree tree whose ambient remote is a different repo had its internal CI/PR-view check silently resolve against the wrong repo — confirmed live this session merging asliamh11/deus-v2PR from inside thesliamh11/Deusworktree tree, where the gate checked an unrelated same-numbered PR on the wrong repo and produced a false "CI is red" block._extract_repo_flag(command_parse.py) to capture an explicit-R/--repovalue already present in the gated command (global or subcommand-local position, last-occurrence-wins matching gh's own precedence, gh's attached-Rowner/repoform included), and threaded it through every gh call site:_check_ci_status,_gh_pr_head_branch,_pr_matches_worktree,run_admin_merge_gate,approve_admin_merge.--repo/-Ris present in the gated command (verified by dedicated regression tests).Test plan
python3 -m pytest scripts/tests/test_codex_warden_hooks.py -q→ 294 passed (279 pre-existing + 15 new)codex_warden_hooks.py run admin-merge-gate, against live GitHub state:gh pr merge --repo sliamh11/Deus --admin 1053(real PR with a genuinely failingcicheck) → correctly denied with"CI is red ... failing checks: ci".sliamh11/Deus),gh pr merge --repo sliamh11/deus-v2 --admin --squash 14→ correctly denied using deus-v2-specific data (10 checks, TrueCourse failing in the unfiltered set) — proven distinct from what the unscopedgh pr checks 14call from the same cwd resolves to (a completely unrelatedsliamh11/DeusPR chore(deps): bump @vitest/coverage-v8 from 4.0.18 to 4.1.2 #14, "bump @vitest/coverage-v8", with its own failingci). That mismatch is the exact pre-fix bug.sliamh11/Deus(0 non-passing required checks, independently confirmed) →approve_admin_merge+ the real PreToolUse hook invocation both succeed, hook allows (empty output, nopermissionDecision), one-shot marker consumed as designed.Known follow-up (not in scope here)
_extract_pr_refstill doesn't recognize gh's attached-Rowner/reposhort form (only_extract_repo_flag, added in this PR, handles it). Low-probability edge case (would only matter if the attached form is combined with a positional PR ref); flagged by code-review as a candidate follow-up, not a regression from this change.sliamh11/deus-v2is a private repo without GitHub Pro, so GitHub's classic branch-protection API returns 403 for it, making_check_ci_statusclassify every deus-v2 PR as_CI_STATUS_NO_REQUIRED(fail-closed) regardless of actual CI state. This is an orthogonal billing/plan constraint, surfaced to the user separately — this PR's fix is still correct and necessary (it was actively causing the gate to check a totally wrong, unrelated PR).🤖 Generated with Claude Code