fix(seidroid-review): withdraw the reactions on a cancelled run, from a step that cannot post

[bdchatham]

A run cancelled by a newer @seidroid review now clears the reactions it left on its own
trigger comment, from a step that cannot post one. It left the 👀 there for good.

Carries PLT-1166. PLT-1159 comes out ruled out, on the evidence below. Nobody can
do it as written. The permission comment records the reason beside the scope the ticket
asked to drop.

PLT-1166 — the defect

Answer the request took !cancelled(). Two @seidroid review comments in quick
succession put both runs in one concurrency group under cancel-in-progress, so the newer
one cancels the older. By then the older comment already wears the 👀 — the
acknowledgement is the first step of the job. The newer run answers its own comment
id, so nothing ever reads the older one again.

The result: a comment that asked for a review, wears eyes, and never gets an answer. That
is the defect PLT-1144 fixed on the no-verdict path, reached by the one path that fix does
not cover.

What ships

Answer the request keeps !cancelled() — unchanged from the base. The withdrawal is a
new step, last in the job:

- name: Withdraw the reactions on a cancelled run
  if: ${{ inputs.mode == 'review' && cancelled()
    && needs.guard.outputs.comment_id != ''
    && steps.verdict.outputs.posted != 'true' }}

Post the verdict gains id: verdict and a posted output so that last term can read
it, and Answer the request gains id: answer. Nothing else in the workflow changes.

Why not always() on Answer the request

That was the first shape here and it was wrong. A step output persists once its step
completes, so a cancellation landing any time after drive finishes leaves check_path
and verdict_produced populated. Answer the request would read a real conclusion and
post a thumb, while every publisher skips on !cancelled(). A thumb reads as an answer.
That is worse than the stale eyes this PR set out to remove.

Why the new step cannot state an outcome

It contains no POST. The script lists this bot's reactions and deletes the three this
workflow posts. No code path in it adds one. Nothing the step receives can therefore make
it state an outcome. That property holds whatever its inputs are, which is what makes it
structural rather than a matter of what a cancellation happens to look like.

It reads no check file, no verdict_produced and no conclusion.

What the steps.verdict.outputs.posted term is, and why it does not break that

A cancellation can arrive once the verdict is already on the pull request — during thread
resolution, say — and the thumb Answer the request posted answers it correctly.
Withdrawing it there leaves a published review with no reaction on the request that asked
for it, which reads as never answered. That is this step's own defect, one window later.

The posting step's posted output separates the two. It is one boolean about another
step, written from the comment POST's own result. No conclusion is in it. It says whether
an answer already stands, never which answer it would be, so reading it gives the step
nothing to state. Handing it verdict_produced instead would have restored the
conditional reasoning above: that flag is true whenever the driver reached a verdict,
including when nothing published.

Its outcome will not do, and that took a second pass to see. Post the verdict runs
under continue-on-error and tolerates a refused comment POST. Its failure path ends on
a call whose failure it swallows. The step therefore exits 0, and its outcome reads
success whether the verdict landed or not. The first version of this gate read that
outcome. A refused POST followed by a cancellation then kept a thumb standing for a
review nobody can see. That step already tracked the POST's result in a shell variable.
It now writes it as an output.

Anything but a posted verdict withdraws. A value the step cannot read therefore clears.
It does not leave a thumb standing for a verdict that may not be on the pull request.

The cross-run half of the same problem

A re-run replays the trigger comment id. The comment can therefore already carry a thumb
from an earlier run whose verdict is on the pull request. A re-run cancelled before it
answered took that thumb along with its own eyes. The comment then ended bare while the
verdict it asked for still stood.

Answer the request gains id: answer, and the withdrawal reads its outcome to decide
what this run may take:

steps.answer.outcome	what it means	withdrawn
skipped	this run never touched the comment, so a thumb there is an earlier run's	eyes only
success	this run withdrew the stale thumb and posted its own, and published nothing	+1 -1 eyes
failure, cancelled, unreadable	the step ran partway and most likely took the earlier thumb already	+1 -1 eyes

The structural property is untouched: still no POST, and an outcome is still four words
about another step with no conclusion among them.

One case survives, and the comment states it rather than claiming it away. A run
answers, which withdraws an earlier thumb and posts its own. A cancellation then arrives
before publishing, and the comment ends bare. The answer step already took the earlier
thumb, so nothing at the end of the job can put it back. Knowing it happened would need a
read of the pull request this step deliberately does not make. A cancellation lands during
the driver far more often than in that gap. The step's comment records the limit instead
of asserting the invariant outright.

One tension with the stated acceptance criterion, deliberately. "Given a run cancelled
by a newer request, its trigger comment carries no reaction from this bot" now fails on
one path. A cancelled re-run leaves an earlier run's thumb. The criterion's intent holds.
The
comment does not wear 👀 with no answer coming, because the answer is on the pull request.
Satisfying the literal wording would restore the defect above. I flag it rather than read
the criterion loosely.

Why last in the job, and what holds it there

The runner evaluates a step's condition when it reaches the step. Any step after the
withdrawal is a step during which a cancellation leaves the eyes standing. The runner
already evaluated the withdrawal and skipped it by then. Placed last it also reads
steps.verdict.outputs.posted after that step has reported.

conditions.py checks the position rather than any one ordering, which covers a step
appended later. Four mutations fail it. Move the withdrawal ahead of Post the verdict,
ahead of the resolve step, or ahead of the no-verdict report. Or append a step after it.
The id-ordering check caught only the first. In the other three Post the verdict still
ran earlier.

Why not the fix as named

