diff --git a/scripts/__tests__/spec-main-shape-gate.test.ts b/scripts/__tests__/spec-main-shape-gate.test.ts index 12a487907b..d6051acd9c 100644 --- a/scripts/__tests__/spec-main-shape-gate.test.ts +++ b/scripts/__tests__/spec-main-shape-gate.test.ts @@ -11,6 +11,8 @@ import { satisfiesRange, parseWorkspaceGlobs, MARKER_FILE, + SPEC_PACKAGE_NAME, + TURBO_HASH_MARKER, } from '../spec-main-shape-gate.mjs'; import { pullRequestTrigger, readWorkflows, repoRoot, subscribesMergeGroup } from './workflow-checks.js'; @@ -564,3 +566,162 @@ describe('the injection reads the spec against the dependencies it was built wit expect(() => satisfiesRange('4.6.1', 'workspace:*')).toThrow(/does not read/); }); }); + +/** + * objectui#12114 — a linked git worktree shares the main checkout's turbo + * cache, and `--force` skips cache reads but still WRITES. So a local + * reproduction of this gate recorded main-spec verdicts that a sibling + * worktree's ordinary, installed-spec `pnpm type-check` replayed as its own: a + * false green. The fix puts the injection inside turbo's hash, through + * `turbo.json`'s `globalDependencies`; the script header carries the why. + * + * The second case drives a REAL turbo (`--dry=json`, which computes the hashes + * and runs nothing) over a fixture that copies this repository's `turbo.json`, + * so it fails the day that file stops hashing the marker. + */ +describe('a local reproduction cannot leave a verdict an installed tree replays (objectui#12114)', () => { + const made: string[] = []; + afterEach(() => { + for (const dir of made.splice(0)) fs.rmSync(dir, { recursive: true, force: true }); + }); + + const STORE_SPEC = 'node_modules/.pnpm/@objectstack+spec@17.4.0/node_modules/@objectstack/spec'; + + /** + * An installed tree, linked the way pnpm links it (RELATIVE symlinks into the + * virtual store), in a git repository that ignores `node_modules` as this one + * does: the file turbo has to hash is an ignored one. + */ + function installedTree({ rootLink }: { rootLink: boolean }) { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'spec-gate-turbo-')); + made.push(root); + const repo = path.join(root, 'objectui'); + fs.mkdirSync(path.join(repo, 'scripts'), { recursive: true }); + for (const file of ['spec-main-shape-gate.mjs', 'invoked-as.mjs']) { + fs.copyFileSync(path.join(repoRoot, 'scripts', file), path.join(repo, 'scripts', file)); + } + fs.copyFileSync(path.join(repoRoot, 'turbo.json'), path.join(repo, 'turbo.json')); + + const store = path.join(repo, STORE_SPEC); + // What a reinstall does to an injected copy: the directory is replaced. + const install = () => { + fs.rmSync(store, { recursive: true, force: true }); + fs.mkdirSync(store, { recursive: true }); + fs.writeFileSync(path.join(store, 'package.json'), JSON.stringify({ name: SPEC_PACKAGE_NAME, version: '17.4.0' })); + }; + install(); + const devDependencies = rootLink ? { [SPEC_PACKAGE_NAME]: '^17.4.0' } : {}; + if (rootLink) { + fs.mkdirSync(path.join(repo, 'node_modules', '@objectstack'), { recursive: true }); + fs.symlinkSync(`../.pnpm/@objectstack+spec@17.4.0/node_modules/@objectstack/spec`, path.join(repo, 'node_modules', '@objectstack', 'spec')); + } + const app = path.join(repo, 'packages', 'app'); + fs.mkdirSync(path.join(app, 'node_modules', '@objectstack'), { recursive: true }); + fs.symlinkSync(`../../../../${STORE_SPEC}`, path.join(app, 'node_modules', '@objectstack', 'spec')); + fs.writeFileSync( + path.join(app, 'package.json'), + JSON.stringify({ name: 'app', scripts: { 'type-check': 'tsc --noEmit' }, dependencies: { [SPEC_PACKAGE_NAME]: '^17.4.0' } }), + ); + fs.writeFileSync( + path.join(repo, 'package.json'), + JSON.stringify({ name: 'root', private: true, packageManager: 'pnpm@10.31.0', devDependencies }), + ); + fs.writeFileSync(path.join(repo, 'pnpm-workspace.yaml'), "packages:\n - 'packages/*'\n"); + fs.writeFileSync(path.join(repo, '.gitignore'), 'node_modules\n.turbo\n'); + execFileSync('git', ['init', '-q'], { cwd: repo }); + + const upstream = path.join(root, 'objectstack'); + fs.mkdirSync(path.join(upstream, 'packages', 'spec'), { recursive: true }); + fs.writeFileSync(path.join(upstream, 'packages', 'spec', 'package.json'), JSON.stringify({ name: SPEC_PACKAGE_NAME })); + const pack = path.join(root, 'pack', 'package'); + fs.mkdirSync(path.join(pack, 'dist'), { recursive: true }); + fs.writeFileSync(path.join(pack, 'dist', 'index.d.ts'), 'export {};\n'); + fs.writeFileSync( + path.join(pack, 'package.json'), + JSON.stringify({ name: SPEC_PACKAGE_NAME, version: '17.4.0', exports: { '.': { types: './dist/index.d.ts' } } }), + ); + const tarball = path.join(root, 'spec.tgz'); + execFileSync('tar', ['-czf', tarball, '-C', path.join(root, 'pack'), 'package']); + + const inject = () => + spawnSync( + 'node', + [path.join(repo, 'scripts', 'spec-main-shape-gate.mjs'), 'inject', '--tarball', tarball, '--sha', 'f'.repeat(40), '--upstream-checkout', upstream], + { encoding: 'utf8' }, + ); + // The global files turbo hashed, and every task's hash. + const hashes = () => { + const env = Object.fromEntries(Object.entries(process.env).filter(([name]) => !name.startsWith('TURBO_'))); + const dry = JSON.parse( + execFileSync(path.join(repoRoot, 'node_modules', '.bin', 'turbo'), ['run', 'type-check', '--dry=json'], { + cwd: repo, + encoding: 'utf8', + env: { ...env, TURBO_TELEMETRY_DISABLED: '1' }, + // turbo warns that the fixture has no lockfile; a failure still throws with it. + stdio: ['ignore', 'pipe', 'pipe'], + }), + ); + const tasks: Record = {}; + for (const task of dry.tasks) tasks[task.taskId] = task.hash; + return { files: Object.keys(dry.globalCacheInputs.files), tasks }; + }; + return { install, inject, hashes }; + } + + it('turbo.json hashes the marker inject writes, by a literal path through the root link', () => { + expect(TURBO_HASH_MARKER).toBe(`node_modules/${SPEC_PACKAGE_NAME}/${MARKER_FILE}`); + const turbo = JSON.parse(fs.readFileSync(path.join(repoRoot, 'turbo.json'), 'utf8')); + expect(turbo.globalDependencies ?? [], 'without it an injected tree and an installed one hash alike').toContain( + TURBO_HASH_MARKER, + ); + // turbo does not expand a wildcard inside node_modules: a glob there would + // hash nothing, silently. + expect(TURBO_HASH_MARKER).not.toMatch(/[*?[{!]/); + // pnpm creates the root link the path goes through only because the root + // manifest declares the spec. + const manifest = JSON.parse(fs.readFileSync(path.join(repoRoot, 'package.json'), 'utf8')); + expect( + ['dependencies', 'devDependencies', 'optionalDependencies'].some((field) => manifest[field]?.[SPEC_PACKAGE_NAME]), + `the root manifest no longer declares ${SPEC_PACKAGE_NAME}, so ${TURBO_HASH_MARKER} never exists`, + ).toBe(true); + // The marker can never be committed. + expect(spawnSync('git', ['check-ignore', '-q', 'node_modules'], { cwd: repoRoot }).status).toBe(0); + }); + + it('real turbo: an injected tree hashes apart from an installed one, and back once the injection is undone', () => { + const tree = installedTree({ rootLink: true }); + const installed = tree.hashes(); + expect(installed.tasks).toHaveProperty(['app#type-check']); + expect(installed.files).not.toContain(TURBO_HASH_MARKER); + + const first = tree.inject(); + expect(first.status, first.stderr).toBe(0); + expect(first.stdout).toContain(`turbo hashes ${TURBO_HASH_MARKER}`); + const injected = tree.hashes(); + expect(injected.files).toContain(TURBO_HASH_MARKER); + for (const [task, hash] of Object.entries(installed.tasks)) { + expect(injected.tasks[task], `${task} hashes as installed after an injection`).not.toBe(hash); + } + + // A second injection at the same sha shares no entry with the first. + expect(tree.inject().status).toBe(0); + const again = tree.hashes(); + for (const [task, hash] of Object.entries(injected.tasks)) expect(again.tasks[task]).not.toBe(hash); + + // Undone the way a reinstall undoes it, the tree hashes as installed again: + // the marker cannot outlive the injection. + tree.install(); + expect(tree.hashes()).toEqual(installed); + }); + + it('the firing control: without the root link the same injection hashes as installed, and inject says so', () => { + // The marker inside the virtual store alone is invisible to turbo, so the + // hash above moved because of the root-linked path and nothing else. + const tree = installedTree({ rootLink: false }); + const installed = tree.hashes(); + const result = tree.inject(); + expect(result.status, result.stderr).toBe(0); + expect(result.stdout).toContain(`⚠️ ${TURBO_HASH_MARKER} is absent`); + expect(tree.hashes()).toEqual(installed); + }); +}); diff --git a/scripts/__tests__/turbo-build-inputs.test.ts b/scripts/__tests__/turbo-build-inputs.test.ts index 39dcedc02f..f571f4c81a 100644 --- a/scripts/__tests__/turbo-build-inputs.test.ts +++ b/scripts/__tests__/turbo-build-inputs.test.ts @@ -19,7 +19,7 @@ import { CONFIG_FILES as VITEST_CONFIG_FILES } from './helpers/vitest-config-pro * the one with the widest blast radius. * * Turbo hashes a task from its `inputs`. `$TURBO_DEFAULT$` covers only files - * inside the package directory, and `globalDependencies` is unset — so any file + * inside the package directory, and `globalDependencies` holds no source file — so any file * the task reads from elsewhere in the repo is invisible to the cache key. When * such a file changes, turbo does not re-run the task; it REPLAYS the previous * verdict. `build` declared no `inputs` AT ALL, so it ran on that default, diff --git a/scripts/__tests__/turbo-lint-inputs.test.ts b/scripts/__tests__/turbo-lint-inputs.test.ts index 394297dfd7..a9fcdd0531 100644 --- a/scripts/__tests__/turbo-lint-inputs.test.ts +++ b/scripts/__tests__/turbo-lint-inputs.test.ts @@ -23,7 +23,7 @@ import { * `test`, and it is the starkest of the three. * * Turbo hashes a task from its `inputs`. `$TURBO_DEFAULT$` covers only files - * inside the package directory, and `globalDependencies` is unset — so any file + * inside the package directory, and `globalDependencies` holds no source file — so any file * the task reads from elsewhere in the repo is invisible to the cache key. When * such a file changes, turbo does not re-run the task; it REPLAYS the previous * verdict. `lint` declared no `inputs` AT ALL, so it ran on that default: diff --git a/scripts/__tests__/turbo-test-inputs.test.ts b/scripts/__tests__/turbo-test-inputs.test.ts index cb8e359598..df8fee509a 100644 --- a/scripts/__tests__/turbo-test-inputs.test.ts +++ b/scripts/__tests__/turbo-test-inputs.test.ts @@ -19,7 +19,7 @@ import { * that objectui#3514 closed for `type-check`, and it has it worse. * * Turbo hashes a task from its `inputs`. `$TURBO_DEFAULT$` covers only files - * inside the package directory, and `globalDependencies` is unset — so any file + * inside the package directory, and `globalDependencies` holds no source file — so any file * the task reads from elsewhere in the repo is invisible to the cache key. When * such a file changes, turbo does not re-run the task; it REPLAYS the previous * verdict. Before this guard, EVERY entry in the `test` task's list was diff --git a/scripts/__tests__/turbo-type-check-inputs.test.ts b/scripts/__tests__/turbo-type-check-inputs.test.ts index a0d2694039..ce96167d0e 100644 --- a/scripts/__tests__/turbo-type-check-inputs.test.ts +++ b/scripts/__tests__/turbo-type-check-inputs.test.ts @@ -14,7 +14,7 @@ import { outOfPackageFiles } from './helpers/tsc-program'; * program reaches OUTSIDE its own package directory. * * Turbo hashes a task from its `inputs`. `$TURBO_DEFAULT$` covers only files - * inside the package directory, and `globalDependencies` is unset — so any file + * inside the package directory, and `globalDependencies` holds no source file — so any file * the program reads from elsewhere in the repo is invisible to the cache key. * When such a file changes, turbo does not re-run the task; it REPLAYS the * previous verdict. That is a gate whose answer depends on cache state rather diff --git a/scripts/spec-main-shape-gate.mjs b/scripts/spec-main-shape-gate.mjs index 2e2862818f..b85652599e 100644 --- a/scripts/spec-main-shape-gate.mjs +++ b/scripts/spec-main-shape-gate.mjs @@ -158,12 +158,45 @@ * * `turbo`'s `type-check` task is cached, and its hash covers this repository's * sources, its lockfile and a declared env list -- it does NOT cover the CONTENT - * of `node_modules`. An injected spec therefore changes nothing turbo hashes: - * a second run after an injection can replay the verdict recorded BEFORE it, and - * a run at a new objectstack sha can replay the verdict from the old one. Both - * are green answers to a question that was never asked. The workflow runs the - * typecheck with the cache bypassed for exactly this reason, and - * `scripts/__tests__/spec-main-shape-gate.test.ts` pins that it keeps doing so. + * of `node_modules`. The injected spec's own files therefore change nothing + * turbo hashes: a second run after an injection could replay the verdict + * recorded BEFORE it, and a run at a new objectstack sha the verdict from the + * old one. Both are green answers to a question that was never asked. The + * workflow runs the typecheck with the cache bypassed for exactly this reason, + * and `scripts/__tests__/spec-main-shape-gate.test.ts` pins that it keeps doing + * so. The marker below closes both cases too; the bypass does not lean on it. + * + * ## ...and an injected tree must not leave a verdict an INSTALLED tree replays (objectui#12114) + * + * The reverse direction is local, and the bypass does not close it: `--force` + * (`TURBO_FORCE=true`) skips cache READS and still WRITES. A linked git worktree + * shares the main checkout's `.turbo/cache` (turbo logs "using shared worktree + * cache"), so a reproduction in one worktree recorded main-spec verdicts that a + * later ordinary `pnpm type-check` in a sibling -- same sources, installed spec + * -- replayed as its own. The measurement is on this issue's pull request. + * + * So the injection is part of turbo's hash: `turbo.json`'s `globalDependencies` + * names `TURBO_HASH_MARKER`, the `MARKER_FILE` `inject` writes into every + * replaced copy, reached through the root link to the spec. Measured on turbo + * 2.10: + * + * - Absent, a global dependency hashes as nothing: installed trees share + * entries with each other as before, and an injected tree hashes apart from + * all of them in both directions, with no flag to remember. The marker holds + * the sha and the injection time, so two injections never share one either. + * - The path is LITERAL because turbo does not expand a wildcard inside + * `node_modules`: a glob over the virtual store hashes nothing, silently. The + * root link exists because the root manifest declares the spec; the consumer + * proof below requires it to resolve to a marked copy, and `inject` says so + * out loud when there is no root link to hash. + * - The marker lives INSIDE the replaced copy, so it cannot outlive the + * injection: `pnpm install --force` or a fresh install replaces that + * directory, and the tree hashes as installed again. A plain `pnpm install` + * leaves the injection and its marker both in place. + * + * A local reproduction is therefore `inject`, then `pnpm type-check --continue` + * with no flag, then `pnpm install --frozen-lockfile --force` to undo it. The + * test file above pins the wiring and drives a real turbo over a fixture. */ import fs from 'node:fs'; @@ -183,6 +216,13 @@ export const UPSTREAM_REPO = 'objectstack-ai/objectstack'; /** The marker `inject` leaves inside every replaced copy. */ export const MARKER_FILE = '.spec-main-shape-gate.json'; +/** + * That marker as `turbo.json`'s `globalDependencies` names it: a LITERAL path + * through the repository root's own link to the spec. See "...and an injected + * tree must not leave a verdict" in the header. + */ +export const TURBO_HASH_MARKER = ['node_modules', SPEC_PACKAGE_NAME, MARKER_FILE].join('/'); + const repoRootDefault = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); /** @@ -833,7 +873,20 @@ export function inject({ tarball, sha, upstreamCheckout, repoRoot = repoRootDefa ); } - return { targets, consumers, version: manifest.version, sha, substitutions, resolutions, oneCopy }; + // The marker turbo hashes (objectui#12114). The root link, when there is one, + // already passed the consumer proof above, so this reads where turbo will look. + const hashMarker = fs.existsSync(path.join(repoRoot, TURBO_HASH_MARKER)) ? TURBO_HASH_MARKER : null; + log( + hashMarker + ? `spec-main-shape-gate: turbo hashes ${hashMarker}, so this tree's task hashes no longer match an ` + + `installed tree's: a type-check here neither replays an installed verdict nor leaves one for a sibling ` + + `worktree. \`pnpm install --frozen-lockfile --force\` undoes the injection and the marker together.` + : `spec-main-shape-gate: ⚠️ ${TURBO_HASH_MARKER} is absent -- no root link to the spec -- so turbo hashes ` + + `this tree like an installed one, and a type-check here leaves verdicts a sibling worktree's installed ` + + `type-check can replay.`, + ); + + return { targets, consumers, version: manifest.version, sha, substitutions, resolutions, oneCopy, hashMarker }; } /** diff --git a/turbo.json b/turbo.json index b74c7b6b71..05f98df9e1 100644 --- a/turbo.json +++ b/turbo.json @@ -1,6 +1,7 @@ { "$schema": "https://turbo.build/schema.json", "globalEnv": ["NODE_ENV"], + "globalDependencies": ["node_modules/@objectstack/spec/.spec-main-shape-gate.json"], "tasks": { "build": { "dependsOn": ["^build"],