test: gate the docs and the built types before publishing, not after - #42
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a “published-surface” quality gate intended to catch documentation/link, type-surface, and changelog defects before a release, plus a scheduled workflow to detect peer dependency ranges drifting into known advisories.
Changes:
- Add
pnpm check:publishedand implementscripts/check-published-surface.mjsto validate README links/snippets againstdist/and verify CHANGELOG sections against git tags. - Update repository metadata to support the new gate (
.gitignorefor.docs-gate/, reorganizedpackage.jsonscripts). - Add a scheduled “Peer Advisory Drift” GitHub Actions workflow using the org reusable workflow.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| scripts/check-published-surface.mjs | New published-surface gate script (README link + snippet compilation, CHANGELOG/tag validation). |
| package.json | Adds check:published script and rearranges scripts section. |
| .gitignore | Ignores .docs-gate/ consumer fixture directory. |
| .github/workflows/peer-advisory-drift.yml | Adds scheduled workflow to audit declared peer dependency ranges against advisories. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (1)
scripts/check-published-surface.mjs:104
- The link probe uses
fetch()with no timeout, so a single stalled host can hangpnpm check:published(and therefore CI / prepublishOnly) indefinitely. Also, hard-failing on transient 5xx responses can make releases flaky even when links are valid.
let res = await fetch(probe, { method: 'HEAD', redirect: 'follow', headers })
if (res.status === 405 || res.status === 403 || res.status === 429) {
res = await fetch(probe, { method: 'GET', redirect: 'follow', headers })
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (2)
scripts/check-published-surface.mjs:115
- When falling back to
GET, the response body is never consumed or cancelled. With Node’sfetch(undici), leaving bodies unconsumed can keep sockets/resources open longer than necessary, which can make this gate flaky on READMEs with many links.
const opts = { redirect: 'follow', headers, signal: AbortSignal.timeout(10_000) }
let res = await fetch(probe, { method: 'HEAD', ...opts })
if (res.status === 405 || res.status === 403 || res.status === 429) {
res = await fetch(probe, { method: 'GET', ...opts })
}
.github/workflows/release.yml:98
pnpm prepublishOnly(run earlier in this workflow) already executespnpm check:publishedvia package.json. Running it again here makes the release job slower and increases exposure to flaky network checks without adding coverage.
run: |
pnpm size
pnpm check:exports
pnpm check:published
node scripts/dogfood-smoke-test.mjs
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (1)
scripts/check-published-surface.mjs:41
TYPE_TESTS_GLOBonly includes../test/types/**/*.ts, but this repo’s published type tests aretest/types/public-api.test-d.ts(a.d.ts), so the gate currently never compiles the type tests againstdist/(only the README snippets).
/** The published type tests, compiled here against `dist/` as well. Empty when a
* library has none — the check then covers only the README snippets. */
const TYPE_TESTS_GLOB = existsSync(join(ROOT, 'test', 'types')) ? '../test/types/**/*.ts' : '*.ts'
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (4)
scripts/check-published-surface.mjs:334
file?.split('/')assumes POSIX path separators. On Windows,tscdiagnostics use backslashes, so this lookup will fail and may misclassify errors as "real" (or vice versa). Split on both separators to keep the gate cross-platform.
const file = /^([^(]+)\(/.exec(line)?.[1]
const own = sources.get(file?.split('/').pop() ?? '') ?? ''
return new RegExp(`\\b(class|interface|type)\\s+${m[1]}\\b`).test(own)
scripts/check-published-surface.mjs:203
git fetch --tagsis run viaexecFileSyncwith no timeout. If the remote is slow/unreachable, this can hangpnpm check:published(and thereforeprepublishOnly) indefinitely. Passing a boundedtimeoutkeeps the gate reliable.
try {
execFileSync('git', ['fetch', '--tags', '--quiet'], { cwd: ROOT, stdio: 'ignore' })
} catch {
scripts/check-published-surface.mjs:20
pnpm lintruns ESLint overscripts/, andeslint.config.mjssetsno-consoleto warn (allowing onlyconsole.warn/console.error). This script usesconsole.login several places, which will add avoidable lint warnings in CI and duringprepublishOnly. Consider disablingno-consolefor this script (or switching toprocess.stdout.write).
This issue also appears in the following locations of the same file:
- line 201
- line 332
import { execFileSync } from 'node:child_process'
.github/workflows/release.yml:98
pnpm prepublishOnlyalready runspnpm check:published(per package.json). Running it again here duplicates the external link probes and TypeScript compilation, adding time and extra flake surface to releases. Consider running it only once in the release workflow.
run: |
pnpm size
pnpm check:exports
pnpm check:published
node scripts/dogfood-smoke-test.mjs
|
Four suppressed comments on the last review. Three applied, one does not hold here — all in a67ddc1.
|
The nest-queue releases 1.0.3 and 1.0.4 each existed only to fix a defect that surfaced after publishing: a README link returning 404, and an exported type that rejected the snippet the README shows. Neither had a gate, and the same gaps exist here. check:published scaffolds a throwaway consumer, symlinks the package into its own node_modules so resolution goes through the exports map into dist/, and then verifies that the README's links resolve, that its TypeScript snippets and the type tests compile against the built package, and that every v*.*.* tag has a CHANGELOG section -- the tags being an outside source of truth a file cannot contradict about itself. test:types maps the package to ./src through tsconfig paths, so it can never see a divergence between the source and the .d.ts a consumer receives. This closes that.
…Only The previous commit added the script without making it a gate. It now runs in CI after the build, in release.yml before the publish, and inside prepublishOnly -- the last one because the first publish of a package is manual by design (npm trusted publishing requires the package to exist), which is precisely the path that bypasses the tag workflow. The checkout for that job fetches full history. actions/checkout is shallow by default and carries no tags, so git tag --list returns an empty set with exit code 0 and the changelog cross-check would find nothing and pass -- the silent no-op the gate exists to prevent. The script also fetches tags itself and fails when none are visible while the changelog documents releases. Unlike check:exports this gate packs no tarball, so it can live in prepublishOnly without the nested-pack failure that keeps attw out.
Per-request timeout on every link probe, since the gate now runs inside prepublishOnly and one hung host must not stall a publish. The repository's pinned TypeScript instead of npx tsc. Badge images are no longer probed -- a badge is [](target) and a naive link regex captures the image, so every run hit shields.io. Headings are read with fenced blocks removed, so a '# Using pnpm' inside a bash fence no longer enters the anchor set and lets a broken anchor pass. Anchors in raw <a href> are checked, which the header navigation uses. releaseNotes strips only trailing whitespace, mirroring the awk in release.yml exactly. The shallow-clone guard keys on git rev-parse --is-shallow-repository rather than on the absence of tags, which is the correct state for a package that was never released.
README.md and CHANGELOG.md are in files, so they are the published package. A documentation fix left on main means the npm page -- where people actually read it -- stays wrong, with no telling when the next release comes. Nothing in this branch touches a shipped file, so no version bump belongs here.
…ition A version heading is reference-link syntax and renders as plain text without a matching [x.y.z]: definition. check:published now checks it; here it found that none of the released versions had one.
Path separators: tsc emits backslashes on Windows, so taking the basename by splitting on '/' alone would fail to match a snippet back to its source and could misclassify a context diagnostic as an API defect. Splits on either now. git fetch --tags ran unbounded. The gate lives inside prepublishOnly, so an unreachable remote could hang a publish rather than a CI job; bounded at 20s, after which the local tags are all there is and the shallow guard still fires. release.yml ran check:published a second time. The step above it invokes pnpm prepublishOnly, which already includes it, so the repeat re-probed every README link and recompiled the snippets for no new information -- time and one more chance of a network flake in the one job that must not fail spuriously. Removed, with a note saying why it is absent so nobody re-adds it. Not applied: the no-console warning. eslint.config.mjs already sets no-console to off for this file's block, and pnpm lint reports zero warnings.
a67ddc1 to
d5f0e6f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (1)
scripts/check-published-surface.mjs:267
symlinkSync(..., 'dir')can fail on Windows unless the process has the right privileges (directory symlinks often require admin/Developer Mode). Since this gate may be run locally viaprepublishOnly, prefer a Windows-friendly link type (junction) to keep the script cross-platform.
const scope = join(GATE_DIR, 'node_modules', ...PKG.name.split('/').slice(0, -1))
mkdirSync(scope, { recursive: true })
symlinkSync(ROOT, join(GATE_DIR, 'node_modules', PKG.name), 'dir')
The published-surface gate runs on pull_request and fetches every http(s) URL it finds in the README. A fork could therefore point a link at a loopback, RFC 1918 or link-local address and have the CI runner probe an endpoint the author cannot reach themselves. Hosts in those ranges — and plain-http URLs — are now rejected before the request is made and reported as findings, since a published README has no business linking there either.
A directory symlink requires elevation or Developer Mode on Windows, which would make the gate unrunnable locally there; a junction needs neither and is ignored as a type on POSIX.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (1)
scripts/check-published-surface.mjs:52
- The section header says every README link "resolves", but
checkLinks()treats transport errors (DNS failure, timeout, connection reset) as non-fatal notes and returns success. To avoid overstating what the gate guarantees, reword this header to reflect that only HTTP status failures are fatal while reachability issues are reported as notes.
// ---------------------------------------------------------------------------
// 1. Every link in the README resolves.
// ---------------------------------------------------------------------------
|
Also applied the suppressed comment on |
A bad HTTP status fails the gate, but a link the machine could not reach at all is only reported as a note — an offline runner is not a defect in the documentation. The section header claimed both were enforced.
|
Third suppressed comment applied in ed0006a — Copilot is factually right. The |
…IPv4 The previous guard enumerated the IPv4 private ranges and ::1, which left unique-local, link-local and IPv4-mapped IPv6 literals able to send the CI runner at a non-public address. Enumerating ranges means enumerating them again for the next notation, so the whole class goes instead: a link in the documentation points at a hostname. The private ranges keep their own message because naming the range is the more useful diagnosis.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (1)
scripts/check-published-surface.mjs:125
checkLinks()only collectshttps?://…URLs, so README links to repo files (e.g../docs/technical_specification.md,./LICENSE) are never validated. This contradicts the stated goal that every README link resolves, and it would still allow a broken relative link to ship.
return 'is a private address'
}
if (a === 169 && b === 254) return 'is a link-local address (cloud metadata range)'
return 'is an IPv4 literal, not a hostname'
}
nest-queuehad to publish1.0.3and1.0.4for defects that only surfaced after the release. Neither had a gate, and the same gaps exist here.test:typesmaps the package to./srcthrough tsconfigpaths, so it never compares the source against the.d.tsa consumer receives## [x.y.z]heading deleted while adding the next onepnpm check:publishedScaffolds a throwaway consumer in
.docs-gate/, symlinking the package into its ownnode_modulesso resolution goes through theexportsmap intodist/— what a consumer sees, not what the sources declare. Then:v*.*.*git tag has a## [x.y.z]CHANGELOG sectionThe tag cross-check uses tags deliberately: a check that only validates the versions a file happens to list cannot notice one that is absent, which is exactly how a heading was lost.
It runs in CI after
buildand inrelease.ymlbesidesize/check:exports, so a tag cannot publish past it.Red-checked in nest-queue
Each of the three defects, reintroduced, fails the gate. Worth saying that two of the three passed on my first attempt — the changelog check only inspected versions the file listed, and the snippet check compiled the inline form that always worked. Shipping either as-is would have been worse than no gate, because it would have looked like coverage.
What it found here
Running it across all nine libraries surfaced defects in three of them. This repository's results are in the commit; the notable ones overall were in
nest-logger, where all three custom-destination examples and the API reference table declaredwrite(entry: LogEntry)against a publishedwrite(payload: string).Gate
Green in this repository.
Ready for your review — not merging it.