Skip to content

Commit 0e65ca3

Browse files
authored
fix(copilot): surface document render failures (#6629)
* fix(copilot): surface document render failures * Address PR review feedback (#6629) - validate render errors against workspace-file provenance before returning details\n- cover blocked provenance with a regression test * Address PR review feedback (#6629) - mark every non-throwing render failure as a failed dynamic read\n- cover all soft render failure paths with producer-level tests\n\nNote: pre-existing type-check failures in HEIC and provider files are not addressed by this PR.
1 parent 8a0b328 commit 0e65ca3

5 files changed

Lines changed: 200 additions & 28 deletions

File tree

apps/sim/lib/copilot/tools/handlers/vfs.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,50 @@ describe('vfs handlers oversize policy', () => {
313313
expect(vfs.read).not.toHaveBeenCalled()
314314
})
315315

316+
it('surfaces dynamic file read errors as failed tool calls', async () => {
317+
const vfs = makeVfs()
318+
const error = 'Document compiler not configured (MOTHERSHIP_E2B_DOC_TEMPLATE_ID is unset)'
319+
vfs.readFileContent.mockResolvedValue({
320+
content: JSON.stringify({ ok: false, error }),
321+
totalLines: 1,
322+
error,
323+
})
324+
getOrMaterializeVFS.mockResolvedValue(vfs)
325+
326+
const result = await executeVfsRead({ path: 'files/reports/brief.pdf/render' }, GREP_CTX)
327+
328+
expect(result).toEqual({ success: false, error })
329+
})
330+
331+
it('does not expose dynamic file read errors when provenance cannot be verified', async () => {
332+
const vfs = makeVfs()
333+
const error = 'Document compiler not configured (MOTHERSHIP_E2B_DOC_TEMPLATE_ID is unset)'
334+
vfs.readFileContentWithProvenance.mockResolvedValue({
335+
value: {
336+
content: JSON.stringify({ ok: false, error }),
337+
totalLines: 1,
338+
error,
339+
},
340+
file: { fileId: 'file-1', key: 'workspace/key-1', context: 'workspace' },
341+
})
342+
getOrMaterializeVFS.mockResolvedValue(vfs)
343+
importWorkspaceFileSecretProvenanceForModelView.mockResolvedValueOnce(false)
344+
345+
const result = await executeVfsRead({ path: 'files/reports/brief.pdf/render' }, GREP_CTX)
346+
347+
expect(result).toEqual({
348+
success: false,
349+
error:
350+
'This file result cannot be shared safely because its secret provenance is unavailable.',
351+
})
352+
expect(importWorkspaceFileSecretProvenanceForModelView).toHaveBeenCalledWith(
353+
expect.objectContaining({
354+
identity: { fileId: 'file-1', key: 'workspace/key-1', context: 'workspace' },
355+
view: 'derived',
356+
})
357+
)
358+
})
359+
316360
it('marks a windowed read as a derived provenance view', async () => {
317361
const vfs = makeVfs()
318362
vfs.readFileContentWithProvenance.mockResolvedValue({

apps/sim/lib/copilot/tools/handlers/vfs.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,9 @@ export async function executeVfsRead(
442442
'This file result cannot be shared safely because its secret provenance is unavailable.',
443443
}
444444
}
445+
if (fileContent.error !== undefined) {
446+
return { success: false, error: fileContent.error }
447+
}
445448
logger.debug('vfs_read resolved workspace file', {
446449
path,
447450
totalLines: fileContent.totalLines,

apps/sim/lib/copilot/vfs/file-reader.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,8 @@ export interface FileReadResult {
447447
totalLines: number
448448
/** Set when `content` stands in for the file rather than being it — see `readPlaceholder`. */
449449
placeholder?: PlaceholderKind
450+
/** Set when a dynamic read resolved the file but failed to produce its requested view. */
451+
error?: string
450452
attachment?: {
451453
type: string
452454
name?: string
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
5+
import { beforeEach, describe, expect, it, vi } from 'vitest'
6+
7+
const { renderDocToGrid } = vi.hoisted(() => ({
8+
renderDocToGrid: vi.fn(),
9+
}))
10+
11+
const { findWorkspaceFileRecord, listAllWorkspaceFilesExecute, readWorkspaceFileContentExecute } =
12+
vi.hoisted(() => ({
13+
findWorkspaceFileRecord: vi.fn(),
14+
listAllWorkspaceFilesExecute: vi.fn(),
15+
readWorkspaceFileContentExecute: vi.fn(),
16+
}))
17+
18+
vi.mock('@/lib/copilot/tools/server/files/doc-render', () => ({
19+
// `odt` exposes the defensive missing-task branch independently from the extension guard.
20+
isRenderableDocExt: (ext: string) => ['docx', 'odt', 'pdf', 'pptx'].includes(ext.toLowerCase()),
21+
renderDocToGrid,
22+
}))
23+
24+
vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({
25+
findWorkspaceFileRecord,
26+
}))
27+
28+
vi.mock('@/lib/workspace-files/application/list-workspace-files', () => ({
29+
listAllWorkspaceFiles: { execute: listAllWorkspaceFilesExecute },
30+
}))
31+
32+
vi.mock('@/lib/workspace-files/application/read-workspace-file-content', () => ({
33+
readWorkspaceFileContent: { execute: readWorkspaceFileContentExecute },
34+
}))
35+
36+
import { WorkspaceVFS } from '@/lib/copilot/vfs/workspace-vfs'
37+
38+
const MAX_DOC_READ_INPUT_BYTES = 50 * 1024 * 1024
39+
const MAX_DOCUMENT_PREVIEW_CODE_BYTES = 1024 * 1024
40+
41+
function arrangeRenderRead({
42+
name = 'brief.pdf',
43+
size = 8,
44+
content = Buffer.from('%PDF-1.7'),
45+
}: {
46+
name?: string
47+
size?: number
48+
content?: Buffer | { length: number }
49+
} = {}) {
50+
const record = {
51+
id: 'file-1',
52+
workspaceId: 'ws-1',
53+
name,
54+
key: name,
55+
path: `/api/files/serve/${name}`,
56+
size,
57+
type: 'application/octet-stream',
58+
uploadedBy: 'user-1',
59+
deletedAt: null,
60+
uploadedAt: new Date('2026-01-01T00:00:00.000Z'),
61+
updatedAt: new Date('2026-01-01T00:00:00.000Z'),
62+
storageContext: 'mothership' as const,
63+
}
64+
listAllWorkspaceFilesExecute.mockResolvedValue({ files: [record] })
65+
findWorkspaceFileRecord.mockReturnValue(record)
66+
readWorkspaceFileContentExecute.mockResolvedValue({ content })
67+
68+
const vfs = new WorkspaceVFS({ kind: 'session', userId: 'user-1', sessionId: 'session-1' })
69+
Object.assign(vfs, { _workspaceId: 'ws-1' })
70+
return vfs
71+
}
72+
73+
describe('WorkspaceVFS dynamic render reads', () => {
74+
beforeEach(() => {
75+
vi.clearAllMocks()
76+
})
77+
78+
it('marks render exceptions as file read errors', async () => {
79+
const vfs = arrangeRenderRead()
80+
renderDocToGrid.mockRejectedValue(
81+
new Error('Document compiler not configured (MOTHERSHIP_E2B_DOC_TEMPLATE_ID is unset)')
82+
)
83+
84+
const result = await vfs.readFileContent('files/brief.pdf/render')
85+
86+
expect(result).toEqual({
87+
content:
88+
'{"ok":false,"error":"Document compiler not configured (MOTHERSHIP_E2B_DOC_TEMPLATE_ID is unset)"}',
89+
totalLines: 1,
90+
error: 'Document compiler not configured (MOTHERSHIP_E2B_DOC_TEMPLATE_ID is unset)',
91+
})
92+
})
93+
94+
it.each([
95+
{
96+
label: 'unsupported extensions',
97+
name: 'brief.txt',
98+
error: 'Render supports .pptx, .docx, and .pdf only',
99+
},
100+
{
101+
label: 'oversized file metadata',
102+
size: MAX_DOC_READ_INPUT_BYTES + 1,
103+
error: 'File is too large to render',
104+
},
105+
{
106+
label: 'oversized fetched buffers',
107+
content: { length: MAX_DOC_READ_INPUT_BYTES + 1 },
108+
error: 'File is too large to render',
109+
},
110+
{
111+
label: 'oversized source',
112+
content: Buffer.alloc(MAX_DOCUMENT_PREVIEW_CODE_BYTES + 1, 'a'),
113+
error: 'File source exceeds maximum size',
114+
},
115+
{
116+
label: 'missing render tasks',
117+
name: 'brief.odt',
118+
content: Buffer.from('document source'),
119+
error: 'Cannot render this file',
120+
},
121+
])('marks $label as file read errors', async ({ name, size, content, error }) => {
122+
const vfs = arrangeRenderRead({ name, size, content })
123+
124+
const result = await vfs.readFileContent(`files/${name ?? 'brief.pdf'}/render`)
125+
126+
expect(result).toEqual({
127+
content: JSON.stringify({ ok: false, error }),
128+
totalLines: 1,
129+
error,
130+
})
131+
})
132+
})

apps/sim/lib/copilot/vfs/workspace-vfs.ts

Lines changed: 19 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,14 @@ function bindWorkspaceFileResult<T>(
184184
}
185185
}
186186

187+
function renderErrorResult(error: string): FileReadResult {
188+
return {
189+
content: JSON.stringify({ ok: false, error }),
190+
totalLines: 1,
191+
error,
192+
}
193+
}
194+
187195
function recordContributingFile(
188196
files: Map<string, WorkspaceFileSecretProvenanceIdentity>,
189197
identity: WorkspaceFileSecretProvenanceIdentity
@@ -1103,10 +1111,7 @@ export class WorkspaceVFS {
11031111
contributingFiles: Map<string, WorkspaceFileSecretProvenanceIdentity>
11041112
): Promise<FileReadResult> {
11051113
if (typeof record.size === 'number' && record.size > MAX_DOC_READ_INPUT_BYTES) {
1106-
return {
1107-
content: JSON.stringify({ ok: false, error: 'File is too large to render' }),
1108-
totalLines: 1,
1109-
}
1114+
return renderErrorResult('File is too large to render')
11101115
}
11111116
const { content: buffer } = await readWorkspaceFileContent.execute({
11121117
principal: this.requireFilePrincipal(),
@@ -1117,10 +1122,7 @@ export class WorkspaceVFS {
11171122
},
11181123
})
11191124
if (buffer.length > MAX_DOC_READ_INPUT_BYTES) {
1120-
return {
1121-
content: JSON.stringify({ ok: false, error: 'File is too large to render' }),
1122-
totalLines: 1,
1123-
}
1125+
return renderErrorResult('File is too large to render')
11241126
}
11251127
// Already-binary uploads render directly; source files are compiled first
11261128
// (E2B regime -> doc sandbox: Node pptx/docx, Python pdf; otherwise
@@ -1131,10 +1133,7 @@ export class WorkspaceVFS {
11311133
} else {
11321134
const code = buffer.toString('utf-8')
11331135
if (Buffer.byteLength(code, 'utf-8') > MAX_DOCUMENT_PREVIEW_CODE_BYTES) {
1134-
return {
1135-
content: JSON.stringify({ ok: false, error: 'File source exceeds maximum size' }),
1136-
totalLines: 1,
1137-
}
1136+
return renderErrorResult('File source exceeds maximum size')
11381137
}
11391138
if (isDocSandboxEnabled && (await getE2BDocFormat(record.name))) {
11401139
bin = (
@@ -1148,10 +1147,7 @@ export class WorkspaceVFS {
11481147
} else {
11491148
const taskId = BINARY_DOC_TASKS[ext]
11501149
if (!taskId) {
1151-
return {
1152-
content: JSON.stringify({ ok: false, error: 'Cannot render this file' }),
1153-
totalLines: 1,
1154-
}
1150+
return renderErrorResult('Cannot render this file')
11551151
}
11561152
bin = await runSandboxTask(
11571153
taskId,
@@ -1337,13 +1333,10 @@ export class WorkspaceVFS {
13371333
if (!record) return null
13381334
const ext = record.name.split('.').pop()?.toLowerCase() ?? ''
13391335
if (!isRenderableDocExt(ext)) {
1340-
return bindWorkspaceFileResult(record, {
1341-
content: JSON.stringify({
1342-
ok: false,
1343-
error: 'Render supports .pptx, .docx, and .pdf only',
1344-
}),
1345-
totalLines: 1,
1346-
})
1336+
return bindWorkspaceFileResult(
1337+
record,
1338+
renderErrorResult('Render supports .pptx, .docx, and .pdf only')
1339+
)
13471340
}
13481341
const renderName = record.name
13491342
const rendered = await this.renderDocRecordResult(
@@ -1355,19 +1348,17 @@ export class WorkspaceVFS {
13551348
)
13561349
return bindWorkspaceFileResult(record, rendered, 'derived', [...contributingFiles.values()])
13571350
} catch (err) {
1351+
const error = toError(err).message
13581352
logger.warn('Render read failed via VFS', {
13591353
workspaceId: this._workspaceId,
13601354
path,
13611355
fileId: record?.id,
1362-
error: toError(err).message,
1356+
error,
13631357
})
13641358
// Return an explicit error (not null) once the file resolved — a null read
13651359
// looks like a missing path and sends the agent hunting for the "correct"
13661360
// render path instead of surfacing the real compile/render failure.
1367-
const errorResult = {
1368-
content: JSON.stringify({ ok: false, error: toError(err).message }),
1369-
totalLines: 1,
1370-
}
1361+
const errorResult = renderErrorResult(error)
13711362
return record ? bindWorkspaceFileResult(record, errorResult) : { value: errorResult }
13721363
}
13731364
}

0 commit comments

Comments
 (0)