chore(ci): fail the on-demand review when it posts nothing

[thecodedrift]

The on-demand review workflow can report success while posting nothing. This makes that impossible.

What happened

On #182 (run 32933651692) the review job went green having done no work:

{ "type": "result", "subtype": "success", "is_error": false,
  "duration_ms": 147421, "num_turns": 0,
  "total_cost_usd": 1.076006, "permission_denials_count": 4 }

The tracking comment kept its placeholder — "I'll analyze this and get back to you" — the run logged No buffered inline comments, and the PR received zero inline comments. The final ✅ Updated issue comment … with job link step stamped the job URL onto that stub, which is easy to mistake for a review being posted.

A silent no-op is indistinguishable from a clean review. The PR reads as reviewed when nothing read it, and the run is billed either way. That is worse than the job failing outright.

Why is_error cannot gate it

is_error was false in that run. The load-bearing signals are in the action's execution file:

Signal	Why it fails the job
num_turns == 0	A review that never took a turn cannot have posted anything.
permission_denials_count > 0	A denial is how a review dies quietly: the tool it needs is not on the allowlist, it has nowhere to say so, and it stops. Failed even with turns — a review that lost a tool could not see everything.
is_error	Kept, but as one signal among several rather than the gate.
missing file / no result record	Cannot confirm a review ran, so it is not assumed.

The step also writes the counters to the job summary on every run, pass or fail.

Naming the denied tools

When the record carries them, the denied tool names are printed. Tool names are structured data the runner produced, so they are safe here; the tool input is never printed, because it can quote the untrusted diff. This is what makes the next occurrence self-diagnosing instead of a log excavation — the root cause of the #182 run is still unknown precisely because nothing surfaced it.

show_full_output: true would answer the same question and is deliberately not used: this is a public repo, and it dumps text the model produced while ingesting an untrusted diff into a world-readable log. Counters are not attacker-controlled; model prose is.

Verification

Both execution-file shapes the action has written are accepted (a JSON array of messages, and JSONL) rather than pinning a shape it may change. Tested against six fixtures, including the real record from run 32933651692:

Fixture	Expected	Result
the #182 failure shape (num_turns: 0, 4 denials)	fail	exit 1
healthy run (14 turns, 0 denials)	pass	exit 0
denials with turns	fail	exit 1
denials with named tools	fail, names them	exit 1, Denied tool(s): TodoWrite, Bash
no result record	fail	exit 1
missing file / unset variable	fail	exit 1

if: always() so the step still runs when the action itself fails.

Not addressed here

The root cause of the four denials on that run. It is not visible in the log (full output hidden for security), and shipping a speculative permissions change to a security-sensitive workflow is worse than shipping a guard that turns the next occurrence into a loud, self-diagnosing failure. One plausible mechanism is the action restoring .claude from origin/main and running with settingSources: ["user","project","local"], so this repo's own rules reach the review agent on top of the action's allowedTools allowlist — but that is a hypothesis, not a measurement, and the guard is what will produce the measurement.

CI-only, so no changeset.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.