Replace sphinx-multiversion with per-branch docs builds

[AntoineRichard]

Summary

• Removes sphinx-multiversion (unmaintained since 2020) from the docs build
• Each deploy branch now builds only its own docs using plain sphinx-build
• Deploys to versioned subdirectories on gh-pages with keep_files: true
• Adds pydata-sphinx-theme version switcher (via sphinx-book-theme)
• Adds standalone version-switcher.js injected into old version pages
• Adds one-time docs-migration.yaml workflow to rebuild all historical versions

Context

sphinx-multiversion installs one Sphinx version and builds all branches/tags
against it — no dependency isolation. When Sphinx 9.0 changed Config.read(),
every version broke. This replaces it with the industry-standard pattern used
by NumPy and CPython.

See spec: docs/superpowers/specs/2026-04-10-multi-version-docs-design.md

Test plan

• Docs CI builds successfully on this PR
• After merge, trigger docs-migration.yaml to rebuild all versions
• Verify version switcher works on deployed docs
• Verify old version pages have the injected JS switcher

[greptile-apps]

Greptile Summary

This PR replaces sphinx-multiversion with per-branch Sphinx builds, deploys to versioned gh-pages subdirectories with keep_files: true, and adds a pydata-sphinx-theme version switcher for new builds plus a standalone injected JS switcher for old versions.

• Race condition on versions.json: docs.yaml fetches and rewrites versions.json non-atomically; two branches pushing at the same time will both read the pre-deploy state and the last writer will silently drop the other branch's entry from the switcher.
• DOCS_VERSION_SLUG=\"\" for PRs: The slug step falls through to \"\" for refs/pull/* refs; os.getenv(\"DOCS_VERSION_SLUG\", version) returns \"\" (not the default) when the variable is set-but-empty, which may cause a Sphinx warning promoted to an error by -W.
• peaceiris/actions-gh-pages@v4 is not SHA-pinned: this is a new third-party action not present in any other repo workflow; per AGENTS.md, new actions require admin approval and must be pinned by commit SHA.

Confidence Score: 3/5

Not safe to merge without addressing the versions.json race condition and the empty DOCS_VERSION_SLUG for PRs, which could break the version switcher and PR doc builds.

Three P1 findings: a genuine race condition that can silently drop entries from the version switcher when concurrent branch deploys occur, an empty env var that bypasses the Python default and may cause a Sphinx -W error on every PR build, and a new unpinned third-party action that violates the repo's explicit supply-chain policy.

.github/workflows/docs.yaml — race condition in versions.json upsert (lines 137-196) and empty DOCS_VERSION_SLUG for PRs (lines 59-69); also peaceiris/actions-gh-pages@v4 pinning in both workflow files.

Important Files Changed

