mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-27 04:07:05 +00:00
🚀 feat: Decouple File Attachment Persistence from Preview Rendering (#12957)
* 🗂️ feat: add `status` lifecycle to file records for two-phase previews
Schema and model foundation for decoupling the agent's final response
from CPU-heavy office-format HTML extraction.
- `MongoFile.status: 'pending' | 'ready' | 'failed'` (indexed) and
`previewError?: string` mirror the lifecycle: phase-1 emits the file
record at `pending` so the response is unblocked; phase-2 transitions
to `ready` (with text/textFormat) or `failed` (with previewError) in
the background. Absent for legacy records — clients treat that as
`ready` for back-compat.
- Mirror types added to `TFile` in data-provider so frontend cache
consumers see the new fields.
- New `sweepOrphanedPreviews(maxAgeMs)` method on the file model
recovers stale `pending` records left behind by a process restart
mid-extraction; transitions them to `failed` with
`previewError: 'orphaned'`. Cheap because `status` is indexed.
* ⚡ 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.
* 🔍 feat: `GET /api/files/:file_id/preview` endpoint and boot orphan sweep
- New `GET /api/files/:file_id/preview` route returns
`{ status, text?, textFormat?, previewError? }`. The frontend's
`useFilePreview` React Query hook polls this while phase-2 is in
flight, then auto-stops on terminal status. ACL identical to the
download route (reuses `fileAccess` middleware). Defaults `status`
to `'ready'` for legacy records so back-compat is implicit.
`text` only included when `status === 'ready'` and non-null —
preserves the HTML-or-null security contract from #12934.
- `sweepOrphanedPreviews()` invoked on boot in both `server/index.js`
and `server/experimental.js`. Recovers any `pending` records left
behind by a process restart mid-extraction (the only case the
in-process two-phase flow can't handle on its own). Fire-and-forget
so a transient sweep failure doesn't block startup.
* 🖥️ feat: frontend two-phase preview consumer (polling + UI states)
Wires the React side to the new lifecycle so the user sees what's
happening with their file while phase-2 extraction runs in the
background and after the response stream closes.
- `useAttachmentHandler` upserts by `file_id` (was append-only) so
the phase-2 SSE update event merges over the pending placeholder
in place. Lightweight attachments without a `file_id`
(web_search / file_search citations) keep the legacy append path.
- `useFilePreview(file_id)` React Query hook with
`refetchInterval: (data) => data?.status === 'pending' ? 2500 : false`
so polling auto-stops on the first terminal response without the
caller having to flip `enabled`.
- `useAttachmentPreviewSync(attachment)` bridges polled data into
`messageAttachmentsMap`. Polling enabled iff
`status === 'pending' && isAnySubmitting` — per the design ask:
active polling while the LLM is still generating, then quiet.
Process-restart and post-stream cases are covered by polling on
the next interaction.
- `Attachment.tsx` renders a small `PreviewStatusIndicator` (spinner +
"Preparing preview…" for pending, alert icon + "Preview unavailable"
for failed) inside `FileAttachment`. Download button stays fully
functional in both states. Two new English locale keys.
- Data-provider scaffolding: `TFilePreview` type, `endpoints.filePreview`,
`dataService.getFilePreview`, `QueryKeys.filePreview`.
* 🧪 fix: stub `useAttachmentPreviewSync` in pre-existing Attachment test mocks
The new `useAttachmentPreviewSync` hook is called unconditionally inside
`FileAttachment` (added in the prior commit). Two pre-existing test
files mock `~/hooks` to provide `useLocalize` only — the un-mocked
preview hook reference resolved to undefined and crashed render with
`(0 , _hooks.useAttachmentPreviewSync) is not a function` on the
Ubuntu/Windows CI runners.
Fix is local to the test mocks: add a no-op stub that returns
`{ status: 'ready' }` so the component renders the legacy chip path.
The two-phase preview behavior itself has its own dedicated suites
(`useAttachmentHandler.spec.tsx`, `useAttachmentPreviewSync.spec.tsx`).
* 🐛 fix: route phase-2 attachment update to current-run messageId
Codex P1 review on PR #12957. `processCodeOutput` intentionally
preserves the original DB `messageId` across cross-turn filename reuse
so `getCodeGeneratedFiles` can still trace a file back to the
assistant message that originally produced it. The phase-1 SSE emit
already routes by the current run's messageId — `processCodeOutput`
runtime-overlays it via `Object.assign(file, { messageId, toolCallId })`
and the callback writes `result.file` directly.
Phase-2 was passing the raw `updateFile` return through
`attachmentFromFileMetadata`, which read `messageId` straight off the
DB record. On a turn-N run that re-emitted a filename from turn-1
(e.g. agent writes `output.csv` again), the phase-2 SSE update
routed to `turn-1-msg` instead of `turn-N-msg`. Frontend's
`useAttachmentHandler` upserts under the wrong messageAttachmentsMap
slot — turn-N's pending chip stays stuck at "preparing preview…"
while turn-1's already-resolved attachment gets re-merged.
Fix: thread `runtimeMessageId` through `attachmentFromFileMetadata`
and pass `metadata.run_id` from the phase-2 emit site. Mirrors how
phase-1 sources its messageId. Tests cover the cross-turn reuse case
plus the writableEnded / null-finalize / no-finalize paths to lock
in the broader phase-2 emit contract.
* 🛠️ refactor: address codex audit findings (wire-shape parity, DRY, defensive catch)
Comprehensive audit on PR #12957. Resolves all valid findings:
- **MAJOR #1 — Wire-shape parity**: phase-1 ships the full `fileMetadata`
record over SSE; phase-2 was using a tight `attachmentFromFileMetadata`
projection. Drop the projection and have phase-2 spread `{...updated,
messageId, toolCallId}` so both events match the long-standing
legacy phase-1 shape clients depend on.
- **MAJOR #2 — DRY**: extract `runPhase2Finalize({ finalize, fileId,
onResolved })` into `process.js` (alongside `processCodeOutput` whose
contract it pairs with). Both `callbacks.js` paths and `tools.js`
now flow through it. Single catch path eliminates divergence
surface — the fix landed in 01704d4f0 (cross-turn messageId routing)
was a symptom of this duplication risk.
- **MINOR #3 — JSDoc accuracy**: `finalizePreview`'s buffer is bounded
by `fileSizeLimit`, not the 1MB extractor cap. Updated and added a
note about peak heap from queued buffers.
- **MINOR #4 — Defensive catch**: `runPhase2Finalize`'s catch attempts
a best-effort `updateFile({ status: 'failed', previewError:
'unexpected' })` for the file_id, so a programming bug in
`finalizePreview` doesn't leave the record stuck `'pending'` until
the next boot-time orphan sweep.
- **NIT #6 — Stale PR refs**: 12952 → 12957 in 3 places.
- **NIT #7 — Schema bound**: `previewError` capped at `maxlength: 200`
to prevent a future codepath from accidentally persisting a stack
trace.
Skipped per audit verdict (non-blocking):
- #5 (memory pressure): documented in JSDoc; impl change was reviewer's
"consider", not actionable.
- #8 (double DB query per poll): low cost, indexed by_id, polling is
gated narrow.
- #9 (TAttachment cast): the union type is intentional; the casts are
safe widening, refactoring TAttachment is invasive and out of scope.
Tests: 11 new (7 `runPhase2Finalize` unit tests covering happy path,
null-finalize, throws, double-fail, no-fileId, no-onResolved; +4
wire-shape parity assertions in the existing cross-turn test). 328
backend tests pass; 528 frontend tests pass; lint and typecheck clean.
* 🛡️ refactor: address codex P1+P2 + rename to drop phase-1/2 jargon
Codex round 2 review on PR #12957 caught two race conditions and one
recovery gap, all triggered by cross-turn filename reuse (`claimCodeFile`
intentionally returns the same `file_id` for the same
`(filename, conversationId)` across turns). Plus naming cleanup the
user requested — internal "phase 1 / phase 2" vocabulary leaks across
sprints, replace it everywhere with terms describing what's actually
happening.
P1 — stale render overwrites newer revision (process.js)
Two turns reusing `output.csv` share a `file_id`. If turn-1's
background render resolves AFTER turn-2's persist step, the
unconditional `updateFile` writes turn-1's stale text/status over
turn-2's pending placeholder. Fix: stamp a fresh `previewRevision`
UUID on every emit, thread it through `finalizePreview`, and make
the commit conditional via a new optional `extraFilter` argument
on `updateFile` (`{ previewRevision: <expected> }`). The defensive
`updateFile` in `runPreviewFinalize`'s catch uses the same guard
so a programming error from an older render also can't override a
newer turn.
P1 — stale React Query cache on pending remount (queries.ts)
Same root cause from the frontend side. Cache key
`[QueryKeys.filePreview, file_id]` may hold a prior turn's `'ready'`
payload; with `refetchOnMount: false` and the polling gate on
`pending`, polling never starts for the new placeholder. Fix:
`useAttachmentHandler` invalidates that query whenever an attachment
with a `file_id` arrives. Both initial-emit and update events
trigger invalidation — uniform gate.
P2 — quick-restart orphans skipped by boot sweep (files.js)
Boot `sweepOrphanedPreviews` uses a 5-min cutoff for multi-instance
safety. A crash + restart inside the cutoff leaves `pending` records
that never get touched again. Fix: lazy sweep inside the preview
endpoint — if a polled record is `pending` and `updatedAt` is older
than 5 min, mark it `failed:orphaned` on the spot before responding.
Conditional on the same `updatedAt` we observed so a concurrent
legitimate update wins. Cheap, bounded by user activity.
Naming cleanup
- `runPhase2Finalize` → `runPreviewFinalize`
- `PHASE_TWO_TIMEOUT_MS` → `PREVIEW_FINALIZE_TIMEOUT_MS`
- All `phase-1` / `phase-2` / `two-phase` prose replaced with
"the immediate emit", "the deferred render", "the persist step",
"the deferred preview", etc. Skill-feature `phase 1/2` references
(different feature) left alone.
Tests: 10 new (4 lazy-sweep × preview endpoint, 3 cache-invalidation ×
useAttachmentHandler, 3 extraFilter × updateFile data-schemas).
Backend 332/332, frontend 531/531, data-schemas 37/37, lint clean.
* 🛠️ refactor: address comprehensive review (round 3) — stale-cache MAJOR + 3 minors
Comprehensive review on PR #12957 caught a P1 follow-on bug from the
prior `invalidateQueries` fix, plus 3 maintainability findings.
MAJOR: stale React Query cache not actually fixed by `invalidateQueries`
The previous fix called `invalidateQueries` to flush stale cached
preview data on cross-turn filename reuse. But `useFilePreview` had
`refetchOnMount: false`, which made the new observer read the
stale-marked 'ready' data without refetching. The polling
`refetchInterval` then evaluated against stale 'ready' → returned
`false` → polling never started → user stuck on stale content.
Fix (belt-and-suspenders):
a) `useAttachmentHandler` switched to `removeQueries` — drops the
cache entry entirely so the next mount has nothing to read and
must fetch.
b) `useFilePreview` no longer sets `refetchOnMount: false`, so the
React Query default (`true`) kicks in — second line of defense
if any future codepath observes stale data before the handler
has a chance to evict.
MINOR: `finalizePreview` JSDoc missing `previewRevision` param
Added with explanation of the conditional update guard.
MINOR: asymmetric stream-writable guard between SSE protocols
Chat-completions delegated the gate to `writeAttachmentUpdate`;
Open Responses inlined `!res.writableEnded && res.headersSent`.
Extracted `isStreamWritable(res, streamId)` predicate; both paths
+ `writeAttachmentUpdate` now share the single source of truth.
NIT: `(data as Partial<TFile>).file_id` cast repeated 4 times
Extracted to a `fileId` local at the top of the handler.
Tests: existing 9 invalidate-tests rewritten as remove-tests; +1 new
lock-in test asserts removeQueries is called and invalidateQueries
is NOT (regression guard against round-3 finding). 332 backend pass,
532 frontend pass, lint clean.
Skipped findings (deferred / acceptable):
- MINOR: post-submission pending state has no auto-recovery — the
`isAnySubmitting` polling gate was the user's explicit design;
LLM context surfaces failed/pending so the model can volunteer.
Worth a follow-up if real users hit it.
- NIT: double DB query per preview poll — reviewer marked acceptable;
changing `fileAccess` middleware is out of scope.
* 🛡️ test: address comprehensive review NITs (initial-emit guard + isStreamWritable coverage)
NIT — chat-completions initial emit skips writableEnded check
The Open Responses initial emit was switched to use the new
`isStreamWritable` predicate in the round-3 commit, but the
chat-completions initial emit kept the older narrower check
(`streamId || res.headersSent`). On a client disconnect mid-stream
(`writableEnded === true`) it would still hit `res.write` and
raise `ERR_STREAM_WRITE_AFTER_END` — caught by the outer IIFE
catch but logged as noise. Switch this site to `isStreamWritable`
too so both initial-emit paths share the same gate as the
deferred update emits.
NIT — `isStreamWritable` not directly unit-tested
The predicate was only covered indirectly via the deferred-preview
SSE tests (writableEnded skip, headersSent check). Export from
`callbacks.js` and add 5 parametric tests pinning down each branch
(streamId truthy, res null, !headersSent, writableEnded, happy
path) so a future condition addition can't silently regress.
* 🐛 fix: stuck "Preparing preview…" + inline the chip subtitle
Two related fixes for a stuck-spinner bug a user reported in manual
testing of PR #12957.
**Stuck spinner (the bug)**
The deferred preview render can complete a few seconds AFTER the SSE
stream closes (typical case: PPTX render finishes ~3s after the LLM
emits FINAL). When that happens, the SSE update is silently dropped
(`isStreamWritable` returns false on a closed stream) and polling is
the only recovery path.
The earlier polling gate was `status === 'pending' && isAnySubmitting`,
which mirrored the original design intent ("only query while the LLM
is still generating"). But `isAnySubmitting` flips false the moment
the model emits FINAL — milliseconds before the deferred render
commits. Polling never runs, the chip stays "Preparing preview…"
forever even though the DB has `status: 'ready'` with valid HTML.
Drop the `isAnySubmitting` part of the gate. `useFilePreview`'s
`refetchInterval` is already a function-form that returns `false` on
the first terminal response, so polling auto-stops within one tick of
resolution. The server-side render ceiling (60s) plus the lazy sweep
in the preview endpoint cap the worst case to ~24 polls per pending
attachment. Polling itself never blocks UX — the gate's purpose was
"don't waste cycles", and capping by terminal status is the correct
expression of that.
**Inline the chip subtitle (the visual)**
The previous design rendered "Preparing preview…" as a loose-feeling
spinner+text BELOW the file chip. The chip itself looked done while a
floating annotation said it wasn't.
`FileContainer` gains an optional `subtitle?: ReactNode` prop that
overrides the default file-type label. `Attachment.tsx` passes a
`PreviewStatusSubtitle` (spinner + "Preparing preview…" / alert +
"Preview unavailable") into that slot when the file's preview is
pending or failed. The chip footprint stays identical to its `'ready'`
form — just the second row swaps from "PowerPoint Presentation" to
the status indicator. No floating element, no layout shift.
Tests: regression test pinning down "polling stays enabled after the
LLM finishes" so a future revert can't reintroduce the stuck-spinner
bug. Existing FileContainer tests pass unchanged (subtitle override
is opt-in). 522 frontend tests pass; lint clean.
* 🐛 fix: deferred-preview survives reload + matches artifact card chrome
Fixes the remaining stuck-pending case after the polling gate fix: on
a reloaded conversation, message.attachments come from the DB frozen at
the immediate-persist `status: 'pending'`, but `messageAttachmentsMap`
is empty because no SSE handler ever fired for that messageId. Polling
now INSERTS a new live entry when no record matches the file_id, and
`useAttachments` merges live entries onto DB entries by file_id so the
resolved text/textFormat reach `artifactTypeForAttachment` and the
chip routes through the proper PanelArtifact card.
Also replaces the small file chip used during the pending state with
a PreviewPlaceholderCard that mirrors ToolArtifactCard chrome, so the
transition to the resolved PanelArtifact no longer reshapes the UI.
* ✨ feat: auto-open panel when deferred preview resolves pending→ready
The legacy auto-open path is gated only on `isSubmitting`, so an
office-file preview that resolves *after* the SSE stream closes would
render in place but never auto-open the panel — even though that's
exactly the moment the result becomes meaningful to the user. Adds a
per-file_id one-shot signal that `useAttachmentPreviewSync` flips on
the pending→ready edge; `ToolArtifactCard` consumes it on mount and
auto-opens regardless of submission state. The signal is *only* set on
the actual transition (history loads of pre-resolved files don't
trigger it) and is consumed once (panel close + reopen on the same
card stays user-controlled).
* 🐛 fix: drop placeholder Terminal overlay + scope auto-open to fresh resolutions
Two fixes for issues spotted in manual testing of the deferred-preview
auto-open feature:
1. PreviewPlaceholderCard was passing `file={attachment}` to FilePreview,
which triggered SourceIcon's Terminal overlay (`metadata.fileIdentifier`
is set on every code-execution file). The artifact card itself doesn't
show that overlay; the placeholder shouldn't either, so the
pending→resolved transition is visually seamless.
2. The `previewJustResolved` flag flipped on every pending→ready
transition observed by the polling hook — including stale-pending
DB records that resolve via the first poll on a *history load*.
Conversations whose immediate-persist snapshot left attachments at
`status: 'pending'` would yank the panel open every revisit.
Adds `mountedDuringStreamRef` to the hook (mirroring ToolArtifactCard)
so the flag fires only when the hook itself was mounted during an
active turn — preserving the pre-PR contract that the panel only
auto-opens for results the user is actively waiting on, never for
history.
* 🐛 fix: don't downgrade preview to failed when only the SSE emit throws
Codex P2 finding on PR #12957: the original chain placed `.catch` after
`.then(onResolved)`, so a throw inside `onResolved` (transport-side
errors — SSE write race after stream close, an emitter listener
throwing) would propagate into the finalize catch and persist
`status: 'failed'` / `previewError: 'unexpected'`. That surfaced
"preview unavailable" in the UI for a perfectly valid file, and
degraded next-turn LLM context to reflect a non-existent failure.
Wraps `onResolved` in its own try/catch so emit errors are logged but
do not affect the file's persisted status. Extraction success and
emit success are now independent: if extraction succeeds and
`finalizePreview` writes the terminal status, the polling layer / next
page load surfaces the resolved preview even if this turn's SSE emit
didn't land.
* 🛡️ fix: run boot-time orphan sweep under system tenant context
Codex P2 finding on PR #12957: `File` is tenant-isolated, so under
`TENANT_ISOLATION_STRICT=true` the boot-time `sweepOrphanedPreviews`
threw `[TenantIsolation] Query attempted without tenant context in
strict mode` and the recovery path silently failed every restart.
Stale `status: 'pending'` records would be stuck until a user happened
to poll the preview endpoint and trigger the lazy sweep — which only
covers the file the user is currently looking at, not the bulk
candidate set the boot sweep is designed to recover.
Wraps the sweep in `runAsSystem(...)` in both boot paths
(`api/server/index.js` and `api/server/experimental.js`) and pins the
contract with regression tests in `file.spec.ts` — one test asserts
the bare call throws under strict mode, the other asserts the
`runAsSystem`-wrapped call succeeds.
* 🧹 chore: trim verbose comments from previous commit
* 🧹 chore: address review findings (dead branch, lazy-sweep cutoff, stale JSDoc)
- finalizePreview: drop unreachable !isOfficeBucket branch (caller
already gates on hasOfficeHtmlPath, so this path is always office)
- preview endpoint: drop lazy-sweep cutoff from 5min to 2min — anything
past the 60s render ceiling is definitively orphaned, and per-request
sweep can be tighter than the per-instance boot sweep
- strip stale `isSubmitting` references from JSDoc in 3 spots (the
client-side gate was removed in 9a65840)
Skipped: function-length (#3) and client-side polling cap (#4) —
refactors without correctness/perf wins; remaining NITs.
* 🧹 fix: trim 1 query off pending polls + clear stale lifecycle on cross-shape updates
- Preview endpoint: reuse fileAccess middleware's record for the
lifecycle check; only re-fetch with text on the terminal ready
response. Cuts the typical poll lifecycle from 2(N+1) to N+1
queries, since the vast majority of polls hit while pending and
don't need text at all.
- processCodeOutput non-office branch: explicitly null out status,
previewError, previewRevision (codex P2). Without this, an update at
the same (filename, conversationId) where the prior emit was an
office file leaves stale lifecycle fields and the client renders
the wrong state for the now non-office artifact.
- Tests: rewire preview.spec mocks for the new shape, add boundary
test pinning the 2min cutoff, add regression test for the
cross-shape update.
* 🐛 fix: keep polling on transient errors but cap permanently-broken endpoint
Codex P2: the previous `data?.status === 'pending' ? 2500 : false` gate
killed polling on the first transient error. With `retry: false`, a 500
left `data` undefined, the callback returned false, and the chip was
stuck "Preparing preview…" forever — exactly the bug the polling layer
was supposed to recover from.
Inverts the gate: stop on terminal success (`ready`/`failed`) or after
5 consecutive errors. Transient errors keep retrying; a permanently
broken endpoint caps at ~12.5s instead of polling forever. Predicate
extracted as `previewRefetchInterval` for direct unit testing without
fighting React Query's timer machinery.
* ✨ feat: render pending-preview files in their own row
Pending deferred-preview chips now bucket into a separate row above
the resolved attachments — reads as "this is still happening" rather
than mixing with completed downloads. Once status flips to ready, the
chip re-buckets into panelArtifacts; failed re-buckets into the file
row alongside other downloads.
* 🎨 fix: render pending-preview chips in the panel-artifact row, not the file row
Previous bucketing put pending chips in the file row (since
`artifactTypeForAttachment` returns null for empty-text records). The
pending placeholder is a future panel artifact — sharing the row keeps
the chip in place when it resolves instead of jumping rows.
Plain files still get their own row.
* 🐛 fix: phase-1 SSE replay must not regress a resolved attachment
Codex P1: useEventHandlers.finalHandler iterates
responseMessage.attachments at stream end and dispatches each through
the attachment handler. Those records are the immediate-persist
snapshot (status:pending, text:null) — if a deferred update has
already moved the same file_id to ready/failed, the existing merge
let the pending fields win and downgraded the resolved record. Result:
chip flickers back to pending and polling restarts until the lazy
sweep corrects.
Pin the terminal lifecycle fields (status, text, textFormat,
previewError) when existing is ready/failed and incoming is pending.
Other field updates still go through.
* 🐛 fix: track preview-poll error cap outside React Query state
Codex P2: the previous cap relied on `query.state.fetchFailureCount`,
but React Query v4's reducer resets that to 0 on every fetch dispatch
(the `'fetch'` action). With `retry: false`, each failed poll left
count at 1 and the next dispatch reset it back to 0, so the `>= 5`
branch never fired and a permanently-broken endpoint polled forever.
Track consecutive errors in a module-level Map keyed by file_id,
incremented in a thin `fetchFilePreview` wrapper around the data
service call. The Map is cleared on success and on cap-stop, so
memory is bounded by in-flight pending file_ids per session.
This commit is contained in:
parent
cf0657509c
commit
6c6c72def7
35 changed files with 3861 additions and 142 deletions
|
|
@ -28,6 +28,28 @@ jest.mock('~/server/services/Files/Citations', () => ({
|
|||
|
||||
jest.mock('~/server/services/Files/Code/process', () => ({
|
||||
processCodeOutput: jest.fn(),
|
||||
/* `runPreviewFinalize` is the runtime pairing for `finalize` (defined
|
||||
* alongside processCodeOutput in process.js). The callback wires
|
||||
* the deferred render through it; reproduce the basic happy-path here so the
|
||||
* SSE-emit assertions still work. The catch/defensive-updateFile
|
||||
* branch is unit-tested directly against the real helper in
|
||||
* process.spec.js — exercising it here would add test coupling
|
||||
* without coverage benefit. */
|
||||
runPreviewFinalize: ({ finalize, onResolved }) => {
|
||||
if (typeof finalize !== 'function') {
|
||||
return;
|
||||
}
|
||||
finalize()
|
||||
.then((updated) => {
|
||||
if (!updated || !onResolved) {
|
||||
return;
|
||||
}
|
||||
onResolved(updated);
|
||||
})
|
||||
.catch(() => {
|
||||
/* swallowed in the mock — see process.spec.js for catch coverage */
|
||||
});
|
||||
},
|
||||
}));
|
||||
|
||||
jest.mock('~/server/services/Tools/credentials', () => ({
|
||||
|
|
@ -326,4 +348,266 @@ describe('createToolEndCallback', () => {
|
|||
expect(res.write).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('code execution deferred-preview emit', () => {
|
||||
/* The deferred-preview code-execution flow emits the attachment twice over
|
||||
* SSE: the initial emit with `status: 'pending'` and the current run's
|
||||
* messageId, the deferred render with the resolved record. The preview update emit
|
||||
* must use the CURRENT run's messageId (not the persisted DB one)
|
||||
* because `processCodeOutput` intentionally preserves the original
|
||||
* `messageId` on cross-turn filename reuse — `getCodeGeneratedFiles`
|
||||
* needs that for prior-turn priming.
|
||||
*
|
||||
* Codex P1 review on PR #12957: shipping `updated.messageId`
|
||||
* straight from the DB record routed preview-update patches to the wrong
|
||||
* message slot, leaving the current turn's pending chip stuck. */
|
||||
|
||||
const { processCodeOutput } = require('~/server/services/Files/Code/process');
|
||||
|
||||
function makeCodeExecutionEvent({ runId, threadId, toolCallId, fileId, name }) {
|
||||
return {
|
||||
output: {
|
||||
name: 'execute_code',
|
||||
tool_call_id: toolCallId,
|
||||
artifact: {
|
||||
session_id: 'sess-1',
|
||||
files: [{ id: fileId, name, session_id: 'sess-1' }],
|
||||
},
|
||||
},
|
||||
metadata: { run_id: runId, thread_id: threadId },
|
||||
};
|
||||
}
|
||||
|
||||
/** Parse the SSE frame `res.write` produces back to a payload object. */
|
||||
function parseSseAttachment(call) {
|
||||
const frame = call[0];
|
||||
const dataLine = frame.split('\n').find((l) => l.startsWith('data: '));
|
||||
return JSON.parse(dataLine.slice('data: '.length));
|
||||
}
|
||||
|
||||
it('the preview update emit uses the current run messageId, not the persisted DB messageId (cross-turn filename reuse)', async () => {
|
||||
/* Simulate turn-2 reusing `output.csv` from turn-1. The DB record
|
||||
* surfaced by `updateFile` carries the original `turn-1-msg`
|
||||
* messageId; the runtime emit must rewrite to `turn-2-msg`. */
|
||||
res.headersSent = true;
|
||||
const finalize = jest.fn().mockResolvedValue({
|
||||
file_id: 'fid-shared',
|
||||
filename: 'output.csv',
|
||||
filepath: '/uploads/output.csv',
|
||||
type: 'text/csv',
|
||||
conversationId: 'thread789',
|
||||
messageId: 'turn-1-original-msg', // persisted DB id (older turn)
|
||||
status: 'ready',
|
||||
text: '<table></table>',
|
||||
textFormat: 'html',
|
||||
});
|
||||
processCodeOutput.mockResolvedValue({
|
||||
file: {
|
||||
file_id: 'fid-shared',
|
||||
filename: 'output.csv',
|
||||
filepath: '/uploads/output.csv',
|
||||
type: 'text/csv',
|
||||
conversationId: 'thread789',
|
||||
messageId: 'turn-2-current-run', // runtime overlay (current turn)
|
||||
toolCallId: 'tool-2',
|
||||
status: 'pending',
|
||||
text: null,
|
||||
textFormat: null,
|
||||
},
|
||||
finalize,
|
||||
});
|
||||
|
||||
const toolEndCallback = createToolEndCallback({ req, res, artifactPromises });
|
||||
const event = makeCodeExecutionEvent({
|
||||
runId: 'turn-2-current-run',
|
||||
threadId: 'thread789',
|
||||
toolCallId: 'tool-2',
|
||||
fileId: 'fid-shared',
|
||||
name: 'output.csv',
|
||||
});
|
||||
await toolEndCallback({ output: event.output }, event.metadata);
|
||||
await Promise.all(artifactPromises);
|
||||
// Wait one more tick so the fire-and-forget finalize() chain settles.
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
|
||||
// Two SSE writes: the initial emit (pending) and the deferred render (ready).
|
||||
expect(res.write).toHaveBeenCalledTimes(2);
|
||||
const phase1 = parseSseAttachment(res.write.mock.calls[0]);
|
||||
const phase2 = parseSseAttachment(res.write.mock.calls[1]);
|
||||
|
||||
// Initial emit already used the runtime messageId (sourced from result.file).
|
||||
expect(phase1.messageId).toBe('turn-2-current-run');
|
||||
expect(phase1.status).toBe('pending');
|
||||
|
||||
/* The preview update MUST also route to the current run's messageId so the
|
||||
* frontend's `useAttachmentHandler` upserts under the same
|
||||
* messageAttachmentsMap slot as the initial emit. Routing to
|
||||
* `turn-1-original-msg` would land the patch on a stale message
|
||||
* and leave turn-2's pending chip stuck. */
|
||||
expect(phase2.messageId).toBe('turn-2-current-run');
|
||||
expect(phase2.file_id).toBe('fid-shared');
|
||||
expect(phase2.status).toBe('ready');
|
||||
expect(phase2.text).toBe('<table></table>');
|
||||
expect(phase2.toolCallId).toBe('tool-2');
|
||||
/* Wire-shape parity with the initial emit: preview update emits the full updated
|
||||
* record so the client doesn't see one shape on the initial emit and a
|
||||
* narrower projection on the deferred render. (Codex audit on PR #12957
|
||||
* Finding 1.) */
|
||||
expect(phase2.filename).toBe('output.csv');
|
||||
expect(phase2.filepath).toBe('/uploads/output.csv');
|
||||
expect(phase2.type).toBe('text/csv');
|
||||
expect(phase2.conversationId).toBe('thread789');
|
||||
expect(phase2.textFormat).toBe('html');
|
||||
});
|
||||
|
||||
it('the preview update emit is skipped when finalize resolves to null (no DB update happened)', async () => {
|
||||
res.headersSent = true;
|
||||
processCodeOutput.mockResolvedValue({
|
||||
file: {
|
||||
file_id: 'fid-1',
|
||||
filename: 'data.xlsx',
|
||||
filepath: '/uploads/data.xlsx',
|
||||
type: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
|
||||
messageId: 'run-1',
|
||||
toolCallId: 'tool-1',
|
||||
status: 'pending',
|
||||
},
|
||||
finalize: jest.fn().mockResolvedValue(null),
|
||||
});
|
||||
|
||||
const toolEndCallback = createToolEndCallback({ req, res, artifactPromises });
|
||||
const event = makeCodeExecutionEvent({
|
||||
runId: 'run-1',
|
||||
threadId: 'thread-1',
|
||||
toolCallId: 'tool-1',
|
||||
fileId: 'fid-1',
|
||||
name: 'data.xlsx',
|
||||
});
|
||||
await toolEndCallback({ output: event.output }, event.metadata);
|
||||
await Promise.all(artifactPromises);
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
|
||||
// Only the initial emit fired; preview update noop'd because finalize returned null.
|
||||
expect(res.write).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('the preview update emit is skipped when the response stream has already closed', async () => {
|
||||
res.headersSent = true;
|
||||
/* Hand-rolled deferred so we can hold finalize() open until
|
||||
* AFTER setting `res.writableEnded = true`. Otherwise the mock
|
||||
* resolves synchronously, the .then() runs in the same microtask
|
||||
* queue as the artifactPromises await, and writableEnded is set
|
||||
* too late. */
|
||||
let resolveFinalize;
|
||||
const finalizeDeferred = new Promise((resolve) => {
|
||||
resolveFinalize = resolve;
|
||||
});
|
||||
processCodeOutput.mockResolvedValue({
|
||||
file: {
|
||||
file_id: 'fid-1',
|
||||
filename: 'data.xlsx',
|
||||
filepath: '/uploads/data.xlsx',
|
||||
type: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
|
||||
messageId: 'run-1',
|
||||
toolCallId: 'tool-1',
|
||||
status: 'pending',
|
||||
},
|
||||
finalize: jest.fn().mockReturnValue(finalizeDeferred),
|
||||
});
|
||||
|
||||
const toolEndCallback = createToolEndCallback({ req, res, artifactPromises });
|
||||
const event = makeCodeExecutionEvent({
|
||||
runId: 'run-1',
|
||||
threadId: 'thread-1',
|
||||
toolCallId: 'tool-1',
|
||||
fileId: 'fid-1',
|
||||
name: 'data.xlsx',
|
||||
});
|
||||
await toolEndCallback({ output: event.output }, event.metadata);
|
||||
await Promise.all(artifactPromises);
|
||||
// Simulate the response closing AFTER the initial emit fires but BEFORE
|
||||
// the deferred render lands. The frontend's polling path will catch the
|
||||
// resolved record on its next tick.
|
||||
res.writableEnded = true;
|
||||
// Now resolve finalize and let the .then() chain run.
|
||||
resolveFinalize({
|
||||
file_id: 'fid-1',
|
||||
filename: 'data.xlsx',
|
||||
messageId: 'run-1',
|
||||
status: 'ready',
|
||||
text: '<x/>',
|
||||
textFormat: 'html',
|
||||
});
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
|
||||
// Initial emit wrote; preview update noop'd because writableEnded.
|
||||
expect(res.write).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('does not call finalize for a non-office file (no preview expected)', async () => {
|
||||
res.headersSent = true;
|
||||
processCodeOutput.mockResolvedValue({
|
||||
file: {
|
||||
file_id: 'fid-txt',
|
||||
filename: 'note.txt',
|
||||
filepath: '/uploads/note.txt',
|
||||
type: 'text/plain',
|
||||
messageId: 'run-1',
|
||||
toolCallId: 'tool-1',
|
||||
// No status — non-office files skip the deferred render entirely.
|
||||
},
|
||||
// No finalize key — caller should not call anything.
|
||||
});
|
||||
|
||||
const toolEndCallback = createToolEndCallback({ req, res, artifactPromises });
|
||||
const event = makeCodeExecutionEvent({
|
||||
runId: 'run-1',
|
||||
threadId: 'thread-1',
|
||||
toolCallId: 'tool-1',
|
||||
fileId: 'fid-txt',
|
||||
name: 'note.txt',
|
||||
});
|
||||
await toolEndCallback({ output: event.output }, event.metadata);
|
||||
await Promise.all(artifactPromises);
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
|
||||
expect(res.write).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('isStreamWritable', () => {
|
||||
/* Direct parametric coverage of the predicate that gates SSE writes
|
||||
* in both the chat-completions and Open Responses callbacks. The
|
||||
* existing deferred-preview tests cover this indirectly via the
|
||||
* `writeAttachmentUpdate` writableEnded path; these tests pin down
|
||||
* each individual branch so a future modification (e.g. adding a
|
||||
* new condition) can't silently regress.
|
||||
* (Comprehensive review NIT on PR #12957.) */
|
||||
const { isStreamWritable } = require('../callbacks');
|
||||
|
||||
it('returns true when streamId is truthy regardless of res state', () => {
|
||||
/* Resumable mode writes go to the job emitter; res state is
|
||||
* irrelevant. Even a closed res with no headers should not block. */
|
||||
expect(isStreamWritable(null, 'stream-1')).toBe(true);
|
||||
expect(isStreamWritable({ headersSent: false, writableEnded: true }, 'stream-1')).toBe(true);
|
||||
expect(isStreamWritable(undefined, 'stream-1')).toBe(true);
|
||||
});
|
||||
|
||||
it('returns false when streamId is falsy and res is null/undefined', () => {
|
||||
expect(isStreamWritable(null, null)).toBe(false);
|
||||
expect(isStreamWritable(undefined, null)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false when headers have not been sent yet', () => {
|
||||
expect(isStreamWritable({ headersSent: false, writableEnded: false }, null)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns false when the stream has already ended', () => {
|
||||
expect(isStreamWritable({ headersSent: true, writableEnded: true }, null)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns true on the happy path: headers sent, not ended, no streamId', () => {
|
||||
expect(isStreamWritable({ headersSent: true, writableEnded: false }, null)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ const {
|
|||
createToolExecuteHandler,
|
||||
} = require('@librechat/api');
|
||||
const { processFileCitations } = require('~/server/services/Files/Citations');
|
||||
const { processCodeOutput } = require('~/server/services/Files/Code/process');
|
||||
const { processCodeOutput, runPreviewFinalize } = require('~/server/services/Files/Code/process');
|
||||
const { saveBase64Image } = require('~/server/services/Files/process');
|
||||
|
||||
class ModelEndHandler {
|
||||
|
|
@ -397,6 +397,55 @@ function writeAttachment(res, streamId, attachment) {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Predicate: is it safe to push an SSE write to the caller right now?
|
||||
*
|
||||
* In `streamId` (resumable) mode, writes go to the job emitter and the
|
||||
* `res` state is irrelevant — always writable.
|
||||
*
|
||||
* In standard mode, the caller's `res` must have headers sent (the
|
||||
* stream has been opened) and not yet be `writableEnded` (the response
|
||||
* hasn't closed). Writing to a closed stream raises
|
||||
* `ERR_STREAM_WRITE_AFTER_END`.
|
||||
*
|
||||
* Used by deferred preview emits in both `createToolEndCallback`
|
||||
* (chat-completions) and `createResponsesToolEndCallback` (Open
|
||||
* Responses) so the gate logic stays in one place. (Comprehensive
|
||||
* review #3 on PR #12957.)
|
||||
*/
|
||||
function isStreamWritable(res, streamId) {
|
||||
if (streamId) {
|
||||
return true;
|
||||
}
|
||||
return !!res && res.headersSent && !res.writableEnded;
|
||||
}
|
||||
|
||||
/**
|
||||
* Emit an 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 the deferred
|
||||
* preview 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 (!isStreamWritable(res, streamId)) {
|
||||
return;
|
||||
}
|
||||
writeAttachment(res, streamId, attachment);
|
||||
}
|
||||
|
||||
/**
|
||||
*
|
||||
* @param {Object} params
|
||||
|
|
@ -556,14 +605,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 +633,54 @@ 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);
|
||||
/* Initial 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.
|
||||
*
|
||||
* Use the shared `isStreamWritable` predicate rather than the
|
||||
* narrower `streamId || res.headersSent` check that lived
|
||||
* here before — a client disconnect mid-stream
|
||||
* (`res.writableEnded`) would otherwise hit `res.write` and
|
||||
* raise `ERR_STREAM_WRITE_AFTER_END` (caught by the outer
|
||||
* IIFE catch but logged as noise). Same gate the Responses
|
||||
* path uses below. */
|
||||
if (isStreamWritable(res, streamId)) {
|
||||
writeAttachment(res, streamId, fileMetadata);
|
||||
}
|
||||
/* Deferred preview rendering: extraction continues running
|
||||
* even after the HTTP response closes. If the stream is still
|
||||
* open when the preview resolves, push an `attachment`
|
||||
* update event so the UI patches in place; otherwise React
|
||||
* Query polling on `/api/files/:file_id/preview` picks it up.
|
||||
*
|
||||
* Spread the full updated record (mirroring the initial emit
|
||||
* shape) and overlay `messageId`/`toolCallId` from the
|
||||
* current run. The DB record preserves the original
|
||||
* `messageId` across cross-turn filename reuse so
|
||||
* `getCodeGeneratedFiles` can trace the file back to its
|
||||
* original assistant message; routing the update SSE by the
|
||||
* persisted id would land the patch on a stale message
|
||||
* slot — turn-N's pending placeholder would stay stuck while
|
||||
* turn-1's already-resolved attachment got re-merged.
|
||||
* (Codex P1 review on PR #12957.) */
|
||||
runPreviewFinalize({
|
||||
finalize,
|
||||
fileId: fileMetadata.file_id,
|
||||
previewRevision: result?.previewRevision,
|
||||
onResolved: (updated) => {
|
||||
writeAttachmentUpdate(res, streamId, {
|
||||
...updated,
|
||||
messageId: metadata.run_id,
|
||||
toolCallId,
|
||||
});
|
||||
},
|
||||
});
|
||||
return fileMetadata;
|
||||
})().catch((error) => {
|
||||
logger.error('Error processing code output:', error);
|
||||
|
|
@ -782,14 +871,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,38 +899,45 @@ 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
|
||||
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);
|
||||
/* Initial emit (Open Responses extension format). The agent's
|
||||
* response no longer blocks on extraction. */
|
||||
if (isStreamWritable(res, null)) {
|
||||
writeResponsesAttachment(
|
||||
res,
|
||||
tracker,
|
||||
buildResponsesAttachment(fileMetadata, toolCallId),
|
||||
metadata,
|
||||
);
|
||||
}
|
||||
|
||||
/* Deferred preview rendering: extract HTML 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. */
|
||||
runPreviewFinalize({
|
||||
finalize,
|
||||
fileId: fileMetadata.file_id,
|
||||
previewRevision: result?.previewRevision,
|
||||
onResolved: (updated) => {
|
||||
if (!isStreamWritable(res, null)) {
|
||||
return;
|
||||
}
|
||||
writeResponsesAttachment(
|
||||
res,
|
||||
tracker,
|
||||
buildResponsesAttachment(updated, toolCallId),
|
||||
metadata,
|
||||
);
|
||||
},
|
||||
});
|
||||
|
||||
return fileMetadata;
|
||||
})().catch((error) => {
|
||||
logger.error('Error processing code output:', error);
|
||||
|
|
@ -851,6 +948,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 deferred preview 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) {
|
||||
|
|
@ -906,6 +1025,7 @@ module.exports = {
|
|||
agentLogHandlerObj,
|
||||
getDefaultHandlers,
|
||||
createToolEndCallback,
|
||||
isStreamWritable,
|
||||
markSummarizationUsage,
|
||||
buildSummarizationHandlers,
|
||||
createResponsesToolEndCallback,
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ const {
|
|||
} = require('librechat-data-provider');
|
||||
const { getRoleByName, createToolCall, getToolCallsByConvo, getMessage } = require('~/models');
|
||||
const { processFileURL, uploadImageBuffer } = require('~/server/services/Files/process');
|
||||
const { processCodeOutput } = require('~/server/services/Files/Code/process');
|
||||
const { processCodeOutput, runPreviewFinalize } = require('~/server/services/Files/Code/process');
|
||||
const { loadAuthValues } = require('~/server/services/Tools/credentials');
|
||||
const { loadTools } = require('~/app/clients/tools/util');
|
||||
|
||||
|
|
@ -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,22 @@ 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 persisted record immediately
|
||||
* (with `status: 'pending'` for office buckets) and run the
|
||||
* preview render in the background. The client polls
|
||||
* `/api/files/:file_id/preview` for the resolved record.
|
||||
* No `onResolved` — there's no live stream to write to here. */
|
||||
runPreviewFinalize({
|
||||
finalize,
|
||||
fileId: fileMetadata.file_id,
|
||||
previewRevision: result?.previewRevision,
|
||||
});
|
||||
return fileMetadata;
|
||||
})().catch((error) => {
|
||||
logger.error('Error processing code output:', error);
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ const express = require('express');
|
|||
const passport = require('passport');
|
||||
const compression = require('compression');
|
||||
const cookieParser = require('cookie-parser');
|
||||
const { logger } = require('@librechat/data-schemas');
|
||||
const { logger, runAsSystem } = require('@librechat/data-schemas');
|
||||
const mongoSanitize = require('express-mongo-sanitize');
|
||||
const {
|
||||
isEnabled,
|
||||
|
|
@ -26,7 +26,12 @@ const initializeOAuthReconnectManager = require('./services/initializeOAuthRecon
|
|||
const createValidateImageRequest = require('./middleware/validateImageRequest');
|
||||
const { jwtLogin, ldapLogin, passportLogin } = require('~/strategies');
|
||||
const { updateInterfacePermissions: updateInterfacePerms } = require('@librechat/api');
|
||||
const { getRoleByName, updateAccessPermissions, seedDatabase } = require('~/models');
|
||||
const {
|
||||
getRoleByName,
|
||||
updateAccessPermissions,
|
||||
seedDatabase,
|
||||
sweepOrphanedPreviews,
|
||||
} = require('~/models');
|
||||
const { checkMigrations } = require('./services/start/migration');
|
||||
const initializeMCPs = require('./services/initializeMCPs');
|
||||
const configureSocialLogins = require('./socialLogins');
|
||||
|
|
@ -220,6 +225,11 @@ if (cluster.isMaster) {
|
|||
/** Seed database (idempotent) */
|
||||
await seedDatabase();
|
||||
|
||||
/* Mirrors `server/index.js`; `runAsSystem` for tenant-isolated File. */
|
||||
runAsSystem(sweepOrphanedPreviews).catch((err) => {
|
||||
logger.error('[sweepOrphanedPreviews] Background sweep failed:', err);
|
||||
});
|
||||
|
||||
/** Initialize app configuration */
|
||||
const appConfig = await getAppConfig();
|
||||
initializeFileStorage(appConfig);
|
||||
|
|
|
|||
|
|
@ -25,7 +25,12 @@ const {
|
|||
} = require('@librechat/api');
|
||||
const { connectDb, indexSync } = require('~/db');
|
||||
const initializeOAuthReconnectManager = require('./services/initializeOAuthReconnectManager');
|
||||
const { getRoleByName, updateAccessPermissions, seedDatabase } = require('~/models');
|
||||
const {
|
||||
getRoleByName,
|
||||
updateAccessPermissions,
|
||||
seedDatabase,
|
||||
sweepOrphanedPreviews,
|
||||
} = require('~/models');
|
||||
const { capabilityContextMiddleware } = require('./middleware/roles/capabilities');
|
||||
const createValidateImageRequest = require('./middleware/validateImageRequest');
|
||||
const { jwtLogin, ldapLogin, passportLogin } = require('~/strategies');
|
||||
|
|
@ -69,6 +74,13 @@ const startServer = async () => {
|
|||
}
|
||||
|
||||
await runAsSystem(seedDatabase);
|
||||
/* Recover stuck `status: 'pending'` records from a crash mid-render.
|
||||
* `runAsSystem` is required — `File` is tenant-isolated and strict
|
||||
* mode rejects unscoped queries. Lazy sweep in the preview endpoint
|
||||
* covers anything younger than the boot cutoff. */
|
||||
runAsSystem(sweepOrphanedPreviews).catch((err) => {
|
||||
logger.error('[sweepOrphanedPreviews] Background sweep failed:', err);
|
||||
});
|
||||
const appConfig = await getAppConfig({ baseOnly: true });
|
||||
initializeFileStorage(appConfig);
|
||||
await runAsSystem(async () => {
|
||||
|
|
|
|||
|
|
@ -295,6 +295,86 @@ router.get('/code/download/:session_id/:fileId', async (req, res) => {
|
|||
}
|
||||
});
|
||||
|
||||
/* Lazy-sweep cutoff: pending records older than this are marked failed
|
||||
* on the next poll. 2min is well past the 60s render ceiling, so any
|
||||
* `pending` past it is definitively orphaned. Tighter than the boot
|
||||
* sweep (5min) since this runs per-request, not per-instance. */
|
||||
const PREVIEW_LAZY_SWEEP_CUTOFF_MS = 2 * 60 * 1000;
|
||||
|
||||
/**
|
||||
* Poll the lifecycle status of a code-execution file's inline preview.
|
||||
*
|
||||
* Deferred-preview flow: the immediate persist step writes the file
|
||||
* record at `status: 'pending'`; the background render transitions
|
||||
* it to `'ready'` (with `text` + `textFormat`) or `'failed'` (with
|
||||
* `previewError`). The frontend's `useFilePreview` React Query hook
|
||||
* polls this endpoint at ~2.5s intervals while `status === 'pending'`,
|
||||
* then auto-stops on terminal status.
|
||||
*
|
||||
* Returns the smallest viable shape:
|
||||
* - `status` always present (defaults to `'ready'` for legacy records
|
||||
* that never had the field — clients treat absent as ready).
|
||||
* - `text` and `textFormat` only when status is 'ready' AND text
|
||||
* is non-null (preserves the security contract from PR #12934 —
|
||||
* office bucket files MUST NOT receive plain-text fallbacks).
|
||||
* - `previewError` only when status is 'failed'.
|
||||
*
|
||||
* Lazy-sweeps stale `pending` records on the spot — see
|
||||
* `PREVIEW_LAZY_SWEEP_CUTOFF_MS` for the rationale.
|
||||
*
|
||||
* Reuses the `fileAccess` middleware so ACL is identical to download.
|
||||
*
|
||||
* @route GET /files/:file_id/preview
|
||||
*/
|
||||
router.get('/:file_id/preview', fileAccess, async (req, res) => {
|
||||
try {
|
||||
const { file_id } = req.params;
|
||||
/* `fileAccess` already fetched the record (sans `text`, the default
|
||||
* projection drops it). Reuse for the lifecycle check; only re-fetch
|
||||
* with `text` on a terminal ready response — the typical lifecycle
|
||||
* is N pending polls + 1 ready, so this avoids ~N redundant text
|
||||
* reads per file. */
|
||||
let file = req.fileAccess.file;
|
||||
/* Lazy sweep: if stuck `pending` past the cutoff, mark `failed`
|
||||
* conditional on the observed `updatedAt` (concurrent legitimate
|
||||
* updates win). */
|
||||
if (file.status === 'pending' && file.updatedAt instanceof Date) {
|
||||
const ageMs = Date.now() - file.updatedAt.getTime();
|
||||
if (ageMs > PREVIEW_LAZY_SWEEP_CUTOFF_MS) {
|
||||
const swept = await db.updateFile(
|
||||
{ file_id, status: 'failed', previewError: 'orphaned' },
|
||||
{ status: 'pending', updatedAt: file.updatedAt },
|
||||
);
|
||||
if (swept) {
|
||||
file = swept;
|
||||
logger.info(
|
||||
`[/files/:file_id/preview] Lazy-swept orphaned pending record ${file_id} (age ${Math.round(ageMs / 1000)}s)`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
/* Default to 'ready' for back-compat: legacy records pre-date the
|
||||
* field, and non-office files never get a status set on persist. */
|
||||
const status = file.status ?? 'ready';
|
||||
const payload = { file_id, status };
|
||||
if (status === 'ready') {
|
||||
const withText = await db.findFileById(file_id);
|
||||
if (withText?.text != null) {
|
||||
payload.text = withText.text;
|
||||
payload.textFormat = withText.textFormat ?? null;
|
||||
}
|
||||
} else if (status === 'failed' && file.previewError) {
|
||||
payload.previewError = file.previewError;
|
||||
}
|
||||
return res.status(200).json(payload);
|
||||
} catch (error) {
|
||||
logger.error('[/files/:file_id/preview] Error fetching preview status:', error);
|
||||
return res
|
||||
.status(500)
|
||||
.json({ error: 'Internal Server Error', message: 'Failed to fetch preview status' });
|
||||
}
|
||||
});
|
||||
|
||||
router.get('/download/:userId/:file_id', fileAccess, async (req, res) => {
|
||||
try {
|
||||
const { userId, file_id } = req.params;
|
||||
|
|
|
|||
372
api/server/routes/files/preview.spec.js
Normal file
372
api/server/routes/files/preview.spec.js
Normal file
|
|
@ -0,0 +1,372 @@
|
|||
/**
|
||||
* Coverage for the new GET /files/:file_id/preview endpoint.
|
||||
*
|
||||
* Deferred-preview code-execution flow: the immediate persist step
|
||||
* emits a file record at `status: 'pending'`; the background render
|
||||
* transitions it to `'ready'` (with text) or `'failed'` (with
|
||||
* previewError). The frontend polls this endpoint until status is
|
||||
* terminal. This suite asserts the response shape across all four
|
||||
* states (pending, ready, failed, legacy/back-compat) and the auth
|
||||
* boundary (404 vs 403).
|
||||
*/
|
||||
|
||||
jest.mock('@librechat/data-schemas', () => ({
|
||||
logger: { warn: jest.fn(), debug: jest.fn(), error: jest.fn(), info: jest.fn() },
|
||||
SystemCapabilities: {},
|
||||
}));
|
||||
|
||||
jest.mock('@librechat/api', () => ({
|
||||
refreshS3FileUrls: jest.fn(),
|
||||
resolveUploadErrorMessage: jest.fn(),
|
||||
verifyAgentUploadPermission: jest.fn(),
|
||||
}));
|
||||
|
||||
const mockFindFileById = jest.fn();
|
||||
const mockGetFiles = jest.fn();
|
||||
const mockUpdateFile = jest.fn();
|
||||
jest.mock('~/models', () => ({
|
||||
findFileById: (...args) => mockFindFileById(...args),
|
||||
getFiles: (...args) => mockGetFiles(...args),
|
||||
updateFile: (...args) => mockUpdateFile(...args),
|
||||
getAgents: jest.fn().mockResolvedValue([]),
|
||||
batchUpdateFiles: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/services/Files/process', () => ({
|
||||
filterFile: jest.fn(),
|
||||
processFileUpload: jest.fn(),
|
||||
processDeleteRequest: jest.fn(),
|
||||
processAgentFileUpload: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/services/Files/strategies', () => ({
|
||||
getStrategyFunctions: jest.fn(() => ({})),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/controllers/assistants/helpers', () => ({
|
||||
getOpenAIClient: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/middleware/roles/capabilities', () => ({
|
||||
hasCapability: jest.fn(() => (_req, _res, next) => next()),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/services/PermissionService', () => ({
|
||||
checkPermission: jest.fn(() => (_req, _res, next) => next()),
|
||||
getEffectivePermissions: jest.fn().mockResolvedValue(0),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/services/Files', () => ({
|
||||
hasAccessToFilesViaAgent: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/utils/files', () => ({
|
||||
cleanFileName: (name) => name,
|
||||
}));
|
||||
|
||||
jest.mock('~/cache', () => ({
|
||||
getLogStores: jest.fn(() => ({ get: jest.fn(), set: jest.fn() })),
|
||||
}));
|
||||
|
||||
const express = require('express');
|
||||
const request = require('supertest');
|
||||
const filesRouter = require('./files');
|
||||
|
||||
/**
|
||||
* Mount the router with a per-request user injector so we can simulate
|
||||
* a logged-in user without spinning up the full auth stack.
|
||||
*/
|
||||
function buildApp({ user = { id: 'user-123', role: 'user' } } = {}) {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use((req, _res, next) => {
|
||||
req.user = user;
|
||||
req.config = { fileStrategy: 'local' };
|
||||
next();
|
||||
});
|
||||
app.use('/files', filesRouter);
|
||||
return app;
|
||||
}
|
||||
|
||||
const OWNER_USER_ID = 'user-123';
|
||||
|
||||
describe('GET /files/:file_id/preview', () => {
|
||||
beforeEach(() => {
|
||||
mockFindFileById.mockReset();
|
||||
mockGetFiles.mockReset();
|
||||
mockUpdateFile.mockReset();
|
||||
});
|
||||
|
||||
it('returns 404 when the file does not exist (auth check fails first via fileAccess)', async () => {
|
||||
/* `fileAccess` middleware does its own getFiles lookup and returns
|
||||
* 404 before our handler ever runs. This test asserts the boundary
|
||||
* lives there, not that the handler duplicates the check. */
|
||||
mockGetFiles.mockResolvedValueOnce([]);
|
||||
const res = await request(buildApp()).get('/files/missing-id/preview');
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.body).toMatchObject({ error: 'Not Found' });
|
||||
expect(mockFindFileById).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('returns 403 when the requester does not own the file and has no agent-based access', async () => {
|
||||
/* fileAccess returns 403 — the file exists but belongs to someone
|
||||
* else and no agent grants access. The preview handler should
|
||||
* never run. */
|
||||
mockGetFiles.mockResolvedValueOnce([
|
||||
{ file_id: 'someone-elses', user: 'other-user', filename: 'x.xlsx' },
|
||||
]);
|
||||
const res = await request(buildApp()).get('/files/someone-elses/preview');
|
||||
expect(res.status).toBe(403);
|
||||
expect(mockFindFileById).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('returns status:pending without text/textFormat while the deferred render is in flight', async () => {
|
||||
mockGetFiles.mockResolvedValueOnce([
|
||||
{
|
||||
file_id: 'fid-pending',
|
||||
user: OWNER_USER_ID,
|
||||
filename: 'data.xlsx',
|
||||
status: 'pending',
|
||||
},
|
||||
]);
|
||||
const res = await request(buildApp()).get('/files/fid-pending/preview');
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toEqual({ file_id: 'fid-pending', status: 'pending' });
|
||||
/* Pending must NOT leak `text` and must NOT trigger the text re-fetch. */
|
||||
expect(res.body).not.toHaveProperty('text');
|
||||
expect(mockFindFileById).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('returns status:ready with text + textFormat when the deferred render succeeded', async () => {
|
||||
mockGetFiles.mockResolvedValueOnce([
|
||||
{ file_id: 'fid-ready', user: OWNER_USER_ID, filename: 'data.xlsx', status: 'ready' },
|
||||
]);
|
||||
/* Text is fetched only on the terminal ready response. */
|
||||
mockFindFileById.mockResolvedValueOnce({
|
||||
file_id: 'fid-ready',
|
||||
text: '<table><tr><td>1</td></tr></table>',
|
||||
textFormat: 'html',
|
||||
});
|
||||
const res = await request(buildApp()).get('/files/fid-ready/preview');
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toEqual({
|
||||
file_id: 'fid-ready',
|
||||
status: 'ready',
|
||||
text: '<table><tr><td>1</td></tr></table>',
|
||||
textFormat: 'html',
|
||||
});
|
||||
});
|
||||
|
||||
it('returns status:failed with previewError when the deferred render errored', async () => {
|
||||
mockGetFiles.mockResolvedValueOnce([
|
||||
{
|
||||
file_id: 'fid-failed',
|
||||
user: OWNER_USER_ID,
|
||||
filename: 'data.xlsx',
|
||||
status: 'failed',
|
||||
previewError: 'parser-error',
|
||||
},
|
||||
]);
|
||||
const res = await request(buildApp()).get('/files/fid-failed/preview');
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toEqual({
|
||||
file_id: 'fid-failed',
|
||||
status: 'failed',
|
||||
previewError: 'parser-error',
|
||||
});
|
||||
expect(mockFindFileById).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('defaults to status:ready for legacy records with no status field (back-compat)', async () => {
|
||||
mockGetFiles.mockResolvedValueOnce([
|
||||
{
|
||||
file_id: 'fid-legacy',
|
||||
user: OWNER_USER_ID,
|
||||
filename: 'old.csv',
|
||||
// status intentionally absent
|
||||
},
|
||||
]);
|
||||
mockFindFileById.mockResolvedValueOnce({
|
||||
file_id: 'fid-legacy',
|
||||
text: 'csv,header\n1,2',
|
||||
textFormat: 'text',
|
||||
});
|
||||
const res = await request(buildApp()).get('/files/fid-legacy/preview');
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toEqual({
|
||||
file_id: 'fid-legacy',
|
||||
status: 'ready',
|
||||
text: 'csv,header\n1,2',
|
||||
textFormat: 'text',
|
||||
});
|
||||
});
|
||||
|
||||
it('returns status:ready with no text when the record is ready but text is null (binary/oversized)', async () => {
|
||||
mockGetFiles.mockResolvedValueOnce([
|
||||
{ file_id: 'fid-binary', user: OWNER_USER_ID, filename: 'image.bin' },
|
||||
]);
|
||||
mockFindFileById.mockResolvedValueOnce({
|
||||
file_id: 'fid-binary',
|
||||
text: null,
|
||||
textFormat: null,
|
||||
});
|
||||
const res = await request(buildApp()).get('/files/fid-binary/preview');
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toEqual({ file_id: 'fid-binary', status: 'ready' });
|
||||
});
|
||||
|
||||
it('returns ready with no text when ready record was deleted between fileAccess and text fetch', async () => {
|
||||
/* `fileAccess` saw the record but the concurrent delete removed it
|
||||
* before the text fetch. Surface ready-without-text rather than
|
||||
* 500 — the client routes to download-only and stops polling. */
|
||||
mockGetFiles.mockResolvedValueOnce([
|
||||
{ file_id: 'fid-race', user: OWNER_USER_ID, filename: 'data.xlsx', status: 'ready' },
|
||||
]);
|
||||
mockFindFileById.mockResolvedValueOnce(null);
|
||||
const res = await request(buildApp()).get('/files/fid-race/preview');
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toEqual({ file_id: 'fid-race', status: 'ready' });
|
||||
});
|
||||
|
||||
it('returns 500 with a stable shape if the text fetch throws unexpectedly', async () => {
|
||||
mockGetFiles.mockResolvedValueOnce([
|
||||
{ file_id: 'fid-boom', user: OWNER_USER_ID, filename: 'data.xlsx', status: 'ready' },
|
||||
]);
|
||||
mockFindFileById.mockRejectedValueOnce(new Error('mongo down'));
|
||||
const res = await request(buildApp()).get('/files/fid-boom/preview');
|
||||
expect(res.status).toBe(500);
|
||||
expect(res.body).toMatchObject({ error: 'Internal Server Error' });
|
||||
});
|
||||
|
||||
describe('lazy sweep for stale pending records', () => {
|
||||
/* The boot-time `sweepOrphanedPreviews` only runs once at startup
|
||||
* with a 5-min cutoff. A backend crash + quick restart can leave
|
||||
* `pending` records younger than 5 min that never get touched
|
||||
* again. This endpoint sweeps them on the spot whenever a polling
|
||||
* request lands on one — the user is exactly the consumer who
|
||||
* cares, so on-demand sweep is the right shape. (Codex P2 review
|
||||
* on PR #12957.) */
|
||||
const STALE_MS = 6 * 60 * 1000;
|
||||
const FRESH_MS = 30 * 1000;
|
||||
|
||||
it('marks a stale pending record as failed:orphaned and returns the swept state', async () => {
|
||||
const updatedAt = new Date(Date.now() - STALE_MS);
|
||||
mockGetFiles.mockResolvedValueOnce([
|
||||
{
|
||||
file_id: 'fid-stale',
|
||||
user: OWNER_USER_ID,
|
||||
filename: 'data.xlsx',
|
||||
status: 'pending',
|
||||
updatedAt,
|
||||
},
|
||||
]);
|
||||
mockUpdateFile.mockResolvedValueOnce({
|
||||
file_id: 'fid-stale',
|
||||
status: 'failed',
|
||||
previewError: 'orphaned',
|
||||
});
|
||||
|
||||
const res = await request(buildApp()).get('/files/fid-stale/preview');
|
||||
|
||||
expect(mockUpdateFile).toHaveBeenCalledWith(
|
||||
{ file_id: 'fid-stale', status: 'failed', previewError: 'orphaned' },
|
||||
{ status: 'pending', updatedAt },
|
||||
);
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toEqual({
|
||||
file_id: 'fid-stale',
|
||||
status: 'failed',
|
||||
previewError: 'orphaned',
|
||||
});
|
||||
});
|
||||
|
||||
it('does NOT sweep a fresh pending record (within the cutoff window)', async () => {
|
||||
mockGetFiles.mockResolvedValueOnce([
|
||||
{
|
||||
file_id: 'fid-fresh',
|
||||
user: OWNER_USER_ID,
|
||||
filename: 'data.xlsx',
|
||||
status: 'pending',
|
||||
updatedAt: new Date(Date.now() - FRESH_MS),
|
||||
},
|
||||
]);
|
||||
|
||||
const res = await request(buildApp()).get('/files/fid-fresh/preview');
|
||||
|
||||
expect(mockUpdateFile).not.toHaveBeenCalled();
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toEqual({ file_id: 'fid-fresh', status: 'pending' });
|
||||
});
|
||||
|
||||
it('sweeps a record past the 2min cutoff but below the 5min boot-sweep threshold', async () => {
|
||||
/* Pins the cutoff change from 5min to 2min — without this, a
|
||||
* future revert wouldn't fail the suite. */
|
||||
const updatedAt = new Date(Date.now() - 3 * 60 * 1000);
|
||||
mockGetFiles.mockResolvedValueOnce([
|
||||
{
|
||||
file_id: 'fid-mid',
|
||||
user: OWNER_USER_ID,
|
||||
filename: 'data.xlsx',
|
||||
status: 'pending',
|
||||
updatedAt,
|
||||
},
|
||||
]);
|
||||
mockUpdateFile.mockResolvedValueOnce({
|
||||
file_id: 'fid-mid',
|
||||
status: 'failed',
|
||||
previewError: 'orphaned',
|
||||
});
|
||||
|
||||
const res = await request(buildApp()).get('/files/fid-mid/preview');
|
||||
|
||||
expect(mockUpdateFile).toHaveBeenCalled();
|
||||
expect(res.body).toEqual({
|
||||
file_id: 'fid-mid',
|
||||
status: 'failed',
|
||||
previewError: 'orphaned',
|
||||
});
|
||||
});
|
||||
|
||||
it('does NOT sweep a stale ready record (only pending qualifies)', async () => {
|
||||
mockGetFiles.mockResolvedValueOnce([
|
||||
{
|
||||
file_id: 'fid-ready',
|
||||
user: OWNER_USER_ID,
|
||||
filename: 'data.xlsx',
|
||||
status: 'ready',
|
||||
updatedAt: new Date(Date.now() - STALE_MS),
|
||||
},
|
||||
]);
|
||||
mockFindFileById.mockResolvedValueOnce({
|
||||
file_id: 'fid-ready',
|
||||
text: 'final',
|
||||
textFormat: 'html',
|
||||
});
|
||||
|
||||
const res = await request(buildApp()).get('/files/fid-ready/preview');
|
||||
|
||||
expect(mockUpdateFile).not.toHaveBeenCalled();
|
||||
expect(res.body).toMatchObject({ status: 'ready', text: 'final' });
|
||||
});
|
||||
|
||||
it('falls through to the original pending payload if the conditional sweep loses the race', async () => {
|
||||
const updatedAt = new Date(Date.now() - STALE_MS);
|
||||
mockGetFiles.mockResolvedValueOnce([
|
||||
{
|
||||
file_id: 'fid-race',
|
||||
user: OWNER_USER_ID,
|
||||
filename: 'data.xlsx',
|
||||
status: 'pending',
|
||||
updatedAt,
|
||||
},
|
||||
]);
|
||||
mockUpdateFile.mockResolvedValueOnce(null);
|
||||
|
||||
const res = await request(buildApp()).get('/files/fid-race/preview');
|
||||
|
||||
expect(mockUpdateFile).toHaveBeenCalled();
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toEqual({ file_id: 'fid-race', status: 'pending' });
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -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 deferred preview 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,235 @@ 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 the deferred preview rendering (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 PREVIEW_FINALIZE_TIMEOUT_MS = 60_000;
|
||||
|
||||
/**
|
||||
* Render the inline HTML preview for a code-execution file (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 the `attachment` update SSE event (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 full downloaded file contents,
|
||||
* bounded by the server's `fileSizeLimit` config (defaults far above
|
||||
* the 1MB extractor cap). The buffer is captured by the closure
|
||||
* returned in `{ finalize }`, so when many office files queue behind
|
||||
* the inner concurrency limiter (cap 2), all queued buffers stay
|
||||
* resident until each one's slot frees. For a tool result emitting
|
||||
* N office files, peak heap usage from this path is up to
|
||||
* `N * fileSizeLimit`. Acceptable for typical agent runs (a handful
|
||||
* of files at a few hundred KB each); pathological cases are bounded
|
||||
* by the inner per-file 12s timeout and the outer 60s render cap.
|
||||
* @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.
|
||||
* @param {string} [params.previewRevision] - Generation marker stamped
|
||||
* by the immediate persist step. The DB commit is conditional on
|
||||
* this — if a newer emit (cross-turn filename reuse) has rotated
|
||||
* the revision before this render finishes, `updateFile` returns
|
||||
* null and the stale render is silently discarded rather than
|
||||
* overwriting the newer record.
|
||||
* @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) or if the
|
||||
* `previewRevision` guard rejected the write.
|
||||
*/
|
||||
const finalizePreview = async ({
|
||||
buffer,
|
||||
leafName,
|
||||
mimeType,
|
||||
category,
|
||||
file_id,
|
||||
previewRevision,
|
||||
}) => {
|
||||
let text = null;
|
||||
let previewError;
|
||||
try {
|
||||
text = await withTimeout(
|
||||
extractCodeArtifactText(buffer, leafName, mimeType, category),
|
||||
PREVIEW_FINALIZE_TIMEOUT_MS,
|
||||
`Preview extraction exceeded ${PREVIEW_FINALIZE_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 ${PREVIEW_FINALIZE_TIMEOUT_MS}ms`,
|
||||
);
|
||||
}
|
||||
/* HTML-or-null contract (PR #12934): null result on an office file
|
||||
* must NOT fall back to plain text — surface as failed. Caller gates
|
||||
* on `hasOfficeHtmlPath`, so reaching here always means office. */
|
||||
const textFormat = getExtractedTextFormat(leafName, mimeType, text);
|
||||
const failed = text == null;
|
||||
const status = failed ? 'failed' : 'ready';
|
||||
if (failed && !previewError) {
|
||||
previewError = 'parser-error';
|
||||
}
|
||||
try {
|
||||
/* Conditional update: commit only if `previewRevision` still
|
||||
* matches what the immediate persist step stamped. If a newer
|
||||
* emit has rotated the revision (cross-turn filename reuse),
|
||||
* `updateFile` returns null and the stale render is silently
|
||||
* discarded. (Codex P1 review on PR #12957.) */
|
||||
const updated = await updateFile(
|
||||
{
|
||||
file_id,
|
||||
text,
|
||||
textFormat,
|
||||
status,
|
||||
previewError: failed ? previewError : null,
|
||||
},
|
||||
previewRevision ? { previewRevision } : undefined,
|
||||
);
|
||||
if (!updated && previewRevision) {
|
||||
logger.debug(
|
||||
`[finalizePreview] ${file_id}: stale render skipped — newer emit has superseded revision ${previewRevision}`,
|
||||
);
|
||||
}
|
||||
return updated;
|
||||
} catch (error) {
|
||||
logger.error(
|
||||
`[finalizePreview] ${file_id}: failed to persist preview result: ${error?.message ?? error}`,
|
||||
);
|
||||
return null;
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Run the background `finalize` thunk returned by `processCodeOutput`
|
||||
* and route the resolved record to the caller's emit logic. Shared
|
||||
* between `callbacks.js` (chat-completions + Open Responses) and
|
||||
* `tools.js` (direct tool endpoint) so the fire-and-forget pattern
|
||||
* doesn't drift across callsites.
|
||||
*
|
||||
* `onResolved` receives the post-update DB record and is the only piece
|
||||
* that varies — chat-completions writes the legacy `attachment` SSE
|
||||
* event, Open Responses writes the spec-shaped `librechat:attachment`
|
||||
* event with a sequence number, and the direct tool endpoint has no
|
||||
* stream to write to (caller passes a no-op).
|
||||
*
|
||||
* The catch path is the safety net for unexpected programming errors
|
||||
* inside `finalizePreview` ONLY. The function is designed to never
|
||||
* throw (extraction and DB failures are translated to `status: 'failed'`
|
||||
* inside it), but a ref error or future regression would otherwise
|
||||
* leave the DB record stuck at `'pending'` until the boot-time orphan
|
||||
* sweep — potentially hours away on a stable server. We attempt a
|
||||
* best-effort `updateFile` to mark the record `'failed'` with
|
||||
* `previewError: 'unexpected'` so the UI stops polling and the
|
||||
* next-turn LLM context surfaces the failure.
|
||||
*
|
||||
* `onResolved` errors are deliberately isolated in their own try/catch.
|
||||
* Without that isolation, a transient transport-side failure (SSE write
|
||||
* race after the stream closed, an emitter listener throwing) would
|
||||
* propagate into the finalize catch and downgrade an *already-resolved*
|
||||
* record to `failed` with `previewError: 'unexpected'` — surfacing
|
||||
* "preview unavailable" in the UI even though extraction succeeded
|
||||
* and the file is on disk. The emit failure is logged but the DB
|
||||
* record stays at whatever `finalizePreview` wrote (typically
|
||||
* `'ready'`), so the polling layer / next page load still sees the
|
||||
* resolved preview.
|
||||
*
|
||||
* @param {object} params
|
||||
* @param {(() => Promise<object | null>) | undefined} params.finalize - The
|
||||
* thunk returned by `processCodeOutput`. No-op when undefined.
|
||||
* @param {string | undefined} params.fileId - DB key for the failure
|
||||
* marker; if absent the catch only logs.
|
||||
* @param {string | undefined} [params.previewRevision] - Generation
|
||||
* marker stamped by the immediate persist step. The defensive
|
||||
* `updateFile` in the catch is conditional on this — if a newer
|
||||
* emit has rotated the revision, the stale failure marker is
|
||||
* silently discarded so a programming error from an older render
|
||||
* doesn't override a newer turn's record.
|
||||
* @param {(updated: object) => void} [params.onResolved] - Called once
|
||||
* on success with the post-update record.
|
||||
*/
|
||||
const runPreviewFinalize = ({ finalize, fileId, previewRevision, onResolved }) => {
|
||||
if (typeof finalize !== 'function') {
|
||||
return;
|
||||
}
|
||||
finalize()
|
||||
.then((updated) => {
|
||||
if (!updated || !onResolved) {
|
||||
return;
|
||||
}
|
||||
/* Isolated try/catch — a throw inside `onResolved` (transport-side
|
||||
* SSE write race, emitter listener error) MUST NOT propagate to
|
||||
* the outer `.catch`, which would downgrade an already-resolved
|
||||
* record to `failed` with `previewError: 'unexpected'`.
|
||||
* Extraction succeeded at this point and `finalizePreview` has
|
||||
* already persisted the terminal status; the polling layer / next
|
||||
* page load will surface the resolved preview even if this turn's
|
||||
* SSE emit didn't land. */
|
||||
try {
|
||||
onResolved(updated);
|
||||
} catch (emitError) {
|
||||
logger.error(
|
||||
`[runPreviewFinalize] onResolved threw for ${fileId}; record stays at the finalized status:`,
|
||||
emitError,
|
||||
);
|
||||
}
|
||||
})
|
||||
.catch((error) => {
|
||||
logger.error('Error rendering deferred preview:', error);
|
||||
if (!fileId) {
|
||||
return;
|
||||
}
|
||||
updateFile(
|
||||
{
|
||||
file_id: fileId,
|
||||
status: 'failed',
|
||||
previewError: 'unexpected',
|
||||
},
|
||||
previewRevision ? { previewRevision } : undefined,
|
||||
).catch((updateErr) => {
|
||||
logger.error(
|
||||
`[runPreviewFinalize] also failed to mark ${fileId} as failed after error:`,
|
||||
updateErr,
|
||||
);
|
||||
});
|
||||
});
|
||||
};
|
||||
|
||||
/**
|
||||
* 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`: persisted metadata (file is on disk, downloadable, and
|
||||
* has `status: 'pending'` if a preview is still being rendered).
|
||||
* - `finalize` (optional): a thunk returning the deferred preview
|
||||
* 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 +307,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 +352,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 +437,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 +445,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 +504,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 = {
|
||||
/* 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 +530,70 @@ 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) {
|
||||
/* Persist with `status: 'pending'` and explicit
|
||||
* `text: null` / `textFormat: null` so an update that previously
|
||||
* had cached text gets cleared. The deferred finalize transitions
|
||||
* to 'ready' (with text/textFormat) or 'failed' (with
|
||||
* previewError).
|
||||
*
|
||||
* `previewRevision` is a fresh UUID stamped on every emit. The
|
||||
* deferred finalize's `updateFile` is conditional on this — if
|
||||
* a newer turn (cross-turn filename reuse) has rotated the
|
||||
* revision before this render finishes, the stale render is
|
||||
* silently discarded rather than overwriting the newer record.
|
||||
* (Codex P1 review on PR #12957.) */
|
||||
const previewRevision = v4();
|
||||
const file = {
|
||||
...baseFile,
|
||||
text: null,
|
||||
textFormat: null,
|
||||
status: 'pending',
|
||||
previewError: null,
|
||||
previewRevision,
|
||||
};
|
||||
await createFile(file, true);
|
||||
return {
|
||||
file: Object.assign(file, { messageId, toolCallId }),
|
||||
finalize: () =>
|
||||
finalizePreview({ buffer, leafName, mimeType, category, file_id, previewRevision }),
|
||||
previewRevision,
|
||||
};
|
||||
}
|
||||
|
||||
/* 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,
|
||||
// Clear deferred-preview lifecycle fields in case the prior emit
|
||||
// at this (filename, conversationId) was an office file —
|
||||
// otherwise stale `pending`/`failed` would persist and the client
|
||||
// would render the wrong state for the now non-office artifact.
|
||||
status: null,
|
||||
previewError: null,
|
||||
previewRevision: 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 +606,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 +753,23 @@ const primeFiles = async (options) => {
|
|||
}
|
||||
|
||||
const entity_id = overrideEntityId ?? queryParams.entity_id;
|
||||
toolContext += `\n\t- /mnt/data/${file.filename}${fileSuffix}`;
|
||||
|
||||
/* Surface the preview lifecycle so the LLM knows when a
|
||||
* prior-turn artifact's rich preview didn't materialize. The
|
||||
* file blob is always available (`processCodeOutput` 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,
|
||||
|
|
@ -624,4 +928,5 @@ module.exports = {
|
|||
getSessionInfo,
|
||||
processCodeOutput,
|
||||
readSandboxFile,
|
||||
runPreviewFinalize,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -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 persist-then-render 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];
|
||||
|
|
@ -465,6 +476,31 @@ describe('Code Process', () => {
|
|||
expect(mockClassifyCodeArtifact).not.toHaveBeenCalled();
|
||||
expect(mockExtractCodeArtifactText).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('clears deferred-preview lifecycle fields so a prior office record at this file_id stops looking pending', async () => {
|
||||
/* Codex P2: same (filename, conversationId) was previously an
|
||||
* office artifact, leaving status/previewError/previewRevision
|
||||
* populated. The non-office update must reset them or the
|
||||
* client renders the wrong state for the now non-office file. */
|
||||
mockClaimCodeFile.mockResolvedValueOnce({
|
||||
file_id: 'reused-id',
|
||||
filename: 'output.txt',
|
||||
usage: 1,
|
||||
createdAt: '2024-01-01T00:00:00.000Z',
|
||||
});
|
||||
mockAxios.mockResolvedValue({ data: Buffer.from('hello') });
|
||||
determineFileType.mockResolvedValue({ mime: 'text/plain' });
|
||||
mockClassifyCodeArtifact.mockReturnValueOnce('text');
|
||||
mockHasOfficeHtmlPath.mockReturnValueOnce(false);
|
||||
mockExtractCodeArtifactText.mockResolvedValueOnce('hello');
|
||||
|
||||
await processCodeOutput({ ...baseParams, name: 'output.txt' });
|
||||
|
||||
const createCall = createFile.mock.calls[0][0];
|
||||
expect(createCall).toHaveProperty('status', null);
|
||||
expect(createCall).toHaveProperty('previewError', null);
|
||||
expect(createCall).toHaveProperty('previewRevision', null);
|
||||
});
|
||||
});
|
||||
|
||||
describe('file size limit enforcement', () => {
|
||||
|
|
@ -475,7 +511,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 +530,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 +542,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 +556,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 +570,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 +583,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 +594,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 +605,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 +614,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 +754,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 +825,329 @@ describe('Code Process', () => {
|
|||
expect(callConfig.httpsAgent.keepAlive).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('deferred-preview flow (office-bucket files)', () => {
|
||||
/* Office-bucket files (DOCX/XLSX/etc.) split into:
|
||||
* the initial emit (await): persist `text: null, status: 'pending'`,
|
||||
* return `{ file, finalize }` so the caller can ship the
|
||||
* attachment to the client immediately;
|
||||
* the deferred render (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 the deferred render.
|
||||
*
|
||||
* 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 the initial emit 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 deferred preview work.
|
||||
expect(mockExtractCodeArtifactText).not.toHaveBeenCalled();
|
||||
// Persisted record 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);
|
||||
/* Update is conditional on `previewRevision` so an older render
|
||||
* can't overwrite a newer turn's record on cross-turn filename
|
||||
* reuse. The uuid mock returns the same value for every v4()
|
||||
* call, so file_id and previewRevision happen to coincide here
|
||||
* — what matters is the second arg carries the revision filter. */
|
||||
expect(updateFile).toHaveBeenCalledWith(
|
||||
{
|
||||
file_id: 'mock-uuid-1234',
|
||||
text: '<table><tr><td>1</td></tr></table>',
|
||||
textFormat: 'html',
|
||||
status: 'ready',
|
||||
previewError: null,
|
||||
},
|
||||
{ previewRevision: 'mock-uuid-1234' },
|
||||
);
|
||||
});
|
||||
|
||||
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',
|
||||
}),
|
||||
{ previewRevision: 'mock-uuid-1234' },
|
||||
);
|
||||
});
|
||||
|
||||
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('Preview 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',
|
||||
}),
|
||||
{ previewRevision: 'mock-uuid-1234' },
|
||||
);
|
||||
});
|
||||
|
||||
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 preview 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('runPreviewFinalize', () => {
|
||||
/* The runtime pairing for `processCodeOutput`'s `finalize` thunk.
|
||||
* `finalizePreview` is designed to never throw (translates errors
|
||||
* to `status: 'failed'` internally). The helper's catch is the
|
||||
* safety net for unexpected programming errors that would
|
||||
* otherwise leave the DB record stuck at `status: 'pending'`
|
||||
* forever — we attempt a best-effort `updateFile` to mark it
|
||||
* `'failed'` with `previewError: 'unexpected'` so the UI stops
|
||||
* polling and the next-turn LLM context surfaces the failure.
|
||||
* (Codex audit on PR #12957 Finding 4.) */
|
||||
const { runPreviewFinalize } = require('./process');
|
||||
const { updateFile } = require('~/models');
|
||||
|
||||
beforeEach(() => {
|
||||
updateFile.mockReset();
|
||||
updateFile.mockResolvedValue({});
|
||||
});
|
||||
|
||||
it('is a no-op when finalize is undefined (non-office files)', () => {
|
||||
expect(() =>
|
||||
runPreviewFinalize({ finalize: undefined, fileId: 'fid-1', onResolved: jest.fn() }),
|
||||
).not.toThrow();
|
||||
expect(updateFile).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('calls onResolved with the resolved record on success', async () => {
|
||||
const onResolved = jest.fn();
|
||||
const finalize = jest
|
||||
.fn()
|
||||
.mockResolvedValue({ file_id: 'fid-1', status: 'ready', text: '<x/>' });
|
||||
runPreviewFinalize({ finalize, fileId: 'fid-1', onResolved });
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
expect(onResolved).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ file_id: 'fid-1', status: 'ready' }),
|
||||
);
|
||||
expect(updateFile).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('skips onResolved when finalize resolves to null (DB write failed inside finalizePreview)', async () => {
|
||||
const onResolved = jest.fn();
|
||||
const finalize = jest.fn().mockResolvedValue(null);
|
||||
runPreviewFinalize({ finalize, fileId: 'fid-1', onResolved });
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
expect(onResolved).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('marks the record as failed (previewError: "unexpected") when finalize throws', async () => {
|
||||
const onResolved = jest.fn();
|
||||
const finalize = jest.fn().mockRejectedValue(new Error('unexpected ref error'));
|
||||
runPreviewFinalize({
|
||||
finalize,
|
||||
fileId: 'fid-boom',
|
||||
previewRevision: 'rev-A',
|
||||
onResolved,
|
||||
});
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
expect(onResolved).not.toHaveBeenCalled();
|
||||
/* Defensive update is conditional on the same `previewRevision`
|
||||
* the deferred render started with — a newer turn that has
|
||||
* since rotated the revision is left untouched. */
|
||||
expect(updateFile).toHaveBeenCalledWith(
|
||||
{
|
||||
file_id: 'fid-boom',
|
||||
status: 'failed',
|
||||
previewError: 'unexpected',
|
||||
},
|
||||
{ previewRevision: 'rev-A' },
|
||||
);
|
||||
expect(logger.error).toHaveBeenCalledWith(
|
||||
'Error rendering deferred preview:',
|
||||
expect.any(Error),
|
||||
);
|
||||
});
|
||||
|
||||
it('logs but does not throw when the defensive updateFile itself fails', async () => {
|
||||
const onResolved = jest.fn();
|
||||
const finalize = jest.fn().mockRejectedValue(new Error('original error'));
|
||||
updateFile.mockRejectedValueOnce(new Error('mongo down'));
|
||||
runPreviewFinalize({ finalize, fileId: 'fid-doublefail', onResolved });
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
expect(onResolved).not.toHaveBeenCalled();
|
||||
// Two logger.error calls: one for the original throw, one for the failed mark.
|
||||
expect(logger.error.mock.calls.some((c) => /also failed to mark/.test(c[0]))).toBe(true);
|
||||
});
|
||||
|
||||
it('does not attempt the defensive updateFile when fileId is missing', async () => {
|
||||
const finalize = jest.fn().mockRejectedValue(new Error('boom'));
|
||||
runPreviewFinalize({ finalize, fileId: undefined });
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
expect(updateFile).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('skips onResolved gracefully when caller omits it (e.g., tools.js direct endpoint)', async () => {
|
||||
const finalize = jest.fn().mockResolvedValue({ file_id: 'fid-1', status: 'ready' });
|
||||
// No onResolved — non-streaming caller.
|
||||
expect(() => runPreviewFinalize({ finalize, fileId: 'fid-1' })).not.toThrow();
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
expect(updateFile).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does NOT downgrade the file to failed when finalize succeeds but onResolved throws', async () => {
|
||||
/* Regression for the codex P2 finding: the original chain put the
|
||||
* `.catch` after `.then(onResolved)`, so a throw inside
|
||||
* `onResolved` (transport-side: SSE write race after stream
|
||||
* close, an emitter listener throwing) propagated into the
|
||||
* finalize catch and persisted `status: 'failed'` /
|
||||
* `previewError: 'unexpected'` — even though extraction
|
||||
* succeeded and the file was already on disk and marked ready.
|
||||
* That surfaced "preview unavailable" in the UI for a perfectly
|
||||
* valid file, and degraded next-turn LLM context. The fix wraps
|
||||
* `onResolved` in its own try/catch so emit errors stay isolated
|
||||
* from finalize errors. */
|
||||
const onResolved = jest.fn(() => {
|
||||
throw new Error('SSE write after stream closed');
|
||||
});
|
||||
const finalize = jest.fn().mockResolvedValue({
|
||||
file_id: 'fid-emit-throw',
|
||||
status: 'ready',
|
||||
text: '<table>x</table>',
|
||||
});
|
||||
runPreviewFinalize({
|
||||
finalize,
|
||||
fileId: 'fid-emit-throw',
|
||||
previewRevision: 'rev-A',
|
||||
onResolved,
|
||||
});
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
expect(onResolved).toHaveBeenCalledTimes(1);
|
||||
/* The defensive "mark failed" path MUST NOT fire — the file is
|
||||
* resolved and on disk; only the SSE emit failed. */
|
||||
expect(updateFile).not.toHaveBeenCalled();
|
||||
/* Emit error is logged so the failure is observable in the
|
||||
* server log without affecting UX. */
|
||||
expect(
|
||||
logger.error.mock.calls.some((c) => /onResolved threw for fid-emit-throw/.test(c[0])),
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('readSandboxFile', () => {
|
||||
|
|
@ -1109,4 +1468,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 deferred-preview 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');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
import type { ReactNode } from 'react';
|
||||
import type { TFile } from 'librechat-data-provider';
|
||||
import type { ExtendedFile } from '~/common';
|
||||
import { getFileType, cn } from '~/utils';
|
||||
|
|
@ -8,6 +9,7 @@ const FileContainer = ({
|
|||
file,
|
||||
overrideType,
|
||||
displayName,
|
||||
subtitle,
|
||||
buttonClassName,
|
||||
containerClassName,
|
||||
onDelete,
|
||||
|
|
@ -21,6 +23,15 @@ const FileContainer = ({
|
|||
* persisted user files leave this undefined and render the raw filename.
|
||||
*/
|
||||
displayName?: string;
|
||||
/**
|
||||
* Optional override for the subtitle line (defaults to the file
|
||||
* type's localized title — e.g. "PowerPoint Presentation"). Used by
|
||||
* the deferred-preview flow to surface "Preparing preview…" /
|
||||
* "Preview unavailable" inline within the chip rather than as a
|
||||
* loose-feeling annotation below it. Pass a ReactNode so callers
|
||||
* can include icons (spinner, alert) alongside the text.
|
||||
*/
|
||||
subtitle?: ReactNode;
|
||||
buttonClassName?: string;
|
||||
containerClassName?: string;
|
||||
onDelete?: () => void;
|
||||
|
|
@ -49,9 +60,13 @@ const FileContainer = ({
|
|||
<div className="truncate font-medium" title={visibleName}>
|
||||
{visibleName}
|
||||
</div>
|
||||
<div className="truncate text-text-secondary" title={fileType.title}>
|
||||
{fileType.title}
|
||||
</div>
|
||||
{subtitle != null ? (
|
||||
subtitle
|
||||
) : (
|
||||
<div className="truncate text-text-secondary" title={fileType.title}>
|
||||
{fileType.title}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { memo, useEffect, useId, useLayoutEffect, useMemo, useRef, useState } from 'react';
|
||||
import { Loader2, AlertCircle, Download } from 'lucide-react';
|
||||
import { Tools } from 'librechat-data-provider';
|
||||
import type { TAttachment, TFile, TAttachmentMetadata } from 'librechat-data-provider';
|
||||
import type { ToolArtifactType } from '~/utils/artifacts';
|
||||
|
|
@ -12,17 +13,111 @@ import {
|
|||
isTextAttachment,
|
||||
renderAttachmentKey,
|
||||
} from './attachmentTypes';
|
||||
import FilePreview from '~/components/Chat/Input/Files/FilePreview';
|
||||
import FileContainer from '~/components/Chat/Input/Files/FileContainer';
|
||||
import { fileToArtifact, TOOL_ARTIFACT_TYPES } from '~/utils/artifacts';
|
||||
import Image from '~/components/Chat/Messages/Content/Image';
|
||||
import ToolMermaidArtifact from './ToolMermaidArtifact';
|
||||
import ToolArtifactCard from './ToolArtifactCard';
|
||||
import { useAttachmentLink } from './LogLink';
|
||||
import { useLocalize } from '~/hooks';
|
||||
import { cn } from '~/utils';
|
||||
import { useLocalize, useAttachmentPreviewSync } from '~/hooks';
|
||||
import { cn, getFileType } from '~/utils';
|
||||
|
||||
const COLLAPSED_MAX_HEIGHT = 320;
|
||||
|
||||
/**
|
||||
* Card-shaped placeholder for a code-execution office file whose
|
||||
* inline preview is still rendering (or failed). Visually mirrors
|
||||
* `ToolArtifactCard`'s chrome — same rounded card, split body +
|
||||
* download — so when the deferred render lands and the routing
|
||||
* upgrades to the real `PanelArtifact` card the user sees a smooth
|
||||
* transition between two card-shaped things, not a jarring jump from
|
||||
* a small file chip to a big artifact card.
|
||||
*
|
||||
* The body is non-interactive while pending (there's no panel to
|
||||
* open yet). On `'failed'` the body is also non-interactive — the
|
||||
* download button is the only meaningful action since extraction
|
||||
* never produced anything to render. Status reads via the spinner /
|
||||
* alert subtitle inside the card, mirroring `ToolArtifactCard`'s
|
||||
* "click to open" subtitle slot.
|
||||
*/
|
||||
const PreviewPlaceholderCard = memo(
|
||||
({
|
||||
attachment,
|
||||
status,
|
||||
previewError,
|
||||
}: {
|
||||
attachment: Partial<TAttachment>;
|
||||
status: 'pending' | 'failed';
|
||||
previewError?: string;
|
||||
}) => {
|
||||
const localize = useLocalize();
|
||||
const file = attachment as TFile & TAttachmentMetadata;
|
||||
const { handleDownload } = useAttachmentLink({
|
||||
href: attachment.filepath ?? '',
|
||||
filename: attachment.filename ?? '',
|
||||
file_id: file.file_id,
|
||||
user: file.user,
|
||||
source: file.source,
|
||||
});
|
||||
const fileType = getFileType('artifact');
|
||||
const visibleFilename = displayFilename(attachment.filename);
|
||||
const subtitleText =
|
||||
status === 'pending'
|
||||
? localize('com_ui_preview_preparing')
|
||||
: localize('com_ui_preview_failed');
|
||||
return (
|
||||
<div className="group relative my-2 inline-flex max-w-fit items-stretch gap-px overflow-hidden rounded-xl text-sm text-text-primary shadow-sm">
|
||||
<div
|
||||
aria-disabled="true"
|
||||
aria-busy={status === 'pending'}
|
||||
className="relative overflow-hidden rounded-l-xl border-border-light bg-surface-tertiary"
|
||||
title={status === 'failed' ? (previewError ?? subtitleText) : undefined}
|
||||
>
|
||||
<div className="w-fit p-2">
|
||||
<div className="flex flex-row items-center gap-2">
|
||||
{/* Don't pass `file` here — it triggers `SourceIcon`'s
|
||||
Terminal overlay for code-exec files (matches the
|
||||
`metadata.fileIdentifier` marker), which is the
|
||||
download-chip look. The artifact card doesn't show
|
||||
that overlay; the placeholder shouldn't either, so
|
||||
the pending→resolved transition is visually seamless. */}
|
||||
<FilePreview fileType={fileType} className="relative" />
|
||||
<div className="overflow-hidden text-left">
|
||||
<div className="truncate font-medium" title={visibleFilename}>
|
||||
{visibleFilename}
|
||||
</div>
|
||||
<div className="flex items-center gap-1.5 truncate text-xs text-text-secondary">
|
||||
{status === 'pending' ? (
|
||||
<Loader2 className="h-3 w-3 shrink-0 animate-spin" aria-hidden="true" />
|
||||
) : (
|
||||
<AlertCircle className="h-3 w-3 shrink-0" aria-hidden="true" />
|
||||
)}
|
||||
<span className="truncate">{subtitleText}</span>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
<button
|
||||
type="button"
|
||||
onClick={handleDownload}
|
||||
aria-label={`${localize('com_ui_download')} ${visibleFilename}`}
|
||||
title={localize('com_ui_download')}
|
||||
className={cn(
|
||||
'flex shrink-0 items-center justify-center px-3 transition-colors duration-200',
|
||||
'rounded-r-xl bg-surface-tertiary text-text-secondary hover:bg-surface-hover hover:text-text-primary',
|
||||
'border-l border-border-light focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-border-heavy',
|
||||
)}
|
||||
>
|
||||
<Download className="size-4" aria-hidden="true" />
|
||||
</button>
|
||||
</div>
|
||||
);
|
||||
},
|
||||
);
|
||||
PreviewPlaceholderCard.displayName = 'PreviewPlaceholderCard';
|
||||
|
||||
const FileAttachment = memo(({ attachment }: { attachment: Partial<TAttachment> }) => {
|
||||
const [isVisible, setIsVisible] = useState(false);
|
||||
const file = attachment as TFile & TAttachmentMetadata;
|
||||
|
|
@ -34,6 +129,13 @@ const FileAttachment = memo(({ attachment }: { attachment: Partial<TAttachment>
|
|||
source: file.source,
|
||||
});
|
||||
const extension = attachment.filename?.split('.').pop();
|
||||
/* Bridge the deferred-preview lifecycle: poll the backend for the
|
||||
* resolved record while the file is still pending. The hook is a
|
||||
* no-op for terminal states (legacy records, ready, failed
|
||||
* already-known) so calling it unconditionally is cheap. */
|
||||
const { status: previewStatus, previewError } = useAttachmentPreviewSync(
|
||||
attachment as TAttachment,
|
||||
);
|
||||
|
||||
useEffect(() => {
|
||||
const timer = setTimeout(() => setIsVisible(true), 50);
|
||||
|
|
@ -43,6 +145,33 @@ const FileAttachment = memo(({ attachment }: { attachment: Partial<TAttachment>
|
|||
if (!attachment.filepath) {
|
||||
return null;
|
||||
}
|
||||
/* Pending or failed: render the card-shaped placeholder rather than
|
||||
* the small file chip. Visual continuity with `ToolArtifactCard` so
|
||||
* when the deferred render lands and the routing upgrades to
|
||||
* `PanelArtifact`, the user sees a smooth card→card transition
|
||||
* instead of a jump from "file download" to "artifact card". */
|
||||
if (previewStatus === 'pending' || previewStatus === 'failed') {
|
||||
return (
|
||||
<div
|
||||
className={cn(
|
||||
'file-attachment-container',
|
||||
'transition-all duration-300 ease-out',
|
||||
isVisible ? 'translate-y-0 opacity-100' : 'translate-y-2 opacity-0',
|
||||
)}
|
||||
style={{
|
||||
transformOrigin: 'center top',
|
||||
willChange: 'opacity, transform',
|
||||
WebkitFontSmoothing: 'subpixel-antialiased',
|
||||
}}
|
||||
>
|
||||
<PreviewPlaceholderCard
|
||||
attachment={attachment}
|
||||
status={previewStatus}
|
||||
previewError={previewError}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
return (
|
||||
<div
|
||||
className={cn(
|
||||
|
|
@ -267,7 +396,11 @@ export function AttachmentGroup({ attachments }: { attachments?: TAttachment[] }
|
|||
const fileAttachments: TAttachment[] = [];
|
||||
const imageAttachments: TAttachment[] = [];
|
||||
const textAttachments: TAttachment[] = [];
|
||||
const panelArtifacts: Array<{ attachment: TAttachment; type: ToolArtifactType }> = [];
|
||||
/* Pending-preview chips share this row with their future selves —
|
||||
* `type` is null while pending so the renderer falls back to
|
||||
* FileAttachment (PreviewPlaceholderCard); on resolution it switches
|
||||
* to PanelArtifact in place. */
|
||||
const panelRow: Array<{ attachment: TAttachment; type: ToolArtifactType | null }> = [];
|
||||
const mermaidArtifacts: TAttachment[] = [];
|
||||
|
||||
attachments.forEach((attachment) => {
|
||||
|
|
@ -281,13 +414,17 @@ export function AttachmentGroup({ attachments }: { attachments?: TAttachment[] }
|
|||
imageAttachments.push(attachment);
|
||||
return;
|
||||
}
|
||||
if ((attachment as Partial<TFile>).status === 'pending') {
|
||||
panelRow.push({ attachment, type: null });
|
||||
return;
|
||||
}
|
||||
const artType = artifactTypeForAttachment(attachment);
|
||||
if (artType === TOOL_ARTIFACT_TYPES.MERMAID) {
|
||||
mermaidArtifacts.push(attachment);
|
||||
return;
|
||||
}
|
||||
if (artType != null) {
|
||||
panelArtifacts.push({ attachment, type: artType });
|
||||
panelRow.push({ attachment, type: artType });
|
||||
return;
|
||||
}
|
||||
if (isTextAttachment(attachment)) {
|
||||
|
|
@ -302,7 +439,13 @@ export function AttachmentGroup({ attachments }: { attachments?: TAttachment[] }
|
|||
// engines (V8 ≥ 7.0) so equal-weight entries keep their input order.
|
||||
fileAttachments.sort(bySalience);
|
||||
textAttachments.sort(bySalience);
|
||||
panelArtifacts.sort(byEntrySalience);
|
||||
/* Sort only the typed (resolved) entries; pending placeholders bubble
|
||||
* to the end of the row so resolved siblings catch the eye first. */
|
||||
const resolvedPanel = panelRow.filter(
|
||||
(e): e is { attachment: TAttachment; type: ToolArtifactType } => e.type != null,
|
||||
);
|
||||
const pendingPanel = panelRow.filter((e) => e.type == null);
|
||||
resolvedPanel.sort(byEntrySalience);
|
||||
mermaidArtifacts.sort(bySalience);
|
||||
imageAttachments.sort(bySalience);
|
||||
|
||||
|
|
@ -320,15 +463,23 @@ export function AttachmentGroup({ attachments }: { attachments?: TAttachment[] }
|
|||
)}
|
||||
</div>
|
||||
)}
|
||||
{panelArtifacts.length > 0 && (
|
||||
{(resolvedPanel.length > 0 || pendingPanel.length > 0) && (
|
||||
<div className="my-2 flex flex-wrap items-center gap-2">
|
||||
{panelArtifacts.map(({ attachment, type }, index) => (
|
||||
{resolvedPanel.map(({ attachment, type }, index) => (
|
||||
<PanelArtifact
|
||||
attachment={attachment}
|
||||
type={type}
|
||||
key={renderAttachmentKey('artifact', attachment, index)}
|
||||
/>
|
||||
))}
|
||||
{pendingPanel.map(({ attachment }, index) =>
|
||||
attachment.filepath ? (
|
||||
<FileAttachment
|
||||
attachment={attachment}
|
||||
key={renderAttachmentKey('pending', attachment, index)}
|
||||
/>
|
||||
) : null,
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
{mermaidArtifacts.length > 0 && (
|
||||
|
|
|
|||
|
|
@ -70,6 +70,8 @@ interface ToolArtifactCardProps {
|
|||
const ToolArtifactCard = memo(({ attachment, artifact }: ToolArtifactCardProps) => {
|
||||
const localize = useLocalize();
|
||||
const claimKey = useId();
|
||||
const file = attachment as TFile & TAttachmentMetadata;
|
||||
const fileId = file.file_id;
|
||||
const setVisible = useSetRecoilState(store.artifactsVisibility);
|
||||
const setArtifacts = useSetRecoilState(store.artifactsState);
|
||||
const setCurrentArtifactId = useSetRecoilState(store.currentArtifactId);
|
||||
|
|
@ -79,6 +81,22 @@ const ToolArtifactCard = memo(({ attachment, artifact }: ToolArtifactCardProps)
|
|||
const [claim, setClaim] = useRecoilState(store.toolArtifactClaim(artifact.id));
|
||||
const isSelected = artifact.id === currentArtifactId;
|
||||
const isMyClaim = claim === claimKey;
|
||||
/* Read+reset on mount only — `useRecoilCallback` avoids subscribing
|
||||
* to the per-file_id flag (no re-renders when other files resolve).
|
||||
* The deferred-preview hook flips this to `true` on the pending→ready
|
||||
* edge; we consume it once and reset, so repeat mounts (panel close
|
||||
* then reopen, history scroll) don't auto-open a second time. */
|
||||
const consumeJustResolved = useRecoilCallback(
|
||||
({ snapshot, reset }) =>
|
||||
(id: string) => {
|
||||
const flagged = snapshot.getLoadable(store.previewJustResolved(id)).valueMaybe() ?? false;
|
||||
if (flagged) {
|
||||
reset(store.previewJustResolved(id));
|
||||
}
|
||||
return flagged;
|
||||
},
|
||||
[],
|
||||
);
|
||||
/**
|
||||
* Captured at first render via a non-subscribing snapshot read so the
|
||||
* downstream effect doesn't re-fire (and the component doesn't
|
||||
|
|
@ -136,11 +154,6 @@ const ToolArtifactCard = memo(({ attachment, artifact }: ToolArtifactCardProps)
|
|||
}, [artifact, existingEntry, isMyClaim, setArtifacts]);
|
||||
|
||||
useEffect(() => {
|
||||
if (!mountedDuringStreamRef.current) {
|
||||
// Card mounted as part of conversation history — leave focus and
|
||||
// visibility alone so the side panel doesn't auto-open on navigation.
|
||||
return;
|
||||
}
|
||||
if (isCodeOnlyArtifact(artifact.type)) {
|
||||
// Source-code artifacts (`.py`, `.js`, `.cpp`, `Dockerfile`, …) are
|
||||
// click-to-open only. They're typically supporting scripts the
|
||||
|
|
@ -151,16 +164,33 @@ const ToolArtifactCard = memo(({ attachment, artifact }: ToolArtifactCardProps)
|
|||
// an HTML deliverable still surfaces immediately.
|
||||
return;
|
||||
}
|
||||
// Streaming arrival: focus the new artifact AND force the panel
|
||||
// visible. Without `setVisible(true)`, a session where the user had
|
||||
// previously closed the panel (visibility=false) would surface the
|
||||
// selection in the chip ("click to close") but never actually open
|
||||
// — `Presentation` gates rendering on visibility.
|
||||
/* Two paths qualify the card for auto-open:
|
||||
* 1. Streaming-time mount — ref captured `isSubmitting === true`
|
||||
* at first render. The card is part of the live response, so
|
||||
* the legacy "panel pops open as artifacts arrive" UX applies.
|
||||
* 2. Just-resolved deferred preview — `useAttachmentPreviewSync`
|
||||
* sets a one-shot flag on the pending→ready edge. The
|
||||
* deferred render can complete *after* the SSE stream closes,
|
||||
* so checking only `isSubmitting` would miss this case (the
|
||||
* chip would render in place but never auto-open). Consuming
|
||||
* the flag also resets it, so subsequent re-mounts (panel
|
||||
* close/reopen, history scroll) do not re-steal focus.
|
||||
* History mounts (file already resolved on page load) hit neither
|
||||
* path, so the panel stays closed on navigation — no jarring
|
||||
* auto-open just from scrolling past an old artifact. */
|
||||
const justResolved = fileId ? consumeJustResolved(fileId) : false;
|
||||
if (!mountedDuringStreamRef.current && !justResolved) {
|
||||
return;
|
||||
}
|
||||
// Streaming arrival or just-resolved preview: focus the new artifact
|
||||
// AND force the panel visible. Without `setVisible(true)`, a session
|
||||
// where the user had previously closed the panel (visibility=false)
|
||||
// would surface the selection in the chip ("click to close") but
|
||||
// never actually open — `Presentation` gates rendering on visibility.
|
||||
setCurrentArtifactId(artifact.id);
|
||||
setVisible(true);
|
||||
}, [artifact.id, artifact.type, setCurrentArtifactId, setVisible]);
|
||||
}, [artifact.id, artifact.type, fileId, consumeJustResolved, setCurrentArtifactId, setVisible]);
|
||||
|
||||
const file = attachment as TFile & TAttachmentMetadata;
|
||||
const { handleDownload } = useAttachmentLink({
|
||||
href: attachment.filepath ?? '',
|
||||
filename: attachment.filename ?? '',
|
||||
|
|
|
|||
|
|
@ -11,6 +11,11 @@ jest.mock('~/hooks', () => ({
|
|||
() =>
|
||||
(key: string): string =>
|
||||
key,
|
||||
/* `FileAttachment` calls this hook unconditionally to bridge the
|
||||
* deferred-preview lifecycle into the attachment cache. The
|
||||
* routing tests don't exercise the preview flow itself — stub it
|
||||
* to a no-op so it doesn't blow up jsdom rendering. */
|
||||
useAttachmentPreviewSync: () => ({ status: 'ready', previewError: undefined, isPolling: false }),
|
||||
}));
|
||||
|
||||
jest.mock('../LogLink', () => ({
|
||||
|
|
@ -521,6 +526,106 @@ describe('ToolArtifactCard click behaviour', () => {
|
|||
expect(snapshot.currentArtifactId).toBeNull();
|
||||
});
|
||||
|
||||
it('auto-opens a non-streaming card when the deferred preview just resolved', () => {
|
||||
/* Regression for the deferred-preview UX gap: when an office file's
|
||||
* background HTML extraction lands AFTER the SSE stream has closed
|
||||
* (`isSubmitting=false`), the freshly resolved chip would render in
|
||||
* place but never auto-open the panel — the legacy auto-open path
|
||||
* is gated only on streaming. `useAttachmentPreviewSync` flips the
|
||||
* `previewJustResolved(file_id)` flag on the pending→ready edge to
|
||||
* bridge that gap; `ToolArtifactCard` consumes it on mount and
|
||||
* auto-opens regardless of submission state. The flag is one-shot:
|
||||
* a subsequent re-mount (panel close/reopen, history scroll) must
|
||||
* NOT fire again — covered by the next test. */
|
||||
const xlsx = baseAttachment({
|
||||
file_id: 'just-resolved-xlsx',
|
||||
filename: 'data.xlsx',
|
||||
type: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
|
||||
text: '<table>resolved</table>',
|
||||
textFormat: 'html',
|
||||
} as Partial<TAttachment>);
|
||||
const initializeState = (snap: MutableSnapshot) => {
|
||||
snap.set(store.isSubmittingFamily(0), false);
|
||||
snap.set(store.artifactsVisibility, false);
|
||||
snap.set(store.previewJustResolved('just-resolved-xlsx'), true);
|
||||
};
|
||||
let snapshot: ArtifactsSnapshot = {
|
||||
visibility: false,
|
||||
currentArtifactId: null,
|
||||
artifactIds: [],
|
||||
};
|
||||
render(
|
||||
<RecoilRoot initializeState={initializeState}>
|
||||
<StateProbe
|
||||
onSnapshot={(snap) => {
|
||||
snapshot = snap;
|
||||
}}
|
||||
/>
|
||||
<Attachment attachment={xlsx} />
|
||||
</RecoilRoot>,
|
||||
);
|
||||
expect(snapshot.currentArtifactId).toBe('tool-artifact-just-resolved-xlsx');
|
||||
expect(snapshot.visibility).toBe(true);
|
||||
});
|
||||
|
||||
it('does NOT re-auto-open on a second mount after the just-resolved flag is consumed', () => {
|
||||
/* The flag is one-shot — first card to mount consumes it. A second
|
||||
* mount of the same file_id (panel close + reopen, history scroll
|
||||
* onto the same card) must NOT re-steal focus, otherwise the user
|
||||
* could never close the panel without it popping back open. */
|
||||
const xlsx = baseAttachment({
|
||||
file_id: 'one-shot-xlsx',
|
||||
filename: 'data.xlsx',
|
||||
type: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
|
||||
text: '<table>resolved</table>',
|
||||
textFormat: 'html',
|
||||
} as Partial<TAttachment>);
|
||||
const initializeState = (snap: MutableSnapshot) => {
|
||||
snap.set(store.isSubmittingFamily(0), false);
|
||||
snap.set(store.artifactsVisibility, false);
|
||||
snap.set(store.previewJustResolved('one-shot-xlsx'), true);
|
||||
};
|
||||
let snapshot: ArtifactsSnapshot = {
|
||||
visibility: false,
|
||||
currentArtifactId: null,
|
||||
artifactIds: [],
|
||||
};
|
||||
const { unmount } = render(
|
||||
<RecoilRoot initializeState={initializeState}>
|
||||
<StateProbe
|
||||
onSnapshot={(snap) => {
|
||||
snapshot = snap;
|
||||
}}
|
||||
/>
|
||||
<Attachment attachment={xlsx} />
|
||||
</RecoilRoot>,
|
||||
);
|
||||
/* First mount auto-opened. Now simulate a fresh Recoil tree with the
|
||||
* flag in the post-consume state (false) and assert the second
|
||||
* mount stays closed. We use a fresh RecoilRoot to mirror what a
|
||||
* real "panel was closed and the user then revealed the chip
|
||||
* again" pathway would look like at the state level. */
|
||||
unmount();
|
||||
snapshot = { visibility: false, currentArtifactId: null, artifactIds: [] };
|
||||
const secondInit = (snap: MutableSnapshot) => {
|
||||
snap.set(store.isSubmittingFamily(0), false);
|
||||
snap.set(store.artifactsVisibility, false);
|
||||
// flag stays at default (false) — already consumed
|
||||
};
|
||||
render(
|
||||
<RecoilRoot initializeState={secondInit}>
|
||||
<StateProbe
|
||||
onSnapshot={(snap) => {
|
||||
snapshot = snap;
|
||||
}}
|
||||
/>
|
||||
<Attachment attachment={xlsx} />
|
||||
</RecoilRoot>,
|
||||
);
|
||||
expect(snapshot.currentArtifactId).toBeNull();
|
||||
expect(snapshot.visibility).toBe(false);
|
||||
});
|
||||
|
||||
it('clicking a CODE artifact focuses it even though it skipped auto-open', () => {
|
||||
// Counterpart to the streaming-CODE no-auto-open test: confirm the
|
||||
// click path still surfaces a `.py` chip in the panel. Even on a
|
||||
|
|
@ -656,6 +761,40 @@ describe('AttachmentGroup routing', () => {
|
|||
expect(chip?.textContent).toBe('.config.zip');
|
||||
});
|
||||
|
||||
it('renders pending-preview chips in the panel-artifact row alongside resolved siblings', () => {
|
||||
/* A pending preview is a future panel artifact — render it in the
|
||||
* same row so when it resolves the chip stays put instead of
|
||||
* jumping between rows. Plain files keep their own row. */
|
||||
const attachments = [
|
||||
baseAttachment({
|
||||
file_id: 'resolved',
|
||||
filename: 'index.html',
|
||||
text: '<h1>hi</h1>',
|
||||
} as Partial<TAttachment>),
|
||||
baseAttachment({
|
||||
file_id: 'pending-1',
|
||||
filename: 'data.xlsx',
|
||||
type: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
|
||||
status: 'pending',
|
||||
} as Partial<TAttachment>),
|
||||
baseAttachment({
|
||||
file_id: 'plain',
|
||||
filename: 'archive.zip',
|
||||
text: undefined as unknown as string,
|
||||
} as Partial<TAttachment>),
|
||||
] as TAttachment[];
|
||||
|
||||
const { container } = renderWith(<AttachmentGroup attachments={attachments} />);
|
||||
|
||||
/* Two rows: file row (plain.zip) + panel row (resolved + pending). */
|
||||
const rows = container.querySelectorAll('div.flex.flex-wrap');
|
||||
expect(rows.length).toBe(2);
|
||||
/* Resolved artifact card title visible. */
|
||||
expect(screen.getByText('index.html')).toBeInTheDocument();
|
||||
/* Pending placeholder is a FileContainer rendering. */
|
||||
expect(screen.getAllByTestId('file-container').length).toBeGreaterThanOrEqual(1);
|
||||
});
|
||||
|
||||
it('renders separate buckets for panel artifacts, mermaid, text, and plain files', () => {
|
||||
const attachments = [
|
||||
baseAttachment({
|
||||
|
|
|
|||
|
|
@ -13,6 +13,10 @@ jest.mock('~/hooks', () => ({
|
|||
};
|
||||
return translations[key] ?? key;
|
||||
},
|
||||
/* `FileAttachment` calls this hook unconditionally to bridge the
|
||||
* deferred-preview lifecycle. Stub to a no-op for tests that
|
||||
* don't exercise the preview flow. */
|
||||
useAttachmentPreviewSync: () => ({ status: 'ready', previewError: undefined, isPolling: false }),
|
||||
}));
|
||||
|
||||
const mockHandleDownload = jest.fn();
|
||||
|
|
|
|||
|
|
@ -0,0 +1,97 @@
|
|||
import type { TFilePreview } from 'librechat-data-provider';
|
||||
|
||||
const mockGetFilePreview = jest.fn();
|
||||
jest.mock('librechat-data-provider', () => {
|
||||
const actual = jest.requireActual('librechat-data-provider');
|
||||
return {
|
||||
...actual,
|
||||
dataService: {
|
||||
...actual.dataService,
|
||||
getFilePreview: (...args: unknown[]) => mockGetFilePreview(...args),
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
import {
|
||||
PREVIEW_MAX_CONSECUTIVE_ERRORS,
|
||||
_resetPreviewErrorCounter,
|
||||
fetchFilePreview,
|
||||
previewRefetchInterval,
|
||||
} from '../queries';
|
||||
|
||||
const q = (fileId: string) => ({ queryKey: ['filePreview' as const, fileId] });
|
||||
const FID = 'fid-test';
|
||||
|
||||
beforeEach(() => {
|
||||
_resetPreviewErrorCounter();
|
||||
mockGetFilePreview.mockReset();
|
||||
});
|
||||
|
||||
describe('previewRefetchInterval', () => {
|
||||
it('polls every 2.5s when no data has arrived yet', () => {
|
||||
expect(previewRefetchInterval(undefined, q(FID))).toBe(2500);
|
||||
});
|
||||
|
||||
it('keeps polling while server reports pending', () => {
|
||||
expect(previewRefetchInterval({ file_id: FID, status: 'pending' }, q(FID))).toBe(2500);
|
||||
});
|
||||
|
||||
it('stops on terminal ready', () => {
|
||||
const data: TFilePreview = {
|
||||
file_id: FID,
|
||||
status: 'ready',
|
||||
text: 'x',
|
||||
textFormat: 'html',
|
||||
};
|
||||
expect(previewRefetchInterval(data, q(FID))).toBe(false);
|
||||
});
|
||||
|
||||
it('stops on terminal failed', () => {
|
||||
const data: TFilePreview = { file_id: FID, status: 'failed', previewError: 'oops' };
|
||||
expect(previewRefetchInterval(data, q(FID))).toBe(false);
|
||||
});
|
||||
|
||||
it('keeps polling while consecutive errors are below the cap', async () => {
|
||||
/* Regression for the bug where the counter relied on
|
||||
* `query.state.fetchFailureCount`, which React Query v4 resets to
|
||||
* 0 on every fetch dispatch — so the cap never fired and a broken
|
||||
* endpoint polled forever. The counter now lives outside query
|
||||
* state, incremented in the fetch wrapper. */
|
||||
mockGetFilePreview.mockRejectedValue(new Error('500'));
|
||||
for (let i = 0; i < PREVIEW_MAX_CONSECUTIVE_ERRORS - 1; i++) {
|
||||
await expect(fetchFilePreview(FID)).rejects.toThrow();
|
||||
expect(previewRefetchInterval(undefined, q(FID))).toBe(2500);
|
||||
}
|
||||
});
|
||||
|
||||
it('caps polling after MAX_CONSECUTIVE_ERRORS errors', async () => {
|
||||
mockGetFilePreview.mockRejectedValue(new Error('500'));
|
||||
for (let i = 0; i < PREVIEW_MAX_CONSECUTIVE_ERRORS; i++) {
|
||||
await expect(fetchFilePreview(FID)).rejects.toThrow();
|
||||
}
|
||||
expect(previewRefetchInterval(undefined, q(FID))).toBe(false);
|
||||
});
|
||||
|
||||
it('resets the error counter on a successful poll', async () => {
|
||||
mockGetFilePreview.mockRejectedValueOnce(new Error('500'));
|
||||
await expect(fetchFilePreview(FID)).rejects.toThrow();
|
||||
mockGetFilePreview.mockResolvedValueOnce({ file_id: FID, status: 'pending' });
|
||||
await fetchFilePreview(FID);
|
||||
/* After a success, even an immediate cap-worth of new errors should
|
||||
* be allowed before stopping again. */
|
||||
mockGetFilePreview.mockRejectedValue(new Error('500'));
|
||||
for (let i = 0; i < PREVIEW_MAX_CONSECUTIVE_ERRORS - 1; i++) {
|
||||
await expect(fetchFilePreview(FID)).rejects.toThrow();
|
||||
}
|
||||
expect(previewRefetchInterval(undefined, q(FID))).toBe(2500);
|
||||
});
|
||||
|
||||
it('tracks counters per file_id (one broken endpoint does not affect another)', async () => {
|
||||
mockGetFilePreview.mockRejectedValue(new Error('500'));
|
||||
for (let i = 0; i < PREVIEW_MAX_CONSECUTIVE_ERRORS; i++) {
|
||||
await expect(fetchFilePreview('fid-broken')).rejects.toThrow();
|
||||
}
|
||||
expect(previewRefetchInterval(undefined, q('fid-broken'))).toBe(false);
|
||||
expect(previewRefetchInterval(undefined, q('fid-healthy'))).toBe(2500);
|
||||
});
|
||||
});
|
||||
|
|
@ -105,3 +105,83 @@ export const useCodeOutputDownload = (url = ''): QueryObserverResult<string> =>
|
|||
},
|
||||
);
|
||||
};
|
||||
|
||||
/* Stop on terminal success or after 5 consecutive errors. The cap is
|
||||
* tracked in a module-level Map keyed by file_id because React Query
|
||||
* v4 resets `state.fetchFailureCount` to 0 on every fetch dispatch
|
||||
* (the `'fetch'` action in the reducer), so it can't be used to count
|
||||
* errors *across* polls. */
|
||||
export const PREVIEW_MAX_CONSECUTIVE_ERRORS = 5;
|
||||
const consecutivePreviewErrors = new Map<string, number>();
|
||||
|
||||
export const fetchFilePreview = async (fileId: string): Promise<t.TFilePreview> => {
|
||||
try {
|
||||
const data = await dataService.getFilePreview(fileId);
|
||||
consecutivePreviewErrors.delete(fileId);
|
||||
return data;
|
||||
} catch (err) {
|
||||
consecutivePreviewErrors.set(fileId, (consecutivePreviewErrors.get(fileId) ?? 0) + 1);
|
||||
throw err;
|
||||
}
|
||||
};
|
||||
|
||||
export const previewRefetchInterval = (
|
||||
data: t.TFilePreview | undefined,
|
||||
query: { queryKey: readonly unknown[] },
|
||||
): number | false => {
|
||||
const fileId = String(query.queryKey[1] ?? '');
|
||||
if (data?.status === 'ready' || data?.status === 'failed') {
|
||||
consecutivePreviewErrors.delete(fileId);
|
||||
return false;
|
||||
}
|
||||
if ((consecutivePreviewErrors.get(fileId) ?? 0) >= PREVIEW_MAX_CONSECUTIVE_ERRORS) {
|
||||
consecutivePreviewErrors.delete(fileId);
|
||||
return false;
|
||||
}
|
||||
return 2500;
|
||||
};
|
||||
|
||||
/** Test-only: clear the consecutive-error counter. */
|
||||
export const _resetPreviewErrorCounter = (fileId?: string): void => {
|
||||
if (fileId) consecutivePreviewErrors.delete(fileId);
|
||||
else consecutivePreviewErrors.clear();
|
||||
};
|
||||
|
||||
/**
|
||||
* Poll the lifecycle of an inline file preview while background HTML
|
||||
* extraction runs.
|
||||
*
|
||||
* Caller wires `enabled` to `attachment.status === 'pending'` so the
|
||||
* query is dormant for terminal-status records. Once enabled, React
|
||||
* Query's `refetchInterval` runs at 2.5s; see `previewRefetchInterval`
|
||||
* for the auto-stop rules. Idle by default.
|
||||
*
|
||||
* Cache key: `[QueryKeys.filePreview, file_id]`. Sibling components
|
||||
* watching the same `file_id` get a single shared poller.
|
||||
*/
|
||||
export const useFilePreview = (
|
||||
file_id: string | undefined,
|
||||
config?: UseQueryOptions<t.TFilePreview, unknown, t.TFilePreview>,
|
||||
): QueryObserverResult<t.TFilePreview, unknown> => {
|
||||
return useQuery<t.TFilePreview, unknown, t.TFilePreview>(
|
||||
[QueryKeys.filePreview, file_id],
|
||||
() => fetchFilePreview(file_id ?? ''),
|
||||
{
|
||||
refetchOnWindowFocus: false,
|
||||
refetchOnReconnect: false,
|
||||
/* Note: `refetchOnMount` left at the React Query default (`true`)
|
||||
* so a freshly-mounted observer with stale cached data refetches.
|
||||
* Cross-turn filename reuse keeps the same `file_id`; the cache
|
||||
* may hold a prior turn's `'ready'` payload. `useAttachmentHandler`
|
||||
* removes the entry on every new attachment for safety, but this
|
||||
* default is the second line of defense — without it, an observer
|
||||
* that mounts before the handler runs would read the stale cache
|
||||
* and `refetchInterval` would never start polling. (Codex P1
|
||||
* round-3 review on PR #12957.) */
|
||||
retry: false,
|
||||
refetchInterval: previewRefetchInterval,
|
||||
...config,
|
||||
enabled: !!file_id && (config?.enabled ?? true),
|
||||
},
|
||||
);
|
||||
};
|
||||
|
|
|
|||
|
|
@ -0,0 +1,442 @@
|
|||
/**
|
||||
* Coverage for `useAttachmentPreviewSync` — the bridge between the
|
||||
* deferred-preview code-execution lifecycle and the attachment cache.
|
||||
*
|
||||
* Behavior under test:
|
||||
* 1. Polling enables only when `attachment.status === 'pending'` AND
|
||||
* some conversation is submitting (per the user's explicit gate).
|
||||
* 2. On a terminal poll response (ready/failed), the resolved record
|
||||
* is upserted into `messageAttachmentsMap` by `file_id`.
|
||||
* 3. The hook's returned `status` reflects the polled value once
|
||||
* it arrives, not just the prop snapshot.
|
||||
*
|
||||
* The underlying `useFilePreview` hook is mocked here — its own
|
||||
* polling cadence and refetchInterval semantics are React Query's
|
||||
* concern, not this hook's.
|
||||
*/
|
||||
|
||||
import { useEffect } from 'react';
|
||||
import { renderHook } from '@testing-library/react';
|
||||
import { RecoilRoot, useRecoilValue, useSetRecoilState } from 'recoil';
|
||||
import type { ReactNode } from 'react';
|
||||
import type { TAttachment, TFilePreview } from 'librechat-data-provider';
|
||||
import store from '~/store';
|
||||
|
||||
const mockUseFilePreview = jest.fn();
|
||||
jest.mock('~/data-provider', () => ({
|
||||
useFilePreview: (...args: unknown[]) => mockUseFilePreview(...args),
|
||||
}));
|
||||
|
||||
import useAttachmentPreviewSync from '../useAttachmentPreviewSync';
|
||||
|
||||
const wrapper = ({ children }: { children: ReactNode }) => <RecoilRoot>{children}</RecoilRoot>;
|
||||
|
||||
const messageId = 'msg-1';
|
||||
const fileId = 'fid-1';
|
||||
|
||||
function makeAttachment(overrides: Partial<TAttachment> = {}): TAttachment {
|
||||
return {
|
||||
file_id: fileId,
|
||||
filename: 'data.xlsx',
|
||||
filepath: '/uploads/data.xlsx',
|
||||
type: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
|
||||
messageId,
|
||||
toolCallId: 'tc-1',
|
||||
text: null,
|
||||
textFormat: null,
|
||||
status: 'pending',
|
||||
...overrides,
|
||||
} as unknown as TAttachment;
|
||||
}
|
||||
|
||||
/** Read messageAttachmentsMap and bridge it out via a mutable ref. */
|
||||
function setup({
|
||||
attachment,
|
||||
isSubmitting,
|
||||
preview,
|
||||
isFetching = false,
|
||||
seedLiveMap = true,
|
||||
}: {
|
||||
attachment: TAttachment;
|
||||
isSubmitting: boolean;
|
||||
preview?: TFilePreview;
|
||||
isFetching?: boolean;
|
||||
/* When false, simulates a loaded conversation: the message's
|
||||
* attachments come from the DB but `messageAttachmentsMap[messageId]`
|
||||
* is empty (no SSE handler ever fired for this messageId). The
|
||||
* upsert must INSERT (not just update) the resolved record into the
|
||||
* live map so the parent's `useAttachments` merge picks it up. */
|
||||
seedLiveMap?: boolean;
|
||||
}) {
|
||||
mockUseFilePreview.mockReset();
|
||||
mockUseFilePreview.mockReturnValue({ data: preview, isFetching });
|
||||
const ref: { current: Record<string, TAttachment[] | undefined> } = { current: {} };
|
||||
let lastEnabled: boolean | undefined;
|
||||
/* Spy on the second arg of useFilePreview to assert the gate. */
|
||||
mockUseFilePreview.mockImplementation((_id: unknown, opts: { enabled?: boolean }) => {
|
||||
lastEnabled = opts?.enabled;
|
||||
return { data: preview, isFetching };
|
||||
});
|
||||
|
||||
const Bridge = () => {
|
||||
/* Seed the messageAttachmentsMap with the test attachment so the
|
||||
* hook's upsert path has something to find. Also expose setters
|
||||
* for tests that simulate `isAnySubmitting` toggling — the
|
||||
* selector reads `conversationKeysAtom` × `isSubmittingFamily(key)`
|
||||
* so we have to populate both for the selector to fire. */
|
||||
const setMap = useSetRecoilState(store.messageAttachmentsMap);
|
||||
const setKeys = useSetRecoilState(store.conversationKeysAtom);
|
||||
const setSubmitting = useSetRecoilState(store.isSubmittingFamily(0));
|
||||
useEffect(() => {
|
||||
if (seedLiveMap) {
|
||||
setMap({ [messageId]: [attachment] });
|
||||
}
|
||||
setKeys([0]);
|
||||
setSubmitting(isSubmitting);
|
||||
}, [setMap, setKeys, setSubmitting]);
|
||||
const map = useRecoilValue(store.messageAttachmentsMap);
|
||||
ref.current = map;
|
||||
return null;
|
||||
};
|
||||
|
||||
const { result } = renderHook(
|
||||
() => {
|
||||
return useAttachmentPreviewSync(attachment);
|
||||
},
|
||||
{
|
||||
wrapper: ({ children }: { children: ReactNode }) =>
|
||||
wrapper({
|
||||
children: (
|
||||
<>
|
||||
<Bridge />
|
||||
{children}
|
||||
</>
|
||||
),
|
||||
}),
|
||||
},
|
||||
);
|
||||
|
||||
return {
|
||||
result,
|
||||
get map() {
|
||||
return ref.current;
|
||||
},
|
||||
get enabled() {
|
||||
return lastEnabled;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Render the hook with controllable preview output (so a test can
|
||||
* trigger the pending→ready edge by re-rendering with a new value)
|
||||
* AND expose a snapshot read of the per-file_id `previewJustResolved`
|
||||
* flag so the test can assert the hook flipped it on the edge.
|
||||
*
|
||||
* `isSubmittingAtMount` seeds `isSubmittingFamily(0)` *before* the hook
|
||||
* runs (via `initializeState`) so the hook's mount-time snapshot read
|
||||
* captures the intended value. Setting it after mount via a child
|
||||
* effect would race with the hook's first render and the gate would
|
||||
* see the default (`false`), defeating the whole point of the test.
|
||||
*/
|
||||
function setupWithTransitions(
|
||||
initialPreview?: TFilePreview,
|
||||
{ isSubmittingAtMount = true }: { isSubmittingAtMount?: boolean } = {},
|
||||
) {
|
||||
let currentPreview = initialPreview;
|
||||
mockUseFilePreview.mockReset();
|
||||
mockUseFilePreview.mockImplementation(() => ({
|
||||
data: currentPreview,
|
||||
isFetching: false,
|
||||
}));
|
||||
|
||||
const flagRef: { current: boolean | undefined } = { current: undefined };
|
||||
const FlagProbe = ({ id }: { id: string }) => {
|
||||
/* Subscribing read — re-renders the probe whenever the flag flips,
|
||||
* so `flagRef.current` is updated after the consumer hook commits
|
||||
* the transition. (A non-subscribing snapshot read inside an
|
||||
* effect would capture the value as of the previous commit, which
|
||||
* misses the flag set fired in *this* render's effect tick.) */
|
||||
const flag = useRecoilValue(store.previewJustResolved(id));
|
||||
useEffect(() => {
|
||||
flagRef.current = flag;
|
||||
}, [flag]);
|
||||
return null;
|
||||
};
|
||||
|
||||
const { rerender } = renderHook(
|
||||
({ attachment }: { attachment: TAttachment }) => useAttachmentPreviewSync(attachment),
|
||||
{
|
||||
initialProps: { attachment: makeAttachment({ status: 'pending' }) },
|
||||
wrapper: ({ children }: { children: ReactNode }) => (
|
||||
<RecoilRoot
|
||||
initializeState={(snap) => {
|
||||
snap.set(store.isSubmittingFamily(0), isSubmittingAtMount);
|
||||
}}
|
||||
>
|
||||
<FlagProbe id={fileId} />
|
||||
{children}
|
||||
</RecoilRoot>
|
||||
),
|
||||
},
|
||||
);
|
||||
|
||||
return {
|
||||
setPreview: (preview: TFilePreview | undefined) => {
|
||||
currentPreview = preview;
|
||||
rerender({ attachment: makeAttachment({ status: 'pending' }) });
|
||||
},
|
||||
get justResolved() {
|
||||
return flagRef.current;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
describe('useAttachmentPreviewSync', () => {
|
||||
it('enables polling whenever status=pending (no longer gated on isSubmitting)', () => {
|
||||
const ctx = setup({
|
||||
attachment: makeAttachment({ status: 'pending' }),
|
||||
isSubmitting: true,
|
||||
});
|
||||
expect(ctx.enabled).toBe(true);
|
||||
});
|
||||
|
||||
it('does NOT enable polling when status is already ready', () => {
|
||||
const ctx = setup({
|
||||
attachment: makeAttachment({ status: 'ready' }),
|
||||
isSubmitting: true,
|
||||
});
|
||||
expect(ctx.enabled).toBe(false);
|
||||
});
|
||||
|
||||
it('does NOT enable polling when status is already failed', () => {
|
||||
const ctx = setup({
|
||||
attachment: makeAttachment({ status: 'failed', previewError: 'timeout' }),
|
||||
isSubmitting: true,
|
||||
});
|
||||
expect(ctx.enabled).toBe(false);
|
||||
});
|
||||
|
||||
it('STILL enables polling for pending records even after the LLM has finished generating', () => {
|
||||
/* Regression for the stuck-spinner bug: the deferred render can
|
||||
* complete a few seconds AFTER the SSE stream closes. With the
|
||||
* earlier `isAnySubmitting` gate, polling stopped the moment the
|
||||
* model finished and the resolved-but-not-yet-emitted state would
|
||||
* never reach the UI. Polling now runs on `status === 'pending'`
|
||||
* alone; `useFilePreview`'s `refetchInterval` auto-stops on the
|
||||
* first terminal response, and the server-side render ceiling +
|
||||
* lazy sweep cap the worst case. */
|
||||
const ctx = setup({
|
||||
attachment: makeAttachment({ status: 'pending' }),
|
||||
isSubmitting: false,
|
||||
});
|
||||
expect(ctx.enabled).toBe(true);
|
||||
});
|
||||
|
||||
it('does NOT enable polling when the attachment has no file_id', () => {
|
||||
const noId = { ...makeAttachment({ status: 'pending' }) } as Partial<TAttachment>;
|
||||
delete (noId as { file_id?: string }).file_id;
|
||||
const ctx = setup({
|
||||
attachment: noId as TAttachment,
|
||||
isSubmitting: true,
|
||||
});
|
||||
expect(ctx.enabled).toBe(false);
|
||||
});
|
||||
|
||||
it('returns ready when no preview and no status (legacy back-compat)', () => {
|
||||
const legacy = makeAttachment();
|
||||
delete (legacy as Partial<TAttachment & { status?: string }>).status;
|
||||
const ctx = setup({ attachment: legacy as TAttachment, isSubmitting: false });
|
||||
expect(ctx.result.current.status).toBe('ready');
|
||||
});
|
||||
|
||||
it('upserts the resolved preview into messageAttachmentsMap when preview reports ready', () => {
|
||||
const ctx = setup({
|
||||
attachment: makeAttachment({ status: 'pending' }),
|
||||
isSubmitting: true,
|
||||
preview: {
|
||||
file_id: fileId,
|
||||
status: 'ready',
|
||||
text: '<table>final</table>',
|
||||
textFormat: 'html',
|
||||
},
|
||||
});
|
||||
const updated = ctx.map[messageId]?.[0] as TAttachment & { text?: string };
|
||||
expect(updated.status).toBe('ready');
|
||||
expect(updated.text).toBe('<table>final</table>');
|
||||
expect(ctx.result.current.status).toBe('ready');
|
||||
});
|
||||
|
||||
it('upserts the resolved preview when preview reports failed (with previewError)', () => {
|
||||
const ctx = setup({
|
||||
attachment: makeAttachment({ status: 'pending' }),
|
||||
isSubmitting: true,
|
||||
preview: {
|
||||
file_id: fileId,
|
||||
status: 'failed',
|
||||
previewError: 'parser-error',
|
||||
},
|
||||
});
|
||||
const updated = ctx.map[messageId]?.[0] as TAttachment & { previewError?: string };
|
||||
expect(updated.status).toBe('failed');
|
||||
expect(updated.previewError).toBe('parser-error');
|
||||
expect(ctx.result.current.status).toBe('failed');
|
||||
expect(ctx.result.current.previewError).toBe('parser-error');
|
||||
});
|
||||
|
||||
it('does NOT upsert while the polled status is still pending', () => {
|
||||
const ctx = setup({
|
||||
attachment: makeAttachment({ status: 'pending' }),
|
||||
isSubmitting: true,
|
||||
preview: { file_id: fileId, status: 'pending' },
|
||||
});
|
||||
/* Map should be unchanged from the initial seed — no patch. */
|
||||
const list = ctx.map[messageId] ?? [];
|
||||
expect(list).toHaveLength(1);
|
||||
expect((list[0] as TAttachment & { status?: string }).status).toBe('pending');
|
||||
});
|
||||
|
||||
it('reports isPolling true when the query is fetching and the gate is open', () => {
|
||||
const ctx = setup({
|
||||
attachment: makeAttachment({ status: 'pending' }),
|
||||
isSubmitting: true,
|
||||
isFetching: true,
|
||||
});
|
||||
expect(ctx.result.current.isPolling).toBe(true);
|
||||
});
|
||||
|
||||
it('reports isPolling false when the query is not fetching', () => {
|
||||
const ctx = setup({
|
||||
attachment: makeAttachment({ status: 'pending' }),
|
||||
isSubmitting: true,
|
||||
isFetching: false,
|
||||
});
|
||||
expect(ctx.result.current.isPolling).toBe(false);
|
||||
});
|
||||
|
||||
it('INSERTS a new live entry when messageAttachmentsMap has no record for this file_id (loaded-conversation path)', () => {
|
||||
/* Regression for the "DB-frozen pending" bug: on a reloaded
|
||||
* conversation, messages come back from the DB with the
|
||||
* immediate-persist snapshot (`status: 'pending'`) and there is no
|
||||
* SSE handler running for the historical messageId — so
|
||||
* `messageAttachmentsMap[messageId]` is empty. The polling layer
|
||||
* must INSERT a new entry (not just update an existing one) so the
|
||||
* parent's `useAttachments` merge can overlay the resolved
|
||||
* lifecycle fields onto the DB attachment by file_id. */
|
||||
const ctx = setup({
|
||||
attachment: makeAttachment({ status: 'pending' }),
|
||||
isSubmitting: false,
|
||||
seedLiveMap: false,
|
||||
preview: {
|
||||
file_id: fileId,
|
||||
status: 'ready',
|
||||
text: '<table>resolved-on-reload</table>',
|
||||
textFormat: 'html',
|
||||
},
|
||||
});
|
||||
const list = ctx.map[messageId] ?? [];
|
||||
expect(list).toHaveLength(1);
|
||||
const inserted = list[0] as TAttachment & { text?: string; textFormat?: string };
|
||||
expect(inserted.file_id).toBe(fileId);
|
||||
expect(inserted.status).toBe('ready');
|
||||
expect(inserted.text).toBe('<table>resolved-on-reload</table>');
|
||||
expect(inserted.textFormat).toBe('html');
|
||||
/* The inserted entry must carry forward the original attachment's
|
||||
* non-lifecycle fields (filename, type, messageId, toolCallId) so
|
||||
* the renderer can still classify it correctly. */
|
||||
expect(inserted.filename).toBe('data.xlsx');
|
||||
expect(inserted.messageId).toBe(messageId);
|
||||
});
|
||||
|
||||
it('INSERTS a failed entry on a loaded conversation when polling reports failed', () => {
|
||||
const ctx = setup({
|
||||
attachment: makeAttachment({ status: 'pending' }),
|
||||
isSubmitting: false,
|
||||
seedLiveMap: false,
|
||||
preview: {
|
||||
file_id: fileId,
|
||||
status: 'failed',
|
||||
previewError: 'render-timeout',
|
||||
},
|
||||
});
|
||||
const list = ctx.map[messageId] ?? [];
|
||||
expect(list).toHaveLength(1);
|
||||
const inserted = list[0] as TAttachment & { previewError?: string };
|
||||
expect(inserted.status).toBe('failed');
|
||||
expect(inserted.previewError).toBe('render-timeout');
|
||||
});
|
||||
|
||||
describe('previewJustResolved signal (auto-open trigger)', () => {
|
||||
/* The signal is the bridge to ToolArtifactCard's auto-open path:
|
||||
* the card mounts after the routing re-runs (post-transition), so
|
||||
* it can't observe the transition itself. Setting a one-shot flag
|
||||
* keyed by file_id lets the card consume the signal on its very
|
||||
* first effect tick. We assert on the flag directly here; the
|
||||
* consume+open behavior lives in `ToolArtifactCard`'s coverage. */
|
||||
it('flips the per-file_id flag on the pending→ready transition', () => {
|
||||
const ctx = setupWithTransitions({ file_id: fileId, status: 'pending' });
|
||||
expect(ctx.justResolved).toBe(false);
|
||||
ctx.setPreview({
|
||||
file_id: fileId,
|
||||
status: 'ready',
|
||||
text: '<table>x</table>',
|
||||
textFormat: 'html',
|
||||
});
|
||||
expect(ctx.justResolved).toBe(true);
|
||||
});
|
||||
|
||||
it('does NOT flip the flag when the polled status is "failed"', () => {
|
||||
/* Failed previews stay as a download-only chip — nothing to
|
||||
* auto-open. The signal must stay false so the eventual
|
||||
* routing decision (PreviewPlaceholderCard with the alert
|
||||
* subtitle) doesn't get hijacked into opening an empty panel. */
|
||||
const ctx = setupWithTransitions({ file_id: fileId, status: 'pending' });
|
||||
ctx.setPreview({
|
||||
file_id: fileId,
|
||||
status: 'failed',
|
||||
previewError: 'render-timeout',
|
||||
});
|
||||
expect(ctx.justResolved).toBe(false);
|
||||
});
|
||||
|
||||
it('does NOT flip the flag when the first observed status is already "ready" (history load)', () => {
|
||||
/* A page-load mount where the file resolved long ago must not
|
||||
* trigger auto-open — the user is scrolling through history,
|
||||
* not awaiting a result. Without this guard, every loaded
|
||||
* conversation would yank the panel open on first paint. */
|
||||
const ctx = setupWithTransitions({
|
||||
file_id: fileId,
|
||||
status: 'ready',
|
||||
text: '<table>x</table>',
|
||||
textFormat: 'html',
|
||||
});
|
||||
expect(ctx.justResolved).toBe(false);
|
||||
});
|
||||
|
||||
it('does NOT flip the flag on a navigation-time pending→ready (hook mounted with isSubmitting=false)', () => {
|
||||
/* Regression for the "panel auto-opens on every revisit" bug:
|
||||
* the immediate-persist snapshot saves the message's attachment
|
||||
* at `status: 'pending'`, which never gets rewritten when the
|
||||
* file record itself transitions to `'ready'`. When the user
|
||||
* navigates back, the hook mounts with `isSubmitting=false`,
|
||||
* polls once, and sees a pending→ready transition — but this
|
||||
* is NOT a fresh resolution from the user's perspective, just
|
||||
* polling catching up to long-resolved data. The
|
||||
* `mountedDuringStreamRef` gate must drop this transition on
|
||||
* the floor so the panel stays closed. The pre-PR commit
|
||||
* history explicitly removed history-load auto-open; this
|
||||
* preserves that contract. */
|
||||
const ctx = setupWithTransitions(
|
||||
{ file_id: fileId, status: 'pending' },
|
||||
{ isSubmittingAtMount: false },
|
||||
);
|
||||
ctx.setPreview({
|
||||
file_id: fileId,
|
||||
status: 'ready',
|
||||
text: '<table>resolved-on-revisit</table>',
|
||||
textFormat: 'html',
|
||||
});
|
||||
expect(ctx.justResolved).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -1,3 +1,4 @@
|
|||
export { default as useAttachmentPreviewSync } from './useAttachmentPreviewSync';
|
||||
export { default as useDeleteFilesFromTable } from './useDeleteFilesFromTable';
|
||||
export { default as useSetFilesToDelete } from './useSetFilesToDelete';
|
||||
export { default as useFileHandling, useFileHandlingNoChatContext } from './useFileHandling';
|
||||
|
|
|
|||
197
client/src/hooks/Files/useAttachmentPreviewSync.ts
Normal file
197
client/src/hooks/Files/useAttachmentPreviewSync.ts
Normal file
|
|
@ -0,0 +1,197 @@
|
|||
import { useEffect, useRef } from 'react';
|
||||
import { useRecoilCallback, useSetRecoilState } from 'recoil';
|
||||
import type { TAttachment, TFile, TFilePreview } from 'librechat-data-provider';
|
||||
import { useFilePreview } from '~/data-provider';
|
||||
import store from '~/store';
|
||||
|
||||
interface UseAttachmentPreviewSyncResult {
|
||||
/**
|
||||
* Effective lifecycle status: `'pending'` while background HTML
|
||||
* extraction is in flight, `'ready'` once it completes successfully
|
||||
* (or for legacy / non-office files that never had a status), and
|
||||
* `'failed'` when extraction errored or hit the 60s ceiling. Drives
|
||||
* UI state (spinner, badge, etc.) without callers needing to read
|
||||
* the attachment shape directly.
|
||||
*/
|
||||
status: 'pending' | 'ready' | 'failed';
|
||||
/** Short machine-readable failure reason from the backend. */
|
||||
previewError?: string;
|
||||
/** True while React Query is actively polling the preview endpoint. */
|
||||
isPolling: boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Bridge the deferred-preview code-execution lifecycle to the
|
||||
* attachment cache.
|
||||
*
|
||||
* The immediate persist step (in callbacks.js + processCodeOutput)
|
||||
* emits the attachment record at `status: 'pending'` so the agent's
|
||||
* response stops blocking on extraction. The background render runs
|
||||
* separately; if the SSE stream is still open when it lands, an
|
||||
* `attachment` update event arrives and the SSE handler upserts by
|
||||
* `file_id`. If the stream has already closed (the model finished
|
||||
* generating before the render resolved), this hook covers the gap by
|
||||
* polling `GET /api/files/:file_id/preview` and writing the resolved
|
||||
* record back into `messageAttachmentsMap` — which triggers
|
||||
* re-classification through `artifactTypeForAttachment`, so the file
|
||||
* chip transitions from a plain download to the rich preview card
|
||||
* (or to a download-with-error state) without remounting.
|
||||
*
|
||||
* Polling is gated on:
|
||||
* - `attachment.file_id` present (no id → nothing to poll for)
|
||||
* - Effective status is `'pending'` (terminal states need no work).
|
||||
* `useFilePreview`'s `refetchInterval` returns `false` the moment
|
||||
* the server reports `ready`/`failed`, so polling auto-terminates
|
||||
* within one tick of resolution. Bounded ceiling: the server-side
|
||||
* render timeout is 60s, so a stuck pending record gets ~24 polls
|
||||
* max before the lazy sweep in the preview endpoint forces it to
|
||||
* `'failed'`.
|
||||
*
|
||||
* NOTE: an earlier version of this hook also gated on `isAnySubmitting`
|
||||
* (the LLM still generating). That gate was removed because the
|
||||
* deferred render can complete *after* the SSE stream closes — when it
|
||||
* does, the SSE update is silently dropped, and polling is the only
|
||||
* recovery path. With the gate in place, the chip would stay stuck on
|
||||
* "Preparing preview…" forever (until manual refresh) for any render
|
||||
* that landed even seconds after submission ended. The polling itself
|
||||
* doesn't block UX; the user can keep messaging regardless.
|
||||
*/
|
||||
export default function useAttachmentPreviewSync(
|
||||
attachment: TAttachment | undefined,
|
||||
): UseAttachmentPreviewSyncResult {
|
||||
const setAttachmentsMap = useSetRecoilState(store.messageAttachmentsMap);
|
||||
/* `useRecoilCallback` reads/writes without subscribing this hook to
|
||||
* the per-file_id flag — we only ever set it on the pending→ready
|
||||
* edge, so subscribing would cause needless re-renders. */
|
||||
const flagJustResolved = useRecoilCallback(
|
||||
({ set }) =>
|
||||
(id: string) => {
|
||||
set(store.previewJustResolved(id), true);
|
||||
},
|
||||
[],
|
||||
);
|
||||
/* Capture `isAnySubmitting` at first render via a non-subscribing
|
||||
* snapshot read. Mirrors `ToolArtifactCard`'s `mountedDuringStreamRef`
|
||||
* pattern so this hook applies the same "is the user actively in a
|
||||
* turn?" classification as the card itself. The ref is the gate that
|
||||
* distinguishes a *fresh* deferred-preview resolution (auto-open
|
||||
* eligible) from a *stale* DB-pending record resolving on a history
|
||||
* load (auto-open must NOT fire — the user is scrolling old data,
|
||||
* not awaiting a result). Without this gate, navigating back to a
|
||||
* conversation whose immediate-persist snapshot left the message's
|
||||
* attachments at `status: 'pending'` would re-trigger auto-open
|
||||
* every time the polling layer caught up — which is exactly the
|
||||
* pre-PR "panel pops open on every visit" UX the team explicitly
|
||||
* removed. */
|
||||
const readInitialIsSubmitting = useRecoilCallback(
|
||||
({ snapshot }) =>
|
||||
() =>
|
||||
snapshot.getLoadable(store.isSubmittingFamily(0)).valueMaybe() ?? false,
|
||||
[],
|
||||
);
|
||||
const mountedDuringStreamRef = useRef<boolean | null>(null);
|
||||
if (mountedDuringStreamRef.current === null) {
|
||||
mountedDuringStreamRef.current = readInitialIsSubmitting();
|
||||
}
|
||||
|
||||
const file = (attachment ?? undefined) as Partial<TFile> | undefined;
|
||||
const fileId = file?.file_id;
|
||||
const baseStatus: 'pending' | 'ready' | 'failed' = file?.status ?? 'ready';
|
||||
const messageId = (attachment as Partial<TAttachment> | undefined)?.messageId;
|
||||
|
||||
const enabled = !!fileId && baseStatus === 'pending';
|
||||
|
||||
const previewQuery = useFilePreview(fileId, { enabled });
|
||||
|
||||
/* Effective status: prefer the polled record once it arrives, since
|
||||
* the SSE handler may have already moved the cache forward and the
|
||||
* `attachment` prop will catch up on the next render anyway. */
|
||||
const polled = previewQuery.data as TFilePreview | undefined;
|
||||
const effectiveStatus: 'pending' | 'ready' | 'failed' = polled?.status ?? baseStatus;
|
||||
const previewError = polled?.previewError ?? file?.previewError;
|
||||
|
||||
/* Track the previous effective status so we can fire the
|
||||
* pending→ready edge exactly once per session. Two gates have to
|
||||
* pass for the auto-open flag to flip:
|
||||
* 1. We actually observed the transition (prev → curr).
|
||||
* 2. The hook mounted during an active stream — i.e. the file is
|
||||
* part of the user's current turn, not a history load. A
|
||||
* page-navigation mount (or refresh) of a stale-pending DB
|
||||
* record will see the same transition when polling catches
|
||||
* up, but we must NOT auto-open in that case — the user is
|
||||
* revisiting old work, not waiting on a fresh result.
|
||||
* Refs are read inline so the effect doesn't have to list them as
|
||||
* deps (mutating a ref doesn't subscribe). */
|
||||
const prevStatusRef = useRef<'pending' | 'ready' | 'failed' | null>(null);
|
||||
useEffect(() => {
|
||||
const prev = prevStatusRef.current;
|
||||
prevStatusRef.current = effectiveStatus;
|
||||
if (
|
||||
prev === 'pending' &&
|
||||
effectiveStatus === 'ready' &&
|
||||
fileId &&
|
||||
mountedDuringStreamRef.current === true
|
||||
) {
|
||||
flagJustResolved(fileId);
|
||||
}
|
||||
}, [effectiveStatus, fileId, flagJustResolved]);
|
||||
|
||||
/* On a terminal poll response (ready or failed), upsert into the
|
||||
* shared attachments map. Mirrors the SSE handler's by-file_id
|
||||
* upsert (`useAttachmentHandler`) — the attachment object is
|
||||
* patched in place so siblings sharing the same atom re-render
|
||||
* with the resolved data and `artifactTypeForAttachment` re-runs
|
||||
* its empty-text gate, transitioning the file chip into a panel
|
||||
* artifact card.
|
||||
*
|
||||
* Two paths:
|
||||
* 1. Live SSE flow (active turn): the SSE handler already wrote
|
||||
* the attachment into messageAttachmentsMap. `existingIndex`
|
||||
* finds it; we patch in place.
|
||||
* 2. Loaded conversation (no SSE): the message's `attachments`
|
||||
* come from the DB (frozen at the immediate-persist state of
|
||||
* `status: 'pending'`); messageAttachmentsMap is empty for
|
||||
* this messageId. `existingIndex` is `-1`. We INSERT a new
|
||||
* entry that overlays the polled fields onto the original
|
||||
* `attachment` prop. `useAttachments` (the hook the renderer
|
||||
* reads through) merges live entries onto DB entries by
|
||||
* `file_id`, so the inserted entry takes precedence and the
|
||||
* parent re-routes to the proper PanelArtifact card. */
|
||||
useEffect(() => {
|
||||
if (!polled || polled.status === 'pending' || !messageId || !fileId || !attachment) {
|
||||
return;
|
||||
}
|
||||
setAttachmentsMap((prevMap) => {
|
||||
const messageAttachments =
|
||||
(prevMap as Record<string, TAttachment[] | undefined>)[messageId] || [];
|
||||
const existingIndex = messageAttachments.findIndex(
|
||||
(a) => (a as Partial<TFile>).file_id === fileId,
|
||||
);
|
||||
const resolvedFields = {
|
||||
status: polled.status,
|
||||
text: polled.text ?? null,
|
||||
textFormat: polled.textFormat ?? null,
|
||||
previewError: polled.previewError,
|
||||
};
|
||||
if (existingIndex >= 0) {
|
||||
const existing = messageAttachments[existingIndex] as Partial<TFile> & TAttachment;
|
||||
const merged = [...messageAttachments];
|
||||
merged[existingIndex] = {
|
||||
...existing,
|
||||
...resolvedFields,
|
||||
text: polled.text ?? existing.text ?? null,
|
||||
textFormat: polled.textFormat ?? existing.textFormat ?? null,
|
||||
} as TAttachment;
|
||||
return { ...prevMap, [messageId]: merged };
|
||||
}
|
||||
const inserted = { ...attachment, ...resolvedFields } as TAttachment;
|
||||
return { ...prevMap, [messageId]: [...messageAttachments, inserted] };
|
||||
});
|
||||
}, [polled, fileId, messageId, attachment, setAttachmentsMap]);
|
||||
|
||||
return {
|
||||
status: effectiveStatus,
|
||||
previewError,
|
||||
isPolling: enabled && previewQuery.isFetching,
|
||||
};
|
||||
}
|
||||
129
client/src/hooks/Messages/__tests__/useAttachments.spec.tsx
Normal file
129
client/src/hooks/Messages/__tests__/useAttachments.spec.tsx
Normal file
|
|
@ -0,0 +1,129 @@
|
|||
/**
|
||||
* Coverage for `useAttachments` — the merge layer that overlays live
|
||||
* (SSE / poll-driven) attachment lifecycle fields onto DB-loaded
|
||||
* attachments by `file_id`.
|
||||
*
|
||||
* The merge is the only thing that lets the deferred-preview flow
|
||||
* recover on a reloaded conversation: messages persist with the
|
||||
* immediate-snapshot `status: 'pending'`, but the file record itself
|
||||
* resolves to `'ready'` later. Without the by-file_id overlay, the
|
||||
* renderer would route through the plain file chip forever.
|
||||
*/
|
||||
|
||||
import { useEffect } from 'react';
|
||||
import { renderHook } from '@testing-library/react';
|
||||
import { RecoilRoot, useSetRecoilState } from 'recoil';
|
||||
import type { ReactNode } from 'react';
|
||||
import type { TAttachment } from 'librechat-data-provider';
|
||||
import store from '~/store';
|
||||
|
||||
jest.mock('~/hooks/useLocalize', () => () => (key: string) => key);
|
||||
|
||||
import useAttachments from '../useAttachments';
|
||||
|
||||
const messageId = 'msg-1';
|
||||
|
||||
function makeAttachment(overrides: Partial<TAttachment> = {}): TAttachment {
|
||||
return {
|
||||
file_id: 'fid-1',
|
||||
filename: 'data.xlsx',
|
||||
filepath: '/uploads/data.xlsx',
|
||||
type: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
|
||||
messageId,
|
||||
toolCallId: 'tc-1',
|
||||
text: null,
|
||||
textFormat: null,
|
||||
status: 'pending',
|
||||
...overrides,
|
||||
} as unknown as TAttachment;
|
||||
}
|
||||
|
||||
function setup({
|
||||
attachments,
|
||||
liveMap,
|
||||
}: {
|
||||
attachments?: TAttachment[];
|
||||
liveMap?: Record<string, TAttachment[]>;
|
||||
}) {
|
||||
const Seed = () => {
|
||||
const setMap = useSetRecoilState(store.messageAttachmentsMap);
|
||||
useEffect(() => {
|
||||
if (liveMap) {
|
||||
setMap(liveMap);
|
||||
}
|
||||
}, [setMap]);
|
||||
return null;
|
||||
};
|
||||
|
||||
return renderHook(() => useAttachments({ messageId, attachments }), {
|
||||
wrapper: ({ children }: { children: ReactNode }) => (
|
||||
<RecoilRoot>
|
||||
<Seed />
|
||||
{children}
|
||||
</RecoilRoot>
|
||||
),
|
||||
});
|
||||
}
|
||||
|
||||
describe('useAttachments', () => {
|
||||
it('returns DB attachments when no live entries exist', () => {
|
||||
const db = makeAttachment({ status: 'pending' });
|
||||
const { result } = setup({ attachments: [db] });
|
||||
expect(result.current.attachments).toHaveLength(1);
|
||||
expect(result.current.attachments[0]).toBe(db);
|
||||
});
|
||||
|
||||
it('returns live entries when no DB attachments exist (active SSE turn)', () => {
|
||||
const live = makeAttachment({ status: 'ready', text: '<table>x</table>' });
|
||||
const { result } = setup({
|
||||
attachments: undefined,
|
||||
liveMap: { [messageId]: [live] },
|
||||
});
|
||||
expect(result.current.attachments).toHaveLength(1);
|
||||
expect((result.current.attachments[0] as TAttachment & { text?: string }).text).toBe(
|
||||
'<table>x</table>',
|
||||
);
|
||||
});
|
||||
|
||||
it('overlays live lifecycle fields onto matching DB attachment by file_id', () => {
|
||||
/* This is the regression test for the stuck-pending bug on a
|
||||
* reloaded conversation: the DB record was frozen at the
|
||||
* immediate-persist snapshot; the polling layer fetched the
|
||||
* resolved record and inserted it into messageAttachmentsMap; the
|
||||
* merge here must surface the resolved fields to the renderer. */
|
||||
const db = makeAttachment({ status: 'pending', text: null, textFormat: null });
|
||||
const live = makeAttachment({
|
||||
status: 'ready',
|
||||
text: '<table>resolved</table>',
|
||||
textFormat: 'html',
|
||||
});
|
||||
const { result } = setup({
|
||||
attachments: [db],
|
||||
liveMap: { [messageId]: [live] },
|
||||
});
|
||||
expect(result.current.attachments).toHaveLength(1);
|
||||
const merged = result.current.attachments[0] as TAttachment & {
|
||||
text?: string;
|
||||
textFormat?: string;
|
||||
};
|
||||
expect(merged.status).toBe('ready');
|
||||
expect(merged.text).toBe('<table>resolved</table>');
|
||||
expect(merged.textFormat).toBe('html');
|
||||
/* Non-lifecycle DB fields stay intact. */
|
||||
expect(merged.filename).toBe('data.xlsx');
|
||||
});
|
||||
|
||||
it('leaves DB attachments untouched when no live entry shares the file_id', () => {
|
||||
const db = makeAttachment({ file_id: 'fid-A', status: 'pending' });
|
||||
const live = makeAttachment({ file_id: 'fid-B', status: 'ready' });
|
||||
const { result } = setup({
|
||||
attachments: [db],
|
||||
liveMap: { [messageId]: [live] },
|
||||
});
|
||||
/* DB attachments are the authoritative list for THIS message — a
|
||||
* live entry without a matching file_id must NOT bleed in. */
|
||||
expect(result.current.attachments).toHaveLength(1);
|
||||
expect((result.current.attachments[0] as TAttachment).file_id).toBe('fid-A');
|
||||
expect((result.current.attachments[0] as TAttachment).status).toBe('pending');
|
||||
});
|
||||
});
|
||||
|
|
@ -1,6 +1,6 @@
|
|||
import { useMemo } from 'react';
|
||||
import { useRecoilValue } from 'recoil';
|
||||
import type { TAttachment } from 'librechat-data-provider';
|
||||
import type { TAttachment, TFile } from 'librechat-data-provider';
|
||||
import { useSearchResultsByTurn } from './useSearchResultsByTurn';
|
||||
import store from '~/store';
|
||||
|
||||
|
|
@ -12,10 +12,43 @@ export default function useAttachments({
|
|||
attachments?: TAttachment[];
|
||||
}) {
|
||||
const messageAttachmentsMap = useRecoilValue(store.messageAttachmentsMap);
|
||||
const messageAttachments = useMemo(
|
||||
() => attachments ?? messageAttachmentsMap[messageId ?? ''] ?? [],
|
||||
[attachments, messageAttachmentsMap, messageId],
|
||||
);
|
||||
const messageAttachments = useMemo<TAttachment[]>(() => {
|
||||
const live = messageAttachmentsMap[messageId ?? ''];
|
||||
if (!attachments || attachments.length === 0) {
|
||||
return live ?? [];
|
||||
}
|
||||
if (!live || live.length === 0) {
|
||||
return attachments;
|
||||
}
|
||||
/* DB-loaded attachments are the source of truth for which
|
||||
* attachments belong to this message, but live entries (from the
|
||||
* SSE handler / `useAttachmentPreviewSync` polling) carry fresher
|
||||
* lifecycle fields — `status`, `text`, `textFormat`,
|
||||
* `previewError`. Without this merge, the deferred-preview flow
|
||||
* would render "stuck pending" forever on a loaded conversation:
|
||||
* the message saved to DB at end-of-run has the immediate-persist
|
||||
* snapshot (`status: 'pending'`, `text: null`); the file record
|
||||
* itself updates to `'ready'` later, but the message's
|
||||
* `attachments` array doesn't get rewritten. Polling fetches the
|
||||
* resolved record into `messageAttachmentsMap`; merging here lets
|
||||
* `artifactTypeForAttachment` see the resolved text/textFormat
|
||||
* and route through the proper PanelArtifact card. */
|
||||
const liveByFileId = new Map<string, TAttachment>();
|
||||
for (const a of live) {
|
||||
const id = (a as Partial<TFile>).file_id;
|
||||
if (id) {
|
||||
liveByFileId.set(id, a);
|
||||
}
|
||||
}
|
||||
return attachments.map((db) => {
|
||||
const id = (db as Partial<TFile>).file_id;
|
||||
if (!id) {
|
||||
return db;
|
||||
}
|
||||
const liveEntry = liveByFileId.get(id);
|
||||
return liveEntry ? ({ ...db, ...liveEntry } as TAttachment) : db;
|
||||
});
|
||||
}, [attachments, messageAttachmentsMap, messageId]);
|
||||
|
||||
const searchResults = useSearchResultsByTurn(messageAttachments);
|
||||
|
||||
|
|
|
|||
279
client/src/hooks/SSE/__tests__/useAttachmentHandler.spec.tsx
Normal file
279
client/src/hooks/SSE/__tests__/useAttachmentHandler.spec.tsx
Normal file
|
|
@ -0,0 +1,279 @@
|
|||
/**
|
||||
* Coverage for the upsert-by-file_id behavior in `useAttachmentHandler`.
|
||||
*
|
||||
* Deferred-preview code-execution flow: the immediate persist step
|
||||
* emits an attachment with `status: 'pending'`; the deferred render
|
||||
* emits the same `file_id` again with `status: 'ready'` (and
|
||||
* resolved `text`/`textFormat`) or `'failed'` (with `previewError`).
|
||||
* The handler MUST merge over the pending placeholder in place —
|
||||
* appending would render the artifact twice in the UI (once stuck
|
||||
* pending, once resolved).
|
||||
*
|
||||
* Lightweight attachments without a `file_id` (web_search citations,
|
||||
* file_search results) keep the legacy append-only behavior so two
|
||||
* unrelated citations both show up.
|
||||
*/
|
||||
|
||||
import { renderHook, act } from '@testing-library/react';
|
||||
import { RecoilRoot, useRecoilValue } from 'recoil';
|
||||
import { QueryClient } from '@tanstack/react-query';
|
||||
import { Tools } from 'librechat-data-provider';
|
||||
import type { ReactNode } from 'react';
|
||||
import type { TAttachment, EventSubmission } from 'librechat-data-provider';
|
||||
import useAttachmentHandler from '../useAttachmentHandler';
|
||||
import store from '~/store';
|
||||
|
||||
const wrapper = ({ children }: { children: ReactNode }) => <RecoilRoot>{children}</RecoilRoot>;
|
||||
|
||||
const submission = {} as EventSubmission;
|
||||
const messageId = 'msg-1';
|
||||
|
||||
function makeAttachment(overrides: Partial<TAttachment>): TAttachment {
|
||||
return {
|
||||
file_id: 'fid-1',
|
||||
filename: 'data.xlsx',
|
||||
filepath: '/uploads/data.xlsx',
|
||||
type: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
|
||||
messageId,
|
||||
toolCallId: 'tc-1',
|
||||
text: null,
|
||||
textFormat: null,
|
||||
status: 'pending',
|
||||
...overrides,
|
||||
} as unknown as TAttachment;
|
||||
}
|
||||
|
||||
/* Co-mount the handler and a reader of the messageAttachmentsMap atom in
|
||||
* the same RecoilRoot so each act() shows the post-write state. The
|
||||
* `attachmentsMap` ref is mutated by the reader on every render, so
|
||||
* callers read it after their `act()` to assert.
|
||||
*
|
||||
* Also exposes the underlying `queryClient` so tests can spy on
|
||||
* `removeQueries` for the deferred-preview cache-staleness fix
|
||||
* (Codex P1 review on PR #12957; round 3 swapped invalidate→remove). */
|
||||
function setup() {
|
||||
const queryClient = new QueryClient();
|
||||
const removeSpy = jest.spyOn(queryClient, 'removeQueries');
|
||||
const ref: { current: Record<string, TAttachment[] | undefined> } = { current: {} };
|
||||
const { result } = renderHook(
|
||||
() => {
|
||||
const handler = useAttachmentHandler(queryClient);
|
||||
const map = useRecoilValue(store.messageAttachmentsMap);
|
||||
ref.current = map;
|
||||
return handler;
|
||||
},
|
||||
{ wrapper },
|
||||
);
|
||||
return {
|
||||
handle: (data: TAttachment) => act(() => result.current({ data, submission })),
|
||||
get list(): TAttachment[] {
|
||||
return ref.current[messageId] ?? [];
|
||||
},
|
||||
removeSpy,
|
||||
};
|
||||
}
|
||||
|
||||
describe('useAttachmentHandler upsert-by-file_id', () => {
|
||||
it('appends a new attachment when no record with the same file_id exists', () => {
|
||||
const ctx = setup();
|
||||
ctx.handle(makeAttachment({ status: 'pending' }));
|
||||
expect(ctx.list).toHaveLength(1);
|
||||
expect(ctx.list[0]).toMatchObject({ file_id: 'fid-1', status: 'pending' });
|
||||
});
|
||||
|
||||
it('upserts in place when a second event arrives for the same file_id (initial pending → deferred ready)', () => {
|
||||
const ctx = setup();
|
||||
ctx.handle(makeAttachment({ status: 'pending' }));
|
||||
ctx.handle(makeAttachment({ status: 'ready', text: '<table></table>', textFormat: 'html' }));
|
||||
/* Critical: still ONE attachment, not two. The deferred update
|
||||
* patches the pending record in place. */
|
||||
expect(ctx.list).toHaveLength(1);
|
||||
expect(ctx.list[0]).toMatchObject({
|
||||
file_id: 'fid-1',
|
||||
status: 'ready',
|
||||
text: '<table></table>',
|
||||
textFormat: 'html',
|
||||
});
|
||||
});
|
||||
|
||||
it('upserts a failed deferred update over the pending placeholder', () => {
|
||||
const ctx = setup();
|
||||
ctx.handle(makeAttachment({ status: 'pending' }));
|
||||
ctx.handle(makeAttachment({ status: 'failed', previewError: 'timeout' }));
|
||||
expect(ctx.list).toHaveLength(1);
|
||||
expect(ctx.list[0]).toMatchObject({ status: 'failed', previewError: 'timeout' });
|
||||
});
|
||||
|
||||
it('keeps multiple distinct file_ids as separate entries', () => {
|
||||
const ctx = setup();
|
||||
ctx.handle(makeAttachment({ file_id: 'fid-A' }));
|
||||
ctx.handle(makeAttachment({ file_id: 'fid-B' }));
|
||||
expect(ctx.list).toHaveLength(2);
|
||||
expect(ctx.list.map((a) => (a as { file_id: string }).file_id).sort()).toEqual([
|
||||
'fid-A',
|
||||
'fid-B',
|
||||
]);
|
||||
});
|
||||
|
||||
it('appends (does NOT merge) attachments with no file_id', () => {
|
||||
/* Lightweight attachments like file_search citations and web_search
|
||||
* results don't carry file_id. The handler must keep its legacy
|
||||
* append behavior for them — merging would lose distinct citations
|
||||
* and is unnecessary because they're never the target of a
|
||||
* deferred preview update. */
|
||||
const ctx = setup();
|
||||
const noFileId = {
|
||||
messageId,
|
||||
toolCallId: 'tc-1',
|
||||
type: Tools.web_search,
|
||||
} as unknown as TAttachment;
|
||||
ctx.handle(noFileId);
|
||||
ctx.handle(noFileId);
|
||||
expect(ctx.list).toHaveLength(2);
|
||||
});
|
||||
|
||||
it('preserves fields from the first event when the second omits them', () => {
|
||||
/* The deferred preview update only carries the deltas (text, status,
|
||||
* textFormat). Fields set in the initial emit (filename, type, etc.)
|
||||
* must survive the merge — the second event uses spread-over-
|
||||
* existing semantics. */
|
||||
const ctx = setup();
|
||||
ctx.handle(makeAttachment({ status: 'pending', filename: 'phase1-name.xlsx' }));
|
||||
ctx.handle({
|
||||
file_id: 'fid-1',
|
||||
messageId,
|
||||
status: 'ready',
|
||||
text: 'final',
|
||||
textFormat: 'html',
|
||||
} as unknown as TAttachment);
|
||||
expect(ctx.list).toHaveLength(1);
|
||||
expect(ctx.list[0]).toMatchObject({
|
||||
filename: 'phase1-name.xlsx',
|
||||
status: 'ready',
|
||||
text: 'final',
|
||||
});
|
||||
});
|
||||
|
||||
it('does NOT regress a resolved record when finalHandler replays the phase-1 snapshot', () => {
|
||||
/* `useEventHandlers.finalHandler` iterates `responseMessage.attachments`
|
||||
* at stream end — which is the immediate-persist snapshot
|
||||
* (status:pending, text:null). If a deferred update has already
|
||||
* moved this file_id to ready/failed, that replay must NOT
|
||||
* downgrade it. Otherwise the chip flickers back to "pending" and
|
||||
* polling restarts until the lazy sweep catches up. (Codex P1.) */
|
||||
const ctx = setup();
|
||||
ctx.handle(makeAttachment({ status: 'pending', text: null }));
|
||||
ctx.handle({
|
||||
file_id: 'fid-1',
|
||||
messageId,
|
||||
status: 'ready',
|
||||
text: '<table>resolved</table>',
|
||||
textFormat: 'html',
|
||||
} as unknown as TAttachment);
|
||||
/* Phase-1 replay arrives last (finalHandler at stream end). */
|
||||
ctx.handle(makeAttachment({ status: 'pending', text: null }));
|
||||
expect(ctx.list).toHaveLength(1);
|
||||
expect(ctx.list[0]).toMatchObject({
|
||||
status: 'ready',
|
||||
text: '<table>resolved</table>',
|
||||
textFormat: 'html',
|
||||
});
|
||||
});
|
||||
|
||||
it('does NOT regress a failed record when finalHandler replays the phase-1 snapshot', () => {
|
||||
const ctx = setup();
|
||||
ctx.handle(makeAttachment({ status: 'pending' }));
|
||||
ctx.handle({
|
||||
file_id: 'fid-1',
|
||||
messageId,
|
||||
status: 'failed',
|
||||
previewError: 'parser-error',
|
||||
} as unknown as TAttachment);
|
||||
ctx.handle(makeAttachment({ status: 'pending' }));
|
||||
expect(ctx.list[0]).toMatchObject({
|
||||
status: 'failed',
|
||||
previewError: 'parser-error',
|
||||
});
|
||||
});
|
||||
|
||||
describe('filePreview cache eviction (cross-turn filename reuse)', () => {
|
||||
/* Cross-turn filename reuse keeps the same `file_id`. If a prior
|
||||
* turn left `[QueryKeys.filePreview, file_id]` cached at
|
||||
* `status: 'ready'`, a new pending attachment would mount against
|
||||
* stale cache and `useFilePreview`'s polling (gated on `pending`)
|
||||
* would never start.
|
||||
*
|
||||
* The handler uses `removeQueries` (not `invalidateQueries`) for
|
||||
* this — invalidate alone only marks data stale, and the hook's
|
||||
* default `refetchOnMount` semantics combined with the
|
||||
* polling-gate-on-pending shape meant the new observer would still
|
||||
* read the stale 'ready' cache and never trigger a fetch. Removing
|
||||
* the entry forces a fresh server hit on next mount. (Codex P1
|
||||
* round-3 review on PR #12957.) */
|
||||
|
||||
it('removes [QueryKeys.filePreview, file_id] when an attachment with a file_id arrives', () => {
|
||||
const ctx = setup();
|
||||
ctx.handle(makeAttachment({ status: 'pending' }));
|
||||
expect(ctx.removeSpy).toHaveBeenCalledWith(['filePreview', 'fid-1']);
|
||||
});
|
||||
|
||||
it('removes the same query key for the deferred preview update event too', () => {
|
||||
/* Both the initial emit and the preview update should evict —
|
||||
* the second event is itself the authoritative new state, but
|
||||
* the upsert below races with the React Query cache and we want
|
||||
* the next mount to start clean. */
|
||||
const ctx = setup();
|
||||
ctx.handle(makeAttachment({ status: 'pending' }));
|
||||
ctx.handle(makeAttachment({ status: 'ready', text: '<table></table>', textFormat: 'html' }));
|
||||
const previewEvictions = ctx.removeSpy.mock.calls.filter(
|
||||
(c) => Array.isArray(c[0]) && c[0][0] === 'filePreview' && c[0][1] === 'fid-1',
|
||||
);
|
||||
expect(previewEvictions.length).toBe(2);
|
||||
});
|
||||
|
||||
it('does not evict when the attachment carries no file_id', () => {
|
||||
const ctx = setup();
|
||||
const noFileId = {
|
||||
messageId,
|
||||
toolCallId: 'tc-1',
|
||||
type: Tools.web_search,
|
||||
} as unknown as TAttachment;
|
||||
ctx.handle(noFileId);
|
||||
const previewEvictions = ctx.removeSpy.mock.calls.filter(
|
||||
(c) => Array.isArray(c[0]) && c[0][0] === 'filePreview',
|
||||
);
|
||||
expect(previewEvictions).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('does NOT use invalidateQueries (would leave stale cache readable per round-3 review)', () => {
|
||||
/* Lock in the choice of `removeQueries` over `invalidateQueries`.
|
||||
* Invalidate marks data stale but does not remove it; an
|
||||
* observer reading a stale cache still gets the value back and
|
||||
* the polling `refetchInterval` evaluates against stale 'ready'
|
||||
* → returns false → polling never starts. removeQueries forces
|
||||
* a refetch on next mount. */
|
||||
const queryClient = new QueryClient();
|
||||
const invalidateSpy = jest.spyOn(queryClient, 'invalidateQueries');
|
||||
const removeSpy = jest.spyOn(queryClient, 'removeQueries');
|
||||
const ref: { current: Record<string, TAttachment[] | undefined> } = { current: {} };
|
||||
const { result } = renderHook(
|
||||
() => {
|
||||
const handler = useAttachmentHandler(queryClient);
|
||||
const map = useRecoilValue(store.messageAttachmentsMap);
|
||||
ref.current = map;
|
||||
return handler;
|
||||
},
|
||||
{ wrapper },
|
||||
);
|
||||
act(() => result.current({ data: makeAttachment({ status: 'pending' }), submission }));
|
||||
const usedRemove = removeSpy.mock.calls.some(
|
||||
(c) => Array.isArray(c[0]) && c[0][0] === 'filePreview',
|
||||
);
|
||||
const usedInvalidate = invalidateSpy.mock.calls.some(
|
||||
(c) => Array.isArray(c[0]) && c[0][0] === 'filePreview',
|
||||
);
|
||||
expect(usedRemove).toBe(true);
|
||||
expect(usedInvalidate).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -15,6 +15,7 @@ export default function useAttachmentHandler(queryClient?: QueryClient) {
|
|||
|
||||
return ({ data }: { data: TAttachment; submission: EventSubmission }) => {
|
||||
const { messageId } = data;
|
||||
const fileId = (data as Partial<TFile>).file_id;
|
||||
|
||||
const fileData = data as TFile;
|
||||
if (
|
||||
|
|
@ -49,9 +50,62 @@ export default function useAttachmentHandler(queryClient?: QueryClient) {
|
|||
});
|
||||
}
|
||||
|
||||
/* Cross-turn filename reuse keeps the same `file_id` across runs.
|
||||
* If a prior turn left `[QueryKeys.filePreview, file_id]` cached at
|
||||
* `status: 'ready'`, a new pending attachment would mount against
|
||||
* that stale cache and `useFilePreview`'s `refetchInterval` (which
|
||||
* only polls on `pending`) would never start. Drop the cache entry
|
||||
* entirely so a fresh mount has nothing to read and must fetch.
|
||||
*
|
||||
* `removeQueries` (vs the earlier `invalidateQueries`) is the
|
||||
* stronger fix: invalidate only marks data stale, but
|
||||
* `refetchOnMount: false` was masking that — the new observer
|
||||
* read the stale 'ready' value and `refetchInterval` shut polling
|
||||
* off before it ever started. (Codex P1 round-3 review on
|
||||
* PR #12957.) */
|
||||
if (queryClient && fileId) {
|
||||
queryClient.removeQueries([QueryKeys.filePreview, fileId]);
|
||||
}
|
||||
|
||||
setAttachmentsMap((prevMap) => {
|
||||
const messageAttachments =
|
||||
(prevMap as Record<string, TAttachment[] | undefined>)[messageId] || [];
|
||||
/* Upsert by `file_id` rather than always appending. The
|
||||
* deferred-preview flow emits the same attachment twice: first
|
||||
* with `status: 'pending'` and `text: null`, then again with
|
||||
* `status: 'ready'` (and text/textFormat) or `'failed'` (with
|
||||
* previewError). The second event must merge over the first in
|
||||
* place — appending would render the artifact card twice, once
|
||||
* stuck pending and once resolved. Attachments without a
|
||||
* `file_id` (lightweight types like web_search / file_search
|
||||
* citations) keep the legacy append behavior. */
|
||||
if (fileId) {
|
||||
const existingIndex = messageAttachments.findIndex(
|
||||
(a) => (a as Partial<TFile>).file_id === fileId,
|
||||
);
|
||||
if (existingIndex > -1) {
|
||||
const existing = messageAttachments[existingIndex] as Partial<TFile>;
|
||||
const incoming = data as Partial<TFile>;
|
||||
const next = { ...existing, ...data } as TAttachment;
|
||||
/* Don't let a phase-1 replay (finalHandler iterates
|
||||
* `responseMessage.attachments`, which is the immediate-persist
|
||||
* snapshot at status:pending) regress a record a deferred
|
||||
* update has already moved to ready/failed. Pin the terminal
|
||||
* lifecycle fields when the merge would downgrade. */
|
||||
if (
|
||||
(existing.status === 'ready' || existing.status === 'failed') &&
|
||||
incoming.status === 'pending'
|
||||
) {
|
||||
(next as Partial<TFile>).status = existing.status;
|
||||
(next as Partial<TFile>).text = existing.text;
|
||||
(next as Partial<TFile>).textFormat = existing.textFormat;
|
||||
(next as Partial<TFile>).previewError = existing.previewError;
|
||||
}
|
||||
const merged = [...messageAttachments];
|
||||
merged[existingIndex] = next;
|
||||
return { ...prevMap, [messageId]: merged };
|
||||
}
|
||||
}
|
||||
return {
|
||||
...prevMap,
|
||||
[messageId]: [...messageAttachments, data],
|
||||
|
|
|
|||
|
|
@ -1305,6 +1305,8 @@
|
|||
"com_ui_prev": "Prev",
|
||||
"com_ui_prev_result": "Previous result",
|
||||
"com_ui_preview": "Preview",
|
||||
"com_ui_preview_failed": "Preview unavailable",
|
||||
"com_ui_preview_preparing": "Preparing preview…",
|
||||
"com_ui_preview_unavailable": "Preview not available for this file type",
|
||||
"com_ui_privacy_policy": "Privacy policy",
|
||||
"com_ui_privacy_policy_url": "Privacy Policy URL",
|
||||
|
|
|
|||
|
|
@ -88,6 +88,32 @@ export const artifactByIdSelector = selectorFamily<Artifact | undefined, string>
|
|||
},
|
||||
});
|
||||
|
||||
/**
|
||||
* One-shot signal that an attachment's deferred preview just transitioned
|
||||
* from `pending` to `ready` during the current session — keyed by
|
||||
* `file_id` (raw, NOT the `tool-artifact-${file_id}` form).
|
||||
*
|
||||
* The preview-sync hook flips this to `true` on the pending→ready edge.
|
||||
* `ToolArtifactCard` reads it on mount; if set, it auto-opens the panel
|
||||
* (even when no submission is in flight) and then resets the flag, so
|
||||
* subsequent re-mounts (panel close/reopen, re-render of the same card
|
||||
* from history) do not steal focus a second time.
|
||||
*
|
||||
* Why a separate signal rather than reusing `mountedDuringStreamRef`:
|
||||
* the deferred render can complete *after* the SSE stream has closed,
|
||||
* so the card mounts with `isSubmitting === false` and the existing
|
||||
* focus/open path skips. Without this signal, a freshly resolved
|
||||
* artifact would render in place but not auto-open — which is exactly
|
||||
* the bug the deferred-preview flow was designed to mask in the first
|
||||
* place. Auto-open ONLY on the pending→ready edge means a user
|
||||
* scrolling through history doesn't get the panel popping open every
|
||||
* time a previously resolved chip enters the viewport.
|
||||
*/
|
||||
export const previewJustResolved = atomFamily<boolean, string>({
|
||||
key: 'previewJustResolved',
|
||||
default: false,
|
||||
});
|
||||
|
||||
export const visibleArtifacts = atom<Record<string, Artifact | undefined> | null>({
|
||||
key: 'visibleArtifacts',
|
||||
default: null,
|
||||
|
|
|
|||
|
|
@ -39,8 +39,13 @@ 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 `processCodeOutput`'s deferred-preview flow can decide
|
||||
* whether to mark a freshly-persisted 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;
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -284,6 +284,11 @@ export const fileUpload = () => `${BASE_URL}/api/files`;
|
|||
export const fileDelete = () => `${BASE_URL}/api/files`;
|
||||
export const fileDownload = (userId: string, fileId: string) =>
|
||||
`${BASE_URL}/api/files/download/${userId}/${fileId}`;
|
||||
/* Deferred-preview lifecycle endpoint. Returns
|
||||
* `{ status, text?, textFormat?, previewError? }` so the frontend can
|
||||
* poll while background HTML extraction is in flight. See PR #12957. */
|
||||
export const filePreview = (fileId: string) =>
|
||||
`${BASE_URL}/api/files/${encodeURIComponent(fileId)}/preview`;
|
||||
export const fileConfig = () => `${BASE_URL}/api/files/config`;
|
||||
export const agentFiles = (agentId: string) => `${BASE_URL}/api/files/agent/${agentId}`;
|
||||
|
||||
|
|
|
|||
|
|
@ -402,6 +402,22 @@ export const getFiles = (): Promise<f.TFile[]> => {
|
|||
return request.get(endpoints.files());
|
||||
};
|
||||
|
||||
/**
|
||||
* Poll the lifecycle of an inline file preview. Returns the smallest
|
||||
* shape needed to drive the UI:
|
||||
* - `status` always present (defaults to `'ready'` server-side for
|
||||
* legacy records that pre-date the field).
|
||||
* - `text` and `textFormat` only when `status === 'ready'` and text
|
||||
* was extracted (preserves the HTML-or-null security contract).
|
||||
* - `previewError` only when `status === 'failed'`.
|
||||
*
|
||||
* Called from `useFilePreview`; React Query's `refetchInterval`
|
||||
* polls while `status === 'pending'` and stops on terminal status.
|
||||
*/
|
||||
export const getFilePreview = (fileId: string): Promise<f.TFilePreview> => {
|
||||
return request.get(endpoints.filePreview(fileId));
|
||||
};
|
||||
|
||||
export const getAgentFiles = (agentId: string): Promise<f.TFile[]> => {
|
||||
return request.get(endpoints.agentFiles(agentId));
|
||||
};
|
||||
|
|
|
|||
|
|
@ -34,6 +34,7 @@ export enum QueryKeys {
|
|||
assistantDocs = 'assistantDocs',
|
||||
agentDocs = 'agentDocs',
|
||||
fileDownload = 'fileDownload',
|
||||
filePreview = 'filePreview',
|
||||
voices = 'voices',
|
||||
customConfigSpeech = 'customConfigSpeech',
|
||||
prompts = 'prompts',
|
||||
|
|
|
|||
|
|
@ -127,6 +127,20 @@ export type TFile = {
|
|||
* See Codex P1 review on PR #12934.
|
||||
*/
|
||||
textFormat?: 'html' | 'text' | null;
|
||||
/**
|
||||
* Lifecycle of the inline preview rendered from `text`. `'pending'`
|
||||
* while background HTML extraction is in flight (deferred-preview
|
||||
* code-execution flow), `'ready'` once `text`/`textFormat` are set,
|
||||
* `'failed'` if extraction errored or hit the 60s ceiling. `undefined`
|
||||
* for legacy records and for files that never expect a preview —
|
||||
* clients MUST treat that as `'ready'`.
|
||||
*/
|
||||
status?: 'pending' | 'ready' | 'failed';
|
||||
/**
|
||||
* Short machine-readable failure reason when `status === 'failed'`.
|
||||
* Suitable for tooltip text but not user-facing prose.
|
||||
*/
|
||||
previewError?: string;
|
||||
metadata?: { fileIdentifier?: string };
|
||||
createdAt?: string | Date;
|
||||
updatedAt?: string | Date;
|
||||
|
|
@ -136,6 +150,28 @@ export type TFileUpload = TFile & {
|
|||
temp_file_id: string;
|
||||
};
|
||||
|
||||
/**
|
||||
* Shape returned by `GET /api/files/:file_id/preview`. The deferred-
|
||||
* preview code-execution flow polls this until status is terminal:
|
||||
* - `pending`: HTML extraction is still running. No `text`.
|
||||
* - `ready`: extraction succeeded; `text` + `textFormat` populated
|
||||
* iff the file produced inline preview content (binary/oversized
|
||||
* files reach `ready` with no text — render download-only).
|
||||
* - `failed`: extraction errored or hit the 60s ceiling;
|
||||
* `previewError` carries the short reason (`timeout`,
|
||||
* `parser-error`, `orphaned`, etc.).
|
||||
*
|
||||
* Legacy records pre-dating the field are surfaced as `'ready'` server-
|
||||
* side so existing attachments keep rendering normally.
|
||||
*/
|
||||
export type TFilePreview = {
|
||||
file_id: string;
|
||||
status: 'pending' | 'ready' | 'failed';
|
||||
text?: string;
|
||||
textFormat?: 'html' | 'text' | null;
|
||||
previewError?: string;
|
||||
};
|
||||
|
||||
export type AvatarUploadResponse = {
|
||||
url: string;
|
||||
};
|
||||
|
|
|
|||
|
|
@ -4,6 +4,8 @@ import { MongoMemoryServer } from 'mongodb-memory-server';
|
|||
import { EToolResources, FileContext } from 'librechat-data-provider';
|
||||
import { createFileMethods } from './file';
|
||||
import { createModels } from '~/models';
|
||||
import { runAsSystem } from '~/config/tenantContext';
|
||||
import { _resetStrictCache } from '~/models/plugins/tenantIsolation';
|
||||
|
||||
let File: mongoose.Model<unknown>;
|
||||
let fileMethods: ReturnType<typeof createFileMethods>;
|
||||
|
|
@ -256,6 +258,89 @@ describe('File Methods', () => {
|
|||
expect(updated?.bytes).toBe(200);
|
||||
expect(updated?.expiresAt).toBeUndefined();
|
||||
});
|
||||
|
||||
/* The optional `extraFilter` enables conditional updates — used by
|
||||
* the deferred-preview render's `finalizePreview` to guard against
|
||||
* an older render of the same `file_id` overwriting a newer turn's
|
||||
* record on cross-turn filename reuse. (Codex P1 review on PR
|
||||
* #12957.) */
|
||||
describe('extraFilter (conditional update)', () => {
|
||||
it('commits when the extra filter matches the current document', async () => {
|
||||
const fileId = uuidv4();
|
||||
const userId = new mongoose.Types.ObjectId();
|
||||
await fileMethods.createFile({
|
||||
file_id: fileId,
|
||||
user: userId,
|
||||
filename: 'data.xlsx',
|
||||
filepath: '/uploads/data.xlsx',
|
||||
type: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
|
||||
bytes: 100,
|
||||
status: 'pending',
|
||||
previewRevision: 'rev-A',
|
||||
});
|
||||
|
||||
const updated = await fileMethods.updateFile(
|
||||
{ file_id: fileId, status: 'ready', text: '<table></table>' },
|
||||
{ previewRevision: 'rev-A' },
|
||||
);
|
||||
|
||||
expect(updated).not.toBeNull();
|
||||
expect(updated?.status).toBe('ready');
|
||||
expect(updated?.text).toBe('<table></table>');
|
||||
});
|
||||
|
||||
it('returns null and skips the write when the extra filter does NOT match', async () => {
|
||||
const fileId = uuidv4();
|
||||
const userId = new mongoose.Types.ObjectId();
|
||||
await fileMethods.createFile({
|
||||
file_id: fileId,
|
||||
user: userId,
|
||||
filename: 'data.xlsx',
|
||||
filepath: '/uploads/data.xlsx',
|
||||
type: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
|
||||
bytes: 100,
|
||||
status: 'pending',
|
||||
previewRevision: 'rev-B', // newer turn has rotated the revision
|
||||
});
|
||||
|
||||
/* An older render that started while revision was 'rev-A' tries
|
||||
* to commit. The newer turn has since rotated to 'rev-B'. The
|
||||
* conditional update silently no-ops. */
|
||||
const updated = await fileMethods.updateFile(
|
||||
{ file_id: fileId, status: 'ready', text: '<stale/>' },
|
||||
{ previewRevision: 'rev-A' },
|
||||
);
|
||||
|
||||
expect(updated).toBeNull();
|
||||
|
||||
/* Critical: the newer record's text MUST be untouched. */
|
||||
const fresh = await fileMethods.findFileById(fileId);
|
||||
expect(fresh?.previewRevision).toBe('rev-B');
|
||||
expect(fresh?.status).toBe('pending');
|
||||
expect(fresh?.text).toBeUndefined();
|
||||
});
|
||||
|
||||
it('falls back to single-key update when extraFilter is omitted (back-compat)', async () => {
|
||||
const fileId = uuidv4();
|
||||
const userId = new mongoose.Types.ObjectId();
|
||||
await fileMethods.createFile({
|
||||
file_id: fileId,
|
||||
user: userId,
|
||||
filename: 'plain.txt',
|
||||
filepath: '/uploads/plain.txt',
|
||||
type: 'text/plain',
|
||||
bytes: 50,
|
||||
});
|
||||
|
||||
const updated = await fileMethods.updateFile({
|
||||
file_id: fileId,
|
||||
bytes: 99,
|
||||
});
|
||||
|
||||
expect(updated).not.toBeNull();
|
||||
expect(updated?.bytes).toBe(99);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('updateFileUsage', () => {
|
||||
|
|
@ -529,4 +614,133 @@ describe('File Methods', () => {
|
|||
await expect(fileMethods.batchUpdateFiles([])).resolves.toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe('sweepOrphanedPreviews', () => {
|
||||
/* The deferred-preview flow runs the deferred render in-process. If the
|
||||
* backend restarts mid-extraction, records stay at `status: 'pending'`
|
||||
* forever. The boot-time sweep transitions stale ones to 'failed'
|
||||
* with `previewError: 'orphaned'` so the frontend stops polling. */
|
||||
const userId = new mongoose.Types.ObjectId();
|
||||
|
||||
/**
|
||||
* Stamp a precise `updatedAt` on a file record. Mongoose timestamps
|
||||
* insist on the current time during create, so we backdate via a
|
||||
* direct collection write afterwards.
|
||||
*/
|
||||
async function makeFile(opts: {
|
||||
ageMs: number;
|
||||
status?: 'pending' | 'ready' | 'failed';
|
||||
}): Promise<string> {
|
||||
const fileId = uuidv4();
|
||||
await fileMethods.createFile({
|
||||
file_id: fileId,
|
||||
user: userId,
|
||||
filename: `${fileId}.xlsx`,
|
||||
filepath: `/uploads/${fileId}.xlsx`,
|
||||
type: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
|
||||
bytes: 1024,
|
||||
...(opts.status ? { status: opts.status } : {}),
|
||||
});
|
||||
const backdated = new Date(Date.now() - opts.ageMs);
|
||||
await File.collection.updateOne({ file_id: fileId }, { $set: { updatedAt: backdated } });
|
||||
return fileId;
|
||||
}
|
||||
|
||||
it('marks stale pending records as failed with previewError:orphaned', async () => {
|
||||
const stale = await makeFile({ ageMs: 10 * 60 * 1000, status: 'pending' });
|
||||
const fresh = await makeFile({ ageMs: 30 * 1000, status: 'pending' });
|
||||
|
||||
const count = await fileMethods.sweepOrphanedPreviews();
|
||||
expect(count).toBe(1);
|
||||
|
||||
const staleAfter = (await fileMethods.findFileById(stale)) as {
|
||||
status?: string;
|
||||
previewError?: string;
|
||||
} | null;
|
||||
expect(staleAfter?.status).toBe('failed');
|
||||
expect(staleAfter?.previewError).toBe('orphaned');
|
||||
|
||||
const freshAfter = (await fileMethods.findFileById(fresh)) as {
|
||||
status?: string;
|
||||
} | null;
|
||||
expect(freshAfter?.status).toBe('pending');
|
||||
});
|
||||
|
||||
it('does not touch records that are already ready or failed (idempotent)', async () => {
|
||||
const ready = await makeFile({ ageMs: 60 * 60 * 1000, status: 'ready' });
|
||||
const failed = await makeFile({ ageMs: 60 * 60 * 1000, status: 'failed' });
|
||||
|
||||
const count = await fileMethods.sweepOrphanedPreviews();
|
||||
expect(count).toBe(0);
|
||||
|
||||
const readyAfter = (await fileMethods.findFileById(ready)) as { status?: string } | null;
|
||||
const failedAfter = (await fileMethods.findFileById(failed)) as { status?: string } | null;
|
||||
expect(readyAfter?.status).toBe('ready');
|
||||
expect(failedAfter?.status).toBe('failed');
|
||||
});
|
||||
|
||||
it('does not touch legacy records with no status field (back-compat)', async () => {
|
||||
const legacy = await makeFile({ ageMs: 60 * 60 * 1000 }); // no status set
|
||||
const count = await fileMethods.sweepOrphanedPreviews();
|
||||
expect(count).toBe(0);
|
||||
const after = (await fileMethods.findFileById(legacy)) as { status?: string } | null;
|
||||
expect(after?.status).toBeUndefined();
|
||||
});
|
||||
|
||||
it('respects a custom maxAgeMs cutoff', async () => {
|
||||
const old10s = await makeFile({ ageMs: 10 * 1000, status: 'pending' });
|
||||
const old1m = await makeFile({ ageMs: 60 * 1000, status: 'pending' });
|
||||
|
||||
// Cutoff = 30s — only the 60s-old record should be swept.
|
||||
const count = await fileMethods.sweepOrphanedPreviews(30 * 1000);
|
||||
expect(count).toBe(1);
|
||||
|
||||
const tenSecAfter = (await fileMethods.findFileById(old10s)) as { status?: string } | null;
|
||||
const oneMinAfter = (await fileMethods.findFileById(old1m)) as {
|
||||
status?: string;
|
||||
previewError?: string;
|
||||
} | null;
|
||||
expect(tenSecAfter?.status).toBe('pending');
|
||||
expect(oneMinAfter?.status).toBe('failed');
|
||||
expect(oneMinAfter?.previewError).toBe('orphaned');
|
||||
});
|
||||
|
||||
it('returns 0 when there are no stale pending records', async () => {
|
||||
await makeFile({ ageMs: 30 * 1000, status: 'pending' });
|
||||
const count = await fileMethods.sweepOrphanedPreviews();
|
||||
expect(count).toBe(0);
|
||||
});
|
||||
|
||||
describe('strict tenant isolation (boot-time recovery)', () => {
|
||||
afterEach(() => {
|
||||
delete process.env.TENANT_ISOLATION_STRICT;
|
||||
_resetStrictCache();
|
||||
});
|
||||
|
||||
it('throws under strict mode without runAsSystem', async () => {
|
||||
await runAsSystem(() => makeFile({ ageMs: 10 * 60 * 1000, status: 'pending' }));
|
||||
process.env.TENANT_ISOLATION_STRICT = 'true';
|
||||
_resetStrictCache();
|
||||
await expect(fileMethods.sweepOrphanedPreviews()).rejects.toThrow(
|
||||
/Query attempted without tenant context in strict mode/,
|
||||
);
|
||||
});
|
||||
|
||||
it('succeeds under strict mode when wrapped in runAsSystem', async () => {
|
||||
const stale = await runAsSystem(() =>
|
||||
makeFile({ ageMs: 10 * 60 * 1000, status: 'pending' }),
|
||||
);
|
||||
process.env.TENANT_ISOLATION_STRICT = 'true';
|
||||
_resetStrictCache();
|
||||
const count = await runAsSystem(() => fileMethods.sweepOrphanedPreviews());
|
||||
expect(count).toBe(1);
|
||||
const after = (await runAsSystem(() => fileMethods.findFileById(stale))) as {
|
||||
status?: string;
|
||||
previewError?: string;
|
||||
} | null;
|
||||
expect(after?.status).toBe('failed');
|
||||
expect(after?.previewError).toBe('orphaned');
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -229,11 +229,24 @@ export function createFileMethods(mongoose: typeof import('mongoose')) {
|
|||
|
||||
/**
|
||||
* Updates a file identified by file_id with new data and removes the TTL.
|
||||
*
|
||||
* `extraFilter` extends the by-id query with additional conditions
|
||||
* (e.g. `{ previewRevision: '<expected uuid>' }`). When provided, the
|
||||
* update is conditional — it commits only if the document still
|
||||
* matches the extra filter, returning `null` otherwise. Used by the
|
||||
* deferred-preview render to guard against an older render of the
|
||||
* same `file_id` overwriting a newer turn's record on cross-turn
|
||||
* filename reuse.
|
||||
*
|
||||
* @param data - The data to update, must contain file_id
|
||||
* @returns A promise that resolves to the updated file document
|
||||
* @param extraFilter - Optional extra equality filter merged into the query.
|
||||
* @returns A promise that resolves to the updated file document, or
|
||||
* null if the conditional filter excluded it (or the file was
|
||||
* deleted).
|
||||
*/
|
||||
async function updateFile(
|
||||
data: Partial<IMongoFile> & { file_id: string },
|
||||
extraFilter?: FilterQuery<IMongoFile>,
|
||||
): Promise<IMongoFile | null> {
|
||||
const File = mongoose.models.File as Model<IMongoFile>;
|
||||
const { file_id, ...update } = data;
|
||||
|
|
@ -241,7 +254,8 @@ export function createFileMethods(mongoose: typeof import('mongoose')) {
|
|||
$set: update,
|
||||
$unset: { expiresAt: '' },
|
||||
};
|
||||
return File.findOneAndUpdate({ file_id }, updateOperation, {
|
||||
const query: FilterQuery<IMongoFile> = extraFilter ? { file_id, ...extraFilter } : { file_id };
|
||||
return File.findOneAndUpdate(query, updateOperation, {
|
||||
new: true,
|
||||
}).lean();
|
||||
}
|
||||
|
|
@ -368,6 +382,38 @@ export function createFileMethods(mongoose: typeof import('mongoose')) {
|
|||
return results.filter((result): result is IMongoFile => result != null);
|
||||
}
|
||||
|
||||
/**
|
||||
* Mark stale `status: 'pending'` file records as `'failed'` with
|
||||
* `previewError: 'orphaned'`. Recovers from the one case the
|
||||
* in-process deferred-preview render can't handle on its own: a
|
||||
* backend restart mid-render loses the in-memory promise, leaving
|
||||
* the record stuck pending forever.
|
||||
*
|
||||
* Cheap to run on boot — the `status` field is indexed and the typical
|
||||
* cutoff (5 min) bounds the candidate set to whatever was in flight at
|
||||
* the prior shutdown. The 60s render timeout means anything older
|
||||
* than a few minutes that's still pending is definitively orphaned.
|
||||
*
|
||||
* @param maxAgeMs - Cutoff in milliseconds; records whose `updatedAt`
|
||||
* is older than `now - maxAgeMs` are marked failed. Defaults to 5
|
||||
* minutes (well above the 60s render ceiling).
|
||||
* @returns Number of records updated.
|
||||
*/
|
||||
async function sweepOrphanedPreviews(maxAgeMs: number = 5 * 60 * 1000): Promise<number> {
|
||||
const File = mongoose.models.File as Model<IMongoFile>;
|
||||
const cutoff = new Date(Date.now() - maxAgeMs);
|
||||
const result = await File.updateMany(
|
||||
{ status: 'pending', updatedAt: { $lt: cutoff } },
|
||||
{ $set: { status: 'failed', previewError: 'orphaned' } },
|
||||
);
|
||||
if (result.modifiedCount > 0) {
|
||||
logger.info(
|
||||
`[sweepOrphanedPreviews] Marked ${result.modifiedCount} stale 'pending' files as 'failed' (cutoff: ${cutoff.toISOString()})`,
|
||||
);
|
||||
}
|
||||
return result.modifiedCount ?? 0;
|
||||
}
|
||||
|
||||
return {
|
||||
findFileById,
|
||||
getFiles,
|
||||
|
|
@ -383,6 +429,7 @@ export function createFileMethods(mongoose: typeof import('mongoose')) {
|
|||
deleteFileByFilter,
|
||||
batchUpdateFiles,
|
||||
updateFilesUsage,
|
||||
sweepOrphanedPreviews,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -64,6 +64,36 @@ const file: Schema<IMongoFile> = new Schema(
|
|||
type: String,
|
||||
enum: ['html', 'text'],
|
||||
},
|
||||
status: {
|
||||
/* Deferred-preview code-execution flow: the immediate persist
|
||||
* step writes the record with 'pending'; the background render
|
||||
* (HTML extraction) updates to 'ready' or 'failed'. Absent on
|
||||
* legacy records and on file kinds that never expect a preview. */
|
||||
type: String,
|
||||
enum: ['pending', 'ready', 'failed'],
|
||||
index: true,
|
||||
},
|
||||
previewError: {
|
||||
type: String,
|
||||
/* Bounded to short machine-readable reasons (`'timeout'`,
|
||||
* `'parser-error'`, `'orphaned'`, `'unexpected'`). Cap prevents a
|
||||
* future codepath from accidentally persisting a stack trace or
|
||||
* full error message — would bloat documents and ship a wall of
|
||||
* text into the UI tooltip. */
|
||||
maxlength: 200,
|
||||
},
|
||||
previewRevision: {
|
||||
/* Generation marker for the deferred-preview lifecycle. Stamped
|
||||
* by the immediate persist step on every emit (each new emit
|
||||
* gets a fresh UUID); the deferred preview render's `updateFile`
|
||||
* only commits when the marker still matches what it was when
|
||||
* extraction started. Without this, two turns reusing the same
|
||||
* `(filename, conversationId)` share a `file_id`, and an older
|
||||
* render finishing after a newer one would silently overwrite
|
||||
* the newer record with stale `text`/`status`. (Codex P1 review
|
||||
* on PR #12957.) */
|
||||
type: String,
|
||||
},
|
||||
context: {
|
||||
type: String,
|
||||
},
|
||||
|
|
|
|||
|
|
@ -19,6 +19,35 @@ export interface IMongoFile extends Omit<Document, 'model'> {
|
|||
* become executable markup. See Codex P1 review on PR #12934.
|
||||
*/
|
||||
textFormat?: 'html' | 'text';
|
||||
/**
|
||||
* Lifecycle of the inline preview rendered from `text`. Tracks the
|
||||
* deferred-preview code-execution flow (PR #12951 follow-up): the
|
||||
* immediate persist step saves the file blob and emits the attachment
|
||||
* record with `status: 'pending'`; a background render runs HTML
|
||||
* extraction and updates the record to `'ready'` (with `text` +
|
||||
* `textFormat`) or `'failed'` (with `previewError`). Decouples the
|
||||
* agent's final response from CPU-heavy office-format rendering.
|
||||
*
|
||||
* Absent for legacy records and for files that never expect a preview
|
||||
* (RAG uploads, images, plain-text artifacts). Clients MUST treat
|
||||
* `undefined` as `'ready'` so prior-version records render normally.
|
||||
*/
|
||||
status?: 'pending' | 'ready' | 'failed';
|
||||
/**
|
||||
* Short machine-readable reason when `status === 'failed'` —
|
||||
* `'timeout'`, `'parser-error'`, `'oversized'`, `'orphaned'`. UI hint
|
||||
* for tooltip text; not user-facing prose. Absent otherwise.
|
||||
*/
|
||||
previewError?: string;
|
||||
/**
|
||||
* Generation marker for the deferred-preview lifecycle. The
|
||||
* immediate persist step stamps a fresh UUID on every emit; the
|
||||
* deferred render's update only commits when the marker still
|
||||
* matches. Guards against an older render overwriting a newer
|
||||
* record on cross-turn filename reuse. Absent for legacy records
|
||||
* and for files that never expect a preview.
|
||||
*/
|
||||
previewRevision?: string;
|
||||
filename: string;
|
||||
filepath: string;
|
||||
object: 'file';
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue