From b7a3e4dd4d0900025394fc417f19b61bca186ad7 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 12 Aug 2026 05:35:40 -0700 Subject: [PATCH] fix(copilot): keep a file-preview failure from stripping a tool call's arguments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The workspace file preview adapter runs while the tool-call frame is still on the wire, so the execution context it gets is turn-scoped and carries no toolCallId — the file delegation requires one, so resolving a path target threw on every call. The SSE handler swallows that throw and abandons the rest of the event, so the frame never registered its arguments and the call was later dispatched with an empty payload, failing schema validation. - bind the frame's own tool call id before entering the file use cases - resolve the preview target best effort, matching the preview base load - stop a preview failure from dropping the tool-call frame in the stream loop - drop the synthetic toolCallId the stream fixtures put on a turn context --- .../request/go/file-preview-adapter.test.ts | 114 ++++++++++++++++-- .../request/go/file-preview-adapter.ts | 62 +++++++--- .../sim/lib/copilot/request/go/stream.test.ts | 33 ++--- apps/sim/lib/copilot/request/go/stream.ts | 28 +++-- 4 files changed, 187 insertions(+), 50 deletions(-) diff --git a/apps/sim/lib/copilot/request/go/file-preview-adapter.test.ts b/apps/sim/lib/copilot/request/go/file-preview-adapter.test.ts index c9409958477..27851eabba6 100644 --- a/apps/sim/lib/copilot/request/go/file-preview-adapter.test.ts +++ b/apps/sim/lib/copilot/request/go/file-preview-adapter.test.ts @@ -9,14 +9,20 @@ import { MothershipStreamV1ToolPhase, } from '@/lib/copilot/generated/mothership-stream-v1' -const { peekFileIntentMock } = vi.hoisted(() => ({ +const { peekFileIntentMock, executeCopilotFileUseCaseMock } = vi.hoisted(() => ({ peekFileIntentMock: vi.fn(), + executeCopilotFileUseCaseMock: vi.fn(), })) vi.mock('@/lib/copilot/tools/server/files/file-intent-store', () => ({ peekFileIntent: peekFileIntentMock, })) +vi.mock('@/lib/copilot/application/execute-file-use-case', () => ({ + executeCopilotFileUseCase: executeCopilotFileUseCaseMock, + resolveCopilotWorkspaceFileReference: vi.fn(), +})) + import { createStreamingContext } from '@/lib/copilot/request/context/request-context' import { createFilePreviewAdapterState, @@ -31,8 +37,7 @@ const EDIT_TOOL_CALL_ID = 'edit-content-1' const WORKSPACE_FILE_TOOL_CALL_ID = 'workspace-file-1' const BASE_VERSION_MS = 900_000 -/** One args_delta chunk of the streamed `edit_content` JSON, as a driveable StreamEvent. */ -function editContentDelta(argumentsDelta: string): StreamEvent { +function toolEvent(payload: Record): StreamEvent { return eventToStreamEvent( createEvent({ streamId: STREAM_ID, @@ -41,17 +46,38 @@ function editContentDelta(argumentsDelta: string): StreamEvent { requestId: 'req-1', type: MothershipStreamV1EventType.tool, payload: { - toolCallId: EDIT_TOOL_CALL_ID, - toolName: 'edit_content', executor: MothershipStreamV1ToolExecutor.sim, mode: MothershipStreamV1ToolMode.async, - phase: MothershipStreamV1ToolPhase.args_delta, - argumentsDelta, + ...payload, }, }) ) } +/** One args_delta chunk of the streamed `edit_content` JSON, as a driveable StreamEvent. */ +function editContentDelta(argumentsDelta: string): StreamEvent { + return toolEvent({ + toolCallId: EDIT_TOOL_CALL_ID, + toolName: 'edit_content', + phase: MothershipStreamV1ToolPhase.args_delta, + argumentsDelta, + }) +} + +/** The authoritative `workspace_file` call frame for a path-targeted update. */ +function workspaceFileCall(): StreamEvent { + return toolEvent({ + toolCallId: WORKSPACE_FILE_TOOL_CALL_ID, + toolName: 'workspace_file', + phase: MothershipStreamV1ToolPhase.call, + arguments: { + operation: 'update', + title: 'Refresh the runbook', + target: { kind: 'path', path: 'files/notes.md' }, + }, + }) +} + function makeIntent(overrides: { operation: string fileId?: string @@ -140,3 +166,77 @@ describe('processFilePreviewStreamEvent — preview content emission', () => { expect(combined).toContain('world') }) }) + +/** + * The adapter runs while the tool-call frame is still on the wire, so the + * execution context it receives is turn-scoped and carries no `toolCallId`. + * The file delegation requires one, so the adapter has to supply the frame's + * own id — otherwise resolving the preview target throws, the SSE handler + * abandons the rest of the event, and the tool call is later dispatched with + * no arguments at all. + */ +describe('processFilePreviewStreamEvent — preview target resolution', () => { + const TURN_EXEC_CONTEXT: ExecutionContext = { + userId: 'user-1', + workflowId: 'workflow-1', + workspaceId: 'workspace-1', + chatId: 'chat-1', + messageId: 'msg-1', + copilotToolExecution: true, + } + + beforeEach(() => { + vi.clearAllMocks() + executeCopilotFileUseCaseMock.mockResolvedValue({ + files: [{ id: 'file-9', name: 'notes.md', folderPath: null }], + }) + }) + + it('resolves the path target under the streaming tool call identity', async () => { + const context = createStreamingContext() + + await processFilePreviewStreamEvent({ + streamId: STREAM_ID, + streamEvent: workspaceFileCall(), + context, + execContext: TURN_EXEC_CONTEXT, + options: { onEvent: () => {} }, + state: createFilePreviewAdapterState(), + }) + + expect(executeCopilotFileUseCaseMock).toHaveBeenCalled() + expect(executeCopilotFileUseCaseMock.mock.calls[0][0]).toMatchObject({ + userId: 'user-1', + workspaceId: 'workspace-1', + copilotToolExecution: true, + toolCallId: WORKSPACE_FILE_TOOL_CALL_ID, + }) + expect(context.activeFileIntents.get('')?.target).toEqual({ + kind: 'file_id', + fileId: 'file-9', + fileName: 'notes.md', + path: 'files/notes.md', + }) + }) + + it('keeps the path target and does not throw when resolution fails', async () => { + executeCopilotFileUseCaseMock.mockRejectedValue(new Error('workspace file listing unavailable')) + const context = createStreamingContext() + + await expect( + processFilePreviewStreamEvent({ + streamId: STREAM_ID, + streamEvent: workspaceFileCall(), + context, + execContext: TURN_EXEC_CONTEXT, + options: { onEvent: () => {} }, + state: createFilePreviewAdapterState(), + }) + ).resolves.toBeUndefined() + + expect(context.activeFileIntents.get('')?.target).toEqual({ + kind: 'path', + path: 'files/notes.md', + }) + }) +}) diff --git a/apps/sim/lib/copilot/request/go/file-preview-adapter.ts b/apps/sim/lib/copilot/request/go/file-preview-adapter.ts index feaa0822416..b79254a211e 100644 --- a/apps/sim/lib/copilot/request/go/file-preview-adapter.ts +++ b/apps/sim/lib/copilot/request/go/file-preview-adapter.ts @@ -66,6 +66,24 @@ function toPreviewTargetKind(kind: string | undefined): FilePreviewTargetKind | return kind === 'new_file' || kind === 'file_id' ? kind : undefined } +/** + * Binds the turn-scoped execution context to the tool call whose preview is + * being rendered. This adapter runs while the tool-call frame is still on the + * wire, before per-call dispatch builds a call-scoped context, so the turn + * context carries no `toolCallId` of its own — the frame is the only place that + * identity exists, and the file delegation requires one. + */ +function bindPreviewToolCall(context: ExecutionContext, toolCallId: string): ExecutionContext { + return { ...context, toolCallId } +} + +/** + * Upgrades a model-supplied path target to the backing file id so the preview + * can seed itself from the existing content. Resolution is best effort, exactly + * like the preview base load: leaving the target as a path only costs an + * un-seeded preview until the tool result arrives with the real file id, so a + * failure here must never escape into the stream loop. + */ async function resolvePreviewTarget(args: { context: ExecutionContext workspaceId?: string @@ -74,24 +92,29 @@ async function resolvePreviewTarget(args: { if (args.target.kind !== 'path' || !args.workspaceId || !args.target.path) { return args.target } - if (!args.context.copilotToolExecution || !args.context.toolCallId) { - throw new Error('Workspace file preview requires a trusted Copilot execution context') - } - const { files } = await executeCopilotFileUseCase(args.context, listAllWorkspaceFiles, { - workspaceId: args.workspaceId, - scope: 'active', - }) - const file = findWorkspaceFileRecord(files, args.target.path) - if (!file) { - return args.target - } + try { + const { files } = await executeCopilotFileUseCase(args.context, listAllWorkspaceFiles, { + workspaceId: args.workspaceId, + scope: 'active', + }) + const file = findWorkspaceFileRecord(files, args.target.path) + if (!file) { + return args.target + } - return { - kind: 'file_id', - fileId: file.id, - fileName: args.target.fileName ?? file.name, - path: args.target.path, + return { + kind: 'file_id', + fileId: file.id, + fileName: args.target.fileName ?? file.name, + path: args.target.path, + } + } catch (error) { + logger.warn('Failed to resolve workspace file preview target', { + toolCallId: args.context.toolCallId, + error: toError(error).message, + }) + return args.target } } @@ -375,8 +398,9 @@ export async function processFilePreviewStreamEvent(input: { const parsedArgs = parseWorkspaceFileArgs(streamEvent.payload.arguments) if (toolCallId && parsedArgs) { const { operation, title, contentType, edit } = parsedArgs + const previewContext = bindPreviewToolCall(execContext, toolCallId) const target = await resolvePreviewTarget({ - context: execContext, + context: previewContext, workspaceId: execContext.workspaceId, target: parsedArgs.target, }) @@ -405,7 +429,7 @@ export async function processFilePreviewStreamEvent(input: { (operation === 'append' || operation === 'patch') ) { previewBase = await loadWorkspaceFileTextForPreview( - execContext, + previewContext, execContext.workspaceId, fileId ) @@ -480,7 +504,7 @@ export async function processFilePreviewStreamEvent(input: { (intent.operation === 'append' || intent.operation === 'patch') ) { previewBase = await loadWorkspaceFileTextForPreview( - execContext, + bindPreviewToolCall(execContext, intent.toolCallId), execContext.workspaceId, result.fileId ) diff --git a/apps/sim/lib/copilot/request/go/stream.test.ts b/apps/sim/lib/copilot/request/go/stream.test.ts index e39654762ef..5921c6e89b7 100644 --- a/apps/sim/lib/copilot/request/go/stream.test.ts +++ b/apps/sim/lib/copilot/request/go/stream.test.ts @@ -147,6 +147,21 @@ function createStreamingContext(): StreamingContext { } } +/** + * The turn-scoped execution context exactly as the chat lifecycle builds it: no + * `toolCallId`, because that identity only exists per dispatched tool call. The + * file preview adapter has to take it from the frame it is processing. + */ +function turnScopedExecContext(): ExecutionContext { + return { + userId: 'user-1', + workflowId: 'workflow-1', + workspaceId: 'workspace-1', + messageId: 'msg-1', + copilotToolExecution: true, + } +} + describe('copilot go stream helpers', () => { beforeEach(() => { vi.stubGlobal('fetch', vi.fn()) @@ -301,14 +316,7 @@ describe('copilot go stream helpers', () => { const onEvent = vi.fn() const context = createStreamingContext() - const execContext: ExecutionContext = { - userId: 'user-1', - workflowId: 'workflow-1', - workspaceId: 'workspace-1', - messageId: 'msg-1', - copilotToolExecution: true, - toolCallId: 'stream-tool-1', - } + const execContext = turnScopedExecContext() await runStreamLoop('https://example.com/mothership/stream', {}, context, execContext, { onEvent, @@ -432,14 +440,7 @@ describe('copilot go stream helpers', () => { const onEvent = vi.fn() const context = createStreamingContext() - const execContext: ExecutionContext = { - userId: 'user-1', - workflowId: 'workflow-1', - workspaceId: 'workspace-1', - messageId: 'msg-1', - copilotToolExecution: true, - toolCallId: 'stream-tool-2', - } + const execContext = turnScopedExecContext() await runStreamLoop('https://example.com/mothership/stream', {}, context, execContext, { onEvent, diff --git a/apps/sim/lib/copilot/request/go/stream.ts b/apps/sim/lib/copilot/request/go/stream.ts index 471904c16fe..ab924af5e67 100644 --- a/apps/sim/lib/copilot/request/go/stream.ts +++ b/apps/sim/lib/copilot/request/go/stream.ts @@ -368,14 +368,26 @@ export async function runStreamLoop( return } - await processFilePreviewStreamEvent({ - streamId: envelope.stream.streamId, - streamEvent, - context, - execContext, - options, - state: filePreviewAdapterState, - }) + // Presentation only. A throw here abandons the rest of the event, so + // the tool-call frame never registers its arguments and the call is + // later dispatched with an empty payload. + try { + await processFilePreviewStreamEvent({ + streamId: envelope.stream.streamId, + streamEvent, + context, + execContext, + options, + state: filePreviewAdapterState, + }) + } catch (error) { + logger.warn('Failed to process file preview stream event', { + type: streamEvent.type, + requestId: context.requestId, + messageId: context.messageId, + error: getErrorMessage(error), + }) + } await prePersistClientExecutableToolCall(streamEvent, context, options, execContext)