From 04ba5caf63adc31a45b3239bec024ad4ee9f43fd Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 24 Oct 2026 07:38:15 +0800 Subject: [PATCH] fix(artifacts): preserve cancellation observed during directory sync --- src/artifacts/SafeOutputTree.ts | 2 ++ .../firmware/firmwareIntegration.test.ts | 11 ++++++++-- .../firmware/firmwarePublicationPaths.test.ts | 7 +++++- .../firmwareFileLifetime.test.ts | 22 ++++++++++++++----- 4 files changed, 34 insertions(+), 8 deletions(-) diff --git a/src/artifacts/SafeOutputTree.ts b/src/artifacts/SafeOutputTree.ts index 4ba1a108b5..6a6a8b1908 100644 --- a/src/artifacts/SafeOutputTree.ts +++ b/src/artifacts/SafeOutputTree.ts @@ -379,10 +379,12 @@ export class SafeOutputTree { handles.push(directory); await assertHandleIdentity(directory.handle, identity, path); await directory.handle.sync(); + abortIfNeeded(signal); await this.#closeHandle(directory); handles.pop(); } await parent.handle.sync(); + abortIfNeeded(signal); await this.#assertExactEntries(); abortIfNeeded(signal); this.#published = true; diff --git a/tests/boundary/firmware/firmwareIntegration.test.ts b/tests/boundary/firmware/firmwareIntegration.test.ts index 1cd77804c5..b6326e423e 100644 --- a/tests/boundary/firmware/firmwareIntegration.test.ts +++ b/tests/boundary/firmware/firmwareIntegration.test.ts @@ -1,5 +1,6 @@ import { STDIO_DEFAULT_MAX_BUFFER_SIZE } from "@modelcontextprotocol/server"; import { access, mkdir, readFile, writeFile } from "node:fs/promises"; +import { dirname } from "node:path"; import { expect, it as test, vi } from "vitest"; import { firmwareFixture, @@ -38,6 +39,7 @@ const gapsWithContinuations = ( const directoryCloseFailure = vi.hoisted(() => ({ mode: "off" as "off" | "transient" | "persistent", attempts: 0, + parent: "", })); vi.mock("node:fs/promises", async (importOriginal) => { @@ -48,6 +50,7 @@ vi.mock("node:fs/promises", async (importOriginal) => { const handle = await filesystem.open(...args); if ( directoryCloseFailure.mode !== "off" && + String(args[0]) === directoryCloseFailure.parent && (await handle.stat()).isDirectory() ) { const close = handle.close.bind(handle); @@ -251,7 +254,7 @@ it("returns firmware output after a transient published-directory close failure" expect(await readFile(normalized.files[0]?.path ?? "", "utf8")).toBe( "firmware=true\n", ); - expect(closeFailure.attempts()).toBeGreaterThanOrEqual(4); + expect(closeFailure.attempts()).toBeGreaterThanOrEqual(2); } finally { closeFailure.restore(); } @@ -300,7 +303,7 @@ it("retains the complete firmware result when published output cleanup stays unc expect(await readFile(`${fixture.output}/rootfs/config`, "utf8")).toBe( "firmware=true\n", ); - expect(closeFailure.attempts()).toBeGreaterThanOrEqual(4); + expect(closeFailure.attempts()).toBeGreaterThanOrEqual(2); } finally { closeFailure.restore(); } @@ -631,12 +634,16 @@ const injectPublishedDirectoryCloseFailure = (persistent: boolean) => { .spyOn(SafeOutputTree.prototype, "commit") .mockImplementation(async function (this: SafeOutputTree) { directoryCloseFailure.mode = persistent ? "persistent" : "transient"; + // Only the parent descriptor closes after the publication cutoff. A + // child/root close failure aborts sync before output becomes published. + directoryCloseFailure.parent = dirname(this.outputRoot); await commit.call(this); }); return { attempts: () => directoryCloseFailure.attempts, restore: () => { directoryCloseFailure.mode = "off"; + directoryCloseFailure.parent = ""; spy.mockRestore(); }, }; diff --git a/tests/boundary/firmware/firmwarePublicationPaths.test.ts b/tests/boundary/firmware/firmwarePublicationPaths.test.ts index 2a9d06fd54..6ec6c7a700 100644 --- a/tests/boundary/firmware/firmwarePublicationPaths.test.ts +++ b/tests/boundary/firmware/firmwarePublicationPaths.test.ts @@ -35,7 +35,12 @@ it.each(["existing", "missing-parent", "relative"] as const)( ok: false, error: { _tag: "AnalysisInputError", - issues: [{ path: ["output_directory"], reason: "invalid_value" }], + issues: [ + { + path: ["output_directory"], + reason: kind === "relative" ? "invalid_format" : "invalid_value", + }, + ], }, }); expect(launcher).not.toHaveBeenCalled(); diff --git a/tests/process-global/firmwareFileLifetime.test.ts b/tests/process-global/firmwareFileLifetime.test.ts index 0b05c688c1..ae94efbfed 100644 --- a/tests/process-global/firmwareFileLifetime.test.ts +++ b/tests/process-global/firmwareFileLifetime.test.ts @@ -443,7 +443,10 @@ it.each([null, "caller cancellation", new Error("caller cancellation")])( engine: "binwalk", operation: "inspect_firmware_regions", }); - expect(error).toBeInstanceOf(AnalysisCancelledError); + expect( + error, + error instanceof Error ? error.stack : String(error), + ).toBeInstanceOf(AnalysisCancelledError); expect(error.cause).toBe(reason); await files.close(); }, @@ -729,14 +732,17 @@ it("preserves primitive cancellation before publication commit and rolls back th const fixture = await publication(controller.signal); const create = SafeOutputTree.create; vi.spyOn(SafeOutputTree, "create").mockImplementation(async (...args) => { - const tree = await create(...args); + const tree = await create.call(SafeOutputTree, ...args); controller.abort(null); return tree; }); const error = await publishFirmwareExtraction(fixture.context).catch( (cause: unknown) => cause, ); - expect(error).toBeInstanceOf(AnalysisCancelledError); + expect( + error, + error instanceof Error ? error.stack : String(error), + ).toBeInstanceOf(AnalysisCancelledError); if (error instanceof AnalysisCancelledError) expect(error.cause).toBeNull(); await expect(access(fixture.context.outputDirectory)).rejects.toMatchObject({ code: "ENOENT", @@ -765,8 +771,10 @@ it.each(["caller", "deadline"])( : undefined, ); const create = SafeOutputTree.create; + let outputTree: SafeOutputTree | undefined; vi.spyOn(SafeOutputTree, "create").mockImplementation(async (...args) => { - const tree = await create(...args); + const tree = await create.call(SafeOutputTree, ...args); + outputTree = tree; fault.selected = tree.outputRoot; fault.onSync = () => controller.abort(reason); return tree; @@ -774,9 +782,13 @@ it.each(["caller", "deadline"])( const error = await publishFirmwareExtraction(fixture.context).catch( (cause: unknown) => cause, ); + expect(outputTree?.published).toBe(false); if (!(error instanceof AnalysisError)) throw new Error("Expected typed tree failure"); - expect(error.cleanupIncomplete).toBe(true); + expect( + error.cleanupIncomplete, + JSON.stringify(projectAnalysisError(error)), + ).toBe(true); expect(error.diagnostics?.primary_error).toMatchObject({ code: "cleanup_incomplete", details: {