What happened
Migrating d-morrison/qwt's hand-rolled claude-code-review.yml to a thin
caller of Morrison-Lab/gha/.github/workflows/claude-code-review.yml@v2
(part of d-morrison/qwt#116 / d-morrison/qwt#135), I preserved the old
standalone workflow's own per-PR dedup by adding this at the top of the
caller:
concurrency:
group: claude-review-${{ github.event.pull_request.number || inputs.pr_number }}
cancel-in-progress: true
Every pull_request-triggered run of that caller then failed instantly with:
Canceling since a deadlock was detected for concurrency group:
'claude-review-135' between a top level workflow and 'review / claude-review'
Root cause
claude-code-review.yml's own claude-review job already declares:
concurrency:
group: claude-review-${{ github.event.pull_request.number || inputs.pr-number }}
cancel-in-progress: true
A caller-level concurrency: block using the identical group name deadlocks
against the nested job's own group -- GitHub Actions detects the top-level
workflow run and the reusable workflow's own job both trying to hold the same
group and cancels the run rather than running it. Removing the caller-level
block (and just relying on the reusable workflow's own internal dedup) fixed
it immediately -- confirmed live on d-morrison/qwt#135.
Why this is easy to hit
None of examples/claude-code-review.yml, the README, or the workflow's own
top-of-file comments warn that adding any caller-level concurrency: block
using a PR-scoped group name is unsafe. A consumer migrating from a
hand-rolled workflow that had its own per-PR concurrency group (a very
natural thing to have written, and a very natural thing to want to preserve
during a migration) will reach for exactly this pattern, since it's the
obvious way to express "serialize per PR" -- and the group name collision is
invisible until the first live pull_request run.
Suggested fix
Add a note to examples/claude-code-review.yml (and/or the README) stating
explicitly that callers should NOT declare their own concurrency: block --
the reusable workflow already serializes per PR internally on the
claude-review job, and doing so risks a group-name collision that
deadlocks the run. If a distinct caller-level concurrency need ever exists,
it should be documented to use a clearly distinct group name/prefix so it
can't collide with the internal one.
What happened
Migrating
d-morrison/qwt's hand-rolledclaude-code-review.ymlto a thincaller of
Morrison-Lab/gha/.github/workflows/claude-code-review.yml@v2(part of d-morrison/qwt#116 / d-morrison/qwt#135), I preserved the old
standalone workflow's own per-PR dedup by adding this at the top of the
caller:
Every
pull_request-triggered run of that caller then failed instantly with:Root cause
claude-code-review.yml's ownclaude-reviewjob already declares:A caller-level
concurrency:block using the identical group name deadlocksagainst the nested job's own group -- GitHub Actions detects the top-level
workflow run and the reusable workflow's own job both trying to hold the same
group and cancels the run rather than running it. Removing the caller-level
block (and just relying on the reusable workflow's own internal dedup) fixed
it immediately -- confirmed live on d-morrison/qwt#135.
Why this is easy to hit
None of
examples/claude-code-review.yml, the README, or the workflow's owntop-of-file comments warn that adding any caller-level
concurrency:blockusing a PR-scoped group name is unsafe. A consumer migrating from a
hand-rolled workflow that had its own per-PR concurrency group (a very
natural thing to have written, and a very natural thing to want to preserve
during a migration) will reach for exactly this pattern, since it's the
obvious way to express "serialize per PR" -- and the group name collision is
invisible until the first live
pull_requestrun.Suggested fix
Add a note to
examples/claude-code-review.yml(and/or the README) statingexplicitly that callers should NOT declare their own
concurrency:block --the reusable workflow already serializes per PR internally on the
claude-reviewjob, and doing so risks a group-name collision thatdeadlocks the run. If a distinct caller-level concurrency need ever exists,
it should be documented to use a clearly distinct group name/prefix so it
can't collide with the internal one.