Skip to content

Commit beeb425

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(files): persist large Agiloft and Cursor downloads before serialization
1 parent 23afa2a commit beeb425

16 files changed

Lines changed: 433 additions & 135 deletions

File tree

.agents/skills/add-integration/SKILL.md

Lines changed: 25 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -523,46 +523,31 @@ registry/direct-handler test. There is no HTTP fallback.
523523

524524
### File Output Pattern (Downloads)
525525

526-
For tools that return files, use `FileToolProcessor` to store files and return `UserFile` objects.
527-
528-
#### In Tool transformResponse
529-
530-
```typescript
531-
import { FileToolProcessor } from '@/executor/utils/file-tool-processor'
532-
533-
transformResponse: async (response, context) => {
534-
const data = await response.json()
535-
536-
// Process file outputs to UserFile objects
537-
const fileProcessor = new FileToolProcessor(context)
538-
const file = await fileProcessor.processFileData({
539-
data: data.content, // base64 or buffer
540-
mimeType: data.mimeType,
541-
filename: data.filename,
542-
})
543-
544-
return {
545-
success: true,
546-
output: { file },
547-
}
548-
}
549-
```
550-
551-
#### In the operation handler (for complex file handling)
552-
553-
```typescript
554-
// Return file data that FileToolProcessor can handle. No API route is involved.
555-
return Response.json({
556-
success: true,
557-
output: {
558-
file: {
559-
data: base64Content,
560-
mimeType: 'application/pdf',
561-
filename: 'document.pdf',
562-
},
563-
},
564-
})
565-
```
526+
Declare downloads as `file` / `file[]` outputs and return canonical `UserFile` objects.
527+
Internal operation responses are capped at 10 MiB **before** `transformResponse` and
528+
`FileToolProcessor` run. Inline base64 expands the bytes by roughly one third, so it
529+
cannot carry a download near that limit. Persist downloads in the server operation
530+
**before `Response.json`**, not in a response transform.
531+
532+
Follow `executeQuickBooksDownloadDocument` or `executeAgiloftRetrieveAttachment`:
533+
534+
- Derive storage scope only from trusted `request.context`, never tool parameters.
535+
Use `uploadExecutionFile` for a complete workspace/workflow/execution scope;
536+
otherwise use `uploadCopilotFile` with the trusted user identity. Reject missing
537+
storage authority before downloading. Do not fabricate an `ExecutionContext`.
538+
- Keep provider authentication, DNS-pinned downloads, byte caps and cancellation.
539+
Normalize image metadata with `resolveStoredFileMetadata` before uploading.
540+
- Return the stored file unchanged through the response schema and transform; use
541+
`userFileSchema` / `UserFile` rather than rebuilding a base64-only shape.
542+
`FileToolProcessor` passes stored files through; the executor records them for
543+
execution consumers. Do not call its private `processFileData` method.
544+
- Surface storage failures instead of falling back to an oversized inline payload.
545+
Storage helpers do not promise rollback when later execution steps fail.
546+
547+
Test provider bytes larger than the inline JSON budget through the actual handler,
548+
bounded response reader, transform and file processor, mocking provider/storage
549+
boundaries only. Preserve explicit legacy base64 outputs when they are a separate
550+
versioned contract; do not silently convert those outputs or raise the global cap.
566551

567552
### Key Helpers Reference
568553

apps/sim/executor/utils/file-tool-processor.ts

Lines changed: 1 addition & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,12 @@ import { isCanonicalBase64 } from '@/lib/api/contracts/primitives'
44
import { isUserFile } from '@/lib/core/utils/user-file'
55
import { uploadExecutionFile, uploadFileFromRawData } from '@/lib/uploads/contexts/execution'
66
import { downloadFileFromUrl } from '@/lib/uploads/utils/file-utils.server'
7-
import { MAX_FILE_SIZE, sniffImageContentType } from '@/lib/uploads/utils/validation'
7+
import { MAX_FILE_SIZE, resolveStoredFileMetadata } from '@/lib/uploads/utils/validation'
88
import type { ExecutionContext, UserFile } from '@/executor/types'
99
import type { ToolDefinition, ToolFileData } from '@/tools/types'
1010

