mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-22 01:53:33 +00:00
⚡ feat: two-phase code-execution preview flow (unblocks final response)
The agent's final response no longer waits on CPU-heavy office HTML
extraction. Phase-1 (download + storage save + DB record at
`status: 'pending'`) is awaited as before; phase-2 (extract +
`updateFile`) runs in the background with a hard 60s ceiling.
Three flows, all funneling through `processCodeOutput` and updated to
the new `{ file, finalize? }` return shape:
- `callbacks.js` (chat-completions + Open Responses streaming): emit
the phase-1 attachment immediately (carries `status: 'pending'` for
office buckets so the UI shows "preparing preview…"), then
fire-and-forget `finalize()`. If the SSE stream is still open when
phase-2 lands, push an `attachment` update event with the same
`file_id` so the client merges over the placeholder in place.
- `tools.js` direct endpoint: same split — return the phase-1
metadata immediately, run extraction in the background. Client
polls for the resolved record.
`finalize()` wraps the existing 12s per-render timeout in a 60s outer
`withTimeout`. The HTML-or-null contract from #12934 is preserved:
office types that fail extraction transition to `status: 'failed'`
with `previewError: 'parser-error' | 'timeout'` rather than falling
back to plain text (would be an XSS vector).
Promises continue running after the HTTP response closes (Node
doesn't kill them). The boot-time orphan sweep covers the only case
that loses progress — actual process restart mid-extraction.
`primeFiles` annotates the agent's `toolContext` line for prior-turn
files: `(preview not yet generated)` for pending, `(preview
unavailable: <reason>)` for failed. The model can volunteer "you can
still download it" instead of pretending the preview is fine.
`hasOfficeHtmlPath` exported from `@librechat/api` so `processCodeOutput`
can decide whether a file expects a preview at all.
This commit is contained in:
Danny Avila
parent
54b570ff24
commit
5130d1d38e
6 changed files with 683 additions and 107 deletions
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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 }),
|
||||
};
|
||||
|
|
|
|||
|
|
@ -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<MongoFile | null>} 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<MongoFile & { messageId: string, toolCallId: string } | undefined>} The file metadata or undefined if an error occurs.
|
||||
* @returns {Promise<{ file: MongoFile & { messageId: string, toolCallId: string }, finalize?: () => Promise<MongoFile | null> }>}
|
||||
*/
|
||||
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,
|
||||
|
|
|
|||
|
|
@ -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('<table><tr><td>1</td></tr></table>');
|
||||
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: '<table><tr><td>1</td></tr></table>',
|
||||
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 <script> in cell text from rendering as HTML).
|
||||
mockHasOfficeHtmlPath.mockReturnValue(true);
|
||||
|
||||
const { finalize } = await processCodeOutput({ ...baseParams, name: 'data.xlsx' });
|
||||
await finalize();
|
||||
|
||||
expect(updateFile).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
file_id: 'mock-uuid-1234',
|
||||
text: null,
|
||||
status: 'failed',
|
||||
previewError: 'parser-error',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('finalize() transitions to failed with previewError:timeout when the outer timeout rejects', async () => {
|
||||
/* The passthrough `withTimeout` mock at the file scope returns
|
||||
* its inner promise unchanged, so the only way the catch branch
|
||||
* fires here is if the extractor itself throws. The real
|
||||
* production path: `extractCodeArtifactText` swallows its own
|
||||
* errors and returns null, so any throw reaching `finalizePreview`
|
||||
* came from the outer `withTimeout` rejection. Simulate it by
|
||||
* having the extractor throw with the same shape. */
|
||||
mockAxios.mockResolvedValue({ data: Buffer.alloc(100) });
|
||||
determineFileType.mockResolvedValue({
|
||||
mime: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
|
||||
});
|
||||
mockExtractCodeArtifactText.mockImplementationOnce(async () => {
|
||||
throw new Error('Phase-2 extraction exceeded 60000ms');
|
||||
});
|
||||
|
||||
const { finalize } = await processCodeOutput({ ...baseParams, name: 'data.xlsx' });
|
||||
await finalize();
|
||||
|
||||
expect(updateFile).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
file_id: 'mock-uuid-1234',
|
||||
status: 'failed',
|
||||
previewError: 'timeout',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('survives a failing updateFile in finalize() without throwing', async () => {
|
||||
mockAxios.mockResolvedValue({ data: Buffer.alloc(100) });
|
||||
determineFileType.mockResolvedValue({
|
||||
mime: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
|
||||
});
|
||||
mockExtractCodeArtifactText.mockResolvedValueOnce('<table></table>');
|
||||
mockGetExtractedTextFormat.mockReturnValueOnce('html');
|
||||
updateFile.mockRejectedValueOnce(new Error('mongo down'));
|
||||
|
||||
const { finalize } = await processCodeOutput({ ...baseParams, name: 'data.xlsx' });
|
||||
await expect(finalize()).resolves.toBeNull();
|
||||
expect(logger.error).toHaveBeenCalledWith(
|
||||
expect.stringContaining('failed to persist phase-2 result'),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('legacy single-phase flow (non-office files)', () => {
|
||||
/* Lock in that non-office files (txt/json/pdf/binary) keep the
|
||||
* inline extract+create flow with NO finalize key — the caller
|
||||
* gets a fully-resolved record, no background work to run. */
|
||||
it('returns no finalize key for plain text', async () => {
|
||||
mockAxios.mockResolvedValue({ data: Buffer.alloc(100) });
|
||||
const result = await processCodeOutput({ ...baseParams, name: 'note.txt' });
|
||||
expect(result.finalize).toBeUndefined();
|
||||
expect(result.file).toMatchObject({ filename: 'note.txt' });
|
||||
});
|
||||
|
||||
it('returns no finalize key for the size-limit fallback', async () => {
|
||||
mockAxios.mockResolvedValue({ data: Buffer.alloc(100 * 1024 * 1024) });
|
||||
const result = await processCodeOutput(baseParams);
|
||||
expect(result.finalize).toBeUndefined();
|
||||
expect(result.file.filepath).toContain('/api/files/code/download/');
|
||||
});
|
||||
|
||||
it('returns no finalize key for the saveBuffer-unavailable fallback', async () => {
|
||||
getStrategyFunctions.mockReturnValueOnce({});
|
||||
mockAxios.mockResolvedValue({ data: Buffer.alloc(100) });
|
||||
const result = await processCodeOutput(baseParams);
|
||||
expect(result.finalize).toBeUndefined();
|
||||
expect(result.file.filepath).toContain('/api/files/code/download/');
|
||||
});
|
||||
|
||||
it('returns no finalize key for the axios-error fallback', async () => {
|
||||
mockAxios.mockRejectedValue(new Error('network'));
|
||||
const result = await processCodeOutput(baseParams);
|
||||
expect(result.finalize).toBeUndefined();
|
||||
expect(result.file.filepath).toContain('/api/files/code/download/');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('readSandboxFile', () => {
|
||||
|
|
@ -1109,4 +1291,101 @@ describe('Code Process', () => {
|
|||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('primeFiles toolContext surfaces preview status to the LLM', () => {
|
||||
/* When a prior-turn code-execution file's HTML preview never resolved
|
||||
* (still pending, or failed), the agent context for this turn must
|
||||
* carry that signal so the model can tell the user "you can still
|
||||
* download it, but the preview isn't available." Otherwise the model
|
||||
* would refer to the file as if everything is fine and the user gets
|
||||
* a confusing UI mismatch. */
|
||||
|
||||
const { getStrategyFunctions } = require('~/server/services/Files/strategies');
|
||||
const { getFiles } = require('~/models');
|
||||
const { filterFilesByAgentAccess } = require('~/server/services/Files/permissions');
|
||||
|
||||
function makeFile(overrides) {
|
||||
return {
|
||||
file_id: `fid-${overrides.status ?? 'ready'}`,
|
||||
filename: `data-${overrides.status ?? 'ready'}.xlsx`,
|
||||
filepath: `/uploads/${overrides.status ?? 'ready'}.xlsx`,
|
||||
source: 'local',
|
||||
context: 'execute_code',
|
||||
metadata: { fileIdentifier: 'CURRENT_SESSION/CURRENT_ID' },
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function setupSessionInfoOk() {
|
||||
/* `getSessionInfo` returns `lastModified`; `checkIfActive` parses
|
||||
* that as a Date and decides whether the sandbox copy is still
|
||||
* fresh (under 23 hours). Use `now` so we always go straight to
|
||||
* `pushFile` and exercise the toolContext annotation logic. */
|
||||
mockAxios.mockResolvedValue({ data: { lastModified: new Date().toISOString() } });
|
||||
getStrategyFunctions.mockReturnValue({});
|
||||
filterFilesByAgentAccess.mockImplementation(({ files }) => Promise.resolve(files));
|
||||
}
|
||||
|
||||
it('annotates a pending file with "(preview not yet generated)"', async () => {
|
||||
setupSessionInfoOk();
|
||||
getFiles.mockResolvedValue([makeFile({ status: 'pending' })]);
|
||||
const result = await primeFiles({
|
||||
req: { user: { id: 'user-123', role: 'USER' } },
|
||||
tool_resources: { execute_code: { file_ids: ['fid-pending'], files: [] } },
|
||||
agentId: 'agent-id',
|
||||
});
|
||||
expect(result.toolContext).toContain('data-pending.xlsx');
|
||||
expect(result.toolContext).toContain('(preview not yet generated)');
|
||||
});
|
||||
|
||||
it('annotates a failed file with "(preview unavailable: <reason>)"', async () => {
|
||||
setupSessionInfoOk();
|
||||
getFiles.mockResolvedValue([makeFile({ status: 'failed', previewError: 'timeout' })]);
|
||||
const result = await primeFiles({
|
||||
req: { user: { id: 'user-123', role: 'USER' } },
|
||||
tool_resources: { execute_code: { file_ids: ['fid-failed'], files: [] } },
|
||||
agentId: 'agent-id',
|
||||
});
|
||||
expect(result.toolContext).toContain('data-failed.xlsx');
|
||||
expect(result.toolContext).toContain('(preview unavailable: timeout)');
|
||||
});
|
||||
|
||||
it('falls back to bare "(preview unavailable)" when previewError is absent', async () => {
|
||||
setupSessionInfoOk();
|
||||
getFiles.mockResolvedValue([makeFile({ status: 'failed' })]);
|
||||
const result = await primeFiles({
|
||||
req: { user: { id: 'user-123', role: 'USER' } },
|
||||
tool_resources: { execute_code: { file_ids: ['fid-failed'], files: [] } },
|
||||
agentId: 'agent-id',
|
||||
});
|
||||
expect(result.toolContext).toContain('(preview unavailable)');
|
||||
expect(result.toolContext).not.toContain('(preview unavailable:');
|
||||
});
|
||||
|
||||
it('does not annotate a ready file (no extra suffix)', async () => {
|
||||
setupSessionInfoOk();
|
||||
getFiles.mockResolvedValue([makeFile({ status: 'ready' })]);
|
||||
const result = await primeFiles({
|
||||
req: { user: { id: 'user-123', role: 'USER' } },
|
||||
tool_resources: { execute_code: { file_ids: ['fid-ready'], files: [] } },
|
||||
agentId: 'agent-id',
|
||||
});
|
||||
expect(result.toolContext).toContain('data-ready.xlsx');
|
||||
expect(result.toolContext).not.toContain('preview');
|
||||
});
|
||||
|
||||
it('does not annotate a legacy file (no status field, back-compat)', async () => {
|
||||
/* Records pre-dating the two-phase flow have no `status`. They
|
||||
* MUST render exactly as before — no suffix at all. */
|
||||
setupSessionInfoOk();
|
||||
getFiles.mockResolvedValue([makeFile({})]); // no status override
|
||||
const result = await primeFiles({
|
||||
req: { user: { id: 'user-123', role: 'USER' } },
|
||||
tool_resources: { execute_code: { file_ids: ['fid-ready'], files: [] } },
|
||||
agentId: 'agent-id',
|
||||
});
|
||||
expect(result.toolContext).toContain('data-ready.xlsx');
|
||||
expect(result.toolContext).not.toContain('preview');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -39,8 +39,12 @@ const officeHtmlLimit = createConcurrencyLimiter(OFFICE_HTML_CONCURRENCY);
|
|||
* (e.g. a tool emitting `data` with `text/csv`, which would otherwise
|
||||
* classify as `utf8-text`, skip the office gate, and ship raw CSV text
|
||||
* to the client's SPREADSHEET bucket that expects full HTML).
|
||||
*
|
||||
* Exported so the two-phase `processCodeOutput` flow can decide whether
|
||||
* to mark a phase-1 file record as `status: 'pending'` (preview expected
|
||||
* later) or skip the status field entirely (no preview ever expected).
|
||||
*/
|
||||
const hasOfficeHtmlPath = (name: string, mimeType: string): boolean =>
|
||||
export const hasOfficeHtmlPath = (name: string, mimeType: string): boolean =>
|
||||
officeHtmlBucket(name, mimeType) !== null;
|
||||
|
||||
/**
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue