Skip to content

Commit 85f90fa

Browse files
committed
fix(mothership): sim lows #9,#13-#16 — honest errors, one terminal predicate
#16: Stop during a permission wait no longer reads "Timed out" — the message names the actual reason. #13: subscription-plan and sealed-context catches log the caught error. #14: registerPendingToolPromise's .finally() derived an unhandled rejection chain; the rejection is now observed and logged. #15: the attach tail->batch reconnect cycle paces 1s on an empty non-terminal batch instead of spinning against a prompt-closing middlebox. #9: the terminal-status predicate (defined 3x, one a drifted Set copy) lives once in request/session; all consumers import it. Claude-Session: https://claude.ai/code/session_01CgaxNAaeD3taGdghbXn17w
1 parent ecf39d3 commit 85f90fa

11 files changed

Lines changed: 49 additions & 37 deletions

File tree

apps/sim/app/api/copilot/chat/stream/route.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ vi.mock('@/lib/mothership/async-runs/repository', () => ({
2323
}))
2424

2525
vi.mock('@/lib/mothership/request/session', () => ({
26+
isTerminalStreamStatus: (status: string | null | undefined) =>
27+
status === 'complete' || status === 'error' || status === 'cancelled',
2628
readEvents,
2729
readFilePreviewSessions,
2830
checkForReplayGap,

apps/sim/app/api/copilot/chat/stream/route.ts

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
createEvent,
2626
encodeSSEComment,
2727
encodeSSEEnvelope,
28+
isTerminalStreamStatus,
2829
readEvents,
2930
readFilePreviewSessions,
3031
SSE_RESPONSE_HEADERS,
@@ -58,16 +59,6 @@ function extractEnvelopeRequestId(envelope: { trace?: { requestId?: unknown } })
5859
return extractCanonicalRequestId(envelope.trace?.requestId)
5960
}
6061

61-
function isTerminalStatus(
62-
status: string | null | undefined
63-
): status is MothershipStreamV1CompletionStatus {
64-
return (
65-
status === MothershipStreamV1CompletionStatus.complete ||
66-
status === MothershipStreamV1CompletionStatus.error ||
67-
status === MothershipStreamV1CompletionStatus.cancelled
68-
)
69-
}
70-
7162
function buildResumeTerminalEnvelopes(options: {
7263
streamId: string
7364
afterCursor: string
@@ -425,7 +416,7 @@ async function handleResumeRequestBody({
425416
if (controllerClosed) {
426417
break
427418
}
428-
if (isTerminalStatus(currentRun.status)) {
419+
if (isTerminalStreamStatus(currentRun.status)) {
429420
emitTerminalIfMissing(currentRun.status, {
430421
message:
431422
currentRun.status === MothershipStreamV1CompletionStatus.error

apps/sim/app/workspace/[workspaceId]/home/hooks/stream-protocol.ts

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -128,12 +128,6 @@ export function resolveChatIdFromStreamBatch(batch: StreamBatchResponse): string
128128
return undefined
129129
}
130130

131-
const TERMINAL_STREAM_STATUSES = new Set(['complete', 'error', 'cancelled'])
132-
133-
export function isTerminalStreamStatus(status: string | null | undefined): boolean {
134-
return TERMINAL_STREAM_STATUSES.has(status ?? '')
135-
}
136-
137131
export function isAlreadyProcessedStreamCursor(
138132
eventCursor: string | undefined,
139133
currentCursor: string

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ import {
5656
import { MOTHERSHIP_CHAT_API_PATH } from '@/lib/mothership/constants'
5757
import { sendMothershipMessage } from '@/lib/mothership/events'
5858
import { MothershipStreamV1ToolOutcome } from '@/lib/mothership/generated/mothership-stream-v1'
59+
import { isTerminalStreamStatus } from '@/lib/mothership/request/session'
5960
import { parsePersistedStreamEventEnvelopeJson } from '@/lib/mothership/request/session/contract'
6061
import type { FilePreviewSession } from '@/lib/mothership/request/session/file-preview-session-contract'
6162
import { canDisplayResource } from '@/lib/mothership/resources/availability'
@@ -159,7 +160,6 @@ import {
159160
isAlreadyProcessedStreamCursor,
160161
isStreamGoneError,
161162
isStreamSchemaValidationError,
162-
isTerminalStreamStatus,
163163
parseStreamBatchResponse,
164164
resolveChatIdFromStreamBatch,
165165
type StreamBatchResponse,
@@ -2472,6 +2472,10 @@ export function useChat(
24722472
if (activeAbort.signal.aborted || streamGenRef.current !== expectedGen) {
24732473
return { error: false, aborted: true }
24742474
}
2475+
/* A middlebox that closes the SSE tail promptly makes this loop spin
2476+
tail->batch->tail with zero delay. An empty non-terminal batch means
2477+
nothing new arrived — pace the next cycle instead of hammering. */
2478+
await sleep(1_000)
24752479
}
24762480
}
24772481

apps/sim/lib/mothership/chat/effective-transcript.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
MothershipStreamV1ToolOutcome,
1414
MothershipStreamV1ToolPhase,
1515
} from '@/lib/mothership/generated/mothership-stream-v1'
16+
import { isTerminalStreamStatus } from '@/lib/mothership/request/session'
1617
import type { FilePreviewSession } from '@/lib/mothership/request/session/file-preview-session-contract'
1718
import type { StreamBatchEvent } from '@/lib/mothership/request/session/types'
1819
import {
@@ -52,14 +53,6 @@ function asPayloadRecord(value: unknown): Record<string, unknown> | undefined {
5253
return isRecordLike(value) ? value : undefined
5354
}
5455

55-
function isTerminalStreamStatus(status: string | null | undefined): boolean {
56-
return (
57-
status === MothershipStreamV1CompletionStatus.complete ||
58-
status === MothershipStreamV1CompletionStatus.error ||
59-
status === MothershipStreamV1CompletionStatus.cancelled
60-
)
61-
}
62-
6356
/**
6457
* Error codes that describe the USER stopping the turn, not a failure.
6558
*

apps/sim/lib/mothership/request/handlers/types.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { createLogger } from '@sim/logger'
2-
import { toError } from '@sim/utils/errors'
2+
import { getErrorMessage, toError } from '@sim/utils/errors'
33
import { isRecordLike, toRecord } from '@sim/utils/object'
44
import type {
55
AsyncCompletionSignal,
@@ -131,11 +131,19 @@ export function registerPendingToolPromise(
131131
pendingPromise: Promise<AsyncCompletionSignal>
132132
): void {
133133
context.pendingToolPromises.set(toolCallId, pendingPromise)
134-
pendingPromise.finally(() => {
135-
if (context.pendingToolPromises.get(toolCallId) === pendingPromise) {
136-
context.pendingToolPromises.delete(toolCallId)
137-
}
138-
})
134+
/* .finally() derives a NEW promise that re-throws the rejection with no handler — an
135+
unhandled-rejection crash waiting for the first rejecting tool promise. Observe the
136+
chain: cleanup on both settles, rejection logged (the original promise's consumers
137+
still see it through the map). */
138+
pendingPromise
139+
.catch((error) => {
140+
logger.warn('Pending tool promise rejected', { toolCallId, error: getErrorMessage(error) })
141+
})
142+
.finally(() => {
143+
if (context.pendingToolPromises.get(toolCallId) === pendingPromise) {
144+
context.pendingToolPromises.delete(toolCallId)
145+
}
146+
})
139147
}
140148

141149
/**

apps/sim/lib/mothership/request/session/contract.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import type {
77
MothershipStreamV1Trace,
88
} from '@/lib/mothership/generated/mothership-stream-v1'
99
import {
10+
MothershipStreamV1CompletionStatus,
1011
MothershipStreamV1EventType,
1112
MothershipStreamV1ResourceOp,
1213
MothershipStreamV1RunKind,
@@ -402,6 +403,17 @@ export function isSyntheticFilePreviewEventEnvelope(
402403

403404
// Stream event type guards
404405

406+
/** The one terminal-status predicate — complete | error | cancelled. */
407+
export function isTerminalStreamStatus(
408+
status: string | null | undefined
409+
): status is MothershipStreamV1CompletionStatus {
410+
return (
411+
status === MothershipStreamV1CompletionStatus.complete ||
412+
status === MothershipStreamV1CompletionStatus.error ||
413+
status === MothershipStreamV1CompletionStatus.cancelled
414+
)
415+
}
416+
405417
export function isToolCallStreamEvent(event: SessionStreamEvent): event is ToolCallStreamEvent {
406418
return event.type === 'tool' && isRecordLike(event.payload) && event.payload.phase === 'call'
407419
}

apps/sim/lib/mothership/request/session/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ export {
4646
isContractStreamEventEnvelope,
4747
isSubagentSpanStreamEvent,
4848
isSyntheticFilePreviewEventEnvelope,
49+
isTerminalStreamStatus,
4950
isToolArgsDeltaStreamEvent,
5051
isToolCallStreamEvent,
5152
isToolResultStreamEvent,

apps/sim/lib/mothership/request/tools/billing.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { createLogger } from '@sim/logger'
2+
import { getErrorMessage } from '@sim/utils/errors'
23
import { getHighestPrioritySubscription } from '@/lib/billing/core/plan'
34
import { isEnterprise, isPaid } from '@/lib/billing/plan-helpers'
45
import { isOrgScopedSubscription } from '@/lib/billing/subscriptions/utils'
@@ -59,8 +60,10 @@ export async function handleBillingLimitResponse(
5960
"You've reached your usage limit for this billing period. Please increase your usage limit from billing settings to continue."
6061
}
6162
}
62-
} catch {
63-
logger.warn('Failed to determine subscription plan, defaulting to upgrade_plan')
63+
} catch (error) {
64+
logger.warn('Failed to determine subscription plan, defaulting to upgrade_plan', {
65+
error: getErrorMessage(error),
66+
})
6467
}
6568

6669
const upgradePayload = JSON.stringify({

apps/sim/lib/mothership/request/tools/client.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,10 +124,11 @@ export async function waitForClientToolCompletion({
124124
}
125125
}
126126
}
127-
} catch {
127+
} catch (error) {
128128
toolRegistry?.markIncomplete('client-tool-seal-failed', {
129129
origin: 'copilotToolClient.sealedContext',
130130
})
131+
logger.warn('Sealed-context unseal failed', { error: getErrorMessage(error) })
131132
} finally {
132133
finishPendingActivation?.()
133134
}

0 commit comments

Comments
 (0)