fix(ci): unblock claude-review and coverage badge on main#105
Merged
Conversation
claude-review.yml: `authorAssociation` is not a valid `gh pr view --json` field (REST-only). Every workflow_run-triggered review since main CI turned green has errored with "Unknown JSON field: authorAssociation". Fetch the field via the REST PR endpoint instead, after the early-exit gates pass. ci.yml: bump vladopajic/go-test-coverage v2.18.7 → v2.18.8. v2.18.5–.7 have a regression where findFilePathMatchingSearch returns -1 on an exact filename match (returns initial `bestIndex` instead of `i`), surfacing as "could not find file [github.com/.../internal/api/dlq.go]" on the first profile entry whose stripped name matched a walked file at position 0. Fixed upstream in PR #312. 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
taitelee
approved these changes
May 12, 2026
taitelee
left a comment
taitelee
left a comment
Member
There was a problem hiding this comment.
Swaps the restricted gh pr view call for a direct REST API request to see user role (trust level like member or admin/owner). A "Unknown JSON field" error was being thrown because of this bug previously. Also bumps the coverage tool to v2.18.8 because there was some file path matching bug in the previous version.
3 tasks
EricAndrechek
added a commit
that referenced
this pull request
May 12, 2026
) ## Summary Second-order fix on top of #105. The author-trust + required-check gate in `claude-review.yml` now uses the REST `check-runs` endpoint instead of GraphQL's `statusCheckRollup` field on the PR view. ## Why `gh pr view --json statusCheckRollup` asks GraphQL to navigate `checkSuite.workflowRun` on every check context on the PR's head commit. The workflow's `GITHUB_TOKEN` is 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: ``` GraphQL: Resource not accessible by integration (...statusCheckRollup.contexts.nodes.5) (...nodes.0..4.checkSuite.workflowRun) ``` We don't actually use any of that traversal — the gate just needs `name`, `status`, `conclusion` for 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 `authorAssociation` just 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 --json` drops `statusCheckRollup`; keeps `isDraft`, `author`, `headRefOid`. - New `gh api repos/$REPO/commits/$head_sha/check-runs?per_page=100` call for the rollup, only inside the `workflow_run` branch (other event paths bypass the snapshot like before). - Status / conclusion comparisons updated from uppercase (GraphQL enum: `COMPLETED`, `SUCCESS`, ...) to lowercase (REST enum: `completed`, `success`, ...). - Gate behavior is otherwise identical: same set of pass-through conclusions, same skip notices, same fall-through to the Claude action when CI is green. ## Verified `gh api repos/Wave-RF/WaveHouse/commits/a4f5a6b1cc83ffdca5429c0d72fb593f3f232eda/check-runs` (PR #88's current head) returns both `CI` and `PR housekeeping` with `status=completed, conclusion=success`. No GraphQL errors. ## Out of scope (mentioned in investigation, not changed here) - `observability` branch and `context_aware_mq_2` branch still carry pre-#89 workflow files (`board-state-sync.yml`, `claude-agent.yml`) and a pre-#77 runner label. Rebase those branches onto current `main` to drop them. - The \"Claude\" run on `context_aware_mq_2` (event `dynamic`, runs on `ubuntu-24.04`) is GitHub Copilot Coding Agent in Claude mode, not our workflow file. - PR #107 (observability) `PR housekeeping` failures are legitimate — the PR title exceeds 72 chars. ## Test plan - [ ] Next workflow_run for PR #88 (or any internal PR with cross-integration checks) — the gate completes without GraphQL errors. - [ ] Draft / Dependabot / untrusted-author PRs still skip with their respective notices. - [ ] A green CI on a trusted-author PR still falls through to the Claude review step. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
Two independent CI failures surfaced after #89 merged. Both are fixed here. A third issue (project-orchestrator) was diagnosed in the same investigation but is PAT-scope only — handled out-of-band by adding Pull requests: Read to
PROJECT_BOARD_TOKEN.1.
claude-review.yml—Unknown JSON field: \"authorAssociation\"The author-trust gate did
gh pr view --json ...,authorAssociation,....authorAssociationis not a validgh pr view --jsonfield — it's REST-only — and the runner'sgherrors out before the gate can evaluate. Every workflow_run-triggered review since main CI turned green has failed at this step.Fix: drop
authorAssociationfrom thegh pr viewcall, fetch it viagh api repos/$REPO/pulls/$PR_NUM --jq '.author_association'after the draft/Dependabot early-exits. (Saves a round-trip when those gates skip the review.)2.
ci.yml—could not find file [github.com/.../internal/api/dlq.go]vladopajic/go-test-coveragev2.18.5–v2.18.7 has a regression infindFilePathMatchingSearch:When a profile entry's stripped name matches a walked file at position 0, the sentinel
-1is returned and the caller reports the file as missing. The badge step is gatedif: github.ref == 'refs/heads/main', so the bug never showed up in #89's PR CI — only fired on the post-merge main run. Fixed upstream in PR #312 (v2.18.8).Fix: bump
vladopajic/go-test-coveragev2.18.7 → v2.18.8 (SHA-pinned).3. Project orchestrator —
Resource not accessible by personal access token(out-of-PR)Diagnosed but not changed in this PR.
board-fetch-item.shrunsgh api graphql resource(url:)on a PR URL withPROJECT_BOARD_TOKEN. That GraphQL field requires PR-read scope on the token, which the existing PAT lacked. The composite-action docstring at.github/actions/board-upsert-status/action.yml:12already calls this requirement out ("project:write AND repo:read scopes"). The PAT scope has been updated; no workflow change needed.Why it surfaced now: this step is gated on
bot-clean == true(CI green + threads resolved). The recent successful orchestrator runs werepull_request_targetevents where bot-clean was false and the step skipped. The first workflow_run + bot-clean=true combination tripped it.Local impact
None.
make cidoesn't use the badge action — it runsscripts/covdirectly for thresholds and breakdown. The action only runs in thePublish coverage badgestep on main pushes, where it now reads the already-producedtmp/coverage/unit/coverage.txt, computes percentages, and pushes the SVG to thebadgesbranch.local-prefixin.testcoverage.ymlis silently ignored by go-test-coverage as of v2.18.5 but still load-bearing forscripts/cov/main.go(prefix stripping in the breakdown), so it stays.Test plan
🤖 Generated with Claude Code