From 21607ba3d7df0ec4f9a0ac3b308ceb7e0359485c Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sat, 6 Jun 2026 10:03:32 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=93=8E=20fix:=20Preserve=20Provider=20Doc?= =?UTF-8?q?ument=20Uploads=20(#13550)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: Preserve provider document uploads * test: Add provider upload e2e coverage --- api/app/clients/BaseClient.js | 4 +- api/server/controllers/agents/client.test.js | 55 +++++++++++++ e2e/setup/fake-model.js | 87 +++++++++++++++++++- e2e/specs/mock/chat.spec.ts | 58 +++++++++++++ 4 files changed, 199 insertions(+), 5 deletions(-) diff --git a/api/app/clients/BaseClient.js b/api/app/clients/BaseClient.js index d36e54f015..dfa8d2201b 100644 --- a/api/app/clients/BaseClient.js +++ b/api/app/clients/BaseClient.js @@ -1217,8 +1217,8 @@ class BaseClient { const provider = this.options.agent?.provider ?? this.options.endpoint; const isBedrock = provider === EModelEndpoint.bedrock; - if (!this._mergedFileConfig && this.options.req?.config?.fileConfig) { - this._mergedFileConfig = mergeFileConfig(this.options.req.config.fileConfig); + if (!this._mergedFileConfig) { + this._mergedFileConfig = mergeFileConfig(this.options.req?.config?.fileConfig); const endpoint = this.options.agent?.endpoint ?? this.options.endpoint; this._endpointFileConfig = getEndpointFileConfig({ fileConfig: this._mergedFileConfig, diff --git a/api/server/controllers/agents/client.test.js b/api/server/controllers/agents/client.test.js index 296c4a1266..193c8d8c06 100644 --- a/api/server/controllers/agents/client.test.js +++ b/api/server/controllers/agents/client.test.js @@ -1497,6 +1497,19 @@ describe('AgentClient - titleConvo', () => { text, }); + const makeUploadedFile = (file_id, filename, type) => ({ + user: 'user-123', + file_id, + filename, + filepath: `/uploads/${filename}`, + object: 'file', + type, + bytes: 128, + embedded: false, + usage: 0, + source: 'local', + }); + beforeEach(() => { jest.clearAllMocks(); mockFormatInstructions.mockResolvedValue(''); @@ -1545,6 +1558,48 @@ describe('AgentClient - titleConvo', () => { client.useMemory = jest.fn().mockResolvedValue(undefined); }); + it.each([ + ['CSV', 'csv-file', 'sample.csv', 'text/csv'], + [ + 'XLSX', + 'xlsx-file', + 'sample.xlsx', + 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', + ], + ])( + 'routes default-supported provider uploads like %s as request documents without custom file config', + async (_label, file_id, filename, type) => { + const currentFile = makeUploadedFile(file_id, filename, type); + const message = { + messageId: 'msg-1', + parentMessageId: null, + sender: 'User', + text: `Read this ${filename}.`, + isCreatedByUser: true, + }; + + client.addDocuments = jest.fn(async (targetMessage, attachments) => { + targetMessage.documents = attachments.map((file) => ({ + type: 'input_file', + filename: file.filename, + file_data: `data:${file.type};base64,Y29sMQox`, + })); + return attachments; + }); + + const files = await client.processAttachments(message, [currentFile]); + + expect(client.addDocuments).toHaveBeenCalledWith(message, [currentFile]); + expect(message.documents).toEqual([ + expect.objectContaining({ + type: 'input_file', + filename, + }), + ]); + expect(files).toEqual([currentFile]); + }, + ); + it('places request context inline and applies each agent context doc only once', async () => { const requestFile = makeTextFile('request-file', 'request.txt', 'Shared request context'); const primaryContext = makeTextFile( diff --git a/e2e/setup/fake-model.js b/e2e/setup/fake-model.js index 032399ece1..eea805be5d 100644 --- a/e2e/setup/fake-model.js +++ b/e2e/setup/fake-model.js @@ -15,9 +15,11 @@ const CHUNK_DELAY_MS = Number(process.env.MOCK_LLM_CHUNK_DELAY_MS) || 10; const CREATE_SKILL_MARKER = 'E2E_CREATE_SKILL:'; const EDIT_SKILL_MARKER = 'E2E_EDIT_SKILL:'; const ASSERT_MODEL_SPEC_SKILLS_MARKER = 'E2E_ASSERT_MODEL_SPEC_SKILLS'; +const ASSERT_PROVIDER_FILE_MARKER = 'E2E_ASSERT_PROVIDER_FILE:'; const CREATE_FILE_AUTHORING_FINAL_TEXT = 'E2E file authoring complete'; const EDIT_FILE_AUTHORING_FINAL_TEXT = 'E2E file edit complete'; const MODEL_SPEC_SKILL_ASSERTION_FINAL_TEXT = 'E2E model spec skill assertion passed'; +const PROVIDER_FILE_ASSERTION_FINAL_TEXT = 'E2E provider file assertion passed'; const CREATE_FILE_TOOL_NAME = 'create_file'; const EDIT_FILE_TOOL_NAME = 'edit_file'; const BASH_TOOL_NAME = 'bash_tool'; @@ -64,8 +66,13 @@ function getContentText(content) { } function getLatestUserText(messages) { + const message = getLatestUserMessage(messages); + return message ? getContentText(message.content) : ''; +} + +function getLatestUserMessage(messages) { if (!Array.isArray(messages)) { - return ''; + return null; } for (let index = messages.length - 1; index >= 0; index--) { const message = messages[index]; @@ -74,10 +81,10 @@ function getLatestUserText(messages) { } const type = messageType(message); if (type === 'human' || type === 'user') { - return getContentText(message.content); + return message; } } - return ''; + return null; } function getRequestedSkillName(text, marker) { @@ -89,6 +96,14 @@ function getRequestedSkillName(text, marker) { return afterMarker.match(/[a-z0-9][a-z0-9-]*/)?.[0] ?? ''; } +function getMarkerValue(text, marker) { + const markerIndex = text.indexOf(marker); + if (markerIndex === -1) { + return ''; + } + return text.slice(markerIndex + marker.length).trim().split(/\s+/, 1)[0] ?? ''; +} + function collectToolNames(agents) { const names = new Set(); const add = (name) => { @@ -134,6 +149,67 @@ function collectSkillPrimeMessages(messages) { })); } +function collectProviderFileNames(value, names = new Set()) { + if (value == null) { + return names; + } + + if (Array.isArray(value)) { + for (const item of value) { + collectProviderFileNames(item, names); + } + return names; + } + + if (typeof value !== 'object') { + return names; + } + + if (value.type === 'input_file' && typeof value.filename === 'string') { + names.add(value.filename); + } + + if (value.type === 'file' && typeof value.file?.filename === 'string') { + names.add(value.file.filename); + } + + if (value.type === 'document' && typeof value.context === 'string') { + const match = value.context.match(/File:\s*"([^"]+)"/); + if (match?.[1]) { + names.add(match[1]); + } + } + + for (const child of Object.values(value)) { + collectProviderFileNames(child, names); + } + + return names; +} + +function providerFileAssertionResponses({ messages, text }) { + const filename = getMarkerValue(text, ASSERT_PROVIDER_FILE_MARKER); + if (!filename) { + return null; + } + + const latestUserMessage = getLatestUserMessage(messages); + const providerFileNames = collectProviderFileNames(latestUserMessage?.content); + if (providerFileNames.has(filename)) { + return { + responses: [`${PROVIDER_FILE_ASSERTION_FINAL_TEXT}: ${filename}`], + }; + } + + return { + responses: [ + `E2E provider file assertion failed: expected ${filename}; saw ${ + Array.from(providerFileNames).join(', ') || 'no provider files' + }`, + ], + }; +} + function modelSpecSkillAssertionResponses({ agents, messages, toolNames }) { const failures = []; const additionalInstructions = collectAdditionalInstructions(agents); @@ -235,6 +311,11 @@ function fileAuthoringResponses(operation, toolNames) { } function resolveResponses({ agents, messages, text, toolNames }) { + const providerFileAssertion = providerFileAssertionResponses({ messages, text }); + if (providerFileAssertion) { + return providerFileAssertion; + } + if (text.includes(ASSERT_MODEL_SPEC_SKILLS_MARKER)) { return modelSpecSkillAssertionResponses({ agents, messages, toolNames }); } diff --git a/e2e/specs/mock/chat.spec.ts b/e2e/specs/mock/chat.spec.ts index 60a45d5310..9d08c845a8 100644 --- a/e2e/specs/mock/chat.spec.ts +++ b/e2e/specs/mock/chat.spec.ts @@ -1,5 +1,6 @@ import { expect, test } from '@playwright/test'; import { + isAgentsStream, MOCK_ENDPOINTS, NEW_CHAT_PATH, mockReply, @@ -34,4 +35,61 @@ test.describe('core chat loop', () => { await expect(mockReply(page)).toBeVisible(); await expect(page.getByTestId('convo-item').first()).toBeVisible(); }); + + test('keeps upload-to-provider CSV attached to the sent message and model input', async ({ + page, + }) => { + test.setTimeout(90000); + + const filename = 'provider-upload.csv'; + const assertionText = `E2E_ASSERT_PROVIDER_FILE:${filename}`; + const fileChip = page.getByTestId('messages-view').getByRole('button', { name: filename }); + + await page.goto(NEW_CHAT_PATH, { timeout: 10000 }); + await selectMockEndpoint(page, MOCK_ENDPOINTS[0]); + + await page.getByRole('button', { name: 'Attach File Options' }).click(); + const fileChooserPromise = page.waitForEvent('filechooser'); + await page.getByText('Upload to Provider').click(); + const fileChooser = await fileChooserPromise; + + const uploadResponsePromise = page.waitForResponse( + (response) => + response.url().includes('/api/files') && + response.request().method() === 'POST' && + response.status() === 200, + { timeout: 30000 }, + ); + await fileChooser.setFiles({ + name: filename, + mimeType: 'text/csv', + buffer: Buffer.from('name,value\nalpha,1\n'), + }); + await uploadResponsePromise; + + await expect(page.getByRole('button', { name: filename })).toBeVisible(); + + const input = page.getByRole('textbox', { name: 'Message input' }); + await input.click(); + await input.fill(assertionText); + await expect(page.getByTestId('send-button')).toBeEnabled(); + + const [response] = await Promise.all([ + page.waitForResponse(isAgentsStream, { timeout: 30000 }), + page.getByTestId('send-button').click(), + ]); + expect(response.ok()).toBeTruthy(); + + await expect( + page + .getByTestId('messages-view') + .getByText(`E2E provider file assertion passed: ${filename}`), + ).toBeVisible(); + await expect(fileChip).toBeVisible(); + + const conversationUrl = page.url(); + await page.reload({ timeout: 10000 }); + await expect(page).toHaveURL(conversationUrl); + await expect(fileChip).toBeVisible(); + }); });