Skip to content

altdoc-multiversion-docs: PR concurrency keyed by run_id never cancels superseded runs #306

Description

@d-morrison

Problem

altdoc-multiversion-docs.yml's job-level concurrency group keys PR builds by
github.run_id:

concurrency:
  group: altdoc-multiversion-docs-${{ github.event_name == 'pull_request' && github.run_id || github.ref }}
  cancel-in-progress: true

run_id is unique to each run, so on a pull request every run lands in its
own group and cancel-in-progress can never fire
. Superseded runs are not
cancelled; they all render and all deploy.

The step's comment states the intent as "PR builds/previews are keyed by run id
so concurrent PRs don't block each other." That intent is right, but run_id is
a stronger key than it needs: it also disables cancellation within a single
PR.

Observed

UCD-SERG/serocalculator#600 produced three completed Docs runs for one
PR, each a full ~16-minute render, all deploying to the same
pr-preview/pr-600/ directory:

Run Head SHA Created Finished
30139940407 231b571 02:10:38 02:27:30
30139954938 231b571 02:11:06 02:26:49
30140051666 ddfa5c7 02:14:12 02:30:29

Two runs on a commit that was already superseded, and two on the same SHA
(one from synchronize, one from ready_for_review).

The user-visible symptom is the pr-preview sticky comment being recreated
three times, at 02:26, 02:27, and 02:30 — the last one from the oldest
still-running build, so the final comment does not necessarily correspond to
the newest commit.

Why it matters

  • CI time. Three full renders where one suffices. This workflow is one of
    the slowest in the suite; a PR with several quick pushes multiplies it.
  • Racing deploys. All three runs push to the same pr-preview/pr-<n>/
    path on gh-pages. They happened to serialize cleanly here (the deploy
    action retries), but nothing orders them, so the surviving preview content
    is whichever run finishes last — not necessarily the newest commit.
  • Misleading sticky comment. Its timestamp is the last run to finish, which
    can be an older commit's build.

Suggested fix

Key PR runs by the PR number rather than the run id:

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

That preserves the stated intent --- different PRs still get different groups
and never block each other --- while restoring cancel-in-progress for
superseded runs of the same PR. It is also what
UCD-SERG/serocalculator's pre-migration workflow did
(group: docs-${{ github.event.pull_request.number || github.ref }}), so this
is a behavior regression introduced by migrating to the reusable workflow, not
a new requirement.

Worth confirming while making the change that the resulting group string cannot
collide with a caller's own workflow-level group, which is the deadlock
UCD-SERG/serocalculator#590 documents. The altdoc-multiversion-docs- prefix
plus a caller that sets no workflow-level concurrency: (what
UCD-SERG/serocalculator#600 does) keeps them distinct.

Acceptance criteria

  • A second push to an open PR cancels the first push's still-running docs
    build.
  • Two different open PRs still build concurrently without blocking.
  • The pr-preview sticky comment reflects the newest commit's build.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions