Reviewer
/reviewThe 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
Triggers
Section titled “Triggers”| Event | Condition |
|---|---|
| Issue comment | /review on the feature/bug issue, or directly on its pull request |
| Pull request | ready_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 dispatch | pr_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.
Definition of Ready
Section titled “Definition of Ready”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.
Behavior
Section titled “Behavior”- Reacts 👀, posts the status comment, applies
Review:reviewing— the label and status comment are mirrored to both the feature/bug issue and its PR. - 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. - [LLM] Explores the repository read-only (
Read,Grep,Glob, read-onlyBash— 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) withfile:linecitations, and a summary. - Submits the review to the PR via the Git provider (
git::submit_pr_review). Review:reviewing→Review:done(verdictapproveorcomment) orReview:changes(verdictrequest-changes). Edits the status comment to ✅ with the next step; continues the#auto:chain if one is queued.- On
request-changes, ifreview.auto_reworkis enabled (the default), auto-dispatches a headless rework round instead of waiting on a human — see Bounded auto-rework loop.
Curated design context
Section titled “Curated design context”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.
Comments-are-claims
Section titled “Comments-are-claims”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.
Output contract
Section titled “Output contract”The LLM writes exactly one word to /tmp/review-verdict, which post.sh maps to a GitHub review event:
| Verdict | Meaning | Label applied | GitHub review event | Check-run |
|---|---|---|---|---|
approve | No findings above nit, and the proposed solution and every constraint are met | Review:done | COMMENT | success |
comment | Only minor/nit findings, or a partially met proposed solution/constraint with nothing blocking | Review:done | COMMENT | success |
request-changes | At least one blocker/major finding, or the proposed solution or any constraint is missing | Review:changes | REQUEST_CHANGES | failure |
A missing or malformed verdict file falls back to comment, the conservative middle ground.
Required check (gating merges)
Section titled “Required check (gating merges)”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 review
Section titled “Security review”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.
Read-only, never-merge
Section titled “Read-only, never-merge”The Reviewer is constrained on both sides:
- Prompt-level: the LLM’s allowed tools are
Read,Grep,Glob,Write, and read-onlyBash(including read-onlygit/ghexploration such asgit log/git diff/gh pr view/gh pr diff) — noEdit, and no mutatinggit/gh. It may onlyWriteto/tmp/review.mdand/tmp/review-verdict. - Script-level:
pre.sh/post.shnever check out a branch to modify, never commit, and never call a merge function — the only Git-provider write is posting the review itself.
Bounded auto-rework loop
Section titled “Bounded auto-rework loop”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.
Next: rework or defer
Section titled “Next: rework or defer”The status comment’s Next line depends on the verdict:
| Verdict | Next line |
|---|---|
approve | Merge PR #<n> when you’re ready — the bot review is informational only (posted as a comment, not a formal approval). |
comment | Run /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 dispatched | A round was just auto-dispatched: 🔁 Auto-rework round <n>/<max> dispatched. — no action needed. |
request-changes, auto-rework off or capped | Run /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:
/reworkdistills the unresolved feedback into one follow-up task, reverts the PR to draft, and re-dispatches execution so the Maestro/Developer builds the fix./defercaptures 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.
Where it sits
Section titled “Where it sits”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:reviewing → Review: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.
Overrides
Section titled “Overrides”/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.