diff --git a/packages/server/src/server/hub/execution-session.websocket.test.ts b/packages/server/src/server/hub/execution-session.websocket.test.ts index 589b82794..6a14cbae2 100644 --- a/packages/server/src/server/hub/execution-session.websocket.test.ts +++ b/packages/server/src/server/hub/execution-session.websocket.test.ts @@ -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)); diff --git a/packages/server/src/server/paseo-worktree-service.test.ts b/packages/server/src/server/paseo-worktree-service.test.ts index 5fec6bad0..614820dd4 100644 --- a/packages/server/src/server/paseo-worktree-service.test.ts +++ b/packages/server/src/server/paseo-worktree-service.test.ts @@ -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); }, ); diff --git a/packages/server/src/server/workspace-create-worktree-source.e2e.test.ts b/packages/server/src/server/workspace-create-worktree-source.e2e.test.ts index 2fbdc930c..0ae15e526 100644 --- a/packages/server/src/server/workspace-create-worktree-source.e2e.test.ts +++ b/packages/server/src/server/workspace-create-worktree-source.e2e.test.ts @@ -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); diff --git a/packages/server/src/server/worktree-core.posix.test.ts b/packages/server/src/server/worktree-core.posix.test.ts index 40bd983c7..94f654de1 100644 --- a/packages/server/src/server/worktree-core.posix.test.ts +++ b/packages/server/src/server/worktree-core.posix.test.ts @@ -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 () => { diff --git a/packages/server/src/server/worktree-core.ts b/packages/server/src/server/worktree-core.ts index 98ffa777d..7189d75c2 100644 --- a/packages/server/src/server/worktree-core.ts +++ b/packages/server/src/server/worktree-core.ts @@ -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, diff --git a/packages/server/src/utils/worktree.posix.test.ts b/packages/server/src/utils/worktree.posix.test.ts index 74e2fcf47..2d45b0fe5 100644 --- a/packages/server/src/utils/worktree.posix.test.ts +++ b/packages/server/src/utils/worktree.posix.test.ts @@ -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 () => { diff --git a/packages/server/src/utils/worktree.ts b/packages/server/src/utils/worktree.ts index 983823adc..1403ef7ce 100644 --- a/packages/server/src/utils/worktree.ts +++ b/packages/server/src/utils/worktree.ts @@ -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 { - 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 {