diff --git a/README.md b/README.md index a20a78a..c0cf7d3 100644 --- a/README.md +++ b/README.md @@ -188,7 +188,15 @@ Each emitted plugin names one `source`. Optional `overrides` are applied after the source, so it can add or replace files for that target. `repositoryFiles` copies a whole directory into the generated repository root. -`pluginpack` writes a `.pluginpack/.json` managed-file manifest for each built target. That manifest lets builds and cleanup commands remove stale generated files without touching source files or unmanaged repo content. +`pluginpack` writes a `.pluginpack/.json` managed-file manifest for each built target. That manifest lets builds and cleanup commands remove stale generated files without touching source files or unmanaged repo content. It also records a sha256 of each file as written. If you edit a generated file and a later `build`, `prune` or `clean` would overwrite or delete it, pluginpack prints a warning naming the file first. It still goes ahead, because the output belongs to pluginpack; move the edit into the source to keep it. Manifests from 0.11 and earlier have no hashes, so they produce no warnings until the next build rewrites them. + +Before it deletes anything, pluginpack compares real paths, with symlinks resolved and, on a case-insensitive filesystem, case folded. It refuses an `outDir` that lands inside a source, overrides, partials, `repositoryFiles` or `rootFiles` path, even through a symlink. It also refuses two targets that write the same file through different spellings, such as `out` and `OUT` on macOS, or through a generated file that is a symlink to another target's file. Under the default `plugins/` root, a directory with a component directory counts as legacy source and is never deleted, unless pluginpack recorded writing every file in it. `--dry-run` runs the same checks, so it fails wherever the real command would. + +If an `outDir` holds a manifest for a target the config no longer builds there, `build` and `prune` refuse to run. That happens when a config drops a target, or moves an outDir to another target, such as `codex` to `agent-plugins`, without cleaning first. No configured target would prune those files, so they would stay forever. Run `pluginpack clean --target ` to delete them. It finds the old target's manifest in any outDir the config uses, even when that target is no longer configured. A plain `clean` deletes them too. To keep the files unmanaged instead, delete the manifest. + +An `outDir` outside the project, such as a sibling output repository (`../cursor-plugins`), works. The guard there knows only that repository's `.git` directory and the files the manifest lists, so it cannot protect anything else in that repository from a manifest someone edited by hand. + +**Upgrading from 0.11 (managed files):** the first build rewrites each manifest as version 2. pluginpack 0.11 cannot read a version 2 manifest, so upgrade every machine and CI job that builds the same output together. A manifest from a newer pluginpack fails with a message saying to upgrade. Some configs that 0.11 accepted now fail at load or build: `source.partials` outside the project, an `outDir` inside a source path, and a `rootFiles` entry that copies a file onto itself (`"README.md": "README.md"` with `outDir: "."`). Each error names the field to change. A build into an outDir that still holds a dropped target's manifest also fails until you run `pluginpack clean --target `. ## Components @@ -583,7 +591,7 @@ rest of the artifact. | ------------ | ------ | -------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `plugins` | string | no | Directory to discover source plugins from. Defaults to `plugins`. | | `skills` | string | no | Repo-level skills directory; creates a root source plugin from sibling component dirs. | -| `partials` | string | no | Directory of reusable `{{> name}}` text fragments, shared across every source plugin (see **Partials**). | +| `partials` | string | no | Directory of reusable `{{> name}}` text fragments, shared across every source plugin (see **Partials**). Safe relative: it must sit inside the project. | | `rootPlugin` | object | no | Metadata for that root skills plugin. Accepts all **`metadata`** fields plus `id`, `name`, `description`. `id` is the source-plugin name used in each target's `from` array. | **`metadata`** (and `source.rootPlugin`) @@ -606,7 +614,7 @@ rest of the artifact. | Field | Type | Required | Meaning | | ------------------ | ------------------------------- | -------- | ------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `outDir` | string | yes | Output directory for this target, relative to the config root. | +| `outDir` | string | yes | Output directory for this target, relative to the config root. May be outside the project; it may not sit inside a source path. | | `format` | `"agent-plugins"` \| `"legacy"` | no | `codex` only. `"legacy"` keeps the `.codex-plugin/plugin.json` layout; default `"agent-plugins"`. | | `plugins` | record | yes | Emitted plugins, keyed by emitted plugin name (see **`targets..plugins.`**). | | `marketplaceDir` | string (safe relative) | no | Override the marketplace dir (defaults: `.cursor-plugin` / `.claude-plugin`). | @@ -803,7 +811,7 @@ Exit codes: ### `clean` -Remove all managed files for configured target outputs. +Remove all managed files in configured target outputs, including those of a target the config no longer builds there. ```bash pluginpack clean [--target copilot|antigravity|cursor|claude|codex|agent-plugins] [--dry-run] @@ -811,7 +819,7 @@ pluginpack clean [--target copilot|antigravity|cursor|claude|codex|agent-plugins Options: -- `--target `: Clean only one configured target. +- `--target `: Clean only one target. It may be one the config no longer builds, if its manifest is in a configured outDir. - `--dry-run`: Print managed files without deleting them. - `--force`: Delete even paths that resolve inside the source tree or config. diff --git a/src/build.ts b/src/build.ts index 1ec3bdb..f3a6f33 100644 --- a/src/build.ts +++ b/src/build.ts @@ -1,12 +1,20 @@ +import { promises as fs } from "node:fs"; import path from "node:path"; -import { loadConfig } from "./config.js"; -import { writeArtifact } from "./fs.js"; +import { assertOutDirsOutsideSource, loadConfig } from "./config.js"; +import { physicalKey, withPathCache, writeArtifact } from "./fs.js"; import { + assertManagedManifestContained, buildDeleteGuard, + executePrune, + managedManifestPath, + managedPathKey, normalizeManagedPath, - pruneManagedFiles, + planPrune, readManagedManifest, + recordHandedOverHashes, + warnOnEditedOverwrites, writeManagedManifest, + type PrunePlan, } from "./managed.js"; import { emitTarget, targetNames } from "./adapters.js"; import { @@ -17,23 +25,16 @@ import type { Artifact, BuildOptions, ResolvedProject, + ResolvedProjectConfig, TargetName, } from "./types.js"; /** Builds every configured (or explicitly selected) target and writes its output, unless `dryRun` is set. */ export async function build(options: BuildOptions = {}): Promise { - const project = await loadConfig(options.cwd, options.configPath); - const targets = options.target - ? [options.target] - : targetNames.filter((target) => project.config.targets[target]); - const guard = await buildDeleteGuard(project); - const artifacts: Artifact[] = []; - for (const target of targets) { - artifacts.push(await emitTarget(project, target, options.outDir)); + const { artifacts, prunes } = await planBuild(options); + for (const plan of prunes) { + await warnOnEditedOverwrites(plan, options.dryRun); } - const owner = assertNoCrossTargetCollisions(artifacts); - await assertNoCollisionsWithBuiltTargets(project, targets, owner); - assertNoUnsubstitutedPartialTags(artifacts); if (!options.dryRun) { // Write every target's new files before pruning any target's stale ones. // If a later target's write throws, no target has had files pruned yet — @@ -44,14 +45,92 @@ export async function build(options: BuildOptions = {}): Promise { for (const artifact of artifacts) { await writeArtifact(artifact.outDir, artifact.files); } - for (const artifact of artifacts) { - await pruneManagedFiles(artifact, { guard }); - await writeManagedManifest(artifact); + // Before any target prunes or gives up a path, every target's manifest + // claims its new paths and still lists its old ones. A path handed from + // one target to another is then always listed by at least one manifest: + // if the recipient's prune fails after the donor's final manifest is + // written, the recipient's interim manifest still owns the path, and a + // later clean can remove it. + for (const plan of prunes) { + await writeManagedManifest(plan.artifact, plan.previous); + } + for (const plan of prunes) { + await executePrune(plan); + await writeManagedManifest(plan.artifact); } } return artifacts; } +/** + * Emits every selected target and runs every check that could refuse the + * build, without touching disk: cross-target collisions, collisions with other + * targets' manifests, unsubstituted partials, self-referential rootFiles, + * manifest containment, and the delete guard over each target's stale files. + * + * `build` and `prune` both start here, and a dry run stops here, so a dry run + * fails exactly where the real command would, and a refused prune never + * leaves a half-written build behind. `force` relaxes only the delete guard. + */ +export function planBuild( + options: BuildOptions, + force?: boolean, +): Promise<{ artifacts: Artifact[]; prunes: PrunePlan[] }> { + // Nothing in the plan writes, so it can resolve each directory once. + return withPathCache(() => planBuildChecked(options, force)); +} + +async function planBuildChecked( + options: BuildOptions, + force?: boolean, +): Promise<{ artifacts: Artifact[]; prunes: PrunePlan[] }> { + const project = await loadConfig(options.cwd, options.configPath); + const targets = options.target + ? [options.target] + : targetNames.filter((target) => project.config.targets[target]); + const guard = await buildDeleteGuard(project, force); + const artifacts: Artifact[] = []; + for (const target of targets) { + artifacts.push(await emitTarget(project, target, options.outDir)); + } + if (options.outDir) { + await assertOutDirsOutsideSource( + project, + artifacts.map((artifact) => ({ + target: artifact.target, + outDir: artifact.outDir, + })), + ); + } + await assertNoOrphanManifests( + project, + artifacts.map(({ target, outDir }) => ({ target, outDir })), + ); + const incoming = await assertNoCrossTargetCollisions(artifacts); + const claimedElsewhere = await assertNoCollisionsWithBuiltTargets( + project, + targets, + incoming, + ); + assertNoUnsubstitutedPartialTags(artifacts); + await assertNoSelfReferentialRootFiles(project, artifacts); + for (const artifact of artifacts) { + await assertManagedManifestContained(artifact.outDir, artifact.target); + } + const owners = new Map(claimedElsewhere); + for (const [key, claim] of incoming.byEntry) { + owners.set(key, claim.target); + } + const prunes: PrunePlan[] = []; + for (const artifact of artifacts) { + prunes.push( + await planPrune(artifact, { dryRun: options.dryRun, guard, owners }), + ); + } + await recordHandedOverHashes(prunes); + return { artifacts, prunes }; +} + /** * A `{{> name}}` tag that reaches output is broken content: whatever reads the * file gets a template marker instead of the text it was meant to inline. @@ -74,34 +153,98 @@ function assertNoUnsubstitutedPartialTags(artifacts: Artifact[]): void { } } +/** + * A `rootFiles` entry whose destination is its own source makes the authored + * file a managed output: the build copies it onto itself, and the next `clean` + * (or a prune after the entry is removed) deletes the only copy. Refuse it + * before writing anything. (A `repositoryFiles` directory that is the outDir + * is caught earlier, at config load, as an outDir inside a source path.) + */ +async function assertNoSelfReferentialRootFiles( + project: ResolvedProject, + artifacts: Artifact[], +): Promise { + const problems: string[] = []; + for (const artifact of artifacts) { + const targetConfig = project.config.targets[artifact.target]; + for (const [dest, source] of Object.entries( + targetConfig?.rootFiles ?? {}, + )) { + const [destKey, sourceKey] = await Promise.all([ + physicalKey(path.resolve(artifact.outDir, dest)), + physicalKey(path.resolve(project.rootDir, source)), + ]); + if (destKey === sourceKey) { + problems.push( + ` targets.${artifact.target}.rootFiles["${dest}"] copies ${path.resolve(project.rootDir, source)} onto itself`, + ); + } + } + } + if (problems.length > 0) { + throw new Error( + `Invalid pluginpack config: a generated file would replace its own source:\n${problems.join("\n")}\n` + + `pluginpack would then manage, and later delete, the authored file. ` + + `Map each entry to a different destination, or remove it if the file already lives in the output repository.`, + ); + } +} + +/** Which target claims a path, and the path as that target names it. */ +type Claim = { target: TargetName; absolute: string }; + +/** + * Every path this build writes, keyed two ways. `byWrite` keys the file a + * write lands on, with every symlink resolved, the final entry included: + * writing through a symlinked `SKILL.md` changes the file it points at. Write + * collisions compare these. `byEntry` keys the entry itself (parent resolved, + * final name kept), which is what a prune unlinks; ownership compares these. + */ +type IncomingPaths = { + byWrite: Map; + byEntry: Map; +}; + /** * Two targets pointed at overlapping output paths would silently overwrite each * other (and one target's prune could delete the other's files). Catch it. - * Returns the absolute-path -> owning-target map so - * `assertNoCollisionsWithBuiltTargets` can reuse it. + * Keys on physical paths, so `out` and `OUT` on a case-insensitive filesystem, + * an outDir reached through a symlink, or a generated file that is itself a + * symlink to another target's file, are one path, as they are on disk. + * Returns the keyed claims so later checks can reuse them. */ -function assertNoCrossTargetCollisions( +async function assertNoCrossTargetCollisions( artifacts: Artifact[], -): Map { - const owner = new Map(); +): Promise { + const byWrite = new Map(); + const byEntry = new Map(); const collisions: string[] = []; for (const artifact of artifacts) { for (const managedPath of artifact.managedPaths) { const absolute = path.resolve(artifact.outDir, managedPath); - const previous = owner.get(absolute); - if (previous && previous !== artifact.target) { - collisions.push(` ${previous} and ${artifact.target}: ${absolute}`); - } else { - owner.set(absolute, artifact.target); + const claim = { target: artifact.target, absolute }; + const writeKey = await physicalKey(absolute); + const previous = byWrite.get(writeKey); + if (previous && previous.target !== artifact.target) { + collisions.push( + ` ${previous.target} and ${artifact.target}: ${describeAlias(previous.absolute, absolute)}`, + ); + continue; + } + byWrite.set(writeKey, claim); + const entryKey = await physicalKey(absolute, { entry: true }); + if (!byEntry.has(entryKey)) { + byEntry.set(entryKey, claim); } } } if (collisions.length > 0) { throw new Error( - `Targets write overlapping output paths; give them distinct outDirs:\n${collisions.join("\n")}`, + `Targets write overlapping output paths; give them distinct outDirs:\n${collisions.join("\n")}\n` + + `If an output path is a symlink to another target's file, replace it with a regular file or remove it.`, ); } - return owner; + return { byWrite, byEntry }; } /** @@ -113,13 +256,17 @@ function assertNoCrossTargetCollisions( * against that by also checking incoming paths against every other * *configured* target's on-disk managed manifest, not just artifacts present * in the current invocation. + * + * Returns the physical key of every path those other manifests claim, so + * prune can leave a stale path alone when another target owns it now. */ async function assertNoCollisionsWithBuiltTargets( project: ResolvedProject, targets: TargetName[], - incoming: Map, -): Promise { + incoming: IncomingPaths, +): Promise> { const building = new Set(targets); + const claimed = new Map(); const collisions: string[] = []; for (const other of targetNames) { if (building.has(other)) { @@ -136,10 +283,16 @@ async function assertNoCollisionsWithBuiltTargets( } for (const file of manifest.files) { const absolute = path.resolve(otherOutDir, normalizeManagedPath(file)); - const conflicting = incoming.get(absolute); + const key = await managedPathKey(otherOutDir, other, file, { + entry: true, + }); + claimed.set(key, other); + const conflicting = + incoming.byEntry.get(key) ?? + incoming.byWrite.get(await managedPathKey(otherOutDir, other, file)); if (conflicting) { collisions.push( - ` ${other} (previously built) and ${conflicting}: ${absolute}`, + ` ${other} (previously built) and ${conflicting.target}: ${describeAlias(absolute, conflicting.absolute)}`, ); } } @@ -150,4 +303,101 @@ async function assertNoCollisionsWithBuiltTargets( `Run a full \`build\` (no --target) to see every target's paths at once, or fix the outDir configuration.`, ); } + return claimed; +} + +/** Where a target's managed manifest lives: `/.pluginpack/.json`. */ +export type ManifestLocation = { target: TargetName; outDir: string }; + +/** + * Managed manifests in the outDirs this config uses that no target builds + * into: a target the config dropped, or one whose outDir moved. Switching an + * outDir from codex to agent-plugins, say, leaves `.pluginpack/codex.json` + * behind. No configured target prunes the files it lists, so they would stay + * forever, and some (a codex hooks file inside an agent-plugins package) + * make the new output invalid. + * + * Scans the outDir of every configured target, plus `building` (the outDirs + * of this invocation, which a `--out-dir` override moves). A manifest is + * expected where its target is configured to write or is writing now. + */ +export async function findOrphanManifests( + project: ResolvedProjectConfig, + building: ManifestLocation[] = [], +): Promise { + const expected = new Map>(); + const scanned = new Map(); + const add = async ({ target, outDir }: ManifestLocation) => { + const key = await physicalKey(outDir); + expected.set(target, (expected.get(target) ?? new Set()).add(key)); + if (!scanned.has(key)) { + scanned.set(key, outDir); + } + }; + for (const [target, config] of Object.entries(project.config.targets)) { + if (config) { + await add({ + target: target as TargetName, + outDir: path.resolve(project.rootDir, config.outDir), + }); + } + } + for (const location of building) { + await add(location); + } + const known = new Set(targetNames); + const orphans: ManifestLocation[] = []; + for (const [key, outDir] of scanned) { + const names = await fs + .readdir(path.join(outDir, ".pluginpack")) + .catch(() => [] as string[]); + for (const name of names.sort()) { + const target = name.endsWith(".json") ? name.slice(0, -5) : ""; + if (!known.has(target)) { + continue; + } + if (!expected.get(target as TargetName)?.has(key)) { + orphans.push({ target: target as TargetName, outDir }); + } + } + } + return orphans; +} + +/** + * Refuses to build or prune while an outDir holds a manifest no target builds + * there (see `findOrphanManifests`). + * + * Pruning those files instead would delete on the authority of a manifest + * alone, with no config left describing that output: a target dropped from + * the config by mistake, or a second project that shares the output + * repository, would lose its files without a word. Refusing costs one + * command, and `clean --target ` (which finds these manifests) deletes + * the files through the same guard and checks as any clean. + */ +async function assertNoOrphanManifests( + project: ResolvedProjectConfig, + building: ManifestLocation[], +): Promise { + const orphans = await findOrphanManifests(project, building); + if (orphans.length === 0) { + return; + } + const lines = orphans.map( + ({ target, outDir }) => + ` ${path.join(outDir, managedManifestPath(target))} (${target})`, + ); + const commands = [...new Set(orphans.map(({ target }) => target))] + .map((target) => `\`pluginpack clean --target ${target}\``) + .join(", then "); + throw new Error( + `Found managed output from a target this config does not build there:\n${lines.join("\n")}\n` + + `No configured target prunes the files these manifests list, so they would stay in the output. ` + + `Run ${commands} to delete them first. To keep the files unmanaged instead, delete the manifest.`, + ); +} + +/** One path, or both spellings when two different strings name the same file. */ +export function describeAlias(first: string, second: string): string { + return first === second ? first : `${first} (same file as ${second})`; } diff --git a/src/cleanup.ts b/src/cleanup.ts index 3037f14..0ee3948 100644 --- a/src/cleanup.ts +++ b/src/cleanup.ts @@ -1,13 +1,24 @@ import path from "node:path"; -import { build } from "./build.js"; +import { + describeAlias, + findOrphanManifests, + planBuild, + type ManifestLocation, +} from "./build.js"; import { targetNames } from "./adapters.js"; import { loadProjectConfig } from "./config.js"; +import { withPathCache } from "./fs.js"; import { buildDeleteGuard, - cleanManagedFiles, + cleanResult, + executeClean, + executePrune, + managedPathKey, + planClean, normalizeManagedPath, - pruneManagedFiles, + pruneResult, readManagedManifest, + type CleanPlan, } from "./managed.js"; import type { CleanupResult, @@ -15,7 +26,12 @@ import type { TargetName, } from "./types.js"; -/** Deletes managed files no longer produced by the current build, without rewriting current output. */ +/** + * Deletes managed files no longer produced by the current build, without + * rewriting current output. Runs the same plan, and so the same checks, as + * `build`: the delete guard, cross-target ownership of stale paths, and every + * collision check. `--dry-run` runs them too and only skips the deletes. + */ export async function prune( options: { cwd?: string; @@ -25,21 +41,21 @@ export async function prune( force?: boolean; } = {}, ): Promise { - const project = await loadProjectConfig(options.cwd, options.configPath); - const guard = await buildDeleteGuard(project, options.force); - const artifacts = await build({ - cwd: options.cwd, - configPath: options.configPath, - target: options.target, - dryRun: true, - }); - const results: CleanupResult[] = []; - for (const artifact of artifacts) { - results.push( - await pruneManagedFiles(artifact, { dryRun: options.dryRun, guard }), - ); + const { prunes } = await planBuild( + { + cwd: options.cwd, + configPath: options.configPath, + target: options.target, + dryRun: options.dryRun, + }, + options.force, + ); + if (!options.dryRun) { + for (const plan of prunes) { + await executePrune(plan); + } } - return results; + return prunes.map(pruneResult); } /** Deletes every managed file for the given target(s), tearing the generated output down entirely. */ @@ -52,74 +68,109 @@ export async function clean( force?: boolean; } = {}, ): Promise { - const project = await loadProjectConfig(options.cwd, options.configPath); - const guard = await buildDeleteGuard(project, options.force); - const targets = options.target - ? [options.target] - : (Object.keys(project.config.targets) as TargetName[]); - await assertNoManifestCollisions(project, targets, options.force); - const results: CleanupResult[] = []; - for (const target of targets) { - const targetConfig = project.config.targets[target]; - if (!targetConfig) { - throw new Error(`Target "${target}" is not configured.`); + // Check every target before deleting anything, so a refusal for one target + // does not leave another already torn down. + const plans = await withPathCache(async () => { + const project = await loadProjectConfig(options.cwd, options.configPath); + const guard = await buildDeleteGuard(project, options.force); + const configured = configuredLocations(project); + const orphans = await findOrphanManifests(project); + const selected = (location: ManifestLocation) => + !options.target || location.target === options.target; + // A target the config dropped, or one whose outDir moved, left its + // manifest in an outDir this config uses; cleaning it is how a user + // removes that output, so it counts as that target's to clean. + const cleaning = [...configured, ...orphans].filter(selected); + if (options.target && cleaning.length === 0) { + throw new Error( + `Target "${options.target}" is not configured, and no outDir this config uses holds a .pluginpack/${options.target}.json manifest.`, + ); } - const outDir = path.resolve(project.rootDir, targetConfig.outDir); - results.push( - await cleanManagedFiles(outDir, target, { - dryRun: options.dryRun, - guard, - }), + await assertNoManifestCollisions( + [...configured, ...orphans], + cleaning, + options.force, ); + const planned: CleanPlan[] = []; + for (const { target, outDir } of cleaning) { + planned.push( + await planClean(outDir, target, { dryRun: options.dryRun, guard }), + ); + } + return planned; + }); + if (!options.dryRun) { + for (const plan of plans) { + await executeClean(plan); + } } - return results; + return plans.map(cleanResult); +} + +/** Where each configured target's managed manifest lives, in registry order. */ +function configuredLocations( + project: ResolvedProjectConfig, +): ManifestLocation[] { + return targetNames.flatMap((target) => { + const config = project.config.targets[target]; + return config + ? [{ target, outDir: path.resolve(project.rootDir, config.outDir) }] + : []; + }); } /** - * Refuses to clean a path another configured target's manifest also claims. + * Refuses to clean a path another target's manifest also claims. * - * `build()` rejects two targets writing overlapping output paths, and `prune()` - * inherits that check for free by running `build({ dryRun: true })` first. - * `clean()` deliberately never builds — teardown has to keep working when the - * source tree or config no longer does, which is often exactly why someone is - * cleaning — so it cannot inherit the check the same way. This is the - * standalone equivalent, working purely from the manifests on disk. + * `build()` and `prune()` share one plan that rejects overlapping output and + * leaves a stale path alone when another target owns it. `clean()` + * deliberately never builds — teardown has to keep working when the source + * tree or config no longer does, which is often exactly why someone is + * cleaning — so it cannot share that plan. This is the standalone check, + * working purely from the manifests on disk. * - * Overlapping manifests can only come from a pluginpack older than that build - * check, or from a manifest edited out of band; either way the file being - * deleted is another target's live output, so refuse rather than delete it. - * `--force` still wins, so nobody ends up unable to tear down their own repo. + * Paths are compared physically, so two outDirs that differ only in case on a + * case-insensitive filesystem, or that alias one directory through a symlink, + * collide here just as they do on disk. Overlapping manifests otherwise come + * from a pluginpack older than the build check, or from a manifest edited out + * of band; either way the file being deleted is another target's live output, + * so refuse rather than delete it. `--force` still wins, so nobody ends up + * unable to tear down their own repo. */ async function assertNoManifestCollisions( - project: ResolvedProjectConfig, - cleaning: TargetName[], + manifests: ManifestLocation[], + cleaning: ManifestLocation[], force?: boolean, ): Promise { if (force) { return; } - const owners = new Map(); + const isCleaning = (location: ManifestLocation) => + cleaning.includes(location); + const owners = new Map< + string, + { location: ManifestLocation; absolute: string } + >(); const collisions: string[] = []; - for (const target of targetNames) { - const targetConfig = project.config.targets[target]; - if (!targetConfig) { - continue; - } - const outDir = path.resolve(project.rootDir, targetConfig.outDir); + for (const location of manifests) { + const { target, outDir } = location; const manifest = await readManagedManifest(outDir, target); if (!manifest) { continue; } for (const file of manifest.files) { const absolute = path.resolve(outDir, normalizeManagedPath(file)); - const owner = owners.get(absolute); - if (owner && owner !== target) { - if (cleaning.includes(owner) || cleaning.includes(target)) { - collisions.push(` ${owner} and ${target}: ${absolute}`); + const key = await managedPathKey(outDir, target, file, { entry: true }); + const owner = owners.get(key); + if (owner && owner.location !== location) { + if (isCleaning(owner.location) || isCleaning(location)) { + collisions.push( + ` ${owner.location.target} and ${target}: ${describeAlias(owner.absolute, absolute)}`, + ); } continue; } - owners.set(absolute, target); + owners.set(key, { location, absolute }); } } if (collisions.length > 0) { diff --git a/src/cli.ts b/src/cli.ts index 84ac224..4eb7db5 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -173,12 +173,14 @@ function createProgram(): Command { program .command("clean") - .description("Remove all managed files for configured target outputs.") + .description( + "Remove all managed files in configured target outputs, including those of a target the config no longer builds there.", + ) .usage(`[--target ${targetList}] [--dry-run]`) .addOption( new Option( "--target ", - "Clean only one configured target.", + "Clean only one target. It may be one the config no longer builds, if its manifest is in a configured outDir.", ).choices([...targetNames]), ) .option("--dry-run", "Print managed files without deleting them.") diff --git a/src/config.ts b/src/config.ts index 29322b0..07006f8 100644 --- a/src/config.ts +++ b/src/config.ts @@ -4,7 +4,7 @@ import { pathToFileURL } from "node:url"; import { createJiti } from "jiti"; import { z } from "zod"; import { componentDirs } from "./components.js"; -import { exists } from "./fs.js"; +import { exists, isInside, physicalKey } from "./fs.js"; import { loadPartials } from "./partials.js"; import { configSchema, sourcePluginManifestSchema } from "./schema.js"; import { createFilesystemSourceProvider } from "./source.js"; @@ -27,6 +27,7 @@ export async function loadConfig( configPath?: string, ): Promise { const projectConfig = await loadProjectConfig(cwd, configPath); + await assertOutDirsOutsideSource(projectConfig); const { config, rootDir } = projectConfig; const sourceRoot = path.resolve(rootDir, config.source?.plugins ?? "plugins"); const plugins = await discoverSourcePlugins(sourceRoot); @@ -68,8 +69,12 @@ export async function loadProjectConfig( const resolvedConfigPath = configPath ? path.resolve(cwd, configPath) : await findConfig(cwd); + // No module cache: a long-lived process (a watcher, a test harness) that + // calls build() again after the config file changed must get the new + // config, not the module jiti evaluated the first time. const jiti = createJiti(pathToFileURL(resolvedConfigPath).href, { interopDefault: true, + moduleCache: false, }); const loaded = await jiti.import(resolvedConfigPath, { default: true }); const config = parseWithContext( @@ -148,19 +153,207 @@ async function discoverSourcePlugins( return plugins; } +/** An authored path the delete guard protects, with the config field that names it. */ +export type SourcePath = { + path: string; + field: string; +}; + /** - * Directories under `sourceRoot` that are actually source plugins. + * Every authored path the config reads from: the paths prune and clean must + * never delete and a target's outDir must never sit inside. * - * The delete guard needs this to protect source without protecting generated - * output: the recommended layout writes every target's output under - * `plugins//`, which is also the default source-plugin root, so - * protecting that root wholesale refuses to prune the tool's own output. A - * shallow filesystem scan is deliberate — `clean` must keep working when the - * config or source tree no longer loads. + * Derived from the config, not from what is on disk, so `clean` keeps working + * when the source tree no longer loads. `source.plugins` is protected whole + * only when the config sets it. Under the default `plugins/` root — where the + * README's layouts also write generated output — only the legacy source + * plugins a target actually reads through `from`, and directories carrying a + * `plugin.pluginpack.json` (which generated output never contains), count as + * source here. Other directories legacy discovery would read as source plugins + * are listed by `listUnreferencedSourcePluginDirs`; the delete guard protects + * them unless pluginpack recorded writing every file in them, since guessing + * from component directories alone misreads a generated package + * (`plugins/acme/skills/`) as source and blocks its own prune. */ -export async function listSourcePluginDirs( - sourceRoot: string, +export async function listSourcePaths( + project: ResolvedProjectConfig, +): Promise { + const { rootDir, config } = project; + const paths: SourcePath[] = []; + const add = (value: string, field: string) => + paths.push({ path: path.resolve(rootDir, value), field }); + if (config.source?.skills) { + add(config.source.skills, "source.skills"); + } + if (config.source?.partials) { + add(config.source.partials, "source.partials"); + } + if (config.source?.plugins) { + add(config.source.plugins, "source.plugins"); + } else { + const defaultRoot = path.resolve(rootDir, "plugins"); + const legacyIds = referencedLegacyIds(project); + for (const id of legacyIds) { + paths.push({ + path: path.join(defaultRoot, id), + field: `legacy source plugin "${id}" (from)`, + }); + } + for (const dir of await listManifestPluginDirs(defaultRoot)) { + if (!legacyIds.has(path.basename(dir))) { + paths.push({ path: dir, field: "plugin.pluginpack.json" }); + } + } + } + for (const [name, target] of Object.entries(config.targets)) { + if (!target) { + continue; + } + if (target.repositoryFiles) { + add(target.repositoryFiles, `targets.${name}.repositoryFiles`); + } + for (const [dest, source] of Object.entries(target.rootFiles ?? {})) { + add(source, `targets.${name}.rootFiles["${dest}"]`); + } + for (const [pluginName, plugin] of Object.entries(target.plugins)) { + if (plugin.source) { + add(plugin.source, `targets.${name}.plugins.${pluginName}.source`); + } + if (plugin.overrides) { + add( + plugin.overrides, + `targets.${name}.plugins.${pluginName}.overrides`, + ); + } + } + } + return paths; +} + +/** + * Legacy source plugin ids a target reads through `from`, except the root + * skills plugin, which reads only `source.skills`. + */ +function referencedLegacyIds(project: ResolvedProjectConfig): Set { + const { config } = project; + const rootPluginId = config.source?.skills + ? (config.source.rootPlugin?.id ?? "core") + : undefined; + const ids = new Set(); + for (const target of Object.values(config.targets)) { + for (const plugin of Object.values(target?.plugins ?? {})) { + for (const id of plugin.from ?? []) { + if (id !== rootPluginId) { + ids.add(id); + } + } + } + } + return ids; +} + +/** + * Directories under the default `plugins/` root that legacy discovery reads + * as source plugins (a component directory, no `plugin.pluginpack.json`) but + * that `listSourcePaths` does not already protect, because no target reads + * them through `from`. Such a directory is either authored source nobody + * references yet or a generated package (`plugins/acme/skills/` under + * `outDir: "."`); the delete guard tells them apart by provenance. Empty when + * the config sets `source.plugins`, which is then protected whole. + */ +export async function listUnreferencedSourcePluginDirs( + project: ResolvedProjectConfig, ): Promise { + if (project.config.source?.plugins) { + return []; + } + const defaultRoot = path.resolve(project.rootDir, "plugins"); + const legacyIds = referencedLegacyIds(project); + const dirs: string[] = []; + for (const dir of await listSourcePluginDirs(defaultRoot)) { + if ( + !legacyIds.has(path.basename(dir)) && + !(await exists(path.join(dir, "plugin.pluginpack.json"))) + ) { + dirs.push(dir); + } + } + return dirs; +} + +/** + * Rejects a config whose target outDir physically resolves inside an authored + * path: building there overwrites source, and prune/clean would then have to + * delete it. Compares physical paths, so an outDir that only reaches source + * through a symlink, or through a case variant the filesystem forgives, is + * caught too. An outDir that is merely a symlink to somewhere safe still works. + * + * Checks every configured outDir, or only `outDirs` when given (the absolute + * outDirs a `--out-dir` override produced). + */ +export async function assertOutDirsOutsideSource( + project: ResolvedProjectConfig, + outDirs?: Array<{ target: string; outDir: string }>, +): Promise { + const sources = await listSourcePaths(project); + const sourceKeys = await Promise.all( + sources.map(async (source) => ({ + ...source, + key: await physicalKey(source.path), + })), + ); + const checked = + outDirs ?? + Object.entries(project.config.targets).flatMap(([target, config]) => + config + ? [{ target, outDir: path.resolve(project.rootDir, config.outDir) }] + : [], + ); + const problems: string[] = []; + for (const { target: name, outDir } of checked) { + const outKey = await physicalKey(outDir); + for (const source of sourceKeys) { + if (isInside(source.key, outKey)) { + problems.push( + ` targets.${name}.outDir (${outDir}) resolves inside ${source.path} (${source.field})`, + ); + } + } + } + if (problems.length > 0) { + throw new Error( + `Invalid pluginpack config in ${project.configPath}: a target writes its output inside an authored source path:\n${problems.join("\n")}\n` + + `A build there overwrites source files, and prune or clean would delete them. ` + + `Point each outDir at a directory outside every source, overrides, partials, repositoryFiles and rootFiles path.`, + ); + } +} + +/** + * Directories directly under `sourceRoot` that carry a + * `plugin.pluginpack.json`. Generated output never contains that file, so + * these are authored source whether or not a target reads them. + */ +async function listManifestPluginDirs(sourceRoot: string): Promise { + if (!(await exists(sourceRoot))) { + return []; + } + const dirs: string[] = []; + const entries = await fs.readdir(sourceRoot, { withFileTypes: true }); + for (const entry of entries) { + const dir = path.join(sourceRoot, entry.name); + if ( + entry.isDirectory() && + (await exists(path.join(dir, "plugin.pluginpack.json"))) + ) { + dirs.push(dir); + } + } + return dirs; +} + +/** Directories under `sourceRoot` that legacy discovery reads as source plugins. */ +async function listSourcePluginDirs(sourceRoot: string): Promise { if (!(await exists(sourceRoot))) { return []; } diff --git a/src/fs.ts b/src/fs.ts index 1b77464..bcf5112 100644 --- a/src/fs.ts +++ b/src/fs.ts @@ -1,3 +1,5 @@ +import { AsyncLocalStorage } from "node:async_hooks"; +import { randomBytes } from "node:crypto"; import { promises as fs } from "node:fs"; import path from "node:path"; import type { FileValue } from "./types.js"; @@ -12,7 +14,9 @@ export function toPosix(value: string): string { * Dotfiles are included. Symlinks are followed: a link to a file is listed * as a file and a link to a directory is walked, except a link back to a * directory already being walked, which would loop. Broken links are - * skipped. A missing `dir` yields no files. + * skipped. A missing `dir` yields no files; a `dir` that is a file is an + * error, since reading it as an empty directory would silently drop content + * the author meant to ship. */ export async function walkFiles(dir: string): Promise { const files: string[] = []; @@ -23,6 +27,12 @@ export async function walkFiles(dir: string): Promise { real = await fs.realpath(current); entries = await fs.readdir(current, { withFileTypes: true }); } catch (err) { + if (await isExistingNonDirectory(err, current)) { + throw new Error( + `Expected a directory but found a file: ${current}. Move the file into a directory of that name, or remove it.`, + { cause: err }, + ); + } if (isNotFoundError(err)) { return; } @@ -73,6 +83,231 @@ function fold(value: string): string { return value.normalize("NFC").toLowerCase(); } +/** + * The key two paths share when they name the same file on this host: the + * physical path (symlinks resolved on the deepest existing ancestor), folded + * for case and Unicode normalization when that ancestor's volume compares + * names case-insensitively (APFS and NTFS by default). + * + * Every check that decides whether a delete or write is safe, or whether two + * targets claim the same file, compares these keys. Comparing `path.resolve` + * strings instead lets an alias walk past the check: `out` and `OUT` on APFS, + * or a symlink into a source tree, are different strings but one file. + * + * With `entry` set, only the parent is resolved and the final name is kept, + * which is the file `fs.rm` removes when the final entry is itself a symlink. + * Use it for deletions and ownership; leave it off for a root being protected. + */ +export async function physicalKey( + target: string, + options: { entry?: boolean } = {}, +): Promise { + const absolute = path.resolve(target); + const parent = path.dirname(absolute); + const real = + options.entry && parent !== absolute + ? path.join(await cachedRealpathDeep(parent), path.basename(absolute)) + : await cachedRealpathDeep(absolute); + return (await isCaseInsensitiveAt(real)) ? fold(real) : real; +} + +/** Lookups `physicalKey` memoizes inside `withPathCache`. */ +type PathCache = { + realpaths: Map>; + directories: Map>; + caseInsensitiveDevices: Map>; +}; + +const pathCache = new AsyncLocalStorage(); + +/** + * Runs `fn` with `physicalKey` memoizing real paths and per-volume case + * sensitivity, so a preflight over thousands of files resolves each + * directory and probes each volume once rather than once per file. + * + * The memo assumes the paths it has seen do not change while `fn` runs. Wrap + * read-only checks only, never code that writes or deletes. Nested calls share + * the outer cache. + */ +export function withPathCache(fn: () => Promise): Promise { + if (pathCache.getStore()) { + return fn(); + } + return pathCache.run( + { + realpaths: new Map(), + directories: new Map(), + caseInsensitiveDevices: new Map(), + }, + fn, + ); +} + +function memoize( + map: Map> | undefined, + key: K, + compute: () => Promise, +): Promise { + if (!map) { + return compute(); + } + let value = map.get(key); + if (!value) { + value = compute(); + map.set(key, value); + } + return value; +} + +/** `realpathDeep`, memoized per path (and so per ancestor) inside `withPathCache`. */ +function cachedRealpathDeep(target: string): Promise { + const absolute = path.resolve(target); + return memoize(pathCache.getStore()?.realpaths, absolute, async () => { + try { + return await fs.realpath(absolute); + } catch (error) { + if (!isNotFoundError(error)) { + throw error; + } + const parent = path.dirname(absolute); + if (parent === absolute) { + return absolute; + } + return path.join( + await cachedRealpathDeep(parent), + path.basename(absolute), + ); + } + }); +} + +/** + * Whether names on the volume that holds `real` compare case-insensitively. + * `real` may not exist yet; the volume is the one holding its nearest + * existing directory, since that is where the missing tail will be created. + */ +async function isCaseInsensitiveAt(real: string): Promise { + const nearest = await nearestDirectory(real); + if (!nearest) { + return platformFoldsCase(); + } + return memoize( + pathCache.getStore()?.caseInsensitiveDevices, + nearest.dev, + () => probeVolumeCase(nearest.dir, nearest.dev), + ); +} + +/** The nearest existing directory at or above `real`, and its device. */ +function nearestDirectory( + real: string, +): Promise<{ dir: string; dev: number } | null> { + return memoize(pathCache.getStore()?.directories, real, async () => { + // lstat: `real` is resolved except, for an entry key, its final name, + // which may be a symlink. That name lives in its parent's directory. + const stats = await fs.lstat(real).catch(() => null); + if (stats?.isDirectory()) { + return { dir: real, dev: stats.dev }; + } + const parent = path.dirname(real); + return parent === real ? null : nearestDirectory(parent); + }); +} + +/** + * Probes whether the volume `dir` lives on (device `dev`) folds case. + * + * A name lookup in a directory is answered by the directory's own + * filesystem, so the probe looks up names inside `dir`, or inside an + * ancestor on the same device, never a mount point's name in the parent + * volume. For an existing name with a cased letter: if its case-swapped + * spelling is also listed, the directory holds both and is case-sensitive; + * otherwise, the volume folds case exactly when looking the swapped spelling + * up finds something. (Comparing inodes instead would be fooled by two hard + * links whose names differ only by case.) + * + * When no directory on the volume has a usable name, as on an empty volume + * just mounted, the probe creates and removes one empty, uniquely named file + * in `dir`. Only when that also fails does it fall back to the platform + * default, folding on macOS and Windows: over-matching is the safe direction + * for a check that decides whether to refuse. + */ +async function probeVolumeCase(dir: string, dev: number): Promise { + let current = dir; + for (;;) { + const answer = await probeDirectoryEntries(current); + if (answer !== undefined) { + return answer; + } + const parent = path.dirname(current); + if (parent === current) { + break; + } + const parentStats = await fs.stat(parent).catch(() => null); + if (!parentStats || parentStats.dev !== dev) { + break; + } + current = parent; + } + return (await probeWithScratchFile(dir)) ?? platformFoldsCase(); +} + +async function probeDirectoryEntries( + dir: string, +): Promise { + const names = await fs.readdir(dir).catch(() => null); + if (!names) { + return undefined; + } + const listed = new Set(names); + for (const name of names) { + const swapped = swapCase(name); + // Skip names whose case swap does not round-trip ("ß" -> "SS"): the + // swapped name would be a different name, not a different case. + if (swapped === name || swapCase(swapped) !== name) { + continue; + } + if (listed.has(swapped)) { + return false; + } + return fs.lstat(path.join(dir, swapped)).then( + () => true, + () => false, + ); + } + return undefined; +} + +async function probeWithScratchFile(dir: string): Promise { + const name = `.pluginpack-case-probe-${process.pid}-${randomBytes(6).toString("hex")}`; + const probe = path.join(dir, name); + try { + await fs.writeFile(probe, "", { flag: "wx" }); + } catch { + return null; + } + try { + return await fs.lstat(path.join(dir, swapCase(name))).then( + () => true, + () => false, + ); + } finally { + await fs.rm(probe, { force: true }); + } +} + +function platformFoldsCase(): boolean { + return process.platform === "darwin" || process.platform === "win32"; +} + +function swapCase(value: string): string { + return [...value] + .map((char) => + char === char.toLowerCase() ? char.toUpperCase() : char.toLowerCase(), + ) + .join(""); +} + /** * Resolves `target` with symlinks followed, or returns `null` if the result * escapes `root`. @@ -153,7 +388,12 @@ export function json(value: unknown): string { return `${JSON.stringify(value, null, 2)}\n`; } -/** Whether `value` is a URL, or a relative path that can't escape its base directory. */ +/** + * Whether `value` is a URL, or a relative path that can't escape its base + * directory. For a value that may be either, such as a marketplace entry's + * `source`. A value that is only ever read from or written to disk must use + * `isSafeRelativeFilePath` instead. + */ export function isSafeRelativePath(value: string): boolean { if (!value) { return false; @@ -161,11 +401,27 @@ export function isSafeRelativePath(value: string): boolean { if (value.startsWith("http://") || value.startsWith("https://")) { return true; } - if (path.isAbsolute(value)) { + return isSafeRelativeFilePath(value); +} + +/** + * Whether `value` is a relative filesystem path that can't escape its base + * directory. Unlike `isSafeRelativePath`, a URL gets no exemption: passed to + * `path.resolve`, `https://../../outside` is just a path with a `..` in it, + * and lands outside the base. + */ +export function isSafeRelativeFilePath(value: string): boolean { + if (!value || path.isAbsolute(value)) { return false; } const normalized = path.posix.normalize(value.replace(/\\/g, "/")); - return normalized !== ".." && !normalized.startsWith("../"); + return ( + normalized !== ".." && + !normalized.startsWith("../") && + !path.posix.isAbsolute(normalized) && + // A Windows drive-letter path resolves as absolute on Windows. + !/^[a-zA-Z]:/.test(normalized) + ); } /** Whether the file at `filePath` exists. */ @@ -193,3 +449,22 @@ export function isNotFoundError(error: unknown): boolean { const code = (error as NodeJS.ErrnoException).code; return code === "ENOENT" || code === "ENOTDIR"; } + +/** + * Whether `error` is an ENOTDIR raised because `dir` itself exists as a file, + * rather than because some ancestor of it is a file (which means `dir` is + * simply missing). + */ +async function isExistingNonDirectory( + error: unknown, + dir: string, +): Promise { + if ( + !(error instanceof Error) || + (error as NodeJS.ErrnoException).code !== "ENOTDIR" + ) { + return false; + } + const stats = await fs.stat(dir).catch(() => null); + return stats !== null && !stats.isDirectory(); +} diff --git a/src/managed.ts b/src/managed.ts index 86f86c4..483eaa6 100644 --- a/src/managed.ts +++ b/src/managed.ts @@ -1,26 +1,43 @@ +import { createHash } from "node:crypto"; import { promises as fs } from "node:fs"; import path from "node:path"; import { isInside, isInsideFolded, + isNotFoundError, json, + physicalKey, resolveInside, toPosix, + walkFiles, + writeArtifact, } from "./fs.js"; -import { listSourcePluginDirs } from "./config.js"; +import { listSourcePaths, listUnreferencedSourcePluginDirs } from "./config.js"; +import { targets as targetRegistry } from "./targets/registry.js"; import type { Artifact, CleanupEntry, CleanupResult, DeleteGuard, + FileValue, ResolvedProjectConfig, TargetName, } from "./types.js"; -type ManagedManifest = { - version: 1; +/** The newest managed-manifest version this pluginpack reads and the one it writes. */ +const MANIFEST_VERSION = 2; + +/** + * A target's managed-file manifest, as read. Version 2 records a sha256 of + * each file as written, so a later build, prune or clean can tell a file it + * wrote from one someone edited since. A version 1 manifest (paths only) reads + * as version 2 with no hashes and is upgraded by the next build. + */ +export type ManagedManifest = { + version: typeof MANIFEST_VERSION; target: TargetName; files: string[]; + sha256: Record; }; /** Output-relative path of a target's managed-file manifest. */ @@ -28,19 +45,71 @@ export function managedManifestPath(target: TargetName): string { return toPosix(path.join(".pluginpack", `${target}.json`)); } -/** Writes the list of files a build produced, for later prune/clean/diff to compare against. */ -export async function writeManagedManifest(artifact: Artifact): Promise { +/** The hex sha256 of a file's contents, as recorded in the managed manifest. */ +function contentHash(value: FileValue): string { + return createHash("sha256").update(value).digest("hex"); +} + +/** + * Writes the list of files a build produced, with their hashes, for later + * prune/clean/diff to compare against. Goes through the same containment check + * as the artifact itself, so a symlinked `.pluginpack/` cannot redirect the + * write outside the output directory. + */ +export async function writeManagedManifest( + artifact: Artifact, + retained: ManagedManifest | null = null, +): Promise { + const files = [...artifact.managedPaths]; + const sha256: Record = {}; + for (const managedPath of artifact.managedPaths) { + const value = artifact.files.get(managedPath); + if (value !== undefined) { + sha256[managedPath] = contentHash(value); + } + } + const current = new Set(artifact.managedPaths.map(normalizeManagedPath)); + for (const file of retained?.files ?? []) { + if (!current.has(normalizeManagedPath(file))) { + files.push(file); + const hash = retained?.sha256[file]; + if (hash !== undefined) { + sha256[file] = hash; + } + } + } const manifest: ManagedManifest = { - version: 1, + version: MANIFEST_VERSION, target: artifact.target, - files: artifact.managedPaths, + files, + sha256, }; - const destination = path.join( + await writeArtifact( artifact.outDir, - managedManifestPath(artifact.target), + new Map([[managedManifestPath(artifact.target), json(manifest)]]), ); - await fs.mkdir(path.dirname(destination), { recursive: true }); - await fs.writeFile(destination, json(manifest)); +} + +/** + * Throws if writing the target's managed manifest would leave the output + * directory. Build runs this before writing anything, so a planted + * `.pluginpack` symlink fails the build up front instead of after the + * artifact is already on disk. + */ +export async function assertManagedManifestContained( + outDir: string, + target: TargetName, +): Promise { + const relative = managedManifestPath(target); + if ( + (await resolveInside(path.resolve(outDir), path.join(outDir, relative))) === + null + ) { + throw new Error( + `Refusing to write through a symlink that leaves the output directory: ${relative} (under ${outDir}). ` + + `Replace the .pluginpack symlink with a real directory.`, + ); + } } /** Reads a target's managed-file manifest, or `null` if none exists yet. */ @@ -58,159 +127,459 @@ export async function readManagedManifest( } throw error; } - let parsed: Partial | null; + const invalid = (reason: string) => + new Error( + `Invalid managed manifest: ${manifestPath} (${reason}). ` + + `It records which files pluginpack may delete. Restore it from version control, ` + + `or delete it and rebuild; pluginpack then leaves any files it listed in place.`, + ); + let parsed: unknown; try { - parsed = JSON.parse(raw) as Partial | null; + parsed = JSON.parse(raw); } catch { - throw new Error(`Invalid managed manifest: ${manifestPath}`); + throw invalid("not valid JSON"); } + // `JSON.parse` happily returns null, a number, or a string for a truncated + // or hand-edited manifest; those must land on the same clear error as + // malformed JSON, not a TypeError from the field checks below. + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { + throw invalid("not a JSON object"); + } + const record = parsed as Record; + const version = record.version; if ( - // `JSON.parse` happily returns null, a number, or a string for a - // truncated or hand-edited manifest; those must land on the same clear - // error as malformed JSON, not a TypeError from the field checks below. - !parsed || - typeof parsed !== "object" || - Array.isArray(parsed) || - parsed.version !== 1 || - parsed.target !== target || - !Array.isArray(parsed.files) || - !parsed.files.every((file) => typeof file === "string") + typeof version === "number" && + Number.isInteger(version) && + version > MANIFEST_VERSION ) { - throw new Error(`Invalid managed manifest: ${manifestPath}`); + throw new Error( + `Managed manifest ${manifestPath} has version ${version}, so a newer pluginpack wrote it. ` + + `This pluginpack reads versions 1 to ${MANIFEST_VERSION}. Upgrade pluginpack to build, prune or clean this output.`, + ); + } + if (version !== 1 && version !== 2) { + throw invalid(`unsupported version ${JSON.stringify(version)}`); + } + if (record.target !== target) { + throw invalid( + `target is ${JSON.stringify(record.target)}, expected "${target}"`, + ); + } + const files = record.files; + if ( + !Array.isArray(files) || + !files.every((file): file is string => typeof file === "string") + ) { + throw invalid('"files" must be an array of paths'); + } + let sha256: Record = {}; + if (version === 2) { + const hashes = record.sha256; + if ( + !hashes || + typeof hashes !== "object" || + Array.isArray(hashes) || + !Object.values(hashes).every((hash) => typeof hash === "string") + ) { + throw invalid('"sha256" must map each path to a hash'); + } + sha256 = hashes as Record; + } + return { version: MANIFEST_VERSION, target, files, sha256 }; +} + +/** A hash a target's manifest recorded for a file it wrote. */ +type RecordedHash = { target: TargetName; sha256: string }; + +/** What a prune will delete, decided before anything is written or removed. */ +export type PrunePlan = { + artifact: Artifact; + previous: ManagedManifest | null; + stale: string[]; + /** + * For a path this target takes over from another target in this build, the + * hash the previous owner recorded, so an edit to it still warns. Filled + * in by `recordHandedOverHashes`. + */ + handedOver: Map; +}; + +/** + * Fills in each plan's `handedOver` hashes from the previous manifests of the + * other targets in the same build. A path that moves from one target to + * another (the agent-plugins Codex marketplace handed to the codex target, + * say) is new to the recipient's manifest, so without this an edit to it + * would be overwritten without a warning. + */ +export async function recordHandedOverHashes( + plans: PrunePlan[], +): Promise { + const recorded = new Map(); + for (const plan of plans) { + const { outDir, target } = plan.artifact; + for (const [file, sha256] of Object.entries(plan.previous?.sha256 ?? {})) { + const key = await managedPathKey(outDir, target, file, { entry: true }); + recorded.set(key, { target, sha256 }); + } + } + for (const plan of plans) { + const { outDir, target } = plan.artifact; + const own = new Set( + (plan.previous?.files ?? []).map((file) => normalizeManagedPath(file)), + ); + for (const file of plan.artifact.managedPaths) { + if (own.has(normalizeManagedPath(file))) { + continue; + } + const key = await managedPathKey(outDir, target, file, { entry: true }); + const previousOwner = recorded.get(key); + if (previousOwner && previousOwner.target !== target) { + plan.handedOver.set(file, previousOwner); + } + } } - return parsed as ManagedManifest; } -/** Deletes files the previous build managed but the current one no longer produces. */ -export async function pruneManagedFiles( +/** + * Decides which previously managed files are stale and checks it is safe to + * delete them, without touching disk. Runs on dry runs too, so `--dry-run` + * fails exactly where the real command would. + * + * `owners` maps the physical key of every path another target produces in + * this build, or claims in its own manifest, to that target. A stale path + * another target owns has changed hands (for example a marketplace file moved + * from one target to another sharing the outDir) and is left in place rather + * than deleted from under its new owner. This target's next manifest no + * longer lists it, so the ownership transfer completes on the next build. A + * stale path that is physically one of this target's own current paths (a + * case-only rename on a case-insensitive volume) is kept for the same reason. + */ +export async function planPrune( artifact: Artifact, - options: { dryRun?: boolean; guard?: DeleteGuard } = {}, -): Promise { + options: { + dryRun?: boolean; + guard?: DeleteGuard; + owners?: Map; + } = {}, +): Promise { const previous = await readManagedManifest(artifact.outDir, artifact.target); const current = new Set(artifact.managedPaths.map(normalizeManagedPath)); - const stale = (previous?.files ?? []) - .map(normalizeManagedPath) - .filter((file) => !current.has(file)); - if (!options.dryRun) { - assertNoProtectedDeletions(artifact.outDir, stale, options.guard, "prune"); - } - const entries: CleanupEntry[] = []; - for (const normalized of stale) { - entries.push({ - type: "stale", - target: artifact.target, - path: normalized, - }); - if (!options.dryRun) { - await removeManagedPath(artifact.outDir, normalized); + const stale: string[] = []; + for (const file of (previous?.files ?? []).map(normalizeManagedPath)) { + if (current.has(file)) { + continue; + } + if (options.owners) { + const key = await managedPathKey(artifact.outDir, artifact.target, file, { + entry: true, + }); + // Another owner means the path changed hands. This target as the owner + // means the stale spelling is one of its current files under another + // name (`first/` renamed `FIRST/` on a case-insensitive volume), and + // deleting it would delete the file just written. + if (options.owners.has(key)) { + continue; + } } + stale.push(file); + } + await assertNoProtectedDeletions( + artifact.outDir, + artifact.target, + stale, + options.guard, + "prune", + ); + await assertRemovalsContained(artifact.outDir, artifact.target, stale); + await warnOnEditedFiles( + artifact.outDir, + artifact.target, + stale, + ownHashes(artifact.target, previous), + options.dryRun ? "a real run would delete it" : "deleting it", + ); + return { artifact, previous, stale, handedOver: new Map() }; +} + +/** Deletes the stale files a `planPrune` decided on. */ +export async function executePrune(plan: PrunePlan): Promise { + for (const file of plan.stale) { + await removeManagedPath(plan.artifact.outDir, file, plan.artifact.target); } +} + +/** The cleanup report for a prune plan. */ +export function pruneResult(plan: PrunePlan): CleanupResult { return { - target: artifact.target, - outDir: artifact.outDir, - entries, + target: plan.artifact.target, + outDir: plan.artifact.outDir, + entries: plan.stale.map((file): CleanupEntry => ({ + type: "stale", + target: plan.artifact.target, + path: file, + })), }; } -/** Deletes every file the previous build managed for a target, including its manifest. */ -export async function cleanManagedFiles( +/** + * Warns before a build overwrites a managed file someone edited since the + * last build. The build still overwrites it: pluginpack owns its output, but + * the edit should not vanish without a word. + */ +export async function warnOnEditedOverwrites( + plan: PrunePlan, + dryRun?: boolean, +): Promise { + const { artifact, previous, handedOver } = plan; + const incoming = new Map(); + for (const [file, value] of artifact.files) { + incoming.set(file, contentHash(value)); + } + const own = ownHashes(artifact.target, previous); + await warnOnEditedFiles( + artifact.outDir, + artifact.target, + artifact.managedPaths, + (file) => own(file) ?? handedOver.get(file), + dryRun ? "a real build would overwrite it" : "overwriting it", + incoming, + ); +} + +/** What a clean will delete for one target, decided before anything is removed. */ +export type CleanPlan = { + target: TargetName; + outDir: string; + /** Every managed path, then the manifest itself; empty when no manifest exists. */ + paths: string[]; +}; + +/** + * Checks it is safe to delete every file the previous build managed for a + * target, without touching disk: the delete guard, containment of every path, + * and a warning for each edited file. Runs on dry runs too. + */ +export async function planClean( outDir: string, target: TargetName, options: { dryRun?: boolean; guard?: DeleteGuard } = {}, -): Promise { +): Promise { const previous = await readManagedManifest(outDir, target); - const entries: CleanupEntry[] = []; if (!previous) { - return { target, outDir, entries }; + return { target, outDir, paths: [] }; } - const files = (previous.files ?? []).map(normalizeManagedPath); - if (!options.dryRun) { - assertNoProtectedDeletions(outDir, files, options.guard, "clean"); + const files = previous.files.map(normalizeManagedPath); + await assertNoProtectedDeletions( + outDir, + target, + files, + options.guard, + "clean", + ); + await assertRemovalsContained(outDir, target, files); + await warnOnEditedFiles( + outDir, + target, + files, + ownHashes(target, previous), + options.dryRun ? "a real run would delete it" : "deleting it", + ); + return { target, outDir, paths: [...files, managedManifestPath(target)] }; +} + +/** Deletes what a `planClean` decided on, the manifest last. */ +export async function executeClean(plan: CleanPlan): Promise { + for (const file of plan.paths) { + await removeManagedPath(plan.outDir, file, plan.target); } - for (const normalized of files) { - entries.push({ type: "deleted", target, path: normalized }); - if (!options.dryRun) { - await removeManagedPath(outDir, normalized); +} + +/** The cleanup report for a clean plan. */ +export function cleanResult(plan: CleanPlan): CleanupResult { + return { + target: plan.target, + outDir: plan.outDir, + entries: plan.paths.map((file): CleanupEntry => ({ + type: "deleted", + target: plan.target, + path: file, + })), + }; +} + +/** + * Prints a warning for each of `files` whose on-disk content no longer + * matches the hash the manifest recorded when pluginpack wrote it. Files with + * no recorded hash (a version 1 manifest) and files already gone are skipped, + * as are files whose edit already matches what `replacements` will write. + */ +async function warnOnEditedFiles( + outDir: string, + target: TargetName, + files: string[], + recordedHash: (file: string) => RecordedHash | undefined, + action: string, + replacements?: Map, +): Promise { + for (const file of files) { + const recorded = recordedHash(file); + if (recorded === undefined) { + continue; + } + let contents: Buffer; + try { + contents = await fs.readFile(path.resolve(outDir, file)); + } catch { + continue; + } + const actual = contentHash(contents); + if (actual !== recorded.sha256 && actual !== replacements?.get(file)) { + const from = + recorded.target === target + ? "" + : ` (handed over from ${recorded.target})`; + console.warn( + `warning: ${target} managed file ${path.join(outDir, file)}${from} was edited after pluginpack wrote it; ${action}. ` + + `Keep the edit by moving it into the source, or recover it from version control.`, + ); } } - const manifestPath = managedManifestPath(target); - entries.push({ type: "deleted", target, path: manifestPath }); - if (!options.dryRun) { - await removeManagedPath(outDir, manifestPath); - } - return { target, outDir, entries }; +} + +/** Looks up the hashes a target's own previous manifest recorded. */ +function ownHashes( + target: TargetName, + previous: ManagedManifest | null, +): (file: string) => RecordedHash | undefined { + return (file) => { + const sha256 = previous?.sha256[file]; + return sha256 === undefined ? undefined : { target, sha256 }; + }; } /** - * Builds the guard that stops prune/clean from deleting the config or a source tree. - * - * `source.plugins` gets different treatment depending on whether it was set: - * when the user names a directory as their source-plugin root, that whole - * directory is protected. When they don't, the default root is `plugins/` — - * which is also where the recommended layout writes every target's output, so - * protecting it wholesale would refuse to prune generated files. Under the - * default, protect only the directories that really are source plugins. + * Builds the guard that stops prune/clean from deleting the config, a source + * tree, or an output repository's `.git` directory. * - * Takes the loaded config rather than plugin discovery results, so `clean` keeps - * working when the source tree no longer loads: the scan below is a shallow - * readdir, not a build. + * Takes the loaded config rather than plugin discovery results, so `clean` + * keeps working when the source tree no longer loads. See `listSourcePaths` + * for which authored paths count as source. */ export async function buildDeleteGuard( project: ResolvedProjectConfig, force?: boolean, ): Promise { - const { rootDir, config } = project; - const protectedRoots: string[] = []; - if (config.source?.skills) { - protectedRoots.push(path.resolve(rootDir, config.source.skills)); - } - if (config.source?.partials) { - protectedRoots.push(path.resolve(rootDir, config.source.partials)); - } - if (config.source?.plugins) { - protectedRoots.push(path.resolve(rootDir, config.source.plugins)); - } else { - protectedRoots.push( - ...(await listSourcePluginDirs(path.resolve(rootDir, "plugins"))), - ); + const protectedRoots = (await listSourcePaths(project)).map( + (source) => source.path, + ); + protectedRoots.push(...(await listDiscoveredSourceDirs(project))); + // An outDir can sit outside the source repo (a sibling output repo), where + // none of the source paths above apply. Its version-control data is the one + // thing there no manifest should ever name. + for (const target of Object.values(project.config.targets)) { + if (target) { + protectedRoots.push(path.resolve(project.rootDir, target.outDir, ".git")); + } + } + return { + protectedRoots, + configPath: path.resolve(project.configPath), + force, + }; +} + +/** + * The directories legacy discovery reads as source plugins that nothing else + * protects, minus the ones pluginpack provably generated. + * + * A directory counts as generated only when every file in it (managed + * manifests aside) is one pluginpack recorded writing. A file counts when a + * configured target's managed manifest lists it with a hash, or lists it + * without one (a version 1 manifest from 0.11 or earlier) and the file sits + * inside a plugin directory that same target's config places its output in, + * such as `plugins/acme` for `codex` with `outDir: "."`. The second case + * keeps a project upgrading from 0.11 able to prune a path the new build no + * longer writes; without it the first build refuses and the manifest never + * gains hashes. Anything else is treated as authored and protected: a + * directory a version 1 manifest names but no target's config writes to, and + * one holding any file no manifest lists. Absence of a `from` reference is not + * evidence: authored legacy source does not need one to exist. + */ +async function listDiscoveredSourceDirs( + project: ResolvedProjectConfig, +): Promise { + const dirs = await listUnreferencedSourcePluginDirs(project); + if (dirs.length === 0) { + return []; } - for (const target of Object.values(config.targets)) { + const written = new Set(); + for (const [name, target] of Object.entries(project.config.targets)) { if (!target) { continue; } - if (target.repositoryFiles) { - protectedRoots.push(path.resolve(rootDir, target.repositoryFiles)); + const outDir = path.resolve(project.rootDir, target.outDir); + const manifest = await readManagedManifest( + outDir, + name as TargetName, + ).catch(() => null); + if (!manifest) { + continue; } - for (const plugin of Object.values(target.plugins)) { - if (plugin.source) { - protectedRoots.push(path.resolve(rootDir, plugin.source)); - } - if (plugin.overrides) { - protectedRoots.push(path.resolve(rootDir, plugin.overrides)); + const definition = targetRegistry[name as TargetName]; + const layout = definition.forConfig?.(target) ?? definition; + const pluginDirs = Object.entries(target.plugins).map( + ([pluginName, pluginConfig]) => + path.resolve( + outDir, + layout.resolvePluginPath(pluginName, pluginConfig, target), + ), + ); + for (const file of manifest.files) { + const absolute = path.resolve(outDir, file); + if ( + manifest.sha256[file] !== undefined || + pluginDirs.some((dir) => isInside(dir, absolute)) + ) { + written.add(absolute); } } } - return { - protectedRoots, - configPath: path.resolve(project.configPath), - force, - }; + const protectedDirs: string[] = []; + for (const dir of dirs) { + const files = (await walkFiles(dir).catch(() => null))?.filter( + (file) => !isInside(path.join(dir, ".pluginpack"), file), + ); + if (!files?.length || !files.every((file) => written.has(file))) { + protectedDirs.push(dir); + } + } + return protectedDirs; } -function assertNoProtectedDeletions( +async function assertNoProtectedDeletions( outDir: string, + target: TargetName, paths: string[], guard: DeleteGuard | undefined, command: string, -): void { - if (!guard || guard.force) { +): Promise { + if (!guard || guard.force || paths.length === 0) { return; } - const blocked = paths - .map((file) => ({ file, root: protectingRoot(outDir, file, guard) })) - .filter( - (entry): entry is { file: string; root: string } => entry.root !== null, - ); + const roots = [ + ...(guard.configPath ? [guard.configPath] : []), + ...guard.protectedRoots, + ]; + const rootKeys = await Promise.all( + roots.map(async (root) => ({ root, key: await physicalKey(root) })), + ); + const blocked: Array<{ file: string; root: string }> = []; + for (const file of paths) { + const root = await protectingRoot(outDir, target, file, rootKeys); + if (root !== null) { + blocked.push({ file, root }); + } + } if (blocked.length === 0) { return; } @@ -218,6 +587,9 @@ function assertNoProtectedDeletions( `Refusing to ${command} ${blocked.length} path(s) that resolve inside your source tree or config:\n` + `${blocked.map(({ file, root }) => ` ${file} -> resolves inside ${root}`).join("\n")}\n` + `This usually means a target outDir overlaps source, overrides, repositoryFiles, or legacy source config. ` + + `A plugins/ directory with a component directory counts as legacy source unless pluginpack recorded writing every file in it ` + + `(a managed manifest lists each file with a hash, or lists it inside a plugin directory that target's config writes). ` + + `A file you added there that no manifest lists also makes it count as source; move it out. ` + `Fix the config, or re-run with --force to delete anyway.`, ); } @@ -228,20 +600,69 @@ function assertNoProtectedDeletions( * name what it collided with — "resolves inside " is actionable in a way * that a bare list of refused paths is not. * - * Containment is compared with case and Unicode normalization folded, since the - * filesystem may treat paths differing only in case as the same file. + * Two comparisons, either of which refuses. The lexical one folds case and + * Unicode normalization on every host, so a manifest entry differing from a + * root only by case is refused even where the filesystem would not alias them. + * The physical one compares the entry `fs.rm` would remove against each root + * with symlinks resolved, so an outDir that reaches source through a symlink + * or a case variant is refused too. */ -function protectingRoot( +async function protectingRoot( outDir: string, + target: TargetName, relativePath: string, - guard: DeleteGuard, -): string | null { + roots: Array<{ root: string; key: string }>, +): Promise { const absolute = path.resolve(outDir, normalizeManagedPath(relativePath)); - if (guard.configPath && isInsideFolded(guard.configPath, absolute)) { - return guard.configPath; + const lexical = roots.find(({ root }) => isInsideFolded(root, absolute)); + if (lexical) { + return lexical.root; } + const key = await managedPathKey(outDir, target, relativePath, { + entry: true, + }); + return roots.find((root) => isInside(root.key, key))?.root ?? null; +} + +/** + * `physicalKey` for a path a target's managed manifest lists. A failure to + * resolve it (EACCES on a locked directory, say) names the manifest and the + * way out, since every caller runs before anything is deleted. + */ +export async function managedPathKey( + outDir: string, + target: TargetName, + relativePath: string, + options: { entry?: boolean } = {}, +): Promise { + try { + return await physicalKey( + path.resolve(outDir, normalizeManagedPath(relativePath)), + options, + ); + } catch (error) { + throw uncheckedPathError(outDir, target, relativePath, error); + } +} + +function uncheckedPathError( + outDir: string, + target: TargetName, + relativePath: string, + error: unknown, +): Error { + return new Error( + `Cannot check managed path "${relativePath}" under ${outDir}: ${(error as Error).message}. ` + + nothingDeleted(outDir, target), + { cause: error }, + ); +} + +function nothingDeleted(outDir: string, target: TargetName): string { + const manifest = path.join(path.resolve(outDir), managedManifestPath(target)); return ( - guard.protectedRoots.find((root) => isInsideFolded(root, absolute)) ?? null + `Nothing was deleted. The managed manifest ${manifest} lists this path. ` + + `Fix the cause and re-run the same command, or remove the entry from that manifest to leave the path alone.` ); } @@ -265,32 +686,44 @@ export function normalizeManagedPath(value: string): string { return normalized.startsWith("./") ? normalized.slice(2) : normalized; } -async function removeManagedPath( - outDir: string, +/** A managed path that cannot be removed safely. The message says why. */ +class UnsafeRemovalError extends Error {} + +/** + * The absolute path `relativePath` names under `root`, or `null` when there is + * nothing to remove: the entry is missing, or an ancestor is now a file, so + * the managed file cannot exist. Throws `UnsafeRemovalError` when removing the + * entry would reach outside `root`. + */ +async function inspectRemoval( + root: string, relativePath: string, -): Promise { - const root = path.resolve(outDir); +): Promise { const normalized = normalizeManagedPath(relativePath); const destination = path.resolve(root, normalized); if (!isInside(root, destination)) { - throw new Error(`Managed path escapes output directory: ${relativePath}`); + throw new UnsafeRemovalError( + `Managed path escapes output directory: ${relativePath}`, + ); } let stats; try { stats = await fs.lstat(destination); } catch (error) { - if (isNotFound(error)) { - return; + // ENOTDIR: an ancestor was replaced by a file, so the managed file is + // gone and the file in its way is not pluginpack's to delete. + if (isNotFoundError(error)) { + return null; } throw error; } if (stats.isSymbolicLink()) { - const target = path.resolve( + const linkTarget = path.resolve( path.dirname(destination), await fs.readlink(destination), ); - if (!isInside(root, target)) { - throw new Error( + if (!isInside(root, linkTarget)) { + throw new UnsafeRemovalError( `Refusing to remove a symlink pointing outside the output directory: ${relativePath}`, ); } @@ -300,16 +733,70 @@ async function removeManagedPath( // string test passes and the final entry is an ordinary file. Resolve the // real destination as the catch-all before deleting. if ((await resolveInside(root, destination)) === null) { - throw new Error( + throw new UnsafeRemovalError( `Refusing to remove a path that resolves outside the output directory: ${relativePath}`, ); } + return destination; +} + +/** + * Runs the containment checks `removeManagedPath` runs, for every path, before + * anything is written or deleted. Dry runs run it too, so `--dry-run` refuses + * a path that escapes the output directory exactly where the real command + * would, and a build refuses before writing new output. + */ +async function assertRemovalsContained( + outDir: string, + target: TargetName, + paths: string[], +): Promise { + const root = path.resolve(outDir); + for (const file of paths) { + try { + await inspectRemoval(root, file); + } catch (error) { + if (error instanceof UnsafeRemovalError) { + throw new Error( + `${error.message} (under ${outDir}). ${nothingDeleted(outDir, target)}`, + { cause: error }, + ); + } + throw uncheckedPathError(outDir, target, file, error); + } + } +} + +async function removeManagedPath( + outDir: string, + relativePath: string, + target: TargetName, +): Promise { + const root = path.resolve(outDir); + const manifest = path.join(root, managedManifestPath(target)); + const recovery = + `The managed manifest ${manifest} still lists every remaining path, so fix the cause and re-run the same command. ` + + `To stop pluginpack managing these files instead, delete that manifest.`; + let destination: string | null; + try { + destination = await inspectRemoval(root, relativePath); + } catch (error) { + if (error instanceof UnsafeRemovalError) { + throw new Error(`${error.message}. ${recovery}`, { cause: error }); + } + throw new Error( + `Failed to check managed path "${relativePath}" under ${outDir}: ${(error as Error).message}. ${recovery}`, + { cause: error }, + ); + } + if (destination === null) { + return; + } try { await fs.rm(destination, { force: true }); } catch (error) { throw new Error( - `Failed to remove managed path "${relativePath}" under ${outDir}: ${(error as Error).message}. ` + - `The managed manifest still lists every path, so re-running is safe once the cause is fixed.`, + `Failed to remove managed path "${relativePath}" under ${outDir}: ${(error as Error).message}. ${recovery}`, { cause: error }, ); } diff --git a/src/schema.ts b/src/schema.ts index f6acb7d..e6625aa 100644 --- a/src/schema.ts +++ b/src/schema.ts @@ -1,14 +1,16 @@ import { z } from "zod"; -import { isSafeRelativePath } from "./fs.js"; +import { isSafeRelativeFilePath } from "./fs.js"; /** * A path written to or read from disk relative to a root — rejects absolute - * paths and `..` escapes so a config can't write or read outside it. + * paths and `..` escapes so a config can't write or read outside it. Every + * field using it is a filesystem path, so a URL-shaped value gets no + * exemption: `path.resolve` would read `https://../x` as `../x`. */ const safeRelativePath = z .string() .refine( - isSafeRelativePath, + isSafeRelativeFilePath, 'must be a safe relative path (no absolute paths or ".." segments)', ); @@ -41,7 +43,9 @@ const rootPluginSchema = metadataSchema.extend({ const sourceSchema = z.object({ plugins: z.string().optional(), skills: z.string().optional(), - partials: z.string().optional(), + // Partials are inlined into shipped output, so reading them from outside the + // project would publish files that are not part of it. + partials: safeRelativePath.optional(), rootPlugin: rootPluginSchema.optional(), }); @@ -160,6 +164,10 @@ export const CLIENT_ONLY_CONTENT_KINDS = [ /** One target's output configuration: where it's written, and which plugins it emits. */ const targetSchema = z.object({ + // Deliberately not safeRelativePath: an outDir may be a sibling output + // repository ("../cursor-plugins"). The delete guard protects this + // project's source and the outDir's .git; it knows nothing else about an + // external repo, which the README calls out. outDir: z.string().min(1), // codex only: "agent-plugins" (default) emits Agent Plugins packages; // "legacy" emits the .codex-plugin layout. See docs/adr/0001. diff --git a/tests/destructive.test.ts b/tests/destructive.test.ts index d5e2002..9196541 100644 --- a/tests/destructive.test.ts +++ b/tests/destructive.test.ts @@ -192,7 +192,12 @@ describe("writeArtifact refuses to write outside its output directory", () => { describe("readManagedManifest rejects a manifest it cannot trust", () => { const invalid: Array<[string, unknown]> = [ - ["a wrong version", { version: 2, target: "cursor", files: [] }], + ["an unknown older version", { version: 0, target: "cursor", files: [] }], + ["a non-numeric version", { version: "2", target: "cursor", files: [] }], + [ + "a version 2 manifest with no hashes", + { version: 2, target: "cursor", files: [] }, + ], ["a mismatched target", { version: 1, target: "claude", files: [] }], [ "a non-array files field", @@ -274,7 +279,10 @@ describe("the delete guard protects a source tree from clean, not just prune", ( await access(path.join(root, "pluginpack.config.ts")); }); - it("refuses to clean the default source.plugins root even when unset in config", async () => { + it("refuses to clean a legacy source plugin a target reads through from, even with source.plugins unset", async () => { + // Under the default plugins/ root only the source plugins a target reads + // are protected, not every directory there: generated output lives there + // too in the README layouts. const created = await makeProject(` cursor: { outDir: ".", plugins: { demo: { from: ["demo"], path: "out/demo", components: ["skills"] } } diff --git a/tests/physical-path-safety.test.ts b/tests/physical-path-safety.test.ts new file mode 100644 index 0000000..ed982b3 --- /dev/null +++ b/tests/physical-path-safety.test.ts @@ -0,0 +1,1411 @@ +import { execFileSync } from "node:child_process"; +import { existsSync } from "node:fs"; +import { + access, + chmod, + link, + mkdir, + mkdtemp, + readFile, + rm, + symlink, + writeFile, +} from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { Project, type ProjectArgs } from "fixturify-project"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { build } from "../src/build.js"; +import { clean, prune } from "../src/cleanup.js"; +import { loadConfig } from "../src/config.js"; +import { + exists, + json, + physicalKey, + walkFiles, + withPathCache, +} from "../src/fs.js"; +import { managedManifestPath, readManagedManifest } from "../src/managed.js"; + +type DirJSON = NonNullable; + +let project: Project | undefined; + +afterEach(async () => { + vi.restoreAllMocks(); + await project?.dispose(); + project = undefined; +}); + +const skill = (name: string) => + `---\nname: ${name}\ndescription: ${name} skill.\n---\n\n${name} body.\n`; + +/** + * A project with one canonical source plugin at `shared/acme` holding the + * skills `alpha` and `beta`. `targetsBlock` and `sourceBlock` are spliced into + * the config verbatim. + */ +function configText(targetsBlock: string, sourceBlock = ""): string { + return `import { defineConfig } from "${path.resolve("src/index.ts")}"; + +export default defineConfig({ + name: "physical-demo", + version: "1.0.0", + ${sourceBlock} + metadata: { description: "P", author: { name: "P" }, license: "MIT" }, + targets: { +${targetsBlock} + } +}); +`; +} + +async function makeProject( + targetsBlock: string, + extra: DirJSON = {}, + sourceBlock = "", +): Promise { + project = new Project("pluginpack-physical", "1.0.0", { + files: { + "pluginpack.config.ts": configText(targetsBlock, sourceBlock), + shared: { + acme: { + skills: { + alpha: { "SKILL.md": skill("alpha") }, + beta: { "SKILL.md": skill("beta") }, + }, + }, + }, + "UNMANAGED.txt": "USER FILE\n", + ...extra, + }, + }); + await project.write(); + return project.baseDir; +} + +/** Rewrites a target's managed manifest as an older (version 1) pluginpack would. */ +async function writeManifest( + root: string, + outDir: string, + target: string, + files: string[], +): Promise { + const manifestPath = path.join( + root, + outDir, + managedManifestPath(target as never), + ); + await mkdir(path.dirname(manifestPath), { recursive: true }); + await writeFile(manifestPath, json({ version: 1, target, files })); +} + +/** Messages passed to console.warn while `fn` runs. */ +async function captureWarnings(fn: () => Promise): Promise { + const warnings: string[] = []; + vi.spyOn(console, "warn").mockImplementation((message: unknown) => { + warnings.push(String(message)); + }); + await fn(); + return warnings; +} + +const caseInsensitiveFs = existsSync( + path.join( + path.dirname(fileURLToPath(import.meta.url)), + "PHYSICAL-PATH-SAFETY.TEST.TS", + ), +); + +/** + * Mounts an empty case-sensitive APFS volume at a directory under the + * (case-insensitive by default) system temp directory, or returns null where + * that is not possible: not macOS, or `hdiutil` unavailable or refused. + */ +async function mountCaseSensitiveVolume(): Promise { + if (process.platform !== "darwin") { + return null; + } + const base = await mkdtemp(path.join(os.tmpdir(), "pluginpack-casefs-")); + const image = path.join(base, "volume.sparseimage"); + const mountPoint = path.join(base, "mnt"); + try { + await mkdir(mountPoint); + execFileSync("hdiutil", [ + "create", + "-quiet", + "-size", + "16m", + "-fs", + "Case-sensitive APFS", + "-type", + "SPARSE", + "-volname", + "pluginpack-cs", + image, + ]); + execFileSync("hdiutil", [ + "attach", + "-quiet", + "-nobrowse", + "-mountpoint", + mountPoint, + image, + ]); + } catch { + await rm(base, { recursive: true, force: true }); + return null; + } + return mountPoint; +} + +function unmountVolume(mountPoint: string): void { + execFileSync("hdiutil", ["detach", "-quiet", "-force", mountPoint]); + execFileSync("rm", ["-rf", path.dirname(mountPoint)]); +} + +const sourceSurvives = async (root: string) => { + await access(path.join(root, "shared/acme/skills/alpha/SKILL.md")); + await access(path.join(root, "shared/acme/skills/beta/SKILL.md")); + await access(path.join(root, "pluginpack.config.ts")); + await access(path.join(root, "UNMANAGED.txt")); +}; + +describe("the delete guard protects source, not generated packages under plugins/", () => { + it("keeps outDir '.' with the default plugins/ package path operable through a full lifecycle", async () => { + // The agent-plugins default writes the package to plugins/acme/, which + // has a skills/ directory, so a guard that guesses source plugins from + // component directories under plugins/ refuses to prune its own output. + const root = await makeProject(` "agent-plugins": { + outDir: ".", + plugins: { acme: { source: "shared/acme" } } + }`); + await build({ cwd: root }); + await access(path.join(root, "plugins/acme/skills/beta/SKILL.md")); + await rm(path.join(root, "shared/acme/skills/beta"), { recursive: true }); + + await expect(build({ cwd: root })).resolves.toBeDefined(); + await expect( + exists(path.join(root, "plugins/acme/skills/beta/SKILL.md")), + ).resolves.toBe(false); + await expect(prune({ cwd: root })).resolves.toBeDefined(); + await expect(clean({ cwd: root })).resolves.toBeDefined(); + + await expect(exists(path.join(root, "plugins/acme"))).resolves.toBe(false); + await access(path.join(root, "shared/acme/skills/alpha/SKILL.md")); + await access(path.join(root, "UNMANAGED.txt")); + }); + + it("still protects a plugins/ directory that carries plugin.pluginpack.json", async () => { + const root = await makeProject( + ` "agent-plugins": { + outDir: ".", + plugins: { acme: { source: "shared/acme", path: "out/acme" } } + }`, + { + plugins: { + authored: { + "plugin.pluginpack.json": "{}\n", + skills: { gamma: { "SKILL.md": skill("gamma") } }, + }, + }, + }, + ); + await build({ cwd: root }); + await writeManifest(root, ".", "agent-plugins", [ + "plugins/authored/skills/gamma/SKILL.md", + ]); + + await expect(clean({ cwd: root })).rejects.toThrow( + /Refusing to clean 1 path\(s\)[\s\S]*resolves inside .*plugins\/authored/, + ); + await access(path.join(root, "plugins/authored/skills/gamma/SKILL.md")); + }); +}); + +describe("the delete guard protects discovered legacy sources nothing references", () => { + const generatedElsewhere = ` antigravity: { + outDir: ".", + plugins: { acme: { source: "shared/acme", path: "generated" } } + }`; + const authored = { + plugins: { + authored: { + skills: { + alpha: { "SKILL.md": skill("alpha") }, + beta: { "SKILL.md": skill("beta") }, + }, + }, + }, + }; + + it("refuses to clean one that a version 1 manifest names", async () => { + const root = await makeProject(generatedElsewhere, authored); + await writeManifest(root, ".", "antigravity", [ + "plugins/authored/skills/alpha/SKILL.md", + ]); + + await expect(clean({ cwd: root })).rejects.toThrow( + /Refusing to clean 1 path\(s\)[\s\S]*plugins\/authored\/skills\/alpha\/SKILL\.md -> resolves inside .*plugins\/authored/, + ); + await access(path.join(root, "plugins/authored/skills/alpha/SKILL.md")); + }); + + it("refuses to clean one holding a file no manifest records writing", async () => { + const root = await makeProject(generatedElsewhere, authored); + const manifestPath = path.join(root, managedManifestPath("antigravity")); + await mkdir(path.dirname(manifestPath), { recursive: true }); + await writeFile( + manifestPath, + json({ + version: 2, + target: "antigravity", + files: ["plugins/authored/skills/alpha/SKILL.md"], + sha256: { "plugins/authored/skills/alpha/SKILL.md": "0".repeat(64) }, + }), + ); + + await expect(clean({ cwd: root })).rejects.toThrow( + /Refusing to clean 1 path\(s\)[\s\S]*resolves inside .*plugins\/authored/, + ); + await access(path.join(root, "plugins/authored/skills/alpha/SKILL.md")); + await access(path.join(root, "plugins/authored/skills/beta/SKILL.md")); + }); +}); + +describe("upgrading a codex outDir '.' build from 0.11 (version 1 manifest)", () => { + const codexAtRoot = ` codex: { + outDir: ".", + plugins: { acme: { source: "shared/acme" } } + }`; + + /** + * The files and version 1 manifest pluginpack 0.11.1 writes for `codexAtRoot` + * when the source also had a skill `gamma` (since deleted). 0.11 wrote the + * Codex manifest under `.codex-plugin/`; the package format now writes it at + * the package root, so the first build after upgrading must prune it. + */ + async function seed011(root: string) { + const files: Record = { + ".agents/plugins/marketplace.json": json({ + name: "physical-demo", + interface: { displayName: "physical-demo" }, + plugins: [ + { + name: "acme", + source: "./plugins/acme", + description: "P", + version: "1.0.0", + }, + ], + }), + "plugins/acme/.codex-plugin/plugin.json": json({ + name: "acme", + version: "1.0.0", + description: "P", + author: { name: "P" }, + license: "MIT", + skills: "./skills/", + }), + "plugins/acme/skills/alpha/SKILL.md": skill("alpha"), + "plugins/acme/skills/beta/SKILL.md": skill("beta"), + "plugins/acme/skills/gamma/SKILL.md": skill("gamma"), + }; + for (const [file, body] of Object.entries(files)) { + await mkdir(path.dirname(path.join(root, file)), { recursive: true }); + await writeFile(path.join(root, file), body); + } + await writeManifest(root, ".", "codex", Object.keys(files)); + } + + it("builds, prunes the stale 0.11 paths, and upgrades the manifest", async () => { + const root = await makeProject(codexAtRoot); + await seed011(root); + + await build({ cwd: root }); + + const manifest = await readManagedManifest(root, "codex"); + expect(manifest?.version).toBe(2); + expect( + await exists(path.join(root, "plugins/acme/.codex-plugin/plugin.json")), + ).toBe(false); + expect( + await exists(path.join(root, "plugins/acme/skills/gamma/SKILL.md")), + ).toBe(false); + await access(path.join(root, "plugins/acme/plugin.json")); + await sourceSurvives(root); + }); + + it("prunes and cleans straight off the version 1 manifest", async () => { + const root = await makeProject(codexAtRoot); + await seed011(root); + + await prune({ cwd: root }); + expect( + await exists(path.join(root, "plugins/acme/skills/gamma/SKILL.md")), + ).toBe(false); + await clean({ cwd: root }); + + expect(await exists(path.join(root, "plugins/acme"))).toBe(false); + expect(await exists(path.join(root, managedManifestPath("codex")))).toBe( + false, + ); + await sourceSurvives(root); + }); + + it("still refuses when the package directory holds a file the manifest does not list", async () => { + const root = await makeProject(codexAtRoot, { + plugins: { acme: { agents: { "mine.md": "authored\n" } } }, + }); + await seed011(root); + + await expect(clean({ cwd: root })).rejects.toThrow( + /Refusing to clean \d+ path\(s\)[\s\S]*resolves inside .*plugins\/acme/, + ); + await access(path.join(root, "plugins/acme/agents/mine.md")); + await access(path.join(root, "plugins/acme/skills/alpha/SKILL.md")); + }); +}); + +describe("source protection compares physical paths", () => { + async function aliasedSource(): Promise { + const root = await makeProject(` "agent-plugins": { + outDir: "linked-out", + plugins: { acme: { source: "shared/acme", path: "." } } + }`); + await symlink( + path.join(root, "shared/acme"), + path.join(root, "linked-out"), + ); + return root; + } + + it("refuses to build when the outDir is a symlink into a source plugin", async () => { + const root = await aliasedSource(); + + await expect(build({ cwd: root })).rejects.toThrow( + /targets\.agent-plugins\.outDir .* resolves inside .*shared\/acme \(targets\.agent-plugins\.plugins\.acme\.source\)/, + ); + await sourceSurvives(root); + }); + + it("refuses to clean managed paths that reach source through a symlinked outDir", async () => { + // clean never loads the source tree, so this is the delete guard on its own. + const root = await aliasedSource(); + await writeManifest(root, "linked-out", "agent-plugins", [ + "skills/alpha/SKILL.md", + "skills/beta/SKILL.md", + ]); + + await expect(clean({ cwd: root })).rejects.toThrow( + /Refusing to clean 2 path\(s\)[\s\S]*skills\/alpha\/SKILL\.md -> resolves inside .*shared\/acme/, + ); + await sourceSurvives(root); + }); + + it("refuses to load a config whose outDir sits inside a source plugin", async () => { + const root = await makeProject(` antigravity: { + outDir: "shared/acme", + plugins: { acme: { source: "shared/acme" } } + }`); + + await expect(loadConfig(root)).rejects.toThrow( + /targets\.antigravity\.outDir .* resolves inside .*shared\/acme \(targets\.antigravity\.plugins\.acme\.source\)[\s\S]*Point each outDir at a directory outside/, + ); + }); + + it("refuses a --out-dir override inside source", async () => { + const root = await makeProject(` antigravity: { + outDir: "dist/antigravity", + plugins: { acme: { source: "shared/acme" } } + }`); + + await expect( + build({ cwd: root, target: "antigravity", outDir: "shared/acme/out" }), + ).rejects.toThrow(/resolves inside .*shared\/acme/); + await expect(exists(path.join(root, "shared/acme/out"))).resolves.toBe( + false, + ); + }); + + it("still builds and cleans through an outDir symlink that points somewhere safe", async () => { + const root = await makeProject(` antigravity: { + outDir: "linked-out", + plugins: { acme: { source: "shared/acme" } } + }`); + await mkdir(path.join(root, "real-out")); + await symlink(path.join(root, "real-out"), path.join(root, "linked-out")); + + await build({ cwd: root }); + await access(path.join(root, "real-out/acme/skills/alpha/SKILL.md")); + await clean({ cwd: root }); + await sourceSurvives(root); + }); +}); + +/** + * In a case-sensitive `dir`, two hard links whose names differ only by case + * are one inode: comparing inodes of `name` and `NAME` would conclude the + * directory folds case and alias the distinct files `other` and `OTHER`. + */ +async function expectHardLinksKeepCase(dir: string): Promise { + const links = path.join(dir, "links"); + await mkdir(links); + await writeFile(path.join(links, "name"), "x\n"); + await link(path.join(links, "name"), path.join(links, "NAME")); + await writeFile(path.join(links, "other"), "a\n"); + await writeFile(path.join(links, "OTHER"), "b\n"); + + const keys = await withPathCache(async () => [ + await physicalKey(path.join(links, "name"), { entry: true }), + await physicalKey(path.join(links, "other"), { entry: true }), + await physicalKey(path.join(links, "OTHER"), { entry: true }), + ]); + + expect(new Set(keys).size).toBe(3); +} + +describe("cross-target collision checks compare physical paths", () => { + const codexAndAgentPlugins = ( + codexOut: string, + apOut: string, + ) => ` codex: { + outDir: "${codexOut}", + plugins: { acme: { source: "shared/acme" } } + }, + "agent-plugins": { + outDir: "${apOut}", + clients: { codex: {} }, + plugins: { acme: { source: "shared/acme" } } + }`; + + it("rejects two outDirs that alias one directory through a symlink", async () => { + const root = await makeProject(codexAndAgentPlugins("out", "alias-out")); + await mkdir(path.join(root, "out")); + await symlink(path.join(root, "out"), path.join(root, "alias-out")); + + await expect(build({ cwd: root })).rejects.toThrow( + /Targets write overlapping output paths[\s\S]*codex and agent-plugins: .*out\/\.agents\/plugins\/marketplace\.json \(same file as .*alias-out\/\.agents\/plugins\/marketplace\.json\)/, + ); + await expect(exists(path.join(root, "out/plugins"))).resolves.toBe(false); + }); + + it.skipIf(!caseInsensitiveFs)( + "rejects outDirs that differ only by case on a case-insensitive filesystem", + async () => { + const root = await makeProject(codexAndAgentPlugins("out", "OUT")); + + await expect(build({ cwd: root })).rejects.toThrow( + /Targets write overlapping output paths[\s\S]*codex and agent-plugins/, + ); + }, + ); + + it.skipIf(!caseInsensitiveFs)( + "rejects a --target build whose outDir aliases another target's built output by case", + async () => { + const root = await makeProject(codexAndAgentPlugins("out", "OUT")); + await build({ cwd: root, target: "codex" }); + + await expect( + build({ cwd: root, target: "agent-plugins" }), + ).rejects.toThrow( + /overlaps a previously built target's managed files[\s\S]*codex \(previously built\) and agent-plugins/, + ); + }, + ); + + it("builds outDirs that differ only by case on a case-sensitive volume mounted under a case-insensitive parent", async (context) => { + // The volume is the destination filesystem. Probing how its mount point's + // name compares in the parent directory asks the wrong filesystem. + const volume = await mountCaseSensitiveVolume(); + if (!volume) { + context.skip(); + return; + } + try { + const root = await makeProject( + codexAndAgentPlugins( + path.join(volume, "out"), + path.join(volume, "OUT"), + ), + ); + + await build({ cwd: root }); + await access(path.join(volume, "out/plugins/acme/plugin.json")); + await access(path.join(volume, "OUT/plugins/acme/plugin.json")); + await clean({ cwd: root }); + await expect(exists(path.join(volume, "out/plugins"))).resolves.toBe( + false, + ); + await expectHardLinksKeepCase(volume); + } finally { + unmountVolume(volume); + } + }); + + it.skipIf(caseInsensitiveFs)( + "does not read a hard link with a case-swapped name as a case-insensitive directory", + async () => { + const root = await makeProject(codexAndAgentPlugins("out", "OUT")); + await expectHardLinksKeepCase(root); + await build({ cwd: root }); + }, + ); + + it("rejects a generated file that is a symlink to another target's generated file", async () => { + const root = await makeProject( + ` antigravity: { + outDir: "out", + plugins: { first: { source: "shared/acme", path: "first" } } + }, + cursor: { + outDir: "out", + plugins: { second: { source: "shared/other", path: "second" } } + }`, + { + out: { + second: { skills: { alpha: { "SKILL.md": "OLD\n" } } }, + first: { skills: { alpha: {} } }, + }, + }, + ); + await mkdir(path.join(root, "shared/other/skills/alpha"), { + recursive: true, + }); + await writeFile( + path.join(root, "shared/other/skills/alpha/SKILL.md"), + skill("alpha").replace("body", "OTHER"), + ); + await symlink( + "../../../second/skills/alpha/SKILL.md", + path.join(root, "out/first/skills/alpha/SKILL.md"), + ); + + await expect(build({ cwd: root })).rejects.toThrow( + /Targets write overlapping output paths[\s\S]*antigravity and cursor: .*out\/first\/skills\/alpha\/SKILL\.md \(same file as .*out\/second\/skills\/alpha\/SKILL\.md\)/, + ); + expect( + await readFile( + path.join(root, "out/second/skills/alpha/SKILL.md"), + "utf8", + ), + ).toBe("OLD\n"); + }); + + it("refuses to clean one target's manifest entries that alias another's through a symlink", async () => { + const root = await makeProject(codexAndAgentPlugins("out", "alias-out")); + await mkdir(path.join(root, "out")); + await symlink(path.join(root, "out"), path.join(root, "alias-out")); + await mkdir(path.join(root, "out/plugins/acme"), { recursive: true }); + await writeFile(path.join(root, "out/plugins/acme/plugin.json"), "{}\n"); + // Two manifests an older pluginpack could have left behind. + await writeManifest(root, "out", "codex", ["plugins/acme/plugin.json"]); + await writeManifest(root, "alias-out", "agent-plugins", [ + "plugins/acme/plugin.json", + ]); + + await expect(clean({ cwd: root, target: "codex" })).rejects.toThrow( + /Refusing to clean paths claimed by more than one target's managed manifest:[\s\S]*codex and agent-plugins/, + ); + await access(path.join(root, "out/plugins/acme/plugin.json")); + }); +}); + +describe("a stale path another target now owns is left in place", () => { + const entry = `entry: { policy: { installation: "AVAILABLE", authentication: "ON_INSTALL" }, category: "Productivity" }`; + const agentPluginsWithCodex = ` "agent-plugins": { + outDir: "out", + clients: { codex: {} }, + plugins: { acme: { source: "shared/acme", clients: { codex: { ${entry} } } } } + }`; + const codexTarget = ` codex: { + outDir: "out", + plugins: { other: { source: "shared/acme", ${entry} } } + }`; + const agentPluginsWithoutCodex = ` "agent-plugins": { + outDir: "out", + plugins: { acme: { source: "shared/acme" } } + }`; + const marketplace = "out/.agents/plugins/marketplace.json"; + + /** Writes a later revision of the project config under its own name. */ + async function revision( + root: string, + name: string, + targets: string, + ): Promise { + const configPath = path.join(root, `${name}.config.ts`); + await writeFile(configPath, configText(targets)); + return configPath; + } + + it("does not delete a marketplace handed from agent-plugins to codex in one plain build", async () => { + const root = await makeProject(agentPluginsWithCodex); + await build({ cwd: root }); + const configPath = await revision( + root, + "handoff", + `${codexTarget},\n${agentPluginsWithoutCodex}`, + ); + + await build({ cwd: root, configPath }); + + const written = JSON.parse( + await readFile(path.join(root, marketplace), "utf8"), + ) as { plugins: Array<{ name: string }> }; + expect(written.plugins.map((plugin) => plugin.name)).toEqual(["other"]); + expect( + (await readManagedManifest(path.join(root, "out"), "agent-plugins")) + ?.files, + ).not.toContain(".agents/plugins/marketplace.json"); + }); + + it("warns before overwriting an edited file handed over from another target", async () => { + const root = await makeProject(agentPluginsWithCodex); + await build({ cwd: root }); + await writeFile( + path.join(root, marketplace), + `${await readFile(path.join(root, marketplace), "utf8")}HAND EDIT\n`, + ); + const configPath = await revision( + root, + "handoff", + `${codexTarget},\n${agentPluginsWithoutCodex}`, + ); + + const warnings = await captureWarnings(() => + build({ cwd: root, configPath }), + ); + + expect(warnings).toEqual([ + expect.stringMatching( + /codex managed file .*out\/\.agents\/plugins\/marketplace\.json \(handed over from agent-plugins\) was edited after pluginpack wrote it; overwriting it/, + ), + ]); + }); + + it("keeps a handed-over path owned when the recipient's prune fails", async () => { + const one = `outDir: "out", plugins: { one: { source: "shared/acme" } }`; + const two = `outDir: "out", plugins: { two: { source: "shared/acme" } }`; + const root = await makeProject( + ` antigravity: { ${one}, rootFiles: { "transfer.txt": "authored.txt" } }, + cursor: { ${two}, rootFiles: { "blocked": "authored.txt" } }`, + { "authored.txt": "AUTHORED\n" }, + ); + await build({ cwd: root }); + // A directory where cursor's stale file was makes cursor's prune fail. + await rm(path.join(root, "out/blocked")); + await mkdir(path.join(root, "out/blocked")); + const configPath = await revision( + root, + "handoff", + ` antigravity: { ${one} }, + cursor: { ${two}, rootFiles: { "transfer.txt": "authored.txt" } }`, + ); + + await expect(build({ cwd: root, configPath })).rejects.toThrow( + /Failed to remove managed path "blocked"/, + ); + + const owners: string[] = []; + for (const target of ["antigravity", "cursor"] as const) { + const manifest = await readManagedManifest( + path.join(root, "out"), + target, + ); + if (manifest?.files.includes("transfer.txt")) { + owners.push(target); + } + } + expect(owners).toEqual(["cursor"]); + await rm(path.join(root, "out/blocked"), { recursive: true }); + await clean({ cwd: root, configPath }); + await expect(exists(path.join(root, "out/transfer.txt"))).resolves.toBe( + false, + ); + await access(path.join(root, "authored.txt")); + }); + + it.skipIf(!caseInsensitiveFs)( + "does not prune a path renamed only by case, which is the file just written", + async () => { + const root = await makeProject(` antigravity: { + outDir: "out", + plugins: { acme: { source: "shared/acme", path: "first" } } + }`); + await build({ cwd: root }); + const configPath = await revision( + root, + "renamed", + ` antigravity: { + outDir: "out", + plugins: { acme: { source: "shared/acme", path: "FIRST" } } + }`, + ); + + await build({ cwd: root, configPath }); + + await access(path.join(root, "out/FIRST/skills/alpha/SKILL.md")); + await access(path.join(root, "out/FIRST/plugin.json")); + }, + ); + + it("does not prune a stale path another configured target's manifest claims", async () => { + const root = await makeProject(agentPluginsWithCodex); + await build({ cwd: root }); + // A codex manifest an older pluginpack could have left claiming the + // marketplace agent-plugins still lists. + await writeManifest(root, "out", "codex", [ + ".agents/plugins/marketplace.json", + ]); + const configPath = await revision( + root, + "both", + `${codexTarget},\n${agentPluginsWithoutCodex}`, + ); + + const [result] = await prune({ + cwd: root, + configPath, + target: "agent-plugins", + }); + + expect(result.entries.map((entry) => entry.path)).not.toContain( + ".agents/plugins/marketplace.json", + ); + await access(path.join(root, marketplace)); + }); +}); + +describe("managed output from a target the config no longer builds there", () => { + const entry = `entry: { policy: { installation: "AVAILABLE", authentication: "ON_INSTALL" }, category: "Productivity" }`; + const codexIn = (outDir: string) => ` codex: { + outDir: "${outDir}", + plugins: { acme: { source: "shared/acme", ${entry} } } + }`; + const agentPlugins = ` "agent-plugins": { + outDir: "out", + plugins: { acme: { source: "shared/acme" } } + }`; + const hooks = { + "hooks.json": `${JSON.stringify({ + hooks: { + SessionStart: [{ hooks: [{ type: "command", command: "echo hi" }] }], + }, + })}\n`, + }; + + /** Builds codex into out, then switches the config to agent-plugins there. */ + async function switchedWithoutCleaning(): Promise<{ + root: string; + configPath: string; + }> { + const root = await makeProject(codexIn("out")); + await mkdir(path.join(root, "shared/acme/hooks")); + await writeFile( + path.join(root, "shared/acme/hooks/hooks.json"), + hooks["hooks.json"], + ); + await build({ cwd: root }); + await access(path.join(root, "out/plugins/acme/hooks/hooks.json")); + const configPath = path.join(root, "switched.config.ts"); + await writeFile(configPath, configText(agentPlugins)); + return { root, configPath }; + } + + it("refuses to build or prune, naming the manifest and the clean command", async () => { + const { root, configPath } = await switchedWithoutCleaning(); + const manifest = path.join(root, "out", managedManifestPath("codex")); + const message = `Found managed output from a target this config does not build there:\n ${manifest} (codex)\n`; + + await expect(build({ cwd: root, configPath })).rejects.toThrow(message); + await expect(build({ cwd: root, configPath })).rejects.toThrow( + "Run `pluginpack clean --target codex` to delete them first.", + ); + await expect(prune({ cwd: root, configPath })).rejects.toThrow(message); + await expect( + exists(path.join(root, "out", managedManifestPath("agent-plugins"))), + ).resolves.toBe(false); + }); + + it("clean --target removes it, after which the new target builds clean output", async () => { + const { root, configPath } = await switchedWithoutCleaning(); + + const [result] = await clean({ cwd: root, configPath, target: "codex" }); + expect(result.target).toBe("codex"); + await expect(exists(path.join(root, "out/plugins"))).resolves.toBe(false); + await expect(exists(path.join(root, "out/.pluginpack"))).resolves.toBe( + false, + ); + + const [artifact] = await build({ cwd: root, configPath }); + expect(artifact.managedPaths).not.toContain( + "plugins/acme/hooks/hooks.json", + ); + await expect( + exists(path.join(root, "out/plugins/acme/hooks/hooks.json")), + ).resolves.toBe(false); + await clean({ cwd: root, configPath }); + await expect(walkFiles(path.join(root, "out"))).resolves.toEqual([]); + await sourceSurvives(root); + }); + + it("a plain clean removes it along with the configured targets' output", async () => { + const { root, configPath } = await switchedWithoutCleaning(); + + const results = await clean({ cwd: root, configPath }); + + expect(results.map((result) => result.target)).toEqual([ + "agent-plugins", + "codex", + ]); + await expect(walkFiles(path.join(root, "out"))).resolves.toEqual([]); + await sourceSurvives(root); + }); + + it("clean --target finds the manifest a target left in its old outDir", async () => { + const root = await makeProject(`${codexIn("out")},\n${agentPlugins}`); + await build({ cwd: root, target: "codex" }); + const configPath = path.join(root, "moved.config.ts"); + await writeFile( + configPath, + configText(`${codexIn("out2")},\n${agentPlugins}`), + ); + + await expect(build({ cwd: root, configPath })).rejects.toThrow( + /Found managed output[\s\S]*out\/\.pluginpack\/codex\.json \(codex\)/, + ); + await clean({ cwd: root, configPath, target: "codex" }); + + await expect( + exists(path.join(root, "out", managedManifestPath("codex"))), + ).resolves.toBe(false); + await expect(build({ cwd: root, configPath })).resolves.toHaveLength(2); + }); +}); + +describe("dry runs run the same destructive preflight as the real command", () => { + async function protectedStalePath(): Promise { + const root = await makeProject( + ` antigravity: { + outDir: ".", + plugins: { acme: { from: ["core"], path: "generated" } } + }`, + { skills: { gamma: { "SKILL.md": skill("gamma") } } }, + `source: { skills: "skills", rootPlugin: { id: "core" } },`, + ); + await build({ cwd: root }); + const manifest = await readManagedManifest(root, "antigravity"); + await writeManifest(root, ".", "antigravity", [ + ...(manifest?.files ?? []), + "skills/gamma/SKILL.md", + ]); + return root; + } + + it("clean --dry-run fails on a protected path, as clean does", async () => { + const root = await protectedStalePath(); + + await expect(clean({ cwd: root, dryRun: true })).rejects.toThrow( + /Refusing to clean 1 path\(s\)/, + ); + await expect(clean({ cwd: root })).rejects.toThrow( + /Refusing to clean 1 path\(s\)/, + ); + }); + + it("prune --dry-run fails on a protected path, as prune does", async () => { + const root = await protectedStalePath(); + + await expect(prune({ cwd: root, dryRun: true })).rejects.toThrow( + /Refusing to prune 1 path\(s\)/, + ); + await expect(prune({ cwd: root })).rejects.toThrow( + /Refusing to prune 1 path\(s\)/, + ); + await access(path.join(root, "skills/gamma/SKILL.md")); + }); + + describe("with a stale path that crosses a symlink out of the outDir", () => { + async function escapingStalePath(): Promise { + const root = await makeProject( + ` antigravity: { + outDir: "out", + plugins: { acme: { source: "shared/acme" } } + }`, + { outside: { "sentinel.txt": "UNMANAGED\n" }, out: {} }, + ); + await symlink("../outside", path.join(root, "out/link")); + await writeManifest(root, "out", "antigravity", ["link/sentinel.txt"]); + return root; + } + const refusal = + /Refusing to remove a path that resolves outside the output directory: link\/sentinel\.txt \(under .*out\)\. Nothing was deleted\. The managed manifest .*out\/\.pluginpack\/antigravity\.json lists this path/; + + it.each(["clean", "prune"] as const)( + "%s --dry-run refuses it, as the real command does", + async (command) => { + const root = await escapingStalePath(); + const run = command === "clean" ? clean : prune; + + await expect(run({ cwd: root, dryRun: true })).rejects.toThrow(refusal); + await expect(run({ cwd: root })).rejects.toThrow(refusal); + await access(path.join(root, "outside/sentinel.txt")); + }, + ); + + it("build refuses it before writing any output", async () => { + const root = await escapingStalePath(); + + await expect(build({ cwd: root, dryRun: true })).rejects.toThrow(refusal); + await expect(build({ cwd: root })).rejects.toThrow(refusal); + await expect(exists(path.join(root, "out/acme"))).resolves.toBe(false); + await access(path.join(root, "outside/sentinel.txt")); + }); + }); + + it("build --dry-run fails where build would, and build refuses before writing", async () => { + const root = await protectedStalePath(); + await rm(path.join(root, "generated"), { recursive: true }); + + await expect(build({ cwd: root, dryRun: true })).rejects.toThrow( + /Refusing to prune 1 path\(s\)/, + ); + await expect(build({ cwd: root })).rejects.toThrow( + /Refusing to prune 1 path\(s\)/, + ); + await expect(exists(path.join(root, "generated"))).resolves.toBe(false); + await access(path.join(root, "skills/gamma/SKILL.md")); + }); +}); + +describe("rootFiles cannot make an authored file a managed output", () => { + const selfCopy = ` antigravity: { + outDir: ".", + rootFiles: { "README.md": "README.md" }, + plugins: { acme: { source: "shared/acme", path: "generated/acme" } } + }`; + + it("rejects a rootFiles entry that copies a file onto itself, before writing", async () => { + const root = await makeProject(selfCopy, { + "README.md": "HAND-WRITTEN README\n", + }); + + await expect(build({ cwd: root })).rejects.toThrow( + /targets\.antigravity\.rootFiles\["README\.md"\] copies .*README\.md onto itself/, + ); + expect(await readFile(path.join(root, "README.md"), "utf8")).toBe( + "HAND-WRITTEN README\n", + ); + await expect(exists(path.join(root, "generated"))).resolves.toBe(false); + }); + + it("refuses to clean a rootFiles source an older build left in the manifest", async () => { + const root = await makeProject(selfCopy, { + "README.md": "HAND-WRITTEN README\n", + }); + await writeManifest(root, ".", "antigravity", ["README.md"]); + + await expect(clean({ cwd: root })).rejects.toThrow( + /README\.md -> resolves inside .*README\.md/, + ); + await access(path.join(root, "README.md")); + }); + + it("rejects a repositoryFiles directory that is the target's own outDir", async () => { + const root = await makeProject( + ` antigravity: { + outDir: "repo-root", + repositoryFiles: "repo-root", + plugins: { acme: { source: "shared/acme" } } + }`, + { "repo-root": { "README.md": "HAND-WRITTEN README\n" } }, + ); + + // Caught at load, since the outDir is a protected repositoryFiles path. + await expect(build({ cwd: root })).rejects.toThrow( + /targets\.antigravity\.outDir .* resolves inside .*repo-root \(targets\.antigravity\.repositoryFiles\)/, + ); + await access(path.join(root, "repo-root/README.md")); + }); +}); + +describe("the managed manifest records hashes and warns before losing an edit", () => { + const target = ` antigravity: { + outDir: "out", + plugins: { acme: { source: "shared/acme" } } + }`; + + it("writes version 2 with a sha256 for every managed file", async () => { + const root = await makeProject(target); + await build({ cwd: root }); + + const raw = JSON.parse( + await readFile( + path.join(root, "out", managedManifestPath("antigravity")), + "utf8", + ), + ) as { version: number; files: string[]; sha256: Record }; + + expect(raw.version).toBe(2); + expect(Object.keys(raw.sha256).sort()).toEqual([...raw.files].sort()); + expect(raw.sha256["acme/skills/alpha/SKILL.md"]).toMatch(/^[0-9a-f]{64}$/); + }); + + it("reads a version 1 manifest and upgrades it on the next build", async () => { + const root = await makeProject(target); + await build({ cwd: root }); + await writeManifest(root, "out", "antigravity", [ + "acme/skills/alpha/SKILL.md", + ]); + + await expect( + readManagedManifest(path.join(root, "out"), "antigravity"), + ).resolves.toMatchObject({ version: 2, sha256: {} }); + await build({ cwd: root }); + await expect( + readManagedManifest(path.join(root, "out"), "antigravity"), + ).resolves.toMatchObject({ + sha256: { "acme/skills/alpha/SKILL.md": expect.any(String) }, + }); + }); + + it("warns, then still prunes, a stale managed file someone edited", async () => { + const root = await makeProject(target); + await build({ cwd: root }); + const generated = path.join(root, "out/acme/skills/beta/SKILL.md"); + await writeFile(generated, `${skill("beta")}HOTFIX\n`); + await rm(path.join(root, "shared/acme/skills/beta"), { recursive: true }); + + const warnings = await captureWarnings(() => build({ cwd: root })); + + expect(warnings).toEqual([ + expect.stringMatching( + /antigravity managed file .*out\/acme\/skills\/beta\/SKILL\.md was edited after pluginpack wrote it; deleting it/, + ), + ]); + await expect(exists(generated)).resolves.toBe(false); + }); + + it("reports an edited stale file from prune --dry-run without deleting it", async () => { + const root = await makeProject(target); + await build({ cwd: root }); + const generated = path.join(root, "out/acme/skills/beta/SKILL.md"); + await writeFile(generated, "EDITED\n"); + await rm(path.join(root, "shared/acme/skills/beta"), { recursive: true }); + + const warnings = await captureWarnings(() => + prune({ cwd: root, dryRun: true }), + ); + + expect(warnings).toEqual([ + expect.stringMatching( + /was edited after pluginpack wrote it; a real run would delete it/, + ), + ]); + await access(generated); + }); + + it("warns before a build overwrites an edited managed file, but not an unedited one", async () => { + const root = await makeProject(target); + await build({ cwd: root }); + await writeFile( + path.join(root, "out/acme/skills/alpha/SKILL.md"), + "EDITED\n", + ); + + const warnings = await captureWarnings(() => build({ cwd: root })); + + expect(warnings).toEqual([ + expect.stringMatching( + /acme\/skills\/alpha\/SKILL\.md was edited after pluginpack wrote it; overwriting it/, + ), + ]); + expect( + await readFile(path.join(root, "out/acme/skills/alpha/SKILL.md"), "utf8"), + ).toBe(skill("alpha")); + }); + + it("warns before clean deletes an edited managed file", async () => { + const root = await makeProject(target); + await build({ cwd: root }); + await writeFile( + path.join(root, "out/acme/skills/alpha/SKILL.md"), + "EDITED\n", + ); + + const warnings = await captureWarnings(() => clean({ cwd: root })); + + expect(warnings).toEqual([ + expect.stringMatching( + /alpha\/SKILL\.md was edited after pluginpack wrote it; deleting it/, + ), + ]); + }); + + it("tells a manifest from a newer pluginpack apart from a corrupt one", async () => { + const root = await makeProject(target); + const outDir = path.join(root, "out"); + const manifestPath = path.join(outDir, managedManifestPath("antigravity")); + await mkdir(path.dirname(manifestPath), { recursive: true }); + + await writeFile( + manifestPath, + json({ version: 3, target: "antigravity", files: [] }), + ); + await expect(readManagedManifest(outDir, "antigravity")).rejects.toThrow( + `Managed manifest ${manifestPath} has version 3, so a newer pluginpack wrote it. This pluginpack reads versions 1 to 2. Upgrade pluginpack`, + ); + + await writeFile(manifestPath, "{ truncated"); + await expect(readManagedManifest(outDir, "antigravity")).rejects.toThrow( + `Invalid managed manifest: ${manifestPath} (not valid JSON)`, + ); + }); +}); + +describe("managed-manifest writes stay inside the output directory", () => { + it("refuses to build through a symlinked .pluginpack directory", async () => { + const root = await makeProject(` antigravity: { + outDir: "out", + plugins: { acme: { source: "shared/acme" } } + }`); + const outside = path.join(root, "outside-state"); + await mkdir(outside); + const state = json({ + version: 1, + target: "antigravity", + files: [], + note: "USER STATE", + }); + await writeFile(path.join(outside, "antigravity.json"), state); + await mkdir(path.join(root, "out")); + await symlink(outside, path.join(root, "out/.pluginpack")); + + await expect(build({ cwd: root })).rejects.toThrow( + /Refusing to write through a symlink that leaves the output directory: \.pluginpack\/antigravity\.json/, + ); + expect(await readFile(path.join(outside, "antigravity.json"), "utf8")).toBe( + state, + ); + await expect(exists(path.join(root, "out/acme"))).resolves.toBe(false); + }); +}); + +describe("a failed clean says how to recover", () => { + it("names the managed manifest and the recovery step", async () => { + const root = await makeProject(` antigravity: { + outDir: "out", + plugins: { acme: { source: "shared/acme" } } + }`); + await build({ cwd: root }); + const manifest = await readManagedManifest( + path.join(root, "out"), + "antigravity", + ); + await mkdir(path.join(root, "out/blocked")); + await writeManifest(root, "out", "antigravity", [ + ...(manifest?.files ?? []), + "blocked", + ]); + + const manifestPath = path.join( + root, + "out", + managedManifestPath("antigravity"), + ); + await expect(clean({ cwd: root })).rejects.toThrow( + `The managed manifest ${manifestPath} still lists every remaining path, so fix the cause and re-run the same command.`, + ); + await access(manifestPath); + }); +}); + +describe("clean when generated directories were replaced or locked", () => { + const target = ` antigravity: { + outDir: "out", + plugins: { acme: { source: "shared/acme" } } + }`; + + it("treats managed files below a directory now replaced by a file as gone", async () => { + const root = await makeProject(target); + await build({ cwd: root }); + await rm(path.join(root, "out/acme/skills"), { recursive: true }); + await writeFile(path.join(root, "out/acme/skills"), "USER FILE\n"); + + await clean({ cwd: root }); + + expect(await readFile(path.join(root, "out/acme/skills"), "utf8")).toBe( + "USER FILE\n", + ); + await expect(exists(path.join(root, "out/acme/plugin.json"))).resolves.toBe( + false, + ); + await expect( + exists(path.join(root, "out", managedManifestPath("antigravity"))), + ).resolves.toBe(false); + }); + + it.skipIf(process.getuid?.() === 0 || process.platform === "win32")( + "names the manifest and deletes nothing when a managed path cannot be inspected", + async () => { + const root = await makeProject(target); + await build({ cwd: root }); + const locked = path.join(root, "out/acme/skills"); + await chmod(locked, 0o000); + try { + const manifestPath = path.join( + root, + "out", + managedManifestPath("antigravity"), + ); + await expect(clean({ cwd: root })).rejects.toThrow( + new RegExp( + `Cannot check managed path "acme/skills/alpha/SKILL\\.md" under .*out: EACCES[\\s\\S]*Nothing was deleted\\. The managed manifest ${manifestPath.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")} lists this path`, + ), + ); + await access(path.join(root, "out/acme/plugin.json")); + await access(manifestPath); + } finally { + await chmod(locked, 0o755); + } + }, + ); +}); + +describe("an external outDir keeps its version-control data", () => { + it("refuses to clean a manifest entry inside the outDir's .git", async () => { + const root = await makeProject( + ` antigravity: { + outDir: "../sibling", + plugins: { acme: { source: "shared/acme" } } + }`, + ); + const sibling = path.join(root, "../sibling"); + await mkdir(path.join(sibling, ".git"), { recursive: true }); + await writeFile(path.join(sibling, ".git/config"), "[core]\n"); + try { + await build({ cwd: root }); + await writeManifest(root, "../sibling", "antigravity", [".git/config"]); + + await expect(clean({ cwd: root })).rejects.toThrow( + /\.git\/config -> resolves inside .*sibling\/\.git/, + ); + await access(path.join(sibling, ".git/config")); + } finally { + await rm(sibling, { recursive: true, force: true }); + } + }); +}); + +describe("config path fields stay inside the project", () => { + it("rejects source.partials outside the project at config load", async () => { + const root = await makeProject( + ` antigravity: { + outDir: "out", + plugins: { acme: { source: "shared/acme" } } + }`, + {}, + `source: { partials: "../outside" },`, + ); + + await expect(loadConfig(root)).rejects.toThrow( + /Invalid pluginpack config in .*: source\.partials: must be a safe relative path/, + ); + }); + + it.each([ + ["source.partials", `source: { partials: "https://../../outside" },`, ""], + [ + "targets.antigravity.plugins.acme.source", + "", + `source: "https://../../outside"`, + ], + ])( + "rejects a URL-shaped %s that resolves outside the project", + async (field, sourceBlock, pluginSource) => { + const root = await makeProject( + ` antigravity: { + outDir: "out", + plugins: { acme: { ${pluginSource || `source: "shared/acme"`} } } + }`, + {}, + sourceBlock, + ); + // path.resolve reads the value as a plain path with ".." segments. + expect(path.resolve(root, "https://../../outside")).toBe( + path.resolve(root, "../outside"), + ); + + await expect(loadConfig(root)).rejects.toThrow( + `${field}: must be a safe relative path`, + ); + }, + ); +}); + +describe("the config loader reads the config file fresh each time", () => { + it.each(["pluginpack.config.ts", "pluginpack.config.mjs"])( + "builds the edited %s on a second build() in the same process", + async (name) => { + const antigravity = ` antigravity: { + outDir: "out", + plugins: { acme: { source: "shared/acme" } } + }`; + const root = await makeProject(antigravity, { + "pluginpack.config.ts": null as never, + [name]: configText(antigravity), + }); + expect(existsSync(path.join(root, "pluginpack.config.ts"))).toBe( + name === "pluginpack.config.ts", + ); + const [first] = await build({ cwd: root }); + expect(first.target).toBe("antigravity"); + + await writeFile( + path.join(root, name), + configText( + antigravity + .replace("antigravity", "cursor") + .replace('"out"', '"out-cursor"'), + ), + ); + const [second] = await build({ cwd: root }); + + expect(second.target).toBe("cursor"); + }, + ); +}); + +describe("walkFiles distinguishes a missing directory from a file", () => { + it("throws when the directory is a file", async () => { + const root = await makeProject(` antigravity: { + outDir: "out", + plugins: { acme: { source: "shared/acme" } } + }`); + await writeFile(path.join(root, "not-a-dir"), "x\n"); + + await expect(walkFiles(path.join(root, "not-a-dir"))).rejects.toThrow( + `Expected a directory but found a file: ${path.join(root, "not-a-dir")}`, + ); + await expect(walkFiles(path.join(root, "missing"))).resolves.toEqual([]); + await expect( + walkFiles(path.join(root, "not-a-dir/below")), + ).resolves.toEqual([]); + }); + + it("fails a build whose source component directory is a file", async () => { + const root = await makeProject(` antigravity: { + outDir: "out", + plugins: { acme: { source: "shared/acme" } } + }`); + await rm(path.join(root, "shared/acme/skills"), { recursive: true }); + await writeFile(path.join(root, "shared/acme/skills"), "oops\n"); + + await expect(build({ cwd: root })).rejects.toThrow( + /Expected a directory but found a file: .*shared\/acme\/skills/, + ); + }); +});