Skip to content

ai-guarded-review: run can succeed and label 'AI reviewed' without posting any review comment (max-turns exhausted, silently tolerated) #50

Description

@mattgoud

Summary

The reusable ai-guarded-review.yml workflow can complete successfully and apply the AI reviewed label while posting no review comment at all. The PR is left looking reviewed, but there is nothing to read.

Reproduction

Observed on a large PR (PrestaShop/ps_apiresources#410, ~2 750-line diff, 29 files), and it is non-deterministic — two runs on the exact same PR/commit:

  • Run A — job success, label swapped to AI reviewed, 0 comments posted.
  • Run B (same PR, re-triggered) — job success, label AI reviewed, 1 proper pre-review comment posted.

Smaller single-file PRs have always posted correctly.

Root cause

The review comment is not posted by a dedicated step — it is posted by Claude itself calling gh pr comment (it's in allowed-tools), within the --max-turns 30 budget (claude_args, default max-turns: 30).

On a large PR, reading the prompt + gh pr diff (huge) + gh pr view + reading files can consume the whole turn budget before Claude reaches the gh pr comment call. That produces subtype: error_max_turns, which the Validate Claude execution step deliberately tolerates as a warning, not a failure:

# Reaching max turns is the one tolerated error: a partial review may
# have been posted, so warn but do not fail.
if [ "$SUBTYPE" = "error_max_turns" ]; then
  echo "::warning::Claude reached the maximum number of turns ..."

Because the job does not fail, the Remove trigger label on failure step is skipped and Update labels runs — swapping Need AI reviewAI reviewed. No step verifies that a comment was actually created, so an empty review is silently marked as done.

Impact

A PR gets the AI reviewed label with no review. Reviewers/QA trust the label and skip the AI pass that never happened. It's silent and size-correlated (worse on big PRs), so it will recur as PRs grow.

Proposed fix

  1. Verify a comment was actually posted this run, before applying AI reviewed. Snapshot a start timestamp, then after the Claude step check that a github-actions[bot] comment was created/updated at/after it; if none, exit 1 so the existing Remove trigger label on failure path runs (restore Need AI review, do not add AI reviewed). This makes the tolerated-error_max_turns case safe: tolerated only when a comment really exists.
  2. Bump the max-turns default (e.g. 30 → 40) to reduce how often large PRs hit the wall in the first place.

I'll open a PR implementing both.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

Status
Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions