fix(seidroid-review): fall back to GITHUB_TOKEN for the position and the dismissal

[bdchatham]

The step that records the review's position required the App identity, so a repository with no SEIDROID_APP_ID got a verdict comment and never appeared in the reviewers list. It now falls back to github.token, which is what every sibling publisher in this file already does.

What changed

• State the review's position on the pull request drops steps.identity.outputs.token != '' from its condition and reads steps.identity.outputs.token || github.token. The App still authors the position wherever a caller configured one.
• The position post gains a COMMENT rung, mirroring .github/workflows/ai-review.yml:983-986. When the API refuses APPROVE or REQUEST_CHANGES, the step retries the same body as COMMENT and warns.
• Two comments now state the present: the position step names what the fallback identity costs, and the Answer the request step names why it stands apart from the position step.

The withdrawal, its interlocks and its loud failure on a stuck block are untouched.

What a GITHUB_TOKEN review can and cannot do

Measured in this repository. github-actions[bot] — the login GITHUB_TOKEN carries — has created APPROVE reviews here through ai-review.yml's identical fallback: PRs #84, #83, #75, #74, #73, #72, #71, #70 and #61 all carry a github-actions[bot] APPROVED review whose body opens with <!-- ai-combined-review -->. It has also created REQUEST_CHANGES reviews and dismissed them: the timeline of #75 records two review_dismissed events with actor=github-actions[bot], state=changes_requested, message Superseded: latest AI review found no blocking issues. So the create path and the dismissal path both work under this repository's settings.

That the approvals land at all means sei-protocol allows Actions to approve a pull request. GitHub refuses the call outright when that setting is off, and none of these calls was refused.

Cannot. A COMMENT review casts no vote. Where the rung fires, an APPROVE clears no approval requirement and a REQUEST_CHANGES blocks no merge. The check run remains the gate in both cases.

The branch-protection question

Neither identity clears the review requirement on sei-chain today. sei-chain PRs #4094 and #4100 each carry exactly one review — seidroid[bot] APPROVED, from the App — and GraphQL reviewDecision on both reads REVIEW_REQUIRED. sei-chain's main is protected and carries .github/CODEOWNERS. So a bot approval, App-authored or not, does not satisfy that gate now. The fallback costs nothing there that the App identity was buying.

uci itself has no gate to satisfy. main reports protected: false, and the one ruleset (id 7955617, ~DEFAULT_BRANCH) is enforcement: disabled. Were it enabled it would ask for required_approving_review_count: 1, required_reviewers: [], require_code_owner_review: false, dismissal_restriction.enabled: false — a rule that names no reviewer and restricts no dismissal.

What I could not verify

• Whether a github-actions[bot] approval satisfies a required-approval rule. GET /repos/sei-protocol/uci/actions/permissions/workflow and the org equivalent both returned 403 for my token, so I could not read can_approve_pull_request_reviews directly — the successful approvals above are the inference. No repository in reach has a merge gate that an Actions approval has ever been put to.
• sei-chain's protection detail. GET /repos/sei-protocol/sei-chain/branches/main/protection returns 404 for my token. CODEOWNERS is the probable reason reviewDecision stays REVIEW_REQUIRED, not a measured one.
• A live run. Nothing here ran on a runner. The evidence is the incumbent's history, the API state above and the local battery below.
• The COMMENT rung under a real refusal. I never saw the API refuse a position, so the rung is exercised against a stub, not against GitHub.

Two consequences worth naming

1. A repository with no App now runs the withdrawal where it previously skipped the whole step. A dismissal the API refuses fails the job, by the design this step already states. That is the intended loud failure, and it is new exposure for those repositories.
2. If the first post lands server-side but the client reports a failure, the COMMENT retry writes a second review. ai-review.yml:968-991 carries the same hazard; the cost is one extra review, never a changed gate.

Verification

actionlint 1.7.12, against base 543f11f:

base: 4 findings, exit 1  -> 4 SC2102:info
new:  4 findings, exit 1  -> 4 SC2102:info
rule set diff: identical

Both findings pre-date this change and sit in steps it does not touch.

$ python3 -c "import yaml; yaml.safe_load(open('.github/workflows/seidroid-review.yml'))"
yaml ok
$ shellcheck -s bash <the position step's script>
(clean)

Behaviour battery. I extracted the step's script from the YAML at both revisions, put a stubbed gh on PATH, and ran twelve paths against each: event accepted, event refused, every event refused, approve off, blocking, blocking with the event refused, dismissals refused, review list unreadable, neutral with a blocker, neutral with nothing written down, neutral with no counts, and no check file. The base and the new step produce byte-identical output on every path but the two where the new COMMENT rung fires:

