Skip to content

feat(docs): versioned documentation site#1536

Merged
alistair3149 merged 2 commits into
mainfrom
docs-versioning
May 11, 2026
Merged

feat(docs): versioned documentation site#1536
alistair3149 merged 2 commits into
mainfrom
docs-versioning

Conversation

@alistair3149

@alistair3149 alistair3149 commented May 11, 2026

Copy link
Copy Markdown
Member

Summary

Serve documentation for past releases at /v{minor}/ alongside main, using a Codex-style frozen-per-tag build model on a gh-pages branch.

  • Switcher: a Vue component in main's nav fetches versions.json at runtime and path-preserves into the chosen version (with a HEAD-on-404 fallback to the target's root). It correctly resolves URLs against the project base (/mediawiki-skins-Citizen/) and is also correct when served from a versioned subpath.
  • Per-tag builds: a new workflow triggered on release: published builds each released tag's docs from that tag's source and deploys to /v{minor}/. Latest patch of each minor wins by overwriting.
  • Storage: migrates the docs deploy off actions/deploy-pages onto a gh-pages branch managed by peaceiris/actions-gh-pages. Needed because the artifact flow replaces the entire site on each deploy and can't preserve /v*/ subpaths.
  • Concurrency: both workflows share concurrency: gh-pages-deploy to serialize deploys and prevent force-push races.

Seeded historical versions (v3.13/v3.14/v3.15) will not carry the switcher in their own nav — by design. Users discover older versions from main and switch in.

Commits

Split by path so release-please doesn't surface this as a skin feature:

  • feat(docs): — switcher + theme wiring + vitest harness. Falls under release-please's exclude-paths: ["docs/**"] filter, so it won't bump the skin's version.
  • ci: — workflows + versions.json generator. Non-bumping under release-please.

Operational steps (not in this PR — required before users see the change)

  1. Initialize an empty gh-pages branch on the remote.
  2. Flip repo Settings → Pages source from "GitHub Actions" → "Deploy from a branch: gh-pages / (root)".
  3. Seed historical versions:
    • gh workflow run deploy-version-docs.yml -f tag=v3.13.0
    • gh workflow run deploy-version-docs.yml -f tag=v3.14.0
    • gh workflow run deploy-version-docs.yml -f tag=v3.15.1
  4. Trigger ci-docs.yml once to deploy main with the switcher.

Test plan

  • `cd docs && npm test` — 9/9 vitest cases including production-base path-preservation
  • `cd docs && npm run lint` — 0 warnings
  • `cd docs && npm run docs:build` — green, switcher classes present in `dist/index.html`
  • `node --test .github/scripts/generate-versions-json.test.mjs` — 5/5
  • Local smoke: generator against a fake `gh-pages/` dir produces a valid `versions.json`
  • After merge: verify `ci-docs.yml` run deploys main to `gh-pages` (gated on Pages source flip)
  • After merge: verify a seed run with `tag=v3.13.0` deploys to `/v3.13/` and updates `versions.json`
  • After merge: verify the switcher dropdown on the deployed `/` lists main + v3.13/v3.14/v3.15 and path-preserves correctly

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented May 11, 2026

Copy link
Copy Markdown

Walkthrough

This PR implements a complete multi-version documentation system for VitePress, enabling the publication and discovery of multiple semantic versions via manifest-driven deployment. A Node.js script generates version metadata, GitHub Actions workflows automate building and deploying to versioned paths, and a Vue component provides runtime version switching with intelligent navigation fallback.

Changes

Multi-version documentation deployment

Layer / File(s) Summary
Version configuration model
docs/.vitepress/config.ts
VitePress config now reads DOCS_VERSION environment variable (defaulting to "main") instead of package.json version, enabling dynamic versioning at build time.
Version manifest generation script
.github/scripts/generate-versions-json.mjs
New Node.js CLI that scans a directory for semantic version subdirectories (v<major>.<minor>), generates a versions.json manifest with entries sorted descending by version and identifies the stable (newest) version.
Manifest generator tests
.github/scripts/generate-versions-json.test.mjs
Tests verify correct sorting of semantic versions (numeric minor/major), stable version selection, empty input handling, and path formatting with leading/trailing slashes.
Main branch docs CI and deployment
.github/workflows/ci-docs.yml
Workflow extended to run npm test in docs, replaced GitHub Pages artifact/deploy with manual git worktree sync that preserves versioned subdirectories, regenerates versions.json, and deploys via peaceiris/actions-gh-pages. Pull requests build without deployment.
Versioned release deployment workflow
.github/workflows/deploy-version-docs.yml
New workflow triggered on published releases validates semantic tags, builds docs with versioned BASE_URL and DOCS_VERSION, copies output to gh-pages/{vMAJOR.MINOR}/, regenerates manifest, and deploys. Supports manual triggering with tag input.
VersionSwitcher component logic
docs/.vitepress/theme/components/VersionSwitcher.vue
Vue component fetches /versions.json on mount, renders dropdown menu with current/unreleased/stable badges, probes candidate URLs with HEAD requests, and navigates while preserving route paths or falling back to version roots.
Component theme integration
docs/.vitepress/config.ts, docs/.vitepress/theme/index.ts
VersionSwitcher imported and registered in theme, integrated into top navigation config under a "Resources" section.
Test framework and infrastructure
docs/vitest.config.ts, docs/package.json
Adds Vitest with jsdom environment, Vue plugin, and configures test discovery for tests/**/*.spec.ts. npm test scripts added to package.json.
VersionSwitcher component tests
docs/tests/VersionSwitcher.spec.ts
Comprehensive test suite mocks VitePress hooks and global APIs, validating manifest loading, dropdown rendering with version badges, navigation with path preservation and 404 fallback, and correct manifest URLs for both root and project-subpath deployments.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Poem

🐰 A clever bunny hops through versions old and new,
With manifests and switchers making docs ring true.
From semantic tags to paths that lead the way,
The gh-pages garden grows with every deployment day! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat(docs): versioned documentation site' clearly and directly summarizes the main change: adding versioned documentation support with a version switcher and per-release deployments.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs-versioning

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Adds a runtime-driven version switcher to the docs nav and the
build wiring that makes `/v{minor}/` deploys reachable.

The switcher fetches a manifest from the deploy root and offers
path-preserving navigation across versions, with a HEAD-on-404
fallback to the target version's root. It derives the deployment
root from `site.base` minus the current version segment, so it
works correctly both when served at /mediawiki-skins-Citizen/ and
when served from a versioned subpath.

Latest patch of each minor wins by overwriting. Past versions are
never rebuilt — the tag is the source of truth.

Seeded historical versions (v3.13/v3.14/v3.15) won't carry the
switcher in their own nav — by design. Users discover older
versions from main and switch in.

Includes a Vitest harness (vitest + jsdom + @vue/test-utils +
@vitejs/plugin-vue) and 9 cases covering rendering, dropdown
listing, path preservation under both root and project-subpath
bases, HEAD-404 fallback, and manifest-fetch failure.
Migrates the docs deploy off `actions/deploy-pages` artifact flow
onto a long-lived `gh-pages` branch, which is required for
versioned deploys: the artifact flow replaces the entire site on
each deploy and can't preserve `/v*/` subpaths across runs.

- ci-docs.yml (modified): builds main, syncs the result into
  gh-pages root via an explicit worktree (versioned subpaths and
  .git are preserved), regenerates versions.json, and pushes via
  peaceiris/actions-gh-pages. Also runs vitest in the lint job.
- deploy-version-docs.yml (new): triggered on release: published
  (with a workflow_dispatch tag input for one-time seeding). Builds
  the released tag's docs with DOCS_VERSION={minor} and BASE_URL set
  to the versioned subpath, places the build at gh-pages/{minor}/,
  regenerates versions.json. Pulls the generator script from main
  because tags predating this commit don't carry it.
- Both workflows share `concurrency: group: gh-pages-deploy` to
  serialize deploys and prevent force-push races.
- generate-versions-json.mjs (new): pure stdlib script that lists
  v*.* subdirs of gh-pages, sorts descending by semver-minor, and
  emits a manifest. Covered by 5 node:test cases.

Operational follow-up (after merge): init the gh-pages branch,
flip Settings → Pages source to "Deploy from a branch: gh-pages",
and seed v3.13/v3.14/v3.15 by running deploy-version-docs.yml
three times via workflow_dispatch.
@sonarqubecloud

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/ci-docs.yml (1)

6-10: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Broaden the path filters to include the new deploy logic.

Right now this workflow only runs for docs/** changes, so edits to .github/scripts/generate-versions-json.mjs, its tests, or this workflow itself can merge without any validation.

Suggested change
   push:
     branches: [main]
     paths:
       - "docs/**"
+      - ".github/scripts/generate-versions-json*.mjs"
+      - ".github/workflows/ci-docs.yml"
   pull_request:
     paths:
       - "docs/**"
+      - ".github/scripts/generate-versions-json*.mjs"
+      - ".github/workflows/ci-docs.yml"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci-docs.yml around lines 6 - 10, Update the CI docs
workflow path filters so it triggers on changes to the docs plus the deploy
logic and related files; include ".github/scripts/generate-versions-json.mjs",
its tests (e.g. files under "test/**" or a specific test path), and the
workflow/script files themselves (e.g. ".github/scripts/**" and
".github/workflows/**") in both the push and pull_request "paths" arrays so
edits to generate-versions-json.mjs, its tests, or the workflow cannot merge
without running validation.
🧹 Nitpick comments (4)
.github/workflows/deploy-version-docs.yml (2)

88-95: 💤 Low value

Consider adding a clarifying comment about keep_files: false.

The keep_files: false setting might appear counterintuitive since the workflow is deploying to a versioned subdirectory. However, it's correct because publish_dir contains the entire gh-pages worktree (including all existing versions). A brief comment explaining this could help future maintainers.

📝 Suggested clarification
       - name: Deploy to gh-pages
         uses: peaceiris/actions-gh-pages@v4
         with:
           github_token: ${{ secrets.GITHUB_TOKEN }}
+          # publish_dir contains the full gh-pages worktree with all versions,
+          # so keep_files: false correctly replaces the entire branch content
           publish_dir: gh-pages
           publish_branch: gh-pages
           keep_files: false
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/deploy-version-docs.yml around lines 88 - 95, Add a brief
clarifying comment above the "Deploy to gh-pages" step (the job/step that uses
peaceiris/actions-gh-pages@v4 and sets publish_dir, publish_branch, and
keep_files: false) that explains why keep_files: false is intentionally used
even though publishing into versioned subdirectories—specifically note that
publish_dir contains the full gh-pages worktree (all versions) so we want the
action to replace the branch contents rather than merge, preventing stale files;
place the comment directly above the with: block for clarity.

23-37: ⚡ Quick win

Consider validating tag existence before checkout.

The workflow validates the tag format but doesn't verify that the tag exists in the repository. If a non-existent tag is provided via manual dispatch, the error will occur during the checkout step with a less clear error message. Consider adding a validation step after tag resolution.

🔍 Proposed validation step
       echo "tag=$TAG" >> "$GITHUB_OUTPUT"
       echo "minor=$MINOR" >> "$GITHUB_OUTPUT"
+      
+      # Validate tag exists
+      if ! git ls-remote --tags origin | grep -q "refs/tags/$TAG$"; then
+        echo "Tag '$TAG' does not exist in repository" >&2
+        exit 1
+      fi
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/deploy-version-docs.yml around lines 23 - 37, After
resolving TAG in the "Resolve tag" step (variable TAG), add a validation that
confirms the tag actually exists in the repository before emitting outputs or
proceeding to checkout; for example, run a fetch of tags (git fetch --tags) and
verify the tag with git rev-parse --verify "refs/tags/$TAG" or git ls-remote
--tags origin "$TAG", and if the verification fails echo a clear error like "Tag
'$TAG' does not exist" and exit non-zero; keep the existing outputs (echo
"tag=$TAG" and "minor=$MINOR" to $GITHUB_OUTPUT) only after the existence check
passes so the workflow fails early with a helpful message instead of failing at
checkout.
docs/.vitepress/theme/components/VersionSwitcher.vue (2)

35-38: 💤 Low value

Consider defensive path handling in fromSiteRoot.

The function assumes siteRoot ends with /. While VitePress base paths conventionally end with /, adding a defensive check would prevent malformed URLs if this assumption is violated.

🛡️ Defensive path construction
 function fromSiteRoot(rootRelative: string): string {
   const stripped = rootRelative.startsWith("/") ? rootRelative.slice(1) : rootRelative;
-  return `${siteRoot.value}${stripped}`;
+  const root = siteRoot.value.endsWith("/") ? siteRoot.value : `${siteRoot.value}/`;
+  return `${root}${stripped}`;
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/.vitepress/theme/components/VersionSwitcher.vue` around lines 35 - 38,
The fromSiteRoot function assumes siteRoot.value ends with a slash and can
produce malformed URLs if it doesn't; update fromSiteRoot to defensively
normalize both parts: ensure siteRoot.value has a single trailing slash (add one
if missing) and strip any leading slash from the rootRelative input (as already
done) so concatenation always yields exactly one slash between them; reference
the fromSiteRoot function and the siteRoot.value symbol when making this
normalization.

112-152: ⚡ Quick win

Consider adding click-outside handler.

The dropdown can be toggled via the button, but there's no click-outside handler to automatically close it when clicking elsewhere on the page. This is a minor UX improvement that would align with common dropdown behavior.

♻️ Example click-outside implementation

You can use VitePress's onContentUpdated or add a simple event listener:

import { onMounted, onUnmounted } from "vue";

function handleClickOutside(event: MouseEvent) {
  const target = event.target as HTMLElement;
  if (!target.closest('.VersionSwitcher')) {
    open.value = false;
  }
}

onMounted(() => {
  document.addEventListener('click', handleClickOutside);
});

onUnmounted(() => {
  document.removeEventListener('click', handleClickOutside);
});

Note: You'll need to adjust the click handler to only attach when the dropdown is open to avoid unnecessary event listeners.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/.vitepress/theme/components/VersionSwitcher.vue` around lines 112 - 152,
Add a click-outside handler so the VersionSwitcher dropdown closes when clicking
elsewhere: in the VersionSwitcher component (referencing the reactive open, the
root .VersionSwitcher element, and menuId/buttonId), add a document click
listener that when open is true checks if the event target is outside the
.VersionSwitcher root (e.g., using closest('.VersionSwitcher')) and sets open =
false if so; attach the listener only while open (or attach onMounted and guard
by open) and remove it onUnmounted (or when open becomes false) to avoid leaks,
ensuring existing handlers like selectVersion, badgeFor, entries, currentVersion
and manifestError continue to work.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ci-docs.yml:
- Around line 133-137: The PR build step "Build (no deploy)" currently runs npm
run docs:build with DOCS_VERSION but without the deployed base path, so set the
same BASE_URL used in the deploy job (e.g. BASE_URL=/mediawiki-skins-Citizen/)
in that job's env or reference a shared variable so the "Build (no deploy)" step
runs with the same BASE_URL as the deploy job; update the workflow job named
"Build (no deploy)" where run: npm run docs:build and env: DOCS_VERSION to
include BASE_URL (or pull it from a shared workflow/env var) so PR builds mirror
the deployed base path.
- Around line 39-41: The CI currently only runs the docs Vitest suite; add a
separate workflow step after the "Run docs tests" step to execute the manifest
generator test file (.github/scripts/generate-versions-json.test.mjs) so it has
CI coverage. Update the workflow to run the test file (using node/Node.js test
runner invocation) referencing the exact script name
.github/scripts/generate-versions-json.test.mjs and give the step a clear name
like "Run manifest generator tests".

In `@docs/.vitepress/theme/components/VersionSwitcher.vue`:
- Around line 46-57: The fetch handler in onMounted currently casts the response
to VersionsManifest without validation, so update the onMounted async block to
validate the parsed JSON before assigning to manifest.value: parse the JSON into
a temporary variable, validate required fields/types (e.g., presence and types
of stable, versions array) either with a lightweight manual check or a schema
validator like zod, and only set manifest.value when validation passes; if
validation fails, set manifestError.value = true and log the validation errors
(keep the existing console.warn semantics). Ensure you reference the same
symbols (onMounted, fromSiteRoot, manifest, manifestError, VersionsManifest)
when adding the validation and error handling so downstream code that reads
manifest.value.stable is safe.

In `@docs/tests/VersionSwitcher.spec.ts`:
- Around line 42-48: The test helper stubLocation() currently replaces
globalThis.location via Object.defineProperty which Vitest cannot automatically
restore; change it to use vi.stubGlobal('location', { href: '' }) (or
vi.stubGlobal(globalThis, 'location', { href: '' }) per project conventions) so
the mock is tracked and cleaned up by vi.unstubAllGlobals()/afterEach; update
any callers of stubLocation() to rely on the vi stub and remove manual
defineProperty logic in stubLocation().

---

Outside diff comments:
In @.github/workflows/ci-docs.yml:
- Around line 6-10: Update the CI docs workflow path filters so it triggers on
changes to the docs plus the deploy logic and related files; include
".github/scripts/generate-versions-json.mjs", its tests (e.g. files under
"test/**" or a specific test path), and the workflow/script files themselves
(e.g. ".github/scripts/**" and ".github/workflows/**") in both the push and
pull_request "paths" arrays so edits to generate-versions-json.mjs, its tests,
or the workflow cannot merge without running validation.

---

Nitpick comments:
In @.github/workflows/deploy-version-docs.yml:
- Around line 88-95: Add a brief clarifying comment above the "Deploy to
gh-pages" step (the job/step that uses peaceiris/actions-gh-pages@v4 and sets
publish_dir, publish_branch, and keep_files: false) that explains why
keep_files: false is intentionally used even though publishing into versioned
subdirectories—specifically note that publish_dir contains the full gh-pages
worktree (all versions) so we want the action to replace the branch contents
rather than merge, preventing stale files; place the comment directly above the
with: block for clarity.
- Around line 23-37: After resolving TAG in the "Resolve tag" step (variable
TAG), add a validation that confirms the tag actually exists in the repository
before emitting outputs or proceeding to checkout; for example, run a fetch of
tags (git fetch --tags) and verify the tag with git rev-parse --verify
"refs/tags/$TAG" or git ls-remote --tags origin "$TAG", and if the verification
fails echo a clear error like "Tag '$TAG' does not exist" and exit non-zero;
keep the existing outputs (echo "tag=$TAG" and "minor=$MINOR" to $GITHUB_OUTPUT)
only after the existence check passes so the workflow fails early with a helpful
message instead of failing at checkout.

In `@docs/.vitepress/theme/components/VersionSwitcher.vue`:
- Around line 35-38: The fromSiteRoot function assumes siteRoot.value ends with
a slash and can produce malformed URLs if it doesn't; update fromSiteRoot to
defensively normalize both parts: ensure siteRoot.value has a single trailing
slash (add one if missing) and strip any leading slash from the rootRelative
input (as already done) so concatenation always yields exactly one slash between
them; reference the fromSiteRoot function and the siteRoot.value symbol when
making this normalization.
- Around line 112-152: Add a click-outside handler so the VersionSwitcher
dropdown closes when clicking elsewhere: in the VersionSwitcher component
(referencing the reactive open, the root .VersionSwitcher element, and
menuId/buttonId), add a document click listener that when open is true checks if
the event target is outside the .VersionSwitcher root (e.g., using
closest('.VersionSwitcher')) and sets open = false if so; attach the listener
only while open (or attach onMounted and guard by open) and remove it
onUnmounted (or when open becomes false) to avoid leaks, ensuring existing
handlers like selectVersion, badgeFor, entries, currentVersion and manifestError
continue to work.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 83f404ae-2724-45bd-9686-379dbe4bbdfb

📥 Commits

Reviewing files that changed from the base of the PR and between d720f8e and 3683744.

⛔ Files ignored due to path filters (1)
  • docs/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (10)
  • .github/scripts/generate-versions-json.mjs
  • .github/scripts/generate-versions-json.test.mjs
  • .github/workflows/ci-docs.yml
  • .github/workflows/deploy-version-docs.yml
  • docs/.vitepress/config.ts
  • docs/.vitepress/theme/components/VersionSwitcher.vue
  • docs/.vitepress/theme/index.ts
  • docs/package.json
  • docs/tests/VersionSwitcher.spec.ts
  • docs/vitest.config.ts

Comment on lines +39 to +41
- name: Run docs tests
working-directory: docs
run: npm test

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Run the manifest generator tests in CI too.

npm test here only covers the docs/ Vitest suite. The new .github/scripts/generate-versions-json.test.mjs file sits outside that package, so the manifest generator currently has no CI coverage.

Suggested change
       - name: Run docs tests
         working-directory: docs
         run: npm test
+
+      - name: Run versions manifest tests
+        run: node --test .github/scripts/generate-versions-json.test.mjs
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Run docs tests
working-directory: docs
run: npm test
- name: Run docs tests
working-directory: docs
run: npm test
- name: Run versions manifest tests
run: node --test .github/scripts/generate-versions-json.test.mjs
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci-docs.yml around lines 39 - 41, The CI currently only
runs the docs Vitest suite; add a separate workflow step after the "Run docs
tests" step to execute the manifest generator test file
(.github/scripts/generate-versions-json.test.mjs) so it has CI coverage. Update
the workflow to run the test file (using node/Node.js test runner invocation)
referencing the exact script name
.github/scripts/generate-versions-json.test.mjs and give the step a clear name
like "Run manifest generator tests".

Comment on lines +133 to +137
- name: Build (no deploy)
working-directory: docs
env:
DOCS_VERSION: main
run: npm run docs:build

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Build PRs with the deployed base path.

The deploy job builds with BASE_URL: /mediawiki-skins-Citizen/, but the PR build uses the default /. That means project-subpath breakage can pass PR CI and only show up after merge.

Suggested change
       - name: Build (no deploy)
         working-directory: docs
         env:
+          BASE_URL: /mediawiki-skins-Citizen/
           DOCS_VERSION: main
         run: npm run docs:build
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Build (no deploy)
working-directory: docs
env:
DOCS_VERSION: main
run: npm run docs:build
- name: Build (no deploy)
working-directory: docs
env:
BASE_URL: /mediawiki-skins-Citizen/
DOCS_VERSION: main
run: npm run docs:build
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci-docs.yml around lines 133 - 137, The PR build step
"Build (no deploy)" currently runs npm run docs:build with DOCS_VERSION but
without the deployed base path, so set the same BASE_URL used in the deploy job
(e.g. BASE_URL=/mediawiki-skins-Citizen/) in that job's env or reference a
shared variable so the "Build (no deploy)" step runs with the same BASE_URL as
the deploy job; update the workflow job named "Build (no deploy)" where run: npm
run docs:build and env: DOCS_VERSION to include BASE_URL (or pull it from a
shared workflow/env var) so PR builds mirror the deployed base path.

Comment on lines +46 to +57
onMounted(async () => {
try {
const res = await fetch(fromSiteRoot("/versions.json"), { cache: "no-cache" });
if (!res.ok) {
throw new Error(`status ${res.status}`);
}
manifest.value = (await res.json()) as VersionsManifest;
} catch (error) {
manifestError.value = true;
console.warn("[VersionSwitcher] failed to load versions.json", error);
}
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add runtime validation for the versions manifest.

The fetched JSON is cast to VersionsManifest without validation. If the manifest structure doesn't match the interface (e.g., missing fields, wrong types), accessing properties like manifest.value.stable later in the code could cause runtime errors or unexpected behavior.

🛡️ Proposed validation
       manifest.value = (await res.json()) as VersionsManifest;
+      // Basic validation
+      if (!manifest.value.main || !Array.isArray(manifest.value.versions)) {
+        throw new Error("Invalid manifest structure");
+      }
     } catch (error) {
       manifestError.value = true;

For more robust validation, consider using a library like zod to validate the manifest structure at runtime.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
onMounted(async () => {
try {
const res = await fetch(fromSiteRoot("/versions.json"), { cache: "no-cache" });
if (!res.ok) {
throw new Error(`status ${res.status}`);
}
manifest.value = (await res.json()) as VersionsManifest;
} catch (error) {
manifestError.value = true;
console.warn("[VersionSwitcher] failed to load versions.json", error);
}
});
onMounted(async () => {
try {
const res = await fetch(fromSiteRoot("/versions.json"), { cache: "no-cache" });
if (!res.ok) {
throw new Error(`status ${res.status}`);
}
manifest.value = (await res.json()) as VersionsManifest;
// Basic validation
if (!manifest.value.main || !Array.isArray(manifest.value.versions)) {
throw new Error("Invalid manifest structure");
}
} catch (error) {
manifestError.value = true;
console.warn("[VersionSwitcher] failed to load versions.json", error);
}
});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/.vitepress/theme/components/VersionSwitcher.vue` around lines 46 - 57,
The fetch handler in onMounted currently casts the response to VersionsManifest
without validation, so update the onMounted async block to validate the parsed
JSON before assigning to manifest.value: parse the JSON into a temporary
variable, validate required fields/types (e.g., presence and types of stable,
versions array) either with a lightweight manual check or a schema validator
like zod, and only set manifest.value when validation passes; if validation
fails, set manifestError.value = true and log the validation errors (keep the
existing console.warn semantics). Ensure you reference the same symbols
(onMounted, fromSiteRoot, manifest, manifestError, VersionsManifest) when adding
the validation and error handling so downstream code that reads
manifest.value.stable is safe.

Comment on lines +42 to +48
function stubLocation() {
Object.defineProperty(globalThis, "location", {
configurable: true,
writable: true,
value: { href: "" },
});
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Restore location through Vitest, not a raw defineProperty.

This mock is never actually undone by vi.unstubAllGlobals(), so the suite can leak a fake location into later specs. Using vi.stubGlobal here keeps cleanup aligned with the existing afterEach.

Suggested change
 function stubLocation() {
-	Object.defineProperty(globalThis, "location", {
-		configurable: true,
-		writable: true,
-		value: { href: "" },
-	});
+	vi.stubGlobal("location", { href: "" });
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function stubLocation() {
Object.defineProperty(globalThis, "location", {
configurable: true,
writable: true,
value: { href: "" },
});
}
function stubLocation() {
vi.stubGlobal("location", { href: "" });
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/tests/VersionSwitcher.spec.ts` around lines 42 - 48, The test helper
stubLocation() currently replaces globalThis.location via Object.defineProperty
which Vitest cannot automatically restore; change it to use
vi.stubGlobal('location', { href: '' }) (or vi.stubGlobal(globalThis,
'location', { href: '' }) per project conventions) so the mock is tracked and
cleaned up by vi.unstubAllGlobals()/afterEach; update any callers of
stubLocation() to rely on the vi stub and remove manual defineProperty logic in
stubLocation().

@alistair3149 alistair3149 merged commit eea8bdf into main May 11, 2026
13 checks passed
@alistair3149 alistair3149 deleted the docs-versioning branch May 11, 2026 04:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant