workflows(pr-review): one review comment per PR, edited in place

Merged
KH
2mo ago
+601-26

Closes the "one comment per wake" hole: the reviewer now keeps one comment per PR. The GitHub step is a small Node script (built-in fetch, no more shell-parsed JSON): it walks the PR's comments (paginated, hard cap → fail, never post), finds the bot's <!-- flows-pr-review <sha> --> comment, skips when it already carries this head SHA (re-dispatch safe), PATCHes it with the new verdict when the SHA moved, and POSTs only when none exists. Body bounded from the front under GitHub's limit.

Exercised for real against closed PR #478 with my token: post → skip (same SHA) → update (new SHA) — exactly one comment; test comment deleted afterwards.

This PR is itself the first live run of the deployed reviewer with a working comment step (deployment flows-pr-review, stored copy = #479's version); it also answers whether the PR-head grant can write issue comments. After merge: redeploy so the stored copy edits in place.

Still assumes GH_TOKEN in the sandbox rather than a helper mount — cloud #3812 tracks giving authored runs the relayfile mounts.

🤖 Generated with Claude Code


[!NOTE]
Medium Risk
Changes live GitHub comment behavior for the repo’s automated PR reviewer, including ordering and race logic; mistakes could post duplicates, skip updates, or leave stale verdicts visible.

Overview
The flows-pr-review workflow no longer posts a new GitHub comment on every wake. It keeps one review comment per PR and PATCHes it when the head SHA changes, skipping when the marker already matches the current head.

Comment delivery moves from shell curl + grep into workflows/pr-review-post.cjs (Node fetch), embedded verbatim in pr-review.flow.ts at run time. The script tags comments with <!-- flows-pr-review <sha> by:<login> -->, ties ownership to the real author (ignores spoofed markers), adopts legacy markers, dedupes races, and uses GitHub compare / commit dates so older wakes never overwrite newer verdicts (including after rebases). Transient API failures are conservative: no accidental overwrites.

packages/sdk/tests/pr-review-post.test.ts exercises the publisher against a fake GitHub API and asserts the flow embed stays in sync. review/ is gitignored for workflow scratch artifacts; workflows/README.md documents the in-place behavior.

Reviewed by Cursor Bugbot for commit 62a72a4c0d70fac7823eb6e6adc41966fc5258b4. Bugbot is set up for automated code reviews on this repo. Configure here.


Summary by cubic

Previously, each reviewer wake could add a new PR comment; now the reviewer keeps one comment per PR, edits it for new heads, and refuses older wakes so the PR shows only the newest verdict.

  • Embeds workflows/pr-review-post.cjs in the flow and covers it with a fake GitHub API test suite.
  • Scans paginated comments, skips the current head SHA, PATCHes changed heads, and POSTs only when no owned marker exists.
  • Re-reads before writing; compare results and commit dates resolve races and rebases. Transient API failures stop the write, while 404s mean a head loses or a vanished comment is re-posted.
  • Matches marker logins to comment authors, ignores spoofed markers, adopts legacy markers, and replaces temporary by:pending markers with the actual author login.
  • Removes duplicate markers, keeping the comment with the newest head, and leaves duplicates alone when ordering is uncertain.

Migration

  • Redeploy the workflow after merging so the stored copy uses in-place comment updates.