Incremental review has never worked.
The bug
prepareReview embeds the state marker in the review summary body. parseReviewState is only ever called on pr.body — in runReviewForPR, in the @buddy comment handler, and in the CLI. Nothing copied it across.
So reviewedSha and fingerprints were written on every review and read on none.
The consequences were quiet rather than loud:
skipIfReviewednever skipped — a scheduled run re-reviewed a commit it had already seen- fingerprint dedupe never engaged — every push repeated findings already posted
Only @buddy pause behaved, because setPaused writes to pr.body directly. That is also the proof that writing there is acceptable, and what this fix follows.
The doc comment on
serializeReviewStatealready says "Stored on the pull request rather than in a database". The intent was never ambiguous — the write just went to the wrong body.
The fix
upsertReviewState(body, state) replaces the marker in a pull request body. Both review paths call it once the review has landed, and setPaused now shares it instead of carrying its own copy of the strip-and-append regex.
Persistence is wrapped and demoted to a warning: the review is already posted by that point, so a token without write access to the description costs a repeated review next run rather than a lost one.
prepareReview now returns the state it computed rather than only baking it into the summary. The summary keeps its copy as a record of what that particular review saw; only the copy on the PR is read.
A second bug, found on the way
The CLI review path — the one the generated CI workflow uses — set headSha from state?.reviewedSha, the previously reviewed commit rather than the one in front of it. Even once the marker was persisted, it would have recorded the wrong sha.
It now asks the provider for the head sha, as runReviewForPR already did. That closes one of the two places these paths had drifted apart.
Tests
Six, covering the round-trip that did not exist:
- state survives a write to a pull request body and reads back
needsReviewthen answers correctly for the same and a later commit- a second write replaces the marker rather than stacking a second one
- persisting a review does not silently resume a paused pull request
- an empty pull request body is tolerated
The existing test asserted parseReviewState(prepared.body) — that is, it pinned the broken design, which is part of why this survived.
Verification
- 1896 pass / 0 fail (+6), lint clean, typecheck clean
check:docsgreen: 75 pages, 33 commands, 221 config keys
🤖 Generated with Claude Code