ci: serialize gh-pages writes by adopting gha's two-tier preview split by d-morrison · Pull Request #65 · Morrison-Lab/qbt

d-morrison · 2026-09-15T04:14:03Z

Fixes the intermittent build-deploy failures at the Deploy PR Preview step by removing the gh-pages write race.

The push is not what fails

The issue reads the failure as concurrent pushes racing. The push succeeds, and reports deployment_status: success. What fails is pr-preview-action's wait-for-pages-deployment step, which then polls the GitHub Pages build for that commit and finds it errored.

Pages builds gh-pages one commit at a time, so a commit landing while a build is in flight supersedes it and the older build errors — with every git push having succeeded.

Measured 2026-09-14 over the 100 most recent Pages builds (gh api "repos/Morrison-Lab/qbt/pages/builds?per_page=100", comparing consecutive created_at):

	n	gap to next build
errored	27	26 of 27 under 38s (outlier: 500s)
built	73	minimum 38s; duration 29–75s

No successful build had another commit land inside 38s; almost every errored one did.

What changed

preview.yml splits build-deploy into a read-only build job (group preview-${{ github.ref }}, cancel-in-progress: true — a stale render is worthless) and a deploy job holding every write (group gh-pages, cancel-in-progress: false). The rendered site moves between them as an artifact. No render or post-processing step was edited — they were partitioned. Verified by comparing the step sets: zero lost, the only additions being the artifact handoff and the deploy job's own checkout.
publish.yml joins the same group. This is load-bearing: the run that failed was a preview removal two minutes after a publish, so a preview-only fix leaves publish-vs-preview collisions in place. cleanup-pr-previews.yml already serializes on this group name inside the gha reusable workflow it calls, and concurrency groups are repository-scoped, so all three writers now queue together.
queue: max on that group. The default (queue: single) keeps one pending run and cancels it whenever a newer one queues — and the group spans every PR, so the dropped run need not be a superseded version of itself. This is the "silently skipped preview" the issue warned a naive fix would trade for. It also prevents a regression this PR would otherwise introduce: publish.yml had no group before, so under queue: single a pending publish of main could be dropped by an unrelated preview deploy.
A build-deploy aggregator job preserves the required status check of that name. Splitting the job would otherwise retire the context and hang the merge gate on every PR — exactly as .github/rulesets/README.md warns. It fails on any dependency result other than success, because GitHub counts a skipped required check as satisfied.
.github/actionlint.yaml narrowly suppresses actionlint's unexpected key "queue" false positive (rhysd/actionlint#657, open, unfixed in v1.7.12). Negative control: an unrelated bogus key in the same concurrency block still reports.

Why not migrate to gha's reusable preview (Option 1 in the issue)

