Code Review — Docs Wiki (feature/docs-wiki)

• Date: 2026-07-16
• Branch: feature/docs-wiki (16 commits, 0a139a6..2c00ef0)
• Scope: VitePress docs wiki as the Nx project docs (Tasks 1–5 of docs/superpowers/plans/2026-07-16-docs-wiki.md). Task 6 (Cloudflare Access gate) is operational and still pending.
• Method: subagent-driven — per-task spec+quality review with fix loops, then a whole-branch review on the most capable model.

Deterministic gate (all green)

Check	Result
bun test ./docs/.vitepress/sidebar.test.ts	3 pass / 0 fail (11 assertions)
bunx nx build docs	success → docs/.vitepress/dist/index.html
bunx nx lint docs	success (0 errors) — with generated .vitepress/cache present
bunx nx run-many -t lint (all 6 projects)	success, 0 errors (14 pre-existing no-explicit-any warnings in apps/web spec files, unrelated to this branch)
git status	clean; dist/cache untracked (gitignored)

Findings

🔴 Blocking

None.

🟡 Should-fix / Decision required

• docs is in the product CI lint/build gate — contrary to the spec. @nx/eslint/plugin infers a lint target and docs/project.json declares a build target, so ci.yml (nx run-many -t lint test build) and .husky/pre-push (nx affected -t …) both now run nx lint docs + nx build docs. The spec §"Nx targets & dependencies" says "the docs project stays out of the product web/api lint/test/build gate and the husky pre-push hook." Green today, but a future doc with unescaped Vue {{ }} (which ignoreDeadLinks does not cover) would fail nx build docs and turn an unrelated code+docs PR's required ci check red — the coupling the spec meant to avoid. Decision: (a) accept docs in the gate (docs build is now CI-verified — arguably desirable) and correct the spec sentence; or (b) exclude docs from the gate (--exclude=docs on the run-many/affected calls, or a project tag). Not a code bug; the human picks which governs.
• Sidebar spec never runs in CI (coupled to the above). sidebar.test.ts runs only via bun test; there is no Nx test target on docs, so nx run-many -t test skips it and the scanner can regress unnoticed. If option (a) above is chosen, adding a test target (bun test) to docs/project.json closes this; if (b), leave as-is.

🟢 Nits (reviewed → acceptable, logged for the record)

• docs/.vitepress/sidebar.ts:84 — the contract-qa check in the files loop is unreachable (dirs are skipped a level up at :74); harmless defensive dead code.
• docs/.vitepress/sidebar.ts:66-68 — file sort comparator isn't a strict weak order for a folder mixing dated + undated .md; all real folders are homogeneous, and JS sort never throws — worst case is a cosmetic ordering quirk.
• docs/.vitepress/sidebar.test.ts — no assertion for the buildSidebar-level H1-fallback / unreadable-file path (titleFromContent(null) is unit-tested; titleForFile is a trivial try/catch).
• docs/.vitepress/theme/custom.css:9 — --vp-c-brand-soft has no .dark override; the 20%-opacity lime reads fine on both themes.

What was built (per task)

1. VitePress skeleton (docs/.vitepress/config.mts, index.md, project.json, .gitignore) — Nx project docs rooted at docs/, srcExclude contract-qa, ignoreDeadLinks, local search. Fix: narrowed .nxignore so Nx discovers the project; kept vendored contract-qa out of eslint scope; <code v-pre> on two self-referential planning docs that literally contain {{ … }}.
2. Sidebar scanner (TDD) (sidebar.ts + sidebar.test.ts) — cwd-independent (import.meta.url), H1 titles, date-desc sort, contract-qa/index excluded, empty folders omitted. 3/3 bun tests.
3. Live sidebar (live-sidebar.ts) — config-file mtime touch (not server.restart()), because a plugin server.restart() breaks local search (MiniSearch: duplicate ID, vitepress#4251). Controller-verified end-to-end: clean config-triggered restarts (0 duplicate-ID / 0 failed), search intact, git clean. Fix: swallow transient utimes rejection so a one-off fs error can't crash the dev server.
4. Brand accent theme (theme/index.ts, theme/custom.css) — thin Dispatch accent from #C1D841/#10130B; light-theme link colour darkened to #66761f (~5:1, clears WCAG AA). Also fixed a repo-wide build-then-lint failure by ignoring **/.vitepress/cache/** in eslint.
5. Deploy artifacts (infra/cloudflare/docs.wrangler.toml, .github/workflows/deploy-docs.yml) — manual workflow_dispatch only, publishes docs/.vitepress/dist to Pages project dispatch-docs, secrets via ${{ secrets.* }}, mirrors the existing web deploy. No issues.

Security

• No secrets committed; dist/cache gitignored and untracked.
• Sensitive files provably not served — the built dist/ contains no google_client_secret*.json, no samples/*.pdf, no contract-qa/** (VitePress serves only .md; there is no docs/public/).
• Deploy cannot self-fire (manual only). Internal docs (authz-matrix.md, auth-setup.md, dispatch-production-design.md, …) would be world-readable if deployed ungated — the Cloudflare Access gate (Task 6) is a hard prerequisite before any real deploy, and the workflow must not gain a push: trigger until it's in place.

Verdict

READY to merge (code). Two things remain the human's call/action:

1. The 🟡 decision above (docs in the CI gate — accept & correct spec, or exclude).
2. Task 6 — Cloudflare Access gate — operational, needs the Cloudflare account: stand up the dispatch-docs Pages project + an Access application (explicit email allow-list: damien@seeg.dev, damien@seeg.io, damien@seeg.co, damiens@gmail.com, damien@damien.in) covering the production hostname and *.dispatch-docs.pages.dev, verified to block anonymous access before the first real deploy.

Nothing has been pushed.