Skip to content

Commit aa4e79d

Browse files
fix(slack-search): allow one authorized status cleanup attempt
1 parent 30bb95c commit aa4e79d

2 files changed

Lines changed: 69 additions & 3 deletions

File tree

‎apps/sim/lib/slack-search/assistant-stream.test.ts‎

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -349,14 +349,65 @@ describe('Slack lazy stream lifecycle', () => {
349349
)
350350
})
351351

352-
it('propagates an empty-run status failure without retrying or posting a reply', async () => {
352+
it.each([false, true])(
353+
'attempts status cleanup once after both notification and status fail (cleanup fails: %s)',
354+
async (cleanupFails) => {
355+
const { stream, controller, beforeCleanup } = setup()
356+
await stream.start()
357+
api.start.mockRejectedValueOnce(new Error('notification response lost'))
358+
api.status.mockRejectedValueOnce(new Error('status response lost'))
359+
await expect(stream.finishWithError()).rejects.toThrow('status response lost')
360+
expect(controller.signal.aborted).toBe(true)
361+
if (cleanupFails) {
362+
api.status.mockRejectedValueOnce(new Error('cleanup response lost'))
363+
await expect(stream.terminateAfterFailure()).rejects.toThrow('cleanup response lost')
364+
} else {
365+
await stream.terminateAfterFailure()
366+
}
367+
await stream.terminateAfterFailure()
368+
expect(api.status).toHaveBeenCalledTimes(3)
369+
const cleanupSignal = api.status.mock.calls[2][3]
370+
expect(cleanupSignal).not.toBe(controller.signal)
371+
expect(cleanupSignal.aborted).toBe(false)
372+
expect(beforeCleanup).toHaveBeenCalledExactlyOnceWith(cleanupSignal)
373+
expect(beforeCleanup.mock.invocationCallOrder[0]).toBeLessThan(
374+
api.status.mock.invocationCallOrder[2]
375+
)
376+
expect(api.status).toHaveBeenLastCalledWith(
377+
'test-token',
378+
{ channel: 'D1', threadTs: '1.1' },
379+
'active',
380+
cleanupSignal
381+
)
382+
expect(api.start).toHaveBeenCalledOnce()
383+
expect(api.append).not.toHaveBeenCalled()
384+
expect(api.stop).not.toHaveBeenCalled()
385+
}
386+
)
387+
388+
it('requires fresh authority before retrying a failed status reset', async () => {
389+
const { stream, beforeCleanup } = setup()
390+
await stream.start()
391+
api.start.mockRejectedValueOnce(new Error('notification response lost'))
392+
api.status.mockRejectedValueOnce(new Error('status response lost'))
393+
await expect(stream.finishWithError()).rejects.toThrow('status response lost')
394+
beforeCleanup.mockRejectedValueOnce(new Error('authority revoked'))
395+
await expect(stream.terminateAfterFailure()).rejects.toThrow('authority revoked')
396+
await stream.terminateAfterFailure()
397+
expect(api.status).toHaveBeenCalledTimes(2)
398+
expect(api.start).toHaveBeenCalledOnce()
399+
expect(api.stop).not.toHaveBeenCalled()
400+
})
401+
402+
it('propagates an empty-run status failure and cleans up without posting a reply', async () => {
353403
const { stream, controller } = setup()
354404
await stream.start()
355405
api.status.mockRejectedValueOnce(new Error('status response lost'))
356406
await expect(stream.finish(result)).rejects.toThrow('status response lost')
357407
expect(controller.signal.aborted).toBe(true)
358408
await stream.terminateAfterFailure()
359-
expect(api.status).toHaveBeenCalledTimes(2)
409+
await stream.terminateAfterFailure()
410+
expect(api.status).toHaveBeenCalledTimes(3)
360411
expect(api.start).not.toHaveBeenCalled()
361412
})
362413
})

‎apps/sim/lib/slack-search/assistant-stream.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ export class SlackSearchAssistantStream {
126126
private failure?: Error
127127
private closed = false
128128
private closeAttempted = false
129+
private cleanupAttempted = false
129130
private pendingEvents: Promise<void> = Promise.resolve()
130131
private evidence = new Map<string, Record<string, unknown>>()
131132
private toolProgress = new Map<string, { toolName: string; chunk: ToolProgress }>()
@@ -369,10 +370,24 @@ export class SlackSearchAssistantStream {
369370

370371
/** Settle once after abort, with fresh authority and no replay of ambiguous sends. */
371372
async terminateAfterFailure() {
372-
if (!this.sessionStarted || this.closed || this.closeAttempted) return
373+
if (
374+
!this.sessionStarted ||
375+
this.closed ||
376+
this.cleanupAttempted ||
377+
(this.stream && this.closeAttempted)
378+
)
379+
return
380+
this.cleanupAttempted = true
373381
const signal = AbortSignal.timeout(5000)
374382
await this.options.beforeCleanup(signal)
375383
signal.throwIfAborted()
384+
if (this.closeAttempted) {
385+
/** Only the idempotent status reset can repeat; never replay an unconfirmed message send. */
386+
const { token, channel, threadTs } = this.options
387+
await setSlackAgentSessionStatus(token, { channel, threadTs }, 'active', signal)
388+
this.closed = true
389+
return
390+
}
376391
await this.close(true, signal)
377392
}
378393

0 commit comments

Comments
 (0)