ci: gate the docs site, and stop two files claiming it was already gated - #28
Merged
Conversation
docs/ is 48 MDX files and a 22-key docs.json, and nothing in CI read any of it. The Mintlify GitHub App's own check reports "skipped" on pull requests — it runs on the deploy branch only — so a malformed docs.json, an unclosed component tag or a dead internal link could reach openntl.org with every check green. The new Docs site job runs `mint validate` (strict: fails on warnings too) and `mint broken-links`. Both pass on the current tree, so this lands green and the first red run will be a real regression rather than pre-existing debt. I proved each one fails when it should, rather than assuming: a link to a nonexistent page exits 1 and names the file, and an unclosed <Warning> exits 1 with "Build validation failed with 1 warning(s)". A separate job rather than an addition to Docs, because that one is rustdoc over the Rust workspace: conflating them would make a red Docs ambiguous between "rustdoc broke" and "a page broke". The CLI version is pinned because Mintlify ships often, and an unpinned global install hands a third party the ability to turn this repo red on a commit that touched nothing. Two files were asserting a guarantee that did not exist, which is how this went unnoticed: - docs/README.md said "Broken internal links fail the `Docs` job in CI". That job runs `cargo doc` and would not fail on one. A promise about enforcement is worse than silence, because a contributor who reads it skips the check. - docs/package.json called `mintlify build`. There is no `build` subcommand in the CLI, so `npm run build` could never have worked. The command that does what it reached for is `mint validate`. Scripts now use the current package name and the same pin as CI, so a local pass means a CI pass. Deliberately not gated: `mint a11y` exits 1 on the accent palette and cannot be made to pass. It cross-checks every colour against both backgrounds, including pairings the rendered site never shows. The two colours do look swapped — the key names the colour, not the mode, the same inversion that bit the favicon — and fixing that is a visible design change, so it stays a follow-up on #27 rather than riding along with a CI gate. Refs #27 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0122JtRC4Dbptgefu7QR8gvZ
bryanfawcett
marked this pull request as ready for review
September 8, 2026 06:58
This was referenced Sep 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the in-scope part of #27. The colour swap and any
mint formatpass stay out, as that issue says.What this changes
A new
Docs sitejob in.github/workflows/ci.yml, running two commands againstdocs/:mint validatemint broken-linksTwo files that asserted a guarantee which did not exist:
docs/README.mdsaid "Broken internal links fail theDocsjob in CI". That job runscargo doc --workspace --no-depsand nothing else; it does not readdocs/and would not fail on a broken MDX link. It now names the real job and both commands.docs/package.jsoncalledmintlify build. There is nobuildsubcommand in the CLI, sonpm run buildindocs/could never have worked. Scripts now usemint validate, the current package name, and the same pinned version as CI.Why this way
A separate job, not an addition to
Docs. That job is rustdoc over the Rust workspace. Merging the two would need both toolchains in one job and would make a redDocsambiguous between "rustdoc broke" and "a page broke".The CLI version is pinned (
mint@4.2.876). Mintlify ships often; an unpinned global install would let a release of theirs turn this repo red on a commit that touched nothing — the failure would name our PR while the cause was upstream. Bump it deliberately, in its own commit. The same pin is indocs/package.json, so a contributor's local run and CI are the same tool, anddocs/README.mdsays to keep the two in step.Two steps rather than one
npm run, so a red run names which guarantee broke.No
paths:filter, matching every other job in this workflow. A path-filtered job that skips can sit as a permanently-pending required check.node-version: 22, following the two existing Node jobs in this workflow — #27 suggested 20; the repo's own convention wins. Nocache: npm, sincedocs/has no lockfile and no dependencies to cache.How it was verified
Both gates pass, locally and now in CI — so this lands green, and the first red run will be a real regression rather than pre-existing debt.
More importantly, I proved each one fails when it should, in throwaway copies rather than assuming a green check means a working check — the exact "green for the wrong reason" trap this repo has hit twice before:
That second one also confirms the strictness claim: a warning is what failed the build.
ci.ymlparses as YAML and the job's five steps resolve as intended. I also grepped the repo for any other file claiming a docs check that does not run —docs/README.mdwas the only one.The open question is answered: no authentication needed
Authoring this, the one thing I could not settle locally was whether
mint validatewould needmint loginon a clean runner. It does not. From the firstDocs siterun onc83d505(job log):Whole job: 59 seconds, on Node 22.23.2, with no secret, token or login step. The fallback I had lined up — dropping
validatefor adocs.jsonschema check — is not needed and is not in this diff. It stays recorded here only so a future reader knows what to reach for if Mintlify ever changes that, and the rule that went with it still stands: do not make the stepcontinue-on-error, which would restore exactly the non-guarantee this PR removes.Deliberately not gated
mint a11yexits 1 on the accent palette and cannot be made to pass. The MDX is clean (48 files, every image has alt text); the failures are colour contrast, and the tool cross-checks every accent colour against both backgrounds — including pairings the rendered site never shows. I tested the swap that #27 describes: it fixes both real pairings (7.82:1 and 6.34:1) and still exits 1. The swap is worth doing on its own merits, since the current values do look inverted — the key names the colour, not the mode, the same inversion that bit the favicon — but it is a visible design change and stays a follow-up on #27.One pre-existing warning this run surfaced, not mine to fix here
The runner now warns that
actions/checkout@v4andactions/setup-node@v4target Node.js 20, which GitHub is deprecating, and forces them onto Node 24. That applies to every job in this workflow, not just the new one — it is repo-wide and predates this diff. Bumping those actions is its own change; widening this PR to include it would blur what is being reviewed.Checks
cargo fmt --all --check— green in CI on this headcargo clippy --workspace --all-targets --all-features -- -D warnings— green in CIcargo test --workspace --all-features— green in CIcargo check -p ntl-core --target wasm32-unknown-unknown --no-default-features— green in CI (wasm32 core)RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps --all-features— green in CI (Docs)mcp/ntl-postgres-mcp-server— not touchednpm/ntl-cli— not touchedAll 11 jobs are green on
c83d505. No Rust source, manifest or lockfile is in this diff — only.github/workflows/ci.yml,docs/README.mdanddocs/package.json.Protocol impact
CI and documentation tooling only. No file under
docs/spec/is touched.Security impact
spec/threat-modelis weakened by thisWorth a reviewer's eye: this adds a third-party CLI to CI, installed globally from npm at a pinned version — 962 transitive packages, per the install log. The pin is the mitigation: it removes "whatever npm serves today" from the build. The job needs no secrets, no tokens and no login, touches nothing outside
docs/, and its two commands are read-only over the working tree.🤖 Generated with Claude Code
https://claude.ai/code/session_0122JtRC4Dbptgefu7QR8gvZ