From 55656d5ca633ebc363b379b7a6611eaf059a7d2d Mon Sep 17 00:00:00 2001 From: Josh Mabry Date: Sat, 22 Aug 2026 16:59:39 -0700 Subject: [PATCH 1/2] =?UTF-8?q?feat:=20v0.7.0=20=E2=80=94=20the=20PM=20ver?= =?UTF-8?q?bs:=20list=20PRs,=20merge-readiness=20get=5Fpr,=20issue=20comme?= =?UTF-8?q?nts,=20search-before-filing,=20body-gated=20create=5Fissue?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The verbs a Project Manager needs that the rail still lacked. - github_list_prs(repo?, state="open", limit=30): number/title/author/state/ draft/head->base/reviewDecision/mergeStateStatus/url per row. Reuses api.fetch_prs (its --json field list gained baseRefName + mergeStateStatus; the board rows ignore extras) so the tool and the board can't disagree. - github_get_pr now carries the merge-readiness picture: isDraft, reviewDecision, mergeable, mergeStateStatus, statusCheckRollup summarised (N pass / N fail / N pending [/ N skipped] + the failing check names — both the CheckRun and StatusContext shapes, verified against gh 2.92), reviews (author, state, body <= 300 chars, <= 10 shown). Output bounded at 12k. - github_issue_comments(repo?, number, limit=30): `gh issue view --json comments` (works on PRs); the newest `limit` in chronological order, each body <= 1000 chars, null rows tolerated. - github_search_issues(repo?, query, state="open", limit=20): `gh search issues --repo R`; `all` omits --state (gh only knows open|closed). Documented as DEDUPE BEFORE FILING; create_issue's description points at it. - github_create_issue runs the SAME body gate /issue enforces (gh_issue.missing_sections): a thin body / a bug without repro / a feature without a direction-or-acceptance section is refused with the scaffold and never posted. New `kind` arg picks the gate's sections and adds the type label via labels_for (bug / enhancement first, like /issue). Existing create-issue tests now pass gate-clearing bodies. - api.fetch_issues/fetch_prs type-check the gh body via parse_json — the no-raise sweep caught `gh pr list` returning a non-list crashing the new tool (and, latently, the board route). - Inventory recounted from register(): 15 read / 8 write / 3 review = 26 (the brief said 16; three read tools were added, get_pr was enriched). A test pins the counts against README/PROTO so they can't drift. - Version 0.7.0 in manifest + pyproject (lockstep). Co-Authored-By: Claude Fable 5 --- PROTO.md | 21 +++- README.md | 26 ++-- api.py | 25 ++-- gh_issue.py | 6 + protoagent.plugin.yaml | 7 +- pyproject.toml | 2 +- read_tools.py | 233 +++++++++++++++++++++++++++++++++-- tests/test_no_raise_sweep.py | 11 +- tests/test_read_tools.py | 226 +++++++++++++++++++++++++++++++++ tests/test_register.py | 22 ++++ tests/test_write_tools.py | 86 ++++++++++++- write_tools.py | 40 ++++-- 12 files changed, 653 insertions(+), 52 deletions(-) diff --git a/PROTO.md b/PROTO.md index 341110c..29cc8f9 100644 --- a/PROTO.md +++ b/PROTO.md @@ -40,7 +40,7 @@ gh_cli.py # vendored async `gh` runner: binary resolution (PATH + # injection (config secret > env), check_gh_error CLASSIFICATION, bad_repo status.py # the first-run probe: compute_status / summarize_status / report_gaps (setup-gap seam) projects.py # repo sources: host projects: registry (ADR 0095) + checkout `origin` remote parsing -read_tools.py # 12 read tools (6 ported core + file/contents/pr-file/path-exists/pr-diff + github_status) +read_tools.py # 15 read tools (6 ported core + file/contents/pr-file/path-exists/pr-diff + status + list_prs/comments/search) write_tools.py # 8 write tools (create/edit/merge/close/comment/labels/assignees) — gated review_tools.py # 3 verdict tools (comment/approve/request-changes, guarded) — gated gh_issue.py # /issue chat command logic + repo resolution (resolve_repo, default_repo_error) @@ -88,7 +88,7 @@ data routes, the `token` secret — reads through them per call. Never capture a value at register time: an `onboard_project` mid-session, or a token pasted in Settings, must be seen by the very next call. -## 5. Tools (all implemented — 12 read / 8 write / 3 review = 23) +## 5. Tools (all implemented — 15 read / 8 write / 3 review = 26) Each tool mocks `run_gh` in its test and asserts the exact argv + readable errors. `tests/test_no_raise_sweep.py` additionally invokes EVERY registered tool against a @@ -102,10 +102,23 @@ sweep covers it automatically (it enumerates `register()`'s output). — a file at a PR's head; `github_repo_contents` — directory listing, says "is a file — use github_read_file" on a file; `github_path_exists` — the EXISTS/MISSING probe), and `github_status` — is `gh` installed / authenticated / as whom / which default repo, the -self-diagnosis tool the model calls when another tool errors (no `write` gate). +self-diagnosis tool the model calls when another tool errors (no `write` gate), and the +PM verbs (v0.7.0): `github_list_prs` (reuses `api.fetch_prs`, so the tool and the board +can't disagree — draft / review decision / merge state per row), `github_issue_comments` +(`gh issue view --json comments`; works on PRs; newest `limit` in chronological order, +bodies ≤ 1000 chars), `github_search_issues` (`gh search issues --repo`; `state: all` = +no `--state` flag, gh only knows open|closed — documented as "dedupe before filing"). +`github_get_pr` carries the merge-readiness picture: `reviewDecision`, `mergeable`, +`mergeStateStatus`, `statusCheckRollup` summarised (N pass / fail / pending + the +failing names — both the CheckRun and StatusContext shapes), `reviews` (author, state, +≤ 300-char body, ≤ 10 shown), `isDraft`; total output bounded at 12k chars. **Write (gated on `github.write`)** — -`github_create_issue` / `github_comment` / `github_create_pr` (return the new URL), +`github_create_issue` (**body-gated**: the SAME `missing_sections` gate the `/issue` +command enforces — a thin body, or a `bug` without repro / a `feature` without a +direction-or-acceptance section, is refused with the scaffold and never posted; `kind` +also adds the type label via `labels_for`) / `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` diff --git a/README.md b/README.md index adb1bf4..02f1192 100644 --- a/README.md +++ b/README.md @@ -10,20 +10,28 @@ read-only in-tree `github` plugin. ## Tools (all implemented) -**Read** (always, 12): `github_get_pr`, `github_get_issue`, `github_list_issues`, -`github_get_commit_diff`, `github_pr_diff`, `github_ci_runs`, `github_run_failure`, -`github_read_file`, `github_read_pr_file`, `github_repo_contents`, `github_path_exists`, -and `github_status` (is `gh` installed / signed in, as whom, which default repo — the -self-diagnosis probe the model calls when another tool errors). - -**Write** (only when `github.write: true`, 8): `github_create_issue`, `github_comment`, -`github_create_pr`, `github_edit_pr`, `github_merge_pr` (`confirm`-guarded), -`github_close`, `github_set_labels`, `github_set_assignees`. +**Read** (always, 15): `github_get_pr` (the merge-readiness picture: review decision, +mergeability, a checks summary with the failing check names, the reviews), +`github_list_prs` (the PR board: draft / review decision / merge state per row), +`github_get_issue`, `github_list_issues`, `github_issue_comments` (the thread on an +issue or PR), `github_search_issues` (dedupe **before** filing), `github_get_commit_diff`, +`github_pr_diff`, `github_ci_runs`, `github_run_failure`, `github_read_file`, +`github_read_pr_file`, `github_repo_contents`, `github_path_exists`, and `github_status` +(is `gh` installed / signed in, as whom, which default repo — the self-diagnosis probe +the model calls when another tool errors). + +**Write** (only when `github.write: true`, 8): `github_create_issue` (body-**gated** — +the same Problem / repro / acceptance sections the `/issue` command requires; a thin +body gets the scaffold back, never posted), `github_comment`, `github_create_pr`, +`github_edit_pr`, `github_merge_pr` (`confirm`-guarded), `github_close`, +`github_set_labels`, `github_set_assignees`. **Review** (also behind `github.write`, 3): `github_review_comment`, `github_review_approve`, `github_review_request_changes` — the formal verdict tools, with the CI-terminal and self-review guards enforced inside the tool (ADR 0078). +26 tools in all, every one covered by the no-raise sweep. + Plus the **user-only `/issue` chat command** (file an issue from the composer on any agent, without the model) and two console views — the read-only Issues/PRs **board** and the **New issue** form — both of which show a **setup card** when `gh` is missing diff --git a/api.py b/api.py index 94ec32b..3b70087 100644 --- a/api.py +++ b/api.py @@ -19,13 +19,16 @@ from __future__ import annotations import asyncio -import json -from .gh_cli import bad_repo, check_gh_error, resolve_gh, run_gh +from .gh_cli import bad_repo, check_gh_error, parse_json, resolve_gh, run_gh # The JSON fields we ask `gh` for — kept lean: enough for a board row + the detail link. _ISSUE_FIELDS = "number,title,state,author,labels,url,createdAt,comments" -_PR_FIELDS = "number,title,state,author,labels,url,createdAt,isDraft,headRefName,reviewDecision" +# `baseRefName` + `mergeStateStatus` are for `github_list_prs` (read_tools reuses fetch_prs); +# the board rows simply ignore them. +_PR_FIELDS = ( + "number,title,state,author,labels,url,createdAt,isDraft,headRefName,baseRefName,reviewDecision,mergeStateStatus" +) def gh_available() -> bool: @@ -57,16 +60,16 @@ async def fetch_issues(repo: str, state: str = "open", limit: int = 30) -> dict: ) if gh_err := check_gh_error(rc, serr, repo=repo): return {"error": gh_err} - try: - return {"items": json.loads(out or "[]")} - except json.JSONDecodeError: - return {"error": f"Error: could not parse gh output: {out[:200]}"} + items, perr = parse_json(out or "[]", list) # a non-list body is an error, never a crash + return {"error": perr} if perr else {"items": items} async def fetch_prs(repo: str, state: str = "open", limit: int = 30) -> dict: """List pull requests for ``repo`` as ``{"items": [...]}`` (or ``{"error": "..."}``). - Each item is the raw `gh pr list --json` row (adds isDraft/headRefName/reviewDecision). + Each item is the raw `gh pr list --json` row (adds isDraft/headRefName/baseRefName/ + reviewDecision/mergeStateStatus). Shared by the board's /prs route and the + `github_list_prs` tool, so the two can never disagree about a PR's state. """ if err := bad_repo(repo): return {"error": err} @@ -79,10 +82,8 @@ async def fetch_prs(repo: str, state: str = "open", limit: int = 30) -> dict: ) if gh_err := check_gh_error(rc, serr, repo=repo): return {"error": gh_err} - try: - return {"items": json.loads(out or "[]")} - except json.JSONDecodeError: - return {"error": f"Error: could not parse gh output: {out[:200]}"} + items, perr = parse_json(out or "[]", list) # a non-list body is an error, never a crash + return {"error": perr} if perr else {"items": items} def _repos(cfg: dict) -> list[str]: diff --git a/gh_issue.py b/gh_issue.py index 3410cb9..277aa8d 100644 --- a/gh_issue.py +++ b/gh_issue.py @@ -86,6 +86,12 @@ def _scaffold(kind: str) -> str: return {"bug": _BUG_SCAFFOLD, "feature": _FEATURE_SCAFFOLD}.get(kind, _GENERIC_SCAFFOLD) +def scaffold_for(kind: str) -> str: + """The fill-in scaffold for an issue ``kind`` — what both the `/issue` command and + the `github_create_issue` tool hand back when the body fails the gate.""" + return _scaffold(kind) + + def missing_sections(body: str, kind: str) -> list[str]: """The gate-required sections absent from ``body`` for this issue ``kind``.""" miss: list[str] = [] diff --git a/protoagent.plugin.yaml b/protoagent.plugin.yaml index da3f150..ab8da08 100644 --- a/protoagent.plugin.yaml +++ b/protoagent.plugin.yaml @@ -2,14 +2,15 @@ # Keep `version` in lockstep with pyproject.toml (tests/test_version.py asserts it). id: github name: GitHub (read/write tools) -version: 0.6.0 +version: 0.7.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 + read tools (PRs — list + merge readiness, issues, comments, search-before-filing, + diffs, CI, repo files/contents) are always on; the write 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. + Merging is `confirm`-guarded; issue creation is body-gated like `/issue`. Supersedes the read-only in-tree `github` plugin. Tools degrade to a readable, classified error (not authenticated / repo not found / rate-limited / `gh` missing) — and `github_status` + a setup card in the views say exactly what to fix on a fresh machine. diff --git a/pyproject.toml b/pyproject.toml index 9e182cc..95587cf 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "github-plugin" -version = "0.6.0" +version = "0.7.0" description = "Read/write GitHub tools for protoAgent over the gh CLI, with per-agent write gating." requires-python = ">=3.11" diff --git a/read_tools.py b/read_tools.py index 422ed8c..cfe0f24 100644 --- a/read_tools.py +++ b/read_tools.py @@ -1,9 +1,13 @@ """GitHub READ tools over `gh` — always registered (read-only is the safe default). -Twelve tools, all implemented: six ported from protoAgent's tools/github_tools.py +Fifteen tools, all implemented: six ported from protoAgent's tools/github_tools.py (PRs, issues, diffs, CI), the repo-content readers (`github_read_file`, `github_read_pr_file`, `github_repo_contents`, `github_path_exists`), `github_pr_diff`, -and `github_status` (the self-diagnosis probe — is `gh` installed / authenticated). +`github_status` (the self-diagnosis probe — is `gh` installed / authenticated), and the +PM verbs (v0.7.0): `github_list_prs` (the PR board: draft / review decision / merge +state), `github_issue_comments` (the thread on an issue or PR), and +`github_search_issues` (dedupe BEFORE filing). `github_get_pr` carries the merge +readiness picture — review decision, mergeability, a checks summary, the reviews. Each tool takes an `owner/name` repo — or falls back to the configured default when it's omitted (a LIVE getter, so a Settings edit or an onboarded project is seen by the next call) — and degrades to a readable, classified `Error: ...` string (not @@ -28,6 +32,67 @@ ) +_MAX_PR_CHARS = 12000 # github_get_pr's total output bound +_MAX_REVIEWS = 10 +_MAX_COMMENT_CHARS = 1000 + +# statusCheckRollup carries two shapes (verified against gh 2.92): a CheckRun +# {name, status: COMPLETED|IN_PROGRESS|QUEUED|…, conclusion: SUCCESS|FAILURE|SKIPPED| +# CANCELLED|NEUTRAL|TIMED_OUT|ACTION_REQUIRED|""} and a StatusContext {context, +# state: SUCCESS|PENDING|FAILURE|ERROR|EXPECTED}. +_CHECK_FAIL = {"FAILURE", "ERROR", "TIMED_OUT", "CANCELLED", "ACTION_REQUIRED", "STARTUP_FAILURE"} +_CHECK_SKIP = {"SKIPPED", "NEUTRAL", "STALE"} + + +def _one_line(text: str, cap: int) -> str: + """Collapse whitespace and cap — for a review/comment excerpt on one line.""" + t = " ".join((text or "").split()) + return t if len(t) <= cap else t[: cap - 1] + "…" + + +def _bounded(text: str, cap: int) -> str: + return text if len(text) <= cap else text[:cap] + f"\n… (truncated at {cap} chars)" + + +def _summarize_checks(rollup) -> str: + """``N pass / N fail / N pending (/ N skipped) — failing: a, b`` from a PR's + statusCheckRollup; ``none`` when the commit has no checks.""" + items = dicts(rollup) + if not items: + return "none" + n_pass = n_fail = n_pending = n_skip = 0 + failing: list[str] = [] + for c in items: + name = str(c.get("name") or c.get("context") or "?") + if c.get("__typename") == "StatusContext" or ("state" in c and "status" not in c): + state = str(c.get("state") or "").upper() + if state == "SUCCESS": + n_pass += 1 + elif state in _CHECK_FAIL: + n_fail += 1 + failing.append(name) + else: # PENDING / EXPECTED / unknown + n_pending += 1 + continue + status = str(c.get("status") or "").upper() + conclusion = str(c.get("conclusion") or "").upper() + if status and status != "COMPLETED": + n_pending += 1 + elif conclusion == "SUCCESS": + n_pass += 1 + elif conclusion in _CHECK_FAIL: + n_fail += 1 + failing.append(name) + elif conclusion in _CHECK_SKIP: + n_skip += 1 + else: + n_pending += 1 + parts = f"{n_pass} pass / {n_fail} fail / {n_pending} pending" + (f" / {n_skip} skipped" if n_skip else "") + if failing: + parts += " — failing: " + ", ".join(failing[:10]) + (" …" if len(failing) > 10 else "") + return parts + + def get_read_tools(default_repo="", repos=None, registry=None) -> list: """Build the read tools. ``default_repo`` (``owner/name``, or a zero-arg getter returning it — the live-config case) is used whenever a tool's ``repo`` arg is @@ -44,7 +109,10 @@ def _repos() -> list[str]: @tool async def github_get_pr(number: int, repo: str = "") -> str: - """Fetch a GitHub pull request: title, state, author, body, branch, and changed files. + """Fetch a GitHub pull request — the merge-readiness picture in one call: title, + state, draft?, author, branch, review decision, mergeability + merge state, a + checks summary (N pass / N fail / N pending + the failing check names), the + reviews (author, verdict, excerpt), changed files, and the body. Args: repo: Repository as ``owner/name``. Omit to use the agent's configured default repo. @@ -61,7 +129,8 @@ async def github_get_pr(number: int, repo: str = "") -> str: "--repo", repo, "--json", - "number,title,state,author,body,additions,deletions,files,url,headRefName,baseRefName", + "number,title,state,isDraft,author,body,additions,deletions,files,url,headRefName,baseRefName," + "reviewDecision,mergeable,mergeStateStatus,statusCheckRollup,reviews", ] ) if gh_err := check_gh_error(rc, serr, repo=repo): @@ -70,13 +139,29 @@ async def github_get_pr(number: int, repo: str = "") -> str: if perr: return perr files = ", ".join(str(f.get("path", "?")) for f in dicts(d.get("files"))[:20]) - return ( - f"PR #{d.get('number')} [{d.get('state')}] {d.get('title')}\n" + draft = " (DRAFT)" if d.get("isDraft") else "" + checks = _summarize_checks(d.get("statusCheckRollup")) + reviews = dicts(d.get("reviews")) + review_lines = [ + f" - {str((r.get('author') or {}).get('login') or '?') if isinstance(r.get('author'), dict) else '?'} " + f"[{r.get('state') or '?'}]" + + (f": {_one_line(str(r.get('body') or ''), 300)}" if str(r.get("body") or "").strip() else "") + for r in reviews[:_MAX_REVIEWS] + ] + if len(reviews) > _MAX_REVIEWS: + review_lines.append(f" … {len(reviews) - _MAX_REVIEWS} more review(s)") + text = ( + f"PR #{d.get('number')} [{d.get('state')}]{draft} {d.get('title')}\n" f"branch: {d.get('headRefName', '?')} -> {d.get('baseRefName', '?')}\n" - f"by {(d.get('author') or {}).get('login', '?')} | " + f"by {(d.get('author') or {}).get('login', '?') if isinstance(d.get('author'), dict) else '?'} | " f"+{d.get('additions', 0)}/-{d.get('deletions', 0)} | {d.get('url')}\n" + f"review decision: {d.get('reviewDecision') or 'none yet'} | " + f"mergeable: {d.get('mergeable') or '?'} | merge state: {d.get('mergeStateStatus') or '?'}\n" + f"checks: {checks}\n" + f"reviews ({len(reviews)}):" + ("\n" + "\n".join(review_lines) if review_lines else " none") + "\n" f"files: {files or '(none)'}\n\n{(d.get('body') or '').strip()[:2000]}" ) + return _bounded(text, _MAX_PR_CHARS) @tool async def github_get_issue(number: int, repo: str = "") -> str: @@ -421,6 +506,137 @@ async def github_repo_contents(repo: str = "", path: str = "", ref: str = "") -> lines.append(f"{t:4s} {size:>8s} {entry.get('name', '?')} ({entry.get('path', '')})") return "\n".join(lines) + @tool + async def github_list_prs(repo: str = "", state: str = "open", limit: int = 30) -> str: + """List pull requests — the PR board: number, title, author, state, draft?, branch, + review decision and merge state per row. Use ``github_get_pr`` for one PR's full picture. + + Args: + repo: Repository as ``owner/name``. Omit to use the agent's configured default repo. + state: ``open`` (default) | ``closed`` | ``merged`` | ``all``. + limit: Max PRs to return (1-100, default 30). + """ + from .api import fetch_prs + + repo = resolve_repo(repo, default_repo) or "" + if err := bad_repo(repo): + return err + try: + capped = max(1, min(int(limit), 100)) + except (TypeError, ValueError): + capped = 30 + res = await fetch_prs(repo, state, capped) + if res.get("error"): + return str(res["error"]) + items = dicts(res.get("items")) + if not items: + return f"No {state} pull requests in {repo}." + lines = [f"{len(items)} {state} pull request(s) in {repo}:"] + for it in items: + author = (it.get("author") or {}).get("login", "?") if isinstance(it.get("author"), dict) else "?" + flags = [ + x + for x in (("draft" if it.get("isDraft") else ""), it.get("reviewDecision"), it.get("mergeStateStatus")) + if x + ] + lines.append( + f" #{it.get('number')} [{it.get('state')}] {it.get('title')} — {author} | " + f"{it.get('headRefName', '?')} -> {it.get('baseRefName', '?')}" + + (f" | {', '.join(str(f) for f in flags)}" if flags else "") + + f" | {it.get('url')}" + ) + return "\n".join(lines) + + @tool + async def github_issue_comments(number: int, repo: str = "", limit: int = 30) -> str: + """Read the comment thread on an issue or pull request (a PR is an issue for + comments): author, date and body per comment, newest ``limit`` in chronological + order. Use it to catch up on a discussion before replying or deciding. + + Args: + repo: Repository as ``owner/name``. Omit to use the agent's configured default repo. + number: Issue or PR number. + limit: Max comments to return (1-100, default 30 — the most recent ones). + """ + repo = resolve_repo(repo, default_repo) or "" + if err := bad_repo(repo): + return err + try: + capped = max(1, min(int(limit), 100)) + except (TypeError, ValueError): + capped = 30 + rc, out, serr = await run_gh(["issue", "view", str(number), "--repo", repo, "--json", "comments"]) + if gh_err := check_gh_error(rc, serr, repo=repo): + return gh_err + d, perr = parse_json(out, dict) + if perr: + return perr + comments = dicts(d.get("comments")) + if not comments: + return f"No comments on {repo}#{number}." + shown = comments[-capped:] + head = f"{len(comments)} comment(s) on {repo}#{number}" + ( + f" — showing the last {len(shown)}" if len(shown) < len(comments) else "" + ) + lines = [head + ":"] + for c in shown: + author = (c.get("author") or {}).get("login", "?") if isinstance(c.get("author"), dict) else "?" + body = " ".join(str(c.get("body") or "").split()) + if len(body) > _MAX_COMMENT_CHARS: + body = body[: _MAX_COMMENT_CHARS - 1] + "…" + lines.append(f"--- {author} · {c.get('createdAt') or '?'}\n{body or '(empty)'}") + return "\n".join(lines) + + @tool + async def github_search_issues(query: str, repo: str = "", state: str = "open", limit: int = 20) -> str: + """Search a repo's issues by text — DEDUPE BEFORE FILING: call this with the + gist of a problem before ``github_create_issue`` and reference or reopen a + match instead of filing a duplicate. Returns number, state, title and URL per hit. + + Args: + query: Free-text search (GitHub issue search syntax; e.g. ``"default_repo" label:bug``). + repo: Repository as ``owner/name``. Omit to use the agent's configured default repo. + state: ``open`` (default) | ``closed`` | ``all``. + limit: Max results (1-100, default 20). + """ + repo = resolve_repo(repo, default_repo) or "" + if err := bad_repo(repo): + return err + if not (query or "").strip(): + return "Error: `query` is empty — say what you're looking for." + if state not in ("open", "closed", "all"): + return f"Error: state must be open|closed|all (got {state!r})." + try: + capped = max(1, min(int(limit), 100)) + except (TypeError, ValueError): + capped = 20 + args = [ + "search", + "issues", + "--repo", + repo, + query.strip(), + "--limit", + str(capped), + "--json", + "number,title,state,url", + ] + if state != "all": # gh's --state is open|closed only; "all" = no filter + args += ["--state", state] + rc, out, serr = await run_gh(args) + if gh_err := check_gh_error(rc, serr, repo=repo): + return gh_err + items, perr = parse_json(out or "[]", list) + if perr: + return perr + items = dicts(items) + if not items: + return f"No {state} issues in {repo} match {query.strip()!r} — nothing to dedupe against." + lines = [f"{len(items)} {state} issue(s) in {repo} matching {query.strip()!r}:"] + for it in items: + lines.append(f" #{it.get('number')} [{it.get('state')}] {it.get('title')} — {it.get('url')}") + return "\n".join(lines) + @tool async def github_status() -> str: """Check whether the GitHub CLI is installed and authenticated, and which repo the @@ -451,5 +667,8 @@ async def github_status() -> str: github_read_file, github_read_pr_file, github_repo_contents, + github_list_prs, + github_issue_comments, + github_search_issues, github_status, ] diff --git a/tests/test_no_raise_sweep.py b/tests/test_no_raise_sweep.py index 7951723..7fd1c3f 100644 --- a/tests/test_no_raise_sweep.py +++ b/tests/test_no_raise_sweep.py @@ -16,6 +16,11 @@ import pytest from ghplugin import register +_GATE_OK_BODY = ( + "## Problem\nThe widget crashes on empty input and we should handle it gracefully " + "throughout the pipeline instead of raising.\n## Acceptance\nNo crash on empty input." +) + # Minimal valid args per tool — enough to get PAST the arg validation and into the # `run_gh` result handling, which is what the sweep is about. Any tool not listed # here is invoked with `{}` (its defaults), and the sweep fails loudly if a NEW tool @@ -32,8 +37,12 @@ "github_read_file": {"repo": "o/n", "path": "README.md"}, "github_read_pr_file": {"repo": "o/n", "number": 1, "path": "x"}, "github_repo_contents": {"repo": "o/n", "path": "src"}, + "github_list_prs": {"repo": "o/n"}, + "github_issue_comments": {"repo": "o/n", "number": 1}, + "github_search_issues": {"repo": "o/n", "query": "crash"}, "github_status": {}, - "github_create_issue": {"repo": "o/n", "title": "t"}, + # create_issue is body-GATED (v0.7.0) — a gate-passing body so the sweep reaches gh. + "github_create_issue": {"repo": "o/n", "title": "t", "body": _GATE_OK_BODY}, "github_comment": {"repo": "o/n", "number": 1, "body": "b"}, "github_create_pr": {"repo": "o/n", "head": "h", "title": "t"}, "github_edit_pr": {"repo": "o/n", "number": 1, "title": "t", "state": "ready"}, diff --git a/tests/test_read_tools.py b/tests/test_read_tools.py index ff9316c..0970b2d 100644 --- a/tests/test_read_tools.py +++ b/tests/test_read_tools.py @@ -493,3 +493,229 @@ async def test_path_exists_classifies_before_the_missing_verdict(): with patch("ghplugin.read_tools.run_gh", new=AsyncMock(return_value=(1, "", "gh: Not Found (HTTP 404)"))): out = await _path_exists_tool().ainvoke({"repo": "owner/name", "path": "x"}) assert out.startswith("MISSING: owner/name/x") and "repo is inaccessible" in out + + +# ── v0.7.0: the PM verbs ───────────────────────────────────────────────────────── + + +def _tool(name, default_repo=""): + for t in get_read_tools(default_repo): + if t.name == name: + return t + raise AssertionError(f"{name} not found") + + +_RICH_PR_JSON = json.dumps( + { + **json.loads(_PR_JSON), + "isDraft": True, + "reviewDecision": "CHANGES_REQUESTED", + "mergeable": "MERGEABLE", + "mergeStateStatus": "BLOCKED", + "statusCheckRollup": [ + {"__typename": "CheckRun", "name": "test", "status": "COMPLETED", "conclusion": "SUCCESS"}, + {"__typename": "CheckRun", "name": "lint", "status": "COMPLETED", "conclusion": "FAILURE"}, + {"__typename": "CheckRun", "name": "e2e", "status": "IN_PROGRESS", "conclusion": ""}, + {"__typename": "CheckRun", "name": "docs", "status": "COMPLETED", "conclusion": "SKIPPED"}, + {"__typename": "StatusContext", "context": "CodeRabbit", "state": "SUCCESS"}, + {"__typename": "StatusContext", "context": "deploy/preview", "state": "PENDING"}, + {"__typename": "StatusContext", "context": "security", "state": "ERROR"}, + ], + "reviews": [ + {"author": {"login": "quinn"}, "state": "CHANGES_REQUESTED", "body": "Blocker: " + "x" * 400}, + {"author": {"login": "kj"}, "state": "APPROVED", "body": ""}, + {"author": None, "state": "COMMENTED", "body": "drive-by"}, + ], + } +) + + +@pytest.mark.asyncio +async def test_get_pr_requests_and_renders_the_merge_readiness_fields(): + mock = AsyncMock(return_value=(0, _RICH_PR_JSON, "")) + with patch("ghplugin.read_tools.run_gh", mock): + out = await _get_pr_tool().ainvoke({"repo": "owner/name", "number": 38}) + json_arg = mock.call_args.args[0][mock.call_args.args[0].index("--json") + 1] + for f in ("isDraft", "reviewDecision", "mergeable", "mergeStateStatus", "statusCheckRollup", "reviews"): + assert f in json_arg + assert "PR #38 [OPEN] (DRAFT) feat: fix the ephemeral label" in out + assert "review decision: CHANGES_REQUESTED | mergeable: MERGEABLE | merge state: BLOCKED" in out + assert "checks: 2 pass / 2 fail / 2 pending / 1 skipped — failing: lint, security" in out + assert "reviews (3):" in out + assert " - quinn [CHANGES_REQUESTED]: Blocker: " in out and "x" * 300 not in out # body capped at 300 + assert " - kj [APPROVED]" in out and " - ? [COMMENTED]: drive-by" in out # null author tolerated + assert "files: view.py" in out and out.rstrip().endswith("fixed it") + + +@pytest.mark.asyncio +async def test_get_pr_with_no_checks_or_reviews_says_so(): + d = {**json.loads(_PR_JSON), "statusCheckRollup": [], "reviews": [], "reviewDecision": ""} + with patch("ghplugin.read_tools.run_gh", new=AsyncMock(return_value=(0, json.dumps(d), ""))): + out = await _get_pr_tool().ainvoke({"repo": "owner/name", "number": 38}) + assert "checks: none" in out and "reviews (0): none" in out and "review decision: none yet" in out + + +@pytest.mark.asyncio +async def test_get_pr_output_is_bounded(): + from ghplugin.read_tools import _MAX_PR_CHARS + + d = { + **json.loads(_RICH_PR_JSON), + "body": "b" * 50000, + "reviews": [{"author": {"login": "r"}, "state": "COMMENTED", "body": "y" * 2000}] * 40, + } + with patch("ghplugin.read_tools.run_gh", new=AsyncMock(return_value=(0, json.dumps(d), ""))): + out = await _get_pr_tool().ainvoke({"repo": "owner/name", "number": 1}) + assert len(out) <= _MAX_PR_CHARS + 60 and "… 30 more review(s)" in out + + +def test_summarize_checks_shapes(): + from ghplugin.read_tools import _summarize_checks + + assert _summarize_checks(None) == "none" and _summarize_checks([]) == "none" + assert _summarize_checks("garbage") == "none" + assert ( + _summarize_checks([{"name": "a", "status": "COMPLETED", "conclusion": "SUCCESS"}]) + == "1 pass / 0 fail / 0 pending" + ) + assert _summarize_checks([{"context": "ci", "state": "FAILURE"}]) == "0 pass / 1 fail / 0 pending — failing: ci" + assert _summarize_checks([{"name": "q", "status": "QUEUED", "conclusion": None}]) == "0 pass / 0 fail / 1 pending" + + +# github_list_prs — reuses api.fetch_prs (so patch run_gh where api binds it) + + +@pytest.mark.asyncio +async def test_list_prs_reuses_the_board_fetch_and_renders_flags(): + rows = json.dumps( + [ + { + "number": 5, + "title": "PR", + "state": "OPEN", + "author": {"login": "kj"}, + "isDraft": True, + "headRefName": "f", + "baseRefName": "main", + "reviewDecision": "", + "mergeStateStatus": "BLOCKED", + "url": "u5", + }, + { + "number": 6, + "title": "Q", + "state": "OPEN", + "author": None, + "isDraft": False, + "headRefName": "g", + "baseRefName": "main", + "reviewDecision": "APPROVED", + "mergeStateStatus": "CLEAN", + "url": "u6", + }, + ] + ) + mock = AsyncMock(return_value=(0, rows, "")) + with patch("ghplugin.api.run_gh", mock): + out = await _tool("github_list_prs").ainvoke({"repo": "owner/name", "state": "all", "limit": 500}) + argv = mock.call_args.args[0] + assert ( + argv[:4] == ["pr", "list", "--repo", "owner/name"] + and "--state" in argv + and argv[argv.index("--limit") + 1] == "100" + ) + json_arg = argv[argv.index("--json") + 1] + for f in ("isDraft", "headRefName", "baseRefName", "reviewDecision", "mergeStateStatus"): + assert f in json_arg + assert out.startswith("2 all pull request(s) in owner/name:") + assert " #5 [OPEN] PR — kj | f -> main | draft, BLOCKED | u5" in out + assert " #6 [OPEN] Q — ? | g -> main | APPROVED, CLEAN | u6" in out + + +@pytest.mark.asyncio +async def test_list_prs_empty_bad_state_and_errors(): + with patch("ghplugin.api.run_gh", new=AsyncMock(return_value=(0, "[]", ""))): + assert ( + await _tool("github_list_prs").ainvoke({"repo": "owner/name"}) + ) == "No open pull requests in owner/name." + with patch("ghplugin.api.run_gh", new=AsyncMock(return_value=(4, "", "gh auth login"))): + out = await _tool("github_list_prs").ainvoke({"repo": "owner/name"}) + assert out.startswith("Error: GitHub CLI is not authenticated") + assert "state must be" in (await _tool("github_list_prs").ainvoke({"repo": "owner/name", "state": "weird"})) + assert (await _tool("github_list_prs").ainvoke({"repo": "bad"})).startswith("Error: no usable repo") + + +# github_issue_comments + + +@pytest.mark.asyncio +async def test_issue_comments_renders_newest_limit_in_order_and_caps_bodies(): + comments = [ + {"author": {"login": f"u{i}"}, "createdAt": f"2026-08-{i:02d}", "body": f"c{i} " + "z" * 1500} + for i in range(1, 6) + ] + mock = AsyncMock(return_value=(0, json.dumps({"comments": comments}), "")) + with patch("ghplugin.read_tools.run_gh", mock): + out = await _tool("github_issue_comments").ainvoke({"repo": "owner/name", "number": 7, "limit": 2}) + assert mock.call_args.args[0] == ["issue", "view", "7", "--repo", "owner/name", "--json", "comments"] + assert out.startswith("5 comment(s) on owner/name#7 — showing the last 2:") + assert "--- u4 · 2026-08-04" in out and "--- u5 · 2026-08-05" in out and "u3" not in out + assert out.index("u4") < out.index("u5") # chronological + assert "z" * 1000 not in out and "…" in out # each body capped at 1000 + + +@pytest.mark.asyncio +async def test_issue_comments_empty_and_odd_rows(): + with patch("ghplugin.read_tools.run_gh", new=AsyncMock(return_value=(0, '{"comments": []}', ""))): + assert ( + await _tool("github_issue_comments").ainvoke({"repo": "owner/name", "number": 1}) + ) == "No comments on owner/name#1." + odd = json.dumps({"comments": [None, {"author": None, "createdAt": None, "body": None}]}) + with patch("ghplugin.read_tools.run_gh", new=AsyncMock(return_value=(0, odd, ""))): + out = await _tool("github_issue_comments").ainvoke({"repo": "owner/name", "number": 1}) + assert "1 comment(s)" in out and "--- ? · ?\n(empty)" in out + + +# github_search_issues + + +@pytest.mark.asyncio +async def test_search_issues_argv_and_render(): + hits = json.dumps([{"number": 23, "title": "Validate default_repo", "state": "closed", "url": "u23"}]) + mock = AsyncMock(return_value=(0, hits, "")) + with patch("ghplugin.read_tools.run_gh", mock): + out = await _tool("github_search_issues").ainvoke( + {"repo": "owner/name", "query": " default_repo typo ", "state": "closed", "limit": 5} + ) + argv = mock.call_args.args[0] + assert argv[:4] == ["search", "issues", "--repo", "owner/name"] and argv[4] == "default_repo typo" + assert argv[argv.index("--limit") + 1] == "5" and argv[argv.index("--state") + 1] == "closed" + assert ( + out + == "1 closed issue(s) in owner/name matching 'default_repo typo':\n #23 [closed] Validate default_repo — u23" + ) + + +@pytest.mark.asyncio +async def test_search_issues_all_state_omits_the_flag_and_validates(): + """gh's --state is open|closed only — `all` means no filter (verified against gh 2.92).""" + mock = AsyncMock(return_value=(0, "[]", "")) + with patch("ghplugin.read_tools.run_gh", mock): + out = await _tool("github_search_issues").ainvoke({"repo": "owner/name", "query": "x", "state": "all"}) + assert "--state" not in mock.call_args.args[0] + assert out == "No all issues in owner/name match 'x' — nothing to dedupe against." + assert "query` is empty" in (await _tool("github_search_issues").ainvoke({"repo": "owner/name", "query": " "})) + assert "state must be" in ( + await _tool("github_search_issues").ainvoke({"repo": "owner/name", "query": "x", "state": "merged"}) + ) + mock.assert_called_once() + + +def test_search_issues_description_says_dedupe_before_filing(): + assert "DEDUPE BEFORE FILING" in _tool("github_search_issues").description + assert ( + "duplicate" + in {t.name: t for t in __import__("ghplugin.write_tools", fromlist=["get_write_tools"]).get_write_tools()}[ + "github_create_issue" + ].description + ) diff --git a/tests/test_register.py b/tests/test_register.py index aab5819..60c22bf 100644 --- a/tests/test_register.py +++ b/tests/test_register.py @@ -6,6 +6,8 @@ from __future__ import annotations +from pathlib import Path + from ghplugin import register READ_TOOLS = { @@ -13,11 +15,17 @@ "github_get_issue", "github_list_issues", "github_get_commit_diff", + "github_pr_diff", + "github_path_exists", "github_ci_runs", "github_run_failure", "github_read_file", + "github_read_pr_file", "github_repo_contents", "github_status", # the self-diagnosis probe — always on, no write gate (v0.6.0) + "github_list_prs", # the PM verbs (v0.7.0) + "github_issue_comments", + "github_search_issues", } WRITE_TOOLS = { "github_create_issue", @@ -175,3 +183,17 @@ async def test_configured_default_repo_skips_the_picker_computation(make_registr with patch("ghplugin.read_tools.run_gh", AsyncMock(return_value=(0, "[]", ""))): await tool.ainvoke({}) assert calls == [1] # …only when the default is unset + + +def test_inventory_counts_match_the_docs(make_registry): + """README / PROTO.md say 15 read / 8 write / 3 review = 26. Recount from what + register() actually produces so the docs can't drift from the code again.""" + reg = make_registry({"write": True}) + register(reg) + names = set(reg.tool_names) + review = {"github_review_comment", "github_review_approve", "github_review_request_changes"} + assert len(READ_TOOLS) == 15 and len(WRITE_TOOLS) == 8 and len(review) == 3 + assert names == READ_TOOLS | WRITE_TOOLS | review and len(names) == 26 + for doc in ("README.md", "PROTO.md"): + text = (Path(__file__).resolve().parent.parent / doc).read_text() + assert "15" in text and ("= 26" in text or "26 tools" in text), f"{doc} inventory stale" diff --git a/tests/test_write_tools.py b/tests/test_write_tools.py index fd1775a..b55bdeb 100644 --- a/tests/test_write_tools.py +++ b/tests/test_write_tools.py @@ -17,6 +17,14 @@ def _create_issue(): return {t.name: t for t in get_write_tools()}["github_create_issue"] +# A body that clears the gate the tool shares with `/issue` (v0.7.0): >= 80 collapsed +# chars + a Problem section (+ repro for a bug / direction-or-acceptance for a feature). +_OK_BODY = ( + "## Problem\nIt broke on empty input and the whole pipeline raised instead of " + "handling it gracefully.\n## Acceptance\nEmpty input is handled without raising." +) + + def _labels_in(args: list[str]) -> list[str]: """The value following each ``--label`` flag, in order.""" return [args[i + 1] for i, a in enumerate(args) if a == "--label"] @@ -27,12 +35,12 @@ async def test_returns_issue_url(): tool = _create_issue() fake = AsyncMock(return_value=(0, "https://github.com/o/n/issues/42\n", "")) with patch("ghplugin.write_tools.run_gh", fake): - out = await tool.ainvoke({"repo": "o/n", "title": "Bug", "body": "It broke"}) + out = await tool.ainvoke({"repo": "o/n", "title": "Bug", "body": _OK_BODY}) assert out == "https://github.com/o/n/issues/42" args = fake.call_args.args[0] assert args[:4] == ["issue", "create", "--repo", "o/n"] assert "--title" in args and args[args.index("--title") + 1] == "Bug" - assert "--body" in args and args[args.index("--body") + 1] == "It broke" + assert "--body" in args and args[args.index("--body") + 1] == _OK_BODY async def test_no_labels_omits_label_flag(): @@ -40,7 +48,7 @@ async def test_no_labels_omits_label_flag(): tool = _create_issue() fake = AsyncMock(return_value=(0, "https://github.com/o/n/issues/1", "")) with patch("ghplugin.write_tools.run_gh", fake): - await tool.ainvoke({"repo": "o/n", "title": "t"}) + await tool.ainvoke({"repo": "o/n", "title": "t", "body": _OK_BODY}) assert "--label" not in fake.call_args.args[0] @@ -49,7 +57,7 @@ async def test_labels_split_into_separate_flags(): tool = _create_issue() fake = AsyncMock(return_value=(0, "https://github.com/o/n/issues/7", "")) with patch("ghplugin.write_tools.run_gh", fake): - await tool.ainvoke({"repo": "o/n", "title": "t", "labels": "bug, enhancement ,"}) + await tool.ainvoke({"repo": "o/n", "title": "t", "body": _OK_BODY, "labels": "bug, enhancement ,"}) assert _labels_in(fake.call_args.args[0]) == ["bug", "enhancement"] @@ -69,10 +77,78 @@ async def test_gh_failure_returns_check_gh_error(): tool = _create_issue() fake = AsyncMock(return_value=(1, "", "could not create issue: forbidden")) with patch("ghplugin.write_tools.run_gh", fake): - out = await tool.ainvoke({"repo": "o/n", "title": "t"}) + out = await tool.ainvoke({"repo": "o/n", "title": "t", "body": _OK_BODY}) assert out == "Error (gh exit 1): could not create issue: forbidden" +# ── the body gate (v0.7.0) — the same one `/issue` enforces ────────────────────── + + +async def test_create_issue_refuses_a_gate_failing_body_with_the_scaffold(): + """A minimal body is NOT posted; the tool says what's missing and hands back the + scaffold — exactly what the /issue command does (gh_issue.missing_sections).""" + tool = _create_issue() + fake = AsyncMock() + with patch("ghplugin.write_tools.run_gh", fake): + out = await tool.ainvoke({"repo": "o/n", "title": "Bug", "body": "It broke"}) + assert out.startswith("Not filed — the issue body is missing ") + assert "a substantive description (>= 80 chars)" in out + assert "a Problem / What's-wrong / Motivation section" in out + assert "## Problem" in out and "## Acceptance" in out # the generic scaffold + fake.assert_not_called() # never shells out when the gate fails + + +async def test_create_issue_bug_kind_needs_repro_and_adds_the_bug_label(): + tool = _create_issue() + fake = AsyncMock(return_value=(0, "https://github.com/o/n/issues/9", "")) + no_repro = ( + "## Problem\nThe parser crashes on empty input and the whole pipeline raises " + "instead of handling it gracefully — a user-visible failure." + ) + with patch("ghplugin.write_tools.run_gh", fake): + out = await tool.ainvoke({"repo": "o/n", "title": "Crash", "body": no_repro, "kind": "bug"}) + assert out.startswith("Not filed") and "Steps to reproduce" in out and "## Steps to reproduce" in out + fake.assert_not_called() + with_repro = no_repro + "\n## Steps to reproduce\nRun it with an empty string." + with patch("ghplugin.write_tools.run_gh", fake): + out = await tool.ainvoke({"repo": "o/n", "title": "Crash", "body": with_repro, "kind": "bug", "labels": "p0"}) + assert out.endswith("/issues/9") + assert _labels_in(fake.call_args.args[0]) == ["bug", "p0"] # type label first, like /issue + + +async def test_create_issue_feature_kind_needs_direction_or_acceptance(): + tool = _create_issue() + fake = AsyncMock(return_value=(0, "https://github.com/o/n/issues/10", "")) + body = "## Motivation\nWe need this capability badly for the next release cycle to ship on time." + with patch("ghplugin.write_tools.run_gh", fake): + out = await tool.ainvoke({"repo": "o/n", "title": "Feat", "body": body, "kind": "feature"}) + assert out.startswith("Not filed") and "Proposed-direction or Acceptance" in out + with patch("ghplugin.write_tools.run_gh", fake): + out = await tool.ainvoke( + {"repo": "o/n", "title": "Feat", "body": body + "\n## Acceptance\nIt ships.", "kind": "feature"} + ) + assert out.endswith("/issues/10") and _labels_in(fake.call_args.args[0]) == ["enhancement"] + + +async def test_create_issue_rejects_an_unknown_kind(): + out = await _create_issue().ainvoke({"repo": "o/n", "title": "t", "body": _OK_BODY, "kind": "epic"}) + assert out.startswith("Error: kind must be") + + +async def test_create_issue_gate_matches_the_issue_command(): + """One gate, two entry points: whatever /issue refuses, the tool refuses, and vice versa.""" + from ghplugin.gh_issue import missing_sections + + for kind, body in (("generic", "too short"), ("bug", _OK_BODY), ("feature", _OK_BODY), ("generic", _OK_BODY)): + tool_out = ( + await _create_issue().ainvoke({"repo": "o/n", "title": "t", "body": body, "kind": kind}) + if missing_sections(body, kind) + else None + ) + if missing_sections(body, kind): + assert tool_out.startswith("Not filed") + + def _comment(): return {t.name: t for t in get_write_tools()}["github_comment"] diff --git a/write_tools.py b/write_tools.py index ab929a5..21257f0 100644 --- a/write_tools.py +++ b/write_tools.py @@ -5,7 +5,10 @@ readable `Error: ...` string. The set: - - github_create_issue — `gh issue create` (returns the new issue URL). + - github_create_issue — `gh issue create` (returns the new issue URL). Runs the SAME + body gate the user-only `/issue` command enforces (gh_issue.missing_sections): a + body without a Problem section (+ repro for a bug, a direction/acceptance for a + feature) is refused with the scaffold, never posted. - github_comment — `gh issue comment` (works for PRs too — a PR is an issue). - github_create_pr — `gh pr create` (returns the new PR URL). - github_edit_pr — `gh pr edit` (+ `gh pr ready`) to change title/body/draft. @@ -28,7 +31,7 @@ from langchain_core.tools import tool from .gh_cli import bad_repo, check_gh_error, run_gh -from .gh_issue import resolve_repo +from .gh_issue import labels_for, missing_sections, resolve_repo, scaffold_for def _csv(value: str) -> list[str]: @@ -61,25 +64,42 @@ def _emit(topic: str, data: dict) -> None: pass @tool - async def github_create_issue(title: str, repo: str = "", body: str = "", labels: str = "") -> str: - """Create a GitHub issue. + async def github_create_issue( + title: str, repo: str = "", body: str = "", labels: str = "", kind: str = "generic" + ) -> str: + """Create a GitHub issue. Search first (``github_search_issues``) so you don't file + a duplicate. The body is GATED like the `/issue` command: it needs a substantive + description (>= 80 chars) with a ``## Problem`` (or Motivation/Context) section; a + ``bug`` also needs a ``## Steps to reproduce`` / Evidence / Expected-vs-actual + section; a ``feature`` needs a ``## Proposed direction`` or ``## Acceptance`` + section. A body that fails the gate is NOT posted — the tool returns what's + missing plus a scaffold to fill in. Args: repo: Repository as ``owner/name``. Omit to use the agent's configured default repo. title: Issue title. - body: Issue body (Markdown). + body: Issue body (Markdown) — use headings for the sections above. labels: Optional comma-separated label names. + kind: ``generic`` (default) | ``bug`` | ``feature`` — picks the gate's required + sections and adds the type label (``bug`` / ``enhancement``). - Returns the new issue URL. + Returns the new issue URL, or the gate's "Not filed — missing …" with a scaffold. """ repo = resolve_repo(repo, default_repo) or "" if err := bad_repo(repo): return err + kind = (kind or "generic").strip().lower() + if kind not in ("bug", "feature", "generic"): + return f"Error: kind must be 'bug', 'feature' or 'generic' (got {kind!r})." + if miss := missing_sections(body or "", kind): + return ( + "Not filed — the issue body is missing " + "; ".join(miss) + ". " + "Add the section(s) and call again. Scaffold for a " + f"{kind} issue:\n```\n{scaffold_for(kind)}```" + ) args = ["issue", "create", "--repo", repo, "--title", title, "--body", body] - for label in labels.split(","): - label = label.strip() - if label: - args += ["--label", label] + for label in labels_for(kind, _csv(labels)): + args += ["--label", label] rc, out, serr = await run_gh(args) if gh_err := check_gh_error(rc, serr, repo=repo): return gh_err From 7b0632bf2a327207a38bf1152482e17bc09e373e Mon Sep 17 00:00:00 2001 From: Josh Mabry Date: Sat, 22 Aug 2026 17:09:43 -0700 Subject: [PATCH 2/2] fix(review): search query after --, latestReviews, total dicts(), CI-gate regex parity + label-inferred kind MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of #28, all items: 1. github_search_issues: flags first, the query LAST after a -- separator — a leading qualifier (-label:bug) was read by gh as an unknown shorthand flag (verified live, gh 2.92). argv test with -label:x. 2. github_get_pr shows latestReviews (one per reviewer, their most recent — accepted by gh 2.92) so a dozen bot COMMENTED reviews can't bury a human CHANGES_REQUESTED; an older gh without the field falls back to the NEWEST reviews, never the oldest ten. Tested both ways. 3. dicts() is total: a non-list (statusCheckRollup: 42 / reviews: 42 / comments: 42) is []. A nested-scalars row joined the no-raise sweep — and caught github_get_issue's (author or {}).get on an int; every actor read now goes through _login(). 4. Body-gate parity with protoAgent's CI issue gate (issue-gate.yml): problem += observed|symptom|idea|current behavior|\bwhat\b, repro += root cause, proposal += \bwork\b|plan. infer_kind(): a generic call with a bug / enhancement LABEL is gated as that kind (CI keys on the label) — in the tool AND the /issue command. A test pins a sample heading per regex alternative. Nits: gh_issue.py docstring says the two paths share the GATE; _bounded on github_issue_comments; test_get_pr_output_is_bounded actually reaches 12k; test_register pins the literal '15 read / 8 write / 3 review = 26'; the tool/command agreement test exercises both entry points in BOTH directions (same refusal, same gh argv head + labels when filing). Co-Authored-By: Claude Fable 5 --- PROTO.md | 19 +++++++---- gh_cli.py | 8 +++-- gh_issue.py | 42 ++++++++++++++++++----- read_tools.py | 46 +++++++++++++------------ tests/test_gh_issue.py | 51 ++++++++++++++++++++++++++++ tests/test_no_raise_sweep.py | 19 +++++++++++ tests/test_read_tools.py | 66 ++++++++++++++++++++++++++++++++++-- tests/test_register.py | 7 ++-- tests/test_write_tools.py | 63 +++++++++++++++++++++++++++------- write_tools.py | 7 ++-- 10 files changed, 270 insertions(+), 58 deletions(-) diff --git a/PROTO.md b/PROTO.md index 29cc8f9..eb00258 100644 --- a/PROTO.md +++ b/PROTO.md @@ -106,19 +106,26 @@ self-diagnosis tool the model calls when another tool errors (no `write` gate), PM verbs (v0.7.0): `github_list_prs` (reuses `api.fetch_prs`, so the tool and the board can't disagree — draft / review decision / merge state per row), `github_issue_comments` (`gh issue view --json comments`; works on PRs; newest `limit` in chronological order, -bodies ≤ 1000 chars), `github_search_issues` (`gh search issues --repo`; `state: all` = -no `--state` flag, gh only knows open|closed — documented as "dedupe before filing"). +bodies ≤ 1000 chars), `github_search_issues` (`gh search issues --repo … -- `: flags first, the query +LAST after `--` so a leading qualifier like `-label:bug` isn't read as a flag; `state: +all` = no `--state` flag, gh only knows open|closed — documented as "dedupe before +filing"). `github_get_pr` carries the merge-readiness picture: `reviewDecision`, `mergeable`, `mergeStateStatus`, `statusCheckRollup` summarised (N pass / fail / pending + the -failing names — both the CheckRun and StatusContext shapes), `reviews` (author, state, -≤ 300-char body, ≤ 10 shown), `isDraft`; total output bounded at 12k chars. +failing names — both the CheckRun and StatusContext shapes), `latestReviews` (ONE per +reviewer, their most recent — so bot COMMENTED reviews can't bury a human's +CHANGES_REQUESTED; older gh falls back to the NEWEST `reviews`), `isDraft`; total output +bounded at 12k chars (`github_issue_comments` too). **Write (gated on `github.write`)** — `github_create_issue` (**body-gated**: the SAME `missing_sections` gate the `/issue` command enforces — a thin body, or a `bug` without repro / a `feature` without a direction-or-acceptance section, is refused with the scaffold and never posted; `kind` -also adds the type label via `labels_for`) / `github_comment` / `github_create_pr` -(return the new URL), +also adds the type label via `labels_for`, and a `generic` call whose labels carry +`bug` / `enhancement` is gated as that kind because protoAgent's CI issue gate keys on +the label. The section regexes are in LOCKSTEP with `.github/workflows/issue-gate.yml` +in protoAgent — change both, `test_section_regexes_match_protoagents_ci_gate` pins a +sample per alternative) / `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` diff --git a/gh_cli.py b/gh_cli.py index 90cc473..3c10b21 100644 --- a/gh_cli.py +++ b/gh_cli.py @@ -234,8 +234,12 @@ def parse_json(out: str, expect: type | tuple[type, ...] = dict) -> tuple[Any, s def dicts(items) -> list[dict]: - """Only the dict rows of a `gh --json` list — nulls/scalars in a row list are skipped.""" - return [x for x in (items or []) if isinstance(x, dict)] + """Only the dict rows of a `gh --json` list — nulls/scalars in a row list are skipped, + and a value that isn't a list at all (a nested scalar like ``"reviews": 42``) is ``[]``. + Total by design: a tool feeds it any nested field without a type check of its own.""" + if not isinstance(items, list): + return [] + return [x for x in items if isinstance(x, dict)] # ── error classification ────────────────────────────────────────────────────────── diff --git a/gh_issue.py b/gh_issue.py index 277aa8d..4161434 100644 --- a/gh_issue.py +++ b/gh_issue.py @@ -5,9 +5,11 @@ so a person can file from the composer on any agent — including a read-only one — without the model being involved. The `github_create_issue` AGENT tool exists too (write_tools.py), behind the per-agent `github.write` gate: a PM/coding agent with -write on can file autonomously; a research agent without it cannot. Both paths share -`file_issue` below, so the gate check and the `gh issue create` argv can't diverge. -This module is the pure, host-free logic. +write on can file autonomously; a research agent without it cannot. Both paths run +the SAME body gate (`missing_sections` / `infer_kind` / `labels_for` below), so what +the command refuses the tool refuses — and what passes here passes protoAgent's CI +issue gate too (the regexes are kept in lockstep with it). This module is the pure, +host-free logic. `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 @@ -36,12 +38,19 @@ log = logging.getLogger("protoagent.plugins.github") -# 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 detectors — kept in LOCKSTEP with protoAgent's CI issue gate +# (.github/workflows/issue-gate.yml: hasProblem / hasRepro / hasProposal / hasAcceptance) +# so the local check and the server-side gate can never disagree about what +# "conforms" means: a body this gate passes must pass CI, and vice versa. When the +# host's regexes change, change these — tests/test_gh_issue.py pins a sample per +# alternative. _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), + "problem": re.compile( + r"problem|what'?s? wrong|motivation|background|context|summary|observed|symptom|idea|current behavior|\bwhat\b", + re.I, + ), + "repro": re.compile(r"repro|reproduce|steps|evidence|expected|actual|observed|symptom|root cause", re.I), + "proposal": re.compile(r"propos|solution|approach|direction|fix|design|\bwork\b|plan", 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. @@ -108,6 +117,22 @@ def missing_sections(body: str, kind: str) -> list[str]: return miss +def infer_kind(kind: str, labels: list[str] | None = None) -> str: + """The gate ``kind`` to enforce: an explicit ``bug`` / ``feature`` wins; a + ``generic`` request carrying a ``bug`` / ``enhancement`` LABEL is promoted to + that kind — because the CI gate keys on the labels, so a ``labels="bug"`` issue + filed without a repro section would be refused server-side anyway.""" + k = (kind or "generic").strip().lower() + if k in ("bug", "feature"): + return k + low = {(lbl or "").strip().lower() for lbl in (labels or [])} + if "bug" in low: + return "bug" + if "enhancement" in low: + return "feature" + return "generic" + + 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.""" @@ -274,6 +299,7 @@ def _parse(rest: str, *, default_repo: str = "") -> IssueRequest | str: i += 1 title = " ".join(title_parts).strip() + kind = infer_kind(kind, labels) # `--label bug` demands the repro section, like CI labels = labels_for(kind, labels) explicit_repo = repo repo = resolve_repo(repo, default_repo) diff --git a/read_tools.py b/read_tools.py index cfe0f24..f6ec075 100644 --- a/read_tools.py +++ b/read_tools.py @@ -44,6 +44,11 @@ _CHECK_SKIP = {"SKIPPED", "NEUTRAL", "STALE"} +def _login(actor) -> str: + """``author.login`` from a gh actor object — ``?`` for a missing/null/scalar actor.""" + return str(actor.get("login") or "?") if isinstance(actor, dict) else "?" + + def _one_line(text: str, cap: int) -> str: """Collapse whitespace and cap — for a review/comment excerpt on one line.""" t = " ".join((text or "").split()) @@ -130,7 +135,7 @@ async def github_get_pr(number: int, repo: str = "") -> str: repo, "--json", "number,title,state,isDraft,author,body,additions,deletions,files,url,headRefName,baseRefName," - "reviewDecision,mergeable,mergeStateStatus,statusCheckRollup,reviews", + "reviewDecision,mergeable,mergeStateStatus,statusCheckRollup,reviews,latestReviews", ] ) if gh_err := check_gh_error(rc, serr, repo=repo): @@ -142,18 +147,23 @@ async def github_get_pr(number: int, repo: str = "") -> str: draft = " (DRAFT)" if d.get("isDraft") else "" checks = _summarize_checks(d.get("statusCheckRollup")) reviews = dicts(d.get("reviews")) + # `latestReviews` = ONE review per reviewer, their most recent — the set that + # actually decides reviewDecision, so a dozen bot COMMENTED reviews can never + # push a human's CHANGES_REQUESTED out of view. Older gh without the field (or + # a PR with none) falls back to the NEWEST `reviews`, never the oldest. + latest = dicts(d.get("latestReviews")) or reviews[-_MAX_REVIEWS:] review_lines = [ - f" - {str((r.get('author') or {}).get('login') or '?') if isinstance(r.get('author'), dict) else '?'} " + f" - {_login(r.get('author'))} " f"[{r.get('state') or '?'}]" + (f": {_one_line(str(r.get('body') or ''), 300)}" if str(r.get("body") or "").strip() else "") - for r in reviews[:_MAX_REVIEWS] + for r in latest[-_MAX_REVIEWS:] ] - if len(reviews) > _MAX_REVIEWS: - review_lines.append(f" … {len(reviews) - _MAX_REVIEWS} more review(s)") + if len(reviews) > len(review_lines): + review_lines.append(f" … {len(reviews) - len(review_lines)} more review(s) (older / superseded)") text = ( f"PR #{d.get('number')} [{d.get('state')}]{draft} {d.get('title')}\n" f"branch: {d.get('headRefName', '?')} -> {d.get('baseRefName', '?')}\n" - f"by {(d.get('author') or {}).get('login', '?') if isinstance(d.get('author'), dict) else '?'} | " + f"by {_login(d.get('author'))} | " f"+{d.get('additions', 0)}/-{d.get('deletions', 0)} | {d.get('url')}\n" f"review decision: {d.get('reviewDecision') or 'none yet'} | " f"mergeable: {d.get('mergeable') or '?'} | merge state: {d.get('mergeStateStatus') or '?'}\n" @@ -185,7 +195,7 @@ async def github_get_issue(number: int, repo: str = "") -> str: labels = ", ".join(str(lbl.get("name", "")) for lbl in dicts(d.get("labels"))) return ( f"Issue #{d.get('number')} [{d.get('state')}] {d.get('title')}\n" - f"by {(d.get('author') or {}).get('login', '?')} | labels: {labels or '(none)'} | " + f"by {_login(d.get('author'))} | labels: {labels or '(none)'} | " f"{d.get('url')}\n\n{(d.get('body') or '').strip()[:2000]}" ) @@ -533,7 +543,7 @@ async def github_list_prs(repo: str = "", state: str = "open", limit: int = 30) return f"No {state} pull requests in {repo}." lines = [f"{len(items)} {state} pull request(s) in {repo}:"] for it in items: - author = (it.get("author") or {}).get("login", "?") if isinstance(it.get("author"), dict) else "?" + author = _login(it.get("author")) flags = [ x for x in (("draft" if it.get("isDraft") else ""), it.get("reviewDecision"), it.get("mergeStateStatus")) @@ -580,12 +590,12 @@ async def github_issue_comments(number: int, repo: str = "", limit: int = 30) -> ) lines = [head + ":"] for c in shown: - author = (c.get("author") or {}).get("login", "?") if isinstance(c.get("author"), dict) else "?" + author = _login(c.get("author")) body = " ".join(str(c.get("body") or "").split()) if len(body) > _MAX_COMMENT_CHARS: body = body[: _MAX_COMMENT_CHARS - 1] + "…" lines.append(f"--- {author} · {c.get('createdAt') or '?'}\n{body or '(empty)'}") - return "\n".join(lines) + return _bounded("\n".join(lines), _MAX_PR_CHARS) @tool async def github_search_issues(query: str, repo: str = "", state: str = "open", limit: int = 20) -> str: @@ -610,19 +620,13 @@ async def github_search_issues(query: str, repo: str = "", state: str = "open", capped = max(1, min(int(limit), 100)) except (TypeError, ValueError): capped = 20 - args = [ - "search", - "issues", - "--repo", - repo, - query.strip(), - "--limit", - str(capped), - "--json", - "number,title,state,url", - ] + # Flags first, the query LAST after `--`: a query starting with a qualifier + # like `-label:bug` is otherwise parsed as an unknown shorthand flag (verified + # live against gh 2.92 — with `--` it works). + args = ["search", "issues", "--repo", repo, "--limit", str(capped), "--json", "number,title,state,url"] if state != "all": # gh's --state is open|closed only; "all" = no filter args += ["--state", state] + args += ["--", query.strip()] rc, out, serr = await run_gh(args) if gh_err := check_gh_error(rc, serr, repo=repo): return gh_err diff --git a/tests/test_gh_issue.py b/tests/test_gh_issue.py index 602fe5c..eca332d 100644 --- a/tests/test_gh_issue.py +++ b/tests/test_gh_issue.py @@ -8,6 +8,7 @@ from unittest.mock import AsyncMock, patch +import pytest from ghplugin.gh_issue import ( current_default, default_repo_error, @@ -36,6 +37,56 @@ def test_missing_sections_by_kind(): assert "a Proposed-direction or Acceptance section" in missing_sections(feat, "feature") +@pytest.mark.parametrize( + "key,heading", + [ + ("problem", "Observed"), + ("problem", "Symptom"), + ("problem", "Idea"), + ("problem", "Current behavior"), + ("problem", "What happens"), + ("repro", "Root cause"), + ("repro", "Symptom"), + ("proposal", "Proposed work"), + ("proposal", "Plan"), + ], +) +def test_section_regexes_match_protoagents_ci_gate(key, heading): + """Lockstep with .github/workflows/issue-gate.yml — a heading CI accepts must be + accepted here (else the tool refuses a body CI would pass).""" + from ghplugin.gh_issue import _has_section + + assert _has_section(f"## {heading}\nbody", key) + assert _has_section(f"**{heading}**\nbody", key) # the bold-line form too + + +def test_whatever_does_not_match_the_word_boundary_what(): + from ghplugin.gh_issue import _has_section + + assert not _has_section("## Whatever\nbody", "problem") + assert not _has_section("## Network\nbody", "proposal") # \bwork\b, not "network" + + +def test_infer_kind_promotes_generic_by_label(): + from ghplugin.gh_issue import infer_kind + + assert infer_kind("generic", ["bug"]) == "bug" + assert infer_kind("generic", ["Enhancement", "p1"]) == "feature" + assert infer_kind("generic", ["p1"]) == "generic" + assert infer_kind("feature", ["bug"]) == "feature" # explicit wins + assert infer_kind("", None) == "generic" + + +async def test_issue_command_label_bug_demands_repro(monkeypatch): + """`/issue t --label bug` without --bug is still a bug for the gate (CI keys on the label).""" + fake = AsyncMock() + body = "## Problem\nThe parser crashes on empty input and we should handle it gracefully across the whole pipeline." + with patch("ghplugin.gh_issue.run_gh", fake): + out = await run_issue_command(f"Crash --label bug --repo o/n\n{body}", default_repo="") + assert out.startswith("Not filed") and "Steps to reproduce" in out + fake.assert_not_called() + + def test_labels_for_prepends_type_label(): assert labels_for("bug", ["p0"]) == ["bug", "p0"] assert labels_for("feature") == ["enhancement"] diff --git a/tests/test_no_raise_sweep.py b/tests/test_no_raise_sweep.py index 7fd1c3f..06b8f58 100644 --- a/tests/test_no_raise_sweep.py +++ b/tests/test_no_raise_sweep.py @@ -65,6 +65,25 @@ ), "list": (0, json.dumps([{"type": "file", "name": "x", "path": "x", "size": 1}, "not-a-dict", 7]), ""), "list-of-nulls": (0, "[null, null]", ""), + # every nested field a tool reads is a SCALAR — dicts()/parse_json must be total + "nested-scalars": ( + 0, + json.dumps( + { + "files": 7, + "labels": 1, + "statusCheckRollup": 42, + "reviews": 42, + "latestReviews": 0, + "comments": 42, + "author": 3, + "head": 5, + "hosts": 1, + "check_runs": 2, + } + ), + "", + ), "empty": (0, "", ""), "garbage": (0, "<<>> \x00\xff", ""), "number": (0, "42", ""), diff --git a/tests/test_read_tools.py b/tests/test_read_tools.py index 0970b2d..4987bbc 100644 --- a/tests/test_read_tools.py +++ b/tests/test_read_tools.py @@ -557,16 +557,63 @@ async def test_get_pr_with_no_checks_or_reviews_says_so(): @pytest.mark.asyncio async def test_get_pr_output_is_bounded(): + """A huge body + 40 long reviews + long file paths must end at the bound — and the + bound must actually be hit: the body is capped at 2000, reviews at 10 × 300, files + at 20 entries, so only the (uncapped) path lengths can carry this past 12k.""" from ghplugin.read_tools import _MAX_PR_CHARS d = { **json.loads(_RICH_PR_JSON), "body": "b" * 50000, - "reviews": [{"author": {"login": "r"}, "state": "COMMENTED", "body": "y" * 2000}] * 40, + "files": [{"path": "p" * 700 + "/module_%02d.py" % i} for i in range(20)], + "reviews": [{"author": {"login": f"r{i}"}, "state": "COMMENTED", "body": "y" * 2000} for i in range(40)], + "latestReviews": [{"author": {"login": f"r{i}"}, "state": "COMMENTED", "body": "y" * 2000} for i in range(40)], } with patch("ghplugin.read_tools.run_gh", new=AsyncMock(return_value=(0, json.dumps(d), ""))): out = await _get_pr_tool().ainvoke({"repo": "owner/name", "number": 1}) - assert len(out) <= _MAX_PR_CHARS + 60 and "… 30 more review(s)" in out + assert out.endswith(f"… (truncated at {_MAX_PR_CHARS} chars)") + assert len(out) == _MAX_PR_CHARS + len(f"\n… (truncated at {_MAX_PR_CHARS} chars)") + assert "… 30 more review(s)" in out + + +@pytest.mark.asyncio +async def test_get_pr_shows_latest_review_per_reviewer_not_the_oldest_ten(): + """11 bot COMMENTED reviews must not push a human's CHANGES_REQUESTED out of view: + `latestReviews` (one per reviewer) is what's shown; without it, the NEWEST reviews.""" + bots = [{"author": {"login": "coderabbitai"}, "state": "COMMENTED", "body": f"nit {i}"} for i in range(11)] + human = {"author": {"login": "kj"}, "state": "CHANGES_REQUESTED", "body": "blocker"} + d = {**json.loads(_PR_JSON), "reviews": bots + [human], "latestReviews": [bots[-1], human]} + with patch("ghplugin.read_tools.run_gh", new=AsyncMock(return_value=(0, json.dumps(d), ""))): + out = await _get_pr_tool().ainvoke({"repo": "owner/name", "number": 1}) + assert " - kj [CHANGES_REQUESTED]: blocker" in out and "nit 10" in out and "nit 0" not in out + assert "reviews (12):" in out and "… 10 more review(s) (older / superseded)" in out + # older gh (no latestReviews): the newest N, never the oldest N + d.pop("latestReviews") + with patch("ghplugin.read_tools.run_gh", new=AsyncMock(return_value=(0, json.dumps(d), ""))): + out = await _get_pr_tool().ainvoke({"repo": "owner/name", "number": 1}) + assert "kj [CHANGES_REQUESTED]" in out and "nit 0" not in out and "nit 1]" not in out + + +@pytest.mark.asyncio +async def test_get_pr_and_comments_tolerate_nested_scalars(): + """`statusCheckRollup: 42` / `reviews: 42` / `comments: 42` — dicts() is total.""" + d = {**json.loads(_PR_JSON), "statusCheckRollup": 42, "reviews": 42, "latestReviews": "x", "files": 7} + with patch("ghplugin.read_tools.run_gh", new=AsyncMock(return_value=(0, json.dumps(d), ""))): + out = await _get_pr_tool().ainvoke({"repo": "owner/name", "number": 1}) + assert "checks: none" in out and "reviews (0): none" in out and "files: (none)" in out + with patch("ghplugin.read_tools.run_gh", new=AsyncMock(return_value=(0, '{"comments": 42}', ""))): + out = await _tool("github_issue_comments").ainvoke({"repo": "owner/name", "number": 1}) + assert out == "No comments on owner/name#1." + + +@pytest.mark.asyncio +async def test_issue_comments_output_is_bounded(): + from ghplugin.read_tools import _MAX_PR_CHARS + + comments = [{"author": {"login": "u"}, "createdAt": "d", "body": "z" * 999} for _ in range(30)] + with patch("ghplugin.read_tools.run_gh", new=AsyncMock(return_value=(0, json.dumps({"comments": comments}), ""))): + out = await _tool("github_issue_comments").ainvoke({"repo": "owner/name", "number": 1, "limit": 30}) + assert out.endswith(f"… (truncated at {_MAX_PR_CHARS} chars)") def test_summarize_checks_shapes(): @@ -688,7 +735,8 @@ async def test_search_issues_argv_and_render(): {"repo": "owner/name", "query": " default_repo typo ", "state": "closed", "limit": 5} ) argv = mock.call_args.args[0] - assert argv[:4] == ["search", "issues", "--repo", "owner/name"] and argv[4] == "default_repo typo" + assert argv[:4] == ["search", "issues", "--repo", "owner/name"] + assert argv[-2:] == ["--", "default_repo typo"] # the query LAST, after the separator assert argv[argv.index("--limit") + 1] == "5" and argv[argv.index("--state") + 1] == "closed" assert ( out @@ -711,6 +759,18 @@ async def test_search_issues_all_state_omits_the_flag_and_validates(): mock.assert_called_once() +@pytest.mark.asyncio +async def test_search_issues_query_starting_with_a_qualifier_is_not_a_flag(): + """`-label:bug` as the first token was parsed by gh as an unknown shorthand flag; + with flags first and `--` before the query it's a query (verified live, gh 2.92).""" + mock = AsyncMock(return_value=(0, "[]", "")) + with patch("ghplugin.read_tools.run_gh", mock): + await _tool("github_search_issues").ainvoke({"repo": "owner/name", "query": "-label:bug crash"}) + argv = mock.call_args.args[0] + assert argv[-2:] == ["--", "-label:bug crash"] + assert all(not a.startswith("-label") for a in argv[: argv.index("--")]) # never before the separator + + def test_search_issues_description_says_dedupe_before_filing(): assert "DEDUPE BEFORE FILING" in _tool("github_search_issues").description assert ( diff --git a/tests/test_register.py b/tests/test_register.py index 60c22bf..a7a7a87 100644 --- a/tests/test_register.py +++ b/tests/test_register.py @@ -194,6 +194,7 @@ def test_inventory_counts_match_the_docs(make_registry): review = {"github_review_comment", "github_review_approve", "github_review_request_changes"} assert len(READ_TOOLS) == 15 and len(WRITE_TOOLS) == 8 and len(review) == 3 assert names == READ_TOOLS | WRITE_TOOLS | review and len(names) == 26 - for doc in ("README.md", "PROTO.md"): - text = (Path(__file__).resolve().parent.parent / doc).read_text() - assert "15" in text and ("= 26" in text or "26 tools" in text), f"{doc} inventory stale" + root = Path(__file__).resolve().parent.parent + assert "15 read / 8 write / 3 review = 26" in (root / "PROTO.md").read_text(), "PROTO.md inventory stale" + readme = (root / "README.md").read_text() + assert "**Read** (always, 15)" in readme and "26 tools in all" in readme, "README inventory stale" diff --git a/tests/test_write_tools.py b/tests/test_write_tools.py index b55bdeb..da7b434 100644 --- a/tests/test_write_tools.py +++ b/tests/test_write_tools.py @@ -57,8 +57,8 @@ async def test_labels_split_into_separate_flags(): tool = _create_issue() fake = AsyncMock(return_value=(0, "https://github.com/o/n/issues/7", "")) with patch("ghplugin.write_tools.run_gh", fake): - await tool.ainvoke({"repo": "o/n", "title": "t", "body": _OK_BODY, "labels": "bug, enhancement ,"}) - assert _labels_in(fake.call_args.args[0]) == ["bug", "enhancement"] + await tool.ainvoke({"repo": "o/n", "title": "t", "body": _OK_BODY, "labels": "p0, needs-triage ,"}) + assert _labels_in(fake.call_args.args[0]) == ["p0", "needs-triage"] async def test_bad_repo_short_circuits(): @@ -135,18 +135,55 @@ async def test_create_issue_rejects_an_unknown_kind(): assert out.startswith("Error: kind must be") -async def test_create_issue_gate_matches_the_issue_command(): - """One gate, two entry points: whatever /issue refuses, the tool refuses, and vice versa.""" - from ghplugin.gh_issue import missing_sections - - for kind, body in (("generic", "too short"), ("bug", _OK_BODY), ("feature", _OK_BODY), ("generic", _OK_BODY)): - tool_out = ( - await _create_issue().ainvoke({"repo": "o/n", "title": "t", "body": body, "kind": kind}) - if missing_sections(body, kind) - else None +async def test_create_issue_generic_with_a_bug_label_demands_repro_like_ci(): + """protoAgent's CI issue gate keys on the LABEL: a `labels="bug"` issue without a repro + section would be flagged server-side, so a generic call with that label is gated as + a bug here (and `enhancement` as a feature).""" + tool = _create_issue() + fake = AsyncMock(return_value=(0, "https://github.com/o/n/issues/11", "")) + with patch("ghplugin.write_tools.run_gh", fake): + out = await tool.ainvoke({"repo": "o/n", "title": "t", "body": _OK_BODY, "labels": "bug"}) + assert out.startswith("Not filed") and "Steps to reproduce" in out + fake.assert_not_called() + with patch("ghplugin.write_tools.run_gh", fake): + out = await tool.ainvoke( + {"repo": "o/n", "title": "t", "body": _OK_BODY + "\n## Root cause\nOff by one.", "labels": "bug"} ) - if missing_sections(body, kind): - assert tool_out.startswith("Not filed") + assert out.endswith("/issues/11") and _labels_in(fake.call_args.args[0]) == ["bug"] # not doubled + # `enhancement` → feature; _OK_BODY has an Acceptance section, so it passes + with patch("ghplugin.write_tools.run_gh", fake): + out = await tool.ainvoke({"repo": "o/n", "title": "t", "body": _OK_BODY, "labels": "enhancement, p1"}) + assert out.endswith("/issues/11") and _labels_in(fake.call_args.args[0]) == ["enhancement", "p1"] + + +async def test_create_issue_gate_matches_the_issue_command(): + """One gate, two entry points: for every (kind, body) the tool and the /issue command + BOTH refuse or BOTH file — checked in both directions against the same gh stub.""" + from ghplugin.gh_issue import run_issue_command + + cases = [ + ("generic", "too short", False), + ("generic", _OK_BODY, True), + ("bug", _OK_BODY, False), # no repro + ("bug", _OK_BODY + "\n## Steps to reproduce\nRun it.", True), + ("feature", "## Motivation\nWe need this capability badly for the next release cycle to ship on time.", False), + ("feature", _OK_BODY, True), # Acceptance satisfies a feature + ] + for kind, body, should_file in cases: + tool_gh = AsyncMock(return_value=(0, "https://github.com/o/n/issues/1", "")) + cmd_gh = AsyncMock(return_value=(0, "https://github.com/o/n/issues/1", "")) + with patch("ghplugin.write_tools.run_gh", tool_gh): + tool_out = await _create_issue().ainvoke({"repo": "o/n", "title": "t", "body": body, "kind": kind}) + flag = {"bug": " --bug", "feature": " --feature"}.get(kind, "") + with patch("ghplugin.gh_issue.run_gh", cmd_gh): + cmd_out = await run_issue_command(f"t{flag} --repo o/n\n{body}", default_repo="") + assert (tool_gh.called, cmd_gh.called) == (should_file, should_file), (kind, body[:30]) + if should_file: + assert tool_out.endswith("/issues/1") and cmd_out.endswith("/issues/1") + assert tool_gh.call_args.args[0][:6] == cmd_gh.call_args.args[0][:6] # same gh argv head + assert _labels_in(tool_gh.call_args.args[0]) == _labels_in(cmd_gh.call_args.args[0]) + else: + assert tool_out.startswith("Not filed") and cmd_out.startswith("Not filed") def _comment(): diff --git a/write_tools.py b/write_tools.py index 21257f0..cf49322 100644 --- a/write_tools.py +++ b/write_tools.py @@ -31,7 +31,7 @@ from langchain_core.tools import tool from .gh_cli import bad_repo, check_gh_error, run_gh -from .gh_issue import labels_for, missing_sections, resolve_repo, scaffold_for +from .gh_issue import infer_kind, labels_for, missing_sections, resolve_repo, scaffold_for def _csv(value: str) -> list[str]: @@ -81,7 +81,9 @@ async def github_create_issue( body: Issue body (Markdown) — use headings for the sections above. labels: Optional comma-separated label names. kind: ``generic`` (default) | ``bug`` | ``feature`` — picks the gate's required - sections and adds the type label (``bug`` / ``enhancement``). + sections and adds the type label (``bug`` / ``enhancement``). A ``generic`` + call whose ``labels`` include ``bug`` / ``enhancement`` is treated as that + kind (the repo's CI gate keys on the label). Returns the new issue URL, or the gate's "Not filed — missing …" with a scaffold. """ @@ -91,6 +93,7 @@ async def github_create_issue( kind = (kind or "generic").strip().lower() if kind not in ("bug", "feature", "generic"): return f"Error: kind must be 'bug', 'feature' or 'generic' (got {kind!r})." + kind = infer_kind(kind, _csv(labels)) if miss := missing_sections(body or "", kind): return ( "Not filed — the issue body is missing " + "; ".join(miss) + ". "