From c09587bd1e981fab7105db7917c4002584d5da95 Mon Sep 17 00:00:00 2001 From: Josh Mabry Date: Tue, 21 Jul 2026 02:32:06 -0700 Subject: [PATCH] =?UTF-8?q?feat(read):=20github=5Fread=5Fpr=5Ffile=20?= =?UTF-8?q?=E2=80=94=20reads=20pinned=20to=20the=20PR=20head=20(v0.3.0)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `github_read_file`'s `ref` is optional and defaults to the repo's DEFAULT BRANCH. A reviewer that omits it reads the PRE-PR file, then "confirms" that symbols the PR ADDS don't exist — blocker findings on correct code. That is not hypothetical: pr-reviewer-plugin#20. On protoAgent#2088 the verifier reported `goalDetailQuery` missing from apps/web/src/lib/queries.ts; it is exported at line 90 of the reviewed head. Prompt-level ref discipline was already in place in BOTH the review-finder and verifier system prompts, and the recipe passed the head SHA to every step — it still read the default branch. Telling the model to pass a ref is not a guarantee. `github_read_pr_file(number, path, repo)` takes no ref at all: it resolves the head SHA server-side from the PR, and a failed pinned read ERRORS rather than falling back to the default branch (a silent fallback is the bug itself). Additive — `github_read_file` is untouched, so nothing existing changes. Co-Authored-By: Claude Opus 4.8 (1M context) --- protoagent.plugin.yaml | 2 +- pyproject.toml | 2 +- read_tools.py | 46 +++++++++++++++++++++++++++++++ tests/test_read_tools.py | 58 ++++++++++++++++++++++++++++++++++++++++ uv.lock | 8 ++++++ 5 files changed, 114 insertions(+), 2 deletions(-) create mode 100644 uv.lock 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 = "." }