From 7a7926f2220790ebe346ce9828fa8b5497db87bf Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 19:30:44 +0900 Subject: [PATCH 1/9] chore(tmux-subagent): remove dead event-handler modules Drop cleanup.ts, session-created-handler.ts, and session-deleted-handler.ts which were never wired up; the lifecycle logic they contained lives inline in TmuxSessionManager. Barrels trimmed to match. --- src/features/tmux-subagent/cleanup.ts | 42 ----- src/features/tmux-subagent/event-handlers.ts | 4 - src/features/tmux-subagent/index.ts | 3 - .../tmux-subagent/session-created-handler.ts | 175 ------------------ .../tmux-subagent/session-deleted-handler.ts | 50 ----- 5 files changed, 274 deletions(-) delete mode 100644 src/features/tmux-subagent/cleanup.ts delete mode 100644 src/features/tmux-subagent/session-created-handler.ts delete mode 100644 src/features/tmux-subagent/session-deleted-handler.ts 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/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() - } -} From 2a99a524ea99ac3eba5717da3bd0e4c5b8bc53fe Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 19:31:04 +0900 Subject: [PATCH 2/9] fix(tmux): drain kill-pane stdout to prevent pipe backpressure hang closeTmuxPane spawned kill-pane with stdout: "pipe" but never drained the stream, which could leave the subprocess hanging indefinitely when tmux wrote anything to stdout (for example under --force-close race conditions). - send-keys now uses stdout: "ignore" so there is no pipe to drain - kill-pane keeps the pipe but drains stdout/stderr alongside proc.exited - switch imports to the new spawn-process helper so the behavior is covered by hermetic tests that mock the spawn boundary --- src/shared/tmux/tmux-utils/pane-close.test.ts | 197 ++++++++++++++++++ src/shared/tmux/tmux-utils/pane-close.ts | 28 ++- src/shared/tmux/tmux-utils/spawn-process.ts | 1 + 3 files changed, 216 insertions(+), 10 deletions(-) create mode 100644 src/shared/tmux/tmux-utils/pane-close.test.ts create mode 100644 src/shared/tmux/tmux-utils/spawn-process.ts 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..ca5d74684 --- /dev/null +++ b/src/shared/tmux/tmux-utils/pane-close.test.ts @@ -0,0 +1,197 @@ +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 #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 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..76d9dd11b 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,12 +36,15 @@ 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() }) 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" From de8a0167e61876aa30467d52641bd9094c8f0f97 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 19:31:22 +0900 Subject: [PATCH 3/9] feat(tmux): add killTmuxSessionIfExists utility for explicit session teardown Adds killTmuxSessionIfExists(sessionName), a best-effort no-op when the named session is absent. Drains both stdio streams so it does not leak pipe buffers the way closeTmuxPane historically did. Also exports ISOLATED_SESSION_NAME ("omo-agents") from session-spawn so callers can tear down the shared isolated session without hard-coding the name in multiple places. --- src/shared/tmux/tmux-utils.ts | 3 +- src/shared/tmux/tmux-utils/index.ts | 1 + .../tmux/tmux-utils/session-kill.test.ts | 173 ++++++++++++++++++ src/shared/tmux/tmux-utils/session-kill.ts | 51 ++++++ src/shared/tmux/tmux-utils/session-spawn.ts | 2 +- 5 files changed, 228 insertions(+), 2 deletions(-) create mode 100644 src/shared/tmux/tmux-utils/index.ts create mode 100644 src/shared/tmux/tmux-utils/session-kill.test.ts create mode 100644 src/shared/tmux/tmux-utils/session-kill.ts diff --git a/src/shared/tmux/tmux-utils.ts b/src/shared/tmux/tmux-utils.ts index a9aab095a..587704536 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, ISOLATED_SESSION_NAME } 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/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..dd9f5addd 100644 --- a/src/shared/tmux/tmux-utils/session-spawn.ts +++ b/src/shared/tmux/tmux-utils/session-spawn.ts @@ -6,7 +6,7 @@ import { isInsideTmux } from "./environment" import { isServerRunning } from "./server-health" import { shellEscapeForDoubleQuotedCommand } from "../../shell-env" -const ISOLATED_SESSION_NAME = "omo-agents" +export const ISOLATED_SESSION_NAME = "omo-agents" async function getWindowDimensions( tmux: string, From 21554be8709fc73b1a9dd19ac248bb5b86621167 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 19:31:45 +0900 Subject: [PATCH 4/9] fix(tmux-subagent): tighten serve/attach cleanup paths so panes and sessions are torn down reliably Three defects observed with tmux.isolation="session" where the omo-agents session was left with orphan fish panes after subagents finished: 1. cleanup() never ran 'tmux kill-session -t omo-agents'. If any pane lingered (for example because opencode attach stayed blocked on SSE), the isolated session survived process shutdown. Now we explicitly kill the shared session through killTmuxSessionIfExists when isolation is "session". 2. session.error events bypassed tmux cleanup entirely. Only session.deleted closed panes, so any provider error that did not escalate into a delete left the pane behind. Added onSessionError on TmuxSessionManager, wired from plugin/event.ts, which funnels through the same onSessionDeleted close path for tracked sessions only. 3. retryPendingCloses() only ran when a new session was created. If the main process went idle after a failed close, the pending session stayed pending forever. TmuxPollingManager now accepts the retry callback and fires it on every tick, alongside the existing stability-based close sweep. Manager tests cover isolation=session kill invocation, inline/window isolation skipping the kill, the onSessionError happy + untracked paths, and an isolated-session kill failure that must not break cleanup. --- src/features/tmux-subagent/manager.test.ts | 109 ++++++++++++++++++ src/features/tmux-subagent/manager.ts | 32 ++++- src/features/tmux-subagent/polling-manager.ts | 11 +- src/plugin/event.ts | 4 + 4 files changed, 154 insertions(+), 2 deletions(-) diff --git a/src/features/tmux-subagent/manager.test.ts b/src/features/tmux-subagent/manager.test.ts index c8cf2c1d2..f828ff9d7 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, + ISOLATED_SESSION_NAME: 'omo-agents', } }) @@ -1852,6 +1855,112 @@ describe('TmuxSessionManager', () => { // then expect(mockExecuteAction).toHaveBeenCalledTimes(2) }) + + test('#given tmux isolation is "session" #when cleanup runs #then killTmuxSessionIfExists is invoked for the 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).toHaveBeenCalledWith('omo-agents') + }) + + 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..ee1b8b508 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, + ISOLATED_SESSION_NAME, } 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,21 @@ export class TmuxSessionManager { this.isolatedContainerPaneId = undefined this.isolatedWindowPaneId = undefined + if (this.tmuxConfig.isolation === "session") { + try { + const killed = await killTmuxSessionIfExists(ISOLATED_SESSION_NAME) + log("[tmux-session-manager] isolated session teardown", { + session: ISOLATED_SESSION_NAME, + killed, + }) + } catch (error) { + log("[tmux-session-manager] isolated session teardown failed", { + session: ISOLATED_SESSION_NAME, + 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/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 }; From b9d2acdcf9b49067ad23a8f0dd8d8ceb62333439 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 19:32:04 +0900 Subject: [PATCH 5/9] fix(background-agent): run manager cleanup on uncaughtException and unhandledRejection Signal handlers covered SIGINT/SIGTERM/SIGBREAK/beforeExit/exit, but a synchronous throw or a top-level rejected promise terminated the process without letting TmuxSessionManager (or any other registered manager) run its shutdown hook. That reliably left orphan tmux panes after an opencode crash. Added registration for uncaughtException and unhandledRejection that fan out through the existing cleanupAll() path, set process.exitCode = 1, and arm the same 6 second forced-exit guard we use for signals. Test helpers hold process-level spies so the new tests do not leak listeners between runs. --- .../process-cleanup.test-helpers.ts | 27 ++++ .../background-agent/process-cleanup.test.ts | 148 ++++++++++++++---- .../background-agent/process-cleanup.ts | 56 +++++-- 3 files changed, 183 insertions(+), 48 deletions(-) create mode 100644 src/features/background-agent/process-cleanup.test-helpers.ts 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..4d2975fe0 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,36 +7,12 @@ 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 @@ -92,13 +70,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 +89,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 +134,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 +199,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 } From ea4f3c81f47d24ca25f927a6cd0be11506c0845c Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 19:34:54 +0900 Subject: [PATCH 6/9] fix(tmux): treat pane-already-closed as success in closeTmuxPane After send-keys C-c the subprocess running inside the pane (for example "opencode attach") exits on SIGINT, which causes tmux to destroy the pane automatically. The subsequent kill-pane then returns exit 1 with stderr "can't find pane: %NN" even though the end state is exactly what we wanted. Before this fix closeTmuxPane reported failure for that branch, which kept TmuxSessionManager's retryPendingCloses loop marking the (now deleted) pane as still-pending forever and left stale entries behind in the tracked sessions map. This is the behavior the user observed as "screen opens, streaming runs, but cleanup doesn't finish" when running with tmux.isolation="session". Now we detect the "can't find pane" stderr and return true, treating the auto-destroy path the same as an explicit successful kill. --- src/shared/tmux/tmux-utils/pane-close.test.ts | 26 ++++++++++++++++++- src/shared/tmux/tmux-utils/pane-close.ts | 18 +++++++++---- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/src/shared/tmux/tmux-utils/pane-close.test.ts b/src/shared/tmux/tmux-utils/pane-close.test.ts index ca5d74684..b8d5d7887 100644 --- a/src/shared/tmux/tmux-utils/pane-close.test.ts +++ b/src/shared/tmux/tmux-utils/pane-close.test.ts @@ -170,7 +170,7 @@ describe("closeTmuxPane", () => { expect(spawnCalls).toHaveLength(0) }) - it("#given kill-pane fails #when closeTmuxPane called #then returns false", async () => { + 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)) @@ -182,6 +182,30 @@ describe("closeTmuxPane", () => { 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() diff --git a/src/shared/tmux/tmux-utils/pane-close.ts b/src/shared/tmux/tmux-utils/pane-close.ts index 76d9dd11b..e62e46296 100644 --- a/src/shared/tmux/tmux-utils/pane-close.ts +++ b/src/shared/tmux/tmux-utils/pane-close.ts @@ -46,11 +46,19 @@ export async function closeTmuxPane(paneId: string): Promise { 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 } From f8a1a11bb7ce56b5f8a453b8d5ac4fe82eef3c9d Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 19:42:19 +0900 Subject: [PATCH 7/9] fix(team-mode): refactor layout to use testable spawn-process helper The existing layout.test.ts relied on mock.module("bun", ...) registered at the top level, but test-setup.ts calls mock.restore() + restoreModuleMocks() in afterEach, so every test except the first one lost its mocks. CI has been red on this file since e303feef. Two changes: 1. layout.ts now imports spawn from the existing spawn-process helper instead of "bun" directly, matching the pattern established for closeTmuxPane and killTmuxSessionIfExists. This does not change runtime behavior - spawn-process just re-exports Bun's spawn. 2. layout.test.ts registers module mocks inside beforeEach and uses the ?test=UUID cache-busting dynamic-import pattern so the mocks apply on every test run, not just the first. All 4 layout.test.ts cases now pass. --- .../team-mode/team-layout-tmux/layout.test.ts | 26 +++++++++++++++---- .../team-mode/team-layout-tmux/layout.ts | 2 +- 2 files changed, 22 insertions(+), 6 deletions(-) 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" From 257b6cf951d442a4d5c451b33c77bae0b919cdce Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 19:47:36 +0900 Subject: [PATCH 8/9] fix(tmux): scope isolated session name per plugin instance (Oracle review) Oracle flagged the previous commit: "omo-agents" was a shared constant, so when two plugin instances ran in the same tmux server they wrote into the same session. One instance's cleanup would then kill-session on the shared name and tear down the other instance's live attached panes. Replace the const ISOLATED_SESSION_NAME with getIsolatedSessionName(pid) which defaults to process.pid, so every opencode process owns its own "omo-agents-" session. spawnTmuxSession and cleanup both resolve the name through this helper. Discovery is straightforward from the host tmux via 'tmux list-sessions | grep omo-agents-'. Manager test covers two concurrent managers and asserts each kills a per-pid session name, proving they no longer collide on a global name. --- src/features/tmux-subagent/manager.test.ts | 31 +++++++++++++++++++-- src/features/tmux-subagent/manager.ts | 9 +++--- src/shared/tmux/tmux-utils.ts | 2 +- src/shared/tmux/tmux-utils/session-spawn.ts | 17 +++++++---- 4 files changed, 45 insertions(+), 14 deletions(-) diff --git a/src/features/tmux-subagent/manager.test.ts b/src/features/tmux-subagent/manager.test.ts index f828ff9d7..9a03d7641 100644 --- a/src/features/tmux-subagent/manager.test.ts +++ b/src/features/tmux-subagent/manager.test.ts @@ -101,7 +101,7 @@ mock.module('../../shared/tmux', () => { spawnTmuxWindow: mockSpawnTmuxWindow, spawnTmuxSession: mockSpawnTmuxSession, killTmuxSessionIfExists: mockKillTmuxSessionIfExists, - ISOLATED_SESSION_NAME: 'omo-agents', + getIsolatedSessionName: (pid: number = 12345) => `omo-agents-${pid}`, } }) @@ -1856,7 +1856,7 @@ describe('TmuxSessionManager', () => { expect(mockExecuteAction).toHaveBeenCalledTimes(2) }) - test('#given tmux isolation is "session" #when cleanup runs #then killTmuxSessionIfExists is invoked for the isolated session', async () => { + 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') @@ -1870,7 +1870,32 @@ describe('TmuxSessionManager', () => { // then expect(mockKillTmuxSessionIfExists).toHaveBeenCalledTimes(1) - expect(mockKillTmuxSessionIfExists).toHaveBeenCalledWith('omo-agents') + 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 () => { diff --git a/src/features/tmux-subagent/manager.ts b/src/features/tmux-subagent/manager.ts index ee1b8b508..9d9c65159 100644 --- a/src/features/tmux-subagent/manager.ts +++ b/src/features/tmux-subagent/manager.ts @@ -11,7 +11,7 @@ import { spawnTmuxWindow, spawnTmuxSession, killTmuxSessionIfExists, - ISOLATED_SESSION_NAME, + getIsolatedSessionName, } from "../../shared/tmux" import { queryWindowState } from "./pane-state-querier" import { decideSpawnActions, decideCloseAction, type SessionMapping } from "./decision-engine" @@ -970,15 +970,16 @@ export class TmuxSessionManager { this.isolatedWindowPaneId = undefined if (this.tmuxConfig.isolation === "session") { + const isolatedSessionName = getIsolatedSessionName() try { - const killed = await killTmuxSessionIfExists(ISOLATED_SESSION_NAME) + const killed = await killTmuxSessionIfExists(isolatedSessionName) log("[tmux-session-manager] isolated session teardown", { - session: ISOLATED_SESSION_NAME, + session: isolatedSessionName, killed, }) } catch (error) { log("[tmux-session-manager] isolated session teardown failed", { - session: ISOLATED_SESSION_NAME, + session: isolatedSessionName, error: String(error), }) } diff --git a/src/shared/tmux/tmux-utils.ts b/src/shared/tmux/tmux-utils.ts index 587704536..d80d1e720 100644 --- a/src/shared/tmux/tmux-utils.ts +++ b/src/shared/tmux/tmux-utils.ts @@ -10,7 +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, ISOLATED_SESSION_NAME } 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/session-spawn.ts b/src/shared/tmux/tmux-utils/session-spawn.ts index dd9f5addd..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" -export 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 } } From aa79284dc56860cec0ab837251533625f2cb1d34 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 19:57:26 +0900 Subject: [PATCH 9/9] fix(background-agent): reset process.exitCode to 0 between cleanup tests CI test suite exited 1 despite 0 failing tests because process-cleanup.test.ts assertions left process.exitCode=1 in place. The afterEach hook only reset to originalExitCode (which starts undefined), not 0, so Bun picked up exitCode=1 on shutdown and reported the shared batch as failing. Explicitly set process.exitCode = 0 in beforeEach and afterEach so each test starts and ends with a clean exit state. --- src/features/background-agent/process-cleanup.test.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/features/background-agent/process-cleanup.test.ts b/src/features/background-agent/process-cleanup.test.ts index 4d2975fe0..a9ce35435 100644 --- a/src/features/background-agent/process-cleanup.test.ts +++ b/src/features/background-agent/process-cleanup.test.ts @@ -15,10 +15,9 @@ type CleanupManager = { describe("#given process cleanup registration", () => { const registeredManagers: CleanupManager[] = [] - const originalExitCode = process.exitCode beforeEach(() => { - process.exitCode = originalExitCode + process.exitCode = 0 registeredManagers.length = 0 _resetForTesting() }) @@ -28,7 +27,7 @@ describe("#given process cleanup registration", () => { unregisterManagerForCleanup(manager) } - process.exitCode = originalExitCode + process.exitCode = 0 _resetForTesting() })