fix(farm): make the ca tools suite pass on Windows CI (#541, #542)

CHANGELOG.md

@@ -12,6 +12,36 @@ predate the plugin rewrite and are grouped by date.
 
 ## [Unreleased]
 
+## [2.10.4] — 2026-07-29
+
+### Fixed
+
+- **The symlink-safe worktree writer accepts an absolute destination spelled
+  through a link (#541).** `writeWorktreeFile` canonicalized the worktree ROOT
+  but resolved the caller's absolute path as-is, then compared the two — so a
+  caller building a destination from the root it had itself passed in was
+  refused for writing inside its own worktree. Same root cause as #539, one
+  module down, and it never reproduced on a host whose paths are already
+  canonical.
+
+  Only the ANCESTOR chain is canonicalized; the final component stays literal,
+  so a symlinked destination is still refused rather than written through. Like
+  #539 this tightens the guard: a path whose ancestors traverse a link out of
+  the root is now caught by containment instead of relying on a later check.
+
+- **A timed-out farm test no longer abandons a live subprocess (#542).** Every
+  launch in `farm.test.ts` is a real child process and none were tracked, so a
+  vitest timeout left one running with the temp directory as its cwd and the
+  teardown then failed `EBUSY` — reporting the cleanup instead of the timeout
+  that caused it. Children are now reaped before the directory is removed, and
+  the removal tolerates Windows' asynchronous handle release.
+
+  The heaviest case (12 tasks at `FARM_CONCURRENCY` 6) also gets a budget drawn
+  from measurement rather than a nudge: 3553 ms isolated on a developer box
+  against a 5000 ms default, 5331 ms under load on a CI runner. It is now 30 s —
+  bounded, so a genuine hang still fails. The case itself is unchanged;
+  concurrency stays at 6 and every assertion stands.
+
 ## [2.10.3] — 2026-07-28
 
 ### Fixed

README.md

@@ -9,7 +9,7 @@ Every intent routes through a gated skill or reviewer agent. Nothing commits unt
 <img alt="Claude Code plugin" src="https://img.shields.io/badge/Claude_Code-plugin-d97757">
 <img alt="Codex plugin" src="https://img.shields.io/badge/OpenAI_Codex-plugin-10a37f">
 <img alt="Pi Feature Forge preview" src="https://img.shields.io/badge/ca--pi-Feature_Forge_preview-d97757">
-<img alt="version 2.10.3" src="https://img.shields.io/badge/version-2.10.3-2b7489">
+<img alt="version 2.10.4" src="https://img.shields.io/badge/version-2.10.4-2b7489">
 <img alt="commands" src="https://img.shields.io/badge/commands-40-555">
 <img alt="skills" src="https://img.shields.io/badge/skills-23-555">
 <img alt="agents" src="https://img.shields.io/badge/agents-28-555">

plugins/ca/.claude-plugin/plugin.json

@@ -2,7 +2,7 @@
   "name": "ca",
   "displayName": "codeArbiter",
   "description": "Orchestration layer for Claude Code. Routes every intent through gated skills and reviewer agents, drives spec-driven TDD, mechanically enforces the commit and audit-trail gates, decides via SMARTS, and keeps an append-only audit trail. Requires Python 3 on PATH. Dormant until you opt a repo in; run /ca:init to activate.",
-  "version": "2.10.3",
+  "version": "2.10.4",
   "author": { "name": "arbiterForge" },
   "license": "AGPL-3.0-only",
   "homepage": "https://github.com/arbiterForge/codeArbiter",

plugins/ca/tools/farm.js

@@ -286,6 +286,24 @@ function contained(root, candidate) {
   const relative = path2.relative(root, candidate);
   return relative === "" || !relative.startsWith(`..${path2.sep}`) && relative !== ".." && !path2.isAbsolute(relative);
 }
+async function canonicalizeAncestors(target, resolvePath = realpath) {
+  const resolved = path2.resolve(target);
+  const leaf = path2.basename(resolved);
+  const missing = [];
+  let cursor = path2.dirname(resolved);
+  for (; ; ) {
+    try {
+      const real = await resolvePath(cursor);
+      return path2.join(real, ...missing.slice().reverse(), leaf);
+    } catch (error) {
+      if (!isMissing(error)) throw error;
+      const parent = path2.dirname(cursor);
+      if (parent === cursor) return resolved;
+      missing.push(path2.basename(cursor));
+      cursor = parent;
+    }
+  }
+}
 function sameFile(left, right) {
   return left.dev === right.dev && left.ino === right.ino;
 }
@@ -308,7 +326,8 @@ async function writeWorktreeFile(worktree, relPath, contents, testHooks = {}) {
         if (!contained(canonicalRoot, await realpath(verified.path))) unsafe();
       }
     };
-    const target = path2.resolve(canonicalRoot, relPath);
+    const requested = path2.isAbsolute(relPath) ? await canonicalizeAncestors(relPath) : relPath;
+    const target = path2.resolve(canonicalRoot, requested);
     const relative = path2.relative(canonicalRoot, target);
     if (relative === "" || relative === ".." || relative.startsWith(`..${path2.sep}`) || path2.isAbsolute(relative)) unsafe();
     const segments = relative.split(path2.sep);

plugins/ca/tools/farm.test.ts

@@ -72,6 +72,55 @@ function createTempRepo(dir: string) {
 // --------------------------------------------------------------------------
 // Run farm.ts via tsx (dev path) against the temp repo
 // --------------------------------------------------------------------------
+// #542 — every farm launch here is a real subprocess, and nothing tracked it.
+// When vitest abandons a case on timeout the child keeps running, keeps its cwd
+// open, and the afterEach `rmSync(tmpDir)` then fails with EBUSY on Windows. The
+// teardown error is louder than the timeout that caused it, so the log names the
+// wrong problem.
+//
+// Track every live child so teardown can release the directory before deleting
+// it. A test that times out is already failing; it must not also corrupt the
+// diagnosis of the next one.
+const liveChildren = new Set<ReturnType<typeof spawn>>();
+
+function trackChild(child: ReturnType<typeof spawn>): void {
+  liveChildren.add(child);
+  child.on("close", () => liveChildren.delete(child));
+}
+
+/** Kill any subprocess still running, and wait for the OS to release its handles. */
+async function reapStrayChildren(): Promise<void> {
+  if (liveChildren.size === 0) return;
+  for (const child of liveChildren) {
+    try {
+      child.kill("SIGKILL");
+    } catch {
+      /* already gone */
+    }
+  }
+  // Windows releases the cwd handle asynchronously after the process dies, so a
+  // kill alone is not enough to make rmSync succeed on the next line.
+  for (let i = 0; i < 40 && liveChildren.size > 0; i++) {
+    await new Promise((r) => setTimeout(r, 25));
+  }
+  liveChildren.clear();
+}
+
+/** `rmSync` that tolerates Windows' asynchronous handle release. */
+function rmWithRetry(target: string): void {
+  for (let attempt = 1; attempt <= 10; attempt++) {
+    try {
+      rmSync(target, { recursive: true, force: true });
+      return;
+    } catch (error) {
+      const code = (error as NodeJS.ErrnoException).code;
+      if ((code !== "EBUSY" && code !== "EPERM" && code !== "ENOTEMPTY") || attempt === 10) throw error;
+      const until = Date.now() + 50 * attempt;
+      while (Date.now() < until) { /* brief spin; afterEach is sync */ }
+    }
+  }
+}
+
 function runFarm(
   repoDir: string,
   planPath: string,
@@ -101,6 +150,7 @@ function runFarm(
         },
       },
     );
+    trackChild(child);
     let out = "";
     child.stdout.on("data", (d: Buffer) => (out += d));
     child.stderr.on("data", (d: Buffer) => (out += d));
@@ -141,6 +191,7 @@ function runFarmWithArgs(
       ["--import", TSX_LOADER, farmTs, ...extraArgs, planPath],
       { cwd: repoDir, env: spawnEnv },
     );
+    trackChild(child);
     let out = "";
     child.stdout.on("data", (d: Buffer) => (out += d));
     child.stderr.on("data", (d: Buffer) => (out += d));
@@ -181,9 +232,12 @@ describe("farm.ts smoke tests", () => {
     createTempRepo(tmpDir);
   });
 
-  afterEach(() => {
+  afterEach(async () => {
     mockServer?.close();
-    rmSync(tmpDir, { recursive: true, force: true });
+    // #542, same reasoning as the artifact block: this suite launches the same
+    // real subprocesses, so it inherits the same abandoned-child teardown.
+    await reapStrayChildren();
+    rmWithRetry(tmpDir);
     // Clean up any leftover farm worktrees
     rmSync(join(tmpDir, "../.codearbiter-farm"), { recursive: true, force: true });
   });
@@ -1483,9 +1537,13 @@ describe("farm artifact publication (#397 / #387)", () => {
     createTempRepo(tmpDir);
   });
 
-  afterEach(() => {
+  afterEach(async () => {
     mockServer?.close();
-    rmSync(tmpDir, { recursive: true, force: true });
+    // #542: release any subprocess a timed-out case abandoned BEFORE deleting
+    // its cwd, or the delete fails EBUSY and reports the teardown instead of
+    // the timeout that caused it.
+    await reapStrayChildren();
+    rmWithRetry(tmpDir);
   });
 
   // -------------------------------------------------------------------------
@@ -1727,7 +1785,20 @@ describe("farm artifact publication (#397 / #387)", () => {
     expect(result.out).toContain(join(".farm", "runs", "mdonly"));
   });
 
-  it("#387: the unavailable-diff list is bounded and still reports the true total", async () => {
+  // #542 — MEASURED, not nudged. This is the heaviest case in the file: 12 tasks
+  // at FARM_CONCURRENCY 6, each spawning a real gate subprocess. Isolated on a
+  // developer Windows box it takes 3553ms against the 5000ms default — 29%
+  // headroom before any load at all. Under full-suite load on a windows-latest
+  // runner it took 5331ms and timed out, which then abandoned a live child and
+  // turned the teardown into a second, louder EBUSY failure.
+  //
+  // 30s is ~6x the slowest observed run: comfortable for a loaded runner, still
+  // bounded, so a genuine hang fails rather than sitting until the job timeout.
+  // The case is NOT made cheaper - FARM_CONCURRENCY stays 6 and the bounded-at-11
+  // / true-total-12 assertions are untouched (#515 AC-4, #542 AC-2). The six-way
+  // path is the thing under test; buying headroom by shrinking it would delete
+  // the coverage this case exists for.
+  it("#387: the unavailable-diff list is bounded and still reports the true total", { timeout: 30_000 }, async () => {
     ({ server: mockServer, port } = await startMockServer(greenHandler()));
     const ids = Array.from({ length: 12 }, (_, i) => `bulk${i}`);
     const planPath = writePlan("plan.json", planFor("bounded", ids, port));

plugins/ca/tools/worktree-fs.test.ts

@@ -1,9 +1,9 @@
-import { link, lstat, mkdir, mkdtemp, readdir, readFile, rm, symlink, writeFile } from "node:fs/promises";
+import { link, lstat, mkdir, mkdtemp, readdir, readFile, realpath, rm, symlink, writeFile } from "node:fs/promises";
 import { existsSync } from "node:fs";
 import { tmpdir } from "node:os";
 import path from "node:path";
 import { describe, expect, it } from "vitest";
-import { isUnsafeWorktreePathError, writeWorktreeFile } from "./worktree-fs.ts";
+import { canonicalizeAncestors, isUnsafeWorktreePathError, writeWorktreeFile } from "./worktree-fs.ts";
 
 type WorktreeFs = typeof import("./worktree-fs.ts");
 
@@ -133,11 +133,15 @@ describe("symlink-safe worktree writer", () => {
  * byte and 40 concurrent calls reached three of them through the public API with
  * no seam at all.
  *
- * Platform note: CI runs the tools job on ubuntu-latest ONLY, so Linux is the
- * platform that matters for these numbers. Windows and Linux do not agree here —
- * an over-long path segment reaches the mkdir failure arm on Windows but fails
- * earlier at `lstat` with ENAMETOOLONG on Linux, so that case bought nothing on
- * CI and is not in this block.
+ * Platform note: this tree now runs on BOTH ubuntu-latest and windows-latest —
+ * the tools job is still ubuntu-only, but #521's coverage-union cells execute
+ * the suite on each host and merge the reports, so Linux is no longer the only
+ * platform whose numbers count. Windows and Linux still do not agree here: an
+ * over-long path segment reaches the mkdir failure arm on Windows but fails
+ * earlier at `lstat` with ENAMETOOLONG on Linux. That divergence is the reason
+ * the union exists rather than a reason to ignore one side, and it is why #541
+ * — an absolute destination refused on Windows alone — went unseen until a
+ * second host ran this file.
  */
 describe("symlink-safe worktree writer — refusal paths", () => {
   const MESSAGE = "unsafe worktree path rejected";
@@ -234,6 +238,133 @@ describe("symlink-safe worktree writer — refusal paths", () => {
     }
   });
 
+  // #541 — the absolute-destination check compared the caller's spelling of the
+  // root against `realpath(worktree)`. Those are the same directory and
+  // different strings whenever a link or a Windows 8.3 short name is on the
+  // path, so a caller writing INSIDE its own worktree was refused. Same root
+  // cause as #539, one module down; it never reproduced on a developer box
+  // whose tmpdir is already canonical, and surfaced only when #521's coverage
+  // union first ran this tree on Windows CI.
+  //
+  // A junction gives two real spellings of one directory on any platform.
+  it("accepts an absolute destination spelled through a link into the same root", async () => {
+    const base = await realpath(await mkdtemp(path.join(tmpdir(), "farm-541-")));
+    try {
+      const real = path.join(base, "real");
+      const root = path.join(real, "wt");
+      await mkdir(root, { recursive: true });
+      const link = path.join(base, "link");
+      try {
+        await symlink(real, link, "junction");
+      } catch {
+        return; // unprivileged Windows without developer mode
+      }
+      const rootViaLink = path.join(link, "wt");
+
+      // Precondition: the two spellings are NOT lexically comparable, which is
+      // the whole reason this case exists.
+      expect(path.relative(root, rootViaLink).startsWith("..")).toBe(true);
+
+      // The caller passes its own spelling as the worktree AND builds the
+      // destination from it — the natural thing to do.
+      await writeWorktreeFile(rootViaLink, path.join(rootViaLink, "inside.txt"), "accepted\n");
+      expect(await readFile(path.join(root, "inside.txt"), "utf8")).toBe("accepted\n");
+
+      // Mirror: canonical root, destination spelled through the link.
+      await writeWorktreeFile(root, path.join(rootViaLink, "mirror.txt"), "accepted\n");
+      expect(await readFile(path.join(root, "mirror.txt"), "utf8")).toBe("accepted\n");
+
+      // Deep, not-yet-created ancestors through the link still land inside.
+      await writeWorktreeFile(rootViaLink, path.join(rootViaLink, "a", "b", "deep.txt"), "deep\n");
+      expect(await readFile(path.join(root, "a", "b", "deep.txt"), "utf8")).toBe("deep\n");
+    } finally {
+      await rm(base, { recursive: true, force: true });
+    }
+  });
+
+  it("does NOT pre-resolve the leaf, so a symlinked destination is still refused", async () => {
+    // The canonicalization added for #541 walks the ANCESTOR chain only. If it
+    // resolved the final component too, a symlink AT the destination pointing
+    // to another file INSIDE the root would resolve to a contained path, pass
+    // containment, and be written THROUGH — silently overwriting the target the
+    // per-segment checks exist to protect. Refusing symlinked destinations is
+    // this module's contract, and canonicalizing must not quietly undo it.
+    const base = await realpath(await mkdtemp(path.join(tmpdir(), "farm-541leaf-")));
+    try {
+      const root = path.join(base, "wt");
+      await mkdir(root, { recursive: true });
+      await writeFile(path.join(root, "real.txt"), "original\n", "utf8");
+      try {
+        await symlink(path.join(root, "real.txt"), path.join(root, "alias.txt"), "file");
+      } catch {
+        return; // unprivileged Windows without developer mode
+      }
+      await expectRefused(writeWorktreeFile(root, path.join(root, "alias.txt"), "written-through\n"));
+      expect(await readFile(path.join(root, "real.txt"), "utf8")).toBe("original\n");
+    } finally {
+      await rm(base, { recursive: true, force: true });
+    }
+  });
+
+  it("REFUSES when an ancestor cannot be canonicalized for a reason other than absence", async () => {
+    // EACCES and ELOOP are not portably inducible, so the failure is injected.
+    // Without this the refusal branch never runs and the fail-closed stance is
+    // a comment rather than behaviour.
+    const err = (code: string) => {
+      const e = new Error(`simulated ${code}`) as NodeJS.ErrnoException;
+      e.code = code;
+      return e;
+    };
+    await expect(
+      canonicalizeAncestors(path.join(path.sep, "repo", "wt", "f.txt"), async () => { throw err("EACCES"); }),
+    ).rejects.toThrow(/EACCES/);
+    await expect(
+      canonicalizeAncestors(path.join(path.sep, "repo", "wt", "f.txt"), async () => { throw err("ELOOP"); }),
+    ).rejects.toThrow(/ELOOP/);
+  });
+
+  it("treats a missing ancestor as normal and walks up to the deepest existing one", async () => {
+    const seen: string[] = [];
+    const target = path.join(path.sep, "a", "b", "c", "f.txt");
+    const out = await canonicalizeAncestors(target, async (p) => {
+      seen.push(p);
+      if (p === path.resolve(path.sep, "a")) return path.resolve(path.sep, "REAL");
+      const e = new Error("missing") as NodeJS.ErrnoException;
+      e.code = "ENOENT";
+      throw e;
+    });
+    expect(out).toBe(path.join(path.resolve(path.sep, "REAL"), "b", "c", "f.txt"));
+    expect(seen.length).toBeGreaterThan(1);
+  });
+
+  it("still refuses an absolute destination whose ancestors link OUT of the root", async () => {
+    // The tightening half. Canonicalizing the ancestor chain means a directory
+    // inside the root that links outside is caught HERE, by containment, rather
+    // than relying on a later check to notice.
+    const base = await realpath(await mkdtemp(path.join(tmpdir(), "farm-541e-")));
+    try {
+      const root = path.join(base, "wt");
+      const outside = path.join(base, "outside");
+      await mkdir(root, { recursive: true });
+      await mkdir(outside, { recursive: true });
+      await writeFile(path.join(outside, "sentinel.txt"), "outside-must-survive", "utf8");
+
+      const escape = path.join(root, "escape");
+      try {
+        await symlink(outside, escape, "junction");
+      } catch {
+        return;
+      }
+      // Lexically this looks contained — that is what makes it worth refusing.
+      expect(path.relative(root, escape).startsWith("..")).toBe(false);
+
+      await expectRefused(writeWorktreeFile(root, path.join(escape, "sentinel.txt"), "overwrite"));
+      expect(await readFile(path.join(outside, "sentinel.txt"), "utf8")).toBe("outside-must-survive");
+    } finally {
+      await rm(base, { recursive: true, force: true });
+    }
+  });
+
   it("refuses a NUL byte in the destination segment", async () => {
     // Reaches the non-ENOENT arm of the destination lstat: the error is neither
     // "missing" nor an `unsafe()` call, so the catch has to funnel it. An