diff --git a/.changeset/dist-freshness-declaration-stamp.md b/.changeset/dist-freshness-declaration-stamp.md new file mode 100644 index 0000000000..ae718158c2 --- /dev/null +++ b/.changeset/dist-freshness-declaration-stamp.md @@ -0,0 +1,13 @@ +--- +"@objectstack/spec": patch +--- + +`check:api-surface` (and every other gate that reads `packages/spec/dist`) no longer refuses a dist that is exactly current because a source file's mtime moved without its bytes changing. + +The freshness rule shared by four gates and the pre-commit hook compares `dist/**/*.d.ts` mtimes against `src/**/*.ts` mtimes. That is the right primitive — it is the artifact those gates consume, and it sees the hand-edited dist and the toolchain change no content digest can — but it cannot tell a real edit from a rewrite that left the bytes alone. A `git merge` re-checks-out an unchanged source file and bumps its mtime; the build that follows correctly does not run, because turbo's cache hashes content, so it is a cache hit that rewrites nothing and leaves every `dist/` mtime where the previous build left it. The gate then refused a correct dist, and prescribed a full rebuild — minutes, under the shared verify lock — of an artifact that needed none. + +The mtime rule keeps its power to convict and gains one way to be answered. `packages/spec`'s build now records a second stamp beside the existing one, `dist/.build-input-hash-dts`, holding the same build-input digest — but written **only** by a build that actually emitted declarations, so `OS_SKIP_DTS=1` leaves it alone. When that digest equals the sources on disk, the declarations demonstrably describe them and the refusal is cleared. The evidence may only ever **acquit**: a missing, unreadable or mismatched stamp leaves the mtime verdict standing, so nothing that passed before can start failing, and the `OS_SKIP_DTS=1`-on-a-built-tree shape that ruled out `dist/.build-input-hash` for this purpose still fails, because that build never refreshes the new file. + +The refusal message was wrong in the same case and is now driven by what was measured: it names a real content change and prints both digests when the stamp disagrees, says plainly that there is nothing to compare against when no stamp exists, and no longer sends every reader after `OS_SKIP_DTS` regardless of cause. It also notes that a repo-wide `pnpm build` may be a cache hit that rewrites nothing, so the remedy names the package build directly. + +The published tarball gains one 65-byte file next to the stamp it already shipped. diff --git a/packages/spec/scripts/build-api-surface.ts b/packages/spec/scripts/build-api-surface.ts index 668eac0dea..e7e3d11582 100644 --- a/packages/spec/scripts/build-api-surface.ts +++ b/packages/spec/scripts/build-api-surface.ts @@ -48,8 +48,10 @@ * older than `src/`. On a stale dist this script does not fail, it writes a * baseline missing every export added since the build — and `--check` then * agrees with it against the same stale dist, so the phantom breaking removal is - * green at every step. See lib/dist-freshness.ts for the mechanism and for why - * the mtime rule, not `dist/.build-input-hash`, is the primitive that covers it. + * green at every step. See lib/dist-freshness.ts for the mechanism, for why the + * mtime rule — not `dist/.build-input-hash` — is the primitive that convicts, + * and for the sibling stamp (`dist/.build-input-hash-dts`) that may acquit a + * tree whose sources were re-checked-out unchanged. */ import ts from 'typescript'; import { createHash } from 'node:crypto'; diff --git a/packages/spec/scripts/dist-freshness.test.ts b/packages/spec/scripts/dist-freshness.test.ts index db7838fe11..1879d0174c 100644 --- a/packages/spec/scripts/dist-freshness.test.ts +++ b/packages/spec/scripts/dist-freshness.test.ts @@ -32,6 +32,7 @@ import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { inspectDistFreshness, packageDirLabel } from './lib/dist-freshness'; +import { declarationStamp } from '../../../scripts/check-regen-pending.mjs'; const HERE = path.dirname(fileURLToPath(import.meta.url)); const PKG = path.resolve(HERE, '..'); @@ -56,6 +57,27 @@ function write(rel: string, content: string, mtimeEpochSeconds: number): string const OLD = Math.floor(Date.now() / 1000) - 3600; const NEW = Math.floor(Date.now() / 1000) - 60; +/** + * The digest `--stamp` would write for `sandbox` as it stands right now. + * + * Asked of the rule's own reader rather than hardcoded, deliberately: the input + * set includes turbo.json's `globalDependencies` and the repo-relative path of + * every file, so a literal here would be a fixture that rots on the next + * unrelated edit to either — and a rotted literal fails as `mismatch`, which + * reads exactly like the refusal these cases are trying to distinguish from. + * + * Two steps, because `declarationStamp` computes `actual` only when there is a + * recorded digest to compare it against (the ~30ms hash stays off the path where + * no stamp exists): seed a syntactically valid placeholder, read what the + * sources really hash to, then let the caller write that. + */ +function currentDigest(): string { + write('dist/.build-input-hash-dts', `${'0'.repeat(64)}\n`, OLD); + const { actual } = declarationStamp(sandbox); + if (!actual) throw new Error('the sandbox digest could not be computed — the fixture is wrong'); + return actual; +} + // The caller's own re-run command. Passed rather than assumed since #7181: the // wording used to name `api-surface` by hand, so the three gates that adopted // this next each printed a fourth gate's name. `dist-freshness-adoption.test.ts` @@ -210,10 +232,16 @@ describe('inspectDistFreshness — the dist precondition gen:api-surface never e // // Fresh JS, fresh stamp, stale declarations: a stamp-based guard is GREEN // here. This one is red. + // + // Since #14985 the guard DOES consult a digest, so the fixture writes the + // REAL one rather than a placeholder: `dist/.build-input-hash` holding the + // exact hash of these sources is the strongest possible form of the shape, + // and it must still be refused. What acquits is the sibling file an + // OS_SKIP_DTS=1 build never writes, and this fixture deliberately has none. write('src/contracts/job-service.ts', 'export interface JobRunOutcome { ok: boolean }', NEW); write('dist/contracts/index.d.ts', 'export {};', OLD); write('dist/contracts/index.js', 'export {};', NEW); - write('dist/.build-input-hash', `${'a'.repeat(64)}\n`, NEW); + write('dist/.build-input-hash', `${currentDigest()}\n`, NEW); const verdict = inspectDistFreshness(sandbox, 'generate', GEN_RERUN); expect(verdict.fresh).toBe(false); @@ -221,6 +249,70 @@ describe('inspectDistFreshness — the dist precondition gen:api-surface never e expect(verdict.state).toBe('stale'); }); + it('ACQUITS an mtime-stale tree whose declaration stamp matches the sources (#14985)', () => { + // The false refusal this card was filed for, in miniature. A `git merge` + // re-checks-out an UNCHANGED `src/**` file — bytes identical, mtime bumped — + // and the build that follows is a turbo cache hit that rewrites nothing, so + // every `dist/` mtime stays where the previous build left it. The mtime rule + // alone calls that stale and prescribes a multi-minute rebuild of a dist + // that is already exactly right. + write('src/contracts/job-service.ts', 'export interface JobRunOutcome { ok: boolean }', NEW); + write('dist/contracts/index.d.ts', 'export {};', OLD); + write('dist/.build-input-hash-dts', `${currentDigest()}\n`, OLD); + + expect(inspectDistFreshness(sandbox, 'generate', GEN_RERUN)).toEqual({ fresh: true }); + expect(inspectDistFreshness(sandbox, 'check', CHECK_RERUN)).toEqual({ fresh: true }); + }); + + it('and CONVICTS the same tree the moment a source byte actually changes', () => { + // The half that makes the case above non-vacuous. If the acquittal were + // keyed on the stamp's mere PRESENCE rather than on the digest, this would + // stay green — and that is #7122's false green restored, one file over. The + // stamp is written first and the source edited after, so the recorded digest + // is genuinely stale rather than never-valid. + write('src/contracts/job-service.ts', 'export interface JobRunOutcome { ok: boolean }', NEW); + write('dist/contracts/index.d.ts', 'export {};', OLD); + write('dist/.build-input-hash-dts', `${currentDigest()}\n`, OLD); + write('src/contracts/job-service.ts', 'export interface JobRunOutcome { ok: boolean; at: number }', NEW); + + const verdict = inspectDistFreshness(sandbox, 'check', CHECK_RERUN); + expect(verdict.fresh).toBe(false); + if (verdict.fresh) return; + expect(verdict.state).toBe('stale'); + // and it names the cause it actually measured, rather than sending the + // reader after OS_SKIP_DTS — the wrong-cause half of #14985. + expect(verdict.message).toContain('describe DIFFERENT sources'); + expect(verdict.message).toContain('now hashes to'); + expect(verdict.message).not.toContain('is absent or unreadable'); + }); + + it('reports a stale dist with NO declaration stamp as exactly that — no evidence, not a diagnosis', () => { + // The state every tree built before this stamp existed is in, and the one an + // OS_SKIP_DTS=1 build leaves behind. The mtime verdict stands, but the + // message may not claim a content change it never measured: `unstamped` is + // "cannot tell", and saying so is what stops the next reader spending a + // round on the wrong cause. + write('src/contracts/job-service.ts', 'export interface JobRunOutcome { ok: boolean }', NEW); + write('dist/contracts/index.d.ts', 'export {};', OLD); + + const verdict = inspectDistFreshness(sandbox, 'check', CHECK_RERUN); + expect(verdict.fresh).toBe(false); + if (verdict.fresh) return; + expect(verdict.message).toContain('is absent or unreadable'); + expect(verdict.message).not.toContain('describe DIFFERENT sources'); + }); + + it('ignores a declaration stamp that is not a digest at all', () => { + // Absence of the freshness input is not licence to acquit (#4690), and + // neither is a truncated write or a half-flushed file. Anything that is not + // 64 hex characters is `unstamped`, which leaves the refusal standing. + write('src/contracts/job-service.ts', 'export interface JobRunOutcome { ok: boolean }', NEW); + write('dist/contracts/index.d.ts', 'export {};', OLD); + write('dist/.build-input-hash-dts', 'not-a-digest\n', OLD); + + expect(inspectDistFreshness(sandbox, 'check', CHECK_RERUN).fresh).toBe(false); + }); + it('finds the newest source at ANY depth, not just the top level', () => { // A walk that stopped one level down would call this fresh and hand the // generator a dist that predates the only edit in the tree. diff --git a/packages/spec/scripts/lib/dist-freshness.ts b/packages/spec/scripts/lib/dist-freshness.ts index e26d2b49c4..167a8d2138 100644 --- a/packages/spec/scripts/lib/dist-freshness.ts +++ b/packages/spec/scripts/lib/dist-freshness.ts @@ -44,33 +44,49 @@ * carries `schemaTreeIsStale` itself "so EVERY caller is covered rather than * this one". Same shape, same reason. * - * ## Why the mtime rule and NOT `dist/.build-input-hash` - * - * #7122 suggested reusing the content stamp that `scripts/check-dev-prereqs.mjs` - * writes. Measured, it is the wrong primitive FOR THIS CONSUMER, in the - * dangerous direction: - * - * - The stamp is written by `packages/spec`'s build unconditionally, INCLUDING - * under `OS_SKIP_DTS=1` — that build emits JS, leaves whatever `.d.ts` was - * there before, and stamps anyway. check-dev-prereqs.mjs lists this as a - * known FALSE GREEN and names this gate as the one it breaks; AGENTS.md - * §"Added or removed a `packages/spec` export?" says the same in the other - * direction ("skips exactly the artifact the gate inspects, and the check - * passes locally while failing in CI"). A stamp-based guard would therefore - * be green on the one local build flag that guarantees the declarations this - * generator reads are stale. + * ## Why the mtime rule CONVICTS, and what may acquit + * + * #7122 suggested REPLACING the mtime rule with the content stamp that + * `scripts/check-dev-prereqs.mjs` writes. Measured, that swap is wrong FOR THIS + * CONSUMER, in the dangerous direction, and it stays rejected: + * + * - `dist/.build-input-hash` is written by `packages/spec`'s build + * unconditionally, INCLUDING under `OS_SKIP_DTS=1` — that build emits JS, + * leaves whatever `.d.ts` was there before, and stamps anyway. + * check-dev-prereqs.mjs lists this as a known FALSE GREEN and names this + * gate as the one it breaks; AGENTS.md §"Added or removed a `packages/spec` + * export?" says the same in the other direction ("skips exactly the artifact + * the gate inspects, and the check passes locally while failing in CI"). * - `distIsStale` keys on `dist/**` + `.d.ts` mtimes against `src/**` + `.ts` * — the artifact this generator actually consumes — so it catches the - * `OS_SKIP_DTS=1` shape as well as #7122's rebase-behind-the-dist shape. + * `OS_SKIP_DTS=1` shape as well as #7122's rebase-behind-the-dist shape, + * and it sees the hand-edited dist and the toolchain change no digest can. * - It is also the rule the other three consumers already read, so this adds * no second notion of "is packages/spec/dist current"; a second copy drifts, * and the direction it drifts in is the one that writes a confident wrong * baseline (#4675). * - * The stamp's own strength — content, not mtime (#5864) — is real and NOT - * discarded: it still guards `pnpm dev`. It is simply blind to the half of the - * dist that this generator is made of. Closing the `OS_SKIP_DTS` hole in the - * stamp itself is a separate change to a separate script. + * What mtimes cannot see is a rewrite that left the BYTES alone, and #14985 + * measured the cost of that blind spot. A `git merge` re-checks-out an unchanged + * `src/**` file, bumping its mtime; the build that follows correctly does not + * run (turbo's cache hashes content — measured `>>> FULL TURBO`, 56ms) and + * rewrites nothing; every `dist/` mtime stays where the previous build left it, + * and this guard refuses a dist that is exactly current. The remedy on offer was + * a full rebuild, minutes long and under the shared verify lock, of an artifact + * that needed none — and the refusal named `OS_SKIP_DTS` as the cause, which it + * was not. + * + * So the mtime rule keeps its power to CONVICT and gains one way to be answered: + * `dist/.build-input-hash-dts`, a sibling stamp written only by a build that + * really emitted declarations (`--stamp` skips it under `OS_SKIP_DTS=1`). When + * its digest equals the inputs on disk, the declarations demonstrably describe + * these sources. That evidence may only ACQUIT — absent or unreadable leaves the + * refusal standing — so the change cannot turn a pass into a refusal, and #7122's + * shape still convicts because its stamp is the other file. + * + * The `dist/.build-input-hash` stamp's own strength — content, not mtime (#5864) + * — is unchanged and still guards `pnpm dev`; it is simply not the file this + * reads. * * ## Why the caller names ITSELF, and why that is not a third `mode` (#7181) * @@ -97,7 +113,11 @@ import { existsSync, readdirSync, readFileSync } from 'node:fs'; import { join } from 'node:path'; -import { bundlesAreStale, distIsStale } from '../../../../scripts/check-regen-pending.mjs'; +import { + bundlesAreStale, + declarationStamp, + distIsStale, +} from '../../../../scripts/check-regen-pending.mjs'; /** What the generator is about to do, so the refusal can name the real damage. */ export type DistReadMode = 'generate' | 'check'; @@ -219,12 +239,38 @@ export function inspectDistFreshness( ` a FALSE GREEN on exactly the change it exists to catch (#7122).`; const label = packageDirLabel(pkgDir); + // Which of the three stale causes this is. Recomputed rather than threaded out + // of `distIsStale` because the boolean is the rule's whole contract and the + // wording is this file's; the second digest costs ~30ms and is spent only on + // the refusal path — the one that used to prescribe a multi-minute rebuild. + const stamp = state === 'stale' ? declarationStamp(pkgDir) : { state: 'unstamped' as const }; + const digests = + 'recorded' in stamp && stamp.recorded && stamp.actual + ? `\n recorded ${stamp.recorded.slice(0, 16)}… · ${label}/src now hashes to ${stamp.actual.slice(0, 16)}…` + : ''; const cause = state === 'missing' ? `${label}/dist holds no .d.ts declarations — the package is not built (or was built\n` + ` with OS_SKIP_DTS=1, which emits JS and skips exactly the artifact this reads).` - : `${label}/dist/**/*.d.ts is OLDER than ${label}/src — the declarations on disk\n` + - ` predate the sources. If you built with OS_SKIP_DTS=1, that build did not rebuild them.`; + : stamp.state === 'mismatch' + ? `${label}/dist/**/*.d.ts describe DIFFERENT sources than the ones on disk. The last\n` + + ` build that emitted declarations recorded a build-input digest in\n` + + ` ${label}/dist/.build-input-hash-dts, and ${label}/src no longer hashes to it — so this\n` + + ` is a real content change, not a timestamp artefact.${digests}` + : stamp.state === 'unstamped' + ? `${label}/dist/**/*.d.ts is OLDER than ${label}/src, and ${label}/dist/.build-input-hash-dts\n` + + ` is absent or unreadable, so there is nothing to check that timestamp against. Either\n` + + ` this dist predates that stamp, or the build that wrote it ran with OS_SKIP_DTS=1 and\n` + + ` never emitted declarations. One real build settles it and records the stamp, after\n` + + ` which a re-checkout that only bumps mtimes stops being refused.` + : // `match` while still refusing: the freshness rule and this wording + // read the tree at two different instants, so a build that landed + // between them arrives here. Say only what was measured — claiming + // a missing stamp that is right there is the wrong-cause defect + // #14985 was filed for, one branch over. + `${label}/dist/**/*.d.ts is OLDER than ${label}/src, while\n` + + ` ${label}/dist/.build-input-hash-dts matches those sources. The tree moved between the\n` + + ` two readings — most likely a build finished alongside this one. Re-run this check.`; return { fresh: false, @@ -235,8 +281,9 @@ export function inspectDistFreshness( ` Build first, then re-run:\n\n` + ` pnpm --filter ${packageName(pkgDir)} build\n` + ` ${rerun}\n\n` + - ` (Do NOT use OS_SKIP_DTS=1 for this one — AGENTS.md §9 names it as the flag that emits JS\n` + - ` and skips exactly the declarations this reads.)`, + ` (Build THAT package directly — a repo-wide \`pnpm build\` may be a turbo cache hit that\n` + + ` rewrites nothing. And do NOT use OS_SKIP_DTS=1 for this one — AGENTS.md §9 names it as\n` + + ` the flag that emits JS and skips exactly the declarations this reads.)`, }; } diff --git a/scripts/build-input-hash.mjs b/scripts/build-input-hash.mjs new file mode 100644 index 0000000000..40100b0a59 --- /dev/null +++ b/scripts/build-input-hash.mjs @@ -0,0 +1,218 @@ +#!/usr/bin/env node +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * build-input-hash -- the ONE definition of "which sources was this dist built + * from", and the two stamps that record the answer. + * + * ## Why this is a module and not part of `check-dev-prereqs.mjs` + * + * It was part of that file, and for one consumer that was right: the digest is + * written by `--stamp` at the end of a build and read by the gate at `pnpm dev`, + * and "they must be the same function or the comparison means nothing" is why + * the stamper lived beside its reader rather than in a script of its own. + * + * A SECOND consumer arrived with #14985 — `distIsStale` in + * `scripts/check-regen-pending.mjs` — and the same sentence now argues for a + * module: three call sites over one function, none of them a copy. Two things + * made the alternative (importing the gate) actively wrong rather than merely + * inelegant: + * + * - `check-dev-prereqs.mjs` is a GATE FILE (lint.yml runs its `--self-test`), + * and `scripts/pm/dispatch-gates.mjs` refuses to follow a gate file for + * inherited watch hints. Importing it from a gate that IS followed silently + * subtracts the `packages/spec` hint that import would otherwise have + * carried — measured, and refused by that tool's own self-test. + * - it would have run a workspace scan and `process.exit`ed inside its + * importer, whose callers include the pre-commit hook. + * + * This module has no CLI, declares no path population and spells no watch hint + * of its own: every path it touches arrives as an argument, so following it + * subtracts nothing from anybody. + * + * ## The two stamps, and why there are two + * + * Both hold the SAME digest over the SAME inputs. The difference is which build + * writes them, and that difference is the whole reason the second one exists — + * see each constant's docblock, and `inspectDeclarationStamp` for the reader. + */ +import { createHash } from 'node:crypto'; +import { existsSync, readdirSync, readFileSync } from 'node:fs'; +import path from 'node:path'; + +/** Where a build records the hash of the inputs it was built from. */ +export const STAMP_BASENAME = '.build-input-hash'; + +/** + * Where a build records that its DECLARATION pass ran, and over which inputs. + * + * Same digest, same writer, same directory as STAMP_BASENAME. The only + * difference is WHEN it is written — and that difference is the whole point: + * `--stamp` skips this file under `OS_SKIP_DTS=1`, the build flag that emits JS + * and leaves whatever `.d.ts` was there before. So this stamp asserts something + * STAMP_BASENAME cannot: *the declarations on disk were emitted from inputs with + * this digest*. + * + * Written because a second consumer needs that fact and only that fact. + * `distIsStale` in scripts/check-regen-pending.mjs answers "may a gate read + * `/dist/**\/*.d.ts` and believe it" from mtimes, which is conservative in + * the right direction but fires on any rewrite that leaves the bytes alone — a + * `git merge` or `git checkout` that re-checks-out an UNCHANGED source bumps its + * mtime, and the build then legitimately does not run (turbo's cache hashes + * content, so it is a cache hit that rewrites nothing), leaving the gate + * refusing a dist that is in fact exactly current. `declarationStampState` + * below is the evidence that clears that one case, and only that one case. + * + * ⛔ It is NOT a replacement for the mtime rule and must never become one: the + * digest cannot see a hand-edited dist, a toolchain change or dependency drift, + * which is why it may only ever ACQUIT a tree the mtime rule has already + * accused, never accuse one the mtime rule cleared. + */ +export const DTS_STAMP_BASENAME = '.build-input-hash-dts'; + +/** Per-package build configuration that changes the output without being under src/. */ +export const PACKAGE_BUILD_CONFIG = ['package.json', 'tsconfig.json', 'tsconfig.build.json', 'tsup.config.ts', 'tsdown.config.ts']; + +/** Thrown for conditions that must fail the gate rather than shrink its coverage. */ +export class CoverageError extends Error {} + +export const rel = (root, p) => path.relative(root, p) || '.'; +export const posixRel = (root, p) => rel(root, p).split(path.sep).join('/'); + +/** + * Global build inputs, read from turbo.json's own `globalDependencies` rather + * than restated here: the build's declaration of what invalidates every package + * is the freshness definition's too, and a new entry there is covered without + * anyone remembering this file. Only literal paths are understood — a glob would + * silently hash fewer inputs, so it throws. + */ +export function globalBuildInputs(root) { + const file = path.join(root, 'turbo.json'); + if (!existsSync(file)) throw new CoverageError(`turbo.json is missing — cannot determine the build's global inputs, so freshness cannot be judged.`); + let cfg; + try { + cfg = JSON.parse(readFileSync(file, 'utf-8')); + } catch (err) { + throw new CoverageError(`turbo.json is not readable as JSON (${err.message}) — cannot determine the build's global inputs.`); + } + const declared = cfg.globalDependencies ?? []; + if (!Array.isArray(declared)) throw new CoverageError(`turbo.json 'globalDependencies' is not an array — cannot determine the build's global inputs.`); + return declared.map((entry) => { + if (typeof entry !== 'string' || entry.includes('*') || entry.startsWith('$')) { + throw new CoverageError( + `turbo.json globalDependencies entry ${JSON.stringify(entry)} is not a shape this gate can hash.\n` + + ` Teach scripts/check-dev-prereqs.mjs the new shape — hashing fewer inputs than the build\n` + + ` reads would make the freshness verdict pass vacuously.`, + ); + } + return path.join(root, entry); + }); +} + +/** Every file under `dir`, sorted, node_modules excluded. */ +export function filesUnder(dir, out = []) { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + if (entry.name === 'node_modules') continue; + const p = path.join(dir, entry.name); + if (entry.isDirectory()) filesUnder(p, out); + else if (entry.isFile()) out.push(p); + } + return out; +} + +/** + * sha256 over a package's build inputs — the freshness definition, in one place, + * used by BOTH `--stamp` (at build time) and the gate (at boot time). They must + * be the same function or the comparison means nothing, which is why the stamper + * lives in this file rather than in a script of its own. + * + * Framing is length-prefixed (`:` then the bytes), so no separator + * can be forged by a file's contents and no control character is needed to + * delimit records. + */ +export function buildInputHash(root, pkgDir) { + const src = path.join(pkgDir, 'src'); + if (!existsSync(src)) { + throw new CoverageError(`${posixRel(root, pkgDir)}/src does not exist, so there is nothing to hash — this gate cannot vouch for its dist.`); + } + const inputs = [...filesUnder(src)]; + for (const name of PACKAGE_BUILD_CONFIG) inputs.push(path.join(pkgDir, name)); + inputs.push(...globalBuildInputs(root)); + + const seen = new Set(); + const records = []; + for (const file of inputs) { + const key = posixRel(root, file); + if (seen.has(key)) continue; + seen.add(key); + records.push([key, file]); + } + records.sort((a, b) => (a[0] < b[0] ? -1 : a[0] > b[0] ? 1 : 0)); + + const hash = createHash('sha256'); + for (const [key, file] of records) { + // An ABSENT input is hashed as absent rather than skipped: creating a + // tsconfig where there was none changes how the package builds, so it has + // to change the hash. + if (!existsSync(file)) { + hash.update(`${key}:absent\n`); + continue; + } + const bytes = readFileSync(file); + hash.update(`${key}:${bytes.length}\n`); + hash.update(bytes); + } + return hash.digest('hex'); +} + +/** + * Did a declaration-emitting build produce THIS dist from THESE sources? + * + * The reader for DTS_STAMP_BASENAME, exported because the caller that needs it + * is `distIsStale` in scripts/check-regen-pending.mjs — and it has to be THIS + * function over THIS hash, or the comparison means nothing (the same argument + * that keeps `--stamp` in this file rather than in a script of its own). + * + * Three verdicts, and the asymmetry between them is deliberate: + * + * - `match` the recorded digest equals the inputs on disk right now, so + * the declarations describe exactly these sources. This is the + * ONLY verdict that may clear an mtime accusation. + * - `mismatch` a declaration-emitting build ran, and the sources have moved + * since. Nameable in a refusal message: this is not an mtime + * artefact, the content really did change. + * - `unstamped` NO EVIDENCE — no stamp, an unreadable one, a digest that + * cannot be computed, or a package whose build does not stamp + * at all. Every one of those collapses to the same answer on + * purpose: "cannot vouch" must never read as "vouched for" + * (#4690), and absence of the input is not licence to acquit. + * + * It never throws: it is called from inside a freshness predicate whose failure + * direction is a silently wrong artifact, so an unreadable tree has to degrade + * to `unstamped` rather than take the caller down. + * + * Both digests come back with the verdict so a refusal can SHOW its evidence + * instead of asserting it — the defect #14985 filed was half a wrong message, + * and "recorded X, sources now hash to Y" is a claim the reader can recompute. + * `actual` is computed only when there is a valid digest to compare it against, + * so the ~30ms hash stays off the path where no amplifier stamp exists at all. + */ +export function inspectDeclarationStamp(root, pkgDir) { + const none = { state: 'unstamped', recorded: null, actual: null }; + const stampFile = path.join(pkgDir, 'dist', DTS_STAMP_BASENAME); + let recorded; + try { + if (!existsSync(stampFile)) return none; + recorded = readFileSync(stampFile, 'utf-8').trim(); + } catch { + return none; + } + if (!/^[0-9a-f]{64}$/.test(recorded)) return none; + let actual; + try { + actual = buildInputHash(root, pkgDir); + } catch { + return { ...none, recorded }; + } + return { state: recorded === actual ? 'match' : 'mismatch', recorded, actual }; +} diff --git a/scripts/check-dev-prereqs.mjs b/scripts/check-dev-prereqs.mjs index 4b19abcc05..332f2ebb50 100644 --- a/scripts/check-dev-prereqs.mjs +++ b/scripts/check-dev-prereqs.mjs @@ -124,6 +124,13 @@ * so; the declarations may not be. This gate has never probed `.d.ts` * (dev boot needs JS), and AGENTS.md §9 already names the flag as the one * that cannot serve `gen:api-surface`. Recorded, not silently inherited. + * STILL TRUE OF THIS GATE, and it stays that way: `pnpm dev` boots JS, so + * narrowing THIS verdict on account of the declarations would be a false + * red on the documented fast local build. What changed is that the hole is + * no longer unattended — `--stamp` now writes a SECOND file next door, + * `dist/.build-input-hash-dts`, which it skips under this flag exactly so + * that a `.d.ts` reader can tell the two builds apart. Nothing here reads + * it; see DTS_STAMP_BASENAME and `declarationStampState` below. * - A HAND-EDITED dist. The hash covers inputs, not outputs. Nothing here * can see someone editing `dist/index.mjs` directly, and nothing should * have to. @@ -232,11 +239,19 @@ * layout, an amplifier or its build inputs could not be read (a gate that * cannot enumerate members must fail loudly, not pass vacuously — #4690) */ -import { createHash } from 'node:crypto'; -import { existsSync, mkdtempSync, mkdirSync, readdirSync, readFileSync, rmSync, utimesSync, writeFileSync } from 'node:fs'; +import { existsSync, mkdtempSync, mkdirSync, readFileSync, rmSync, utimesSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import path from 'node:path'; import { fileURLToPath } from 'url'; +import { + CoverageError, + DTS_STAMP_BASENAME, + STAMP_BASENAME, + buildInputHash, + inspectDeclarationStamp, + posixRel, + rel, +} from './build-input-hash.mjs'; import { WorkspaceEnumerationError, selfTest as workspaceEnumeratorSelfTest, @@ -277,11 +292,12 @@ const SELF_TEST_BATTERIES = Object.freeze({ '14. Every other way the freshness half can lose its subject is red too.': 4, '15. The hash reads the inputs it claims to. A global build input (from': 3, '16. Existence outranks freshness: a workspace that is not built reports': 4, + '17. The DECLARATIONS stamp (#14985), whose only job is to be written by a': 9, }); // DELETING an entry silences that battery's floor exactly as effectively as // zeroing it, so the roster's own size is pinned too. -const SELF_TEST_BATTERY_FLOOR = 16; +const SELF_TEST_BATTERY_FLOOR = 17; // The key an assertion is filed under when no battery is open. It is not a // declared battery, so it reds by the same set difference rather than silently @@ -301,18 +317,9 @@ const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); */ const AMPLIFIERS = ['packages/spec']; -/** Where a build records the hash of the inputs it was built from. */ -const STAMP_BASENAME = '.build-input-hash'; - /** What an amplifier's build script must contain for its stamp to be maintained. */ const STAMP_INVOCATION = 'check-dev-prereqs.mjs --stamp'; -/** Per-package build configuration that changes the output without being under src/. */ -const PACKAGE_BUILD_CONFIG = ['package.json', 'tsconfig.json', 'tsconfig.build.json', 'tsup.config.ts', 'tsdown.config.ts']; - -/** Thrown for conditions that must fail the gate rather than shrink its coverage. */ -class CoverageError extends Error {} - /** * Workspace member directories, from pnpm-workspace.yaml — the workspace's own * declaration of what it contains. @@ -352,95 +359,6 @@ function declaredEntry(pkg) { } const isBuildArtifact = (entry) => /(^|\/)dist\//.test(entry.replace(/^\.\//, '')); -const rel = (root, p) => path.relative(root, p) || '.'; -const posixRel = (root, p) => rel(root, p).split(path.sep).join('/'); - -/** - * Global build inputs, read from turbo.json's own `globalDependencies` rather - * than restated here: the build's declaration of what invalidates every package - * is the freshness definition's too, and a new entry there is covered without - * anyone remembering this file. Only literal paths are understood — a glob would - * silently hash fewer inputs, so it throws. - */ -function globalBuildInputs(root) { - const file = path.join(root, 'turbo.json'); - if (!existsSync(file)) throw new CoverageError(`turbo.json is missing — cannot determine the build's global inputs, so freshness cannot be judged.`); - let cfg; - try { - cfg = JSON.parse(readFileSync(file, 'utf-8')); - } catch (err) { - throw new CoverageError(`turbo.json is not readable as JSON (${err.message}) — cannot determine the build's global inputs.`); - } - const declared = cfg.globalDependencies ?? []; - if (!Array.isArray(declared)) throw new CoverageError(`turbo.json 'globalDependencies' is not an array — cannot determine the build's global inputs.`); - return declared.map((entry) => { - if (typeof entry !== 'string' || entry.includes('*') || entry.startsWith('$')) { - throw new CoverageError( - `turbo.json globalDependencies entry ${JSON.stringify(entry)} is not a shape this gate can hash.\n` + - ` Teach scripts/check-dev-prereqs.mjs the new shape — hashing fewer inputs than the build\n` + - ` reads would make the freshness verdict pass vacuously.`, - ); - } - return path.join(root, entry); - }); -} - -/** Every file under `dir`, sorted, node_modules excluded. */ -function filesUnder(dir, out = []) { - for (const entry of readdirSync(dir, { withFileTypes: true })) { - if (entry.name === 'node_modules') continue; - const p = path.join(dir, entry.name); - if (entry.isDirectory()) filesUnder(p, out); - else if (entry.isFile()) out.push(p); - } - return out; -} - -/** - * sha256 over a package's build inputs — the freshness definition, in one place, - * used by BOTH `--stamp` (at build time) and the gate (at boot time). They must - * be the same function or the comparison means nothing, which is why the stamper - * lives in this file rather than in a script of its own. - * - * Framing is length-prefixed (`:` then the bytes), so no separator - * can be forged by a file's contents and no control character is needed to - * delimit records. - */ -function buildInputHash(root, pkgDir) { - const src = path.join(pkgDir, 'src'); - if (!existsSync(src)) { - throw new CoverageError(`${posixRel(root, pkgDir)}/src does not exist, so there is nothing to hash — this gate cannot vouch for its dist.`); - } - const inputs = [...filesUnder(src)]; - for (const name of PACKAGE_BUILD_CONFIG) inputs.push(path.join(pkgDir, name)); - inputs.push(...globalBuildInputs(root)); - - const seen = new Set(); - const records = []; - for (const file of inputs) { - const key = posixRel(root, file); - if (seen.has(key)) continue; - seen.add(key); - records.push([key, file]); - } - records.sort((a, b) => (a[0] < b[0] ? -1 : a[0] > b[0] ? 1 : 0)); - - const hash = createHash('sha256'); - for (const [key, file] of records) { - // An ABSENT input is hashed as absent rather than skipped: creating a - // tsconfig where there was none changes how the package builds, so it has - // to change the hash. - if (!existsSync(file)) { - hash.update(`${key}:absent\n`); - continue; - } - const bytes = readFileSync(file); - hash.update(`${key}:${bytes.length}\n`); - hash.update(bytes); - } - return hash.digest('hex'); -} - /** * The freshness verdict for the declared amplifiers. Every way of NOT being able * to answer throws (#4690): a listed package that is not a workspace member, has @@ -618,9 +536,25 @@ function stamp(root, cwd, amplifiers = AMPLIFIERS) { const hash = buildInputHash(root, dir); writeFileSync(path.join(dist, STAMP_BASENAME), `${hash}\n`); console.log(`✓ ${relDir}/dist/${STAMP_BASENAME} ← ${hash.slice(0, 16)}…`); + + // The declarations half, written ONLY by a build that actually emitted them. + // Under OS_SKIP_DTS=1 the previous file is left exactly as it was: whatever + // `.d.ts` are on disk still came from the build that wrote it, so the old + // digest is the true one and refreshing it here is precisely the false green + // this stamp exists to avoid. + if (process.env.OS_SKIP_DTS) { + console.log( + `ℹ ${relDir}/dist/${DTS_STAMP_BASENAME} left as-is — OS_SKIP_DTS=1 skipped the declaration pass,\n` + + ` so this build cannot vouch for ${relDir}/dist/**/*.d.ts.`, + ); + return 0; + } + writeFileSync(path.join(dist, DTS_STAMP_BASENAME), `${hash}\n`); + console.log(`✓ ${relDir}/dist/${DTS_STAMP_BASENAME} ← ${hash.slice(0, 16)}…`); return 0; } + /** * --self-test — a gate only ever observed green is indistinguishable from a gate * that matches nothing (#4690). These fixtures drive `inspect` to every verdict @@ -891,6 +825,46 @@ function selfTest() { const halfRed = capture(() => report(v)); expect('precedence/verdict-lines', halfRed.text.split('\n').filter((l) => l.startsWith('✗')).length, 1); expect('precedence/is-the-build-one', halfRed.text.includes('The workspace is not built'), true); + + // 17. The DECLARATIONS stamp (#14985), whose only job is to be written by a + // build that emitted `.d.ts` and NOT by one that skipped them. Nothing + // in this file reads it — `distIsStale` does — so its whole value is + // that OS_SKIP_DTS=1 leaves it alone. A stamp written unconditionally + // here would restore, one file over, exactly the false green the mtime + // rule was chosen over `.build-input-hash` to avoid. + battery('17. The DECLARATIONS stamp (#14985), whose only job is to be written by a'); + const dts = amplifierFixture('dts'); + const dtsSpec = path.join(dts, 'packages/spec'); + capture(() => stamp(dts, dtsSpec, ['packages/spec'])); + expect('dts/full-build-stamps', inspectDeclarationStamp(dts, dtsSpec).state, 'match'); + expect('dts/reports-both-digests', inspectDeclarationStamp(dts, dtsSpec).recorded, buildInputHash(dts, dtsSpec)); + + write(dts, 'packages/spec/src/index.ts', 'export const token = 3;\n'); + const moved = inspectDeclarationStamp(dts, dtsSpec); + expect('dts/source-edit-is-mismatch', moved.state, 'mismatch'); + expect('dts/mismatch-shows-both', moved.recorded !== moved.actual && moved.actual !== null, true); + + // The whole point: re-stamping under the flag must NOT adopt the new digest. + const previousFlag = process.env.OS_SKIP_DTS; + try { + process.env.OS_SKIP_DTS = '1'; + capture(() => stamp(dts, dtsSpec, ['packages/spec'])); + } finally { + if (previousFlag === undefined) delete process.env.OS_SKIP_DTS; + else process.env.OS_SKIP_DTS = previousFlag; + } + expect('dts/skip-dts-does-not-refresh', inspectDeclarationStamp(dts, dtsSpec).state, 'mismatch'); + // …while the JS half, which that build really did re-emit, is refreshed. + expect('dts/skip-dts-still-stamps-js', inspect(dts, ['packages/spec']).freshness[0]?.state, 'fresh'); + + // No evidence and unreadable evidence are the same answer, and it is never + // an acquittal (#4690). + const noDts = amplifierFixture('no-dts-stamp'); + const noDtsSpec = path.join(noDts, 'packages/spec'); + expect('dts/absent-is-unstamped', inspectDeclarationStamp(noDts, noDtsSpec).state, 'unstamped'); + expect('dts/absent-computes-nothing', inspectDeclarationStamp(noDts, noDtsSpec).actual, null); + write(noDts, 'packages/spec/dist/' + DTS_STAMP_BASENAME, 'not-a-hash\n'); + expect('dts/garbled-is-unstamped', inspectDeclarationStamp(noDts, noDtsSpec).state, 'unstamped'); } finally { rmSync(tmp, { recursive: true, force: true }); } @@ -953,7 +927,7 @@ function selfTest() { console.error(''); return 1; } - console.log('✓ check:dev-prereqs --self-test — every verdict reachable, exclusions and freshness coverage pinned (16 cases), plus the shared workspace enumerator.'); + console.log('✓ check:dev-prereqs --self-test — every verdict reachable, exclusions and freshness coverage pinned (17 cases), plus the shared workspace enumerator.'); selfTestReachedVerdict = true; return 0; } diff --git a/scripts/check-regen-pending.d.mts b/scripts/check-regen-pending.d.mts index 21a9838761..3a1944d522 100644 --- a/scripts/check-regen-pending.d.mts +++ b/scripts/check-regen-pending.d.mts @@ -11,7 +11,7 @@ // guard exists to prevent — would type-check clean. // // Declared rather than inferred (no `allowJs`) because the module sits at the -// repo root, outside the consuming program's `rootDir`. The surface is three +// repo root, outside the consuming program's `rootDir`. The surface is four // functions with one optional argument; keep this file in step with them by // hand, and keep it small enough that doing so stays trivial. @@ -23,6 +23,23 @@ */ export function distIsStale(specDir?: string): boolean; +/** + * Did a declaration-emitting build produce `specDir/dist` from the sources on + * disk right now? `'match'` is the only verdict that clears an mtime + * accusation; `'unstamped'` is "no evidence" and covers every way of not being + * able to tell. `recorded` is the digest the stamp holds and `actual` the one + * the sources hash to now — both `null` when there is nothing to report. Read + * the function's own docblock before reusing it: it may only ever ACQUIT a tree + * `distIsStale` has already accused. + * + * @param specDir Absolute path to the spec package; defaults to this repo's. + */ +export function declarationStamp(specDir?: string): { + state: 'match' | 'mismatch' | 'unstamped'; + recorded: string | null; + actual: string | null; +}; + /** * Is `packages/spec/json-schema` older than the sources it was generated from? * Missing counts as stale. diff --git a/scripts/check-regen-pending.mjs b/scripts/check-regen-pending.mjs index b24d8a1393..992b5ff661 100755 --- a/scripts/check-regen-pending.mjs +++ b/scripts/check-regen-pending.mjs @@ -71,6 +71,7 @@ import { dirname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { PENDING_MARKER, entryForPath, ownerDir, ownerOf, ownerRunCommand } from './regen-artifacts.mjs'; +import { inspectDeclarationStamp } from './build-input-hash.mjs'; import { isEntrypoint } from './invoked-as.mjs'; import { EXIT_PREREQUISITE_NOT_MET, @@ -133,15 +134,79 @@ function newestMtime(dir, pred, depth = 0) { return newest; } +/** + * Did a declaration-emitting build produce `specDir/dist` from the sources on + * disk right now? `{ state, recorded, actual }` with state `match` / `mismatch` + * / `unstamped` — the reader's docblock in `check-dev-prereqs.mjs` is the + * authority on what each verdict means and why every way of not knowing + * collapses into `unstamped`. + * + * Re-exported through this module because `dist-freshness.ts` needs it to NAME + * the cause in a refusal and SHOW the two digests behind it, and this file is + * already the one place `packages/spec` gates import freshness from. Wrapped + * rather than re-exported raw so the repo root is supplied here — a caller that + * had to pass it could pass the wrong one, and hashing against the wrong root + * reads as `mismatch`, which is a refusal nobody can act on. + */ +export function declarationStamp(specDir = SPEC_DIR) { + try { + return inspectDeclarationStamp(REPO_ROOT, specDir); + } catch { + return { state: 'unstamped', recorded: null, actual: null }; + } +} + /** * Is `packages/spec/dist` older than the sources it claims to describe? Missing * counts as stale. Deliberately conservative: a false "stale" costs a build, a * false "fresh" costs a silently wrong artifact. + * + * ## The mtime rule accuses; the declaration stamp may acquit + * + * mtimes are the right primitive here and remain the only one that can convict: + * they key on `dist/**` + `.d.ts` against `src/**` + `.ts`, the artifact these + * gates actually consume, and they see the hand-edited dist and the toolchain + * change that a content digest is blind to. + * + * What they cannot see is that a rewrite left the BYTES alone. `git merge`, + * `git checkout` and `git worktree add` all re-check-out unchanged sources and + * bump their mtimes, and the build that follows correctly does not run — + * turbo's cache hashes content, so it is a cache hit (measured: `>>> FULL + * TURBO`, 56ms) that rewrites nothing, leaving every `dist/` mtime where the + * previous build left it. The tree is exactly current and this rule said stale: + * a refusal whose only remedy was a full rebuild — minutes — of a dist that was + * already correct, under a message naming a cause (`OS_SKIP_DTS=1`) that had + * nothing to do with it. + * + * So the accusation now gets one chance to be answered, by the one piece of + * evidence that speaks to exactly this: `dist/.build-input-hash-dts`, written by + * a build that DID emit declarations, over the digest of the inputs it emitted + * them from. Equal to the inputs on disk ⇒ the declarations describe these + * sources ⇒ fresh. + * + * ## Why this is not #7122's rejected direction + * + * #7122 proposed replacing this rule with `dist/.build-input-hash`, and that was + * measured to be wrong in the dangerous direction: that stamp is written by + * EVERY build, `OS_SKIP_DTS=1` included, so it says fresh on the one local flag + * that guarantees these declarations are stale. Two things keep this different: + * + * - the digest read here is a DIFFERENT file, written only by a build that + * emitted declarations. The `OS_SKIP_DTS=1` build does not refresh it, so + * the shape #7122 named still convicts (pinned in `dist-freshness.test.ts`); + * - it may only ever ACQUIT. `unstamped` — no stamp, unreadable, or a package + * whose build does not stamp at all — leaves the mtime verdict standing, so + * the change can only ever turn a refusal into a pass, never a pass into a + * refusal, and every acquittal carries a digest anyone can recompute. + * + * The digest costs ~30ms over `packages/spec/src`, and it is spent only on the + * branch that was previously about to cost a full rebuild. */ export function distIsStale(specDir = SPEC_DIR) { const dist = newestMtime(join(specDir, 'dist'), (n) => n.endsWith('.d.ts')); if (!dist) return true; - return newestMtime(join(specDir, 'src'), (n) => n.endsWith('.ts')) > dist; + if (newestMtime(join(specDir, 'src'), (n) => n.endsWith('.ts')) <= dist) return false; + return declarationStamp(specDir).state !== 'match'; } /**