Skip to content

Reviewer

/review

The Reviewer judges whether an already-implemented pull request actually satisfies the design and task it was built for. It posts a real GitHub PR review with a verdict and file-level findings — but it is strictly read-only: it never edits code, never runs git/gh write commands, and never merges anything.

Type: LLM (Claude) · Scripts: .autoducks/agents/reviewer/{pre,post}.sh · Verb: review

flowchart LR
  Triggers@{ shape: bolt, label: "Triggers" }
  Reviewer["Reviewer"]
  PR@{ shape: notch-rect, label: "PR review\nverdict + findings\nlabel: Review:done or Review:changes" }

  Triggers --triggers--> Reviewer
  Reviewer -- posts --> PR

  class Triggers triggersOrange
  class Reviewer reviewTeal

  classDef triggersOrange fill:#ffe8d4,stroke:#d66a28,color:#d66a28
  classDef reviewTeal fill:#d4f4f8,stroke:#0e8a9c,color:#0e8a9c
EventCondition
Issue comment/review on the feature/bug issue, or directly on its pull request
Pull requestready_for_review — fires automatically when a final feature//fix/ PR (base = integration branch) is marked ready. Task PRs, which target a pipeline branch, are excluded.
Workflow dispatchpr_number or issue_number

The Reviewer still sits outside the four-agent pipeline (no Definition-of-Ready cascade), but on a final PR it now runs automatically the moment the Maestro marks it ready-for-review — the same run you can also trigger manually with /review. The two entry points are serialized per PR (workflow concurrency), so a PR is never reviewed by two concurrent runs.

A reviewable feature/bug PR must exist. The Reviewer resolves the target PR itself:

  • Commented directly on a PR → reviews that PR.
  • Commented on the feature/bug issue → looks up the open pipeline PR for that issue’s branch.

If no open PR is found, the diff is empty, or the PR is already closed/merged, the run posts “Nothing to review”, clears the in-progress label, and exits green — this is not treated as a failure.

  1. Reacts 👀, posts the status comment, applies Review:reviewing — the label and status comment are mirrored to both the feature/bug issue and its PR.
  2. Resolves the target PR and the feature/bug issue it implements, then gathers context per context.reviewer.parts — which pieces are populated is configurable per repository; see Curated design context below for the default.
  3. [LLM] Explores the repository read-only (Read, Grep, Glob, read-only Bash — no write tools) and writes /tmp/review.md: a verdict line, a plan-conformance checklist judging the diff against the design’s proposed solution and constraints, scope/boundary notes, severity-ranked findings (blocker/major/minor/nit) with file:line citations, and a summary.
  4. Submits the review to the PR via the Git provider (git::submit_pr_review).
  5. Review:reviewingReview:done (verdict approve or comment) or Review:changes (verdict request-changes). Edits the status comment to ✅ with the next step; continues the #auto: chain if one is queued.
  6. On request-changes, if review.auto_rework is enabled (the default), auto-dispatches a headless rework round instead of waiting on a human — see Bounded auto-rework loop.

Which context files are populated is controlled by context.reviewer.parts — see the part catalog for every available part. This repository’s own autoducks.json ships a curated manifest: issue_title, design.problem_statement, design.proposed_solution, design.constraints, design.out_of_scope, pr_diff, pr_meta, and security_guidelines — the individual design-section extracts rather than the whole design zone or a separate task-acceptance-criteria file. This narrows the Plan conformance judgment (see Output contract below) to the design’s proposed solution and constraints specifically, rather than a broader mix of the full design plus every task’s acceptance criteria.

A project that omits context.reviewer entirely falls back to the Reviewer’s built-in default manifest (issue_title, issue_description, task_criteria, design.full, pr_diff, pr_meta, security_guidelines) — see Default manifests.

The Reviewer treats code comments, docstrings, TODOs, and assertions in the PR body as claims to verify against the actual code, never as evidence on their own that the described behavior is implemented. A comment saying // validates input or a PR description saying “adds rate limiting” only counts once the LLM has read the referenced code and confirmed it does what the comment/description claims. This closes off a class of false-negative reviews where a plausible-sounding comment (accurate or not, written by a human or a prior agent run) is taken at face value instead of checked.

The LLM writes exactly one word to /tmp/review-verdict, which post.sh maps to a GitHub review event:

VerdictMeaningLabel appliedGitHub review eventCheck-run
approveNo findings above nit, and the proposed solution and every constraint are metReview:doneCOMMENTsuccess
commentOnly minor/nit findings, or a partially met proposed solution/constraint with nothing blockingReview:doneCOMMENTsuccess
request-changesAt least one blocker/major finding, or the proposed solution or any constraint is missingReview:changesREQUEST_CHANGESfailure

A missing or malformed verdict file falls back to comment, the conservative middle ground.

On a final PR the Reviewer also emits a GitHub Check-run (default name Autoducks: Reviewer) whose conclusion mirrors the verdict — see the last column above. Only request-changes concludes failure; approve/comment, an empty diff (“nothing to review”), all conclude success, so the gate stays advisory and never deadlocks a PR. A run that errors concludes failure.

