From 1e304110f730138da6cbd189676899daeefe1d5a Mon Sep 17 00:00:00 2001 From: prateek Date: Thu, 19 Feb 2026 01:35:49 +0530 Subject: [PATCH] fix: destroy old runtime before restore, cleanup clone on failure (#109) Two bugbot follow-ups from PR #104: 1. Destroy old runtime handle before creating new one in restore(). When an agent crashes, the tmux session survives. Creating a new tmux session with the same name fails. Now we destroy the old handle first (best-effort, non-blocking). 2. Wrap git clone in try-catch in workspace-clone restore(). On clone failure (network error, disk full), clean up the partial directory so future restore attempts aren't blocked. Co-authored-by: Claude Opus 4.6 --- .../src/__tests__/session-manager.test.ts | 40 ++++++++++++++++++- packages/core/src/session-manager.ts | 17 ++++++-- packages/plugins/workspace-clone/src/index.ts | 26 +++++++----- 3 files changed, 68 insertions(+), 15 deletions(-) diff --git a/packages/core/src/__tests__/session-manager.test.ts b/packages/core/src/__tests__/session-manager.test.ts index 0e1376490..a11a2b9fb 100644 --- a/packages/core/src/__tests__/session-manager.test.ts +++ b/packages/core/src/__tests__/session-manager.test.ts @@ -905,7 +905,8 @@ describe("restore", () => { expect(restored.runtimeHandle).toEqual(makeHandle("rt-1")); expect(restored.restoredAt).toBeInstanceOf(Date); - // Verify runtime was created + // Verify old runtime was destroyed before creating new one + expect(mockRuntime.destroy).toHaveBeenCalledWith(makeHandle("rt-old")); expect(mockRuntime.create).toHaveBeenCalled(); // Verify metadata was updated (not rewritten) const meta = readMetadataRaw(sessionsDir, "app-1"); @@ -917,6 +918,43 @@ describe("restore", () => { expect(meta!["createdAt"]).toBe("2025-01-01T00:00:00.000Z"); }); + it("continues restore even if old runtime destroy fails", async () => { + const wsPath = join(tmpDir, "ws-app-1"); + mkdirSync(wsPath, { recursive: true }); + + // Make destroy throw — should not block restore + const failingRuntime = { + ...mockRuntime, + destroy: vi.fn().mockRejectedValue(new Error("session not found")), + create: vi.fn().mockResolvedValue(makeHandle("rt-new")), + }; + + const registryWithFailingDestroy: PluginRegistry = { + ...mockRegistry, + get: vi.fn().mockImplementation((slot: string) => { + if (slot === "runtime") return failingRuntime; + if (slot === "agent") return mockAgent; + if (slot === "workspace") return mockWorkspace; + return null; + }), + }; + + writeMetadata(sessionsDir, "app-1", { + worktree: wsPath, + branch: "feat/TEST-1", + status: "killed", + project: "my-app", + runtimeHandle: JSON.stringify(makeHandle("rt-old")), + }); + + const sm = createSessionManager({ config, registry: registryWithFailingDestroy }); + const restored = await sm.restore("app-1"); + + expect(restored.status).toBe("spawning"); + expect(failingRuntime.destroy).toHaveBeenCalled(); + expect(failingRuntime.create).toHaveBeenCalled(); + }); + it("recreates workspace when missing and plugin supports restore", async () => { const wsPath = join(tmpDir, "ws-app-1"); // DO NOT create the directory — it's missing diff --git a/packages/core/src/session-manager.ts b/packages/core/src/session-manager.ts index 47c9b80ab..fa42bc17b 100644 --- a/packages/core/src/session-manager.ts +++ b/packages/core/src/session-manager.ts @@ -964,7 +964,16 @@ export function createSessionManager(deps: SessionManagerDeps): SessionManager { } } - // 6. Get launch command — try restore command first, fall back to fresh launch + // 6. Destroy old runtime if still alive (e.g. tmux session survives agent crash) + if (session.runtimeHandle) { + try { + await plugins.runtime.destroy(session.runtimeHandle); + } catch { + // Best effort — may already be gone + } + } + + // 7. Get launch command — try restore command first, fall back to fresh launch let launchCommand: string; const agentLaunchConfig = { sessionId, @@ -983,7 +992,7 @@ export function createSessionManager(deps: SessionManagerDeps): SessionManager { const environment = plugins.agent.getEnvironment(agentLaunchConfig); - // 7. Create runtime (reuse tmuxName from metadata) + // 8. Create runtime (reuse tmuxName from metadata) const tmuxName = raw["tmuxName"]; const handle = await plugins.runtime.create({ sessionId: tmuxName ?? sessionId, @@ -998,7 +1007,7 @@ export function createSessionManager(deps: SessionManagerDeps): SessionManager { }, }); - // 8. Update metadata — merge updates, preserving existing fields + // 9. Update metadata — merge updates, preserving existing fields const now = new Date().toISOString(); updateMetadata(sessionsDir, sessionId, { status: "spawning", @@ -1006,7 +1015,7 @@ export function createSessionManager(deps: SessionManagerDeps): SessionManager { restoredAt: now, }); - // 9. Run postLaunchSetup (non-fatal) + // 10. Run postLaunchSetup (non-fatal) const restoredSession: Session = { ...session, status: "spawning", diff --git a/packages/plugins/workspace-clone/src/index.ts b/packages/plugins/workspace-clone/src/index.ts index d6bc7f924..d8ab278fe 100644 --- a/packages/plugins/workspace-clone/src/index.ts +++ b/packages/plugins/workspace-clone/src/index.ts @@ -190,16 +190,22 @@ export function create(config?: Record): Workspace { remoteUrl = repoPath; } - // Clone fresh - await execFileAsync("git", [ - "clone", - "--reference", - repoPath, - "--branch", - cfg.project.defaultBranch, - remoteUrl, - workspacePath, - ]); + // Clone fresh — clean up partial directory on failure + try { + await execFileAsync("git", [ + "clone", + "--reference", + repoPath, + "--branch", + cfg.project.defaultBranch, + remoteUrl, + workspacePath, + ]); + } catch (cloneErr: unknown) { + rmSync(workspacePath, { recursive: true, force: true }); + const msg = cloneErr instanceof Error ? cloneErr.message : String(cloneErr); + throw new Error(`Clone failed during restore: ${msg}`, { cause: cloneErr }); + } // Try to checkout the branch try {