fix(acp): prevent orphaned claude-agent-acp processes after session kill (#4691)

src/__tests__/fix-4691-orphaned-acp-processes.test.ts

@@ -0,0 +1,141 @@
+/**
+ * fix-4691-orphaned-acp-processes.test.ts — Regression test for issue #4691.
+ *
+ * Ensures:
+ * 1. The orphan reaper excludes terminal sessions (killed, completed, crashed)
+ *    from getActiveSessionIds, so orphaned ACP runtimes for killed sessions
+ *    are detected and reaped.
+ * 2. shutdownRuntime catches session/close request failures and still proceeds
+ *    to client.shutdown(), preventing orphaned child processes when the ACP
+ *    bridge is unresponsive.
+ */
+
+import { describe, expect, it, vi } from 'vitest';
+import { reapOrphanAcpRuntimes } from '../services/acp/orphan-reaper.js';
+import { shutdownRuntime } from '../services/acp/backend/runtime.js';
+import type { RuntimeLifecycleDeps } from '../services/acp/backend/runtime.js';
+import type { AcpBackendRuntime } from '../services/acp/backend/types.js';
+import type { AcpSessionRecord } from '../services/acp/types.js';
+import type { StructuredLogger } from '../logger.js';
+
+const log = {
+  info: vi.fn(),
+  warn: vi.fn(),
+  error: vi.fn(),
+} as unknown as StructuredLogger;
+
+describe('Issue #4691 — orphaned claude-agent-acp processes after session kill', () => {
+  describe('orphan reaper excludes terminal sessions', () => {
+    it('reaps ACP runtime for a killed session', async () => {
+      const shutdownAcpRuntime = vi.fn(async () => {});
+      const result = await reapOrphanAcpRuntimes({
+        getActiveSessionIds: () => ['sess-active'],
+        getActiveAcpRuntimeIds: () => ['sess-active', 'sess-killed'],
+        shutdownAcpRuntime,
+        log,
+      });
+
+      expect(result.scanned).toBe(2);
+      expect(result.reaped).toBe(1);
+      expect(result.orphanIds).toEqual(['sess-killed']);
+      expect(shutdownAcpRuntime).toHaveBeenCalledWith('sess-killed');
+    });
+
+    it('does NOT reap runtime for an active session', async () => {
+      const shutdownAcpRuntime = vi.fn(async () => {});
+      const result = await reapOrphanAcpRuntimes({
+        getActiveSessionIds: () => ['sess-active'],
+        getActiveAcpRuntimeIds: () => ['sess-active'],
+        shutdownAcpRuntime,
+        log,
+      });
+
+      expect(result.scanned).toBe(1);
+      expect(result.reaped).toBe(0);
+      expect(result.orphanIds).toEqual([]);
+      expect(shutdownAcpRuntime).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('shutdownRuntime catches session/close failure', () => {
+    it('still calls client.shutdown() when session/close request throws', async () => {
+      const session = {
+        id: 'sess-1',
+        status: 'closing',
+        tenantId: 'SYSTEM',
+        ownerKeyId: 'master',
+        acpAgentSessionId: 'acp-sess-1',
+      } as AcpSessionRecord;
+
+      const shutdown = vi.fn(async () => ({ code: 0, signal: null }));
+      const request = vi.fn(async () => {
+        throw new Error('session/close timeout');
+      });
+      const client = { request, shutdown };
+
+      const runtime = {
+        sessionId: 'sess-1',
+        scope: { tenantId: 'SYSTEM', ownerKeyId: 'master' },
+        backendRunId: 'run-1',
+        client,
+        disposers: [],
+      } as unknown as AcpBackendRuntime;
+
+      const deps = {
+        sessionService: {
+          transition: vi.fn(async (_id, _scope, transition) => ({ ...session, status: transition.type === 'close_completed' ? 'closed' : 'closing' })),
+          getSession: vi.fn(async () => session),
+        },
+        options: {},
+        runtimes: new Map(),
+        inFlightPrompts: new Map(),
+        pendingApprovals: new Map(),
+      } as unknown as RuntimeLifecycleDeps;
+
+      const result = await shutdownRuntime(deps, session, runtime);
+
+      expect(request).toHaveBeenCalledWith('session/close', { sessionId: 'acp-sess-1' });
+      expect(shutdown).toHaveBeenCalledTimes(1);
+      expect(deps.runtimes.has('sess-1')).toBe(false);
+      expect(result.session).toBeDefined();
+    });
+
+    it('returns successfully even if both session/close and shutdown throw', async () => {
+      const session = {
+        id: 'sess-2',
+        status: 'closing',
+        tenantId: 'SYSTEM',
+        ownerKeyId: 'master',
+        acpAgentSessionId: 'acp-sess-2',
+      } as AcpSessionRecord;
+
+      const client = {
+        request: vi.fn(async () => { throw new Error('close failed'); }),
+        shutdown: vi.fn(async () => { throw new Error('shutdown failed'); }),
+      };
+
+      const runtime = {
+        sessionId: 'sess-2',
+        scope: { tenantId: 'SYSTEM', ownerKeyId: 'master' },
+        backendRunId: 'run-2',
+        client,
+        disposers: [],
+      } as unknown as AcpBackendRuntime;
+
+      const deps = {
+        sessionService: {
+          transition: vi.fn(async () => session),
+          getSession: vi.fn(async () => session),
+        },
+        options: {},
+        runtimes: new Map([['sess-2', runtime]]),
+        inFlightPrompts: new Map(),
+        pendingApprovals: new Map(),
+      } as unknown as RuntimeLifecycleDeps;
+
+      const result = await shutdownRuntime(deps, session, runtime);
+      expect(result.session).toBeDefined();
+      expect(deps.runtimes.has('sess-2')).toBe(false);
+    });
+  });
+});

src/boot/boot-shutdown.ts

@@ -17,6 +17,8 @@ import type { MetricsCache } from '../services/metrics-cache.js';
 import type { TimerRegistry } from '../utils/timer-registry.js';
 import type { BudgetTimer } from '../budgets/timer.js';
 import { killAllSessions } from '../signal-cleanup-helper.js';
+import { shutdownAcpRuntime } from '../session-cleanup.js';
+import { SYSTEM_TENANT } from '../config.js';
 import { removePidFile } from '../startup.js';
 import { shutdownTracing } from '../tracing.js';
 import { getRateLimiter } from '../middleware/auth-setup.js';
@@ -154,6 +156,21 @@ export function registerShutdownHandler(deps: ShutdownDeps): void {
         }
       }
 
+      // Issue #4691: Shut down ACP runtimes before killing sessions to prevent orphaned processes
+      if (ctx.acpBackend) {
+        for (const session of ctx.sessions.listSessions()) {
+          try {
+            await shutdownAcpRuntime(session.id, ctx);
+          } catch (e) {
+            logger.warn({
+              component: 'server',
+              operation: 'graceful_shutdown_acp_runtime',
+              attributes: { sessionId: session.id, error: e instanceof Error ? e.message : String(e) },
+            });
+          }
+        }
+      }
+
       // Issue #569: Kill all CC sessions before exit
       try {
         await killAllSessions(ctx.sessions, { monitor: ctx.monitor, metrics: ctx.metrics, toolRegistry: ctx.toolRegistry });

src/monitor/dead-detector.ts

@@ -27,6 +27,8 @@ export interface DeadDetectorDeps {
   statusChange: (payload: SessionEventPayload) => void;
   /** Remove session from internal tracking (maps, sets, watchers). */
   removeSession: (sessionId: string) => void;
+  /** Issue #4691: Shut down ACP runtime before killing session. */
+  shutdownAcpRuntime?: (sessionId: string) => Promise<void>;
 }
 
 /**
@@ -100,6 +102,21 @@ export class DeadDetector {
 
     this.deps.removeSession(session.id);
 
+    // Issue #4691: Shut down ACP runtime before killing session to prevent orphans
+    if (this.deps.shutdownAcpRuntime) {
+      try {
+        await this.deps.shutdownAcpRuntime(session.id);
+      } catch (e) {
+        logger.warn({
+          component: 'monitor',
+          operation: 'check_dead_sessions',
+          sessionId: session.id,
+          errorCode: 'ACP_SHUTDOWN_FAILED',
+          attributes: { error: e instanceof Error ? e.message : String(e) },
+        });
+      }
+    }
+
     // #262: Also remove from SessionManager so dead sessions don't linger
     try {
       await this.deps.sessions.killSession(session.id);

src/server-bootstrap.ts

@@ -356,7 +356,7 @@ new JsonFileBackend(path.join(ctx.config.stateDir, 'analytics-cache.json')),
     if (!ctx.acpBackend) return;
     const { reapOrphanAcpRuntimes } = await import('./services/acp/orphan-reaper.js');
     await reapOrphanAcpRuntimes({
-      getActiveSessionIds: () => ctx.sessions.listSessions().map(s => s.id),
+      getActiveSessionIds: () => ctx.sessions.listSessions().filter(s => s.status !== 'killed' && s.status !== 'completed' && s.status !== 'crashed').map(s => s.id),
       getActiveAcpRuntimeIds: () => ctx.acpBackend!.getActiveRuntimeIds(),
       shutdownAcpRuntime: (id) => ctx.acpBackend!.shutdownSession({
         sessionId: id,

src/services/acp/backend/runtime.ts

@@ -319,7 +319,15 @@ export async function shutdownRuntime(
     }
     const acpAgentSessionId = current.acpAgentSessionId;
     if (acpAgentSessionId) {
-      await runtime.client.request('session/close', { sessionId: acpAgentSessionId });
+      try {
+        await runtime.client.request('session/close', { sessionId: acpAgentSessionId });
+      } catch (closeError) {
+        log.warn({
+          component: 'acp-backend',
+          operation: 'sessionCloseRequestFailed',
+          attributes: { sessionId: session.id, error: String(closeError) },
+        });
+      }
     }
     exit = await runtime.client.shutdown();
     if (current.status === 'closing') {
@@ -329,12 +337,23 @@ export async function shutdownRuntime(
     } else {
       current = await deps.sessionService.getSession(session.id, runtime.scope);
     }
-    return { session: current, exit };
+  } catch (shutdownError) {
+    log.warn({
+      component: 'acp-backend',
+      operation: 'runtimeShutdownFailed',
+      attributes: { sessionId: session.id, error: String(shutdownError) },
+    });
+    try {
+      current = await deps.sessionService.getSession(session.id, runtime.scope);
+    } catch {
+      // Session may have been deleted; use the last known state
+    }
   } finally {
     disposeRuntime(deps, runtime);
     deps.runtimes.delete(session.id);
     deps.inFlightPrompts.delete(session.id);
   }
+  return { session: current, exit };
 }
 
 export async function handleRuntimeExit(