Merge pull request #3510 from code-yeongyu/fix/tmux-stale-session-sweep

feat(tmux): sweep stale omo-agents-<pid> sessions on first spawn

src/features/tmux-subagent/manager.test.ts

@@ -58,6 +58,7 @@ const mockSpawnTmuxSession = mock<(
   paneId: '%isolated-session',
 }))
 const mockKillTmuxSessionIfExists = mock<(sessionName: string) => Promise<boolean>>(async () => true)
+const mockSweepStaleOmoAgentSessions = mock<() => Promise<number>>(async () => 0)
 const mockIsInsideTmux = mock<() => boolean>(() => true)
 const mockGetCurrentPaneId = mock<() => string | undefined>(() => '%0')
 
@@ -102,6 +103,7 @@ mock.module('../../shared/tmux', () => {
     spawnTmuxSession: mockSpawnTmuxSession,
     killTmuxSessionIfExists: mockKillTmuxSessionIfExists,
     getIsolatedSessionName: (pid: number = 12345) => `omo-agents-${pid}`,
+    sweepStaleOmoAgentSessions: mockSweepStaleOmoAgentSessions,
   }
 })
 
@@ -1930,29 +1932,32 @@ describe('TmuxSessionManager', () => {
       expect(mockKillTmuxSessionIfExists).toHaveBeenCalledTimes(0)
     })
 
