From 6ed97a22ed37feb414ebbfa683e53250d052e342 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Tue, 27 Oct 2026 13:19:20 +0800 Subject: [PATCH] fix(export): retain verification cleanup failures --- .../decompileDirectory/decompileAll.test.ts | 57 ++++++++++++++++++- .../decompileDirectory/decompileAll.ts | 18 +++++- .../decompileDirectory/exportResources.ts | 18 +++++- .../decompileDirectory/exportTree.ts | 17 +++++- 4 files changed, 103 insertions(+), 7 deletions(-) diff --git a/src/application/decompileDirectory/decompileAll.test.ts b/src/application/decompileDirectory/decompileAll.test.ts index b84cc3928b..17470a61d3 100644 --- a/src/application/decompileDirectory/decompileAll.test.ts +++ b/src/application/decompileDirectory/decompileAll.test.ts @@ -1,4 +1,11 @@ -import { mkdir, readFile, rm, symlink, writeFile } from "node:fs/promises"; +import { + mkdir, + readFile, + rm, + stat, + symlink, + writeFile, +} from "node:fs/promises"; import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; @@ -13,6 +20,7 @@ import { AnalysisCleanupLease } from "../../artifacts/AnalysisCleanupLease.js"; import { EnhancedTools } from "../EnhancedTools.js"; import { DecompileExport } from "./exportTree.js"; import { ExportResources } from "./exportResources.js"; +import { OwnedFileHandle } from "../../filesystem/OwnedFileHandle.js"; import { OwnedStagedOutput } from "../../filesystem/OwnedStagedOutput.js"; import { composedLimitationBudget, @@ -1930,3 +1938,50 @@ describe("decompile_all publication cleanup", () => { }, ); }); + +describe("decompile_all resumed file cleanup", () => { + const directory = useOutputDirectory(); + afterEach(() => vi.restoreAllMocks()); + it.each(["journaled", "unjournaled"])( + "reports a %s file verification close failure after final cleanup succeeds", + async (kind) => { + const { tools } = harness(); + expect((await run(tools, directory())).ok).toBe(true); + if (kind === "unjournaled") { + const journal = join(directory(), DECOMPILE_DIRECTORY_JOURNAL_NAME); + const header = (await readFile(journal, "utf8")).split("\n")[0]; + await writeFile(journal, `${header}\n`); + } + const identity = await stat(join(directory(), "functions/0x1000.c")); + const close = OwnedFileHandle.prototype.close; + let failed = false; + vi.spyOn(OwnedFileHandle.prototype, "close").mockImplementation( + async function (this: OwnedFileHandle) { + const observed = await this.handle.stat(); + if ( + !failed && + observed.dev === identity.dev && + observed.ino === identity.ino + ) { + failed = true; + throw new Error("verification close denied"); + } + return close.call(this); + }, + ); + const result = await run(tools, directory(), { resume: true }); + expect(failed).toBe(true); + expect(result.ok).toBe(false); + if (result.ok) return; + expect(result.error.message).toContain("verification cleanup failed"); + expect(result.error.cleanupIncomplete).toBe(false); + expect( + await readFile(join(directory(), "functions/0x1000.c"), "utf8"), + ).toContain(sentinel); + const resumed = await run(tools, directory(), { resume: true }); + expect(resumed.ok).toBe(true); + if (resumed.ok) + expect(resumed.value).toMatchObject({ counts: { skipped: 3 } }); + }, + ); +}); diff --git a/src/application/decompileDirectory/decompileAll.ts b/src/application/decompileDirectory/decompileAll.ts index 7a441969d1..c93d83edc1 100644 --- a/src/application/decompileDirectory/decompileAll.ts +++ b/src/application/decompileDirectory/decompileAll.ts @@ -1,6 +1,8 @@ import { join } from "node:path"; import { z } from "zod"; +import { ArtifactReaderFailure } from "../../artifacts/ArtifactReader.js"; + import { DECOMPILE_DIRECTORY_INDEX_NAME, DECOMPILE_DIRECTORY_INLINE_FAILURE_LIMIT, @@ -33,6 +35,7 @@ import { err, ok, type Result } from "../../domain/result.js"; import { classifyOutputPath, DecompileExport, + exportOutputFailure, type ExportIdentity, } from "./exportTree.js"; import { @@ -355,6 +358,7 @@ const publishSelected = async ( procedure.address, input.signal, ); + if (typeof existing === "object") return err(existing.error); if (isAborted(input.signal)) return err(cancelled(input.signal)); if (existing === "skip") { input.state.skipped += 1; @@ -707,7 +711,12 @@ const readBody = async ( }; }; -type ExistingFile = "skip" | "write" | "conflict" | "io"; +type ExistingFile = + | "skip" + | "write" + | "conflict" + | "io" + | { readonly error: AnalysisError }; const existingFile = async ( session: DecompileExport, @@ -724,6 +733,13 @@ const existingFile = async ( const digest = await session .hashFile(record.relative_path, signal) .catch((cause: unknown) => cause); + if (digest instanceof ArtifactReaderFailure && digest.cleanup !== undefined) + return { + error: exportOutputFailure( + "decompile file verification cleanup failed", + digest, + ), + }; if (typeof digest === "string") return digest === record.sha256 ? "skip" : "conflict"; return nodeAbsent(digest) ? "write" : "io"; diff --git a/src/application/decompileDirectory/exportResources.ts b/src/application/decompileDirectory/exportResources.ts index 77474e4b0e..01c41fb28c 100644 --- a/src/application/decompileDirectory/exportResources.ts +++ b/src/application/decompileDirectory/exportResources.ts @@ -11,6 +11,8 @@ import { import { join } from "node:path"; import { AnalysisCleanupLease } from "../../artifacts/AnalysisCleanupLease.js"; +import { ArtifactFileCleanupFailure } from "../../artifacts/ArtifactFileOperation.js"; +import { RegularFileCleanupFailure } from "../../filesystem/OwnedRegularFile.js"; import { ArtifactResourceScope } from "../../artifacts/ArtifactResourceScope.js"; import { DECOMPILE_DIRECTORY_JOURNAL_NAME } from "../../contracts/decompileDirectory.js"; import type { AnalysisError } from "../../domain/analysisErrorBase.js"; @@ -97,14 +99,26 @@ export class ExportResources { /** Check existing output with bounded memory and the shared file identity owner. */ async hashFile(path: string, signal?: AbortSignal): Promise { + let cleanupLease: AnalysisCleanupLease | undefined; try { const hash = await hashStableFile(path, { ...(signal === undefined ? {} : { signal }), - retainFailedClose: (handle) => - this.#files.retain({ kind: "file-handle", resource: path, handle }), + retainFailedClose: (handle) => { + cleanupLease = new AnalysisCleanupLease(() => handle.close(), [path]); + this.#files.retain({ + kind: "lease", + resource: path, + lease: cleanupLease, + }); + }, }); return hash.sha256; } catch (cause: unknown) { + if ( + cause instanceof RegularFileCleanupFailure && + cleanupLease !== undefined + ) + throw new ArtifactFileCleanupFailure(cause, cleanupLease); throw stableFileHashCause(cause); } } diff --git a/src/application/decompileDirectory/exportTree.ts b/src/application/decompileDirectory/exportTree.ts index 0298480dfd..8a79e19082 100644 --- a/src/application/decompileDirectory/exportTree.ts +++ b/src/application/decompileDirectory/exportTree.ts @@ -396,7 +396,7 @@ const commitFunctionFile = async ( if (linked instanceof ArtifactReaderFailure && linked.cleanup !== undefined) return { status: "error", - error: exportPublicationFailure( + error: exportOutputFailure( "decompile function publication cleanup failed", linked, ), @@ -405,6 +405,17 @@ const commitFunctionFile = async ( const existing = await resources .hashFile(finalPath) .catch((cause: unknown) => cause); + if ( + existing instanceof ArtifactReaderFailure && + existing.cleanup !== undefined + ) + return { + status: "error", + error: exportOutputFailure( + "decompile file verification cleanup failed", + existing, + ), + }; if (typeof existing !== "string") return { status: "io" }; return existing === digest ? { status: "matched", sha256: digest } @@ -972,7 +983,7 @@ const writeOwnedIndex = async ( return ok(undefined); } catch (cause: unknown) { return err( - exportPublicationFailure( + exportOutputFailure( cause instanceof ArtifactReaderFailure && cause.cleanup !== undefined ? "decompile index publication cleanup failed" : "decompile index could not be written", @@ -983,7 +994,7 @@ const writeOwnedIndex = async ( }; /** Preserve the publisher's parsed failure and exact cleanup authority. */ -const exportPublicationFailure = ( +export const exportOutputFailure = ( message: string, cause: unknown, ): AnalysisOutputError => {