Skip to content

fix(altdoc): key PR-build concurrency group by PR number, not run_id - #537

Merged
d-morrison merged 1 commit into
mainfrom
fix/altdoc-pr-concurrency-306-311
Aug 20, 2026
Merged

fix(altdoc): key PR-build concurrency group by PR number, not run_id#537
d-morrison merged 1 commit into
mainfrom
fix/altdoc-pr-concurrency-306-311

Conversation

@d-morrison

Copy link
Copy Markdown
Collaborator

Closes #306
Closes #311

Summary

altdoc-multiversion-docs.yml's docs job keyed its concurrency group by github.run_id for pull_request events. Since run_id is unique per run, every run landed in its own group, so cancel-in-progress: true could never fire for a PR's own superseded runs. Two independent issues observed the same underlying bug from different angles:

Both issues independently suggested the identical fix: key the group by PR number instead.

Fix

-      group: altdoc-multiversion-docs-${{ github.event_name == 'pull_request' && github.run_id || github.ref }}
+      group: altdoc-multiversion-docs-${{ github.event_name == 'pull_request' && github.event.pull_request.number || github.ref }}

This matches the pre-migration behavior in both source repos (UCD-SERG/serocalculator's and d-morrison/rpt's bespoke workflows both keyed on the PR number), so it's a regression fix rather than a new requirement -- cancel-in-progress was already true and needed no other change.

Checked per #311's suggestion

preview.yml and preview-deploy.yml for the same run_id-keyed shape -- neither has it. preview.yml keys on github.ref (stable per-branch) and preview-deploy.yml keys on head_repository.id + head_branch (also stable per-branch, with cancel-in-progress: false by design). This bug was isolated to altdoc-multiversion-docs.yml.

Out of scope

#311's third acceptance criterion -- porting the same fix to d-morrison/rpt's bespoke docs.yaml -- is a different repository, not addressed here.

Testing

  • python3 -c "import yaml; yaml.safe_load(...)" -- valid.
  • actionlint -- zero new warnings (same 36 pre-existing SC2086 hits as before this change, at shifted line numbers).
  • A concurrency: group key isn't something _selftest.yml can exercise (it requires genuinely concurrent real runs, which a single selftest job can't simulate) -- no other workflow's concurrency group in this repo has test coverage either, so this isn't a coverage regression.
  • check-new-line-breaks and check-non-standard-chars.py -- clean on the changelog fragment.

🤖 Generated with Claude Code

Fixes #306, #311. altdoc-multiversion-docs.yml's docs job keyed its
concurrency group by github.run_id for PR events, which is unique per run --
so cancel-in-progress could never fire for a PR's own superseded runs.
Several full renders raced to deploy to the same pr-preview/pr-<N>/ path on
gh-pages, sometimes leaving it mid-rebase (add/add conflicts from
JamesIves/github-pages-deploy-action) and sometimes letting an older
commit's run finish last and overwrite a newer commit's preview.

Now keyed by the PR number instead, matching this workflow's pre-migration
behavior in its source repos: different PRs still build concurrently, but a
newer push to the same PR cancels that PR's still-running build.

Checked preview.yml and preview-deploy.yml (per #311's suggestion) for the
same shape -- both already key on a stable per-branch/per-PR value (github.ref
and head_repository.id+head_branch respectively), not run_id, so neither has
this bug.

Out of scope for this PR: #311's third acceptance criterion (porting the same
fix to d-morrison/rpt's bespoke docs.yaml) is a different repo.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@d-morrison

Copy link
Copy Markdown
Collaborator Author

Working on this --- paws off until I'm done.

@github-actions

Copy link
Copy Markdown
Contributor

Claude finished reviewView run


All CI green except review/claude-review still in progress (another review dispatch, not mine). This is a small, well-scoped fix. My analysis is complete — the diff is correct, matches the stated intent, and no CLAUDE.md doc-sync gaps or hidden regressions were found.

Code Review — PR #537 (Morrison-Lab/gha)

Summary of change: altdoc-multiversion-docs.yml's docs job concurrency group key changes from github.run_id (unique per run — made cancel-in-progress a no-op for a PR's own superseded builds) to github.event.pull_request.number (stable per PR — lets a newer push cancel that PR's in-flight build while different PRs still build concurrently). Fixes #306 and #311. Includes a matching changelog fragment.

Verification performed:

  • Confirmed github.event.pull_request.number is available on every pull_request sub-event the caller stub (examples/altdoc-multiversion-docs.yml) actually triggers on (opened, reopened, synchronize, closed) — the key will never resolve empty.
  • Confirmed the non-PR fallback (github.ref) is untouched, so push/release/workflow_dispatch behavior is unaffected.
  • Grepped the repo for other references to the old run_id-keyed pattern or hardcoded group-name strings (altdoc-multiversion-docs-, run_id in website//README.md) — none found, so there's no doc-sync gap to close (this is an internal concurrency: fix, not a new input, so the "new workflow_call input" doc-sync rule in CLAUDE.md doesn't apply).
  • Cross-checked preview.yml/preview-deploy.yml per the PR's own claim that neither shares this bug — both key on stable per-branch values (github.ref, head_repository.id+head_branch), consistent with the PR description.
  • The changelog fragment (changelog.d/altdoc-pr-concurrency-key.fixed.md) follows the <slug>.<category>.md convention in changelog.d/README.md, and its prose is accurate and free of AI-tell vocabulary.
  • Considered the interaction with the unconditional checkout / "Remove PR Preview" step (lines 170–175 of the workflow, gated to always run so closed-event cleanup works): under the new key, a PR-close event now shares its concurrency group with an in-flight build for that same PR, so closing the PR while a preview build is running now cancels that build. This is a reasonable side effect (no point finishing a preview render for a PR about to have its preview removed), not a regression — cancel-in-progress was already true, only the grouping changed.
  • CI on the PR is green across all jobs except review / claude-review, which is a separate in-flight review dispatch, not something this review depends on.

No bugs, CLAUDE.md violations, hallucinated APIs/paths, or unnecessary complexity found. The fix is minimal, correctly scoped, matches the pre-migration behavior the PR cites from the source repos, and is well-documented in both the workflow comment and the changelog fragment.

Verdict

Ready for merge

Reviewed commit: 106ce97

@github-actions

Copy link
Copy Markdown
Contributor

💰 Cost: $4.9778 (review) — run

@d-morrison
d-morrison merged commit c03f91d into main Aug 20, 2026
38 of 40 checks passed
@d-morrison
d-morrison deleted the fix/altdoc-pr-concurrency-306-311 branch August 20, 2026 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant