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 <noreply@anthropic.com>
This commit is contained in:
parent
65fa811b3b
commit
1e304110f7
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -190,16 +190,22 @@ export function create(config?: Record<string, unknown>): 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 {
|
||||
|
|
|
|||
Loading…
Reference in New Issue