--- clean, approve on, event refused
-::warning::could not record APPROVE ...; the verdict comment stands
+::warning::... would not take APPROVE, so this review is recorded as a comment;
+  the check run carries the success conclusion
--- blocking, request changes refused
-::warning::could not record REQUEST_CHANGES ...; the verdict comment stands
+::warning::... would not take REQUEST_CHANGES, so this review is recorded as a
+  comment; the check run carries the failure conclusion

Both loud failures still exit 1: the stuck dismissal and the unreadable review list. All four withdrawal interlocks are unchanged — a blocker beside a soft conclusion stops the withdrawal, a soft conclusion with zero counts stops it, absent counts let it proceed with a warning, and a failure conclusion exits before it.

One thing the ticket got wrong

The ticket's phrasing — "drop the identity requirement from the step's condition" — was written against a base that still carried the cross-repository target grammar. bf507f3 removed that grammar, so the step now only ever posts to github.repository and dropping the requirement outright is safe. Against the older base it was not: github.token reaches no other repository, the withdrawal's review listing would have 404'd, and the step would have failed the job reporting a block it could not see.

🤖 Generated with Claude Code

[cursor]

PR Summary

Medium Risk
Changes merge-adjacent GitHub behavior (review votes, dismissals, skip-label gating) and expands loud job failure when dismissals are refused under the fallback identity.

Overview
Review position and dismissals no longer require a minted App token. The step that records APPROVE / REQUEST_CHANGES always runs when there is a verdict, using steps.identity.outputs.token || github.token, so repos without SEIDROID_APP_* still show up in the reviewers list and can run block withdrawal under github-actions[bot].

When GitHub refuses the requested review event (detected via 422 on the error body), the workflow retries as COMMENT and warns—matching the ai-review fallback—so policy refusals (e.g. Actions cannot approve) still leave a review thread while the check run stays the merge gate.

App identity is treated as all-or-nothing: HAS_REVIEWER_IDENTITY and the guard mint step require both SEIDROID_APP_ID and SEIDROID_APP_PRIVATE_KEY. Half-configured secrets get a workflow warning, and the skip-review label check denies review (fail closed) instead of silently ignoring the label. Docs and dismissal error text call out branch-protection dismissal restrictions when the posting identity changes.

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

[github-actions]

The fallback to github.token is correct — the job already declares pull-requests: write, every sibling publisher uses the same steps.identity.outputs.token || github.token form, and the new post_position helper is sound bash. Two non-blocking notes: the COMMENT rung retries on any failure rather than only on a refusal, and repositories without an App now execute the withdrawal path that was previously skipped.

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

Blockers

• None at the file/PR level.

Non-blocking

• Dropping the identity gate means a repository with no SEIDROID_APP_ID now runs the withdrawal block that was previously skipped along with the whole step, and that path exit 1s on a dismissal the API refuses. The narrow-but-real case is a repository that had the App, accumulated seidroid[bot] CHANGES_REQUESTED reviews, then dropped the credential: github-actions[bot] may not be permitted to dismiss those (protected branch with dismissal restrictions), and every subsequent clean run then fails the job persistently. The PR description names this as the intended loud failure; it is worth recording in the SEIDROID_APP_ID secret description too (line 257), since that is where an operator removing the credential will look.
• 1 suggestion(s)/nit(s) flagged inline on specific lines.

[github-actions]

[suggestion] The rung fires on any non-zero exit from the first POST, not just on the refusal it is written for. Two consequences:

• A transient 5xx, or a connection reset after the write lands server-side, produces two reviews on the pull request (the description acknowledges this; ai-review.yml carries it too).
• A 422 caused by a stale commit_id retries with the same commit_id, so the COMMENT attempt fails identically and only costs a second API call.

Scoping the downgrade to the statuses that actually mean "this identity may not hold this opinion" avoids both. Capturing the status inside post_position and gating on it is cheap, e.g. have it return a distinct code when stderr shows HTTP 403/HTTP 422 and retry as COMMENT only on that code, letting anything else fall straight through to the existing warning.

Non-blocking — the current behaviour is never worse than one extra review — but it would make the fallback mean what the comment above it says it means.

[bdchatham]

