fix(seidroid-review): recheck the head before withdrawing, and find a batch review that landed without an answer

[bdchatham]

Summary

This fixes the two Cursor Bugbot findings on #115. The fix was pushed after #115 had already merged, so it didn't make it in.

1. Stale head state. Since fix(seidroid-review): take the review's position on the review that carries the verdict, not a second one #115, head_state is read in "Decide the review's position", before placement. A push after that read and before the position step could still let the withdrawal clear a block. The position step now reads the head again when HEAD_STATE is same:

   now == REVIEWED_SHA -> same;  other sha -> moved;  empty/garbage -> unknown
   

   moved and unknown still skip the withdrawal.

2. Ambiguous batch writes. If the batched review POST fails with a non-4xx status or no response, the review may still have landed. The batch body now ends with a hidden <!-- seidroid-run:$GITHUB_RUN_ID-$GITHUB_RUN_ATTEMPT --> marker. On an ambiguous failure, placement lists the PR's reviews and looks for one whose body == review_body and commit_id == head_sha. Because of the run marker, that can only be this attempt's own review.

   • If it finds one, it treats the batch as landed. It sets review_id to the found id and sets position_posted. So the verdict step appends to that review, and the position step posts no second review.
   • If it finds none, the fallback is unchanged: findings go to the summary and the position step records the position.

Validation: actionlint with shellcheck gives the same 8 infos as main. I ran the scripts against a stubbed gh. When a 502 hid a review that did land, the run edited that review and posted nothing else. When a 502 hid a review that did not land, the run posted exactly one position review.

Link to Devin session: https://app.devin.ai/sessions/9e65f0b138c84a248927a0aac3c24060
Open in Devin Desktop: https://app.devin.ai/desktop/session/9e65f0b138c84a248927a0aac3c24060?variant=devin
Requested by: @bdchatham

[devin-ai-integration]

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

• Disable automatic comment, CI, and merge conflict monitoring

[cursor]

PR Summary

Medium Risk
Changes when PR reviews and blocking positions are posted in CI; misbehavior could duplicate reviews or leave blocks in the wrong state, but scope is limited to the review workflow.

Overview
Hardens the seidroid-review GitHub Action so placement and position steps behave correctly when the API response is missing or the PR head moves mid-run.

Stale head before withdrawal: When the position step still believes head_state is same, it re-fetches the PR head SHA. If it no longer matches REVIEWED_SHA, state becomes moved (or unknown on a bad read), so a push between “decide position” and posting does not incorrectly clear a block.

Ambiguous batch review POST: Batched review bodies now include a hidden seidroid-run marker tied to the workflow run. If the create-review call does not succeed but the HTTP status is not 4xx, the workflow lists existing reviews and treats a match on body + commit as a successful batch. It reuses that review id for verdict append and avoids posting a duplicate position review when the write actually landed (e.g. after a 502).

^{Reviewed by Cursor Bugbot for commit 384281e. Bugbot is set up for automated code reviews on this repo. Configure here.}

[seidroid]

Re-reading the head before the withdrawal is correct and closes most of the race. The new recovery for an ambiguous batch POST is useful, but its rule for deciding which review "landed" is too loose: it can pick up a review from an earlier run, and when it does, this run's findings are silently dropped.

Findings: 0 blocking | 2 non-blocking | 1 posted inline

Blockers

• None at the file/PR level.

Non-blocking

• No test or fixture covers the new ambiguous-POST recovery path. It is only checked by hand against a stubbed gh, so a regression here, such as matching the wrong review or appending to it twice, would go unnoticed.
• 1 suggestion(s)/nit(s) flagged inline on specific lines.

[seidroid]

[suggestion] Matching only on body == review_body and commit_id == head_sha can select a review that this POST did not create. When the driver writes no verdict, review_body is the fixed string "Findings on the changed lines. The verdict and the summary are in this tool's comment…", so any earlier run on the same commit also matches (a re-run, or a rerun after a failed attempt). The same happens if a verdict repeats, or another identity posted the same text. On a false match the run:

• sets landed=true and drops the anchored findings, which reach neither the diff nor the summary,
• appends the linkage for comments that were never posted, so later superseding and resolving act on the wrong threads,
• sets position_posted, so the position step skips its review, and
• has the verdict step append to the old review.

To narrow it:

1. Record the highest existing review id (or the current time) before the POST, and only accept .id > $before / submitted_at >= $start.
2. Filter on .user.login matching the posting identity.
3. Preferably, embed a per-run nonce (for example $GITHUB_RUN_ID-$GITHUB_RUN_ATTEMPT) in a hidden HTML comment in review_body and match on that.

(Codex raised the same point.)

[devin-ai-integration]

Fixed in 384281e. The batch review body now ends with a hidden <!-- seidroid-run:$GITHUB_RUN_ID-$GITHUB_RUN_ATTEMPT --> marker. The body == review_body match therefore only finds this attempt's own review, not an earlier run's, even for the fixed no-verdict text. The marker comes after the text, so the body still opens with VERDICT_MARKER. On the missing test: uci has no workflow test harness yet, so that would be a separate piece of work.

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 384281e. Configure here.}

[cursor]

Reviews lookup omits page size

Medium Severity

The 5xx recovery lists PR reviews without ?per_page=100 in the path, so GitHub pages at the default 30. This collection is oldest-first, and the review that just landed is last. A short or incomplete list misses it, so placement treats the batch as unlanded and the leftover position POST can still record a second vote.

 

^{Triggered by learned rule: seidroid-review: VERDICT_MARKER via $ENV; GET page size in path}

^{Reviewed by Cursor Bugbot for commit 384281e. Configure here.}