From 9efd683ce54c65fe39d7f6221784b01dd3bdcc92 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 12 Oct 2026 10:52:22 -0700 Subject: [PATCH] fix(chat): name stackless error subclasses and compact stored unfinished tool params --- .../remote-sandbox/code-failure.test.ts | 11 ++++++++ .../execution/remote-sandbox/code-failure.ts | 16 +++++++----- .../lib/mothership/chat/display-message.ts | 4 +-- .../lib/mothership/chat/live-message-id.ts | 15 ++++++++--- .../mothership/chat/persisted-message.test.ts | 26 +++++++++++-------- .../lib/mothership/chat/persisted-message.ts | 14 ++++++---- 6 files changed, 58 insertions(+), 28 deletions(-) diff --git a/apps/sim/lib/execution/remote-sandbox/code-failure.test.ts b/apps/sim/lib/execution/remote-sandbox/code-failure.test.ts index fdec0aac400..64b59629886 100644 --- a/apps/sim/lib/execution/remote-sandbox/code-failure.test.ts +++ b/apps/sim/lib/execution/remote-sandbox/code-failure.test.ts @@ -118,6 +118,16 @@ throw e Node.js v22.19.0 ` +/** A stackless error whose class keeps its parent's `name`, annotated by Node. */ +const STACKLESS_SUBCLASS_ERROR = `node:internal/modules/run_main:123 + triggerUncaughtException( + ^ + +[QuotaExceeded [Error]: over quota] { code: 'E_Q' } + +Node.js v22.19.0 +` + const STACKLESS_ERROR_WITH_INLINE_PROPERTIES = `file:///home/user/.sim-function-abc.mjs:1 throw e ^ @@ -168,6 +178,7 @@ describe('processCodeFailure', () => { ], ['a stackless error with no properties', STACKLESS_RANGE_ERROR, 'RangeError', 'limit hit'], ['a stackless plain Error', STACKLESS_PLAIN_ERROR, 'Error', 'plain failure'], + ['a stackless error subclass', STACKLESS_SUBCLASS_ERROR, 'QuotaExceeded [Error]', 'over quota'], [ 'a stackless error with inline properties', STACKLESS_ERROR_WITH_INLINE_PROPERTIES, diff --git a/apps/sim/lib/execution/remote-sandbox/code-failure.ts b/apps/sim/lib/execution/remote-sandbox/code-failure.ts index d4a3ae80f3f..990a4120f16 100644 --- a/apps/sim/lib/execution/remote-sandbox/code-failure.ts +++ b/apps/sim/lib/execution/remote-sandbox/code-failure.ts @@ -2,15 +2,17 @@ import type { SandboxCodeResult, SandboxCommandResult } from '@/lib/execution/re /** * A top-level error headline in a Python traceback or Node crash report: `Name: message`, - * Node's `Name [CODE]: message`, or `[NameError: message]` (any inline properties after it) - * for an error printed without a stack. It starts the line, so the properties Node prints indented beneath an error - * (`code: 'X'`, `bytesRead: 0`) never match, and a space follows the colon, so - * source-location headers (`node:internal/x:12`, `file:///x.mjs:3`) don't either. The - * bracketed form needs an error-type name, since Node brackets other thrown values the - * same way (`[Object: null prototype] {`, `[Function: handler]`). + * Node's `Name [CODE]: message` (or `Class [name]: message` for an error class that keeps + * its parent's name), or the same bracketed with any inline properties after it + * (`[NameError: message] { … }`) for an error printed without a stack. It starts the line, + * so the properties Node prints indented beneath an error (`code: 'X'`, `bytesRead: 0`) + * never match, and a space follows the colon, so source-location headers + * (`node:internal/x:12`, `file:///x.mjs:3`) don't either. The bracketed form needs an + * error-type name or that annotation, since Node brackets other thrown values the same + * way (`[Object: null prototype] {`, `[Function: handler]`). */ const ERROR_HEADLINE = - /^(?:\[(?:[A-Za-z_$][\w.$]*)?(?:Error|Exception)|[A-Za-z_$][\w.$]*(?: \[[\w.$-]+\])?):(?:\s|$)/ + /^(?:\[(?:[A-Za-z_$][\w.$]*)?(?:Error|Exception)|\[?[A-Za-z_$][\w.$]* \[[\w.$-]+\]|[A-Za-z_$][\w.$]*):(?:\s|$)/ /** * Converts a failed code command into a code result, naming the error from the diff --git a/apps/sim/lib/mothership/chat/display-message.ts b/apps/sim/lib/mothership/chat/display-message.ts index b97f5d97e3d..8ec7ff0685e 100644 --- a/apps/sim/lib/mothership/chat/display-message.ts +++ b/apps/sim/lib/mothership/chat/display-message.ts @@ -1,6 +1,6 @@ import type { PersistedContentBlock } from '@/lib/api/contracts/copilot-messages' import { getMothershipAttachmentPreviewUrl } from '@/lib/mothership/chat/attachment-preview' -import { isLiveAssistantMessageId } from '@/lib/mothership/chat/live-message-id' +import { isStoredAssistantMessage } from '@/lib/mothership/chat/live-message-id' import type { PersistedMessage } from '@/lib/mothership/chat/persisted-message' import { copyPersistedMessageContext, @@ -216,7 +216,7 @@ export function toDisplayMessage(msg: PersistedMessage): ChatMessage { } if (msg.contentBlocks && msg.contentBlocks.length > 0) { - const stored = msg.role === 'assistant' && !isLiveAssistantMessageId(msg.id) + const stored = isStoredAssistantMessage(msg) const displayBlocks = msg.contentBlocks .map((block) => toDisplayBlock(block, stored)) .filter((block): block is ContentBlock => !!block) diff --git a/apps/sim/lib/mothership/chat/live-message-id.ts b/apps/sim/lib/mothership/chat/live-message-id.ts index 5babcfd2512..957bc465c22 100644 --- a/apps/sim/lib/mothership/chat/live-message-id.ts +++ b/apps/sim/lib/mothership/chat/live-message-id.ts @@ -4,10 +4,19 @@ export function getLiveAssistantMessageId(streamId: string): string { /** * True for the synthetic id of a streaming/just-streamed assistant message. - * These ids exist only in the client's effective transcript — never in the - * persisted one — so message-scoped server actions (e.g. fork) must not be - * offered until the transcript refetch swaps in the persisted message id. + * These ids exist only in the effective transcript built for the client — + * never in the persisted one — so message-scoped server actions (e.g. fork) + * must not be offered until the transcript refetch swaps in the persisted + * message id. */ export function isLiveAssistantMessageId(messageId: string): boolean { return messageId.startsWith('live-assistant:') } + +/** + * True for a persisted assistant turn. Its turn has ended, so a tool row it left + * pending, executing or awaiting approval did not finish and never runs again. + */ +export function isStoredAssistantMessage(message: { role: string; id: string }): boolean { + return message.role === 'assistant' && !isLiveAssistantMessageId(message.id) +} diff --git a/apps/sim/lib/mothership/chat/persisted-message.test.ts b/apps/sim/lib/mothership/chat/persisted-message.test.ts index c92dff8505f..b95399e8028 100644 --- a/apps/sim/lib/mothership/chat/persisted-message.test.ts +++ b/apps/sim/lib/mothership/chat/persisted-message.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest' import { copilotChatStopBodySchema } from '@/lib/api/contracts/copilot' import type { PersistedToolState } from '@/lib/api/contracts/copilot-messages' import { toDisplayMessage } from '@/lib/mothership/chat/display-message' +import { getLiveAssistantMessageId } from '@/lib/mothership/chat/live-message-id' import type { OrchestratorResult } from '@/lib/mothership/request/types' import { getToolDisplayTitle } from '@/lib/mothership/tools/tool-display' import { resolveMessageCitations } from '@/app/workspace/[workspaceId]/home/components/message-content/resolve-citations' @@ -635,22 +636,25 @@ describe('compactToolCallParams', () => { }) it.each(['pending', 'executing', 'awaiting_approval'] as const)( - 'keeps a %s call whole: it can still run or await approval from these params', + 'keeps a live %s call whole: it can still run or await approval from these params', (state) => { - const message = sandboxCall(state) + const message = { ...sandboxCall(state), id: getLiveAssistantMessageId('stream-1') } expect(compactToolCallParams(message)).toBe(message) } ) - it('bounds an interrupted call, which no reload re-runs or asks to approve', () => { - const compacted = compactToolCallParams(sandboxCall('interrupted')) - const stdout = ( - compacted.contentBlocks?.[0].toolCall?.params?.request as { - invocation: { stdout: string } - } - ).invocation.stdout - expect(stdout.length).toBeLessThanOrEqual(1024) - }) + it.each(['pending', 'executing', 'awaiting_approval', 'interrupted'] as const)( + 'bounds a stored %s call, which renders as interrupted and never re-runs or asks to approve', + (state) => { + const compacted = compactToolCallParams(sandboxCall(state)) + const stdout = ( + compacted.contentBlocks?.[0].toolCall?.params?.request as { + invocation: { stdout: string } + } + ).invocation.stdout + expect(stdout.length).toBeLessThanOrEqual(1024) + } + ) it('is idempotent and keeps the reference once compacted', () => { const compacted = compactToolCallParams(sandboxCall('error')) diff --git a/apps/sim/lib/mothership/chat/persisted-message.ts b/apps/sim/lib/mothership/chat/persisted-message.ts index 1bfa824d8c3..7f10bc6cad4 100644 --- a/apps/sim/lib/mothership/chat/persisted-message.ts +++ b/apps/sim/lib/mothership/chat/persisted-message.ts @@ -7,6 +7,7 @@ import type { PersistedToolState, } from '@/lib/api/contracts/copilot-messages' import { buildMothershipErrorTag } from '@/lib/mothership/chat/error-tag' +import { isStoredAssistantMessage } from '@/lib/mothership/chat/live-message-id' import { compactRetrievalCitations } from '@/lib/mothership/chat/retrieval-citations' import { mergeAndRedactPersistedBlocks, @@ -228,10 +229,12 @@ function compactParamRecord(record: Record): Record): Record { const toolCall = block.toolCall - if (!toolCall?.params || isUnsettledToolState(toolCall.state)) return block + if (!toolCall?.params || (!stored && isUnsettledToolState(toolCall.state))) return block const params = compactParamRecord(toolCall.params) if (params === toolCall.params) return block changed = true