PR review tooling models a review as a submit action, not as a durable artifact #7

Found while using an agent to prepare a 9-comment review on a large PR, then correcting it after an adversarial verification pass. The agent could author review text. It could not read back what was staged, could not converge on a corrected version, and had no way to carry the review into the re-review that follows the author's next push. One claim reached the submitted review carrying text that had already been retracted, because the handoff was copy-paste.

The through-line: a review is a durable artifact with a lifecycle. The tooling models it as a single submit action.

What the workflow needs

One review object, the same shape read and written, at every stage. Whatever pr_review_submit accepts, a read tool should return: body, event, and comments with their anchors, whether the review is a pending draft, freshly submitted, or months old. Verification then becomes a diff between what was sent and what came back. Every wrong conclusion in this session traces to those two shapes not matching, which forced interpretation where comparison would have done.

One anchor vocabulary. path, line, start_line, side. position should not appear in a response at all. It was the only anchor field populated on the read side, and it is the one field that cannot be converted back to a file line, so it invites exactly the reverse-engineering that produced two false alarms here.

Declarative apply over the whole review. Hand it the final set of comments plus the body; it converges. No create-then-patch distinction visible to the caller, no change in call shape before and after submit. Idempotent, so re-running after a correction pass yields the corrected review rather than a duplicate. Deletion of comments absent from the input stays opt-in, since it is the one destructive move.

Anchor validation as a dry run. Given a path and a line range, is that anchorable against this head. Offered as a validate mode on apply, so a bad anchor is rejected before anything is created. Both false alarms in this session were an attempt to answer this question by hand from the raw diff.

Re-review as a first-class case. Each comment carries the head sha it was written against. After the author pushes, the questions are: which comments sit on lines that changed since that sha, which threads are resolved, which are outdated, which are untouched. Without this, a second pass re-reads every changed line and re-derives findings it already holds.

Threads, not just comments. Read replies, distinguish resolved from unresolved from outdated, reply into a thread, resolve one. A review is a conversation that persists across pushes; the current model is a write-once batch.

Concrete gaps behind this

These are the specific things that blocked the session. They are symptoms of the framing above, and each is independently fixable.

A pending review cannot be read back

pr_reviews maps to GET /pulls/{n}/reviews, which lists a pending review but returns body: "" for it. Its comments come only from GET /pulls/{n}/reviews/{id}/comments, and for a pending review that payload is stripped: line, start_line, side, start_side and subject_type all return null. position and diff_hunk are populated. GET /pulls/comments/{id} returns 404 for a pending comment.

So the question "is this comment anchored where I intended" has only half an answer. The last line of diff_hunk shows the line a comment ends on. The start line of a multi-line comment is not exposed; in the original session it was settled by the user pasting a screenshot showing Comment on lines R14 to R16.

pr_comments maps to GET /pulls/{n}/comments, which does not return pending comments: on shopware/shopware#20270, with one pending review holding 16 inline comments, it returned none. No read tool takes a review id, so api_read on GET /pulls/{n}/reviews/{id}/comments is the only tool route to them.

That route closes when a project sets block_api_tool_read: true in .mcp-gh-tooling.json and has not set enforce_mcp_tools: false. plugins/github-mcp/hooks/scripts/check-api-tools.sh:70 (at 8b40acb) matches pulls/[0-9]+/reviews without an anchor, so it also catches pulls/{n}/reviews/{id}/comments, blocks the call and points to pr_reviews, which cannot return those comments. No bats test covers reviews/{id}/comments; check_api_tools.bats:48-53 tests only pulls/123/reviews. The flag defaults to false (README.md:134).

A rejected submit can leave an empty pending review behind

On shopwareLabs/ai-coding-tools#128, a pr_review_submit call with one inline comment on a line outside the diff was rejected with 422 Unprocessable Entity and Line could not be resolved. The tool sends a single POST /pulls/{n}/reviews with the event set (plugins/github-mcp/mcp-server-gh/lib/review_write.sh:119 at e7ab864). Afterwards the web UI showed a started review on the caller's account that nobody had begun on purpose. GET /pulls/{n}/reviews did not list it while it was empty; it appeared, as PENDING, once a comment had been added to it in the UI. That the rejected call created it is the likeliest explanation, not a confirmed one. A later pr_review_submit on the same PR succeeded.

Editing inside a pending review

Since #21, comment_edit edits a comment in the caller's pending review. GET /pulls/comments/{id} answers 404 for such a comment, so the tool finds it through GraphQL (reviews(states: [PENDING]), matched on fullDatabaseId) and edits it with the updatePullRequestReviewComment mutation. A pending review's own body cannot be set that way: PUT /pulls/{n}/reviews/{id} on a pending review with an empty body answered 422 with Could not edit a review with a missing body.

The api escape hatch cannot send a request body

GraphQL was the obvious fallback. It is unreachable. The write server's api tool builds its command at plugins/github-mcp/mcp-server-gh/lib/api.sh:75 and adds only -X <method>. No parameter maps to gh api -f / --field / --raw-field, so POST /graphql fails with A query attribute must be specified and must be a string. The same gap blocks every other POST or PATCH needing a body, which leaves the escape hatch read-only in practice whatever the method parameter says.

A fields object expanding to repeated -f key=value, plus --raw-field for values containing newlines (a GraphQL query always does), would unblock this and most of the section above.

pr_review_submit already covers the authoring half, and nothing points at it

It accepts comments[] with path, line, start_line, side, start_side, batches them into one review, and supports suggestion blocks. That is the right tool, and it went unused. The agent produced copyable comment text for a human to paste instead, which is how the stale paragraph reached the submitted review.

Nothing in the read-side tool descriptions mentions it. An agent starting from pr_view and pr_diff and working toward "help me review this" does not encounter pr_review_submit unless it already knows the name. Cross-referencing it from pr_comments, pr_reviews and pr_diff would cost nothing.

position needs a warning until it is gone

It is an index into the file's diff counted from the first @@ header, not a line number, and for a multi-line comment it does not reliably identify either end of the range. One sentence in the pr_comments and pr_reviews descriptions stating that it is diff-relative and must not be converted to a file line would have prevented both false alarms.

Where to start

The api body passthrough is the unblocker; most of the design above needs GraphQL and nothing can reach it today. The pr_review_submit cross-reference is close to free and removes the copy-paste handoff that caused the one substantive error. The position warning is a docs change. The review object and re-review support are the larger work and depend on the first item.