-    test('#given a tracked session #when onSessionError is invoked #then the pane is closed like onSessionDeleted', async () => {
+    test('#given sweepStaleOmoAgentSessions throws on first onSessionCreated #when second onSessionCreated fires #then sweep is retried instead of skipped forever', async () => {
       // given
+      mockSweepStaleOmoAgentSessions.mockClear()
+      mockSweepStaleOmoAgentSessions.mockImplementationOnce(async () => {
+        throw new Error('simulated sweep failure')
+      })
       mockIsInsideTmux.mockReturnValue(true)
-      mockExecuteAction.mockClear()
       const { TmuxSessionManager } = await import('./manager')
       const manager = new TmuxSessionManager(createMockContext(), createTmuxConfig({
         enabled: true,
         isolation: 'session',
       }), mockTmuxDeps)
-      await manager.onSessionCreated(createSessionCreatedEvent('ses_err', 'ses_parent', 'Errored Task'))
-      mockExecuteAction.mockClear()
 
       // when
-      await manager.onSessionError({ sessionID: 'ses_err' })
+      await manager.onSessionCreated(createSessionCreatedEvent('ses_first', 'ses_parent', 'First'))
+      await manager.onSessionCreated(createSessionCreatedEvent('ses_second', 'ses_parent', 'Second'))
 
       // then
-      expect(mockExecuteAction).toHaveBeenCalled()
+      expect(mockSweepStaleOmoAgentSessions).toHaveBeenCalledTimes(2)
     })
 
-    test('#given an untracked session #when onSessionError is invoked #then it is a no-op and does not throw', async () => {
+    test('#given sweepStaleOmoAgentSessions succeeds #when additional onSessionCreated events fire in same process #then sweep runs exactly once', async () => {
       // given
+      mockSweepStaleOmoAgentSessions.mockClear()
+      mockSweepStaleOmoAgentSessions.mockImplementation(async () => 0)
       mockIsInsideTmux.mockReturnValue(true)
-      mockExecuteAction.mockClear()
       const { TmuxSessionManager } = await import('./manager')
       const manager = new TmuxSessionManager(createMockContext(), createTmuxConfig({
         enabled: true,
@@ -1960,11 +1965,12 @@ describe('TmuxSessionManager', () => {
       }), mockTmuxDeps)
 
       // when
-      const errorHandler = manager.onSessionError({ sessionID: 'ses_unknown' })
+      await manager.onSessionCreated(createSessionCreatedEvent('ses_a', 'ses_parent', 'A'))
+      await manager.onSessionCreated(createSessionCreatedEvent('ses_b', 'ses_parent', 'B'))
+      await manager.onSessionCreated(createSessionCreatedEvent('ses_c', 'ses_parent', 'C'))
 
       // then
-      await expect(errorHandler).resolves.toBeUndefined()
-      expect(mockExecuteAction).not.toHaveBeenCalled()
+      expect(mockSweepStaleOmoAgentSessions).toHaveBeenCalledTimes(1)
     })
 
     test('#given killTmuxSessionIfExists throws #when cleanup runs #then cleanup still completes without throwing', async () => {

src/features/tmux-subagent/manager.ts

@@ -12,6 +12,7 @@ import {
   spawnTmuxSession,
   killTmuxSessionIfExists,
   getIsolatedSessionName,
+  sweepStaleOmoAgentSessions,
 } from "../../shared/tmux"
 import { queryWindowState } from "./pane-state-querier"
 import { decideSpawnActions, decideCloseAction, type SessionMapping } from "./decision-engine"
@@ -65,6 +66,8 @@ export class TmuxSessionManager {
   private isolatedContainerPaneId: string | undefined
   private isolatedWindowPaneId: string | undefined
   private isolatedContainerNullStateCount = 0
+  private staleSweepCompleted = false
+  private staleSweepInProgress = false
   constructor(ctx: PluginInput, tmuxConfig: TmuxConfig, deps: TmuxUtilDeps = defaultTmuxDeps) {
     this.client = ctx.client
     this.tmuxConfig = tmuxConfig
@@ -668,6 +671,7 @@ export class TmuxSessionManager {
       return
     }
 
+    await this.sweepStaleIsolatedSessionsOnce()
     await this.retryPendingCloses()
 
     if (
@@ -835,18 +839,6 @@ export class TmuxSessionManager {
     await this.spawnQueue
   }
 
-  async onSessionError(event: { sessionID: string }): Promise<void> {
-    if (!this.isEnabled()) return
-    if (!this.getEffectiveSourcePaneId()) return
-    if (!this.sessions.has(event.sessionID)) return
-
-    log("[tmux-session-manager] onSessionError - routing to cleanup", {
-      sessionId: event.sessionID,
-    })
-
-    await this.onSessionDeleted(event)
-  }
-
   async onSessionDeleted(event: { sessionID: string }): Promise<void> {
     if (!this.isEnabled()) return
     if (!this.getEffectiveSourcePaneId()) return
@@ -985,6 +977,33 @@ export class TmuxSessionManager {
       }
     }
 
+    this.staleSweepCompleted = false
+    this.staleSweepInProgress = false
+
     log("[tmux-session-manager] cleanup complete")
   }
+
+  private async sweepStaleIsolatedSessionsOnce(): Promise<void> {
+    if (this.staleSweepCompleted) return
+    if (this.staleSweepInProgress) return
+    if (this.tmuxConfig.isolation !== "session") {
+      this.staleSweepCompleted = true
+      return
+    }
+
+    this.staleSweepInProgress = true
+    try {
+      const killed = await sweepStaleOmoAgentSessions()
+      if (killed > 0) {
+        log("[tmux-session-manager] stale isolated sessions swept", { killed })
+      }
+      this.staleSweepCompleted = true
+    } catch (error) {
+      log("[tmux-session-manager] stale sweep failed", {
+        error: String(error),
+      })
+    } finally {
+      this.staleSweepInProgress = false
+    }
+  }
 }

src/plugin/event.ts

@@ -615,10 +615,6 @@ export function createEventHandler(args: {
         const sessionID = props?.sessionID as string | undefined;
         const error = props?.error;
 
-        if (tmuxIntegrationEnabled && sessionID) {
-          await managers.tmuxSessionManager.onSessionError({ sessionID });
-        }
-
         const errorName = extractErrorName(error);
         const errorMessage = extractErrorMessage(error);
         const errorInfo = { name: errorName, message: errorMessage };

src/shared/tmux/tmux-utils.ts

@@ -12,5 +12,6 @@ export { replaceTmuxPane } from "./tmux-utils/pane-replace"
 export { spawnTmuxWindow } from "./tmux-utils/window-spawn"
 export { spawnTmuxSession, getIsolatedSessionName } from "./tmux-utils/session-spawn"
 export { killTmuxSessionIfExists } from "./tmux-utils/session-kill"
+export { sweepStaleOmoAgentSessions } from "./tmux-utils/stale-session-sweep"
 
 export { applyLayout, enforceMainPaneWidth } from "./tmux-utils/layout"

src/shared/tmux/tmux-utils/stale-session-sweep.test.ts

@@ -0,0 +1,154 @@
+import { beforeEach, describe, expect, it, mock } from "bun:test"
+import { sweepStaleOmoAgentSessionsWith, type SweepDeps } from "./stale-session-sweep"
+
+type SweepFixture = {
+	deps: SweepDeps
+	candidates: string[]
+	killed: string[]
+	killSessionMock: ReturnType<typeof mock>
+	setCandidates: (sessions: string[]) => void
+	setAlive: (predicate: (pid: number) => boolean) => void
+}
+
+function createFixture(): SweepFixture {
+	const candidates: string[] = []
+	const killed: string[] = []
+	let aliveCheck: (pid: number) => boolean = () => false
+
+	const killSessionMock = mock(async (sessionName: string): Promise<boolean> => {
+		killed.push(sessionName)
+		return true
+	})
+
+	const deps: SweepDeps = {
+		isInsideTmux: () => true,
+		getTmuxPath: async () => "tmux",
+		listCandidateSessions: async () => [...candidates],
+		killSession: killSessionMock,
+		processAlive: (pid) => aliveCheck(pid),
+		currentPid: 12345,
+		log: () => undefined,
+	}
+
+	return {
+		deps,
+		candidates,
+		killed,
+		killSessionMock,
+		setCandidates: (sessions) => {
+			candidates.length = 0
+			candidates.push(...sessions)
+		},
+		setAlive: (predicate) => {
+			aliveCheck = predicate
+		},
+	}
+}
+
+describe("sweepStaleOmoAgentSessionsWith", () => {
+	let fixture: SweepFixture
+
+	beforeEach(() => {
+		fixture = createFixture()
+	})
+
+	it("#given not inside tmux #when sweep called #then returns 0 without listing", async () => {
+		// given
+		const deps: SweepDeps = { ...fixture.deps, isInsideTmux: () => false }
+
+		// when
+		const result = await sweepStaleOmoAgentSessionsWith(deps)
+
+		// then
+		expect(result).toBe(0)
+	})
+
+	it("#given tmux not found #when sweep called #then returns 0 without listing", async () => {
+		// given
+		const deps: SweepDeps = { ...fixture.deps, getTmuxPath: async () => undefined }
+
+		// when
+		const result = await sweepStaleOmoAgentSessionsWith(deps)
+
+		// then
+		expect(result).toBe(0)
+	})
+
+	it("#given candidate list is empty #when sweep called #then returns 0 and does not kill anything", async () => {
+		// given
+		fixture.setCandidates([])
+
+		// when
+		const result = await sweepStaleOmoAgentSessionsWith(fixture.deps)
+
+		// then
+		expect(result).toBe(0)
+		expect(fixture.killed).toEqual([])
+	})
+
+	it("#given sessions with dead PIDs #when sweep called #then each dead session is killed once", async () => {
+		// given
+		fixture.setCandidates(["omo-agents-99991", "omo-agents-99992"])
+		fixture.setAlive(() => false)
+
+		// when
+		const result = await sweepStaleOmoAgentSessionsWith(fixture.deps)
+
+		// then
+		expect(result).toBe(2)
+		expect(fixture.killed).toEqual(["omo-agents-99991", "omo-agents-99992"])
+	})
+
+	it("#given session matches current PID #when sweep called #then it is NOT killed", async () => {
+		// given
+		fixture.setCandidates([`omo-agents-${fixture.deps.currentPid}`, "omo-agents-99999"])
+		fixture.setAlive(() => false)
+
+		// when
+		const result = await sweepStaleOmoAgentSessionsWith(fixture.deps)
+
+		// then
+		expect(result).toBe(1)
+		expect(fixture.killed).toEqual(["omo-agents-99999"])
+	})
+
+	it("#given session PID is still alive #when sweep called #then it is NOT killed", async () => {
+		// given
+		fixture.setCandidates(["omo-agents-88888"])
+		fixture.setAlive((pid) => pid === 88888)
+
+		// when
+		const result = await sweepStaleOmoAgentSessionsWith(fixture.deps)
+
+		// then
+		expect(result).toBe(0)
+		expect(fixture.killed).toEqual([])
+	})
+
+	it("#given killSession returns false #when sweep called #then session is not counted toward killedCount", async () => {
+		// given
+		fixture.setCandidates(["omo-agents-55555"])
+		fixture.setAlive(() => false)
+		fixture.killSessionMock.mockImplementation(async () => false)
+
+		// when
+		const result = await sweepStaleOmoAgentSessionsWith(fixture.deps)
+
+		// then
+		expect(result).toBe(0)
+		expect(fixture.killSessionMock).toHaveBeenCalledTimes(1)
+	})
+
+	it("#given non-matching sessions mixed in #when sweep called #then only omo-agents-<pid> sessions are considered", async () => {
+		// given
+		fixture.setCandidates(["main", "omo-agents-99999", "other-session", "omo-agents-abc"])
+		fixture.setAlive(() => false)
+
+		// when
+		const result = await sweepStaleOmoAgentSessionsWith(fixture.deps)
+
+		// then
+		expect(result).toBe(1)
+		expect(fixture.killed).toEqual(["omo-agents-99999"])
+	})
+})

src/shared/tmux/tmux-utils/stale-session-sweep.ts

@@ -0,0 +1,99 @@
+const STALE_SESSION_PATTERN = /^omo-agents-(\d+)$/
+
+function isProcessAlive(pid: number): boolean {
+	try {
+		process.kill(pid, 0)
+		return true
+	} catch (error) {
+		const err = error as NodeJS.ErrnoException
+		return err?.code === "EPERM"
+	}
+}
+
+async function listOmoAgentSessionsViaTmux(tmux: string): Promise<string[]> {
+	const { spawn } = await import("./spawn-process")
+	const proc = spawn([tmux, "list-sessions", "-F", "#{session_name}"], {
+		stdout: "pipe",
+		stderr: "pipe",
+	})
+	const [stdout, , exitCode] = await Promise.all([
+		new Response(proc.stdout).text(),
+		new Response(proc.stderr).text(),
+		proc.exited,
+	])
+
+	if (exitCode !== 0) {
+		return []
+	}
+
+	return stdout
+		.split("\n")
+		.map((line) => line.trim())
+		.filter((name) => STALE_SESSION_PATTERN.test(name))
+}
+
+export type SweepDeps = {
+	isInsideTmux: () => boolean
+	getTmuxPath: () => Promise<string | null | undefined>
+	listCandidateSessions: (tmux: string) => Promise<string[]>
+	killSession: (sessionName: string) => Promise<boolean>
+	processAlive: (pid: number) => boolean
+	currentPid: number
+	log: (message: string, payload?: unknown) => void
+}
+
+async function buildRuntimeDeps(): Promise<SweepDeps> {
+	const [{ log }, { isInsideTmux }, { getTmuxPath }, { killTmuxSessionIfExists }] = await Promise.all([
+		import("../../logger"),
+		import("./environment"),
+		import("../../../tools/interactive-bash/tmux-path-resolver"),
+		import("./session-kill"),
+	])
+
+	return {
+		isInsideTmux,
+		getTmuxPath,
+		listCandidateSessions: listOmoAgentSessionsViaTmux,
+		killSession: killTmuxSessionIfExists,
+		processAlive: isProcessAlive,
+		currentPid: process.pid,
+		log,
+	}
+}
+
+export async function sweepStaleOmoAgentSessionsWith(deps: SweepDeps): Promise<number> {
+	if (!deps.isInsideTmux()) {
+		return 0
+	}
+
+	const tmux = await deps.getTmuxPath()
+	if (!tmux) {
+		return 0
+	}
+
+	const candidateSessions = await deps.listCandidateSessions(tmux)
+	let killedCount = 0
+
+	for (const sessionName of candidateSessions) {
+		const pidMatch = sessionName.match(STALE_SESSION_PATTERN)
+		if (!pidMatch) continue
+
+		const pid = Number.parseInt(pidMatch[1], 10)
+		if (!Number.isFinite(pid)) continue
+		if (pid === deps.currentPid) continue
+		if (deps.processAlive(pid)) continue
+
+		deps.log("[sweepStaleOmoAgentSessions] killing stale session", { sessionName, deadPid: pid })
+		const killed = await deps.killSession(sessionName)
+		if (killed) {
+			killedCount += 1
+		}
+	}
+
+	return killedCount
+}
+
+export async function sweepStaleOmoAgentSessions(): Promise<number> {
+	const deps = await buildRuntimeDeps()
+	return sweepStaleOmoAgentSessionsWith(deps)
+}