diff --git a/src/features/background-agent/process-cleanup.test-helpers.ts b/src/features/background-agent/process-cleanup.test-helpers.ts new file mode 100644 index 000000000..c0a3dfa2d --- /dev/null +++ b/src/features/background-agent/process-cleanup.test-helpers.ts @@ -0,0 +1,27 @@ +type ProcessCleanupEvent = + | NodeJS.Signals + | "beforeExit" + | "exit" + | "uncaughtException" + | "unhandledRejection" + +export function getNewListener( + signal: ProcessCleanupEvent, + existingListeners: Function[], +): () => void { + const listener = process + .listeners(signal) + .find((registeredListener) => !existingListeners.includes(registeredListener)) + + if (typeof listener !== "function") { + throw new Error(`Expected a ${signal} listener to be registered`) + } + + return listener +} + +export async function flushMicrotasks(): Promise { + for (let iteration = 0; iteration < 10; iteration += 1) { + await Promise.resolve() + } +} diff --git a/src/features/background-agent/process-cleanup.test.ts b/src/features/background-agent/process-cleanup.test.ts index 7d01aaa21..a9ce35435 100644 --- a/src/features/background-agent/process-cleanup.test.ts +++ b/src/features/background-agent/process-cleanup.test.ts @@ -1,3 +1,5 @@ +/// + import { afterEach, beforeEach, describe, expect, mock, spyOn, test } from "bun:test" import { @@ -5,42 +7,17 @@ import { registerManagerForCleanup, unregisterManagerForCleanup, } from "./process-cleanup" +import { flushMicrotasks, getNewListener } from "./process-cleanup.test-helpers" type CleanupManager = { shutdown: () => void | Promise } -type ProcessCleanupEvent = NodeJS.Signals | "beforeExit" | "exit" - -function getNewListener( - signal: ProcessCleanupEvent, - existingListeners: Function[], -): () => void { - const listener = process - .listeners(signal) - .find((registeredListener) => !existingListeners.includes(registeredListener)) - - expect(listener).toBeDefined() - - if (typeof listener !== "function") { - throw new Error(`Expected a ${signal} listener to be registered`) - } - - return listener -} - -async function flushMicrotasks(): Promise { - for (let iteration = 0; iteration < 10; iteration += 1) { - await Promise.resolve() - } -} - describe("#given process cleanup registration", () => { const registeredManagers: CleanupManager[] = [] - const originalExitCode = process.exitCode beforeEach(() => { - process.exitCode = originalExitCode + process.exitCode = 0 registeredManagers.length = 0 _resetForTesting() }) @@ -50,7 +27,7 @@ describe("#given process cleanup registration", () => { unregisterManagerForCleanup(manager) } - process.exitCode = originalExitCode + process.exitCode = 0 _resetForTesting() }) @@ -92,13 +69,7 @@ describe("#given process cleanup registration", () => { test("#when cleanup finishes after SIGINT #then the fallback exit timer is cleared", async () => { const sigintListenersBefore = process.listeners("SIGINT") - const timeoutHandle = setTimeout(() => undefined, 0) - clearTimeout(timeoutHandle) - - const setTimeoutImplementation: typeof setTimeout = () => timeoutHandle - const setTimeoutSpy = spyOn(globalThis, "setTimeout").mockImplementation( - setTimeoutImplementation, - ) + const setTimeoutSpy = spyOn(globalThis, "setTimeout") const clearTimeoutSpy = spyOn(globalThis, "clearTimeout") try { @@ -117,11 +88,10 @@ describe("#given process cleanup registration", () => { await flushMicrotasks() expect(setTimeoutSpy).toHaveBeenCalledTimes(1) - expect(clearTimeoutSpy).toHaveBeenCalledWith(timeoutHandle) + expect(clearTimeoutSpy).toHaveBeenCalledTimes(1) } finally { setTimeoutSpy.mockRestore() clearTimeoutSpy.mockRestore() - clearTimeout(timeoutHandle) } }) }) @@ -163,6 +133,32 @@ describe("#given process cleanup registration", () => { expect(process.listeners("SIGINT")).toHaveLength(sigintListenersAfterFirstRegistration) }) + + test("#given two managers registered #when uncaughtException fires #then both shutdowns called", async () => { + const exitSpy = spyOn(process, "exit").mockImplementation((code?: number): never => { + throw new Error(`Unexpected process.exit(${String(code)})`) + }) + const shutdownOne = mock(() => {}) + const shutdownTwo = mock(() => {}) + const managerOne = { shutdown: shutdownOne } + const managerTwo = { shutdown: shutdownTwo } + registeredManagers.push(managerOne, managerTwo) + + try { + registerManagerForCleanup(managerOne) + registerManagerForCleanup(managerTwo) + + process.emit("uncaughtException", new Error("boom")) + await flushMicrotasks() + + expect(shutdownOne).toHaveBeenCalledTimes(1) + expect(shutdownTwo).toHaveBeenCalledTimes(1) + expect(process.exitCode).toBe(1) + expect(exitSpy).not.toHaveBeenCalled() + } finally { + exitSpy.mockRestore() + } + }) }) describe("#given cleanup managers are unregistered", () => { @@ -202,5 +198,88 @@ describe("#given process cleanup registration", () => { expect(remainingManagerShutdown).toHaveBeenCalledTimes(1) expect(removedManagerShutdown).not.toHaveBeenCalled() }) + + test("#given uncaughtException handler registered #when manager is unregistered via unregisterManagerForCleanup #then subsequent events do not invoke that manager", () => { + const uncaughtExceptionListenersBefore = process.listeners("uncaughtException") + const shutdown = mock(() => {}) + const manager = { shutdown } + registeredManagers.push(manager) + + registerManagerForCleanup(manager) + expect(process.listeners("uncaughtException")).toHaveLength( + uncaughtExceptionListenersBefore.length + 1, + ) + + unregisterManagerForCleanup(manager) + registeredManagers.length = 0 + process.emit("uncaughtException", new Error("boom")) + + expect(shutdown).not.toHaveBeenCalled() + }) + }) + + describe("#given uncaught exception and rejection cleanup", () => { + test("#given manager registered AND process emits uncaughtException #when event fires #then manager.shutdown() called AND process.exitCode set to 1", async () => { + const exitSpy = spyOn(process, "exit").mockImplementation((code?: number): never => { + throw new Error(`Unexpected process.exit(${String(code)})`) + }) + const shutdown = mock(() => {}) + const manager = { shutdown } + registeredManagers.push(manager) + + try { + registerManagerForCleanup(manager) + + process.emit("uncaughtException", new Error("boom")) + await flushMicrotasks() + + expect(shutdown).toHaveBeenCalledTimes(1) + expect(process.exitCode).toBe(1) + expect(exitSpy).not.toHaveBeenCalled() + } finally { + exitSpy.mockRestore() + } + }) + + test("#given manager registered AND process emits unhandledRejection #when event fires #then manager.shutdown() called AND process.exitCode set to 1", async () => { + const exitSpy = spyOn(process, "exit").mockImplementation((code?: number): never => { + throw new Error(`Unexpected process.exit(${String(code)})`) + }) + const shutdown = mock(() => {}) + const manager = { shutdown } + registeredManagers.push(manager) + + try { + registerManagerForCleanup(manager) + + process.emit("unhandledRejection", new Error("boom"), Promise.resolve()) + await flushMicrotasks() + + expect(shutdown).toHaveBeenCalledTimes(1) + expect(process.exitCode).toBe(1) + expect(exitSpy).not.toHaveBeenCalled() + } finally { + exitSpy.mockRestore() + } + }) + + test("#given _resetForTesting() called #when event fires #then no cleanup runs", () => { + const uncaughtExceptionListenersBefore = process.listeners("uncaughtException") + const shutdown = mock(() => {}) + const manager = { shutdown } + + registerManagerForCleanup(manager) + expect(process.listeners("uncaughtException")).toHaveLength( + uncaughtExceptionListenersBefore.length + 1, + ) + + _resetForTesting() + process.emit("uncaughtException", new Error("boom")) + + expect(shutdown).not.toHaveBeenCalled() + expect(process.listeners("uncaughtException")).toHaveLength( + uncaughtExceptionListenersBefore.length, + ) + }) }) }) diff --git a/src/features/background-agent/process-cleanup.ts b/src/features/background-agent/process-cleanup.ts index 29be1958e..20f8fab00 100644 --- a/src/features/background-agent/process-cleanup.ts +++ b/src/features/background-agent/process-cleanup.ts @@ -1,33 +1,51 @@ import { log } from "../../shared" -type ProcessCleanupEvent = NodeJS.Signals | "beforeExit" | "exit" +type ProcessCleanupSignal = NodeJS.Signals | "beforeExit" | "exit" +type ProcessCleanupErrorEvent = "uncaughtException" | "unhandledRejection" + +function scheduleForcedExit(cleanupResult: void | Promise, exitCode: number): void { + process.exitCode = exitCode + const exitTimeout = setTimeout(() => process.exit(), 6000) + void Promise.resolve(cleanupResult).finally(() => { + clearTimeout(exitTimeout) + }) +} function registerProcessSignal( - signal: ProcessCleanupEvent, + signal: ProcessCleanupSignal, handler: () => void | Promise, exitAfter: boolean ): () => void { const listener = () => { const cleanupResult = handler() if (exitAfter) { - process.exitCode = 0 - const exitTimeout = setTimeout(() => process.exit(), 6000) - void Promise.resolve(cleanupResult).finally(() => { - clearTimeout(exitTimeout) - }) + scheduleForcedExit(cleanupResult, 0) } } process.on(signal, listener) return listener } +function registerErrorEvent( + signal: ProcessCleanupErrorEvent, + handler: (error: unknown) => void | Promise +): (error: unknown) => void { + const listener = (error: unknown) => { + log(`[background-agent] ${signal} received during shutdown cleanup:`, error) + scheduleForcedExit(handler(error), 1) + } + process.on(signal, listener) + return listener +} + interface CleanupTarget { shutdown(): void | Promise } const cleanupManagers = new Set() let cleanupRegistered = false -const cleanupHandlers = new Map void>() +const cleanupSignalHandlers = new Map void>() +const cleanupErrorHandlers = new Map void>() export function registerManagerForCleanup(manager: CleanupTarget): void { cleanupManagers.add(manager) @@ -59,9 +77,9 @@ export function registerManagerForCleanup(manager: CleanupTarget): void { return cleanupPromise } - const registerSignal = (signal: ProcessCleanupEvent, exitAfter: boolean): void => { + const registerSignal = (signal: ProcessCleanupSignal, exitAfter: boolean): void => { const listener = registerProcessSignal(signal, cleanupAll, exitAfter) - cleanupHandlers.set(signal, listener) + cleanupSignalHandlers.set(signal, listener) } registerSignal("SIGINT", true) @@ -71,6 +89,8 @@ export function registerManagerForCleanup(manager: CleanupTarget): void { } registerSignal("beforeExit", false) registerSignal("exit", false) + cleanupErrorHandlers.set("uncaughtException", registerErrorEvent("uncaughtException", cleanupAll)) + cleanupErrorHandlers.set("unhandledRejection", registerErrorEvent("unhandledRejection", cleanupAll)) } export function unregisterManagerForCleanup(manager: CleanupTarget): void { @@ -78,10 +98,14 @@ export function unregisterManagerForCleanup(manager: CleanupTarget): void { if (cleanupManagers.size > 0) return - for (const [signal, listener] of cleanupHandlers.entries()) { + for (const [signal, listener] of cleanupSignalHandlers.entries()) { process.off(signal, listener) } - cleanupHandlers.clear() + for (const [signal, listener] of cleanupErrorHandlers.entries()) { + process.off(signal, listener) + } + cleanupSignalHandlers.clear() + cleanupErrorHandlers.clear() cleanupRegistered = false } @@ -90,9 +114,13 @@ export function _resetForTesting(): void { for (const manager of [...cleanupManagers]) { cleanupManagers.delete(manager) } - for (const [signal, listener] of cleanupHandlers.entries()) { + for (const [signal, listener] of cleanupSignalHandlers.entries()) { process.off(signal, listener) } - cleanupHandlers.clear() + for (const [signal, listener] of cleanupErrorHandlers.entries()) { + process.off(signal, listener) + } + cleanupSignalHandlers.clear() + cleanupErrorHandlers.clear() cleanupRegistered = false } diff --git a/src/features/team-mode/team-layout-tmux/layout.test.ts b/src/features/team-mode/team-layout-tmux/layout.test.ts index c23f878f4..aa9a90ff5 100644 --- a/src/features/team-mode/team-layout-tmux/layout.test.ts +++ b/src/features/team-mode/team-layout-tmux/layout.test.ts @@ -1,21 +1,32 @@ import { beforeEach, describe, expect, mock, test } from "bun:test" +type LayoutModule = typeof import("./layout") + const spawnMock = mock(() => ({ exited: Promise.resolve(0), stdout: new ReadableStream({ start(controller) { controller.enqueue(new TextEncoder().encode("%1\n")); controller.close() } }), stderr: new ReadableStream({ start(controller) { controller.close() } }), })) -mock.module("bun", () => ({ spawn: spawnMock })) +const layoutSpecifier = import.meta.resolve("./layout") +const spawnProcessSpecifier = import.meta.resolve("../../../shared/tmux/tmux-utils/spawn-process") +const tmuxPathResolverSpecifier = import.meta.resolve("../../../tools/interactive-bash/tmux-path-resolver") +const sharedSpecifier = import.meta.resolve("../../../shared") -mock.module("../../../tools/interactive-bash/tmux-path-resolver", () => ({ getTmuxPath: mock(() => Promise.resolve("tmux")) })) +function registerModuleMocks(): void { + mock.module(spawnProcessSpecifier, () => ({ spawn: spawnMock })) + mock.module(tmuxPathResolverSpecifier, () => ({ getTmuxPath: mock(() => Promise.resolve("tmux")) })) + mock.module(sharedSpecifier, () => ({ log: mock(() => undefined) })) +} -mock.module("../../../shared", () => ({ log: mock(() => undefined) })) - -import { createTeamLayout, removeTeamLayout, canVisualize } from "./layout" +async function loadLayoutModule(): Promise { + const module = await import(`${layoutSpecifier}?test=${crypto.randomUUID()}`) + return module as LayoutModule +} describe("team-layout-tmux", () => { beforeEach(() => { + registerModuleMocks() spawnMock.mockClear() process.env.TMUX = "/tmp/tmux-1" }) @@ -23,6 +34,7 @@ describe("team-layout-tmux", () => { test("returns null and makes no tmux calls when visualization unavailable", async () => { // given delete process.env.TMUX + const { createTeamLayout, canVisualize } = await loadLayoutModule() // when const result = await createTeamLayout("run-1", [], {} as never) @@ -35,6 +47,7 @@ describe("team-layout-tmux", () => { test("creates focus and grid windows", async () => { // given + const { createTeamLayout } = await loadLayoutModule() const members = [ { name: "lead", sessionId: "s1", color: "red" }, { name: "m2", sessionId: "s2" }, @@ -54,6 +67,7 @@ describe("team-layout-tmux", () => { test("returns null when tmux command fails", async () => { // given + const { createTeamLayout } = await loadLayoutModule() spawnMock.mockImplementationOnce(() => ({ exited: Promise.resolve(1), stdout: new ReadableStream({ start(controller) { controller.close() } }), @@ -69,6 +83,8 @@ describe("team-layout-tmux", () => { test("cleans up the tmux session", async () => { // given + const { removeTeamLayout } = await loadLayoutModule() + // when await removeTeamLayout("run-4", {} as never) diff --git a/src/features/team-mode/team-layout-tmux/layout.ts b/src/features/team-mode/team-layout-tmux/layout.ts index b41354d06..f414ccbfe 100644 --- a/src/features/team-mode/team-layout-tmux/layout.ts +++ b/src/features/team-mode/team-layout-tmux/layout.ts @@ -1,4 +1,4 @@ -import { spawn } from "bun" +import { spawn } from "../../../shared/tmux/tmux-utils/spawn-process" import { log } from "../../../shared" import { getTmuxPath } from "../../../tools/interactive-bash/tmux-path-resolver" import type { TmuxSessionManager } from "../../tmux-subagent/manager" diff --git a/src/features/tmux-subagent/cleanup.ts b/src/features/tmux-subagent/cleanup.ts deleted file mode 100644 index 414ad00bc..000000000 --- a/src/features/tmux-subagent/cleanup.ts +++ /dev/null @@ -1,42 +0,0 @@ -import type { TmuxConfig } from "../../config/schema" -import { log } from "../../shared" -import type { TrackedSession } from "./types" -import { queryWindowState } from "./pane-state-querier" -import { executeAction } from "./action-executor" - -export async function cleanupTmuxSessions(params: { - tmuxConfig: TmuxConfig - serverUrl: string - sourcePaneId: string | undefined - sessions: Map - stopPolling: () => void -}): Promise { - params.stopPolling() - - if (params.sessions.size === 0) { - log("[tmux-session-manager] cleanup complete") - return - } - - log("[tmux-session-manager] closing all panes", { count: params.sessions.size }) - const state = params.sourcePaneId ? await queryWindowState(params.sourcePaneId) : null - - if (state) { - const closePromises = Array.from(params.sessions.values()).map((tracked) => - executeAction( - { type: "close", paneId: tracked.paneId, sessionId: tracked.sessionId }, - { config: params.tmuxConfig, serverUrl: params.serverUrl, windowState: state }, - ).catch((error) => - log("[tmux-session-manager] cleanup error for pane", { - paneId: tracked.paneId, - error: String(error), - }), - ), - ) - - await Promise.all(closePromises) - } - - params.sessions.clear() - log("[tmux-session-manager] cleanup complete") -} diff --git a/src/features/tmux-subagent/event-handlers.ts b/src/features/tmux-subagent/event-handlers.ts index 0991d10e2..2916c7439 100644 --- a/src/features/tmux-subagent/event-handlers.ts +++ b/src/features/tmux-subagent/event-handlers.ts @@ -1,6 +1,2 @@ export { coerceSessionCreatedEvent } from "./session-created-event" export type { SessionCreatedEvent } from "./session-created-event" -export { handleSessionCreated } from "./session-created-handler" -export type { SessionCreatedHandlerDeps } from "./session-created-handler" -export { handleSessionDeleted } from "./session-deleted-handler" -export type { SessionDeletedHandlerDeps } from "./session-deleted-handler" diff --git a/src/features/tmux-subagent/index.ts b/src/features/tmux-subagent/index.ts index e900555fb..cba66fa6b 100644 --- a/src/features/tmux-subagent/index.ts +++ b/src/features/tmux-subagent/index.ts @@ -1,10 +1,7 @@ export * from "./manager" export * from "./event-handlers" export * from "./polling" -export * from "./cleanup" export * from "./session-created-event" -export * from "./session-created-handler" -export * from "./session-deleted-handler" export * from "./polling-constants" export * from "./session-status-parser" export * from "./session-message-count" diff --git a/src/features/tmux-subagent/manager.test.ts b/src/features/tmux-subagent/manager.test.ts index c8cf2c1d2..9a03d7641 100644 --- a/src/features/tmux-subagent/manager.test.ts +++ b/src/features/tmux-subagent/manager.test.ts @@ -57,6 +57,7 @@ const mockSpawnTmuxSession = mock<( success: true, paneId: '%isolated-session', })) +const mockKillTmuxSessionIfExists = mock<(sessionName: string) => Promise>(async () => true) const mockIsInsideTmux = mock<() => boolean>(() => true) const mockGetCurrentPaneId = mock<() => string | undefined>(() => '%0') @@ -99,6 +100,8 @@ mock.module('../../shared/tmux', () => { SESSION_READY_TIMEOUT_MS: 500, spawnTmuxWindow: mockSpawnTmuxWindow, spawnTmuxSession: mockSpawnTmuxSession, + killTmuxSessionIfExists: mockKillTmuxSessionIfExists, + getIsolatedSessionName: (pid: number = 12345) => `omo-agents-${pid}`, } }) @@ -1852,6 +1855,137 @@ describe('TmuxSessionManager', () => { // then expect(mockExecuteAction).toHaveBeenCalledTimes(2) }) + + test('#given tmux isolation is "session" #when cleanup runs #then killTmuxSessionIfExists is invoked for the per-pid isolated session', async () => { + // given + mockKillTmuxSessionIfExists.mockClear() + const { TmuxSessionManager } = await import('./manager') + const manager = new TmuxSessionManager(createMockContext(), createTmuxConfig({ + enabled: true, + isolation: 'session', + }), mockTmuxDeps) + + // when + await manager.cleanup() + + // then + expect(mockKillTmuxSessionIfExists).toHaveBeenCalledTimes(1) + expect(mockKillTmuxSessionIfExists.mock.calls[0]?.[0]).toMatch(/^omo-agents-\d+$/) + }) + + test('#given two manager instances #when both cleanup #then each kills its own isolated session name, not a shared one', async () => { + // given + mockKillTmuxSessionIfExists.mockClear() + const { TmuxSessionManager } = await import('./manager') + const managerA = new TmuxSessionManager(createMockContext(), createTmuxConfig({ + enabled: true, + isolation: 'session', + }), mockTmuxDeps) + const managerB = new TmuxSessionManager(createMockContext(), createTmuxConfig({ + enabled: true, + isolation: 'session', + }), mockTmuxDeps) + + // when + await managerA.cleanup() + await managerB.cleanup() + + // then + expect(mockKillTmuxSessionIfExists).toHaveBeenCalledTimes(2) + const firstTarget = mockKillTmuxSessionIfExists.mock.calls[0]?.[0] + const secondTarget = mockKillTmuxSessionIfExists.mock.calls[1]?.[0] + expect(firstTarget).toMatch(/^omo-agents-\d+$/) + expect(secondTarget).toMatch(/^omo-agents-\d+$/) + }) + + test('#given tmux isolation is "inline" #when cleanup runs #then killTmuxSessionIfExists is NOT invoked', async () => { + // given + mockKillTmuxSessionIfExists.mockClear() + const { TmuxSessionManager } = await import('./manager') + const manager = new TmuxSessionManager(createMockContext(), createTmuxConfig({ + enabled: true, + isolation: 'inline', + }), mockTmuxDeps) + + // when + await manager.cleanup() + + // then + expect(mockKillTmuxSessionIfExists).toHaveBeenCalledTimes(0) + }) + + test('#given tmux isolation is "window" #when cleanup runs #then killTmuxSessionIfExists is NOT invoked', async () => { + // given + mockKillTmuxSessionIfExists.mockClear() + const { TmuxSessionManager } = await import('./manager') + const manager = new TmuxSessionManager(createMockContext(), createTmuxConfig({ + enabled: true, + isolation: 'window', + }), mockTmuxDeps) + + // when + await manager.cleanup() + + // then + 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() + mockKillTmuxSessionIfExists.mockImplementationOnce(async () => { + throw new Error('simulated teardown failure') + }) + const { TmuxSessionManager } = await import('./manager') + const manager = new TmuxSessionManager(createMockContext(), createTmuxConfig({ + enabled: true, + isolation: 'session', + }), mockTmuxDeps) + + // when + const cleanupPromise = manager.cleanup() + + // then + await expect(cleanupPromise).resolves.toBeUndefined() + expect(mockKillTmuxSessionIfExists).toHaveBeenCalledTimes(1) + }) }) }) diff --git a/src/features/tmux-subagent/manager.ts b/src/features/tmux-subagent/manager.ts index e379bce96..9d9c65159 100644 --- a/src/features/tmux-subagent/manager.ts +++ b/src/features/tmux-subagent/manager.ts @@ -10,6 +10,8 @@ import { SESSION_READY_TIMEOUT_MS, spawnTmuxWindow, spawnTmuxSession, + killTmuxSessionIfExists, + getIsolatedSessionName, } from "../../shared/tmux" import { queryWindowState } from "./pane-state-querier" import { decideSpawnActions, decideCloseAction, type SessionMapping } from "./decision-engine" @@ -89,7 +91,8 @@ export class TmuxSessionManager { this.pollingManager = new TmuxPollingManager( this.client, this.sessions, - this.closeSessionById.bind(this) + this.closeSessionById.bind(this), + this.retryPendingCloses.bind(this) ) log("[tmux-session-manager] initialized", { configEnabled: this.tmuxConfig.enabled, @@ -832,6 +835,18 @@ 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 @@ -954,6 +969,22 @@ export class TmuxSessionManager { this.isolatedContainerPaneId = undefined this.isolatedWindowPaneId = undefined + if (this.tmuxConfig.isolation === "session") { + const isolatedSessionName = getIsolatedSessionName() + try { + const killed = await killTmuxSessionIfExists(isolatedSessionName) + log("[tmux-session-manager] isolated session teardown", { + session: isolatedSessionName, + killed, + }) + } catch (error) { + log("[tmux-session-manager] isolated session teardown failed", { + session: isolatedSessionName, + error: String(error), + }) + } + } + log("[tmux-session-manager] cleanup complete") } } diff --git a/src/features/tmux-subagent/polling-manager.ts b/src/features/tmux-subagent/polling-manager.ts index d7a972d40..1a74be801 100644 --- a/src/features/tmux-subagent/polling-manager.ts +++ b/src/features/tmux-subagent/polling-manager.ts @@ -16,7 +16,8 @@ export class TmuxPollingManager { constructor( private client: OpencodeClient, private sessions: Map, - private closeSessionById: (sessionId: string) => Promise + private closeSessionById: (sessionId: string) => Promise, + private retryPendingCloses?: () => Promise ) {} handleEvent(event: { type: string; properties?: Record }): void { @@ -134,6 +135,14 @@ export class TmuxPollingManager { log("[tmux-session-manager] closing session due to poll", { sessionId }) await this.closeSessionById(sessionId) } + + if (this.retryPendingCloses) { + try { + await this.retryPendingCloses() + } catch (err) { + log("[tmux-session-manager] retry pending closes failed", { error: String(err) }) + } + } } catch (err) { log("[tmux-session-manager] poll error", { error: String(err) }) } finally { diff --git a/src/features/tmux-subagent/session-created-handler.ts b/src/features/tmux-subagent/session-created-handler.ts deleted file mode 100644 index 6dd1f21eb..000000000 --- a/src/features/tmux-subagent/session-created-handler.ts +++ /dev/null @@ -1,175 +0,0 @@ -import type { PluginInput } from "@opencode-ai/plugin" -import type { TmuxConfig } from "../../config/schema" -import type { CapacityConfig, TrackedSession } from "./types" -import { log } from "../../shared" -import { queryWindowState } from "./pane-state-querier" -import { decideSpawnActions, type SessionMapping } from "./decision-engine" -import { executeActions } from "./action-executor" -import type { SessionCreatedEvent } from "./session-created-event" -import { createTrackedSession } from "./tracked-session-state" - -type OpencodeClient = PluginInput["client"] - -export interface SessionCreatedHandlerDeps { - client: OpencodeClient - tmuxConfig: TmuxConfig - serverUrl: string - sourcePaneId: string | undefined - sessions: Map - pendingSessions: Set - isInsideTmux: () => boolean - isEnabled: () => boolean - getCapacityConfig: () => CapacityConfig - getSessionMappings: () => SessionMapping[] - waitForSessionReady: (sessionId: string) => Promise - startPolling: () => void -} - -export async function handleSessionCreated( - deps: SessionCreatedHandlerDeps, - event: SessionCreatedEvent, -): Promise { - const enabled = deps.isEnabled() - log("[tmux-session-manager] onSessionCreated called", { - enabled, - tmuxConfigEnabled: deps.tmuxConfig.enabled, - isInsideTmux: deps.isInsideTmux(), - eventType: event.type, - infoId: event.properties?.info?.id, - infoParentID: event.properties?.info?.parentID, - }) - - if (!enabled) return - if (event.type !== "session.created") return - - const info = event.properties?.info - if (!info?.id || !info?.parentID) return - - const sessionId = info.id - const title = info.title ?? "Subagent" - - if (deps.sessions.has(sessionId) || deps.pendingSessions.has(sessionId)) { - log("[tmux-session-manager] session already tracked or pending", { sessionId }) - return - } - - if (!deps.sourcePaneId) { - log("[tmux-session-manager] no source pane id") - return - } - - deps.pendingSessions.add(sessionId) - - try { - const state = await queryWindowState(deps.sourcePaneId) - if (!state) { - log("[tmux-session-manager] failed to query window state") - return - } - - log("[tmux-session-manager] window state queried", { - windowWidth: state.windowWidth, - mainPane: state.mainPane?.paneId, - agentPaneCount: state.agentPanes.length, - agentPanes: state.agentPanes.map((p) => p.paneId), - }) - - const decision = decideSpawnActions( - state, - sessionId, - title, - deps.getCapacityConfig(), - deps.getSessionMappings(), - ) - - log("[tmux-session-manager] spawn decision", { - canSpawn: decision.canSpawn, - reason: decision.reason, - actionCount: decision.actions.length, - actions: decision.actions.map((a) => { - if (a.type === "close") return { type: "close", paneId: a.paneId } - if (a.type === "replace") { - return { type: "replace", paneId: a.paneId, newSessionId: a.newSessionId } - } - return { type: "spawn", sessionId: a.sessionId } - }), - }) - - if (!decision.canSpawn) { - log("[tmux-session-manager] cannot spawn", { reason: decision.reason }) - return - } - - const result = await executeActions(decision.actions, { - config: deps.tmuxConfig, - serverUrl: deps.serverUrl, - windowState: state, - }) - - for (const { action, result: actionResult } of result.results) { - if (action.type === "close" && actionResult.success) { - deps.sessions.delete(action.sessionId) - log("[tmux-session-manager] removed closed session from cache", { - sessionId: action.sessionId, - }) - } - if (action.type === "replace" && actionResult.success) { - deps.sessions.delete(action.oldSessionId) - log("[tmux-session-manager] removed replaced session from cache", { - oldSessionId: action.oldSessionId, - newSessionId: action.newSessionId, - }) - } - } - - if (!result.success || !result.spawnedPaneId) { - log("[tmux-session-manager] spawn failed", { - success: result.success, - results: result.results.map((r) => ({ - type: r.action.type, - success: r.result.success, - error: r.result.error, - })), - }) - return - } - - const sessionReady = await deps.waitForSessionReady(sessionId) - if (!sessionReady) { - log("[tmux-session-manager] session not ready after timeout, closing spawned pane", { - sessionId, - paneId: result.spawnedPaneId, - }) - - await executeActions( - [{ type: "close", paneId: result.spawnedPaneId, sessionId }], - { - config: deps.tmuxConfig, - serverUrl: deps.serverUrl, - windowState: state, - }, - ) - - return - } - - deps.sessions.set( - sessionId, - createTrackedSession({ - sessionId, - paneId: result.spawnedPaneId, - description: title, - }), - ) - - log("[tmux-session-manager] pane spawned and tracked", { - sessionId, - paneId: result.spawnedPaneId, - sessionReady, - }) - - deps.startPolling() - } finally { - deps.pendingSessions.delete(sessionId) - } -} diff --git a/src/features/tmux-subagent/session-deleted-handler.ts b/src/features/tmux-subagent/session-deleted-handler.ts deleted file mode 100644 index f832cf481..000000000 --- a/src/features/tmux-subagent/session-deleted-handler.ts +++ /dev/null @@ -1,50 +0,0 @@ -import type { TmuxConfig } from "../../config/schema" -import type { TrackedSession } from "./types" -import { log } from "../../shared" -import { queryWindowState } from "./pane-state-querier" -import { decideCloseAction, type SessionMapping } from "./decision-engine" -import { executeAction } from "./action-executor" - -export interface SessionDeletedHandlerDeps { - tmuxConfig: TmuxConfig - serverUrl: string - sourcePaneId: string | undefined - sessions: Map - isEnabled: () => boolean - getSessionMappings: () => SessionMapping[] - stopPolling: () => void -} - -export async function handleSessionDeleted( - deps: SessionDeletedHandlerDeps, - event: { sessionID: string }, -): Promise { - if (!deps.isEnabled()) return - if (!deps.sourcePaneId) return - - const tracked = deps.sessions.get(event.sessionID) - if (!tracked) return - - log("[tmux-session-manager] onSessionDeleted", { sessionId: event.sessionID }) - - const state = await queryWindowState(deps.sourcePaneId) - if (!state) { - deps.sessions.delete(event.sessionID) - return - } - - const closeAction = decideCloseAction(state, event.sessionID, deps.getSessionMappings()) - if (closeAction) { - await executeAction(closeAction, { - config: deps.tmuxConfig, - serverUrl: deps.serverUrl, - windowState: state, - }) - } - - deps.sessions.delete(event.sessionID) - - if (deps.sessions.size === 0) { - deps.stopPolling() - } -} diff --git a/src/plugin/event.ts b/src/plugin/event.ts index 5a5f177b6..0f79a84d5 100644 --- a/src/plugin/event.ts +++ b/src/plugin/event.ts @@ -615,6 +615,10 @@ 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 a9aab095a..d80d1e720 100644 --- a/src/shared/tmux/tmux-utils.ts +++ b/src/shared/tmux/tmux-utils.ts @@ -10,6 +10,7 @@ export { spawnTmuxPane } from "./tmux-utils/pane-spawn" export { closeTmuxPane } from "./tmux-utils/pane-close" export { replaceTmuxPane } from "./tmux-utils/pane-replace" export { spawnTmuxWindow } from "./tmux-utils/window-spawn" -export { spawnTmuxSession } from "./tmux-utils/session-spawn" +export { spawnTmuxSession, getIsolatedSessionName } from "./tmux-utils/session-spawn" +export { killTmuxSessionIfExists } from "./tmux-utils/session-kill" export { applyLayout, enforceMainPaneWidth } from "./tmux-utils/layout" diff --git a/src/shared/tmux/tmux-utils/index.ts b/src/shared/tmux/tmux-utils/index.ts new file mode 100644 index 000000000..e55436a1c --- /dev/null +++ b/src/shared/tmux/tmux-utils/index.ts @@ -0,0 +1 @@ +export { killTmuxSessionIfExists } from "./session-kill" diff --git a/src/shared/tmux/tmux-utils/pane-close.test.ts b/src/shared/tmux/tmux-utils/pane-close.test.ts new file mode 100644 index 000000000..b8d5d7887 --- /dev/null +++ b/src/shared/tmux/tmux-utils/pane-close.test.ts @@ -0,0 +1,221 @@ +import { beforeEach, describe, expect, it, mock } from "bun:test" + +type CloseTmuxPane = typeof import("./pane-close").closeTmuxPane + +type SpawnCall = { + command: string[] + options: { + stdout?: string + stderr?: string + } +} + +type FakeSubprocess = { + exited: Promise + stdout: ReadableStream + stderr: ReadableStream +} + +const TIMEOUT = Symbol("timeout") +const spawnCalls: SpawnCall[] = [] +const queuedProcesses: FakeSubprocess[] = [] + +function createClosedStream(): ReadableStream { + return new ReadableStream({ + start(controller) { + controller.close() + }, + }) +} + +type DrainSignal = { onPull: () => void } + +function createDrainSensitiveStream(byteLength: number, signal: DrainSignal): ReadableStream { + let remainingBytes = byteLength + const chunk = new TextEncoder().encode("x".repeat(16 * 1024)) + + return new ReadableStream({ + pull(controller) { + signal.onPull() + + if (remainingBytes <= 0) { + controller.close() + return + } + + const nextChunkSize = Math.min(remainingBytes, chunk.byteLength) + controller.enqueue(chunk.subarray(0, nextChunkSize)) + remainingBytes -= nextChunkSize + }, + }) +} + +function createProcess(exitCode: number): FakeSubprocess { + return { + exited: Promise.resolve(exitCode), + stdout: createClosedStream(), + stderr: createClosedStream(), + } +} + +function createStdoutSensitiveProcess(exitCode: number, stdoutBytes: number): FakeSubprocess { + let resolveDrained: () => void = () => undefined + const drained = new Promise((resolve) => { + resolveDrained = resolve + }) + const stdout = createDrainSensitiveStream(stdoutBytes, { onPull: () => resolveDrained() }) + + return { + exited: drained.then(() => exitCode), + stdout, + stderr: createClosedStream(), + } +} + +const spawnMock = mock((command: string[], options: { stdout?: string; stderr?: string } = {}): FakeSubprocess => { + spawnCalls.push({ command, options }) + + 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 paneCloseSpecifier = import.meta.resolve("./pane-close") +const environmentSpecifier = import.meta.resolve("./environment") +const loggerSpecifier = import.meta.resolve("../../logger") +const spawnProcessSpecifier = import.meta.resolve("./spawn-process") +const tmuxPathResolverSpecifier = import.meta.resolve("../../../tools/interactive-bash/tmux-path-resolver") + +async function loadCloseTmuxPane(): Promise { + const module = await import(`${paneCloseSpecifier}?test=${crypto.randomUUID()}`) + return module.closeTmuxPane +} + +function registerModuleMocks(): void { + mock.module(spawnProcessSpecifier, () => ({ spawn: spawnMock })) + mock.module(environmentSpecifier, () => ({ isInsideTmux: isInsideTmuxMock })) + mock.module(tmuxPathResolverSpecifier, () => ({ getTmuxPath: getTmuxPathMock })) + mock.module(loggerSpecifier, () => ({ log: logMock })) +} + +function resolveWithin(promise: Promise, milliseconds: number): Promise { + return Promise.race([ + promise, + new Promise((resolve) => { + setTimeout(() => resolve(TIMEOUT), milliseconds) + }), + ]) +} + +describe("closeTmuxPane", () => { + beforeEach(() => { + registerModuleMocks() + spawnCalls.length = 0 + queuedProcesses.length = 0 + spawnMock.mockClear() + isInsideTmuxMock.mockClear() + getTmuxPathMock.mockClear() + logMock.mockClear() + + isInsideTmuxMock.mockImplementation((): boolean => true) + getTmuxPathMock.mockImplementation(async (): Promise => "tmux") + }) + + it("#given pane exists #when closeTmuxPane called #then returns true and invokes send-keys + kill-pane in order", async () => { + // given + const closeTmuxPane = await loadCloseTmuxPane() + queuedProcesses.push(createProcess(0), createProcess(0)) + + // when + const result = await closeTmuxPane("%42") + + // then + expect(result).toBe(true) + expect(spawnCalls).toEqual([ + { command: ["tmux", "send-keys", "-t", "%42", "C-c"], options: { stdout: "ignore", stderr: "ignore" } }, + { command: ["tmux", "kill-pane", "-t", "%42"], options: { stdout: "pipe", stderr: "pipe" } }, + ]) + }) + + it("#given not inside tmux #when closeTmuxPane called #then returns false without spawn", async () => { + // given + const closeTmuxPane = await loadCloseTmuxPane() + isInsideTmuxMock.mockImplementation((): boolean => false) + + // when + const result = await closeTmuxPane("%42") + + // then + expect(result).toBe(false) + expect(spawnCalls).toHaveLength(0) + }) + + it("#given tmux not found #when closeTmuxPane called #then returns false without spawn", async () => { + // given + const closeTmuxPane = await loadCloseTmuxPane() + getTmuxPathMock.mockImplementation(async (): Promise => undefined) + + // when + const result = await closeTmuxPane("%42") + + // then + expect(result).toBe(false) + expect(spawnCalls).toHaveLength(0) + }) + + it("#given kill-pane fails with unknown error #when closeTmuxPane called #then returns false", async () => { + // given + const closeTmuxPane = await loadCloseTmuxPane() + queuedProcesses.push(createProcess(0), createProcess(1)) + + // when + const result = await closeTmuxPane("%42") + + // then + expect(result).toBe(false) + }) + + it("#given pane already closed by Ctrl+C (kill-pane reports 'can't find pane') #when closeTmuxPane called #then returns true", async () => { + // given + const closeTmuxPane = await loadCloseTmuxPane() + queuedProcesses.push( + createProcess(0), + { + exited: Promise.resolve(1), + stdout: createClosedStream(), + stderr: new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode("can't find pane: %42\n")) + controller.close() + }, + }), + }, + ) + + // when + const result = await closeTmuxPane("%42") + + // then + expect(result).toBe(true) + }) + + it("#given kill-pane stdout stream waits for drain #when closeTmuxPane called #then returns true once drainer consumes stdout", async () => { + // given + const closeTmuxPane = await loadCloseTmuxPane() + queuedProcesses.push(createProcess(0), createStdoutSensitiveProcess(0, 16 * 1024)) + + // when + const result = await resolveWithin(closeTmuxPane("%42"), 2000) + + // then + expect(result).not.toBe(TIMEOUT) + expect(result).toBe(true) + }) +}) diff --git a/src/shared/tmux/tmux-utils/pane-close.ts b/src/shared/tmux/tmux-utils/pane-close.ts index cc6f4b6c4..e62e46296 100644 --- a/src/shared/tmux/tmux-utils/pane-close.ts +++ b/src/shared/tmux/tmux-utils/pane-close.ts @@ -1,13 +1,18 @@ -import { spawn } from "bun" -import { getTmuxPath } from "../../../tools/interactive-bash/tmux-path-resolver" -import { isInsideTmux } from "./environment" - function delay(milliseconds: number): Promise { return new Promise((resolve) => setTimeout(resolve, milliseconds)) } +async function readStream(stream: ReadableStream | null | undefined): Promise { + return stream ? new Response(stream).text() : "" +} + export async function closeTmuxPane(paneId: string): Promise { - const { log } = await import("../../logger") + const [{ log }, { isInsideTmux }, { getTmuxPath }, { spawn }] = await Promise.all([ + import("../../logger"), + import("./environment"), + import("../../../tools/interactive-bash/tmux-path-resolver"), + import("./spawn-process"), + ]) if (!isInsideTmux()) { log("[closeTmuxPane] SKIP: not inside tmux") @@ -22,8 +27,8 @@ export async function closeTmuxPane(paneId: string): Promise { log("[closeTmuxPane] sending Ctrl+C for graceful shutdown", { paneId }) const ctrlCProc = spawn([tmux, "send-keys", "-t", paneId, "C-c"], { - stdout: "pipe", - stderr: "pipe", + stdout: "ignore", + stderr: "ignore", }) await ctrlCProc.exited @@ -31,18 +36,29 @@ export async function closeTmuxPane(paneId: string): Promise { log("[closeTmuxPane] killing pane", { paneId }) - const proc = spawn([tmux, "kill-pane", "-t", paneId], { + const killPaneProc = spawn([tmux, "kill-pane", "-t", paneId], { stdout: "pipe", stderr: "pipe", }) - const exitCode = await proc.exited - const stderr = await new Response(proc.stderr).text() + const [, stderr, exitCode] = await Promise.all([ + readStream(killPaneProc.stdout), + readStream(killPaneProc.stderr), + killPaneProc.exited, + ]) - if (exitCode !== 0) { - log("[closeTmuxPane] FAILED", { paneId, exitCode, stderr: stderr.trim() }) - } else { - log("[closeTmuxPane] SUCCESS", { paneId }) + const trimmedStderr = stderr.trim() + const paneAlreadyGone = exitCode !== 0 && /can't find pane/i.test(trimmedStderr) + + if (paneAlreadyGone) { + log("[closeTmuxPane] SUCCESS (pane already closed by Ctrl+C)", { paneId }) + return true } - return exitCode === 0 + if (exitCode !== 0) { + log("[closeTmuxPane] FAILED", { paneId, exitCode, stderr: trimmedStderr }) + return false + } + + log("[closeTmuxPane] SUCCESS", { paneId }) + return true } diff --git a/src/shared/tmux/tmux-utils/session-kill.test.ts b/src/shared/tmux/tmux-utils/session-kill.test.ts new file mode 100644 index 000000000..ca185d980 --- /dev/null +++ b/src/shared/tmux/tmux-utils/session-kill.test.ts @@ -0,0 +1,173 @@ +import { beforeEach, describe, expect, it, mock } from "bun:test" + +type KillTmuxSessionIfExists = typeof import("./session-kill").killTmuxSessionIfExists + +type SpawnCall = { + command: string[] + options: { + stdout?: string + stderr?: string + } +} + +type FakeSubprocess = { + exited: Promise + stdout: ReadableStream + stderr: ReadableStream +} + +const spawnCalls: SpawnCall[] = [] +const queuedProcesses: FakeSubprocess[] = [] + +function createStream(chunks: string[] = []): ReadableStream { + const textEncoder = new TextEncoder() + + return new ReadableStream({ + start(controller) { + for (const chunk of chunks) { + controller.enqueue(textEncoder.encode(chunk)) + } + + controller.close() + }, + }) +} + +function createProcess(exitCode: number, output: { stdout?: string[]; stderr?: string[] } = {}): FakeSubprocess { + return { + exited: Promise.resolve(exitCode), + stdout: createStream(output.stdout), + stderr: createStream(output.stderr), + } +} + +const spawnMock = mock((command: string[], options: { stdout?: string; stderr?: string } = {}) => { + spawnCalls.push({ command, options }) + + 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 sessionKillSpecifier = import.meta.resolve("./session-kill") +const environmentSpecifier = import.meta.resolve("./environment") +const loggerSpecifier = import.meta.resolve("../../logger") +const spawnProcessSpecifier = import.meta.resolve("./spawn-process") +const tmuxPathResolverSpecifier = import.meta.resolve("../../../tools/interactive-bash/tmux-path-resolver") + +async function loadKillTmuxSessionIfExists(): Promise { + const module = await import(`${sessionKillSpecifier}?test=${crypto.randomUUID()}`) + return module.killTmuxSessionIfExists +} + +function registerModuleMocks(): void { + mock.module(spawnProcessSpecifier, () => ({ spawn: spawnMock })) + mock.module(environmentSpecifier, () => ({ isInsideTmux: isInsideTmuxMock })) + mock.module(tmuxPathResolverSpecifier, () => ({ getTmuxPath: getTmuxPathMock })) + mock.module(loggerSpecifier, () => ({ log: logMock })) +} + +describe("killTmuxSessionIfExists", () => { + beforeEach(() => { + registerModuleMocks() + spawnCalls.length = 0 + queuedProcesses.length = 0 + spawnMock.mockClear() + isInsideTmuxMock.mockClear() + getTmuxPathMock.mockClear() + logMock.mockClear() + + isInsideTmuxMock.mockImplementation((): boolean => true) + getTmuxPathMock.mockImplementation(async (): Promise => "tmux") + }) + + it("#given omo-agents session exists #when killTmuxSessionIfExists called #then kill-session invoked and returns true", async () => { + // given + const killTmuxSessionIfExists = await loadKillTmuxSessionIfExists() + queuedProcesses.push(createProcess(0), createProcess(0, { stdout: ["killed"], stderr: [] })) + + // when + const result = await killTmuxSessionIfExists("omo-agents") + + // then + expect(result).toBe(true) + expect(spawnCalls).toEqual([ + { + command: ["tmux", "has-session", "-t", "omo-agents"], + options: { stdout: "ignore", stderr: "ignore" }, + }, + { + command: ["tmux", "kill-session", "-t", "omo-agents"], + options: { stdout: "pipe", stderr: "pipe" }, + }, + ]) + }) + + it("#given omo-agents session does NOT exist (has-session exits non-zero) #when killTmuxSessionIfExists called #then NO kill-session invocation and returns false", async () => { + // given + const killTmuxSessionIfExists = await loadKillTmuxSessionIfExists() + queuedProcesses.push(createProcess(1)) + + // when + const result = await killTmuxSessionIfExists("omo-agents") + + // then + expect(result).toBe(false) + expect(spawnCalls).toEqual([ + { + command: ["tmux", "has-session", "-t", "omo-agents"], + options: { stdout: "ignore", stderr: "ignore" }, + }, + ]) + }) + + it("#given not inside tmux #when killTmuxSessionIfExists called #then returns false without any spawn", async () => { + // given + const killTmuxSessionIfExists = await loadKillTmuxSessionIfExists() + isInsideTmuxMock.mockReturnValue(false) + + // when + const result = await killTmuxSessionIfExists("omo-agents") + + // then + expect(result).toBe(false) + expect(spawnCalls).toHaveLength(0) + expect(getTmuxPathMock).toHaveBeenCalledTimes(0) + }) + + it("#given tmux not found #when killTmuxSessionIfExists called #then returns false without spawn", async () => { + // given + const killTmuxSessionIfExists = await loadKillTmuxSessionIfExists() + getTmuxPathMock.mockResolvedValue(undefined) + + // when + const result = await killTmuxSessionIfExists("omo-agents") + + // then + expect(result).toBe(false) + expect(spawnCalls).toHaveLength(0) + }) + + it("#given kill-session itself fails (e.g., race between has-session and kill) #when killTmuxSessionIfExists called #then returns false but does not throw", async () => { + // given + const killTmuxSessionIfExists = await loadKillTmuxSessionIfExists() + queuedProcesses.push( + createProcess(0), + createProcess(1, { stdout: [], stderr: ["no session"] }), + ) + + // when + const result = await killTmuxSessionIfExists("omo-agents") + + // then + expect(result).toBe(false) + expect(spawnCalls).toHaveLength(2) + }) +}) diff --git a/src/shared/tmux/tmux-utils/session-kill.ts b/src/shared/tmux/tmux-utils/session-kill.ts new file mode 100644 index 000000000..fc5f765df --- /dev/null +++ b/src/shared/tmux/tmux-utils/session-kill.ts @@ -0,0 +1,51 @@ +async function readStream(stream: ReadableStream | null | undefined): Promise { + return stream ? new Response(stream).text() : "" +} + +export async function killTmuxSessionIfExists(sessionName: string): Promise { + const [{ log }, { isInsideTmux }, { getTmuxPath }, { spawn }] = await Promise.all([ + import("../../logger"), + import("./environment"), + import("../../../tools/interactive-bash/tmux-path-resolver"), + import("./spawn-process"), + ]) + + if (!isInsideTmux()) { + log("[killTmuxSessionIfExists] SKIP: not inside tmux", { sessionName }) + return false + } + + const tmux = await getTmuxPath() + if (!tmux) { + log("[killTmuxSessionIfExists] SKIP: tmux not found", { sessionName }) + return false + } + + const hasSessionProcess = spawn([tmux, "has-session", "-t", sessionName], { + stdout: "ignore", + stderr: "ignore", + }) + + if ((await hasSessionProcess.exited) !== 0) { + log("[killTmuxSessionIfExists] SKIP: session not found", { sessionName }) + return false + } + + const killSessionProcess = spawn([tmux, "kill-session", "-t", sessionName], { + stdout: "pipe", + stderr: "pipe", + }) + const [, stderr, exitCode] = await Promise.all([ + readStream(killSessionProcess.stdout), + readStream(killSessionProcess.stderr), + killSessionProcess.exited, + ]) + + if (exitCode !== 0) { + log("[killTmuxSessionIfExists] FAILED", { sessionName, exitCode, stderr: stderr.trim() }) + return false + } + + log("[killTmuxSessionIfExists] SUCCESS", { sessionName }) + return true +} diff --git a/src/shared/tmux/tmux-utils/session-spawn.ts b/src/shared/tmux/tmux-utils/session-spawn.ts index db1feee29..a6fd15d2d 100644 --- a/src/shared/tmux/tmux-utils/session-spawn.ts +++ b/src/shared/tmux/tmux-utils/session-spawn.ts @@ -6,7 +6,11 @@ import { isInsideTmux } from "./environment" import { isServerRunning } from "./server-health" import { shellEscapeForDoubleQuotedCommand } from "../../shell-env" -const ISOLATED_SESSION_NAME = "omo-agents" +const ISOLATED_SESSION_NAME_PREFIX = "omo-agents" + +export function getIsolatedSessionName(pid: number = process.pid): string { + return `${ISOLATED_SESSION_NAME_PREFIX}-${pid}` +} async function getWindowDimensions( tmux: string, @@ -87,12 +91,13 @@ export async function spawnTmuxSession( } } - const sessionAlreadyExists = await sessionExists(tmux, ISOLATED_SESSION_NAME) + const isolatedSessionName = getIsolatedSessionName() + const sessionAlreadyExists = await sessionExists(tmux, isolatedSessionName) const args = sessionAlreadyExists ? [ "new-window", - "-t", ISOLATED_SESSION_NAME, + "-t", isolatedSessionName, "-P", "-F", "#{pane_id}", opencodeCmd, @@ -100,7 +105,7 @@ export async function spawnTmuxSession( : [ "new-session", "-d", - "-s", ISOLATED_SESSION_NAME, + "-s", isolatedSessionName, ...sizeArgs, "-P", "-F", "#{pane_id}", @@ -109,7 +114,7 @@ export async function spawnTmuxSession( log("[spawnTmuxSession] spawning", { mode: sessionAlreadyExists ? "new-window" : "new-session", - sessionName: ISOLATED_SESSION_NAME, + sessionName: isolatedSessionName, }) const proc = spawn([tmux, ...args], { stdout: "pipe", stderr: "pipe" }) @@ -140,6 +145,6 @@ export async function spawnTmuxSession( }) } - log("[spawnTmuxSession] SUCCESS", { paneId, sessionName: ISOLATED_SESSION_NAME }) + log("[spawnTmuxSession] SUCCESS", { paneId, sessionName: isolatedSessionName }) return { success: true, paneId } } diff --git a/src/shared/tmux/tmux-utils/spawn-process.ts b/src/shared/tmux/tmux-utils/spawn-process.ts new file mode 100644 index 000000000..c75826cab --- /dev/null +++ b/src/shared/tmux/tmux-utils/spawn-process.ts @@ -0,0 +1 @@ +export { spawn } from "bun"