From 6a1c376094ac1194e5a24d8effc51a084ea9c803 Mon Sep 17 00:00:00 2001 From: Taylor Buchanan Date: Sat, 15 Aug 2026 12:02:27 -0500 Subject: [PATCH 1/3] Stop packing and caching stale dist/source content MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tsc doesn't record what it emitted, so output whose source was since renamed or deleted survived every rebuild and pack:npm shipped it. compile:ts now clears its output directory before emitting, dropping the stale tsbuildinfo with it — left in place it reports the removed files as up to date and suppresses the re-emit. Entries another task owns are kept: compile:skills has no edge ordering it against compile:ts, and the pack:npm docs and manifest come back from that task's own cache entry. compile:ts also declared 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. Each writer of the output directory now declares only what it produces, including the .npmignore pack:npm had been writing without claiming, and the ownership split lives in one module that both the turbo globs and the clean read. The remaining item, whether pack:npm's dist/source input needs deferred hashing, is split out to #409. Closes #353 Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/stale-dist-source.md | 19 +++++ .../cli/skills/gtb-build-pipeline/SKILL.md | 6 ++ packages/cli/src/commands/task/compile-ts.ts | 31 +++++++ packages/cli/src/lib/dist-source.ts | 42 +++++++++ packages/cli/src/lib/tsconfig-gen.ts | 3 +- packages/cli/src/lib/turbo-config.ts | 42 ++++++--- packages/cli/test/compile-ts.test.ts | 85 +++++++++++++++++++ .../cli/test/turbo-json-dist-source.test.ts | 71 ++++++++++++++++ packages/cli/test/turbo-json.test.ts | 10 --- turbo.json | 9 +- 10 files changed, 293 insertions(+), 25 deletions(-) create mode 100644 .changeset/stale-dist-source.md create mode 100644 packages/cli/src/lib/dist-source.ts create mode 100644 packages/cli/test/compile-ts.test.ts create mode 100644 packages/cli/test/turbo-json-dist-source.test.ts diff --git a/.changeset/stale-dist-source.md b/.changeset/stale-dist-source.md new file mode 100644 index 00000000..8195f1ed --- /dev/null +++ b/.changeset/stale-dist-source.md @@ -0,0 +1,19 @@ +--- +'@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. Entries another task owns +(the `compile:skills` subtree, the `pack:npm` docs and manifest) are kept. + +`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..38551f3f 100644 --- a/packages/cli/skills/gtb-build-pipeline/SKILL.md +++ b/packages/cli/skills/gtb-build-pipeline/SKILL.md @@ -127,6 +127,12 @@ 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, keeping only the entries above. 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 `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..0d0c6dc6 100644 --- a/packages/cli/src/commands/task/compile-ts.ts +++ b/packages/cli/src/commands/task/compile-ts.ts @@ -1,6 +1,36 @@ +import { existsSync, readdirSync, rmSync } from 'node:fs'; +import path from 'node:path'; import { defineCommand } from 'citty'; +import { buildOutDir, foreignOutDirEntries } from '../../lib/dist-source.ts'; import { run } from '../../lib/process.ts'; +/** + * 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. + * + * Entries another task owns are left alone. `compile:skills` has no edge + * ordering it against this task, so deleting its subtree would race it, and + * the `pack:npm` docs and manifest are restored from that task's own cache + * entry rather than re-derived here. + */ +export const clearCompiledOutput = (pkgDir: string): void => { + const outDir = path.join(pkgDir, buildOutDir); + if (!existsSync(outDir)) { + return; + } + + for (const entry of readdirSync(outDir)) { + if (foreignOutDirEntries.includes(entry)) { + continue; + } + rmSync(path.join(outDir, entry), { force: true, recursive: true }); + } +}; + /** * Runs `tsc -p tsconfig.build.json` to emit compiled output. */ @@ -10,6 +40,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..b0ab7232 --- /dev/null +++ b/packages/cli/src/lib/dist-source.ts @@ -0,0 +1,42 @@ +/* + * 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. + */ +export const skillsOutDirEntry = 'skills'; + +/** + * Entries inside {@link buildOutDir} that belong to a task other than + * `compile:ts`. + */ +export const foreignOutDirEntries: readonly string[] = [ + ...packNpmOutDirEntries, + skillsOutDirEntry, +]; + +/** + * 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..a7fd358b --- /dev/null +++ b/packages/cli/test/compile-ts.test.ts @@ -0,0 +1,85 @@ +import { existsSync, mkdirSync, writeFileSync } from 'node:fs'; +import path from 'node:path'; +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'; + +/** + * 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 = (): { readonly outDir: string; readonly pkgDir: string } => { + const pkgDir = createTempDir(); + const outDir = path.join(pkgDir, buildOutDir); + mkdirSync(path.join(outDir, 'src'), { recursive: true }); + mkdirSync(path.join(outDir, 'skills', 'my-skill'), { recursive: true }); + for (const file of [ + path.join('src', 'renamed.js'), + path.join('src', 'renamed.d.ts'), + path.join('skills', 'my-skill', 'SKILL.md'), + 'tsconfig.tsbuildinfo', + '.npmignore', + 'LICENSE', + 'README.md', + 'package.json', + ]) { + writeFileSync(path.join(outDir, file), ''); + } + + return { outDir, pkgDir }; +}; + +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 } = createPackage(); + + clearCompiledOutput(pkgDir); + + expect(existsSync(path.join(outDir, 'skills', 'my-skill', 'SKILL.md'))).toBe(true); + }); + + 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" From 80b88d962c80da491a2d25c4ced964683e689eed Mon Sep 17 00:00:00 2001 From: Taylor Buchanan Date: Fri, 21 Aug 2026 09:53:54 -0500 Subject: [PATCH 2/3] Clear compiled skills once a package stops authoring them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The clean kept dist/source/skills unconditionally, on the grounds that compile:skills owns it and nothing orders that task against compile:ts. That holds only while the package still authors skills. Delete the authored directory and compile:skills early-returns without touching its destination — and stops being generated at all once no package has skills — so the compiled copy had nobody left to clear it and shipped forever. Exactly the staleness this task exists to prevent. The subtree is now preserved only while the authored directory exists, which is the same signal discovery derives hasSkills from. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/stale-dist-source.md | 6 +++-- .../cli/skills/gtb-build-pipeline/SKILL.md | 4 ++- packages/cli/src/commands/task/compile-ts.ts | 27 ++++++++++++++----- packages/cli/src/lib/dist-source.ts | 12 ++------- packages/cli/test/compile-ts.test.ts | 27 ++++++++++++++++++- 5 files changed, 55 insertions(+), 21 deletions(-) diff --git a/.changeset/stale-dist-source.md b/.changeset/stale-dist-source.md index 8195f1ed..c0024387 100644 --- a/.changeset/stale-dist-source.md +++ b/.changeset/stale-dist-source.md @@ -8,8 +8,10 @@ Stop packing and caching stale `dist/source` content 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. Entries another task owns -(the `compile:skills` subtree, the `pack:npm` docs and manifest) are kept. +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 diff --git a/packages/cli/skills/gtb-build-pipeline/SKILL.md b/packages/cli/skills/gtb-build-pipeline/SKILL.md index 38551f3f..7d6f27c6 100644 --- a/packages/cli/skills/gtb-build-pipeline/SKILL.md +++ b/packages/cli/skills/gtb-build-pipeline/SKILL.md @@ -131,7 +131,9 @@ The aggregate stays empty rather than naming leaves the root can't define, becau 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, keeping only the entries above. 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. +`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 diff --git a/packages/cli/src/commands/task/compile-ts.ts b/packages/cli/src/commands/task/compile-ts.ts index 0d0c6dc6..0d905b96 100644 --- a/packages/cli/src/commands/task/compile-ts.ts +++ b/packages/cli/src/commands/task/compile-ts.ts @@ -1,9 +1,26 @@ import { existsSync, readdirSync, rmSync } from 'node:fs'; import path from 'node:path'; import { defineCommand } from 'citty'; -import { buildOutDir, foreignOutDirEntries } from '../../lib/dist-source.ts'; +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. * @@ -11,11 +28,6 @@ import { run } from '../../lib/process.ts'; * 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. - * - * Entries another task owns are left alone. `compile:skills` has no edge - * ordering it against this task, so deleting its subtree would race it, and - * the `pack:npm` docs and manifest are restored from that task's own cache - * entry rather than re-derived here. */ export const clearCompiledOutput = (pkgDir: string): void => { const outDir = path.join(pkgDir, buildOutDir); @@ -23,8 +35,9 @@ export const clearCompiledOutput = (pkgDir: string): void => { return; } + const preserved = preservedEntries(pkgDir); for (const entry of readdirSync(outDir)) { - if (foreignOutDirEntries.includes(entry)) { + if (preserved.includes(entry)) { continue; } rmSync(path.join(outDir, entry), { force: true, recursive: true }); diff --git a/packages/cli/src/lib/dist-source.ts b/packages/cli/src/lib/dist-source.ts index b0ab7232..2f9083b7 100644 --- a/packages/cli/src/lib/dist-source.ts +++ b/packages/cli/src/lib/dist-source.ts @@ -23,19 +23,11 @@ export const packNpmOutDirEntries = [ ] as const; /** - * Subdirectory of {@link buildOutDir} `compile:skills` writes. + * Subdirectory of {@link buildOutDir} `compile:skills` writes. Named the same + * as the authored source directory it mirrors. */ export const skillsOutDirEntry = 'skills'; -/** - * Entries inside {@link buildOutDir} that belong to a task other than - * `compile:ts`. - */ -export const foreignOutDirEntries: readonly string[] = [ - ...packNpmOutDirEntries, - skillsOutDirEntry, -]; - /** * Prefixes a {@link buildOutDir} entry to form a turbo glob. */ diff --git a/packages/cli/test/compile-ts.test.ts b/packages/cli/test/compile-ts.test.ts index a7fd358b..d02328be 100644 --- a/packages/cli/test/compile-ts.test.ts +++ b/packages/cli/test/compile-ts.test.ts @@ -5,13 +5,30 @@ 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; +} + /** * 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 = (): { readonly outDir: string; readonly pkgDir: string } => { +const createPackage = ( + options: PackageOptions = {}, +): { readonly outDir: string; readonly pkgDir: string } => { const pkgDir = createTempDir(); const outDir = path.join(pkgDir, buildOutDir); + if (options.authorsSkills !== false) { + mkdirSync(path.join(pkgDir, 'skills'), { recursive: true }); + } mkdirSync(path.join(outDir, 'src'), { recursive: true }); mkdirSync(path.join(outDir, 'skills', 'my-skill'), { recursive: true }); for (const file of [ @@ -55,6 +72,14 @@ describe.concurrent(clearCompiledOutput, () => { expect(existsSync(path.join(outDir, 'skills', 'my-skill', '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, 'skills'))).toBe(false); + }); + it('preserves the docs and manifest pack:npm owns', ({ expect }) => { const { outDir, pkgDir } = createPackage(); From 2bc972ecc54415667ee4eb6fde925e96ad193d29 Mon Sep 17 00:00:00 2001 From: Taylor Buchanan Date: Fri, 21 Aug 2026 12:05:51 -0500 Subject: [PATCH 3/3] Generate the incidental skill name in compile:ts tests The fixture hardcoded the compiled skill's directory name and the assertion repeated the literal, which is the duplicate-literal pattern the test-data convention exists to avoid. Nothing branches on it, so it is generated and captured now. The `skills` directory itself stays hardcoded and moves to a named constant: the production code branches on that exact name, so generating it would stop exercising the branch. Co-Authored-By: Claude Opus 5 (1M context) --- packages/cli/test/compile-ts.test.ts | 38 ++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/packages/cli/test/compile-ts.test.ts b/packages/cli/test/compile-ts.test.ts index d02328be..377d3cf6 100644 --- a/packages/cli/test/compile-ts.test.ts +++ b/packages/cli/test/compile-ts.test.ts @@ -1,5 +1,6 @@ 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'; @@ -17,24 +18,41 @@ interface PackageOptions { 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 = {}, -): { readonly outDir: string; readonly pkgDir: string } => { +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, 'skills'), { recursive: true }); + mkdirSync(path.join(pkgDir, skillsDir), { recursive: true }); } mkdirSync(path.join(outDir, 'src'), { recursive: true }); - mkdirSync(path.join(outDir, 'skills', 'my-skill'), { 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('skills', 'my-skill', 'SKILL.md'), + path.join(skillsDir, skillName, 'SKILL.md'), 'tsconfig.tsbuildinfo', '.npmignore', 'LICENSE', @@ -44,7 +62,7 @@ const createPackage = ( writeFileSync(path.join(outDir, file), ''); } - return { outDir, pkgDir }; + return { outDir, pkgDir, skillName }; }; describe.concurrent(clearCompiledOutput, () => { @@ -65,11 +83,11 @@ describe.concurrent(clearCompiledOutput, () => { }); it('preserves the skills subtree compile:skills owns', ({ expect }) => { - const { outDir, pkgDir } = createPackage(); + const { outDir, pkgDir, skillName } = createPackage(); clearCompiledOutput(pkgDir); - expect(existsSync(path.join(outDir, 'skills', 'my-skill', 'SKILL.md'))).toBe(true); + expect(existsSync(path.join(outDir, skillsDir, skillName, 'SKILL.md'))).toBe(true); }); it('removes the skills subtree once the package stops authoring skills', ({ expect }) => { @@ -77,7 +95,7 @@ describe.concurrent(clearCompiledOutput, () => { clearCompiledOutput(pkgDir); - expect(existsSync(path.join(outDir, 'skills'))).toBe(false); + expect(existsSync(path.join(outDir, skillsDir))).toBe(false); }); it('preserves the docs and manifest pack:npm owns', ({ expect }) => {