Clean up after the unify, fingerprint and MCP fixes

package.json

@@ -1,6 +1,6 @@
 {
   "name": "craftar",
-  "version": "0.2.2",
+  "version": "0.2.3",
   "description": "Craft, sync and convert AI-coding workspace harnesses across clients and tools.",
   "license": "MIT",
   "type": "module",

src/cli.ts

@@ -25,7 +25,7 @@ import { UnifyPlanSchema, type IngredientRef, type Take, type UnifyPlan } from "
 process.stdout.on("error", (e: NodeJS.ErrnoException) => { if (e.code === "EPIPE") process.exit(0); });
 
 const program = new Command();
-program.name("craftar").description("Craft, sync and convert AI-coding workspace harnesses.").version("0.2.2");
+program.name("craftar").description("Craft, sync and convert AI-coding workspace harnesses.").version("0.2.3");
 
 /* ---------------------------------------------------------------- import */
 program
@@ -342,7 +342,7 @@ forge
     }
 
     const result = await applyPlan(base, variant, diff, toApply, { discardVariantMeta: o.take === "base" });
-    // Ruling 33, dry pass: every recipe and profile edit the cascade will make is checked before
+    // Ruling 33, dry pass: every recipe `ingredients` rewrite the cascade will make is checked before
     // the first byte is written, so a refusal (an aliased reference) leaves the Forge untouched.
     const cascadeFiles = result.resolved ? await checkRecipeCascade(f, base.ref, variant.ref) : [];
 
@@ -518,10 +518,12 @@ async function readText(p: string): Promise<string | null> {
  * path to exist, and a plan target usually does not yet — so the part that exists is resolved
  * (symlinks and all) and the part that does not is appended as written.
  *
- * It walks up only past a component that truly does not exist (`lstat` says ENOENT/ENOTDIR). A
- * component that exists but will not resolve — a dangling symlink, a symlink loop, a permission
- * error — throws instead: resolving the rest lexically would let a link that points into the Forge
- * pass the containment check, so unify fails closed rather than guess where the target lands.
+ * It walks up only past a component that truly does not exist (`lstat` says ENOENT/ENOTDIR). Any
+ * other outcome throws instead. A component `lstat` cannot inspect (a symlink loop or a permission
+ * error on Linux) "cannot be inspected"; one it can inspect but `realpath` cannot follow (a
+ * dangling symlink or junction, a loop on Windows) "exists but cannot be resolved". Resolving the
+ * rest lexically would let a link that points into the Forge pass the containment check, so unify
+ * fails closed rather than guess where the target lands.
  */
 async function realpathOfNearest(abs: string): Promise<string> {
   let head = abs;
@@ -541,16 +543,21 @@ async function realpathOfNearest(abs: string): Promise<string> {
           head = parent;
           continue;
         }
-        throw cannotResolve(head, lstatError);
+        throw cannotResolve(head, lstatError, "cannot be inspected");
       }
-      throw cannotResolve(head, realpathError);
+      throw cannotResolve(head, realpathError, "exists but cannot be resolved");
     }
   }
 }
 
