fix(ci): report docs-check on Claude-authored PRs - #131
Conversation
`docs-check / docs-check` is a required status check in the `main` ruleset, and that two-part context name only exists while the job runs: the job calls a reusable workflow, so GitHub reports one check per inner job prefixed by the caller's job id. When the caller is skipped there are no inner jobs, so a single check named `docs-check` is reported instead and the required `docs-check / docs-check` context never reports at all. A required context that never reports counts as pending forever, so any bot-authored PR sat at `mergeable_state: blocked` with no failing check to point at. PR #121 is currently deadlocked this way. The job's `if:` skipped every bot via `!contains(github.event.pull_request.user.login, '[bot]')`. Replace that with a check on the immutable numeric account id so this repo's agent PRs are documentation-checked while unrelated bots stay skipped: github.event.pull_request.user.type != 'Bot' || github.event.pull_request.user.id == 209825114 209825114 is the `claude[bot]` account (Claude GitHub App, app id 1236702), read off the author of PRs #120 and #121. GitHub assigns that id at account creation and it survives renames, whereas a login is mutable and a `[bot]` suffix is a naming convention rather than an identity. `user.type` is GitHub's own account-kind field, so Dependabot and Renovate remain skipped without matching on that suffix. The check itself is unchanged — it now runs and reports on bot PRs rather than being bypassed. Fork PRs are still skipped, as they have no access to the secrets the reusable workflow needs.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🤖 Claude PR Metadata GenerationStatus: ❌ Error
|
|
|
● Reviewed · against Note Approved. Fixes the workflow-level gate so AssessmentGate switches from login pattern matching to immutable numeric ID (209825114). Fork protection preserved; unrelated bots still skipped. Partial fix: resolves the workflow-level gate but not the inner Iteration history · 4 reviews2026-08-10 19:38 UTC · ✅ approved · 0 findings ·
|
Clearing the workflow-level `if:` was necessary but not sufficient. The
job now starts and reports the required `docs-check / docs-check` context
(the deadlock is broken), but it then fails inside
`anthropics/claude-code-action`'s own actor check:
Workflow initiated by non-human actor: claude (type: Bot).
Add bot to allowed_bots list or use '*' to allow all bots.
That action gates on its `allowed_bots` input, which defaults to ""
(allow no bots). This repo cannot set it: `_claude-docs-check.yml` in
Uniswap/ai-toolkit declares 14 workflow_call inputs, none of them
`allowed_bots`, and neither of its two claude-code-action steps forwards
one. Verified against the pinned SHA c4820d6 and against the toolkit's
`main` and `next`, which are byte-identical to it — so there is no input
to pass and no newer ref that helps. The fix belongs in ai-toolkit.
Document both gates in the workflow comment and in
.github/workflows/CLAUDE.md so the next person does not conclude the
workflow-level condition is the whole story, and record why the
follow-up gate is name-based: the action matches `github.actor` with
case-folding and a stripped `[bot]` suffix, consults the list only for
accounts GitHub reports as non-User, and offers no id-based equivalent.
The gate this repo does control keeps using the numeric account id.
No behaviour change; comments and docs only.
…lers Pin down why the inner actor check cannot be satisfied from this repo, so nobody spends another cycle looking for a value to set here. Both ai-toolkit reusable workflows this repo calls decide `allowed_bots` themselves and expose no input for it: _claude-docs-check.yml passes nothing -> action default "" _generate-pr-metadata.yml passes `allowed_bots: dependabot` (hardcoded) So `claude` is rejected by the first because no bot is allowed, and by the second because it is not in the list. Same actor check, same root cause, and neither is reachable from a caller. That also explains why generate-metadata fails on bot PRs. Also correct a plausible-looking dead end: the error text prints the actor as `claude` rather than `claude[bot]`, which reads like a spelling mismatch. It is not. The action's `isAllowedBot` lowercases and strips a trailing `[bot]` from both the configured entries and the actor, so `claude` and `claude[bot]` are the same entry; the message just prints the normalized form. Record the diagnostic tell as well: on a rejected run `Run Claude Docs Check` fails and `Process Results` is skipped, so no verdict exists, while `Set Exit Code` still prints "Documentation check passed" against an empty VERDICT. Reading only the last line of the log suggests the docs were checked and passed, which is the opposite of what happened. No behaviour change; docs only.
…n gate The actor gate is inherited by any workflow in this repo that wraps `anthropics/claude-code-action`, and only by those. `claude-code-review.yml` runs `@uniswap/review-cli` directly and references the action nowhere, so it has no actor check to satisfy. Confirmed empirically: on the two claude[bot]-authored PRs (#121, #131) the reviewer's `Triage` and `AI review` checks pass, while both claude-code-action-based workflows (docs-check, generate-metadata) fail on those same PRs. This is a substantive argument for folding the docs check into the review CLI rather than repairing the toolkit call, so record it next to the gate it bears on. No behaviour change; docs only.
There was a problem hiding this comment.
Note
✅ Approved — see full review in the sticky comment ↑
Set `auto_fix: true` on the `_claude-docs-check.yml` call so a FAIL verdict is repaired and pushed instead of only reported. The toolkit runs a second Claude pass scoped to Read/Edit/Write/Grep/Glob/Bash(git:*), commits as `docs(claude): fix documentation issues (auto-fix)`, and pushes directly to the PR head branch using `WORKFLOW_PAT` so the push re-triggers the check. `auto_commit` is deliberately left dispatch-only. It is a different mechanism — it overwrites whole files with the model's `suggested_content` and carries no loop guard — and its commit subject does not match the anchored regex `auto_fix`'s loop guard keys on, so enabling both would make the two push over each other indefinitely. Fork PRs are unaffected: the job-level `if:` already requires `head.repo.full_name == github.repository`, and every auto-fix step inside the reusable workflow re-checks the PR's live head repo independently. Auto-fix is dormant on bot-authored PRs until ai-toolkit plumbs `allowed_bots` through, because the job still fails at claude-code-action's actor check before any verdict is produced.
Removes `auto_fix: true` from this change at Nick's direction. The identity-gate fix is kept: the required context `docs-check / docs-check` now reports on PRs authored by the Claude GitHub App, matched on the immutable numeric account id 209825114 rather than a mutable `[bot]` login substring. Other bots and fork PRs are still skipped. The docs check stays read-only. It reports a verdict and leaves suggestions; it never commits to or pushes a contributor's branch. `auto_commit` remains manual-dispatch-only and is unchanged. Documentation for the deadlock mechanism is retained; the section describing auto-committing doc fixes is removed along with the feature.
📚 Documentation Check ✅Verdict: Passed Documentation change accurately mirrors the workflow code change; no plugin package touched so version bump rule is not applicable. This PR changes the docs-check job if condition in claude-docs-check.yml to an id-based bot check and documents the change thoroughly and accurately in .github/workflows/CLAUDE.md. No packages/plugins files changed, so no version bump required. Suggestions (1)
🤖 Generated by Claude Documentation Validator | Mode: |
There was a problem hiding this comment.
Note
✅ Approved — see full review in the sticky comment ↑
The web-UI edit dropped the blank line before `### Generate PR Title & Description` in `.github/workflows/CLAUDE.md` and left a trailing blank line at the end of that file and of `.github/workflows/claude-docs-check.yml`, which failed `nx format:check`. Applied via `nx format:write`; whitespace only, no semantic change.
There was a problem hiding this comment.
Note
✅ Approved — see full review in the sticky comment ↑
Requested via Slack thread
Before / after
Before. Any PR opened by the repo's own agent could never be merged. The
mainruleset requires the status contextdocs-check / docs-check, but thedocs-checkjob skipped every bot author — and a skipped job does not report that context, so the requirement sat unsatisfied forever. There was no failing check to look at and nothing to re-run; the PR just showedmergeable_state: blockedindefinitely. PR #121 is deadlocked this way right now.After. The job runs on the agent's PRs and reports the required context under the exact required name, so the requirement is satisfiable rather than permanently pending. Unrelated bots (Dependabot, Renovate) are still skipped, and the check itself is unchanged — nothing is bypassed or weakened.
On head
2cb77e0the reported conclusion wassuccess, carrying a real documentation verdict rather than an infrastructure error:PASS, zero missing updates, with the one attached suggestion being informational ("documentation already covers the change").Process Resultsran, so that verdict is genuine and not the empty-VERDICTfalse pass described below.Still outstanding. That pass is not evidence the second gate is fixed, because it is not. The run that produced it was triggered by a human push, so
github.actorwaswkoutre(User) and the actor check never engaged. The very next commit on this branch (7eb5122, a whitespace-only prettier fix pushed by the bot) putgithub.actorback toclaude[bot], and the same context returned tofailurewithWorkflow initiated by non-human actor— same docs, same gate condition, different pusher. What this PR durably delivers is the workflow-level fix — the required context reports at all, under the exactly required name — which is a precondition for any docs verdict, human- or bot-triggered. Making that verdict independent of who pushed last needs a change inUniswap/ai-toolkit, not here.Why a skip deadlocked instead of passing
This is the non-obvious part, and it is why the usual intuition ("a skipped required job reports
skippedand satisfies the rule") does not apply.docs-checkcalls a reusable workflow. A job that does so reports one check per job inside that workflow, named<caller-job-id> / <inner-job-name>— hence the requireddocs-check / docs-check. Skip the caller and there are no inner jobs to report, so GitHub emits a single check named plaindocs-check, which nothing requires. The required two-part context never appears at all, and GitHub treats a required context that never reports as still pending.Observed directly, before and after:
claude[bot]wkoutre(User)docs-check(not the required name)skippedwkoutrewkoutre(User)docs-check / docs-checksuccessc25b390claude[bot]claude[bot](Bot)docs-check / docs-checkfailure— inner actor gate, no docs verdict2cb77e0claude[bot]wkoutre(User)docs-check / docs-checksuccess— real verdict,PASS7eb5122(current)claude[bot]claude[bot](Bot)docs-check / docs-checkfailure— inner actor gate, no docs verdictThe name now matches the required context exactly, and the bare
docs-checkvariant is gone. That is the behaviour change this PR delivers, and it is visible in every row where the job ran.Row 1 is worth reading carefully: #121's most recent
Claude Docs Checkrun was itself triggered by a human (wkoutre), and it still skipped and still reported the wrong context name. That is because the gate that skipped it is the workflow-level one, read from #121's own branch, which still carries the old[bot]-substring condition. A human trigger cannot rescue #121 until it picks up this fix — the two gates are independent and #121 is currently stopped at the first one.The last three rows are the same branch and the same PR author, and the gate condition under test is byte-identical across all three heads — the only difference relevant to the outcome is who triggered the run.
2cb77e0→7eb5122is the cleanest pair: a whitespace-only commit, no documentation change whatsoever, and the conclusion flipped fromsuccessback tofailurepurely because the pusher changed from aUserto aBot.That is the sharpest available statement of what is fixed and what is not: the workflow-level
if:no longer skips, so the context always reports; the inner actor gate still decides whether that report is a docs verdict or an actor rejection, and it decides per run.How
One change, in
.github/workflows/claude-docs-check.yml.The gate condition
Before:
After:
Plus comments recording what
209825114is.Two gates, and only the first is in this repo
1. The workflow-level
if:— decides whether the job runs at all. This caused the deadlock; this PR fixes it, durably and on every trigger, and this PR proves it.2.
anthropics/claude-code-action's own actor check — reached only once the job starts, and it rejects any bot-triggered run, whoever authored the PR:This PR does not fix gate 2 and does not touch it. The gate compares
github.actor— the actor that triggered the run — so it is evaluated per run rather than per PR. A human push to a bot-authored PR walks straight past it, which is exactly why head2cb77e0produced aPASS; the next bot-triggered run on the same branch fails again on the same line, which is exactly what7eb5122did. Do not read the green docs check on2cb77e0as the gate being resolved.No value can be passed from this repo, because both ai-toolkit reusable workflows decide
allowed_botsthemselves and expose no input for it:allowed_botsclaude_claude-docs-check.yml""_generate-pr-metadata.ymlallowed_bots: dependabot, hardcoded at line 950_claude-docs-check.ymldeclares 14workflow_callinputs, none namedallowed_bots, and neither of its twoclaude-code-actionsteps forwards one. Checked at the pinned SHAc4820d6and at the toolkit'smainandnext— byte-identical, so there is no input to pass and no newer ref that helps. Nothing was guessed at here and nothing was set speculatively: there is no field in this repo that accepts the value.I did not use
'*', and did not route around the gate, because either would defeat the purpose of the PR.What the matching code actually says
Read at the pinned action SHA
787c5a0,src/github/validation/actor.ts:checkHumanActorreadsgithubContext.actor— i.e.github.actor, GitHub's authenticated triggering actor — and resolves its account type viaoctokit.users.getByUsernamebefore consulting the allow-list, so the allow-list can only ever widen access for a non-Useraccount.isAllowedBotsplits the input on commas, then lowercases and strips a trailing[bot]from both the configured entries and the actor.claude,claude[bot], andClaude[bot]are therefore one and the same entry.actor.toLowerCase().replace(/\[bot\]$/, ""), which is why it printsclaude.So the
claudevsclaude[bot]spelling is a red herring: the suffix is normalised away on both sides, and the printed form is cosmetic. Had there been anywhere to put the value, either spelling would have worked.Reading the log correctly
On a rejected run,
Run Claude Docs Check(step 18) fails andProcess Resultsis skipped, so no verdict exists — yetSet Exit Codeprints✅ Documentation check passedagainst an emptyVERDICT. The tail of the log therefore reads as a pass when nothing was checked. Judge these runs by the step list, not the last line.The passing run on
2cb77e0is distinguishable on exactly that basis: steps 18 and 19 both succeeded andSet Exit CodeprintedVERDICT: PASS— a populated verdict, not an empty one. When quoting a green docs check as evidence, check thatProcess Resultsactually ran.The second gate is name-based, and I could not make it otherwise
Flagging this explicitly against Nick's "nothing easily spoofable" instruction rather than letting it be discovered later:
allowed_botsmatches on name, and the action exposes no id-based equivalent anywhere. Within that interface there is no stronger option.Two properties make it defensible; one does not:
github.actor, populated by GitHub from the authenticated triggering actor. Nothing in a PR's branch, diff, or title can influence it.User, so a human registering the accountclaudenever reaches it and gains nothing.claude, the name would match.That asymmetry is exactly why the gate this repo does control uses the numeric id.
Which of Nick's two options this is — and why option 2 now looks stronger
This PR is option 1 (enable
docs-checkfor this bot). It was the right first move: it is the only one of the two that lands entirely in this repo, and it is what proved where the real blocker is.But the investigation strengthened the case for option 2, and that is worth saying plainly. The actor gate is inherited by any workflow here that wraps
claude-code-action, and by only those.claude-code-review.ymlinvokes@uniswap/review-clidirectly and references the action nowhere, so it has no actor check to satisfy. Empirically, the reviewer'sTriageandAI reviewchecks pass regardless of who triggered the run, while the twoclaude-code-action-based workflows pass only when a human triggered it:claude-code-action?claude-code-review.yml(review-cli)claude-docs-check.ymlgenerate-pr-title-description.ymlBoth
claude-code-actionworkflows flipped fromfailuretosuccessbetween headsc25b390and2cb77e0on this PR, and both flipped straight back tofailureon7eb5122, without either workflow's logic changing on that axis — the only difference was the triggering actor. That is the dependency option 2 removes.So option 2 does not merely relocate the docs check — it moves it onto the one execution path in this repo that is not subject to the gate, and removes a cross-repo dependency instead of adding a fix to it. The status quo it replaces is one where a merge-blocking docs verdict is contingent on a human having pushed last.
What still has to be true for option 2, and why it cannot be this PR:
@uniswap/review-clilives inUniswap/internal-toolsand is consumed here as a published GitHub Packages version pin (REVIEW_CLI_VERSION, default1.10.1). It means changing that repo, cutting a release, then bumping the pin here.docs-check / docs-checkis required by repo ruleset12307596, and this repo has no ruleset-as-code (no.github/rulesets). Removing the workflow would stop the required context reporting for every PR from every author, turning a bot-only deadlock into a repo-wide one. Only an admin can retire the context; a PR cannot. Correct order: ship the capability in the CLI → admin drops the required context → then remove this workflow.auto_commit/auto_fix, holdingWORKFLOW_PATwithcontents: write. The review CLI deliberately pushes nothing and runs onGITHUB_TOKENonly. Because this PR leaves the write-back path dormant, that gap stays theoretical rather than becoming something the repo depends on — which keeps option 2 open.Recommendation. Land this PR for the workflow-level fix and the documented diagnosis, then choose one of:
allowed_botsinput to_claude-docs-check.yml, forward it to both action steps; addclaudeto the metadata workflow's hardcoded list). Fixesdocs-checkandgenerate-metadatatogether, keeps the inner gate name-based.@uniswap/review-cli, have an admin swap the required context, then delete this workflow. More work, but it retires the gate, the cross-repo reusable-workflow dependency, and the fragile two-part required context in one go.The identity check, and why it is not spoofable
The gate matches
github.event.pull_request.user.id == 209825114. Values read off the live API,GET /repos/Uniswap/uniswap-ai/pulls/121→.user:loginclaude[bot]id209825114node_idBOT_kgDODIGtWgtypeBothtml_urlhttps://github.com/apps/claudeavatar_urlhttps://avatars.githubusercontent.com/in/1236702?v=4→ App id 1236702Same id on both bot-authored PRs in this repo (#120, #121), so it is the account's stable handle and not a per-PR artifact.
Why this holds where a name does not:
contains(login, '[bot]')tests a naming convention — trust granted to a shape of string rather than to an identity.github.event.pull_request.useris populated server-side from the authenticated actor that opened the PR. Nothing in the branch, diff, title, or commit metadata can influence it — unlikegithub.actor, which on some event types reflects whoever last triggered the run rather than the PR author.user.typeis GitHub's own account-kind field, so "is this a bot" no longer leans on the[bot]suffix either.Considered and rejected:
node_id(BOT_kgDODIGtWg) is equally immutable but opaque and no more secure; the App id 1236702 is arguably the most precise identity, but thepull_requestpayload does not expose it for the PR author — it appears only inside the avatar URL, and parsing a URL for it would be strictly more fragile than reading the documented numeric field.Scope of the claim: the id proves the PR was opened by that app installation. It does not attest to branch contents, which remain author-controlled — but that is equally true of every human PR the check already runs on, so this is not a new trust surface. The fork guard is untouched, so no untrusted fork gains access to secrets.
Verifying it works
This PR is its own test case — it is authored by
claude[bot], so it exercises the change on itself.docs-check / docs-checkreports here; on #121 it never appeared at all. On the human-triggered head2cb77e0it went the whole way to a genuine documentation verdict (PASS), which is what the required context is supposed to produce and what it has never produced on a bot-authored PR in this repo before. On the bot-triggered headsc25b390and7eb5122it reportsfailurefrom the inner actor gate instead. Both outcomes are the workflow-level fix working as designed; only gate 2 decides which of the two you get, and it will keep deciding that until ai-toolkit changes.Practical consequence for merging this: because the current head was pushed by the bot,
docs-check / docs-checkis currently red, and the required context is satisfied-but-failing rather than pending. A human pushing any commit — or an empty one — to this branch is enough to turn it green again, as2cb77e0showed. That is a workaround, not a fix.The workflow-level behaviour change is one row:
typeidclaude[bot]BotwkoutreUserdependabot[bot]Botrenovate[bot]BotUserTo check the context name on any bot-authored PR:
On #121 that prints
docs-check :: skipped— wrong name, requirement unsatisfied. Here it printsdocs-check / docs-check :: successon the human-triggered head2cb77e0, anddocs-check / docs-check :: failureon the bot-triggered headsc25b390and7eb5122. Either way it is the required name, so the requirement reports instead of hanging; the conclusion tells you which gate you landed on.Checks run locally
actionlintv1.7.11 — the exact versionzizmor.ymlpins — invoked as CI does (-shellcheck=""): clean across all workflows, exit 0.bunx nx format:writethenbunx nx format:check: clean, exit 0. (The web-editor edit that removedauto_fixreintroduced prettier drift — a dropped blank line before one heading and two trailing blank lines — which failedValidate Installation & Build; a follow-up whitespace-only commit restores it. No semantic change.)bunx markdownlint-cli2 --fixon the touched markdown: 0 errors.auto_fixandauto_commitwere read at the pinned toolkit SHAc4820d6(_claude-docs-check.ymllines 61 and 122) to confirm that leaving both off means no write path is exercised.Follow-ups, not done here
[bot]-substring condition, so the job keeps skipping there and the required context keeps not reporting. It needs this merged intomainand picked up on that branch, and then a human-triggered run to clear gate 2 — two steps, not one.WORKFLOW_PATis populated on this repo. Passed by this call before and after this PR; the run on2cb77e0loggedHAS_WORKFLOW_PAT: true, so it is present, though this change pushes nothing either way.generate-metadata / generate-metadatabehaves identically and is out of scope: it passed on the human-triggered head2cb77e0and fails on the bot-triggered heads, for the same actor-gate reason. Fixing the metadata workflow's hardcodedallowed_bots: dependabotin ai-toolkit clears it. It is not a required check, so it does not block merges.claude-code-review.ymlstill gates onuser.login == 'claude[bot]'in three places. Not a required check, so a mis-fire skips a review rather than deadlocking a merge — lower stakes, but worth converting to the numeric id for consistency.