test: cover the destructive paths, and fix two gaps they exposed

src/cleanup.ts

@@ -1,12 +1,19 @@
 import path from "node:path";
 import { build } from "./build.js";
+import { targetNames } from "./adapters.js";
 import { loadProjectConfig } from "./config.js";
 import {
   buildDeleteGuard,
   cleanManagedFiles,
+  normalizeManagedPath,
   pruneManagedFiles,
+  readManagedManifest,
 } from "./managed.js";
-import type { CleanupResult, TargetName } from "./types.js";
+import type {
+  CleanupResult,
+  ResolvedProjectConfig,
+  TargetName,
+} from "./types.js";
 
 /** Deletes managed files no longer produced by the current build, without rewriting current output. */
 export async function prune(
@@ -60,6 +67,7 @@ export async function clean(
   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];
@@ -76,3 +84,58 @@ export async function clean(
   }
   return results;
 }
+
+/**
+ * Refuses to clean a path another configured 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.
+ *
+ * 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.
+ */
+async function assertNoManifestCollisions(
+  project: ResolvedProjectConfig,
+  cleaning: TargetName[],
+  force?: boolean,
+): Promise<void> {
+  if (force) {
+    return;
+  }
+  const owners = new Map<string, TargetName>();
+  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);
+    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}`);
+        }
+        continue;
+      }
+      owners.set(absolute, target);
+    }
+  }
+  if (collisions.length > 0) {
+    throw new Error(
+      `Refusing to clean paths claimed by more than one target's managed manifest:\n${collisions.join("\n")}\n` +
+        `Deleting them would remove another target's live output. Give the targets distinct outDirs and rebuild, or re-run with --force to delete anyway.`,
+    );
+  }
+}

src/managed.ts

@@ -51,13 +51,19 @@ export async function readManagedManifest(
     }
     throw error;
   }
-  let parsed: Partial<ManagedManifest>;
+  let parsed: Partial<ManagedManifest> | null;
   try {
-    parsed = JSON.parse(raw) as Partial<ManagedManifest>;
+    parsed = JSON.parse(raw) as Partial<ManagedManifest> | null;
   } catch {
     throw new Error(`Invalid managed manifest: ${manifestPath}`);
   }
   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) ||

tests/core.test.ts

@@ -3347,6 +3347,122 @@ export default defineConfig({
     );
   });
 
+  it("rejects a root skills plugin id that collides with a discovered source plugin", async () => {
+    const project = await fixtureProject({
+      "pluginpack.config.ts": `import { defineConfig } from "${path.resolve("src/index.ts")}";
+
+export default defineConfig({
+  name: "id-collision-plugins",
+  version: "1.0.0",
+  source: { skills: "skills", rootPlugin: { id: "demo" } },
+  metadata: { description: "C", author: { name: "C" }, license: "MIT" },
+  targets: {
+    claude: { outDir: "dist/claude", plugins: { demo: { from: ["demo"] } } }
+  }
+});
+`,
+      skills: { root: { "SKILL.md": skill("root", "Root skill.") } },
+      plugins: {
+        demo: {
+          skills: { demo: { "SKILL.md": skill("demo", "Demo skill.") } },
+        },
+      },
+    });
+
+    await expect(
+      build({ cwd: project.baseDir, target: "claude" }),
+    ).rejects.toThrow(
+      /Root skills source plugin "demo" conflicts with an existing source plugin/,
+    );
+  });
+
+  it("rejects a configured source.skills directory that does not exist", async () => {
+    const project = await fixtureProject({
+      "pluginpack.config.ts": `import { defineConfig } from "${path.resolve("src/index.ts")}";
+
+export default defineConfig({
+  name: "missing-skills-plugins",
+  version: "1.0.0",
+  source: { skills: "skills", rootPlugin: { id: "core" } },
+  metadata: { description: "M", author: { name: "M" }, license: "MIT" },
+  targets: {
+    claude: { outDir: "dist/claude", plugins: { core: { from: ["core"] } } }
+  }
+});
+`,
+    });
+
+    await expect(
+      build({ cwd: project.baseDir, target: "claude" }),
+    ).rejects.toThrow(/Root skills source directory is missing/);
+  });
+
+  it("rejects a rootFiles source that cannot be read", async () => {
+    const project = await fixtureProject({
+      "pluginpack.config.ts": `import { defineConfig } from "${path.resolve("src/index.ts")}";
+
+export default defineConfig({
+  name: "rootfiles-missing-plugins",
+  version: "1.0.0",
+  metadata: { description: "R", author: { name: "R" }, license: "MIT" },
+  targets: {
+    claude: {
+      outDir: "dist/claude",
+      plugins: { demo: { from: ["demo"] } },
+      rootFiles: { "README.md": "NOT_THERE.md" }
+    }
+  }
+});
+`,
+      plugins: {
+        demo: {
+          skills: { demo: { "SKILL.md": skill("demo", "Demo skill.") } },
+        },
+      },
+    });
+
+    await expect(
+      build({ cwd: project.baseDir, target: "claude" }),
+    ).rejects.toThrow(/rootFiles source "NOT_THERE\.md" could not be read/);
+  });
+
+  // The engine also checks rootFiles destinations with isSafeRelativePath, but
+  // config validation rejects an escaping key first, so that check is
+  // unreachable defense-in-depth. Pin the guarantee at the layer that enforces
+  // it, rather than asserting an error the user can never actually see.
+  it("rejects a rootFiles destination that escapes the output directory", async () => {
+    const project = await fixtureProject({
+      "pluginpack.config.ts": `import { defineConfig } from "${path.resolve("src/index.ts")}";
+
+export default defineConfig({
+  name: "rootfiles-unsafe-plugins",
+  version: "1.0.0",
+  metadata: { description: "R", author: { name: "R" }, license: "MIT" },
+  targets: {
+    claude: {
+      outDir: "dist/claude",
+      plugins: { demo: { from: ["demo"] } },
+      rootFiles: { "../escaped.md": "SOURCE.md" }
+    }
+  }
+});
+`,
+      "SOURCE.md": "# Source\n",
+      plugins: {
+        demo: {
+          skills: { demo: { "SKILL.md": skill("demo", "Demo skill.") } },
+        },
+      },
+    });
+
+    await expect(
+      build({ cwd: project.baseDir, target: "claude" }),
+    ).rejects.toThrow(
+      /Invalid pluginpack config in .*: targets\.claude\.rootFiles\.\.\.\/escaped\.md: Invalid key in record/,
+    );
+    await expectMissing(path.join(project.baseDir, "escaped.md"));
+  });
+
   it("rejects malformed JSON in a source plugin's .mcp.json", async () => {
     const project = await fixtureProject({
       "pluginpack.config.ts": `import { defineConfig } from "${path.resolve("src/index.ts")}";