Gating the conclusion read inside the script needs the job status in the shell, and GitHub
does not offer it there. cancelled() is readable only in a step or job if.
PipelineTemplateEvaluator.EvaluateStepEnvironment calls
CreateContext(contextData, expressionFunctions) with no expressionState, where
EvaluateStepIf passes step.ExecutionContext.ToExpressionState(). And
CancelledFunction.EvaluateCore reads templateContext.State[nameof(IExecutionContext)]
and ArgUtil.NotNulls it. StepsRunner turns that throw into
CompleteStep(step, TaskResult.Failed), so env: CANCELLED: ${{ cancelled() }} fails the
step on every run, before the runner evaluates its condition. actionlint refuses it too:
calling function "cancelled" is not allowed here. "cancelled" is only available in "jobs.<job_id>.if", "jobs.<job_id>.steps.if". The same holds for run:.

The cancellation shapes, and which this covers

when the cancellation lands	what runs	outcome	covered
while queued, job never starts	nothing	no eyes were ever posted	n/a
before drive completes	withdrawal	all three withdrawn, no thumb	yes
after drive completes	withdrawal	it cannot read the populated outputs	yes
while Answer the request runs	its !cancelled() re-test fires, the runner kills it, then the withdrawal	the withdrawal takes whatever it left	yes
after the thumb, before the verdict published	answer, then withdrawal	the withdrawal takes the thumb: it would stand for nothing	yes
after the verdict published	answer only	thumb survives beside the published verdict	yes
the verdict POST refused, then cancelled	answer, then withdrawal	thumb withdrawn: its outcome still reads success	yes
during the withdrawal step	withdrawal	its own condition is cancelled(), so the re-test keeps it alive	yes
once the runner reached every step	answer only	a thumb this run earned stays	correct
a re-run, cancelled before it answered	withdrawal	an earlier run's thumb stays, its eyes go	yes
a re-run, answered then cancelled before publishing	answer, then withdrawal	bare comment, earlier verdict stands	no
runner process shutdown (RunnerShutdownToken)	nothing	StepsRunner skips condition evaluation outright	no

Two rows are gaps. In the answered-then-cancelled row the answer step has already taken
the thumb, so no later step can restore it. A hard kill of the runner leaves the eyes on
the comment, and nothing inside a workflow closes that.

The reaction table

Each case declares two job states: the one the runner reached Answer the request in, and
the one it reached the withdrawal step in. success>cancelled is a cancellation that
arrived after the answer, so the answer step posts its own thumb and the fixture places
nothing by hand.

case	states	verdict outcome	ran	left on the comment
success	success>success	success	answer	bot:+1
failure	success>success	success	answer	bot:-1
no verdict	success>success	skipped	answer	none
neutral	success>success	skipped	answer	none
cancelled after drive, outputs populated	cancelled>cancelled	skipped	withdraw	none
cancelled before drive finished	cancelled>cancelled	skipped	withdraw	none
cancelled mid-publish	success>cancelled	cancelled	answer+withdraw	none
the verdict failed to post	success>cancelled	failure	answer+withdraw	none
the outcome went unreported	success>cancelled	empty	answer+withdraw	none
cancelled after the verdict published	success>cancelled	success	answer	bot:+1
the same, beside a human's	success>cancelled	success	answer	brandon:-1, bot:+1
success, human +1 -1 eyes	success>success	success	answer	human ×3, bot:+1
failure, human ×3	success>success	success	answer	human ×3, bot:-1
no verdict, human ×3	success>success	skipped	answer	human ×3
cancelled, human ×3	cancelled>cancelled	skipped	withdraw	human ×3
stale bot:-1 + human:+1	success>success	success	answer	human:+1, bot:+1
the same, cancelled	cancelled>cancelled	skipped	withdraw	human:+1
stale bot:+1, no verdict	success>success	skipped	answer	none
bot:rocket from another workflow	success>success	success	answer	human:+1, bot:+1, bot:rocket
the same, cancelled	cancelled>cancelled	skipped	withdraw	human:+1, bot:rocket
the list call refused	success>success	success	answer	human:+1, bot:+1, bot:eyes + warning
a delete refused	success>success	success	answer	bot:+1, bot:eyes + warning
the add refused	success>success	success	answer	none + warning
the list refused, cancelled	cancelled>cancelled	skipped	withdraw	bot:eyes + warning
a delete refused, cancelled	cancelled>cancelled	skipped	withdraw	bot:eyes + warning
the acknowledgement refused	success>success	success	answer	bot:+1

A human's reaction survives every path. A bot:rocket some other workflow left survives
too, because each step deletes only the contents this workflow posts. And a thumb that
answers a published verdict survives a later cancellation.

PLT-1159 — ruled out, with the evidence

The ticket's premise is that ai-review.yml reaches the same reactions through GraphQL
addReaction under pull-requests: write. It does not, and the workflow file does not
decide the question.

1. ai-review.yml does call both mutations, and its permissions: blocks do lack
issues. preflight is contents: read + pull-requests: write and calls
addReaction(content: EYES); complete_review_reaction is pull-requests: write alone
and calls addReaction(THUMBS_UP) then removeReaction(EYES).

2. But neither call uses GITHUB_TOKEN. Both steps pass
github-token: ${{ steps.app-token.outputs.token || github.token }}, and in production
the App token wins. I checked live comments. Every reaction on an @seidroid review
trigger in sei-chain belongs to seidroid[bot], not to github-actions[bot]:

comment 5536974191  +1 by seidroid[bot]   sei-chain#4088
comment 5544795940  +1 by seidroid[bot]   sei-chain#4101
comment 5531689281  +1 by seidroid[bot]   sei-chain#4095
comment 5493838638  +1 by seidroid[bot]   sei-chain#4063

The workflow's permissions: block does not bound an App installation token. Its own
installation grant governs, and that App holds Issues: write — ai-assistant.yml posts
POST /repos/{o}/{r}/issues/comments/{id}/reactions with the same token. ai-review is
therefore no evidence at all about what pull-requests: write alone can do. It is the
same class of wrong premise as the enable-cursor: false one.