Filename	Overview
.github/workflows/docs.yaml	Core docs CI workflow: replaces sphinx-multiversion with per-branch Sphinx builds and gh-pages deploy; has a race condition on versions.json, an empty DOCS_VERSION_SLUG for PRs, and a new unpinned third-party action (peaceiris/actions-gh-pages@v4).
.github/workflows/docs-migration.yaml	One-time migration workflow that rebuilds all historical version docs via a matrix strategy; peaceiris/actions-gh-pages@v4 is not SHA-pinned per project policy, and the all-or-nothing deploy behaviour (keep_files:false, needs all matrix jobs) is undocumented.
docs/_static/version-switcher.js	New standalone JS version switcher injected into old docs pages; works correctly for GitHub Pages sub-path URLs but base-path logic fails for custom-domain root deployments (contradicts its own comment).
docs/conf.py	Adds pydata-sphinx-theme version switcher config driven by DOCS_VERSION_SLUG env var; superpowers/specs/ directory is missing from exclude_patterns so the design spec will be included in the generated docs.
docs/requirements.txt	Removes sphinx-multiversion and the sphinx<9 pin; sphinx-book-theme pin raised to >=1.1 to enable built-in version switcher support.
docs/Makefile	Minimal Makefile; replaces multi-version build target with a single current-docs target using sphinx-build -W --keep-going.
docs/superpowers/specs/2026-04-10-multi-version-docs-design.md	Design spec document for this migration; well-written and matches the implementation, but not excluded from the Sphinx build so it will appear in the published docs.

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant CI as GitHub Actions
    participant Pages as gh-pages
    participant Switcher as versions.json

    Dev->>CI: Push to main/develop/release/**
    CI->>CI: build-docs (sphinx-build -W)
    CI->>CI: Upload artifact docs-html

    CI->>Pages: Fetch current versions.json
    Pages-->>CI: existing entries
    CI->>CI: Upsert this branch's entry
    CI->>CI: Assemble site/slug + inject version-switcher.js
    CI->>Pages: Deploy with keep_files:true
    Pages->>Switcher: Overwrite versions.json

    Note over CI,Pages: Race: concurrent branch deploys both fetch same versions.json, last writer wins

    Dev->>CI: PR opened/updated
    CI->>CI: build-docs only (no deploy)
    Note over CI: DOCS_VERSION_SLUG empty for PRs

Loading

Comments Outside Diff (1)

1. docs/conf.py, line 155 (link)

   superpowers/specs/ not excluded from the Sphinx build

   The design spec at docs/superpowers/specs/2026-04-10-multi-version-docs-design.md is not listed in exclude_patterns, so Sphinx will attempt to parse and include it in the generated HTML. The plans directory is already excluded; superpowers should be too, unless publishing the spec is intentional.

_{Reviews (1): Last reviewed commit: "Replace sphinx-multiversion with per-bra..." | Re-trigger Greptile}

[greptile-apps]

Race condition on versions.json

When two branches push at the same time (e.g. main and develop), both jobs concurrently fetch the same initial versions.json, each add only their own entry, and the last to deploy overwrites the file — silently dropping the other branch's entry from the switcher until that branch is next pushed.

The concurrency group scopes cancellation to ${{ github.ref }}, so same-ref runs cancel each other but different refs run freely in parallel. In practice, main and develop often advance together (e.g. after a version bump).

A safe fix: make the upsert atomic by pushing to a dedicated branch and using git merge semantics, or add a retry loop with optimistic locking on the JSON file. A simpler but less correct workaround is to use a separate repository_dispatch event from each deploy that serialises all versions.json writes into a single workflow.

[greptile-apps]

New third-party action not SHA-pinned

peaceiris/actions-gh-pages@v4 is a new third-party action not present in any other workflow in this repo. Per the project's CI policy (AGENTS.md), all actions must be pinned to a commit SHA (action@<sha> # vX.Y.Z) and new actions require repo admin approval before use.

This applies to docs-migration.yaml line 168 as well.

[greptile-apps]

Empty DOCS_VERSION_SLUG for PR builds

For pull-request events, github.ref is refs/pull/<n>/merge, which does not match either branch pattern. The else branch leaves SLUG="", so the make current-docs step gets DOCS_VERSION_SLUG="" in its environment.

In conf.py, os.getenv("DOCS_VERSION_SLUG", version) returns "" (not the fallback version) when the variable is set-but-empty. This makes version_match: "" in the switcher config, which will not match any entry in versions.json. Depending on how sphinx-book-theme/pydata-sphinx-theme handle an unmatched version, this could emit a Sphinx warning that's promoted to an error by the -W flag in the Makefile, breaking the PR build.

Or exclude the variable from the env: block when slug is empty so os.getenv sees it as unset and uses the default version.

[greptile-apps]

Base-path detection fails for custom domain root deployments

The comment on line 24 claims the script "works for both /IsaacLab/main/… and custom domain setups," but the logic is only correct for the GitHub Pages sub-path case.

For a custom domain where docs live at the root (e.g. https://docs.isaaclab.io/main/page.html):

• pathParts = ["main", "page.html"]
• basePath = "/main/" ← wrong, should be "/"
• currentSlug = "page.html" ← wrong, should be "main"
• JSON_URL = "/main/versions.json" ← 404

This is a pre-wired assumption of a two-level URL prefix. If a custom domain is ever used, the switcher will silently fail to load. Consider removing the misleading comment or parameterising basePath via a data attribute set at injection time.

[greptile-apps]

keep_files: false all-or-nothing behaviour is undocumented

The migration deploy uses keep_files: false, which fully replaces gh-pages. If any matrix build job fails (even though fail-fast: false keeps other jobs running), the deploy-all job is skipped because needs: [build-version] requires all matrix jobs to succeed. In that scenario, the old gh-pages content remains, which is safe.

However, it is worth documenting this intentional "all-or-nothing" behaviour with a comment, since it's non-obvious that a single failed historical tag prevents the entire migration from deploying.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[isaaclab-review-bot]

Review: Replace sphinx-multiversion with per-branch docs builds

Great initiative — sphinx-multiversion has been a well-known pain point and the per-branch build pattern (à la NumPy/CPython) is the right direction. The design spec is thorough and the overall architecture is sound. I have several issues to flag, ranging from a potential data-loss race condition to smaller improvements.

---

🔴 Critical: Race condition in versions.json updates (docs.yaml deploy-docs job)

The deploy-docs job fetches versions.json from the live GitHub Pages URL, upserts the current version, and writes it back. If two deploy branches push simultaneously (e.g. main and develop within the same minute), both will fetch the same versions.json, each add their entry, and whichever deploys last will overwrite the other's entry. This is a real risk since keep_files: true only preserves files not in the publish directory — both deploys write versions.json to the root.

The existing concurrency block uses ${{ github.workflow }}-${{ github.ref }} which is per-ref, so two different refs can still race.

Fix: Add a separate concurrency group on just the deploy-docs job to serialize all deploys:

deploy-docs:
  concurrency:
    group: docs-deploy-gh-pages
    cancel-in-progress: false

---

🟡 Medium: Shell variable injection in Python heredoc (docs.yaml lines ~119-120)

In the "Update versions.json" step, the Python script uses ${SLUG} and ${NAME} via shell interpolation directly inside Python string literals:

slug = '${SLUG}'
name = '${NAME}'

If a branch name ever contains a single quote (unlikely but possible in git), this breaks the Python script — or worse, allows injection. The SLUG env var is already set for this step. Safer to use os.environ['SLUG'] inside the Python code instead of shell interpolation.

---

🟡 Medium: Migration workflow keep_files: false is destructive (docs-migration.yaml line 172)

The docs-migration.yaml deploy step uses keep_files: false, doing a full overwrite of gh-pages. This is correct for a one-time bootstrap, but if accidentally re-triggered after regular deploys have added new versions not in the matrix (e.g. v3.0.1), those versions get wiped. Consider:

1. Adding a prominent warning comment in the workflow
2. Adding a second confirmation input (e.g. type_to_confirm)
3. Deleting/disabling this workflow after the migration is complete

---

🟢 Minor: Tag trigger pattern is broader than intended (docs.yaml line 14)

The tag pattern 'v[1-9]*' matches v1, v1-rc, v100anything, etc. The spec says v*.*.*. If you only want semver release tags, consider tightening to 'v[1-9]+.[0-9]+.[0-9]+*' or similar.

---

🟢 Minor: version-switcher.js uses deprecated XMLHttpRequest

The HEAD request for page-existence checking (line ~67) uses XMLHttpRequest, which is deprecated. Since docs pages target modern browsers, fetch() with { method: 'HEAD' } would be more future-proof.

---

🟢 Nit: Design spec committed to repo

The 281-line spec at docs/superpowers/specs/2026-04-10-multi-version-docs-design.md is thorough documentation, but consider if it belongs in the repo long-term or if the PR description is sufficient context.

---

Overall: Well-designed migration with a solid spec. The versions.json race condition is the main concern — adding a deploy-level concurrency group would make this production-safe. The rest is polish. 👍

[isaaclab-review-bot]

🤖 Isaac Lab Review Bot

Summary

This PR replaces the unmaintained sphinx-multiversion with a per-branch/tag docs build pipeline — the same pattern used by NumPy and CPython. The architecture is well-designed: each push builds only its own ref, deploys to a versioned subdirectory on gh-pages, and a JSON-driven version switcher ties everything together. The author has addressed the critical findings from the previous review (race condition fix via deploy concurrency group, shell injection fix via os.environ). A few remaining items are worth addressing before merge.

Design Assessment

Design is sound. The per-branch build model with keep_files: true is the industry standard for multi-version docs. The two-tier version switcher approach (pydata-sphinx-theme built-in for new versions, injected standalone JS for old versions) is pragmatic and handles the migration gracefully. The deploy-level concurrency group correctly serializes all deploys to prevent the versions.json race condition.

Findings

🟡 Warning: Migration workflow keep_files: false has no re-run guard — .github/workflows/docs-migration.yaml:194

The migration workflow does a full overwrite of gh-pages (keep_files: false). This is correct for the initial bootstrap, but if accidentally re-triggered after regular deploys have added versions not in the matrix (e.g., a new v3.1.0 tag deployed via docs.yaml), those versions get wiped. The dry_run input helps, but a distracted operator could still nuke live docs.

Consider adding a confirmation input or a prominent comment:

inputs:
  confirm:
    description: "Type 'OVERWRITE' to confirm full gh-pages replacement"
    required: true
    type: string

Then gate deploy-all on inputs.confirm == 'OVERWRITE'. Alternatively, add a comment noting this workflow should be deleted or disabled after the migration is complete.

🟡 Warning: Tag trigger pattern is broader than intended — .github/workflows/docs.yaml:14

The tag pattern 'v[1-9]*' matches any tag starting with v followed by a non-zero digit — including v1, v1-rc, v100anything, etc. The design spec calls for semver tags (v*.*.*). While unlikely to cause issues in practice (who creates a v1 tag?), tightening the pattern prevents accidental builds:

tags:
  - 'v[1-9]+.[0-9]+.[0-9]+*'

The trailing * allows pre-release suffixes like v3.0.0-beta (which is in the migration matrix).

🔵 Suggestion: Hardcoded /IsaacLab/ path in sed injection could use a variable — .github/workflows/docs.yaml:148 and .github/workflows/docs-migration.yaml:131

Both workflows hardcode /IsaacLab/ in the sed script tag injection and in versions.json URL entries. If the repository is ever renamed or forked, these would silently break. Consider deriving it from github.repository:

env:
  SITE_PATH: /IsaacLab  # or: /${{ github.event.repository.name }}

Then use ${SITE_PATH} in the sed command and Python script. This is low-risk since renaming is unlikely, but it would make the workflows more portable.

🔵 Suggestion: version-switcher.js uses XMLHttpRequest — docs/_static/version-switcher.js:61,98

Both the versions.json fetch and the HEAD page-existence check use XMLHttpRequest, which is a legacy API. Since the docs target modern browsers, fetch() would be more concise and future-proof:

fetch(targetUrl, { method: "HEAD" })
  .then(function (res) {
    window.location.href = res.ok ? targetUrl : targetBase;
  })
  .catch(function () {
    window.location.href = targetBase;
  });

Not blocking — XHR works fine — but worth modernizing since this is new code.

Test Coverage

This is a documentation/CI infrastructure PR with no runtime code changes. The manual test plan in the PR description is appropriate:

• Docs CI builds successfully on this PR
• After merge, trigger docs-migration.yaml to rebuild all versions
• Verify version switcher works on deployed docs
• Verify old version pages have the injected JS switcher

No automated unit tests are needed. The CI checks (pre-commit, broken links, docs build) provide adequate validation for this type of change.

One addition to the test plan: After running the migration, verify that versions.json contains all expected entries and that the sort order is correct (branches first, then releases, then tags).

CI Status

• ✅ labeler, pre-commit, Check Secrets, Check for Broken Links: passing
• ⏳ license-check, Build Docs: pending

Verdict

COMMENT

The critical issues from the prior review have been addressed. The remaining findings are all preventive improvements (migration guard, tighter tag pattern, portability, API modernization) — none are blocking. This is well-designed infrastructure work. Ship it once the pending CI checks pass and consider the migration-guard suggestion to protect against accidental re-runs.

[isaaclab-review-bot]

🟡 Warning: keep_files: false does a full overwrite of gh-pages. If this workflow is accidentally re-triggered after regular docs.yaml deploys have added new versions (e.g. v3.1.0), those versions get wiped.

Consider adding a confirmation input to gate deploy-all:

inputs:
  confirm:
    description: "Type 'OVERWRITE' to confirm full gh-pages replacement"
    required: true
    type: string

Or add a comment noting this workflow should be deleted/disabled after the migration is complete.

[isaaclab-review-bot]

🟡 Warning: Pattern 'v[1-9]*' matches any tag starting with v + a non-zero digit (e.g. v1, v1-rc, v100anything). The design spec calls for semver tags. Consider tightening:

tags:
  - 'v[1-9]+.[0-9]+.[0-9]+*'

The trailing * still allows pre-release suffixes like v3.0.0-beta.

[isaaclab-review-bot]

🔵 Suggestion: The /IsaacLab/ path is hardcoded here and in the Python versions.json generator. If the repository is ever renamed or forked, this silently breaks. Consider deriving from github.repository:

env:
  SITE_PATH: /${{ github.event.repository.name }}

Then use ${SITE_PATH} in the sed command. Low risk but improves portability.

[isaaclab-review-bot]

🔵 Suggestion: XMLHttpRequest is a legacy API. Since this is new code targeting modern browsers, fetch() would be more concise:

fetch(targetUrl, { method: "HEAD" })
  .then(function (res) {
    window.location.href = res.ok ? targetUrl : targetBase;
  })
  .catch(function () {
    window.location.href = targetBase;
  });

Not blocking — XHR works fine.

[isaaclab-review-bot]

🤖 Isaac Lab Review Bot — Follow-up

New commits (b8f8fef→552f936): Split docs.yaml into docs-dev.yml for develop-only builds (fixes the empty DOCS_VERSION_SLUG issue), and added a /stable/ alias + semver-aware sorting to the migration workflow. Previous review concerns (race condition, empty slug) are addressed.

One new issue with the /stable/ alias below.

[isaaclab-review-bot]

🟡 Warning: /stable/ directory gets included in versions.json as a duplicate entry

The "Generate versions.json" step iterates all directories under site/, which now includes stable alongside the tag it aliases (e.g. v3.0.0-beta). This means users see both stable and v3.0.0-beta in the version switcher, pointing to identical content.

Additionally, _version_tuple("stable") returns () (no digits), so it sorts before all versioned tags — appearing between the release-* entries and v1.0.0, which is an odd position for a "latest" alias.

Fix: Exclude stable from versions.json generation (it's an alias, not a real version):

SKIP_DIRS = {"stable"}
for name in sorted(os.listdir("site")):
    path = os.path.join("site", name)
    if not os.path.isdir(path) or name in SKIP_DIRS:
        continue
    ...

Alternatively, if you want stable in the switcher, handle it specially in sort_key (e.g. return (-1, ()) to put it first) and consider adding a display name like "name": "stable (v3.0.0-beta)" so users understand the alias.