Skip to content

Commit 6971170

Browse files
claude[bot]claude
andauthored
fix(tooling): the bundle freshness rule can be answered — the build stamp acquits a tree whose bytes never moved (#16240)
* fix(tooling): the bundle freshness rule can be answered — the build stamp acquits a tree whose sources were re-checked-out unchanged `bundlesAreStale` compared `dist/**/*.mjs`/`*.js` mtimes against `src/**/*.ts` and `tsup.config.ts` mtimes and had no way to be answered. A `git merge`, `git checkout` or `git worktree add` re-checks-out a source file with identical bytes and bumps its mtime; the build that follows correctly does not run (turbo's cache hashes content, so it is a cache hit that rewrites nothing) and every `dist/` mtime stays where the previous build left it. The rule then refused `check:browser-reachable-entries` over bundles that were exactly current, and the only remedy was a multi-minute rebuild under the shared verify lock. The mtime rule keeps its power to convict and gains one way to be answered: `dist/.build-input-hash`, already written by every build of the package. That is the file #7122 proposed for the DECLARATION rule, where it was measured wrong and stays rejected — `OS_SKIP_DTS=1` writes it while skipping the declarations. On the bundle axis that same flag emits exactly the artifact being vouched for, and what makes the evidence sound is the build script's order rather than the flag: the unconditional `tsup` pass runs before `--stamp` in one `&&` chain, so nothing writes this stamp without having emitted bundles first. 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. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8 * test: pin the bundle-stamp acquittal and the two cases that keep it non-vacuous Three cases in `check:browser-reachable-entries --self-test`, beside the mtime cases that already drive `inspectBundleFreshness`: the acquittal itself, the conviction that returns the moment a source byte changes (so the rule is keyed on the digest and not on the stamp's presence), and a stamp that is not a digest at all leaving the refusal standing. A fourth asserts the sandbox digest can be computed, so a fixture that silently stopped producing one cannot pass the rest by accident. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vbw3RPgdtqesx4azk9SbW8 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7d711c9 commit 6971170

4 files changed

Lines changed: 219 additions & 19 deletions

File tree

packages/spec/scripts/check-browser-reachable-entries.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ import { tmpdir } from 'node:os';
152152
import { dirname, join, relative, resolve } from 'node:path';
153153
import { fileURLToPath } from 'node:url';
154154

155+
import { buildStamp } from '../../../scripts/check-regen-pending.mjs';
155156
import { scanSource } from '../../../scripts/js-comment-mask.mjs';
156157
import { inspectBundleFreshness } from './lib/dist-freshness';
157158

@@ -927,6 +928,71 @@ function selfTest(): never {
927928
!inspectBundleFreshness(fresh, 'check', RERUN).fresh,
928929
mtimes(),
929930
);
931+
932+
// ── The acquittal: an mtime accusation that CAN be answered (#16175) ────
933+
// Every case above is the mtime rule convicting, and until #16175 there was
934+
// nothing it would accept as an answer. So it also refused the tree a
935+
// `git merge`, `git checkout` or `git worktree add` leaves behind: those
936+
// re-check-out an UNCHANGED source file — bytes identical, mtime bumped —
937+
// and the build that follows is a turbo cache hit that rewrites nothing, so
938+
// every dist/ mtime stays where the previous build left it. Measured on the
939+
// real tree: `touch packages/spec/src/data/query.zod.ts` with `git status`
940+
// empty made this gate exit 1 and prescribe a multi-minute rebuild of
941+
// bundles that were already exactly current.
942+
//
943+
// The evidence is `dist/.build-input-hash`, which every build of the package
944+
// writes AFTER its unconditional `tsup` pass. It may only ever ACQUIT, and
945+
// the two cases after this one are what keep that non-vacuous.
946+
const stampFile = join(fresh, 'dist', '.build-input-hash');
947+
// Two steps, for the reason `dist-freshness.test.ts` gives: `buildStamp`
948+
// computes `actual` only when a syntactically valid digest is recorded, so
949+
// seed a placeholder, read what the sources really hash to, then write that.
950+
// Asked of the rule's own reader rather than hardcoded — the input set
951+
// includes turbo.json's globalDependencies and every input's repo-relative
952+
// path, so a literal here would rot into a `mismatch` that reads exactly
953+
// like the refusal these cases exist to tell apart. The stamp is written
954+
// last, so it records the sandbox as it now stands, tsup.config.ts included
955+
// (that file is in the digest's input set as well as in the mtime rule's).
956+
writeFileSync(stampFile, `${'0'.repeat(64)}\n`);
957+
const digest = buildStamp(fresh).actual;
958+
check(
959+
'the sandbox build-input digest can be computed at all (the fixture is not vacuous)',
960+
typeof digest === 'string' && /^[0-9a-f]{64}$/.test(digest),
961+
JSON.stringify(digest),
962+
);
963+
writeFileSync(stampFile, `${digest}\n`);
964+
check(
965+
'ACQUITS an mtime-stale tree whose build stamp matches the sources (#16175)',
966+
inspectBundleFreshness(fresh, 'check', RERUN).fresh,
967+
mtimes(),
968+
);
969+
970+
// The half that makes the case above non-vacuous. If the acquittal were
971+
// keyed on the stamp's mere PRESENCE rather than on the digest, this would
972+
// stay green — and that is #7122's false green restored, one axis over. The
973+
// stamp is written first and the source edited after, so the recorded digest
974+
// is genuinely stale rather than never-valid.
975+
writeFileSync(srcFile, 'export const a = 2;\n');
976+
stamp(srcFile, 0);
977+
check(
978+
'and CONVICTS the same tree the moment a source byte actually changes',
979+
!inspectBundleFreshness(fresh, 'check', RERUN).fresh,
980+
mtimes(),
981+
);
982+
983+
// Absence of the freshness input is not licence to acquit (#4690), and
984+
// neither is a truncated or half-flushed write. Anything that is not 64 hex
985+
// characters is `unstamped`, which leaves the mtime verdict standing. The
986+
// source is restored to the bytes the digest above was taken over first, so
987+
// the ONLY reason this refuses is the stamp itself.
988+
writeFileSync(srcFile, 'export const a = 1;\n');
989+
stamp(srcFile, 0);
990+
writeFileSync(stampFile, 'not-a-digest\n');
991+
check(
992+
'ignores a build stamp that is not a digest at all',
993+
!inspectBundleFreshness(fresh, 'check', RERUN).fresh,
994+
mtimes(),
995+
);
930996
} finally {
931997
rmSync(fresh, { recursive: true, force: true });
932998
}

scripts/build-input-hash.mjs

Lines changed: 78 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,29 @@
3434
*
3535
* Both hold the SAME digest over the SAME inputs. The difference is which build
3636
* writes them, and that difference is the whole reason the second one exists —
37-
* see each constant's docblock, and `inspectDeclarationStamp` for the reader.
37+
* see each constant's docblock, and the two `inspect*Stamp` readers at the
38+
* bottom for what each stamp may and may not be believed about.
3839
*/
3940
import { createHash } from 'node:crypto';
4041
import { existsSync, readdirSync, readFileSync } from 'node:fs';
4142
import path from 'node:path';
4243

43-
/** Where a build records the hash of the inputs it was built from. */
44+
/**
45+
* Where a build records the hash of the inputs it was built from.
46+
*
47+
* Written by `--stamp` at the end of EVERY build of an amplifier package,
48+
* `OS_SKIP_DTS=1` included. Two consumers read it, and the difference between
49+
* what they may conclude is the whole reason the sibling stamp below exists:
50+
*
51+
* - `check-dev-prereqs.mjs`'s boot gate, for which "this dist was built from
52+
* these sources" is the whole question (#5864);
53+
* - `bundlesAreStale` in scripts/check-regen-pending.mjs, for which it is
54+
* evidence about the emitted `.mjs`/`.js` ONLY — see `inspectBuildStamp`.
55+
*
56+
* ⛔ It says NOTHING about `dist/**\/*.d.ts`: the flag it is written under can
57+
* skip the declaration pass entirely. That is #7122's rejected direction and it
58+
* stays rejected; DTS_STAMP_BASENAME is the file that answers for those.
59+
*/
4460
export const STAMP_BASENAME = '.build-input-hash';
4561

4662
/**
@@ -166,28 +182,28 @@ export function buildInputHash(root, pkgDir) {
166182
}
167183

168184
/**
169-
* Did a declaration-emitting build produce THIS dist from THESE sources?
170-
*
171-
* The reader for DTS_STAMP_BASENAME, exported because the caller that needs it
172-
* is `distIsStale` in scripts/check-regen-pending.mjs — and it has to be THIS
173-
* function over THIS hash, or the comparison means nothing (the same argument
174-
* that keeps `--stamp` in this file rather than in a script of its own).
185+
* Read ONE of the two stamps and say what it vouches for. Shared by both
186+
* readers below, because "the stamp and the reader must compute the same
187+
* digest" is exactly as load-bearing between the two stamps as it is between a
188+
* stamp and its reader: two copies of this comparison would drift, and the
189+
* direction drift takes is the one that acquits.
175190
*
176191
* Three verdicts, and the asymmetry between them is deliberate:
177192
*
178193
* - `match` the recorded digest equals the inputs on disk right now, so
179-
* the declarations describe exactly these sources. This is the
180-
* ONLY verdict that may clear an mtime accusation.
181-
* - `mismatch` a declaration-emitting build ran, and the sources have moved
182-
* since. Nameable in a refusal message: this is not an mtime
183-
* artefact, the content really did change.
194+
* the artifact this stamp speaks for was emitted from exactly
195+
* these sources. This is the ONLY verdict that may clear an
196+
* mtime accusation.
197+
* - `mismatch` a build that writes this stamp ran, and the sources have
198+
* moved since. Nameable in a refusal message: this is not an
199+
* mtime artefact, the content really did change.
184200
* - `unstamped` NO EVIDENCE — no stamp, an unreadable one, a digest that
185201
* cannot be computed, or a package whose build does not stamp
186202
* at all. Every one of those collapses to the same answer on
187203
* purpose: "cannot vouch" must never read as "vouched for"
188204
* (#4690), and absence of the input is not licence to acquit.
189205
*
190-
* It never throws: it is called from inside a freshness predicate whose failure
206+
* It never throws: it is called from inside freshness predicates whose failure
191207
* direction is a silently wrong artifact, so an unreadable tree has to degrade
192208
* to `unstamped` rather than take the caller down.
193209
*
@@ -197,9 +213,9 @@ export function buildInputHash(root, pkgDir) {
197213
* `actual` is computed only when there is a valid digest to compare it against,
198214
* so the ~30ms hash stays off the path where no amplifier stamp exists at all.
199215
*/
200-
export function inspectDeclarationStamp(root, pkgDir) {
216+
function inspectStamp(root, pkgDir, basename) {
201217
const none = { state: 'unstamped', recorded: null, actual: null };
202-
const stampFile = path.join(pkgDir, 'dist', DTS_STAMP_BASENAME);
218+
const stampFile = path.join(pkgDir, 'dist', basename);
203219
let recorded;
204220
try {
205221
if (!existsSync(stampFile)) return none;
@@ -216,3 +232,49 @@ export function inspectDeclarationStamp(root, pkgDir) {
216232
}
217233
return { state: recorded === actual ? 'match' : 'mismatch', recorded, actual };
218234
}
235+
236+
/**
237+
* Did a DECLARATION-emitting build produce THIS dist from THESE sources?
238+
*
239+
* The reader for DTS_STAMP_BASENAME, exported because the caller that needs it
240+
* is `distIsStale` in scripts/check-regen-pending.mjs — and it has to be THIS
241+
* function over THIS hash, or the comparison means nothing (the same argument
242+
* that keeps `--stamp` in this file rather than in a script of its own).
243+
*/
244+
export function inspectDeclarationStamp(root, pkgDir) {
245+
return inspectStamp(root, pkgDir, DTS_STAMP_BASENAME);
246+
}
247+
248+
/**
249+
* Did a BUNDLE-emitting build produce THIS dist from THESE sources?
250+
*
251+
* The reader for STAMP_BASENAME, exported for `bundlesAreStale` in
252+
* scripts/check-regen-pending.mjs, which measures `dist/**\/*.mjs` and `*.js`
253+
* — a different artifact from the declarations, produced by a different pass.
254+
*
255+
* ## Why reading STAMP_BASENAME is sound HERE and stays rejected next door
256+
*
257+
* #7122 proposed answering the DECLARATION rule with this stamp, and that was
258+
* measured wrong in the dangerous direction: `--stamp` writes this file under
259+
* `OS_SKIP_DTS=1`, the one build flag that emits JS and leaves whatever `.d.ts`
260+
* was there before — so it says fresh over stale declarations. That rejection
261+
* is pinned (`packages/spec/scripts/dist-freshness.test.ts`) and unchanged.
262+
*
263+
* On the BUNDLE axis the same fact points the other way. `OS_SKIP_DTS=1` emits
264+
* every bundle this stamp would then vouch for, so the case that ruled the file
265+
* out for declarations is not a hole here at all — it is the ordinary case.
266+
* What makes the vouching sound is the build script's ORDER, not the flag:
267+
* `packages/spec`'s `build` runs the unconditional `tsup` (the JS pass) before
268+
* `--stamp` in the same `&&` chain, so this file is never written by a run that
269+
* did not emit bundles. The declaration pass that follows is the one that can be
270+
* skipped, and skipping it cannot refresh this stamp because the stamp is
271+
* written after both either way.
272+
*
273+
* The same one-way property still governs: it may only ever ACQUIT a tree the
274+
* mtime rule has already accused, never accuse one it cleared. The digest still
275+
* cannot see a hand-edited dist, a toolchain change or dependency drift — the
276+
* mtime rule remains the only thing that convicts.
277+
*/
278+
export function inspectBuildStamp(root, pkgDir) {
279+
return inspectStamp(root, pkgDir, STAMP_BASENAME);
280+
}

scripts/check-regen-pending.d.mts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
// guard exists to prevent — would type-check clean.
1212
//
1313
// Declared rather than inferred (no `allowJs`) because the module sits at the
14-
// repo root, outside the consuming program's `rootDir`. The surface is four
14+
// repo root, outside the consuming program's `rootDir`. The surface is five
1515
// functions with one optional argument; keep this file in step with them by
1616
// hand, and keep it small enough that doing so stays trivial.
1717

@@ -61,3 +61,20 @@ export function schemaTreeIsStale(specDir?: string): boolean;
6161
* @param specDir Absolute path to the spec package; defaults to this repo's.
6262
*/
6363
export function bundlesAreStale(specDir?: string): boolean;
64+
65+
/**
66+
* Did a bundle-emitting build produce `specDir/dist`'s `.mjs`/`.js` from the
67+
* sources on disk right now? The bundle axis's counterpart to
68+
* `declarationStamp`, reading the OTHER stamp file (`dist/.build-input-hash`,
69+
* written by every build). `'match'` is the only verdict that clears an mtime
70+
* accusation; `'unstamped'` is "no evidence". Read the function's own docblock
71+
* before reusing it — the same file is deliberately NOT evidence about
72+
* `dist/**\/*.d.ts`.
73+
*
74+
* @param specDir Absolute path to the spec package; defaults to this repo's.
75+
*/
76+
export function buildStamp(specDir?: string): {
77+
state: 'match' | 'mismatch' | 'unstamped';
78+
recorded: string | null;
79+
actual: string | null;
80+
};

scripts/check-regen-pending.mjs

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ import { dirname, join, resolve } from 'node:path';
7171
import { fileURLToPath } from 'node:url';
7272

7373
import { PENDING_MARKER, entryForPath, ownerDir, ownerOf, ownerRunCommand } from './regen-artifacts.mjs';
74-
import { inspectDeclarationStamp } from './build-input-hash.mjs';
74+
import { inspectBuildStamp, inspectDeclarationStamp } from './build-input-hash.mjs';
7575
import { isEntrypoint } from './invoked-as.mjs';
7676
import {
7777
EXIT_PREREQUISITE_NOT_MET,
@@ -156,6 +156,24 @@ export function declarationStamp(specDir = SPEC_DIR) {
156156
}
157157
}
158158

159+
/**
160+
* Did a BUNDLE-emitting build produce `specDir/dist`'s `.mjs`/`.js` from the
161+
* sources on disk right now? Same three verdicts, same one-way meaning, same
162+
* wrapping argument as `declarationStamp` above — the repo root is supplied
163+
* here so no caller can hash against the wrong one.
164+
*
165+
* The OTHER stamp file: `dist/.build-input-hash`, which every build writes.
166+
* `inspectBuildStamp`'s docblock is the authority on why that is the right
167+
* evidence for THIS axis and remains the wrong evidence for the declarations.
168+
*/
169+
export function buildStamp(specDir = SPEC_DIR) {
170+
try {
171+
return inspectBuildStamp(REPO_ROOT, specDir);
172+
} catch {
173+
return { state: 'unstamped', recorded: null, actual: null };
174+
}
175+
}
176+
159177
/**
160178
* Is `packages/spec/dist` older than the sources it claims to describe? Missing
161179
* counts as stale. Deliberately conservative: a false "stale" costs a build, a
@@ -276,6 +294,42 @@ export function schemaTreeIsStale(specDir = SPEC_DIR) {
276294
* Missing counts as stale, and the direction is the same conservative one its
277295
* two siblings take: a false "stale" costs a build, a false "fresh" costs a
278296
* verdict nobody can trust.
297+
*
298+
* ## The mtime rule accuses; the BUILD stamp may acquit
299+
*
300+
* The blind spot `distIsStale` documents above is shared here, and was measured
301+
* on this axis too: after a `git merge`, `git checkout` or `git worktree add`
302+
* re-checks-out a source file with IDENTICAL bytes, the build that follows
303+
* correctly does not run (turbo's cache hashes content, so it is a cache hit
304+
* that rewrites nothing) and every `dist/` mtime stays put — so this rule
305+
* refused `check:browser-reachable-entries` over bundles that were exactly
306+
* current, and the only remedy on offer was a multi-minute rebuild under the
307+
* shared verify lock.
308+
*
309+
* The evidence that answers it is `dist/.build-input-hash` — the file #7122
310+
* proposed for the DECLARATION rule, where it was measured wrong and stays
311+
* rejected. It is the right file HERE for a reason that is specific to this
312+
* axis, not a relaxation of that ruling:
313+
*
314+
* - #7122's hole is that `--stamp` writes this file under `OS_SKIP_DTS=1`,
315+
* which emits JS and skips the declarations. On this axis that flag emits
316+
* exactly the artifact being vouched for — `inspectBundleFreshness`'s own
317+
* refusal already tells the reader "OS_SKIP_DTS=1 is fine for THIS gate";
318+
* - what makes it sound is the build script's ORDER rather than the flag:
319+
* `packages/spec`'s `build` runs the unconditional `tsup` before `--stamp`
320+
* in one `&&` chain, so nothing writes this stamp without having emitted
321+
* bundles first. Only the declaration pass is conditional, and skipping it
322+
* cannot refresh a stamp written after both;
323+
* - the digest's input set is a strict SUPERSET of the source set measured
324+
* above — every file under `src/` (`.test.ts` included) plus
325+
* `tsup.config.ts` and the rest of `PACKAGE_BUILD_CONFIG` plus turbo's
326+
* `globalDependencies` — so a `match` implies every input this rule counts
327+
* is byte-identical to the one the bundles were emitted from. A superset
328+
* can only ever withhold an acquittal, never grant one it should not.
329+
*
330+
* And it may only ACQUIT. `unstamped` — absent, unreadable, not 64 hex
331+
* characters, or a package whose build does not stamp — leaves the mtime
332+
* verdict standing (#4690), so nothing that passes today can start failing.
279333
*/
280334
export function bundlesAreStale(specDir = SPEC_DIR) {
281335
const bundles = newestMtime(
@@ -289,7 +343,8 @@ export function bundlesAreStale(specDir = SPEC_DIR) {
289343
);
290344
const configPath = join(specDir, 'tsup.config.ts');
291345
const config = existsSync(configPath) ? statSync(configPath).mtimeMs : 0;
292-
return Math.max(src, config) > bundles;
346+
if (Math.max(src, config) <= bundles) return false;
347+
return buildStamp(specDir).state !== 'match';
293348
}
294349

295350
/**

0 commit comments

Comments
 (0)