mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-03 22:32:42 +00:00
🩹 fix: Polish code-execution attachment UX (#12870)
* 🧹 chore: Strip code-execution boilerplate from tool output
The bash executor in `@librechat/agents` appends two kinds of noise to
every successful run:
1. Trailing `Note:` paragraphs — long behavioral hints repeating
rules already in the system prompt ("Files from previous executions
are automatically available...", "Files in 'Available files' are
inputs..."). Re-stating these on every tool call adds ~50 tokens of
waste per call, which compounds across long agent traces.
2. Per-file `| <annotation>` suffixes on every line of `Generated
files:` / `Available files (...):`. The two section headers already
convey the new-vs-known distinction; the per-file annotations are
redundant *and* phrased inconsistently ("downloaded by the user"
vs. "displayed to the user" vs. "known to the user").
Strip both in a small `cleanCodeToolOutput` helper invoked from
`packages/api/src/agents/handlers.ts` for every tool listed in
`CODE_EXECUTION_TOOLS`. Non-code-execution tools pass through
unchanged. The cleaning happens *after* tool resolution but *before*
downstream consumers (model context, SSE forwarding, persistence) see
the content, so subsequent model turns get the lean output.
* 🩹 fix: Polish code-execution attachment rendering
Three rough edges visible in code-interpreter conversations:
1. **Sandbox-internal `.dirkeep` placeholders leak as file chips.** The
bash executor creates `.dirkeep` inside any new directory so the
stateless container preserves the folder across executions. After
`sanitizeArtifactPath`'s `_` prefix and 6-hex collision suffix it
surfaces as `_.dirkeep-<hash>` — a 0-byte chip with no value to the
user, sometimes hiding the real artifact behind it. New
`isInternalSandboxArtifact` helper filters them out of every
routing path (`Attachment`, `AttachmentGroup`, `LogContent`).
2. **The `-<hash>` collision suffix is visible in chip labels.** The
suffix is collision-avoidance machinery; users only need to see the
canonical name. New `displayFilename` strips it for display while
leaving the on-disk `attachment.filename` untouched so downloads
resolve. Applied across `FileContainer`, `ToolArtifactCard`,
`ToolMermaidArtifact`, and `LogContent`'s text-attachment label
path.
3. **0-byte / placeholder files outrank real artifacts in render
order.** Bucket sort by salience (non-empty before empty) sinks
stragglers to the bottom. Stable sort preserves arrival order for
peers.
Added regression tests cover the new helpers, the dirkeep filter
across buckets, and the within-bucket salience ordering.
* 🩹 fix: Don't auto-open artifact panel on history navigation
Navigating to a previous conversation full of code-execution artifacts
would auto-open the side panel and focus the most-recent artifact —
the same code path that fires for fresh streaming artifacts. Users
expect that "auto-open" behavior only when an artifact arrives via
SSE, not when they revisit an old chat.
Two-part gate:
1. `ToolArtifactCard`'s focus effect captures `isSubmitting` at first
render via a ref. A card mounted *during* a stream means a new
artifact arrived → steal panel focus (legacy behavior). A card
mounted while `isSubmitting === false` is part of conversation
history → leave focus alone.
2. `Presentation`'s panel-render condition gains `currentArtifactId
!= null`. With (1) keeping `currentArtifactId` null on history
load, the panel stops rendering at all on navigation — even if
`artifactsVisibility` was left `true` by a prior conversation.
User clicks on a chip to re-open (the click handler is unchanged
and unconditional).
Test seeds `isSubmittingFamily(0)` per case: existing tests opt into
streaming (default `true`) so legacy auto-focus assertions still hold;
new tests for history-load opt into `streaming: false` and verify
no auto-focus + click-to-open still works.
* 🩹 fix: Force panel visible on streaming artifact arrival
The previous commit gated `setCurrentArtifactId` on `isSubmitting` but
left `artifactsVisibility` untouched. When a user had explicitly
closed the panel earlier in the session, a fresh SSE artifact would
set `currentArtifactId` (so the chip read "click to close") but
`Presentation`'s render condition still required `visibility === true`
— net effect: the card claimed to be open, the panel stayed hidden.
Streaming arrivals now also call `setVisible(true)`, which is the
explicit "auto-open when first created" behavior the user asked for.
History mounts (`isSubmitting === false`) still leave both focus and
visibility alone, so navigating to an old conversation does not
re-open the panel.
Two regression tests added: one asserts streaming flips visibility on
even when seeded false, the other asserts history mounts leave a
seeded-false visibility alone.
* 🧹 chore: Tighten code-execution attachment polish per audit feedback
Resolves the eight actionable findings from the comprehensive audit:
- Scope `displayFilename` out of `FileContainer`: opt-in via a new
`displayName` prop. User-uploaded chips (input area, persisted
message files) keep their raw filename, eliminating the false-positive
class where `report-abc123.pdf` was silently rewritten to `report.pdf`.
Code-execution artifact paths in `Attachment.tsx` explicitly compute
the de-suffixed name and pass it through.
- Tighten `TRAILING_NOTES_PATTERN` to anchor on the two known boilerplate
openings (`Files from previous executions`, `Files in "Available files"`),
so a user-authored `Note:` line preceded by a blank line in stdout no
longer gets eaten along with everything after it.
- `ToolMermaidArtifact`: compute `visibleFilename` once and reuse for
title, content, and the download `aria-label` (was using the raw
`attachment.filename` for the aria-label, creating a screen-reader
inconsistency).
- `ToolArtifactCard`: read `isSubmittingFamily(0)` once via a
non-subscribing `useRecoilCallback`, instead of subscribing for the
full lifetime to a value the ref only ever needs at first render.
- Extract `bySalience` and `byEntrySalience` comparators from
`attachmentTypes.ts`, replacing the ten duplicated sort lambdas in
`Attachment.tsx` and `LogContent.tsx`.
- Treat `attachmentSalience({ bytes: undefined })` as neutral (`0`)
rather than empty (`1`); only an explicit `bytes === 0` sinks. Stops
non-code-exec sources (web-search inline results, files where the
schema omits the byte count) from silently sinking past real content.
- Pin the click-history test to the panel-open button by name instead
of relying on `getByRole('button', { pressed: false })`, which
matched by DOM order.
- Add the missing blank line between adjacent `it(...)` blocks.
- Drop the verbose narrating comments in `FileContainer` along with the
removed `displayFilename` import.
Adds three regression tests for the new behavior (FileContainer raw
filename, artifact-context displayName flow, user-authored `Note:` line
preserved through cleanup) and updates the salience test for the new
neutral-undefined semantics.
* 🧹 chore: Drop redundant `@testing-library/jest-dom` import in FileContainer spec
`client/test/setupTests.js` already imports the matchers globally for every
Jest test in the client workspace, so the explicit import here was dead code.
Removing it brings the spec in line with the broader convention used by
`ArtifactRouting.test.tsx`, `LogContent.test.tsx`, and `attachmentTypes.test.ts`.
* 🛡️ fix: Narrow `.dirkeep`/`.gitkeep` filter to the sandbox-specific form
`isInternalSandboxArtifact` was filtering bare `.dirkeep` / `.gitkeep`
along with the post-sanitization form. Bare versions never originate
from the bash executor (the dotfile rewrite + disambiguator step in
`sanitizeArtifactPath` always produces `_.dirkeep-<6 hex>`), so the only
real-world source of a bare `.gitkeep` is project scaffolding the user
uploaded — silently hiding it from every attachment bucket meant the
file disappeared with no way to surface or download it.
Tightening to `^_\.(?:dirkeep|gitkeep)-[0-9a-f]{6}$` keeps the
sandbox-placeholder filter intact while letting user-uploaded markers
render normally. Tests inverted accordingly: bare forms now expected to
render; only the post-sanitization form is filtered.
* 🩹 fix: Address comprehensive-review findings on attachment helpers
Five findings from the latest pass:
- **MAJOR — `displayFilename` false-positive on extensionless 6-hex.**
The previous regex `/-[0-9a-f]{6}(?=\.[^.]+$|$)/` stripped any leaf
ending in `-XXXXXX` regardless of context, so a user-named
`build-a1b2c3` (script-emitted hash artifact, no extension) lost its
tail and rendered as `build`. Split into two narrower patterns:
`COLLISION_SUFFIX_BEFORE_EXT` only matches when followed by an
extension; `SANITIZED_DOTFILE_TRAILING_SUFFIX` only fires when the
leaf starts with `_.` AND ends with `-XXXXXX` — the unambiguous
fingerprint of `sanitizeArtifactPath`'s dotfile rewrite.
- **MINOR — `isInternalSandboxArtifact` filter too aggressive.**
`(file.bytes ?? 0) > 0` treated undefined bytes as zero, falling
through to the regex check. Tightened to `file.bytes !== 0`: only
an *explicit* zero counts as the empty-placeholder shape worth
hiding. Non-code-exec sources without `bytes` populated render
normally now.
- **MINOR — `getValue()` could throw on a degenerate atom state.**
Switched the snapshot read in `ToolArtifactCard` to
`valueMaybe() ?? false` so a transient error / loading state on the
upstream selector doesn't crash card mount. The `false` default is
the right history-fallback (don't auto-open if we can't classify).
- **NIT — `attachmentSalience` / `bySalience` over-broad signature.**
Removed the test-only `{ bytes?: number }` arm; functions now accept
`TAttachment` directly. The internal `bytes` read still goes through
a cast since not every TAttachment branch declares it. Tests updated
to use the existing `baseAttachment(...)` helper.
- **MINOR — Missing regression test for extensionless 6-hex.**
Added `'build-a1b2c3'` and `'out/blob-deadbe'` cases that pin the
preservation behavior, plus an `isInternalSandboxArtifact` test that
asserts undefined-bytes attachments are not filtered.
* 🩹 fix: Make code-file artifacts click-to-open only
Removes mount-time auto-open from `ToolArtifactCard`. Streaming
arrivals no longer hijack the panel — even a freshly-emitted SSE
artifact registers silently in `artifactsState` and waits for the
user to click. Combined with `Presentation`'s
`currentArtifactId != null` render gate, the panel stays closed
across history navigation, page reload, and SSE arrival.
Click is the only path that opens the panel. `handleOpen` is
unchanged: first click focuses + reveals, second click on the same
chip closes.
Dropped:
- `useRecoilCallback` snapshot read of `isSubmittingFamily(0)`
- `mountedDuringStreamRef` ref + lazy-init block
- The whole focus + visibility effect (was effect 3)
- `useRef` import (now unused)
Tests:
- `ArtifactRouting.test.tsx` rewritten to exercise the click path:
registers-on-mount-without-focus, click-to-open-then-close, multi-
card-no-auto-focus, click-when-visibility-was-false. The streaming
state is no longer seeded; both `renderWith` and `renderWithProbe`
collapsed back to plain `RecoilRoot`.
- `LogContent.test.tsx` flips its panel-routing assertions from
`pressed: true` (which asserted auto-focus) to `pressed: false`
with a chip-title check (which asserts the panel card rendered
but stayed unfocused).
* Revert "🩹 fix: Make code-file artifacts click-to-open only"
This reverts commit 6761531287.
* 🩹 fix: Exclude CODE bucket from streaming auto-open
Narrows the previous-commit revert: rich-preview artifacts (HTML,
React, Markdown, plain text) keep the legacy SSE auto-open UX, but
the CODE bucket (`.py`, `.js`, `.cpp`, `Dockerfile`, `Makefile`, …)
stays click-to-open even on streaming.
Source-code artifacts are typically supporting helpers the agent
emits alongside a richer deliverable (a Python script that builds
the actual `.html` output, for example). Auto-opening every
helper's panel each time it gets written would shove the panel
in front of the user every tool call. The user explicitly opens
a code chip when they want to inspect it.
Implementation:
- Focus+open effect skips early when `artifact.type === CODE`.
- `artifact.type` added to the dep array so the gate re-evaluates
if the type ever changes (it shouldn't, but the dep is honest).
- JSDoc updated to call out the carve-out.
Tests:
- New `does NOT auto-open a streaming CODE artifact (test.py is
click-to-open)` — seeds isSubmitting=true, mounts a `.py`,
asserts the artifact registers but currentArtifactId stays null.
- New `clicking a CODE artifact focuses it even though it skipped
auto-open` — confirms the click path still surfaces a `.py`.
- All 25 prior auto-open tests for HTML/React/Markdown/plain-text
buckets still pass unchanged: those types continue to auto-open
on streaming.
* 🧹 chore: Address two NITs from the audit-fix follow-up review
- **NIT #1 (conf 60)**: Add a test for the dotfile-with-extension
intersection (`_.config-abcdef.txt` → `.config.txt`). Both halves
of the path were tested separately — extension-anchored suffix
stripping and `_.` underscore restoration — but the combination
wasn't pinned. Adds `expect(displayFilename('_.config-abcdef.txt'))
.toBe('.config.txt')`.
- **NIT #2 (conf 25)**: Tighten the cast in `attachmentSalience` from
the anonymous `{ bytes?: number }` shape to the concrete
`TFile & TAttachmentMetadata` (the actual TAttachment branch that
declares `bytes`). Same runtime behavior; a future retype of
`TFile.bytes` will now surface here at compile time instead of
being silently papered over.
* 🩹 fix: Stop stripping `-<6 hex>` suffixes from non-dotfile filenames
Codex's repeated P2 was correct: the `COLLISION_SUFFIX_BEFORE_EXT`
regex stripped any `-<6 hex>` immediately before an extension
regardless of context. That collapsed legitimate user-named files
like `report-deadbe.csv` and `report-beef01.csv` onto the same chip
label `report.csv`, silently merging distinct files in the UI.
The structural truth: only the dotfile shape (`_.foo-XXXXXX`) carries
an unambiguous discriminator (the leading `_.` that
`sanitizeArtifactPath` adds when rewriting a leading dot). The
extension-only case (`name-<hash>.ext`) has no such discriminator —
we can't distinguish a sanitized `report 1.csv` (which became
`report_1-<hash>.csv`) from a user-named `report-deadbe.csv` from
the filename alone.
Recovering the non-dotfile case cleanly would require a backend
`wasSanitized` metadata flag we don't have. Without it, the safer
choice is to leave non-dotfile names alone — uglier when the file
*was* sanitized, but never collapses distinct files onto a shared
label.
Changes:
- Drop `COLLISION_SUFFIX_BEFORE_EXT`. Replace
`SANITIZED_DOTFILE_TRAILING_SUFFIX` with a unified
`SANITIZED_DOTFILE_PATTERN` that handles both extensionless and
with-extension dotfile shapes in one regex.
- Simplify `displayFilename` to a single match + reconstruct path.
- Update tests: drop the broad-stripping assertion
(`output-deadbe.csv` → `output.csv`), add explicit codex-regression
cases (`report-deadbe.csv` and `report-beef01.csv` preserve
unchanged), document the deliberate non-recovery for sanitized
non-dotfiles, update the AttachmentGroup→FileContainer integration
test to reflect the narrower stripping (non-dotfile `archive-deadbe.zip`
passes through; new dotfile `_.config-abcdef.zip` → `.config.zip`
exercises the recoverable path).
* 🩹 fix: Scope code-tool annotation stripping to file-list sections
Codex was right: the previous global `.replace` would mutate any line
ending in one of the three annotation phrases — even legitimate
stdout. A user script doing
`echo "foo | File is already downloaded by the user"` had its output
silently scrubbed before being fed back into model context.
New `FILE_SECTION_PATTERN` captures `Generated files:` /
`Available files (...)` blocks (header + lines starting with `- /`).
Annotation stripping now only runs *within* the captured file-list
section via a nested `.replace`, so:
- Inside the section: per-file `| <ann>` suffixes still get stripped
(line-per-file ≥ 4 files form, inline `, ` comma-separated ≤ 3
files form — both already covered by existing patterns).
- Outside the section: stdout, stderr, blank lines, the trailing
`Note:` paragraphs (handled by their own pattern), and any user
text that coincidentally contains an annotation phrase pass
through unchanged.
Tests:
- New `does NOT mutate stdout that legitimately contains an
annotation phrase outside a file-list section` pins the codex
regression: three coincidental phrases in stdout, no
`Generated files:` header, all three preserved verbatim.
- New `strips annotations inside a file-list section but preserves
identical phrases in stdout above it` covers the mixed case where
the same phrase appears in both stdout and a file listing —
stdout survives, listing gets cleaned, exactly one occurrence
remains.
- All 9 prior tests still pass (file-section stripping behavior
unchanged for both line-per-file and inline-comma layouts).
This commit is contained in:
Danny Avila
GitHub
parent
4a5fc701d2
commit
756530c2b8
14 changed files with 1120 additions and 36 deletions
|
|
@ -7,6 +7,7 @@ import RemoveFile from './RemoveFile';
|
|||
const FileContainer = ({
|
||||
file,
|
||||
overrideType,
|
||||
displayName,
|
||||
buttonClassName,
|
||||
containerClassName,
|
||||
onDelete,
|
||||
|
|
@ -14,12 +15,19 @@ const FileContainer = ({
|
|||
}: {
|
||||
file: Partial<ExtendedFile | TFile>;
|
||||
overrideType?: string;
|
||||
/**
|
||||
* Optional pre-computed label for the chip. Callers in code-execution
|
||||
* artifact contexts pass the de-suffixed name; upload chips and
|
||||
* persisted user files leave this undefined and render the raw filename.
|
||||
*/
|
||||
displayName?: string;
|
||||
buttonClassName?: string;
|
||||
containerClassName?: string;
|
||||
onDelete?: () => void;
|
||||
onClick?: React.MouseEventHandler<HTMLButtonElement>;
|
||||
}) => {
|
||||
const fileType = getFileType(overrideType ?? file.type);
|
||||
const visibleName = displayName ?? file.filename ?? '';
|
||||
|
||||
return (
|
||||
<div
|
||||
|
|
@ -28,7 +36,7 @@ const FileContainer = ({
|
|||
<button
|
||||
type="button"
|
||||
onClick={onClick}
|
||||
aria-label={file.filename}
|
||||
aria-label={visibleName}
|
||||
className={cn(
|
||||
'relative overflow-hidden rounded-2xl border border-border-light bg-surface-hover-alt',
|
||||
buttonClassName,
|
||||
|
|
@ -38,8 +46,8 @@ const FileContainer = ({
|
|||
<div className="flex flex-row items-center gap-2">
|
||||
<FilePreview file={file} fileType={fileType} className="relative" />
|
||||
<div className="overflow-hidden">
|
||||
<div className="truncate font-medium" title={file.filename}>
|
||||
{file.filename}
|
||||
<div className="truncate font-medium" title={visibleName}>
|
||||
{visibleName}
|
||||
</div>
|
||||
<div className="truncate text-text-secondary" title={fileType.title}>
|
||||
{fileType.title}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,56 @@
|
|||
import React from 'react';
|
||||
import { render, screen } from '@testing-library/react';
|
||||
import type { TFile } from 'librechat-data-provider';
|
||||
import FileContainer from '../FileContainer';
|
||||
|
||||
jest.mock('~/utils', () => ({
|
||||
cn: (...classes: Array<string | false | null | undefined>) => classes.filter(Boolean).join(' '),
|
||||
getFileType: () => ({ paths: [], color: '', title: 'Plain' }),
|
||||
}));
|
||||
|
||||
jest.mock('../FilePreview', () => ({
|
||||
__esModule: true,
|
||||
default: () => <div data-testid="file-preview" />,
|
||||
}));
|
||||
|
||||
jest.mock('../RemoveFile', () => ({
|
||||
__esModule: true,
|
||||
default: () => <button data-testid="remove-file" />,
|
||||
}));
|
||||
|
||||
const baseFile = (overrides: Partial<TFile> = {}): Partial<TFile> => ({
|
||||
file_id: 'f1',
|
||||
filename: 'report.pdf',
|
||||
type: 'application/pdf',
|
||||
...overrides,
|
||||
});
|
||||
|
||||
describe('FileContainer chip label', () => {
|
||||
it('shows the raw filename when no `displayName` is supplied (upload context)', () => {
|
||||
/** A user-uploaded file whose name happens to look like the
|
||||
* code-execution collision suffix (`-<6 hex>` before extension) must
|
||||
* not have its name silently rewritten — historically a global
|
||||
* `displayFilename(file.filename)` call here would strip the suffix
|
||||
* and turn `report-abc123.pdf` into `report.pdf`. Stripping is now
|
||||
* opt-in via `displayName`, so upload chips show the raw name. */
|
||||
render(<FileContainer file={baseFile({ filename: 'report-abc123.pdf' })} />);
|
||||
expect(screen.getByText('report-abc123.pdf')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('uses `displayName` when supplied (artifact context opts in)', () => {
|
||||
render(
|
||||
<FileContainer
|
||||
file={baseFile({ filename: 'archive-deadbe.zip', type: 'application/zip' })}
|
||||
displayName="archive.zip"
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByText('archive.zip')).toBeInTheDocument();
|
||||
expect(screen.queryByText(/-deadbe/)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('falls back to empty string when neither `displayName` nor `filename` is set', () => {
|
||||
const { container } = render(<FileContainer file={{ file_id: 'noname' } as Partial<TFile>} />);
|
||||
/** Title element exists but is empty — no crash, no `undefined`. */
|
||||
expect(container.querySelector('.font-medium')?.textContent).toBe('');
|
||||
});
|
||||
});
|
||||
|
|
@ -4,7 +4,11 @@ import type { TAttachment, TFile, TAttachmentMetadata } from 'librechat-data-pro
|
|||
import type { ToolArtifactType } from '~/utils/artifacts';
|
||||
import {
|
||||
artifactTypeForAttachment,
|
||||
bySalience,
|
||||
byEntrySalience,
|
||||
displayFilename,
|
||||
isImageAttachment,
|
||||
isInternalSandboxArtifact,
|
||||
isTextAttachment,
|
||||
renderAttachmentKey,
|
||||
} from './attachmentTypes';
|
||||
|
|
@ -56,6 +60,7 @@ const FileAttachment = memo(({ attachment }: { attachment: Partial<TAttachment>
|
|||
file={attachment}
|
||||
onClick={handleDownload}
|
||||
overrideType={extension}
|
||||
displayName={displayFilename(attachment.filename)}
|
||||
containerClassName="max-w-fit"
|
||||
buttonClassName="bg-surface-secondary hover:cursor-pointer hover:bg-surface-hover active:bg-surface-secondary focus:bg-surface-hover hover:border-border-heavy active:border-border-heavy"
|
||||
/>
|
||||
|
|
@ -117,6 +122,7 @@ const TextAttachment = memo(({ attachment }: { attachment: Partial<TAttachment>
|
|||
file={attachment}
|
||||
onClick={handleDownload}
|
||||
overrideType={extension}
|
||||
displayName={displayFilename(attachment.filename)}
|
||||
containerClassName="max-w-fit"
|
||||
buttonClassName="bg-surface-secondary hover:cursor-pointer hover:bg-surface-hover active:bg-surface-secondary focus:bg-surface-hover hover:border-border-heavy active:border-border-heavy"
|
||||
/>
|
||||
|
|
@ -224,6 +230,12 @@ export default function Attachment({ attachment }: { attachment?: TAttachment })
|
|||
if (attachment.type === Tools.web_search) {
|
||||
return null;
|
||||
}
|
||||
// Sandbox-internal placeholders (`.dirkeep` etc.) are an implementation
|
||||
// detail of the bash executor's empty-folder preservation; users have
|
||||
// no reason to see them as their own file chips.
|
||||
if (isInternalSandboxArtifact(attachment)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (isImageAttachment(attachment)) {
|
||||
return <ImageAttachment attachment={attachment} />;
|
||||
|
|
@ -262,6 +274,9 @@ export function AttachmentGroup({ attachments }: { attachments?: TAttachment[] }
|
|||
if (attachment.type === Tools.web_search) {
|
||||
return;
|
||||
}
|
||||
if (isInternalSandboxArtifact(attachment)) {
|
||||
return;
|
||||
}
|
||||
if (isImageAttachment(attachment)) {
|
||||
imageAttachments.push(attachment);
|
||||
return;
|
||||
|
|
@ -282,6 +297,15 @@ export function AttachmentGroup({ attachments }: { attachments?: TAttachment[] }
|
|||
fileAttachments.push(attachment);
|
||||
});
|
||||
|
||||
// Sink empty / placeholder-shaped files in each bucket so the user's
|
||||
// eye lands on the real artifact first. `sort` is stable in modern
|
||||
// engines (V8 ≥ 7.0) so equal-weight entries keep their input order.
|
||||
fileAttachments.sort(bySalience);
|
||||
textAttachments.sort(bySalience);
|
||||
panelArtifacts.sort(byEntrySalience);
|
||||
mermaidArtifacts.sort(bySalience);
|
||||
imageAttachments.sort(bySalience);
|
||||
|
||||
return (
|
||||
<>
|
||||
{fileAttachments.length > 0 && (
|
||||
|
|
|
|||
|
|
@ -5,6 +5,10 @@ import type { TFile, TAttachment, TAttachmentMetadata } from 'librechat-data-pro
|
|||
import type { Artifact } from '~/common';
|
||||
import {
|
||||
artifactTypeForAttachment,
|
||||
bySalience,
|
||||
byEntrySalience,
|
||||
displayFilename,
|
||||
isInternalSandboxArtifact,
|
||||
isTextAttachment,
|
||||
renderAttachmentKey,
|
||||
} from './attachmentTypes';
|
||||
|
|
@ -61,6 +65,11 @@ const LogContent: React.FC<LogContentProps> = ({ output = '', renderImages, atta
|
|||
|
||||
const now = new Date();
|
||||
attachments?.forEach((attachment) => {
|
||||
// Sandbox-internal placeholders (`.dirkeep` etc.) are
|
||||
// implementation detail — never list them as their own files.
|
||||
if (isInternalSandboxArtifact(attachment)) {
|
||||
return;
|
||||
}
|
||||
const fileData = attachment as TFile & TAttachmentMetadata;
|
||||
const { filepath = null } = fileData;
|
||||
// LogContent uses a looser image check than Attachment.tsx (no
|
||||
|
|
@ -110,6 +119,15 @@ const LogContent: React.FC<LogContentProps> = ({ output = '', renderImages, atta
|
|||
otherAtts.push(attachment);
|
||||
});
|
||||
|
||||
// Sink empty / placeholder files in each bucket so the user's eye
|
||||
// lands on the real artifact first. Stable sort preserves the
|
||||
// arrival order among non-empty entries.
|
||||
imageAtts.sort(bySalience);
|
||||
textAtts.sort(bySalience);
|
||||
panelAtts.sort(byEntrySalience);
|
||||
mermaidAtts.sort(byEntrySalience);
|
||||
otherAtts.sort(bySalience);
|
||||
|
||||
return {
|
||||
imageAttachments: renderImages === true ? imageAtts : null,
|
||||
textAttachments: textAtts,
|
||||
|
|
@ -125,9 +143,10 @@ const LogContent: React.FC<LogContentProps> = ({ output = '', renderImages, atta
|
|||
'expiresAt' in file && typeof file.expiresAt === 'number' ? new Date(file.expiresAt) : null;
|
||||
const isExpired = expiresAt ? isAfter(now, expiresAt) : false;
|
||||
const filename = file.filename || '';
|
||||
const visibleName = displayFilename(filename);
|
||||
|
||||
if (isExpired) {
|
||||
return `${filename} ${localize('com_download_expired')}`;
|
||||
return `${visibleName} ${localize('com_download_expired')}`;
|
||||
}
|
||||
|
||||
const fileData = file as TFile & TAttachmentMetadata;
|
||||
|
|
@ -142,7 +161,7 @@ const LogContent: React.FC<LogContentProps> = ({ output = '', renderImages, atta
|
|||
source={fileData.source}
|
||||
>
|
||||
{'- '}
|
||||
{filename} {localize('com_click_to_download')}
|
||||
{visibleName} {localize('com_click_to_download')}
|
||||
</LogLink>
|
||||
);
|
||||
};
|
||||
|
|
@ -200,10 +219,10 @@ const LogContent: React.FC<LogContentProps> = ({ output = '', renderImages, atta
|
|||
user={file.user}
|
||||
source={file.source}
|
||||
>
|
||||
{file.filename}
|
||||
{displayFilename(file.filename)}
|
||||
</LogLink>
|
||||
) : (
|
||||
file.filename
|
||||
displayFilename(file.filename)
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
|
|
|
|||
|
|
@ -1,9 +1,17 @@
|
|||
import { memo, useEffect, useId, useLayoutEffect } from 'react';
|
||||
import { memo, useEffect, useId, useLayoutEffect, useRef } from 'react';
|
||||
import { Download } from 'lucide-react';
|
||||
import { useRecoilState, useRecoilValue, useResetRecoilState, useSetRecoilState } from 'recoil';
|
||||
import {
|
||||
useRecoilCallback,
|
||||
useRecoilState,
|
||||
useRecoilValue,
|
||||
useResetRecoilState,
|
||||
useSetRecoilState,
|
||||
} from 'recoil';
|
||||
import type { TAttachment, TFile, TAttachmentMetadata } from 'librechat-data-provider';
|
||||
import type { Artifact } from '~/common';
|
||||
import FilePreview from '~/components/Chat/Input/Files/FilePreview';
|
||||
import { TOOL_ARTIFACT_TYPES } from '~/utils/artifacts';
|
||||
import { displayFilename } from './attachmentTypes';
|
||||
import { useAttachmentLink } from './LogLink';
|
||||
import { useLocalize } from '~/hooks';
|
||||
import { cn, getFileType } from '~/utils';
|
||||
|
|
@ -41,15 +49,23 @@ interface ToolArtifactCardProps {
|
|||
* Without that guard, both cards would observe each other's write
|
||||
* and trade overwrites in a loop.
|
||||
*
|
||||
* 3. **Focus on mount** (deps: artifact.id). A freshly-mounted card
|
||||
* means a new artifact has arrived; we steal panel focus to match
|
||||
* the legacy streaming-artifact UX where the latest artifact
|
||||
* auto-opens. Cards that re-render with the same artifact don't
|
||||
* refire this, so user clicks on older cards aren't overridden.
|
||||
*
|
||||
* Visibility is intentionally not toggled. The Recoil default is `true`,
|
||||
* which auto-opens the panel on first registration; a user who has
|
||||
* explicitly closed the panel keeps it closed until they click.
|
||||
* 3. **Focus + open on mount** (deps: artifact.id, artifact.type) —
|
||||
* gated on `isSubmitting` captured at first render via a ref AND
|
||||
* on `artifact.type !== CODE`. A card mounted *during* streaming
|
||||
* for a rich-preview bucket (HTML, React, Markdown, plain text)
|
||||
* steals panel focus and forces `artifactsVisibility = true` so
|
||||
* the panel auto-opens — matching the legacy SSE auto-open UX.
|
||||
* A card mounted while `isSubmitting === false` is part of
|
||||
* conversation history (page load, back-navigation) and must not
|
||||
* steal focus — `Presentation`'s render condition gates on
|
||||
* `currentArtifactId != null`, so leaving both alone keeps the
|
||||
* panel closed on history load. The CODE bucket (`.py`, `.js`,
|
||||
* `Dockerfile`, …) is click-to-open *even on streaming*: source
|
||||
* files are typically supporting scripts the agent emits alongside
|
||||
* a richer deliverable, and shoving the panel in front of the
|
||||
* user every time a helper script gets written is disruptive.
|
||||
* Click-to-open via `handleOpen` works for every bucket regardless
|
||||
* of context.
|
||||
*/
|
||||
const ToolArtifactCard = memo(({ attachment, artifact }: ToolArtifactCardProps) => {
|
||||
const localize = useLocalize();
|
||||
|
|
@ -63,6 +79,30 @@ const ToolArtifactCard = memo(({ attachment, artifact }: ToolArtifactCardProps)
|
|||
const [claim, setClaim] = useRecoilState(store.toolArtifactClaim(artifact.id));
|
||||
const isSelected = artifact.id === currentArtifactId;
|
||||
const isMyClaim = claim === claimKey;
|
||||
/**
|
||||
* Captured at first render via a non-subscribing snapshot read so the
|
||||
* downstream effect doesn't re-fire (and the component doesn't
|
||||
* re-render) every time `isSubmittingFamily(0)` flips. Cards that mount
|
||||
* mid-stream stay "fresh" for the rest of their lifetime; cards that
|
||||
* mount post-stream stay "history" even if the user sends a new
|
||||
* message while this card stays mounted.
|
||||
*/
|
||||
const readInitialIsSubmitting = useRecoilCallback(
|
||||
({ snapshot }) =>
|
||||
() =>
|
||||
// `valueMaybe()` returns `undefined` if the atom is in an error
|
||||
// or loading state instead of throwing — defensive against an
|
||||
// upstream selector failure surfacing during card mount. The
|
||||
// `?? false` default is correct because a card we can't classify
|
||||
// as streaming is one we should treat as history (don't steal
|
||||
// focus / open the panel).
|
||||
snapshot.getLoadable(store.isSubmittingFamily(0)).valueMaybe() ?? false,
|
||||
[],
|
||||
);
|
||||
const mountedDuringStreamRef = useRef<boolean | null>(null);
|
||||
if (mountedDuringStreamRef.current === null) {
|
||||
mountedDuringStreamRef.current = readInitialIsSubmitting();
|
||||
}
|
||||
|
||||
useLayoutEffect(() => {
|
||||
// Always (re)claim on mount — a later card for the same id displaces
|
||||
|
|
@ -96,8 +136,29 @@ 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 (artifact.type === TOOL_ARTIFACT_TYPES.CODE) {
|
||||
// Source-code artifacts (`.py`, `.js`, `.cpp`, `Dockerfile`, …) are
|
||||
// click-to-open only. They're typically supporting scripts the
|
||||
// agent emits alongside a richer deliverable; auto-opening them
|
||||
// would shove the panel in front of the user every time a tool
|
||||
// call writes a helper file. The rich-preview buckets (HTML,
|
||||
// React, Markdown, plain text) keep the legacy auto-open UX so
|
||||
// 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.
|
||||
setCurrentArtifactId(artifact.id);
|
||||
}, [artifact.id, setCurrentArtifactId]);
|
||||
setVisible(true);
|
||||
}, [artifact.id, artifact.type, setCurrentArtifactId, setVisible]);
|
||||
|
||||
const file = attachment as TFile & TAttachmentMetadata;
|
||||
const { handleDownload } = useAttachmentLink({
|
||||
|
|
@ -130,6 +191,11 @@ const ToolArtifactCard = memo(({ attachment, artifact }: ToolArtifactCardProps)
|
|||
const actionLabel = isSelected
|
||||
? localize('com_ui_click_to_close')
|
||||
: localize('com_ui_artifact_click');
|
||||
const visibleFilename = displayFilename(attachment.filename);
|
||||
// The artifact's stored `title` mirrors the on-disk `filename` for
|
||||
// tool artifacts, so re-derive the user-facing label rather than
|
||||
// showing the collision-suffixed name.
|
||||
const visibleTitle = displayFilename(artifact.title);
|
||||
|
||||
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">
|
||||
|
|
@ -149,8 +215,8 @@ const ToolArtifactCard = memo(({ attachment, artifact }: ToolArtifactCardProps)
|
|||
<div className="flex flex-row items-center gap-2">
|
||||
<FilePreview fileType={fileType} className="relative" />
|
||||
<div className="overflow-hidden text-left">
|
||||
<div className="truncate font-medium" title={attachment.filename ?? ''}>
|
||||
{artifact.title}
|
||||
<div className="truncate font-medium" title={visibleFilename}>
|
||||
{visibleTitle}
|
||||
</div>
|
||||
<div className="truncate text-xs text-text-secondary">{actionLabel}</div>
|
||||
</div>
|
||||
|
|
@ -160,7 +226,7 @@ const ToolArtifactCard = memo(({ attachment, artifact }: ToolArtifactCardProps)
|
|||
<button
|
||||
type="button"
|
||||
onClick={handleDownload}
|
||||
aria-label={`${localize('com_ui_download')} ${attachment.filename ?? ''}`}
|
||||
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',
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import { useRecoilState } from 'recoil';
|
|||
import type { TAttachment, TFile, TAttachmentMetadata } from 'librechat-data-provider';
|
||||
import Mermaid from '~/components/Messages/Content/Mermaid/Mermaid';
|
||||
import { toolArtifactKey } from '~/utils/artifacts';
|
||||
import { displayFilename } from './attachmentTypes';
|
||||
import { useAttachmentLink } from './LogLink';
|
||||
import { useLocalize } from '~/hooks';
|
||||
import { cn } from '~/utils';
|
||||
|
|
@ -49,6 +50,8 @@ const ToolMermaidArtifact = memo(({ attachment, text }: ToolMermaidArtifactProps
|
|||
return null;
|
||||
}
|
||||
|
||||
const visibleFilename = displayFilename(attachment.filename);
|
||||
|
||||
return (
|
||||
<div className="my-2 flex w-full flex-col gap-1">
|
||||
{(attachment.filename || attachment.filepath) && (
|
||||
|
|
@ -56,16 +59,16 @@ const ToolMermaidArtifact = memo(({ attachment, text }: ToolMermaidArtifactProps
|
|||
{attachment.filename && (
|
||||
<div
|
||||
className="truncate text-[10px] font-medium uppercase tracking-wide text-text-secondary"
|
||||
title={attachment.filename}
|
||||
title={visibleFilename}
|
||||
>
|
||||
{attachment.filename}
|
||||
{visibleFilename}
|
||||
</div>
|
||||
)}
|
||||
{attachment.filepath && (
|
||||
<button
|
||||
type="button"
|
||||
onClick={handleDownload}
|
||||
aria-label={`${localize('com_ui_download')} ${attachment.filename ?? ''}`}
|
||||
aria-label={`${localize('com_ui_download')} ${visibleFilename}`}
|
||||
title={localize('com_ui_download')}
|
||||
className={cn(
|
||||
'inline-flex shrink-0 items-center gap-1 rounded-md px-2 py-1 text-xs',
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import React from 'react';
|
||||
import { render, screen, fireEvent, act } from '@testing-library/react';
|
||||
import { RecoilRoot, useRecoilValue } from 'recoil';
|
||||
import type { MutableSnapshot } from 'recoil';
|
||||
import type { TAttachment } from 'librechat-data-provider';
|
||||
import Attachment, { AttachmentGroup } from '../Attachment';
|
||||
import store from '~/store';
|
||||
|
|
@ -18,8 +19,8 @@ jest.mock('../LogLink', () => ({
|
|||
|
||||
jest.mock('~/components/Chat/Input/Files/FileContainer', () => ({
|
||||
__esModule: true,
|
||||
default: ({ file }: { file: { filename?: string } }) => (
|
||||
<div data-testid="file-container">{file.filename ?? ''}</div>
|
||||
default: ({ file, displayName }: { file: { filename?: string }; displayName?: string }) => (
|
||||
<div data-testid="file-container">{displayName ?? file.filename ?? ''}</div>
|
||||
),
|
||||
}));
|
||||
|
||||
|
|
@ -56,7 +57,19 @@ const baseAttachment = (overrides: Partial<TAttachment> = {}): TAttachment =>
|
|||
...overrides,
|
||||
}) as TAttachment;
|
||||
|
||||
const renderWith = (ui: React.ReactElement) => render(<RecoilRoot>{ui}</RecoilRoot>);
|
||||
/**
|
||||
* Seeds `isSubmittingFamily(0) = streaming` so `ToolArtifactCard`'s
|
||||
* mount-time focus effect (which is gated on streaming state) behaves
|
||||
* the way each test expects. Default `streaming: true` matches the
|
||||
* legacy SSE-arrival flow that the bulk of these tests exercise.
|
||||
*/
|
||||
const renderWith = (ui: React.ReactElement, opts: { streaming?: boolean } = {}) => {
|
||||
const streaming = opts.streaming ?? true;
|
||||
const initializeState = (snapshot: MutableSnapshot) => {
|
||||
snapshot.set(store.isSubmittingFamily(0), streaming);
|
||||
};
|
||||
return render(<RecoilRoot initializeState={initializeState}>{ui}</RecoilRoot>);
|
||||
};
|
||||
|
||||
interface ArtifactsSnapshot {
|
||||
visibility: boolean;
|
||||
|
|
@ -78,14 +91,18 @@ const StateProbe = ({ onSnapshot }: { onSnapshot: (snap: ArtifactsSnapshot) => v
|
|||
return null;
|
||||
};
|
||||
|
||||
const renderWithProbe = (ui: React.ReactElement) => {
|
||||
const renderWithProbe = (ui: React.ReactElement, opts: { streaming?: boolean } = {}) => {
|
||||
const streaming = opts.streaming ?? true;
|
||||
const initializeState = (snap: MutableSnapshot) => {
|
||||
snap.set(store.isSubmittingFamily(0), streaming);
|
||||
};
|
||||
let snapshot: ArtifactsSnapshot = {
|
||||
visibility: false,
|
||||
currentArtifactId: null,
|
||||
artifactIds: [],
|
||||
};
|
||||
const utils = render(
|
||||
<RecoilRoot>
|
||||
<RecoilRoot initializeState={initializeState}>
|
||||
<StateProbe
|
||||
onSnapshot={(snap) => {
|
||||
snapshot = snap;
|
||||
|
|
@ -353,9 +370,267 @@ describe('ToolArtifactCard click behaviour', () => {
|
|||
expect.arrayContaining(['tool-artifact-older', 'tool-artifact-newer']),
|
||||
);
|
||||
});
|
||||
|
||||
it('does NOT auto-focus when the card mounts outside an active stream (history load)', () => {
|
||||
// Reproduces "navigating to a previous conversation". Cards mount
|
||||
// for already-completed turns; `isSubmitting` is false. The legacy
|
||||
// behavior would auto-focus the latest artifact and re-open the
|
||||
// panel — we don't want that.
|
||||
const html = baseAttachment({
|
||||
file_id: 'history-html',
|
||||
filename: 'previous.html',
|
||||
text: '<h1>prev</h1>',
|
||||
} as Partial<TAttachment>);
|
||||
const { getSnapshot } = renderWithProbe(<Attachment attachment={html} />, {
|
||||
streaming: false,
|
||||
});
|
||||
const snap = getSnapshot();
|
||||
// Artifact still gets registered (so the panel can find it on click)…
|
||||
expect(snap.artifactIds).toContain('tool-artifact-history-html');
|
||||
// …but currentArtifactId stays null, which means
|
||||
// `Presentation`'s render gate (`currentArtifactId != null`) keeps
|
||||
// the panel closed.
|
||||
expect(snap.currentArtifactId).toBeNull();
|
||||
});
|
||||
|
||||
it('does NOT auto-open a streaming CODE artifact (test.py is click-to-open)', () => {
|
||||
// Source-code artifacts are excluded from streaming auto-open even
|
||||
// when isSubmitting=true. The agent often emits supporting `.py` /
|
||||
// `.js` / `Dockerfile` / etc. helpers alongside a richer
|
||||
// deliverable; the panel shouldn't hijack the viewport every time
|
||||
// a script gets written. Click is the only path that surfaces a
|
||||
// CODE artifact in the panel.
|
||||
const py = baseAttachment({
|
||||
file_id: 'helper-script',
|
||||
filename: 'test.py',
|
||||
text: 'print("hello")',
|
||||
} as Partial<TAttachment>);
|
||||
const initializeState = (snap: MutableSnapshot) => {
|
||||
snap.set(store.isSubmittingFamily(0), true);
|
||||
};
|
||||
let snapshot: ArtifactsSnapshot = {
|
||||
visibility: false,
|
||||
currentArtifactId: null,
|
||||
artifactIds: [],
|
||||
};
|
||||
render(
|
||||
<RecoilRoot initializeState={initializeState}>
|
||||
<StateProbe
|
||||
onSnapshot={(snap) => {
|
||||
snapshot = snap;
|
||||
}}
|
||||
/>
|
||||
<Attachment attachment={py} />
|
||||
</RecoilRoot>,
|
||||
);
|
||||
// Artifact registered (so the panel can find it on click)…
|
||||
expect(snapshot.artifactIds).toContain('tool-artifact-helper-script');
|
||||
// …but currentArtifactId stays null and visibility is untouched.
|
||||
expect(snapshot.currentArtifactId).toBeNull();
|
||||
});
|
||||
|
||||
it('forces panel visibility on streaming mount even if visibility was previously false', () => {
|
||||
// Repro: user closed the panel earlier in the session
|
||||
// (`artifactsVisibility = false`), then a new tool artifact
|
||||
// arrives via SSE. The old behavior set `currentArtifactId` but
|
||||
// left visibility alone, so the chip showed "click to close" while
|
||||
// the panel stayed hidden. Streaming arrivals must re-open the
|
||||
// panel — that's the explicit "auto-open when first created" rule.
|
||||
const html = baseAttachment({
|
||||
file_id: 'fresh-stream',
|
||||
filename: 'fresh.html',
|
||||
text: '<h1>fresh</h1>',
|
||||
} as Partial<TAttachment>);
|
||||
const initializeState = (snap: MutableSnapshot) => {
|
||||
snap.set(store.isSubmittingFamily(0), true);
|
||||
snap.set(store.artifactsVisibility, false);
|
||||
};
|
||||
let snapshot: ArtifactsSnapshot = {
|
||||
visibility: false,
|
||||
currentArtifactId: null,
|
||||
artifactIds: [],
|
||||
};
|
||||
render(
|
||||
<RecoilRoot initializeState={initializeState}>
|
||||
<StateProbe
|
||||
onSnapshot={(snap) => {
|
||||
snapshot = snap;
|
||||
}}
|
||||
/>
|
||||
<Attachment attachment={html} />
|
||||
</RecoilRoot>,
|
||||
);
|
||||
expect(snapshot.visibility).toBe(true);
|
||||
expect(snapshot.currentArtifactId).toBe('tool-artifact-fresh-stream');
|
||||
});
|
||||
|
||||
it('does NOT force visibility on history mount (closed panel stays closed)', () => {
|
||||
// The flip-side of the prior test: a card mounted from history
|
||||
// (isSubmitting=false) must not toggle visibility, so a user who
|
||||
// explicitly closed the panel keeps it closed when revisiting.
|
||||
const html = baseAttachment({
|
||||
file_id: 'history-no-vis',
|
||||
filename: 'historic.html',
|
||||
text: '<h1>old</h1>',
|
||||
} as Partial<TAttachment>);
|
||||
const initializeState = (snap: MutableSnapshot) => {
|
||||
snap.set(store.isSubmittingFamily(0), false);
|
||||
snap.set(store.artifactsVisibility, false);
|
||||
};
|
||||
let snapshot: ArtifactsSnapshot = {
|
||||
visibility: false,
|
||||
currentArtifactId: null,
|
||||
artifactIds: [],
|
||||
};
|
||||
render(
|
||||
<RecoilRoot initializeState={initializeState}>
|
||||
<StateProbe
|
||||
onSnapshot={(snap) => {
|
||||
snapshot = snap;
|
||||
}}
|
||||
/>
|
||||
<Attachment attachment={html} />
|
||||
</RecoilRoot>,
|
||||
);
|
||||
expect(snapshot.visibility).toBe(false);
|
||||
expect(snapshot.currentArtifactId).toBeNull();
|
||||
});
|
||||
|
||||
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
|
||||
// streaming mount the user can click to open; the carve-out only
|
||||
// affects the *automatic* open, not the explicit one.
|
||||
const py = baseAttachment({
|
||||
file_id: 'click-py',
|
||||
filename: 'helper.py',
|
||||
text: 'print("hi")',
|
||||
} as Partial<TAttachment>);
|
||||
const { getSnapshot } = renderWithProbe(<Attachment attachment={py} />);
|
||||
expect(getSnapshot().currentArtifactId).toBeNull();
|
||||
const openButton = screen.getByRole('button', { name: /com_ui_artifact_click/i });
|
||||
act(() => {
|
||||
fireEvent.click(openButton);
|
||||
});
|
||||
expect(getSnapshot().currentArtifactId).toBe('tool-artifact-click-py');
|
||||
expect(getSnapshot().visibility).toBe(true);
|
||||
});
|
||||
|
||||
it('clicking a history-loaded card focuses it (user-initiated open)', () => {
|
||||
// Even though the mount-time auto-focus is suppressed for history,
|
||||
// the click handler is unconditional — users explicitly opening a
|
||||
// chip must always work.
|
||||
const html = baseAttachment({
|
||||
file_id: 'history-click',
|
||||
filename: 'previous.html',
|
||||
text: '<h1>prev</h1>',
|
||||
} as Partial<TAttachment>);
|
||||
const { getSnapshot } = renderWithProbe(<Attachment attachment={html} />, {
|
||||
streaming: false,
|
||||
});
|
||||
expect(getSnapshot().currentArtifactId).toBeNull();
|
||||
/** Pin to the panel-open button by name — the download button has no
|
||||
* `aria-pressed`, but `getByRole('button', { pressed: false })`
|
||||
* relies on DOM order, which silently shifts if the chip's button
|
||||
* order changes. */
|
||||
const openButton = screen.getByRole('button', { name: /com_ui_artifact_click/i });
|
||||
act(() => {
|
||||
fireEvent.click(openButton);
|
||||
});
|
||||
expect(getSnapshot().currentArtifactId).toBe('tool-artifact-history-click');
|
||||
expect(getSnapshot().visibility).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('AttachmentGroup routing', () => {
|
||||
it('filters internal sandbox `.dirkeep` placeholders out of every bucket', () => {
|
||||
// The bash executor's empty-folder marker (`_.dirkeep-<hash>`,
|
||||
// `bytes: 0`) is implementation detail; users shouldn't see it as
|
||||
// its own chip. The filter runs ahead of all routing so the
|
||||
// placeholder doesn't leak into image / panel / text / file buckets.
|
||||
const realFile = baseAttachment({
|
||||
file_id: 'real',
|
||||
filename: 'test_folder/test_file.txt',
|
||||
text: 'hello',
|
||||
bytes: 5,
|
||||
} as Partial<TAttachment>);
|
||||
const dirkeep = baseAttachment({
|
||||
file_id: 'dk',
|
||||
filename: 'test_folder/_.dirkeep-88b30b',
|
||||
bytes: 0,
|
||||
} as Partial<TAttachment>);
|
||||
const { container } = renderWith(<AttachmentGroup attachments={[dirkeep, realFile]} />);
|
||||
// No chip rendered for the dirkeep placeholder.
|
||||
expect(screen.queryByText(/dirkeep/)).not.toBeInTheDocument();
|
||||
expect(container.textContent).not.toMatch(/dirkeep/);
|
||||
// Real file still renders.
|
||||
expect(container.textContent).toMatch(/test_file\.txt/);
|
||||
});
|
||||
|
||||
it('sinks empty files below non-empty siblings within the file bucket', () => {
|
||||
// Without the salience sort, an early-arriving 0-byte file would
|
||||
// render first and visually upstage the real artifact below it.
|
||||
// Both files route to the `fileAttachments` bucket (no text, no
|
||||
// panel-eligible extension) so the test exercises within-bucket
|
||||
// ordering — sort is per-bucket, not cross-bucket.
|
||||
const empty = baseAttachment({
|
||||
file_id: 'empty-zip',
|
||||
filename: 'placeholder.zip',
|
||||
type: 'application/zip',
|
||||
bytes: 0,
|
||||
} as Partial<TAttachment>);
|
||||
const real = baseAttachment({
|
||||
file_id: 'real-zip',
|
||||
filename: 'archive.zip',
|
||||
type: 'application/zip',
|
||||
bytes: 1024,
|
||||
} as Partial<TAttachment>);
|
||||
const { container } = renderWith(<AttachmentGroup attachments={[empty, real]} />);
|
||||
const chips = Array.from(container.querySelectorAll('[data-testid="file-container"]'));
|
||||
expect(chips.length).toBe(2);
|
||||
const filenames = chips.map((c) => c.textContent ?? '');
|
||||
// Real chip must render before the empty placeholder.
|
||||
expect(filenames[0]).toMatch(/archive\.zip/);
|
||||
expect(filenames[1]).toMatch(/placeholder\.zip/);
|
||||
});
|
||||
|
||||
it('passes a non-dotfile filename through to FileContainer unchanged', () => {
|
||||
/** `displayFilename` deliberately leaves non-dotfile names alone —
|
||||
* the `-<6 hex>` tail on `archive-deadbe.zip` could be either a
|
||||
* user-named hash artifact OR a sanitization disambiguator, and
|
||||
* collapsing both onto `archive.zip` would silently merge distinct
|
||||
* files in the chip. Only the leading-`_.` dotfile shape has a
|
||||
* structural discriminator strong enough to safely strip; everything
|
||||
* else passes through verbatim. */
|
||||
const sandboxFile = baseAttachment({
|
||||
file_id: 'sandbox-zip',
|
||||
filename: 'archive-deadbe.zip',
|
||||
type: 'application/zip',
|
||||
bytes: 1024,
|
||||
} as Partial<TAttachment>);
|
||||
const { container } = renderWith(<AttachmentGroup attachments={[sandboxFile]} />);
|
||||
const chip = container.querySelector('[data-testid="file-container"]');
|
||||
expect(chip?.textContent).toBe('archive-deadbe.zip');
|
||||
});
|
||||
|
||||
it('strips the suffix and restores the dot for a sandbox dotfile rendered through FileContainer', () => {
|
||||
/** Counterpart: the dotfile shape `_.config-<hash>.zip` IS recoverable
|
||||
* because the leading `_.` is the structural fingerprint. Confirm
|
||||
* the artifact path's `displayName` flows through `FileContainer`
|
||||
* for this case. Uses `.zip` (no panel-artifact extension mapping)
|
||||
* so the attachment lands in the plain-file bucket rather than the
|
||||
* PLAIN_TEXT panel-artifact bucket. */
|
||||
const sandboxDotfile = baseAttachment({
|
||||
file_id: 'sandbox-config',
|
||||
filename: '_.config-abcdef.zip',
|
||||
type: 'application/zip',
|
||||
bytes: 12,
|
||||
} as Partial<TAttachment>);
|
||||
const { container } = renderWith(<AttachmentGroup attachments={[sandboxDotfile]} />);
|
||||
const chip = container.querySelector('[data-testid="file-container"]');
|
||||
expect(chip?.textContent).toBe('.config.zip');
|
||||
});
|
||||
|
||||
it('renders separate buckets for panel artifacts, mermaid, text, and plain files', () => {
|
||||
const attachments = [
|
||||
baseAttachment({
|
||||
|
|
|
|||
|
|
@ -1,8 +1,10 @@
|
|||
import React from 'react';
|
||||
import { render, screen } from '@testing-library/react';
|
||||
import { RecoilRoot } from 'recoil';
|
||||
import type { MutableSnapshot } from 'recoil';
|
||||
import type { TAttachment } from 'librechat-data-provider';
|
||||
import LogContent from '../LogContent';
|
||||
import store from '~/store';
|
||||
|
||||
jest.mock('~/hooks', () => ({
|
||||
useLocalize:
|
||||
|
|
@ -62,7 +64,19 @@ const baseAttachment = (overrides: Partial<TAttachment> = {}): TAttachment =>
|
|||
...overrides,
|
||||
}) as TAttachment;
|
||||
|
||||
const renderWith = (ui: React.ReactElement) => render(<RecoilRoot>{ui}</RecoilRoot>);
|
||||
/**
|
||||
* Default `streaming: true` so `ToolArtifactCard`'s mount-time
|
||||
* auto-focus (gated on `isSubmittingFamily(0)`) fires for these tests.
|
||||
* The legacy SSE-arrival flow is what every "panel routing" assertion
|
||||
* implicitly assumes.
|
||||
*/
|
||||
const renderWith = (ui: React.ReactElement, opts: { streaming?: boolean } = {}) => {
|
||||
const streaming = opts.streaming ?? true;
|
||||
const initializeState = (snapshot: MutableSnapshot) => {
|
||||
snapshot.set(store.isSubmittingFamily(0), streaming);
|
||||
};
|
||||
return render(<RecoilRoot initializeState={initializeState}>{ui}</RecoilRoot>);
|
||||
};
|
||||
|
||||
describe('LogContent attachment routing', () => {
|
||||
it('routes HTML attachments through ToolArtifactCard (panel)', () => {
|
||||
|
|
|
|||
|
|
@ -1,6 +1,13 @@
|
|||
import type { TAttachment } from 'librechat-data-provider';
|
||||
import { TOOL_ARTIFACT_TYPES } from '~/utils/artifacts';
|
||||
import { artifactTypeForAttachment, isImageAttachment, isTextAttachment } from '../attachmentTypes';
|
||||
import {
|
||||
artifactTypeForAttachment,
|
||||
attachmentSalience,
|
||||
displayFilename,
|
||||
isImageAttachment,
|
||||
isInternalSandboxArtifact,
|
||||
isTextAttachment,
|
||||
} from '../attachmentTypes';
|
||||
|
||||
const baseAttachment = (overrides: Partial<TAttachment> = {}): TAttachment =>
|
||||
({
|
||||
|
|
@ -134,6 +141,181 @@ describe('artifactTypeForAttachment', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('isInternalSandboxArtifact', () => {
|
||||
it('matches the post-sanitization `.dirkeep` form', () => {
|
||||
// The backend renames `.dirkeep` → `_.dirkeep-<hash>` via
|
||||
// `sanitizeArtifactPath`'s collision-avoidance suffix.
|
||||
const attachment = baseAttachment({
|
||||
filename: 'test_folder/_.dirkeep-88b30b',
|
||||
bytes: 0,
|
||||
} as Partial<TAttachment>);
|
||||
expect(isInternalSandboxArtifact(attachment)).toBe(true);
|
||||
});
|
||||
|
||||
it('matches a `.gitkeep` placeholder in its post-sanitization form', () => {
|
||||
const attachment = baseAttachment({
|
||||
filename: 'subdir/_.gitkeep-deadbe',
|
||||
bytes: 0,
|
||||
} as Partial<TAttachment>);
|
||||
expect(isInternalSandboxArtifact(attachment)).toBe(true);
|
||||
});
|
||||
|
||||
it('does NOT match a bare `.dirkeep` (likely user-uploaded scaffolding)', () => {
|
||||
/** A bare `.dirkeep` never originates from the sandbox — the dotfile
|
||||
* rewrite + disambiguator step in `sanitizeArtifactPath` always
|
||||
* produces `_.dirkeep-<6 hex>`. Treat the bare form as user content
|
||||
* (e.g. a checked-in directory marker) and let it render. */
|
||||
const attachment = baseAttachment({
|
||||
filename: '.dirkeep',
|
||||
bytes: 0,
|
||||
} as Partial<TAttachment>);
|
||||
expect(isInternalSandboxArtifact(attachment)).toBe(false);
|
||||
});
|
||||
|
||||
it('does NOT match a bare `.gitkeep` (the canonical project-scaffolding marker)', () => {
|
||||
const attachment = baseAttachment({
|
||||
filename: 'src/components/.gitkeep',
|
||||
bytes: 0,
|
||||
} as Partial<TAttachment>);
|
||||
expect(isInternalSandboxArtifact(attachment)).toBe(false);
|
||||
});
|
||||
|
||||
it('does NOT match a non-empty file even if its leaf matches the sanitized form', () => {
|
||||
// Defense in depth: bytes > 0 means the user actually wrote
|
||||
// content, so don't hide it regardless of name.
|
||||
const attachment = baseAttachment({
|
||||
filename: '_.dirkeep-88b30b',
|
||||
bytes: 12,
|
||||
} as Partial<TAttachment>);
|
||||
expect(isInternalSandboxArtifact(attachment)).toBe(false);
|
||||
});
|
||||
|
||||
it('does NOT match a regular file', () => {
|
||||
const attachment = baseAttachment({
|
||||
filename: 'test_folder/test_file.txt',
|
||||
bytes: 47,
|
||||
} as Partial<TAttachment>);
|
||||
expect(isInternalSandboxArtifact(attachment)).toBe(false);
|
||||
});
|
||||
|
||||
it('does NOT match an empty user file with a normal name', () => {
|
||||
const attachment = baseAttachment({
|
||||
filename: 'empty.md',
|
||||
bytes: 0,
|
||||
} as Partial<TAttachment>);
|
||||
expect(isInternalSandboxArtifact(attachment)).toBe(false);
|
||||
});
|
||||
|
||||
it('does NOT match when bytes is undefined (size unknown ≠ empty)', () => {
|
||||
/** Non-code-exec attachment sources can omit `bytes` entirely. The
|
||||
* filter only fires for an *explicit* zero-byte placeholder so we
|
||||
* don't accidentally hide files we don't have size info for. */
|
||||
const attachment = baseAttachment({
|
||||
filename: '_.dirkeep-88b30b',
|
||||
} as Partial<TAttachment>);
|
||||
expect(isInternalSandboxArtifact(attachment)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('displayFilename', () => {
|
||||
it('strips suffix + restores leading dot for an extensionless sandbox dotfile', () => {
|
||||
expect(displayFilename('test_folder/_.dirkeep-88b30b')).toBe('test_folder/.dirkeep');
|
||||
});
|
||||
|
||||
it('strips suffix + restores leading dot for a sanitized dotfile that has an extension', () => {
|
||||
expect(displayFilename('_.config-abcdef.txt')).toBe('.config.txt');
|
||||
});
|
||||
|
||||
it('preserves directory components on the dotfile path', () => {
|
||||
expect(displayFilename('a/b/_.config-abcdef')).toBe('a/b/.config');
|
||||
});
|
||||
|
||||
it('leaves a non-suffixed filename unchanged', () => {
|
||||
expect(displayFilename('test_file.txt')).toBe('test_file.txt');
|
||||
});
|
||||
|
||||
it('leaves a user-named `_foo.txt` alone (no false-positive de-mangling)', () => {
|
||||
// No collision suffix → don't drop the underscore. A user-named
|
||||
// `_foo.txt` is real content, not sanitized.
|
||||
expect(displayFilename('_foo.txt')).toBe('_foo.txt');
|
||||
});
|
||||
|
||||
it('handles undefined / empty input', () => {
|
||||
expect(displayFilename(undefined)).toBe('');
|
||||
expect(displayFilename('')).toBe('');
|
||||
});
|
||||
|
||||
it('does not strip a trailing hex-looking suffix that is actually part of the stem', () => {
|
||||
// 7 chars after the dash → not the canonical 6-char hash form.
|
||||
expect(displayFilename('build-1234567.log')).toBe('build-1234567.log');
|
||||
});
|
||||
|
||||
it('preserves a non-dotfile filename whose stem ends in `-<6 hex>` (codex regression)', () => {
|
||||
/** The previous regex stripped any `-<6 hex>` immediately before an
|
||||
* extension regardless of context. That collapsed legitimate
|
||||
* user-named files like `report-deadbe.csv` and `report-beef01.csv`
|
||||
* onto the same chip label `report.csv`. Without a structural
|
||||
* discriminator (the leading `_.` we use for the dotfile case),
|
||||
* we deliberately leave non-dotfile names alone — uglier when the
|
||||
* file *was* sanitized, but never collapses distinct files. */
|
||||
expect(displayFilename('report-deadbe.csv')).toBe('report-deadbe.csv');
|
||||
expect(displayFilename('report-beef01.csv')).toBe('report-beef01.csv');
|
||||
// Same shape inside a directory.
|
||||
expect(displayFilename('out/output-deadbe.csv')).toBe('out/output-deadbe.csv');
|
||||
});
|
||||
|
||||
it('preserves an extensionless leaf that ends in 6 hex chars', () => {
|
||||
/** No `_.` prefix → no match. User-named hash-tail artifacts pass
|
||||
* through unchanged. */
|
||||
expect(displayFilename('build-a1b2c3')).toBe('build-a1b2c3');
|
||||
});
|
||||
|
||||
it('preserves an extensionless leaf with hex suffix inside a directory', () => {
|
||||
expect(displayFilename('out/blob-deadbe')).toBe('out/blob-deadbe');
|
||||
});
|
||||
|
||||
it('preserves a sanitized non-dotfile (we cannot tell sanitized from user-named here)', () => {
|
||||
/** `report 1.csv` → backend sanitizes to `report_1-<hash>.csv`.
|
||||
* We can't tell this apart from a user who literally named their
|
||||
* file `report_1-abcdef.csv`, so we deliberately leave it alone.
|
||||
* Recovering it cleanly would require backend metadata. */
|
||||
expect(displayFilename('report_1-abcdef.csv')).toBe('report_1-abcdef.csv');
|
||||
});
|
||||
});
|
||||
|
||||
describe('attachmentSalience', () => {
|
||||
it('returns 0 for non-empty content (sorts first)', () => {
|
||||
expect(attachmentSalience(baseAttachment({ bytes: 47 } as Partial<TAttachment>))).toBe(0);
|
||||
});
|
||||
|
||||
it('returns 1 only for an explicit zero-byte entry (sinks last)', () => {
|
||||
expect(attachmentSalience(baseAttachment({ bytes: 0 } as Partial<TAttachment>))).toBe(1);
|
||||
});
|
||||
|
||||
it('treats undefined `bytes` as neutral so non-code-exec sources do not silently sink', () => {
|
||||
/** Web-search results, uploaded files where the schema omits `bytes`,
|
||||
* etc. should keep their input order — only an explicit `bytes === 0`
|
||||
* counts as the empty-placeholder shape we want to demote. */
|
||||
expect(attachmentSalience(baseAttachment({}))).toBe(0);
|
||||
});
|
||||
|
||||
it('produces a stable bucket sort when used as `(a,b) => salience(a) - salience(b)`', () => {
|
||||
const real = baseAttachment({
|
||||
bytes: 47,
|
||||
filename: 'test_file.txt',
|
||||
} as Partial<TAttachment>);
|
||||
const placeholder = baseAttachment({
|
||||
bytes: 0,
|
||||
filename: '_.dirkeep-88b30b',
|
||||
} as Partial<TAttachment>);
|
||||
const sorted = [placeholder, real].sort(
|
||||
(a, b) => attachmentSalience(a) - attachmentSalience(b),
|
||||
);
|
||||
expect(sorted[0]).toBe(real);
|
||||
expect(sorted[1]).toBe(placeholder);
|
||||
});
|
||||
});
|
||||
|
||||
describe('artifactTypeForAttachment branching', () => {
|
||||
it('returns the mermaid type for .mmd files', () => {
|
||||
const attachment = baseAttachment({
|
||||
|
|
|
|||
|
|
@ -3,6 +3,143 @@ import type { TAttachment, TAttachmentMetadata, TFile } from 'librechat-data-pro
|
|||
import type { ToolArtifactType } from '~/utils/artifacts';
|
||||
import { detectArtifactTypeFromFile } from '~/utils/artifacts';
|
||||
|
||||
/**
|
||||
* Empty-folder placeholders the bash executor drops in the stateless
|
||||
* sandbox so a `mkdir` survives across runs. The user has no reason to
|
||||
* see these — they're an implementation detail of `@librechat/agents`
|
||||
* — so we filter them out before any UI rendering.
|
||||
*
|
||||
* Anchored to the *post-sanitization* form `_.dirkeep-<6 hex>` /
|
||||
* `_.gitkeep-<6 hex>` produced by `sanitizeArtifactPath` (the leading
|
||||
* `.` is always rewritten to `_` and a disambiguator is always
|
||||
* appended because the dotfile rewrite counts as a mutation). A bare
|
||||
* `.dirkeep` / `.gitkeep` never originates from the sandbox — it's
|
||||
* almost always project scaffolding the user uploaded — so the
|
||||
* underscore prefix and the hex suffix are the discriminating signal.
|
||||
*/
|
||||
const SANDBOX_PLACEHOLDER_LEAVES = /^_\.(?:dirkeep|gitkeep)-[0-9a-f]{6}$/i;
|
||||
|
||||
/**
|
||||
* Recovers the user-visible filename from the backend's sanitized form.
|
||||
*
|
||||
* Anchored ONLY to the leading `_.` shape because that prefix is the
|
||||
* unambiguous fingerprint of `sanitizeArtifactPath`'s leading-dot
|
||||
* rewrite (`.dirkeep` → `_.dirkeep-88b30b`,
|
||||
* `.config.txt` → `_.config-abcdef.txt`). No legitimate user-named
|
||||
* file looks like `_.foo`, so the prefix + trailing `-<6 hex>`
|
||||
* combination is safe to strip.
|
||||
*
|
||||
* Captures the leaf in two halves so the optional extension survives
|
||||
* the rewrite:
|
||||
* - group 1: stem after the leading `_.` and before the suffix
|
||||
* - group 2: optional extension (`.txt`, `.csv`, …)
|
||||
*
|
||||
* **Deliberately does NOT cover the non-dotfile sanitization case**
|
||||
* (`report 1.csv` → `report_1-<hash>.csv`). A hex-shaped tail before
|
||||
* an extension has no structural discriminator vs. a user-named
|
||||
* `report-deadbe.csv`, and stripping it would silently collapse two
|
||||
* distinct user files (`report-deadbe.csv`, `report-beef01.csv`) onto
|
||||
* the same chip label. Showing the suffix-bearing form is uglier but
|
||||
* correct; recovering it cleanly would require a backend
|
||||
* `wasSanitized` metadata flag we don't have today.
|
||||
*/
|
||||
const SANITIZED_DOTFILE_PATTERN = /^_\.(.+?)-[0-9a-f]{6}(\.[^.]+)?$/;
|
||||
|
||||
/**
|
||||
* Last segment of a forward-slash path. The backend stores filenames as
|
||||
* forward-slash paths regardless of host OS, so we don't need full
|
||||
* `path.basename` here — just the final segment.
|
||||
*/
|
||||
const leafOf = (filename: string | undefined): string => {
|
||||
const raw = filename ?? '';
|
||||
const slash = raw.lastIndexOf('/');
|
||||
return slash < 0 ? raw : raw.slice(slash + 1);
|
||||
};
|
||||
|
||||
/**
|
||||
* `true` when the attachment is a sandbox-internal placeholder (empty
|
||||
* folder marker) the user shouldn't see as its own file chip.
|
||||
*
|
||||
* `bytes` must be *explicitly* zero to qualify — `undefined` means
|
||||
* "size unknown" (web-search results, archived attachments where the
|
||||
* schema omits the byte count) and we render those normally rather
|
||||
* than hiding them on a name match alone.
|
||||
*/
|
||||
export const isInternalSandboxArtifact = (attachment: TAttachment): boolean => {
|
||||
const file = attachment as TFile & TAttachmentMetadata;
|
||||
if (file.bytes !== 0) {
|
||||
return false;
|
||||
}
|
||||
return SANDBOX_PLACEHOLDER_LEAVES.test(leafOf(attachment.filename));
|
||||
};
|
||||
|
||||
/**
|
||||
* Display-only filename. Strips the collision-disambiguator suffix from
|
||||
* names the backend's leading-dot rewrite produced, so a sandbox-internal
|
||||
* `_.dirkeep-88b30b` reads as `.dirkeep` in the chip. The original
|
||||
* `attachment.filename` stays intact for download/lookup; this just
|
||||
* relabels what the user reads.
|
||||
*
|
||||
* Names that don't match the dotfile pattern pass through unchanged —
|
||||
* notably `report-deadbe.csv` (potential user file with a hex-shaped
|
||||
* tail) and `report_1-<hash>.csv` (sanitized non-dotfile, where we
|
||||
* lack the structural signal needed to strip safely).
|
||||
*/
|
||||
export const displayFilename = (filename: string | undefined): string => {
|
||||
const raw = filename ?? '';
|
||||
if (!raw) {
|
||||
return raw;
|
||||
}
|
||||
const slash = raw.lastIndexOf('/');
|
||||
const dir = slash < 0 ? '' : raw.slice(0, slash);
|
||||
const leaf = slash < 0 ? raw : raw.slice(slash + 1);
|
||||
const match = leaf.match(SANITIZED_DOTFILE_PATTERN);
|
||||
// Reconstruct: leading `.` (drops the sanitization-added `_`) + stem
|
||||
// + optional extension. When no match, leaf stays as-is.
|
||||
const cleanedLeaf = match ? `.${match[1]}${match[2] ?? ''}` : leaf;
|
||||
return dir ? `${dir}/${cleanedLeaf}` : cleanedLeaf;
|
||||
};
|
||||
|
||||
/**
|
||||
* Salience weight for sorting attachments within a bucket. `0` for
|
||||
* normal entries (renders first), `1` only for entries that explicitly
|
||||
* report `bytes === 0` (empty placeholders — sink to the bottom).
|
||||
*
|
||||
* Treating an absent `bytes` field as neutral (`0`) keeps non-code-exec
|
||||
* sources (web-search inline results, uploaded files where the schema
|
||||
* omits the byte count) from silently sinking past real content. The
|
||||
* filter is intentionally narrow: only an explicit zero counts as empty.
|
||||
*
|
||||
* The internal cast targets the `TFile & TAttachmentMetadata` arm of
|
||||
* `TAttachment` (the only one that declares `bytes`) rather than an
|
||||
* anonymous `{ bytes?: number }` shape. Tying the cast to the concrete
|
||||
* source type means a future `bytes` retype on `TFile` (e.g., to
|
||||
* `bigint`) would surface here at compile time instead of being
|
||||
* silently papered over.
|
||||
*/
|
||||
export const attachmentSalience = (item: TAttachment): number => {
|
||||
const bytes = (item as TFile & TAttachmentMetadata).bytes;
|
||||
return bytes === 0 ? 1 : 0;
|
||||
};
|
||||
|
||||
/**
|
||||
* Stable comparator for arrays of `TAttachment` values. Equivalent to
|
||||
* `(a, b) => attachmentSalience(a) - attachmentSalience(b)` but
|
||||
* exported once so the lambda doesn't need to be repeated at every
|
||||
* call site.
|
||||
*/
|
||||
export const bySalience = (a: TAttachment, b: TAttachment): number =>
|
||||
attachmentSalience(a) - attachmentSalience(b);
|
||||
|
||||
/**
|
||||
* Comparator variant for buckets that wrap the attachment in a record
|
||||
* (e.g. `{ attachment, type }` panel entries). Reads salience off the
|
||||
* inner `attachment` field so wrapped buckets sort the same way the
|
||||
* bare ones do.
|
||||
*/
|
||||
export const byEntrySalience = <T extends { attachment: TAttachment }>(a: T, b: T): number =>
|
||||
attachmentSalience(a.attachment) - attachmentSalience(b.attachment);
|
||||
|
||||
/**
|
||||
* An attachment is treated as an image only when it has the dimensions and
|
||||
* filepath needed to render via `<Image>`. Without width/height the image
|
||||
|
|
|
|||
|
|
@ -14,6 +14,14 @@ import store from '~/store';
|
|||
export default function Presentation({ children }: { children: React.ReactNode }) {
|
||||
const artifacts = useRecoilValue(store.artifactsState);
|
||||
const artifactsVisibility = useRecoilValue(store.artifactsVisibility);
|
||||
// Render-gating the panel on `currentArtifactId != null` (in addition
|
||||
// to visibility + non-empty artifacts) means the side panel only opens
|
||||
// when *something* is actively focused. Conversation navigation
|
||||
// resets `currentArtifactId` to null, so the panel stays closed when
|
||||
// a user revisits an old conversation full of artifacts. New artifacts
|
||||
// arriving via SSE auto-focus through `ToolArtifactCard`'s mount effect
|
||||
// (gated on `isSubmitting`), restoring the legacy streaming UX.
|
||||
const currentArtifactId = useRecoilValue(store.currentArtifactId);
|
||||
|
||||
useResetArtifactsOnConversationChange();
|
||||
|
||||
|
|
@ -51,7 +59,11 @@ export default function Presentation({ children }: { children: React.ReactNode }
|
|||
}, [mutateAsync]);
|
||||
|
||||
const artifactsElement = useMemo(() => {
|
||||
if (artifactsVisibility === true && Object.keys(artifacts ?? {}).length > 0) {
|
||||
if (
|
||||
artifactsVisibility === true &&
|
||||
currentArtifactId != null &&
|
||||
Object.keys(artifacts ?? {}).length > 0
|
||||
) {
|
||||
return (
|
||||
<ArtifactsProvider>
|
||||
<EditorProvider>
|
||||
|
|
@ -61,7 +73,7 @@ export default function Presentation({ children }: { children: React.ReactNode }
|
|||
);
|
||||
}
|
||||
return null;
|
||||
}, [artifactsVisibility, artifacts]);
|
||||
}, [artifactsVisibility, artifacts, currentArtifactId]);
|
||||
|
||||
return (
|
||||
<DragDropWrapper className="relative flex w-full grow overflow-hidden bg-presentation">
|
||||
|
|
|
|||
172
packages/api/src/agents/cleanup.spec.ts
Normal file
172
packages/api/src/agents/cleanup.spec.ts
Normal file
|
|
@ -0,0 +1,172 @@
|
|||
import { cleanCodeToolOutput } from './cleanup';
|
||||
|
||||
/**
|
||||
* Real bash_tool outputs captured from a deployed conversation. Patterns
|
||||
* tested live: trailing single `Note:` paragraph, double trailing
|
||||
* `Note:` paragraphs, per-file `| <annotation>` suffixes (downloaded /
|
||||
* displayed / known-as-input), and inline-comma file lists.
|
||||
*/
|
||||
describe('cleanCodeToolOutput', () => {
|
||||
it('returns empty/falsy input unchanged', () => {
|
||||
expect(cleanCodeToolOutput('')).toBe('');
|
||||
});
|
||||
|
||||
it('strips a trailing single `Note:` paragraph (mkdir-only output)', () => {
|
||||
const input = [
|
||||
'stdout:',
|
||||
'Folder created successfully!',
|
||||
'',
|
||||
'Generated files:',
|
||||
'- /mnt/data/test_folder/.dirkeep',
|
||||
'',
|
||||
'',
|
||||
'Note: Files from previous executions are automatically available and can be modified.',
|
||||
].join('\n');
|
||||
const output = cleanCodeToolOutput(input);
|
||||
expect(output).not.toMatch(/Note:/);
|
||||
expect(output).toMatch(/Folder created successfully!/);
|
||||
expect(output).toMatch(/Generated files:/);
|
||||
expect(output).toMatch(/- \/mnt\/data\/test_folder\/\.dirkeep$/);
|
||||
});
|
||||
|
||||
it('strips both trailing `Note:` paragraphs when two are emitted', () => {
|
||||
const input = [
|
||||
'stdout:',
|
||||
'File created successfully!',
|
||||
'',
|
||||
'Generated files:',
|
||||
'- /mnt/data/test_folder/test_file.txt',
|
||||
'Available files (inputs, not generated by this execution):',
|
||||
'- /mnt/data/test_folder/.dirkeep',
|
||||
'',
|
||||
'',
|
||||
'Note: Files from previous executions are automatically available and can be modified.',
|
||||
'',
|
||||
'Note: Files in "Available files" are inputs the user (or a skill) already provided to the sandbox. They were not produced by this execution and you should not present them as new outputs in your response.',
|
||||
].join('\n');
|
||||
const output = cleanCodeToolOutput(input);
|
||||
expect(output).not.toMatch(/Note:/);
|
||||
expect(output).toMatch(/Generated files:/);
|
||||
expect(output).toMatch(/Available files \(inputs/);
|
||||
});
|
||||
|
||||
it('strips per-file "downloaded" / "known to the user" / "displayed" annotations', () => {
|
||||
const input = [
|
||||
'Generated files:',
|
||||
'- /mnt/data/a.txt | File is already downloaded by the user',
|
||||
'- /mnt/data/b.png | Image is already displayed to the user',
|
||||
'Available files (inputs, not generated by this execution):',
|
||||
'- /mnt/data/c.txt | Available as an input — already known to the user',
|
||||
].join('\n');
|
||||
const output = cleanCodeToolOutput(input);
|
||||
expect(output).not.toMatch(/already downloaded/);
|
||||
expect(output).not.toMatch(/already displayed/);
|
||||
expect(output).not.toMatch(/already known to the user/);
|
||||
expect(output).toMatch(/- \/mnt\/data\/a\.txt$/m);
|
||||
expect(output).toMatch(/- \/mnt\/data\/b\.png$/m);
|
||||
expect(output).toMatch(/- \/mnt\/data\/c\.txt$/m);
|
||||
});
|
||||
|
||||
it('handles inline comma-separated file lists (small file count formatting)', () => {
|
||||
// The bash executor inlines the list with `, ` when ≤ 3 files.
|
||||
const input =
|
||||
'Available files (inputs, not generated by this execution):\n- /mnt/data/test_folder/test_file.txt | Available as an input — already known to the user, - /mnt/data/test_folder/.dirkeep | Available as an input — already known to the user';
|
||||
const output = cleanCodeToolOutput(input);
|
||||
expect(output).not.toMatch(/already known to the user/);
|
||||
expect(output).toMatch(/- \/mnt\/data\/test_folder\/test_file\.txt/);
|
||||
expect(output).toMatch(/- \/mnt\/data\/test_folder\/\.dirkeep/);
|
||||
});
|
||||
|
||||
it('passes through output that contains no boilerplate (no false positives)', () => {
|
||||
const input = 'stdout:\nHello, world!\n';
|
||||
expect(cleanCodeToolOutput(input)).toBe('stdout:\nHello, world!');
|
||||
});
|
||||
|
||||
it('does not strip the word "Note:" mid-output (only trailing paragraphs)', () => {
|
||||
// A user-authored echo `Note: this is real content` mid-output should
|
||||
// not be eaten by the trailing-Notes regex; only paragraphs that
|
||||
// start with `Note:` AFTER a blank line near end-of-string are matched.
|
||||
const input = [
|
||||
'stdout:',
|
||||
'Note: this is real content',
|
||||
'more output',
|
||||
'',
|
||||
'Generated files:',
|
||||
'- /mnt/data/foo.txt',
|
||||
].join('\n');
|
||||
const output = cleanCodeToolOutput(input);
|
||||
expect(output).toMatch(/Note: this is real content/);
|
||||
});
|
||||
|
||||
it('preserves a user-authored "Note:" line that follows a blank line in stdout', () => {
|
||||
/** Tightened anchor: the trailing-notes regex now requires one of the
|
||||
* known boilerplate openings (`Files from previous executions`, etc.)
|
||||
* after `Note:`. A user `print()` that happens to output a blank
|
||||
* line followed by `Note: ...something else...` must survive
|
||||
* untouched, along with everything that follows it. */
|
||||
const input = [
|
||||
'stdout:',
|
||||
'first line',
|
||||
'',
|
||||
"Note: this is the user's measurement output",
|
||||
'more output',
|
||||
'',
|
||||
'Generated files:',
|
||||
'- /mnt/data/foo.txt',
|
||||
].join('\n');
|
||||
const output = cleanCodeToolOutput(input);
|
||||
expect(output).toMatch(/Note: this is the user's measurement output/);
|
||||
expect(output).toMatch(/Generated files:/);
|
||||
expect(output).toMatch(/- \/mnt\/data\/foo\.txt$/);
|
||||
});
|
||||
|
||||
it('drops trailing blank lines after stripping', () => {
|
||||
const input =
|
||||
'stdout:\nresult\n\n\nNote: Files from previous executions are automatically available and can be modified.';
|
||||
expect(cleanCodeToolOutput(input)).toBe('stdout:\nresult');
|
||||
});
|
||||
|
||||
it('does NOT mutate stdout that legitimately contains an annotation phrase outside a file-list section', () => {
|
||||
/** Codex regression: the previous global `.replace` would strip
|
||||
* any line ending in one of the three annotation phrases — even
|
||||
* if that line was the user's own stdout. A script doing
|
||||
* `echo "foo | File is already downloaded by the user"` had its
|
||||
* output silently mangled before being fed back into model
|
||||
* context. With section-scoped stripping, the script's stdout
|
||||
* passes through unchanged because there's no `Generated files:`
|
||||
* or `Available files (...)` header above it. */
|
||||
const input = [
|
||||
'stdout:',
|
||||
'foo | File is already downloaded by the user',
|
||||
'bar | Image is already displayed to the user',
|
||||
'baz | Available as an input — already known to the user',
|
||||
].join('\n');
|
||||
const output = cleanCodeToolOutput(input);
|
||||
expect(output).toMatch(/foo \| File is already downloaded by the user/);
|
||||
expect(output).toMatch(/bar \| Image is already displayed to the user/);
|
||||
expect(output).toMatch(/baz \| Available as an input — already known to the user/);
|
||||
});
|
||||
|
||||
it('strips annotations inside a file-list section but preserves identical phrases in stdout above it', () => {
|
||||
/** Mixed case: the same execution's stdout coincidentally echoes
|
||||
* one of the annotation phrases AND a file gets generated. The
|
||||
* stdout phrase must survive; the file-listing annotation must
|
||||
* still get stripped. */
|
||||
const input = [
|
||||
'stdout:',
|
||||
'echo output: foo | File is already downloaded by the user',
|
||||
'',
|
||||
'Generated files:',
|
||||
'- /mnt/data/foo.txt | File is already downloaded by the user',
|
||||
].join('\n');
|
||||
const output = cleanCodeToolOutput(input);
|
||||
// Stdout line preserved verbatim.
|
||||
expect(output).toMatch(/echo output: foo \| File is already downloaded by the user/);
|
||||
// File-listing line had its annotation stripped.
|
||||
expect(output).toMatch(/- \/mnt\/data\/foo\.txt$/m);
|
||||
// The annotation phrase appears EXACTLY once now (only in stdout,
|
||||
// not on the file-listing line).
|
||||
const phraseCount = (output.match(/File is already downloaded by the user/g) ?? []).length;
|
||||
expect(phraseCount).toBe(1);
|
||||
});
|
||||
});
|
||||
103
packages/api/src/agents/cleanup.ts
Normal file
103
packages/api/src/agents/cleanup.ts
Normal file
|
|
@ -0,0 +1,103 @@
|
|||
/**
|
||||
* Strips repetitive boilerplate from `@librechat/agents` code-execution
|
||||
* tool output before LibreChat re-injects it into the assistant's
|
||||
* conversation history.
|
||||
*
|
||||
* The bash executor in `@librechat/agents` appends two kinds of noise
|
||||
* to every successful run:
|
||||
*
|
||||
* 1. **Trailing "Note:" paragraphs** — long behavioral hints repeating
|
||||
* rules the agent already has via its system prompt
|
||||
* ("Files from previous executions are automatically available...",
|
||||
* "Files in 'Available files' are inputs..."). Re-stating them on
|
||||
* every tool call wastes tokens at scale (50+ tokens × N tool calls).
|
||||
*
|
||||
* 2. **Per-file annotations** — each file in the `Generated files:` /
|
||||
* `Available files (...):` lists gets a `| <annotation>` suffix
|
||||
* (`File is already downloaded by the user`,
|
||||
* `Image is already displayed to the user`,
|
||||
* `Available as an input — already known to the user`). The two
|
||||
* section headers already convey the new-vs-known distinction; the
|
||||
* per-file annotations are redundant *and* phrased inconsistently
|
||||
* ("downloaded" vs. "displayed" vs. "known to the user").
|
||||
*
|
||||
* Stripping happens in LibreChat (this file), not upstream, so the
|
||||
* cleaning is reversible — pin to a specific upstream version and the
|
||||
* patterns adjust here without releasing a new agents build. The
|
||||
* patterns are anchored conservatively: only the documented forms are
|
||||
* matched, so a future upstream string change leaves user-authored
|
||||
* `Note:` lines (or legitimate `|`-delimited filenames) untouched.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Matches a `Generated files:` or `Available files (...)` block —
|
||||
* the section header line plus the file-listing lines that follow.
|
||||
*
|
||||
* Group 1 captures the header (with trailing newline). Group 2
|
||||
* captures the file lines (each starts with `- /` per the executor's
|
||||
* format, accommodating both line-per-file (≥ 4 files) and inline
|
||||
* comma-separated (≤ 3 files: `- /a.txt | <ann>, - /b.txt | <ann>`)
|
||||
* layouts because the latter is still a single line that begins with
|
||||
* `- /`).
|
||||
*
|
||||
* The `Available files (...)` form has parenthesized content
|
||||
* (`(inputs, not generated by this execution)`) which the
|
||||
* `\([^)]*\)` segment matches without baking the exact phrasing in.
|
||||
*
|
||||
* Block ends at the first line that doesn't start with `- /` (next
|
||||
* iteration of the `+` quantifier fails). The blank line preceding
|
||||
* the trailing `Note:` paragraphs reliably terminates the block.
|
||||
*/
|
||||
const FILE_SECTION_PATTERN =
|
||||
/((?:Generated files:|Available files \([^)]*\):)\n)((?:- \/[^\n]*(?:\n|$))+)/g;
|
||||
|
||||
/**
|
||||
* Matches the per-file `| <annotation>` suffix the bash executor adds to
|
||||
* each file inside a `Generated files:` / `Available files (...):` block.
|
||||
* Two listing formats: line-per-file (≥ 4 files) and inline
|
||||
* comma-separated (≤ 3 files: `- /mnt/a.txt | <ann>, - /mnt/b.txt | <ann>`).
|
||||
* The non-greedy `[^,\n]*?` plus the `(?=,|\n|$)` boundary stops the match
|
||||
* at either end-of-line or the inline `, ` separator so an inline list
|
||||
* strips each file's annotation independently.
|
||||
*
|
||||
* Applied ONLY within file-section blocks captured by
|
||||
* `FILE_SECTION_PATTERN`, never globally — a user script that legitimately
|
||||
* echoes one of these phrases (`echo 'foo | File is already downloaded by
|
||||
* the user'`) would otherwise have its stdout silently mutated.
|
||||
*/
|
||||
const PER_FILE_ANNOTATION_PATTERN =
|
||||
/\s*\|\s*(?:File is already downloaded by the user|Image is already displayed to the user|Available as an input[^,\n]*?)(?=,|\n|$)/g;
|
||||
|
||||
/**
|
||||
* Matches the bash executor's trailing `Note:` paragraphs. Anchored to
|
||||
* the two known boilerplate openings (`Files from previous executions`,
|
||||
* `Files in "Available files"`) so a user-authored `Note:` line — even
|
||||
* one preceded by a blank line in stdout — is not eaten by the
|
||||
* `[\s\S]*$` tail. New upstream variants need to be added here
|
||||
* explicitly; that's the trade-off for not silently dropping content.
|
||||
*/
|
||||
const TRAILING_NOTES_PATTERN =
|
||||
/\n\s*\n\s*Note:\s*(?:Files from previous executions|Files in "Available files")[\s\S]*$/;
|
||||
|
||||
/**
|
||||
* Returns `content` with the bash-executor boilerplate removed. Safe
|
||||
* to call on any tool output — non-matching text is returned unchanged.
|
||||
*
|
||||
* Annotation stripping is scoped: only the contents of `Generated
|
||||
* files:` / `Available files (...):` blocks are mutated. Stdout that
|
||||
* happens to contain a phrase like
|
||||
* `| File is already downloaded by the user` (e.g. a script echoing
|
||||
* the very string we're scrubbing) passes through unchanged.
|
||||
*/
|
||||
export function cleanCodeToolOutput(content: string): string {
|
||||
if (!content) {
|
||||
return content;
|
||||
}
|
||||
const noAnnotations = content.replace(
|
||||
FILE_SECTION_PATTERN,
|
||||
(_match, header: string, files: string) =>
|
||||
header + files.replace(PER_FILE_ANNOTATION_PATTERN, ''),
|
||||
);
|
||||
const noTrailingNotes = noAnnotations.replace(TRAILING_NOTES_PATTERN, '');
|
||||
return noTrailingNotes.trimEnd();
|
||||
}
|
||||
|
|
@ -12,6 +12,7 @@ import type {
|
|||
import { Types } from 'mongoose';
|
||||
import type { StructuredToolInterface } from '@langchain/core/tools';
|
||||
import type { ServerRequest } from '~/types';
|
||||
import { cleanCodeToolOutput } from './cleanup';
|
||||
import { primeSkillFiles } from './skillFiles';
|
||||
import type { SkillFileRecord } from './skillFiles';
|
||||
import { buildSkillPrimeMessage } from './skills';
|
||||
|
|
@ -1041,13 +1042,25 @@ export function createToolExecuteHandler(options: ToolExecuteOptions): EventHand
|
|||
metadata,
|
||||
} as Record<string, unknown>);
|
||||
|
||||
// Code-execution tools emit per-call boilerplate
|
||||
// ("Note: ..." paragraphs and `| <annotation>` per-file
|
||||
// suffixes) that wastes tokens when re-injected into
|
||||
// every subsequent model turn. Strip it here, *after*
|
||||
// the tool resolved but *before* downstream consumers
|
||||
// (model context, SSE forwarding, persistence) see it.
|
||||
// Non-code-execution tools pass through unchanged.
|
||||
const cleanedContent =
|
||||
CODE_EXECUTION_TOOLS.has(tc.name) && typeof result.content === 'string'
|
||||
? cleanCodeToolOutput(result.content)
|
||||
: result.content;
|
||||
|
||||
if (toolEndCallback) {
|
||||
await toolEndCallback(
|
||||
{
|
||||
output: {
|
||||
name: tc.name,
|
||||
tool_call_id: tc.id,
|
||||
content: result.content,
|
||||
content: cleanedContent,
|
||||
artifact: result.artifact,
|
||||
},
|
||||
},
|
||||
|
|
@ -1063,7 +1076,7 @@ export function createToolExecuteHandler(options: ToolExecuteOptions): EventHand
|
|||
|
||||
return {
|
||||
toolCallId: tc.id,
|
||||
content: result.content,
|
||||
content: cleanedContent,
|
||||
artifact: result.artifact,
|
||||
status: 'success' as const,
|
||||
};
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue