diff --git a/protoagent.plugin.yaml b/protoagent.plugin.yaml index 5c1f5b7..5167318 100644 --- a/protoagent.plugin.yaml +++ b/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.2.0 +version: 0.3.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 diff --git a/pyproject.toml b/pyproject.toml index b2581da..4134304 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "github-plugin" -version = "0.2.0" +version = "0.3.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 a728278..47f0bf5 100644 --- a/read_tools.py +++ b/read_tools.py @@ -286,6 +286,51 @@ 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_read_pr_file(number: int, path: str, repo: str = "") -> str: + """Read a file as it exists IN a pull request — at the PR's head commit. + + Use this for every code-context read while reviewing a PR. Unlike + ``github_read_file``, there is no ref to get wrong: the head SHA is resolved + server-side from the PR number, so the file you get is the PR's version, + including anything the PR adds. + + Args: + number: PR number. + repo: Repository as ``owner/name``. Omit to use the agent's configured default repo. + path: Path to the file within the repo (e.g. ``src/lib/queries.ts``). + """ + repo = resolve_repo(repo, default_repo) or "" + if err := bad_repo(repo): + return err + # The ref is resolved HERE, from the PR — never supplied by the caller. A + # model that omits (or mistypes) a ref on a plain read silently gets the + # DEFAULT branch, i.e. the pre-PR file: it then "confirms" that symbols the + # PR adds don't exist, which reads as a blocker finding on correct code. + rc, out, serr = await run_gh(["api", f"repos/{repo}/pulls/{number}", "--jq", ".head.sha"]) + if gh_err := check_gh_error(rc, serr): + return gh_err + head = out.strip() + if not head: + return f"Error: could not resolve the head SHA for {repo}#{number}." + rc, out, serr = await run_gh( + [ + "api", + f"repos/{repo}/contents/{path}", + "-H", + "Accept: application/vnd.github.raw+json", + "-f", + f"ref={head}", + ] + ) + if gh_err := check_gh_error(rc, serr): + # Fail LOUD, never fall back to the default branch: a silent fallback is + # exactly the bug this tool exists to prevent. + return f"Error reading {path} at {repo}#{number} head {head[:12]}: {gh_err}" + if len(out) > 20000: + out = out[:20000] + "\n… (truncated at 20000 chars)" + return f"{path} @ {repo}#{number} head {head[:12]}:\n\n{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 @@ -370,5 +415,6 @@ async def github_repo_contents(repo: str = "", path: str = "", ref: str = "") -> github_ci_runs, github_run_failure, github_read_file, + github_read_pr_file, github_repo_contents, ] diff --git a/tests/test_read_tools.py b/tests/test_read_tools.py index c4cb668..d88f3a8 100644 --- a/tests/test_read_tools.py +++ b/tests/test_read_tools.py @@ -314,3 +314,61 @@ async def test_path_exists_other_error_is_unverified_not_a_verdict(): 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") + + +# ── github_read_pr_file: the ref is resolved server-side, never by the caller ──── +# Regression for pr-reviewer-plugin#20: a review that read a PR's files at the +# DEFAULT branch "confirmed" that symbols the PR ADDS did not exist, producing +# blocker findings on correct PRs. This tool removes the ref from the caller's +# hands entirely. + + +def _read_pr_file_tool(): + for t in get_read_tools(): + if t.name == "github_read_pr_file": + return t + raise AssertionError("github_read_pr_file tool not found") + + +@pytest.mark.asyncio +async def test_read_pr_file_reads_at_the_prs_head_sha(): + head = "19c37f7a7db6f0c1a2b3c4d5e6f708192a3b4c5d" + calls = [] + + async def fake_gh(args, **kw): + calls.append(args) + if "/pulls/" in args[1]: + return 0, head + "\n", "" + return 0, "export const goalDetailQuery = () => {}\n", "" + + with patch("ghplugin.read_tools.run_gh", new=AsyncMock(side_effect=fake_gh)): + out = await _read_pr_file_tool().ainvoke({"repo": "o/r", "number": 2088, "path": "src/lib/queries.ts"}) + + # the content read is pinned to the head SHA the PR reported + content_call = " ".join(calls[-1]) + assert f"ref={head}" in content_call + assert "goalDetailQuery" in out and head[:12] in out + + +@pytest.mark.asyncio +async def test_read_pr_file_never_falls_back_to_the_default_branch(): + """A failed pinned read must ERROR, not silently serve the pre-PR file.""" + + async def fake_gh(args, **kw): + if "/pulls/" in args[1]: + return 0, "a" * 40 + "\n", "" + return 1, "", "Not Found (HTTP 404)" + + with patch("ghplugin.read_tools.run_gh", new=AsyncMock(side_effect=fake_gh)): + out = await _read_pr_file_tool().ainvoke({"repo": "o/r", "number": 1, "path": "nope.ts"}) + assert out.startswith("Error reading nope.ts") + + +@pytest.mark.asyncio +async def test_read_pr_file_errors_when_the_head_sha_is_unresolvable(): + async def fake_gh(args, **kw): + return 0, "\n", "" # empty head — a PR we cannot pin + + with patch("ghplugin.read_tools.run_gh", new=AsyncMock(side_effect=fake_gh)): + out = await _read_pr_file_tool().ainvoke({"repo": "o/r", "number": 1, "path": "x.ts"}) + assert "could not resolve the head SHA" in out diff --git a/uv.lock b/uv.lock new file mode 100644 index 0000000..667418e --- /dev/null +++ b/uv.lock @@ -0,0 +1,8 @@ +version = 1 +revision = 3 +requires-python = ">=3.11" + +[[package]] +name = "github-plugin" +version = "0.3.0" +source = { virtual = "." }