Both taken, in the commit above, and the change is rebased onto 19cb2fb (#85).

Scoping the rung. Right, and the comment above it claimed the narrower meaning it did not have. post_position now returns 2 for a 403 or a 422 and 1 for everything else, and only 2 earns the downgrade. Exercised against a stub:

first call succeeds        -> recorded APPROVE          1 call
422, COMMENT accepted      -> downgraded to COMMENT     2 calls
502 transient              -> could not record          1 call
403 on both                -> could not record          2 calls

The transient row is the one that changed: it used to spend a second call and could post a second review when the first write landed before the connection dropped.

Your second consequence stands and I did not close it: a 422 from a stale commit_id still downgrades and meets the same answer, because the status does not distinguish it from a refusal. That costs one call and no review, so it is no worse than today. Said in the code rather than left for the next reader to find.

The dropped-credential hazard. Recorded on SEIDROID_APP_ID, which is exactly where an operator removing it will look — the PR body was the wrong place for it. The description now says the workflow's own identity may not clear a block the App left, that every later clean run then fails on the dismissal, and to dismiss those reviews by hand when removing the credential.

Verification

actionlint base 4 SC2102 / head 4 SC2102   unchanged
yaml.safe_load                              parses
downgrade matrix                            4/4 as above

Still not exercised on a runner. I have never seen the API refuse a position from either identity here, so the rung itself is stub-proven only.

[github-actions]

The github.token fallback for the position step is correct and matches the pattern already used elsewhere in this file (e.g. the Read the threads step at line 710); the new post_position helper is sound bash and its 403/422-only retry gate genuinely avoids the duplicate-review hazard that ai-review.yml carries. One non-blocking note on the 403 classification, plus a pre-existing gap in how the optional-App gate is computed.

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

Blockers

• None at the file/PR level.

Non-blocking

• The change is well-scoped and internally consistent: a repo with no App now posts the position as github-actions[bot] and, because the same identity later runs the withdrawal, the block/clear loop stays self-consistent. approve-on-success remains opt-in and off by default, so the fallback does not silently start casting approving votes anywhere. The newly-exposed failure mode (App removed after it recorded CHANGES_REQUESTED) is documented on the SEIDROID_APP_ID secret description.
• 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] HAS_REVIEWER_IDENTITY (lines 295 and 593) is derived from secrets.SEIDROID_APP_ID != '' alone. A caller that configures the app id but not SEIDROID_APP_PRIVATE_KEY makes the actions/create-github-app-token step run and fail; that step has no continue-on-error, so the whole job aborts rather than falling back to github.token. Gating on both secrets would make the "App is optional" contract hold for a half-configured caller too.

[github-actions]

[suggestion] The comment above this line reads a 403 as "the API saying this identity may not hold this opinion," but 403 is also what GitHub returns for a secondary rate limit / abuse-detection trip, and for a token that simply lacks pull-requests: write. In the rate-limit case the immediate COMMENT retry can land where a short backoff on the original event would have succeeded, silently downgrading a REQUEST_CHANGES or APPROVE to a non-voting comment behind only a ::warning::. (The permission case is harmless — the retry meets the same 403 and falls through.)

422 is the code GitHub actually returns for both refusals the rung is built for ("GitHub Actions is not permitted to approve pull requests" and self-approval), so narrowing the rung to 422, or keeping 403 but excluding messages matching rate limit/abuse, would keep the vote in the one case where retrying the original event still had a chance.

[bdchatham]

Both taken in 9d0bb47. I narrowed the rung to 422 alone rather than filtering 403 by message.

Finding 1 — the 403 classification. You are right that 403 is overloaded here, and I verified the rate-limit half two ways. GitHub's rate-limit page states it outright: "If you exceed a secondary rate limit, you will receive a 403 or 429 response and an error message that indicates that you exceeded a secondary rate limit." And this endpoint's own OpenAPI description names itself as a trigger: "Creating content too quickly using this endpoint may result in secondary rate limiting." So the concern is not hypothetical for pulls/create-review specifically.

I measured the self-review half live against this API, on this PR, which I authored:

$ gh api -X POST repos/sei-protocol/uci/pulls/88/reviews -f event=APPROVE -f body=probe
gh: Unprocessable Entity (HTTP 422)
{"message":"Unprocessable Entity","errors":["Review Can not approve your own pull request"],"status":"422"}
$ gh api -X POST repos/sei-protocol/uci/pulls/88/reviews -f event=REQUEST_CHANGES -f body=probe
gh: Unprocessable Entity (HTTP 422)

Review count on #88 was 3 before and 3 after, so the probe created nothing.

Why 422 alone and not a message filter. The two errors are not symmetric. Excluding rate limit/abuse by message leaves a block hostage to GitHub's wording, which no contract fixes — the docs promise only "an error message that indicates" the limit. A miss downgrades a REQUEST_CHANGES to a comment that blocks nothing, and rate limits fire exactly when the tool is busy. The narrow rule's cost is the reverse: a policy refusal arriving as 403 gets no comment review, only the ::warning::. That loses an entry in the reviewers list while the verdict comment and the check run still carry everything. A vote is worth more than a list entry, so the rule protects the vote. 422-only is also immune to the 429 form, which a 403 filter would miss entirely.

What I could not verify. Your claim that 422 covers both refusals is confirmed for self-review and unconfirmed for "GitHub Actions is not permitted to approve pull requests". I cannot produce that error: GET /repos/sei-protocol/uci/actions/permissions/workflow returns 403 for my token, and the setting is evidently on here anyway — github-actions[bot] has landed APPROVE reviews on #84, #83, #75, #74, #73, #72, #71, #70 and #61. GitHub's Actions settings docs describe the toggle without stating a status code. If that refusal is a 403, this rung will not answer it; that is the cost I chose, and it is named in the code. I have still never seen the API refuse a position on a real run, so the rung remains stub-proven.

Finding 2 — the half-configured caller. Confirmed and fixed. HAS_REVIEWER_IDENTITY is derived at exactly two sites, jobs.guard.env and jobs.review.env, and consumed at exactly two, the guard's mint and the review's mint. There are no others; grep -n "HAS_REVIEWER_IDENTITY\|secrets.SEIDROID" returns those four plus the two app-id/private-key pairs. Neither mint carries continue-on-error, so your read is right: an id with no key aborted the job. Both sites now require both secrets.

Verification. actionlint 1.7.12 against origin/feat/seidroid-review (19cb2fb): 4 findings before, 4 after, all SC2102:info, rule set diff empty. YAML parses; shellcheck -s bash on the extracted step is clean.

Stub matrix, blocking conclusion, counting review POSTs — one POST means the rung did not fire:

MODE            POSTs  OUTCOME
ok              1      recorded REQUEST_CHANGES
422-self        2      would not take REQUEST_CHANGES -> COMMENT
403-rate        1      could not record REQUEST_CHANGES
403-perm        1      could not record REQUEST_CHANGES
429-rate        1      could not record REQUEST_CHANGES
500-flake       1      could not record REQUEST_CHANGES
timeout         1      could not record REQUEST_CHANGES
comment-fails   2      could not record REQUEST_CHANGES

Diffing the full 18-path matrix against cb17813 shows three changed lines, all three the 403 cases. Every 422 path, both loud failures and all four withdrawal interlocks are byte-identical.

[github-actions]

The fallback to github.token for the position step is correct and consistent with every sibling publisher in this file, and the post_position helper is sound bash (redirection order, local array, errexit-safe call sites all check out). Three non-blocking observations: the COMMENT rung's 422-only trigger may not cover the Actions-policy refusal the step header cites as its motivation, the widened if newly exposes App-less repositories to a job-failing dismissal, and requiring both credential halves converts a loud misconfiguration into a silent one.

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

Blockers

• None at the file/PR level.

Non-blocking

• The COMMENT rung is exercised only against a stub per the PR description — no run has seen the API actually refuse a position. That is acceptable given this repository has no workflow test harness, but the rung's trigger condition (422) is the part that is unverified, and it is also the part that decides whether the rung fires at all.
• 1 suggestion(s)/nit(s) flagged inline on specific lines.

Comments that couldn't be anchored to the diff

• .github/workflows/seidroid-review.yml:1299 -- [suggestion] The rung is gated on 422 only, but the step header at lines 1199-1205 names "a repository whose Actions policy forbids an approval refuses the event outright" as the case "the COMMENT rung below answers" — and the comment at 1282-1284 then concedes that same refusal may arrive as 403 and earn no rung. If it does arrive as 403, a no-App repository running with approve-on-success gets only a warning and never appears in the reviewers list, which is the gap this PR set out to close.

Worth noting too that ai-review.yml:979-989, cited as the precedent, retries as COMMENT on any error from the body-only post, not on a status code — so this is narrower than the thing it mirrors.

The asymmetry the surrounding comment argues for is real, but it only applies to REQUEST_CHANGES: downgrading a rate-limited block to a comment loses a gate, whereas an APPROVE that the API just refused was never going to cast a vote. That points at a rung keyed on the event rather than on the status alone:

case "$err" in
  (*"HTTP 422"*) rc=2 ;;
  (*"HTTP 403"*) if [ "$1" = APPROVE ]; then rc=2; fi ;;
esac

That covers the policy refusal whichever status carries it, and keeps REQUEST_CHANGES on the 422-only rule that protects the block.

• .github/workflows/seidroid-review.yml:310 -- [suggestion] Requiring both halves is the right fix for the crash this describes, but it changes a loud misconfiguration into a silent one. With SEIDROID_APP_ID set and SEIDROID_APP_PRIVATE_KEY empty, the previous behaviour was a red guard job from create-github-app-token. Now the mint is skipped, GH_TOKEN in Admit the request is empty, and — for any caller that also sets allowed-team — line 491 takes the deny "no identity to check ... denying" branch, so every comment-triggered review is refused with a ::notice:: and no signal on the pull request at all.

A half-configured App is a typo, not a supported configuration. Consider naming it where the machine-client check already names its own missing secret (line 517), e.g. an ::error:: when exactly one of the two is set, so the operator learns it from a failing job rather than from a review that quietly stops happening.

[github-actions]

[suggestion] Dropping the identity gate also means the withdrawal at the end of this step now runs in repositories that have no App — where it previously never ran — and a refused dismissal exits 1 (line ~1414), reddening every subsequent clean run until a human clears the block. The PR names this as intended, and the loud failure is the right default.

The mitigation, though, lives only in the SEIDROID_APP_ID secret description, which is the one place an operator staring at a red job will not look. The ::error:: in the dismissal loop currently says only "could not withdraw review $id ... it still blocks the merge on a finding this run did not reproduce". Adding the likely cause and the remedy there — a block recorded under an identity this run no longer holds, dismiss it by hand — would put the answer in the log next to the failure.

[bdchatham]

Taken in 80bb314, rebased onto bf0fd23 (#87 merged). The exit and the interlocks are untouched — only the message changed.

::error::could not withdraw review $id on $REPO#$PR; it still blocks the merge on a
finding this run did not reproduce. This run may not hold the identity that recorded
it: removing SEIDROID_APP_ID leaves the App's blocks behind. Dismiss review $id by
hand to clear it

The secret description stays as well. One is for the operator configuring, the other for the operator debugging, and neither reaches the other's reader.

Also folded in: the half-configured caller. Requiring both credential halves did convert a loud misconfiguration into a silent one, so the guard now warns and names the half that is missing. I broadened it past what was raised — either half alone earns the warning, not just a missing key. A diagnostic with a blind spot sends the reader to inspect the half that is already set, which costs more time than the four lines it saves.

both secrets set     -> (silent)
id set, key missing  -> ::warning::SEIDROID_APP_PRIVATE_KEY is not set beside the
                        other half of the App credential, so this review posts as
                        github-actions[bot] rather than as the App. ...
key set, id missing  -> ::warning::SEIDROID_APP_ID is not set beside the other half ...
neither set          -> (silent)

On the rung's 422 trigger, restating the gap plainly. You raised this on the previous thread and my own measurement is the honest answer: 422 is confirmed for self-review, and unconfirmed for the Actions-policy refusal. I measured the first live against this API on this PR, which I authored — gh: Unprocessable Entity (HTTP 422), {"errors":["Review Can not approve your own pull request"]}, with the review count on #88 at 3 before and 3 after, so the probe created nothing. I cannot produce the second. GET /repos/sei-protocol/uci/actions/permissions/workflow returns 403 for my token, the setting is evidently on in this org anyway, and GitHub's Actions settings docs describe the toggle without naming a status code. If that refusal arrives as 403 this rung will not answer it, and an approval the policy forbids gets no comment review — only the ::warning::. That is the cost I chose over leaving a block hostage to error-message wording, and it is stated in the code. I have still never seen the API refuse a position on a real run, so the rung stays stub-proven.

Verification. actionlint 1.7.12 against origin/feat/seidroid-review (bf0fd23): 4 findings before, 4 after, all SC2102:info, rule-set diff empty. YAML parses; shellcheck -s bash clean on both extracted steps.

The 18-path stub matrix differs from cfafeb0 in exactly two lines, both the new error text, one per stuck review id:

### clean, dismissals refused   [mode=stuck]   expect: LOUD failure rc=1
    recorded APPROVE on owner/name#7
    gh: Forbidden (HTTP 403)
    ::error::could not withdraw review 901 ... Dismiss review 901 by hand to clear it
    gh: Forbidden (HTTP 403)
    ::error::could not withdraw review 902 ... Dismiss review 902 by hand to clear it
    superseded blocks: 0 withdrawn, 2 still standing
    rc=1

Every exit code across all 18 paths is identical to the previous commit — four rc=1 before, four after — and the review-list failure, the four withdrawal interlocks and every 422/403 classification path are byte-identical.

[github-actions]

The fallback from the App token to github.token for the position step is sound and consistent with the sibling publishers in this file, and the 422→COMMENT rung mirrors ai-review.yml. No blockers; a few notes on operator-facing accuracy and one behaviour change the tightened HAS_REVIEWER_IDENTITY introduces for half-configured repositories.

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

Blockers

• None at the file/PR level.

Non-blocking

• The withdrawal now runs for repositories that never configured an App, where the whole step was previously skipped. That is the stated intent, but it means a repository whose main restricts who may dismiss reviews will get exit 1 from this step on every subsequent clean run once github-actions[bot] has recorded one CHANGES_REQUESTED — a permanently red job until a human dismisses. Worth a line in the caller-facing docs alongside the SEIDROID_APP_ID description, since the failure surfaces on a run that found nothing wrong.
• The 422→COMMENT rung is only exercised against a stub (per the PR description). Since the whole rung hinges on GitHub answering a refused position with 422 and not 403, a single recorded real refusal — even pasted into the step comment — would be worth more than the reasoning currently there.
• 3 suggestion(s)/nit(s) flagged inline on specific lines.

[github-actions]

[suggestion] This names App removal as the cause of a stuck dismissal, but that is usually not it. PUT .../reviews/{id}/dismissals does not require the review's author — any token with write access on the pull request can dismiss another actor's review. What it does require, per the API docs, is that on a protected branch the caller be a repository admin or be listed under "restrict who can dismiss pull request reviews". So the likely causes here are a dismissal restriction on the protected branch, or a read-only token, not the identity that authored the review.

Sending an operator to check SEIDROID_APP_ID during a permanently-red pull request costs them the first thing they look at. Consider naming both causes, or dropping the attribution and keeping the actionable half ("Dismiss review $id by hand to clear it"). The same claim in the new SEIDROID_APP_ID description ("That identity also may not clear a block the App left") reads on the same assumption.

[bdchatham]

You are right, and the earlier text named the wrong cause. That is on the record: 80bb314 told an operator that a stuck dismissal came from removing SEIDROID_APP_ID, and the API does not work that way. Fixed in e7e326a.

I checked it against GitHub's own OpenAPI description rather than taking it on trust. PUT /repos/{owner}/{repo}/pulls/{pull_number}/reviews/{review_id}/dismissals carries exactly one note:

To dismiss a pull request review on a protected branch, you must be a repository administrator or be included in the list of people or teams who can dismiss pull request reviews.

Nothing conditions it on having authored the review, and the endpoint declares only 200, 404 and 422 — no 403. The ruleset schema agrees: dismissal_restriction: {enabled, allowed_actors} is the list the note means, and this repository's own ruleset carries it with enabled: false.

The error now names the causes that are true:

::error::could not withdraw review $id on $REPO#$PR; it still blocks the merge on a
finding this run did not reproduce. On a protected branch the API takes a dismissal
only from a repository admin or an actor named in the branch's dismissal restriction,
so check that this run's identity is one of them. A token without pull-requests: write
is refused as well. Dismiss review $id by hand to clear it

Identity survives only as a sub-question of the restriction list — that list names actors, so which identity the run holds decides whether it is on it. It no longer leads.

The SEIDROID_APP_ID description carried the same wrong assumption and is rewritten on the same evidence: changing the credential changes who withdraws a block because the restriction list names actors, not because the recorder has a claim on its own review.

[github-actions]

[suggestion] Requiring both halves is right for the mint, but it silently changes what a half-configured repository does with skip-review-label. Before: SEIDROID_APP_ID set with no private key made HAS_REVIEWER_IDENTITY true, the mint ran, failed, and took the guard job with it — no review. Now the mint is skipped, so Admit the request (line 447) runs with an empty GH_TOKEN, and its label check is gated on [ -n "${GH_TOKEN:-}" ] and fails open. A pull request carrying the skip label gets reviewed, where it previously could not.

The team check is unaffected — it still fails closed at line 490. The new warning step announces the half-credential, but it runs after admit, so the label is already ignored by then.

If you want the label gate to survive the fallback, Admit the request would need GH_TOKEN: ${{ steps.identity.outputs.token || github.token }} plus pull-requests: read on the guard job, which currently declares permissions: {}. Otherwise it is worth stating in the label input's description that the gate needs the App identity.

[bdchatham]

Confirmed and closed in e7e326a. Your trace is exact, including that the warning step runs after admit and so cannot help.

How I closed it. The label check now fails open only for a caller that configured no App, and closed for one that configured half of one. Admit the request reads both secret-presence values as step-level env and denies when it holds no token but a half-credential says one was intended.

I did not take the github.token + pull-requests: read route. That fixes the gate for every no-App caller, which is a user-visible change for repositories that deliberately run without the App and rely on the label failing open today — and it is already filed as PLT-1169. Widening the guard from permissions: {} is that ticket's call, not this PR's. What I closed is the regression this change introduced, no more.

Measured, both revisions, same stub:

                                                     80bb314      e7e326a
App configured, PR carries the label                  false        false
App configured, PR clean                              true         true
no App at all, PR carries the label                   true         true     <- PLT-1169, untouched
HALF configured (id, no key), PR carries the label    true         FALSE    <- the regression
HALF configured, PR clean                             true         FALSE
HALF configured (key, no id), PR carries the label    true         FALSE
HALF configured, teardown                             true         true     <- sandbox reclaim survives
HALF configured, caller disabled the label            true         true     <- no control, no denial

The denial says why: half of the App credential is set, so ai: skip-review cannot be read on owner/name#7; not reviewing. Pass both secrets, or unset the half that is set.

On the net effect for a half-configured caller. Before this PR that configuration reviewed nothing — the mint failed and took the guard with it. It still reviews nothing, but now says which secret is missing instead of dying inside create-github-app-token. The improvement this PR set out to make was a legible failure, not a review under an identity the caller did not choose, and the label invariant holds either way.

[github-actions]

[nit] This couples the rung to gh's human-readable stderr. Current gh formats a REST error as gh: <message> (HTTP <code>), so this matches today, but that string is not part of gh's compatibility surface and nothing else in this file depends on it — a reformat upstream turns the rung into a silent no-op that degrades to the pre-change behaviour, with no test to catch it.

gh api -i puts the status line on stdout, which this already discards; capturing stdout instead of dropping it would let you read the code directly. Not worth reworking if you would rather keep the shape simple — but the fragility is worth a word in the comment block above, which currently reads as though 422 detection is guaranteed.

[bdchatham]

Taken in e7e326a, and a machine-readable status does exist — I did not need -i. gh api already writes the response body to stdout on a failure, and that body is GitHub's error object with the code in a status field. The old code threw that away with 2>&1 >/dev/null.

Measured against this API, on this PR, which I authored (review count 7 before and 7 after, so it created nothing):

$ gh api -X POST repos/sei-protocol/uci/pulls/88/reviews -f event=APPROVE -f body=probe
stdout: {"message":"Unprocessable Entity","errors":["Review Can not approve your own
         pull request"],"documentation_url":"...","status":"422"}
$ ... | jq -r '.status // empty'
422

So the rung now reads:

body="$(gh api "${args[@]}")" && return 0
status="$(printf '%s' "$body" | jq -r '.status // empty' 2>/dev/null || true)"
if [ "$status" = "422" ]; then
  return 2
fi
return 1

gh's own line is no longer parsed; it still reaches the log unredirected, where it says why.

The fragility that remains, stated in the comment block as you asked. status is documented on GitHub's Basic Error schema but the 422's own schema — validation-error-simple — declares only message, documentation_url and errors. So the field is observed and documented in general, not contract-guaranteed for this exact response. An absent status returns 1: no rung, no downgrade. The vote survives a shape the code cannot read, which is the same direction the 422-over-403 rule already chose.

Proof the coupling is gone. I added a stub mode that keeps the error body intact and reformats only gh's line to error: 422 Unprocessable Entity [reformatted upstream] — your exact scenario:

                      POSTs   (2 = rung fired, 1 = silent no-op)
80bb314  string match   1     <- the silent no-op you predicted
e7e326a  status field   2     <- rung still fires

And the fail-safe, with a body that omits status:

422-nostatus            1     <- no rung, vote kept

Full 18-path matrix against 80bb314: two changed lines, both the corrected dismissal error text. Every exit code identical, all four withdrawal interlocks and both loud failures byte-identical. actionlint 4 findings before and after against bf0fd23, all SC2102:info, rule-set diff empty.

[github-actions]

The core change is sound — the position step's steps.identity.outputs.token || github.token fallback now matches every sibling publisher in this file, and the review job already carries pull-requests: write, so both the create and the dismissal paths work under GITHUB_TOKEN. The undescribed half-credential handling added alongside it is where the problems are: a caller with one of the two App secrets set now has every review silently denied while a new step simultaneously warns that the review will post as github-actions[bot], and the new COMMENT rung fires on a JSON field the 422 schema does not declare.

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

Blockers

• None at the file/PR level.

Non-blocking

• The PR description covers only the position-step fallback and the COMMENT rung, but the diff also changes HAS_REVIEWER_IDENTITY in both jobs, adds a fail-closed deny path to the skip-label check, and adds a whole new guard step. The twelve-path behaviour battery described in the body exercises the position step only, so the guard changes — the ones that can stop a review entirely — arrive unverified. Worth extending both the description and the battery to cover them.
• The inline comment at line 1340 says status is "measured on this endpoint's 422", while the PR description says "I never saw the API refuse a position, so the rung is exercised against a stub, not against GitHub." One of the two is wrong, and the comment is the one a future reader will trust.
• 5 suggestion(s)/nit(s) flagged inline on specific lines.

[github-actions]

[suggestion] skip-review-label defaults to 'ai: skip-review' (line 129), so SKIP_LABEL is non-empty unless a caller deliberately clears it. That means this branch denies every review — automatic and comment-triggered alike — for any repository that has exactly one of SEIDROID_APP_ID / SEIDROID_APP_PRIVATE_KEY set, since HAS_REVIEWER_IDENTITY (line 313) now requires both halves and GH_TOKEN is therefore empty.

The visibility is the sharper half. Before this PR that configuration ran the mint step and failed it, taking the guard job red — loud and impossible to miss. Now deny() writes a ::notice:: and exits 0, so the guard is green, the review job is skipped, and on the comment path the requester gets no reaction and no reply at all. A workflow that silently reviews nothing is a worse failure mode than one that goes red.

If the fail-closed policy is intended, consider making this path ::error:: with a non-zero exit rather than a notice, so the misconfiguration stays as visible as it was.

[github-actions]

[suggestion] This warning contradicts the deny at line 522 in exactly the configuration it fires in. Both steps are gated on steps.parse.outputs.should_run == 'true' — not on steps.admit.outputs.admit — so with a half-set credential and the default skip-label the log carries ::notice::half of the App credential is set ... not reviewing and then ::warning::... so this review posts as github-actions[bot] rather than as the App. No review posts at all.

Either gate this step on steps.admit.outputs.admit == 'true', or reword it so it does not promise a review the step above already refused.

[github-actions]

[suggestion] The whole COMMENT rung hinges on .status being present in the error body, and GitHub's Validation Error schema — the one a 422 on this endpoint returns — declares only message, documentation_url and errors. status is on the Basic Error schema. The comment above acknowledges this and chooses return 1 on an absent code, which is safe, but the consequence is that the rung may simply never fire: the step falls back to the same could not record $event warning it emitted before this PR, and the new code is dead.

Since the PR description says the rung was only ever exercised against a stub, this is untested against a real refusal. A reading that does not depend on an undeclared body field: capture the response line with gh api -i and match on 422, which is part of the HTTP contract rather than the payload schema — and unlike gh's gh: ... (HTTP 422) stderr string, it is not a human-facing message a reformat could silently break.

[github-actions]

[nit] When the 422 came from a stale commit_id, retrying COMMENT with the same commit_id costs a call and lands nothing — the pull request ends up with no review at all, which is the case the rung exists to avoid. Dropping commit_id on the retry would let the comment land on the current head, which is already the announced fallback at line 1299 when the SHA was never recorded. E.g. give post_position a second argument that suppresses the commit_id append, and pass it on the COMMENT call.

[github-actions]

[suggestion] This PR applies steps.identity.outputs.token || github.token to every publisher in the review job, but the guard's GH_TOKEN at line 451 keeps the App-only form — which is what forces the fail-open / fail-closed split being added here. Giving the guard job permissions: { pull-requests: read } (it currently declares permissions: {}) and using the same || github.token fallback would make the skip-label readable in every configuration, including the no-App one that still fails open today, and would let this branch collapse back to the single gh api ... | grep -qxF check. Worth considering as the simpler shape, consistent with what the rest of the diff does.