From c6cdbe8e03a13bfaa4eb1d1b7161ac2b55a2bc7c Mon Sep 17 00:00:00 2001 From: d-morrison Date: Thu, 20 Aug 2026 15:56:32 -0700 Subject: [PATCH 1/5] fix(claude-code-review): tell the reviewer not to redirect/pipe gh pr diff On a large PR, the reviewer would reach for `gh pr diff ... > file; wc -l file` (or a pipe) to chunk the diff, and every such compound command was denied wholesale, while writing to any file (including /tmp) is a hard sandbox block rather than a permission prompt. It burned through the denial budget on variant after variant with no verdict ever produced (33 denials measured on ucdavis/win#78, run 32415477507). Fixes #541. --- .../run-claude-review-attempt/action.yml | 31 +++++++++++++++++++ .../reviewer-diff-redirect-stub.fixed.md | 10 ++++++ 2 files changed, 41 insertions(+) create mode 100644 changelog.d/reviewer-diff-redirect-stub.fixed.md diff --git a/.github/actions/run-claude-review-attempt/action.yml b/.github/actions/run-claude-review-attempt/action.yml index ef7a3bb7..e8b8f7d4 100644 --- a/.github/actions/run-claude-review-attempt/action.yml +++ b/.github/actions/run-claude-review-attempt/action.yml @@ -271,6 +271,24 @@ runs: # `Agent` calls, `is_error: false`, `stop_reason: end_turn`, # $9.76, and a final message reading "Waiting for the remaining # background agents to complete." in place of a verdict. + # + # A fourth stub-review cause, gha#541, needs no new tool grant — + # `Bash(gh pr diff:*)` above already covers it; the failure is in HOW + # the reviewer used it. On a large diff, the agent reached for + # `gh pr diff ... > /tmp/pr.diff; wc -l ...` (or `gh pr diff ... | + # grep ...`) to chunk the output, and every variant was denied: a + # command chaining an allowed pattern with `;`/`&&`/`|`/a redirect is + # rejected as a WHOLE command even though the allowed part matches + # exactly, and writing to a file — `/tmp`, or a `mkdir`-created + # subdirectory of the checkout — is a hard block in this sandbox, not + # a permission prompt, so no allowlist entry can satisfy it. Measured + # on ucdavis/win#78, run 32415477507: 15+ redirect/pipe/mkdir variants + # denied, `permission_denials_count:33`, `total_cost_usd:2.29`, no + # verdict. A BARE, unredirected `gh pr diff --repo /` + # needs none of that: its full output returns as the Bash tool's own + # result text, already matches the allowlist, and needs no file at + # all — the prompt below tells the reviewer to use exactly that form + # instead of trying to save/count/grep the diff. claude_args: >- --allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(python3:*),Bash(maxima:*),WebFetch,WebSearch,Bash(curl:*),Bash(gh pr view:*),Bash(gh pr diff:*),Bash(gh pr list:*),Bash(gh issue view:*),Bash(gh issue list:*),Bash(gh search:*)" @@ -298,6 +316,19 @@ runs: ### Verdict line required below -- do not end the run silently or wait for approval. + To read the PR diff, call `gh pr diff --repo + /` BARE -- no `>` redirect, no `|` pipe, no `;`/`&&` + chaining with `wc`, `grep`, or anything else, even to save or count + a large diff. Its full output comes back as this tool call's own + result text; read it directly from there, however large it is. A + command combining an allowed pattern with a redirect, pipe, or + chain is rejected as a whole, and writing the diff to a file -- + anywhere, including /tmp or a directory you create yourself with + mkdir -- is a hard block in this sandbox that no retry or different + path will get past. If a redirected or piped attempt is denied, + do not try another variant of the same idea: switch immediately to + the bare, unredirected form and read its output inline. + Do NOT post a top-level PR comment yourself: the `gh pr comment` tool is intentionally disallowed, and the workflow posts your review for you from your final output. Write your complete review diff --git a/changelog.d/reviewer-diff-redirect-stub.fixed.md b/changelog.d/reviewer-diff-redirect-stub.fixed.md new file mode 100644 index 00000000..e0a54be1 --- /dev/null +++ b/changelog.d/reviewer-diff-redirect-stub.fixed.md @@ -0,0 +1,10 @@ +- **`claude-code-review` reviewer prompt now warns against redirecting or + piping `gh pr diff`** (#541). On a large PR, the reviewer previously reached + for `gh pr diff ... > file; wc -l file` (or a similar pipe) to chunk the + diff, but a compound command combining an allowed pattern with a redirect, + pipe, or `;`/`&&` chain is denied as a whole, and writing to any file -- + `/tmp` included -- is a hard sandbox block, not a permission prompt. The + reviewer would retry variant after variant until the denial budget was + exhausted with no verdict (33 denials measured on ucdavis/win#78). The + system prompt now tells it to call `gh pr diff --repo /` + bare and read the returned text directly instead. From 17de2c2ea03779d628629d0260a0d7b9187fff58 Mon Sep 17 00:00:00 2001 From: d-morrison Date: Thu, 20 Aug 2026 15:58:37 -0700 Subject: [PATCH 2/5] fix: semantic line breaks in changelog fragment --- .../reviewer-diff-redirect-stub.fixed.md | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/changelog.d/reviewer-diff-redirect-stub.fixed.md b/changelog.d/reviewer-diff-redirect-stub.fixed.md index e0a54be1..9ee71b35 100644 --- a/changelog.d/reviewer-diff-redirect-stub.fixed.md +++ b/changelog.d/reviewer-diff-redirect-stub.fixed.md @@ -1,10 +1,14 @@ - **`claude-code-review` reviewer prompt now warns against redirecting or - piping `gh pr diff`** (#541). On a large PR, the reviewer previously reached - for `gh pr diff ... > file; wc -l file` (or a similar pipe) to chunk the - diff, but a compound command combining an allowed pattern with a redirect, - pipe, or `;`/`&&` chain is denied as a whole, and writing to any file -- - `/tmp` included -- is a hard sandbox block, not a permission prompt. The - reviewer would retry variant after variant until the denial budget was - exhausted with no verdict (33 denials measured on ucdavis/win#78). The - system prompt now tells it to call `gh pr diff --repo /` - bare and read the returned text directly instead. + piping `gh pr diff`** (#541). + On a large PR, the reviewer previously reached for + `gh pr diff ... > file; wc -l file` (or a similar pipe) to chunk the diff. + A compound command combining an allowed pattern with a redirect, pipe, or + `;`/`&&` chain is denied as a whole, + and writing to any file -- `/tmp` included -- is a hard sandbox block, + not a permission prompt. + The reviewer would retry variant after variant until the denial budget + was exhausted with no verdict + (33 denials measured on ucdavis/win#78). + The system prompt now tells it to call + `gh pr diff --repo /` bare + and read the returned text directly instead. From 0c0abf3033482eb954df77c9c781ed2de15f3b57 Mon Sep 17 00:00:00 2001 From: d-morrison Date: Thu, 20 Aug 2026 16:14:57 -0700 Subject: [PATCH 3/5] fix: correct false "however large it is" claim in gh pr diff guidance Claude Code's Bash tool caps inline output at ~30,000 characters; past that a valid result comes back as a preview plus a harness-saved file path, not the full text. This is true of the very PR this fix cites as its motivating evidence (ucdavis/win#78, 114,587 bytes / 2,464 lines). Tell the reviewer to Read that harness-saved path instead of claiming the bare command always returns everything inline -- the file itself is fine to read, since it's the harness's own write, not the agent's. Addresses review finding on PR #542. --- .../run-claude-review-attempt/action.yml | 49 ++++++++++++++----- .../reviewer-diff-redirect-stub.fixed.md | 6 ++- 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/.github/actions/run-claude-review-attempt/action.yml b/.github/actions/run-claude-review-attempt/action.yml index e8b8f7d4..b97d36eb 100644 --- a/.github/actions/run-claude-review-attempt/action.yml +++ b/.github/actions/run-claude-review-attempt/action.yml @@ -285,10 +285,25 @@ runs: # on ucdavis/win#78, run 32415477507: 15+ redirect/pipe/mkdir variants # denied, `permission_denials_count:33`, `total_cost_usd:2.29`, no # verdict. A BARE, unredirected `gh pr diff --repo /` - # needs none of that: its full output returns as the Bash tool's own - # result text, already matches the allowlist, and needs no file at - # all — the prompt below tells the reviewer to use exactly that form - # instead of trying to save/count/grep the diff. + # needs none of that: it already matches the allowlist and needs no + # file write by the agent at all — the prompt below tells the + # reviewer to use exactly that form instead of trying to save/count/ + # grep the diff itself. + # + # That bare call's own OUTPUT can still exceed Claude Code's Bash + # tool's ~30,000-character inline ceiling on a genuinely large diff — + # ucdavis/win#78's own diff measures 114,587 bytes / 2,464 lines, + # about 3.8x that ceiling. + # Past it, a valid (exit 0) result comes back as a preview plus the + # path of a file the HARNESS already saved, not the full text inline + # (code.claude.com/docs/en/tools-reference). + # That harness-written file sits in the session's own working + # directory, unlike the agent's own redirect/mkdir attempts above, so + # Reading it back is not subject to the same hard block — Read is + # already in the action's base allowlist (see the top-of-file + # comment). + # The prompt below tells the reviewer to Read that path rather than + # trying to redirect/pipe its way to the same information. claude_args: >- --allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(python3:*),Bash(maxima:*),WebFetch,WebSearch,Bash(curl:*),Bash(gh pr view:*),Bash(gh pr diff:*),Bash(gh pr list:*),Bash(gh issue view:*),Bash(gh issue list:*),Bash(gh search:*)" @@ -319,15 +334,23 @@ runs: To read the PR diff, call `gh pr diff --repo /` BARE -- no `>` redirect, no `|` pipe, no `;`/`&&` chaining with `wc`, `grep`, or anything else, even to save or count - a large diff. Its full output comes back as this tool call's own - result text; read it directly from there, however large it is. A - command combining an allowed pattern with a redirect, pipe, or - chain is rejected as a whole, and writing the diff to a file -- - anywhere, including /tmp or a directory you create yourself with - mkdir -- is a hard block in this sandbox that no retry or different - path will get past. If a redirected or piped attempt is denied, - do not try another variant of the same idea: switch immediately to - the bare, unredirected form and read its output inline. + a large diff. A command combining an allowed pattern with a + redirect, pipe, or chain is rejected as a whole, and writing the + diff to a file YOURSELF -- anywhere, including /tmp or a directory + you create with mkdir -- is a hard block in this sandbox that no + retry or different path will get past. If a redirected or piped + attempt is denied, do not try another variant of the same idea: + switch immediately to the bare, unredirected form. + + The bare command's result is not always the complete diff inline: + past roughly 30,000 characters, the tool result becomes a preview + plus the path of a file already saved for you, and you can Read + that path for the rest. That saved file is NOT the same thing as a + file you redirect or mkdir yourself -- it is written by the + harness, to this session's own working directory, so reading it + back is not blocked the way your own redirect is. Never try to + recreate or replace it with your own redirect or pipe; just Read + the path the tool result already gives you. Do NOT post a top-level PR comment yourself: the `gh pr comment` tool is intentionally disallowed, and the workflow posts your diff --git a/changelog.d/reviewer-diff-redirect-stub.fixed.md b/changelog.d/reviewer-diff-redirect-stub.fixed.md index 9ee71b35..2850a81d 100644 --- a/changelog.d/reviewer-diff-redirect-stub.fixed.md +++ b/changelog.d/reviewer-diff-redirect-stub.fixed.md @@ -10,5 +10,7 @@ was exhausted with no verdict (33 denials measured on ucdavis/win#78). The system prompt now tells it to call - `gh pr diff --repo /` bare - and read the returned text directly instead. + `gh pr diff --repo /` bare instead of redirecting or + piping it, and -- since a diff that large still exceeds the Bash tool's + inline output ceiling -- to Read the file path the harness already saves + for it, rather than trying to recreate that file itself. From 72b7dda6da8c937d4942b9770ded8bc003315b51 Mon Sep 17 00:00:00 2001 From: d-morrison Date: Thu, 20 Aug 2026 16:16:37 -0700 Subject: [PATCH 4/5] fix: scope the file-write hard-block claim to the observed sandbox Non-blocking review note on PR #542: "anywhere ... no retry or different path will get past" read as a universal property of every runs-on runner, when it's only confirmed for the observed sandbox. Reworded to state what was actually tried and observed blocked. --- .github/actions/run-claude-review-attempt/action.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/actions/run-claude-review-attempt/action.yml b/.github/actions/run-claude-review-attempt/action.yml index b97d36eb..79ab27a1 100644 --- a/.github/actions/run-claude-review-attempt/action.yml +++ b/.github/actions/run-claude-review-attempt/action.yml @@ -336,11 +336,11 @@ runs: chaining with `wc`, `grep`, or anything else, even to save or count a large diff. A command combining an allowed pattern with a redirect, pipe, or chain is rejected as a whole, and writing the - diff to a file YOURSELF -- anywhere, including /tmp or a directory - you create with mkdir -- is a hard block in this sandbox that no - retry or different path will get past. If a redirected or piped - attempt is denied, do not try another variant of the same idea: - switch immediately to the bare, unredirected form. + diff to a file YOURSELF -- /tmp and a directory you create with + mkdir have both been tried and both blocked -- has not worked in + this sandbox. If a redirected or piped attempt is denied, do not + try another variant of the same idea: switch immediately to the + bare, unredirected form. The bare command's result is not always the complete diff inline: past roughly 30,000 characters, the tool result becomes a preview From f7e8697cb5b7d24c450baf74bcbaf5f505bb7978 Mon Sep 17 00:00:00 2001 From: d-morrison Date: Thu, 20 Aug 2026 16:20:36 -0700 Subject: [PATCH 5/5] fix: scope changelog fragment's hard-block claim, matching action.yml Non-blocking observation from the latest review round on PR #542: the changelog fragment still stated the file-write block as a general fact rather than scoped to what was observed, the same overclaim 72b7dda already fixed in action.yml's own prompt text. --- changelog.d/reviewer-diff-redirect-stub.fixed.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/changelog.d/reviewer-diff-redirect-stub.fixed.md b/changelog.d/reviewer-diff-redirect-stub.fixed.md index 2850a81d..9fc165b7 100644 --- a/changelog.d/reviewer-diff-redirect-stub.fixed.md +++ b/changelog.d/reviewer-diff-redirect-stub.fixed.md @@ -4,8 +4,9 @@ `gh pr diff ... > file; wc -l file` (or a similar pipe) to chunk the diff. A compound command combining an allowed pattern with a redirect, pipe, or `;`/`&&` chain is denied as a whole, - and writing to any file -- `/tmp` included -- is a hard sandbox block, - not a permission prompt. + and writing to any file -- `/tmp` and a `mkdir`-created directory were + both tried and both blocked -- is a hard sandbox block, not a permission + prompt. The reviewer would retry variant after variant until the denial budget was exhausted with no verdict (33 denials measured on ucdavis/win#78).