From 2c41695e1f3d1a0c5ef07475232b34f001c804d8 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 3 Jun 2026 18:06:05 -0700 Subject: [PATCH] fix(test): dependency-aware and dirty-aware test cache invalidation - cache key v2 = own hash + sorted transitive workspace dep hashes + shared inputs (lockfile, tsconfig.base, core __test-utils__ tree) - working-tree-dirty files hashed by content (fixes false cache HIT on unstaged edits) - core __test-utils__ folded globally: 16+ packages import it without a workspace dep on core - dep folding adds ~5ms to the inner loop (memoized own-hashes) - docs: cache semantics, --no-cache / FUSION_TEST_NO_CACHE, TTL rationale --- docs/testing.md | 59 +++ scripts/__tests__/test-changed.test.mjs | 475 ++++++++++++++++++++++-- scripts/test-changed.mjs | 254 ++++++++++--- 3 files changed, 707 insertions(+), 81 deletions(-) diff --git a/docs/testing.md b/docs/testing.md index df86358eb4..2f602f8029 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -113,6 +113,65 @@ For a single Vitest file, use package-local `exec vitest`: pnpm --filter @fusion/core exec vitest run src/__tests__/central-db.test.ts --silent=passed-only --reporter=dot ``` +## Changed-only test cache (`pnpm test`) + +`pnpm test` runs `scripts/test-changed.mjs`, which selects only the workspace +packages affected by your branch diff (plus their reverse-dependents) and skips +packages whose content hasn't changed since they last passed. A per-package +pass-cache lives at `node_modules/.cache/fusion/test-cache.json`. + +### What a cache entry's hash covers (dependency-aware invalidation) + +Each package's cache hash (`computePackageHash`) folds in, so any of these +changing forces that package to re-run: + +- **The package's own tracked files**, hashed via the **working-tree bytes** for + any file that is dirty (unstaged/uncommitted edits) or untracked-not-ignored, + and via git's index blob SHA only when the file is fully clean. This means an + **unstaged edit to a tracked file busts the cache** — no false HIT on a stale + index blob. +- **Every transitive workspace dependency's own hash.** A change to `@fusion/core` + invalidates the cache entries of `engine`, `dashboard`, `cli`, and everything + else that (transitively) depends on it, even when the dependent's own files are + untouched. This is the R11 correctness fix: a dependent is never cache-skipped + when a dependency it consumes has changed. +- **Shared inputs folded into *every* package**: `pnpm-lock.yaml`, + `tsconfig.base.json`, and the shared `packages/core/src/__test-utils__` tree. + The test-utils tree is imported by nearly every package's vitest config via a + relative cross-package path, including packages that have **no** `@fusion/core` + workspace dependency (mobile, droid-cli, pi-\*, and the plugins). Folding it in + globally (like `tsconfig.base.json`) guarantees an edit there invalidates the + whole workspace. + +The hash carries a version prefix (`HASH_VERSION_PREFIX`). Bumping it (done in U4: +`v1` → `v2`) invalidates every pre-existing entry exactly once; old-format cache +files are discarded gracefully rather than crashed on. + +### Escape hatches + +If you suspect a stale or wrong cache result (e.g. a flaky test that happened to +pass got cached, or you want to force a clean re-run), bypass the cache: + +```bash +pnpm test --no-cache # bypass cache reads AND writes for this run +FUSION_TEST_NO_CACHE=1 pnpm test +``` + +`--no-cache` re-runs every selected package without consulting or clearing the +cache file; a subsequent normal `pnpm test` still hits the cache. `pnpm test:full` +already passes `--no-cache` (a full run means full). These flags already exist; +this section documents them. + +### TTL rationale (7-day expiry) + +Entries older than **7 days** are treated as a MISS even on a hash match +(`CACHE_MAX_AGE_MS`). The TTL is intentionally retained even though dep-aware +hashing makes content-staleness impossible: it guards against **environmental +drift** that the content hash cannot see — toolchain/Node upgrades, OS or native +dependency changes, and other host-level shifts that can change test outcomes +without changing any hashed file. Seven days bounds that blind spot while keeping +the cache useful across a normal work week. + ## Engine test helper convention `packages/engine/src/__tests__/executor-test-helpers.ts` defaults both `isUsableTaskWorktree` to `true` and `classifyTaskWorktree` to `{ ok: true }` via a helper-level `worktree-pool` mock. To test failure paths, override with `vi.spyOn(worktreePool, "classifyTaskWorktree").mockResolvedValueOnce({ ok: false, classification: "unregistered", reason: "..." })` (or `isUsableTaskWorktree` for legacy call sites). Production liveness assertions in `executor.ts` are unchanged. diff --git a/scripts/__tests__/test-changed.test.mjs b/scripts/__tests__/test-changed.test.mjs index 5b5e05cd65..0ca4160a8e 100644 --- a/scripts/__tests__/test-changed.test.mjs +++ b/scripts/__tests__/test-changed.test.mjs @@ -29,11 +29,19 @@ import { __setCleanupRmSyncForTests, emitModeDecision, pruneFusionTestHomes, + buildForwardDependencyMap, + collectTransitiveDependencies, + computeOwnHash, } from "../test-changed.mjs"; import { mkdirSync, writeFileSync, mkdtempSync, rmSync, existsSync } from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; +import { spawnSync } from "node:child_process"; +import { fileURLToPath } from "node:url"; + +const thisFile = fileURLToPath(import.meta.url); +const scriptModulePath = path.resolve(path.dirname(thisFile), "..", "test-changed.mjs"); // --------------------------------------------------------------------------- // Helpers @@ -66,14 +74,22 @@ function withTmpDir(fn) { /** * A deterministic fake gitFn that returns a fixed blob sha for any path. * + * Handles the two subcommands the U4 dirty-aware hash issues: + * - `ls-files -s [--] ` → one tracked entry per path (the same + * fixed blob sha), so the hash is content-stable. + * - `status --porcelain ...` → empty (clean tree; nothing dirty/untracked). + * * @param {string} blobSha * @returns {(args: string[]) => string} */ function fakeGit(blobSha = "aabbccdd00112233aabbccdd00112233aabbccdd") { return (args) => { - // ls-files -s output format: " \t" - const pathArg = args[args.length - 1]; - return `100644 ${blobSha} 0\t${pathArg}`; + if (args[0] === "status") return ""; // clean working tree + // ls-files -s [--] → " \t" per path. + const paths = args.filter((a, i) => i >= 2 && a !== "--"); + return paths + .map((p) => `100644 ${blobSha} 0\t${p}`) + .join("\n"); }; } @@ -387,38 +403,34 @@ test("computePackageHash: different blob sha produces different hash", () => { assert.notEqual(h1, h2); }); -test("computePackageHash: hash includes pnpm-lock.yaml so lockfile change busts everything", () => { - // Two fakeGit functions that return different blob SHAs for pnpm-lock.yaml. - const gitWithLockA = (args) => { - const p = args[args.length - 1]; - if (p === "pnpm-lock.yaml") return `100644 locksha-AAAA 0\tpnpm-lock.yaml`; - return `100644 pkgsha-same 0\t${p}`; - }; - const gitWithLockB = (args) => { - const p = args[args.length - 1]; - if (p === "pnpm-lock.yaml") return `100644 locksha-BBBB 0\tpnpm-lock.yaml`; - return `100644 pkgsha-same 0\t${p}`; +// Build a clean-tree gitFn that emits one ls-files entry per requested path, +// letting the caller override a specific path's blob sha (for shared-input tests). +function cleanGitWithOverrides(overrides = {}, fallbackSha = "pkgsha-same") { + return (args) => { + if (args[0] === "status") return ""; + const paths = args.filter((a, i) => i >= 2 && a !== "--"); + return paths + .map((p) => `100644 ${overrides[p] ?? fallbackSha} 0\t${p}`) + .join("\n"); }; +} - const hashA = computePackageHash("packages/engine", gitWithLockA); - const hashB = computePackageHash("packages/engine", gitWithLockB); +test("computePackageHash: hash includes pnpm-lock.yaml so lockfile change busts everything", () => { + const hashA = computePackageHash("packages/engine", cleanGitWithOverrides({ "pnpm-lock.yaml": "locksha-AAAA" })); + const hashB = computePackageHash("packages/engine", cleanGitWithOverrides({ "pnpm-lock.yaml": "locksha-BBBB" })); assert.notEqual(hashA, hashB); }); test("computePackageHash: hash includes tsconfig.base.json so shared TS config change busts cache", () => { - const gitWithTsA = (args) => { - const p = args[args.length - 1]; - if (p === "tsconfig.base.json") return `100644 tsconfig-SHA-AAA 0\ttsconfig.base.json`; - return `100644 same-blob 0\t${p}`; - }; - const gitWithTsB = (args) => { - const p = args[args.length - 1]; - if (p === "tsconfig.base.json") return `100644 tsconfig-SHA-BBB 0\ttsconfig.base.json`; - return `100644 same-blob 0\t${p}`; - }; + const hashA = computePackageHash("packages/engine", cleanGitWithOverrides({ "tsconfig.base.json": "tsconfig-SHA-AAA" })); + const hashB = computePackageHash("packages/engine", cleanGitWithOverrides({ "tsconfig.base.json": "tsconfig-SHA-BBB" })); + assert.notEqual(hashA, hashB); +}); - const hashA = computePackageHash("packages/engine", gitWithTsA); - const hashB = computePackageHash("packages/engine", gitWithTsB); +test("computePackageHash: hash includes shared __test-utils__ so editing it busts every package", () => { + const testUtilsPath = "packages/core/src/__test-utils__"; + const hashA = computePackageHash("plugins/fusion-plugin-roadmap", cleanGitWithOverrides({ [testUtilsPath]: "tu-AAAA" })); + const hashB = computePackageHash("plugins/fusion-plugin-roadmap", cleanGitWithOverrides({ [testUtilsPath]: "tu-BBBB" })); assert.notEqual(hashA, hashB); }); @@ -628,11 +640,16 @@ test("applyCacheToPlan: mixed HIT and MISS across multiple packages", () => { // lookup so that root-file blob SHAs (pnpm-lock.yaml, tsconfig.base.json) // are identical in both contexts. const gitFnMulti = (args) => { - const p = args[args.length - 1]; - if (p === "packages/engine") return `100644 sha-engine 0\tpackages/engine/src/index.ts`; - if (p === "packages/core") return `100644 sha-core 0\tpackages/core/src/index.ts`; - // Root files (pnpm-lock.yaml, tsconfig.base.json) get a stable blob sha. - return `100644 common-root-sha 0\t${p}`; + if (args[0] === "status") return ""; + const paths = args.filter((a, i) => i >= 2 && a !== "--"); + return paths + .map((p) => { + if (p === "packages/engine") return `100644 sha-engine 0\tpackages/engine/src/index.ts`; + if (p === "packages/core") return `100644 sha-core 0\tpackages/core/src/index.ts`; + // Shared inputs (pnpm-lock.yaml, tsconfig.base.json, __test-utils__) get a stable blob sha. + return `100644 common-root-sha 0\t${p}`; + }) + .join("\n"); }; // Pre-compute the engine hash using the SAME gitFnMulti so the stored hash @@ -918,6 +935,398 @@ test("pruneFusionTestHomes: bounded — removes at most maxEntries per call", () } }); +// --------------------------------------------------------------------------- +// U4: real-git-fixture integration (dirty working tree + transitive deps). +// +// These drive the REAL module against a throwaway git repo via a subprocess +// (FUSION_PROJECT_DIR), so git status / working-tree byte reads execute for real +// rather than through stubs. +// --------------------------------------------------------------------------- + +function git(cwd, args) { + const r = spawnSync("git", args, { cwd, encoding: "utf8" }); + if (r.status !== 0) throw new Error(`git ${args.join(" ")} failed: ${r.stderr}`); + return r.stdout; +} + +/** Build a tiny 3-package chain repo: a <- b <- c, plus unrelated d. */ +function makeChainRepo(dir) { + git(dir, ["init", "-q"]); + git(dir, ["config", "user.email", "t@t.t"]); + git(dir, ["config", "user.name", "t"]); + writeFileSync(path.join(dir, "pnpm-lock.yaml"), "lockfileVersion: '9.0'\n"); + writeFileSync(path.join(dir, "tsconfig.base.json"), "{}\n"); + writeFileSync(path.join(dir, "pnpm-workspace.yaml"), "packages:\n - 'packages/*'\n"); + // Shared __test-utils__ tree consumed by every package. + mkdirSync(path.join(dir, "packages", "core", "src", "__test-utils__"), { recursive: true }); + writeFileSync(path.join(dir, "packages", "core", "src", "__test-utils__", "vitest-setup.ts"), "export const setup = 1;\n"); + const pkgs = [ + ["a", "@x/a", {}], + ["b", "@x/b", { "@x/a": "workspace:*" }], + ["c", "@x/c", { "@x/b": "workspace:*" }], + ["d", "@x/d", {}], + ]; + for (const [folder, name, deps] of pkgs) { + const pkgDir = path.join(dir, "packages", folder, "src"); + mkdirSync(pkgDir, { recursive: true }); + writeFileSync(path.join(pkgDir, "index.ts"), `export const x = "${folder}-orig";\n`); + writeFileSync( + path.join(dir, "packages", folder, "package.json"), + JSON.stringify({ name, version: "1.0.0", scripts: { test: "true" }, dependencies: deps }, null, 2), + ); + } + git(dir, ["add", "-A"]); + git(dir, ["commit", "-q", "-m", "init"]); +} + +/** + * Run a snippet inside a subprocess with FUSION_PROJECT_DIR set, importing the + * real test-changed module. The snippet receives `mod` and must console.log a + * single JSON line, which we parse and return. + */ +function runInRepo(repoDir, snippet) { + const code = ` + import * as mod from ${JSON.stringify(scriptModulePath)}; + const out = (${snippet})(mod); + console.log(JSON.stringify(out)); + `; + const r = spawnSync(process.execPath, ["--input-type=module", "-e", code], { + cwd: repoDir, + encoding: "utf8", + env: { ...process.env, FUSION_PROJECT_DIR: repoDir }, + }); + if (r.status !== 0) throw new Error(`subprocess failed: ${r.stderr}\n${r.stdout}`); + const lastLine = r.stdout.trim().split("\n").filter(Boolean).pop(); + return JSON.parse(lastLine); +} + +const packageHashSnippet = (pkgName) => `(mod) => { + const infos = mod.listWorkspacePackageInfos(); + const dirByName = mod.buildPackageDirByName(infos); + const fwd = mod.buildForwardDependencyMap(infos); + return { hash: mod.computePackageHash(dirByName.get(${JSON.stringify(pkgName)}), undefined, { + packageName: ${JSON.stringify(pkgName)}, + forwardDependencyMap: fwd, + packageDirByName: dirByName, + }) }; +}`; + +test("integration: mutating core (a) changes transitive dependents b,c but not unrelated d", () => { + const dir = mkdtempSync(path.join(tmpdir(), "tc-chain-")); + try { + makeChainRepo(dir); + const before = { + b: runInRepo(dir, packageHashSnippet("@x/b")).hash, + c: runInRepo(dir, packageHashSnippet("@x/c")).hash, + d: runInRepo(dir, packageHashSnippet("@x/d")).hash, + }; + // Mutate + commit package a. + writeFileSync(path.join(dir, "packages", "a", "src", "index.ts"), `export const x = "a-CHANGED";\n`); + git(dir, ["commit", "-qam", "change a"]); + const after = { + b: runInRepo(dir, packageHashSnippet("@x/b")).hash, + c: runInRepo(dir, packageHashSnippet("@x/c")).hash, + d: runInRepo(dir, packageHashSnippet("@x/d")).hash, + }; + assert.notEqual(after.b, before.b, "b (depends on a) must change"); + assert.notEqual(after.c, before.c, "c (transitively depends on a) must change"); + assert.equal(after.d, before.d, "d (unrelated) must NOT change"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("integration: unstaged edit to a tracked file changes the hash (no false cache HIT)", () => { + const dir = mkdtempSync(path.join(tmpdir(), "tc-dirty-")); + try { + makeChainRepo(dir); + const clean = runInRepo(dir, packageHashSnippet("@x/a")).hash; + // Unstaged edit (NOT committed, NOT staged) — index blob SHA stays identical. + writeFileSync(path.join(dir, "packages", "a", "src", "index.ts"), `export const x = "a-DIRTY-UNSTAGED";\n`); + const dirty = runInRepo(dir, packageHashSnippet("@x/a")).hash; + assert.notEqual(dirty, clean, "unstaged working-tree edit must bust the hash"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("integration: editing shared __test-utils__ invalidates an unrelated package (d) with no core dep", () => { + const dir = mkdtempSync(path.join(tmpdir(), "tc-tu-")); + try { + makeChainRepo(dir); + const before = runInRepo(dir, packageHashSnippet("@x/d")).hash; + // d has no @fusion/core / @x/a..c dependency, yet must invalidate when the + // globally-folded shared test-utils tree changes. + writeFileSync( + path.join(dir, "packages", "core", "src", "__test-utils__", "vitest-setup.ts"), + "export const setup = 999;\n", + ); + git(dir, ["commit", "-qam", "change test-utils"]); + const after = runInRepo(dir, packageHashSnippet("@x/d")).hash; + assert.notEqual(after, before, "shared __test-utils__ edit must invalidate every package"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("integration: end-to-end cache HIT then dep-change MISS via applyCacheToPlan/recordCachePass", () => { + const dir = mkdtempSync(path.join(tmpdir(), "tc-e2e-")); + try { + makeChainRepo(dir); + const e2e = `(mod) => { + const infos = mod.listWorkspacePackageInfos(); + const dirByName = mod.buildPackageDirByName(infos); + const fwd = mod.buildForwardDependencyMap(infos); + let store = { version: 1, entries: {} }; + const readCacheFn = () => store; + const writeCacheFn = (c) => { store = c; }; + // Record b as passing now. + mod.recordCachePass(["@x/b"], dirByName, { forwardDependencyMap: fwd, readCacheFn, writeCacheFn }); + // Immediate re-check: HIT. + const hit = mod.applyCacheToPlan({ mode: "changed", packages: ["@x/b"] }, + { packageDirByName: dirByName, forwardDependencyMap: fwd, readCacheFn, writeCacheFn }); + return { cachedAfterRecord: hit.cachedPackages, activeAfterRecord: hit.activePackages }; + }`; + const phase1 = runInRepo(dir, e2e); + assert.deepEqual(phase1.cachedAfterRecord, ["@x/b"], "unchanged dependent hits cache"); + assert.deepEqual(phase1.activeAfterRecord, []); + + // Record b's passing hash under the CURRENT (pre-change) tree, capturing the + // serialized cache so we can replay it after mutating the dependency. + const recordSnippet = `(mod) => { + const infos = mod.listWorkspacePackageInfos(); + const dirByName = mod.buildPackageDirByName(infos); + const fwd = mod.buildForwardDependencyMap(infos); + let store = { version: 1, entries: {} }; + mod.recordCachePass(["@x/b"], dirByName, { forwardDependencyMap: fwd, + readCacheFn: () => store, writeCacheFn: (c) => { store = c; } }); + return { recorded: store }; + }`; + const recorded = runInRepo(dir, recordSnippet).recorded; + writeFileSync(path.join(dir, "packages", "a", "src", "index.ts"), `export const x = "a-CHANGED-2";\n`); + git(dir, ["commit", "-qam", "change a again"]); + const checkMissSnippet = `(mod) => { + const infos = mod.listWorkspacePackageInfos(); + const dirByName = mod.buildPackageDirByName(infos); + const fwd = mod.buildForwardDependencyMap(infos); + const store = ${JSON.stringify(recorded)}; + const res = mod.applyCacheToPlan({ mode: "changed", packages: ["@x/b"] }, + { packageDirByName: dirByName, forwardDependencyMap: fwd, readCacheFn: () => store }); + return { cached: res.cachedPackages, active: res.activePackages }; + }`; + const phase2 = runInRepo(dir, checkMissSnippet); + assert.deepEqual(phase2.active, ["@x/b"], "after dep a changed, b cache MISSES"); + assert.deepEqual(phase2.cached, []); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +// --------------------------------------------------------------------------- +// U4: dependency-aware cache invalidation +// --------------------------------------------------------------------------- + +// Stub gitFn that returns per-package blob SHAs from a lookup table, treating +// each directory arg as a single tracked file. Clean tree (status empty). +function depGraphGit(blobByDir) { + return (args) => { + if (args[0] === "status") return ""; + const paths = args.filter((a, i) => i >= 2 && a !== "--"); + return paths + .map((p) => `100644 ${blobByDir[p] ?? "default-sha"} 0\t${p}/index.ts`) + .join("\n"); + }; +} + +const chainPackages = [ + { name: "@x/a", dir: "packages/a", dependencyNames: [] }, + { name: "@x/b", dir: "packages/b", dependencyNames: ["@x/a"] }, + { name: "@x/c", dir: "packages/c", dependencyNames: ["@x/b"] }, + { name: "@x/d", dir: "packages/d", dependencyNames: [] }, +]; + +const chainDirByName = dirByName([ + ["@x/a", "packages/a"], + ["@x/b", "packages/b"], + ["@x/c", "packages/c"], + ["@x/d", "packages/d"], +]); + +test("buildForwardDependencyMap: maps each package to its workspace deps", () => { + const fwd = buildForwardDependencyMap(chainPackages); + assert.deepEqual(fwd.get("@x/a"), []); + assert.deepEqual(fwd.get("@x/b"), ["@x/a"]); + assert.deepEqual(fwd.get("@x/c"), ["@x/b"]); + assert.deepEqual(fwd.get("@x/d"), []); +}); + +test("collectTransitiveDependencies: a <- b <- c chain resolves full closure", () => { + const fwd = buildForwardDependencyMap(chainPackages); + assert.deepEqual(collectTransitiveDependencies("@x/c", fwd), ["@x/a", "@x/b"]); + assert.deepEqual(collectTransitiveDependencies("@x/b", fwd), ["@x/a"]); + assert.deepEqual(collectTransitiveDependencies("@x/a", fwd), []); +}); + +test("collectTransitiveDependencies: tolerates dependency cycles without looping", () => { + const cyclic = buildForwardDependencyMap([ + { name: "@y/a", dir: "packages/a", dependencyNames: ["@y/b"] }, + { name: "@y/b", dir: "packages/b", dependencyNames: ["@y/a"] }, + ]); + assert.deepEqual(collectTransitiveDependencies("@y/a", cyclic), ["@y/b"]); +}); + +test("computePackageHash: mutating core invalidates transitive dependents, not unrelated package", () => { + const fwd = buildForwardDependencyMap(chainPackages); + const hashOf = (pkgName, blobByDir) => + computePackageHash(chainDirByName.get(pkgName), depGraphGit(blobByDir), { + packageName: pkgName, + forwardDependencyMap: fwd, + packageDirByName: chainDirByName, + }); + + const before = { "packages/a": "a-1", "packages/b": "b-1", "packages/c": "c-1", "packages/d": "d-1" }; + const after = { ...before, "packages/a": "a-2" }; // mutate a only + + const bBefore = hashOf("@x/b", before); + const cBefore = hashOf("@x/c", before); + const dBefore = hashOf("@x/d", before); + + // b depends on a; c depends on b->a. Both must change. d is unrelated. + assert.notEqual(hashOf("@x/b", after), bBefore, "b (direct dependent of a) must invalidate"); + assert.notEqual(hashOf("@x/c", after), cBefore, "c (transitive dependent of a) must invalidate"); + assert.equal(hashOf("@x/d", after), dBefore, "d (unrelated) must stay stable"); +}); + +test("computePackageHash: hashing without dep options ignores transitive deps (own-only fallback)", () => { + // Same dir, no packageName/forwardDependencyMap → only own + shared inputs. + const g = depGraphGit({ "packages/b": "b-1" }); + const h1 = computePackageHash("packages/b", g); + const h2 = computePackageHash("packages/b", g); + assert.equal(h1, h2); +}); + +test("computeOwnHash: memoizes per packageDir (same content -> same hash, computed once)", () => { + let lsCalls = 0; + const countingGit = (args) => { + if (args[0] === "status") return ""; + if (args[0] === "ls-files") lsCalls += 1; + const paths = args.filter((a, i) => i >= 2 && a !== "--"); + return paths.map((p) => `100644 same-sha 0\t${p}/index.ts`).join("\n"); + }; + const memo = new Map(); + const h1 = computeOwnHash("packages/a", countingGit, memo); + const callsAfterFirst = lsCalls; + const h2 = computeOwnHash("packages/a", countingGit, memo); + assert.equal(h1, h2, "same content -> same hash"); + assert.equal(lsCalls, callsAfterFirst, "second call served from memo, no extra git calls"); +}); + +test("computePackageHash: shared memo computes each dependency own-hash once across packages", () => { + const fwd = buildForwardDependencyMap(chainPackages); + const blobByDir = { "packages/a": "a", "packages/b": "b", "packages/c": "c", "packages/d": "d" }; + const lsByDir = new Map(); + const countingGit = (args) => { + if (args[0] === "status") return ""; + const paths = args.filter((a, i) => i >= 2 && a !== "--"); + for (const p of paths) lsByDir.set(p, (lsByDir.get(p) ?? 0) + 1); + return paths.map((p) => `100644 ${blobByDir[p] ?? "x"} 0\t${p}/index.ts`).join("\n"); + }; + const memo = new Map(); + for (const name of ["@x/b", "@x/c"]) { + computePackageHash(chainDirByName.get(name), countingGit, { + packageName: name, + forwardDependencyMap: fwd, + packageDirByName: chainDirByName, + memo, + }); + } + // packages/a is a dependency of both b and c; with the shared memo its + // own-hash ls-files runs exactly once, not once per dependent. + assert.equal(lsByDir.get("packages/a"), 1, "core own-hash computed once via memo"); +}); + +test("readCache: old cache version is discarded (version-prefix bump invalidates entries)", () => { + // HASH_VERSION_PREFIX bumped v1->v2 in U4: a v1-era stored hash for the same + // package will no longer match the freshly-computed v2 hash, so the entry is a + // MISS rather than a crash or false hit. + const g = depGraphGit({ "packages/a": "a-1" }); + const v2Hash = computePackageHash("packages/a", g, { + packageName: "@x/a", + forwardDependencyMap: buildForwardDependencyMap(chainPackages), + packageDirByName: chainDirByName, + }); + const staleV1LikeHash = "0".repeat(64); // a pre-bump digest shape + assert.notEqual(v2Hash, staleV1LikeHash); + + const cache = { + version: 1, + entries: { "@x/a": { hash: staleV1LikeHash, passedAt: new Date().toISOString(), command: "test" } }, + }; + const result = applyCacheToPlan( + { mode: "changed", packages: ["@x/a"] }, + { + gitFn: g, + readCacheFn: () => cache, + packageDirByName: chainDirByName, + forwardDependencyMap: buildForwardDependencyMap(chainPackages), + }, + ); + assert.deepEqual(result.cachedPackages, []); + assert.deepEqual(result.activePackages, ["@x/a"]); +}); + +test("applyCacheToPlan: dependency change forces dependent re-run even with fresh own hash", () => { + const fwd = buildForwardDependencyMap(chainPackages); + // Cache b as passing under blob a-1. Then a changes to a-2: b must re-run. + const cachedHash = computePackageHash("packages/b", depGraphGit({ "packages/a": "a-1", "packages/b": "b-1" }), { + packageName: "@x/b", + forwardDependencyMap: fwd, + packageDirByName: chainDirByName, + }); + const cache = { + version: 1, + entries: { "@x/b": { hash: cachedHash, passedAt: new Date().toISOString(), command: "test" } }, + }; + + // Dependency a mutated; b's own files unchanged. + const result = applyCacheToPlan( + { mode: "changed", packages: ["@x/b"] }, + { + gitFn: depGraphGit({ "packages/a": "a-2", "packages/b": "b-1" }), + readCacheFn: () => cache, + packageDirByName: chainDirByName, + forwardDependencyMap: fwd, + }, + ); + assert.deepEqual(result.activePackages, ["@x/b"], "b must re-run after its dep changed"); + assert.deepEqual(result.cachedPackages, []); +}); + +test("applyCacheToPlan: genuinely unchanged dependent still hits cache (fast path preserved)", () => { + const fwd = buildForwardDependencyMap(chainPackages); + const blobs = { "packages/a": "a-1", "packages/b": "b-1" }; + const cachedHash = computePackageHash("packages/b", depGraphGit(blobs), { + packageName: "@x/b", + forwardDependencyMap: fwd, + packageDirByName: chainDirByName, + }); + const cache = { + version: 1, + entries: { "@x/b": { hash: cachedHash, passedAt: new Date().toISOString(), command: "test" } }, + }; + const result = applyCacheToPlan( + { mode: "changed", packages: ["@x/b"] }, + { + gitFn: depGraphGit(blobs), // nothing changed + readCacheFn: () => cache, + packageDirByName: chainDirByName, + forwardDependencyMap: fwd, + }, + ); + assert.deepEqual(result.cachedPackages, ["@x/b"]); + assert.deepEqual(result.activePackages, []); +}); + test("pruneFusionTestHomes: only targets the fusion-test-home-root- prefix", () => { const ours = path.join(tmpdir(), `fusion-test-home-root-prune-prefix-${process.pid}`); const foreign = path.join(tmpdir(), `not-ours-prune-prefix-${process.pid}`); diff --git a/scripts/test-changed.mjs b/scripts/test-changed.mjs index d7e138b3f4..c1a0b9bd31 100644 --- a/scripts/test-changed.mjs +++ b/scripts/test-changed.mjs @@ -9,6 +9,7 @@ import { cpus, tmpdir } from "node:os"; import { createRequire } from "node:module"; import { ensureTestArtifacts } from "./ensure-test-artifacts.mjs"; import { isSkillSyncCheckCached } from "./sync-fusion-skill-tools.mjs"; +import { computeContentHash } from "./lib/content-hash.mjs"; const currentFilePath = fileURLToPath(import.meta.url); const scriptDir = path.dirname(currentFilePath); @@ -88,8 +89,35 @@ const rootDir = process.env.FUSION_PROJECT_DIR /** @type {string} Cache format version — bump when the shape or hash inputs change. */ const CACHE_FORMAT_VERSION = 1; -/** @type {string} Constant mixed into every content hash so format rev busts all entries. */ -const HASH_VERSION_PREFIX = "v1"; +/** + * @type {string} Constant mixed into every content hash so format rev busts all entries. + * + * U4 bumped v1 -> v2: the hash now (a) folds in every transitive workspace + * dependency's own-hash, (b) folds in the shared `packages/core/src/__test-utils__` + * tree globally, and (c) hashes working-tree bytes for dirty/untracked files + * instead of trusting the (stale) index blob SHA. Any of these shifts the digest, + * so the bump invalidates every pre-U4 entry exactly once. + */ +const HASH_VERSION_PREFIX = "v2"; + +/** + * @type {string[]} Repo-relative paths whose content is folded into EVERY + * package's hash. These are shared inputs that any package's test run depends on + * regardless of the workspace dependency graph: + * - pnpm-lock.yaml / tsconfig.base.json: global build/resolution config. + * - packages/core/src/__test-utils__: the shared vitest setup/teardown/workers + * helpers are imported by nearly every package's vitest config via a relative + * cross-package path (e.g. `../../core/src/__test-utils__/vitest-setup.ts`), + * INCLUDING packages that have no `@fusion/core` workspace dependency + * (mobile, droid-cli, pi-*, and every plugin/example). Dep-aware hashing + * alone would miss those, so we fold the tree in globally — the simplest + * provably-correct choice (mirrors the tsconfig.base.json treatment). + */ +const SHARED_HASH_INPUT_PATHS = [ + "pnpm-lock.yaml", + "tsconfig.base.json", + "packages/core/src/__test-utils__", +]; /** @type {number} Max age (ms) for a cache entry to count as a pass. */ const CACHE_MAX_AGE_MS = 7 * 24 * 60 * 60 * 1000; // 7 days @@ -321,6 +349,48 @@ export function buildReverseDependencyMap(workspacePackages) { return reverseDependencyMap; } +/** + * Build forward dependency map: package name → [workspace dependency names]. + * Only workspace-internal dependencies are included (external npm deps are + * already captured by the shared pnpm-lock.yaml hash). + * + * @param {{ name: string, dependencyNames?: string[] }[]} workspacePackages + * @returns {Map} + */ +export function buildForwardDependencyMap(workspacePackages) { + const workspaceNames = new Set(workspacePackages.map((p) => p.name)); + const forwardDependencyMap = new Map(); + for (const pkg of workspacePackages) { + const deps = (pkg.dependencyNames ?? []).filter((dep) => workspaceNames.has(dep) && dep !== pkg.name); + forwardDependencyMap.set(pkg.name, [...new Set(deps)]); + } + return forwardDependencyMap; +} + +/** + * Collect the transitive closure of workspace dependencies for a package + * (excluding the package itself), returned sorted for hash stability. + * + * @param {string} packageName + * @param {Map} forwardDependencyMap + * @returns {string[]} sorted transitive dependency names + */ +export function collectTransitiveDependencies(packageName, forwardDependencyMap) { + const seen = new Set(); + const queue = [...(forwardDependencyMap.get(packageName) ?? [])]; + + while (queue.length > 0) { + const current = queue.shift(); + if (seen.has(current) || current === packageName) continue; + seen.add(current); + for (const next of forwardDependencyMap.get(current) ?? []) { + if (!seen.has(next)) queue.push(next); + } + } + + return [...seen].sort((a, b) => a.localeCompare(b)); +} + export function expandWithReverseDependents(packageNames, reverseDependencyMap) { const expanded = new Set(packageNames); const queue = [...packageNames]; @@ -518,64 +588,132 @@ export function writeCache(filePath, cache) { } /** - * Compute a stable content hash for a package directory. + * Adapt a 1-arg test-changed gitFn `(args) => string|null` into the 2-arg + * `(args, cwd) => string|null` shape that scripts/lib/content-hash.mjs expects. + * The cwd is ignored because the inner-loop gitFn already runs with cwd=rootDir. * - * The hash is SHA-256 over: - * - The constant version prefix HASH_VERSION_PREFIX - * - The blob SHA of pnpm-lock.yaml at HEAD - * - The blob SHA of tsconfig.base.json at HEAD - * - Every (relativePath, blobSha) pair from `git ls-files -s `, - * sorted lexicographically by path for stability. + * @param {(args: string[]) => string|null} gitFn + * @returns {(args: string[], cwd: string) => string|null} + */ +function adaptGitFnForContentHash(gitFn) { + return (args) => gitFn(args); +} + +/** + * Compute the OWN content hash for a single package directory: a SHA-256 over + * just that directory's files, WITHOUT any shared inputs or transitive deps. * - * Using git blob SHAs means we never read file contents ourselves — git - * already hashes them, so this is fast even for large packages. + * R11 / U4 correctness: this defers to scripts/lib/content-hash.mjs, which hashes + * the working-tree bytes of dirty (modified-tracked) and untracked-not-ignored + * files instead of the stale index blob SHA. The pre-U4 implementation used + * `git ls-files -s` (index only), so an UNSTAGED edit to a tracked file produced + * an identical hash → a false cache HIT that skipped a package whose on-disk + * source had actually changed. Routing through computeContentHash fixes that. + * + * Results are memoized per (packageDir, gitFn) for the lifetime of a `memo` Map + * so that folding a dependency's own-hash into many dependents stays O(packages), + * not O(packages^2). * * @param {string} packageDir Relative path to the package dir (e.g. "packages/engine") * @param {(args: string[]) => string|null} gitFn Injectable git runner (for tests) + * @param {Map} [memo] Per-call memo keyed by packageDir. * @returns {string} 64-char hex SHA-256 */ -export function computePackageHash(packageDir, gitFn = gitOutput) { +export function computeOwnHash(packageDir, gitFn = gitOutput, memo) { + if (memo?.has(packageDir)) return memo.get(packageDir); + + const ownHash = computeContentHash({ + rootDir, + inputPaths: [packageDir], + versionPrefix: `${HASH_VERSION_PREFIX}:own`, + gitFn: adaptGitFnForContentHash(gitFn), + }); + + memo?.set(packageDir, ownHash); + return ownHash; +} + +/** + * Compute the hash for the SHARED inputs folded into every package's hash + * (pnpm-lock.yaml, tsconfig.base.json, and the shared __test-utils__ tree). + * Memoized per call so it's computed at most once per run. + * + * @param {(args: string[]) => string|null} gitFn + * @param {Map} [memo] + * @returns {string} + */ +function computeSharedInputsHash(gitFn = gitOutput, memo) { + const memoKey = "\0shared-inputs\0"; + if (memo?.has(memoKey)) return memo.get(memoKey); + + const sharedHash = computeContentHash({ + rootDir, + inputPaths: SHARED_HASH_INPUT_PATHS, + versionPrefix: `${HASH_VERSION_PREFIX}:shared`, + gitFn: adaptGitFnForContentHash(gitFn), + }); + + memo?.set(memoKey, sharedHash); + return sharedHash; +} + +/** + * Compute the dependency-aware cache hash for a package directory. + * + * The hash is SHA-256 over, in a stable order: + * - The constant version prefix HASH_VERSION_PREFIX. + * - The shared-inputs hash (pnpm-lock.yaml + tsconfig.base.json + the shared + * packages/core/src/__test-utils__ tree). + * - The package's own dirty-aware content hash. + * - Every TRANSITIVE workspace dependency's own dirty-aware hash, sorted by + * dependency name. + * + * Folding transitive dependencies in means a change to (say) @fusion/core busts + * the cache entry of every package that transitively depends on it, even when + * the dependent's own files are untouched — closing the R11 correctness hole + * where a stale-but-own-hash-matching dependent could be cache-skipped after its + * dependency's source changed. + * + * @param {string} packageDir Relative path to the package dir (e.g. "packages/engine") + * @param {(args: string[]) => string|null} gitFn Injectable git runner (for tests) + * @param {object} [options] + * @param {string} [options.packageName] Package name, to resolve transitive deps. + * @param {Map} [options.forwardDependencyMap] name → [dep names]. + * @param {Map} [options.packageDirByName] name → relative dir. + * @param {Map} [options.memo] Per-run own-hash memo (perf). + * @returns {string} 64-char hex SHA-256 + */ +export function computePackageHash(packageDir, gitFn = gitOutput, options = {}) { + const { packageName, forwardDependencyMap, packageDirByName, memo = new Map() } = options; + const hash = createHash("sha256"); hash.update(HASH_VERSION_PREFIX); hash.update("\0"); - // Bust when lock file or shared TS config changes. - for (const rootFile of ["pnpm-lock.yaml", "tsconfig.base.json"]) { - // `git ls-files -s ` → " \t" - const out = gitFn(["ls-files", "-s", rootFile]); - const blobSha = out ? out.split(/\s+/)[1] ?? "" : ""; - hash.update(rootFile); - hash.update("="); - hash.update(blobSha); - hash.update("\0"); - } + // Shared inputs (lockfile, base tsconfig, shared __test-utils__ tree). + hash.update("shared="); + hash.update(computeSharedInputsHash(gitFn, memo)); + hash.update("\0"); - // All tracked files inside the package directory. - const lsOut = gitFn(["ls-files", "-s", packageDir]); - const entries = []; - if (lsOut) { - for (const line of lsOut.split("\n")) { - const trimmed = line.trim(); - if (!trimmed) continue; - // Format: SP SP TAB - const tabIdx = trimmed.indexOf("\t"); - if (tabIdx === -1) continue; - const fields = trimmed.slice(0, tabIdx).split(/\s+/); - const blobSha = fields[1] ?? ""; - const filePath = trimmed.slice(tabIdx + 1); - entries.push({ filePath, blobSha }); + // This package's own dirty-aware content. + hash.update("own="); + hash.update(computeOwnHash(packageDir, gitFn, memo)); + hash.update("\0"); + + // Transitive workspace dependencies' own hashes (sorted by name for stability). + if (packageName && forwardDependencyMap && packageDirByName) { + const transitiveDeps = collectTransitiveDependencies(packageName, forwardDependencyMap); + for (const depName of transitiveDeps) { + const depDir = packageDirByName.get(depName); + if (!depDir) continue; // Unknown dir (defensive); skip rather than crash. + hash.update("dep:"); + hash.update(depName); + hash.update("="); + hash.update(computeOwnHash(depDir, gitFn, memo)); + hash.update("\0"); } } - // Sort for determinism (git output is usually sorted, but let's be explicit). - entries.sort((a, b) => a.filePath.localeCompare(b.filePath)); - for (const { filePath, blobSha } of entries) { - hash.update(filePath); - hash.update("="); - hash.update(blobSha); - hash.update("\0"); - } - return hash.digest("hex"); } @@ -604,6 +742,7 @@ function relativeTime(isoTimestamp) { * @property {() => CacheFile} [readCacheFn] Injectable cache reader. * @property {(cache: CacheFile) => void} [writeCacheFn] Injectable cache writer. * @property {Map} [packageDirByName] pkg-name → relative dir. + * @property {Map} [forwardDependencyMap] pkg-name → workspace dep names. */ /** @@ -628,6 +767,7 @@ export function applyCacheToPlan(plan, options = {}) { readCacheFn, writeCacheFn, packageDirByName = new Map(), + forwardDependencyMap = new Map(), } = options; // Full suite runs always bypass cache (full means full). @@ -641,10 +781,18 @@ export function applyCacheToPlan(plan, options = {}) { const cachedPackages = []; const activePackages = []; + // Shared per-call memo so each package/dependency own-hash is computed once, + // keeping dep-aware hashing O(packages) rather than O(packages^2). + const memo = new Map(); for (const pkg of plan.packages ?? []) { const pkgDir = packageDirByName.get(pkg) ?? `packages/${pkg.replace(/^@[^/]+\//, "")}`; - const computedHash = computePackageHash(pkgDir, gitFn); + const computedHash = computePackageHash(pkgDir, gitFn, { + packageName: pkg, + forwardDependencyMap, + packageDirByName, + memo, + }); const entry = cache.entries[pkg]; const isHit = @@ -678,6 +826,7 @@ export function recordCachePass(packages, packageDirByName, options = {}) { gitFn = gitOutput, readCacheFn, writeCacheFn, + forwardDependencyMap = new Map(), } = options; if (noCache || packages.length === 0) return; @@ -685,10 +834,17 @@ export function recordCachePass(packages, packageDirByName, options = {}) { const filePath = cacheFilePath(); const cache = readCacheFn ? readCacheFn() : readCache(filePath); const now = new Date().toISOString(); + // Shared per-call memo (see applyCacheToPlan): keep own-hashing O(packages). + const memo = new Map(); for (const pkg of packages) { const pkgDir = packageDirByName.get(pkg) ?? `packages/${pkg.replace(/^@[^/]+\//, "")}`; - const hash = computePackageHash(pkgDir, gitFn); + const hash = computePackageHash(pkgDir, gitFn, { + packageName: pkg, + forwardDependencyMap, + packageDirByName, + memo, + }); cache.entries[pkg] = { hash, passedAt: now, command: "test" }; } @@ -927,6 +1083,7 @@ export function main(argv = process.argv.slice(2)) { const packageNameByDir = listWorkspacePackages(workspacePackages); const packageDirByName = buildPackageDirByName(workspacePackages); const reverseDependencyMap = buildReverseDependencyMap(workspacePackages); + const forwardDependencyMap = buildForwardDependencyMap(workspacePackages); const plan = decideExecutionPlan({ forceFullSuite, @@ -947,6 +1104,7 @@ export function main(argv = process.argv.slice(2)) { ({ cachedPackages, activePackages } = applyCacheToPlan(plan, { noCache: noCache || forceFullSuite, packageDirByName, + forwardDependencyMap, })); } @@ -1022,7 +1180,7 @@ export function main(argv = process.argv.slice(2)) { }); // Tests passed — record in cache (never cache failures; process.exit on failure above). - recordCachePass(activePackages, packageDirByName, { noCache }); + recordCachePass(activePackages, packageDirByName, { noCache, forwardDependencyMap }); } finally { cleanupIsolatedHome(); }