-function cannotResolve(p: string, e: unknown): Error {
-  const code = (e as NodeJS.ErrnoException).code ?? (e instanceof Error ? e.message : String(e));
-  return new Error(`${p} exists but cannot be resolved (${code}) — unify cannot prove the target lies outside the Forge`);
+/**
+ * The refusal for a `--save-plan` target that cannot be resolved. `what` is "cannot be inspected"
+ * when `lstat` itself failed, so the path is not known to exist, and "exists but cannot be
+ * resolved" when `lstat` succeeded and `realpath` did not.
+ */
+function cannotResolve(p: string, e: unknown, what: "cannot be inspected" | "exists but cannot be resolved"): Error {
+  const code = (e as NodeJS.ErrnoException | null)?.code ?? (e instanceof Error ? e.message : String(e));
+  return new Error(`${p} ${what} (${code}) — unify cannot prove the target lies outside the Forge`);
 }
 
 /** Whether anything — a file, a directory, even a dangling symlink — already sits at `p`. */

src/emitters/shared.ts

@@ -36,6 +36,10 @@ export function mcpServers(ctx: EmitContext, target: string, file: string): Reco
     const key = outName(ing.meta);
     const prev = writtenBy.get(key);
     if (prev) ctx.warn(`${target}: two ingredients write the MCP server "${key}" into ${file}: ${prev} and ${ing.ref} (last wins)`);
+    // Last wins on the value, but a reassigned key keeps the slot where it was first written, so
+    // after a collision the surviving server can sit in the dropped one's position. It is warned
+    // about above. Such a file does not adopt: `sameJson` is key-order sensitive, so a workspace
+    // file with no lock entry reads as `collision`.
     servers[key] = ing.meta.server;
     writtenBy.set(key, ing.ref);
   }

test/cli.test.ts

@@ -771,8 +771,6 @@ describe("cli", () => {
     expect(await fs.readFile(path.join(root, "recipes/base.yaml"), "utf8")).toBe(before);
   });
 
-  // Ruling 42 withdrew this behaviour (the cascade no longer deletes recipes or edits profiles or extends); its test was removed.
-
   // Ruling 30: this test used to check Ruling 25's hint. That hint is gone, so its old assertion
   // (`not.toContain("sits inside the Forge")`) could never fail; it now checks the refusal and the
   // outside plan's content instead.
@@ -894,8 +892,6 @@ describe("cli — forge unify final review", () => {
   });
 });
 
-// Ruling 42 withdrew this behaviour (the cascade no longer deletes recipes or edits profiles or extends); its test was removed.
-
 function gitStatus(dir: string): string {
   return execFileSync("git", ["-C", dir, "status", "--porcelain"], { encoding: "utf8" });
 }
@@ -916,8 +912,6 @@ describe("cli — forge unify cascade refusals write nothing (Ruling 33)", () =>
     expect(await fs.readFile(path.join(root, "ingredients/rules/wf/rule.md"), "utf8")).toBe("a\n");
   });
 
-  // Ruling 42 withdrew this behaviour (the cascade no longer deletes recipes or edits profiles or extends); its test was removed.
-
   // A write that fails after writing began (a read-only recipe here; an I/O error or a locked file
   // on Windows in the wild) cannot be prevented by the dry pass — it must name what was written.
   // Root ignores the read-only bit on POSIX, so the scenario cannot fail there.
