feat(seidroid-review): answer the request with the verdict, and drop a redundant label

[bdchatham]

The verdict now reaches the request

A requested review opens with 👀 and then says nothing on the request itself.
The person who asked has to open the run, or scroll for the verdict comment, to
learn how it ended.

It now reacts on the triggering comment:

position recorded	reaction
APPROVE	👍
REQUEST_CHANGES	👎
none	(nothing)

Together with the 👀 the job opens with, the pair separates a review still
running from one that decided.

Keyed on the position, not on the job succeeding. A run that finishes
without reading the diff is not an approval and must not wear one — that is the
credential-outage shape, where the workflow goes green and the review read
nothing. A conclusion that records no position gets no reaction, which is the
honest answer: the verdict comment carries what it found.

Comment path only — an automatic review has no comment to react to — and never
fatal, like the 👀 before it.

The ai-review tooling this replaced did the same thing:

Explicit requests receive a best-effort 👀 reaction while the review runs and
👍 when it completes successfully.

This restores the second half, with the trigger narrowed from "the job
completed" to "the review took a position".

A redundant label on inline findings

Inline findings render as:

**review · suggestion** — the error message implies amm has no knobs

Every one of them is a review comment on a review's own pull request, so the
word says nothing the surrounding context does not. The severity leads now:

**suggestion** — the error message implies amm has no knobs

Nothing matches on the removed text. Threads carry forward on the
<!-- seidroid-review:finding --> HTML marker, which is untouched.

Verification

actionlint reports the same 5 pre-existing findings as the base
(lines 326/1001/1183); none from this change. YAML parses.

The reaction needs a live comment-triggered review to confirm, since it depends
on needs.guard.outputs.comment_id being populated — the same path that proved
the 👀 on
sei-load#96.

🤖 Generated with Claude Code

[cursor]

PR Summary

Medium Risk
Changes merge-gate behavior (check run hard-fail without SHA, withdrawal can fail the job) and how reviews attach to commits mid-push; mistakes could leave PRs blocked or publish against the wrong semantics.

Overview
Publishing is tied to the commit recorded before the review runs. Downstream steps no longer re-fetch the PR head or fall back to “current head” when that record is missing. Inline comments are skipped without a SHA (findings roll into the summary with a clearer header); the review check run fails the step if there is no recorded commit, with continue-on-error only when a SHA exists.

Comment-triggered requests get a verdict on the asking comment. A new Answer the request step clears stale github-actions[bot] thumbs and posts 👍/👎 from the check conclusion (success / failure), not from whether an APPROVE was recorded—so neutral or no-diff runs do not look approved.

Inline finding text drops the redundant review · prefix (**severity** only). Withdrawal of prior CHANGES_REQUESTED reviews is stricter: it uses driver counts to avoid clearing blocks when the run did not really re-review the change, tries every dismiss before failing, and drops continue-on-error on that step so a stuck merge gate fails the job. Posting APPROVE/REQUEST_CHANGES is guarded so API errors do not skip withdrawal.

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

[github-actions]

The 👍/👎 reaction on the triggering comment is a clean, correctly-scoped addition (keyed on the recorded position, comment-path only, non-fatal), and dropping the review · label is safe — the <!-- seidroid-review:finding --> marker, not the label, is what thread recall matches on. One issue: the reaction reuses the reviewed-repo REPO and the target-scoped app token, so it silently no-ops on cross-repository requests.

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

Blockers

• None at the file/PR level.

Non-blocking

• The reaction is never cleared, so a re-run of the same triggering comment that flips its conclusion leaves both 👍 and 👎 on it (reactions are idempotent per (user, content), but the opposite one isn't removed). Deleting the opposite reaction before adding the new one — or accepting the ambiguity explicitly in the comment block — would keep the signal readable across re-runs.
• 1 suggestion(s)/nit(s) flagged inline on specific lines.

[github-actions]

[suggestion] This reuses the step's REPO (review_repo || github.repository) and GH_TOKEN (the app token), but the triggering comment lives in the asking repository, not the reviewed one. On a cross-repository request (@seidroid review owner/other#12) this POSTs repos/owner/other/issues/comments/<id>/reactions for a comment id that belongs to this repo — a 404 swallowed by 2>&1 into the warning branch. Even with the right path it would fail: the identity token is minted with repositories: review_repo_name || github.event.repository.name, so it has no access to the asking repo. So the reaction silently never lands in exactly the case the target grammar exists for ("ask here and read there").

The 👀 step above gets this right — it uses REPO: ${{ github.repository }} and GH_TOKEN: ${{ github.token }} (the job already carries issues: write). Mirroring it here, e.g. a separate TRIGGER_REPO: ${{ github.repository }} plus GH_TOKEN=${{ github.token }} gh api ... for this one call, makes the answer land where the request was made.

[bdchatham]

Correct, and both of you found it. Fixed in 2247e64.

The reaction reused the position step's REPO — the reviewed repository — while TRIGGER_ID names a comment in the asking one. On a targeted request those differ, so it POSTed repos/owner/other/issues/comments/<id>/reactions for an id belonging to this repo, and the 404 landed in the warning branch the step swallows into. The identity token would have refused it regardless, being minted scoped to the reviewed repository.

So the answer never arrived in exactly the case the target grammar exists for.

It now uses TRIGGER_REPO: ${{ github.repository }} and TRIGGER_TOKEN: ${{ github.token }} for that one call, mirroring the eyes step at the top of the job and covered by the same issues: write grant. The log line names the repository too, so a future mismatch is visible rather than silent.

[cursor]

Stale Bugbot comment from a previous run.

[github-actions]

Two small, well-scoped changes to the review workflow: a verdict reaction on the triggering comment and removal of a redundant label on inline findings. No blockers, but the reaction fires in a narrower set of cases than the description claims (nothing at all for a clean review under the default approve-on-success: false), can leave both 👍 and 👎 on a re-run, and is placed after an unguarded API call that can abort the step first.

Findings: 0 blocking | 4 non-blocking | 3 posted inline

Blockers

• None at the file/PR level.

Non-blocking

• Dropping the review · prefix is safe: I checked and no dedupe/lookup logic keys on that string — the <!-- seidroid-review:finding --> marker at line 907 is what identifies these comments, and the unplaced-findings fallback at line 964 already renders severity on its own.
• 3 suggestion(s)/nit(s) flagged inline on specific lines.

[github-actions]

The reaction-on-trigger feature and the inline-label cleanup are both sound in intent, and the label removal is safe (nothing matches on the removed text). Three non-blocking robustness points on the new reaction block: an unguarded pipeline under set -e -o pipefail that can abort the step before the stale-review withdrawal, a reaction-delete filter that is not scoped to the bot's own reactions, and the feature being gated behind the identity-token condition so repos without the app never get the answer.

Findings: 0 blocking | 6 non-blocking | 3 posted inline

Blockers

• None at the file/PR level.

Non-blocking

• The PR description's table and the sentence "Keyed on the position, not on the job succeeding" describe an APPROVE/REQUEST_CHANGES-keyed reaction, but the code keys on the check-run conclusion (success/failure/other), as its own comment at line 1128 states. Concretely, a repo with approve-on-success off records no position yet now gets a 👍. The implementation looks like the intended one — the description is what is stale, and it is what a maintainer will read when deciding whether to enable this.
• The label removal checks out: nothing in the repo matches on review · , and thread carry-forward keys on the <!-- seidroid-review:finding --> marker (lines 742/908), which is untouched.
• 3 suggestion(s)/nit(s) flagged inline on specific lines.
• 1 non-blocking pre-existing issue(s) listed below under pre-existing issues.

Pre-existing issues

• [suggestion] Same set -euo pipefail exposure in the pre-existing withdrawal block at .github/workflows/seidroid-review.yml:1168: if the gh api .../reviews --paginate listing fails, the pipeline aborts the step before any stale CHANGES_REQUESTED review is dismissed. Worth the same || true treatment as the new block above it.

[github-actions]

[suggestion] This pipeline runs unguarded under set -euo pipefail. If the list-reactions call fails (trigger comment deleted, 403, transient 5xx), pipefail makes the pipeline non-zero and set -e aborts the step right here — skipping the reaction POST below and the stale-CHANGES_REQUESTED withdrawal block at the end of the step. That is exactly the failure the new comment at line 1106 says it guarded the review POST against, reintroduced a few lines later: a clean re-review would leave the PR red for a finding that is gone. Suggest ... | while read -r rid; do ...; done || true (or wrapping the whole withdraw-the-opposite block in { ...; } || true).

[github-actions]

[suggestion] The filter selects every reaction of the opposite content on the comment, regardless of who left it, so the delete loop targets humans' 👍/👎 on the request comment as well as the bot's own. GitHub restricts reaction deletion to the reaction's author, so in practice these DELETEs 404 and are swallowed by || true — meaning the loop is either a no-op or, wherever the token does have the rights, it silently clears someone else's reaction. Either way, narrow it: select(.content == \"$opposite\" and .user.login == \"github-actions[bot]\").

[github-actions]

[suggestion] The reaction lives inside "State the review's position", whose if requires steps.identity.outputs.token != '' (line 1042). A repo without SEIDROID_APP_ID still runs a full review and still posts the verdict comment (that step falls back to github.token), but this step never runs — so the requester gets 👀 and then silence, on the exact path where they most need the answer. That is the same reasoning the comment above gives for keying on the conclusion rather than the position. Since the reaction uses TRIGGER_TOKEN (github.token) and not the identity token, it would be more robust as its own step gated only on needs.guard.outputs.comment_id != '' and !cancelled().

[cursor]

Stale Bugbot comment from a previous run.

[bdchatham]

@seidroid review

[github-actions]

Adds a 👍/👎 reaction on the triggering comment and drops the redundant review · label from inline findings; both are sensibly guarded and non-fatal. Three non-blocking issues: the opposite-reaction withdrawal isn't scoped to the bot's own reactions, the reaction rides in a step gated on the App identity token (so identity-less repos never get an answer), and one new warning message can misattribute a failure.

Findings: 0 blocking | 5 non-blocking | 3 posted inline

Blockers

• None at the file/PR level.

Non-blocking

• The PR description's table says the reaction is keyed on the recorded position (APPROVE → 👍, REQUEST_CHANGES → 👎, none → nothing), but the implementation keys on the check-run conclusion and its inline comment explicitly says so ("On the CONCLUSION, not on the position"). The difference is observable: a repo with approve-on-success: false records no position yet still gets 👍. Worth updating the description so it matches the code that ships.
• The removal of the review · prefix is safe as far as I can tell — the thread carry-forward at line 742 matches only on the <!-- seidroid-review:finding --> marker and line 745 strips just that marker, and nothing else in the repo greps the old text.
• 3 suggestion(s)/nit(s) flagged inline on specific lines.

[github-actions]

[suggestion] This selects every reaction whose content is the opposite one, regardless of who left it — not just the bot's own. If a human thumbs-downed the @seidroid review comment (a common way to voice disagreement) and the run concludes success, this deletes their 👎 along with the bot's. issues: write on the caller repo is generally enough to remove other users' reactions, and the || true means it happens silently either way.

Scoping the filter to the reacting identity fixes it and also makes the intent ("withdraw my previous reaction") explicit:

--jq ".[] | select(.content == \"$opposite\" and .user.login == \"github-actions[bot]\") | .id"

Secondary: this list call has no --paginate, so it only sees the first 30 reactions — on a busy comment the bot's own stale reaction can fall off the page and the comment ends up wearing both, which is the exact state this block exists to prevent.

[bdchatham]

This is the one I am glad you caught — it deletes a person's reaction. Fixed in e8bdd0c.

A human who thumbs down an @seidroid review comment is voicing an opinion, and a clean run removed it silently, with || true hiding that it happened. The filter is scoped to the reacting identity now, so the withdrawal only ever touches this bot's own previous reaction. The comment says why, since "delete the opposite reaction" reads as obviously correct until you ask whose.

[github-actions]

[suggestion] The reaction depends only on TRIGGER_TOKEN (github.token), TRIGGER_ID and $CHECK — but it lives inside a step gated on steps.identity.outputs.token != '' (line 1042). Every sibling publisher (lines 901, 996, 1194) falls back to steps.identity.outputs.token || github.token; this one cannot, because posting a review position genuinely needs the App identity.

Consequence: in a repo without SEIDROID_APP_ID, a comment-triggered review still posts its verdict comment but the requester gets 👀 and never an answer — the precise gap this change is meant to close. Consider lifting the reaction into its own small step (if: needs.guard.outputs.comment_id != '' && !cancelled() && steps.drive.outputs.verdict_produced == 'true') so it is not coupled to the identity token.

[bdchatham]

Correct, and it defeated the point of the change. Fixed in e8bdd0c.

The reaction is its own step now — Answer the request — gated on !cancelled() && comment_id != '' && verdict_produced == 'true', with no identity requirement. It reads the conclusion from check.json rather than from the position step's shell, so it needs only github.token and the job's issues: write.

The position step keeps its identity gate, which it genuinely needs: posting a review position is not something GITHUB_TOKEN can do as the bot.

So a repository without SEIDROID_APP_ID now gets an answer, where before it got eyes and silence.

[github-actions]

[nit] || binds to the whole pipeline, so this message also fires when the listing succeeded and a dismissals PUT inside the loop failed on the last iteration — reporting "could not list the reviews" for a failed withdrawal sends the next debugger to the wrong call. Attaching the warning to the gh api ... --paginate list itself (or adding a distinct || echo on the PUT) keeps the two failures distinguishable.

[bdchatham]

Fixed in e8bdd0c. The warning sits on the listing itself now, inside a brace group, so a failed dismissals PUT inside the loop no longer reports a listing failure. The two stay distinguishable for whoever debugs it.

[github-actions]

A small, well-commented workflow change that reacts 👍/👎 on the triggering comment and drops the redundant review · prefix from inline findings; nothing in the repo matches the removed text and the thread-carrying marker is untouched. No blockers, but the new || echo on the review-withdrawal listing feeds its warning into the pipeline instead of the log, and reaction cleanup has a couple of gaps worth tightening.

Findings: 0 blocking | 5 non-blocking | 3 posted inline

Blockers

• None at the file/PR level.

Non-blocking

• The PR description's table says the reaction is keyed on the recorded position (APPROVE → 👍, REQUEST_CHANGES → 👎, none → nothing), but the step actually keys on the check-run conclusion (success/failure), as its own inline comment says. With approve-on-success: false, or in a repo with no App identity (where "State the review's position" is skipped entirely), a clean run still gets 👍 with no APPROVE recorded. The behaviour looks deliberate — the description is what's stale and should be updated so the contract readers rely on matches the code.
• Both newly guarded gh api calls discard stderr (>/dev/null 2>&1), so ::warning::could not record $event and could not react on comment carry no reason. A 422 on self-approval and a transient 5xx become indistinguishable in the log. Capturing stderr into the warning (or letting it through) would keep these paths debuggable without changing the non-fatal behaviour.
• 3 suggestion(s)/nit(s) flagged inline on specific lines.

[github-actions]

[suggestion] This echo writes to the group's stdout, which is the pipeline's input — so on a gh api failure the warning never reaches the job log as an annotation, and instead read -r id picks up the whole line ::warning::could not list the reviews to withdraw on owner/name#5 as an id. The loop then issues gh api -X PUT "repos/$REPO/pulls/$PR/reviews/::warning::could not list …/dismissals", whose stderr is unredirected, so the log gets a confusing gh error in place of the intended warning.

Redirect it: || echo "::warning::could not list the reviews to withdraw on $REPO#$PR" >&2. Workflow commands are picked up on stderr too, and the loop then sees an empty stream.

[github-actions]

[nit] This step runs before "Post the verdict", so the 👍/👎 lands before the comment it points at exists — and since that step is continue-on-error, a failure there leaves the request wearing a verdict with no verdict comment to back it. Placing this after "Post the verdict" costs nothing (it depends only on check_path and GITHUB_TOKEN) and keeps the stated contract — "the verdict comment carries what it found" — true at the moment the reaction appears.

[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 e8bdd0c. Configure here.}

[github-actions]

The change adds a conclusion-keyed 👍/👎 reaction on the triggering comment in its own step (correctly scoped to the asking repository, GITHUB_TOKEN, the job's issues: write, and non-fatal) and drops the redundant review · prefix from inline findings — nothing matches on the removed text, since threads carry on the <!-- seidroid-review:finding --> marker. No blocking problems; three non-blocking notes about stale-reaction cleanup, swallowed error output, and step ordering.

Findings: 0 blocking | 5 non-blocking | 3 posted inline

Blockers

• None at the file/PR level.

Non-blocking

• The reactions listing (repos/.../issues/comments/$TRIGGER_ID/reactions) is unpaginated, so the withdrawal only ever sees the first page. Harmless for a typical @seidroid review comment, but --paginate would make it unconditional.
• Answer the request is the only publish step in this job without an inputs.mode == 'review' guard. It is safe today because close mode always writes verdict_produced=false, but that safety is implicit rather than stated the way the sibling steps state it.
• 3 suggestion(s)/nit(s) flagged inline on specific lines.

[github-actions]

[nit] This step runs before Post the verdict, which is continue-on-error: true. So a run can react 👍 on the request and then fail to publish the comment the reaction sends the reader to, leaving the reaction as the only thing on the pull request — and this step's own comment ("a review that ran and published") reads as if publishing had already happened. Placing it after Post the verdict costs nothing and makes the reaction mean what it says.

[github-actions]

The new "Answer the request" step and the hardening of the position step are well-reasoned and safely gated (continue-on-error, guarded API calls, list-into-variable instead of piping, correct issues: write scope for the issue-comments reactions endpoint). One gap: a conclusion that earns no reaction returns before the stale-reaction withdrawal, so a re-run that degrades to neutral leaves an earlier 👍 standing.

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

Blockers

• None at the file/PR level.

Non-blocking

• The PR description's table (APPROVE → 👍, REQUEST_CHANGES → 👎, none → nothing) does not match the implementation. The reaction keys on the conclusion, not on the position recorded — and since approve-on-success is off by default, the common clean run posts 👍 while recording no APPROVE review at all. The in-code comment at line 1191 states and justifies the actual behaviour; the description should be corrected to match it, since "keyed on the position, not on the job succeeding" is the claim a reviewer would check.
• The step is gated on steps.drive.outputs.verdict_produced == 'true', so a run that dies before producing a verdict (the driver crashing, install failing) still leaves the requester with a bare 👀 and no answer — the exact gap the PR opens by describing. The neutral conclusion covers "ran but could not read the diff"; nothing covers "never got that far". Worth a follow-up (e.g. a 😕 on a no-verdict run) rather than a change here.
• 1 suggestion(s)/nit(s) flagged inline on specific lines.
• 1 non-blocking pre-existing issue(s) listed below under pre-existing issues.

Pre-existing issues

• [suggestion] .github/workflows/seidroid-review.yml:928 ("Place findings on the code") still runs now="$(gh api "repos/$REPO/pulls/$PR" --jq .head.sha)" unguarded under set -e, purely to emit a warning. A transient failure on that read aborts the step before a single inline finding is posted. This PR removed exactly that pattern from the position step for the same reason; the findings step kept it.

[github-actions]

[suggestion] A conclusion that earns no reaction exits here, before the withdrawal block below — so it cannot clear a thumb an earlier run of the same comment left.

Re-running the job replays the same comment_id (the comment right above says so). First run concludes success → 👍 posted. The job is re-run, the credential path degrades, this run concludes neutral → the step logs "earns no reaction" and exits, and the comment keeps wearing the 👍. That is the credential-outage shape the PR description says must not carry an approval: a review that cannot show it read the diff, sitting under a thumbs-up.

The comment above claims "the reaction and the one it replaces are chosen in one statement, so the two cannot drift apart" — true for the failure↔success flip, but the third branch has no opposite and so never withdraws.

Fix: hoist the withdrawal above the case and have the no-reaction branch clear both +1 and -1 from this bot before exiting (e.g. set opposite="+1","-1"-style list, or make the loop iterate over the contents to withdraw). The rest of the block is already scoped to github-actions[bot], so a human's thumb is still untouched.

[github-actions]

The consolidation onto a single recorded head SHA and the new "Answer the request" reaction step are well-constructed, but the withdrawal gate was narrowed from "not failure" to "success only", which strands stale CHANGES_REQUESTED blocks on the ordinary neutral outcome. The same misreading of neutral also leaves the reaction path silent — and stale — for the most common verdict.

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

Blockers

• None at the file/PR level.
• 1 blocking issue(s) flagged inline on specific lines.

Non-blocking

• The PR description's table keys the reaction on the recorded position (APPROVE → 👍, REQUEST_CHANGES → 👎), but the code keys on the check conclusion: with approve-on-success: false a clean review records no position yet still gets 👍. The in-code comment at .github/workflows/seidroid-review.yml:1229-1231 states this deliberately — worth correcting the description so the two agree.
• Both ::warning::head moved from … during the review messages were dropped along with the per-publisher head reads. Publishing against the reviewed commit is the right call, but when the head does move the review check run lands on a commit that is no longer the PR head, so a required review check silently never appears and the log no longer says why. Consider keeping one cheap head read in the last publish step purely to emit that warning, or noting the reviewed SHA in the verdict comment.
• 1 suggestion(s)/nit(s) flagged inline on specific lines.

[github-actions]

[blocker] This narrows the withdrawal from "any non-failure conclusion" to "success only", and the justification above it — "A neutral conclusion is what a review reaches when it cannot show it read the diff" — does not match what neutral actually means here.

The review prompt defines it explicitly (.github/seidroid/ai-review/review.md:109):

"neutral" → no blockers, but non-blocking notes exist.

and "success" → "clean; nothing of note". So neutral is the ordinary result of a review that found only suggestions or nits — not a signal that the diff went unread. The predecessor this replaced agrees: ai-review.yml:1081 dismisses on verdict !== "failure".

Failure scenario: a PR gets a blocking finding and a CHANGES_REQUESTED review. The author fixes it. The re-review finds the blocker gone but leaves one suggestion → neutral. The step exits here, the old block is never withdrawn, and the PR stays red for a finding that no longer exists — precisely the state the comment three lines up warns "only a human can clear". Because almost every real re-review carries at least one non-blocking note, this is the common path, not the edge case.

Suggest restoring if [ "$conclusion" = "failure" ]; then exit 0; fi. If the goal is to also exclude a conclusion the step could not read, that case already returned early at line 1113.

[bdchatham]

I'm pushing back on this one, with evidence — I think the blocker rests on the wrong definition of neutral.

The citation is .github/seidroid/ai-review/review.md:109, which is the ai-review prompt. This workflow never reads it (grep -c 'ai-review/' seidroid-review.yml → 0). It drives sei-agent-driver, and that binary computes the conclusion itself, in internal/review/verdict.go:

if v.Decision() == "request_changes" || len(Blockers(v)) > 0 || v.hasBlockingFinding() { return "failure" }
if v.Decision() == "comment" && !v.wroteAnythingDown()                                 { return "neutral" }
if !v.readTheDiff()                                                                    { return "neutral" }
if v.hasPreExistingBlocker()                                                           { return "neutral" }
return "success"

So under the pinned driver, neutral means one of three things: a comment decision over empty buckets (a failed read), a reply that cannot show it read the diff, or a pre-existing blocker. A notes-only review returns success.

That changed today. Until v0.12.0 the driver had a hasNotes() gate that made any observation yield neutral — which matches the definition you quoted, and which is exactly why approval was unreachable. sei-internal-skills#399 removed it so that "a review with three suggestions and nothing blocking is a review that says the change is fine".

So the failure scenario — "re-review finds the blocker gone but leaves one suggestion → neutral" — returns success under this driver, and success withdraws. And ai-review.yml:1081 dismissing on verdict !== "failure" is correct for its semantics, where neutral did mean notes-only.

Restoring != failure would let a review that could not read the diff dismiss a standing block under the message "the latest review found nothing blocking" — a false statement clearing a merge gate, on precisely the credential-outage path that produced a day of green-but-empty runs here.

Happy to be shown wrong if the pinned driver differs from what I read on main.

[bdchatham]

I was wrong, and you were right about the outcome. Fixed in 7d36283.

My rejection assumed the driver on main, where neutral cannot mean "found only notes". But this is a reusable workflow and callers pin their own driver-version. I verified hasNotes per revision:

revision	notes-only concludes
09ee41de	neutral
v0.11.0	neutral
v0.12.0+	success

sei-protocol/sei-internal-skills pins driver-version: 09ee41de…, so for that caller a re-review that fixes a blocker and leaves one note concludes neutral — and my gate stranded its block on the common path, exactly as you described. I was reasoning from the driver I had just changed rather than from what callers run.

I also confirmed every caller's current uses: pin gates on = "failure". So this branch introduced the tightening; you were defending the status quo.

One correction to the reasoning, which matters for the fix. .github/seidroid/ai-review/review.md is ai-review's prompt and this workflow never reads it — the driver builds its prompt in Go, and its schema is "decision": "approve" | "comment" | "request_changes", with neutral and success existing only as the conclusion CheckConclusion() derives. The real cause is hasNotes(), live in every driver before v0.12.0.

That matters because != failure is too loose in the other direction: it lets a review that could not read the diff clear a merge gate under "the latest review found nothing blocking". neutral is one word over three states — unread diff, pre-existing blocker, notes-only — and only the first should not withdraw.

So the gate is on evidence rather than the word, using the counts the driver already writes:

state	withdraw?
failure	no
blocking > 0 beside a non-failing conclusion	no, ::warning::
success	yes
neutral, counts readable, nothing written down	no, ::notice::
neutral, counts readable, something written down	yes
no counts (pre-v0.11.0)	yes, ::warning:: naming the gap

The last row keeps your caller working: it reproduces what both callers get from their current pin, so this PR ships no regression there.

Two follow-ups this exposed, neither in this PR: check.json does not publish read, so a reply with read: 0 that still writes a note is indistinguishable from a real review by counts alone — the durable fix is driver-side. And the dismissal message now says "found nothing blocking in this change", since the ambiguity your argument leaned on was real.

[github-actions]

[suggestion] Two consequences of routing neutral through this arm, both stemming from the same reading of neutral flagged on the withdrawal gate — it means "no blockers, only non-blocking notes", which is the most common outcome of a real review, not "nobody looked".

1. The gap this PR sets out to close stays open for that outcome: a requester whose review concludes neutral sees 👀 and nothing else, and still cannot tell a finished review from a running one without opening the run.

2. More concretely, this arm exits before the withdrawal block below, so a reaction from an earlier run is left in place. Run 1 concludes failure → 👎. The author fixes the blocker; run 2 concludes neutral → early exit here, and the comment still wears 👎 for a verdict this run did not reach. The comment at 1237-1238 ("the two cannot drift apart") holds within a reacting run, but not across runs.

At minimum, delete both +1 and -1 from this bot before exiting, so a non-reacting conclusion clears the record rather than inheriting it. Reacting 👍 on neutral would also be defensible given what the verdict means.

[bdchatham]

Point 2 is right and is fixed in 3c089d0 — thank you, it is a real defect independent of what neutral means.

A conclusion earning no reaction now clears both contents from this bot and posts nothing, so the request stops wearing a verdict the current run did not reach. The listing returns content beside id, so one read covers every case, and the loop deletes only the contents this run calls stale — a human's thumb is still untouched.

Verified against a stub: a standing 👎 is cleared by a clean re-run, cleared by a verdictless re-run, kept by a failing one, and a comment wearing both self-heals on either path.

Point 1 I disagree with, for the reason on the withdrawal thread: under the pinned driver a notes-only review concludes success, not neutral, so the requester does get an answer on the common outcome. neutral there means the review could not show it read the diff — and a 👍 on that would say the change is fine when nobody looked.

[github-actions]

The new "Answer the request" reaction step and the **review · …** → **severity** label change are both well-built and safe (correct endpoint, right token/identity, idempotent, non-fatal, stale thumbs cleared). But the same commit re-gates the stale-review withdrawal on conclusion == "success", which — given this repo's own spec defines neutral as "no blockers, but non-blocking notes exist" — leaves a previous blocking review standing on the very common outcome where the blocker was fixed and only suggestions remain.

Findings: 1 blocking | 4 non-blocking | 3 posted inline

Blockers

• None at the file/PR level.
• 1 blocking issue(s) flagged inline on specific lines.

Non-blocking

• Both head-re-read removals also dropped the head moved from $head_sha to $now during the review warning. Behaviour is unchanged (publish against the reviewed commit), but the run log no longer says anything when the check run lands on a commit that is no longer the head — so the PR shows no review check on its actual head and nothing explains why. Consider keeping the diagnostic in one place (e.g. a single read in the check-run step, log-only) rather than in all three.
• 2 suggestion(s)/nit(s) flagged inline on specific lines.
• 1 non-blocking pre-existing issue(s) listed below under pre-existing issues.

Pre-existing issues

• [suggestion] .github/workflows/seidroid-review.yml:1044-1047 — the check-run publish step still reads jq -r .conclusion "$CHECK" without // empty, so a check file with no conclusion posts the literal string null and the API rejects it. This PR added the // empty hardening to the two sibling steps; the check-run step was left on the old pattern.

[github-actions]

[blocker] This narrows the withdrawal from "anything that is not failure" to "success only", and the justification above it — "a neutral conclusion is what a review reaches when it cannot show it read the diff" — does not match how neutral is defined for this driver. The repo's own review spec says the opposite: .github/seidroid/ai-review/review.md:109 — "neutral" → no blockers, but non-blocking notes exist.

So neutral is the ordinary outcome of a review that read the diff fine and found only suggestions, not a degraded one. Concretely: seidroid concludes failure and posts CHANGES_REQUESTED; the author fixes the blocker but leaves one nit; the re-review concludes neutral; the stale block is no longer withdrawn and the PR stays red on a finding that is gone — which is exactly what the comment at line 1149-1151 says this code exists to prevent, and it now needs a human to clear.

If the driver really does emit neutral for "could not show it read the diff", that state needs its own signal (an empty-findings marker, a distinct title, a separate field) rather than being inferred from a conclusion that also means "clean apart from suggestions". Otherwise [ "$conclusion" = "failure" ] && exit 0 was the correct gate.

[github-actions]

[suggestion] Same neutral semantics as the withdrawal gate below: per .github/seidroid/ai-review/review.md:109, neutral means "no blockers, but non-blocking notes exist" — a review that read the diff and had something to say. Under this case, that request ends with 👀 and nothing else, which is the gap the PR description sets out to close ("A requested review opens with 👀 and then says nothing on the request itself").

If the concern is that 👍 overstates a run with open suggestions, a third content (eyes is taken; confused or rocket are available) or simply reusing 👍 for both non-blocking outcomes would still answer the question. The current mapping answers only two of the three conclusions the driver can produce.

[github-actions]

[suggestion] Keying on the position rather than on the job succeeding is the right call for the credential-outage case you describe, but it now also covers a case where the two disagree in the other direction. With this PR, State the review's position has no continue-on-error and exits 1 when a dismissal fails (line 1190), and Publish the review check run exits 1 when head_sha is empty. Both run before this step, and this step runs on !cancelled().

So: conclusion success, the dismissal of an older CHANGES_REQUESTED 403s → job is red, the PR is still blocked by a review this run disagreed with, and the requester's comment gets a 👍. The 👍 is the only thing they see without opening the run, and it says the opposite of the PR's state.

Gating the 👍 on steps.<id>.outcome == 'success' for the two steps that can now fail hard (or falling through to no reaction there) would keep the reaction honest without giving up the position-based trigger.

[github-actions]

The reaction step and the label drop are correct, and the supporting hardening (single head read, guarded list pipelines, non-aborting withdrawal loop, counts-gated dismissal) is sound bash. Two non-blocking issues: escalating the check-run publish to a job failure has no cross-repository guard, and the PR description no longer matches what the code does.

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

Blockers

• None at the file/PR level.

Non-blocking

• The description covers only the reaction and the label, but the diff also consolidates the head read across all three publishers, escalates two steps from continue-on-error: true to job-failing, and gates the CHANGES_REQUESTED withdrawal on the driver's finding counts. Those are the changes an operator will notice first (new red-job conditions), and they are worth naming in the body.
• The description's table is keyed on "position recorded", but the implementation keys on the check conclusion — and the code comment at line 1269-1271 argues for that deliberately. Since approve-on-success is off by default (line 96-102), the default configuration records no position on a clean review yet still posts 👍, which the table says should be (nothing). The code is the behaviour you want; the table just needs to say "conclusion" instead of "position recorded".
• 2 suggestion(s)/nit(s) flagged inline on specific lines.

Superseded: latest AI review found no blocking issues.

[github-actions]

Adds a thumbs-up/down reaction on the triggering comment keyed on the review's recorded position, stops every publisher from re-reading the PR head (degrading or failing instead), makes the withdrawal step fatal, and drops the redundant review · prefix from inline findings. The shell is careful and the failure modes are well reasoned; I found no blockers, only two consistency gaps around the new guards.

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

Blockers

• None at the file/PR level.

Non-blocking

• The 👀 from Acknowledge the trigger is never withdrawn, so a neutral conclusion — the outcome for "no blockers, but non-blocking notes", which is most re-reviews — leaves the request wearing only 👀. That reads identically to a review still running, which is the state this PR set out to distinguish. Clearing the eyes in Answer the request (or reacting rocket/confused for neutral) would close that gap; the current design relies entirely on the reader noticing the verdict comment.
• Answer the request is skipped for inputs.mode == 'close', but Acknowledge the trigger has no mode gate, so a @seidroid review close comment permanently wears 👀 with no follow-up. Pre-existing behaviour, but the new 👀/thumb pairing makes it more visible.
• 2 suggestion(s)/nit(s) flagged inline on specific lines.

[github-actions]

[suggestion] This guard protects the withdrawal but not the APPROVE the same step already posted at lines 1122-1144. In the exact case it was written for — a success conclusion beside counts.blocking > 0, the driver disagreeing with itself — the step records an approving review first and only then declines to clear the earlier block, which leaves the PR approved by this tool while the review names a blocker. Since blocking is read from $CHECK, which is already open at line 1089, hoisting the count read and this check above the event/gh api block would cover both merge-gate-clearing actions with one condition.

[github-actions]

[suggestion] The reaction is keyed on the recorded position only, so when the withdrawal above fails (stuck > 0, line 1230-1232) the job goes red with a stale CHANGES_REQUESTED block still standing — and this step, running under !cancelled(), still puts 👍 on the request. The requester is told the change is clear while the PR is in fact still blocked, which is precisely the "block nobody knows to look for" the fatal exit was added to surface. Consider gating the 👍 on steps.position.outcome == 'success' (or having the position step export a withdrawal_stuck output this step reads) so the answer cannot contradict a gate the run failed to clear. Keeping 👎 unconditional on failure is fine — the concern is only the false-positive direction.