From 149a0dce9d4b76e2d9d4614f5b1a9096c882b080 Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Tue, 30 Jun 2026 16:13:22 +0200 Subject: [PATCH] fix: require a usable route before downloading the original artifact file A shared link to a non-snapshotted code-execution office artifact strips source/user and deletes filepath while keeping file_id (share sanitization + applyShareFileRoute). The preview-panel download gate treated that lone file_id as sufficient, so it routed to an empty useCodeOutputDownload fetch and downloaded nothing instead of falling back to the preview-content blob. Take the original-file branch only when useAttachmentLink can actually fetch: a non-empty filepath (http target, share route, or code-output URL) or full local-file metadata (isLocallyStoredSource + file_id + user). Export isLocallyStoredSource from LogLink so the panel reuses the same predicate. --- .../components/Artifacts/DownloadArtifact.tsx | 22 ++++++-- .../__tests__/DownloadArtifact.test.tsx | 53 +++++++++++++++++++ .../Chat/Messages/Content/Parts/LogLink.tsx | 2 +- 3 files changed, 72 insertions(+), 5 deletions(-) diff --git a/client/src/components/Artifacts/DownloadArtifact.tsx b/client/src/components/Artifacts/DownloadArtifact.tsx index 4e836ba798..a973575de0 100644 --- a/client/src/components/Artifacts/DownloadArtifact.tsx +++ b/client/src/components/Artifacts/DownloadArtifact.tsx @@ -2,7 +2,10 @@ import React, { useState } from 'react'; import { Button } from '@librechat/client'; import { Download, CircleCheckBig } from 'lucide-react'; import type { Artifact } from '~/common'; -import { useAttachmentLink } from '~/components/Chat/Messages/Content/Parts/LogLink'; +import { + useAttachmentLink, + isLocallyStoredSource, +} from '~/components/Chat/Messages/Content/Parts/LogLink'; import useArtifactProps from '~/hooks/Artifacts/useArtifactProps'; import { isPreviewOnlyArtifact } from '~/utils/artifacts'; import { useCodeState } from '~/Providers/EditorContext'; @@ -21,9 +24,20 @@ const DownloadArtifact = ({ artifact }: { artifact: Artifact }) => { * text artifacts keep the blob path: their `content` IS the file (and * reflects any in-panel edits). */ const { download } = artifact; - const downloadOriginalFile = - isPreviewOnlyArtifact(artifact.type) && - (download?.filepath != null || download?.file_id != null); + /* Only take the original-file branch when `useAttachmentLink` can + * actually fetch something: a usable `filepath` (http target, share + * route, or code-output URL) OR enough metadata for the local-file + * API path (`isLocallyStoredSource` + file_id + user). A shared link + * to a non-snapshotted code-execution artifact strips source/user and + * deletes filepath while keeping file_id; without this guard that lone + * file_id would route to an empty fetch and download nothing instead + * of falling back to the preview-content blob. */ + const hasUsableRoute = + (download?.filepath != null && download.filepath !== '') || + (download?.file_id != null && + download?.user != null && + isLocallyStoredSource(download?.source)); + const downloadOriginalFile = isPreviewOnlyArtifact(artifact.type) && hasUsableRoute; const { handleDownload: downloadAttachment } = useAttachmentLink({ href: download?.filepath ?? '', filename: artifact.title ?? fileName, diff --git a/client/src/components/Artifacts/__tests__/DownloadArtifact.test.tsx b/client/src/components/Artifacts/__tests__/DownloadArtifact.test.tsx index 8078dfbe3b..513ed6fe36 100644 --- a/client/src/components/Artifacts/__tests__/DownloadArtifact.test.tsx +++ b/client/src/components/Artifacts/__tests__/DownloadArtifact.test.tsx @@ -24,6 +24,8 @@ jest.mock('~/Providers/EditorContext', () => ({ jest.mock('~/components/Chat/Messages/Content/Parts/LogLink', () => ({ useAttachmentLink: () => ({ handleDownload: mockFileDownload }), + isLocallyStoredSource: (source?: string) => + ['local', 'firebase', 's3', 'cloudfront', 'azure_blob'].includes(source ?? ''), })); const officeArtifact: Artifact = { @@ -48,6 +50,37 @@ const htmlArtifact: Artifact = { content: '

hello

', }; +/* Shared link to a non-snapshotted code-execution office artifact: share + * sanitization strips source/user and `applyShareFileRoute` deletes + * filepath, leaving only file_id. There is no route to fetch the + * original, so the panel must fall back to the preview-content blob. */ +const sharedNoRouteArtifact: Artifact = { + id: 'tool-artifact-fid-2', + lastUpdateTime: 0, + type: TOOL_ARTIFACT_TYPES.PRESENTATION, + title: 'deck.pptx', + content: 'slide text scrape', + download: { + file_id: 'fid-2', + }, +}; + +/* Locally-stored office artifact with no filepath but full local-file + * metadata: the API download path (isLocallyStoredSource + file_id + + * user) can still fetch the original. */ +const localMetadataArtifact: Artifact = { + id: 'tool-artifact-fid-3', + lastUpdateTime: 0, + type: TOOL_ARTIFACT_TYPES.SPREADSHEET, + title: 'book.xlsx', + content: 'sheet scrape', + download: { + file_id: 'fid-3', + source: 'local', + user: 'user-3', + }, +}; + describe('DownloadArtifact', () => { let createObjectURL: jest.Mock; let revokeObjectURL: jest.Mock; @@ -92,4 +125,24 @@ describe('DownloadArtifact', () => { expect(createObjectURL).toHaveBeenCalledTimes(1); expect(mockFileDownload).not.toHaveBeenCalled(); }); + + it('falls back to the preview blob when an office artifact has only a lone file_id (no usable route)', async () => { + render(); + await act(async () => { + fireEvent.click(screen.getByRole('button')); + }); + // No filepath/share route and no local metadata: must NOT call the + // empty attachment fetch; serialize the preview content instead. + expect(mockFileDownload).not.toHaveBeenCalled(); + expect(createObjectURL).toHaveBeenCalledTimes(1); + }); + + it('downloads the original via the local-file path when filepath is absent but local metadata is present', async () => { + render(); + await act(async () => { + fireEvent.click(screen.getByRole('button')); + }); + expect(mockFileDownload).toHaveBeenCalledTimes(1); + expect(createObjectURL).not.toHaveBeenCalled(); + }); }); diff --git a/client/src/components/Chat/Messages/Content/Parts/LogLink.tsx b/client/src/components/Chat/Messages/Content/Parts/LogLink.tsx index 7d81a7c42e..bff5aca2b2 100644 --- a/client/src/components/Chat/Messages/Content/Parts/LogLink.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/LogLink.tsx @@ -27,7 +27,7 @@ interface AttachmentLinkOptions { * Files with these sources are stored on the LibreChat server and should * use the /api/files/download endpoint instead of direct URL access. */ -const isLocallyStoredSource = (source?: string): boolean => { +export const isLocallyStoredSource = (source?: string): boolean => { if (!source) { return false; }