Skip to content

Commit b7a3e4d

Browse files
committed
fix(copilot): keep a file-preview failure from stripping a tool call's arguments
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
1 parent 7b3e6a6 commit b7a3e4d

4 files changed

Lines changed: 187 additions & 50 deletions

File tree

apps/sim/lib/copilot/request/go/file-preview-adapter.test.ts

Lines changed: 107 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,20 @@ import {
99
MothershipStreamV1ToolPhase,
1010
} from '@/lib/copilot/generated/mothership-stream-v1'
1111

12-
const { peekFileIntentMock } = vi.hoisted(() => ({
12+
const { peekFileIntentMock, executeCopilotFileUseCaseMock } = vi.hoisted(() => ({
1313
peekFileIntentMock: vi.fn(),
14+
executeCopilotFileUseCaseMock: vi.fn(),
1415
}))
1516

1617
vi.mock('@/lib/copilot/tools/server/files/file-intent-store', () => ({
1718
peekFileIntent: peekFileIntentMock,
1819
}))
1920

21+
vi.mock('@/lib/copilot/application/execute-file-use-case', () => ({
22+
executeCopilotFileUseCase: executeCopilotFileUseCaseMock,
23+
resolveCopilotWorkspaceFileReference: vi.fn(),
24+
}))
25+
2026
import { createStreamingContext } from '@/lib/copilot/request/context/request-context'
2127
import {
2228
createFilePreviewAdapterState,
@@ -31,8 +37,7 @@ const EDIT_TOOL_CALL_ID = 'edit-content-1'
3137
const WORKSPACE_FILE_TOOL_CALL_ID = 'workspace-file-1'
3238
const BASE_VERSION_MS = 900_000
3339

34-
/** One args_delta chunk of the streamed `edit_content` JSON, as a driveable StreamEvent. */
35-
function editContentDelta(argumentsDelta: string): StreamEvent {
40+
function toolEvent(payload: Record<string, unknown>): StreamEvent {
3641
return eventToStreamEvent(
3742
createEvent({
3843
streamId: STREAM_ID,
@@ -41,17 +46,38 @@ function editContentDelta(argumentsDelta: string): StreamEvent {
4146
requestId: 'req-1',
4247
type: MothershipStreamV1EventType.tool,
4348
payload: {
44-
toolCallId: EDIT_TOOL_CALL_ID,
45-
toolName: 'edit_content',
4649
executor: MothershipStreamV1ToolExecutor.sim,
4750
mode: MothershipStreamV1ToolMode.async,
48-
phase: MothershipStreamV1ToolPhase.args_delta,
49-
argumentsDelta,
51+
...payload,
5052
},
5153
})
5254
)
5355
}
5456

57+
/** One args_delta chunk of the streamed `edit_content` JSON, as a driveable StreamEvent. */
58+
function editContentDelta(argumentsDelta: string): StreamEvent {
59+
return toolEvent({
60+
toolCallId: EDIT_TOOL_CALL_ID,
61+
toolName: 'edit_content',
62+
phase: MothershipStreamV1ToolPhase.args_delta,
63+
argumentsDelta,
64+
})
65+
}
66+
67+
/** The authoritative `workspace_file` call frame for a path-targeted update. */
68+
function workspaceFileCall(): StreamEvent {
69+
return toolEvent({
70+
toolCallId: WORKSPACE_FILE_TOOL_CALL_ID,
71+
toolName: 'workspace_file',
72+
phase: MothershipStreamV1ToolPhase.call,
73+
arguments: {
74+
operation: 'update',
75+
title: 'Refresh the runbook',
76+
target: { kind: 'path', path: 'files/notes.md' },
77+
},
78+
})
79+
}
80+
5581
function makeIntent(overrides: {
5682
operation: string
5783
fileId?: string
@@ -140,3 +166,77 @@ describe('processFilePreviewStreamEvent — preview content emission', () => {
140166
expect(combined).toContain('world')
141167
})
142168
})
169+
170+
/**
171+
* The adapter runs while the tool-call frame is still on the wire, so the
172+
* execution context it receives is turn-scoped and carries no `toolCallId`.
173+
* The file delegation requires one, so the adapter has to supply the frame's
174+
* own id — otherwise resolving the preview target throws, the SSE handler
175+
* abandons the rest of the event, and the tool call is later dispatched with
176+
* no arguments at all.
177+
*/
178+
describe('processFilePreviewStreamEvent — preview target resolution', () => {
179+
const TURN_EXEC_CONTEXT: ExecutionContext = {
180+
userId: 'user-1',
181+
workflowId: 'workflow-1',
182+
workspaceId: 'workspace-1',
183+
chatId: 'chat-1',
184+
messageId: 'msg-1',
185+
copilotToolExecution: true,
186+
}
187+
188+
beforeEach(() => {
189+
vi.clearAllMocks()
190+
executeCopilotFileUseCaseMock.mockResolvedValue({
191+
files: [{ id: 'file-9', name: 'notes.md', folderPath: null }],
192+
})
193+
})
194+
195+
it('resolves the path target under the streaming tool call identity', async () => {
196+
const context = createStreamingContext()
197+
198+
await processFilePreviewStreamEvent({
199+
streamId: STREAM_ID,
200+
streamEvent: workspaceFileCall(),
201+
context,
202+
execContext: TURN_EXEC_CONTEXT,
203+
options: { onEvent: () => {} },
204+
state: createFilePreviewAdapterState(),
205+
})
206+
207+
expect(executeCopilotFileUseCaseMock).toHaveBeenCalled()
208+
expect(executeCopilotFileUseCaseMock.mock.calls[0][0]).toMatchObject({
209+
userId: 'user-1',
210+
workspaceId: 'workspace-1',
211+
copilotToolExecution: true,
212+
toolCallId: WORKSPACE_FILE_TOOL_CALL_ID,
213+
})
214+
expect(context.activeFileIntents.get('')?.target).toEqual({
215+
kind: 'file_id',
216+
fileId: 'file-9',
217+
fileName: 'notes.md',
218+
path: 'files/notes.md',
219+
})
220+
})
221+
222+
it('keeps the path target and does not throw when resolution fails', async () => {
223+
executeCopilotFileUseCaseMock.mockRejectedValue(new Error('workspace file listing unavailable'))
224+
const context = createStreamingContext()
225+
226+
await expect(
227+
processFilePreviewStreamEvent({
228+
streamId: STREAM_ID,
229+
streamEvent: workspaceFileCall(),
230+
context,
231+
execContext: TURN_EXEC_CONTEXT,
232+
options: { onEvent: () => {} },
233+
state: createFilePreviewAdapterState(),
234+
})
235+
).resolves.toBeUndefined()
236+
237+
expect(context.activeFileIntents.get('')?.target).toEqual({
238+
kind: 'path',
239+
path: 'files/notes.md',
240+
})
241+
})
242+
})

apps/sim/lib/copilot/request/go/file-preview-adapter.ts

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,24 @@ function toPreviewTargetKind(kind: string | undefined): FilePreviewTargetKind |
6666
return kind === 'new_file' || kind === 'file_id' ? kind : undefined
6767
}
6868

69+
/**
70+
* Binds the turn-scoped execution context to the tool call whose preview is
71+
* being rendered. This adapter runs while the tool-call frame is still on the
72+
* wire, before per-call dispatch builds a call-scoped context, so the turn
73+
* context carries no `toolCallId` of its own — the frame is the only place that
74+
* identity exists, and the file delegation requires one.
75+
*/
76+
function bindPreviewToolCall(context: ExecutionContext, toolCallId: string): ExecutionContext {
77+
return { ...context, toolCallId }
78+
}
79+
80+
/**
81+
* Upgrades a model-supplied path target to the backing file id so the preview
82+
* can seed itself from the existing content. Resolution is best effort, exactly
83+
* like the preview base load: leaving the target as a path only costs an
84+
* un-seeded preview until the tool result arrives with the real file id, so a
85+
* failure here must never escape into the stream loop.
86+
*/
6987
async function resolvePreviewTarget(args: {
7088
context: ExecutionContext
7189
workspaceId?: string
@@ -74,24 +92,29 @@ async function resolvePreviewTarget(args: {
7492
if (args.target.kind !== 'path' || !args.workspaceId || !args.target.path) {
7593
return args.target
7694
}
77-
if (!args.context.copilotToolExecution || !args.context.toolCallId) {
78-
throw new Error('Workspace file preview requires a trusted Copilot execution context')
79-
}
8095

81-
const { files } = await executeCopilotFileUseCase(args.context, listAllWorkspaceFiles, {
82-
workspaceId: args.workspaceId,
83-
scope: 'active',
84-
})
85-
const file = findWorkspaceFileRecord(files, args.target.path)
86-
if (!file) {
87-
return args.target
88-
}
96+
try {
97+
const { files } = await executeCopilotFileUseCase(args.context, listAllWorkspaceFiles, {
98+
workspaceId: args.workspaceId,
99+
scope: 'active',
100+
})
101+
const file = findWorkspaceFileRecord(files, args.target.path)
102+
if (!file) {
103+
return args.target
104+
}
89105

90-
return {
91-
kind: 'file_id',
92-
fileId: file.id,
93-
fileName: args.target.fileName ?? file.name,
94-
path: args.target.path,
106+
return {
107+
kind: 'file_id',
108+
fileId: file.id,
109+
fileName: args.target.fileName ?? file.name,
110+
path: args.target.path,
111+
}
112+
} catch (error) {
113+
logger.warn('Failed to resolve workspace file preview target', {
114+
toolCallId: args.context.toolCallId,
115+
error: toError(error).message,
116+
})
117+
return args.target
95118
}
96119
}
97120

@@ -375,8 +398,9 @@ export async function processFilePreviewStreamEvent(input: {
375398
const parsedArgs = parseWorkspaceFileArgs(streamEvent.payload.arguments)
376399
if (toolCallId && parsedArgs) {
377400
const { operation, title, contentType, edit } = parsedArgs
401+
const previewContext = bindPreviewToolCall(execContext, toolCallId)
378402
const target = await resolvePreviewTarget({
379-
context: execContext,
403+
context: previewContext,
380404
workspaceId: execContext.workspaceId,
381405
target: parsedArgs.target,
382406
})
@@ -405,7 +429,7 @@ export async function processFilePreviewStreamEvent(input: {
405429
(operation === 'append' || operation === 'patch')
406430
) {
407431
previewBase = await loadWorkspaceFileTextForPreview(
408-
execContext,
432+
previewContext,
409433
execContext.workspaceId,
410434
fileId
411435
)
@@ -480,7 +504,7 @@ export async function processFilePreviewStreamEvent(input: {
480504
(intent.operation === 'append' || intent.operation === 'patch')
481505
) {
482506
previewBase = await loadWorkspaceFileTextForPreview(
483-
execContext,
507+
bindPreviewToolCall(execContext, intent.toolCallId),
484508
execContext.workspaceId,
485509
result.fileId
486510
)

apps/sim/lib/copilot/request/go/stream.test.ts

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,21 @@ function createStreamingContext(): StreamingContext {
147147
}
148148
}
149149

150+
/**
151+
* The turn-scoped execution context exactly as the chat lifecycle builds it: no
152+
* `toolCallId`, because that identity only exists per dispatched tool call. The
153+
* file preview adapter has to take it from the frame it is processing.
154+
*/
155+
function turnScopedExecContext(): ExecutionContext {
156+
return {
157+
userId: 'user-1',
158+
workflowId: 'workflow-1',
159+
workspaceId: 'workspace-1',
160+
messageId: 'msg-1',
161+
copilotToolExecution: true,
162+
}
163+
}
164+
150165
describe('copilot go stream helpers', () => {
151166
beforeEach(() => {
152167
vi.stubGlobal('fetch', vi.fn())
@@ -301,14 +316,7 @@ describe('copilot go stream helpers', () => {
301316

302317
const onEvent = vi.fn()
303318
const context = createStreamingContext()
304-
const execContext: ExecutionContext = {
305-
userId: 'user-1',
306-
workflowId: 'workflow-1',
307-
workspaceId: 'workspace-1',
308-
messageId: 'msg-1',
309-
copilotToolExecution: true,
310-
toolCallId: 'stream-tool-1',
311-
}
319+
const execContext = turnScopedExecContext()
312320

313321
await runStreamLoop('https://example.com/mothership/stream', {}, context, execContext, {
314322
onEvent,
@@ -432,14 +440,7 @@ describe('copilot go stream helpers', () => {
432440

433441
const onEvent = vi.fn()
434442
const context = createStreamingContext()
435-
const execContext: ExecutionContext = {
436-
userId: 'user-1',
437-
workflowId: 'workflow-1',
438-
workspaceId: 'workspace-1',
439-
messageId: 'msg-1',
440-
copilotToolExecution: true,
441-
toolCallId: 'stream-tool-2',
442-
}
443+
const execContext = turnScopedExecContext()
443444

444445
await runStreamLoop('https://example.com/mothership/stream', {}, context, execContext, {
445446
onEvent,

apps/sim/lib/copilot/request/go/stream.ts

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -368,14 +368,26 @@ export async function runStreamLoop(
368368
return
369369
}
370370

371-
await processFilePreviewStreamEvent({
372-
streamId: envelope.stream.streamId,
373-
streamEvent,
374-
context,
375-
execContext,
376-
options,
377-
state: filePreviewAdapterState,
378-
})
371+
// Presentation only. A throw here abandons the rest of the event, so
372+
// the tool-call frame never registers its arguments and the call is
373+
// later dispatched with an empty payload.
374+
try {
375+
await processFilePreviewStreamEvent({
376+
streamId: envelope.stream.streamId,
377+
streamEvent,
378+
context,
379+
execContext,
380+
options,
381+
state: filePreviewAdapterState,
382+
})
383+
} catch (error) {
384+
logger.warn('Failed to process file preview stream event', {
385+
type: streamEvent.type,
386+
requestId: context.requestId,
387+
messageId: context.messageId,
388+
error: getErrorMessage(error),
389+
})
390+
}
379391

380392
await prePersistClientExecutableToolCall(streamEvent, context, options, execContext)
381393

0 commit comments

Comments
 (0)