diff --git a/src/git.ts b/src/git.ts index 617c33afc..9eb5b69c4 100644 --- a/src/git.ts +++ b/src/git.ts @@ -185,3 +185,32 @@ export async function getMainWorktreeFromWorktree(worktreePath: string): Promise return null; } } + +export async function removeWorktree( + projectPath: string, + workspacePath: string, + options: { force: boolean } = { force: false } +): Promise { + try { + // Remove the worktree (from the main repository context) + using proc = execAsync( + `git -C "${projectPath}" worktree remove "${workspacePath}" ${options.force ? "--force" : ""}` + ); + await proc.result; + return { success: true }; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + return { success: false, error: message }; + } +} + +export async function pruneWorktrees(projectPath: string): Promise { + try { + using proc = execAsync(`git -C "${projectPath}" worktree prune`); + await proc.result; + return { success: true }; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + return { success: false, error: message }; + } +} diff --git a/src/runtime/LocalRuntime.ts b/src/runtime/LocalRuntime.ts index 8c54701bc..ccbc057fd 100644 --- a/src/runtime/LocalRuntime.ts +++ b/src/runtime/LocalRuntime.ts @@ -12,6 +12,8 @@ import type { WorkspaceCreationResult, WorkspaceInitParams, WorkspaceInitResult, + WorkspaceForkParams, + WorkspaceForkResult, InitLogger, } from "./Runtime"; import { RuntimeError as RuntimeErrorClass } from "./Runtime"; @@ -488,6 +490,21 @@ export class LocalRuntime implements Runtime { // Compute workspace path using the canonical method const deletedPath = this.getWorkspacePath(projectPath, workspaceName); + // Check if directory exists - if not, operation is idempotent + try { + await fsPromises.access(deletedPath); + } catch { + // Directory doesn't exist - operation is idempotent + // Prune stale git records (best effort) + try { + using pruneProc = execAsync(`git -C "${projectPath}" worktree prune`); + await pruneProc.result; + } catch { + // Ignore prune errors - directory is already deleted, which is the goal + } + return { success: true, deletedPath }; + } + try { // Use git worktree remove to delete the worktree // This updates git's internal worktree metadata correctly @@ -502,6 +519,25 @@ export class LocalRuntime implements Runtime { } catch (error) { const message = getErrorMessage(error); + // Check if the error is due to missing/stale worktree + const normalizedError = message.toLowerCase(); + const looksLikeMissingWorktree = + normalizedError.includes("not a working tree") || + normalizedError.includes("does not exist") || + normalizedError.includes("no such file"); + + if (looksLikeMissingWorktree) { + // Worktree records are stale - prune them + try { + using pruneProc = execAsync(`git -C "${projectPath}" worktree prune`); + await pruneProc.result; + } catch { + // Ignore prune errors + } + // Treat as success - workspace is gone (idempotent) + return { success: true, deletedPath }; + } + // If force is enabled and git worktree remove failed, fall back to rm -rf // This handles edge cases like submodules where git refuses to delete if (force) { @@ -531,4 +567,52 @@ export class LocalRuntime implements Runtime { return { success: false, error: `Failed to remove worktree: ${message}` }; } } + + async forkWorkspace(params: WorkspaceForkParams): Promise { + const { projectPath, sourceWorkspaceName, newWorkspaceName, initLogger } = params; + + // Get source workspace path + const sourceWorkspacePath = this.getWorkspacePath(projectPath, sourceWorkspaceName); + + // Get current branch from source workspace + try { + using proc = execAsync(`git -C "${sourceWorkspacePath}" branch --show-current`); + const { stdout } = await proc.result; + const sourceBranch = stdout.trim(); + + if (!sourceBranch) { + return { + success: false, + error: "Failed to detect branch in source workspace", + }; + } + + // Use createWorkspace with sourceBranch as trunk to fork from source branch + const createResult = await this.createWorkspace({ + projectPath, + branchName: newWorkspaceName, + trunkBranch: sourceBranch, // Fork from source branch instead of main/master + directoryName: newWorkspaceName, + initLogger, + }); + + if (!createResult.success || !createResult.workspacePath) { + return { + success: false, + error: createResult.error ?? "Failed to create workspace", + }; + } + + return { + success: true, + workspacePath: createResult.workspacePath, + sourceBranch, + }; + } catch (error) { + return { + success: false, + error: getErrorMessage(error), + }; + } + } } diff --git a/src/runtime/Runtime.ts b/src/runtime/Runtime.ts index 18f11ade5..eab3c2651 100644 --- a/src/runtime/Runtime.ts +++ b/src/runtime/Runtime.ts @@ -155,6 +155,40 @@ export interface WorkspaceInitResult { error?: string; } +/** + * Runtime interface - minimal, low-level abstraction for tool execution environments. + * + * All methods return streaming primitives for memory efficiency. + * Use helpers in utils/runtime/ for convenience wrappers (e.g., readFileString, execBuffered). + +/** + * Parameters for forking an existing workspace + */ +export interface WorkspaceForkParams { + /** Project root path (local path) */ + projectPath: string; + /** Name of the source workspace to fork from */ + sourceWorkspaceName: string; + /** Name for the new workspace */ + newWorkspaceName: string; + /** Logger for streaming initialization events */ + initLogger: InitLogger; +} + +/** + * Result of forking a workspace + */ +export interface WorkspaceForkResult { + /** Whether the fork operation succeeded */ + success: boolean; + /** Path to the new workspace (if successful) */ + workspacePath?: string; + /** Branch that was forked from */ + sourceBranch?: string; + /** Error message (if failed) */ + error?: string; +} + /** * Runtime interface - minimal, low-level abstraction for tool execution environments. * @@ -306,6 +340,17 @@ export interface Runtime { workspaceName: string, force: boolean ): Promise<{ success: true; deletedPath: string } | { success: false; error: string }>; + + /** + * Fork an existing workspace to create a new one + * Creates a new workspace branching from the source workspace's current branch + * - LocalRuntime: Detects source branch via git, creates new worktree from that branch + * - SSHRuntime: Currently unimplemented (returns static error) + * + * @param params Fork parameters (source workspace name, new workspace name, etc.) + * @returns Result with new workspace path and source branch, or error + */ + forkWorkspace(params: WorkspaceForkParams): Promise; } /** diff --git a/src/runtime/SSHRuntime.ts b/src/runtime/SSHRuntime.ts index 04aa3726e..c8bc07982 100644 --- a/src/runtime/SSHRuntime.ts +++ b/src/runtime/SSHRuntime.ts @@ -11,6 +11,8 @@ import type { WorkspaceCreationResult, WorkspaceInitParams, WorkspaceInitResult, + WorkspaceForkParams, + WorkspaceForkResult, InitLogger, } from "./Runtime"; import { RuntimeError as RuntimeErrorClass } from "./Runtime"; @@ -971,6 +973,18 @@ export class SSHRuntime implements Runtime { return { success: false, error: `Failed to delete directory: ${getErrorMessage(error)}` }; } } + + forkWorkspace(_params: WorkspaceForkParams): Promise { + // SSH forking is not yet implemented due to unresolved complexities: + // - Users expect the new workspace's filesystem state to match the remote workspace, + // not the local project (which may be out of sync or on a different commit) + // - This requires: detecting the branch, copying remote state, handling uncommitted changes + // - For now, users should create a new workspace from the desired branch instead + return Promise.resolve({ + success: false, + error: "Forking SSH workspaces is not yet implemented. Create a new workspace instead.", + }); + } } /** diff --git a/src/services/gitService.test.ts b/src/services/gitService.test.ts deleted file mode 100644 index 4f7cb665e..000000000 --- a/src/services/gitService.test.ts +++ /dev/null @@ -1,267 +0,0 @@ -import { describe, test, expect, beforeEach, afterEach } from "@jest/globals"; -import * as fs from "fs/promises"; -import * as path from "path"; -import { execSync } from "child_process"; -import { removeWorktreeSafe, isWorktreeClean, hasSubmodules } from "./gitService"; -import { createWorktree, detectDefaultTrunkBranch } from "@/git"; -import type { Config } from "@/config"; - -// Helper to create a test git repo -async function createTestRepo(basePath: string): Promise { - const repoPath = path.join(basePath, "test-repo"); - await fs.mkdir(repoPath, { recursive: true }); - - execSync("git init", { cwd: repoPath }); - execSync("git config user.email 'test@test.com'", { cwd: repoPath }); - execSync("git config user.name 'Test User'", { cwd: repoPath }); - - // Create initial commit - await fs.writeFile(path.join(repoPath, "README.md"), "# Test Repo"); - execSync("git add .", { cwd: repoPath }); - execSync('git commit -m "Initial commit"', { cwd: repoPath }); - - return repoPath; -} - -// Mock config for createWorktree -const mockConfig = { - srcDir: path.join(__dirname, "..", "test-workspaces"), - getWorkspacePath: (projectPath: string, branchName: string) => { - return path.join(path.dirname(projectPath), "workspaces", branchName); - }, -} as unknown as Config; - -describe("removeWorktreeSafe", () => { - let tempDir: string; - let repoPath: string; - let defaultBranch: string; - - beforeEach(async () => { - tempDir = await fs.mkdtemp(path.join(__dirname, "..", "test-temp-")); - repoPath = await createTestRepo(tempDir); - defaultBranch = await detectDefaultTrunkBranch(repoPath); - }); - - afterEach(async () => { - try { - await fs.rm(tempDir, { recursive: true, force: true }); - } catch { - // Ignore cleanup errors - } - }); - - test("should instantly remove clean worktree via rename", async () => { - // Create a worktree - const result = await createWorktree(mockConfig, repoPath, "test-branch", { - trunkBranch: defaultBranch, - }); - expect(result.success).toBe(true); - const worktreePath = result.path!; - - // Verify worktree exists - const existsBefore = await fs - .access(worktreePath) - .then(() => true) - .catch(() => false); - expect(existsBefore).toBe(true); - - // Remove it (should be instant since it's clean) - const startTime = Date.now(); - const removeResult = await removeWorktreeSafe(repoPath, worktreePath); - const duration = Date.now() - startTime; - - expect(removeResult.success).toBe(true); - - // Should complete quickly (<200ms accounting for CI overhead) - expect(duration).toBeLessThan(200); - - // Worktree should be gone immediately - const existsAfter = await fs - .access(worktreePath) - .then(() => true) - .catch(() => false); - expect(existsAfter).toBe(false); - }); - - test("should block removal of dirty worktree", async () => { - // Create a worktree - const result = await createWorktree(mockConfig, repoPath, "dirty-branch", { - trunkBranch: defaultBranch, - }); - expect(result.success).toBe(true); - const worktreePath = result.path!; - - // Make it dirty by adding uncommitted changes - await fs.writeFile(path.join(worktreePath, "new-file.txt"), "uncommitted content"); - - // Verify it's dirty - const isClean = await isWorktreeClean(worktreePath); - expect(isClean).toBe(false); - - // Try to remove it - should fail due to uncommitted changes - const removeResult = await removeWorktreeSafe(repoPath, worktreePath); - - expect(removeResult.success).toBe(false); - expect(removeResult.error).toMatch(/modified|untracked|changes/i); - - // Worktree should still exist - const existsAfter = await fs - .access(worktreePath) - .then(() => true) - .catch(() => false); - expect(existsAfter).toBe(true); - }); - - test("should handle already-deleted worktree gracefully", async () => { - // Create a worktree - const result = await createWorktree(mockConfig, repoPath, "temp-branch", { - trunkBranch: defaultBranch, - }); - expect(result.success).toBe(true); - const worktreePath = result.path!; - - // Manually delete it (simulating external deletion) - await fs.rm(worktreePath, { recursive: true, force: true }); - - // Remove via removeWorktreeSafe - should succeed and prune git records - const removeResult = await removeWorktreeSafe(repoPath, worktreePath); - - expect(removeResult.success).toBe(true); - }); - - test("should remove clean worktree with staged changes using git", async () => { - // Create a worktree - const result = await createWorktree(mockConfig, repoPath, "staged-branch", { - trunkBranch: defaultBranch, - }); - expect(result.success).toBe(true); - const worktreePath = result.path!; - - // Add staged changes - await fs.writeFile(path.join(worktreePath, "staged.txt"), "staged content"); - execSync("git add .", { cwd: worktreePath }); - - // Verify it's dirty (staged changes count as dirty) - const isClean = await isWorktreeClean(worktreePath); - expect(isClean).toBe(false); - - // Try to remove it - should fail - const removeResult = await removeWorktreeSafe(repoPath, worktreePath); - - expect(removeResult.success).toBe(false); - }); - - test("should call onBackgroundDelete callback on errors", async () => { - // Create a worktree - const result = await createWorktree(mockConfig, repoPath, "callback-branch", { - trunkBranch: defaultBranch, - }); - expect(result.success).toBe(true); - const worktreePath = result.path!; - - const errors: Array<{ tempDir: string; error?: Error }> = []; - - // Remove it - const removeResult = await removeWorktreeSafe(repoPath, worktreePath, { - onBackgroundDelete: (tempDir, error) => { - errors.push({ tempDir, error }); - }, - }); - - expect(removeResult.success).toBe(true); - - // Wait a bit for background deletion to complete - await new Promise((resolve) => setTimeout(resolve, 100)); - - // Callback should be called for successful background deletion - // (or not called at all if deletion succeeds without error) - // This test mainly ensures the callback doesn't crash - }); -}); - -describe("isWorktreeClean", () => { - let tempDir: string; - let repoPath: string; - let defaultBranch: string; - - beforeEach(async () => { - tempDir = await fs.mkdtemp(path.join(__dirname, "..", "test-temp-")); - repoPath = await createTestRepo(tempDir); - defaultBranch = await detectDefaultTrunkBranch(repoPath); - }); - - afterEach(async () => { - try { - await fs.rm(tempDir, { recursive: true, force: true }); - } catch { - // Ignore cleanup errors - } - }); - - test("should return true for clean worktree", async () => { - const result = await createWorktree(mockConfig, repoPath, "clean-check", { - trunkBranch: defaultBranch, - }); - expect(result.success).toBe(true); - - const isClean = await isWorktreeClean(result.path!); - expect(isClean).toBe(true); - }); - - test("should return false for worktree with uncommitted changes", async () => { - const result = await createWorktree(mockConfig, repoPath, "dirty-check", { - trunkBranch: defaultBranch, - }); - expect(result.success).toBe(true); - const worktreePath = result.path!; - - // Add uncommitted file - await fs.writeFile(path.join(worktreePath, "uncommitted.txt"), "content"); - - const isClean = await isWorktreeClean(worktreePath); - expect(isClean).toBe(false); - }); - - test("should return false for non-existent path", async () => { - const isClean = await isWorktreeClean("/non/existent/path"); - expect(isClean).toBe(false); - }); -}); - -describe("hasSubmodules", () => { - let tempDir: string; - - beforeEach(async () => { - tempDir = await fs.mkdtemp(path.join(__dirname, "..", "test-temp-")); - }); - - afterEach(async () => { - try { - await fs.rm(tempDir, { recursive: true, force: true }); - } catch { - // Ignore cleanup errors - } - }); - - test("should return true when .gitmodules exists", async () => { - const testDir = path.join(tempDir, "with-submodule"); - await fs.mkdir(testDir, { recursive: true }); - await fs.writeFile(path.join(testDir, ".gitmodules"), '[submodule "test"]\n\tpath = test\n'); - - const result = await hasSubmodules(testDir); - expect(result).toBe(true); - }); - - test("should return false when .gitmodules does not exist", async () => { - const testDir = path.join(tempDir, "no-submodule"); - await fs.mkdir(testDir, { recursive: true }); - - const result = await hasSubmodules(testDir); - expect(result).toBe(false); - }); - - test("should return false for non-existent path", async () => { - const result = await hasSubmodules("/non/existent/path"); - expect(result).toBe(false); - }); -}); diff --git a/src/services/gitService.ts b/src/services/gitService.ts deleted file mode 100644 index 896fe2c4e..000000000 --- a/src/services/gitService.ts +++ /dev/null @@ -1,188 +0,0 @@ -import * as fsPromises from "fs/promises"; -import * as path from "path"; -import { execAsync } from "@/utils/disposableExec"; - -export interface WorktreeResult { - success: boolean; - path?: string; - error?: string; -} - -/** - * Check if a worktree has uncommitted changes or untracked files - * Returns true if the worktree is clean (safe to delete), false otherwise - */ -export async function isWorktreeClean(workspacePath: string): Promise { - try { - // Check for uncommitted changes (staged or unstaged) - using proc = execAsync(`git -C "${workspacePath}" status --porcelain`); - const { stdout: statusOutput } = await proc.result; - return statusOutput.trim() === ""; - } catch { - // If git command fails, assume not clean (safer default) - return false; - } -} - -/** - * Check if a worktree contains submodules - * Returns true if .gitmodules file exists, false otherwise - */ -export async function hasSubmodules(workspacePath: string): Promise { - try { - const gitmodulesPath = path.join(workspacePath, ".gitmodules"); - await fsPromises.access(gitmodulesPath); - return true; - } catch { - return false; - } -} - -export async function removeWorktree( - projectPath: string, - workspacePath: string, - options: { force: boolean } = { force: false } -): Promise { - try { - // Remove the worktree (from the main repository context) - using proc = execAsync( - `git -C "${projectPath}" worktree remove "${workspacePath}" ${options.force ? "--force" : ""}` - ); - await proc.result; - return { success: true }; - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - return { success: false, error: message }; - } -} - -export async function pruneWorktrees(projectPath: string): Promise { - try { - using proc = execAsync(`git -C "${projectPath}" worktree prune`); - await proc.result; - return { success: true }; - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - return { success: false, error: message }; - } -} - -/** - * Remove a worktree with optimized UX for clean worktrees. - * - * Strategy: - * - Clean worktrees (no uncommitted changes): Instant removal via rename+background delete - * - Dirty worktrees: Standard git removal (blocks UI but prevents data loss) - * - Missing worktrees: Prune from git records - * - * This provides instant feedback for the common case (clean worktrees) while - * preserving git's safety checks for uncommitted changes. - * - * IMPORTANT: This function NEVER uses --force. It will fail and return an error if: - * - Worktree has uncommitted changes - * - Worktree contains submodules (git refuses to remove without --force) - * The caller (frontend) will show a force delete modal, and the user can retry with force: true. - * - * @param projectPath - Path to the main git repository - * @param workspacePath - Path to the worktree to remove - * @param options.onBackgroundDelete - Optional callback for background deletion (for logging) - * @returns WorktreeResult indicating success or failure - */ -export async function removeWorktreeSafe( - projectPath: string, - workspacePath: string, - options?: { onBackgroundDelete?: (tempDir: string, error?: Error) => void } -): Promise { - // Check if worktree exists - const worktreeExists = await fsPromises - .access(workspacePath) - .then(() => true) - .catch(() => false); - - if (!worktreeExists) { - // Worktree already deleted - prune git records - const pruneResult = await pruneWorktrees(projectPath); - if (!pruneResult.success) { - // Log but don't fail - worktree is gone which is what we wanted - options?.onBackgroundDelete?.(workspacePath, new Error(pruneResult.error)); - } - return { success: true }; - } - - // Check if worktree is clean (no uncommitted changes) - const isClean = await isWorktreeClean(workspacePath); - - if (isClean) { - // Strategy: Instant removal for clean worktrees - // Rename to temp directory (instant), prune git records, delete in background - const tempDir = path.join( - path.dirname(workspacePath), - `.deleting-${path.basename(workspacePath)}-${Date.now()}` - ); - - try { - // Rename to temp location (instant operation) - await fsPromises.rename(workspacePath, tempDir); - - // Prune the worktree from git's records - await pruneWorktrees(projectPath); - - // Delete the temp directory in the background - void fsPromises.rm(tempDir, { recursive: true, force: true }).catch((err) => { - options?.onBackgroundDelete?.(tempDir, err as Error); - }); - - return { success: true }; - } catch { - // Rollback rename if it succeeded - const tempExists = await fsPromises - .access(tempDir) - .then(() => true) - .catch(() => false); - - if (tempExists) { - await fsPromises.rename(tempDir, workspacePath).catch(() => { - // If rollback fails, fall through to sync removal - }); - } - // Fall through to sync removal below - } - } - - // For dirty worktrees OR if instant removal failed: - // Use regular git worktree remove (respects git safety checks) - const stillExists = await fsPromises - .access(workspacePath) - .then(() => true) - .catch(() => false); - - if (stillExists) { - // Try normal git removal without force - // If worktree has uncommitted changes or submodules, this will fail - // and the error will be shown to the user who can then force delete - const gitResult = await removeWorktree(projectPath, workspacePath, { force: false }); - - if (!gitResult.success) { - const errorMessage = gitResult.error ?? "Unknown error"; - const normalizedError = errorMessage.toLowerCase(); - const looksLikeMissingWorktree = - normalizedError.includes("not a working tree") || - normalizedError.includes("does not exist") || - normalizedError.includes("no such file"); - - if (looksLikeMissingWorktree) { - // Path is missing from git's perspective - prune it - const pruneResult = await pruneWorktrees(projectPath); - if (!pruneResult.success) { - options?.onBackgroundDelete?.(workspacePath, new Error(pruneResult.error)); - } - return { success: true }; - } - - // Real git error (e.g., uncommitted changes) - propagate to caller - return gitResult; - } - } - - return { success: true }; -} diff --git a/src/services/ipcMain.ts b/src/services/ipcMain.ts index 5b5c37380..ec23980a1 100644 --- a/src/services/ipcMain.ts +++ b/src/services/ipcMain.ts @@ -5,13 +5,7 @@ import * as fs from "fs"; import * as fsPromises from "fs/promises"; import * as path from "path"; import type { Config, ProjectConfig } from "@/config"; -import { - createWorktree, - listLocalBranches, - detectDefaultTrunkBranch, - getCurrentBranch, -} from "@/git"; -import { removeWorktree, pruneWorktrees } from "@/services/gitService"; +import { listLocalBranches, detectDefaultTrunkBranch } from "@/git"; import { AIService } from "@/services/aiService"; import { HistoryService } from "@/services/historyService"; import { PartialService } from "@/services/partialService"; @@ -545,7 +539,7 @@ export class IpcMain { await this.partialService.commitToHistory(sourceWorkspaceId); } - // Get source workspace metadata and paths + // Get source workspace metadata const sourceMetadataResult = this.aiService.getWorkspaceMetadata(sourceWorkspaceId); if (!sourceMetadataResult.success) { return { @@ -555,47 +549,57 @@ export class IpcMain { } const sourceMetadata = sourceMetadataResult.data; const foundProjectPath = sourceMetadata.projectPath; + const projectName = sourceMetadata.projectName; - // Compute source workspace path from metadata (use name for directory lookup) using Runtime - const sourceRuntime = createRuntime( - sourceMetadata.runtimeConfig ?? { type: "local", srcBaseDir: this.config.srcDir } - ); - const sourceWorkspacePath = sourceRuntime.getWorkspacePath( - foundProjectPath, - sourceMetadata.name - ); - - // Get current branch from source workspace (fork from current branch, not trunk) - const sourceBranch = await getCurrentBranch(sourceWorkspacePath); - if (!sourceBranch) { - return { - success: false, - error: "Failed to detect current branch in source workspace", - }; - } + // Create runtime for source workspace + const sourceRuntimeConfig = sourceMetadata.runtimeConfig ?? { + type: "local", + srcBaseDir: this.config.srcDir, + }; + const runtime = createRuntime(sourceRuntimeConfig); // Generate stable workspace ID for the new workspace const newWorkspaceId = this.config.generateStableId(); - // Create new workspace branching from source workspace's branch - const result = await createWorktree(this.config, foundProjectPath, newName, { - trunkBranch: sourceBranch, - directoryName: newName, + // Create session BEFORE forking so init events can be forwarded + const session = this.getOrCreateSession(newWorkspaceId); + + // Start init tracking + this.initStateManager.startInit(newWorkspaceId, foundProjectPath); + + // Create InitLogger + const initLogger = { + logStep: (message: string) => { + this.initStateManager.appendOutput(newWorkspaceId, message, false); + }, + logStdout: (line: string) => { + this.initStateManager.appendOutput(newWorkspaceId, line, false); + }, + logStderr: (line: string) => { + this.initStateManager.appendOutput(newWorkspaceId, line, true); + }, + logComplete: (exitCode: number) => { + void this.initStateManager.endInit(newWorkspaceId, exitCode); + }, + }; + + // Delegate fork operation to runtime + const forkResult = await runtime.forkWorkspace({ + projectPath: foundProjectPath, + sourceWorkspaceName: sourceMetadata.name, + newWorkspaceName: newName, + initLogger, }); - if (!result.success || !result.path) { - return { success: false, error: result.error ?? "Failed to create worktree" }; + if (!forkResult.success) { + return { success: false, error: forkResult.error }; } - const newWorkspacePath = result.path; - const projectName = sourceMetadata.projectName; - - // Copy chat history from source to destination + // Copy session files (chat.jsonl, partial.json) - local backend operation const sourceSessionDir = this.config.getSessionDir(sourceWorkspaceId); const newSessionDir = this.config.getSessionDir(newWorkspaceId); try { - // Create new session directory await fsPromises.mkdir(newSessionDir, { recursive: true }); // Copy chat.jsonl if it exists @@ -604,7 +608,6 @@ export class IpcMain { try { await fsPromises.copyFile(sourceChatPath, newChatPath); } catch (error) { - // chat.jsonl doesn't exist yet - that's okay, continue if ( !(error && typeof error === "object" && "code" in error && error.code === "ENOENT") ) { @@ -612,13 +615,12 @@ export class IpcMain { } } - // Copy partial.json if it exists (preserves in-progress streaming response) + // Copy partial.json if it exists const sourcePartialPath = path.join(sourceSessionDir, "partial.json"); const newPartialPath = path.join(newSessionDir, "partial.json"); try { await fsPromises.copyFile(sourcePartialPath, newPartialPath); } catch (error) { - // partial.json doesn't exist - that's okay, continue if ( !(error && typeof error === "object" && "code" in error && error.code === "ENOENT") ) { @@ -627,33 +629,29 @@ export class IpcMain { } } catch (copyError) { // If copy fails, clean up everything we created - // 1. Remove the workspace - await removeWorktree(foundProjectPath, newWorkspacePath); - // 2. Remove the session directory (may contain partial copies) + await runtime.deleteWorkspace(foundProjectPath, newName, true); try { await fsPromises.rm(newSessionDir, { recursive: true, force: true }); } catch (cleanupError) { - // Log but don't fail - worktree cleanup is more important log.error(`Failed to clean up session dir ${newSessionDir}:`, cleanupError); } const message = copyError instanceof Error ? copyError.message : String(copyError); return { success: false, error: `Failed to copy chat history: ${message}` }; } - // Initialize workspace metadata with stable ID and name + // Initialize workspace metadata const metadata: WorkspaceMetadata = { id: newWorkspaceId, - name: newName, // Name is separate from ID + name: newName, projectName, projectPath: foundProjectPath, createdAt: new Date().toISOString(), }; - // Write metadata directly to config.json (single source of truth) + // Write metadata to config.json this.config.addWorkspace(foundProjectPath, metadata); - // Emit metadata event for new workspace - const session = this.getOrCreateSession(newWorkspaceId); + // Emit metadata event session.emitMetadata(metadata); return { @@ -1076,34 +1074,12 @@ export class IpcMain { metadata.runtimeConfig ?? { type: "local", srcBaseDir: this.config.srcDir } ); - // Delegate deletion to runtime - it handles all path computation and existence checks + // Delegate deletion to runtime - it handles all path computation, existence checks, and pruning const deleteResult = await runtime.deleteWorkspace(projectPath, metadata.name, options.force); if (!deleteResult.success) { - const errorMessage = deleteResult.error; - const normalizedError = errorMessage.toLowerCase(); - const looksLikeMissingWorktree = - normalizedError.includes("not a working tree") || - normalizedError.includes("does not exist") || - normalizedError.includes("no such file"); - - if (looksLikeMissingWorktree) { - // Worktree is already gone or stale - prune git records if this is a local worktree - if (metadata.runtimeConfig?.type !== "ssh") { - const pruneResult = await pruneWorktrees(projectPath); - if (!pruneResult.success) { - log.info( - `Failed to prune stale worktrees for ${projectPath} after deleteWorkspace error: ${ - pruneResult.error ?? "unknown error" - }` - ); - } - } - // Treat missing workspace as success (idempotent operation) - } else { - // Real error (e.g., dirty workspace without force) - return it - return { success: false, error: deleteResult.error }; - } + // Real error (e.g., dirty workspace without force) - return it + return { success: false, error: deleteResult.error }; } // Remove the workspace from AI service diff --git a/tests/runtime/runtime.test.ts b/tests/runtime/runtime.test.ts index 36e482bd7..a0663217a 100644 --- a/tests/runtime/runtime.test.ts +++ b/tests/runtime/runtime.test.ts @@ -1043,16 +1043,10 @@ describeIntegration("Runtime integration tests", () => { // Try to delete a workspace that doesn't exist const result = await runtime.deleteWorkspace(workspace.path, "non-existent", false); - // For SSH with rm -rf, deleting non-existent directory succeeds (rm -rf is idempotent) - // For local git worktree, it should fail - if (type === "local") { - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("Failed to remove worktree"); - } - } else { - // SSH: rm -rf non-existent is a no-op (succeeds) - expect(result.success).toBe(true); + // Both local and SSH deleteWorkspace are now idempotent - return success for non-existent workspaces + expect(result.success).toBe(true); + if (result.success) { + expect(result.deletedPath).toBeDefined(); } }); });