3. GitHub documents no permission for any GraphQL mutation. Not a gap in my reading —
checked at the data source. In github/docs, src/graphql/data/fpt/schema-reactions.json
gives addReaction and removeReaction the keys name, id, href, description, isDeprecated, inputFields, returnFields, category and no permission field. The public SDL
(docs.github.com/public/fpt/schema.docs.graphql) carries only
@docsCategory(name: "reactions"). The GraphQL guide's whole statement on the subject is
that the API returns an error naming the permission it wanted. One route therefore remains
to the requirement: make the call.

4. GitHub documents what REST requires, and the alias stops at the reaction.
github/docs, src/github-apps/data/fpt-2026-03-10/server-to-server-permissions.json:

endpoint	permission
GET/PATCH/DELETE /repos/{o}/{r}/issues/comments/{id}	listed under both issues and pull_requests
POST /repos/{o}/{r}/issues/comments/{id}/reactions	issues: write only
DELETE /repos/{o}/{r}/issues/comments/{id}/reactions/{rid}	issues: write only
POST /repos/{o}/{r}/pulls/comments/{id}/reactions	pull_requests: write (a review comment — a different resource)

That file expresses "either permission" by listing an endpoint twice. A single listing on
the reactions endpoints is therefore a distinction, not an omission. It confirms the claim
the guard job already makes in prose.

Verdict. The premise is void and the documentation says nothing. Nothing here can mint
a fine-grained token scoped to pull-requests to test it. Shipping the drop blind would
regress the defect this PR fixes. continue-on-error and a ::warning:: swallow a 403 on
the reaction, so the eyes would stay on every comment and no run would fail. Ruled out.

Two things worth keeping for whoever re-files it

removeReaction beats the REST loop, whatever the scope turns out to be. The ticket
assumed it takes a reaction node id. It does not. RemoveReactionInput is
{content: ReactionContent!, subjectId: ID!}, and the subject is the comment:
IssueComment sits in its @possibleTypes. It takes no actor input, so it can only ever
remove the viewer's own reaction. That makes the human-scoping property structural rather
than a select(.user.login == $me) filter. It also retires the hardcoded
me="github-actions[bot]" login and the paginated list whose miss leaves eyes behind.

A third shape the ticket does not name looks likelier than either. This workflow
already mints an App token (steps.identity.outputs.token) and already computes
REVIEWER_LOGIN from app-slug. Reacting under that identity needs no caller scope at
all, and it is how production already posts these reactions. Two obstacles stand in the
way. The acknowledgement runs before the mint, on purpose. And the mint is optional, so a
GITHUB_TOKEN fallback keeps the scope required — unless a caller without App credentials
may lose the reaction.

The cost this change carries

The list-and-delete block is now duplicated between Answer the request and the
withdrawal step. me="github-actions[bot]" and the set of contents each step may delete
are two copies, and a reader has to keep them in step by hand. Edit one and not the other
and a reaction stays behind on whichever path lost the edit.

That is the price of the separate step, and it buys the structural property: the
withdrawing step has no POST. Sharing the block would mean one step doing both jobs, which
is the shape that produced this PR's blocker. GitHub Actions offers no way to share a
script between two steps without a checkout, and YAML anchors are not supported.

Both harnesses cover both copies, so a drift fails rather than ships. The GraphQL
removeReaction above is what would remove the duplication outright. It needs no login
and no listing, so the whole block collapses to one mutation per content.

Every reaction site

Three steps, six calls, all in the review job, all on the ISSUE comments endpoint:

line	step	call
1036	Acknowledge the trigger	POST .../issues/comments/{id}/reactions (eyes)
2494	Answer the request	GET .../reactions --paginate
2504	Answer the request	DELETE .../reactions/{rid}
2519	Answer the request	POST .../reactions (+1 / -1)
3292	Withdraw the reactions on a cancelled run	GET .../reactions --paginate
3304	Withdraw the reactions on a cancelled run	DELETE .../reactions/{rid}

The withdrawal step has no POST, and that is the fix. guard reacts nowhere.
ai-assistant.yml and ai-review.yml have their own sites; neither is in this workflow.

Verification

Committed, not kept locally. An uncommitted harness is how the first blocker survived
a reading and seven mutations. Two additions to test/seidroid-review/, wired into
workflow-test-self.yml as their own job so the placement check keeps its name.

reactions.sh — 62 assertions over 32 cases. It runs the reaction steps under bash,
extracted from the workflow on every run. No case names the step it runs.
conditions.py --select names it, from the job state and the posting step's outcome, so
the two layers cannot drift. The gh stub keeps the reaction list a comment carries and
serves it through the step's own --jq. It honours idempotence per (user, content). It
can refuse the list, a delete or the add. The acknowledgement's calls go to a separate
log, so every count belongs to the step under test. The stub reports any call it cannot
serve.

conditions.py — 76 assertions. A step condition decides which reaction step runs in
which job state, and a shell harness cannot see it. The model applies the runner's own
rule: a condition naming none of always/cancelled/failure/success becomes
success() && (...). It treats a term it cannot decide as unknown rather than false. Two
checks read the file rather than a table, so they cover a step added later:

Both walk every job's raw steps list and search the whole step:

• No step that can run on a cancelled job may reach check_path or verdict_produced.
• Every steps.<id> a step reads must be a real id on an earlier step.
• The withdrawal is the last step of the review job.

Keyed off a name they dropped an unnamed step. - uses: ... with no name: is the usual
shape, so the step most likely to arrive later was the one they could not see. The guard
job already carries one. The id check also reached only if, which left the withdrawal's
new env read unchecked.

Each fixture, mutation-tested. A fixture that cannot fail the invariant is not a test
of it:

mutation	which case fails
the gate reads steps.verdict.outcome again	THUMB GOES on a refused POST, and the matching condition row
the skipped arm takes all three	EARLIER THUMB SURVIVES, plus 3 more
id: answer deleted	reads steps.answer, which is no step's id
an unnamed always() step reading verdict_produced	step 17 runs on a cancelled run and reads verdict_produced
the posted term dropped	THUMB SURVIVES + 3 more + the condition row
the same term inverted	16 script cases, 5 condition cases
id: verdict deleted	reads steps.verdict, which is no step's id
the withdrawal moved ahead of Post the verdict	the position check, and reads steps.verdict, which runs later
the withdrawal moved ahead of the resolve step	the position check alone — the id check passes it
the withdrawal moved ahead of the no-verdict report	the position check alone
any step appended after the withdrawal	the position check
Answer the request back to always()	its cancelled row, and the sweep
check_path interpolated inline into run:	the sweep — an env-only sweep passes it
the withdrawal step deleted	no step named 'Withdraw the reactions on a cancelled run'
the withdrawal not scoped to this bot	the human and rocket cancelled cases
the withdrawal ignoring which contents it may take	foreign-cancelled loses a rocket
the missing-conclusion arm clearing only the eyes	cancelled-with-stale-thumb, no-verdict-with-stale-thumb
the answer's list not scoped to this bot	every human case
the empty-reaction guard removed	every clear-only case posts a blank reaction
the VERDICT_PRODUCED gate removed	no verdict thumbs the requester down
the success arm no longer naming the eyes	eyes survive a green review

Only conditions.py catches id: answer deleted. reactions.sh derives that outcome
itself, which is exactly why the id check has to exist.

conditions.py also models steps.verdict.outcome beside posted, derived rather than
passed. It reads success whenever that step reported at all, which is what the runner
sees. A gate that regresses to the outcome therefore fails an assertion instead of
crashing the model.

Five checks ran narrower than their own claim. Mutating the thing each claimed to
cover is what found them.

• The haystack read env only, so an inline interpolation passed.
• The model took an outcome as an argument and never checked the id existed.
• The sweep keyed off a step name, so an unnamed step was invisible.
• run_case exported each per-case knob without clearing it. An ANSWERED_AS override
  leaked forward and disarmed a later case.
• The gate read an exit code that cannot express whether the verdict landed.

The first three now read the file. run_case clears the fourth at the top of every case.
The fifth reads an output written from the POST's own result.

actionlint, base vs branch, both rule sets. seidroid-review.yml: 4 × SC2102:info,
unchanged. Whole .github/workflows: 37 findings, identical after line-number
normalisation. workflow-test-self.yml: 0. shellcheck -S warning clean on both new
harness files. The incumbent harness still passes, now 271 assertions after #102.

