Skip to content

fix(read): contents reads must be GETs — -f ref= made gh send POST, a 404 - #29

Merged
mabry1985 merged 1 commit into
mainfrom
fix/read-ref-get
Sep 14, 2026
Merged

fix(read): contents reads must be GETs — -f ref= made gh send POST, a 404#29
mabry1985 merged 1 commit into
mainfrom
fix/read-ref-get

Conversation

@mabry1985

@mabry1985 mabry1985 commented Sep 14, 2026

Copy link
Copy Markdown
Member

Root cause

gh api chooses the HTTP method on its own. From gh api --help: "The default HTTP request method is GET normally and POST if any parameters were added. Override the method with --method." Four tools pass the ref as -f ref=<x> without --method: github_read_file, github_read_pr_file, github_path_exists and github_repo_contents. Every call that pins a ref therefore sends POST /repos/{o}/{r}/contents/{path}, and GitHub answers 404 Not Found, even for files that exist.

Repro with gh 2.92, reading a file that exists (webhook.py on pr-reviewer-plugin main):

gh api repos/protoLabsAI/pr-reviewer-plugin/contents/webhook.py -f ref=main --verbose
  > POST /repos/protoLabsAI/pr-reviewer-plugin/contents/webhook.py   < HTTP/2.0 404 Not Found
gh api --method GET repos/protoLabsAI/pr-reviewer-plugin/contents/webhook.py -f ref=main --verbose
  > GET /repos/protoLabsAI/pr-reviewer-plugin/contents/webhook.py?ref=main   < HTTP/2.0 200 OK

Production impact (Vera audit log, /sandbox/audit/audit.jsonl*, 2026-07-07 → 2026-09-14)

call ok failed
github_read_file without a ref ~85% ok (the rest are real 404s: PR-only files read at the default branch)
github_read_file with a ref 0 every one, every day, every repo
github_read_pr_file (always pins the head) 0 every one since it shipped in v0.3.0 (2026-07-21)

The head-pinned read added for pr-reviewer-plugin#20 has never worked in production. Every "Gap: the PR-head file reads 404'd" line in the panel's finder output comes from this, and so does the blind find_removed_behavior / find_crossfile lanes in pr-reviewer-plugin#118 (protoAgent#3494). The existing tests asserted "-f" in args / "ref=main" in args and passed the whole time. They checked that the ref was passed, not how gh sends it.

Change

  • Add --method GET to all four contents calls. With GET, gh sends -f fields as query parameters (?ref=…, URL-encoded, so branch names like epic/fleet-deck work).
  • New tests model gh's documented method rule, which I checked against --verbose above. They assert that every contents call from each ref-taking read tool is a GET and still carries the ref, and that github_read_pr_file reads at the resolved head. Before the fix they fail 5/5 (… sends POST (404s on GitHub)).
  • One existing test pinned the exact argv, ["api", "repos/owner/name/contents"]. It now includes --method GET and still guards against a trailing slash.

No version bump. It takes a release plus Vera's github-plugin pin bump (qaEngineer) before the panel's reads start working.

Tests

  • uv run --no-project --python 3.12 --with-requirements requirements-dev.txt pytest -q → 278 passed
  • uvx ruff@0.15.10 check . and uvx ruff@0.15.10 format --check . → clean

Part of protoLabsAI/pr-reviewer-plugin#118 (it closes once this fix is released and pinned in Vera’s image)

🤖 Generated with Claude Code

https://claude.ai/code/session_01LSRkzcPyaDrKSY7geFuGr3

… a 404 (pr-reviewer-plugin#118)

`gh api` chooses the HTTP method itself: GET normally, POST as soon as any
`-f/-F` field is added, unless `--method` overrides it. github_read_file,
github_read_pr_file, github_path_exists and github_repo_contents passed the
ref as `-f ref=<x>` with no `--method`, so every ref-pinned call went out as
`POST /repos/{o}/{r}/contents/{path}`, which GitHub answers with 404 Not
Found, including for files that exist. Verified with gh 2.92 --verbose:
`-f ref=main` -> `> POST ... < HTTP/2.0 404`; with `--method GET` ->
`> GET ...?ref=main < HTTP/2.0 200`.

Production impact (Vera's audit log, 2026-07-07 .. 2026-09-14): 0 successful
ref-pinned github_read_file calls and 0 successful github_read_pr_file calls,
out of tens of thousands, on every repo. The review panel's head-pinned reads
(the #20 fix, v0.3.0) have never worked, so its lanes reviewed diffs blind.
The existing tests asserted `"-f" in args` / `"ref=main" in args` and stayed
green the whole time: they checked that the ref was passed, not how gh sends it.

Fix: `--method GET` on all four contents calls (gh then sends fields as query
params). New tests model gh's documented method rule and assert every
contents call is a GET that still carries the ref; they fail 5/5 before this
change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LSRkzcPyaDrKSY7geFuGr3

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA panel review — PASS

code-review · head b5eef1a02124 · formal

Low-risk, well-scoped fix: adding --method GET to four gh api call sites that were silently defaulting to POST (and 404-ing) whenever a query parameter was present. No findings survived the panel — every finder agreed the change is correct, and the verifier independently confirmed all four load-bearing claims against the gh source and GitHub API docs. Nothing to fix first; the PR is ready to merge as-is.

No findings — the review came back clean.

findings JSON (machine-readable)
[]

@protoreview protoreview Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Promoting the PASS verdict for head b5eef1a02124: all checks terminal-green, zero unresolved review threads. (approve-on-green)

@mabry1985
mabry1985 merged commit 9a9c51b into main Sep 14, 2026
3 checks passed
@mabry1985 mabry1985 mentioned this pull request Sep 14, 2026
mabry1985 added a commit that referenced this pull request Sep 14, 2026
- Contents reads are GETs (#29): `github_read_file`, `github_read_pr_file`,
  `github_path_exists` and `github_repo_contents` passed the ref as `-f ref=`, which
  made `gh api` send POST, and GitHub 404s a POST to /contents. Every ref-pinned read
  failed; the QA panel's lanes reviewed diffs blind (pr-reviewer-plugin#118).
- Adds the standard release caller (`.github/workflows/release.yml`, the reusable
  release-tools plugin-release workflow) so this plugin releases like the others.
- uv.lock's own version entry had drifted to 0.5.0; now 0.7.1.


Claude-Session: https://claude.ai/code/session_01LSRkzcPyaDrKSY7geFuGr3

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant