fix(tmux-subagent): revert session.error cleanup (recoverable-error regression)
Oracle flagged a regression introduced in PR #3507 commit 21554be8:
event.ts routed session.error through tmux pane cleanup BEFORE the
existing session-recovery / model-fallback logic ran.
Problem: when session.error was recoverable (context window limit,
quota rate limit, provider fallback), the recovery/fallback code would
successfully continue the SAME session - but by then its tmux pane had
already been destroyed. User-visible symptom is exactly the original
complaint - 'screen appears but streaming stops working' after an
auto-retry.
Fix is the minimal revert: remove the onSessionError funnel from
event.ts and drop onSessionError from the manager. Fatal errors that
actually end a session still fire session.deleted, which continues to
trigger cleanup correctly. Non-fatal error streams stay attached to
the surviving pane.
This commit is contained in:
@@ -1932,43 +1932,6 @@ describe('TmuxSessionManager', () => {
|
||||
expect(mockKillTmuxSessionIfExists).toHaveBeenCalledTimes(0)
|
||||
})
|
||||
|
||||
test('#given a tracked session #when onSessionError is invoked #then the pane is closed like onSessionDeleted', async () => {
|
||||
// given
|
||||
mockIsInsideTmux.mockReturnValue(true)
|
||||
mockExecuteAction.mockClear()
|
||||
const { TmuxSessionManager } = await import('./manager')
|
||||
const manager = new TmuxSessionManager(createMockContext(), createTmuxConfig({
|
||||
enabled: true,
|
||||
isolation: 'session',
|
||||
}), mockTmuxDeps)
|
||||
await manager.onSessionCreated(createSessionCreatedEvent('ses_err', 'ses_parent', 'Errored Task'))
|
||||
mockExecuteAction.mockClear()
|
||||
|
||||
// when
|
||||
await manager.onSessionError({ sessionID: 'ses_err' })
|
||||
|
||||
// then
|
||||
expect(mockExecuteAction).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
test('#given an untracked session #when onSessionError is invoked #then it is a no-op and does not throw', async () => {
|
||||
// given
|
||||
mockIsInsideTmux.mockReturnValue(true)
|
||||
mockExecuteAction.mockClear()
|
||||
const { TmuxSessionManager } = await import('./manager')
|
||||
const manager = new TmuxSessionManager(createMockContext(), createTmuxConfig({
|
||||
enabled: true,
|
||||
isolation: 'session',
|
||||
}), mockTmuxDeps)
|
||||
|
||||
// when
|
||||
const errorHandler = manager.onSessionError({ sessionID: 'ses_unknown' })
|
||||
|
||||
// then
|
||||
await expect(errorHandler).resolves.toBeUndefined()
|
||||
expect(mockExecuteAction).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
test('#given killTmuxSessionIfExists throws #when cleanup runs #then cleanup still completes without throwing', async () => {
|
||||
// given
|
||||
mockKillTmuxSessionIfExists.mockClear()
|
||||
|
||||
@@ -838,18 +838,6 @@ export class TmuxSessionManager {
|
||||
await this.spawnQueue
|
||||
}
|
||||
|
||||
async onSessionError(event: { sessionID: string }): Promise<void> {
|
||||
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<void> {
|
||||
if (!this.isEnabled()) return
|
||||
if (!this.getEffectiveSourcePaneId()) return
|
||||
|
||||
@@ -615,10 +615,6 @@ export function createEventHandler(args: {
|
||||
const sessionID = props?.sessionID as string | undefined;
|
||||
const error = props?.error;
|
||||
|
||||
if (tmuxIntegrationEnabled && sessionID) {
|
||||
await managers.tmuxSessionManager.onSessionError({ sessionID });
|
||||
}
|
||||
|
||||
const errorName = extractErrorName(error);
|
||||
const errorMessage = extractErrorMessage(error);
|
||||
const errorInfo = { name: errorName, message: errorMessage };
|
||||
|
||||
Reference in New Issue
Block a user