Base checked three times, and no move trusted. It went to b1b51f8 (#97), then
2f58b80 (#101), then 98c2619 (#102). #97 rewrote 341 lines of this file and added the
harness. #102 rewrote the resolve step and placement, and renamed the harness job. Each
time all three harnesses and actionlint ran again on the new history. None of them carried
a result from before it. I checked each rebase by reading back four things: the step
order, the step ids, the gate, and the withdrawal step's POST count. Not by its exit
status.

The #102 rebase conflicted once, in the harness README, and I resolved it keeping both
sides. workflow-test-self.yml merged cleanly: place-findings keeps #102's renamed
display name Place findings and resolve threads, and the reaction job keeps its own.

One handoff for whoever lands second. #103 replaces the hardcoded issues/comments
with a comment_api output. Its author wrote it against two reaction steps. This
branch leaves three, carrying six hardcoded paths rather than four: one in
Acknowledge the trigger, three in Answer the request, two in the withdrawal step.
Whichever of us rebases second has to reach all three steps.

Not verified

Nothing here ran on a GitHub runner. Five things stay unverified. That a real cancelled
run reaches the last step of the job. That steps.verdict.outcome reads success on a run
cancelled after that step completed. That steps.answer.outcome reads skipped rather
than empty on a run cancelled before that step. That the runner's condition re-test kills
Answer the request mid-flight, as its source says. And every permission claim above. The
cancellation semantics rest on actions/runner source and on actionlint.
conditions.py models the expression engine; it is not the engine.

🤖 Generated with Claude Code

[cursor]

PR Summary

Medium Risk
Changes GitHub comment UX and reaction cleanup on cancellation; mistakes could remove valid thumbs or leave stale eyes, but behavior is heavily tested and scoped to bot reactions with explicit gates.

Overview
Fixes stale 👀 on superseded @seidroid review triggers when cancel-in-progress cancels an older run: a new last job step removes this bot’s reactions on cancelled runs and never POSTs a reaction, so it cannot thumb a verdict that never published.

Post the verdict now exposes id: verdict and a posted output (from the comment POST result, not step outcome). Answer the request gets id: answer. The withdrawal runs when cancelled(), review mode, a trigger comment exists, and posted != 'true'; it uses steps.answer.outcome to withdraw only eyes when the answer step never ran (preserve an earlier run’s thumb) vs +1/-1/eyes when this run already answered but published nothing.

Workflow comments document why Answer the request stays !cancelled() (late cancel + populated driver outputs would mis-thumb) and issues: write for reaction REST.

CI: workflow-test-self.yml adds a reactions job running reactions.sh (gh stub, 32 cases) and conditions.py (step if modeling, withdrawal-last invariant, no conclusion reads on cancelled steps).

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

[github-actions]

The fix correctly identifies a real defect (a superseded run abandoning 👀 on its own trigger comment) and always() does compose correctly inside the larger &&, but the reasoning that always() only admits a run with no check file is wrong: a cancellation arriving after drive completes leaves the verdict outputs populated, so this step can post a 👍/👎 while every publishing step skips. Gating the conclusion read on cancelled() preserves the fix without that outcome.

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

Blockers

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

Non-blocking

• The workflow's header block (around line 61) still says the surfacing steps use !cancelled() "rather than success() ... while still skipping a run superseded by a newer trigger." That is now false for one of them. A half-line there pointing at Answer the request as the documented exception would keep the file's own map accurate.

[github-actions]

[blocker] always() admits more than the no-check-file case, so this can post a verdict reaction for a review that published nothing.

The justification above ("What always() admits is a run with no check file") assumes cancellation always kills drive before it sets its outputs. Cancellation timing is arbitrary. drive ends at line ~1599; Place findings on the code (1601) is the next step and is the slow one — it can spend a while posting inline comments. A cancellation arriving anywhere from there onward leaves steps.drive.outputs.check_path populated and verdict_produced == 'true', because step outputs persist once the step completes.

In that window every publisher skips on !cancelled() — Place findings (1611), Publish the review check run (1733), State the review's position (1854), Post the verdict (2215), Resolve the threads (2530) — while this step now runs and takes the failure/success arm at 2149-2150. The trigger comment ends up wearing 👎 or 👍 with no verdict comment, no check run, and no findings on the diff. A 👎 there is precisely the outcome the VERDICT_PRODUCED gate and the comment at 2119-2124 exist to prevent: a thumb down on a review nobody can read. Worse, if the superseding run then concludes success, the two comments carry contradictory thumbs.

The withdrawal this PR is after does not need the conclusion at all — it needs the missing-conclusion arm. Routing a cancelled run there unconditionally keeps the fix and closes the gap:

        env:
          # A cancelled run publishes nothing, so it has no conclusion to react
          # to however far `drive` got. It still owes the withdrawal.
          CANCELLED: ${{ cancelled() }}

and in the script:

          conclusion=""
          if [ "${CANCELLED:-}" != "true" ] && [ -s "$CHECK" ] && [ "${VERDICT_PRODUCED:-}" = "true" ]; then

That leaves the cancelled run doing exactly what the comment at 2090-2095 describes — clear all three reactions, post no thumb — for every cancellation, not just the early ones.

[bdchatham]

Correct, and fixed in df981aa. Answer the request keeps !cancelled(); the withdrawal is now its own step, last in the job, under cancelled(), reading no check file and no conclusion.

The suggested env: CANCELLED: ${{ cancelled() }} cannot ship. cancelled() is readable only in a step or job if — PipelineTemplateEvaluator.EvaluateStepEnvironment builds its context without the expression state CancelledFunction asserts on, so the step fails before its condition runs. actionlint refuses it too: "calling function cancelled is not allowed here."

Last in the job, not beside this step: a condition is evaluated when its step is reached, so a clear placed here is already skipped by the time a cancellation lands on Post the verdict. From the end it also withdraws a thumb this step had just posted.

[bdchatham]

Non-blocking finding — the header map: fixed in df981aa, naming a different exception than you expected.

Answer the request is no longer the exception. It keeps !cancelled(), so every surfacing step still skips a superseded run and the original sentence stays true. The exception is the new last step, which withdraws the reactions and reads no verdict, so it can state no outcome. The header now says that, and the acknowledgement's own comment now names both withdrawers instead of claiming Answer the request covers every path.

Blocking finding — replied in thread. You were right, and the suggested env: CANCELLED: ${{ cancelled() }} is not available: cancelled() is readable only in a step or job if, so the gate had to move into condition space as a separate step.

Regression cover, since an uncommitted harness is how this survived. test/seidroid-review/reactions.sh runs all three reaction steps against a gh stub — 38 assertions over 22 cases, including a cancelled run with check_path and verdict_produced populated, which is the case my earlier fixtures missed. test/seidroid-review/conditions.py covers what a shell harness cannot see: 33 assertions on which step runs in which job state, with the invariant stated over the file as well as over a table, so no step reading check_path or verdict_produced may run on a cancelled job. Both wired into workflow-test-self.yml as their own job, so the placement check keeps its name. The incumbent 165 assertions still pass.

Nothing ran on a GitHub runner.

[github-actions]

Adds a final cancelled()-gated step that withdraws this bot's reactions from the trigger comment of a superseded review run, plus a shell harness and a step-condition model as tests. The mechanism is sound and well tested; two narrower points are worth addressing — the unconditional thumb withdrawal can erase a reaction for a verdict that was actually published, and the new invariant check only inspects step env.

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

Blockers

• None at the file/PR level.

Non-blocking

• The PR description documents a different fix than the one implemented: it presents if: ${{ inputs.mode == 'review' && always() && needs.guard.outputs.comment_id != '' }} on Answer the request and argues at length why always() is safe there, while the code instead adds a separate last step gated on cancelled() and its in-file comments argue that always() on Answer the request would be wrong. Worth updating the body so the rationale on record matches the change.
• The list-and-delete block (hardcoded me="github-actions[bot]", paginated --jq list, delete loop, warning strings) is now duplicated verbatim between Answer the request and the new withdrawal step. The two must stay in sync if the reacting identity or the reaction set ever changes; a shared snippet or a small script under test/-adjacent tooling would remove one copy.
• 2 suggestion(s)/nit(s) flagged inline on specific lines.

[github-actions]

[suggestion] Withdrawing +1/-1 unconditionally can strip a thumb that correctly answers a published verdict.

Answer the request (line 2338) and Post the verdict (line 2465) both run before Resolve the threads this review closed (line 2746) and Report a review that reached no verdict (line 2960). A cancellation arriving after Post the verdict has already upserted the verdict comment and published the check run — i.e. during thread resolution — still routes here, and this loop deletes the 👍/👎 that Answer the request just posted. The trigger comment then carries no reaction at all even though the review is fully on the pull request, which reads as "never answered" — the same failure mode this PR sets out to fix, in a different window.

The comment at lines 3101-3102 ("a cancellation arriving during or after Answer the request heals rather than leaving a verdict nobody published") only holds when the verdict was in fact not published.

Two ways out, both local: give Post the verdict an id: and pass steps.<id>.outcome in this step's env so the thumb is kept when the verdict landed, or narrow this case to eyes only — a cancellation before Answer the request leaves no thumb from this run anyway, and a thumb from an earlier attempt on a replayed comment id corresponds to a verdict that was published. Either way, reactions.sh would want a case for "cancelled after the verdict was posted".

[bdchatham]

Right, and fixed in 1ba2010. Post the verdict gains id: verdict, and the condition now also names steps.verdict.outcome != 'success'.

The step still cannot state an outcome, and the reason is stronger than an absent input: it contains no POST, so no code path in it adds a reaction whatever it is told. The gained fact is four words about another step — success, failure, cancelled, skipped — none of which names a conclusion. It says whether an answer already stands, never which answer. verdict_produced would not do: that is true whenever the driver reached a verdict, including when nothing published.

Anything but success withdraws, so an outcome the step cannot read clears rather than leaving a thumb standing for a verdict that may not be there.

New case cancelled-after-publish (success>cancelled, verdict success): the answer step posts the thumb, the withdrawal is skipped, the thumb survives. Dropping the term fails it plus three more; inverting it fails 16. Two more mutations now caught: deleting id: verdict, and moving the withdrawal ahead of Post the verdict — both silently empty the read forever.

[github-actions]

[suggestion] The invariant sweep only inspects step.get("env"), so it misses a step that interpolates ${{ steps.drive.outputs.check_path }} (or verdict_produced) directly into run:, with:, or its own if:. The README claims the check is "stated over the file as well as over a table -- so a step added later is covered"; a later step using inline interpolation would pass silently, which is exactly the case the invariant exists to catch.

Cheap fix: build the haystack from the whole step, e.g. haystack = yaml.safe_dump(step) (or join env, run, with, and if) before the CONCLUSION_INPUTS membership test.

[bdchatham]

Correct, and fixed in 1ba2010. The haystack is now yaml.safe_dump(step), so run:, with:, if: and env: are all covered.

Proved with the exact case you name — echo "check was ${{ steps.drive.outputs.check_path }}" added to the withdrawal step's run:, touching no env: key:

env-only haystack contains check_path:    False
whole-step haystack contains check_path:  True
FAIL Withdraw the reactions on a cancelled run runs on a cancelled run and reads check_path

The README claim now holds. A second file-level check went in beside it: every steps.<id> a reaction condition reads must be a real id on an earlier step, because deleting the id or reordering the step empties the read forever with no error.

Superseded: latest AI review found no blocking issues.

[bdchatham]

All three findings fixed in 1ba2010. Both inline ones answered in thread.

On the body note. The always() rationale was already gone when this landed — gh pr edit had replaced that section before the review, so the copy under review was stale. The body is now rewritten against the current diff regardless, since finding 1 changed it again. It opens with what ships (!cancelled() on Answer the request, unchanged from base; a new last step under cancelled() && steps.verdict.outcome != 'success'), and always() appears only where it is named as the shape that was wrong and why.

Also recorded, per the non-blocking note: a section on the cost. The list-and-delete block is duplicated between the two steps, so me="github-actions[bot]" and the deletable content set are two copies kept in step by hand. That is the price of the separate step, and it buys the no-POST property; sharing the block means one step doing both jobs, which is the shape that produced the first blocker. Actions offers no way to share a script between steps without a checkout, and YAML anchors are unsupported. Both harnesses cover both copies, so a drift fails rather than ships. GraphQL removeReaction is what would collapse it — no login, no listing.

Harnesses: reactions.sh 49 assertions / 26 cases, conditions.py 39 assertions, incumbent run.sh 165 assertions. All green on the current base. actionlint identical base vs branch. shellcheck -S warning clean.

On --force-with-lease: taken, thank you — I will use it pinned to my own last-pushed sha rather than building a merge next time.

Nothing ran on a GitHub runner.

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

[github-actions]

The new Withdraw the reactions on a cancelled run step is correctly placed last, correctly gated on cancelled() + steps.verdict.outcome != 'success', uses the same github.token identity (github-actions[bot]) as the steps that post the reactions, and reads nothing that could let it state an outcome — the core design holds up. The two new harnesses (reactions.sh, conditions.py) are wired into CI and meaningfully cover the behaviour; only minor gaps and a stale comment remain.

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

Blockers

• None at the file/PR level.

Non-blocking

• Note: I could not execute test/seidroid-review/reactions.sh or conditions.py in this environment (sandbox denied python3), so the harnesses were reviewed statically rather than run. The condition model, the gh stub's argument parsing, and each case's expected reaction set line up with the real step scripts on inspection.
• 3 suggestion(s)/nit(s) flagged inline on specific lines.

[github-actions]

[suggestion] steps.verdict.outcome only speaks for this run, but the reactions on the trigger comment can outlive a run. Reachable sequence: run 1 on comment A completes and leaves +1 on A plus a verdict comment on the PR; someone re-runs the workflow (the comments elsewhere note a re-run replays the same trigger comment id); a newer @seidroid review (comment B) cancels the re-run before it reaches Post the verdict, so steps.verdict.outcome is skipped/empty; this step then deletes run 1's +1 along with the eyes. Comment A ends bare even though the verdict it asked for is still upserted on the pull request — exactly the "reads as never answered" failure the != 'success' gate was added to prevent, one run earlier.

Narrow, and arguably an acceptable trade for the "clear on uncertainty" rule this step is built on — but the surrounding comment states the invariant unconditionally ("a thumb that answers a verdict already on the pull request survives a later cancellation"), which is only true within a single run. Worth either narrowing the withdrawal to eyes when this run posted no thumb, or recording the cross-run limit in the comment so the next reader does not rely on the stronger claim.

[bdchatham]

Fixed in 6e7c8c9, and I narrowed rather than documented. Answer the request gains id: answer, and the withdrawal reads its outcome to decide what this run may take:

steps.answer.outcome	withdrawn
skipped — never touched the comment, so a thumb there is an earlier run's	eyes only
success — took the stale thumb, posted its own, published nothing	+1 -1 eyes
failure, cancelled, unreadable	+1 -1 eyes

Narrowed because the fix is cheap, keeps the structural property (still no POST; an outcome is four words with no conclusion in it), and a cancellation lands during the driver far more often than after step 13 — so the sequence you describe is the common one, not the rare one.

One case is left over, and the comment now states it instead of claiming the invariant. A run that answers — withdrawing an earlier thumb and posting its own — then cancelled before publishing leaves the comment bare. The answer step already took the earlier thumb, so nothing at the end of the job can restore it, and knowing it happened needs a read of the pull request this step deliberately does not make.

And one tension I am flagging rather than reading loosely. "Given a run cancelled by a newer request, its trigger comment carries no reaction from this bot" now fails on one path: a cancelled re-run leaves an earlier run's thumb. The intent holds — the comment is not wearing 👀 with no answer coming, because the answer is on the pull request. Satisfying the literal wording restores this defect.

Cases: rerun-cancelled-before-answer (earlier thumb survives), rerun-cancelled-human, rerun-answered-then-cancelled, and the failure/cancelled/unreported arms. Mutation: the skipped arm taking all three fails EARLIER THUMB SURVIVES plus three more.

[github-actions]

[suggestion] steps drops any step without a name, so the sweep below — the one advertised in the README as "stated over the file rather than over a table, so a step added later is covered" — silently skips unnamed steps. Every step in the review job is named today, so nothing is missed now; but a later - uses: actions/checkout@v7 with no name: (the usual shape) would be invisible to the check_path/verdict_produced invariant that is the whole point of the check. Keying the sweep off the raw doc["jobs"]["review"]["steps"] list (falling back to uses/index for the label) keeps the guarantee the README claims.

[bdchatham]

Fixed in 6e7c8c9. Both file-level checks now walk every job's raw steps list and search the whole step, so neither needs telling where to look.

Your prediction was already true, not just hypothetical: the guard job carries an unnamed step today. My sweep only covered review, so nothing was missed — but it now covers both jobs.

Proved with the shape you name:

raw steps: 18   reachable by name: 17   invisible: 1
FAIL step 17 runs on a cancelled run and reads verdict_produced

The label falls back to uses and then to the index.

On your broader point — I found two more. The id check reached only if, so the withdrawal's new env read of steps.answer.outcome would have gone unchecked; it now searches the whole step too. And run_case exported each per-case knob without clearing it, so an ANSWERED_AS override leaked forward and silently disarmed a later case — my own harness caught that one only because the leak made a case fail. Knobs are unset at the top of every case now.

That is five checks narrower than their own claim on this PR. The pattern is the same each time: the check named a property but sampled one key, one name, or one argument. Each fix reads the file instead.

[github-actions]

[nit] Stale duplicate: lines 31-36 are an earlier draft of the block at 37-46, and their key description is now wrong — EXPECTED's keys are 4-tuples (state, mode, comment id, verdict outcome), not job state. Deleting 31-36 leaves the correct header.

[bdchatham]

Deleted in 6e7c8c9. You were right that the key description was wrong — EXPECTED's keys were already 4-tuples, and they are now (job state, mode, comment id, posted).

[bdchatham]

All four findings fixed. 2ffa24a, rebased on 2f58b80. Each answered in thread.

Finding 1 — narrowed, not documented. Answer the request gains id: answer; the withdrawal takes the eyes alone when that step's outcome is skipped, because a thumb on the comment then belongs to an earlier run. Narrowed because a cancellation lands during the driver far more often than after step 13, so your sequence is the common one. One case is left over and the comment states it: a run that answers, then dies before publishing, leaves the comment bare — the answer step already took the earlier thumb, so nothing at the end of the job can restore it. And one tension I am flagging rather than reading loosely: the stated criterion "its trigger comment carries no reaction from this bot" now fails on a cancelled re-run, which keeps an earlier honest thumb. The intent holds; the literal wording does not.

Finding 2 — and two more of the same class. Both file-level checks now walk every job's raw steps list and search the whole step. Your prediction was already live: the guard job carries an unnamed step today. The id check also reached only if, so the withdrawal's new env read would have gone unchecked. And run_case exported per-case knobs without clearing them, so an ANSWERED_AS override leaked forward and disarmed a later case.

Finding 3 — deleted.

Finding 4 — Bugbot, and it was right. steps.verdict.outcome cannot carry "did the comment land": Post the verdict tolerates a refused POST and ends on a swallowed call, so it exits 0 and reads success either way. The gate now reads a posted output written from the POST's own result. conditions.py models the outcome beside it, derived as the runner sees it, so a regression to the outcome fails an assertion rather than crashing the model.

Five checks on this PR ran narrower than their own claim — env-only haystack, id-blind model, name-keyed sweep, leaking knobs, and a gate reading an exit code. Same shape each time: the check named a property but sampled one key, one name, or one argument.

	result
reactions.sh	62 passed, 0 failed (32 cases)
conditions.py	75 passed, 0 failed
incumbent run.sh	165 passed, 0 failed

actionlint identical base vs branch (37 whole-dir, 4× SC2102:info, 0 on the test workflow). shellcheck -S warning clean. YAML parses.

Base moved twice mid-review and neither move was trusted: harnesses and actionlint re-ran on each new history, and I checked the rebase by reading back the step ids, the gate and the withdrawal step's POST count rather than its exit status.

Nothing ran on a GitHub runner.

[github-actions]

The fix is sound: the withdrawal step is genuinely last in the job, reads only two non-conclusion booleans (steps.verdict.outputs.posted, steps.answer.outcome), contains no POST path, and Post the verdict writes its posted output on both branches before its early exit 0. Two non-blocking notes, both about the new test harnesses rather than the workflow behaviour.

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

Blockers

• None at the file/PR level.

Non-blocking

• bin-reactions/gh and bin/gh are now two independent gh stubs with overlapping surface (-X, --jq, --paginate, -f). Not worth unifying today, but a third harness would be the point to factor out the argument parser.
• 2 suggestion(s)/nit(s) flagged inline on specific lines.

[github-actions]

[suggestion] This env: key is the single link between the two steps, and neither new harness asserts it exists.

extract.py copies only a step's run: block, and reactions.sh exports ANSWERED itself (line 127). conditions.py's steps.<id> check searches the dumped step for steps.answer., so it catches a dangling reference — but not a renamed or deleted key.

Concretely: rename this to ANSWER_OUTCOME and leave ${ANSWERED:-} in the run: block. steps.answer. is still present under the new key, so conditions.py passes; reactions.sh exports ANSWERED itself, so all cases pass. In production ${ANSWERED:-} is empty, which falls to the * arm and takes +1 -1 eyes — exactly the case rerun-cancelled-before-answer exists to prevent, silently stripping an earlier run's thumb whose verdict is on the pull request.

The same holds for Answer the request's CHECK / VERDICT_PRODUCED. Cheapest fix: assert the env mapping in conditions.py (e.g. that the withdrawal step's env.ANSWERED is ${{ steps.answer.outcome }}), or have extract.py emit the step's static env keys so the harness inherits the names rather than restating them.

[github-actions]

[nit] ids[ref] > i lets a step read its own id pass. A step can never usefully read steps.<its own id> from any key — if:, env: and with: are all evaluated before the step runs, and run: sees the value the step has not written yet — so the read is empty in every case, which is the same silent failure the check exists to catch. ids[ref] >= i covers it.

[bdchatham]

Rebased on 98c2619 (#102). 0d925ab. Ordering survived: the withdrawal is still the last step of the review job, at 17 of 17. The resolve step is at 15 and Report a review that reached no verdict at 16 — in the base those two are the last pair, so the withdrawal lands after both.

I closed the ordering class instead of adding the mutation. conditions.py now asserts the withdrawal is the last step of the review job, because any step after it is a step during which a cancellation leaves the eyes standing. Four mutations fail it, and only the first was already covered:

mutation	caught by
ahead of Post the verdict	position check and the id-ordering check
ahead of the resolve step	position check alone
ahead of the no-verdict report	position check alone
any step appended after it	position check alone

The id-ordering check passes the middle two, because Post the verdict still runs earlier in both. A single mutation would have tested one ordering; the position check covers a step appended next month.

One conflict, in the harness README, resolved keeping both sides. workflow-test-self.yml merged cleanly: place-findings keeps your renamed Place findings and resolve threads and the reaction job keeps its own name. While there I fixed the README saying "Two checks" where there are three, and describing the gate as Post the verdict having "completed" rather than having landed its comment — the same claim-versus-reality gap as the other five.

	result
reactions.sh	62 passed, 0 failed
conditions.py	76 passed, 0 failed
run.sh (#102)	271 passed, 0 failed

actionlint identical base vs branch (37 whole-dir, 4× SC2102:info, 0 on the test workflow). shellcheck -S warning clean. YAML parses. Withdrawal step still contains zero POSTs.

For #103. It was written against two reaction steps; this branch leaves three, with six hardcoded issues/comments paths rather than four — one in Acknowledge the trigger, three in Answer the request, two in the withdrawal step. Whichever of us rebases second has to reach all three steps.

Nothing ran on a GitHub runner. Third base move, third full re-run rather than a carried result.

[github-actions]

The withdrawal step, the new answer/verdict step ids and the two harnesses are coherent, and the reaction logic reads correctly across the cancellation windows the description enumerates. Two non-blocking issues: a backtick command substitution in the new shell harness, and no test coverage for the posted output the whole gate depends on.

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

Blockers

• None at the file/PR level.

Non-blocking

• The "cancelled mid-publish" row of the PR's table says the withdrawal takes a thumb that "would stand for nothing". That is not guaranteed: Post the verdict writes posted= only after the comment POST returns, so a cancellation that kills the step between a successful POST and that line leaves posted empty and the withdrawal strips a thumb that does answer a published verdict. It is a strictly narrower instance of the gap already documented for the answer step, but the table currently reads as if the row is safe rather than best-effort.
• run_case in test/seidroid-review/reactions.sh accepts a SKIP_ACK knob that no case ever sets, so the acknowledgement always runs. Either exercise it or drop it.
• 2 suggestion(s)/nit(s) flagged inline on specific lines.

[github-actions]

[suggestion] This output is the single fact the whole withdrawal gate rests on, and nothing tests that it is produced. reactions.sh never runs Post the verdict — it injects posted as a fixture argument to run_case — and conditions.py models the value in Ctx.values. So both harnesses stay green if this line is deleted, moved above the POST, or made unreachable by an early exit added higher in the script; the failure would only show up as a silently withdrawn thumb on a live run.

Given that the mutation table deliberately covers id: verdict being deleted and the gate regressing to steps.verdict.outcome, the producing side deserves the same treatment. Either extract this step into reactions.sh against the gh stub and assert posted=true / posted=false land in $GITHUB_OUTPUT on both POST outcomes, or add a conditions.py check that the step whose id the withdrawal reads writes posted= to $GITHUB_OUTPUT at all.

[github-actions]

[nit] The backticks are inside double quotes, so bash treats them as command substitution: it executes Post with args the verdict, prints Post: command not found to stderr, and substitutes empty — the banner comes out as == THE VERDICT COMMENT WAS REFUSED. tolerates that and exits 0,. set -e is not on, so the harness still passes, which is why it slipped through; shellcheck's SC2006 is style, below the -S warning threshold you ran at.

Use single quotes for this banner, or escape the backticks. The other multi-line banners in this file (e.g. lines 156-157, 187-188) avoid backticks and are fine.