mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
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.
This commit is contained in:
@@ -313,6 +313,50 @@ 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('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({
|
||||
|
||||
@@ -442,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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -0,0 +1,132 @@
|
||||
/**
|
||||
* @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', () => ({
|
||||
// `odt` exposes the defensive missing-task branch independently from the extension guard.
|
||||
isRenderableDocExt: (ext: string) => ['docx', 'odt', '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'
|
||||
|
||||
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 vfs = arrangeRenderRead()
|
||||
renderDocToGrid.mockRejectedValue(
|
||||
new Error('Document compiler not configured (MOTHERSHIP_E2B_DOC_TEMPLATE_ID is unset)')
|
||||
)
|
||||
|
||||
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)',
|
||||
})
|
||||
})
|
||||
|
||||
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,
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -184,6 +184,14 @@ function bindWorkspaceFileResult<T>(
|
||||
}
|
||||
}
|
||||
|
||||
function renderErrorResult(error: string): FileReadResult {
|
||||
return {
|
||||
content: JSON.stringify({ ok: false, error }),
|
||||
totalLines: 1,
|
||||
error,
|
||||
}
|
||||
}
|
||||
|
||||
function recordContributingFile(
|
||||
files: Map<string, WorkspaceFileSecretProvenanceIdentity>,
|
||||
identity: WorkspaceFileSecretProvenanceIdentity
|
||||
@@ -1103,10 +1111,7 @@ export class WorkspaceVFS {
|
||||
contributingFiles: Map<string, WorkspaceFileSecretProvenanceIdentity>
|
||||
): Promise<FileReadResult> {
|
||||
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(
|
||||
@@ -1355,19 +1348,17 @@ 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 }),
|
||||
totalLines: 1,
|
||||
}
|
||||
const errorResult = renderErrorResult(error)
|
||||
return record ? bindWorkspaceFileResult(record, errorResult) : { value: errorResult }
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user