diff --git a/.changeset/stale-dist-source.md b/.changeset/stale-dist-source.md new file mode 100644 index 00000000..c0024387 --- /dev/null +++ b/.changeset/stale-dist-source.md @@ -0,0 +1,21 @@ +--- +'@gtbuchanan/cli': patch +--- + +Stop packing and caching stale `dist/source` content + +`compile:ts` now clears its output directory before invoking tsc. tsc doesn't +record what it emitted and so never removes output whose source was since +renamed or deleted — the orphan stayed behind and `pack:npm` shipped it. The +stale `.tsbuildinfo` goes with it, since left in place it reports the removed +files as up to date and suppresses the re-emit. The `pack:npm` docs and +manifest are kept, as is the `compile:skills` subtree — but only while the +package still authors a `skills/` directory, since once it doesn't that +task no longer runs to clear what it last wrote. + +`compile:ts` also no longer declares the files `pack:npm` writes as its own +turbo `outputs`. The overlapping glob let a `compile:ts` cache entry capture +the stamped manifest and replay it on a hit, restoring whatever version was +current when the entry was written. `pack:npm` now declares the `.npmignore` +it writes, and excludes it from its own inputs alongside the other +self-generated files. diff --git a/packages/cli/skills/gtb-build-pipeline/SKILL.md b/packages/cli/skills/gtb-build-pipeline/SKILL.md index 7251ed00..7d6f27c6 100644 --- a/packages/cli/skills/gtb-build-pipeline/SKILL.md +++ b/packages/cli/skills/gtb-build-pipeline/SKILL.md @@ -127,6 +127,14 @@ The aggregate stays empty rather than naming leaves the root can't define, becau `deploy:skills` keys on `skills/**` and `skills-npm.config.ts` only. If you install or remove an agent and want existing skills resymlinked into the new agent's project-local dir, run `gtb turbo run deploy:skills --force` once — turbo's cache otherwise reports HIT and skips the redeploy. +### Who owns what in `dist/source` + +More than one task writes the published output directory, and each declares `outputs` covering only its own share: `compile:ts` emits the compiled tree, `compile:skills` fills `skills/`, and `pack:npm` writes the docs, the stamped manifest, and `.npmignore`. `compile:ts` therefore subtracts the others from its `dist/source/**` glob. Overlapping the globs would let one task capture a file it doesn't produce and replay it on a hit — a `compile:ts` entry holding a manifest stamped with whatever version was current when the entry was written. + +`compile:ts` also clears the directory before emitting. tsc doesn't track what it emitted, so output whose source was since renamed or deleted survives every rebuild and `pack:npm` ships it; the stale `.tsbuildinfo` has to go too, or tsc reports the removed files as up to date and emits nothing. A package that overrides the `compile:ts` script with its own build step owns that clean itself. + +The clean keeps the `pack:npm` files unconditionally, and keeps the compiled skills only while the package still authors a `skills/` directory. That asymmetry is deliberate: `compile:skills` owns the subtree but nothing orders it against `compile:ts`, so deleting it under a package that still has skills would race. Delete the authored directory and that task stops running — and stops being generated once no package has skills — leaving nobody to clear what it last wrote, so the compiled copy becomes an orphan like any other. + ### The `transit` node Turbo folds a workspace dependency's sources into a consumer's task hash only through a task edge. Tasks that read a dependency as **source** rather than as a build artifact have no artifact task to gate on, so without an edge they replay a cached pass after that dependency changed — a stale green. diff --git a/packages/cli/src/commands/task/compile-ts.ts b/packages/cli/src/commands/task/compile-ts.ts index 5d2e1ded..0d905b96 100644 --- a/packages/cli/src/commands/task/compile-ts.ts +++ b/packages/cli/src/commands/task/compile-ts.ts @@ -1,6 +1,49 @@ +import { existsSync, readdirSync, rmSync } from 'node:fs'; +import path from 'node:path'; import { defineCommand } from 'citty'; +import { + buildOutDir, packNpmOutDirEntries, skillsOutDirEntry, +} from '../../lib/dist-source.ts'; import { run } from '../../lib/process.ts'; +/* + * Entries this task must leave in place. The `pack:npm` docs and manifest are + * unconditional — that task restores them from its own cache entry rather than + * re-deriving them here. The compiled skills are conditional on the package + * still authoring any: while it does, `compile:skills` owns the subtree and + * nothing orders that task against this one, so deleting it would race. Once + * the authored directory is gone that task stops running (and stops being + * generated at all), leaving nobody to clear what it last wrote — so the + * subtree becomes ours to remove, exactly like any other orphaned output. + */ +const preservedEntries = (pkgDir: string): readonly string[] => [ + ...packNpmOutDirEntries, + ...(existsSync(path.join(pkgDir, skillsOutDirEntry)) ? [skillsOutDirEntry] : []), +]; + +/** + * Removes the output of a prior `compile:ts` so the next emit is authoritative. + * + * tsc doesn't record what it emitted and so never deletes output whose source + * was since renamed or removed — the orphan stays behind and `pack:npm` ships + * it. The stale `.tsbuildinfo` goes too: left in place after the files it + * describes are gone, it reports them as up to date and tsc emits nothing. + */ +export const clearCompiledOutput = (pkgDir: string): void => { + const outDir = path.join(pkgDir, buildOutDir); + if (!existsSync(outDir)) { + return; + } + + const preserved = preservedEntries(pkgDir); + for (const entry of readdirSync(outDir)) { + if (preserved.includes(entry)) { + continue; + } + rmSync(path.join(outDir, entry), { force: true, recursive: true }); + } +}; + /** * Runs `tsc -p tsconfig.build.json` to emit compiled output. */ @@ -10,6 +53,7 @@ export const compileTs = defineCommand({ name: 'compile:ts', }, run: async ({ rawArgs }) => { + clearCompiledOutput(process.cwd()); await run('tsc', { args: ['-p', 'tsconfig.build.json', ...rawArgs] }); }, }); diff --git a/packages/cli/src/lib/dist-source.ts b/packages/cli/src/lib/dist-source.ts new file mode 100644 index 00000000..2f9083b7 --- /dev/null +++ b/packages/cli/src/lib/dist-source.ts @@ -0,0 +1,34 @@ +/* + * The published output directory is shared: `compile:ts` emits the compiled + * tree into it, `compile:skills` fills a subtree, and `pack:npm` writes the + * docs and the stamped manifest. No task may treat it as its own, so the split + * is declared once here and consumed by both places that depend on it — the + * turbo `outputs` globs that decide which task caches which file, and the + * clean `compile:ts` runs before emitting. + */ + +/** + * The `outDir` every published package compiles into, and the directory its + * `publishConfig.directory` points npm at. A generated tsconfig.build.json + * owns the value (see `buildOwned`) and `gtb verify` fails on drift, so the + * convention — not a per-package lookup — is the source of truth. + */ +export const buildOutDir = 'dist/source'; + +/** + * Entries `pack:npm` writes into {@link buildOutDir}. + */ +export const packNpmOutDirEntries = [ + '.npmignore', 'LICENSE', 'README.md', 'package.json', +] as const; + +/** + * Subdirectory of {@link buildOutDir} `compile:skills` writes. Named the same + * as the authored source directory it mirrors. + */ +export const skillsOutDirEntry = 'skills'; + +/** + * Prefixes a {@link buildOutDir} entry to form a turbo glob. + */ +export const outDirGlob = (entry: string): string => `${buildOutDir}/${entry}`; diff --git a/packages/cli/src/lib/tsconfig-gen.ts b/packages/cli/src/lib/tsconfig-gen.ts index 6fc09c30..bc66fef5 100644 --- a/packages/cli/src/lib/tsconfig-gen.ts +++ b/packages/cli/src/lib/tsconfig-gen.ts @@ -2,6 +2,7 @@ import path from 'node:path'; import { parseTsconfig } from 'get-tsconfig'; import * as v from 'valibot'; import type { PackageCapabilities } from './discovery.ts'; +import { buildOutDir } from './dist-source.ts'; import { readJsonFile } from './file-writer.ts'; import { toPosixRelative } from './paths.ts'; import { StringArray, UnknownRecord } from './schemas.ts'; @@ -109,7 +110,7 @@ export const typeCheckOwned: Readonly> = { * CompilerOptions owned by the per-package build generator. */ export const buildOwned: Readonly> = { - outDir: 'dist/source', + outDir: buildOutDir, rootDir: '.', }; diff --git a/packages/cli/src/lib/turbo-config.ts b/packages/cli/src/lib/turbo-config.ts index a46c3256..4fd99b8a 100644 --- a/packages/cli/src/lib/turbo-config.ts +++ b/packages/cli/src/lib/turbo-config.ts @@ -1,5 +1,8 @@ import { taskNames } from '../commands/task/names.ts'; import type { WorkspaceDiscovery } from './discovery.ts'; +import { + buildOutDir, outDirGlob, packNpmOutDirEntries, skillsOutDirEntry, +} from './dist-source.ts'; import { skillsConfigFilename } from './skills-config.ts'; import { localeComparer } from './sort.ts'; import { typeCheckInclude } from './tsconfig-gen.ts'; @@ -167,6 +170,19 @@ const typecheckTasks = (flags: ToolFlags): readonly ConditionalEntry[ }, ]; +/* + * The compiled tree is everything in the output directory the sibling tasks + * don't write. Subtracting theirs keeps each task's cache entry to what it + * produced: a shared file captured here would be restored on a `compile:ts` + * hit, replaying whatever the entry was written with — a stamped manifest + * carrying a stale version, for instance. + */ +const compileTsOutputs = (flags: ToolFlags): readonly string[] => [ + `${buildOutDir}/**`, + ...packNpmOutDirEntries.map(entry => `!${outDirGlob(entry)}`), + ...(flags.hasSkills ? [`!${outDirGlob(skillsOutDirEntry)}/**`] : []), +]; + const compileTasks = (flags: ToolFlags): readonly ConditionalEntry[] => [ { condition: flags.hasPublished, @@ -177,7 +193,7 @@ const compileTasks = (flags: ToolFlags): readonly ConditionalEntry[] '$TURBO_ROOT$/tsconfig.base.json', '$TURBO_ROOT$/tsconfig.build.json', ...flags.compileIncludes.flatMap(toTurboGlobs), 'tsconfig.build.json', ], - outputs: ['dist/source/**'], + outputs: compileTsOutputs(flags), }, }, ]; @@ -188,7 +204,7 @@ const compileSkillsTasks = (flags: ToolFlags): readonly ConditionalEntry[] => ], /* * pack:npm copies the package README and the package-or-root LICENSE - * into dist/source so the published tarball ships them, and writes - * dist/source/package.json. Those self-generated files are excluded - * from the dist/source input glob — like the manifest, an input whose - * presence depends on a prior run salts the hash and prevents cache - * hits across fresh worktrees. Their sources (the root LICENSE and - * per-package README/LICENSE) are inputs so an edit invalidates the - * cache, and the copies are outputs so a cache-hit publish restores - * them. + * into the output directory so the published tarball ships them, and + * writes the stamped manifest and .npmignore alongside. Those + * self-generated files are excluded from the output-directory input + * glob — an input whose presence depends on a prior run salts the hash + * and prevents cache hits across fresh worktrees. Their sources (the + * root LICENSE and per-package README/LICENSE) are inputs so an edit + * invalidates the cache, and the copies are outputs so a cache-hit + * publish restores them. */ inputs: [ '$TURBO_ROOT$/LICENSE', '$TURBO_ROOT$/package.json', 'LICENSE', 'README.md', - 'dist/source/**', - '!dist/source/LICENSE', '!dist/source/README.md', '!dist/source/package.json', + `${buildOutDir}/**`, + ...packNpmOutDirEntries.map(entry => `!${outDirGlob(entry)}`), 'package.json', ], outputs: [ 'dist/packages/npm/**', - 'dist/source/LICENSE', 'dist/source/README.md', 'dist/source/package.json', + ...packNpmOutDirEntries.map(outDirGlob), ], }, }, diff --git a/packages/cli/test/compile-ts.test.ts b/packages/cli/test/compile-ts.test.ts new file mode 100644 index 00000000..377d3cf6 --- /dev/null +++ b/packages/cli/test/compile-ts.test.ts @@ -0,0 +1,128 @@ +import { existsSync, mkdirSync, writeFileSync } from 'node:fs'; +import path from 'node:path'; +import { faker } from '@faker-js/faker'; +import { describe, it } from 'vitest'; +import { clearCompiledOutput } from '#src/commands/task/compile-ts.js'; +import { buildOutDir } from '#src/lib/dist-source.js'; +import { createTempDir } from './helpers.ts'; + +/** + * Options for {@link createPackage}. + */ +interface PackageOptions { + /** + * Whether the package still authors `skills/`. `false` scaffolds one that + * dropped them, leaving only the compiled copy from a prior run. Defaults + * to `true`. + */ + readonly authorsSkills?: boolean; +} + +/** + * A scaffolded package and the generated facts a test asserts against. + */ +interface Package { + readonly outDir: string; + readonly pkgDir: string; + /** + * Name of the lone compiled skill. Incidental — nothing branches on it. + */ + readonly skillName: string; +} + +/* + * `skills` stays hardcoded throughout: the production code branches on that + * exact directory name, so generating it would stop exercising the branch. + */ +const skillsDir = 'skills'; + +/** + * Scaffolds a package whose `dist/source` holds output from a prior run: + * compiled files, the `compile:skills` subtree, and the `pack:npm` docs. + */ +const createPackage = (options: PackageOptions = {}): Package => { + const pkgDir = createTempDir(); + const outDir = path.join(pkgDir, buildOutDir); + const skillName = faker.lorem.slug(); + if (options.authorsSkills !== false) { + mkdirSync(path.join(pkgDir, skillsDir), { recursive: true }); + } + mkdirSync(path.join(outDir, 'src'), { recursive: true }); + mkdirSync(path.join(outDir, skillsDir, skillName), { recursive: true }); + for (const file of [ + path.join('src', 'renamed.js'), + path.join('src', 'renamed.d.ts'), + path.join(skillsDir, skillName, 'SKILL.md'), + 'tsconfig.tsbuildinfo', + '.npmignore', + 'LICENSE', + 'README.md', + 'package.json', + ]) { + writeFileSync(path.join(outDir, file), ''); + } + + return { outDir, pkgDir, skillName }; +}; + +describe.concurrent(clearCompiledOutput, () => { + it('removes compiled output left by a prior run', ({ expect }) => { + const { outDir, pkgDir } = createPackage(); + + clearCompiledOutput(pkgDir); + + expect(existsSync(path.join(outDir, 'src'))).toBe(false); + }); + + it('removes the tsbuildinfo so tsc re-emits the cleared output', ({ expect }) => { + const { outDir, pkgDir } = createPackage(); + + clearCompiledOutput(pkgDir); + + expect(existsSync(path.join(outDir, 'tsconfig.tsbuildinfo'))).toBe(false); + }); + + it('preserves the skills subtree compile:skills owns', ({ expect }) => { + const { outDir, pkgDir, skillName } = createPackage(); + + clearCompiledOutput(pkgDir); + + expect(existsSync(path.join(outDir, skillsDir, skillName, 'SKILL.md'))).toBe(true); + }); + + it('removes the skills subtree once the package stops authoring skills', ({ expect }) => { + const { outDir, pkgDir } = createPackage({ authorsSkills: false }); + + clearCompiledOutput(pkgDir); + + expect(existsSync(path.join(outDir, skillsDir))).toBe(false); + }); + + it('preserves the docs and manifest pack:npm owns', ({ expect }) => { + const { outDir, pkgDir } = createPackage(); + + clearCompiledOutput(pkgDir); + + for (const file of ['.npmignore', 'LICENSE', 'README.md', 'package.json']) { + expect(existsSync(path.join(outDir, file))).toBe(true); + } + }); + + it('leaves sibling dist directories untouched', ({ expect }) => { + const { pkgDir } = createPackage(); + const coverage = path.join(pkgDir, 'dist', 'coverage'); + mkdirSync(coverage, { recursive: true }); + + clearCompiledOutput(pkgDir); + + expect(existsSync(coverage)).toBe(true); + }); + + it('no-ops when the package has never been compiled', ({ expect }) => { + const pkgDir = createTempDir(); + + clearCompiledOutput(pkgDir); + + expect(existsSync(path.join(pkgDir, buildOutDir))).toBe(false); + }); +}); diff --git a/packages/cli/test/turbo-json-dist-source.test.ts b/packages/cli/test/turbo-json-dist-source.test.ts new file mode 100644 index 00000000..19d6b281 --- /dev/null +++ b/packages/cli/test/turbo-json-dist-source.test.ts @@ -0,0 +1,71 @@ +import { describe, it } from 'vitest'; +import { generateTurboJson } from '#src/lib/turbo-config.js'; +import { makeCapabilities, makeDiscovery } from './turbo-config.helpers.ts'; + +/* + * Every task writing the published output directory must declare only what it + * writes: an overlapping glob lets one task cache a file another produced and + * replay a stale copy of it on a hit. + */ +describe.concurrent('generateTurboJson (dist/source ownership)', () => { + it('excludes the generated manifest from pack:npm inputs', ({ expect }) => { + const discovery = makeDiscovery([ + makeCapabilities({ isPublished: true }), + ]); + + const result = generateTurboJson(discovery); + + expect(result.tasks['pack:npm']?.inputs).toContain('!dist/source/package.json'); + }); + + it('excludes the generated .npmignore from pack:npm inputs', ({ expect }) => { + const discovery = makeDiscovery([ + makeCapabilities({ isPublished: true }), + ]); + + const result = generateTurboJson(discovery); + + expect(result.tasks['pack:npm']?.inputs).toContain('!dist/source/.npmignore'); + }); + + it('claims every file it writes as a pack:npm output', ({ expect }) => { + const discovery = makeDiscovery([ + makeCapabilities({ isPublished: true }), + ]); + + const result = generateTurboJson(discovery); + + expect(result.tasks['pack:npm']?.outputs).toStrictEqual(expect.arrayContaining([ + 'dist/source/.npmignore', + 'dist/source/LICENSE', + 'dist/source/README.md', + 'dist/source/package.json', + ])); + }); + + it('excludes the pack:npm-owned files from compile:ts outputs', ({ expect }) => { + const discovery = makeDiscovery([ + makeCapabilities({ isPublished: true }), + ]); + + const result = generateTurboJson(discovery); + + expect(result.tasks['compile:ts']?.outputs).toStrictEqual([ + 'dist/source/**', + '!dist/source/.npmignore', + '!dist/source/LICENSE', + '!dist/source/README.md', + '!dist/source/package.json', + ]); + }); + + it('excludes the compile:skills subtree from compile:ts outputs', ({ expect }) => { + const discovery = makeDiscovery([ + makeCapabilities({ hasSkills: true, isPublished: true }), + ]); + + const result = generateTurboJson(discovery); + + expect(result.tasks['compile:ts']?.outputs).toContain('!dist/source/skills/**'); + }); +}); diff --git a/packages/cli/test/turbo-json.test.ts b/packages/cli/test/turbo-json.test.ts index 706e68b8..0154e8d7 100644 --- a/packages/cli/test/turbo-json.test.ts +++ b/packages/cli/test/turbo-json.test.ts @@ -242,16 +242,6 @@ describe.concurrent(generateTurboJson, () => { ]); }); - it('excludes the generated manifest from pack:npm inputs', ({ expect }) => { - const discovery = makeDiscovery([ - makeCapabilities({ isPublished: true }), - ]); - - const result = generateTurboJson(discovery); - - expect(result.tasks['pack:npm']?.inputs).toContain('!dist/source/package.json'); - }); - it('omits lint:eslint from deploy:skills dependsOn when no package has ESLint', ({ expect }) => { const discovery = makeDiscovery([ makeCapabilities({ hasSkills: true }), diff --git a/turbo.json b/turbo.json index 1eb8efc4..f3489fed 100644 --- a/turbo.json +++ b/turbo.json @@ -65,7 +65,12 @@ "tsconfig.build.json" ], "outputs": [ - "dist/source/**" + "dist/source/**", + "!dist/source/.npmignore", + "!dist/source/LICENSE", + "!dist/source/README.md", + "!dist/source/package.json", + "!dist/source/skills/**" ] }, "coverage:codecov:upload": { @@ -149,6 +154,7 @@ "LICENSE", "README.md", "dist/source/**", + "!dist/source/.npmignore", "!dist/source/LICENSE", "!dist/source/README.md", "!dist/source/package.json", @@ -156,6 +162,7 @@ ], "outputs": [ "dist/packages/npm/**", + "dist/source/.npmignore", "dist/source/LICENSE", "dist/source/README.md", "dist/source/package.json"