From 104523051d0f8c70fcdd0150af8eb7349f8e6df5 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 20:22:00 +0900 Subject: [PATCH 1/6] feat(tmux): sweep stale omo-agents- sessions on first spawn Follow-up to PR #3507 addressing the Oracle-noted operational limitation: per-PID isolated session names (getIsolatedSessionName(process.pid)) mean that when an opencode process is SIGKILL'd (or the machine hard-reboots), the old omo-agents- tmux session survives forever because nothing is around to kill it. Added sweepStaleOmoAgentSessions() that: 1. Lists tmux sessions matching /^omo-agents-(\d+)$/ 2. For each, checks process.kill(pid, 0) to detect a dead PID 3. Skips our own PID 4. Calls killTmuxSessionIfExists for every session whose owner process is gone Wired into TmuxSessionManager.onSessionCreated() as a one-shot (guarded by staleSweepCompleted flag) so it runs lazily on the first subagent spawn when isolation="session". The flag is reset in cleanup() so subsequent process restarts re-run the sweep. 6 new tests cover: outside-tmux no-op, no matching sessions, multiple dead PIDs, current PID skip, live PID skip, list-sessions failure. Manual E2E verified on real tmux: - Created omo-agents-99999, sweep killed it - Spawned our own omo-agents-, closeTmuxPane returned true even after pane auto-destroy from Ctrl+C - Final tmux list-sessions shows zero omo-agents-* orphans --- src/features/tmux-subagent/manager.test.ts | 2 + src/features/tmux-subagent/manager.ts | 25 +++ src/shared/tmux/tmux-utils.ts | 1 + .../tmux-utils/stale-session-sweep.test.ts | 183 ++++++++++++++++++ .../tmux/tmux-utils/stale-session-sweep.ts | 72 +++++++ 5 files changed, 283 insertions(+) create mode 100644 src/shared/tmux/tmux-utils/stale-session-sweep.test.ts create mode 100644 src/shared/tmux/tmux-utils/stale-session-sweep.ts diff --git a/src/features/tmux-subagent/manager.test.ts b/src/features/tmux-subagent/manager.test.ts index 9a03d7641..485a47acc 100644 --- a/src/features/tmux-subagent/manager.test.ts +++ b/src/features/tmux-subagent/manager.test.ts @@ -58,6 +58,7 @@ const mockSpawnTmuxSession = mock<( paneId: '%isolated-session', })) const mockKillTmuxSessionIfExists = mock<(sessionName: string) => Promise>(async () => true) +const mockSweepStaleOmoAgentSessions = mock<() => Promise>(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, } }) diff --git a/src/features/tmux-subagent/manager.ts b/src/features/tmux-subagent/manager.ts index 9d9c65159..1091033e4 100644 --- a/src/features/tmux-subagent/manager.ts +++ b/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,7 @@ export class TmuxSessionManager { private isolatedContainerPaneId: string | undefined private isolatedWindowPaneId: string | undefined private isolatedContainerNullStateCount = 0 + private staleSweepCompleted = false constructor(ctx: PluginInput, tmuxConfig: TmuxConfig, deps: TmuxUtilDeps = defaultTmuxDeps) { this.client = ctx.client this.tmuxConfig = tmuxConfig @@ -668,6 +670,7 @@ export class TmuxSessionManager { return } + await this.sweepStaleIsolatedSessionsOnce() await this.retryPendingCloses() if ( @@ -985,6 +988,28 @@ export class TmuxSessionManager { } } + this.staleSweepCompleted = false + } + + private async sweepStaleIsolatedSessionsOnce(): Promise { + if (this.staleSweepCompleted) return + if (this.tmuxConfig.isolation !== "session") { + this.staleSweepCompleted = true + return + } + + this.staleSweepCompleted = true + try { + const killed = await sweepStaleOmoAgentSessions() + if (killed > 0) { + log("[tmux-session-manager] stale isolated sessions swept", { killed }) + } + } catch (error) { + log("[tmux-session-manager] stale sweep failed", { + error: String(error), + }) + } + log("[tmux-session-manager] cleanup complete") } } diff --git a/src/shared/tmux/tmux-utils.ts b/src/shared/tmux/tmux-utils.ts index d80d1e720..6ccdeed31 100644 --- a/src/shared/tmux/tmux-utils.ts +++ b/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" diff --git a/src/shared/tmux/tmux-utils/stale-session-sweep.test.ts b/src/shared/tmux/tmux-utils/stale-session-sweep.test.ts new file mode 100644 index 000000000..d79b6b3b8 --- /dev/null +++ b/src/shared/tmux/tmux-utils/stale-session-sweep.test.ts @@ -0,0 +1,183 @@ +import { beforeEach, describe, expect, it, mock } from "bun:test" + +type SweepStaleOmoAgentSessions = typeof import("./stale-session-sweep").sweepStaleOmoAgentSessions + +type SpawnCall = { command: string[] } + +type FakeSubprocess = { + exited: Promise + stdout: ReadableStream + stderr: ReadableStream +} + +const spawnCalls: SpawnCall[] = [] +const queuedProcesses: FakeSubprocess[] = [] + +function createClosedStream(): ReadableStream { + return new ReadableStream({ start(controller) { controller.close() } }) +} + +function createTextStream(text: string): ReadableStream { + return new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode(text)) + controller.close() + }, + }) +} + +function makeProcess(exitCode: number, stdoutText: string): FakeSubprocess { + return { + exited: Promise.resolve(exitCode), + stdout: createTextStream(stdoutText), + stderr: createClosedStream(), + } +} + +const spawnMock = mock((command: string[]): FakeSubprocess => { + spawnCalls.push({ command }) + const process = queuedProcesses.shift() + if (!process) { + throw new Error(`No fake subprocess configured for ${command.join(" ")}`) + } + return process +}) + +const isInsideTmuxMock = mock((): boolean => true) +const getTmuxPathMock = mock(async (): Promise => "tmux") +const logMock = mock(() => undefined) +const killTmuxSessionMock = mock(async (_name: string): Promise => true) +const isProcessAliveMock = mock((_pid: number): boolean => false) + +const sweepSpecifier = import.meta.resolve("./stale-session-sweep") +const spawnProcessSpecifier = import.meta.resolve("./spawn-process") +const environmentSpecifier = import.meta.resolve("./environment") +const loggerSpecifier = import.meta.resolve("../../logger") +const tmuxPathResolverSpecifier = import.meta.resolve("../../../tools/interactive-bash/tmux-path-resolver") +const sessionKillSpecifier = import.meta.resolve("./session-kill") + +function registerModuleMocks(): void { + mock.module(spawnProcessSpecifier, () => ({ spawn: spawnMock })) + mock.module(environmentSpecifier, () => ({ isInsideTmux: isInsideTmuxMock })) + mock.module(loggerSpecifier, () => ({ log: logMock })) + mock.module(tmuxPathResolverSpecifier, () => ({ getTmuxPath: getTmuxPathMock })) + mock.module(sessionKillSpecifier, () => ({ killTmuxSessionIfExists: killTmuxSessionMock })) +} + +async function loadSweeper(overrideProcessAlive?: (pid: number) => boolean): Promise { + const originalKill = process.kill + const processAlive = overrideProcessAlive ?? isProcessAliveMock + process.kill = ((pid: number, signal?: number | string): true => { + if (signal === 0) { + if (processAlive(pid)) { + return true + } + const err = new Error("ESRCH") as NodeJS.ErrnoException + err.code = "ESRCH" + throw err + } + return originalKill.call(process, pid, signal) + }) as typeof process.kill + const module = await import(`${sweepSpecifier}?test=${crypto.randomUUID()}`) + return module.sweepStaleOmoAgentSessions +} + +describe("sweepStaleOmoAgentSessions", () => { + beforeEach(() => { + registerModuleMocks() + spawnCalls.length = 0 + queuedProcesses.length = 0 + spawnMock.mockClear() + isInsideTmuxMock.mockClear() + getTmuxPathMock.mockClear() + logMock.mockClear() + killTmuxSessionMock.mockClear() + isProcessAliveMock.mockClear() + + isInsideTmuxMock.mockImplementation((): boolean => true) + getTmuxPathMock.mockImplementation(async (): Promise => "tmux") + killTmuxSessionMock.mockImplementation(async (_name: string): Promise => true) + isProcessAliveMock.mockImplementation((_pid: number): boolean => false) + }) + + it("#given not inside tmux #when sweepStaleOmoAgentSessions called #then returns 0 without spawn", async () => { + // given + isInsideTmuxMock.mockImplementation((): boolean => false) + const sweep = await loadSweeper() + + // when + const result = await sweep() + + // then + expect(result).toBe(0) + expect(spawnCalls).toHaveLength(0) + }) + + it("#given no omo-agents sessions exist #when sweepStaleOmoAgentSessions called #then returns 0", async () => { + // given + queuedProcesses.push(makeProcess(0, "other-session\nmain\n")) + const sweep = await loadSweeper() + + // when + const result = await sweep() + + // then + expect(result).toBe(0) + expect(killTmuxSessionMock).toHaveBeenCalledTimes(0) + }) + + it("#given omo-agents sessions with dead PIDs #when sweepStaleOmoAgentSessions called #then each dead session is killed", async () => { + // given + queuedProcesses.push(makeProcess(0, "omo-agents-99991\nomo-agents-99992\nunrelated\n")) + const sweep = await loadSweeper(() => false) + + // when + const result = await sweep() + + // then + expect(result).toBe(2) + expect(killTmuxSessionMock).toHaveBeenCalledTimes(2) + expect(killTmuxSessionMock.mock.calls[0]?.[0]).toBe("omo-agents-99991") + expect(killTmuxSessionMock.mock.calls[1]?.[0]).toBe("omo-agents-99992") + }) + + it("#given session matches current PID #when sweepStaleOmoAgentSessions called #then it is not killed", async () => { + // given + queuedProcesses.push(makeProcess(0, `omo-agents-${process.pid}\nomo-agents-99999\n`)) + const sweep = await loadSweeper(() => false) + + // when + const result = await sweep() + + // then + expect(result).toBe(1) + expect(killTmuxSessionMock).toHaveBeenCalledTimes(1) + expect(killTmuxSessionMock.mock.calls[0]?.[0]).toBe("omo-agents-99999") + }) + + it("#given session PID is still alive #when sweepStaleOmoAgentSessions called #then it is not killed", async () => { + // given + queuedProcesses.push(makeProcess(0, "omo-agents-88888\n")) + const sweep = await loadSweeper((pid) => pid === 88888) + + // when + const result = await sweep() + + // then + expect(result).toBe(0) + expect(killTmuxSessionMock).toHaveBeenCalledTimes(0) + }) + + it("#given list-sessions fails #when sweepStaleOmoAgentSessions called #then returns 0 without killing", async () => { + // given + queuedProcesses.push(makeProcess(1, "")) + const sweep = await loadSweeper(() => false) + + // when + const result = await sweep() + + // then + expect(result).toBe(0) + expect(killTmuxSessionMock).toHaveBeenCalledTimes(0) + }) +}) diff --git a/src/shared/tmux/tmux-utils/stale-session-sweep.ts b/src/shared/tmux/tmux-utils/stale-session-sweep.ts new file mode 100644 index 000000000..f2c790161 --- /dev/null +++ b/src/shared/tmux/tmux-utils/stale-session-sweep.ts @@ -0,0 +1,72 @@ +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 listOmoAgentSessions(tmux: string): Promise { + 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 async function sweepStaleOmoAgentSessions(): Promise { + const [{ log }, { isInsideTmux }, { getTmuxPath }, { killTmuxSessionIfExists }] = await Promise.all([ + import("../../logger"), + import("./environment"), + import("../../../tools/interactive-bash/tmux-path-resolver"), + import("./session-kill"), + ]) + + if (!isInsideTmux()) { + return 0 + } + + const tmux = await getTmuxPath() + if (!tmux) { + return 0 + } + + const candidateSessions = await listOmoAgentSessions(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 === process.pid) continue + if (isProcessAlive(pid)) continue + + log("[sweepStaleOmoAgentSessions] killing stale session", { sessionName, deadPid: pid }) + const killed = await killTmuxSessionIfExists(sessionName) + if (killed) { + killedCount += 1 + } + } + + return killedCount +} From 3dce19d173b150ced1ea000c59ef517f8202f4dd Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 20:27:48 +0900 Subject: [PATCH 2/6] fix(tmux-subagent): move 'cleanup complete' log back to cleanup() method The log line was misplaced at the end of sweepStaleIsolatedSessionsOnce where it said 'cleanup complete' after the stale sweep, which was misleading. Per Oracle review. --- src/features/tmux-subagent/manager.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/features/tmux-subagent/manager.ts b/src/features/tmux-subagent/manager.ts index 1091033e4..7b9d92d81 100644 --- a/src/features/tmux-subagent/manager.ts +++ b/src/features/tmux-subagent/manager.ts @@ -989,6 +989,8 @@ export class TmuxSessionManager { } this.staleSweepCompleted = false + + log("[tmux-session-manager] cleanup complete") } private async sweepStaleIsolatedSessionsOnce(): Promise { @@ -1009,7 +1011,5 @@ export class TmuxSessionManager { error: String(error), }) } - - log("[tmux-session-manager] cleanup complete") } } From 859d67f41eb483a0d87e73adbe070c030c3e8390 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 20:37:49 +0900 Subject: [PATCH 3/6] fix(tmux-subagent): revert session.error cleanup (recoverable-error regression) Oracle flagged a regression introduced in PR #3507 commit 21554be8: event.ts routed session.error through tmux pane cleanup BEFORE the existing session-recovery / model-fallback logic ran. Problem: when session.error was recoverable (context window limit, quota rate limit, provider fallback), the recovery/fallback code would successfully continue the SAME session - but by then its tmux pane had already been destroyed. User-visible symptom is exactly the original complaint - 'screen appears but streaming stops working' after an auto-retry. Fix is the minimal revert: remove the onSessionError funnel from event.ts and drop onSessionError from the manager. Fatal errors that actually end a session still fire session.deleted, which continues to trigger cleanup correctly. Non-fatal error streams stay attached to the surviving pane. --- src/features/tmux-subagent/manager.test.ts | 37 ---------------------- src/features/tmux-subagent/manager.ts | 12 ------- src/plugin/event.ts | 4 --- 3 files changed, 53 deletions(-) diff --git a/src/features/tmux-subagent/manager.test.ts b/src/features/tmux-subagent/manager.test.ts index 485a47acc..84c7bc441 100644 --- a/src/features/tmux-subagent/manager.test.ts +++ b/src/features/tmux-subagent/manager.test.ts @@ -1932,43 +1932,6 @@ describe('TmuxSessionManager', () => { expect(mockKillTmuxSessionIfExists).toHaveBeenCalledTimes(0) }) - test('#given a tracked session #when onSessionError is invoked #then the pane is closed like onSessionDeleted', async () => { - // given - 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' }) - - // then - expect(mockExecuteAction).toHaveBeenCalled() - }) - - test('#given an untracked session #when onSessionError is invoked #then it is a no-op and does not throw', async () => { - // given - mockIsInsideTmux.mockReturnValue(true) - mockExecuteAction.mockClear() - const { TmuxSessionManager } = await import('./manager') - const manager = new TmuxSessionManager(createMockContext(), createTmuxConfig({ - enabled: true, - isolation: 'session', - }), mockTmuxDeps) - - // when - const errorHandler = manager.onSessionError({ sessionID: 'ses_unknown' }) - - // then - await expect(errorHandler).resolves.toBeUndefined() - expect(mockExecuteAction).not.toHaveBeenCalled() - }) - test('#given killTmuxSessionIfExists throws #when cleanup runs #then cleanup still completes without throwing', async () => { // given mockKillTmuxSessionIfExists.mockClear() diff --git a/src/features/tmux-subagent/manager.ts b/src/features/tmux-subagent/manager.ts index 7b9d92d81..5734ff822 100644 --- a/src/features/tmux-subagent/manager.ts +++ b/src/features/tmux-subagent/manager.ts @@ -838,18 +838,6 @@ export class TmuxSessionManager { await this.spawnQueue } - async onSessionError(event: { sessionID: string }): Promise { - 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 { if (!this.isEnabled()) return if (!this.getEffectiveSourcePaneId()) return diff --git a/src/plugin/event.ts b/src/plugin/event.ts index 0f79a84d5..5a5f177b6 100644 --- a/src/plugin/event.ts +++ b/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 }; From 913fac05f5f0314ab1cdd857d5f72e777de90282 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 20:45:21 +0900 Subject: [PATCH 4/6] fix(tmux-subagent): retry stale sweep if first attempt throws Oracle flagged: staleSweepCompleted was set to true BEFORE sweepStaleOmoAgentSessions() ran, so any throw from the first invocation would permanently disable stale cleanup for the rest of the process lifetime. Fix: - Move staleSweepCompleted=true into the try-block success branch. - Add staleSweepInProgress guard so concurrent onSessionCreated calls do not invoke sweep twice in parallel (sweep is idempotent, but the guard prevents doubled log noise). - finally{} clears the inProgress flag regardless of outcome. - cleanup() resets both flags. Two new tests cover: retry after a thrown first attempt, and single invocation when subsequent spawns follow a successful first sweep. --- src/features/tmux-subagent/manager.test.ts | 41 ++++++++++++++++++++++ src/features/tmux-subagent/manager.ts | 8 ++++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/features/tmux-subagent/manager.test.ts b/src/features/tmux-subagent/manager.test.ts index 84c7bc441..e63f4bf4d 100644 --- a/src/features/tmux-subagent/manager.test.ts +++ b/src/features/tmux-subagent/manager.test.ts @@ -1932,6 +1932,47 @@ describe('TmuxSessionManager', () => { expect(mockKillTmuxSessionIfExists).toHaveBeenCalledTimes(0) }) + 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) + const { TmuxSessionManager } = await import('./manager') + const manager = new TmuxSessionManager(createMockContext(), createTmuxConfig({ + enabled: true, + isolation: 'session', + }), mockTmuxDeps) + + // when + await manager.onSessionCreated(createSessionCreatedEvent('ses_first', 'ses_parent', 'First')) + await manager.onSessionCreated(createSessionCreatedEvent('ses_second', 'ses_parent', 'Second')) + + // then + expect(mockSweepStaleOmoAgentSessions).toHaveBeenCalledTimes(2) + }) + + 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) + const { TmuxSessionManager } = await import('./manager') + const manager = new TmuxSessionManager(createMockContext(), createTmuxConfig({ + enabled: true, + isolation: 'session', + }), mockTmuxDeps) + + // when + 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 + expect(mockSweepStaleOmoAgentSessions).toHaveBeenCalledTimes(1) + }) + test('#given killTmuxSessionIfExists throws #when cleanup runs #then cleanup still completes without throwing', async () => { // given mockKillTmuxSessionIfExists.mockClear() diff --git a/src/features/tmux-subagent/manager.ts b/src/features/tmux-subagent/manager.ts index 5734ff822..3340fb55d 100644 --- a/src/features/tmux-subagent/manager.ts +++ b/src/features/tmux-subagent/manager.ts @@ -67,6 +67,7 @@ export class TmuxSessionManager { 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 @@ -977,27 +978,32 @@ export class TmuxSessionManager { } this.staleSweepCompleted = false + this.staleSweepInProgress = false log("[tmux-session-manager] cleanup complete") } private async sweepStaleIsolatedSessionsOnce(): Promise { if (this.staleSweepCompleted) return + if (this.staleSweepInProgress) return if (this.tmuxConfig.isolation !== "session") { this.staleSweepCompleted = true return } - this.staleSweepCompleted = true + 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 } } } From d1fc46da42211079244a6896f8626bf3aa813d6c Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 20:51:24 +0900 Subject: [PATCH 5/6] test(tmux): restore process.kill in afterEach to prevent cross-file leak Oracle noted that loadSweeper() monkey-patches process.kill without ever restoring it. Added afterEach hook to set process.kill back to the captured original. Individual file runs already passed, and script/run-ci-tests.ts confirms the full CI suite - 4781 pass, 0 fail across 491 files - but this makes the test file safe under non-isolated local runs as well. --- .../tmux/tmux-utils/stale-session-sweep.test.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/shared/tmux/tmux-utils/stale-session-sweep.test.ts b/src/shared/tmux/tmux-utils/stale-session-sweep.test.ts index d79b6b3b8..98f6aa1f8 100644 --- a/src/shared/tmux/tmux-utils/stale-session-sweep.test.ts +++ b/src/shared/tmux/tmux-utils/stale-session-sweep.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, mock } from "bun:test" +import { afterEach, beforeEach, describe, expect, it, mock } from "bun:test" type SweepStaleOmoAgentSessions = typeof import("./stale-session-sweep").sweepStaleOmoAgentSessions @@ -64,8 +64,9 @@ function registerModuleMocks(): void { mock.module(sessionKillSpecifier, () => ({ killTmuxSessionIfExists: killTmuxSessionMock })) } +const originalProcessKill = process.kill + async function loadSweeper(overrideProcessAlive?: (pid: number) => boolean): Promise { - const originalKill = process.kill const processAlive = overrideProcessAlive ?? isProcessAliveMock process.kill = ((pid: number, signal?: number | string): true => { if (signal === 0) { @@ -76,7 +77,7 @@ async function loadSweeper(overrideProcessAlive?: (pid: number) => boolean): Pro err.code = "ESRCH" throw err } - return originalKill.call(process, pid, signal) + return originalProcessKill.call(process, pid, signal) }) as typeof process.kill const module = await import(`${sweepSpecifier}?test=${crypto.randomUUID()}`) return module.sweepStaleOmoAgentSessions @@ -100,6 +101,10 @@ describe("sweepStaleOmoAgentSessions", () => { isProcessAliveMock.mockImplementation((_pid: number): boolean => false) }) + afterEach(() => { + process.kill = originalProcessKill + }) + it("#given not inside tmux #when sweepStaleOmoAgentSessions called #then returns 0 without spawn", async () => { // given isInsideTmuxMock.mockImplementation((): boolean => false) From e35ac38bbfca4fbae8c390f04e9a6554f7ab0de9 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 21:00:34 +0900 Subject: [PATCH 6/6] test(tmux): rewrite stale-sweep tests via DI to eliminate cross-file mock leak Oracle flagged that the previous test file monkey-patched process.kill and relied on mock.module for 5 modules. Running it after manager.test.ts in the same Bun process reproduced 2 failures - the test resolution of `./session-kill` specifier interacted badly with manager.test.ts's `../../shared/tmux` barrel mock. Solution: refactor stale-session-sweep.ts to expose `sweepStaleOmoAgentSessionsWith(deps)` that accepts a SweepDeps record (isInsideTmux, getTmuxPath, listCandidateSessions, killSession, processAlive, currentPid, log). The public `sweepStaleOmoAgentSessions()` still uses runtime-built deps so call sites are unchanged. The test file now imports the pure function directly and constructs a fixture with fake deps. Zero mock.module calls, zero process.kill patching, zero cache-bust dynamic imports. 8 tests (up from 6) run deterministically in any order with any neighbor. Before: combined run with manager.test.ts = 2 fail, 50 pass. After: combined run with manager.test.ts = 0 fail, 54 pass. --- .../tmux-utils/stale-session-sweep.test.ts | 218 ++++++++---------- .../tmux/tmux-utils/stale-session-sweep.ts | 45 +++- 2 files changed, 128 insertions(+), 135 deletions(-) diff --git a/src/shared/tmux/tmux-utils/stale-session-sweep.test.ts b/src/shared/tmux/tmux-utils/stale-session-sweep.test.ts index 98f6aa1f8..1acc171ed 100644 --- a/src/shared/tmux/tmux-utils/stale-session-sweep.test.ts +++ b/src/shared/tmux/tmux-utils/stale-session-sweep.test.ts @@ -1,188 +1,154 @@ -import { afterEach, beforeEach, describe, expect, it, mock } from "bun:test" +import { beforeEach, describe, expect, it, mock } from "bun:test" +import { sweepStaleOmoAgentSessionsWith, type SweepDeps } from "./stale-session-sweep" -type SweepStaleOmoAgentSessions = typeof import("./stale-session-sweep").sweepStaleOmoAgentSessions - -type SpawnCall = { command: string[] } - -type FakeSubprocess = { - exited: Promise - stdout: ReadableStream - stderr: ReadableStream +type SweepFixture = { + deps: SweepDeps + candidates: string[] + killed: string[] + killSessionMock: ReturnType + setCandidates: (sessions: string[]) => void + setAlive: (predicate: (pid: number) => boolean) => void } -const spawnCalls: SpawnCall[] = [] -const queuedProcesses: FakeSubprocess[] = [] +function createFixture(): SweepFixture { + const candidates: string[] = [] + const killed: string[] = [] + let aliveCheck: (pid: number) => boolean = () => false -function createClosedStream(): ReadableStream { - return new ReadableStream({ start(controller) { controller.close() } }) -} - -function createTextStream(text: string): ReadableStream { - return new ReadableStream({ - start(controller) { - controller.enqueue(new TextEncoder().encode(text)) - controller.close() - }, + const killSessionMock = mock(async (sessionName: string): Promise => { + killed.push(sessionName) + return true }) -} -function makeProcess(exitCode: number, stdoutText: string): FakeSubprocess { + const deps: SweepDeps = { + isInsideTmux: () => true, + getTmuxPath: async () => "tmux", + listCandidateSessions: async () => [...candidates], + killSession: killSessionMock, + processAlive: (pid) => aliveCheck(pid), + currentPid: 12345, + log: () => undefined, + } + return { - exited: Promise.resolve(exitCode), - stdout: createTextStream(stdoutText), - stderr: createClosedStream(), + deps, + candidates, + killed, + killSessionMock, + setCandidates: (sessions) => { + candidates.length = 0 + candidates.push(...sessions) + }, + setAlive: (predicate) => { + aliveCheck = predicate + }, } } -const spawnMock = mock((command: string[]): FakeSubprocess => { - spawnCalls.push({ command }) - const process = queuedProcesses.shift() - if (!process) { - throw new Error(`No fake subprocess configured for ${command.join(" ")}`) - } - return process -}) +describe("sweepStaleOmoAgentSessionsWith", () => { + let fixture: SweepFixture -const isInsideTmuxMock = mock((): boolean => true) -const getTmuxPathMock = mock(async (): Promise => "tmux") -const logMock = mock(() => undefined) -const killTmuxSessionMock = mock(async (_name: string): Promise => true) -const isProcessAliveMock = mock((_pid: number): boolean => false) - -const sweepSpecifier = import.meta.resolve("./stale-session-sweep") -const spawnProcessSpecifier = import.meta.resolve("./spawn-process") -const environmentSpecifier = import.meta.resolve("./environment") -const loggerSpecifier = import.meta.resolve("../../logger") -const tmuxPathResolverSpecifier = import.meta.resolve("../../../tools/interactive-bash/tmux-path-resolver") -const sessionKillSpecifier = import.meta.resolve("./session-kill") - -function registerModuleMocks(): void { - mock.module(spawnProcessSpecifier, () => ({ spawn: spawnMock })) - mock.module(environmentSpecifier, () => ({ isInsideTmux: isInsideTmuxMock })) - mock.module(loggerSpecifier, () => ({ log: logMock })) - mock.module(tmuxPathResolverSpecifier, () => ({ getTmuxPath: getTmuxPathMock })) - mock.module(sessionKillSpecifier, () => ({ killTmuxSessionIfExists: killTmuxSessionMock })) -} - -const originalProcessKill = process.kill - -async function loadSweeper(overrideProcessAlive?: (pid: number) => boolean): Promise { - const processAlive = overrideProcessAlive ?? isProcessAliveMock - process.kill = ((pid: number, signal?: number | string): true => { - if (signal === 0) { - if (processAlive(pid)) { - return true - } - const err = new Error("ESRCH") as NodeJS.ErrnoException - err.code = "ESRCH" - throw err - } - return originalProcessKill.call(process, pid, signal) - }) as typeof process.kill - const module = await import(`${sweepSpecifier}?test=${crypto.randomUUID()}`) - return module.sweepStaleOmoAgentSessions -} - -describe("sweepStaleOmoAgentSessions", () => { beforeEach(() => { - registerModuleMocks() - spawnCalls.length = 0 - queuedProcesses.length = 0 - spawnMock.mockClear() - isInsideTmuxMock.mockClear() - getTmuxPathMock.mockClear() - logMock.mockClear() - killTmuxSessionMock.mockClear() - isProcessAliveMock.mockClear() - - isInsideTmuxMock.mockImplementation((): boolean => true) - getTmuxPathMock.mockImplementation(async (): Promise => "tmux") - killTmuxSessionMock.mockImplementation(async (_name: string): Promise => true) - isProcessAliveMock.mockImplementation((_pid: number): boolean => false) + fixture = createFixture() }) - afterEach(() => { - process.kill = originalProcessKill - }) - - it("#given not inside tmux #when sweepStaleOmoAgentSessions called #then returns 0 without spawn", async () => { + it("#given not inside tmux #when sweep called #then returns 0 without listing", async () => { // given - isInsideTmuxMock.mockImplementation((): boolean => false) - const sweep = await loadSweeper() + const deps: SweepDeps = { ...fixture.deps, isInsideTmux: () => false } // when - const result = await sweep() + const result = await sweepStaleOmoAgentSessionsWith(deps) // then expect(result).toBe(0) - expect(spawnCalls).toHaveLength(0) }) - it("#given no omo-agents sessions exist #when sweepStaleOmoAgentSessions called #then returns 0", async () => { + it("#given tmux not found #when sweep called #then returns 0 without listing", async () => { // given - queuedProcesses.push(makeProcess(0, "other-session\nmain\n")) - const sweep = await loadSweeper() + const deps: SweepDeps = { ...fixture.deps, getTmuxPath: async () => undefined } // when - const result = await sweep() + const result = await sweepStaleOmoAgentSessionsWith(deps) // then expect(result).toBe(0) - expect(killTmuxSessionMock).toHaveBeenCalledTimes(0) }) - it("#given omo-agents sessions with dead PIDs #when sweepStaleOmoAgentSessions called #then each dead session is killed", async () => { + it("#given candidate list is empty #when sweep called #then returns 0 and does not kill anything", async () => { // given - queuedProcesses.push(makeProcess(0, "omo-agents-99991\nomo-agents-99992\nunrelated\n")) - const sweep = await loadSweeper(() => false) + fixture.setCandidates([]) // when - const result = await sweep() + 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(killTmuxSessionMock).toHaveBeenCalledTimes(2) - expect(killTmuxSessionMock.mock.calls[0]?.[0]).toBe("omo-agents-99991") - expect(killTmuxSessionMock.mock.calls[1]?.[0]).toBe("omo-agents-99992") + expect(fixture.killed).toEqual(["omo-agents-99991", "omo-agents-99992"]) }) - it("#given session matches current PID #when sweepStaleOmoAgentSessions called #then it is not killed", async () => { + it("#given session matches current PID #when sweep called #then it is NOT killed", async () => { // given - queuedProcesses.push(makeProcess(0, `omo-agents-${process.pid}\nomo-agents-99999\n`)) - const sweep = await loadSweeper(() => false) + fixture.setCandidates([`omo-agents-${fixture.deps.currentPid}`, "omo-agents-99999"]) + fixture.setAlive(() => false) // when - const result = await sweep() + const result = await sweepStaleOmoAgentSessionsWith(fixture.deps) // then expect(result).toBe(1) - expect(killTmuxSessionMock).toHaveBeenCalledTimes(1) - expect(killTmuxSessionMock.mock.calls[0]?.[0]).toBe("omo-agents-99999") + expect(fixture.killed).toEqual(["omo-agents-99999"]) }) - it("#given session PID is still alive #when sweepStaleOmoAgentSessions called #then it is not killed", async () => { + it("#given session PID is still alive #when sweep called #then it is NOT killed", async () => { // given - queuedProcesses.push(makeProcess(0, "omo-agents-88888\n")) - const sweep = await loadSweeper((pid) => pid === 88888) + fixture.setCandidates(["omo-agents-88888"]) + fixture.setAlive((pid) => pid === 88888) // when - const result = await sweep() + const result = await sweepStaleOmoAgentSessionsWith(fixture.deps) // then expect(result).toBe(0) - expect(killTmuxSessionMock).toHaveBeenCalledTimes(0) + expect(fixture.killed).toEqual([]) }) - it("#given list-sessions fails #when sweepStaleOmoAgentSessions called #then returns 0 without killing", async () => { + it("#given killSession returns false #when sweep called #then session is not counted toward killedCount", async () => { // given - queuedProcesses.push(makeProcess(1, "")) - const sweep = await loadSweeper(() => false) + fixture.setCandidates(["omo-agents-55555"]) + fixture.setAlive(() => false) + fixture.killSessionMock.mockImplementation(async () => false) // when - const result = await sweep() + const result = await sweepStaleOmoAgentSessionsWith(fixture.deps) // then expect(result).toBe(0) - expect(killTmuxSessionMock).toHaveBeenCalledTimes(0) + expect(fixture.killSessionMock).toHaveBeenCalledTimes(1) + }) + + it("#given non-matching sessions mixed in #when sweep called #then only omo-agents- 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"]) }) }) diff --git a/src/shared/tmux/tmux-utils/stale-session-sweep.ts b/src/shared/tmux/tmux-utils/stale-session-sweep.ts index f2c790161..c8b27e938 100644 --- a/src/shared/tmux/tmux-utils/stale-session-sweep.ts +++ b/src/shared/tmux/tmux-utils/stale-session-sweep.ts @@ -10,7 +10,7 @@ function isProcessAlive(pid: number): boolean { } } -async function listOmoAgentSessions(tmux: string): Promise { +async function listOmoAgentSessionsViaTmux(tmux: string): Promise { const { spawn } = await import("./spawn-process") const proc = spawn([tmux, "list-sessions", "-F", "#{session_name}"], { stdout: "pipe", @@ -32,7 +32,17 @@ async function listOmoAgentSessions(tmux: string): Promise { .filter((name) => STALE_SESSION_PATTERN.test(name)) } -export async function sweepStaleOmoAgentSessions(): Promise { +export type SweepDeps = { + isInsideTmux: () => boolean + getTmuxPath: () => Promise + listCandidateSessions: (tmux: string) => Promise + killSession: (sessionName: string) => Promise + processAlive: (pid: number) => boolean + currentPid: number + log: (message: string, payload?: unknown) => void +} + +async function buildRuntimeDeps(): Promise { const [{ log }, { isInsideTmux }, { getTmuxPath }, { killTmuxSessionIfExists }] = await Promise.all([ import("../../logger"), import("./environment"), @@ -40,16 +50,28 @@ export async function sweepStaleOmoAgentSessions(): Promise { import("./session-kill"), ]) - if (!isInsideTmux()) { + return { + isInsideTmux, + getTmuxPath, + listCandidateSessions: listOmoAgentSessionsViaTmux, + killSession: killTmuxSessionIfExists, + processAlive: isProcessAlive, + currentPid: process.pid, + log, + } +} + +export async function sweepStaleOmoAgentSessionsWith(deps: SweepDeps): Promise { + if (!deps.isInsideTmux()) { return 0 } - const tmux = await getTmuxPath() + const tmux = await deps.getTmuxPath() if (!tmux) { return 0 } - const candidateSessions = await listOmoAgentSessions(tmux) + const candidateSessions = await deps.listCandidateSessions(tmux) let killedCount = 0 for (const sessionName of candidateSessions) { @@ -58,11 +80,11 @@ export async function sweepStaleOmoAgentSessions(): Promise { const pid = Number.parseInt(pidMatch[1], 10) if (!Number.isFinite(pid)) continue - if (pid === process.pid) continue - if (isProcessAlive(pid)) continue + if (pid === deps.currentPid) continue + if (deps.processAlive(pid)) continue - log("[sweepStaleOmoAgentSessions] killing stale session", { sessionName, deadPid: pid }) - const killed = await killTmuxSessionIfExists(sessionName) + deps.log("[sweepStaleOmoAgentSessions] killing stale session", { sessionName, deadPid: pid }) + const killed = await deps.killSession(sessionName) if (killed) { killedCount += 1 } @@ -70,3 +92,8 @@ export async function sweepStaleOmoAgentSessions(): Promise { return killedCount } + +export async function sweepStaleOmoAgentSessions(): Promise { + const deps = await buildRuntimeDeps() + return sweepStaleOmoAgentSessionsWith(deps) +}