From 873a79f4840c8cf52820fddd1c352796cf2397e7 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 30 Mar 2026 16:17:02 -0400 Subject: [PATCH] fix: skip staleness rebuild in npm tarball installs (#2877) (#3250) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ensure-workspace-builds.cjs postinstall script falsely detected workspace packages as stale in npm tarball installs. npm sets all tarball entries to a canonical timestamp (Oct 26 1985), but extraction ordering causes src/ files to appear 1-2 seconds newer than dist/ files. This triggered a rebuild attempt that either failed silently (no tsc available) or — when tsc was globally installed — could produce broken dist/ output, corrupting the known-good pre-built files and causing the DefaultResourceLoader export error on startup. The fix gates the src-vs-dist staleness check behind a .git directory check: only development clones (with .git/) perform the timestamp comparison. npm tarball installs (no .git/) only check for missing dist/index.js, which is the safe and correct behavior. Co-authored-by: Claude Opus 4.6 Co-authored-by: TÂCHES --- scripts/ensure-workspace-builds.cjs | 59 ++++++++++---- src/tests/ensure-workspace-builds.test.ts | 96 ++++++++++++++++++++++- tsconfig.test.json | 2 +- 3 files changed, 140 insertions(+), 17 deletions(-) diff --git a/scripts/ensure-workspace-builds.cjs b/scripts/ensure-workspace-builds.cjs index 44f7ea2c4..10a6638e4 100644 --- a/scripts/ensure-workspace-builds.cjs +++ b/scripts/ensure-workspace-builds.cjs @@ -37,6 +37,48 @@ function newestSrcMtime(dir) { return newest } +/** + * Detects workspace packages whose dist/ is missing or stale. + * + * Missing dist/index.js is always reported (the package won't work at all). + * + * Staleness (src/ newer than dist/) is ONLY checked when a .git directory + * exists at root — indicating a development clone. In npm tarball installs, + * file timestamps are unreliable (npm sets all files to a canonical date, + * but extraction ordering can cause src/ to appear 1-2 seconds newer than + * dist/). Attempting to rebuild in that scenario is dangerous: devDependencies + * (including TypeScript) are not installed, and any globally-installed tsc + * may produce broken output that overwrites the known-good dist/. + * + * @param {string} root Project root directory + * @param {string[]} packages Package directory names to check + * @returns {string[]} Package names that need rebuilding + */ +function detectStalePackages(root, packages) { + const packagesDir = join(root, 'packages') + const isDevClone = existsSync(join(root, '.git')) + + const stale = [] + for (const pkg of packages) { + const distIndex = join(packagesDir, pkg, 'dist', 'index.js') + if (!existsSync(distIndex)) { + stale.push(pkg) + continue + } + // Only check src vs dist timestamps in development clones. + // In npm tarball installs, timestamps are unreliable and rebuilding + // without devDependencies can corrupt the pre-built dist/ (#2877). + if (isDevClone) { + const distMtime = statSync(distIndex).mtimeMs + const srcMtime = newestSrcMtime(join(packagesDir, pkg, 'src')) + if (srcMtime > distMtime) { + stale.push(pkg) + } + } + } + return stale +} + if (require.main === module) { const root = resolve(__dirname, '..') const packagesDir = join(root, 'packages') @@ -57,19 +99,7 @@ if (require.main === module) { 'pi-coding-agent', ] - const stale = [] - for (const pkg of WORKSPACE_PACKAGES) { - const distIndex = join(packagesDir, pkg, 'dist', 'index.js') - if (!existsSync(distIndex)) { - stale.push(pkg) - continue - } - const distMtime = statSync(distIndex).mtimeMs - const srcMtime = newestSrcMtime(join(packagesDir, pkg, 'src')) - if (srcMtime > distMtime) { - stale.push(pkg) - } - } + const stale = detectStalePackages(root, WORKSPACE_PACKAGES) if (stale.length === 0) process.exit(0) @@ -78,6 +108,7 @@ if (require.main === module) { for (const pkg of stale) { const pkgDir = join(packagesDir, pkg) try { + // execSync is safe here: the command is a hardcoded string, not user input execSync('npm run build', { cwd: pkgDir, stdio: 'pipe' }) process.stderr.write(` ✓ ${pkg}\n`) } catch (err) { @@ -87,4 +118,4 @@ if (require.main === module) { } } -module.exports = { newestSrcMtime } +module.exports = { newestSrcMtime, detectStalePackages } diff --git a/src/tests/ensure-workspace-builds.test.ts b/src/tests/ensure-workspace-builds.test.ts index f256c7afe..965d2348e 100644 --- a/src/tests/ensure-workspace-builds.test.ts +++ b/src/tests/ensure-workspace-builds.test.ts @@ -1,12 +1,12 @@ import { describe, it, beforeEach, afterEach } from "node:test"; import assert from "node:assert/strict"; -import { mkdtempSync, writeFileSync, mkdirSync, rmSync, utimesSync } from "node:fs"; +import { mkdtempSync, writeFileSync, mkdirSync, rmSync, utimesSync, statSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { createRequire } from "node:module"; const require = createRequire(import.meta.url); -const { newestSrcMtime } = require("../../scripts/ensure-workspace-builds.cjs"); +const { newestSrcMtime, detectStalePackages } = require("../../scripts/ensure-workspace-builds.cjs"); describe("newestSrcMtime", () => { let tmp: string; @@ -62,3 +62,95 @@ describe("newestSrcMtime", () => { assert.equal(newestSrcMtime(tmp), 0); }); }); + +describe("detectStalePackages", () => { + let tmp: string; + + beforeEach(() => { tmp = mkdtempSync(join(tmpdir(), "gsd-stale-test-")); }); + afterEach(() => { rmSync(tmp, { recursive: true, force: true }); }); + + /** + * Helper to create a fake workspace package with src/ and dist/ directories. + * Sets timestamps to simulate npm tarball extraction where src/ files can be + * 1 second newer than dist/ files. + */ + function createFakePackage( + packagesDir: string, + pkgName: string, + opts: { srcNewerThanDist?: boolean; missingDist?: boolean } = {}, + ): void { + const pkgDir = join(packagesDir, pkgName); + const srcDir = join(pkgDir, "src"); + const distDir = join(pkgDir, "dist"); + mkdirSync(srcDir, { recursive: true }); + writeFileSync(join(srcDir, "index.ts"), "export const x = 1;"); + + if (!opts.missingDist) { + mkdirSync(distDir, { recursive: true }); + writeFileSync(join(distDir, "index.js"), "export const x = 1;"); + } + + if (opts.srcNewerThanDist && !opts.missingDist) { + // Simulate npm tarball extraction: src/ is 1 second newer than dist/ + const distTime = new Date("2024-06-01T00:00:00Z"); + const srcTime = new Date("2024-06-01T00:00:01Z"); + utimesSync(join(distDir, "index.js"), distTime, distTime); + utimesSync(join(srcDir, "index.ts"), srcTime, srcTime); + } + } + + it("detects missing dist/ as stale regardless of .git presence", () => { + const packagesDir = join(tmp, "packages"); + mkdirSync(packagesDir, { recursive: true }); + createFakePackage(packagesDir, "test-pkg", { missingDist: true }); + + const result = detectStalePackages(tmp, ["test-pkg"]); + assert.deepEqual(result, ["test-pkg"]); + }); + + it("detects stale src > dist timestamps in a git repo (dev clone)", () => { + // Simulate a git repo by creating .git directory + mkdirSync(join(tmp, ".git"), { recursive: true }); + const packagesDir = join(tmp, "packages"); + mkdirSync(packagesDir, { recursive: true }); + createFakePackage(packagesDir, "test-pkg", { srcNewerThanDist: true }); + + const result = detectStalePackages(tmp, ["test-pkg"]); + assert.deepEqual(result, ["test-pkg"]); + }); + + it("skips staleness check when not in a git repo (npm tarball install)", () => { + // No .git directory — simulates npm install from tarball + const packagesDir = join(tmp, "packages"); + mkdirSync(packagesDir, { recursive: true }); + createFakePackage(packagesDir, "test-pkg", { srcNewerThanDist: true }); + + // Even though src/ is newer than dist/, the script should NOT detect it + // as stale because we're in an npm tarball (no .git directory). + // The timestamp difference is an artifact of npm tarball extraction. + const result = detectStalePackages(tmp, ["test-pkg"]); + assert.deepEqual(result, [], "should not detect staleness in npm tarball installs (no .git)"); + }); + + it("still detects missing dist/ in npm tarball installs", () => { + // No .git directory — simulates npm install from tarball + const packagesDir = join(tmp, "packages"); + mkdirSync(packagesDir, { recursive: true }); + createFakePackage(packagesDir, "test-pkg", { missingDist: true }); + + // Missing dist/ should always be detected, even in npm installs + const result = detectStalePackages(tmp, ["test-pkg"]); + assert.deepEqual(result, ["test-pkg"]); + }); + + it("returns empty array when dist/ is up to date", () => { + mkdirSync(join(tmp, ".git"), { recursive: true }); + const packagesDir = join(tmp, "packages"); + mkdirSync(packagesDir, { recursive: true }); + createFakePackage(packagesDir, "test-pkg"); + // Default: timestamps are equal (both set by writeFileSync at ~same time) + + const result = detectStalePackages(tmp, ["test-pkg"]); + assert.deepEqual(result, []); + }); +}); diff --git a/tsconfig.test.json b/tsconfig.test.json index b790cf982..d1fb9db80 100644 --- a/tsconfig.test.json +++ b/tsconfig.test.json @@ -4,6 +4,6 @@ "declaration": false, "noEmit": false }, - "include": ["src/tests/headless-cli-surface.test.ts", "src/tests/google-search-oauth-shape.test.ts", "src/tests/google-search-auth.repro.test.ts", "src/headless-events.ts", "src/headless-types.ts"], + "include": ["src/tests/headless-cli-surface.test.ts", "src/tests/ensure-workspace-builds.test.ts", "src/headless-events.ts", "src/headless-types.ts", "src/tests/google-search-oauth-shape.test.ts", "src/tests/google-search-auth.repro.test.ts"], "exclude": [] }