1111
const logger = createLogger('FileToolProcessor')
1212

13-
const IMAGE_FILE_EXTENSIONS: Record<string, string> = {
14-
'image/gif': 'gif',
15-
'image/jpeg': 'jpg',
16-
'image/png': 'png',
17-
'image/webp': 'webp',
18-
}
19-
2013
/**
2114
* Strip a base64 `data:` URI prefix, leaving the encoded payload. An empty payload is
2215
* a legitimate zero-byte file; a payload that only looks empty after normalization is
@@ -43,30 +36,6 @@ function assertFileSize(size: number, fileName: string): void {
4336
}
4437
}
4538

46-
function resolveStoredFileMetadata(
47-
fileName: string,
48-
declaredMimeType: string,
49-
buffer: Buffer
50-
): { fileName: string; mimeType: string } {
51-
if (!declaredMimeType.startsWith('image/')) {
52-
return { fileName, mimeType: declaredMimeType }
53-
}
54-
55-
const mimeType = sniffImageContentType(buffer)
56-
if (!mimeType) {
57-
return {
58-
fileName: `${fileName.replace(/\.[^.]+$/, '')}.bin`,
59-
mimeType: 'application/octet-stream',
60-
}
61-
}
62-
63-
const extension = IMAGE_FILE_EXTENSIONS[mimeType]
64-
return {
65-
fileName: extension ? `${fileName.replace(/\.[^.]+$/, '')}.${extension}` : fileName,
66-
mimeType,
67-
}
68-
}
69-
7039
/**
7140
* Processes tool outputs and converts file-typed outputs to UserFile objects.
7241
* This enables tools to return file data that gets automatically stored in the

apps/sim/lib/api/contracts/tools/agiloft.ts

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { z } from 'zod'
2+
import { userFileSchema } from '@/lib/api/contracts/primitives'
23
import type {
34
ContractBody,
45
ContractBodyInput,
@@ -18,17 +19,10 @@ const optionalText = z
1819
.nullish()
1920
.transform((value) => value ?? undefined)
2021

21-
const agiloftFileOutputSchema = z.object({
22-
name: z.string(),
23-
mimeType: z.string(),
24-
data: z.string(),
25-
size: z.number(),
26-
})
27-
2822
export const agiloftRetrieveResponseSchema = z.object({
2923
success: z.literal(true),
3024
output: z.object({
31-
file: agiloftFileOutputSchema,
25+
file: userFileSchema,
3226
}),
3327
})
3428

apps/sim/lib/internal/agiloft/execute-tool.test.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -185,11 +185,14 @@ describe('executeAgiloftTool', () => {
185185
})
186186
)
187187

188-
expect(operationMocks.executeAgiloftCreateRecord).toHaveBeenCalledWith(input, {
189-
requestId: 'request-1',
190-
userId: 'user-origin',
191-
signal: controller.signal,
192-
})
188+
expect(operationMocks.executeAgiloftCreateRecord).toHaveBeenCalledWith(
189+
input,
190+
expect.objectContaining({
191+
requestId: 'request-1',
192+
userId: 'user-origin',
193+
signal: controller.signal,
194+
})
195+
)
193196
})
194197

195198
it('preserves non-object input and canonical validation envelopes', async () => {

apps/sim/lib/internal/agiloft/execute-tool.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,9 @@ async function executeOperation<C extends AnyApiRouteContract>(
7777
const result = await operation(parsed.data, {
7878
requestId: request.requestId,
7979
userId: request.context.executorDelegationOrigin?.subjectUserId ?? request.context.userId,
80+
workspaceId: request.context.workspaceId,
81+
workflowId: request.context.workflowId,
82+
executionId: request.context.executionId,
8083
signal: request.signal,
8184
})
8285
request.signal?.throwIfAborted()

apps/sim/lib/internal/agiloft/operations.test.ts

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ const providerMocks = vi.hoisted(() => ({
2020

2121
const fileMocks = vi.hoisted(() => ({
2222
resolveAgiloftAttachmentFile: vi.fn(),
23+
uploadCopilotFile: vi.fn(),
2324
}))
2425

2526
vi.mock('@/lib/core/security/input-validation.server', () => ({
@@ -28,6 +29,8 @@ vi.mock('@/lib/core/security/input-validation.server', () => ({
2829
}))
2930
vi.mock('@/lib/internal/agiloft/client', () => clientMocks)
3031
vi.mock('@/lib/internal/agiloft/file-input', () => fileMocks)
32+
vi.mock('@/lib/uploads/contexts/copilot/copilot-file-manager', () => fileMocks)
33+
vi.mock('@/lib/uploads/contexts/execution', () => ({ uploadExecutionFile: vi.fn() }))
3134

3235
import {
3336
executeAgiloftCreateRecord,
@@ -148,6 +151,15 @@ describe('Agiloft operations', () => {
148151

149152
it('bounds attachment downloads and preserves binary metadata', async () => {
150153
const controller = new AbortController()
154+
const storedFile = {
155+
id: 'file-1',
156+
key: 'copilot/user-1/file-1',
157+
url: '/api/files/serve/file-1',
158+
name: 'evidence.txt',
159+
type: 'text/plain',
160+
size: 5,
161+
}
162+
fileMocks.uploadCopilotFile.mockResolvedValue(storedFile)
151163
providerMocks.secureFetchWithPinnedIP.mockResolvedValue(
152164
createResponse({
153165
bytes: new TextEncoder().encode('hello'),
@@ -160,20 +172,21 @@ describe('Agiloft operations', () => {
160172

161173
const result = await executeAgiloftRetrieveAttachment(
162174
{ ...BASE, recordId: '1', fieldName: 'files', position: '0' },
163-
{ requestId: 'request-1', signal: controller.signal }
175+
{ requestId: 'request-1', userId: 'user-1', signal: controller.signal }
164176
)
165177

166178
expect(result).toEqual({
167179
success: true,
168180
output: {
169-
file: {
170-
name: 'evidence.txt',
171-
mimeType: 'text/plain',
172-
data: Buffer.from('hello').toString('base64'),
173-
size: 5,
174-
},
181+
file: storedFile,
175182
},
176183
})
184+
expect(fileMocks.uploadCopilotFile).toHaveBeenCalledWith({
185+
buffer: Buffer.from('hello'),
186+
fileName: 'evidence.txt',
187+
contentType: 'text/plain',
188+
userId: 'user-1',
189+
})
177190
expect(providerMocks.secureFetchWithPinnedIP).toHaveBeenCalledWith(
178191
expect.stringContaining('/ewws/EWRetrieve'),
179192
'203.0.113.10',

apps/sim/lib/internal/agiloft/operations.ts

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,10 @@ import {
6565
getLockHttpMethod,
6666
parseFieldList,
6767
} from '@/lib/internal/agiloft/urls'
68+
import { uploadCopilotFile } from '@/lib/uploads/contexts/copilot/copilot-file-manager'
69+
import { uploadExecutionFile } from '@/lib/uploads/contexts/execution'
6870
import { resolveEffectiveMimeType } from '@/lib/uploads/utils/file-utils'
71+
import { resolveStoredFileMetadata } from '@/lib/uploads/utils/validation'
6972
import type {
7073
AgiloftAsyncStatusResponse,
7174
AgiloftAttachmentInfoResponse,
@@ -88,6 +91,9 @@ import type { ToolResponse } from '@/tools/types'
8891
export interface AgiloftOperationContext {
8992
requestId: string
9093
userId?: string
94+
workspaceId?: string
95+
workflowId?: string
96+
executionId?: string
9197
signal?: AbortSignal
9298
}
9399

@@ -894,6 +900,20 @@ export async function executeAgiloftRetrieveAttachment(
894900
input: AgiloftRetrieveBody,
895901
context: AgiloftOperationContext
896902
): Promise<ToolResponse> {
903+
const executionContext =
904+
context.workspaceId && context.workflowId && context.executionId
905+
? {
906+
workspaceId: context.workspaceId,
907+
workflowId: context.workflowId,
908+
executionId: context.executionId,
909+
}
910+
: null
911+
if (!executionContext && !context.userId) {
912+
throw new AgiloftOperationError(401, {
913+
success: false,
914+
error: 'User context is required to store attachments',
915+
})
916+
}
897917
let resolvedIP: string
898918
try {
899919
resolvedIP = await resolveAgiloftInstance(input.instanceUrl, context.signal)
@@ -930,15 +950,25 @@ export async function executeAgiloftRetrieveAttachment(
930950
error: `Agiloft error: ${buffer.toString('utf8').slice(0, 300)}`,
931951
})
932952
}
933-
return {
934-
success: true,
935-
output: {
936-
file: {
937-
name: fileName,
938-
mimeType: resolveEffectiveMimeType(contentType, fileName),
939-
data: buffer.toString('base64'),
940-
size: buffer.length,
941-
},
942-
},
943-
}
953+
const metadata = resolveStoredFileMetadata(
954+
fileName,
955+
resolveEffectiveMimeType(contentType, fileName),
956+
buffer
957+
)
958+
const file = executionContext
959+
? await uploadExecutionFile(
960+
executionContext,
961+
buffer,
962+
metadata.fileName,
963+
metadata.mimeType,
964+
context.userId
965+
)
966+
: await uploadCopilotFile({
967+
buffer,
968+
fileName: metadata.fileName,
969+
contentType: metadata.mimeType,
970+
userId: context.userId!,
971+
})
972+
context.signal?.throwIfAborted()
973+
return { success: true, output: { file } }
944974
}

apps/sim/lib/internal/cursor/execute-tool.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ describe('executeCursorTool', () => {
4747
expect(response.status).toBe(200)
4848
expect(mocks.downloadCursorArtifact).toHaveBeenCalledWith(
4949
{ apiKey: 'cursor-key', agentId: 'agent-1', path: '/src/index.ts' },
50-
{ requestId: 'request-1', signal: controller.signal }
50+
expect.objectContaining({ requestId: 'request-1', signal: controller.signal })
5151
)
5252
}
5353
)

apps/sim/lib/internal/cursor/execute-tool.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,16 @@ export const executeCursorTool: InternalToolOperationHandler = async (request) =
3333
await downloadCursorArtifact(parsed.data, {
3434
requestId: request.requestId,
3535
signal: request.signal,
36+
...(request.toolId === 'cursor_download_artifact_v2'
37+
? {
38+
persistFile: true,
39+
userId:
40+
request.context.executorDelegationOrigin?.subjectUserId ?? request.context.userId,
41+
workspaceId: request.context.workspaceId,
42+
workflowId: request.context.workflowId,
43+
executionId: request.context.executionId,
44+
}
45+
: {}),
3646
})
3747
)
3848
} catch (error) {

apps/sim/lib/internal/cursor/operations.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@ vi.mock('@/lib/core/security/input-validation.server', () => ({
1212
secureFetchWithPinnedIP: mocks.secureFetchWithPinnedIP,
1313
validateUrlWithDNS: mocks.validateUrlWithDNS,
1414
}))
15+
vi.mock('@/lib/uploads/contexts/copilot/copilot-file-manager', () => ({
16+
uploadCopilotFile: vi.fn(),
17+
}))
18+
vi.mock('@/lib/uploads/contexts/execution', () => ({ uploadExecutionFile: vi.fn() }))
1519

1620
import { downloadCursorArtifact } from '@/lib/internal/cursor/operations'
1721

0 commit comments

Comments
 (0)