chore: enforce sdk/dist freshness in gen-*.mjs + add staleness guard helper (#169)

* feat: add requireFreshDist helper for gen-*.mjs scripts

Adds sdk/scripts/_gen-helpers.mjs exporting requireFreshDist(distPath, tsSourcePath).
Performs a synchronous mtime check before any generator reads sdk/dist/ — if
the dist file is missing or older than its TS source, exits 1 with a clear
actionable error naming both paths, both mtimes, and the npm run build:sdk hint.

Single source of truth so all 9 generators import one function rather than
duplicating the logic. See issue #168.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* feat: enforce dist freshness in all 9 gen-*.mjs generators

Each generator now calls requireFreshDist() before reading sdk/dist/.
If the dist artifact is missing or stale relative to its TS source,
the generator exits 1 with a clear error instead of silently emitting
stale CJS output.

Addresses the PR #154 incident where an agent edited a TS source,
regenerated the CJS without rebuilding, and silently overwrote a fix.
gen-configuration.mjs also had its existing throw-on-missing guard
replaced with requireFreshDist() which additionally catches staleness.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test: add staleness-check regression for gen-*.mjs

tests/gen-staleness-check.test.cjs covers three cases for all 9 generators:
- exits 1 with "does not exist" + build hint when dist file is absent
- exits 1 with stale-dist error (paths + mtimes) when TS source is newer than dist
- exits 0 when dist is newer than TS source (skipped if sdk/dist not built)

Uses child_process.spawnSync to exercise the real gen-*.mjs entry path.
27/27 passing locally with sdk/dist present.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(gen-staleness-check): isolate dist-mutation tests with GSD_REPO_ROOT temp dirs

Subtests A and B in gen-staleness-check.test.cjs were renaming/creating real
sdk/dist files in the live repo tree while the suite ran at --test-concurrency=4.
Other parallel test processes (e.g. frontmatter-cli.test.cjs) spawned subprocesses
that attempted dynamic ESM import of sdk/dist/query/schema-detect.js while it was
temporarily absent, producing unhandled ENOENT failures unrelated to the staleness
guard logic under test.

Fix: add a GSD_REPO_ROOT env override to _gen-helpers.mjs so requireFreshDist()
resolves dist/ts paths against the provided root instead of the repo root derived
from import.meta.url. The test creates an isolated tmpdir tree per subtest (with
the real TS source copied in) and passes GSD_REPO_ROOT=<tmpdir> to the generator
subprocess, so no real dist files are ever touched during subtests A or B.

Subtest C (exits 0 on fresh dist) still uses the real repo tree because it needs
actual compiled output to exercise the generator end-to-end, but only sets the TS
source mtime (safe under concurrency) — it does not remove or rename any dist file.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-05-23 19:33:54 -04:00
committed by GitHub
parent 85c67b4b21
commit dde56a1b77
11 changed files with 311 additions and 5 deletions

View File

@@ -0,0 +1,83 @@
/**
* Shared helpers for gen-*.mjs generator scripts.
*
* Provides requireFreshDist(), a pre-generation guard that verifies the
* compiled sdk/dist artifact is newer than its TypeScript source. If the
* dist file is missing or stale, the function exits 1 with a clear,
* actionable error message so the developer knows to run `npm run build:sdk`.
*
* Single source of truth — import from here rather than duplicating the mtime
* logic in each generator.
*
* @example
* import { requireFreshDist } from './_gen-helpers.mjs';
* requireFreshDist('sdk/dist/query/secrets.js', 'sdk/src/query/secrets.ts');
*/
import { statSync, existsSync } from 'node:fs';
import { resolve } from 'node:path';
import { fileURLToPath } from 'node:url';
// Resolve the repo root from this file's location (sdk/scripts/ → repo root).
// GSD_REPO_ROOT env override allows tests to redirect to a temp directory so
// they can freely create/delete dist fixtures without mutating the real tree.
const REPO_ROOT = process.env.GSD_REPO_ROOT
? resolve(process.env.GSD_REPO_ROOT)
: resolve(fileURLToPath(import.meta.url), '..', '..', '..');
/**
* Assert that a compiled dist file is at least as new as its TypeScript source.
*
* Design notes:
* - Single Responsibility: each generator is responsible for verifying its
* own preconditions before touching dist. This keeps the guard co-located
* with the code that depends on it.
* - Composability: callers that have already built can call generators
* directly; the guard is a cheap mtime check, not a rebuild trigger.
* - Debuggability: the error message names both paths and both mtimes so the
* developer can see at a glance what is stale and what to do about it.
* - Performance: two synchronous statSync calls — effectively zero cost
* relative to the import/transform work the generator does next.
*
* NOT auto-building: auto-building inside the generator couples it to the
* build toolchain, bloats its dependency graph, and removes composability —
* a caller that already built cannot skip the redundant rebuild. The guard
* pattern is the correct separation: tell the developer what to do, not do it
* for them.
*
* @param {string} distPath - Repo-relative path to the compiled dist file,
* e.g. 'sdk/dist/query/secrets.js'
* @param {string} tsSourcePath - Repo-relative path to the TypeScript source
* file, e.g. 'sdk/src/query/secrets.ts'
*/
export function requireFreshDist(distPath, tsSourcePath) {
const distAbs = resolve(REPO_ROOT, distPath);
const tsAbs = resolve(REPO_ROOT, tsSourcePath);
if (!existsSync(distAbs)) {
console.error(
`ERROR: ${distPath} does not exist. Run \`npm run build:sdk\` first.`,
);
process.exit(1);
}
if (!existsSync(tsAbs)) {
console.error(
`ERROR: ${tsSourcePath} does not exist. Cannot verify dist freshness.`,
);
process.exit(1);
}
const distMtime = statSync(distAbs).mtimeMs;
const tsMtime = statSync(tsAbs).mtimeMs;
if (distMtime < tsMtime) {
console.error(
`ERROR: ${distPath} is stale relative to ${tsSourcePath} ` +
`(dist mtime ${new Date(distMtime).toISOString()}, ` +
`ts mtime ${new Date(tsMtime).toISOString()}). ` +
`Run \`npm run build:sdk\` first.`,
);
process.exit(1);
}
}

View File

@@ -15,6 +15,9 @@
import { existsSync, readFileSync, writeFileSync } from 'node:fs';
import { fileURLToPath } from 'node:url';
import { resolve, dirname } from 'node:path';
import { requireFreshDist } from './_gen-helpers.mjs';
requireFreshDist('sdk/dist/configuration/index.js', 'sdk/src/configuration/index.ts');
const here = dirname(fileURLToPath(import.meta.url));
const repoRoot = resolve(here, '..', '..');
@@ -22,11 +25,6 @@ const repoRoot = resolve(here, '..', '..');
// ─── Read the compiled dist file for function extraction ─────────────────────
const distPath = resolve(here, '..', 'dist', 'configuration', 'index.js');
if (!existsSync(distPath)) {
throw new Error(
`Missing compiled configuration module at ${distPath}. Run "cd sdk && npm run build" first.`,
);
}
const distSrc = readFileSync(distPath, 'utf-8');
/**

View File

@@ -14,6 +14,9 @@
import { readFile, writeFile } from 'node:fs/promises';
import { fileURLToPath } from 'node:url';
import { requireFreshDist } from './_gen-helpers.mjs';
requireFreshDist('sdk/dist/query/decisions.js', 'sdk/src/query/decisions.ts');
export const BANNER = `'use strict';

View File

@@ -12,6 +12,9 @@
import { writeFile } from 'node:fs/promises';
import { fileURLToPath } from 'node:url';
import { requireFreshDist } from './_gen-helpers.mjs';
requireFreshDist('sdk/dist/query/plan-scan.js', 'sdk/src/query/plan-scan.ts');
export const BANNER = `'use strict';

View File

@@ -12,6 +12,9 @@
import { writeFile } from 'node:fs/promises';
import { fileURLToPath } from 'node:url';
import { requireFreshDist } from './_gen-helpers.mjs';
requireFreshDist('sdk/dist/project-root/index.js', 'sdk/src/project-root/index.ts');
const BANNER = `'use strict';

View File

@@ -13,6 +13,9 @@
import { readFile, writeFile } from 'node:fs/promises';
import { fileURLToPath } from 'node:url';
import { requireFreshDist } from './_gen-helpers.mjs';
requireFreshDist('sdk/dist/query/schema-detect.js', 'sdk/src/query/schema-detect.ts');
export const BANNER = `'use strict';

View File

@@ -12,6 +12,9 @@
import { writeFile } from 'node:fs/promises';
import { fileURLToPath } from 'node:url';
import { requireFreshDist } from './_gen-helpers.mjs';
requireFreshDist('sdk/dist/query/secrets.js', 'sdk/src/query/secrets.ts');
export const BANNER = `'use strict';

View File

@@ -55,6 +55,9 @@
import { readFile, writeFile } from 'node:fs/promises';
import { fileURLToPath } from 'node:url';
import { requireFreshDist } from './_gen-helpers.mjs';
requireFreshDist('sdk/dist/query/validate.js', 'sdk/src/query/validate.ts');
export const BANNER = `'use strict';

View File

@@ -13,6 +13,9 @@
import { readFile, writeFile } from 'node:fs/promises';
import { fileURLToPath } from 'node:url';
import { requireFreshDist } from './_gen-helpers.mjs';
requireFreshDist('sdk/dist/workstream-inventory/builder.js', 'sdk/src/workstream-inventory/builder.ts');
export const BANNER = `'use strict';

View File

@@ -14,6 +14,9 @@
import { readFile, writeFile } from 'node:fs/promises';
import { fileURLToPath } from 'node:url';
import { requireFreshDist } from './_gen-helpers.mjs';
requireFreshDist('sdk/dist/workstream-name-policy.js', 'sdk/src/workstream-name-policy.ts');
export const BANNER = `'use strict';

View File

@@ -0,0 +1,201 @@
'use strict';
/**
* Regression tests for the requireFreshDist() staleness guard in gen-*.mjs scripts.
*
* For each generator, verifies:
* 1. Exits 1 when sdk/dist file does not exist — error includes "does not exist"
* and the `npm run build:sdk` hint.
* 2. Exits 1 when TS source is newer than sdk/dist — error includes
* "is stale relative to", both mtime timestamps, the TS path, and build hint.
* 3. Exits 0 when sdk/dist is newer than TS source (fresh build).
* Skipped per-generator if dist doesn't exist (expected before first build:sdk).
*
* Uses child_process.spawnSync to exercise the real gen-*.mjs entry path.
*
* Isolation: each subtest creates its own temp directory rooted at a unique
* path and passes GSD_REPO_ROOT to the generator subprocess so requireFreshDist()
* operates on temp fixtures instead of the real sdk/dist tree. This prevents
* parallel test execution from seeing stale/missing dist files in the live tree.
*/
const { describe, test } = require('node:test');
const assert = require('node:assert/strict');
const fs = require('node:fs');
const os = require('node:os');
const path = require('node:path');
const { spawnSync } = require('node:child_process');
const REPO_ROOT = path.resolve(__dirname, '..');
const SCRIPTS_DIR = path.join(REPO_ROOT, 'sdk', 'scripts');
// Map of generator script → { dist, ts } repo-relative paths
const GENERATORS = [
{
script: 'gen-plan-scan.mjs',
dist: 'sdk/dist/query/plan-scan.js',
ts: 'sdk/src/query/plan-scan.ts',
},
{
script: 'gen-secrets.mjs',
dist: 'sdk/dist/query/secrets.js',
ts: 'sdk/src/query/secrets.ts',
},
{
script: 'gen-schema-detect.mjs',
dist: 'sdk/dist/query/schema-detect.js',
ts: 'sdk/src/query/schema-detect.ts',
},
{
script: 'gen-decisions.mjs',
dist: 'sdk/dist/query/decisions.js',
ts: 'sdk/src/query/decisions.ts',
},
{
script: 'gen-project-root.mjs',
dist: 'sdk/dist/project-root/index.js',
ts: 'sdk/src/project-root/index.ts',
},
{
script: 'gen-workstream-inventory-builder.mjs',
dist: 'sdk/dist/workstream-inventory/builder.js',
ts: 'sdk/src/workstream-inventory/builder.ts',
},
{
script: 'gen-workstream-name-policy.mjs',
dist: 'sdk/dist/workstream-name-policy.js',
ts: 'sdk/src/workstream-name-policy.ts',
},
{
script: 'gen-validate.mjs',
dist: 'sdk/dist/query/validate.js',
ts: 'sdk/src/query/validate.ts',
},
{
script: 'gen-configuration.mjs',
dist: 'sdk/dist/configuration/index.js',
ts: 'sdk/src/configuration/index.ts',
},
];
/**
* Run a gen script via spawnSync with an optional GSD_REPO_ROOT override.
* Returns { status, stderr, stdout }.
*/
function runGen(scriptName, env = {}) {
const scriptPath = path.join(SCRIPTS_DIR, scriptName);
const result = spawnSync(process.execPath, [scriptPath], {
cwd: REPO_ROOT,
encoding: 'utf-8',
timeout: 15000,
env: { ...process.env, ...env },
});
return {
status: result.status ?? 1,
stdout: result.stdout || '',
stderr: result.stderr || '',
};
}
/**
* Create a minimal temp directory tree that mirrors the repo layout for
* the given dist and ts paths. Returns { tmpRoot, distAbs, tsAbs, cleanup }.
*
* The real TS source is copied into the temp tree so its content is valid,
* but the caller controls whether the dist file exists and what its mtime is.
*/
function makeTempTree(dist, ts) {
const tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-staleness-'));
const distAbs = path.join(tmpRoot, dist);
const tsAbs = path.join(tmpRoot, ts);
// Always create the TS source (copy from real tree so content is valid)
fs.mkdirSync(path.dirname(tsAbs), { recursive: true });
const realTsAbs = path.join(REPO_ROOT, ts);
fs.copyFileSync(realTsAbs, tsAbs);
function cleanup() {
try { fs.rmSync(tmpRoot, { recursive: true, force: true }); } catch { /* ignore */ }
}
return { tmpRoot, distAbs, tsAbs, cleanup };
}
// One describe block per generator — each block is { concurrency: false } so
// its three subtests run in order and don't race on the same files.
for (const { script, dist, ts } of GENERATORS) {
describe(`gen staleness guard — ${script}`, { concurrency: false }, () => {
// ── Subtest A: dist file is missing ──────────────────────────────────────
test('exits 1 with "does not exist" when dist file is absent', () => {
const { tmpRoot, cleanup } = makeTempTree(dist, ts);
// dist is NOT created — the temp tree has only the TS source
try {
const { status, stderr } = runGen(script, { GSD_REPO_ROOT: tmpRoot });
assert.strictEqual(status, 1, `Expected exit 1, got ${status}. stderr: ${stderr}`);
assert.match(stderr, /does not exist/, `Expected "does not exist". Got: ${stderr}`);
assert.match(stderr, /npm run build/, `Expected build hint. Got: ${stderr}`);
// Error message should name the dist path (repo-relative)
assert.ok(stderr.includes(dist), `Expected dist path in message. Got: ${stderr}`);
} finally {
cleanup();
}
});
// ── Subtest B: TS source newer than dist ─────────────────────────────────
test('exits 1 with stale-dist error when TS source is newer than dist', () => {
const { tmpRoot, distAbs, tsAbs, cleanup } = makeTempTree(dist, ts);
// Create a fake dist file
fs.mkdirSync(path.dirname(distAbs), { recursive: true });
fs.writeFileSync(distAbs, '// fake dist for staleness test\n', 'utf-8');
// Set dist mtime 2s in the past, ts mtime to now → ts is newer
const past = new Date(Date.now() - 2000);
const now = new Date();
fs.utimesSync(distAbs, past, past);
fs.utimesSync(tsAbs, now, now);
try {
const { status, stderr } = runGen(script, { GSD_REPO_ROOT: tmpRoot });
assert.strictEqual(status, 1, `Expected exit 1 for stale dist, got ${status}. stderr: ${stderr}`);
assert.match(stderr, /is stale relative to/, `Expected "is stale relative to". Got: ${stderr}`);
assert.match(stderr, /npm run build/, `Expected build hint. Got: ${stderr}`);
// Error must name the TS source path
assert.ok(stderr.includes(ts), `Expected TS path "${ts}" in message. Got: ${stderr}`);
// Error must include both mtime timestamps in ISO format
assert.match(stderr, /dist mtime \d{4}-\d{2}-\d{2}T/, `Expected dist mtime in message. Got: ${stderr}`);
assert.match(stderr, /ts mtime \d{4}-\d{2}-\d{2}T/, `Expected ts mtime in message. Got: ${stderr}`);
} finally {
cleanup();
}
});
// ── Subtest C: dist is fresh → exits 0 ───────────────────────────────────
// This subtest requires a real built dist (the generator reads and transforms
// its content), so it uses the actual sdk/dist tree rather than the temp dir.
// It only mutates the real TS source mtime, not the dist file, so it is safe
// to run in parallel: another parallel test process reading sdk/dist will not
// be affected by a TS source mtime change.
test('exits 0 when dist is newer than TS source', { skip: !fs.existsSync(path.join(REPO_ROOT, dist)) ? `${dist} not built` : false }, () => {
const distAbs = path.join(REPO_ROOT, dist);
const tsAbs = path.join(REPO_ROOT, ts);
const distStat = fs.statSync(distAbs);
const origTsStat = fs.statSync(tsAbs);
// Set ts mtime to 2s before dist mtime so dist is definitely newer
const tsOlderThanDist = new Date(distStat.mtimeMs - 2000);
fs.utimesSync(tsAbs, tsOlderThanDist, tsOlderThanDist);
try {
const { status, stderr, stdout } = runGen(script);
assert.strictEqual(
status,
0,
`Expected exit 0 for fresh dist, got ${status}. stderr: ${stderr}\nstdout: ${stdout}`,
);
} finally {
fs.utimesSync(tsAbs, origTsStat.atime, origTsStat.mtime);
}
});
});
}