From e228acc37f527c0f3900d42fec1e9c1527735cbb Mon Sep 17 00:00:00 2001 From: Jeff <158072326+jeffrey701@users.noreply.github.com> Date: Thu, 9 Jul 2026 20:56:03 -0400 Subject: [PATCH] feat(miner-hands): git-worktree-per-attempt isolation primitive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add packages/gittensory-engine/src/miner/worktree-allocator.ts: the concurrency primitive for parallel coding-agent attempts. Each attempt runs in its own `git worktree` so concurrent attempts (same or different issues) never collide on a shared working directory. The module splits into a PURE planning layer and thin injected-exec wrappers, mirroring the SpawnFn injection convention (#4262/#4266) so all naming/collision/lifecycle logic is unit-testable without shelling out to git in CI: - planWorktree(): deterministic worktree path + branch, keyed on the attempt id (never a random suffix). Two concurrent attempts with distinct ids get distinct paths/branches; the same id always maps to the same location, so a crashed attempt's worktree stays identifiable and cleanable after the fact. Slugs are sanitized to a filesystem- and git-ref-safe token and length-capped. - addWorktree(): `git worktree add -b ` through the injected exec; returns the plan plus git's stderr on failure. - removeWorktree() + shouldRetainWorktree(): retention policy — a SUCCEEDED attempt's worktree is removed once it concludes; a FAILED attempt's is RETAINED for post-mortem (its deterministic name makes it findable). The exec is injected (a real child_process spawn in prod, a fake in tests), so the driver seam (#4262) can accept the planned path as its scoped working directory without retrofitting. Covered by test/unit/worktree-allocator.test.ts (pure naming/collision logic and both exec wrappers via a fake exec; 100% line + branch). Closes #4269 --- packages/gittensory-engine/src/index.ts | 13 +++ .../src/miner/worktree-allocator.ts | 109 ++++++++++++++++++ test/unit/worktree-allocator.test.ts | 106 +++++++++++++++++ 3 files changed, 228 insertions(+) create mode 100644 packages/gittensory-engine/src/miner/worktree-allocator.ts create mode 100644 test/unit/worktree-allocator.test.ts diff --git a/packages/gittensory-engine/src/index.ts b/packages/gittensory-engine/src/index.ts index d7485a4afe..5be4b81c5a 100644 --- a/packages/gittensory-engine/src/index.ts +++ b/packages/gittensory-engine/src/index.ts @@ -197,6 +197,19 @@ export { type CliSubprocessDriverOptions, type CliSubprocessSpawnFn, } from "./miner/cli-subprocess-driver.js"; +export { + addWorktree, + planWorktree, + removeWorktree, + shouldRetainWorktree, + WORKTREE_BRANCH_PREFIX, + WORKTREE_SUBDIR, + type WorktreeAddResult, + type WorktreeExecFn, + type WorktreeExecResult, + type WorktreePlan, + type WorktreeRemoveResult, +} from "./miner/worktree-allocator.js"; export { invokeCodingAgentDriver, type AttemptLogSink, diff --git a/packages/gittensory-engine/src/miner/worktree-allocator.ts b/packages/gittensory-engine/src/miner/worktree-allocator.ts new file mode 100644 index 0000000000..0d29fe1b14 --- /dev/null +++ b/packages/gittensory-engine/src/miner/worktree-allocator.ts @@ -0,0 +1,109 @@ +import { join } from "node:path"; + +// Git-worktree-per-attempt isolation primitive (#4269). Each coding-agent attempt runs in its OWN `git worktree`, +// so concurrent attempts (same or different issues) never collide on a shared working directory. This module is +// split into a PURE planning layer (deterministic path/branch naming) and thin injected-exec wrappers around the +// actual `git worktree add`/`git worktree remove` — the exec is injected (mirroring cli-subprocess-driver's SpawnFn +// convention, #4266), so all naming/collision/lifecycle logic is unit-testable without shelling out to git in CI. +// +// COLLISION: naming is deterministic and keyed on the attempt id (never a random suffix), so two concurrent +// attempts on the same repo can never be handed the same worktree path or branch, AND a crashed attempt's worktree +// stays identifiable and cleanable after the fact. +// +// RETENTION POLICY (see shouldRetainWorktree): a SUCCEEDED attempt's worktree is removed once it concludes; a +// FAILED attempt's worktree is RETAINED for post-mortem inspection (its deterministic name makes it findable). + +export type WorktreeExecResult = { code: number | null; stdout?: string; stderr?: string }; + +/** The injected git exec — a real `child_process` spawn in prod, a fake in tests. */ +export type WorktreeExecFn = ( + cmd: string, + args: readonly string[], + opts: { cwd: string }, +) => Promise; + +/** The deterministic worktree location + branch for one attempt. */ +export type WorktreePlan = { + attemptId: string; + worktreePath: string; + branchName: string; +}; + +/** Worktrees live under this dir inside the repo; the branch carries this prefix. */ +export const WORKTREE_SUBDIR = ".gittensory-worktrees"; +export const WORKTREE_BRANCH_PREFIX = "gittensory/attempt/"; +const MAX_SLUG_LENGTH = 64; + +/** Deterministically slugify an attempt id into a filesystem- and git-ref-safe token (same id → same slug). */ +function slugifyAttemptId(attemptId: string): string { + const slug = attemptId + .trim() + .toLowerCase() + .replace(/[^a-z0-9._-]+/g, "-") + .replace(/^[-.]+|[-.]+$/g, ""); + if (!slug) throw new Error("invalid_attempt_id"); + return slug.slice(0, MAX_SLUG_LENGTH); +} + +/** + * Compute the deterministic worktree path + branch name for an attempt — keyed on the attempt id, never a random + * suffix. Pure. Two concurrent attempts with distinct ids get distinct paths/branches; the same id always maps to + * the same location (so a crashed attempt's worktree is identifiable and cleanable). + */ +export function planWorktree(input: { repoPath: string; attemptId: string }): WorktreePlan { + const slug = slugifyAttemptId(input.attemptId); + return { + attemptId: input.attemptId, + worktreePath: join(input.repoPath, WORKTREE_SUBDIR, slug), + branchName: `${WORKTREE_BRANCH_PREFIX}${slug}`, + }; +} + +export type WorktreeAddResult = { ok: boolean; plan: WorktreePlan; error?: string }; + +/** + * Create the attempt's isolated worktree via `git worktree add -b `, run through the + * injected exec. Returns the plan (so the caller knows the path/branch) and, on failure, git's stderr. + */ +export async function addWorktree(input: { + exec: WorktreeExecFn; + repoPath: string; + baseBranch: string; + attemptId: string; +}): Promise { + const plan = planWorktree({ repoPath: input.repoPath, attemptId: input.attemptId }); + const result = await input.exec( + "git", + ["worktree", "add", "-b", plan.branchName, plan.worktreePath, input.baseBranch], + { cwd: input.repoPath }, + ); + if (result.code === 0) return { ok: true, plan }; + const detail = (result.stderr ?? "").trim() || `git_worktree_add_exit_${result.code}`; + return { ok: false, plan, error: detail }; +} + +export type WorktreeRemoveResult = { ok: boolean; removed: boolean; error?: string }; + +/** Retention policy: retain a FAILED attempt's worktree for post-mortem, remove a SUCCEEDED attempt's. */ +export function shouldRetainWorktree(attemptOk: boolean): boolean { + return !attemptOk; +} + +/** + * Tear down the attempt's worktree via `git worktree remove --force `, through the injected exec. When + * `retain` is set the worktree is KEPT (no exec, `removed: false`) for post-mortem — pass `shouldRetainWorktree(ok)`. + */ +export async function removeWorktree(input: { + exec: WorktreeExecFn; + repoPath: string; + worktreePath: string; + retain?: boolean; +}): Promise { + if (input.retain) return { ok: true, removed: false }; + const result = await input.exec("git", ["worktree", "remove", "--force", input.worktreePath], { + cwd: input.repoPath, + }); + if (result.code === 0) return { ok: true, removed: true }; + const detail = (result.stderr ?? "").trim() || `git_worktree_remove_exit_${result.code}`; + return { ok: false, removed: false, error: detail }; +} diff --git a/test/unit/worktree-allocator.test.ts b/test/unit/worktree-allocator.test.ts new file mode 100644 index 0000000000..15afee24d1 --- /dev/null +++ b/test/unit/worktree-allocator.test.ts @@ -0,0 +1,106 @@ +import { describe, expect, it } from "vitest"; +import { + addWorktree, + planWorktree, + removeWorktree, + shouldRetainWorktree, + WORKTREE_BRANCH_PREFIX, + WORKTREE_SUBDIR, + type WorktreeExecFn, + type WorktreeExecResult, +} from "../../packages/gittensory-engine/src/index"; + +/** A fake git exec that records calls and returns a scripted result. */ +function fakeExec(result: WorktreeExecResult) { + const calls: Array<{ cmd: string; args: readonly string[]; cwd: string }> = []; + const exec: WorktreeExecFn = async (cmd, args, opts) => { + calls.push({ cmd, args, cwd: opts.cwd }); + return result; + }; + return { exec, calls }; +} + +describe("planWorktree (#4269)", () => { + it("derives a deterministic, attempt-id-keyed path and branch", () => { + const a = planWorktree({ repoPath: "/repo", attemptId: "attempt-42" }); + expect(a.branchName).toBe(`${WORKTREE_BRANCH_PREFIX}attempt-42`); + expect(a.worktreePath.replaceAll("\\", "/")).toBe(`/repo/${WORKTREE_SUBDIR}/attempt-42`); + expect(a.attemptId).toBe("attempt-42"); + // same id → identical plan; different id → different plan (no collision) + expect(planWorktree({ repoPath: "/repo", attemptId: "attempt-42" })).toEqual(a); + expect(planWorktree({ repoPath: "/repo", attemptId: "attempt-43" }).worktreePath).not.toBe(a.worktreePath); + }); + + it("sanitizes unsafe characters, trims edge separators, and caps the slug length", () => { + const plan = planWorktree({ repoPath: "/repo", attemptId: " Feat/Fix #99!! " }); + expect(plan.branchName).toBe(`${WORKTREE_BRANCH_PREFIX}feat-fix-99`); + const long = planWorktree({ repoPath: "/repo", attemptId: "x".repeat(200) }); + expect(long.branchName).toBe(`${WORKTREE_BRANCH_PREFIX}${"x".repeat(64)}`); + }); + + it("rejects an attempt id that sanitizes to nothing", () => { + expect(() => planWorktree({ repoPath: "/repo", attemptId: " --- " })).toThrow(/invalid_attempt_id/); + }); +}); + +describe("addWorktree", () => { + it("runs `git worktree add -b ` and returns the plan on exit 0", async () => { + const { exec, calls } = fakeExec({ code: 0 }); + const result = await addWorktree({ exec, repoPath: "/repo", baseBranch: "main", attemptId: "attempt-1" }); + expect(result.ok).toBe(true); + expect(result.plan.branchName).toBe(`${WORKTREE_BRANCH_PREFIX}attempt-1`); + expect(calls[0]?.cmd).toBe("git"); + expect(calls[0]?.cwd).toBe("/repo"); + expect(calls[0]?.args.slice(0, 4)).toEqual(["worktree", "add", "-b", `${WORKTREE_BRANCH_PREFIX}attempt-1`]); + expect(calls[0]?.args.at(-1)).toBe("main"); + }); + + it("surfaces git's stderr on a non-zero exit, with a fallback when stderr is empty", async () => { + const withStderr = await addWorktree({ + exec: fakeExec({ code: 128, stderr: "fatal: 'wt' already exists" }).exec, + repoPath: "/repo", + baseBranch: "main", + attemptId: "attempt-1", + }); + expect(withStderr.ok).toBe(false); + expect(withStderr.error).toBe("fatal: 'wt' already exists"); + + const noStderr = await addWorktree({ + exec: fakeExec({ code: null }).exec, + repoPath: "/repo", + baseBranch: "main", + attemptId: "attempt-1", + }); + expect(noStderr.error).toBe("git_worktree_add_exit_null"); + }); +}); + +describe("removeWorktree + retention policy", () => { + it("retains a failed attempt's worktree and removes a succeeded one", () => { + expect(shouldRetainWorktree(false)).toBe(true); + expect(shouldRetainWorktree(true)).toBe(false); + }); + + it("skips the git call and reports removed:false when retain is set", async () => { + const { exec, calls } = fakeExec({ code: 0 }); + const result = await removeWorktree({ exec, repoPath: "/repo", worktreePath: "/repo/wt", retain: true }); + expect(result).toEqual({ ok: true, removed: false }); + expect(calls).toHaveLength(0); + }); + + it("runs `git worktree remove --force` and reports success or a redacted-free error", async () => { + const ok = fakeExec({ code: 0 }); + const removed = await removeWorktree({ exec: ok.exec, repoPath: "/repo", worktreePath: "/repo/wt" }); + expect(removed).toEqual({ ok: true, removed: true }); + expect(ok.calls[0]?.args).toEqual(["worktree", "remove", "--force", "/repo/wt"]); + + const failed = await removeWorktree({ + exec: fakeExec({ code: 1 }).exec, + repoPath: "/repo", + worktreePath: "/repo/wt", + }); + expect(failed.ok).toBe(false); + expect(failed.removed).toBe(false); + expect(failed.error).toBe("git_worktree_remove_exit_1"); + }); +});