Repository navigation
land-pr skill; outbound-guard refuses raw merges (tools/pr-land.py is the way) - #1056
Merged
Merged
Conversation
…o not allow gate is the read-only verdict: the PR open, not a draft, on main or release/v*, at the asserted head, every required context of the base's rules reported and passing, nothing pending, failed or cancelled, mergeStateStatus CLEAN (UNKNOWN and an all-green BLOCKED are re-polled; BEHIND and DIRTY say to merge the base into the branch). wait polls it with a 30-60 s backoff, never gh run list. merge is wait, then gh pr merge --squash --match-head-commit (never --auto) with a required body, retrying only the head-oid race, then a verify against origin/<base>. The subject and body go through outbound-guard in-process before any gh call. Exit codes: 0 green, 1 red, 2 usage, 3 refused, 4 timeout, 5 head moved. tools/test_pr_land.py runs it against a fake gh on PATH with an injected sleeper, real gh JSON as fixtures, and a temporary git origin for the verify. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bHMduhNfrC8w3qjNhEWdB
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bHMduhNfrC8w3qjNhEWdB
…s the merge went through UNSTABLE is GitHub's "mergeable with a non-passing commit status": the required checks pass while a non-required one (CodeQL) still runs or failed. It was refused by name as an unknown state, so a merge started in that window exited 3 at once and never waited, and a failing non-required check gave exit 3 instead of 1 naming the check. It now goes to the rows like BLOCKED: a failed row is exit 1 naming it, a running row is exit 4, all rows passing is lag (exit 4 after the bounded re-poll), and green still needs CLEAN. The verify step after a successful merge: every VERIFY FAILED line starts "gh pr merge succeeded;" (exit 1 also means a red check with nothing merged); a MERGED state with no merge commit is worded on its own; and odd JSON (a check-run without a name, a non-object mergeCommit) is noted, never a traceback. A required_status_checks entry without a context is refused instead of a KeyError. A head race that never clears while the head stays at S is exit 3, not 5. The module docstring states the order judge() actually applies. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bHMduhNfrC8w3qjNhEWdB
…ow it squashes `restack --worktree W --upper BRANCH --lower PR [--dry-run] [--push]` runs AGENTS.md's four merges on a stacked branch once the pull request below it has squash-merged: the lower PR's final head (fetched from refs/pull/PR/head, since the branch is deleted on merge), the squash's first parent, `-s ours` on the squash, then origin/<base>. It refuses (3) unless the lower PR is MERGED with a one-parent merge commit on origin/<base> and W is a clean checkout on BRANCH; stops at the first failing step, names it and its command, and exits 1 with the tree as git left it (run it again after resolving: the finished steps are no-ops); --dry-run prints the four commands and changes nothing, not even by a fetch; --push pushes BRANCH only, plain, after all four succeeded. It never calls `gh pr merge` and grants no merge authority. tools/test_pr_land.py covers it on a disposable bare origin and clone with the fake gh: the upper diff against origin/main equals its own change (the later main commit edits a file the lower PR added, which only merges cleanly because step 3 made the squash the merge base), a conflict at step 1, 2 and 4 is named and left in progress, a resolved conflict resumes, --dry-run leaves HEAD, status and every ref alone, the three refusals, and --push moves refs/heads/BRANCH only and a rejected push is never forced. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bHMduhNfrC8w3qjNhEWdB
…erges The land-pr skill documents tools/pr-land.py (gate, wait, merge, restack): commands, exit codes, long waits under run_in_background, the squash body, the four-merge restack and the head-oid race, that the tool grants no merge authority, and the owner's PLX_MERGE_BYPASS=1 hatch. AGENTS.md's two manual merge and restack bullets become one pointer to the tool and the skill. outbound-guard gains merge_verdict, a second question beside its private-value scan: it refuses a bare pull-request merge on the CLI, a non-GET gh api call to the pulls/N/merge endpoint (explicit method, or a field that makes gh send a POST) and a GraphQL mergePullRequest, matching the command and never the words (a grep, an echo, a commit message, a heredoc and a GET of the merge status are allowed). The refusal names tools/pr-land.py and the hatch and never echoes the command. outbound-guard also scans pr-land.py merge's --subject and --body-file like any other outbound text. settings.json denies the GitHub MCP connector's merge_pull_request tool; the guard's test asserts the rule is present. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bHMduhNfrC8w3qjNhEWdB
…the credential scan passes ci/source_bundle.py refuses any file that holds the Plex token key next to a 20+ character value. The squash-message shape-rule test deliberately writes such a value, so the literal tripped the repository tree scan (PR #1053). The value is now a module constant formatted into the body, as .claude/hooks/outbound-guard-test.py does; the runtime string is unchanged, so the shape rule is still what refuses it. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bHMduhNfrC8w3qjNhEWdB
…stream on --push, says why a push failed Review round on the restack subcommand, five items: * checked_out also refuses (3) when MERGE_HEAD, CHERRY_PICK_HEAD or REVERT_HEAD resolves. A merge a human resolved and staged but did not commit has an index equal to HEAD, so `status --porcelain` passed it, the fetch ran, step 1 died on "You have not concluded your merge", and stopped() named the wrong step. * --push resolves the branch's upstream. One on origin is pushed as `upper:refs/heads/x` (a checkout made by `git checkout -B tmp origin/x` used to create a stray `tmp` on origin and leave the PR alone), none or a local one as before, and one on any other remote is refused (3) before the first merge. Never forced. * PUSH FAILED prints the `!`, `error:`, `remote:` and `fatal:` lines instead of `To <url>`, the first line of a rejected push, which dropped the reason. * Step 2's message names the lower PR's base instead of main; the usage docstring lists every exit-3 refusal of restack. * --upper equal to the lower PR's base branch is refused (3) before anything is fetched or merged. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bHMduhNfrC8w3qjNhEWdB
…t run time Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bHMduhNfrC8w3qjNhEWdB
…scan fixture Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bHMduhNfrC8w3qjNhEWdB
… skill fixes merge_verdict graded every segment segments() returned, and segments() recurses into $(...) splitting the inside on newlines without units(), so a heredoc body opened inside a substitution (git commit -m "$(cat <<'EOF' ... EOF)", gh pr create --body "$(cat ...)") was graded as commands: a body line starting `gh pr merge` refused the whole command, and only when the body held no parenthesis pair. merge_candidates cuts each substitution out and runs its inside through units(), the way a top-level heredoc is handled. segments() and the private-value scan are unchanged. land-pr SKILL.md: correct the step 2/3/4 sentence, write step 4 as origin/<base>, add the exit-3 case of three unchanged head-race retries, and the restack refusals the restack lane added. pr-land.py and test_pr_land.py: point the three docstrings that cited the removed AGENTS.md bullets at the skill. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bHMduhNfrC8w3qjNhEWdB
The merge guard tests the command text and heredoc bodies only, so a GraphQL mergePullRequest read from a query file (-F query=@f, --input f) is not seen; a --body-from-pr body is scanned once, by the script alone, after it is fetched; the exit-3 row merges origin/<base>, not always origin/main; and one test deliberately lets a hung gh hit a 1 s timeout, so "nothing slept" was overstated. Prose, docstrings and comments only. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014bHMduhNfrC8w3qjNhEWdB
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.
Stacked on #1055, which is stacked on #1053 (the base is #1055's branch, so the diff is only this change). When those squash, restack this with
tools/pr-land.py restack.What merging this changes for everyone
In a Claude session in this repo, a raw merge is now refused by the
outbound-guardPreToolUse hook (exit 2, with a message pointing attools/pr-land.pyand theland-prskill):gh pr merge ...with any arguments, including--autoand--admingh api .../pulls/N/merge(explicit-X/--method, or any-f/-F/--field/--raw-field/--inputthat makes gh send a POST)mergePullRequestThe hatch for the owner's own manual merge is
PLX_MERGE_BYPASS=1on the command.mcp__claude_ai_GitHub_MCP__merge_pull_requestis added topermissions.denyin.claude/settings.json.tools/pr-land.py mergeis not refused: it is the one place that judges the required checks, merges with--match-head-commit, and verifies the squash landed. It grants no merge authority; whether a PR may merge is still the owner's decision, per PR or per series. Its--subjectand--body-filenow get the private-value scan like other outbound text.Files
.agents/skills/land-pr/SKILL.md(99 lines): when to usegate/wait/merge/restack, the exit codes and what to do on each, how to run a long wait in the background, the squash body, the four-merge restack recipe and why each step exists, the hatch.AGENTS.md: the two manual bullets (the stacked-PR restack recipe and the--autohead-oid race) become one pointer to the tool and the skill. Nothing else in AGENTS.md changes..claude/hooks/outbound-guard.pyand its test: the refusals above. A heredoc opened inside$(...)(how a commit message or PR body is written) stays a body, so prose that mentionsgh pr mergein it is not refused..claude/settings.json: the one deny rule; the guard test asserts it is present.ci/check-python-steps.txt: a comment now says gate / wait / merge / restack.tools/pr-land.pyandtools/test_pr_land.pycite the skill instead of the removed AGENTS.md text.Verification
python3 .claude/hooks/outbound-guard-test.py: 72/72 (written first; 45 FAIL lines before the refusal existed, then 9 more before the heredoc fix)python3 tools/test_pr_land.py: 88 tests OK;python3 ci/test_source_bundle.py: 43 OK;python3 ci/test_ci_split.py: OKenv/time/sudo/-R, after&&;|, inside$(), REST and GraphQL). The guard's latency on a no-merge command is unchanged (25 ms median before and after).Known limits (cooperative guard, like the rest of the hook)
gh pr mergeon a line after a-m "$(cat <<'EOF' ... EOF)"commit, separated by a newline, is not refused (units()swallows the later lines as that heredoc's body). The same command joined with&&is refused. Fixing it means changingunits(), which the private-value scan shares; left for its own change.timeout/nicewrappers, backtick substitution,bash -c,eval,xargs, a GraphQL mutation read from a file (-F query=@file,--input file) andenablePullRequestAutoMerge. The base guard has the same gaps for its other checks.PLX_MERGE_BYPASS=1is honoured anywhere in the command, like the three existing hatches (PLX_PUBLISH_BYPASS, the TV guard, the watchlist guard), so the hatch text in one segment opens it for another in the same call.Not verified
permissions.denyrule was not exercised against the real MCP connector. CI path-filters.claude/settings.jsonout of the heavy gates, so only the presence assertion covers it.tools/pr-land.pywas not run against a real GitHub PR for this change; the skill's text comes from its docstring and--help, checked against the argparse definitions.No tracking issue (owner: none).
🤖 Generated with Claude Code
https://claude.ai/code/session_014bHMduhNfrC8w3qjNhEWdB