diff --git a/bun.lock b/bun.lock index 25a0802941c..f65cac4c998 100644 --- a/bun.lock +++ b/bun.lock @@ -111,7 +111,7 @@ }, "apps/desktop": { "name": "@superset/desktop", - "version": "1.8.1", + "version": "1.8.3", "dependencies": { "@ai-sdk/anthropic": "^3.0.43", "@ai-sdk/openai": "3.0.36", diff --git a/packages/host-service/src/trpc/router/workspace-creation/procedures/adopt.ts b/packages/host-service/src/trpc/router/workspace-creation/procedures/adopt.ts index 81420234462..224ae10cd65 100644 --- a/packages/host-service/src/trpc/router/workspace-creation/procedures/adopt.ts +++ b/packages/host-service/src/trpc/router/workspace-creation/procedures/adopt.ts @@ -1,126 +1,26 @@ import { getHostId, getHostName } from "@superset/shared/host-info"; import { TRPCError } from "@trpc/server"; -import { and, eq, ne, or } from "drizzle-orm"; -import { workspaces } from "../../../../db/schema"; -import type { HostServiceContext } from "../../../../types"; import { protectedProcedure } from "../../../index"; -import { gitConfigWrite } from "../../git/utils/config-write"; import { ensureMainWorkspace } from "../../project/utils/ensure-main-workspace"; import { adoptInputSchema } from "../schemas"; +import { adoptExistingWorktree } from "../shared/adopt-existing-worktree"; import { getWorktreeBranchAtPath, listWorktreeBranches, } from "../shared/branch-search"; import { requireLocalProject } from "../shared/local-project"; -import type { GitClient, TerminalDescriptor } from "../shared/types"; - -type HostWorkspace = NonNullable< - Awaited< - ReturnType - > ->; - -function adoptResult(workspace: HostWorkspace) { - return { - workspace, - terminals: [] as TerminalDescriptor[], - warnings: [] as string[], - }; -} - -function deleteLocalWorkspace( - ctx: HostServiceContext, - workspaceId: string, -): void { - ctx.db.delete(workspaces).where(eq(workspaces.id, workspaceId)).run(); -} - -function persistLocalWorkspace( - ctx: HostServiceContext, - args: { - id: string; - projectId: string; - worktreePath: string; - branch: string; - }, -): void { - ctx.db - .insert(workspaces) - .values({ - id: args.id, - projectId: args.projectId, - worktreePath: args.worktreePath, - branch: args.branch, - }) - .onConflictDoUpdate({ - target: workspaces.id, - set: { - projectId: args.projectId, - worktreePath: args.worktreePath, - branch: args.branch, - }, - }) - .run(); -} - -function deleteLocalWorkspaceConflicts( - ctx: HostServiceContext, - args: { - projectId: string; - worktreePath: string; - branch: string; - keepWorkspaceId: string; - }, -): void { - ctx.db - .delete(workspaces) - .where( - and( - eq(workspaces.projectId, args.projectId), - or( - eq(workspaces.branch, args.branch), - eq(workspaces.worktreePath, args.worktreePath), - ), - ne(workspaces.id, args.keepWorkspaceId), - ), - ) - .run(); -} - -async function getHostWorkspace( - ctx: HostServiceContext, - workspaceId: string, -): Promise { - return ctx.api.v2Workspace.getFromHost.query({ - organizationId: ctx.organizationId, - id: workspaceId, - }); -} - -async function recordBaseBranch( - git: GitClient, - branch: string, - baseBranch: string | undefined, -): Promise { - if (!baseBranch) return; - await gitConfigWrite(git as Parameters[0], [ - "config", - `branch.${branch}.base`, - baseBranch, - ]).catch((err) => { - console.warn( - `[workspaceCreation.adopt] failed to record base branch ${baseBranch}:`, - err, - ); - }); -} - +import type { TerminalDescriptor } from "../shared/types"; + +/** + * Adopt a worktree that already exists on disk into a Superset workspace + * row. Currently the only caller is the v1→v2 migration, which passes + * an explicit `worktreePath`. Branch-name-only callers (the v2 picker, + * MCP, agent spawn) go through `workspaces.create`, which handles + * adoption inline via the same shared helper. + */ export const adopt = protectedProcedure .input(adoptInputSchema) .mutation(async ({ ctx, input }) => { - const machineId = getHostId(); - const hostName = getHostName(); - const localProject = requireLocalProject(ctx, input.projectId); await ensureMainWorkspace(ctx, input.projectId, localProject.repoPath); @@ -136,6 +36,9 @@ export const adopt = protectedProcedure let worktreePath: string; if (input.worktreePath) { + // Path-driven adoption (v1→v2 migration): trust the path the + // caller computed, and read back the actual checked-out branch + // so a stale DB branch name doesn't make us miss the worktree. const actualBranch = await getWorktreeBranchAtPath( git, input.worktreePath, @@ -160,168 +63,28 @@ export const adopt = protectedProcedure worktreePath = found; } - if (input.existingWorkspaceId) { - const existingCloud = await getHostWorkspace( - ctx, - input.existingWorkspaceId, - ); - if (existingCloud) { - await recordBaseBranch(git, branch, input.baseBranch); - deleteLocalWorkspaceConflicts(ctx, { - projectId: input.projectId, - worktreePath, - branch, - keepWorkspaceId: existingCloud.id, - }); - try { - persistLocalWorkspace(ctx, { - id: existingCloud.id, - projectId: input.projectId, - worktreePath, - branch, - }); - } catch (err) { - console.error( - "[workspaceCreation.adopt] local workspace relink failed", - err, - ); - throw new TRPCError({ - code: "INTERNAL_SERVER_ERROR", - message: `Failed to persist existing workspace locally: ${err instanceof Error ? err.message : String(err)}`, - }); - } - return adoptResult(existingCloud); - } - } - - const existingLocal = ctx.db.query.workspaces - .findFirst({ - where: and( - eq(workspaces.projectId, input.projectId), - eq(workspaces.branch, branch), - ), - }) - .sync(); - if (existingLocal && existingLocal.worktreePath === worktreePath) { - const existingCloud = await getHostWorkspace(ctx, existingLocal.id); - if (existingCloud) { - await recordBaseBranch(git, branch, input.baseBranch); - return adoptResult(existingCloud); - } - deleteLocalWorkspace(ctx, existingLocal.id); - } - - const existingLocalByPath = ctx.db.query.workspaces - .findFirst({ - where: and( - eq(workspaces.projectId, input.projectId), - eq(workspaces.worktreePath, worktreePath), - ), - }) - .sync(); - if (existingLocalByPath) { - const existingCloud = await getHostWorkspace(ctx, existingLocalByPath.id); - if (existingCloud) { - deleteLocalWorkspaceConflicts(ctx, { - projectId: input.projectId, - worktreePath, - branch, - keepWorkspaceId: existingLocalByPath.id, - }); - const updatedCloud = - await ctx.api.v2Workspace.updateNameFromHost.mutate({ - id: existingCloud.id, - branch, - }); - ctx.db - .update(workspaces) - .set({ branch }) - .where(eq(workspaces.id, existingLocalByPath.id)) - .run(); - await recordBaseBranch(git, branch, input.baseBranch); - return adoptResult(updatedCloud); - } - deleteLocalWorkspace(ctx, existingLocalByPath.id); - } - - let host: { machineId: string }; - try { - host = await ctx.api.host.ensure.mutate({ - organizationId: ctx.organizationId, - machineId, - name: hostName, - }); - } catch (err) { - if (err instanceof TRPCError) throw err; - console.error("[workspaceCreation.adopt] host.ensure failed", err); - throw new TRPCError({ - code: "INTERNAL_SERVER_ERROR", - message: `Failed to register host: ${err instanceof Error ? err.message : String(err)}`, - }); - } - - let cloudRow: Awaited>; - try { - cloudRow = await ctx.api.v2Workspace.create.mutate({ - organizationId: ctx.organizationId, - projectId: input.projectId, - name: input.workspaceName, - branch, - hostId: host.machineId, - }); - } catch (err) { - if (err instanceof TRPCError) throw err; - console.error("[workspaceCreation.adopt] v2Workspace.create failed", err); - throw new TRPCError({ - code: "INTERNAL_SERVER_ERROR", - message: `Failed to create workspace: ${err instanceof Error ? err.message : String(err)}`, - }); - } - - if (!cloudRow) { - throw new TRPCError({ - code: "INTERNAL_SERVER_ERROR", - message: "Cloud workspace create returned no row", - }); - } - - await recordBaseBranch(git, branch, input.baseBranch); + const hostPromise = ctx.api.host.ensure.mutate({ + organizationId: ctx.organizationId, + machineId: getHostId(), + name: getHostName(), + }); + hostPromise.catch(() => {}); - // Replace any stale local row for this (project, branch) — its - // id likely points at a deleted cloud row. The new cloudRow.id - // is the authoritative mapping. - deleteLocalWorkspaceConflicts(ctx, { + const { workspace } = await adoptExistingWorktree({ + ctx, + git, projectId: input.projectId, - worktreePath, branch, - keepWorkspaceId: cloudRow.id, + worktreePath, + workspaceName: input.workspaceName, + baseBranch: input.baseBranch, + existingWorkspaceId: input.existingWorkspaceId, + hostPromise, }); - try { - persistLocalWorkspace(ctx, { - id: cloudRow.id, - projectId: input.projectId, - worktreePath, - branch, - }); - } catch (err) { - console.error( - "[workspaceCreation.adopt] local workspaces insert failed", - err, - ); - await ctx.api.v2Workspace.delete - .mutate({ id: cloudRow.id }) - .catch((cleanupErr) => { - console.warn( - "[workspaceCreation.adopt] failed to rollback cloud workspace", - { workspaceId: cloudRow.id, err: cleanupErr }, - ); - }); - throw new TRPCError({ - code: "INTERNAL_SERVER_ERROR", - message: `Failed to persist workspace locally: ${err instanceof Error ? err.message : String(err)}`, - }); - } - - return adoptResult(cloudRow); + return { + workspace, + terminals: [] as TerminalDescriptor[], + warnings: [] as string[], + }; }); diff --git a/packages/host-service/src/trpc/router/workspace-creation/shared/adopt-existing-worktree.ts b/packages/host-service/src/trpc/router/workspace-creation/shared/adopt-existing-worktree.ts new file mode 100644 index 00000000000..8b898047423 --- /dev/null +++ b/packages/host-service/src/trpc/router/workspace-creation/shared/adopt-existing-worktree.ts @@ -0,0 +1,308 @@ +import { TRPCError } from "@trpc/server"; +import { and, eq, ne, or } from "drizzle-orm"; +import { workspaces } from "../../../../db/schema"; +import type { HostServiceContext } from "../../../../types"; +import { gitConfigWrite } from "../../git/utils/config-write"; +import type { GitClient } from "./types"; + +export type AdoptedWorkspace = NonNullable< + Awaited< + ReturnType + > +>; + +export interface AdoptExistingWorktreeArgs { + ctx: HostServiceContext; + git: GitClient; + projectId: string; + branch: string; + worktreePath: string; + workspaceName: string; + baseBranch?: string; + /** v1→v2 migration relinks to a known cloud id; other callers leave undefined. */ + existingWorkspaceId?: string; + /** Optimistic-UI idempotency key for v2Workspace.create; ignored on relink. */ + idempotencyId?: string; + /** Task link for v2Workspace.create; ignored on relink. */ + taskId?: string; + hostPromise: Promise<{ machineId: string }>; +} + +export interface AdoptExistingWorktreeResult { + workspace: AdoptedWorkspace; + /** True when an existing cloud row was reused; false when a new row was + * created in this call. Used by `workspaces.create` to decide whether + * to dispatch setup terminal + sugar agents. */ + alreadyExists: boolean; +} + +/** + * Register a workspace for a worktree that already exists on disk. Owns + * all the stale-row hygiene (relink by branch, relink-on-rename by path, + * delete-stale-on-cloud-mismatch) so callers don't reinvent it. + * + * Cross-project safety is the caller's responsibility — only pass a + * `worktreePath` that came from `git worktree list` on this project's + * `git`. A path registered against a different repo's git dir won't be + * detected here and will silently land as a row in the wrong project. + */ +export async function adoptExistingWorktree( + args: AdoptExistingWorktreeArgs, +): Promise { + const { + ctx, + git, + projectId, + branch, + worktreePath, + workspaceName, + baseBranch, + existingWorkspaceId, + idempotencyId, + taskId, + hostPromise, + } = args; + + if (existingWorkspaceId) { + const existingCloud = await getHostWorkspace(ctx, existingWorkspaceId); + if (existingCloud) { + await recordBaseBranch(git, branch, baseBranch); + deleteLocalWorkspaceConflicts(ctx, { + projectId, + worktreePath, + branch, + keepWorkspaceId: existingCloud.id, + }); + persistLocalWorkspace(ctx, { + id: existingCloud.id, + projectId, + worktreePath, + branch, + }); + return { + workspace: existingCloud, + alreadyExists: true, + }; + } + } + + // Already linked at this exact (branch, path) — reuse if cloud still has + // the row, otherwise drop the orphaned local row and continue to create. + const existingByBranch = ctx.db.query.workspaces + .findFirst({ + where: and( + eq(workspaces.projectId, projectId), + eq(workspaces.branch, branch), + ), + }) + .sync(); + if (existingByBranch && existingByBranch.worktreePath === worktreePath) { + const existingCloud = await getHostWorkspace(ctx, existingByBranch.id); + if (existingCloud) { + await recordBaseBranch(git, branch, baseBranch); + return { + workspace: existingCloud, + alreadyExists: true, + }; + } + deleteLocalWorkspace(ctx, existingByBranch.id); + } + + // Same path, different branch — branch was renamed in place. Re-point + // the cloud row at the new branch instead of leaving a phantom row. + const existingByPath = ctx.db.query.workspaces + .findFirst({ + where: and( + eq(workspaces.projectId, projectId), + eq(workspaces.worktreePath, worktreePath), + ), + }) + .sync(); + if (existingByPath) { + const existingCloud = await getHostWorkspace(ctx, existingByPath.id); + if (existingCloud) { + deleteLocalWorkspaceConflicts(ctx, { + projectId, + worktreePath, + branch, + keepWorkspaceId: existingByPath.id, + }); + const updatedCloud = await ctx.api.v2Workspace.updateNameFromHost.mutate({ + id: existingCloud.id, + branch, + }); + ctx.db + .update(workspaces) + .set({ branch }) + .where(eq(workspaces.id, existingByPath.id)) + .run(); + await recordBaseBranch(git, branch, baseBranch); + return { + workspace: updatedCloud, + alreadyExists: true, + }; + } + deleteLocalWorkspace(ctx, existingByPath.id); + } + + let host: { machineId: string }; + try { + host = await hostPromise; + } catch (err) { + if (err instanceof TRPCError) throw err; + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: `Failed to register host: ${err instanceof Error ? err.message : String(err)}`, + }); + } + + const cloudRow = await ctx.api.v2Workspace.create + .mutate({ + organizationId: ctx.organizationId, + projectId, + name: workspaceName, + branch, + hostId: host.machineId, + id: idempotencyId, + taskId, + }) + .catch((err) => { + if (err instanceof TRPCError) throw err; + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: `Failed to create workspace: ${err instanceof Error ? err.message : String(err)}`, + }); + }); + + if (!cloudRow) { + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: "Cloud workspace create returned no row", + }); + } + + await recordBaseBranch(git, branch, baseBranch); + + // Stale local row for this (project, branch or path) typically points + // at a deleted cloud row — the new cloudRow.id is authoritative. + deleteLocalWorkspaceConflicts(ctx, { + projectId, + worktreePath, + branch, + keepWorkspaceId: cloudRow.id, + }); + + try { + persistLocalWorkspace(ctx, { + id: cloudRow.id, + projectId, + worktreePath, + branch, + }); + } catch (err) { + await ctx.api.v2Workspace.delete + .mutate({ id: cloudRow.id }) + .catch((cleanupErr) => { + console.warn( + "[adoptExistingWorktree] failed to rollback cloud workspace", + { workspaceId: cloudRow.id, err: cleanupErr }, + ); + }); + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: `Failed to persist workspace locally: ${err instanceof Error ? err.message : String(err)}`, + }); + } + + return { + workspace: cloudRow, + alreadyExists: false, + }; +} + +async function getHostWorkspace( + ctx: HostServiceContext, + workspaceId: string, +): Promise { + return ctx.api.v2Workspace.getFromHost.query({ + organizationId: ctx.organizationId, + id: workspaceId, + }); +} + +function deleteLocalWorkspace( + ctx: HostServiceContext, + workspaceId: string, +): void { + ctx.db.delete(workspaces).where(eq(workspaces.id, workspaceId)).run(); +} + +function persistLocalWorkspace( + ctx: HostServiceContext, + args: { + id: string; + projectId: string; + worktreePath: string; + branch: string; + }, +): void { + ctx.db + .insert(workspaces) + .values({ + id: args.id, + projectId: args.projectId, + worktreePath: args.worktreePath, + branch: args.branch, + }) + .onConflictDoUpdate({ + target: workspaces.id, + set: { + projectId: args.projectId, + worktreePath: args.worktreePath, + branch: args.branch, + }, + }) + .run(); +} + +function deleteLocalWorkspaceConflicts( + ctx: HostServiceContext, + args: { + projectId: string; + worktreePath: string; + branch: string; + keepWorkspaceId: string; + }, +): void { + ctx.db + .delete(workspaces) + .where( + and( + eq(workspaces.projectId, args.projectId), + or( + eq(workspaces.branch, args.branch), + eq(workspaces.worktreePath, args.worktreePath), + ), + ne(workspaces.id, args.keepWorkspaceId), + ), + ) + .run(); +} + +async function recordBaseBranch( + git: GitClient, + branch: string, + baseBranch: string | undefined, +): Promise { + if (!baseBranch) return; + await gitConfigWrite(git as Parameters[0], [ + "config", + `branch.${branch}.base`, + baseBranch, + ]).catch((err) => { + console.warn( + `[adoptExistingWorktree] failed to record base branch ${baseBranch}:`, + err, + ); + }); +} diff --git a/packages/host-service/src/trpc/router/workspace-creation/shared/branch-search.ts b/packages/host-service/src/trpc/router/workspace-creation/shared/branch-search.ts index 4cb8b24a077..97c87b3c50f 100644 --- a/packages/host-service/src/trpc/router/workspace-creation/shared/branch-search.ts +++ b/packages/host-service/src/trpc/router/workspace-creation/shared/branch-search.ts @@ -41,8 +41,11 @@ export function markRefetchRemote(projectId: string): void { // No gating on managed root or workspaces table — foreign worktrees // (user ran `git worktree add` themselves) surface too, so the v2 -// picker shows everything git would. `checkedOutBranches` is used to -// disable Checkout when a branch is already in use elsewhere. +// picker shows everything git would. `checkedOutBranches` disables +// Checkout when a branch is already in use elsewhere. Prunable entries +// (dir deleted without `git worktree remove`) are filtered: not valid +// adoption targets, and `workspaces.create` runs `git worktree prune` +// before re-adding so the branch is freed. export async function listWorktreeBranches(git: GitClient): Promise<{ worktreeMap: Map; checkedOutBranches: Set; @@ -51,6 +54,7 @@ export async function listWorktreeBranches(git: GitClient): Promise<{ const checkedOutBranches = new Set(); for (const wt of await listGitWorktrees(git)) { if (!wt.branch) continue; + if (wt.prunable) continue; checkedOutBranches.add(wt.branch); worktreeMap.set(wt.branch, wt.path); } diff --git a/packages/host-service/src/trpc/router/workspaces/workspaces.ts b/packages/host-service/src/trpc/router/workspaces/workspaces.ts index f9998a16c5b..39196da75fd 100644 --- a/packages/host-service/src/trpc/router/workspaces/workspaces.ts +++ b/packages/host-service/src/trpc/router/workspaces/workspaces.ts @@ -16,7 +16,8 @@ import type { HostServiceContext } from "../../../types"; import { protectedProcedure, router } from "../../index"; import { type AgentRunResult, runAgentInWorkspace } from "../agents"; import { ensureMainWorkspace } from "../project/utils/ensure-main-workspace"; -import { getWorktreeBranchAtPath } from "../workspace-creation/shared/branch-search"; +import { adoptExistingWorktree } from "../workspace-creation/shared/adopt-existing-worktree"; +import { listWorktreeBranches } from "../workspace-creation/shared/branch-search"; import { enablePushAutoSetupRemote } from "../workspace-creation/shared/git-config"; import { requireLocalProject } from "../workspace-creation/shared/local-project"; import { startSetupTerminalIfPresent } from "../workspace-creation/shared/setup-terminal"; @@ -513,6 +514,15 @@ export const workspacesRouter = router({ const git = await ctx.git(localProject.repoPath); + // Free branches still claimed by registrations whose dirs are + // gone — without this, `git worktree add` later fails with + // "branch is already used by worktree at ". + await git + .raw(["worktree", "prune"]) + .catch((err) => + console.warn("[workspaces.create] worktree prune failed:", err), + ); + let resolvedBranch: string; let worktreePath: string; let alreadyExists = false; @@ -545,122 +555,161 @@ export const workspacesRouter = router({ localOid !== null && localOid.toLowerCase() === prMetadata.headRefOid.trim().toLowerCase(); + // If the local branch already lives in a worktree somewhere, + // `git worktree add` will refuse. Look it up first so the + // OID-mismatch error can point at the actual worktree, and + // the matching-OID case can adopt instead of duplicating. + const existingWorktreePath = ( + await listWorktreeBranches(git) + ).worktreeMap.get(resolvedBranch); + if (localOid !== null && !adoptLocalBranch) { + const cleanupHint = existingWorktreePath + ? `Inspect with \`git log ${resolvedBranch}\`, then \`git worktree remove ${existingWorktreePath}\` and \`git branch -D ${resolvedBranch}\` if safe.` + : `Inspect with \`git log ${resolvedBranch}\`, then \`git branch -D ${resolvedBranch}\` if safe.`; throw new TRPCError({ code: "CONFLICT", - message: `Local branch "${resolvedBranch}" exists outside Superset and points at a different commit than PR #${input.pr} (local ${localOid.slice(0, 7)}, PR ${prMetadata.headRefOid.slice(0, 7)}). Inspect with \`git log ${resolvedBranch}\`, then \`git branch -D ${resolvedBranch}\` if safe.`, + message: `Local branch "${resolvedBranch}" exists outside Superset and points at a different commit than PR #${input.pr} (local ${localOid.slice(0, 7)}, PR ${prMetadata.headRefOid.slice(0, 7)}). ${cleanupHint}`, }); } - worktreePath = safeResolveWorktreePath( - localProject.id, - resolvedBranch, - ); - mkdirSync(dirname(worktreePath), { recursive: true }); - - const rollbackWorktree = async () => { - try { - await git.raw(["worktree", "remove", "--force", worktreePath]); - } catch (err) { - console.warn( - "[workspaces.create] failed to rollback PR worktree", - { worktreePath, err }, - ); - } - }; - - if (adoptLocalBranch) { - try { - await git.raw(["worktree", "add", worktreePath, resolvedBranch]); - } catch (err) { - throw new TRPCError({ - code: "CONFLICT", - message: - err instanceof Error - ? err.message - : "Failed to add worktree for existing branch", - }); - } + if (adoptLocalBranch && existingWorktreePath) { + worktreePath = existingWorktreePath; + const result = await adoptExistingWorktree({ + ctx, + git, + projectId: input.projectId, + branch: resolvedBranch, + worktreePath, + workspaceName: input.name ?? prMetadata.title ?? resolvedBranch, + baseBranch: prMetadata.baseRefName, + idempotencyId: input.id, + taskId: input.taskId, + hostPromise, + }); + workspaceRow = result.workspace; + alreadyExists = result.alreadyExists; + await enablePushAutoSetupRemote( + git, + worktreePath, + "[workspaces.create]", + ); } else { - try { - await git.raw(["worktree", "add", "--detach", worktreePath]); - } catch (err) { - throw new TRPCError({ - code: "CONFLICT", - message: - err instanceof Error - ? err.message - : "Failed to add detached worktree", - }); - } + worktreePath = safeResolveWorktreePath( + localProject.id, + resolvedBranch, + ); + mkdirSync(dirname(worktreePath), { recursive: true }); - try { - await execGh( - [ - "pr", - "checkout", - String(input.pr), - "--branch", - resolvedBranch, - "--force", - ], - { cwd: worktreePath, timeout: 120_000 }, - ); - } catch (err) { - let recoveryError: unknown = null; - let recovered = false; + const rollbackWorktree = async () => { try { - const recovery = await recoverPrCheckoutAfterGhFailure({ - git, + await git.raw(["worktree", "remove", "--force", worktreePath]); + } catch (err) { + console.warn( + "[workspaces.create] failed to rollback PR worktree", + { worktreePath, err }, + ); + } + }; + + if (adoptLocalBranch) { + try { + await git.raw([ + "worktree", + "add", worktreePath, - branch: resolvedBranch, - prNumber: input.pr, - remoteName: localProject.remoteName ?? "origin", - expectedHeadOid: prMetadata.headRefOid, - error: err, + resolvedBranch, + ]); + } catch (err) { + throw new TRPCError({ + code: "CONFLICT", + message: + err instanceof Error + ? err.message + : "Failed to add worktree for existing branch", }); - recovered = recovery.recovered; - } catch (e) { - recoveryError = e; } - if (!recovered) { - await rollbackWorktree(); - const recoveryMessage = recoveryError - ? ` Recovery via refs/pull/${input.pr}/head also failed: ${getErrorMessage(recoveryError)}` - : ""; + } else { + try { + await git.raw(["worktree", "add", "--detach", worktreePath]); + } catch (err) { throw new TRPCError({ - code: "INTERNAL_SERVER_ERROR", - message: `gh pr checkout failed: ${err instanceof Error ? err.message : String(err)}${recoveryMessage}`, + code: "CONFLICT", + message: + err instanceof Error + ? err.message + : "Failed to add detached worktree", }); } - } - } - - await enablePushAutoSetupRemote( - git, - worktreePath, - "[workspaces.create]", - ); - workspaceRow = await registerCloudAndLocal({ - ctx, - id: input.id, - projectId: input.projectId, - name: input.name ?? prMetadata.title ?? resolvedBranch, - branch: resolvedBranch, - worktreePath, - taskId: input.taskId, - rollbackWorktree, - hostPromise, - }); + try { + await execGh( + [ + "pr", + "checkout", + String(input.pr), + "--branch", + resolvedBranch, + "--force", + ], + { cwd: worktreePath, timeout: 120_000 }, + ); + } catch (err) { + let recoveryError: unknown = null; + let recovered = false; + try { + const recovery = await recoverPrCheckoutAfterGhFailure({ + git, + worktreePath, + branch: resolvedBranch, + prNumber: input.pr, + remoteName: localProject.remoteName ?? "origin", + expectedHeadOid: prMetadata.headRefOid, + error: err, + }); + recovered = recovery.recovered; + } catch (e) { + recoveryError = e; + } + if (!recovered) { + await rollbackWorktree(); + const recoveryMessage = recoveryError + ? ` Recovery via refs/pull/${input.pr}/head also failed: ${getErrorMessage(recoveryError)}` + : ""; + throw new TRPCError({ + code: "INTERNAL_SERVER_ERROR", + message: `gh pr checkout failed: ${err instanceof Error ? err.message : String(err)}${recoveryMessage}`, + }); + } + } + } - if (prMetadata.baseRefName) { - await recordBaseBranchConfig({ + await enablePushAutoSetupRemote( git, worktreePath, + "[workspaces.create]", + ); + + workspaceRow = await registerCloudAndLocal({ + ctx, + id: input.id, + projectId: input.projectId, + name: input.name ?? prMetadata.title ?? resolvedBranch, branch: resolvedBranch, - baseBranch: prMetadata.baseRefName, + worktreePath, + taskId: input.taskId, + rollbackWorktree, + hostPromise, }); + + if (prMetadata.baseRefName) { + await recordBaseBranchConfig({ + git, + worktreePath, + branch: resolvedBranch, + baseBranch: prMetadata.baseRefName, + }); + } } } } else { @@ -708,37 +757,53 @@ export const workspacesRouter = router({ workspaceRow = existing; alreadyExists = true; } else { - worktreePath = safeResolveWorktreePath( - localProject.id, - resolvedBranch, - ); - - // Adopt: a worktree already exists at the standard path with the - // matching branch checked out (e.g. left behind by a prior session - // or registered outside Superset). Skip `git worktree add` and - // proceed straight to register. Only meaningful when the user - // supplied the branch — auto-gen names are deduped and can't - // collide with anything pre-existing. - const adopted = - !!typedBranch && - (await getWorktreeBranchAtPath(git, worktreePath)) === - resolvedBranch; - - mkdirSync(dirname(worktreePath), { recursive: true }); - - const rollbackWorktree = async () => { - if (adopted) return; - try { - await git.raw(["worktree", "remove", "--force", worktreePath]); - } catch (err) { - console.warn("[workspaces.create] failed to rollback worktree", { - worktreePath, - err, - }); - } - }; + // Adopt at any path git already knows for this branch — git + // refuses a second checkout of the same branch, so falling + // through to `git worktree add` would block re-entry. Auto-gen + // names are deduped against the branch list, so only typed + // branches can collide here. + const existingWorktreePath = typedBranch + ? (await listWorktreeBranches(git)).worktreeMap.get(resolvedBranch) + : undefined; + + if (existingWorktreePath) { + worktreePath = existingWorktreePath; + const baseShortName = + !plan.usedExistingBranch && plan.startPoint.kind !== "head" + ? plan.startPoint.shortName + : undefined; + const result = await adoptExistingWorktree({ + ctx, + git, + projectId: input.projectId, + branch: resolvedBranch, + worktreePath, + workspaceName: input.name ?? aiTitle ?? resolvedBranch, + baseBranch: baseShortName, + idempotencyId: input.id, + taskId: input.taskId, + hostPromise, + }); + workspaceRow = result.workspace; + alreadyExists = result.alreadyExists; + } else { + worktreePath = safeResolveWorktreePath( + localProject.id, + resolvedBranch, + ); + mkdirSync(dirname(worktreePath), { recursive: true }); + + const rollbackWorktree = async () => { + try { + await git.raw(["worktree", "remove", "--force", worktreePath]); + } catch (err) { + console.warn( + "[workspaces.create] failed to rollback worktree", + { worktreePath, err }, + ); + } + }; - if (!adopted) { try { await addBranchWorktree({ git, plan, worktreePath }); } catch (err) { @@ -748,37 +813,37 @@ export const workspacesRouter = router({ err instanceof Error ? err.message : "Failed to add worktree", }); } - } - await enablePushAutoSetupRemote( - git, - worktreePath, - "[workspaces.create]", - ); - - if (!plan.usedExistingBranch && plan.startPoint.kind !== "head") { - const baseShortName = plan.startPoint.shortName; - await git - .raw(["config", `branch.${resolvedBranch}.base`, baseShortName]) - .catch((err) => { - console.warn( - `[workspaces.create] failed to record base branch ${baseShortName}:`, - err, - ); - }); - } + await enablePushAutoSetupRemote( + git, + worktreePath, + "[workspaces.create]", + ); + + if (!plan.usedExistingBranch && plan.startPoint.kind !== "head") { + const baseShortName = plan.startPoint.shortName; + await git + .raw(["config", `branch.${resolvedBranch}.base`, baseShortName]) + .catch((err) => { + console.warn( + `[workspaces.create] failed to record base branch ${baseShortName}:`, + err, + ); + }); + } - workspaceRow = await registerCloudAndLocal({ - ctx, - id: input.id, - projectId: input.projectId, - name: input.name ?? aiTitle ?? resolvedBranch, - branch: resolvedBranch, - worktreePath, - taskId: input.taskId, - rollbackWorktree, - hostPromise, - }); + workspaceRow = await registerCloudAndLocal({ + ctx, + id: input.id, + projectId: input.projectId, + name: input.name ?? aiTitle ?? resolvedBranch, + branch: resolvedBranch, + worktreePath, + taskId: input.taskId, + rollbackWorktree, + hostPromise, + }); + } } } diff --git a/packages/host-service/test/integration/workspace-create-delete.integration.test.ts b/packages/host-service/test/integration/workspace-create-delete.integration.test.ts index 90a8fa961ed..b88353180c4 100644 --- a/packages/host-service/test/integration/workspace-create-delete.integration.test.ts +++ b/packages/host-service/test/integration/workspace-create-delete.integration.test.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, test } from "bun:test"; import { randomUUID } from "node:crypto"; -import { existsSync } from "node:fs"; +import { existsSync, rmSync } from "node:fs"; +import { join } from "node:path"; import { TRPCClientError } from "@trpc/client"; import { eq } from "drizzle-orm"; import { workspaces } from "../../src/db/schema"; @@ -49,6 +50,86 @@ describe("workspace.create + workspace.delete integration", () => { expect(existsSync(persisted?.worktreePath ?? "")).toBe(true); }); + test("create() adopts an existing worktree at a non-canonical path instead of failing on `git worktree add`", async () => { + // Regress: when the user typed a branch that already has a worktree + // somewhere outside `~/.superset/worktrees//`, + // `workspaces.create` used to call `git worktree add` and crash with + // `fatal: '' is already used by worktree at ...`. Adopt the + // existing path instead. + const scenario = await createProjectScenario({ + hostOptions: { apiOverrides: cloudFlows.workspaceCreateOk() }, + }); + dispose = scenario.dispose; + + const branch = "new-workspace-9"; + const nonCanonicalPath = join( + scenario.repo.repoPath, + ".worktrees", + "glorious-ground", + ); + await scenario.repo.git.raw([ + "worktree", + "add", + "-b", + branch, + nonCanonicalPath, + ]); + + const result = await scenario.host.trpc.workspaces.create.mutate({ + projectId: scenario.projectId, + name: "adopted", + branch, + }); + + expect(result?.workspace?.branch).toBe(branch); + const persisted = scenario.host.db + .select() + .from(workspaces) + .where(eq(workspaces.id, result?.workspace?.id ?? "")) + .get(); + expect(persisted?.worktreePath).toBe(nonCanonicalPath); + expect(existsSync(nonCanonicalPath)).toBe(true); + }); + + test("create() prunes a stale worktree (rm-ed dir) before adding a new one", async () => { + // Regress: when a worktree's directory was deleted without + // `git worktree remove`, git still lists it (prunable) and claims + // the branch. `workspaces.create` used to either adopt the missing + // path or fail on `git worktree add`. It should now prune first + // and check the branch out at the canonical path. + const scenario = await createProjectScenario({ + hostOptions: { apiOverrides: cloudFlows.workspaceCreateOk() }, + }); + dispose = scenario.dispose; + + const branch = "stale-feature"; + const stalePath = join( + scenario.repo.repoPath, + ".worktrees", + "stale-feature", + ); + await scenario.repo.git.raw(["worktree", "add", "-b", branch, stalePath]); + // Simulate the user `rm -rf`-ing the worktree without git's blessing. + rmSync(stalePath, { recursive: true, force: true }); + + const result = await scenario.host.trpc.workspaces.create.mutate({ + projectId: scenario.projectId, + name: "fresh", + branch, + }); + + expect(result?.workspace?.branch).toBe(branch); + const persisted = scenario.host.db + .select() + .from(workspaces) + .where(eq(workspaces.id, result?.workspace?.id ?? "")) + .get(); + // Should land at the canonical path, not the missing one. + expect(persisted?.worktreePath).not.toBe(stalePath); + expect(persisted?.worktreePath).toMatch(/stale-feature$/); + expect(existsSync(persisted?.worktreePath ?? "")).toBe(true); + }); + test("create() rolls back the worktree if cloud v2Workspace.create fails", async () => { const scenario = await createProjectScenario({ hostOptions: {