To make it block merges, set reviewer.required_check: true in autoducks.json and run scripts/setup.sh (with repo-admin gh credentials): it creates a branch ruleset requiring that check on the integration/base branch. Because the check is pinned to the head commit it ran against, pushing new commits after a review leaves it pending on the new SHA — re-run /review to revalidate.

Security is a dedicated review dimension, not a separate pass or output file — findings surface as ordinary entries in Findings with a security tag, the standard severity, file:line, and a concrete fix.

pre.sh stages the repository’s optional guidelines file at /tmp/security-guidelines.md (empty if none exists). The LLM reads it alongside the diff and, where the repository provides its own guidelines, applies them with priority. It then falls back to a built-in baseline checklist to cover any classes the repository’s guidelines didn’t already enumerate:

  • AuthZ/AuthN (including bypassing the Authorization Gate)
  • Injection (shell, SQL, path, template, prompt)
  • Secrets (hard-coded credentials, tokens leaked to logs or comments)
  • SSRF / path traversal
  • Deserialization / unsafe eval
  • Crypto misuse
  • Unsafe defaults & scope (over-broad CORS/permissions)
  • Dependencies from untrusted sources

Only the diff (and the surrounding code needed to confirm a finding) is judged — pre-existing issues the PR doesn’t touch are out of scope, same as every other finding category.

The verdict mapping is unchanged: an unaddressed exploitable vulnerability is at least a major finding, which — like any other blocker/major finding — maps to request-changes. See Configuration for how to point the Reviewer at a non-default guidelines path, and Security for how this differs from the Authorization Gate.

The Reviewer is constrained on both sides:

  • Prompt-level: the LLM’s allowed tools are Read, Grep, Glob, Write, and read-only Bash (including read-only git/gh exploration such as git log/git diff/gh pr view/gh pr diff) — no Edit, and no mutating git/gh. It may only Write to /tmp/review.md and /tmp/review-verdict.
  • Script-level: pre.sh/post.sh never check out a branch to modify, never commit, and never call a merge function — the only Git-provider write is posting the review itself.

When review.auto_rework is enabled (the default), a request-changes verdict does not stop and wait for a human — the Reviewer immediately dispatches a headless /rework round on the PR itself, then re-runs /review once the rework lands, repeating until the PR clears (approve/comment) or the round cap is hit.

Round tracking. The current round number for a (feature, PR) pair lives in a single marker-anchored comment on the PR (<!-- autoducks:review-loop: feature=<F> pr=<P> iteration=<N> max=<M> -->), maintained by core/orchestration/review-loop.sh. This is stateless across runs — any workflow run can recover the round by reading the marker, edited in place rather than duplicated on every round.

Bound and hand-off — no silent merges. Each request-changes verdict increments the round counter and re-dispatches /rework only while iteration < review.max_iterations (default 3, clamped [1, 10]). Once the cap is reached, the automatic loop stops without merging or approving anything — the PR stays blocked (Review:changes, and a failure Check-run when reviewer.required_check is on), and the status comment’s Next line falls back to the manual /rework or /defer instructions, plus a note that the iteration cap was hit. A PR can only ever leave the loop via an explicit approve/comment verdict or a human’s own action — the loop never merges on its own.

On the rework side, a headless auto-dispatch (no triggering comment, so there’s no thread for a human to notice a quiet “nothing to rework” on) that finds nothing actionable still posts an explicit human handoff — “Nothing to rework — PR #<n> is still blocked,” with the same /rework//defer next steps — rather than finishing silently green. See /rework for the full behavior.

The status comment’s Next line depends on the verdict:

VerdictNext line
approveMerge PR #<n> when you’re ready — the bot review is informational only (posted as a comment, not a formal approval).
commentRun /rework to address the findings on this PR now, or /defer to save them as a follow-up issue and merge as-is.
request-changes, auto-rework dispatchedA round was just auto-dispatched: 🔁 Auto-rework round <n>/<max> dispatched. — no action needed.
request-changes, auto-rework off or cappedRun /rework to address the findings on this PR now, or /defer to save them as a follow-up issue and merge as-is (plus a cap-reached warning, if applicable).

/review itself never changes code or blocks a merge — it only produces a verdict and findings. Turning those findings into action, whether automatically (above) or by hand, is a separate step:

  • /rework distills the unresolved feedback into one follow-up task, reverts the PR to draft, and re-dispatches execution so the Maestro/Developer builds the fix.
  • /defer captures the same feedback as a standalone follow-up issue instead, leaving the PR untouched so you can merge or close it immediately.

Both read the PR’s reviews and comments directly — neither requires /review to have run first.

The Reviewer runs after the pipeline (or a task) has already produced a PR — it’s a post-execution gate you invoke explicitly with /review, not a stage the Maestro dispatches automatically. Like the utility commands, it has no Definition-of-Ready cascade; unlike them, it does carry its own stage labels (Review:reviewingReview:done/Review:changes); the label and status comment are mirrored to both the feature/bug issue and its PR, so review state is visible on either.

/review model:opus effort:max turns:40

/review can carry a #auto: chain forward, but only to the chainable verbs (architect, engineer, execute, review) — utility verbs (fix/revert/close/rework/defer) cannot be chained. See Slash commands for the full grammar.