Always create a fresh worktree (#3224)

* fix(server): always create requested worktrees A slug is a preferred name, not workspace identity. Let the worktree creator suffix occupied paths and branches so duplicate and detached-slug requests still create fresh worktrees. * fix(server): suffix occupied checkout branches Fresh creation also applies when checkout targets are already in use. Create a suffixed branch from the requested branch instead of rejecting the request. * test(server): align occupied branch coverage * test(server): remove stale worktree reuse expectations
2026-08-11 17:54:59 +02:00
parent 308e9af845
commit b2ff359b60
7 changed files with 175 additions and 95 deletions
@@ -246,7 +246,7 @@ test("a sibling workspace keeps an archived execution's worktree directory alive
  expect(await hub.ownedAgentArchivedAt(created.payload.agentId!)).toEqual(expect.any(String));
 });

-test("archiving an execution in a reused worktree leaves the existing workspace intact", async () => {
+test("archiving a second same-slug execution leaves the first worktree intact", async () => {
  const hub = await launchRelationship();
  const worktree = {
    mode: "branch-off" as const,
@@ -267,6 +267,7 @@ test("archiving an execution in a reused worktree leaves the existing workspace
    prompt: "sleep 30",
  });
  const reused = await hub.ownedCreateResult("reused-worktree-create");
+  const secondWorktreeCwd = hub.latestCreatedCwd()!;
  const reusedWorkspaceId = reused.payload.agent?.workspaceId;
  await hub.ownedRunningUpdate(reused.payload.agentId!);

@@ -278,8 +279,10 @@ test("archiving an execution in a reused worktree leaves the existing workspace
  expect(response).toMatchObject({ success: true, error: null });
  expect(reusedWorkspaceId).toEqual(expect.any(String));
  expect(reusedWorkspaceId).not.toBe(originalWorkspaceId);
-  expect(hub.pathsReferToSameLocation(reused.payload.agent!.cwd, worktreeCwd)).toBe(true);
+  expect(hub.pathsReferToSameLocation(reused.payload.agent!.cwd, worktreeCwd)).toBe(false);
+  expect(hub.pathsReferToSameLocation(reused.payload.agent!.cwd, secondWorktreeCwd)).toBe(true);
  expect(await hub.worktreeState(worktreeCwd)).toEqual({ exists: true, listed: true });
+  expect(await hub.worktreeState(secondWorktreeCwd)).toEqual({ exists: false, listed: false });
  expect(await hub.agentRemainsAvailable(original.payload.agentId!)).toBe(true);
  expect(await hub.ownedAgentArchivedAt(reused.payload.agentId!)).toEqual(expect.any(String));
  expect(await hub.ownedWorkspaceArchivedAt(reused.payload.agentId!)).toEqual(expect.any(String));
@@ -458,9 +458,8 @@ test("an explicit project FK remains unchanged when its worktree comes from anot
  });
 });

-// POSIX-only: Windows git worktree paths need separate canonicalization coverage.
 test.skipIf(isPlatform("win32"))(
-  "reuses an existing worktree and still upserts the workspace",
+  "creates a suffixed worktree when the preferred slug is occupied",
  async () => {
    const { repoDir, tempDir } = createGitRepo();
    cleanupPaths.push(tempDir);
@@ -492,11 +491,10 @@ test.skipIf(isPlatform("win32"))(
      deps,
    );

-    expect(second.created).toBe(false);
-    expect(second.worktree.worktreePath).toBe(first.worktree.worktreePath);
+    expect(second.created).toBe(true);
+    expect(second.worktree.worktreePath).not.toBe(first.worktree.worktreePath);
+    expect(path.basename(second.worktree.worktreePath)).toBe("reuse-me-1");
    expect(events).toContain(`workspace:${second.workspace.workspaceId}`);
-    // Creation never dedupes by directory: the same worktree path yields a
-    // distinct workspace record on the second call.
    expect(second.workspace.workspaceId).not.toBe(first.workspace.workspaceId);
  },
 );
@@ -932,7 +930,7 @@ test.skipIf(isPlatform("win32"))(
    expect(worktreeList).toContain(created.worktreePath);

    // Recreating without pruning fails with the stale registration pinning the
-    // branch — this is the case restore must heal.
+    // path — this is the case restore must heal.
    await expect(
      createWorktree({
        cwd: repoDir,
@@ -941,7 +939,7 @@ test.skipIf(isPlatform("win32"))(
        runSetup: false,
        paseoHome,
      }),
-    ).rejects.toMatchObject({ name: "BranchAlreadyCheckedOutError" });
+    ).rejects.toThrow("missing but already registered worktree");

    // The restore-side prune frees the stale registration; recreate then succeeds.
    execFileSync("git", ["worktree", "prune"], { cwd: repoDir, stdio: "pipe" });
@@ -986,7 +984,7 @@ test.skipIf(isPlatform("win32"))(
 );

 test.skipIf(isPlatform("win32"))(
-  "rejects with BranchAlreadyCheckedOutError when the kept branch is checked out elsewhere",
+  "creates a suffixed branch when the requested branch is checked out elsewhere",
  async () => {
    const { repoDir, tempDir } = createGitRepo();
    cleanupPaths.push(tempDir);
@@ -1002,15 +1000,16 @@ test.skipIf(isPlatform("win32"))(
    });
    expect(existsSync(first.worktreePath)).toBe(true);

-    await expect(
-      createWorktree({
-        cwd: repoDir,
-        worktreeSlug: "busy-branch-again",
-        source: { kind: "checkout-branch", branchName: "busy-branch" },
-        runSetup: false,
-        paseoHome,
-      }),
-    ).rejects.toMatchObject({ name: "BranchAlreadyCheckedOutError" });
+    const second = await createWorktree({
+      cwd: repoDir,
+      worktreeSlug: "busy-branch-again",
+      source: { kind: "checkout-branch", branchName: "busy-branch" },
+      runSetup: false,
+      paseoHome,
+    });
+
+    expect(second.branchName).toBe("busy-branch-1");
+    expect(existsSync(second.worktreePath)).toBe(true);
  },
 );

@@ -95,3 +95,132 @@ test("workspace.create keeps a branch-off name separate from its worktree slug",
    rmSync(tempRoot, { recursive: true, force: true });
  }
 }, 180000);
+
+test("workspace.create always creates a new worktree when the slug is already occupied", async () => {
+  const daemon = await createTestPaseoDaemon();
+  const { repoDir, tempRoot } = createGitRepoWithBranch();
+  const client = new DaemonClient({
+    url: `ws://127.0.0.1:${daemon.port}/ws`,
+    appVersion: "0.1.82",
+  });
+
+  try {
+    await client.connect();
+
+    const first = await client.createWorkspace({
+      source: {
+        kind: "worktree",
+        cwd: repoDir,
+        action: "branch-off",
+        worktreeSlug: "same-slug",
+        baseBranch: "main",
+      },
+    });
+    const second = await client.createWorkspace({
+      source: {
+        kind: "worktree",
+        cwd: repoDir,
+        action: "branch-off",
+        worktreeSlug: "same-slug",
+        baseBranch: "main",
+      },
+    });
+
+    expect(first.error).toBeNull();
+    expect(second.error).toBeNull();
+    expect(path.basename(first.workspace?.workspaceDirectory ?? "")).toBe("same-slug");
+    expect(path.basename(second.workspace?.workspaceDirectory ?? "")).toBe("same-slug-1");
+    expect(second.workspace?.id).not.toBe(first.workspace?.id);
+  } finally {
+    await client.close().catch(() => undefined);
+    await daemon.close();
+    rmSync(tempRoot, { recursive: true, force: true });
+  }
+}, 180000);
+
+test("workspace.create suffixes past an occupied detached worktree", async () => {
+  const daemon = await createTestPaseoDaemon();
+  const { repoDir, tempRoot } = createGitRepoWithBranch();
+  const client = new DaemonClient({
+    url: `ws://127.0.0.1:${daemon.port}/ws`,
+    appVersion: "0.1.82",
+  });
+
+  try {
+    await client.connect();
+
+    const first = await client.createWorkspace({
+      source: {
+        kind: "worktree",
+        cwd: repoDir,
+        action: "branch-off",
+        worktreeSlug: "detached-slug",
+        baseBranch: "main",
+      },
+    });
+    expect(first.error).toBeNull();
+    expect(first.workspace?.workspaceDirectory).toBeTypeOf("string");
+    const firstPath = first.workspace?.workspaceDirectory as string;
+    execFileSync("git", ["checkout", "--detach"], { cwd: firstPath, stdio: "pipe" });
+
+    const second = await client.createWorkspace({
+      source: {
+        kind: "worktree",
+        cwd: repoDir,
+        action: "branch-off",
+        worktreeSlug: "detached-slug",
+        baseBranch: "main",
+      },
+    });
+
+    expect(second.error).toBeNull();
+    expect(path.basename(second.workspace?.workspaceDirectory ?? "")).toBe("detached-slug-1");
+    expect(second.workspace?.gitRuntime?.currentBranch).toBe("detached-slug-1");
+  } finally {
+    await client.close().catch(() => undefined);
+    await daemon.close();
+    rmSync(tempRoot, { recursive: true, force: true });
+  }
+}, 180000);
+
+test("workspace.create suffixes an occupied checkout branch", async () => {
+  const daemon = await createTestPaseoDaemon();
+  const { repoDir, tempRoot } = createGitRepoWithBranch();
+  const client = new DaemonClient({
+    url: `ws://127.0.0.1:${daemon.port}/ws`,
+    appVersion: "0.1.82",
+  });
+
+  try {
+    await client.connect();
+
+    const first = await client.createWorkspace({
+      source: {
+        kind: "worktree",
+        cwd: repoDir,
+        action: "checkout",
+        refName: "feature/existing-branch",
+        worktreeSlug: "existing-branch",
+      },
+    });
+    const second = await client.createWorkspace({
+      source: {
+        kind: "worktree",
+        cwd: repoDir,
+        action: "checkout",
+        refName: "feature/existing-branch",
+        worktreeSlug: "existing-branch",
+      },
+    });
+
+    expect(first.error).toBeNull();
+    expect(first.workspace?.gitRuntime?.currentBranch).toBe("feature/existing-branch");
+    expect(second.error).toBeNull();
+    expect(path.basename(second.workspace?.workspaceDirectory ?? "")).toBe("existing-branch-1");
+    expect(second.workspace?.gitRuntime?.currentBranch).toBe("feature/existing-branch-1");
+  } finally {
+    await client.close().catch(() => undefined);
+    await daemon.close();
+    rmSync(tempRoot, { recursive: true, force: true });
+  }
+}, 180000);
@@ -1438,8 +1438,7 @@ describe.skipIf(isPlatform("win32"))("worktree-core POSIX-only", () => {
      expect(result.worktree.branchName).toBe("agent-worktree");
    });

