From e5da11f504422700eaafd3fc94d320740cd122b0 Mon Sep 17 00:00:00 2001 From: Josh Mabry Date: Thu, 22 Oct 2026 16:24:21 -0700 Subject: [PATCH] feat: github_merge_pr: optional subject/body (gh pr merge --subject/--body), refused on rebase, shown in dry run --- tests/test_write_tools.py | 83 +++++++++++++++++++++++++++++++++++++++ write_tools.py | 21 +++++++++- 2 files changed, 103 insertions(+), 1 deletion(-) diff --git a/tests/test_write_tools.py b/tests/test_write_tools.py index 6710a81..373d1c4 100644 --- a/tests/test_write_tools.py +++ b/tests/test_write_tools.py @@ -429,6 +429,89 @@ async def test_merge_pr_validates_method(): fake.assert_not_called() +async def test_merge_pr_squash_with_subject(): + tool = _by_name("github_merge_pr") + fake = AsyncMock(return_value=(0, "Merged", "")) + with patch("ghplugin.write_tools.run_gh", fake): + await tool.ainvoke( + { + "repo": "o/n", + "number": 9, + "method": "squash", + "subject": "chore: release v0.16.0 (#48)", + "confirm": True, + } + ) + args = fake.call_args.args[0] + assert args == ["pr", "merge", "9", "--repo", "o/n", "--squash", "--subject", "chore: release v0.16.0 (#48)"] + + +async def test_merge_pr_merge_with_subject_body_and_delete_branch(): + tool = _by_name("github_merge_pr") + fake = AsyncMock(return_value=(0, "Merged", "")) + with patch("ghplugin.write_tools.run_gh", fake): + await tool.ainvoke( + { + "repo": "o/n", + "number": 9, + "method": "merge", + "delete_branch": True, + "subject": "a subject", + "body": "a body", + "confirm": True, + } + ) + args = fake.call_args.args[0] + assert args[-6:] == ["--merge", "--delete-branch", "--subject", "a subject", "--body", "a body"] + + +async def test_merge_pr_rebase_refuses_subject(): + tool = _by_name("github_merge_pr") + fake = AsyncMock() + with patch("ghplugin.write_tools.run_gh", fake): + out = await tool.ainvoke({"repo": "o/n", "number": 9, "method": "rebase", "subject": "x", "confirm": True}) + assert out.startswith("Error") and "rebase" in out + fake.assert_not_called() + + +async def test_merge_pr_rebase_refuses_body_on_dry_run(): + tool = _by_name("github_merge_pr") + fake = AsyncMock() + with patch("ghplugin.write_tools.run_gh", fake): + out = await tool.ainvoke({"repo": "o/n", "number": 9, "method": "rebase", "body": "x", "dry_run": True}) + assert out.startswith("Error") and "rebase" in out + fake.assert_not_called() + + +async def test_merge_pr_dry_run_shows_subject_and_body(): + tool = _by_name("github_merge_pr") + fake = AsyncMock() + with patch("ghplugin.write_tools.run_gh", fake): + out = await tool.ainvoke( + { + "repo": "o/n", + "number": 48, + "method": "squash", + "subject": "chore: release v0.16.0 (#48)", + "body": "release notes", + "dry_run": True, + } + ) + assert out.startswith("DRY RUN") + assert 'subject "chore: release v0.16.0 (#48)"' in out + assert 'body "release notes"' in out + fake.assert_not_called() + + +async def test_merge_pr_dry_run_without_subject_body_unchanged(): + tool = _by_name("github_merge_pr") + fake = AsyncMock() + with patch("ghplugin.write_tools.run_gh", fake): + out = await tool.ainvoke({"repo": "o/n", "number": 9, "dry_run": True}) + assert out == "DRY RUN — would merge PR #9 in o/n via squash. Re-call with confirm=true to do it." + fake.assert_not_called() + + # --- github_close / reopen --------------------------------------------------- diff --git a/write_tools.py b/write_tools.py index ba34a96..23d31dc 100644 --- a/write_tools.py +++ b/write_tools.py @@ -289,6 +289,8 @@ async def github_merge_pr( repo: str = "", method: str = "squash", delete_branch: bool = False, + subject: str = "", + body: str = "", dry_run: bool = False, confirm: bool = False, ) -> str: @@ -299,6 +301,11 @@ async def github_merge_pr( number: PR number. method: ``squash`` (default), ``merge``, or ``rebase``. delete_branch: Delete the head branch after merging (default False). + subject: Merge-commit subject line. Squash/merge only — refused on rebase, + which GitHub ignores. A single-commit squash uses that COMMIT's subject, + not the PR title, unless subject is set — so fixing the PR title alone + does not fix the subject that lands on main. + body: Merge-commit body. Squash/merge only — refused on rebase. dry_run: If true, report what WOULD be merged without merging. confirm: Must be true to actually merge — a guard against accidental or autonomous merges even when the per-agent write gate is on. @@ -310,10 +317,18 @@ async def github_merge_pr( return err if method not in ("squash", "merge", "rebase"): return f"Error: method must be 'squash', 'merge', or 'rebase' (got {method!r})." + if method == "rebase" and (subject or body): + return "Error: subject/body only apply to squash or merge — GitHub ignores them on a rebase merge." if dry_run: extra = " and delete its branch" if delete_branch else "" + detail = "" + if subject: + detail += f' with subject "{subject}"' + if body: + detail += f'{" and" if subject else " with"} body "{body}"' return ( - f"DRY RUN — would merge PR #{number} in {repo} via {method}{extra}. Re-call with confirm=true to do it." + f"DRY RUN — would merge PR #{number} in {repo} via {method}{extra}{detail}. " + "Re-call with confirm=true to do it." ) if not confirm: return ( @@ -323,6 +338,10 @@ async def github_merge_pr( args = ["pr", "merge", str(number), "--repo", repo, f"--{method}"] if delete_branch: args.append("--delete-branch") + if subject: + args += ["--subject", subject] + if body: + args += ["--body", body] rc, out, serr = await run_gh(args) if gh_err := check_gh_error(rc, serr, repo=repo): return gh_err