diff --git a/api/server/controllers/agents/callbacks.js b/api/server/controllers/agents/callbacks.js index dc8f6cc65a..e7101edd20 100644 --- a/api/server/controllers/agents/callbacks.js +++ b/api/server/controllers/agents/callbacks.js @@ -397,6 +397,65 @@ function writeAttachment(res, streamId, attachment) { } } +/** + * Emit a phase-2 update for an attachment that was previously sent with + * `status: 'pending'`. Fire-and-forget: if the response stream has + * already closed (the agent finished generating before phase-2 resolved) + * the frontend's React Query polling on `/api/files/:file_id/preview` + * picks up the resolved record on its next tick. Skipping the write in + * that case avoids `ERR_STREAM_WRITE_AFTER_END`. + * + * Reuses the `attachment` SSE event name with a discriminated payload: + * the frontend's `useAttachmentHandler` upserts by `file_id`, so a + * second event with the same id and `status: 'ready' | 'failed'` + * overwrites the pending placeholder in place. No new event type, no + * new client listener. + * + * @param {ServerResponse} res + * @param {string | null} streamId + * @param {Object} attachment - Updated attachment payload (must carry `file_id`). + */ +function writeAttachmentUpdate(res, streamId, attachment) { + if (!streamId && (!res || res.writableEnded || !res.headersSent)) { + return; + } + writeAttachment(res, streamId, attachment); +} + +/** + * Build the wire-format attachment payload from a phase-1 or phase-2 + * file metadata record. Centralizes the field projection so phase-1 + * (pending), phase-2 (ready/failed), and pre-existing legacy callers + * all emit the same shape. + */ +function attachmentFromFileMetadata(fileMetadata, toolCallId) { + return { + file_id: fileMetadata.file_id, + filename: fileMetadata.filename, + filepath: fileMetadata.filepath, + type: fileMetadata.type, + width: fileMetadata.width, + height: fileMetadata.height, + conversationId: fileMetadata.conversationId, + messageId: fileMetadata.messageId, + toolCallId: toolCallId ?? fileMetadata.toolCallId, + /* Inline text / sanitized HTML preview from the extractor — drives + * the file artifact panel's rich preview for DOCX/XLSX/CSV/PPTX. + * Pass null explicitly (rather than undefined) so the wire format + * is stable for the empty-text gate on the client. */ + text: fileMetadata.text ?? null, + /* Trust signal so the client routes office types to the HTML + * bucket only when the backend produced sanitized full-document + * HTML (Codex P1 review on PR #12934). */ + textFormat: fileMetadata.textFormat ?? null, + /* Two-phase preview lifecycle: phase-1 emits 'pending', phase-2 + * patches to 'ready' or 'failed'. Absent for files that don't + * expect a preview (images, plain text). */ + status: fileMetadata.status, + previewError: fileMetadata.previewError, + }; +} + /** * * @param {Object} params @@ -556,14 +615,15 @@ function createToolEndCallback({ req, res, artifactPromises, streamId = null }) continue; } const { id, name } = file; + const toolCallId = output.tool_call_id; artifactPromises.push( (async () => { - const fileMetadata = await processCodeOutput({ + const result = await processCodeOutput({ req, id, name, messageId: metadata.run_id, - toolCallId: output.tool_call_id, + toolCallId, conversationId: metadata.thread_id, /** * Use the FILE's session_id (storage session), not the @@ -583,15 +643,39 @@ function createToolEndCallback({ req, res, artifactPromises, streamId = null }) */ session_id: file.session_id ?? output.artifact.session_id, }); - if (!streamId && !res.headersSent) { - return fileMetadata; - } - + const fileMetadata = result?.file ?? null; + const finalize = result?.finalize; if (!fileMetadata) { return null; } - - writeAttachment(res, streamId, fileMetadata); + /* Phase-1 emit: ship the attachment to the client immediately + * (carries `status: 'pending'` for office buckets so the UI + * shows "preparing preview…"). The agent's response stops + * blocking on extraction here. */ + if (streamId || res.headersSent) { + writeAttachment(res, streamId, fileMetadata); + } + /* Phase-2 fire-and-forget: extraction continues running even + * after the HTTP response closes. If the stream is still open + * when phase-2 lands, push an `attachment` update event so + * the UI patches in place; otherwise React Query polling on + * `/api/files/:file_id/preview` picks it up. */ + if (typeof finalize === 'function') { + finalize() + .then((updated) => { + if (!updated) { + return; + } + writeAttachmentUpdate( + res, + streamId, + attachmentFromFileMetadata(updated, toolCallId), + ); + }) + .catch((error) => { + logger.error('Error in phase-2 preview extraction:', error); + }); + } return fileMetadata; })().catch((error) => { logger.error('Error processing code output:', error); @@ -782,14 +866,15 @@ function createResponsesToolEndCallback({ req, res, tracker, artifactPromises }) continue; } const { id, name } = file; + const toolCallId = output.tool_call_id; artifactPromises.push( (async () => { - const fileMetadata = await processCodeOutput({ + const result = await processCodeOutput({ req, id, name, messageId: metadata.run_id, - toolCallId: output.tool_call_id, + toolCallId, conversationId: metadata.thread_id, /** * Use the FILE's session_id (storage session), not the @@ -809,36 +894,47 @@ function createResponsesToolEndCallback({ req, res, tracker, artifactPromises }) */ session_id: file.session_id ?? output.artifact.session_id, }); - + const fileMetadata = result?.file ?? null; + const finalize = result?.finalize; if (!fileMetadata) { return null; } - // For Responses API, emit attachment during streaming + /* Phase-1 emit (Open Responses extension format). The agent's + * response no longer blocks on extraction. */ if (res.headersSent && !res.writableEnded) { - const attachment = { - file_id: fileMetadata.file_id, - filename: fileMetadata.filename, - type: fileMetadata.type, - url: fileMetadata.filepath, - width: fileMetadata.width, - height: fileMetadata.height, - tool_call_id: output.tool_call_id, - /* Inline text / sanitized HTML preview from - * `extractCodeArtifactText` — drives the file artifact panel's - * rich preview for DOCX/XLSX/CSV/PPTX. Pass null explicitly - * (rather than undefined) so the wire format is stable for the - * empty-text gate on the client. */ - text: fileMetadata.text ?? null, - /* Trust signal so the client can route .docx/.csv/.xlsx/ - * .pptx attachments to the office HTML buckets only when - * the backend produced sanitized full-document HTML. - * RAG-uploaded plain text from mammoth.extractRawText - * arrives without this flag and must NOT be injected as - * HTML — Codex P1 review on PR #12934. */ - textFormat: fileMetadata.textFormat ?? null, - }; - writeResponsesAttachment(res, tracker, attachment, metadata); + writeResponsesAttachment( + res, + tracker, + buildResponsesAttachment(fileMetadata, toolCallId), + metadata, + ); + } + + /* Phase-2: extract HTML preview in the background and emit + * a follow-up `librechat:attachment` with the same `file_id` + * so the client merges the resolved record over the pending + * placeholder. Fire-and-forget — survives response close; + * polling covers the post-close gap. */ + if (typeof finalize === 'function') { + finalize() + .then((updated) => { + if (!updated) { + return; + } + if (!res || res.writableEnded || !res.headersSent) { + return; + } + writeResponsesAttachment( + res, + tracker, + buildResponsesAttachment(updated, toolCallId), + metadata, + ); + }) + .catch((error) => { + logger.error('Error in phase-2 preview extraction:', error); + }); } return fileMetadata; @@ -851,6 +947,28 @@ function createResponsesToolEndCallback({ req, res, tracker, artifactPromises }) }; } +/** + * Project a file metadata record into the Open Responses attachment + * shape. Mirrors the legacy inline projection but adds `status` and + * `previewError` so phase-2 updates carry the lifecycle signal the + * client uses to upsert by `file_id`. + */ +function buildResponsesAttachment(fileMetadata, toolCallId) { + return { + file_id: fileMetadata.file_id, + filename: fileMetadata.filename, + type: fileMetadata.type, + url: fileMetadata.filepath, + width: fileMetadata.width, + height: fileMetadata.height, + tool_call_id: toolCallId, + text: fileMetadata.text ?? null, + textFormat: fileMetadata.textFormat ?? null, + status: fileMetadata.status, + previewError: fileMetadata.previewError, + }; +} + const ALLOWED_LOG_LEVELS = new Set(['debug', 'info', 'warn', 'error']); function agentLogHandler(_event, data) { diff --git a/api/server/controllers/tools.js b/api/server/controllers/tools.js index 8124894584..1557991393 100644 --- a/api/server/controllers/tools.js +++ b/api/server/controllers/tools.js @@ -192,7 +192,7 @@ const callTool = async (req, res) => { const { id, name } = file; artifactPromises.push( (async () => { - const fileMetadata = await processCodeOutput({ + const result = await processCodeOutput({ req, id, name, @@ -201,11 +201,21 @@ const callTool = async (req, res) => { conversationId, session_id: artifact.session_id, }); - + const fileMetadata = result?.file ?? null; + const finalize = result?.finalize; if (!fileMetadata) { return null; } - + /* This endpoint is non-streaming and its contract is "give + * me the artifacts" — return the phase-1 record immediately + * (with `status: 'pending'` for office buckets) and run phase-2 + * extraction in the background. The client polls + * `/api/files/:file_id/preview` for the resolved record. */ + if (typeof finalize === 'function') { + finalize().catch((error) => { + logger.error('Error in phase-2 preview extraction:', error); + }); + } return fileMetadata; })().catch((error) => { logger.error('Error processing code output:', error); diff --git a/api/server/services/Files/Code/__tests__/process-traversal.spec.js b/api/server/services/Files/Code/__tests__/process-traversal.spec.js index 099794821d..04b11ac8b1 100644 --- a/api/server/services/Files/Code/__tests__/process-traversal.spec.js +++ b/api/server/services/Files/Code/__tests__/process-traversal.spec.js @@ -32,6 +32,13 @@ jest.mock('@librechat/api', () => { * mock returns null in lockstep with the null `text` above so * downstream consumers don't see a phantom format. */ getExtractedTextFormat: jest.fn(() => null), + /* Pass-through `withTimeout`: this suite asserts traversal sanitization, + * not phase-2 timing. */ + withTimeout: async (promise) => promise, + /* These traversal cases all use non-office filenames — keep the + * inline (non-finalize) path so existing assertions on a single + * createFile call hold. */ + hasOfficeHtmlPath: jest.fn(() => false), codeServerHttpAgent: new http.Agent({ keepAlive: false }), codeServerHttpsAgent: new https.Agent({ keepAlive: false }), }; diff --git a/api/server/services/Files/Code/process.js b/api/server/services/Files/Code/process.js index 4348d2766f..105c7820b9 100644 --- a/api/server/services/Files/Code/process.js +++ b/api/server/services/Files/Code/process.js @@ -3,8 +3,10 @@ const { v4 } = require('uuid'); const { logger } = require('@librechat/data-schemas'); const { getCodeBaseURL } = require('@librechat/agents'); const { + withTimeout, getBasePath, logAxiosError, + hasOfficeHtmlPath, sanitizeArtifactPath, flattenArtifactPath, createAxiosInstance, @@ -69,8 +71,108 @@ const createDownloadFallback = ({ }; /** - * Process code execution output files - downloads and saves both images and non-image files. - * All files are saved to local storage with fileIdentifier metadata for code env re-upload. + * Hard ceiling on phase-2 (background HTML extraction + DB update). The + * inner office-render path already has its own 12s timeout and a + * concurrency-limited queue; this is the outer guard that catches + * pathological cases where queue wait + render + DB write would + * otherwise hang the file in `status: 'pending'` indefinitely. + * + * If the timeout fires the record is updated to `status: 'failed'` + * with `previewError: 'timeout'` and the UI shows download-only. + */ +const PHASE_TWO_TIMEOUT_MS = 60_000; + +/** + * Run the background phase-2 work for a code-execution file: extract + * the HTML preview (or plain text for non-office buckets that still + * benefit from caching), then atomically transition the DB record to + * `status: 'ready'` (with `text`/`textFormat`) or `status: 'failed'` + * (with `previewError`). + * + * Decoupled from `processCodeOutput` so the agent's final response is + * not blocked on potentially slow office rendering. The caller fires + * this without awaiting; promises continue running after the HTTP + * response closes (Node doesn't kill them) and the frontend learns of + * completion via `attachment.update` SSE (if the stream is still open) + * or via React Query polling otherwise. Process restart is the only + * thing that can lose progress — covered by the boot-time orphan sweep. + * + * @param {object} params + * @param {Buffer} params.buffer - The file contents (held in closure; + * bounded by `MAX_TEXT_EXTRACT_BYTES = 1MB` from the extractor). + * @param {string} params.leafName - Basename for classification. + * @param {string} params.mimeType - Detected/inferred MIME. + * @param {string} params.category - Classifier output. + * @param {string} params.file_id - The DB record key for the update. + * @returns {Promise} The post-update record on + * success; `null` if the DB update itself failed (extraction failure + * is reflected as `status: 'failed'`, not a thrown error). + */ +const finalizePreview = async ({ buffer, leafName, mimeType, category, file_id }) => { + let text = null; + let previewError; + try { + text = await withTimeout( + extractCodeArtifactText(buffer, leafName, mimeType, category), + PHASE_TWO_TIMEOUT_MS, + `Phase-2 extraction exceeded ${PHASE_TWO_TIMEOUT_MS}ms`, + ); + } catch (_error) { + /* `extractCodeArtifactText` swallows its own errors and returns null, + * so the only way to reach here is a `withTimeout` rejection — i.e. + * the queue + render combined exceeded the outer 60s ceiling. */ + previewError = 'timeout'; + logger.warn( + `[finalizePreview] ${file_id}: extraction timed out after ${PHASE_TWO_TIMEOUT_MS}ms`, + ); + } + /* Office-bucket files are HTML-or-null per the PR #12934 SEC fix: + * a null result must NOT fall back to plain text, and must surface + * to the UI as a failed preview so the artifact card downgrades to + * download-only. Non-office files with a null result have nothing + * extractable (binary, oversized) — mark ready with text=null so + * subsequent reads stop polling. */ + const isOfficeBucket = hasOfficeHtmlPath(leafName, mimeType); + const textFormat = getExtractedTextFormat(leafName, mimeType, text); + const failed = isOfficeBucket && text == null; + const status = failed ? 'failed' : 'ready'; + if (failed && !previewError) { + previewError = 'parser-error'; + } + try { + return await updateFile({ + file_id, + text, + textFormat, + status, + previewError: failed ? previewError : null, + }); + } catch (error) { + logger.error( + `[finalizePreview] ${file_id}: failed to persist phase-2 result: ${error?.message ?? error}`, + ); + return null; + } +}; + +/** + * Process code execution output files — downloads and saves both images + * and non-image files. All files are saved to local storage with + * `fileIdentifier` metadata for code env re-upload. + * + * Returns a two-part shape so callers can ship the attachment to the + * client immediately and run preview extraction in the background: + * - `file`: phase-1 metadata (file is persisted, downloadable, and + * has `status: 'pending'` if a preview is still being rendered). + * - `finalize` (optional): a thunk returning the phase-2 result + * promise. Present only when an inline HTML preview is expected + * (office buckets — DOCX/XLSX/XLS/ODS/CSV/PPTX). Caller decides + * whether to await or fire-and-forget. + * + * Existing fallback paths (size limit, missing storage strategy, error + * catch) return `{ file }` with no `finalize` — there's nothing to + * extract. + * * @param {ServerRequest} params.req - The Express request object. * @param {string} params.id - The file ID from the code environment. * @param {string} params.name - The filename. @@ -78,7 +180,7 @@ const createDownloadFallback = ({ * @param {string} params.session_id - The code execution session ID. * @param {string} params.conversationId - The current conversation ID. * @param {string} params.messageId - The current message ID. - * @returns {Promise} The file metadata or undefined if an error occurs. + * @returns {Promise<{ file: MongoFile & { messageId: string, toolCallId: string }, finalize?: () => Promise }>} */ const processCodeOutput = async ({ req, @@ -123,15 +225,17 @@ const processCodeOutput = async ({ logger.warn( `[processCodeOutput] File "${name}" (${(buffer.length / megabyte).toFixed(2)} MB) exceeds size limit of ${(fileSizeLimit / megabyte).toFixed(2)} MB, falling back to download URL`, ); - return createDownloadFallback({ - id, - name, - messageId, - toolCallId, - session_id, - conversationId, - expiresAt: currentDate.getTime() + 86400000, - }); + return { + file: createDownloadFallback({ + id, + name, + messageId, + toolCallId, + session_id, + conversationId, + expiresAt: currentDate.getTime() + 86400000, + }), + }; } const fileIdentifier = `${session_id}/${id}`; @@ -206,7 +310,7 @@ const processCodeOutput = async ({ metadata: { fileIdentifier }, }; await createFile(file, true); - return Object.assign(file, { messageId, toolCallId }); + return { file: Object.assign(file, { messageId, toolCallId }) }; } const { saveBuffer } = getStrategyFunctions(appConfig.fileStrategy); @@ -214,15 +318,17 @@ const processCodeOutput = async ({ logger.warn( `[processCodeOutput] saveBuffer not available for strategy ${appConfig.fileStrategy}, falling back to download URL`, ); - return createDownloadFallback({ - id, - name, - messageId, - toolCallId, - session_id, - conversationId, - expiresAt: currentDate.getTime() + 86400000, - }); + return { + file: createDownloadFallback({ + id, + name, + messageId, + toolCallId, + session_id, + conversationId, + expiresAt: currentDate.getTime() + 86400000, + }), + }; } const detectedType = await determineFileType(buffer, true); @@ -271,18 +377,17 @@ const processCodeOutput = async ({ * what it would have gotten with the old flat-name flow. */ const leafName = path.basename(safeName); const category = classifyCodeArtifact(leafName, mimeType); - const text = await extractCodeArtifactText(buffer, leafName, mimeType, category); - /* `textFormat` accompanies `text` so the client can gate - * office-HTML-bucket routing on a trusted signal — clients MUST - * NOT inject `text` into the iframe as HTML unless `textFormat === - * 'html'`. RAG-uploaded `.docx` etc. arrive with plain text from - * mammoth.extractRawText and would otherwise be hijacked by the - * extension-based office routing into the HTML-injection path - * (Codex P1 review on PR #12934). null on extract failure — the - * client treats absence as 'text' for safety. */ - const textFormat = getExtractedTextFormat(leafName, mimeType, text); - const file = { + /* Two-phase split: office-bucket files (DOCX/XLSX/XLS/ODS/CSV/PPTX) + * route through `bufferToOfficeHtml` which is CPU-heavy. Persist + * the record now with `status: 'pending'` and `text: null` so the + * agent's response isn't blocked, then return a `finalize` thunk + * the caller can run in the background. Non-office files have + * cheap or no extraction — run it inline so the caller gets a + * fully-resolved record without juggling a finalize step. */ + const expectsPreview = hasOfficeHtmlPath(leafName, mimeType); + + const baseFile = { file_id, filepath, messageId: persistedMessageId, @@ -298,16 +403,51 @@ const processCodeOutput = async ({ context: FileContext.execute_code, usage: isUpdate ? (claimed.usage ?? 0) + 1 : 1, createdAt: isUpdate ? claimed.createdAt : formattedDate, - // Always set `text` explicitly (string or null) so that an update which - // produces a binary or oversized artifact clears any previously cached - // text — `createFile` uses findOneAndUpdate with $set semantics, which - // would otherwise leave a stale value behind. + }; + + if (expectsPreview) { + /* Phase-1: persist with `status: 'pending'` and explicit + * `text: null` / `textFormat: null` so an update that previously + * had cached text gets cleared. Phase-2 transitions to 'ready' + * (with text/textFormat) or 'failed' (with previewError). */ + const file = { + ...baseFile, + text: null, + textFormat: null, + status: 'pending', + previewError: null, + }; + await createFile(file, true); + return { + file: Object.assign(file, { messageId, toolCallId }), + finalize: () => finalizePreview({ buffer, leafName, mimeType, category, file_id }), + }; + } + + /* Non-office path: extraction is cheap (utf8 decode, parseDocument + * for PDF/ODT, or null for binaries). Run inline and return a + * fully-resolved record — no `finalize` needed. */ + const text = await extractCodeArtifactText(buffer, leafName, mimeType, category); + /* `textFormat` accompanies `text` so the client can gate + * office-HTML-bucket routing on a trusted signal — clients MUST + * NOT inject `text` into the iframe as HTML unless `textFormat === + * 'html'`. RAG-uploaded `.docx` etc. arrive with plain text from + * mammoth.extractRawText and would otherwise be hijacked by the + * extension-based office routing into the HTML-injection path + * (Codex P1 review on PR #12934). null on extract failure — the + * client treats absence as 'text' for safety. */ + const textFormat = getExtractedTextFormat(leafName, mimeType, text); + const file = { + ...baseFile, + // Always set explicitly so an update which produces a binary or + // oversized artifact clears any previously cached text — createFile + // uses findOneAndUpdate with $set semantics. text: text ?? null, textFormat: textFormat ?? null, }; await createFile(file, true); - return Object.assign(file, { messageId, toolCallId }); + return { file: Object.assign(file, { messageId, toolCallId }) }; } catch (error) { if (error?.message === 'Path traversal detected in filename') { logger.warn( @@ -320,15 +460,17 @@ const processCodeOutput = async ({ }); // Fallback for download errors - return download URL so user can still manually download - return createDownloadFallback({ - id, - name, - messageId, - toolCallId, - session_id, - conversationId, - expiresAt: currentDate.getTime() + 86400000, - }); + return { + file: createDownloadFallback({ + id, + name, + messageId, + toolCallId, + session_id, + conversationId, + expiresAt: currentDate.getTime() + 86400000, + }), + }; } }; @@ -465,7 +607,23 @@ const primeFiles = async (options) => { } const entity_id = overrideEntityId ?? queryParams.entity_id; - toolContext += `\n\t- /mnt/data/${file.filename}${fileSuffix}`; + + /* Surface the two-phase preview lifecycle so the LLM knows when + * a prior-turn artifact's rich preview didn't materialize. The + * file blob is always available (phase-1 persists it before + * returning), so the model can still tell the user "you can + * download it" even when the preview never resolved. Absent + * status means legacy or non-office — render normally. */ + let previewSuffix = ''; + if (file.status === 'pending') { + previewSuffix = ' (preview not yet generated)'; + } else if (file.status === 'failed') { + previewSuffix = file.previewError + ? ` (preview unavailable: ${file.previewError})` + : ' (preview unavailable)'; + } + + toolContext += `\n\t- /mnt/data/${file.filename}${fileSuffix}${previewSuffix}`; files.push({ id: overrideId ?? id, session_id: overrideSessionId ?? session_id, diff --git a/api/server/services/Files/Code/process.spec.js b/api/server/services/Files/Code/process.spec.js index 8b12278c9a..8d9a9eb1a7 100644 --- a/api/server/services/Files/Code/process.spec.js +++ b/api/server/services/Files/Code/process.spec.js @@ -44,6 +44,15 @@ mockAxios.isAxiosError = jest.fn(() => false); const mockClassifyCodeArtifact = jest.fn(() => 'other'); const mockExtractCodeArtifactText = jest.fn(async () => null); const mockGetExtractedTextFormat = jest.fn((_name, _mime, text) => (text == null ? null : 'text')); +/* `hasOfficeHtmlPath` gates the two-phase split: when true, processCodeOutput + * returns `{ file, finalize }` with the file persisted at `status: 'pending'` + * and `finalize` runs the background extraction. Default false here so the + * legacy single-phase tests below (txt/png/etc) exercise the inline path + * unchanged. The dedicated office/finalize describe block toggles it on. */ +const mockHasOfficeHtmlPath = jest.fn(() => false); +/* Pass-through `withTimeout`: tests don't drive timeouts here (those live + * in promise.spec.ts and the finalizePreview unit tests below). */ +const passthroughWithTimeout = async (promise) => promise; jest.mock('@librechat/api', () => { const http = require('http'); const https = require('https'); @@ -53,6 +62,8 @@ jest.mock('@librechat/api', () => { sanitizeArtifactPath: jest.fn((name) => name), flattenArtifactPath: jest.fn((name) => name.replace(/\//g, '__')), createAxiosInstance: jest.fn(() => mockAxios), + withTimeout: (...args) => passthroughWithTimeout(...args), + hasOfficeHtmlPath: (...args) => mockHasOfficeHtmlPath(...args), /** * Arrow-function indirection (vs. a direct `jest.fn()` reference) so * tests can per-case `mockReturnValueOnce` / `mockImplementationOnce` @@ -178,7 +189,7 @@ describe('Code Process', () => { const smallBuffer = Buffer.alloc(100); mockAxios.mockResolvedValue({ data: smallBuffer }); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(mockClaimCodeFile).toHaveBeenCalledWith({ filename: 'test-file.txt', @@ -201,7 +212,7 @@ describe('Code Process', () => { const smallBuffer = Buffer.alloc(100); mockAxios.mockResolvedValue({ data: smallBuffer }); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(result.file_id).toBe('mock-uuid-1234'); expect(result.usage).toBe(1); @@ -221,7 +232,7 @@ describe('Code Process', () => { }; convertImage.mockResolvedValue(convertedFile); - const result = await processCodeOutput(imageParams); + const { file: result } = await processCodeOutput(imageParams); expect(convertImage).toHaveBeenCalledWith( mockReq, @@ -246,7 +257,7 @@ describe('Code Process', () => { mockAxios.mockResolvedValue({ data: imageBuffer }); convertImage.mockResolvedValue({ filepath: '/images/user-123/existing-img-id.webp' }); - const result = await processCodeOutput(imageParams); + const { file: result } = await processCodeOutput(imageParams); expect(convertImage).toHaveBeenCalledWith( mockReq, @@ -272,7 +283,7 @@ describe('Code Process', () => { getStrategyFunctions.mockReturnValue({ saveBuffer: mockSaveBuffer }); determineFileType.mockResolvedValue({ mime: 'text/plain' }); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(mockSaveBuffer).toHaveBeenCalledWith({ userId: 'user-123', @@ -299,7 +310,7 @@ describe('Code Process', () => { const mockSaveBuffer = jest.fn().mockResolvedValue('/uploads/saved.txt'); getStrategyFunctions.mockReturnValue({ saveBuffer: mockSaveBuffer }); - const result = await processCodeOutput({ + const { file: result } = await processCodeOutput({ ...baseParams, name: 'test_folder/test_file.txt', }); @@ -378,7 +389,7 @@ describe('Code Process', () => { mockAxios.mockResolvedValue({ data: smallBuffer }); determineFileType.mockResolvedValue({ mime: 'application/pdf' }); - const result = await processCodeOutput({ ...baseParams, name: 'document.pdf' }); + const { file: result } = await processCodeOutput({ ...baseParams, name: 'document.pdf' }); expect(determineFileType).toHaveBeenCalledWith(smallBuffer, true); expect(result.type).toBe('application/pdf'); @@ -389,7 +400,7 @@ describe('Code Process', () => { mockAxios.mockResolvedValue({ data: smallBuffer }); determineFileType.mockResolvedValue(null); - const result = await processCodeOutput({ ...baseParams, name: 'unknown.xyz' }); + const { file: result } = await processCodeOutput({ ...baseParams, name: 'unknown.xyz' }); expect(result.type).toBe('application/octet-stream'); }); @@ -403,7 +414,7 @@ describe('Code Process', () => { mockClassifyCodeArtifact.mockReturnValueOnce('utf8-text'); mockExtractCodeArtifactText.mockResolvedValueOnce('hello world\n'); - const result = await processCodeOutput({ ...baseParams, name: 'note.txt' }); + const { file: result } = await processCodeOutput({ ...baseParams, name: 'note.txt' }); expect(mockClassifyCodeArtifact).toHaveBeenCalledWith('note.txt', 'text/plain'); expect(mockExtractCodeArtifactText).toHaveBeenCalledWith( @@ -426,7 +437,7 @@ describe('Code Process', () => { mockClassifyCodeArtifact.mockReturnValueOnce('other'); mockExtractCodeArtifactText.mockResolvedValueOnce(null); - const result = await processCodeOutput({ ...baseParams, name: 'archive.zip' }); + const { file: result } = await processCodeOutput({ ...baseParams, name: 'archive.zip' }); expect(result.text).toBeNull(); const createCall = createFile.mock.calls[0][0]; @@ -475,7 +486,7 @@ describe('Code Process', () => { const largeBuffer = Buffer.alloc(5000); // 5KB - exceeds 1KB limit mockAxios.mockResolvedValue({ data: largeBuffer }); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('exceeds size limit')); expect(result.filepath).toContain('/api/files/code/download/session-123/file-id-123'); @@ -494,7 +505,7 @@ describe('Code Process', () => { mockAxios.mockResolvedValue({ data: smallBuffer }); getStrategyFunctions.mockReturnValue({ saveBuffer: null }); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(logger.warn).toHaveBeenCalledWith( expect.stringContaining('saveBuffer not available'), @@ -506,7 +517,7 @@ describe('Code Process', () => { it('should fallback to download URL on axios error', async () => { mockAxios.mockRejectedValue(new Error('Network error')); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(result.filepath).toContain('/api/files/code/download/session-123/file-id-123'); expect(result.conversationId).toBe('conv-123'); @@ -520,7 +531,7 @@ describe('Code Process', () => { const smallBuffer = Buffer.alloc(100); mockAxios.mockResolvedValue({ data: smallBuffer }); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(result.usage).toBe(1); }); @@ -534,7 +545,7 @@ describe('Code Process', () => { const smallBuffer = Buffer.alloc(100); mockAxios.mockResolvedValue({ data: smallBuffer }); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(result.usage).toBe(6); }); @@ -547,7 +558,7 @@ describe('Code Process', () => { const smallBuffer = Buffer.alloc(100); mockAxios.mockResolvedValue({ data: smallBuffer }); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(result.usage).toBe(1); }); @@ -558,7 +569,7 @@ describe('Code Process', () => { const smallBuffer = Buffer.alloc(100); mockAxios.mockResolvedValue({ data: smallBuffer }); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(result.metadata).toEqual({ fileIdentifier: 'session-123/file-id-123', @@ -569,7 +580,7 @@ describe('Code Process', () => { const smallBuffer = Buffer.alloc(100); mockAxios.mockResolvedValue({ data: smallBuffer }); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(result.context).toBe(FileContext.execute_code); }); @@ -578,7 +589,7 @@ describe('Code Process', () => { const smallBuffer = Buffer.alloc(100); mockAxios.mockResolvedValue({ data: smallBuffer }); - const result = await processCodeOutput(baseParams); + const { file: result } = await processCodeOutput(baseParams); expect(result.toolCallId).toBe('tool-call-123'); expect(result.messageId).toBe('msg-123'); @@ -718,7 +729,7 @@ describe('Code Process', () => { const smallBuffer = Buffer.alloc(100); mockAxios.mockResolvedValue({ data: smallBuffer }); - const result = await processCodeOutput({ + const { file: result } = await processCodeOutput({ ...baseParams, name: 'sentinel.txt', messageId: 'turn-2-current-run-msg', @@ -789,6 +800,177 @@ describe('Code Process', () => { expect(callConfig.httpsAgent.keepAlive).toBe(false); }); }); + + describe('two-phase preview flow (office-bucket files)', () => { + /* Office-bucket files (DOCX/XLSX/etc.) split into: + * phase-1 (await): persist `text: null, status: 'pending'`, + * return `{ file, finalize }` so the caller can ship the + * attachment to the client immediately; + * phase-2 (background): finalize() invokes the extractor and + * transitions the record to 'ready' (with text/textFormat) or + * 'failed' (with previewError). The agent's final response + * never blocks on phase-2. + * + * The `hasOfficeHtmlPath` mock is the gate. Other tests keep it + * at `false` (legacy single-phase path); we flip it on here. */ + const { updateFile } = require('~/models'); + + beforeEach(() => { + mockHasOfficeHtmlPath.mockReturnValue(true); + updateFile.mockResolvedValue({ file_id: 'mock-uuid-1234', status: 'ready' }); + }); + + afterEach(() => { + mockHasOfficeHtmlPath.mockReturnValue(false); + }); + + it('persists phase-1 with status:pending and text:null, deferring extraction', async () => { + mockAxios.mockResolvedValue({ data: Buffer.alloc(100) }); + determineFileType.mockResolvedValue({ + mime: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', + }); + + const result = await processCodeOutput({ ...baseParams, name: 'data.xlsx' }); + + expect(result.file).toMatchObject({ + file_id: 'mock-uuid-1234', + filename: 'data.xlsx', + status: 'pending', + text: null, + textFormat: null, + }); + expect(typeof result.finalize).toBe('function'); + // Extractor MUST NOT have been called yet — that's phase-2 work. + expect(mockExtractCodeArtifactText).not.toHaveBeenCalled(); + // Phase-1 record persisted with the pending status. + expect(createFile).toHaveBeenCalledWith( + expect.objectContaining({ status: 'pending', text: null, textFormat: null }), + true, + ); + }); + + it('finalize() runs the extractor, transitions to ready with text+textFormat on success', async () => { + mockAxios.mockResolvedValue({ data: Buffer.alloc(100) }); + determineFileType.mockResolvedValue({ + mime: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', + }); + mockExtractCodeArtifactText.mockResolvedValueOnce('
1
'); + mockGetExtractedTextFormat.mockReturnValueOnce('html'); + + const { finalize } = await processCodeOutput({ ...baseParams, name: 'data.xlsx' }); + await finalize(); + + expect(mockExtractCodeArtifactText).toHaveBeenCalledTimes(1); + expect(updateFile).toHaveBeenCalledWith({ + file_id: 'mock-uuid-1234', + text: '
1
', + textFormat: 'html', + status: 'ready', + previewError: null, + }); + }); + + it('finalize() transitions to failed with previewError when extractor returns null (HTML-or-null contract)', async () => { + mockAxios.mockResolvedValue({ data: Buffer.alloc(100) }); + determineFileType.mockResolvedValue({ + mime: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', + }); + mockExtractCodeArtifactText.mockResolvedValueOnce(null); + // Office bucket + null text → must be 'failed', NEVER raw text fallback + // (PR #12934 SEC fix: prevents