ci(docs): run docs build on every PR + add version-pin and src-citation guards - #88
Conversation
…on guards Closes #51. The docs workflow used to gate behind a paths: filter scoped to docs/**, mkdocs.yml, CHANGELOG.md and the workflow itself, so code-side PRs that invalidated doc facts (Renovate Bun bumps, Dockerfile changes, src/ refactors that shifted cited line numbers) never tripped mkdocs build --strict. Even when --strict ran, it only validates internal links and snippet targets, not prose-vs-source agreement. Drops the paths: filter from pull_request: so every PR runs the docs build (Deploy to GitHub Pages stays gated on push / workflow_dispatch). Adds two project-specific checks that run before the strict build: - scripts/check-docs-versions.ts pins every Bun version reference in docs/ to .tool-versions, and asserts package.json engines.bun / packageManager and the two Dockerfile.* FROM oven/bun:<ver> lines agree. - scripts/check-docs-citations.ts walks docs/**/*.md, extracts every src/<path>:<line> (or :<start>-<end>) citation, and verifies the file exists and the line / range is in bounds. Both scripts are also chained into bun run check. CLAUDE.md's Documentation section is refreshed for the post-reorg docs/ layout (operate/build/use directories) and now declares the two new gates. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
bot workflow 🔍 Code review complete — 6 files, +474/-122. SummarySolid, well-scoped CI hardening PR. The diagnosis is correct (the old What was checked
FindingsAll three posted as inline comments on the diff. None blocking.
ReasoningA few things looked like they might be issues but checked out on closer reading:
Net: ship-ready. Recommend addressing the three minor inline findings, but none of them gate this PR. cost: $3.5897 · turns: 54 · duration: 475s |
| let foundBase = false; | ||
| for (let i = 0; i < lines.length; i++) { | ||
| const line = lines[i] ?? ""; | ||
| const baseMatch = /^FROM\s+oven\/bun:(\d+\.\d+\.\d+)\s+AS\s+base/.exec(line); |
There was a problem hiding this comment.
[minor] The checkDockerfile regex only validates the FROM oven/bun:<ver> AS base line, so a stale oven/bun:<ver> reference in a Dockerfile comment will not be caught. Concrete example: Dockerfile.daemon:193 says # /root is mode 700 in oven/bun:1.3.13 — when .tool-versions next bumps, the FROM line gets fixed (script catches it) but that comment silently rots with the same kind of drift this PR is trying to eliminate.
Suggest applying the same OVEN_RE = /oven\/bun:(\d+\.\d+\.\d+)/g scan you already do for docs to every Dockerfile line, in addition to the anchored FROM ... AS base check.
There was a problem hiding this comment.
Classification: Valid. Fixed in 0bf0164 — checkDockerfile now scans every oven/bun:<ver> occurrence on each Dockerfile line via the existing OVEN_RE regex (the anchored FROM ... AS base check is kept as a presence assertion). The stale-comment case at Dockerfile.daemon:193 would now trip on the next bump.
| // Match `src/<path>.<ext>:<line>` or `:<start>-<end>`. Path may include | ||
| // `[A-Za-z0-9_./-]`; extension is one of the source-code extensions we | ||
| // actually cite. `:<line>` is required — bare paths are ignored. | ||
| const CITATION_RE = /\bsrc\/([A-Za-z0-9_./-]+\.(?:ts|tsx|mts|cts|mjs|cjs|js)):(\d+)(?:-(\d+))?\b/g; |
There was a problem hiding this comment.
[minor] CITATION_RE accepts .. segments inside the path component ([A-Za-z0-9_./-]+), so a doc that wrote src/../README.md.ts:1 or src/sub/../foo.ts:1 would be statSync-ed at a path that escapes src/. The script is read-only so this is hygiene rather than a security bug, but it would silently report "OK" for citations that were never actually pointing into src/, defeating the gate's intent.
Cheap fix: after the regex match, reject any path whose normalized form begins with .. or contains a .. segment, e.g. if (relPath.split("/").includes("..")) { broken.push(...); continue; }.
There was a problem hiding this comment.
Classification: Valid. Fixed in 0bf0164 — after the regex match, relPath.split("/").includes("..") rejects any citation whose path component contains a .. segment with reason path contains a \..` segment — citations must point inside src/. New test test/scripts/check-docs-citations.test.ts includes a regression case (src/sub/../app.ts:1`).
| process.exit(1); | ||
| } | ||
|
|
||
| main(); |
There was a problem hiding this comment.
[minor] No automated test for either new gate. The PR description shows a manual negative-path check (appending src/k8s/ephemeral-daemon-spawner.ts:99999 to a doc, observing exit 1, reverting), but nothing committed will catch it if a future refactor weakens the regex (e.g. tightens the boundary, drops an extension, or accidentally swallows the \b after \d+). These are CI gates — losing them silently is exactly the failure mode this PR is trying to prevent for docs.
Suggest a small test/scripts/check-docs-citations.test.ts (and sibling for versions) that writes a known-good and known-bad fixture into a tmp dir, invokes the check via Bun.spawnSync, and asserts the exit code + stderr substring. Two fixtures per script; ~30 LOC each.
There was a problem hiding this comment.
Classification: Valid. Fixed in 0bf0164 — added test/scripts/check-docs-versions.test.ts (5 cases) and test/scripts/check-docs-citations.test.ts (6 cases). Each test builds a tmp-dir fixture (.tool-versions, package.json, Dockerfile.*, docs/, src/), spawns the script via Bun.spawnSync with DOCS_CHECK_REPO_ROOT pointing at the fixture, and asserts exit code + a stderr substring. Both scripts grew a tiny env-var seam (process.env["DOCS_CHECK_REPO_ROOT"] ?? <import.meta.url-derived default>) so production invocations are unchanged but tests don't have to copy the script. bun test test/scripts/ → 11 pass, 0 fail.
|
bot workflow 🔎 Resolve iteration complete — 0 failing checks, 3 open comment threads (some may already be resolved). Resolve — PR #88 (
|
| Check | State at start | Action |
|---|---|---|
| All required checks | passing on 3455f33 |
None (branch was clean entering resolve) |
No diagnose-and-fix cycle was needed. The follow-up commit 0bf0164 will rerun the same suite. The new tests are scoped to test/scripts/ and stay inside the existing 90% per-file coverage threshold (Bun's coverage only counts files loaded into the test process; the spawned scripts run in a child process and aren't measured by the parent test file's coverage).
Review comments
| ID | File:line | Classification | Action | Reply |
|---|---|---|---|---|
3173541169 |
scripts/check-docs-versions.ts:113 |
Valid | checkDockerfile now scans every oven/bun:<ver> occurrence on each line via the existing OVEN_RE regex; the anchored FROM … AS base check is kept solely as a presence assertion. Stale comments such as Dockerfile.daemon:193 (# /root is mode 700 in oven/bun:1.3.13) now trip on the next bump. Regression test: flags a Dockerfile comment whose oven/bun:<ver> has rotted in test/scripts/check-docs-versions.test.ts. |
3173593316 |
3173541707 |
scripts/check-docs-citations.ts:29 |
Valid | After the regex match, relPath.split("/").includes("..") rejects any citation whose path component contains a .. segment, with reason path contains a \..` segment — citations must point inside src/. Regression test: rejects `..` segments in the path component`. |
3173593722 |
3173542242 |
scripts/check-docs-citations.ts:154 |
Valid | Added test/scripts/check-docs-versions.test.ts (5 cases) + test/scripts/check-docs-citations.test.ts (6 cases). Each builds a tmp-dir fixture (.tool-versions, package.json, Dockerfile.*, docs/, src/), spawns the script via Bun.spawnSync with DOCS_CHECK_REPO_ROOT pointing at the fixture, and asserts exit code + a stderr substring. Both scripts now read process.env["DOCS_CHECK_REPO_ROOT"] ?? <import.meta.url default> so production behaviour is unchanged. bun test test/scripts/ → 11 pass, 0 fail. |
3173594183 |
Commits pushed
0bf0164· ci(docs): harden version + citation gates against PR ci(docs): run docs build on every PR + add version-pin and src-citation guards #88 review feedback
Outstanding
Nothing blocks merge from a resolve perspective:
- All three review comments are addressed and have evidence-backed replies.
- Local
bun run typecheck,bun run lint(0 errors / 276 pre-existing warnings, unchanged),bun run format,bun run check:no-destructive,bun run check:docs-versions,bun run check:docs-citations, andbun test test/scripts/are all green. bun run test(full suite viascripts/test-isolated.sh/ Postgres + Valkey) andbun run docs:build(mkdocs strict, needs Python deps) were not run locally — both run in CI on this PR and were passing on the prior commit.- This bot can post inline replies but cannot submit a formal
APPROVEreview decision (FR-017). Final approval and merge remain a human action.
cost: $4.9707 · turns: 75 · duration: 875s
- check-docs-versions.ts: scan every `oven/bun:<ver>` occurrence on each
Dockerfile line, not just the anchored `FROM ... AS base` form, so a
stale comment such as `Dockerfile.daemon:193` (`# /root is mode 700 in
oven/bun:1.3.13`) now trips on the next bump.
- check-docs-citations.ts: reject `..` segments inside the citation path
component so a doc that wrote `src/sub/../foo.ts:1` no longer silently
reports OK for a pointer that escapes `src/`.
- Add `test/scripts/check-docs-{versions,citations}.test.ts` exercising
both gates against tmp-dir fixtures via `Bun.spawnSync`. Both scripts
now honor a `DOCS_CHECK_REPO_ROOT` env override so tests can point the
gate at a fixture tree without copying the script; production
invocations leave it unset and resolve from the script's own location.
Refs PR #88 review thread.
Co-authored-by: chrisleekr-bot[bot] <chrisleekr-bot[bot]@users.noreply.github.com>
Co-authored-by: chrisleekr-bot[bot] <chrisleekr-bot[bot]@users.noreply.github.com>
|
bot workflow 🔍 Code review complete — 9 files, +768/-209. Review — PR #88 (
|
|
🎉 This PR is included in version 1.8.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
Closes #51. The
docs/workflow used to gate behind apaths:filter scoped todocs/**,mkdocs.yml,CHANGELOG.mdand the workflow itself, so code-side PRs that invalidated doc facts (Renovate Bun bumps,Dockerfile.*changes,src/refactors that shifted cited line numbers) never trippedmkdocs build --strict. Even when--strictran, it only validates internal links and snippet targets, not prose-vs-source agreement. This PR drops thepaths:filter frompull_request:so every PR runs the docs build (theDeploy to GitHub Pagesstep keeps itsif: github.event_name == 'push' || github.event_name == 'workflow_dispatch'guard, so PRs validate but never publish), adds two project-specific guards that run before the strict build, and refreshesCLAUDE.md'sDocumentationmap for the post-reorgdocs/layout.Changes
.github/workflows/docs.ymlpull_request:no longer carries apaths:filter. Thepush:trigger keeps its filter so GitHub Pages only redeploys when doc sources actually change.scripts/check-docs-versions.tsreads the canonical Bun version from.tool-versions, asserts thatpackage.jsonengines.bun/packageManagerand the twoDockerfile.*FROM oven/bun:<ver> AS baselines agree, then scans everydocs/**/*.mdforoven/bun:<ver>and Bun semvers (only inside lines that mentionbun, to avoid false matches on Node / openssl pins) and fails on disagreement.scripts/check-docs-citations.tswalksdocs/**/*.md, extracts everysrc/<path>.<ext>:<line>(or:<start>-<end>) citation, and verifies the file exists and the line / range is in bounds. Baresrc/foo.tsreferences without a:linesuffix are intentionally out of scope — they don't claim a line and can't go stale on a shift.check:docs-versions/check:docs-citationsscripts and are chained intobun run check. The workflow runs them asVerify docs version pins/Verify docs src citationssteps beforemkdocs build --strict(the build step is unchanged), behind a freshoven-sh/setup-bun@v2sobunis onPATH.CLAUDE.md. Refreshes the surface-to-page map for the audience-firstdocs/layout (operate/,build/,use/), and adds a paragraph declaring the two new CI-enforced gates.Files changed
.github/workflows/docs.yml· droppaths:frompull_request:; add Bun setup and twoVerify…steps beforemkdocs build --strict.scripts/check-docs-versions.ts· new — pin every Bun version reference to.tool-versions.scripts/check-docs-citations.ts· new — anchor-verifysrc/<path>:<line>citations.package.json· addcheck:docs-versions/check:docs-citationsscripts; chain both intocheck.CLAUDE.md· refreshDocumentationsurface-to-page map for the post-reorg layout; document the two new gates.IMPLEMENT.md· tracking artifact for the implement-workflow comment.Commits
3455f33· ci(docs): run docs build on every PR + add version-pin and src-citation guardsTests run
bun run typecheck· pass (clean).bun run lint· pass — 0 errors, 261 pre-existing warnings unchanged (none in the two new scripts).bun run format· pass afterbun run format:fixnormalisedscripts/check-docs-citations.ts.bun run check:no-destructive· pass.bun run check:docs-versions· pass on a clean tree (OK: every Bun version reference matches .tool-versions canonical 1.3.13).bun run check:docs-citations· pass on a clean tree (OK: every src/<path>:<line> citation in docs/ points at an in-range location).check:docs-versions: editeddocs/operate/setup.mdto say1.3.8; rerun exited 1 withdocs/operate/setup.md:9 [...] found '1.3.8', expected '1.3.13'. Reverted.check:docs-citations: appendedsrc/k8s/ephemeral-daemon-spawner.ts:99999todocs/operate/deployment.md; rerun exited 1 withstart line 99999 out of range (file has 226 lines). Reverted.bun run testandbun run docs:buildwere not run locally — the test suite goes throughscripts/test-isolated.shand depends on Postgres + Valkey docker containers, andmkdocs build --strictneedsdocs/requirements.txtPython deps. Both run in CI on this PR.Verification
Acceptance criteria from the plan:
pull_request:has nopaths:filter; theDeploy to GitHub Pagesstep keeps itspush/workflow_dispatchguard so PR runs validate without publishing. This PR itself touchessrc/-adjacent and workflow files but nodocs/**content, so theDocs / buildcheck on this PR is the live demonstration.bun run checkand as aVerify docs version pinsstep in the workflow.bun run checkand as aVerify docs src citationsstep in the workflow.bun run checkincludes both gates. Updated inpackage.json.Verify…steps invokebun run …withoutcontinue-on-error, so a non-zero exit fails theDocs / buildcheck.The four originally-cited stale facts (
SETUP.md:11>= 1.3.8,DEPLOYMENT.md:30oven/bun:1.3.12,DEPLOYMENT.md:167andDEPLOYMENT.md:215src/app.ts:<line>pointers) were already re-synced during the audience-first docs reorg in commita9c919d(#87). This PR only adds the structural CI gates that prevent them from rotting again.Related Issues
Test plan
bun run typecheckcleanbun run lintno new errors