diff --git a/apps/desktop/src/main/__tests__/runtime-host-artifacts-ipc-main.test.ts b/apps/desktop/src/main/__tests__/runtime-host-artifacts-ipc-main.test.ts index 283d33f342..841410f8fc 100644 --- a/apps/desktop/src/main/__tests__/runtime-host-artifacts-ipc-main.test.ts +++ b/apps/desktop/src/main/__tests__/runtime-host-artifacts-ipc-main.test.ts @@ -18,11 +18,11 @@ */ import assert from "node:assert/strict"; -import { mkdtemp, readFile, rm } from "node:fs/promises"; +import { mkdtemp, readFile, readdir, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { test } from "node:test"; -import { registerRuntimeHostArtifactsIpc } from "../runtime-host-artifacts-ipc-main.js"; +import { materializeArtifact, registerRuntimeHostArtifactsIpc } from "../runtime-host-artifacts-ipc-main.js"; type Handler = (event: unknown, ...args: any[]) => unknown; type StreamArtifact = ( @@ -190,3 +190,92 @@ test("Attachment byte IPC stops a stream that exceeds its preview admission", as { ok: false, reason: "too_large" }, ); }); + +test("A failed final replacement leaves the previous destination intact", async () => { + const root = await mkdtemp(join(tmpdir(), "maka-host-artifact-ipc-")); + const savedPath = join(root, "saved.bin"); + await writeFile(savedPath, "ORIGINAL"); + const content = Buffer.from("REPLACEMENT"); + const client = { + async streamArtifact( + _sessionId: string, + _artifactId: string, + writeChunk: (chunk: Uint8Array) => Promise, + ) { + await writeChunk(content); + return content.byteLength; + }, + }; + + try { + await assert.rejects( + materializeArtifact( + client as never, + "session-1", + "artifact-1", + savedPath, + content.byteLength, + async () => { + throw Object.assign(new Error("injected EIO"), { code: "EIO" }); + }, + ), + ); + assert.equal(await readFile(savedPath, "utf8"), "ORIGINAL"); + assert.deepEqual(await readdir(root), ["saved.bin"]); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); + +test("app:saveArtifactAs replaces an existing destination on success", async () => { + const root = await mkdtemp(join(tmpdir(), "maka-host-artifact-ipc-")); + const savedPath = join(root, "saved.bin"); + await writeFile(savedPath, "ORIGINAL"); + const content = Buffer.from("REPLACEMENT"); + const handlers = new Map(); + const artifact = { + id: "artifact-1", + sessionId: "session-1", + turnId: "turn-1", + createdAt: 1, + name: "result.bin", + kind: "image", + sizeBytes: content.byteLength, + mimeType: "image/png", + status: "live", + } as const; + + try { + registerRuntimeHostArtifactsIpc({ + ipcMain: { + handle: (channel, handler) => handlers.set(channel, handler as Handler), + }, + client: { + hostEpoch: "host-1", + async getArtifact() { + return artifact; + }, + async streamArtifact( + _sessionId: string, + _artifactId: string, + writeChunk: (chunk: Uint8Array) => Promise, + ) { + await writeChunk(content); + return content.byteLength; + }, + } as never, + mainWindowController: { + showSaveDialog: async () => ({ canceled: false, filePath: savedPath }), + } as never, + showItemInFolder() {}, + }); + + assert.deepEqual( + await handlers.get("app:saveArtifactAs")?.({}, "session-1", "artifact-1"), + { ok: true, saved: "result.bin" }, + ); + assert.equal(await readFile(savedPath, "utf8"), "REPLACEMENT"); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); diff --git a/apps/desktop/src/main/runtime-host-artifacts-ipc-main.ts b/apps/desktop/src/main/runtime-host-artifacts-ipc-main.ts index f5c3b7200b..d391e07072 100644 --- a/apps/desktop/src/main/runtime-host-artifacts-ipc-main.ts +++ b/apps/desktop/src/main/runtime-host-artifacts-ipc-main.ts @@ -182,12 +182,25 @@ export function registerRuntimeHostAttachmentPreviewIpc( ); } -async function materializeArtifact( +/** + * Streams the artifact to a staging file beside the destination, then + * replaces the destination in one atomic step. Exported for the + * fault-injection test of #4832: `replaceDestination` lets a test fail the + * final replacement to prove the previous destination survives; production + * always uses `rename`, which replaces an existing destination atomically on + * every platform (no unlink first — that would lose the destination if the + * rename failed). + */ +export async function materializeArtifact( client: DesktopRuntimeHostClient, sessionId: string, artifactId: string, targetPath: string, expectedBytes: number, + replaceDestination: (stagingPath: string, targetPath: string) => Promise = + async (stagingPath, targetPath) => { + await rename(stagingPath, targetPath); + }, ): Promise { await mkdir(dirname(targetPath), { recursive: true }); const stagingPath = join( @@ -220,8 +233,10 @@ async function materializeArtifact( } await handle.sync(); await handle.close(); - await rm(targetPath, { force: true }); - await rename(stagingPath, targetPath); + // rename(2) replaces an existing destination in one atomic step on every + // platform; unlinking the destination first would turn any rename + // failure into a lost destination (#4832). + await replaceDestination(stagingPath, targetPath); } catch (error) { await handle.close().catch(() => undefined); await rm(stagingPath, { force: true }).catch(() => undefined);