Skip to content

Commit 5a09f8d

Browse files
committed
fix(chat): preserve external downloads and report bulk failures
1 parent a27a62f commit 5a09f8d

2 files changed

Lines changed: 66 additions & 18 deletions

File tree

apps/sim/app/(interfaces)/chat/components/message/components/file-download.test.tsx

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -136,11 +136,14 @@ describe('chat file downloads', () => {
136136
}
137137
)
138138

139-
it('keeps external URL files on their existing URL path', async () => {
140-
fetchMock.mockResolvedValue(new Response('external bytes'))
141-
await clickDownload(renderFile({ ...imageFile, base64: undefined, key: 'url/external' }))
142-
expect(fetchMock).toHaveBeenCalledExactlyOnceWith(imageFile.url, { cache: 'no-store' })
143-
})
139+
it.each(['url/external', 'result-123', ''])(
140+
'keeps external URL files with key "%s" on their existing URL path',
141+
async (key) => {
142+
fetchMock.mockResolvedValue(new Response('external bytes'))
143+
await clickDownload(renderFile({ ...imageFile, base64: undefined, key }))
144+
expect(fetchMock).toHaveBeenCalledExactlyOnceWith(imageFile.url, { cache: 'no-store' })
145+
}
146+
)
144147

145148
it('preserves delivered signed access for public visitors without a workspace session', async () => {
146149
fetchMock
@@ -183,6 +186,36 @@ describe('chat file downloads', () => {
183186
})
184187
expect(fetchMock).toHaveBeenCalledTimes(1)
185188
})
189+
190+
it('reports partial bulk failures, continues the batch, and clears the alert after a successful retry', async () => {
191+
fetchMock.mockResolvedValue(new Response(null, { status: 403 }))
192+
const stored = { ...imageFile, id: 'stored', name: 'stored.png', base64: undefined }
193+
const container = renderFile([stored, imageFile])
194+
await clickDownload(container)
195+
expect(downloadedNames).toEqual(['generated.png'])
196+
expect(container.querySelector('[role="alert"]')?.textContent).toContain(
197+
'Unable to download 1 file'
198+
)
199+
fetchMock.mockResolvedValue(new Response('stored bytes'))
200+
await act(async () => {
201+
container.querySelector('button')!.click()
202+
await vi.waitFor(() =>
203+
expect(downloadedNames).toEqual(['generated.png', 'stored.png', 'generated.png'])
204+
)
205+
})
206+
expect(container.querySelector('[role="alert"]')).toBeNull()
207+
})
208+
209+
it('uses the recognized key context when metadata omits it', async () => {
210+
fetchMock.mockResolvedValue(new Response('workspace bytes'))
211+
await clickDownload(
212+
renderFile({ ...imageFile, base64: undefined, key: 'workspace/id/file.png' })
213+
)
214+
expect(fetchMock).toHaveBeenCalledExactlyOnceWith(
215+
'/api/files/serve/workspace%2Fid%2Ffile.png?context=workspace',
216+
{ cache: 'no-store' }
217+
)
218+
})
186219
})
187220

188221
it('refuses unsafe external file URLs', async () => {

apps/sim/app/(interfaces)/chat/components/message/components/file-download.tsx

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { sleep } from '@sim/utils/helpers'
88
import { DefaultFileIcon, getDocumentIcon } from '@/components/icons/document-icons'
99
import { isSafeHttpUrl } from '@/lib/core/utils/urls'
1010
import { saveBlob } from '@/lib/uploads/client/download'
11+
import { tryInferContextFromKey } from '@/lib/uploads/utils/file-utils'
1112
import type { ChatFile } from '@/app/(interfaces)/chat/components/message/message'
1213

1314
const logger = createLogger('ChatFileDownload')
@@ -69,9 +70,10 @@ async function triggerDownload(file: ChatFile): Promise<void> {
6970
return
7071
}
7172

72-
const hasStorageKey = Boolean(file.key && !file.key.startsWith('url/'))
73+
const storageContext = tryInferContextFromKey(file.key)
74+
const hasStorageKey = storageContext !== null
7375
const url = hasStorageKey
74-
? `/api/files/serve/${encodeURIComponent(file.key)}?context=${encodeURIComponent(file.context || 'execution')}`
76+
? `/api/files/serve/${encodeURIComponent(file.key)}?context=${encodeURIComponent(storageContext)}`
7577
: isSafeHttpUrl(file.url)
7678
? file.url
7779
: null
@@ -167,13 +169,16 @@ export function ChatFileDownload({ file }: ChatFileDownloadProps) {
167169

168170
export function ChatFileDownloadAll({ files }: ChatFileDownloadAllProps) {
169171
const [isDownloading, setIsDownloading] = useState(false)
172+
const [failedCount, setFailedCount] = useState(0)
170173

171174
if (!files || files.length === 0) return null
172175

173176
const handleDownloadAll = async () => {
174177
if (isDownloading) return
175178

176179
setIsDownloading(true)
180+
setFailedCount(0)
181+
let failures = 0
177182

178183
try {
179184
logger.info(`Initiating download for ${files.length} files`)
@@ -189,25 +194,35 @@ export function ChatFileDownloadAll({ files }: ChatFileDownloadAllProps) {
189194
}
190195
} catch (error) {
191196
logger.error(`Failed to download file ${file.name}:`, error)
197+
failures++
192198
}
193199
}
194200
} finally {
201+
setFailedCount(failures)
195202
setIsDownloading(false)
196203
}
197204
}
198205

199206
return (
200-
<Button
201-
variant='ghost-secondary'
202-
onClick={handleDownloadAll}
203-
disabled={isDownloading}
204-
className='p-0'
205-
>
206-
{isDownloading ? (
207-
<Loader className='size-3' animate />
208-
) : (
209-
<Download className='size-3' strokeWidth={2} />
207+
<div className='flex flex-col items-start gap-2'>
208+
<Button
209+
variant='ghost-secondary'
210+
onClick={handleDownloadAll}
211+
disabled={isDownloading}
212+
className='p-0'
213+
>
214+
{isDownloading ? (
215+
<Loader className='size-3' animate />
216+
) : (
217+
<Download className='size-3' strokeWidth={2} />
218+
)}
219+
</Button>
220+
{failedCount > 0 && (
221+
<p role='alert' className='text-[var(--text-error)] text-xs'>
222+
Unable to download {failedCount} {failedCount === 1 ? 'file' : 'files'}. Please try
223+
downloading them individually.
224+
</p>
210225
)}
211-
</Button>
226+
</div>
212227
)
213228
}

0 commit comments

Comments
 (0)