PR / MR Review Workflow¶
review reads a diff and submits a verdict. It is the only writer of the review that is ever posted to the forge, and the only thing it is ever posted as is a COMMENT - GitHub 422s the platform's own bot identity on both APPROVE and REQUEST_CHANGES for a self-authored PR (only COMMENT is permitted), so the decision that actually moves the Task lives in submit_outcome, not in the forge's review-decision field. See Merge and Deploy for what the operator does with an accepted verdict.
Trigger¶
Two independent paths feed review Tasks against a human's PR:
- Webhook: a pull request is opened or synchronized on an enrolled repository.
mrScancron: a periodic scan that lists open PRs/MRs on enrolled repos and picks up any candidate the webhook missed, so a dropped or delayed webhook is not a permanent gap.
Both paths apply the same scope check via spec.scm.prReactionScope:
| Value | Behavior |
|---|---|
| Empty / unset (the default) | Reacts to every open human PR/MR - the historical, permissive behavior |
labeledOrMentioned | Only PRs with the triggerLabel OR that mention the bot |
all | Every PR in every enrolled repository (equivalent to the unset default, explicit) |
Default is permissive, not labeledOrMentioned
The default prReactionScope is empty, which reviews every open PR/MR. Set prReactionScope: labeledOrMentioned explicitly to restrict review to labeled/mentioned PRs.
The PR author must not be the bot itself: a review-kind Task (spec.kind: review) is always opened against someone else's PR, by construction. The platform's own MRs are reviewed at the awaiting-review state of the Task that opened them - a different state of an implement- or refine-originated Task, never a review-kind Task.
Re-review dedup¶
mrScan (and the webhook path) suppress re-review of a PR/MR whose head commit has not changed since the last completed review. Same head + a terminal review Task already exists on that PR -> suppressed. A new commit pushed to the PR changes the head SHA -> re-review proceeds.
Read-only constraint¶
The review agent never pushes and never merges, and never calls a merge API - no MCP tool exposes one to any agent kind. The PR head is checked out in /workspace read-only. Review's only writeback actions are mr_write(comment) / mr_write(reply) - replying to a human's inline thread - plus submit_outcome itself, which the operator turns into the posted review and, where applicable, the merge.
submit_outcome¶
or {"verdict":"request_changes",
"reviewed_shas":[{"repo":"tatara-operator","number":295,"sha":"a3f912c..."}],
"change_significance":"minor",
"findings":[{"repo":"tatara-operator","number":295,"path":"internal/x.go","line":42,
"body":"...","severity":"high"}]}
reviewed_shasis required on both verdicts - the exact head SHA the agent actually checked out and read, per MR. The operator re-reads the live head at acceptance and refuses the verdict if it moved while the agent was reviewing, so anything pushed after checkout never merges unreviewed under this approval.findingsis required (at least one) whenverdict=request_changes. The operator posts these as the SCM review and its inline comments - the agent does not post them itself.mr_writehas noapproveand norequest_changesaction; review keepsmr_writeforcomment/replyonly, so it can still reply on a human's inline thread.change_significanceis optional and may only escalate the levelimplementalready declared -max(implement, review)overpatch < minor < major. A lower value is ignored and logged WARN.- There is no
commentverdict. A review either approves (merge proceeds, on the platform's own MRs) or requests changes (loop back tounder-implementation). A non-decision has no stage to go to.
Transitions¶
| From | Outcome | To |
|---|---|---|
awaiting-review | verdict=approve, spec.kind != review | merged - operator posts a COMMENT review from the verdict, then merges (gated on pendingReview == nil) |
awaiting-review | verdict=request_changes, spec.kind != review | under-implementation - no longer capped by reviewRounds < maxReviewRounds; that cap and review-loop-exhausted are retired (see below) |
awaiting-review | either verdict, spec.kind == review | parked(awaiting-human) - see the carve-out below |
The review-kind carve-out¶
On a review-kind Task - one minted for a human's PR, never the platform's own - neither verdict advances the Task. approve parks it at awaiting-human, because merging a human's PR is a human action. request_changes also parks it at awaiting-human, because a human's PR is fixed by the human, not by an implement pod spawned against code the platform did not write. The review is posted either way - the operator still runs PostReview(COMMENT) from the verdict.
A review-kind Task never spawns an implement pod and never reaches merged - there is no edge, by any path. This holds regardless of which verdict comes back: request_changes is the review agent's normal verdict on a bad human PR, so it is the primary path that has to be closed, not an edge case. The next human comment on the thread un-parks the Task back to awaiting-review, bounded by status.humanReviewRounds (cap 5, a separate counter from reviewRounds, which only advances on request_changes) - so a chatty thread cannot spawn an unbounded run of review pods. That cap is skipped, and no round is spent, when any owned MR is currently externally owned (a maintainer pushed a commit, or never delegated the MR to tatara in the first place) - see Handing an MR to tatara. The cap exists to bound ordinary review back-and-forth, not to leave a maintainer's take-over comment on a stood-down, round-capped MR unable to spawn a pod at all.
Adopted merge requests¶
A merge request a dependency engine (Renovate) opens on its own can be adopted: bound to its own upgrade-kind Task that enters review directly, skipping an implement turn entirely. See Upgrade: adopting the engine's own merge requests for what arms it (upgradePolicy.adoptBranchPrefix + author match) and MR Ownership for how it lands at tatara ownership without a takeover request.
An adopted Task is kind=upgrade, not kind=review - the carve-out above does not apply to it. The dangerous misreading is treating "not tatara's own MR" as a reason to fall into the human-PR row: an approve reasoned that way merges a third party's merge request. Instead, both verdicts behave exactly as they do on any other platform-opened MR: approve goes to merged, request_changes goes to under-implementation where the upgrade agent - not a separate implement pod - pushes complementary commits onto the engine's own branch for another round.
Conversation persistence for reviews¶
Each PR gets its own conversation, distinct from any related issue's conversation. If the PR is synchronized (new commits pushed), the next review turn resumes from the prior conversation, giving the agent context about what it already reviewed.
Budget and cycle caps¶
awaiting-review carries an idle-clock budget (60m default, Project.spec.scm.conversationIdleMinutes); on elapse the Task parks at parked(awaiting-human). reviewRounds, tracked on the MergeRequest, still counts accepted request_changes verdicts, but the maxReviewRounds cap that used to bound the awaiting-review <-> under-implementation loop at it is deprecated with zero effect (tatara-operator#582) - the loop is now bounded only by the 24h residency cap. See Merge and Deploy for the merge-side cycle caps that pick up once a verdict reaches merged.
CI readiness gate on approve¶
Since tatara-operator#594, an approve verdict is subject to the same readiness gate as an implement submission: if the owned MR has red CI or a real base conflict, the outcome is refused with a structured 409 before anything is written. This one is not dropped on an unanswered request_changes - the reviewer cannot push a fix itself, and the reviewer is documented-flaky, so a round that overturns a previous round's findings is legitimate. See CI readiness gate.
Inherited workspace hazard¶
Where the project has the persistent workspace PVC enabled, a review pod for an implement-originated Task inherits the same /workspace the implement stage used - not a fresh clone, because implement and review are sequential stages of one Task on one pod name, never concurrent writers. The read-only constraint above is a platform convention enforced by which MCP tools are exposed, not a filesystem permission: anything the review agent edits in that inherited tree is committed and pushed at turn end, landing in the very MR it is judging, attributed as if the implementer wrote it. The review agent is told this explicitly whenever the pod actually mounts the volume. This does not apply to a review-kind Task against a human's PR - that checkout is always fresh and read-only.