Skip to content

gh pr review denied on 29% of attempts despite being allowlisted #33

Description

@zfarrell

Bash(gh pr review:*) is on the reviewer's allowlist, yet gh pr review is refused on
29% of the 241 times it is attempted. The reviewer retries and almost always gets its
verdict posted, so this is not losing reviews — it is burning turns, wall clock and money on
every fourth run, and it is still happening.

Evidence

Measured from the claude-tool-usage-pr-* artifacts. Two windows: everything since
frontloaded context merged (#22, 2026-08-05T01:48Z), and the settled window after the last
follow-up fix (#31, 2026-08-07T03:32Z).

since #22 (271 runs) settled, after #31 (170 runs)
Runs with ≥1 gh pr review denial 62 (23%) 45 (26%)
Total denials 96 71
Denials per affected run 1.55 (max 3) 1.58 (max 3)
Flagged compound 1 of 96 1 of 71

In the settled window gh pr review is attempted 241 times and denied 71 — a 29% refusal
rate on the one command the reviewer cannot finish without. No other allowlisted command is
close: rg is denied on 3% of 75 attempts, gh pr view and gh pr diff on 0%.

It is not repo-specific. 8 repos affected: polyglot (16), monopoly (15), dlthubworker (12),
www.hotdata.dev (5), hotquery (5), hotmodel (4), runtimedb (3), github-workflows (2).

It is not losing reviews. 44 of 45 affected runs in the settled window still produced a
verdict, against 122 of 125 clean runs — the reviewer retries in a form that passes.

What it costs. Affected vs clean runs, settled window:

affected clean delta
Turns 19.3 13.3 +45%
Cost $1.280 $0.823 +$0.457 (+55%)
Wall clock 195s 122s +73s

At 26% of runs, that +73s is roughly 19s of the median review latency — a real slice of the
gap that still separates the current reviewer from the pre-Opus-5 93s baseline.

First affected run 2026-08-05T05:27Z, most recent 2026-08-10T17:47Z.

Why the artifact cannot currently tell us the cause

The obvious suspect is a compound command, and the artifact carries a compound flag for
exactly that. It says no — 1 of 96. But that flag cannot see the most likely cause. As
shipped it is:

compound: (unquoted | test("\\||&&|;|>"))

unquoted strips double- and single-quoted spans first, and the test never looks for `
or $(. So a command whose only disqualifying feature is command substitution inside a
quoted --body classifies as compound: false. "70 of 71 are not compound" therefore does
not exonerate substitution; it means the flag is blind to it.

A flag that would have answered this shipped in #26 and was reverted in 8b04393 (the run
block went over the 21,000-character expression limit):

has_subst: test("`|\\$\\(")

Confirmed absent: no artifact in the window carries a has_subst field.

Hypothesis

The reviewer posts a multi-line verdict body, and the natural shell form for that is
gh pr review --request-changes --body "$(cat <<'EOF' … EOF)". Command substitution is
refused regardless of how well the prefix matches the allowlist entry. That would explain
the 29% rate, the 1.58 retries per affected run (the retry succeeds once the body is passed
differently), and why compound reports almost nothing.

Stated as a hypothesis — the data above is consistent with it but does not prove it, because
the one flag that would discriminate was reverted.

Suggested next step

Re-land a substitution flag within the expression budget — a single test on the raw
command string, not a second pass over unquoted — and give it a week. If it confirms
substitution, the fix is on the prompt side (have the reviewer pass the body without
substitution) rather than the allowlist side, since widening the allowlist to admit
substitution would defeat the point of the allowlist.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions