feat(review-tools): formal verdict surface with below-the-model guards (v0.2.0)

__init__.py

@@ -55,9 +55,10 @@ def register(registry) -> None:
     n_write = 0
     if write_enabled:
         try:
+            from .review_tools import get_review_tools
             from .write_tools import get_write_tools
 
-            write = get_write_tools(default_repo)
+            write = get_write_tools(default_repo) + get_review_tools(default_repo)
             for t in write:
                 registry.register_tool(t)
             n_write = len(write)

protoagent.plugin.yaml

@@ -2,7 +2,7 @@
 # Keep `version` in lockstep with pyproject.toml (tests/test_version.py asserts it).
 id: github
 name: GitHub (read/write tools)
-version: 0.1.6
+version: 0.2.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

pyproject.toml

@@ -1,6 +1,6 @@
 [project]
 name = "github-plugin"
-version = "0.1.6"
+version = "0.2.0"
 description = "Read/write GitHub tools for protoAgent over the gh CLI, with per-agent write gating."
 requires-python = ">=3.11"
 

read_tools.py

@@ -286,6 +286,47 @@ async def github_read_file(path: str, repo: str = "", ref: str = "") -> str:
             out = out[:20000] + "\n… (truncated at 20000 chars)"
         return out
 
+    @tool
+    async def github_path_exists(path: str, repo: str = "", ref: str = "") -> str:
+        """Check whether a path exists in a GitHub repo — the grounding probe for
+        review claims about external references.
+
+        A diff is often only correct if something OUTSIDE it exists (a cross-repo
+        `COPY --from` path, a workspace package, an action ref). The answer is
+        authoritative: EXISTS means the assumption holds (don't invent a problem
+        around it); MISSING means the dependency is really absent (that's the
+        finding). A tool error means UNVERIFIED — report a Gap, never a severity.
+
+        Args:
+            repo: Repository as ``owner/name`` — may be a DIFFERENT repo than the
+                PR under review (that is the point). Omit for the default repo.
+            path: Repo-relative path to check (file or directory).
+            ref: Optional branch / tag / SHA (default: the repo's default branch).
+        """
+        repo = resolve_repo(repo, default_repo) or ""
+        if err := bad_repo(repo):
+            return err
+        if not path.strip():
+            return "Error: `path` is empty."
+        clean = path.strip().strip("/")
+        args = ["api", f"repos/{repo}/contents/{clean}"]
+        if ref.strip():
+            args += ["-f", f"ref={ref.strip()}"]
+        rc, out, serr = await run_gh(args)
+        if rc == 0:
+            return f"EXISTS: {repo}/{clean}" + (f" @ {ref.strip()}" if ref.strip() else "")
+        blob = (serr or out or "").lower()
+        if "404" in blob or "not found" in blob:
+            return (
+                f"MISSING: {repo}/{clean}"
+                + (f" @ {ref.strip()}" if ref.strip() else "")
+                + " — the path does not exist."
+            )
+        return (
+            check_gh_error(rc, serr)
+            or f"Error (gh exit {rc}): could not verify {repo}/{clean} — treat as UNVERIFIED (a Gap, not a finding)."
+        )
+
     @tool
     async def github_repo_contents(repo: str = "", path: str = "", ref: str = "") -> str:
         """List the contents (files + dirs) of a path in a GitHub repo.
@@ -325,6 +366,7 @@ async def github_repo_contents(repo: str = "", path: str = "", ref: str = "") ->
         github_list_issues,
         github_get_commit_diff,
         github_pr_diff,
+        github_path_exists,
         github_ci_runs,
         github_run_failure,
         github_read_file,

review_tools.py

@@ -0,0 +1,219 @@
+"""Formal PR-review verdict tools over `gh` — GATED behind `github.write: true`.
+
+The verdict surface for the QA-review takeover (protoAgent ADR 0078 Phase B):
+three tools posting formal reviews via the GitHub Review API, with the guards
+that made Quinn's reviewer reliable enforced INSIDE the tools — below the model,
+so a prompt can never bypass them:
+
+  - github_review_comment          — non-blocking COMMENT review. No guards: a
+    comment is always safe to post.
+  - github_review_approve          — formal APPROVE. Guarded.
+  - github_review_request_changes  — formal REQUEST_CHANGES. Guarded.
+
+The guards (ported from protoWorkstacean's pr-inspector, her failure ledger):
+
+  CI-TERMINAL (#863) — a blocking verdict locks in a judgement about the PR's
+  settled state, so APPROVE / REQUEST_CHANGES are refused while any check run
+  is still queued/in-progress, AND when CI cannot be read at all (a 403/error
+  says nothing about whether checks pass — fail closed, never fail open). The
+  refusal tells the model to post a review_comment instead. A repo with no
+  checks at all is terminal by definition.
+
+  SELF-REVIEW — the tool refuses a blocking verdict on a PR authored by the
+  SAME identity the token authenticates as (approve-your-own-work loops).
+  Commenting on your own PR stays allowed.
+
+Keep tool docstrings PLAIN string literals (an f-string docstring → __doc__ is
+None → the tool ships with no description).
+"""
+
+from __future__ import annotations
+
+import json
+
+from langchain_core.tools import tool
+
+from .gh_cli import bad_repo, check_gh_error, run_gh
+from .gh_issue import resolve_repo
+
+# Check-run states that count as still-running. GitHub check runs carry
+# `status` (queued | in_progress | completed | waiting | requested | pending)
+# and only a completed run has a `conclusion`.
+_NON_TERMINAL = {"queued", "in_progress", "waiting", "requested", "pending"}
+
+
+async def _viewer_login() -> str:
+    """The login the token authenticates as ('' on failure — guards fail closed
+    on their own signal, not on this helper)."""
+    rc, out, _err = await run_gh(["api", "user", "--jq", ".login"])
+    return out.strip() if rc == 0 else ""
+
+
+async def _pr_head_and_author(repo: str, number: int) -> tuple[str, str, str]:
+    """(head_sha, author_login, error). error is '' on success."""
+    rc, out, serr = await run_gh(["pr", "view", str(number), "--repo", repo, "--json", "headRefOid,author"])
+    if gh_err := check_gh_error(rc, serr):
+        return "", "", gh_err
+    try:
+        d = json.loads(out)
+    except json.JSONDecodeError:
+        return "", "", f"Error: could not parse gh output: {out[:200]}"
+    return str(d.get("headRefOid") or ""), str((d.get("author") or {}).get("login") or ""), ""
+
+
+async def _ci_state(repo: str, head_sha: str) -> tuple[str, str]:
+    """('terminal' | 'pending' | 'unknown', detail).
+
+    'terminal'  — every check run completed (or the commit has none at all).
+    'pending'   — at least one run is queued/in-progress.
+    'unknown'   — CI could not be read (403, network, parse) — treated exactly
+                  like pending by the guards: you cannot verdict what you
+                  cannot see (fail closed)."""
+    rc, out, serr = await run_gh(
+        ["api", f"repos/{repo}/commits/{head_sha}/check-runs", "--jq", "[.check_runs[].status]"]
+    )
+    if rc != 0:
+        return "unknown", (serr or out).strip()[:200]
+    try:
+        statuses = json.loads(out or "[]")
+    except json.JSONDecodeError:
+        return "unknown", f"unparseable check-runs response: {out[:120]}"
+    pending = [s for s in statuses if str(s).lower() in _NON_TERMINAL]
+    if pending:
+        return "pending", f"{len(pending)} of {len(statuses)} check run(s) still running"
+    return "terminal", f"{len(statuses)} check run(s), all completed"
+
+
+async def _post_review(repo: str, number: int, event: str, body: str) -> str:
+    rc, out, serr = await run_gh(
+        [
+            "api",
+            f"repos/{repo}/pulls/{number}/reviews",
+            "-X",
+            "POST",
+            "-f",
+            f"event={event}",
+            "-f",
+            f"body={body}",
+        ]
+    )
+    if gh_err := check_gh_error(rc, serr):
+        return gh_err
+    try:
+        d = json.loads(out)
+        url = d.get("html_url") or ""
+    except json.JSONDecodeError:
+        url = ""
+    return f"Posted {event} review on {repo}#{number}." + (f" {url}" if url else "")
+
+
+async def _guard_blocking_verdict(repo: str, number: int, verdict: str) -> str:
+    """The two below-the-model guards. Returns '' to allow, or the refusal text.
+
+    Order matters: the cheap identity guard first, then CI. Both refusals
+    instruct the model to use github_review_comment instead — the non-blocking
+    outcome is always available."""
+    head_sha, author, err = await _pr_head_and_author(repo, number)
+    if err:
+        return (
+            f"{err}\nHeld: cannot verify the PR's head/author, so a formal {verdict} "
+            "is refused (fail closed). Post your findings with github_review_comment instead."
+        )
+    me = await _viewer_login()
+    if me and author and me == author:
+        return (
+            f"Held: this PR was authored by {author!r} — the same identity this agent "
+            f"reviews as. A formal {verdict} on your own work is refused (self-approval "
+            "loops). Post observations with github_review_comment and escalate to a "
+            "human reviewer."
+        )
+    state, detail = await _ci_state(repo, head_sha)
+    if state == "pending":
+        return (
+            f"Held: CI is not terminal on {repo}#{number} ({detail}). A formal {verdict} "
+            "locks in a judgement about the PR's settled state, so it is refused while "
+            "checks run. Post your findings NOW with github_review_comment (non-blocking) "
+            "and finish — do not wait or poll; once checks settle, a later pass (or a "
+            "deterministic approve-on-green policy) takes it from there."
+        )
+    if state == "unknown":
+        return (
+            f"Held: CI on {repo}#{number} cannot be read ({detail}) — which says nothing "
+            f"about whether checks pass. A formal {verdict} on unverified CI is refused "
+            "(fail closed). Record the CI-access gap with github_review_comment "
+            "(non-blocking); do not treat unreadable CI as a failure."
+        )
+    return ""
+
+
+def get_review_tools(default_repo: str = "") -> list:
+    """Build the verdict tools. ``default_repo`` (``owner/name``) is used whenever a
+    tool's ``repo`` arg is omitted."""
+
+    @tool
+    async def github_review_comment(number: int, body: str, repo: str = "") -> str:
+        """Post a formal non-blocking COMMENT review on a pull request.
+
+        The always-safe verdict: use it for findings while CI is still running, for
+        observations that shouldn't block a merge, or when a blocking verdict was
+        refused by its guards.
+
+        Args:
+            repo: Repository as ``owner/name``. Omit to use the agent's configured default repo.
+            number: PR number.
+            body: The review body (findings, observations — markdown).
+        """
+        repo = resolve_repo(repo, default_repo) or ""
+        if err := bad_repo(repo):
+            return err
+        if not body.strip():
+            return "Error: `body` is empty — a review needs content."
+        return await _post_review(repo, number, "COMMENT", body)
+
+    @tool
+    async def github_review_approve(number: int, body: str, repo: str = "") -> str:
+        """Post a formal APPROVE review on a pull request. GUARDED.
+
+        Refused (with comment-instead guidance) while any check run is still
+        running, when CI cannot be read at all, or on a PR this agent authored.
+        An APPROVE clears the way to merge — it must reflect the PR's settled,
+        verified state, never a guess.
+
+        Args:
+            repo: Repository as ``owner/name``. Omit to use the agent's configured default repo.
+            number: PR number.
+            body: The review body (what was verified, any non-blocking notes).
+        """
+        repo = resolve_repo(repo, default_repo) or ""
+        if err := bad_repo(repo):
+            return err
+        if not body.strip():
+            return "Error: `body` is empty — a review needs content."
+        if held := await _guard_blocking_verdict(repo, number, "APPROVE"):
+            return held
+        return await _post_review(repo, number, "APPROVE", body)
+
+    @tool
+    async def github_review_request_changes(number: int, body: str, repo: str = "") -> str:
+        """Post a formal REQUEST_CHANGES review on a pull request. GUARDED.
+
+        Refused (with comment-instead guidance) while any check run is still
+        running, when CI cannot be read at all, or on a PR this agent authored.
+        A broken-CI block must rest on a check that COMPLETED with a failure you
+        read — never on one still running or one you couldn't see.
+
+        Args:
+            repo: Repository as ``owner/name``. Omit to use the agent's configured default repo.
+            number: PR number.
+            body: The review body — every blocking finding with file:line evidence.
+        """
+        repo = resolve_repo(repo, default_repo) or ""
+        if err := bad_repo(repo):
+            return err
+        if not body.strip():
+            return "Error: `body` is empty — a review needs content."
+        if held := await _guard_blocking_verdict(repo, number, "REQUEST_CHANGES"):
+            return held
+        return await _post_review(repo, number, "REQUEST_CHANGES", body)
+
+    return [github_review_comment, github_review_approve, github_review_request_changes]

tests/test_read_tools.py

@@ -284,3 +284,33 @@ async def test_omitted_repo_no_default_errors(monkeypatch):
     monkeypatch.delenv("GH_REPO", raising=False)
     result = await _list_issues_tool("").ainvoke({})  # no repo, no default, no env
     assert result.startswith("Error: no usable repo")
+
+
+# ── github_path_exists: the EXISTS/MISSING grounding probe (ADR 0078) ───────────
+def _path_exists_tool():
+    for t in get_read_tools():
+        if t.name == "github_path_exists":
+            return t
+    raise AssertionError("github_path_exists tool not found")
+
+
+@pytest.mark.asyncio
+async def test_path_exists_reports_exists():
+    with patch("ghplugin.read_tools.run_gh", new=AsyncMock(return_value=(0, "{}", ""))):
+        out = await _path_exists_tool().ainvoke({"repo": "owner/name", "path": "packages/x"})
+    assert out.startswith("EXISTS: owner/name/packages/x")
+
+
+@pytest.mark.asyncio
+async def test_path_exists_reports_missing_on_404():
+    with patch("ghplugin.read_tools.run_gh", new=AsyncMock(return_value=(1, "", "HTTP 404: Not Found"))):
+        out = await _path_exists_tool().ainvoke({"repo": "owner/name", "path": "gone.txt", "ref": "main"})
+    assert out.startswith("MISSING: owner/name/gone.txt @ main")
+
+
+@pytest.mark.asyncio
+async def test_path_exists_other_error_is_unverified_not_a_verdict():
+    with patch("ghplugin.read_tools.run_gh", new=AsyncMock(return_value=(1, "", "HTTP 500"))):
+        out = await _path_exists_tool().ainvoke({"repo": "owner/name", "path": "x"})
+    assert "EXISTS" not in out and "MISSING" not in out.split(":")[0]
+    assert out.startswith("Error")