diff --git a/.changeset/assistant-storage-cleanup.md b/.changeset/assistant-storage-cleanup.md new file mode 100644 index 000000000..891460a6b --- /dev/null +++ b/.changeset/assistant-storage-cleanup.md @@ -0,0 +1,5 @@ +--- +"@sapiom/harness": patch +--- + +Preserve Assistant storage commit uncertainty when temporary-file cleanup also fails. diff --git a/packages/harness/docs/assistant-history.md b/packages/harness/docs/assistant-history.md index 7e32e068f..9343c95bb 100644 --- a/packages/harness/docs/assistant-history.md +++ b/packages/harness/docs/assistant-history.md @@ -163,3 +163,7 @@ Continue allows up to 45 seconds total for cold startup and frozen preparation. Selected inspection retains its separate 15-second limit. The caller's remaining deadline and End can cancel either operation sooner; provisional cleanup remains bound to the exact runtime that the operation started. + +Temporary-file cleanup cannot replace the primary storage failure. An unconfirmed +publication remains distinguishable for Resume reconciliation even when rollback +and cleanup also fail; cleanup failures after successful publication are surfaced. diff --git a/packages/harness/src/core/assistant-session-files.test.ts b/packages/harness/src/core/assistant-session-files.test.ts index 27cc0be86..c9a203fa6 100644 --- a/packages/harness/src/core/assistant-session-files.test.ts +++ b/packages/harness/src/core/assistant-session-files.test.ts @@ -3,15 +3,84 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, expect, it, vi } from "vitest"; import { AssistantSessionStore } from "./assistant-session-store.js"; +import { + AssistantStorageCommitUnconfirmedError, + writeAssistantJson, +} from "./assistant-session-files.js"; vi.mock("node:fs/promises", async (original) => { const fs = await original(); - return { ...fs, open: vi.fn(fs.open), rename: vi.fn(fs.rename) }; + return { + ...fs, + open: vi.fn(fs.open), + rename: vi.fn(fs.rename), + rm: vi.fn(fs.rm), + }; }); let root: string; afterEach(async () => { vi.restoreAllMocks(); - if (root) await fs.rm(root, { recursive: true, force: true }); + const actual = await vi.importActual("node:fs/promises"); + if (root) await actual.rm(root, { recursive: true, force: true }); +}); + +it("preserves uncertain publication proof when rollback and temporary cleanup also fail", async () => { + root = await fs.mkdtemp(join(tmpdir(), "assistant-files-")); + const actual = await vi.importActual("node:fs/promises"); + const destination = join(root, "lifecycle.json"); + const prior = { lifecycleRevision: 1 }; + const next = { + lifecycleRevision: 2, + resumeOperation: { operationId: "retained-proof" }, + }; + await actual.writeFile(destination, JSON.stringify(prior)); + let published = false; + vi.mocked(fs.rename).mockImplementation(async (from, to) => { + if (String(from).endsWith(".rollback")) throw new Error("rollback failed"); + await actual.rename(from, to); + if (to === destination) published = true; + }); + vi.mocked(fs.open).mockImplementation(async (path, flags, mode) => { + if (flags === "r" && published) + throw Object.assign(new Error("durability failed"), { code: "EIO" }); + return actual.open(path, flags, mode); + }); + const removed: unknown[] = []; + vi.mocked(fs.rm).mockImplementation(async (path) => { + removed.push(path); + throw new Error("cleanup failed"); + }); + + await expect( + writeAssistantJson(root, "lifecycle.json", next), + ).rejects.toBeInstanceOf(AssistantStorageCommitUnconfirmedError); + expect(removed).toHaveLength(3); + expect(JSON.parse(await actual.readFile(destination, "utf8"))).toEqual(next); + expect( + JSON.parse(await actual.readFile(`${destination}.previous`, "utf8")), + ).toEqual(prior); +}); + +it("preserves a pre-publication failure when temporary cleanup fails", async () => { + root = await fs.mkdtemp(join(tmpdir(), "assistant-files-")); + const primary = Object.assign(new Error("write failed"), { code: "ENOSPC" }); + vi.mocked(fs.open).mockRejectedValue(primary); + vi.mocked(fs.rm).mockRejectedValue(new Error("cleanup failed")); + await expect(writeAssistantJson(root, "lifecycle.json", {})).rejects.toBe( + primary, + ); +}); + +it("reports temporary cleanup failure after an otherwise successful publication", async () => { + root = await fs.mkdtemp(join(tmpdir(), "assistant-files-")); + const cleanup = new Error("cleanup failed"); + vi.mocked(fs.rm).mockRejectedValue(cleanup); + await expect( + writeAssistantJson(root, "lifecycle.json", { revision: 1 }), + ).rejects.toBe(cleanup); + expect( + JSON.parse(await fs.readFile(join(root, "lifecycle.json"), "utf8")), + ).toEqual({ revision: 1 }); }); it.each(["EPERM", "EINVAL", "ENOTSUP"])( diff --git a/packages/harness/src/core/assistant-session-files.ts b/packages/harness/src/core/assistant-session-files.ts index 3dd7961fb..d805ae66e 100644 --- a/packages/harness/src/core/assistant-session-files.ts +++ b/packages/harness/src/core/assistant-session-files.ts @@ -95,6 +95,7 @@ export async function writeAssistantJson( rollback = `${temporary}.rollback`; let retained = false, published = false; + let cleanupFailure: PromiseRejectedResult | undefined; try { const file = await fs.open(temporary, "wx", 0o600); try { @@ -135,10 +136,13 @@ export async function writeAssistantJson( throw new AssistantStorageCommitUnconfirmedError(); throw error; } finally { - await Promise.all( + const cleanup = await Promise.allSettled( [temporary, backup, rollback].map((file) => fs.rm(file, { force: true })), ); + // Cleanup must not disguise whether the durable publication is uncertain. + cleanupFailure = cleanup.find((result) => result.status === "rejected"); } + if (cleanupFailure) throw cleanupFailure.reason; } /** Match DurableFileLock on platforms without directory-handle fsync support. */