From 77655a7c3023c2ca6331c391420893d415019a9c Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Tue, 28 Apr 2026 10:45:26 +0900 Subject: [PATCH] test(tmux-subagent): expand manager test coverage --- src/features/tmux-subagent/manager.test.ts | 588 ++++++++++++++++++--- 1 file changed, 511 insertions(+), 77 deletions(-) diff --git a/src/features/tmux-subagent/manager.test.ts b/src/features/tmux-subagent/manager.test.ts index 943f246fb..11f96d593 100644 --- a/src/features/tmux-subagent/manager.test.ts +++ b/src/features/tmux-subagent/manager.test.ts @@ -17,6 +17,11 @@ type SpawnTmuxContainerResult = { paneId?: string } +type SessionReadyWaitParams = { + client: unknown + sessionId: string +} + const mockQueryWindowState = mock<(paneId: string) => Promise>( async () => ({ windowWidth: 212, @@ -38,6 +43,13 @@ const mockExecuteAction = mock<( action: PaneAction, ctx: ExecuteContext ) => Promise>(async () => ({ success: true })) +const mockSpawnTmuxPane = mock(async (_sessionId?: string) => ({ + success: true, + paneId: '%mock', +})) +const mockWaitForSessionReady = mock<( + params: SessionReadyWaitParams, +) => Promise>(async () => true) const mockSpawnTmuxWindow = mock<( sessionId: string, description: string, @@ -65,49 +77,44 @@ const mockGetCurrentPaneId = mock<() => string | undefined>(() => '%0') const mockTmuxDeps: TmuxUtilDeps = { isInsideTmux: mockIsInsideTmux, getCurrentPaneId: mockGetCurrentPaneId, + queryWindowState: mockQueryWindowState, } -mock.module('./pane-state-querier', () => ({ - queryWindowState: mockQueryWindowState, - paneExists: mockPaneExists, - getRightmostAgentPane: (state: WindowState) => - state.agentPanes.length > 0 - ? state.agentPanes.reduce((r, p) => (p.left > r.left ? p : r)) - : null, - getOldestAgentPane: (state: WindowState) => - state.agentPanes.length > 0 - ? state.agentPanes.reduce((o, p) => (p.left < o.left ? p : o)) - : null, -})) +function registerModuleMocks(): void { + mock.module('./action-executor', () => ({ + executeActions: mockExecuteActions, + executeAction: mockExecuteAction, + executeActionWithDeps: mockExecuteAction, + })) + + mock.module('./session-ready-waiter', () => ({ + waitForSessionReady: mockWaitForSessionReady, + })) + + mock.module('../../shared/tmux', () => { + const { isInsideTmux, getCurrentPaneId } = require('../../shared/tmux/tmux-utils') + const { POLL_INTERVAL_BACKGROUND_MS, SESSION_TIMEOUT_MS, SESSION_MISSING_GRACE_MS } = require('../../shared/tmux/constants') + return { + isInsideTmux, + getCurrentPaneId, + POLL_INTERVAL_BACKGROUND_MS, + SESSION_TIMEOUT_MS, + SESSION_MISSING_GRACE_MS, + SESSION_READY_POLL_INTERVAL_MS: 100, + SESSION_READY_TIMEOUT_MS: 500, + spawnTmuxWindow: mockSpawnTmuxWindow, + spawnTmuxSession: mockSpawnTmuxSession, + killTmuxSessionIfExists: mockKillTmuxSessionIfExists, + getIsolatedSessionName: (pid: number = 12345) => `omo-agents-${pid}`, + sweepStaleOmoAgentSessions: mockSweepStaleOmoAgentSessions, + } + }) +} afterAll(() => { mock.restore() }) -mock.module('./action-executor', () => ({ - executeActions: mockExecuteActions, - executeAction: mockExecuteAction, - executeActionWithDeps: mockExecuteAction, -})) - -mock.module('../../shared/tmux', () => { - const { isInsideTmux, getCurrentPaneId } = require('../../shared/tmux/tmux-utils') - const { POLL_INTERVAL_BACKGROUND_MS, SESSION_TIMEOUT_MS, SESSION_MISSING_GRACE_MS } = require('../../shared/tmux/constants') - return { - isInsideTmux, - getCurrentPaneId, - POLL_INTERVAL_BACKGROUND_MS, - SESSION_TIMEOUT_MS, - SESSION_MISSING_GRACE_MS, - SESSION_READY_POLL_INTERVAL_MS: 100, - SESSION_READY_TIMEOUT_MS: 500, - spawnTmuxWindow: mockSpawnTmuxWindow, - spawnTmuxSession: mockSpawnTmuxSession, - killTmuxSessionIfExists: mockKillTmuxSessionIfExists, - getIsolatedSessionName: (pid: number = 12345) => `omo-agents-${pid}`, - sweepStaleOmoAgentSessions: mockSweepStaleOmoAgentSessions, - } -}) - const trackedSessions = new Set() +const readySessions = new Set() function createMockContext(overrides?: { sessionStatusResult?: { data?: Record } @@ -125,6 +132,9 @@ function createMockContext(overrides?: { for (const sessionId of trackedSessions) { data[sessionId] = { type: 'running' } } + for (const sessionId of readySessions) { + data[sessionId] = { type: 'running' } + } return { data } }), messages: mock(async () => { @@ -161,6 +171,28 @@ function createWindowState(overrides?: Partial): WindowState { } } +function createDeferred() { + let resolvePromise!: (value: TValue | PromiseLike) => void + let rejectPromise!: (reason?: unknown) => void + + const promise = new Promise((resolve, reject) => { + resolvePromise = resolve + rejectPromise = reject + }) + + return { + promise, + resolve: resolvePromise, + reject: rejectPromise, + } +} + +async function flushMicrotasks(turns: number = 5): Promise { + for (let index = 0; index < turns; index += 1) { + await Promise.resolve() + } +} + function createTmuxConfig(overrides?: Partial): TmuxConfig { return { enabled: true, @@ -177,29 +209,57 @@ function getTrackedSessions(manager: object): Map } +function getFailedReadinessSessions(manager: object): Map { + return Reflect.get(manager, 'failedReadinessSessions') as Map +} + describe('TmuxSessionManager', () => { beforeEach(() => { + mock.restore() + registerModuleMocks() mockQueryWindowState.mockClear() mockPaneExists.mockClear() mockExecuteActions.mockClear() mockExecuteAction.mockClear() + mockSpawnTmuxPane.mockClear() + mockWaitForSessionReady.mockClear() mockSpawnTmuxWindow.mockClear() mockSpawnTmuxSession.mockClear() mockIsInsideTmux.mockClear() mockGetCurrentPaneId.mockClear() trackedSessions.clear() + readySessions.clear() mockQueryWindowState.mockImplementation(async () => createWindowState()) - mockExecuteActions.mockImplementation(async (actions: PaneAction[]) => { for (const action of actions) { - if (action.type === 'spawn') { - trackedSessions.add(action.sessionId) + mockExecuteActions.mockImplementation(async (actions: PaneAction[]) => { + const results: ExecuteActionsResult['results'] = [] + let spawnedPaneId: string | undefined + + for (const action of actions) { + if (action.type === 'spawn') { + const spawnResult = await mockSpawnTmuxPane(action.sessionId) + if (!spawnResult.success) { + return { + success: false, + results: [{ action, result: { success: false, error: 'spawn failed' } }], + } + } + trackedSessions.add(action.sessionId) + spawnedPaneId = spawnResult.paneId + results.push({ action, result: { success: true, paneId: spawnResult.paneId } }) + } } - } - return { - success: true, - spawnedPaneId: '%mock', - results: [], - } }) + + return { + success: true, + spawnedPaneId: spawnedPaneId ?? '%mock', + results, + } + }) + mockWaitForSessionReady.mockImplementation(async ({ sessionId }: SessionReadyWaitParams) => { + readySessions.add(sessionId) + return true + }) mockSpawnTmuxWindow.mockImplementation(async (sessionId: string) => { trackedSessions.add(sessionId) return { @@ -382,6 +442,50 @@ describe('TmuxSessionManager', () => { }) }) + describe('getServerUrl', () => { + test('returns normalized serverUrl from ctx', async () => { + // given + mockIsInsideTmux.mockReturnValue(true) + const { TmuxSessionManager } = await import('./manager') + const ctx = { + ...createMockContext(), + serverUrl: new URL('http://127.0.0.1:12345/'), + } + const config = createTmuxConfig({ enabled: true }) + const manager = new TmuxSessionManager(ctx, config, mockTmuxDeps) + + // when + const serverUrl = manager.getServerUrl() + + // then + expect(serverUrl).toBe('http://127.0.0.1:12345/') + }) + + test('returns fallback when port is 0', async () => { + // given + mockIsInsideTmux.mockReturnValue(true) + const originalPort = process.env.OPENCODE_PORT + delete process.env.OPENCODE_PORT + const { TmuxSessionManager } = await import('./manager') + const ctx = { + ...createMockContext(), + serverUrl: new URL('http://127.0.0.1:0/'), + } + const config = createTmuxConfig({ enabled: true }) + const manager = new TmuxSessionManager(ctx, config, mockTmuxDeps) + + // when + const serverUrl = manager.getServerUrl() + + // then + try { + expect(serverUrl).toBe(`http://localhost:${process.env.OPENCODE_PORT ?? '4096'}`) + } finally { + if (originalPort !== undefined) process.env.OPENCODE_PORT = originalPort + } + }) + }) + describe('onSessionCreated', () => { test('first agent spawns from source pane via decision engine', async () => { // given @@ -867,6 +971,86 @@ describe('TmuxSessionManager', () => { expect((manager as any).deferredQueue).toEqual([]) }) + test('skips deferred attach when the session is already pending through another spawn path', async () => { + // given + mockIsInsideTmux.mockReturnValue(true) + mockQueryWindowState.mockImplementation(async () => + createWindowState({ + windowWidth: 160, + windowHeight: 11, + agentPanes: [ + { + paneId: '%1', + width: 80, + height: 11, + left: 80, + top: 0, + title: 'old', + isActive: false, + }, + ], + }) + ) + + const { TmuxSessionManager } = await import('./manager') + const manager = new TmuxSessionManager(createMockContext(), createTmuxConfig({ enabled: true }), mockTmuxDeps) + + await manager.onSessionCreated( + createSessionCreatedEvent('ses_pending_race', 'ses_parent', 'Pending Race Task') + ) + expect((manager as any).deferredQueue).toEqual(['ses_pending_race']) + + mockQueryWindowState.mockImplementation(async () => createWindowState()) + Reflect.get(manager, 'pendingSessions').add('ses_pending_race') + + // when + await Reflect.get(manager, 'tryAttachDeferredSession').call(manager) + + // then + expect(mockSpawnTmuxPane).toHaveBeenCalledTimes(0) + expect((manager as any).deferredQueue).toEqual(['ses_pending_race']) + }) + + test('drops deferred sessions that were already closed by polling', async () => { + // given + mockIsInsideTmux.mockReturnValue(true) + mockQueryWindowState.mockImplementation(async () => + createWindowState({ + windowWidth: 160, + windowHeight: 11, + agentPanes: [ + { + paneId: '%1', + width: 80, + height: 11, + left: 80, + top: 0, + title: 'old', + isActive: false, + }, + ], + }) + ) + + const { TmuxSessionManager } = await import('./manager') + const manager = new TmuxSessionManager(createMockContext(), createTmuxConfig({ enabled: true }), mockTmuxDeps) + + await manager.onSessionCreated( + createSessionCreatedEvent('ses_bounce', 'ses_parent', 'Bounce Task') + ) + expect((manager as any).deferredQueue).toEqual(['ses_bounce']) + + mockQueryWindowState.mockImplementation(async () => createWindowState()) + Reflect.set(manager, 'closedByPolling', new Set(['ses_bounce'])) + + // when + await Reflect.get(manager, 'tryAttachDeferredSession').call(manager) + + // then + expect(mockSpawnTmuxPane).toHaveBeenCalledTimes(0) + expect((manager as any).deferredQueue).toEqual([]) + }) + test('removes deferred session when session is deleted before attach', async () => { // given mockIsInsideTmux.mockReturnValue(true) @@ -1137,55 +1321,304 @@ describe('TmuxSessionManager', () => { }) }) - test('#given session.status never reports session ready #when onSessionCreated runs #then pane is tracked immediately without blocking', async () => { + test('#given session readiness is pending #when onSessionCreated runs #then pane spawn waits until readiness resolves', async () => { // given mockIsInsideTmux.mockReturnValue(true) mockQueryWindowState.mockImplementation(async () => createWindowState()) + const readiness = createDeferred() + mockWaitForSessionReady.mockImplementationOnce(async ({ sessionId }: SessionReadyWaitParams) => { + const ready = await readiness.promise + if (ready) { + readySessions.add(sessionId) + } + return ready + }) const { TmuxSessionManager } = await import('./manager') - const ctx = createMockContext({ sessionStatusResult: { data: {} } }) + const ctx = createMockContext() const config = createTmuxConfig({ enabled: true }) const manager = new TmuxSessionManager(ctx, config, mockTmuxDeps) - const event = createSessionCreatedEvent('ses_fast_track', 'ses_parent', 'Fast Track') + const event = createSessionCreatedEvent('ses_wait', 'ses_parent', 'Wait For Ready') // when - const start = Date.now() - await manager.onSessionCreated(event) - const elapsed = Date.now() - start + const onSessionCreatedPromise = manager.onSessionCreated(event) + await flushMicrotasks() // then - expect(elapsed < 500).toBe(true) - expect(getTrackedSessions(manager).has('ses_fast_track')).toBe(true) + expect(mockWaitForSessionReady).toHaveBeenCalledTimes(1) + expect(mockExecuteActions).toHaveBeenCalledTimes(0) + expect(mockSpawnTmuxPane).toHaveBeenCalledTimes(0) + + // when + readiness.resolve(true) + await onSessionCreatedPromise + + // then + expect(mockExecuteActions).toHaveBeenCalledTimes(1) + expect(mockSpawnTmuxPane).toHaveBeenCalledTimes(1) + expect(getTrackedSessions(manager).has('ses_wait')).toBe(true) + }) + + test('#given readiness probe fails #when onSessionCreated runs #then it logs the structured error and does not spawn a pane', async () => { + // given + mockIsInsideTmux.mockReturnValue(true) + const readinessError = new Error('session readiness timed out') + mockWaitForSessionReady.mockImplementationOnce(async () => { + throw readinessError + }) + const logSpy = spyOn(sharedModule, 'log').mockImplementation(() => {}) + + const { TmuxSessionManager } = await import('./manager') + const manager = new TmuxSessionManager(createMockContext(), createTmuxConfig({ enabled: true }), mockTmuxDeps) + + // when + await manager.onSessionCreated( + createSessionCreatedEvent('ses_timeout', 'ses_parent', 'Timeout Task') + ) + + // then + expect(mockExecuteActions).toHaveBeenCalledTimes(0) + expect(mockSpawnTmuxPane).toHaveBeenCalledTimes(0) + expect(logSpy).toHaveBeenCalledWith( + '[tmux-session-manager] session readiness failed before spawn', + expect.objectContaining({ + sessionId: 'ses_timeout', + stage: 'session.created', + error: String(readinessError), + }), + ) + + logSpy.mockRestore() + }) + + test("skips pane creation when session exists but status is 'error'", async () => { + // given + mockIsInsideTmux.mockReturnValue(true) + mockWaitForSessionReady.mockImplementationOnce(async () => true) + const logSpy = spyOn(sharedModule, 'log').mockImplementation(() => {}) + const sessionStatusResult = { + data: { + ses_error: { type: 'error' }, + }, + } + + const { TmuxSessionManager } = await import('./manager') + const manager = new TmuxSessionManager( + createMockContext({ sessionStatusResult }), + createTmuxConfig({ enabled: true }), + mockTmuxDeps, + ) + + // when + await manager.onSessionCreated( + createSessionCreatedEvent('ses_error', 'ses_parent', 'Errored Session') + ) + + // then + expect(mockExecuteActions).toHaveBeenCalledTimes(0) + expect(mockSpawnTmuxPane).toHaveBeenCalledTimes(0) + expect(getTrackedSessions(manager).has('ses_error')).toBe(false) + expect(getFailedReadinessSessions(manager).has('ses_error')).toBe(true) + expect(logSpy).toHaveBeenCalledWith( + '[tmux-session-manager] session not attachable for pane spawn', + expect.objectContaining({ + sessionId: 'ses_error', + stage: 'session.created', + status: 'error', + }), + ) + + logSpy.mockRestore() + }) + + test('retries pane creation on session.idle after a readiness timeout when status becomes attachable', async () => { + // given + mockIsInsideTmux.mockReturnValue(true) + const readinessError = new Error('session readiness timed out') + mockWaitForSessionReady + .mockImplementationOnce(async () => { + throw readinessError + }) + .mockImplementationOnce(async () => true) + const sessionStatusResult = { + data: {} as Record, + } + + const { TmuxSessionManager } = await import('./manager') + const manager = new TmuxSessionManager( + createMockContext({ sessionStatusResult }), + createTmuxConfig({ enabled: true }), + mockTmuxDeps, + ) + + // when + await manager.onSessionCreated( + createSessionCreatedEvent('ses_retry', 'ses_parent', 'Retry Session') + ) + + // then + expect(mockSpawnTmuxPane).toHaveBeenCalledTimes(0) + expect(getFailedReadinessSessions(manager).has('ses_retry')).toBe(true) + + // when + sessionStatusResult.data.ses_retry = { type: 'idle' } + manager.onEvent({ type: 'session.idle', properties: { sessionID: 'ses_retry' } }) + await flushMicrotasks(20) + + // then + expect(mockSpawnTmuxPane).toHaveBeenCalledTimes(1) + expect(getTrackedSessions(manager).has('ses_retry')).toBe(true) + expect(getFailedReadinessSessions(manager).has('ses_retry')).toBe(false) + }) + + test('does not retry more than once per sessionID', async () => { + // given + mockIsInsideTmux.mockReturnValue(true) + mockWaitForSessionReady + .mockImplementationOnce(async () => { + throw new Error('session readiness timed out') + }) + .mockImplementationOnce(async () => true) + const sessionStatusResult = { + data: { + ses_retry_once: { type: 'idle' }, + }, + } + + const { TmuxSessionManager } = await import('./manager') + const manager = new TmuxSessionManager( + createMockContext({ sessionStatusResult }), + createTmuxConfig({ enabled: true }), + mockTmuxDeps, + ) + + // when + await manager.onSessionCreated( + createSessionCreatedEvent('ses_retry_once', 'ses_parent', 'Retry Once Session') + ) + manager.onEvent({ type: 'session.idle', properties: { sessionID: 'ses_retry_once' } }) + await flushMicrotasks(20) + + // then + expect(mockSpawnTmuxPane).toHaveBeenCalledTimes(1) + expect(getFailedReadinessSessions(manager).has('ses_retry_once')).toBe(false) + + // when + manager.onEvent({ type: 'session.idle', properties: { sessionID: 'ses_retry_once' } }) + await flushMicrotasks(20) + + // then + expect(mockSpawnTmuxPane).toHaveBeenCalledTimes(1) + }) + + test('expires failed readiness sessions after the TTL elapses', async () => { + // given + mockIsInsideTmux.mockReturnValue(true) + const nowSpy = spyOn(Date, 'now') + nowSpy.mockReturnValue(0) + mockWaitForSessionReady.mockImplementationOnce(async () => { + throw new Error('session readiness timed out') + }) + + const { TmuxSessionManager } = await import('./manager') + const manager = new TmuxSessionManager( + createMockContext({ sessionStatusResult: { data: { ses_expired: { type: 'idle' } } } }), + createTmuxConfig({ enabled: true }), + mockTmuxDeps, + ) + + await manager.onSessionCreated( + createSessionCreatedEvent('ses_expired', 'ses_parent', 'Expired Retry Session') + ) + expect(getFailedReadinessSessions(manager).has('ses_expired')).toBe(true) + + // when + nowSpy.mockReturnValue(5 * 60 * 1000 + 1) + manager.onEvent({ type: 'session.idle', properties: { sessionID: 'ses_expired' } }) + await flushMicrotasks(20) + + // then + expect(mockSpawnTmuxPane).toHaveBeenCalledTimes(0) + expect(getFailedReadinessSessions(manager).has('ses_expired')).toBe(false) + + nowSpy.mockRestore() + }) + + test('does not retry failed readiness sessions after polling marked the session closed', async () => { + // given + mockIsInsideTmux.mockReturnValue(true) + + const { TmuxSessionManager } = await import('./manager') + const manager = new TmuxSessionManager( + createMockContext({ sessionStatusResult: { data: { ses_bounce: { type: 'idle' } } } }), + createTmuxConfig({ enabled: true }), + mockTmuxDeps, + ) + + Reflect.get(manager, 'failedReadinessSessions').set('ses_bounce', { + sessionId: 'ses_bounce', + title: 'Bounce Session', + rememberedAt: Date.now(), + }) + Reflect.set(manager, 'closedByPolling', new Set(['ses_bounce'])) + + // when + manager.onEvent({ type: 'session.idle', properties: { sessionID: 'ses_bounce' } }) + await flushMicrotasks(20) + + // then + expect(mockSpawnTmuxPane).toHaveBeenCalledTimes(0) + expect(getFailedReadinessSessions(manager).has('ses_bounce')).toBe(true) + }) + + test('#given duplicate session.created triggers while readiness is pending #when readiness resolves #then only one pane spawn runs', async () => { + // given + mockIsInsideTmux.mockReturnValue(true) + const readiness = createDeferred() + mockWaitForSessionReady.mockImplementationOnce(async ({ sessionId }: SessionReadyWaitParams) => { + const ready = await readiness.promise + if (ready) { + readySessions.add(sessionId) + } + return ready + }) + + const { TmuxSessionManager } = await import('./manager') + const manager = new TmuxSessionManager(createMockContext(), createTmuxConfig({ enabled: true }), mockTmuxDeps) + const event = createSessionCreatedEvent('ses_dup_pending', 'ses_parent', 'Duplicate Pending') + + // when + const firstSpawnPromise = manager.onSessionCreated(event) + const secondSpawnPromise = manager.onSessionCreated(event) + await flushMicrotasks() + + // then + expect(mockWaitForSessionReady).toHaveBeenCalledTimes(1) + expect(mockExecuteActions).toHaveBeenCalledTimes(0) + expect(mockSpawnTmuxPane).toHaveBeenCalledTimes(0) + + // when + readiness.resolve(true) + await Promise.all([firstSpawnPromise, secondSpawnPromise]) + + // then + expect(mockWaitForSessionReady).toHaveBeenCalledTimes(1) + expect(mockExecuteActions).toHaveBeenCalledTimes(1) + expect(mockSpawnTmuxPane).toHaveBeenCalledTimes(1) + expect(getTrackedSessions(manager).has('ses_dup_pending')).toBe(true) }) }) describe('onSessionDeleted', () => { - test('does not track session when readiness timed out', async () => { + test('does nothing when session creation stopped before tracking due to readiness failure', async () => { // given mockIsInsideTmux.mockReturnValue(true) - let stateCallCount = 0 - mockQueryWindowState.mockImplementation(async () => { - stateCallCount++ - if (stateCallCount === 1) { - return createWindowState() - } - return createWindowState({ - agentPanes: [ - { - paneId: '%mock', - width: 40, - height: 44, - left: 100, - top: 0, - title: 'omo-subagent-Timeout Task', - isActive: false, - }, - ], - }) + mockWaitForSessionReady.mockImplementationOnce(async () => { + throw new Error('readiness failed') }) const { TmuxSessionManager } = await import('./manager') - const ctx = createMockContext({ sessionStatusResult: { data: {} } }) + const ctx = createMockContext() const config = createTmuxConfig({ enabled: true, layout: 'main-vertical', main_pane_size: 60, @@ -1202,7 +1635,7 @@ describe('TmuxSessionManager', () => { await manager.onSessionDeleted({ sessionID: 'ses_timeout' }) // then - expect(mockExecuteAction).toHaveBeenCalledTimes(1) + expect(mockExecuteAction).toHaveBeenCalledTimes(0) }) test('closes pane when tracked session is deleted', async () => { @@ -2064,7 +2497,8 @@ describe('TmuxSessionManager', () => { const cleanupPromise = manager.cleanup() // then - expect(await cleanupPromise).toBeUndefined() + const cleanupResult = await cleanupPromise + expect(cleanupResult).toBeUndefined() expect(mockKillTmuxSessionIfExists).toHaveBeenCalledTimes(1) }) })