Investigated in depth. gha's inputs do cover every custom step, but it is not a drop-in — eight non-default inputs, a filename-hack glob (qbt's chapters render flat, not under chapters/), and four permanent feature losses (QR code, TOC highlighting, the banner's DOCX link, and wait-for-pages-deployment).

Two facts decided it:

gha's preview-deploy does not touch publish.yml, and a publish commit was one of the colliding writers — so Option 1 would not fix this issue.
It drops wait-for-pages-deployment, which would mask the symptom rather than remove the collision.

The migration remains worth doing on its own merits; it is not this fix.

Verification

Seven adversarial review rounds; final verdict clean. actionlint clean repo-wide. Every claim in the comments checked against a primary source.

Deferred, both filed

ruleset export main.json is stale: missing link-checker, wrong check-chars context #67 — .github/rulesets/main.json is stale against the live ruleset. This PR corrects the same list in the README prose and surfaces a genuine policy contradiction it exposed (check / link-checker is required live but documented as deliberately excluded), routing the decision there rather than silently picking a side.
publish.yml holds the gh-pages lock for its whole render, delaying PR previews #68 — publish.yml's group wraps its whole render, so a main render delays PR previews. Correctness is unaffected (queue: max queues rather than drops); only latency. Not bundled here because publish.yml runs only on push to main and cannot be exercised on the PR that changes it.

Also filed #66 — a separate failure mode currently red on this repo: step 5 "Set up Quarto" dying on 403 resolving the latest tinytex release. Unrelated to this race, and it will keep producing red build-deploy checks that look like this issue.

Closes #62

🤖 Generated with Claude Code

Empty commit so the PR exists and is visible to other sessions before the implementation lands, per the pr-on-claim convention. Refs #62 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Every writer to gh-pages now shares one concurrency group, so GitHub Pages never has a build in flight when the next commit lands. The stated cause in #62 was concurrent pushes racing. The push is not what fails: it succeeds and reports deployment_status=success. What fails is pr-preview-action's wait-for-pages-deployment step, which then polls the GitHub *Pages build* for that commit and finds it errored, because Pages builds gh-pages one commit at a time and a later commit supersedes an in-flight build. Measured over the 100 most recent Pages builds on this repo: successful builds take 29-75s, and 26 of the 27 errored builds had another gh-pages commit land within 38s of them, while zero successful builds did. Changes: - preview.yml splits the single build-deploy job into a read-only `build` job (grouped per-ref, cancel-in-progress: true, since a superseded render is worthless) and a `deploy` job holding every write (grouped on gh-pages, cancel-in-progress: false, so deploys queue rather than being killed mid-write). The rendered site moves between them as an artifact. No render or post-processing step changed; they were partitioned, not edited. - publish.yml joins the same gh-pages group. This is load-bearing rather than incidental: the run that failed in #62 was a preview removal two minutes after a publish, so a preview-only fix would have left the publish-vs-preview collision in place. cleanup-pr-previews.yml already serializes on this group name inside the gha reusable workflow it calls, and concurrency groups are repository-scoped, so all three now queue together. - A small `build-deploy` aggregator job preserves the required status check of that name in .github/rulesets/main.json. Splitting the job would otherwise retire the context and hang the merge gate on every PR, exactly as .github/rulesets/README.md warns. It fails on any dependency result other than success, because GitHub counts a skipped required check as satisfied. Refs #62 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Adversarial review found the previous commit's own justification false, and the bug it was papering over is the one #62 warned a naive fix would cause. GitHub's default concurrency policy (queue: single) keeps at most ONE pending run per group and cancels that pending run whenever a newer one queues. The `gh-pages` group is shared across every PR's deploy, so the cancelled run need not be a superseded version of itself: with three PRs deploying, PR B's pending deploy is cancelled by PR C arriving, and nothing replaces it. B's preview then never publishes until someone pushes to B again -- the "silently skipped preview" the issue explicitly named. `queue: max` allows up to 100 pending runs, so they queue rather than supersede. It is incompatible with cancel-in-progress: true, which this group does not use. This also closes a regression the previous commit would have introduced on its own: publish.yml had no concurrency group before, so it always ran. Putting it in a shared group under queue: single would let a pending publish of main be cancelled by an unrelated PR preview deploy -- turning a delayed publish into a dropped one. actionlint 1.7.12, the current release, does not know the `queue` key and reports it as unexpected. The key is documented GitHub workflow syntax and lint-workflows.yml runs with `fail: false`, so it surfaces as a warning. Both call sites say so in place. Also from review: - Note that the build job's cancel-in-progress cancels whichever run is RUNNING, not the older commit, so scheduling delay can cancel the newer render. Accepted and documented: it cannot produce a false green, since the build-deploy job fails on a cancelled dependency. - Tighten the measured claim to "in under 38s", since one successful build sat exactly at 38s. Refs #62 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Round 2 review findings, both about the `queue: max` added in the previous commit. Adds .github/actionlint.yaml suppressing exactly one message, the `unexpected key "queue"` that actionlint emits because it does not yet know the concurrency `queue` key (rhysd/actionlint#657, open, still unfixed in v1.7.12 -- the current release). The previous commit dismissed this as a tolerable warning because lint-workflows.yml runs with `fail: false` here. That reasoning does not survive the repo being a template: the reusable workflow it calls defaults to `fail: true`, so a downstream book that does not carry the override inherits a hard lint failure caused by a false positive. I had claimed suppression was impossible, having tested it and watched it fail. The test was invalid -- actionlint discovers its config relative to the project root it detects from .git, and the scratch directory was not a git repository, so the config was never loaded. In a real repository it works. The ignore pattern is deliberately narrow. Negative control: injecting both an unknown runner label and a *different* unexpected key into the same concurrency block still reports both, so the entry matches only the queue message and has not blinded the linter. Also documents that `queue` is github.com/GHEC syntax and is unavailable on GitHub Enterprise Server, where a fork falls back to the default single-pending behaviour. Not a concern for this repo, but this is a template other institutions fork. Refs #62 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Round 3 review findings. The GHES note said a fork "falls back to the default single-pending behaviour". That was an assumption presented as fact, and probably the wrong one: GitHub rejects a workflow file containing an unrecognized key outright, so the likely outcome on GHES is that the file does not run at all. For a template other institutions fork, the difference between "weaker protection" and "CI stopped" is the whole point of the note. It now says to delete the line on GHES, and says plainly that the failure mode is inferred rather than measured, since no GHES instance was available to test against. Widens the actionlint ignore to `.github/workflows/*.yaml` alongside `*.yml`. The globs match literally, so the original pattern missed this repo's two .yaml-named workflows. Verified on a .yaml file that `queue: max` is suppressed while an unrelated `bogus-key` in the same concurrency block is still reported, so the entry stays narrow across both extensions. Corrects the required-status-check list in .github/rulesets/README.md, which named three contexts with one misspelled. The live ruleset (id 11738810) requires four: `check / link-checker`, `Spellcheck`, `check / check-chars`, and `build-deploy`. Pre-existing rather than introduced here, but this PR is already rewriting the sentence it sits in, and the correct values were already to hand from verifying the build-deploy context. Refs #62 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Round 4 review findings. Three addressed here, two deferred to issues. Addressed: - retention-days 1 -> 7 on the build->deploy artifact. The split made "Re-run failed jobs" meaningful, since re-running only `deploy` reuses the original run's artifact -- so a one-day retention turns that button into an unexplained artifact-not-found error for exactly the case this PR exists to handle, a deploy that failed over a weekend and was not noticed until Monday. Still well below GitHub's 90-day default. - Collapse the two actionlint path entries into one `*.{yml,yaml}` brace alternation. Verified on v1.7.12 that it suppresses the queue message in both a .yml and a .yaml workflow while an unrelated `bogus-key` in the same concurrency block still reports. - The GHES note cited "through 3.18". Re-checked: the key is absent from every published GHES doc version through 3.22, the newest listed. Dated the claim rather than naming a version that silently decays. Deferred, both filed: - #67: .github/rulesets/main.json is stale against the live ruleset, missing `check / link-checker` and spelling `check-chars` without its job prefix. This PR corrects the same list in that directory's README prose, but a full re-export is a separate concern and may surface other drifted fields. - #68: publish.yml's concurrency group wraps its whole render rather than just its deploy, so a main-branch render delays every PR preview deploy behind it. Correctness is unaffected (queue: max queues rather than drops), only latency. Not bundled here because publish.yml runs only on push to main and so cannot be exercised on the PR that changes it; the tradeoff is documented in place and points at #68. Refs #62 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…ng it Round 5 review finding. Correcting the required-status-check list in the previous commit put two contradictory claims in one paragraph: the list now names `check / link-checker` as required, while the sentence after it says `check-links.yml` is intentionally excluded from the merge gate. Both cannot be true, and the contradiction is not cosmetic. The live ruleset does require that context, and the context is genuine: `check-links.yml`'s `check` job calls Morrison-Lab/gha's reusable workflow whose inner job is named `link-checker`, producing `check / link-checker`. Resolving it silently in either direction would be a policy decision wearing a documentation edit, and it propagates: `apply-rulesets.sh` pushes this config to every repo created from the template. So the paragraph now states both facts, says they conflict, and routes the decision to #67 -- which must settle it before re-exporting main.json, since the export would otherwise bake in whichever side happens to be live. Commented there with the two coherent resolutions. Refs #62 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Round 6 review finding. The timing figures were stated as flat fact while the GHES claim two lines below had been dated in round 4 for exactly this reason. They are a snapshot of a bursty deploy pattern, so they decay. Now dated, marked as drifting with deploy volume, and framed as how the mechanism was identified rather than as a standing property of the repo. Also records the exact query, because the reviewer who raised this re-measured via the Pages *deployments* API and got a different population (32 succeeded, 5 errored, 63 superseded before running) than the pages/builds endpoint the figures came from -- and read the difference as decay. Two earlier rounds reproduced the original numbers exactly from pages/builds. Naming the query in place stops the next reader repeating that comparison. Refs #62 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

… items Two CI checks caught prose defects in the previous commits' README edits. `check / check-new-line-breaks` flagged 6 added lines packing more than one sentence. The repo enforces semantic line breaks on added markdown and it is blocking (NLB_FAIL: true). Each affected line is now one sentence. `lint / lint-markdown` failed on its list-item merge splice check, which is blocking even though the markdownlint half runs warn-only. A list item placed directly after a continuation line with no blank line between them counts as a splice, so the new bullet spliced onto the previous item's last line and re-flagged the following bullet as well. The items in that list are now separated by blank lines. Both are formatting defects in prose this PR added, not pre-existing debt: the repo's own markdownlint findings elsewhere are untouched and stay warn-only. Refs #62 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

github-actions · 2026-09-15T06:55:04Z

Claude finished review — View run

Review: PR #65 — `ci: serialize gh-pages writes by adopting gha's two-tier preview split`

The prior review round (commit b4e14a98) verified that review comment, verdict Ready for merge — I confirmed it still stands for preview.yml, publish.yml, and .github/actionlint.yaml, which are unchanged since then.

This round's diff (b4e14a98..50326e9a) touches only .github/rulesets/README.md, reformatting the "What's enforced" bullet list: breaking multi-sentence lines into one-sentence-per-line ("semantic line breaks") and inserting blank lines between list items. The commit message attributes this to two blocking CI checks — check / check-new-line-breaks and lint / lint-markdown's "list-item merge splice" rule. I confirmed both workflows exist in this repo (check-new-line-breaks.yml, lint-markdown.yml, the latter's own comments literally reference "list-item merge splices"), so the stated rationale isn't fabricated.

I rendered both the before- and after-versions of the file through a CommonMark parser (markdown-it-py) to check the "renders identically" bar from the review instructions, since text-only reflow can still change list semantics:

The bullet list under "What's enforced" changes from a tight list to a loose list. Per the CommonMark spec, if any blank line separates two items in a list, the whole list becomes loose and every <li> gets wrapped in <p>, adding vertical paragraph spacing on render. That applies here even to the last two bullets ("No force-pushes…", "Bypass…"), which have no blank line inserted between them in the source, yet still render with extra spacing because the list-wide looseness rule applies uniformly. This is a genuine (not cosmetic-only) rendering change — GitHub's renderer will show visibly more spread-out bullets than before.
This isn't a defect: it's the unavoidable consequence of satisfying the repo's own blocking lint rule (blank lines are required around list items to avoid the "merge splice" false-positive), and the extra spacing is a fairly standard, often-preferred GitHub rendering style for prose-heavy bullets. Flagging only as an FYI, not a blocking issue.
Content/meaning is unchanged: same prose, same #67/#68/#62 references, same code-span backticking, no broken links or markup introduced.

No other findings. No R/Quarto files are touched by this round, so the lab-manual's coding-style/practices sections don't apply here either.

Verdict

Ready for merge

Structured Review Data (JSON)

{
  "schema_version": "1.1",
  "reviewer": "claude",
  "commit_sha": "50326e9ae3d5721cc5ea706f6aea4522891dc98d",
  "verdict": "CLEAN",
  "findings": [],
  "detailed_assessment": "Confirmed via markdown-it-py rendering diff that .github/rulesets/README.md's bullet list becomes a loose list (added paragraph spacing) as an accepted side effect of satisfying the repo's blocking lint-markdown splice check, not a defect.",
  "holistic_assessment": "Prior round's verification of preview.yml, publish.yml, and actionlint.yaml stands unchanged; this round's scope is a docs-only reformat with no workflow logic, requirement, or regression impact."
}

Reviewed commit: 653c1fa

github-actions · 2026-09-15T06:55:05Z

💰 Cost: $0.3507 (review) — run

github-actions · 2026-09-15T06:56:46Z

PR Preview Action v1.8.1
🚀 View preview at https://Morrison-Lab.github.io/qbt/pr-preview/pr-65/
Built to branch `gh-pages` at 2026-09-15 06:55 UTC. Preview will be ready when the GitHub Pages deployment is complete.

chore: open PR for #62 (preview gh-pages deploy race)

af7269d

Empty commit so the PR exists and is visible to other sessions before the implementation lands, per the pr-on-claim convention. Refs #62 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

d-morrison mentioned this pull request Sep 15, 2026

PR preview deploys race on gh-pages; adopt gha's two-tier concurrency pattern #62

Closed

d-morrison and others added 4 commits September 14, 2026 21:38

This was referenced Sep 15, 2026

ruleset export main.json is stale: missing link-checker, wrong check-chars context #67

Open

publish.yml holds the gh-pages lock for its whole render, delaying PR previews #68

Open

d-morrison and others added 3 commits September 14, 2026 22:56

d-morrison marked this pull request as ready for review September 15, 2026 06:43

This comment has been minimized.

Sign in to view

d-morrison merged commit 8de5eaa into main Sep 15, 2026
26 checks passed

d-morrison deleted the claude/fix-62-preview-concurrency branch September 15, 2026 06:59

d-morrison mentioned this pull request Sep 15, 2026

preview.yml renders on every label change; gating build naively breaks the required check #71

Open

Navigation Menu

Uh oh!

ci: serialize gh-pages writes by adopting gha's two-tier preview split - #65

ci: serialize gh-pages writes by adopting gha's two-tier preview split#65
d-morrison merged 9 commits into
mainfrom
claude/fix-62-preview-concurrency

d-morrison commented Sep 15, 2026 •

edited

Loading

Uh oh!

This comment has been minimized.

This comment has been minimized.

github-actions Bot commented Sep 15, 2026

Uh oh!

github-actions Bot commented Sep 15, 2026

Uh oh!

github-actions Bot commented Sep 15, 2026

Built to branch `gh-pages` at 2026-09-15 06:55 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

d-morrison commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The push is not what fails

What changed

Why not migrate to gha's reusable preview (Option 1 in the issue)

Verification

Deferred, both filed

Uh oh!

This comment has been minimized.

This comment has been minimized.

github-actions Bot commented Sep 15, 2026

Review: PR #65 — ci: serialize gh-pages writes by adopting gha's two-tier preview split

Verdict

Uh oh!

github-actions Bot commented Sep 15, 2026

Uh oh!

github-actions Bot commented Sep 15, 2026

Built to branch gh-pages at 2026-09-15 06:55 UTC. Preview will be ready when the GitHub Pages deployment is complete.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

d-morrison commented Sep 15, 2026 •

edited

Loading

Review: PR #65 — `ci: serialize gh-pages writes by adopting gha's two-tier preview split`

Built to branch `gh-pages` at 2026-09-15 06:55 UTC.
Preview will be ready when the GitHub Pages deployment is complete.