fix(ci): use REST check-runs for claude-review required-check gate#108
Merged
Conversation
`gh pr view --json statusCheckRollup` asks GraphQL to traverse `checkSuite.workflowRun` on every check context on the PR's head commit. The workflow's GITHUB_TOKEN can't read that sub-field for checks owned by other integrations (Gemini Code Assist, Copilot, status checks from third-party apps). On PRs that accumulate enough cross-integration checks the call fails with "Resource not accessible by integration" even though the data we actually need — just `name`, `status`, `conclusion` for `CI` and `PR housekeeping` — is reachable. Surfaced once #105 unblocked the prior `authorAssociation` failure; visible on PR #88 (long-lived, many integrations) but not PR #7 (fewer integrations). Replace the GraphQL rollup field with the REST `check-runs` endpoint on the same head commit. `filter=latest` is the default so we still get one row per check name; status/conclusion enum values switch from uppercase (GraphQL) to lowercase (REST). Gate logic is otherwise unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
github-actions
Bot
added
github_actions
6 tasks
EricAndrechek
added a commit
that referenced
this pull request
May 12, 2026
) ## Summary Replaces the workflow_run-triggered Claude review with a `pull_request_target`-triggered one, switches the trust gate from "PR author association" to "HEAD commit author/committer permission", and makes the orchestrator wait for Claude review before assigning a reviewer. Closes the chain of regressions we've been working through (#105, #108) and gives the PR a visible `Claude review` check. ## What was broken The previous design — trigger Claude review on `workflow_run` after CI succeeds — kept producing new failures because workflow_run runs with a different token context and the check doesn't attach to the PR: | Symptom | Root cause | |---|---| | `Unknown JSON field: authorAssociation` | Not a valid `gh pr view --json` field. Patched in #105. | | `Resource not accessible by integration` (GraphQL `statusCheckRollup`) | GraphQL navigates `checkSuite.workflowRun` across every context; the workflow's GITHUB_TOKEN can't read that on PRs with other integrations' checks. Patched in #108. | | `author_association: NONE` for jfwoods | The integration `GITHUB_TOKEN` collapses private org membership to `NONE` on REST. No good workaround at that field. | | No `Claude review` indicator on the PR | workflow_run-triggered checks attach to the source workflow run, not the PR head commit. | | Reviewer assigned before Claude review finished | Orchestrator was triggered by CI completion in parallel with Claude review, not after it. | ## What this PR does ### `claude-review.yml`: rebuild around `pull_request_target` - Triggers on `pull_request_target` (open/sync/reopen/ready_for_review) plus the existing `issue_comment` and `workflow_dispatch` paths. - Job renamed `Review` → `Claude review` — that's the check name that appears on the PR and that the orchestrator looks for. Keep them in sync. - Workflow-level `if:` filters Dependabot at the cheap level. All other gating moves inside the job. - Trust gate is now: `HEAD commit's author OR committer must have ≥ read permission on the repo`. Implementation: - `gh api repos/$REPO/commits/$head_sha` → pull `author.login`, `committer.login`. - For each candidate (skipping `web-flow` and `dependabot[bot]`), `gh api repos/$REPO/collaborators/$candidate/permission`. - First candidate whose permission is `admin/maintain/write/triage/read` wins. - This means: if Jack opens a fork PR and I push a fixup, the next HEAD is mine → committer = me, admin → gate passes. If Jack pushes again with no admin involvement → both author and committer = Jack, no perm → gate skips. The check still posts `success` so the orchestrator can move on (drive-by PRs still get a human reviewer assigned, just without AI auto-review). - Required-check snapshot (the part that fetched check-runs and gated on CI success) is gone. Claude can review red CI; it can read the failing diff and say so. The merge gate is still the branch ruleset, not this workflow. - ~80 lines of gate scaffolding deleted overall. ### `project-orchestrator.yml`: wait for Claude review - `workflow_run.workflows: [CI]` → `[CI, "Claude PR review"]`. Orchestrator re-fires when Claude completes. - Bot-clean required-checks list gains `Claude review`. First firing (after CI) sees Claude pending → bot-clean false → skip. Second firing (after Claude) sees all green → assign reviewer + set board state. - Bot-clean switches from `gh pr view --json statusCheckRollup` to REST `check-runs`, pre-empting the same GraphQL `Resource not accessible by integration` failure that hit `claude-review.yml` in #108. PR #7 wasn't tripping it yet; PR #88 (with Gemini) would have. `Claude review` is *not* added to the branch ruleset's `required_status_checks` — it's advisory. The orchestrator just waits for it locally. ## Behavioural changes worth flagging - **Drafts are now reviewed.** Previously the gate skipped drafts; the new gate doesn't filter on draft state. The orchestrator's existing draft → ready auto-flip still works (gated on bot-clean, which now includes Claude). Easy to add back a draft-filter inside the gate step if it turns out to be noisy. - **Claude reviews red CI.** It can read the failing diff and comment on it. Saves the "wait for CI, then review" round-trip; costs a few API calls on PRs that get force-pushed before CI finishes. - **Drive-by fork PRs skip Claude but the check still appears.** Conclusion is `success` with a notice in the step log explaining the skip. The orchestrator treats this as "Claude weighed in" and proceeds to assign a human reviewer. ## Test plan - [ ] Internal PR pushed by a private-org member: `Claude review` check appears on the PR; orchestrator waits for it before assigning. - [ ] Fork PR by a drive-by contributor: `Claude review` posts `success (skipped)`; orchestrator still assigns a reviewer. - [ ] Admin commits a fixup onto a fork PR: next push re-runs Claude review against the new HEAD with trust via committer. - [ ] `@claude` / `/review` re-trigger via comment still works. - [ ] Dependabot PR: filtered at workflow-level `if:`, no Claude check on PR, dependabot-automerge path handles it as before. - [ ] PR #88-style cross-integration check list: orchestrator's bot-clean no longer hits GraphQL Resource-not-accessible. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
EricAndrechek
added a commit
that referenced
this pull request
May 12, 2026
## Two related noise sources Both produce red workflow runs on PR pushes that don't reflect actual problems and were flooding the inbox. ### 1. `claude-review.yml` failing on PRs that edit Claude's own files The Anthropic action runs a self-validation step that fails when its own workflow file (`.github/workflows/claude-review.yml`) or the prompt template (`.github/prompts/pr-review.md`) is in the PR's diff. Every PR touching Claude's wiring (#105, #108, #109, #110, #111, #113, the merge commit of #115) has logged a noisy red Claude check that resolves itself once the change merges. **Fix:** in `claude-review.yml`'s gate step, query the PR's changed files; if either path is in the diff, set `skip=true` and exit 0. The check shows success, the action never runs, the change still takes effect on the next PR's review. ### 2. `dependabot-automerge.yml push failure` ghost runs on every branch push #115 added `reviewers: ${{ replace(env.ADMINS, ',', ' ') }}` to convert the comma-separated `ADMINS` env var to the space-separated form the composite expects. Problem: `replace()` isn't a GitHub Actions expression function. The valid list is `contains`, `startsWith`, `endsWith`, `format`, `join`, `toJSON`, `fromJSON`, `hashFiles`, plus status checks. An unknown function fails workflow validation, and GitHub records a failed run with the file path (not the workflow's `name:`) as the display name and no jobs — every push to any branch since #115 merged. **Fix:** replace the bad expression with a real bash step that uses parameter expansion (`${ADMINS//,/ }`) to write `ADMINS_SPACE` to `$GITHUB_ENV`. The composite's `with:` then references `env.ADMINS_SPACE`. Both steps gated on `update-type == version-update:semver-major` so they only run when actually needed. ## Test plan - [ ] This PR's own Claude check: success (skip-gate catches the self-modification). - [ ] After merge, branch pushes no longer trigger the ghost `.github/workflows/dependabot-automerge.yml push failure` runs. - [ ] Next major-version Dependabot PR: both admins assigned correctly via `ADMINS_SPACE`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Second-order fix on top of #105. The author-trust + required-check gate in
claude-review.ymlnow uses the RESTcheck-runsendpoint instead of GraphQL'sstatusCheckRollupfield on the PR view.Why
gh pr view --json statusCheckRollupasks GraphQL to navigatecheckSuite.workflowRunon every check context on the PR's head commit. The workflow'sGITHUB_TOKENis not allowed to read that sub-field for checks owned by other integrations (Gemini Code Assist, Copilot, third-party statuses). When a PR has enough cross-integration checks the whole call fails with:We don't actually use any of that traversal — the gate just needs
name,status,conclusionfor two named checks (CI,PR housekeeping). The REST endpoint returns exactly that and nothing else, so it doesn't trip the cross-integration permission issue.This failure was always there — #105 fixing
authorAssociationjust let the script reach the next bug. Visible on PR #88 (long-lived, many integrations) but not PR #7 (fewer integrations), which is why we didn't catch it in initial testing.What changed
gh pr view --jsondropsstatusCheckRollup; keepsisDraft,author,headRefOid.gh api repos/$REPO/commits/$head_sha/check-runs?per_page=100call for the rollup, only inside theworkflow_runbranch (other event paths bypass the snapshot like before).COMPLETED,SUCCESS, ...) to lowercase (REST enum:completed,success, ...).Verified
gh api repos/Wave-RF/WaveHouse/commits/a4f5a6b1cc83ffdca5429c0d72fb593f3f232eda/check-runs(PR #88's current head) returns bothCIandPR housekeepingwithstatus=completed, conclusion=success. No GraphQL errors.Out of scope (mentioned in investigation, not changed here)
observabilitybranch andcontext_aware_mq_2branch still carry pre-chore: remove clustered mode and unscoped features #89 workflow files (board-state-sync.yml,claude-agent.yml) and a pre-ci: migrate workflows to ARC runners on self-hosted infra #77 runner label. Rebase those branches onto currentmainto drop them.context_aware_mq_2(eventdynamic, runs onubuntu-24.04) is GitHub Copilot Coding Agent in Claude mode, not our workflow file.PR housekeepingfailures are legitimate — the PR title exceeds 72 chars.Test plan
🤖 Generated with Claude Code