From cd271ed7ae8eb84334ef9fe63ea87ef502f94cae Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Wed, 29 Jul 2026 05:14:18 +0200 Subject: [PATCH] fix(import): roll back stored bytes when the file row fails, and count local resources If saveBuffer succeeded and createFile then rejected, the object was in storage with no row pointing at it - and the caller only learns an asset exists once ingestOne resolves, so nothing could ever find it to clean up. A Claude local_resource references bytes the export does not ship, same as an image, so it counts as unavailable. Rendering its label is not the same as having it. --- packages/api/src/import/assets.ts | 42 +++++++++++++------- packages/api/src/import/claude/tools.spec.ts | 6 ++- packages/api/src/import/claude/tools.ts | 4 ++ 3 files changed, 35 insertions(+), 17 deletions(-) diff --git a/packages/api/src/import/assets.ts b/packages/api/src/import/assets.ts index 8510ac2e46..5f12d9199a 100644 --- a/packages/api/src/import/assets.ts +++ b/packages/api/src/import/assets.ts @@ -302,21 +302,33 @@ async function ingestOne( const { filepath, source } = await deps.saveBuffer({ userId, buffer, fileName, type, tenantId }); - await deps.createFile( - { - user: userId, - file_id: fileId, - bytes: buffer.byteLength, - filepath, - filename: originalName, - type, - source, - context: FileContext.message_attachment, - tenantId, - ...(input.expiredAt ? { expiredAt: input.expiredAt } : {}), - }, - true, - ); + try { + await deps.createFile( + { + user: userId, + file_id: fileId, + bytes: buffer.byteLength, + filepath, + filename: originalName, + type, + source, + context: FileContext.message_attachment, + tenantId, + ...(input.expiredAt ? { expiredAt: input.expiredAt } : {}), + }, + true, + ); + } catch (error) { + /** The bytes are in storage but no row points at them, and the caller only + * learns an asset exists once this function resolves — so nothing else + * could ever find this object to clean up. */ + try { + await deps.deleteFile?.({ file_id: fileId, filepath, filename: originalName, type }); + } catch (cleanupError) { + logger.error(`[import] Could not roll back the stored asset ${fileId}`, cleanupError); + } + throw error; + } return buildImportedAsset( fileId, diff --git a/packages/api/src/import/claude/tools.spec.ts b/packages/api/src/import/claude/tools.spec.ts index 0721ad81d0..1fac2c8a1b 100644 --- a/packages/api/src/import/claude/tools.spec.ts +++ b/packages/api/src/import/claude/tools.spec.ts @@ -117,7 +117,7 @@ describe('completeToolCall', () => { ); }); - it('counts image results as unavailable and names local resources', () => { + it('counts images and local resources as unavailable, and still names the resource', () => { const registry = createToolRegistry(); const sources = createSourceIndex(); beginToolCall(registry, use('tu-1', 'present_files')); @@ -132,7 +132,9 @@ describe('completeToolCall', () => { sources, ); - expect(resolved.unavailable).toBe(2); + /** All three reference bytes the export does not ship. `local_resource` + * renders a label, which is not the same as being available. */ + expect(resolved.unavailable).toBe(3); expect(resolved.output).toBe('- notes'); }); diff --git a/packages/api/src/import/claude/tools.ts b/packages/api/src/import/claude/tools.ts index 85541c4d5d..c0e00bb5e3 100644 --- a/packages/api/src/import/claude/tools.ts +++ b/packages/api/src/import/claude/tools.ts @@ -127,6 +127,10 @@ function renderResult(blocks: ClaudeResultBlock[], sources: SourceIndex): Result } if (block.type === 'local_resource') { + /** The export ships the name or path but never the bytes, so this is a + * missing attachment even though it renders a label. Not counting it made + * the report understate what the import could not bring across. */ + unavailable += 1; const label = block.name ?? block.file_path; if (label) { lines.push(`- ${label}`);