fix(tmux-subagent): retry stale sweep if first attempt throws
Oracle flagged: staleSweepCompleted was set to true BEFORE
sweepStaleOmoAgentSessions() ran, so any throw from the first
invocation would permanently disable stale cleanup for the rest
of the process lifetime.
Fix:
- Move staleSweepCompleted=true into the try-block success branch.
- Add staleSweepInProgress guard so concurrent onSessionCreated calls
do not invoke sweep twice in parallel (sweep is idempotent, but the
guard prevents doubled log noise).
- finally{} clears the inProgress flag regardless of outcome.
- cleanup() resets both flags.
Two new tests cover: retry after a thrown first attempt, and single
invocation when subsequent spawns follow a successful first sweep.
This commit is contained in:
@@ -1932,6 +1932,47 @@ describe('TmuxSessionManager', () => {
|
|||||||
expect(mockKillTmuxSessionIfExists).toHaveBeenCalledTimes(0)
|
expect(mockKillTmuxSessionIfExists).toHaveBeenCalledTimes(0)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test('#given sweepStaleOmoAgentSessions throws on first onSessionCreated #when second onSessionCreated fires #then sweep is retried instead of skipped forever', async () => {
|
||||||
|
// given
|
||||||
|
mockSweepStaleOmoAgentSessions.mockClear()
|
||||||
|
mockSweepStaleOmoAgentSessions.mockImplementationOnce(async () => {
|
||||||
|
throw new Error('simulated sweep failure')
|
||||||
|
})
|
||||||
|
mockIsInsideTmux.mockReturnValue(true)
|
||||||
|
const { TmuxSessionManager } = await import('./manager')
|
||||||
|
const manager = new TmuxSessionManager(createMockContext(), createTmuxConfig({
|
||||||
|
enabled: true,
|
||||||
|
isolation: 'session',
|
||||||
|
}), mockTmuxDeps)
|
||||||
|
|
||||||
|
// when
|
||||||
|
await manager.onSessionCreated(createSessionCreatedEvent('ses_first', 'ses_parent', 'First'))
|
||||||
|
await manager.onSessionCreated(createSessionCreatedEvent('ses_second', 'ses_parent', 'Second'))
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(mockSweepStaleOmoAgentSessions).toHaveBeenCalledTimes(2)
|
||||||
|
})
|
||||||
|
|
||||||
|
test('#given sweepStaleOmoAgentSessions succeeds #when additional onSessionCreated events fire in same process #then sweep runs exactly once', async () => {
|
||||||
|
// given
|
||||||
|
mockSweepStaleOmoAgentSessions.mockClear()
|
||||||
|
mockSweepStaleOmoAgentSessions.mockImplementation(async () => 0)
|
||||||
|
mockIsInsideTmux.mockReturnValue(true)
|
||||||
|
const { TmuxSessionManager } = await import('./manager')
|
||||||
|
const manager = new TmuxSessionManager(createMockContext(), createTmuxConfig({
|
||||||
|
enabled: true,
|
||||||
|
isolation: 'session',
|
||||||
|
}), mockTmuxDeps)
|
||||||
|
|
||||||
|
// when
|
||||||
|
await manager.onSessionCreated(createSessionCreatedEvent('ses_a', 'ses_parent', 'A'))
|
||||||
|
await manager.onSessionCreated(createSessionCreatedEvent('ses_b', 'ses_parent', 'B'))
|
||||||
|
await manager.onSessionCreated(createSessionCreatedEvent('ses_c', 'ses_parent', 'C'))
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(mockSweepStaleOmoAgentSessions).toHaveBeenCalledTimes(1)
|
||||||
|
})
|
||||||
|
|
||||||
test('#given killTmuxSessionIfExists throws #when cleanup runs #then cleanup still completes without throwing', async () => {
|
test('#given killTmuxSessionIfExists throws #when cleanup runs #then cleanup still completes without throwing', async () => {
|
||||||
// given
|
// given
|
||||||
mockKillTmuxSessionIfExists.mockClear()
|
mockKillTmuxSessionIfExists.mockClear()
|
||||||
|
|||||||
@@ -67,6 +67,7 @@ export class TmuxSessionManager {
|
|||||||
private isolatedWindowPaneId: string | undefined
|
private isolatedWindowPaneId: string | undefined
|
||||||
private isolatedContainerNullStateCount = 0
|
private isolatedContainerNullStateCount = 0
|
||||||
private staleSweepCompleted = false
|
private staleSweepCompleted = false
|
||||||
|
private staleSweepInProgress = false
|
||||||
constructor(ctx: PluginInput, tmuxConfig: TmuxConfig, deps: TmuxUtilDeps = defaultTmuxDeps) {
|
constructor(ctx: PluginInput, tmuxConfig: TmuxConfig, deps: TmuxUtilDeps = defaultTmuxDeps) {
|
||||||
this.client = ctx.client
|
this.client = ctx.client
|
||||||
this.tmuxConfig = tmuxConfig
|
this.tmuxConfig = tmuxConfig
|
||||||
@@ -977,27 +978,32 @@ export class TmuxSessionManager {
|
|||||||
}
|
}
|
||||||
|
|
||||||
this.staleSweepCompleted = false
|
this.staleSweepCompleted = false
|
||||||
|
this.staleSweepInProgress = false
|
||||||
|
|
||||||
log("[tmux-session-manager] cleanup complete")
|
log("[tmux-session-manager] cleanup complete")
|
||||||
}
|
}
|
||||||
|
|
||||||
private async sweepStaleIsolatedSessionsOnce(): Promise<void> {
|
private async sweepStaleIsolatedSessionsOnce(): Promise<void> {
|
||||||
if (this.staleSweepCompleted) return
|
if (this.staleSweepCompleted) return
|
||||||
|
if (this.staleSweepInProgress) return
|
||||||
if (this.tmuxConfig.isolation !== "session") {
|
if (this.tmuxConfig.isolation !== "session") {
|
||||||
this.staleSweepCompleted = true
|
this.staleSweepCompleted = true
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
this.staleSweepCompleted = true
|
this.staleSweepInProgress = true
|
||||||
try {
|
try {
|
||||||
const killed = await sweepStaleOmoAgentSessions()
|
const killed = await sweepStaleOmoAgentSessions()
|
||||||
if (killed > 0) {
|
if (killed > 0) {
|
||||||
log("[tmux-session-manager] stale isolated sessions swept", { killed })
|
log("[tmux-session-manager] stale isolated sessions swept", { killed })
|
||||||
}
|
}
|
||||||
|
this.staleSweepCompleted = true
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
log("[tmux-session-manager] stale sweep failed", {
|
log("[tmux-session-manager] stale sweep failed", {
|
||||||
error: String(error),
|
error: String(error),
|
||||||
})
|
})
|
||||||
|
} finally {
|
||||||
|
this.staleSweepInProgress = false
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user