Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions scripts/codex_warden_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
from warden_hooks.command_parse import ( # noqa: E402
_command_hash,
_extract_pr_ref,
_extract_repo_flag,
_gh_command_index_after_global_flags,
_is_admin_merge_command,
_is_gh_executable,
Expand Down Expand Up @@ -866,7 +867,8 @@ def approve_admin_merge(command: str, repo_root: Path) -> int:
pr_ref = _extract_pr_ref(command)
# Current-branch merges (no ref) pass ``gh pr checks`` the branch name
check_ref = pr_ref or "HEAD"
status, detail = _check_ci_status(check_ref)
repo = _extract_repo_flag(command)
status, detail = _check_ci_status(check_ref, repo=repo)
block = _ci_block_reason(check_ref, status, detail)
if block:
print(block, file=sys.stderr)
Expand Down Expand Up @@ -1700,15 +1702,23 @@ def _verdict_in(verdicts: dict[str, Any], marker_name: str) -> str | None:
return None


def _gh_pr_head_branch(ref: str, timeout: int = 3) -> str | None:
def _gh_pr_head_branch(
ref: str, timeout: int = 3, repo: str | None = None
) -> str | None:
"""Resolve a PR ref (number or URL) to its head branch via ``gh pr view``.

Returns None on any failure so the caller fails safe (treats it as an
unverifiable match and falls through to the one-shot approval path).

*repo* scopes the lookup to an explicit ``OWNER/REPO`` instead of ``gh``'s
cwd-based resolution — see ``_query_gh_checks`` for why this matters.
"""
argv = ["gh", "pr", "view", ref, "--json", "headRefName"]
if repo:
argv.extend(["--repo", repo])
try:
result = subprocess.run(
["gh", "pr", "view", ref, "--json", "headRefName"],
argv,
text=True,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
Expand Down Expand Up @@ -1742,7 +1752,8 @@ def _pr_matches_worktree(command: str, wt: Path) -> tuple[bool, str]:
ref = _extract_pr_ref(command)
if ref is None or ref == wt_branch:
return (True, "")
head = _gh_pr_head_branch(ref)
repo = _extract_repo_flag(command)
head = _gh_pr_head_branch(ref, repo=repo)
if head is None:
return (
False,
Expand Down Expand Up @@ -1842,7 +1853,8 @@ def run_admin_merge_gate(event: dict[str, Any], repo_root: Path) -> int:

pr_ref = _extract_pr_ref(command)
check_ref = pr_ref or "HEAD"
ci_status, ci_detail = _check_ci_status(check_ref)
repo = _extract_repo_flag(command)
ci_status, ci_detail = _check_ci_status(check_ref, repo=repo)
ci_block = _ci_block_reason(check_ref, ci_status, ci_detail)
if ci_block:
_block_pre_tool(ci_block)
Expand Down
244 changes: 244 additions & 0 deletions scripts/tests/test_codex_warden_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -2083,6 +2083,61 @@ def fake_run(cmd, *args, **kwargs):
]


def test_check_ci_status_no_repo_argv_unchanged(monkeypatch):
# Backward-compat guard: omitting `repo` must produce byte-identical argv
# to today (no trailing --repo), independent of the assertion above.
hooks = load_hooks()
captured = {}

def fake_run(cmd, *args, **kwargs):
if (
isinstance(cmd, (list, tuple))
and len(cmd) >= 3
and str(cmd[0]).endswith("gh")
and cmd[1] == "pr"
and cmd[2] == "checks"
):
captured["cmd"] = list(cmd)
return subprocess.CompletedProcess(
cmd, 0, stdout=json.dumps([{"bucket": "pass", "name": "ci"}]), stderr=""
)
return _REAL_SUBPROCESS_RUN(cmd, *args, **kwargs)

monkeypatch.setattr(hooks.subprocess, "run", fake_run)
hooks._check_ci_status("123")
assert "--repo" not in captured["cmd"]


def test_check_ci_status_explicit_repo_scopes_gh_call(monkeypatch):
# Fixes the confirmed cross-repo bug: when the gated command carries an
# explicit repo, the gate's own internal gh call must be scoped to it,
# not to whatever repo the cwd's git remote happens to resolve.
hooks = load_hooks()
captured = {}

def fake_run(cmd, *args, **kwargs):
if (
isinstance(cmd, (list, tuple))
and len(cmd) >= 3
and str(cmd[0]).endswith("gh")
and cmd[1] == "pr"
and cmd[2] == "checks"
):
captured["cmd"] = list(cmd)
return subprocess.CompletedProcess(
cmd, 0, stdout=json.dumps([{"bucket": "pass", "name": "ci"}]), stderr=""
)
return _REAL_SUBPROCESS_RUN(cmd, *args, **kwargs)

monkeypatch.setattr(hooks.subprocess, "run", fake_run)
status, _ = hooks._check_ci_status("14", repo="owner/other-repo")
assert status == hooks._CI_STATUS_GREEN
assert captured["cmd"] == [
"gh", "pr", "checks", "14", "--json", "bucket,name", "--required",
"--repo", "owner/other-repo",
]


def test_check_ci_status_advisory_pending_does_not_block(monkeypatch):
# Required checks all pass; advisory checks (TrueCourse etc.) still pending in
# the unfiltered set. The gate sees only required → GREEN (no block).
Expand Down Expand Up @@ -2348,6 +2403,77 @@ def test_extract_pr_ref_body_flag_before_positional():
assert hooks._extract_pr_ref('gh pr merge --squash -b "fix: blah" 294') == "294"


# ── _extract_repo_flag ───────────────────────────────────────────────────────


def test_extract_repo_flag_global_position():
hooks = load_hooks()
assert (
hooks._extract_repo_flag("gh --repo owner/repo pr merge 294 --admin")
== "owner/repo"
)


def test_extract_repo_flag_short_flag_global_position():
hooks = load_hooks()
assert (
hooks._extract_repo_flag("gh -R owner/repo pr merge 294 --admin")
== "owner/repo"
)


def test_extract_repo_flag_short_flag_attached_form():
# gh's short-flag attached form: `-Rowner/repo` (no space).
hooks = load_hooks()
assert (
hooks._extract_repo_flag("gh pr merge -Rowner/repo --admin 294")
== "owner/repo"
)


def test_extract_repo_flag_subcommand_local_position():
# The shape production landing commands actually use:
# `gh pr merge --repo owner/repo --admin --squash <n>`.
hooks = load_hooks()
assert (
hooks._extract_repo_flag("gh pr merge --repo owner/repo --admin --squash 294")
== "owner/repo"
)


def test_extract_repo_flag_equals_form():
hooks = load_hooks()
assert (
hooks._extract_repo_flag("gh pr merge --repo=owner/repo --admin 294")
== "owner/repo"
)


def test_extract_repo_flag_absent_returns_none():
hooks = load_hooks()
assert hooks._extract_repo_flag("gh pr merge --admin 294") is None


def test_extract_repo_flag_duplicate_last_wins():
# Mirrors gh's own last-flag-wins precedence for repeated flags.
hooks = load_hooks()
assert (
hooks._extract_repo_flag(
"gh --repo first/repo pr merge --repo second/repo --admin 294"
)
== "second/repo"
)


def test_extract_repo_flag_does_not_misread_other_flag_values():
# A body value that happens to look flag-like must not be misread as
# a repo flag -- `_FLAGS_WITH_VALUE` skips it correctly.
hooks = load_hooks()
assert (
hooks._extract_repo_flag('gh pr merge --admin -b "--repo" 294') is None
)


# ── CI gate integration: run_admin_merge_gate ────────────────────────────────


Expand All @@ -2371,6 +2497,58 @@ def test_admin_merge_gate_blocks_when_ci_red(monkeypatch, tmp_path, capsys):
assert "gh pr checks 294" in reason


def test_admin_merge_gate_scopes_ci_check_to_explicit_repo(monkeypatch, tmp_path, capsys):
# Regression test for the confirmed cross-repo bug: a `gh pr merge --repo
# <other>` command must be graded against THAT repo's CI, not whatever the
# worktree's own git remote happens to resolve to. Simulate the exact
# failure mode -- CI is genuinely GREEN on the named repo but would read
# RED if the gate ever queried unscoped -- and confirm the gate allows
# (pre-approved) using the scoped query, never the unscoped one.
hooks = load_hooks()
repo = git_repo(tmp_path)
command = "gh pr merge --repo owner/other-repo --admin --squash 14"
captured = {}

def fake_run(cmd, *args, **kwargs):
if (
isinstance(cmd, (list, tuple))
and len(cmd) >= 3
and str(cmd[0]).endswith("gh")
and cmd[1] == "pr"
and cmd[2] == "checks"
):
captured["cmd"] = list(cmd)
if "--repo" in cmd:
# Scoped query: this is the real target repo, CI is green.
return subprocess.CompletedProcess(
cmd, 0, stdout=json.dumps([{"bucket": "pass", "name": "ci"}]), stderr=""
)
# Unscoped query: simulates the bug -- resolves to a DIFFERENT,
# unrelated PR whose CI is red. If the gate ever calls gh without
# --repo here, this branch fires and the test fails loudly below.
return subprocess.CompletedProcess(
cmd, 1, stdout=json.dumps([{"bucket": "fail", "name": "ci"}]), stderr=""
)
return _REAL_SUBPROCESS_RUN(cmd, *args, **kwargs)

monkeypatch.setattr(hooks.subprocess, "run", fake_run)

marker = repo / ".claude" / ".admin-merge-approved"
marker.parent.mkdir(parents=True, exist_ok=True)
marker.write_text(
json.dumps({"command_hash": hooks._command_hash(command), "command": command}),
encoding="utf-8",
)

rc = hooks.run_admin_merge_gate(bash_event(repo, command), repo)

assert rc == 0
assert not marker.exists() # consumed -- approval matched, no denial
assert "permissionDecision" not in capsys.readouterr().out
assert "--repo" in captured["cmd"]
assert captured["cmd"][captured["cmd"].index("--repo") + 1] == "owner/other-repo"


def test_admin_merge_gate_blocks_when_ci_pending(monkeypatch, tmp_path, capsys):
hooks = load_hooks()
repo = git_repo(tmp_path)
Expand Down Expand Up @@ -4821,6 +4999,72 @@ def test_pr_matches_worktree_no_ref_matches_current_branch(tmp_path):
assert matched is True # no explicit ref => current branch => this worktree's PR


def test_gh_pr_head_branch_no_repo_argv_unchanged(monkeypatch):
hooks = load_hooks()
captured = {}

def fake_run(cmd, *args, **kwargs):
captured["cmd"] = list(cmd)
return subprocess.CompletedProcess(
cmd, 0, stdout=json.dumps({"headRefName": "some-branch"}), stderr=""
)

monkeypatch.setattr(hooks.subprocess, "run", fake_run)
head = hooks._gh_pr_head_branch("294")
assert head == "some-branch"
assert captured["cmd"] == ["gh", "pr", "view", "294", "--json", "headRefName"]


def test_gh_pr_head_branch_explicit_repo_scopes_gh_call(monkeypatch):
hooks = load_hooks()
captured = {}

def fake_run(cmd, *args, **kwargs):
captured["cmd"] = list(cmd)
return subprocess.CompletedProcess(
cmd, 0, stdout=json.dumps({"headRefName": "lia-410-branch"}), stderr=""
)

monkeypatch.setattr(hooks.subprocess, "run", fake_run)
head = hooks._gh_pr_head_branch("14", repo="owner/other-repo")
assert head == "lia-410-branch"
assert captured["cmd"] == [
"gh", "pr", "view", "14", "--json", "headRefName",
"--repo", "owner/other-repo",
]


def test_pr_matches_worktree_threads_explicit_repo_to_gh_pr_view(monkeypatch, tmp_path):
# End-to-end: a `gh pr merge --repo o/r <ref>` command must scope the
# PR-head-branch lookup to that same repo, not the worktree's own remote.
hooks = load_hooks()
repo = git_repo(tmp_path)
_commit_repo(repo)
captured = {}

def fake_run(cmd, *args, **kwargs):
if (
isinstance(cmd, (list, tuple))
and len(cmd) >= 3
and str(cmd[0]).endswith("gh")
and cmd[1] == "pr"
and cmd[2] == "view"
):
captured["cmd"] = list(cmd)
return subprocess.CompletedProcess(
cmd, 0, stdout=json.dumps({"headRefName": "other-branch"}), stderr=""
)
return _REAL_SUBPROCESS_RUN(cmd, *args, **kwargs)

monkeypatch.setattr(hooks.subprocess, "run", fake_run)
matched, _ = hooks._pr_matches_worktree(
"gh pr merge --repo owner/other-repo --admin 999", repo
)
assert matched is False # head branch differs from this worktree's branch
assert "--repo" in captured["cmd"]
assert captured["cmd"][captured["cmd"].index("--repo") + 1] == "owner/other-repo"


def test_block_message_diagnoses_bucket_mismatch(tmp_path):
"""A 'not run yet' model backend whose SHIP sits in a SIBLING bucket gets a
pointer to that bucket instead of a silent retry."""
Expand Down
Loading
Loading