From 7505cfd71ba124f3744354c3a5b0c92adfb58697 Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Wed, 12 Aug 2026 10:27:04 -0700 Subject: [PATCH 1/3] fix(copilot): surface document render failures --- .../lib/copilot/tools/handlers/vfs.test.ts | 15 ++++ apps/sim/lib/copilot/tools/handlers/vfs.ts | 3 + apps/sim/lib/copilot/vfs/file-reader.ts | 2 + .../sim/lib/copilot/vfs/workspace-vfs.test.ts | 76 +++++++++++++++++++ apps/sim/lib/copilot/vfs/workspace-vfs.ts | 6 +- 5 files changed, 100 insertions(+), 2 deletions(-) create mode 100644 apps/sim/lib/copilot/vfs/workspace-vfs.test.ts diff --git a/apps/sim/lib/copilot/tools/handlers/vfs.test.ts b/apps/sim/lib/copilot/tools/handlers/vfs.test.ts index 283f63c0709..3aa2e4ac365 100644 --- a/apps/sim/lib/copilot/tools/handlers/vfs.test.ts +++ b/apps/sim/lib/copilot/tools/handlers/vfs.test.ts @@ -313,6 +313,21 @@ describe('vfs handlers oversize policy', () => { expect(vfs.read).not.toHaveBeenCalled() }) + it('surfaces dynamic file read errors as failed tool calls', async () => { + const vfs = makeVfs() + const error = 'Document compiler not configured (MOTHERSHIP_E2B_DOC_TEMPLATE_ID is unset)' + vfs.readFileContent.mockResolvedValue({ + content: JSON.stringify({ ok: false, error }), + totalLines: 1, + error, + }) + getOrMaterializeVFS.mockResolvedValue(vfs) + + const result = await executeVfsRead({ path: 'files/reports/brief.pdf/render' }, GREP_CTX) + + expect(result).toEqual({ success: false, error }) + }) + it('marks a windowed read as a derived provenance view', async () => { const vfs = makeVfs() vfs.readFileContentWithProvenance.mockResolvedValue({ diff --git a/apps/sim/lib/copilot/tools/handlers/vfs.ts b/apps/sim/lib/copilot/tools/handlers/vfs.ts index ba8bd734bca..f74ae8fade5 100644 --- a/apps/sim/lib/copilot/tools/handlers/vfs.ts +++ b/apps/sim/lib/copilot/tools/handlers/vfs.ts @@ -405,6 +405,9 @@ export async function executeVfsRead( : null const fileContent = fileEnvelope?.value if (fileContent) { + if (fileContent.error !== undefined) { + return { success: false, error: fileContent.error } + } const isAttachment = hasModelAttachment(fileContent) if ( !isAttachment && diff --git a/apps/sim/lib/copilot/vfs/file-reader.ts b/apps/sim/lib/copilot/vfs/file-reader.ts index 8c96d54a6a8..3940152b024 100644 --- a/apps/sim/lib/copilot/vfs/file-reader.ts +++ b/apps/sim/lib/copilot/vfs/file-reader.ts @@ -447,6 +447,8 @@ export interface FileReadResult { totalLines: number /** Set when `content` stands in for the file rather than being it — see `readPlaceholder`. */ placeholder?: PlaceholderKind + /** Set when a dynamic read resolved the file but failed to produce its requested view. */ + error?: string attachment?: { type: string name?: string diff --git a/apps/sim/lib/copilot/vfs/workspace-vfs.test.ts b/apps/sim/lib/copilot/vfs/workspace-vfs.test.ts new file mode 100644 index 00000000000..3f57ebe363f --- /dev/null +++ b/apps/sim/lib/copilot/vfs/workspace-vfs.test.ts @@ -0,0 +1,76 @@ +/** + * @vitest-environment node + */ + +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { renderDocToGrid } = vi.hoisted(() => ({ + renderDocToGrid: vi.fn(), +})) + +const { findWorkspaceFileRecord, listAllWorkspaceFilesExecute, readWorkspaceFileContentExecute } = + vi.hoisted(() => ({ + findWorkspaceFileRecord: vi.fn(), + listAllWorkspaceFilesExecute: vi.fn(), + readWorkspaceFileContentExecute: vi.fn(), + })) + +vi.mock('@/lib/copilot/tools/server/files/doc-render', () => ({ + isRenderableDocExt: (ext: string) => ['docx', 'pdf', 'pptx'].includes(ext.toLowerCase()), + renderDocToGrid, +})) + +vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({ + findWorkspaceFileRecord, +})) + +vi.mock('@/lib/workspace-files/application/list-workspace-files', () => ({ + listAllWorkspaceFiles: { execute: listAllWorkspaceFilesExecute }, +})) + +vi.mock('@/lib/workspace-files/application/read-workspace-file-content', () => ({ + readWorkspaceFileContent: { execute: readWorkspaceFileContentExecute }, +})) + +import { WorkspaceVFS } from '@/lib/copilot/vfs/workspace-vfs' + +describe('WorkspaceVFS dynamic render reads', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('marks render exceptions as file read errors', async () => { + const record = { + id: 'file-1', + workspaceId: 'ws-1', + name: 'brief.pdf', + key: 'brief.pdf', + path: '/api/files/serve/brief.pdf', + size: 8, + type: 'application/pdf', + uploadedBy: 'user-1', + deletedAt: null, + uploadedAt: new Date('2026-01-01T00:00:00.000Z'), + updatedAt: new Date('2026-01-01T00:00:00.000Z'), + storageContext: 'mothership' as const, + } + listAllWorkspaceFilesExecute.mockResolvedValue({ files: [record] }) + findWorkspaceFileRecord.mockReturnValue(record) + readWorkspaceFileContentExecute.mockResolvedValue({ content: Buffer.from('%PDF-1.7') }) + renderDocToGrid.mockRejectedValue( + new Error('Document compiler not configured (MOTHERSHIP_E2B_DOC_TEMPLATE_ID is unset)') + ) + + const vfs = new WorkspaceVFS({ kind: 'session', userId: 'user-1', sessionId: 'session-1' }) + Object.assign(vfs, { _workspaceId: 'ws-1' }) + + const result = await vfs.readFileContent('files/brief.pdf/render') + + expect(result).toEqual({ + content: + '{"ok":false,"error":"Document compiler not configured (MOTHERSHIP_E2B_DOC_TEMPLATE_ID is unset)"}', + totalLines: 1, + error: 'Document compiler not configured (MOTHERSHIP_E2B_DOC_TEMPLATE_ID is unset)', + }) + }) +}) diff --git a/apps/sim/lib/copilot/vfs/workspace-vfs.ts b/apps/sim/lib/copilot/vfs/workspace-vfs.ts index 1cdb3dd34b2..e91245e29be 100644 --- a/apps/sim/lib/copilot/vfs/workspace-vfs.ts +++ b/apps/sim/lib/copilot/vfs/workspace-vfs.ts @@ -1355,18 +1355,20 @@ export class WorkspaceVFS { ) return bindWorkspaceFileResult(record, rendered, 'derived', [...contributingFiles.values()]) } catch (err) { + const error = toError(err).message logger.warn('Render read failed via VFS', { workspaceId: this._workspaceId, path, fileId: record?.id, - error: toError(err).message, + error, }) // Return an explicit error (not null) once the file resolved — a null read // looks like a missing path and sends the agent hunting for the "correct" // render path instead of surfacing the real compile/render failure. const errorResult = { - content: JSON.stringify({ ok: false, error: toError(err).message }), + content: JSON.stringify({ ok: false, error }), totalLines: 1, + error, } return record ? bindWorkspaceFileResult(record, errorResult) : { value: errorResult } } From b893927aea5a30b3d17c85ad2328297759bed237 Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Wed, 12 Aug 2026 10:36:22 -0700 Subject: [PATCH 2/3] Address PR review feedback (#6629) - validate render errors against workspace-file provenance before returning details\n- cover blocked provenance with a regression test --- .../lib/copilot/tools/handlers/vfs.test.ts | 29 +++++++++++++++++++ apps/sim/lib/copilot/tools/handlers/vfs.ts | 6 ++-- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/apps/sim/lib/copilot/tools/handlers/vfs.test.ts b/apps/sim/lib/copilot/tools/handlers/vfs.test.ts index 3aa2e4ac365..5f89535f76a 100644 --- a/apps/sim/lib/copilot/tools/handlers/vfs.test.ts +++ b/apps/sim/lib/copilot/tools/handlers/vfs.test.ts @@ -328,6 +328,35 @@ describe('vfs handlers oversize policy', () => { expect(result).toEqual({ success: false, error }) }) + it('does not expose dynamic file read errors when provenance cannot be verified', async () => { + const vfs = makeVfs() + const error = 'Document compiler not configured (MOTHERSHIP_E2B_DOC_TEMPLATE_ID is unset)' + vfs.readFileContentWithProvenance.mockResolvedValue({ + value: { + content: JSON.stringify({ ok: false, error }), + totalLines: 1, + error, + }, + file: { fileId: 'file-1', key: 'workspace/key-1', context: 'workspace' }, + }) + getOrMaterializeVFS.mockResolvedValue(vfs) + importWorkspaceFileSecretProvenanceForModelView.mockResolvedValueOnce(false) + + const result = await executeVfsRead({ path: 'files/reports/brief.pdf/render' }, GREP_CTX) + + expect(result).toEqual({ + success: false, + error: + 'This file result cannot be shared safely because its secret provenance is unavailable.', + }) + expect(importWorkspaceFileSecretProvenanceForModelView).toHaveBeenCalledWith( + expect.objectContaining({ + identity: { fileId: 'file-1', key: 'workspace/key-1', context: 'workspace' }, + view: 'derived', + }) + ) + }) + it('marks a windowed read as a derived provenance view', async () => { const vfs = makeVfs() vfs.readFileContentWithProvenance.mockResolvedValue({ diff --git a/apps/sim/lib/copilot/tools/handlers/vfs.ts b/apps/sim/lib/copilot/tools/handlers/vfs.ts index f74ae8fade5..dfa61ea3881 100644 --- a/apps/sim/lib/copilot/tools/handlers/vfs.ts +++ b/apps/sim/lib/copilot/tools/handlers/vfs.ts @@ -405,9 +405,6 @@ export async function executeVfsRead( : null const fileContent = fileEnvelope?.value if (fileContent) { - if (fileContent.error !== undefined) { - return { success: false, error: fileContent.error } - } const isAttachment = hasModelAttachment(fileContent) if ( !isAttachment && @@ -445,6 +442,9 @@ export async function executeVfsRead( 'This file result cannot be shared safely because its secret provenance is unavailable.', } } + if (fileContent.error !== undefined) { + return { success: false, error: fileContent.error } + } logger.debug('vfs_read resolved workspace file', { path, totalLines: fileContent.totalLines, From 3ad89f29d6bd1c976f46467459fb76ea79cd013a Mon Sep 17 00:00:00 2001 From: Justin Blumencranz <96924014+j15z@users.noreply.github.com> Date: Wed, 12 Aug 2026 11:30:11 -0700 Subject: [PATCH 3/3] 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. --- .../sim/lib/copilot/vfs/workspace-vfs.test.ts | 98 +++++++++++++++---- apps/sim/lib/copilot/vfs/workspace-vfs.ts | 45 ++++----- 2 files changed, 94 insertions(+), 49 deletions(-) diff --git a/apps/sim/lib/copilot/vfs/workspace-vfs.test.ts b/apps/sim/lib/copilot/vfs/workspace-vfs.test.ts index 3f57ebe363f..9ea57ae9350 100644 --- a/apps/sim/lib/copilot/vfs/workspace-vfs.test.ts +++ b/apps/sim/lib/copilot/vfs/workspace-vfs.test.ts @@ -16,7 +16,8 @@ const { findWorkspaceFileRecord, listAllWorkspaceFilesExecute, readWorkspaceFile })) vi.mock('@/lib/copilot/tools/server/files/doc-render', () => ({ - isRenderableDocExt: (ext: string) => ['docx', 'pdf', 'pptx'].includes(ext.toLowerCase()), + // `odt` exposes the defensive missing-task branch independently from the extension guard. + isRenderableDocExt: (ext: string) => ['docx', 'odt', 'pdf', 'pptx'].includes(ext.toLowerCase()), renderDocToGrid, })) @@ -34,36 +35,52 @@ vi.mock('@/lib/workspace-files/application/read-workspace-file-content', () => ( import { WorkspaceVFS } from '@/lib/copilot/vfs/workspace-vfs' +const MAX_DOC_READ_INPUT_BYTES = 50 * 1024 * 1024 +const MAX_DOCUMENT_PREVIEW_CODE_BYTES = 1024 * 1024 + +function arrangeRenderRead({ + name = 'brief.pdf', + size = 8, + content = Buffer.from('%PDF-1.7'), +}: { + name?: string + size?: number + content?: Buffer | { length: number } +} = {}) { + const record = { + id: 'file-1', + workspaceId: 'ws-1', + name, + key: name, + path: `/api/files/serve/${name}`, + size, + type: 'application/octet-stream', + uploadedBy: 'user-1', + deletedAt: null, + uploadedAt: new Date('2026-01-01T00:00:00.000Z'), + updatedAt: new Date('2026-01-01T00:00:00.000Z'), + storageContext: 'mothership' as const, + } + listAllWorkspaceFilesExecute.mockResolvedValue({ files: [record] }) + findWorkspaceFileRecord.mockReturnValue(record) + readWorkspaceFileContentExecute.mockResolvedValue({ content }) + + const vfs = new WorkspaceVFS({ kind: 'session', userId: 'user-1', sessionId: 'session-1' }) + Object.assign(vfs, { _workspaceId: 'ws-1' }) + return vfs +} + describe('WorkspaceVFS dynamic render reads', () => { beforeEach(() => { vi.clearAllMocks() }) it('marks render exceptions as file read errors', async () => { - const record = { - id: 'file-1', - workspaceId: 'ws-1', - name: 'brief.pdf', - key: 'brief.pdf', - path: '/api/files/serve/brief.pdf', - size: 8, - type: 'application/pdf', - uploadedBy: 'user-1', - deletedAt: null, - uploadedAt: new Date('2026-01-01T00:00:00.000Z'), - updatedAt: new Date('2026-01-01T00:00:00.000Z'), - storageContext: 'mothership' as const, - } - listAllWorkspaceFilesExecute.mockResolvedValue({ files: [record] }) - findWorkspaceFileRecord.mockReturnValue(record) - readWorkspaceFileContentExecute.mockResolvedValue({ content: Buffer.from('%PDF-1.7') }) + const vfs = arrangeRenderRead() renderDocToGrid.mockRejectedValue( new Error('Document compiler not configured (MOTHERSHIP_E2B_DOC_TEMPLATE_ID is unset)') ) - const vfs = new WorkspaceVFS({ kind: 'session', userId: 'user-1', sessionId: 'session-1' }) - Object.assign(vfs, { _workspaceId: 'ws-1' }) - const result = await vfs.readFileContent('files/brief.pdf/render') expect(result).toEqual({ @@ -73,4 +90,43 @@ describe('WorkspaceVFS dynamic render reads', () => { error: 'Document compiler not configured (MOTHERSHIP_E2B_DOC_TEMPLATE_ID is unset)', }) }) + + it.each([ + { + label: 'unsupported extensions', + name: 'brief.txt', + error: 'Render supports .pptx, .docx, and .pdf only', + }, + { + label: 'oversized file metadata', + size: MAX_DOC_READ_INPUT_BYTES + 1, + error: 'File is too large to render', + }, + { + label: 'oversized fetched buffers', + content: { length: MAX_DOC_READ_INPUT_BYTES + 1 }, + error: 'File is too large to render', + }, + { + label: 'oversized source', + content: Buffer.alloc(MAX_DOCUMENT_PREVIEW_CODE_BYTES + 1, 'a'), + error: 'File source exceeds maximum size', + }, + { + label: 'missing render tasks', + name: 'brief.odt', + content: Buffer.from('document source'), + error: 'Cannot render this file', + }, + ])('marks $label as file read errors', async ({ name, size, content, error }) => { + const vfs = arrangeRenderRead({ name, size, content }) + + const result = await vfs.readFileContent(`files/${name ?? 'brief.pdf'}/render`) + + expect(result).toEqual({ + content: JSON.stringify({ ok: false, error }), + totalLines: 1, + error, + }) + }) }) diff --git a/apps/sim/lib/copilot/vfs/workspace-vfs.ts b/apps/sim/lib/copilot/vfs/workspace-vfs.ts index e91245e29be..00d7c52d106 100644 --- a/apps/sim/lib/copilot/vfs/workspace-vfs.ts +++ b/apps/sim/lib/copilot/vfs/workspace-vfs.ts @@ -184,6 +184,14 @@ function bindWorkspaceFileResult( } } +function renderErrorResult(error: string): FileReadResult { + return { + content: JSON.stringify({ ok: false, error }), + totalLines: 1, + error, + } +} + function recordContributingFile( files: Map, identity: WorkspaceFileSecretProvenanceIdentity @@ -1103,10 +1111,7 @@ export class WorkspaceVFS { contributingFiles: Map ): Promise { if (typeof record.size === 'number' && record.size > MAX_DOC_READ_INPUT_BYTES) { - return { - content: JSON.stringify({ ok: false, error: 'File is too large to render' }), - totalLines: 1, - } + return renderErrorResult('File is too large to render') } const { content: buffer } = await readWorkspaceFileContent.execute({ principal: this.requireFilePrincipal(), @@ -1117,10 +1122,7 @@ export class WorkspaceVFS { }, }) if (buffer.length > MAX_DOC_READ_INPUT_BYTES) { - return { - content: JSON.stringify({ ok: false, error: 'File is too large to render' }), - totalLines: 1, - } + return renderErrorResult('File is too large to render') } // Already-binary uploads render directly; source files are compiled first // (E2B regime -> doc sandbox: Node pptx/docx, Python pdf; otherwise @@ -1131,10 +1133,7 @@ export class WorkspaceVFS { } else { const code = buffer.toString('utf-8') if (Buffer.byteLength(code, 'utf-8') > MAX_DOCUMENT_PREVIEW_CODE_BYTES) { - return { - content: JSON.stringify({ ok: false, error: 'File source exceeds maximum size' }), - totalLines: 1, - } + return renderErrorResult('File source exceeds maximum size') } if (isDocSandboxEnabled && (await getE2BDocFormat(record.name))) { bin = ( @@ -1148,10 +1147,7 @@ export class WorkspaceVFS { } else { const taskId = BINARY_DOC_TASKS[ext] if (!taskId) { - return { - content: JSON.stringify({ ok: false, error: 'Cannot render this file' }), - totalLines: 1, - } + return renderErrorResult('Cannot render this file') } bin = await runSandboxTask( taskId, @@ -1337,13 +1333,10 @@ export class WorkspaceVFS { if (!record) return null const ext = record.name.split('.').pop()?.toLowerCase() ?? '' if (!isRenderableDocExt(ext)) { - return bindWorkspaceFileResult(record, { - content: JSON.stringify({ - ok: false, - error: 'Render supports .pptx, .docx, and .pdf only', - }), - totalLines: 1, - }) + return bindWorkspaceFileResult( + record, + renderErrorResult('Render supports .pptx, .docx, and .pdf only') + ) } const renderName = record.name const rendered = await this.renderDocRecordResult( @@ -1365,11 +1358,7 @@ export class WorkspaceVFS { // Return an explicit error (not null) once the file resolved — a null read // looks like a missing path and sends the agent hunting for the "correct" // render path instead of surfacing the real compile/render failure. - const errorResult = { - content: JSON.stringify({ ok: false, error }), - totalLines: 1, - error, - } + const errorResult = renderErrorResult(error) return record ? bindWorkspaceFileResult(record, errorResult) : { value: errorResult } } }