-    // POSIX-only: Windows git worktree paths need separate canonicalization coverage.
-    test("reuses an existing branch-off worktree for the same slug", async () => {
+    test("creates a suffixed branch-off worktree for the same slug", async () => {
      const { tempDir, repoDir, paseoHome } = createGitRepo();
      cleanupPaths.push(tempDir);
      const deps = createCoreDeps();
@@ -1454,12 +1453,12 @@ describe.skipIf(isPlatform("win32"))("worktree-core POSIX-only", () => {
      );

      expect(first.created).toBe(true);
-      expect(second.created).toBe(false);
-      expect(second.worktree).toEqual(first.worktree);
+      expect(second.created).toBe(true);
+      expect(path.basename(second.worktree.worktreePath)).toBe("reused-worktree-1");
+      expect(second.worktree.branchName).toBe("reused-worktree-1");
    });

-    // POSIX-only: Windows git worktree paths need separate canonicalization coverage.
-    test("reuses an existing GitHub PR worktree for the resolved slug", async () => {
+    test("creates a suffixed GitHub PR worktree for the resolved slug", async () => {
      const { tempDir, repoDir, paseoHome } = createGitHubPrRemoteRepo();
      cleanupPaths.push(tempDir);
      const deps = createCoreDeps();
@@ -1475,8 +1474,9 @@ describe.skipIf(isPlatform("win32"))("worktree-core POSIX-only", () => {
      const second = await createCoreWorktree(input, deps);

      expect(first.created).toBe(true);
-      expect(second.created).toBe(false);
-      expect(second.worktree).toEqual(first.worktree);
+      expect(second.created).toBe(true);
+      expect(path.basename(second.worktree.worktreePath)).toBe("feature-review-pr-1");
+      expect(second.worktree.branchName).toBe("feature/review-pr-1");
    });

    test("uses an injectable ForgeService dependency for missing PR head refs", async () => {
@@ -3,7 +3,6 @@ import { createNameId } from "mnemonic-id";
 import type { ForgeService } from "../services/forge-service.js";
 import {
  createWorktree,
-  resolveExistingWorktreeForSlug,
  slugify,
  validateBranchSlug,
  type WorktreeConfig,
@@ -118,16 +117,6 @@ async function createWorktreeCoreWithPriority(
    }
  }

-  const existingWorktree = await resolveExistingWorktreeForSlug({
-    slug: normalizedSlug,
-    repoRoot,
-    paseoHome: input.paseoHome,
-    worktreesRoot: input.worktreesRoot,
-  });
-  if (existingWorktree) {
-    return { worktree: existingWorktree, intent, repoRoot, created: false };
-  }
-
  return {
    worktree: await createWorktree({
      cwd: repoRoot,
@@ -2,7 +2,6 @@
 /* eslint-disable max-nested-callbacks */
 import { describe, it, expect, beforeEach, afterEach } from "vitest";
 import {
-  BranchAlreadyCheckedOutError,
  createWorktree as createWorktreePrimitive,
  deriveWorktreeProjectHash,
  deletePaseoWorktree,
@@ -299,22 +298,17 @@ describe.skipIf(isPlatform("win32"))("worktree POSIX-only", () => {
      expect(currentBranch).toBe("release/1.1.15");
    });

-    it("throws a typed error when checking out a branch already checked out in the main repo", async () => {
-      let caughtError: unknown;
-      try {
-        await createLegacyWorktreeForTest({
-          cwd: repoDir,
-          worktreeSlug: "dev-worktree",
-          source: { kind: "checkout-branch", branchName: "main" },
-          runSetup: true,
-          paseoHome,
-        });
-      } catch (error) {
-        caughtError = error;
-      }
+    it("creates a suffixed branch when checking out a branch already checked out in the main repo", async () => {
+      const result = await createLegacyWorktreeForTest({
+        cwd: repoDir,
+        worktreeSlug: "dev-worktree",
+        source: { kind: "checkout-branch", branchName: "main" },
+        runSetup: true,
+        paseoHome,
+      });

-      expect(caughtError).toBeInstanceOf(BranchAlreadyCheckedOutError);
-      expect((caughtError as BranchAlreadyCheckedOutError).branchName).toBe("main");
+      expect(result.branchName).toBe("main-1");
+      expect(existsSync(result.worktreePath)).toBe(true);
    });

    it("fetches a GitHub PR branch, checks it out, writes metadata, and runs setup", async () => {
@@ -207,13 +207,6 @@ export interface CreateWorktreeOptions {
  worktreesRoot?: string;
 }

-interface ResolveExistingWorktreeForSlugOptions {
-  slug: string;
-  repoRoot: string;
-  paseoHome?: string;
-  worktreesRoot?: string;
-}
-
 export class BranchAlreadyCheckedOutError extends Error {
  readonly branchName: string;

@@ -1063,38 +1056,6 @@ export async function listPaseoWorktrees({
    );
 }

-export async function resolveExistingWorktreeForSlug({
-  slug,
-  repoRoot,
-  paseoHome,
-  worktreesRoot,
-}: ResolveExistingWorktreeForSlugOptions): Promise<WorktreeConfig | null> {
-  const worktrees = await listPaseoWorktrees({
-    cwd: repoRoot,
-    paseoHome,
-    worktreesRoot,
-  });
-  const slugSuffix = `${sep}${slug}`;
-  const existingWorktree = worktrees.find((worktree) => worktree.path.endsWith(slugSuffix));
-  if (!existingWorktree) {
-    return null;
-  }
-
-  const { stdout } = await runGitCommand(["branch", "--show-current"], {
-    cwd: existingWorktree.path,
-    envOverlay: READ_ONLY_GIT_ENV,
-  });
-  const branchName = stdout.trim();
-  if (!branchName) {
-    throw new Error(`Unable to resolve branch for existing worktree: ${existingWorktree.path}`);
-  }
-
-  return {
-    branchName,
-    worktreePath: existingWorktree.path,
-  };
-}
-
 export interface DeletePaseoWorktreeOptions {
  cwd: string | null;
  worktreePath?: string;
@@ -1375,7 +1336,12 @@ async function resolveWorktreeSourcePlan({
        }
      }
      if (await isBranchCheckedOut(cwd, source.branchName)) {
-        throw new BranchAlreadyCheckedOutError(source.branchName);
+        const branchName = await resolveUniqueLocalBranchName(cwd, source.branchName);
+        return {
+          branchName,
+          metadataBaseRefName: source.branchName,
+          addArguments: ["-b", branchName, "--no-track", source.branchName],
+        };
      }

      return {