fix(core): reap worktrees for terminated sessions

.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.

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);

packages/core/src/session-manager.ts

@@ -101,6 +101,7 @@ import {
   setupPathWrapperWorkspace,
   PREFERRED_GH_PATH,
 } from "./agent-workspace-hooks.js";
+import { isWindows } from "./platform.js";
 
 const execFileAsync = promisify(execFile);
 const OPENCODE_DISCOVERY_TIMEOUT_MS = 10_000;
@@ -109,7 +110,7 @@ const INDEXED_PR_METADATA_KEY_REGEX = /^(prEnrichment|prReviewComments)_\d+$/;
 // On Windows, execFile cannot resolve .cmd shim extensions without invoking the shell.
 // windowsHide:true suppresses the conhost popup that the shell would otherwise flash.
 const EXEC_SHELL_OPTION =
-  process.platform === "win32" ? ({ shell: true, windowsHide: true } as const) : ({} as const);
+  isWindows() ? ({ shell: true, windowsHide: true } as const) : ({} as const);
 
 
 function errorIncludesSessionNotFound(err: unknown): boolean {
@@ -431,6 +432,63 @@ 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; sessionId?: SessionId } = {},
+  ): Promise<boolean> {
+    if (!workspacePath || !shouldDestroyWorkspacePath(project, projectId, workspacePath)) {
+      return false;
+    }
+
+    if (options.requireExisting && !existsSync(workspacePath)) {
+      return false;
+    }
+
+    const workspacePlugin = project
+      ? resolvePlugins(project).workspace
+      : registry.get<Workspace>("workspace", config.defaults.workspace);
+    if (!workspacePlugin) {
+      return false;
+    }
+
+    try {
+      await workspacePlugin.destroy(workspacePath);
+      return true;
+    } catch (err) {
+      // Workspace might already be gone — emit AE when we know the session so
+      // abandoned worktrees surface for cleanup tooling.
+      if (projectId && options.sessionId) {
+        recordActivityEvent({
+          projectId,
+          sessionId: options.sessionId,
+          source: "session-manager",
+          kind: "workspace.destroy_failed",
+          level: "warn",
+          summary: `workspace.destroy failed during kill: ${options.sessionId}`,
+          data: {
+            workspace: workspacePlugin.name,
+            reason: err instanceof Error ? err.message : String(err),
+          },
+        });
+      }
+      return false;
+    }
+  }
+
   function isOrchestratorSessionRecord(
     sessionId: string,
     raw: Record<string, string> | null | undefined,
@@ -2516,11 +2574,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, sessionId },
+      );
+      if (cleanedWorkspace) {
+        invalidateCache();
+      }
+      return { cleaned: cleanedWorkspace, alreadyTerminated: true };
     }
 
     const killReason: LifecycleKillReason = options?.reason ?? "manually_killed";
@@ -2568,32 +2637,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>("workspace", config.defaults.workspace);
-      if (workspacePlugin) {
-        try {
-          await workspacePlugin.destroy(worktree);
-        } catch (err) {
-          // Workspace might already be gone — emit AE so abandoned worktrees
-          // surface for cleanup tooling.
-          recordActivityEvent({
-            projectId,
-            sessionId,
-            source: "session-manager",
-            kind: "workspace.destroy_failed",
-            level: "warn",
-            summary: `workspace.destroy failed during kill: ${sessionId}`,
-            data: {
-              workspace: workspacePlugin.name,
-              reason: err instanceof Error ? err.message : String(err),
-            },
-          });
-        }
-      }
-    }
+    await destroyManagedWorkspace(project, projectId, raw["worktree"], { sessionId });
 
     let didPurgeOpenCodeSession = false;
     if (options?.purgeOpenCode === true && cleanupAgent === "opencode") {
@@ -2798,10 +2842,22 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM
           continue;
         }
 
+        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"]);
         if (cleanupAgent === "opencode" && terminatedRaw["opencodeCleanedAt"]) {
-          pushSkipped(projectKey, terminatedId);
+          if (cleanedTerminatedWorkspace) {
+            pushKilled(projectKey, terminatedId);
+          } else {
+            pushSkipped(projectKey, terminatedId);
+          }
           continue;
         }
         if (cleanupAgent === "opencode" && mappedOpenCodeSessionId && shouldPurgeOpenCode) {
@@ -2835,6 +2891,8 @@ export function createSessionManager(deps: SessionManagerDeps): OpenCodeSessionM
             }
           }
           pushKilled(projectKey, terminatedId);
+        } else if (cleanedTerminatedWorkspace) {
+          pushKilled(projectKey, terminatedId);
         } else {
           pushSkipped(projectKey, terminatedId);
         }

packages/core/src/types.ts

@@ -1857,8 +1857,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;