Skip to content

Review-dispatch --ref lookup can hard-fail for a fork-originated PR - #290

Merged
d-morrison merged 3 commits into
mainfrom
claude/issue-289-20260724-164930
Jul 24, 2026
Merged

Review-dispatch --ref lookup can hard-fail for a fork-originated PR#290
d-morrison merged 3 commits into
mainfrom
claude/issue-289-20260724-164930

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

PR opened by @claude to address #289.

Triggered by workflow run.

Addresses #289.

claude added 2 commits July 24, 2026 18:15
The @claude agent's own attempt at this fix (workflow run 30110560206)
implemented the change but failed to push -- GitHub rejected a GitHub-App
token update to .github/workflows/claude-review.yml without the
workflows permission. Re-implementing and pushing from a session with
adequate credentials, per the same recovery pattern used for #286.

Addresses #289: .head.ref has no owner prefix, so for a fork-originated
PR it names a branch that doesn't exist in this repo. gh workflow run
--ref would fail to resolve it there, hard-failing claude-review.yml's
/review dispatch job (no warning fallback on that call site) and
degrading the other three claude.yml call sites to a lost dispatch.
Each dispatch site now compares the PR's head repo against the base
repo and omits --ref for a fork PR, falling back to the pre-#285
wrong-SHA-attribution behavior rather than failing the dispatch.
@d-morrison
d-morrison marked this pull request as ready for review July 24, 2026 18:16
Copilot AI review requested due to automatic review settings July 24, 2026 18:16

Copy link
Copy Markdown
Collaborator

Recovered. The @claude agent run that opened this draft PR (run 30110560206) implemented issue #289's fix and committed it locally, but failed to push:

! [remote rejected] claude/issue-289-20260724-164930 -> claude/issue-289-20260724-164930
  (refusing to allow a GitHub App to create or update workflow
  `.github/workflows/claude-review.yml` without `workflows` permission)

Same recurring failure as the one noted in PR #286's original claim comment — the bot's GitHub App token lacks the workflows permission scope needed to push a commit touching .github/workflows/*.yml, and both #285's and #289's fixes edit exactly those files.

Re-implemented from this session (merged latest main first, which already included #286's --ref fix, then added #289's fork-PR guard on top) and pushed successfully with 29000e4. Marking ready for review.


Generated by Claude Code

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

Copy link
Copy Markdown
Contributor Author

👀 Picked up by workflow run #30116307805. Setup runs first; Claude itself responds after that.

@github-actions

Copy link
Copy Markdown
Contributor Author

The state on this branch already matches what's described: commit 29000e4 implements and pushed the #289 fix (fork-PR guard for --ref in review dispatch), the working tree is clean, and the branch is up to date with origin.

The comment that triggered this run (d-morrison's "Recovered..." comment) isn't actually a request — it's a status update from the previous Claude session, and it only tripped this workflow because its body contains the literal substring `@claude` in backticks (referring to the agent, not pinging it). I checked for any newer PR comments, review comments, or reviews after that timestamp and found none containing a real @claude request — only a Copilot bot comment saying it hit its review quota, which isn't actionable by me.

No code changes are needed; the fix is already committed and pushed, and PR #290 is marked ready for review as described.

— posted by @claude post-step from workflow run

💰 Cost: $0.38265269999999996

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@d-morrison
d-morrison merged commit fc15654 into main Jul 24, 2026
28 checks passed
@d-morrison
d-morrison deleted the claude/issue-289-20260724-164930 branch July 24, 2026 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants