Summary
examples/claude-code-review.yml's dispatch-on-comment job detects a fork PR, dispatches anyway, and then posts a comment promising a review. The reusable workflow's own dispatch guard (gha#235) then blocks that run, so the review never happens --- and because both jobs skip rather than fail, nothing on the PR contradicts the promise.
Trace
The example stub notices the fork and downgrades only the --ref handling:
REF_ARGS=(--ref "$PR_BRANCH")
if [ "$PR_HEAD_REPO" != "$REPO" ]; then
echo "::notice::PR #$PR_NUMBER is from a fork ($PR_HEAD_REPO); dispatching $WF_FILE without --ref."
REF_ARGS=()
fi
gh workflow run "$WF_FILE" --repo "$REPO" "${REF_ARGS[@]}" -f pr_number="$PR_NUMBER"
gh issue comment "$PR_NUMBER" --repo "$REPO" \
--body ":mag: \`$TRIGGER\` received -- dispatched a Claude review of this PR ... The review posts as its own comment when it finishes."The dispatched run then hits the guard in claude-code-review.yml's gather-context, which sets blocked=true for isCrossRepository == "true". claude-review's if: requires dispatch-guard-blocked != 'true', so it is skipped; require-review's if: requires claude-review.result != 'skipped', so it is skipped too.
Net result on the PR: one comment saying a review was dispatched and will post when it finishes, no review, and no red check. A maintainer who used /review on a contributor's fork PR has no way to tell the difference between "still running" and "will never run".
Why the two halves are individually reasonable
Neither piece is wrong on its own. The guard is correct --- gha#235 added it deliberately, and its own comment explains it is "failing a mistaken manual dispatch fast with a clear message instead of running a review against an unexpected PR". The --ref fallback is correct too, since a fork's head branch does not exist in the base repo.
The gap is that the stub knows it is looking at a fork one line before it makes a promise it cannot keep, and does not use that knowledge.
Suggested fix
Short-circuit in the stub rather than dispatching, and say what happened:
if [ "$PR_HEAD_REPO" != "$REPO" ]; then
echo "::warning::PR #$PR_NUMBER is from a fork ($PR_HEAD_REPO); the reusable review workflow blocks fork reviews, so no review was dispatched."
gh issue comment "$PR_NUMBER" --repo "$REPO" \
--body ":no_entry: \`$TRIGGER\` received, but this PR is from a fork, and fork PRs cannot be reviewed by this workflow."
exit 0
fiThat keeps the guard as the authoritative backstop while making the user-visible outcome honest. It also removes the now-pointless REF_ARGS fallback, since the fork case no longer reaches the dispatch.
An alternative worth considering instead: have the dispatch guard post its own block notice rather than skipping silently, so any dispatch route (not just this stub) explains itself. That is the more general fix, and it would also cover the Dependabot branch of the same guard. Related in spirit to #573, which is about a dispatch-guard skip being invisible to a comment-based classifier.
Provenance
Found by an adversarial review of a consumer migration adopting this stub (UCD-SERG/ucd-serg.github.io#110), which is where the false promise was traced end-to-end. Fixed consumer-side there by short-circuiting; filing upstream so the next adopter does not inherit it.
Summary
examples/claude-code-review.yml'sdispatch-on-commentjob detects a fork PR, dispatches anyway, and then posts a comment promising a review. The reusable workflow's own dispatch guard (gha#235) then blocks that run, so the review never happens --- and because both jobs skip rather than fail, nothing on the PR contradicts the promise.Trace
The example stub notices the fork and downgrades only the
--refhandling:The dispatched run then hits the guard in
claude-code-review.yml'sgather-context, which setsblocked=trueforisCrossRepository == "true".claude-review'sif:requiresdispatch-guard-blocked != 'true', so it is skipped;require-review'sif:requiresclaude-review.result != 'skipped', so it is skipped too.Net result on the PR: one comment saying a review was dispatched and will post when it finishes, no review, and no red check. A maintainer who used
/reviewon a contributor's fork PR has no way to tell the difference between "still running" and "will never run".Why the two halves are individually reasonable
Neither piece is wrong on its own. The guard is correct --- gha#235 added it deliberately, and its own comment explains it is "failing a mistaken manual dispatch fast with a clear message instead of running a review against an unexpected PR". The
--reffallback is correct too, since a fork's head branch does not exist in the base repo.The gap is that the stub knows it is looking at a fork one line before it makes a promise it cannot keep, and does not use that knowledge.
Suggested fix
Short-circuit in the stub rather than dispatching, and say what happened:
That keeps the guard as the authoritative backstop while making the user-visible outcome honest. It also removes the now-pointless
REF_ARGSfallback, since the fork case no longer reaches the dispatch.An alternative worth considering instead: have the dispatch guard post its own block notice rather than skipping silently, so any dispatch route (not just this stub) explains itself. That is the more general fix, and it would also cover the Dependabot branch of the same guard. Related in spirit to #573, which is about a dispatch-guard skip being invisible to a comment-based classifier.
Provenance
Found by an adversarial review of a consumer migration adopting this stub (UCD-SERG/ucd-serg.github.io#110), which is where the false promise was traced end-to-end. Fixed consumer-side there by short-circuiting; filing upstream so the next adopter does not inherit it.