Skip to content

docs: point the Legion API link at the new docs mount - #7

Closed
jamiegibbs-del wants to merge 2 commits into
mainfrom
jamie/legion-docs-nav-target
Closed

docs: point the Legion API link at the new docs mount#7
jamiegibbs-del wants to merge 2 commits into
mainfrom
jamie/legion-docs-nav-target

Conversation

@jamiegibbs-del

@jamiegibbs-del jamiegibbs-del commented Aug 28, 2026

Copy link
Copy Markdown

The Legion API reference moved from the ReadMe site to docs.picogrid.com/legion, built from the legion-docs repo. The guide header's Legion API link still resolved to /reference/start, and the resolver's root allowlist had no legion entry, so it also refused a LEGION_DOCS_URL override pointing at the new mount. The link fails closed either way.

Change

docs/site/legion-documentation.mjs

-const defaultPath = '/reference/start';
+const defaultPath = '/legion/';

-const documentationRoots = new Set(['docs', 'guides', 'reference']);
+const documentationRoots = new Set(['legion', 'docs', 'guides', 'reference']);

reference is retained deliberately: the ReadMe site it names is still served on this host, so a build can still be pointed back at it while that remains true. It can come out once ReadMe is switched off.

Which target

I used /legion/, the mount root. Two alternatives if you would rather:

  • /legion/reference/ — the API reference section specifically, which matches the link's label most literally
  • /legion/getting-started/start/ — the exact mapped equivalent of the old /reference/start per the redirect table in legion-docs

All three return 200 today. Happy to switch to whichever you prefer.

Tests

Fixtures in legion-documentation.test.mjs and check-deploy-contract.test.mjs follow the new default, including the roots loop and the two error-message assertions that list the allowlist. Run locally:

docs:legion:test        18 tests, 18 pass, 0 fail
docs:check:deploy:test  46 tests, 46 pass, 0 fail

Not in scope

readme-overcapture-probe.mjs still probes the host root and expects it not to be the ECN Worker. That stays correct: it compares against the ECN Worker's own CSP, so a different origin serving the root does not trip it. Worth re-checking when the root is actually cut over.

Reviewer note

CODEOWNERS maps /docs/ to @picogrid/edge, so this requests that team. One approval needed.

Summary by CodeRabbit

  • Documentation
    • Legion documentation now opens at the reviewed /legion/ path by default.
    • The legion documentation root is now recognized alongside existing documentation paths.
  • Bug Fixes
    • Updated deployment and URL validation to correctly accept the new Legion path and reject invalid neighboring paths.

The Legion API reference moved from the ReadMe site to
docs.picogrid.com/legion, built from the legion-docs repo. The guide header's
Legion API link still resolved to /reference/start, and the resolver's root
allowlist had no `legion` entry, so it also refused a LEGION_DOCS_URL override
pointing at the new mount. The link fails closed either way.

- defaultPath becomes /legion/
- documentationRoots gains `legion`

`reference` is retained: the ReadMe site it names is still served on this host,
so a build can still be pointed back at it while that remains true.

Test fixtures follow the new default. Both affected suites pass:
docs:legion:test 18/18, docs:check:deploy:test 46/46.
@jamiegibbs-del
jamiegibbs-del requested a review from a team as a code owner August 28, 2026 14:37
@coderabbitai

coderabbitai Bot commented Aug 28, 2026

Copy link
Copy Markdown

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)
  • docs/package-lock.json is excluded by !**/package-lock.json

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9abe64b1-487d-4f94-9cca-9d34af1a2aa4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: d5aac0ab-9c2e-4080-843e-c4b3f6e02e02

📥 Commits

Reviewing files that changed from the base of the PR and between db68caf and df5d0a1.

📒 Files selected for processing (3)
  • docs/site/check-deploy-contract.test.mjs
  • docs/site/legion-documentation.mjs
  • docs/site/legion-documentation.test.mjs

Included review availability: 8 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour.


📝 Walkthrough

Walkthrough

The Legion documentation target changed from /reference/start to /legion/. Documentation-root validation and deployment contract tests now accept and validate the new path.

Changes

Legion documentation URL

Layer / File(s) Summary
Documentation target and allowlist
docs/site/legion-documentation.mjs, docs/site/legion-documentation.test.mjs
The default target uses /legion/. The legion root is allowlisted and covered by tests.
Deployment contract validation
docs/site/check-deploy-contract.test.mjs
Contract tests accept /legion/, reject /legion/other, and generate invalid URL variants from the new path.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to df5d0

The PR updates the Legion API documentation link and allows the new /legion/ mount while preserving existing documentation roots; no actionable merge-blocking risk remains.

Suggested reviewers: audeberc

Poem

A rabbit hops where Legion docs now gleam
/legion/ replaces the former stream
Roots are checked, and paths align
Contract tests guard every sign
Fresh documentation follows the line

🚥 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%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: updating the Legion API documentation link to the new documentation mount.
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.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jamie/legion-docs-nav-target

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

@coderabbitai
coderabbitai Bot requested a review from audeberc August 28, 2026 14:38
`Exact release artifact` is a required check and it runs
`npm audit --audit-level=high`, which now fails on a high-severity nanoid
advisory: custom generators can loop indefinitely when size is zero, affecting
< 3.3.18. nanoid is transitive here, astro -> vite -> postcss -> nanoid, so the
lockfile is the only place to pin it.

Fails on untouched main too, so this is not fallout from the docs change in this
branch; it is bundled because the gate blocks the merge either way.

npm update nanoid --package-lock-only. Six lines. After it,
npm audit --audit-level=high reports zero vulnerabilities at any severity, and
both docs suites still pass: docs:legion:test 18/18,
docs:check:deploy:test 46/46.
@jamiegibbs-del

Copy link
Copy Markdown
Author

Added a second commit to clear the failing required check.

Exact release artifact runs npm audit --audit-level=high, which started failing on a high-severity nanoid advisory (custom generators can loop indefinitely when size is zero, < 3.3.18). It fails on untouched main as well, so it is not fallout from the docs change here. Verified both ways locally:

my branch, npm audit --audit-level=high     exit 1
unmodified main, same command               exit 1

nanoid is transitive, astro -> vite -> postcss -> nanoid, so the lockfile is the only place to pin it. npm update nanoid --package-lock-only moves 3.3.17 to 3.3.18 in six lines.

After the bump:

npm audit --audit-level=high    exit 0, zero vulnerabilities at any severity
docs:legion:test                18 tests, 18 pass, 0 fail
docs:check:deploy:test          46 tests, 46 pass, 0 fail

Happy to split this into its own PR if you would rather keep the docs change isolated. It is bundled only because the gate is required and blocks the merge either way.

@kazazes kazazes closed this Sep 1, 2026
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.

3 participants