@@ -1002,8 +996,8 @@ describe("cli — forge unify --save-plan never writes into the Forge, never ove
 });
 
 describe("cli — forge unify --save-plan symlink escape (Ruling 30)", () => {
-  // Creating a symlink needs privileges on Windows; this runs on Linux CI.
-  it.skipIf(process.platform === "win32")("refuses a target that reaches inside the Forge through a symlink, and writes nothing", async () => {
+  // On Windows a directory junction needs no privilege, so this runs on both platforms.
+  it("refuses a target that reaches inside the Forge through a symlink, and writes nothing", async () => {
     const root = await tmpDir("craftar-cli-forge-");
     cleanups.push(() => fs.rm(root, { recursive: true, force: true }));
     // A real `recipes/` directory, so the link below resolves inside the Forge — without a recipe
@@ -1018,7 +1012,7 @@ describe("cli — forge unify --save-plan symlink escape (Ruling 30)", () => {
     const outside = await tmpDir("craftar-cli-plan-");
     cleanups.push(() => fs.rm(outside, { recursive: true, force: true }));
     const link = path.join(outside, "into-forge");
-    await fs.symlink(path.join(root, "recipes"), link, "dir");
+    await fs.symlink(path.join(root, "recipes"), link, process.platform === "win32" ? "junction" : "dir");
 
     const before = await snapshot(root);
     const r = runCli(["forge", "unify", "rule/wf", "--profile", "acme", "--save-plan", path.join(link, "plan.yaml"), "--forge", root]);
@@ -1134,8 +1128,6 @@ describe("cli — forge unify refuses index-flagged paths (Ruling 39)", () => {
 });
 
 describe("cli — forge unify checks the cascade's own files are held by git (Ruling 37)", () => {
-  // Ruling 42 withdrew this behaviour (the cascade no longer deletes recipes or edits profiles or extends); its test was removed.
-
   it("refuses when only a recipe the cascade would rewrite is untracked (showUntrackedFiles=no), and leaves it unchanged", async () => {
     const root = await tmpDir("craftar-cli-forge-");
     cleanups.push(() => fs.rm(root, { recursive: true, force: true }));
@@ -1188,8 +1180,6 @@ describe("cli — forge unify's held-by-git refusal names every path, Forge-rela
   });
 });
 
-// Ruling 42 withdrew this behaviour (the cascade no longer deletes recipes or edits profiles or extends); its test was removed.
-
 describe("cli — forge unify's held-by-git refusal caps its list (docs-author nit)", () => {
   it("lists the first ten paths git does not hold and counts the rest", async () => {
     const root = await tmpDir("craftar-cli-forge-");
@@ -1316,8 +1306,8 @@ describe("cli — forge unify's cascade rewrites ingredients only (Ruling 42)",
 });
 
 describe("cli — forge unify --save-plan through a dangling symlink (Ruling 30, CI round)", () => {
-  // Creating a symlink needs privileges on Windows; this runs on Linux CI.
-  it.skipIf(process.platform === "win32")("refuses a target whose path passes through a dangling symlink, and writes nothing", async () => {
+  // On Windows a directory junction needs no privilege, so this runs on both platforms.
+  it("refuses a target whose path passes through a dangling symlink, and writes nothing", async () => {
     const root = await tmpDir("craftar-cli-forge-");
     cleanups.push(() => fs.rm(root, { recursive: true, force: true }));
     await makeForge(root, { ingredients: [rule("wf", "a\n"), rule("wf--acme", "b\n", { as: "wf" })] });
@@ -1327,7 +1317,7 @@ describe("cli — forge unify --save-plan through a dangling symlink (Ruling 30,
     const outside = await tmpDir("craftar-cli-plan-");
     cleanups.push(() => fs.rm(outside, { recursive: true, force: true }));
     const link = path.join(outside, "dangling");
-    await fs.symlink(path.join(root, "not-there-yet"), link, "dir"); // points into the Forge, at nothing
+    await fs.symlink(path.join(root, "not-there-yet"), link, process.platform === "win32" ? "junction" : "dir"); // points into the Forge, at nothing
 
     const before = await snapshot(root);
     const r = runCli(["forge", "unify", "rule/wf", "--profile", "acme", "--save-plan", path.join(link, "plan.yaml"), "--forge", root]);
@@ -1337,4 +1327,28 @@ describe("cli — forge unify --save-plan through a dangling symlink (Ruling 30,
     expect(r.stderr).not.toContain("ENOENT: no such file");
     expect(await snapshot(root)).toEqual(before);
   });
+
+  it("refuses a target whose path passes through a symlink loop, and writes nothing", async () => {
+    const root = await tmpDir("craftar-cli-forge-");
+    cleanups.push(() => fs.rm(root, { recursive: true, force: true }));
+    await makeForge(root, { ingredients: [rule("wf", "a\n"), rule("wf--acme", "b\n", { as: "wf" })] });
+    gitInit(root);
+    gitCommitAll(root, "init");
+
+    const outside = await tmpDir("craftar-cli-plan-");
+    cleanups.push(() => fs.rm(outside, { recursive: true, force: true }));
+    const type = process.platform === "win32" ? "junction" : "dir";
+    await fs.symlink(path.join(outside, "loopb"), path.join(outside, "loopa"), type);
+    await fs.symlink(path.join(outside, "loopa"), path.join(outside, "loopb"), type);
+
+    const before = await snapshot(root);
+    const r = runCli(["forge", "unify", "rule/wf", "--profile", "acme", "--save-plan", path.join(outside, "loopa", "plan.yaml"), "--forge", root]);
+    expect(r.code).toBe(1);
+    expect(r.stderr).toContain("cannot prove the target lies outside the Forge");
+    // `lstat` through a symlink loop fails with ELOOP on Linux, so the path cannot even be
+    // inspected; on Windows `lstat` sees the junction and `realpath` fails, the other branch.
+    if (process.platform !== "win32") expect(r.stderr).toContain("cannot be inspected (ELOOP)");
+    else expect(r.stderr).toContain("exists but cannot be resolved");
+    expect(await snapshot(root)).toEqual(before);
+  });
 });

test/emitters/claude-code.test.ts

@@ -57,6 +57,19 @@ describe("claude-code emitter", () => {
     expect(text(p, ".mcp.json")).toBe(JSON.stringify({ mcpServers: { srv: { command: "npx", args: ["acme-server"] } } }, null, 2) + "\n");
   });
 
+  it("names each dropped writer once when three MCP ingredients emit the same server name", async () => {
+    const p = await planFor([
+      { meta: { type: "mcp", name: "srv", server: { command: "a" } } },
+      { meta: { type: "mcp", name: "srv--b", as: "srv", server: { command: "b" } } },
+      { meta: { type: "mcp", name: "srv--c", as: "srv", server: { command: "c" } } },
+    ]);
+    expect(p.warnings.filter((w) => w.includes('MCP server "srv"'))).toEqual([
+      'claude-code: two ingredients write the MCP server "srv" into .mcp.json: mcp/srv and mcp/srv--b (last wins)',
+      'claude-code: two ingredients write the MCP server "srv" into .mcp.json: mcp/srv--b and mcp/srv--c (last wins)',
+    ]);
+    expect(text(p, ".mcp.json")).toBe(JSON.stringify({ mcpServers: { srv: { command: "c" } } }, null, 2) + "\n");
+  });
+
   it("warns when two MCP ingredients emit the same server name, instead of dropping one silently", async () => {
     const p = await planFor([
       { meta: { type: "mcp", name: "srv", server: { command: "npx", args: ["public-server"] } } },

test/emitters/kiro.test.ts

@@ -85,8 +85,8 @@ describe("kiro emitter", () => {
 
   it("emits an MCP variant under its original server name", async () => {
     const p = await planFor([{ meta: { type: "mcp", name: "srv--acme", as: "srv", server: { command: "npx", args: ["acme-server"] } } }]);
-    const json = JSON.parse(file(p, ".kiro/settings/mcp.json")!.content.toString("utf8"));
-    expect(Object.keys(json.mcpServers)).toEqual(["srv"]);
+    const f = file(p, ".kiro/settings/mcp.json")!;
+    expect(f.content.toString("utf8")).toBe(JSON.stringify({ mcpServers: { srv: { command: "npx", args: ["acme-server"] } } }, null, 2).replace(/\n/g, "\r\n") + "\r\n");
   });
 
   it("warns when two MCP ingredients emit the same server name, instead of dropping one silently", async () => {
@@ -95,5 +95,6 @@ describe("kiro emitter", () => {
       { meta: { type: "mcp", name: "srv--acme", as: "srv", server: { command: "npx", args: ["acme-server"] } } },
     ]);
     expect(p.warnings).toContain('kiro: two ingredients write the MCP server "srv" into .kiro/settings/mcp.json: mcp/srv and mcp/srv--acme (last wins)');
+    expect(JSON.parse(file(p, ".kiro/settings/mcp.json")!.content.toString("utf8")).mcpServers).toEqual({ srv: { command: "npx", args: ["acme-server"] } });
   });
 });

test/unify.test.ts

@@ -370,8 +370,6 @@ describe("rewriteRecipes", () => {
     expect(reloaded.recipes.get("base--acme")!.ingredients).toEqual(["rule/workflow", "rule/other"]);
   });
 
-  // Ruling 42 withdrew this behaviour (the cascade no longer deletes recipes or edits profiles or extends); its test was removed.
-
   it("leaves a suffixed recipe alone when it has no unsuffixed sibling", async () => {
     const forge = await forgeWith({
       ingredients: [rule("workflow", "a\n"), rule("workflow--acme", "b\n", { as: "workflow" })],
@@ -385,7 +383,7 @@ describe("rewriteRecipes", () => {
     expect(reloaded.profiles.get("acme")!.recipes).toEqual(["solo--acme"]);
   });
 
-  it("does not delete a sibling whose ingredients differ beyond the variant", async () => {
+  it("does not report a sibling whose ingredients differ beyond the variant as identical", async () => {
     const forge = await forgeWith({
       ingredients: [rule("workflow", "a\n"), rule("workflow--acme", "b\n", { as: "workflow" })],
       recipes: [
@@ -395,14 +393,14 @@ describe("rewriteRecipes", () => {
       profiles: [profile("acme", ["base--acme"])],
     });
     const out = await rewriteRecipes(forge, "rule/workflow", "rule/workflow--acme", "acme");
-    expect(out.identicalToSibling).toEqual([]); // Ruling 42: `deleted` is gone; not identical, not reported
+    expect(out.identicalToSibling).toEqual([]);
   });
 });
 
 /**
- * Loads a Forge from hand-written files rather than `makeForge`, so a test can put a recipe or
- * profile under a filename/dirname that disagrees with its own `name` field, or control raw
- * bytes (comments, EOL, BOM) precisely. `rewriteRecipes` never touches `ingredients/`, so these
+ * Loads a Forge from hand-written files rather than `makeForge`, so a test can put a recipe under
+ * a filename that disagrees with its own `name` field, or control the raw bytes (comments, EOL,
+ * BOM) of recipes and profiles precisely. `rewriteRecipes` never touches `ingredients/`, so these
  * scenarios skip writing ingredient directories entirely.
  */
 async function bareForge(files: Record<string, string>): Promise<Forge> {
@@ -412,7 +410,7 @@ async function bareForge(files: Record<string, string>): Promise<Forge> {
   return loadForge(root);
 }
 
-describe("rewriteRecipes — file identity, byte fidelity and safety (Rulings 9, 10, 12, 13, 14)", () => {
+describe("rewriteRecipes — file identity, byte fidelity and safety (Rulings 9, 10, 13, 14)", () => {
   it("finds a recipe by its `name` field, not its filename, and never touches a same-named decoy", async () => {
     const forge = await bareForge({
       // The real "base--acme" recipe lives in a file named after something else (Ruling 9).
@@ -435,10 +433,6 @@ describe("rewriteRecipes — file identity, byte fidelity and safety (Rulings 9,
     expect(decoyText).toBe("name: solo\ningredients:\n  - rule/other\n");
   });
 
-  // Ruling 42 withdrew this behaviour (the cascade no longer deletes recipes or edits profiles or extends); its test was removed.
-
-  // Ruling 42 withdrew this behaviour (the cascade no longer deletes recipes or edits profiles or extends); its test was removed.
-
   it("keeps a CRLF, BOM-prefixed recipe file's EOL and BOM after the rewrite (Ruling 13)", async () => {
     const BOM = "";
     const crlf = `${BOM}name: solo--acme\r\ningredients:\r\n  - rule/workflow--acme\r\n  - rule/other\r\n`;
@@ -563,7 +557,3 @@ describe("unify — non-UTF-8 content (Ruling 31)", () => {
     expect(r.write["rule.md"]).toBe(`${BOM}a\nnew\n`);
   });
 });
-
-// Ruling 42 withdrew this behaviour (the cascade no longer deletes recipes or edits profiles or extends); its test was removed.
-
-// Ruling 42 withdrew this behaviour (the cascade no longer deletes recipes or edits profiles or extends); its test was removed.