feat: complete the write toolset (edit/merge/close/labels/assignees)

PROTO.md

@@ -34,8 +34,8 @@ There is no other runner. `ruff` + `pytest` are the sole gate.
 protoagent.plugin.yaml   # manifest (id: github, config_section: github, write: bool)
 __init__.py              # register() — the GATING wiring (read always; write iff github.write)
 gh_cli.py                # vendored async `gh` runner (run_gh, check_gh_error, bad_repo)
-read_tools.py            # 6 ported read tools (done) + read_file/repo_contents (STUBS)
-write_tools.py           # create_issue / comment / create_pr (STUBS — gated)
+read_tools.py            # 8 read tools (6 ported core + read_file/repo_contents)
+write_tools.py           # 8 write tools (create/edit/merge/close/comment/labels/assignees) — gated
 tests/                   # host-free pytest (gating + version coherence)
 ```
 
@@ -46,14 +46,25 @@ tests/                   # host-free pytest (gating + version coherence)
 agent stays read-only; a coding/PM agent gets write. `tests/test_register.py` asserts
 both halves — keep it green.
 
-## 5. What to build (the stubs)
+## 5. Tools (all implemented)
 
-Each stub has a `TODO(team)` with the exact `gh` command. Implement, then add a test
-that mocks `run_gh` and asserts the tool formats the result (and errors readably):
-- **`github_read_file`** — `gh api repos/{repo}/contents/{path}?ref={ref}` (raw media type).
-- **`github_repo_contents`** — `gh api repos/{repo}/contents/{path}` → list of entries.
-- **`github_create_issue` / `github_comment` / `github_create_pr`** — `gh issue create` /
-  `gh issue comment` / `gh pr create`; return the new URL.
+Each tool mocks `run_gh` in its test and asserts the exact argv + readable errors.
+
+**Read (always on)** — 6 ported core tools (`github_get_pr`, `github_get_issue`,
+`github_list_issues`, `github_get_commit_diff`, `github_ci_runs`, `github_run_failure`)
+plus `github_read_file` (`gh api .../contents/{path}`, raw) and `github_repo_contents`
+(directory listing).
+
+**Write (gated on `github.write`)** —
+`github_create_issue` / `github_comment` / `github_create_pr` (return the new URL),
+`github_edit_pr` (`gh pr edit` + `gh pr ready [--undo]`),
+`github_merge_pr` (`gh pr merge` — **refuses without `confirm=true`**, offers `dry_run`),
+`github_close` (close/reopen issue|pr), and `github_set_labels` / `github_set_assignees`
+(`gh {issue,pr} edit --add/--remove-{label,assignee}`). Issue-vs-PR ops take `kind`.
+
+New write op? Validate `bad_repo()`, build argv, `run_gh()`, degrade to `Error: ...`,
+add it to `get_write_tools()`'s return list **and** `WRITE_TOOLS` in `test_register.py`,
+and mirror an existing test. Anything irreversible (merge) must be `confirm`-guarded.
 
 ## 6. Rules
 

protoagent.plugin.yaml

@@ -6,10 +6,11 @@ version: 0.1.0
 description: >-
   Read AND write GitHub tools over the `gh` CLI, with PER-AGENT write gating. The
   read tools (PRs, issues, diffs, CI, repo files/contents) are always on; the write
-  tools (create issue, comment, create PR) load ONLY when `github.write: true` — so a
-  research/Lead agent stays read-only while a coding/PM agent gets write, purely by
-  its own per-instance config (ADR 0019). Supersedes the read-only in-tree `github`
-  plugin. Tools degrade to a readable error if `gh` isn't installed/authenticated.
+  tools (create/edit/merge/close issues & PRs, comment, labels, assignees) load ONLY
+  when `github.write: true` — so a research/Lead agent stays read-only while a
+  coding/PM agent gets write, purely by its own per-instance config (ADR 0019).
+  Merging is `confirm`-guarded. Supersedes the read-only in-tree `github` plugin.
+  Tools degrade to a readable error if `gh` isn't installed/authenticated.
 enabled: false                 # SHIP DISABLED — enabling is the operator's trust decision
 repository: https://github.com/protoLabsAI/github-plugin
 min_protoagent_version: "0.27.0"

tests/test_register.py

@@ -18,7 +18,16 @@
     "github_read_file",
     "github_repo_contents",
 }
-WRITE_TOOLS = {"github_create_issue", "github_comment", "github_create_pr"}
+WRITE_TOOLS = {
+    "github_create_issue",
+    "github_comment",
+    "github_create_pr",
+    "github_edit_pr",
+    "github_merge_pr",
+    "github_close",
+    "github_set_labels",
+    "github_set_assignees",
+}
 
 
 def test_read_only_by_default(make_registry):

tests/test_write_tools.py

@@ -175,3 +175,175 @@ async def test_create_pr_gh_failure_returns_check_gh_error():
     with patch("ghplugin.write_tools.run_gh", fake):
         out = await tool.ainvoke({"repo": "o/n", "head": "feature", "title": "t"})
     assert out == "Error (gh exit 1): could not create pr: forbidden"
+
+
+def _by_name(name: str):
+    return {t.name: t for t in get_write_tools()}[name]
+
+
+# --- github_edit_pr ----------------------------------------------------------
+
+
+async def test_edit_pr_title_and_body_via_pr_edit():
+    """Title/body changes go through `gh pr edit` and return the PR URL."""
+    tool = _by_name("github_edit_pr")
+    fake = AsyncMock(return_value=(0, "https://github.com/o/n/pull/3\n", ""))
+    with patch("ghplugin.write_tools.run_gh", fake):
+        out = await tool.ainvoke({"repo": "o/n", "number": 3, "title": "New", "body": "Updated"})
+    assert out == "https://github.com/o/n/pull/3"
+    args = fake.call_args.args[0]
+    assert args[:5] == ["pr", "edit", "3", "--repo", "o/n"]
+    assert args[args.index("--title") + 1] == "New"
+    assert args[args.index("--body") + 1] == "Updated"
+
+
+async def test_edit_pr_state_ready_and_draft():
+    """state=ready → `gh pr ready`; state=draft → `gh pr ready --undo`."""
+    tool = _by_name("github_edit_pr")
+    fake = AsyncMock(return_value=(0, "", ""))
+    with patch("ghplugin.write_tools.run_gh", fake):
+        await tool.ainvoke({"repo": "o/n", "number": 4, "state": "ready"})
+    assert fake.call_args.args[0] == ["pr", "ready", "4", "--repo", "o/n"]
+    with patch("ghplugin.write_tools.run_gh", fake):
+        await tool.ainvoke({"repo": "o/n", "number": 4, "state": "draft"})
+    assert fake.call_args.args[0] == ["pr", "ready", "4", "--repo", "o/n", "--undo"]
+
+
+async def test_edit_pr_requires_a_change_and_validates_state():
+    tool = _by_name("github_edit_pr")
+    fake = AsyncMock()
+    with patch("ghplugin.write_tools.run_gh", fake):
+        assert (await tool.ainvoke({"repo": "o/n", "number": 1})).startswith("Error: nothing to change")
+        assert "state must be" in await tool.ainvoke({"repo": "o/n", "number": 1, "state": "merged"})
+    fake.assert_not_called()
+
+
+# --- github_merge_pr (guarded) ----------------------------------------------
+
+
+async def test_merge_pr_refuses_without_confirm():
+    """The default call refuses and never shells out — merging is irreversible."""
+    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})
+    assert "confirm=true" in out and "Refusing" in out
+    fake.assert_not_called()
+
+
+async def test_merge_pr_dry_run_previews_without_merging():
+    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.startswith("DRY RUN") and "via squash" in out
+    fake.assert_not_called()
+
+
+async def test_merge_pr_confirm_runs_gh_merge():
+    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": "rebase", "delete_branch": True, "confirm": True})
+    args = fake.call_args.args[0]
+    assert args == ["pr", "merge", "9", "--repo", "o/n", "--rebase", "--delete-branch"]
+
+
+async def test_merge_pr_validates_method():
+    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": "fast-forward", "confirm": True})
+    assert "method must be" in out
+    fake.assert_not_called()
+
+
+# --- github_close / reopen ---------------------------------------------------
+
+
+async def test_close_issue_with_comment():
+    tool = _by_name("github_close")
+    fake = AsyncMock(return_value=(0, "", ""))
+    with patch("ghplugin.write_tools.run_gh", fake):
+        await tool.ainvoke({"repo": "o/n", "number": 5, "comment": "dupe"})
+    args = fake.call_args.args[0]
+    assert args[:5] == ["issue", "close", "5", "--repo", "o/n"]
+    assert args[args.index("--comment") + 1] == "dupe"
+
+
+async def test_close_pr_uses_pr_subcommand():
+    tool = _by_name("github_close")
+    fake = AsyncMock(return_value=(0, "", ""))
+    with patch("ghplugin.write_tools.run_gh", fake):
+        await tool.ainvoke({"repo": "o/n", "number": 5, "kind": "pr"})
+    assert fake.call_args.args[0][:2] == ["pr", "close"]
+
+
+async def test_reopen_ignores_comment_and_uses_reopen():
+    tool = _by_name("github_close")
+    fake = AsyncMock(return_value=(0, "", ""))
+    with patch("ghplugin.write_tools.run_gh", fake):
+        await tool.ainvoke({"repo": "o/n", "number": 5, "reopen": True, "comment": "ignored"})
+    args = fake.call_args.args[0]
+    assert args[:2] == ["issue", "reopen"]
+    assert "--comment" not in args
+
+
+async def test_close_validates_kind():
+    tool = _by_name("github_close")
+    fake = AsyncMock()
+    with patch("ghplugin.write_tools.run_gh", fake):
+        out = await tool.ainvoke({"repo": "o/n", "number": 5, "kind": "discussion"})
+    assert "kind must be" in out
+    fake.assert_not_called()
+
+
+# --- github_set_labels / github_set_assignees -------------------------------
+
+
+async def test_set_labels_add_and_remove():
+    tool = _by_name("github_set_labels")
+    fake = AsyncMock(return_value=(0, "", ""))
+    with patch("ghplugin.write_tools.run_gh", fake):
+        await tool.ainvoke({"repo": "o/n", "number": 2, "add": "bug, p0", "remove": "stale"})
+    args = fake.call_args.args[0]
+    assert args[:5] == ["issue", "edit", "2", "--repo", "o/n"]
+    assert [args[i + 1] for i, a in enumerate(args) if a == "--add-label"] == ["bug", "p0"]
+    assert [args[i + 1] for i, a in enumerate(args) if a == "--remove-label"] == ["stale"]
+
+
+async def test_set_labels_requires_at_least_one():
+    tool = _by_name("github_set_labels")
+    fake = AsyncMock()
+    with patch("ghplugin.write_tools.run_gh", fake):
+        out = await tool.ainvoke({"repo": "o/n", "number": 2})
+    assert out.startswith("Error:") and "label" in out
+    fake.assert_not_called()
+
+
+async def test_set_assignees_pr_kind_add_and_remove():
+    tool = _by_name("github_set_assignees")
+    fake = AsyncMock(return_value=(0, "", ""))
+    with patch("ghplugin.write_tools.run_gh", fake):
+        await tool.ainvoke({"repo": "o/n", "number": 8, "add": "alice,bob", "remove": "carol", "kind": "pr"})
+    args = fake.call_args.args[0]
+    assert args[:5] == ["pr", "edit", "8", "--repo", "o/n"]
+    assert [args[i + 1] for i, a in enumerate(args) if a == "--add-assignee"] == ["alice", "bob"]
+    assert [args[i + 1] for i, a in enumerate(args) if a == "--remove-assignee"] == ["carol"]
+
+
+async def test_new_write_tools_bad_repo_short_circuits():
+    """Every new write tool validates the repo before shelling out."""
+    fake = AsyncMock()
+    cases = [
+        ("github_edit_pr", {"repo": "x", "number": 1, "title": "t"}),
+        ("github_merge_pr", {"repo": "x", "number": 1, "confirm": True}),
+        ("github_close", {"repo": "x", "number": 1}),
+        ("github_set_labels", {"repo": "x", "number": 1, "add": "bug"}),
+        ("github_set_assignees", {"repo": "x", "number": 1, "add": "alice"}),
+    ]
+    for name, kwargs in cases:
+        with patch("ghplugin.write_tools.run_gh", fake):
+            out = await _by_name(name).ainvoke(kwargs)
+        assert out.startswith("Error:") and "owner/name" in out, name
+    fake.assert_not_called()