Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .changeset/dist-freshness-declaration-stamp.md
Original file line number Diff line number Diff line change
@@ -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.
6 changes: 4 additions & 2 deletions packages/spec/scripts/build-api-surface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
94 changes: 93 additions & 1 deletion packages/spec/scripts/dist-freshness.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, '..');
Expand All @@ -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`
Expand Down Expand Up @@ -210,17 +232,87 @@ 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);
if (verdict.fresh) return;
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.
Expand Down
97 changes: 72 additions & 25 deletions packages/spec/scripts/lib/dist-freshness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
*
Expand All @@ -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';
Expand Down Expand Up @@ -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,
Expand All @@ -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.)`,
};
}

Expand Down
Loading
Loading