diff --git a/src/features/tmux-subagent/manager.test.ts b/src/features/tmux-subagent/manager.test.ts index 9a03d7641..e63f4bf4d 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, } }) @@ -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 () => { diff --git a/src/features/tmux-subagent/manager.ts b/src/features/tmux-subagent/manager.ts index 9d9c65159..3340fb55d 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,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 { - 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 @@ -985,6 +977,33 @@ 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.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 + } + } } 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 }; 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..1acc171ed --- /dev/null +++ b/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 + 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 => { + 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- 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 new file mode 100644 index 000000000..c8b27e938 --- /dev/null +++ b/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 { + 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 + 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"), + 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 { + 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 { + const deps = await buildRuntimeDeps() + return sweepStaleOmoAgentSessionsWith(deps) +}