Skip to content

Commit 74858f1

Browse files
committed
fix(mothership): keep closed-view reporting across stream recovery and never let a not-started report end a claimed call
Browser tools also follow a chat-view lifetime, so a tool started on a reader that recovery replaced is still ended, and reported, as a closed view when the view unmounts. A report that says the call never started can now settle only an unclaimed call; one the desktop claimed is settled only by its own result.
1 parent 7cf241e commit 74858f1

6 files changed

Lines changed: 85 additions & 11 deletions

File tree

‎apps/sim/app/api/copilot/confirm/route.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,17 @@ export const POST = withRouteHandler((req: NextRequest) => {
317317
span.setAttribute(TraceAttr.CopilotConfirmOutcome, CopilotConfirmOutcome.ToolCallNotFound)
318318
return createNotFoundResponse('Running client tool call not found')
319319
}
320+
// A reporter that says the call never started (a stale replay, a closed view) cannot speak
321+
// for a call the desktop claimed: only the claim's own result may settle it.
322+
if (
323+
isNativeClientTool &&
324+
isPlainRecord(data) &&
325+
data.notStarted === true &&
326+
existing.status !== ASYNC_TOOL_STATUS.pending
327+
) {
328+
span.setAttribute(TraceAttr.CopilotConfirmOutcome, CopilotConfirmOutcome.ToolCallNotFound)
329+
return createNotFoundResponse('Pending client tool call not found')
330+
}
320331

321332
let effectiveStatus = status
322333
let executionId = submittedExecutionId

‎apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2134,6 +2134,23 @@ describe('useChat remount send recovery', () => {
21342134
expect(toolSignal.aborted).toBe(false)
21352135
})
21362136

2137+
it('still ends as a closed chat view when the view unmounts after a stream recovery', async () => {
2138+
const { toolSignal, replays, unmount } = await startBrowserAction()
2139+
Object.defineProperty(document, 'visibilityState', {
2140+
configurable: true,
2141+
get: () => 'visible',
2142+
})
2143+
await act(async () => {
2144+
document.dispatchEvent(new Event('visibilitychange'))
2145+
})
2146+
await waitFor(() => replays.length > 0)
2147+
2148+
unmount()
2149+
2150+
expect(toolSignal.aborted).toBe(true)
2151+
expect(toolSignal.reason).toBe(CHAT_VIEW_CLOSED_ABORT_REASON)
2152+
})
2153+
21372154
it('ends as a closed chat view, not a Stop, when the chat view unmounts', async () => {
21382155
const { toolSignal, unmount } = await startBrowserAction()
21392156

‎apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -468,19 +468,25 @@ const UNMOUNT_ABORT_REASON = 'unmount:client_cleanup'
468468

469469
/**
470470
* The lifetime a browser tool started from one stream observes. Replacing the reader (a window
471-
* returning to view, a history reconnect) leaves an in-flight action running; an unmount ends it
472-
* as a closed chat view, which still reports its outcome; a Stop ends it silently.
471+
* returning to view, a history reconnect) leaves an in-flight action running. The chat view
472+
* closing ends it as a closed view, which still reports its outcome, even when a later reader
473+
* replaced the one that started it. A Stop ends it silently.
473474
*/
474-
function browserToolLifetime(streamSignal: AbortSignal | undefined): AbortSignal | undefined {
475-
if (!streamSignal) return undefined
475+
function browserToolLifetime(
476+
streamSignal: AbortSignal | undefined,
477+
chatViewSignal: AbortSignal | undefined
478+
): AbortSignal {
476479
const lifetime = new AbortController()
477-
const follow = () => {
478-
const reason: unknown = streamSignal.reason
480+
const followStream = () => {
481+
const reason: unknown = streamSignal?.reason
479482
if (READER_REPLACEMENT_ABORT_REASONS.has(reason)) return
480483
lifetime.abort(reason === UNMOUNT_ABORT_REASON ? CHAT_VIEW_CLOSED_ABORT_REASON : reason)
481484
}
482-
if (streamSignal.aborted) follow()
483-
else streamSignal.addEventListener('abort', follow, { once: true })
485+
const followChatView = () => lifetime.abort(CHAT_VIEW_CLOSED_ABORT_REASON)
486+
if (streamSignal?.aborted) followStream()
487+
else streamSignal?.addEventListener('abort', followStream, { once: true })
488+
if (chatViewSignal?.aborted) followChatView()
489+
else chatViewSignal?.addEventListener('abort', followChatView, { once: true })
484490
return lifetime.signal
485491
}
486492

@@ -953,6 +959,8 @@ export function useChat(
953959
const reconnectExhaustedRecheckTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null)
954960

955961
const abortControllerRef = useRef<AbortController | null>(null)
962+
/** Ends when this chat view unmounts, whichever stream reader started a tool. */
963+
const chatViewLifetimeRef = useRef<AbortController | null>(null)
956964
const detachedChatResolutionControllersRef = useRef<Set<AbortController> | null>(null)
957965
const detachedChatResolutionControllers = (detachedChatResolutionControllersRef.current ??=
958966
new Set())
@@ -2183,7 +2191,10 @@ export function useChat(
21832191
shouldContinue?: () => boolean
21842192
}
21852193
) => {
2186-
const browserToolSignal = browserToolLifetime(abortControllerRef.current?.signal)
2194+
const browserToolSignal = browserToolLifetime(
2195+
abortControllerRef.current?.signal,
2196+
chatViewLifetimeRef.current?.signal
2197+
)
21872198
const activityTracker = getResourceActivityTracker(
21882199
expectedGen ?? streamGenRef.current,
21892200
options?.targetChatId
@@ -4997,6 +5008,12 @@ export function useChat(
49975008
remoteActiveStreamId,
49985009
])
49995010

5011+
useEffect(() => {
5012+
const chatViewLifetime = new AbortController()
5013+
chatViewLifetimeRef.current = chatViewLifetime
5014+
return () => chatViewLifetime.abort(CHAT_VIEW_CLOSED_ABORT_REASON)
5015+
}, [])
5016+
50005017
useEffect(() => {
50015018
return () => {
50025019
cancelActiveStreamRecovery()

‎apps/sim/lib/mothership/request/tools/executor.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ describe('pendingToolWaitBudgetMs', () => {
256256
pendingToolWaitBudgetMs({
257257
name: 'terminal',
258258
status: 'executing',
259-
params: { operation: 'run', args: { command: 'npm install', waitSeconds: 120 } },
259+
params: { operation: 'run', args: { command: 'bun install', waitSeconds: 120 } },
260260
})
261261
).toBeGreaterThan(120_000)
262262
expect(

‎apps/sim/lib/mothership/tools/client/desktop-tool-pickup.integration.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,4 +217,33 @@ describe.runIf(Boolean(redisUrl))('a desktop tool call nobody picks up', () => {
217217
},
218218
TURN_WAIT_MS + 10_000
219219
)
220+
221+
it(
222+
'never lets a stale "not started" report end a call the desktop is running',
223+
async () => {
224+
const { toolCallId, answer } = await agentCalls('terminal', {
225+
operation: 'run',
226+
args: { command: 'bun run test' },
227+
})
228+
expect((await desktopClaims(toolCallId)).status).toBe(200)
229+
230+
const staleReport = await post(confirmPOST, '/api/copilot/confirm', {
231+
toolCallId,
232+
status: 'error',
233+
message: 'Not run: delivered too late',
234+
data: { error: 'Not run: delivered too late', notStarted: true },
235+
})
236+
const realResult = await post(confirmPOST, '/api/copilot/confirm', {
237+
toolCallId,
238+
status: 'success',
239+
message: 'Done',
240+
data: { output: 'tests passed' },
241+
})
242+
243+
expect(staleReport.ok).toBe(false)
244+
expect(realResult.status).toBe(200)
245+
expect(await answer).toMatchObject({ status: 'success' })
246+
},
247+
TURN_WAIT_MS + 10_000
248+
)
220249
})

‎apps/sim/lib/mothership/tools/client/terminal-tool-execution.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ describe('terminal client execution', () => {
5858

5959
executeTerminalToolOnClient(
6060
'terminal-stale',
61-
{ operation: 'run', args: { command: 'npm test' } },
61+
{ operation: 'run', args: { command: 'bun run test' } },
6262
'chat-1',
6363
emittedAt
6464
)

0 commit comments

Comments
 (0)