From 769e0f37d4fecce70466a3ed2b67b91086309e6e Mon Sep 17 00:00:00 2001 From: ChiragArora31 Date: Tue, 28 Apr 2026 20:05:28 +0530 Subject: [PATCH 1/3] fix: reap terminated session worktrees --- .../session-manager/lifecycle.test.ts | 83 +++++++++++++++++ packages/core/src/session-manager.ts | 93 +++++++++++++++---- packages/core/src/types.ts | 5 +- 3 files changed, 162 insertions(+), 19 deletions(-) diff --git a/packages/core/src/__tests__/session-manager/lifecycle.test.ts b/packages/core/src/__tests__/session-manager/lifecycle.test.ts index 0789fe04c0..093b21ec25 100644 --- a/packages/core/src/__tests__/session-manager/lifecycle.test.ts +++ b/packages/core/src/__tests__/session-manager/lifecycle.test.ts @@ -3,6 +3,7 @@ import { mkdirSync, readFileSync, existsSync, + rmSync, } from "node:fs"; import { join } from "node:path"; import { homedir } from "node:os"; @@ -137,6 +138,31 @@ describe("kill", () => { await expect(sm.kill("nonexistent")).rejects.toThrow("not found"); }); + it("reaps a managed workspace for an already terminated session", async () => { + const managedWorktree = join(getProjectWorktreesDir("my-app"), "app-terminated"); + mkdirSync(managedWorktree, { recursive: true }); + + writeMetadata(sessionsDir, "app-terminated", { + worktree: managedWorktree, + branch: "main", + status: "killed", + project: "my-app", + runtimeHandle: makeHandle("rt-terminated"), + }); + updateMetadata(sessionsDir, "app-terminated", { + lifecycle: JSON.stringify({ + session: { state: "terminated", terminatedAt: new Date().toISOString() }, + }), + }); + + const sm = createSessionManager({ config, registry: mockRegistry }); + const result = await sm.kill("app-terminated"); + + expect(result).toEqual({ cleaned: true, alreadyTerminated: true }); + expect(mockRuntime.destroy).not.toHaveBeenCalled(); + expect(mockWorkspace.destroy).toHaveBeenCalledWith(managedWorktree); + }); + it("tolerates runtime destroy failure", async () => { const failRuntime: Runtime = { ...mockRuntime, @@ -397,6 +423,63 @@ describe("cleanup", () => { expect(deleteLog).toContain("session delete ses_archived"); }, 15_000); + it("reaps worktrees left behind by already terminated sessions", async () => { + const managedWorktree = join(getProjectWorktreesDir("my-app"), "app-terminated-cleanup"); + mkdirSync(managedWorktree, { recursive: true }); + + writeMetadata(sessionsDir, "app-terminated-cleanup", { + worktree: managedWorktree, + branch: "main", + status: "killed", + project: "my-app", + runtimeHandle: makeHandle("rt-terminated-cleanup"), + }); + updateMetadata(sessionsDir, "app-terminated-cleanup", { + lifecycle: JSON.stringify({ + session: { state: "terminated", terminatedAt: new Date().toISOString() }, + }), + }); + + const sm = createSessionManager({ config, registry: mockRegistry }); + const result = await sm.cleanup(); + + expect(result.killed).toContain("app-terminated-cleanup"); + expect(result.skipped).not.toContain("app-terminated-cleanup"); + expect(mockRuntime.destroy).not.toHaveBeenCalled(); + expect(mockWorkspace.destroy).toHaveBeenCalledWith(managedWorktree); + }); + + it("skips already terminated sessions after orphan worktree cleanup is idempotent", async () => { + const managedWorktree = join(getProjectWorktreesDir("my-app"), "app-terminated-idempotent"); + mkdirSync(managedWorktree, { recursive: true }); + vi.mocked(mockWorkspace.destroy).mockImplementation(async (workspacePath: string) => { + rmSync(workspacePath, { recursive: true, force: true }); + }); + + writeMetadata(sessionsDir, "app-terminated-idempotent", { + worktree: managedWorktree, + branch: "main", + status: "killed", + project: "my-app", + runtimeHandle: makeHandle("rt-terminated-idempotent"), + }); + updateMetadata(sessionsDir, "app-terminated-idempotent", { + lifecycle: JSON.stringify({ + session: { state: "terminated", terminatedAt: new Date().toISOString() }, + }), + }); + + const sm = createSessionManager({ config, registry: mockRegistry }); + const first = await sm.cleanup(); + const second = await sm.cleanup(); + + expect(first.killed).toContain("app-terminated-idempotent"); + expect(second.killed).not.toContain("app-terminated-idempotent"); + expect(second.skipped).toContain("app-terminated-idempotent"); + expect(mockWorkspace.destroy).toHaveBeenCalledTimes(1); + expect(existsSync(managedWorktree)).toBe(false); + }); + it("does not skip terminated cleanup for matching session IDs in other projects", async () => { const deleteLogPath = join(tmpDir, "opencode-delete-archived-cross-project.log"); const mockBin = installMockOpencode(tmpDir, "[]", deleteLogPath); diff --git a/packages/core/src/session-manager.ts b/packages/core/src/session-manager.ts index 29f6bc3365..c1e5800e7b 100644 --- a/packages/core/src/session-manager.ts +++ b/packages/core/src/session-manager.ts @@ -412,6 +412,48 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM return roots.some((root) => isPathInside(workspacePath, root)); } + function hasReapableManagedWorkspace( + project: ProjectConfig | undefined, + projectId: string | undefined, + workspacePath: string | undefined, + ): boolean { + return Boolean( + workspacePath && + shouldDestroyWorkspacePath(project, projectId, workspacePath) && + existsSync(workspacePath), + ); + } + + async function destroyManagedWorkspace( + project: ProjectConfig | undefined, + projectId: string | undefined, + workspacePath: string | undefined, + options: { requireExisting?: boolean } = {}, + ): Promise { + if (!workspacePath || !shouldDestroyWorkspacePath(project, projectId, workspacePath)) { + return false; + } + + if (options.requireExisting && !existsSync(workspacePath)) { + return false; + } + + const workspacePlugin = project + ? resolvePlugins(project).workspace + : registry.get("workspace", config.defaults.workspace); + if (!workspacePlugin) { + return false; + } + + try { + await workspacePlugin.destroy(workspacePath); + return true; + } catch { + // Workspace might already be gone + return false; + } + } + function isOrchestratorSessionRecord( sessionId: string, raw: Record | null | undefined, @@ -2048,11 +2090,22 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM } const { raw, sessionsDir, project, projectId } = located; - // Idempotency: if lifecycle already says terminated, don't re-run destroys - // (which could double-purge opencode or race with concurrent kills). + // Idempotency: terminal lifecycle means runtime/agent teardown has already + // been recorded, but it does not prove the workspace was reaped. A crashed + // kill or runtime_lost transition can leave the worktree on disk, so allow + // a safe workspace-only cleanup retry before returning. const existingLifecycle = parseCanonicalLifecycle(raw); if (existingLifecycle?.session.state === "terminated") { - return { cleaned: false, alreadyTerminated: true }; + const cleanedWorkspace = await destroyManagedWorkspace( + project, + projectId, + raw["worktree"], + { requireExisting: true }, + ); + if (cleanedWorkspace) { + invalidateCache(); + } + return { cleaned: cleanedWorkspace, alreadyTerminated: true }; } const killReason: LifecycleKillReason = options?.reason ?? "manually_killed"; @@ -2077,19 +2130,7 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM } } - const worktree = raw["worktree"]; - if (worktree && shouldDestroyWorkspacePath(project, projectId, worktree)) { - const workspacePlugin = project - ? resolvePlugins(project).workspace - : registry.get("workspace", config.defaults.workspace); - if (workspacePlugin) { - try { - await workspacePlugin.destroy(worktree); - } catch { - // Workspace might already be gone - } - } - } + await destroyManagedWorkspace(project, projectId, raw["worktree"]); let didPurgeOpenCodeSession = false; if (options?.purgeOpenCode === true && cleanupAgent === "opencode") { @@ -2260,10 +2301,26 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM continue; } + let cleanedTerminatedWorkspace = false; + if (options?.dryRun) { + cleanedTerminatedWorkspace = hasReapableManagedWorkspace( + project, + projectKey, + terminatedRaw["worktree"], + ); + } else { + const killResult = await kill(terminatedId, { purgeOpenCode: false }); + cleanedTerminatedWorkspace = killResult.cleaned; + } + const cleanupAgent = resolveSelectionForSession(project, terminatedId, terminatedRaw).agentName; const mappedOpenCodeSessionId = asValidOpenCodeSessionId(terminatedRaw["opencodeSessionId"]); if (cleanupAgent === "opencode" && terminatedRaw["opencodeCleanedAt"]) { - pushSkipped(projectKey, terminatedId); + if (cleanedTerminatedWorkspace) { + pushKilled(projectKey, terminatedId); + } else { + pushSkipped(projectKey, terminatedId); + } continue; } if (cleanupAgent === "opencode" && mappedOpenCodeSessionId && shouldPurgeOpenCode) { @@ -2284,6 +2341,8 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM } } pushKilled(projectKey, terminatedId); + } else if (cleanedTerminatedWorkspace) { + pushKilled(projectKey, terminatedId); } else { pushSkipped(projectKey, terminatedId); } diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 9e8d73eeee..6a4f0775c3 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -1695,8 +1695,9 @@ export type LifecycleKillReason = "manually_killed" | "pr_merged" | "auto_cleanu /** * Outcome of a kill() call. `cleaned` means resources were torn down this - * invocation; `alreadyTerminated` means the session was already archived and - * kill() was a no-op. Callers can use this to avoid double-notifying. + * invocation; `alreadyTerminated` means terminal lifecycle metadata was already + * recorded before this call. Both may be true when kill() reaps a workspace + * left behind by a previously terminated session. */ export interface KillResult { cleaned: boolean; From c15c97ad252485d7c9996177c4d7049134edada7 Mon Sep 17 00:00:00 2001 From: ChiragArora31 Date: Tue, 28 Apr 2026 20:10:49 +0530 Subject: [PATCH 2/3] chore: add terminated worktree cleanup changeset --- .changeset/reap-terminated-session-worktrees.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/reap-terminated-session-worktrees.md diff --git a/.changeset/reap-terminated-session-worktrees.md b/.changeset/reap-terminated-session-worktrees.md new file mode 100644 index 0000000000..b38d5ce678 --- /dev/null +++ b/.changeset/reap-terminated-session-worktrees.md @@ -0,0 +1,5 @@ +--- +"@aoagents/ao-core": patch +--- + +Reap managed worktrees left behind by sessions whose lifecycle was already marked terminated before cleanup ran. From 8eaa6f497c0ed8cefb065d6077a669deca27dea2 Mon Sep 17 00:00:00 2001 From: ChiragArora31 Date: Tue, 28 Apr 2026 22:22:48 +0530 Subject: [PATCH 3/3] fix(core): remove useless assignment in terminated-worktree cleanup Use a single expression for cleanedTerminatedWorkspace in cleanup() so eslint no-useless-assignment passes while preserving dry-run and kill-path behavior. Made-with: Cursor --- packages/core/src/session-manager.ts | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/packages/core/src/session-manager.ts b/packages/core/src/session-manager.ts index c1e5800e7b..14f404703e 100644 --- a/packages/core/src/session-manager.ts +++ b/packages/core/src/session-manager.ts @@ -2301,17 +2301,13 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM continue; } - let cleanedTerminatedWorkspace = false; - if (options?.dryRun) { - cleanedTerminatedWorkspace = hasReapableManagedWorkspace( - project, - projectKey, - terminatedRaw["worktree"], - ); - } else { - const killResult = await kill(terminatedId, { purgeOpenCode: false }); - cleanedTerminatedWorkspace = killResult.cleaned; - } + const cleanedTerminatedWorkspace = options?.dryRun + ? hasReapableManagedWorkspace( + project, + projectKey, + terminatedRaw["worktree"], + ) + : (await kill(terminatedId, { purgeOpenCode: false })).cleaned; const cleanupAgent = resolveSelectionForSession(project, terminatedId, terminatedRaw).agentName; const mappedOpenCodeSessionId = asValidOpenCodeSessionId(terminatedRaw["opencodeSessionId"]);