diff --git a/PROTO.md b/PROTO.md index cd8a2ca..4080ecd 100644 --- a/PROTO.md +++ b/PROTO.md @@ -31,11 +31,12 @@ There is no other runner. `ruff` + `pytest` are the sole gate. ## 3. Where everything lives ``` -protoagent.plugin.yaml # manifest (id: github, config_section: github, write: bool) -__init__.py # register() — the GATING wiring (read always; write iff github.write) +protoagent.plugin.yaml # manifest (id: github, config_section: github; write/default_repo/repos) +__init__.py # register() — gating wiring (read always; write iff github.write) + /issue gh_cli.py # vendored async `gh` runner (run_gh, check_gh_error, bad_repo) 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 +gh_issue.py # /issue chat command logic (user-only; gate-checked; configured repo) tests/ # host-free pytest (gating + version coherence) ``` @@ -46,6 +47,14 @@ 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. +**`/issue` is a user-only chat command, not an agent tool** — creating an issue is a +write the model must not do autonomously, so `register()` registers it via the host's +`register_chat_command` seam (the logic lives in `gh_issue.py`). The call is guarded +by `hasattr(registry, "register_chat_command")`, so on an older host without the seam +`/issue` is skipped and the tools still load (degrade-safe). It routes to the +configured `default_repo`/`repos` (never a silent default). `tests/test_issue_command.py` +asserts both the seam-present and legacy-host paths — keep them green. + ## 5. Tools (all implemented) Each tool mocks `run_gh` in its test and asserts the exact argv + readable errors. diff --git a/__init__.py b/__init__.py index c5f11ae..689eb75 100644 --- a/__init__.py +++ b/__init__.py @@ -6,6 +6,11 @@ ADR 0019), so the same plugin serves a read-only research agent and a write-capable coding/PM agent. +It also owns the user-only `/issue` chat control command (the write the model must +NOT do autonomously): registered via the host's `register_chat_command` seam when the +host provides it, reading the configured `default_repo`/`repos` for routing. On an +older host without that seam, `/issue` is simply skipped — the tools still load. + Host-only imports stay LAZY (none here) so the test suite imports the modules with no protoAgent host present. """ @@ -46,8 +51,28 @@ def register(registry) -> None: except Exception: # noqa: BLE001 log.exception("[github] registering write tools failed") + # /issue — the user-only chat control command (not an agent tool). Registered via + # the host's chat-command seam; guarded so an older host (no seam) still loads the + # tools above. Reads the plugin's own configured default repo (no host coupling). + issue_cmd = False + if hasattr(registry, "register_chat_command"): + try: + from .gh_issue import effective_default_repo, run_issue_command + + default_repo = effective_default_repo(cfg.get("default_repo", ""), cfg.get("repos", [])) + + async def _issue(rest: str, session_id: str) -> str: + """File a GitHub issue (user-only). Usage: /issue [--bug|--feature] [--repo owner/name].""" + return await run_issue_command(rest, default_repo=default_repo) + + registry.register_chat_command("issue", _issue) + issue_cmd = True + except Exception: # noqa: BLE001 + log.exception("[github] registering the /issue command failed") + log.info( - "[github] registered %d read tool(s)%s", + "[github] registered %d read tool(s)%s%s", n_read, f" + {n_write} write tool(s) (write enabled)" if write_enabled else " (read-only — github.write is false)", + " + /issue command" if issue_cmd else "", ) diff --git a/gh_issue.py b/gh_issue.py new file mode 100644 index 0000000..5f3e121 --- /dev/null +++ b/gh_issue.py @@ -0,0 +1,258 @@ +"""The `/issue` chat control command — file a GitHub issue from a chat message. + +This is the WRITE counterpart to the read tools that is deliberately NOT an agent +tool: creating an issue is a write the model must not do autonomously, so the host +exposes it as a user-only `/issue` chat control command (like `/goal`). The plugin +registers it via `registry.register_chat_command("issue", …)` (host seam); this module +is the pure, host-free logic behind it. + +`run_issue_command(rest, *, default_repo)` takes everything after the `/issue` token +(the host already matched it) and returns the reply string. The issue body is checked +against the SAME requirements the CI issue gate enforces (a non-trivial body + a +Problem/Motivation section; `--bug` also wants repro/evidence; `--feature` wants a +proposed-direction or acceptance section), so an issue filed here always passes. + +Repo resolution (no silent misrouting): explicit `--repo` > the plugin's configured +`github.default_repo` (passed in as `default_repo`) > `GITHUB_DEFAULT_REPO` / `GH_REPO` +env > an error asking for one. Auth rides on `gh_cli` (`GITHUB_TOKEN`/`GH_TOKEN` or +ambient `gh auth`); `gh issue create` needs write scope. +""" + +from __future__ import annotations + +import os +import re +import shlex +from dataclasses import dataclass, field + +from .gh_cli import REPO_RE, check_gh_error, run_gh + +# Section detectors — kept in lockstep with the host CI gate's regexes so the local +# check and the server-side gate can never disagree about what "conforms" means. +_SECTION_RES = { + "problem": re.compile(r"problem|what'?s? wrong|motivation|background|context|summary", re.I), + "repro": re.compile(r"repro|reproduce|steps|evidence|expected|actual|observed", re.I), + "proposal": re.compile(r"propos|solution|approach|direction|fix|design", re.I), + "acceptance": re.compile(r"acceptance|done when|success criteria|definition of done", re.I), +} +# A heading (#..######) or a bold line (**…**) — same shape the gate matches. +_HEADING_RE = re.compile(r"^\s*(?:#{1,6}\s+|\*\*\s*)(.+?)(?:\s*\*\*)?\s*$") + +_BUG_SCAFFOLD = ( + "## Problem / what's wrong\n<what's broken, and where — name the file/subsystem>\n\n" + "## Steps to reproduce / evidence\n<minimal steps, logs, or a stack trace>\n\n" + "## Expected vs. actual\n<what you expected vs. what happened>\n\n" + "## Acceptance\n<how we'll know it's fixed>\n" +) +_FEATURE_SCAFFOLD = ( + "## Problem / motivation\n<what gap or pain motivates this>\n\n" + "## Proposed direction\n<sketch the approach; note trade-offs>\n\n" + "## Acceptance\n<verifiable criteria for done>\n" +) +_GENERIC_SCAFFOLD = ( + "## Problem\n<what's wrong or what you want, and why it matters>\n\n## Acceptance\n<verifiable criteria for done>\n" +) + + +@dataclass +class IssueRequest: + title: str + body: str + kind: str # "bug" | "feature" | "generic" + repo: str | None = None + labels: list[str] = field(default_factory=list) + dry_run: bool = False + + +def _has_section(body: str, key: str) -> bool: + rx = _SECTION_RES[key] + for line in body.splitlines(): + m = _HEADING_RE.match(line) + if m and rx.search(m.group(1)): + return True + return False + + +def _scaffold(kind: str) -> str: + return {"bug": _BUG_SCAFFOLD, "feature": _FEATURE_SCAFFOLD}.get(kind, _GENERIC_SCAFFOLD) + + +def missing_sections(body: str, kind: str) -> list[str]: + """The gate-required sections absent from ``body`` for this issue ``kind``.""" + miss: list[str] = [] + # Same metric as the CI gate (collapsed-whitespace length >= 80), so an issue + # that passes here is guaranteed to clear the server-side gate. + if len(" ".join(body.split())) < 80: + miss.append("a substantive description (>= 80 chars)") + if not _has_section(body, "problem"): + miss.append("a Problem / What's-wrong / Motivation section") + if kind == "bug" and not _has_section(body, "repro"): + miss.append("Steps to reproduce / Evidence / Expected-vs-actual") + if kind == "feature" and not (_has_section(body, "proposal") or _has_section(body, "acceptance")): + miss.append("a Proposed-direction or Acceptance section") + return miss + + +def labels_for(kind: str, extra: list[str] | None = None) -> list[str]: + """Labels for an issue of this ``kind`` — the type label first (``bug`` / + ``enhancement``), then any extras, de-duped in order.""" + out: list[str] = [] + if kind == "bug": + out.append("bug") + elif kind == "feature": + out.append("enhancement") + for lbl in extra or []: + if lbl and lbl not in out: + out.append(lbl) + return out + + +def resolve_repo(explicit: str | None, default_repo: str = "") -> str | None: + """Target repo: explicit ``--repo`` > configured default > GITHUB_DEFAULT_REPO + / GH_REPO env > ``None`` (caller errors — there is no silent default).""" + return ( + (explicit or "").strip() + or (default_repo or "").strip() + or os.environ.get("GITHUB_DEFAULT_REPO") + or os.environ.get("GH_REPO") + or None + ) + + +def effective_default_repo(default_repo: str, repos: list[str] | None = None) -> str: + """The preselected default repo for the dialog + the ``/issue`` command: the + explicit ``github.default_repo`` if set, else the first entry in the + ``github.repos`` picker list, else ``""`` (env still applies via + ``resolve_repo``). Keeps the command and the dialog agreeing on the default.""" + if (default_repo or "").strip(): + return default_repo.strip() + for r in repos or []: + if (r or "").strip(): + return r.strip() + return "" + + +async def file_issue(req: IssueRequest) -> dict: + """Validate ``req`` against the gate rules, then create the issue via ``gh`` + (or, for ``dry_run``, report what would be filed). Returns a structured + result the chat command and the console dialog both render: + + - ``{"ok": False, "missing": [...], "kind": ...}`` — body fails the gate; + - ``{"ok": False, "error": "..."}`` — ``gh`` failed (auth / label / repo); + - ``{"ok": True, "dry_run": True, ...}`` — preview only, nothing created; + - ``{"ok": True, "url": "...", "repo": ..., "labels": [...]}`` — created. + + Assumes ``req.repo`` is already set + validated (the callers do that). + """ + miss = missing_sections(req.body, req.kind) + if miss: + return {"ok": False, "missing": miss, "kind": req.kind} + if req.dry_run: + return { + "ok": True, + "dry_run": True, + "repo": req.repo, + "title": req.title, + "body": req.body, + "labels": req.labels, + } + args = ["issue", "create", "--repo", req.repo, "--title", req.title, "--body", req.body] + for lbl in req.labels: + args += ["--label", lbl] + rc, out, serr = await run_gh(args, timeout=45) + err = check_gh_error(rc, serr) + if err: + if "could not add label" in serr.lower() or "not found" in serr.lower(): + err += f" (the label may not exist in {req.repo}; create it or drop the label.)" + return {"ok": False, "error": err} + url = out.strip().splitlines()[-1] if out.strip() else "" + return {"ok": True, "url": url, "repo": req.repo, "labels": req.labels} + + +def _parse(rest: str, *, default_repo: str = "") -> IssueRequest | str: + """Parse the raw ``/issue`` argument string (everything after the token) into a + request, or an error string. The first line carries the title + flags; everything + after the first newline is the body, taken verbatim.""" + first, _, body = rest.partition("\n") + body = body.strip() + + kind = "generic" + labels: list[str] = [] + repo: str | None = None + dry_run = False + + # Tokenize only the first line (title + flags); the body is taken verbatim. + try: + tokens = shlex.split(first) + except ValueError: + tokens = first.split() + + title_parts: list[str] = [] + i = 0 + while i < len(tokens): + tok = tokens[i] + low = tok.lower() + if low in ("--bug", "--fix"): + kind = "bug" + elif low in ("--feature", "--feat", "--enhancement"): + kind = "feature" + elif low in ("--dry-run", "--dryrun"): + dry_run = True + elif low.startswith("--repo"): + val = tok.split("=", 1)[1] if "=" in tok else (tokens[i + 1] if i + 1 < len(tokens) else "") + if "=" not in tok: + i += 1 + repo = val.strip() + elif low.startswith("--label"): + val = tok.split("=", 1)[1] if "=" in tok else (tokens[i + 1] if i + 1 < len(tokens) else "") + if "=" not in tok: + i += 1 + labels += [s.strip() for s in val.split(",") if s.strip()] + else: + title_parts.append(tok) + i += 1 + + title = " ".join(title_parts).strip() + labels = labels_for(kind, labels) + repo = resolve_repo(repo, default_repo) + + if not title: + return ( + "Usage: `/issue <title> [--bug|--feature] [--repo owner/name] [--dry-run]` " + "then the body on the following lines.\n\n" + "Scaffold to fill in:\n```\n" + _scaffold(kind) + "```" + ) + if not repo: + return ( + "No target repo. Pass `--repo owner/name`, or set `github.default_repo` " + "in Settings (or the `GITHUB_DEFAULT_REPO` env var)." + ) + if not REPO_RE.match(repo): + return f"Error: --repo must be 'owner/name' (got {repo!r})." + + return IssueRequest(title=title, body=body, kind=kind, repo=repo, labels=labels, dry_run=dry_run) + + +async def run_issue_command(rest: str, *, default_repo: str = "") -> str: + """Handle a ``/issue`` chat command: parse ``rest`` (the text after the token), + validate against the gate, then create the issue (or report what's missing / what + a dry-run would do). Returns the reply string the host short-circuits the turn + with. The host has already matched the ``/issue`` token via the chat-command seam.""" + parsed = _parse(rest, default_repo=default_repo) + if isinstance(parsed, str): + return parsed # usage / scaffold / validation error + + result = await file_issue(parsed) + if not result["ok"] and result.get("missing"): + return ( + "Not filed — this issue is missing " + "; ".join(result["missing"]) + ".\n\n" + "Add the section(s) and resend. Scaffold for a " + f"{parsed.kind} issue:\n```\n" + _scaffold(parsed.kind) + "```" + ) + if not result["ok"]: + return result.get("error", "Error filing issue.") + + label_note = f" · labels: {', '.join(parsed.labels)}" if parsed.labels else "" + if result.get("dry_run"): + return f"Dry run — would create in **{parsed.repo}**{label_note}:\n\n**{parsed.title}**\n\n{parsed.body}" + return f"Filed in {parsed.repo}{label_note}: {result.get('url') or '(created)'}" diff --git a/protoagent.plugin.yaml b/protoagent.plugin.yaml index 7b2ffa3..7ef0a40 100644 --- a/protoagent.plugin.yaml +++ b/protoagent.plugin.yaml @@ -17,9 +17,19 @@ min_protoagent_version: "0.27.0" # Config (ADR 0019): defaults are the source of truth; langgraph-config.yaml overlays. # `write` is the per-agent gate — each agent's config decides read-only vs read+write. +# `default_repo`/`repos` route the user-only `/issue` command (and the console views): +# explicit `--repo` > default_repo > first of repos > GITHUB_DEFAULT_REPO/GH_REPO env. config_section: github config: write: false # false = read tools only; true = read + write tools + default_repo: "" # preselected repo for /issue (owner/name); "" = none + repos: [] # repo picker list (each owner/name) for /issue + views + +# Settings (ADR 0019) — surfaced as editable fields in the console. +settings: + - {key: write, label: "Allow write tools (this agent)", type: bool} + - {key: default_repo, label: "Default repo (owner/name)", type: string} + - {key: repos, label: "Repo picker list", type: string_list} # Auth: `gh` uses its own ambient auth (`gh auth login`), or GITHUB_TOKEN / GH_TOKEN # from the env if set. No plugin secret needed for public-repo reads. diff --git a/tests/conftest.py b/tests/conftest.py index 1bf1a5e..609d1bb 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -27,6 +27,27 @@ class _Registry: """A fake registry to smoke-test register() with no host.""" + def __init__(self, config: dict | None = None): + self.config = config or {} + self.tools: list = [] + self.chat_commands: dict = {} # token -> handler (the host's chat-command seam) + + def register_tool(self, t): + self.tools.append(t) + + def register_chat_command(self, name: str, handler): + self.chat_commands[name] = handler + + @property + def tool_names(self) -> list[str]: + return [getattr(t, "name", getattr(t, "__name__", "?")) for t in self.tools] + + +class _LegacyRegistry: + """A host WITHOUT the chat-command seam (older protoAgent) — no + ``register_chat_command``. register() must still load the tools and just skip + ``/issue`` (graceful degrade).""" + def __init__(self, config: dict | None = None): self.config = config or {} self.tools: list = [] @@ -42,3 +63,8 @@ def tool_names(self) -> list[str]: @pytest.fixture def make_registry(): return _Registry + + +@pytest.fixture +def make_legacy_registry(): + return _LegacyRegistry diff --git a/tests/test_gh_issue.py b/tests/test_gh_issue.py new file mode 100644 index 0000000..4dd0244 --- /dev/null +++ b/tests/test_gh_issue.py @@ -0,0 +1,118 @@ +"""The /issue command logic — parsing, the gate-conformance check, and filing. + +Host-free: `run_gh` is patched in the `ghplugin.gh_issue` namespace, so we assert the +gate behavior and the `gh issue create` path without touching GitHub. +""" + +from __future__ import annotations + +from unittest.mock import AsyncMock, patch + +from ghplugin.gh_issue import ( + effective_default_repo, + labels_for, + missing_sections, + resolve_repo, + run_issue_command, +) + +# A body that clears the gate (>= 80 collapsed chars + a Problem + repro section). +_GOOD_BUG_BODY = ( + "## Problem\nThe parser crashes on empty input and we should handle it gracefully " + "across the whole pipeline instead of raising.\n## Steps to reproduce\nRun it with an empty string." +) + + +# --- pure helpers ------------------------------------------------------------ + + +def test_missing_sections_by_kind(): + assert missing_sections("", "generic") # empty → missing description + problem + assert missing_sections(_GOOD_BUG_BODY, "bug") == [] # passes for a bug + # A feature needs a proposal OR acceptance section. + feat = "## Motivation\nWe need this capability badly for the next release cycle to ship on time." + assert "a Proposed-direction or Acceptance section" in missing_sections(feat, "feature") + + +def test_labels_for_prepends_type_label(): + assert labels_for("bug", ["p0"]) == ["bug", "p0"] + assert labels_for("feature") == ["enhancement"] + assert labels_for("generic", ["x", "x"]) == ["x"] # de-duped, no type label + + +def test_resolve_repo_precedence(monkeypatch): + monkeypatch.delenv("GITHUB_DEFAULT_REPO", raising=False) + monkeypatch.delenv("GH_REPO", raising=False) + assert resolve_repo("o/explicit", "o/default") == "o/explicit" # explicit wins + assert resolve_repo(None, "o/default") == "o/default" # then configured default + assert resolve_repo(None, "") is None # then nothing + monkeypatch.setenv("GH_REPO", "o/env") + assert resolve_repo(None, "") == "o/env" # then env + + +def test_effective_default_repo(): + assert effective_default_repo("o/explicit", ["o/a"]) == "o/explicit" + assert effective_default_repo("", ["o/a", "o/b"]) == "o/a" # first of the picker list + assert effective_default_repo("", []) == "" + + +# --- run_issue_command ------------------------------------------------------- + + +async def test_usage_when_no_title(): + out = await run_issue_command("--bug --repo o/n", default_repo="") + assert out.startswith("Usage:") and "Scaffold" in out + + +async def test_no_repo_errors(): + out = await run_issue_command("A title with no repo anywhere", default_repo="") + assert "No target repo" in out + + +async def test_bad_repo_errors(): + out = await run_issue_command("Title --repo not-a-repo", default_repo="") + assert out.startswith("Error:") and "owner/name" in out + + +async def test_dry_run_previews_without_shelling_out(): + fake = AsyncMock() + with patch("ghplugin.gh_issue.run_gh", fake): + out = await run_issue_command(f"Crash on empty --bug --repo o/n --dry-run\n{_GOOD_BUG_BODY}", default_repo="") + assert out.startswith("Dry run") and "**o/n**" in out and "labels: bug" in out + fake.assert_not_called() + + +async def test_files_issue_and_returns_url(): + fake = AsyncMock(return_value=(0, "https://github.com/o/n/issues/42", "")) + with patch("ghplugin.gh_issue.run_gh", fake): + out = await run_issue_command(f"Crash on empty --bug --repo o/n\n{_GOOD_BUG_BODY}", default_repo="") + assert out == "Filed in o/n · labels: bug: https://github.com/o/n/issues/42" + args = fake.call_args.args[0] + assert args[:4] == ["issue", "create", "--repo", "o/n"] + assert "--label" in args and args[args.index("--label") + 1] == "bug" + + +async def test_uses_default_repo_when_no_flag(): + fake = AsyncMock(return_value=(0, "https://github.com/o/d/issues/1", "")) + with patch("ghplugin.gh_issue.run_gh", fake): + out = await run_issue_command(f"Crash on empty --bug\n{_GOOD_BUG_BODY}", default_repo="o/d") + assert "Filed in o/d" in out + assert ( + "--repo" in fake.call_args.args[0] + and fake.call_args.args[0][fake.call_args.args[0].index("--repo") + 1] == "o/d" + ) + + +async def test_missing_sections_blocks_filing(): + fake = AsyncMock() + with patch("ghplugin.gh_issue.run_gh", fake): + out = await run_issue_command("Title --bug --repo o/n\ntoo short", default_repo="") + assert out.startswith("Not filed") and "missing" in out + fake.assert_not_called() # never shells out when the body fails the gate + + +async def test_gh_failure_surfaces_error(): + fake = AsyncMock(return_value=(1, "", "HTTP 403: forbidden")) + with patch("ghplugin.gh_issue.run_gh", fake): + out = await run_issue_command(f"Crash on empty --bug --repo o/n\n{_GOOD_BUG_BODY}", default_repo="") + assert out.startswith("Error (gh exit 1):") and "forbidden" in out diff --git a/tests/test_issue_command.py b/tests/test_issue_command.py new file mode 100644 index 0000000..8889138 --- /dev/null +++ b/tests/test_issue_command.py @@ -0,0 +1,61 @@ +"""The /issue chat command is wired through register() — and degrades gracefully. + +register() owns the user-only `/issue` command via the host's `register_chat_command` +seam. On a host that HAS the seam it registers; on an older host that lacks it (the +`_LegacyRegistry` fake), `/issue` is skipped but the tools still load. The handler +routes through the plugin's configured default repo (no host coupling). +""" + +from __future__ import annotations + +from unittest.mock import AsyncMock, patch + +from ghplugin import register + +_GOOD_BUG_BODY = ( + "## Problem\nThe parser crashes on empty input and we should handle it gracefully " + "across the whole pipeline instead of raising.\n## Steps to reproduce\nRun it with an empty string." +) + + +def test_issue_registered_when_seam_present(make_registry): + reg = make_registry({}) + register(reg) + assert "issue" in reg.chat_commands # the /issue command is owned by the plugin + assert "github_get_pr" in reg.tool_names # tools still load too + + +def test_issue_handler_uses_configured_default_repo(make_registry): + """The handler routes to the plugin's configured repo (here, first of `repos`).""" + reg = make_registry({"repos": ["o/configured"]}) + register(reg) + handler = reg.chat_commands["issue"] + + async def run(): + fake = AsyncMock(return_value=(0, "https://github.com/o/configured/issues/9", "")) + with patch("ghplugin.gh_issue.run_gh", fake): + out = await handler(f"Crash on empty --bug\n{_GOOD_BUG_BODY}", "sess-1") + assert "Filed in o/configured" in out + args = fake.call_args.args[0] + assert args[args.index("--repo") + 1] == "o/configured" + + import asyncio + + asyncio.run(run()) + + +def test_legacy_host_skips_issue_but_loads_tools(make_legacy_registry): + """A host without register_chat_command must not crash — tools load, /issue skipped.""" + reg = make_legacy_registry({"write": True}) + register(reg) # must not raise + assert "github_get_pr" in reg.tool_names + assert "github_create_issue" in reg.tool_names # write gate still works + assert not hasattr(reg, "chat_commands") + + +def test_issue_registered_even_when_write_off(make_registry): + """`/issue` is its own user-only command — independent of the write tool gate.""" + reg = make_registry({"write": False}) + register(reg) + assert "issue" in reg.chat_commands + assert "github_create_issue" not in reg.tool_names # write tools stay gated off