From 921142efbc31253043e6313cf8e8f2a5cde07320 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Mon, 26 Oct 2026 22:42:31 +0800 Subject: [PATCH] fix(hopper): own profile reads and retain completed coordinate evidence --- .../native/fileOffsetPartialObservation.ts | 16 +- src/hopper/HopperAnalysisProfile.test.ts | 342 ++++++++++++++---- src/hopper/HopperAnalysisProfile.ts | 274 +++++++++----- src/hopper/HopperFileOffset.ts | 122 ++++--- .../hopper/hopperFileOffsetClose.test.ts | 241 +++++++++++- 5 files changed, 770 insertions(+), 225 deletions(-) diff --git a/src/domain/native/fileOffsetPartialObservation.ts b/src/domain/native/fileOffsetPartialObservation.ts index 5376790e7a..c84cbdb585 100644 --- a/src/domain/native/fileOffsetPartialObservation.ts +++ b/src/domain/native/fileOffsetPartialObservation.ts @@ -1,12 +1,18 @@ -/** Native mapping facts retained when original-file coordinates cannot be verified. */ +/** Native mapping facts retained through source verification and finalization. */ export interface FileOffsetPartialObservation { readonly operation: "address_to_file_offset"; readonly address: string; readonly provider_file_offset: number; readonly provider_source_path: string | null; readonly provider_image_header_hex: string | null; - readonly original_file_offset: { - readonly available: false; - readonly reason: string; - }; + readonly original_file_offset: + | { + readonly available: false; + readonly reason: string; + } + | { + readonly available: true; + readonly file_offset: number; + readonly source_sha256: string; + }; } diff --git a/src/hopper/HopperAnalysisProfile.test.ts b/src/hopper/HopperAnalysisProfile.test.ts index eae0346efb..f48f5d6b35 100644 --- a/src/hopper/HopperAnalysisProfile.test.ts +++ b/src/hopper/HopperAnalysisProfile.test.ts @@ -1,22 +1,18 @@ import { machoTargetSelectionSchema } from "../domain/apple/machoTargetSelection.js"; import { createFunctionCoordinateContext } from "../domain/native/functionCoordinateContext.js"; import { createHash } from "node:crypto"; -import { PassThrough } from "node:stream"; -import { writeFile } from "node:fs/promises"; +import { open, writeFile } from "node:fs/promises"; +import { execFileSync } from "node:child_process"; +import { RegularFileCleanupFailure } from "../filesystem/OwnedRegularFile.js"; import { join } from "node:path"; import { afterEach, expect, it, vi } from "vitest"; -vi.mock("node:fs", async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - createReadStream: vi.fn(actual.createReadStream), - }; +vi.mock("node:fs/promises", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, open: vi.fn(actual.open) }; }); -import { createReadStream } from "node:fs"; - import { createInspectionResources } from "../../tests/fixtures/inspectionResources.js"; import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js"; import { @@ -28,6 +24,14 @@ let inspectionResources = createInspectionResources(); afterEach(async () => { await inspectionResources.close(); inspectionResources = createInspectionResources(); + vi.mocked(open).mockReset(); + vi.mocked(open).mockImplementation( + ( + await vi.importActual( + "node:fs/promises", + ) + ).open, + ); }); it("routes physical universal Mach-O containers through Hopper's FAT loader", () => { @@ -214,51 +218,30 @@ it("commits a thin header selection only for derived Hopper loader arguments", a ); }); -it("keeps a read failure primary when cancellation arrives with the stream error", async () => { - const path = "/fixture/arm64.macho"; - const target = { - path, - sha256: "a".repeat(64), - kind: "executable" as const, - format: "mach-o" as const, - architecture: "arm64" as const, - availableArchitectures: ["arm64" as const], - }; +it("keeps a read failure primary when cancellation arrives during descriptor cleanup", async () => { + const directory = await createTestTempDirectory("rea-hopper-profile-read-"); + const path = join(directory, "arm64.macho"); + await writeFile(path, Buffer.from([0xfe, 0xed, 0xfa, 0xcf])); const controller = new AbortController(); const reason = { request: "cancel-after-read-error" }; const physicalFailure = new Error("fixture read failure"); - const stream = new PassThrough(); - vi.mocked(createReadStream).mockReturnValueOnce( - stream as unknown as ReturnType, - ); - stream.once("error", () => controller.abort(reason)); - - const resolving = resolveHopperAnalysisProfile(target, { - launcherPath: "/fixture/hopper", - loaderArgsOverride: [], - provider: { id: "hopper", name: "Hopper", version: null }, - resourceScope: inspectionResources.owner, - signal: controller.signal, + const actualFs = + await vi.importActual( + "node:fs/promises", + ); + vi.mocked(open).mockImplementationOnce(async (...args) => { + const handle = await actualFs.open(...args); + vi.spyOn(handle, "read").mockRejectedValueOnce(physicalFailure); + const close = handle.close.bind(handle); + vi.spyOn(handle, "close").mockImplementation(async () => { + controller.abort(reason); + await close(); + }); + return handle; }); - stream.destroy(physicalFailure); - - const result = await resolving; - expect(result.ok).toBe(false); - if (result.ok) throw new Error("A failed header read was accepted"); - expect(result.error._tag).toBe("BinaryTargetError"); - expect(result.error.cause).toBe(physicalFailure); -}); - -it("retains the exact caller reason when the owned header stream is aborted", async () => { - const controller = new AbortController(); - const reason = { request: "cancel-header-read" }; - const stream = new PassThrough(); - vi.mocked(createReadStream).mockReturnValueOnce( - stream as unknown as ReturnType, - ); - const resolving = resolveHopperAnalysisProfile( + const result = await resolveHopperAnalysisProfile( { - path: "/fixture/arm64.macho", + path, sha256: "a".repeat(64), kind: "executable", format: "mach-o", @@ -266,20 +249,60 @@ it("retains the exact caller reason when the owned header stream is aborted", as availableArchitectures: ["arm64"], }, { - launcherPath: "/fixture/hopper", + launcherPath: path, loaderArgsOverride: [], provider: { id: "hopper", name: "Hopper", version: null }, resourceScope: inspectionResources.owner, signal: controller.signal, }, ); - controller.abort(reason); + expect(result).toMatchObject({ + ok: false, + error: { _tag: "BinaryTargetError", cause: physicalFailure }, + }); +}); - const result = await resolving; - expect(result.ok).toBe(false); - if (result.ok) throw new Error("An aborted header read was accepted"); - expect(result.error._tag).toBe("AnalysisCancelledError"); - expect(result.error.cause).toBe(reason); +it("retains the exact caller reason when cancellation arrives during the owned header read", async () => { + const directory = await createTestTempDirectory("rea-hopper-profile-abort-"); + const path = join(directory, "arm64.macho"); + await writeFile(path, Buffer.from([0xfe, 0xed, 0xfa, 0xcf])); + const controller = new AbortController(); + const reason = { request: "cancel-header-read" }; + const actualFs = + await vi.importActual( + "node:fs/promises", + ); + vi.mocked(open).mockImplementationOnce(async (...args) => { + const handle = await actualFs.open(...args); + const read = handle.read.bind(handle); + vi.spyOn(handle, "read").mockImplementationOnce(async (...readArgs) => { + const result = await read(...readArgs); + controller.abort(reason); + return result; + }); + return handle; + }); + const result = await resolveHopperAnalysisProfile( + { + path, + sha256: "a".repeat(64), + kind: "executable", + format: "mach-o", + architecture: "arm64", + availableArchitectures: ["arm64"], + }, + { + launcherPath: path, + loaderArgsOverride: [], + provider: { id: "hopper", name: "Hopper", version: null }, + resourceScope: inspectionResources.owner, + signal: controller.signal, + }, + ); + expect(result).toMatchObject({ + ok: false, + error: { _tag: "AnalysisCancelledError", cause: reason }, + }); }); it("keeps failed Mach-O preflight cleanup attached to the supplied profile owner", async () => { @@ -295,10 +318,6 @@ it("keeps failed Mach-O preflight cleanup attached to the supplied profile owner await vi.importActual( "node:fs/promises", ); - const actualNodeFs = - await vi.importActual("node:fs"); - vi.mocked(createReadStream).mockReset(); - vi.mocked(createReadStream).mockImplementation(actualNodeFs.createReadStream); const closeFailure = new Error("profile preflight close failure"); let allowClose = false; const imageFileSystem = { @@ -354,10 +373,7 @@ it("preserves launcher fingerprint filesystem failures instead of an absent prof new Error("EACCES: fixture launcher read denied"), { code: "EACCES" }, ); - const stream = new PassThrough(); - vi.mocked(createReadStream).mockReturnValueOnce( - stream as unknown as ReturnType, - ); + vi.mocked(open).mockRejectedValueOnce(failure); const resolving = resolveHopperAnalysisProfile( { path: "/fixture/input", @@ -374,7 +390,6 @@ it("preserves launcher fingerprint filesystem failures instead of an absent prof resourceScope: inspectionResources.owner, }, ); - stream.destroy(failure); const result = await resolving; expect(result).toMatchObject({ ok: false, @@ -385,3 +400,198 @@ it("preserves launcher fingerprint filesystem failures instead of an absent prof }, }); }); + +it.skipIf(process.platform === "win32").each(["header", "launcher"] as const)( + "rejects a real FIFO at the %s profile boundary without a writer", + async (kind) => { + const directory = await createTestTempDirectory("rea-hopper-profile-fifo-"); + const path = join(directory, "hopper.fifo"); + execFileSync("mkfifo", [path]); + const result = await resolveHopperAnalysisProfile( + { + path, + sha256: "a".repeat(64), + kind: "executable", + format: kind === "header" ? "mach-o" : "elf", + architecture: "x86_64", + availableArchitectures: ["x86_64"], + }, + { + launcherPath: path, + loaderArgsOverride: [], + provider: { id: "hopper", name: "Hopper", version: null }, + resourceScope: inspectionResources.owner, + }, + ); + expect(result).toMatchObject({ + ok: false, + error: { + _tag: kind === "header" ? "BinaryTargetError" : "ProviderAdapterError", + cause: { name: "NonRegularFileReadError", path }, + }, + }); + }, +); + +it.each([ + ["header", false], + ["header", true], + ["launcher", false], + ["launcher", true], +] as const)( + "retains the %s descriptor through failed close, late cancellation and retry (read failed: %s)", + async (kind, readFails) => { + const directory = await createTestTempDirectory( + "rea-hopper-profile-close-", + ); + const path = join(directory, "selected-file"); + await writeFile(path, Buffer.from([0xfe, 0xed, 0xfa, 0xcf])); + const actualFs = + await vi.importActual( + "node:fs/promises", + ); + const controller = new AbortController(); + const closeFailure = new Error("profile descriptor close failed"); + const physicalFailure = new Error("profile descriptor read failed"); + const reason = { request: "cancel-during-final-close" }; + let allowClose = false; + let closeAttempts = 0; + vi.mocked(open).mockImplementationOnce(async (...args) => { + const handle = await actualFs.open(...args); + if (readFails) + vi.spyOn(handle, "read").mockRejectedValueOnce(physicalFailure); + const close = handle.close.bind(handle); + vi.spyOn(handle, "close").mockImplementation(async () => { + closeAttempts += 1; + if (!allowClose) { + controller.abort(reason); + throw closeFailure; + } + await close(); + }); + return handle; + }); + const target = { + path, + sha256: "a".repeat(64), + kind: "executable" as const, + format: kind === "header" ? ("mach-o" as const) : ("elf" as const), + architecture: "x86_64" as const, + availableArchitectures: ["x86_64" as const], + }; + const options = { + launcherPath: path, + loaderArgsOverride: ["--fixture"], + provider: { id: "hopper", name: "Hopper", version: null }, + resourceScope: inspectionResources.owner, + }; + const first = await resolveHopperAnalysisProfile(target, { + ...options, + signal: controller.signal, + }); + expect(first.ok).toBe(false); + if (first.ok) throw new Error("Failed close was accepted"); + expect(first.error.cleanupIncomplete).toBe(true); + expect(first.error.cleanupResources).toEqual([path]); + expect(first.error.cause).toBe(readFails ? physicalFailure : reason); + const cleanup = first.error.secondaryFailures[0]; + expect(cleanup?.cause).toBeInstanceOf(RegularFileCleanupFailure); + expect(cleanup?.cause).toMatchObject({ + path, + cleanupCause: closeFailure, + outcome: { kind: readFails ? "failed" : "completed" }, + }); + if (!readFails) + expect(first.error.diagnostics).toMatchObject({ + completed_profile_read: + kind === "header" + ? 0xfeedfacf + : { + sha256: createHash("sha256") + .update(Buffer.from([0xfe, 0xed, 0xfa, 0xcf])) + .digest("hex"), + bytes: 4, + }, + }); + expect(first.error._tag).toBe( + readFails + ? kind === "header" + ? "BinaryTargetError" + : "ProviderAdapterError" + : "AnalysisCancelledError", + ); + const retry = await resolveHopperAnalysisProfile(target, options); + expect(retry.ok).toBe(false); + if (retry.ok) throw new Error("Failed cleanup retry was accepted"); + expect(retry.error.cleanupIncomplete).toBe(true); + expect(retry.error.cleanupResources).toEqual([path]); + expect(open).toHaveBeenCalledTimes(1); + expect(closeAttempts).toBe(2); + allowClose = true; + await inspectionResources.close(); + expect(closeAttempts).toBe(3); + expect(first.error.cleanupIncomplete).toBe(false); + expect(retry.error.cleanupIncomplete).toBe(false); + }, +); + +it.each(["header", "launcher"] as const)( + "retains completed %s facts when cancellation arrives during successful final close", + async (kind) => { + const directory = await createTestTempDirectory( + "rea-hopper-profile-late-abort-", + ); + const path = join(directory, "selected-file"); + const bytes = Buffer.from([0xfe, 0xed, 0xfa, 0xcf]); + await writeFile(path, bytes); + const controller = new AbortController(); + const reason = { request: "cancel-on-final-close" }; + const actualFs = + await vi.importActual( + "node:fs/promises", + ); + vi.mocked(open).mockImplementationOnce(async (...args) => { + const handle = await actualFs.open(...args); + const close = handle.close.bind(handle); + vi.spyOn(handle, "close").mockImplementation(async () => { + await close(); + controller.abort(reason); + }); + return handle; + }); + const result = await resolveHopperAnalysisProfile( + { + path, + sha256: "a".repeat(64), + kind: "executable", + format: kind === "header" ? "mach-o" : "elf", + architecture: "x86_64", + availableArchitectures: ["x86_64"], + }, + { + launcherPath: path, + loaderArgsOverride: ["--fixture"], + provider: { id: "hopper", name: "Hopper", version: null }, + resourceScope: inspectionResources.owner, + signal: controller.signal, + }, + ); + expect(result).toMatchObject({ + ok: false, + error: { + _tag: "AnalysisCancelledError", + cause: reason, + cleanupIncomplete: false, + diagnostics: { + completed_profile_read: + kind === "header" + ? 0xfeedfacf + : { + sha256: createHash("sha256").update(bytes).digest("hex"), + bytes: 4, + }, + }, + }, + }); + }, +); diff --git a/src/hopper/HopperAnalysisProfile.ts b/src/hopper/HopperAnalysisProfile.ts index 3045b228b1..159ac5b122 100644 --- a/src/hopper/HopperAnalysisProfile.ts +++ b/src/hopper/HopperAnalysisProfile.ts @@ -1,5 +1,19 @@ -import { createHash } from "node:crypto"; -import { createReadStream } from "node:fs"; +import { AnalysisCleanupLease } from "../artifacts/AnalysisCleanupLease.js"; +import { bindArtifactFileCleanup } from "../artifacts/ArtifactFileOperation.js"; +import { ArtifactResourceRetryFailure } from "../artifacts/ArtifactResourceScope.js"; +import { + hashStableFile, + stableFileHashCause, + type StableFileHashOptions, +} from "../filesystem/StableFileHash.js"; +import { + RegularFileCleanupFailure, + RegularFileCancelled, + regularFileCancellation, + withOwnedRegularFile, +} from "../filesystem/OwnedRegularFile.js"; + +import { assertRegularFileActive } from "../filesystem/RegularFile.js"; import type { AnalysisProfileResolution, @@ -22,6 +36,8 @@ import { } from "../domain/analysisErrorCore.js"; import { BinaryTargetError } from "../domain/configurationErrors.js"; import { ProviderAdapterError } from "../domain/providerAdapterError.js"; +import { ProviderCleanupError } from "../domain/providerCleanupError.js"; +import { jsonValueSchema } from "../domain/jsonValue.js"; import { machoTargetSelectionSchema, type MachoTargetSelection, @@ -54,13 +70,19 @@ export const resolveHopperAnalysisProfile = async ( resolveHopperAnalysisProfileInScope(target, options), ); } catch (cause: unknown) { - return err( - cause instanceof AnalysisError - ? cause - : new ProviderAdapterError("hopper", "resolve_analysis_profile", { - cause, - }), + if (cause instanceof AnalysisError) return err(cause); + const failure = new ProviderAdapterError( + "hopper", + "resolve_analysis_profile", + { + cause, + ...(cause instanceof ArtifactResourceRetryFailure + ? { cleanup: cause.cleanup } + : {}), + }, ); + bindArtifactFileCleanup(cause, failure); + return err(failure); } }; @@ -72,7 +94,11 @@ const resolveHopperAnalysisProfileInScope = async ( return ok({ profile: null }); let machoContainer: "thin" | "fat32" | "fat64" | undefined; if (target.kind === "executable" && isMachOFormat(target.format)) { - const header = await readMachoMagic(target.path, options.signal); + const header = await readMachoMagic( + target.path, + options.resourceScope, + options.signal, + ); if (!header.ok) return header; machoContainer = [0xcafebabf, 0xbfbafeca].includes(header.value) ? "fat64" @@ -127,7 +153,11 @@ const resolveHopperAnalysisProfileInScope = async ( options.loaderArgsOverride.length === 0 ? derived.value : [...options.loaderArgsOverride]; - const launcherDigest = await sha256File(options.launcherPath, options.signal); + const launcherDigest = await sha256File( + options.launcherPath, + options.resourceScope, + options.signal, + ); if (!launcherDigest.ok) return launcherDigest; const provider = { id: options.provider.id, @@ -269,102 +299,146 @@ const hopperArchitectureFlag = (architecture: HeaderArchitecture): string => { } }; -const readMachoMagic = async ( +const readMachoMagic = ( path: string, + resources: RetainedResourceScope, signal?: AbortSignal, -): Promise> => { - if (signalIsAborted(signal)) - return err( - new AnalysisCancelledError("open_binary", { cause: signal?.reason }), - ); - const stream = createReadStream(path, { start: 0, end: 3 }); - const abortError = new Error("Caller cancelled Mach-O header read"); - const onAbort = (): void => { - stream.destroy(abortError); - }; - signal?.addEventListener("abort", onAbort, { once: true }); - if (signal?.aborted) onAbort(); - try { - const chunks: Buffer[] = []; - for await (const chunk of stream) chunks.push(Buffer.from(chunk)); - if (signalIsAborted(signal)) - return err( - new AnalysisCancelledError("open_binary", { cause: signal?.reason }), - ); - const bytes = Buffer.concat(chunks); - return bytes.length === 4 - ? ok(bytes.readUInt32BE(0)) - : err( - new BinaryTargetError( - path, - "Mach-O header became truncated before Hopper loader selection", - ), - ); - } catch (cause: unknown) { - return cause === abortError - ? err( - new AnalysisCancelledError("open_binary", { cause: signal?.reason }), - ) - : err( - new BinaryTargetError( - path, - `Cannot read the Mach-O container header: ${cause instanceof Error ? cause.message : String(cause)}`, - { cause }, - ), - ); - } finally { - signal?.removeEventListener("abort", onAbort); - } -}; +): Promise> => + readHopperProfileFile( + { path, kind: "header" }, + resources, + signal, + (options) => + withOwnedRegularFile(path, options, async ({ handle }) => { + const bytes = Buffer.alloc(4); + let length = 0; + while (length < bytes.length) { + assertRegularFileActive(signal); + const { bytesRead } = await handle.read( + bytes, + length, + bytes.length - length, + length, + ); + if (bytesRead === 0) + throw new BinaryTargetError( + path, + "Mach-O header became truncated before Hopper loader selection", + ); + length += bytesRead; + } + return bytes.readUInt32BE(0); + }), + ); -const sha256File = async ( +const sha256File = ( path: string, + resources: RetainedResourceScope, signal: AbortSignal | undefined, -): Promise> => { - if (signalIsAborted(signal)) - return err( - new AnalysisCancelledError("open_binary", { cause: signal?.reason }), - ); - const stream = createReadStream(path); - const abortError = new Error("Caller cancelled launcher fingerprint read"); - const onAbort = (): void => { - stream.destroy(abortError); - }; - signal?.addEventListener("abort", onAbort, { once: true }); - if (signal?.aborted) onAbort(); +): Promise> => + readHopperProfileFile( + { path, kind: "launcher" }, + resources, + signal, + async (options) => (await hashStableFile(path, options)).sha256, + ); + +/** Both profile reads retain the exact descriptor through final close and retry. */ +const readHopperProfileFile = async ( + source: { readonly path: string; readonly kind: "header" | "launcher" }, + resources: RetainedResourceScope, + signal: AbortSignal | undefined, + read: (options: StableFileHashOptions) => Promise, +): Promise> => { + const { path, kind } = source; + let closeLease: AnalysisCleanupLease | undefined; try { - const hash = createHash("sha256"); - for await (const chunk of stream) hash.update(chunk); - return signalIsAborted(signal) - ? err( - new AnalysisCancelledError("open_binary", { cause: signal?.reason }), - ) - : ok(hash.digest("hex")); - } catch (cause: unknown) { - return cause === abortError - ? err( - new AnalysisCancelledError("open_binary", { cause: signal?.reason }), - ) - : err( - new ProviderAdapterError("hopper", "resolve_analysis_profile", { - cause, - userMessage: `Cannot fingerprint the selected Hopper launcher ${path}: ${cause instanceof Error ? cause.message : String(cause)}`, - remediationAction: - "Restore read access to the selected Hopper launcher, then retry provider selection.", - diagnostics: { - launcher_path: path, - system_code: - cause instanceof Error && - typeof Reflect.get(cause, "code") === "string" - ? (Reflect.get(cause, "code") as string) - : null, + return ok( + await read({ + ...(signal === undefined ? {} : { signal }), + retainFailedClose: (owner) => { + closeLease = AnalysisCleanupLease.forCloseOwner(owner, path); + resources.retain( + { + close: () => closeLease!.release(), + get cleanupResources() { + return closeLease!.cleanupResources; + }, }, - }), - ); - } finally { - signal?.removeEventListener("abort", onAbort); + path, + ); + }, + }), + ); + } catch (cause: unknown) { + const physical = stableFileHashCause( + cause instanceof RegularFileCleanupFailure + ? cause.outcome.kind === "failed" + ? cause.outcome.cause + : cause.cleanupCause + : cause, + ); + const cleanup = + cause instanceof RegularFileCleanupFailure + ? { + reason: + cause.cleanupCause instanceof Error + ? cause.cleanupCause.message + : String(cause.cleanupCause), + resources: [path], + } + : undefined; + const cancellation = regularFileCancellation(cause); + const completed = + (cause instanceof RegularFileCleanupFailure || + cause instanceof RegularFileCancelled) && + cause.outcome.kind === "completed" + ? { completed_profile_read: jsonValueSchema.parse(cause.outcome.value) } + : {}; + const diagnostics = { profile_file: { kind, path }, ...completed }; + const failure = + cancellation !== undefined + ? new AnalysisCancelledError("open_binary", { + cause: cancellation.reason, + cleanup, + diagnostics, + }) + : physical instanceof AnalysisError + ? physical + : kind === "header" + ? new BinaryTargetError( + path, + `Cannot read the Mach-O container header: ${physical instanceof Error ? physical.message : String(physical)}`, + { cause: physical, cleanup }, + ) + : new ProviderAdapterError("hopper", "resolve_analysis_profile", { + cause: physical, + cleanup, + userMessage: `Cannot fingerprint the selected Hopper launcher ${path}: ${physical instanceof Error ? physical.message : String(physical)}`, + remediationAction: + "Select a readable regular Hopper launcher, then retry provider selection.", + diagnostics: { + launcher_path: path, + ...diagnostics, + system_code: + physical instanceof Error && + typeof Reflect.get(physical, "code") === "string" + ? (Reflect.get(physical, "code") as string) + : null, + }, + }); + if (cause instanceof RegularFileCleanupFailure) { + const cleanupFailure = new ProviderCleanupError( + "hopper", + [path], + diagnostics, + { cause, operation: "resolve_analysis_profile" }, + ); + closeLease?.bindCleanupError(cleanupFailure); + failure + .retainSecondaryFailure(cleanupFailure) + .bindCleanupDependencies([cleanupFailure]); + } + return err(failure); } }; - -const signalIsAborted = (signal: AbortSignal | undefined): boolean => - signal?.aborted === true; diff --git a/src/hopper/HopperFileOffset.ts b/src/hopper/HopperFileOffset.ts index f4f5753ae7..71a7d60f52 100644 --- a/src/hopper/HopperFileOffset.ts +++ b/src/hopper/HopperFileOffset.ts @@ -16,6 +16,7 @@ import { BinaryTargetError } from "../domain/configurationErrors.js"; import { ProviderCleanupError } from "../domain/providerCleanupError.js"; import { OwnedFileHandle } from "../filesystem/OwnedFileHandle.js"; import { RegularFileOperationCancelled } from "../filesystem/RegularFile.js"; +import { readFileHandleChunks } from "../filesystem/readFileHandleChunks.js"; import { RegularFileCancelled, RegularFileCleanupFailure, @@ -99,7 +100,6 @@ export async function mapHopperFileOffset( observation, partial, unavailable, - signal, publish, withCleanup: (primary, marker) => analysisErrorWithCleanupFailure( @@ -125,9 +125,25 @@ export async function mapHopperFileOffset( return err( artifactChanged(target.path, captured.error, partial(captured.error)), ); - return err(cancelled(partial, cause.reason)); + const verified = coordinatesForSnapshot(verification, captured); + if (!verified.result.ok) + return err( + retainSnapshot( + verified.result.error, + captured.value, + verification.observation, + ), + ); + return err( + retainSnapshot( + cancelled(partial, cause.reason), + captured.value, + verification.observation, + verified.fileOffset, + ), + ); } - return err(sourceAccessFailure(target.path, cause, partial, signal)); + return err(sourceAccessFailure(target.path, cause, partial)); } } @@ -185,18 +201,9 @@ const sourceAccessFailure = ( path: string, cause: unknown, partial: (reason: string) => FileOffsetPartialObservation, - signal: AbortSignal | undefined, ): AnalysisError => { - if ( - signal?.aborted === true || - cause instanceof RegularFileOperationCancelled - ) - return cancelled( - partial, - cause instanceof RegularFileOperationCancelled - ? cause.reason - : signal?.reason, - ); + if (cause instanceof RegularFileOperationCancelled) + return cancelled(partial, cause.reason); const reason = `Cannot verify original source bytes: ${cause instanceof Error ? cause.message : String(cause)}`; const code = typeof cause === "object" && cause !== null && "code" in cause @@ -233,7 +240,6 @@ interface OffsetVerification { readonly observation: Observation; readonly partial: (reason: string) => FileOffsetPartialObservation; readonly unavailable: (reason: string) => Result; - readonly signal: AbortSignal | undefined; readonly withCleanup: ( primary: AnalysisError, marker: ProviderCleanupError, @@ -245,17 +251,12 @@ const cleanupAfterSnapshot = ( verification: OffsetVerification, failure: RegularFileCleanupFailure, ): Result => { - const { target, partial, signal, withCleanup, publish } = verification; + const { target, partial, withCleanup, publish } = verification; const marker = descriptorCleanup(failure); if (failure.outcome.kind === "failed") return err( withCleanup( - sourceAccessFailure( - target.path, - failure.outcome.cause, - partial, - signal, - ), + sourceAccessFailure(target.path, failure.outcome.cause, partial), marker, ), ); @@ -268,35 +269,63 @@ const cleanupAfterSnapshot = ( marker, ), ); - if (signal?.aborted === true || failure.completionCancellation !== undefined) + const verified = coordinatesForSnapshot(verification, captured); + if (!verified.result.ok) return err( withCleanup( - cancelled( - partial, - failure.completionCancellation?.reason ?? signal?.reason, + retainSnapshot( + verified.result.error, + captured.value, + verification.observation, ), marker, ), ); - const verified = coordinatesForSnapshot(verification, captured); - if (!verified.result.ok) - return err(withCleanup(verified.result.error, marker)); - const published = publish(marker); - published.retainPartialObservation( - partial( - "Source descriptor close failed after the admitted bytes were read", - ), + if (failure.completionCancellation !== undefined) + return err( + withCleanup( + retainSnapshot( + cancelled(partial, failure.completionCancellation.reason), + captured.value, + verification.observation, + verified.fileOffset, + ), + marker, + ), + ); + const published = retainSnapshot( + publish(marker), + captured.value, + verification.observation, + verified.fileOffset, ); - published.retainDiagnostics({ - source_sha256: captured.value.sha256, - ...(verified.fileOffset === undefined - ? {} - : { file_offset: verified.fileOffset }), - source_identity: "verified", - }); return err(published); }; +const retainSnapshot = ( + failure: Failure, + snapshot: SourceSnapshot, + observation: Observation, + fileOffset?: number, +): Failure => { + if (fileOffset !== undefined) + failure.partialObservation = { + operation: OPERATION, + ...observation, + original_file_offset: { + available: true, + file_offset: fileOffset, + source_sha256: snapshot.sha256, + }, + }; + return failure.retainDiagnostics({ + source_sha256: snapshot.sha256, + ...(fileOffset === undefined + ? {} + : { file_offset: fileOffset, source_identity: "verified" }), + }); +}; + const coordinatesForSnapshot = ( verification: OffsetVerification, captured: Result, @@ -304,7 +333,7 @@ const coordinatesForSnapshot = ( readonly result: Result; readonly fileOffset?: number; } => { - const { target, observation, partial, unavailable, signal } = verification; + const { target, observation, partial, unavailable } = verification; if (!captured.ok) return { result: err( @@ -312,8 +341,6 @@ const coordinatesForSnapshot = ( ), }; const snapshot = captured.value; - if (signal?.aborted === true) - return { result: err(cancelled(partial, signal.reason)) }; if (snapshot.sha256 !== target.sha256) { const reason = `Source ${target.path} differs from the admitted artifact: expected SHA-256 ${target.sha256}, observed ${snapshot.sha256}. Reopen the changed source for a new analysis identity.`; return { @@ -384,10 +411,9 @@ async function readSourceSnapshot(file: FileHandle, signal?: AbortSignal) { ); } }; - for await (const chunk of file.createReadStream({ - start: 0, - autoClose: false, - ...(signal === undefined ? {} : { signal }), + for await (const chunk of readFileHandleChunks(file, { + signal, + cancellationFailure: (reason) => new RegularFileOperationCancelled(reason), })) { const bytes = Buffer.from(chunk); hash.update(bytes); diff --git a/tests/boundary/providers/hopper/hopperFileOffsetClose.test.ts b/tests/boundary/providers/hopper/hopperFileOffsetClose.test.ts index 1b7f8e313b..49e6d5783d 100644 --- a/tests/boundary/providers/hopper/hopperFileOffsetClose.test.ts +++ b/tests/boundary/providers/hopper/hopperFileOffsetClose.test.ts @@ -12,8 +12,13 @@ import { join } from "node:path"; import { expect, it, vi } from "vitest"; import { createAnalysisProfile } from "../../../../src/domain/analysisProfile.js"; -import { AnalysisArtifactChangedError } from "../../../../src/domain/analysisErrorCore.js"; +import { + AnalysisAccessDeniedError, + AnalysisArtifactChangedError, + AnalysisCancelledError, +} from "../../../../src/domain/analysisErrorCore.js"; import { projectAnalysisError } from "../../../../src/domain/analysisErrorProjection.js"; +import { BinaryTargetError } from "../../../../src/domain/configurationErrors.js"; import { ProviderCleanupError } from "../../../../src/domain/providerCleanupError.js"; import { parseConfig } from "../../../../src/config/parseConfig.js"; import { silentLogger } from "../../../../src/logger.js"; @@ -27,6 +32,9 @@ const state = vi.hoisted(() => ({ code: "EIO", handles: [] as Array<{ handle: FileHandle; close: () => Promise }>, ended: 0, + statFailure: undefined as Error | undefined, + abortOnClose: undefined as AbortController | undefined, + afterRead: undefined as (() => void) | undefined, })); vi.mock("node:fs/promises", async (importOriginal) => { @@ -37,13 +45,22 @@ vi.mock("node:fs/promises", async (importOriginal) => { const handle = await actual.open(...args); if (String(args[0]) !== state.path) return handle; const close = handle.close.bind(handle); + const stat = handle.stat.bind(handle); + handle.stat = ((...options: Parameters) => { + if (state.statFailure !== undefined) + return Promise.reject(state.statFailure); + return stat(...options); + }) as typeof handle.stat; state.handles.push({ handle, close }); - const stream = handle.createReadStream.bind(handle); - handle.createReadStream = (options) => - stream(options).on("end", () => { - state.ended += 1; - }); + const read = handle.read.bind(handle); + handle.read = (async (...options: Parameters) => { + const result = await read(...options); + state.afterRead?.(); + if (result.bytesRead === 0) state.ended += 1; + return result; + }) as typeof handle.read; handle.close = async () => { + state.abortOnClose?.abort("caller cancelled during source close"); if (state.fail) throw Object.assign( new Error("controlled descriptor close failure"), @@ -122,6 +139,14 @@ it("preserves file-offset reads and retries the exact descriptor after close fai file_offset: 8, source_identity: "verified", }); + expect(failed.error.partialObservation).toMatchObject({ + ...observation, + original_file_offset: { + available: true, + file_offset: 8, + source_sha256: createHash("sha256").update(bytes).digest("hex"), + }, + }); expect(projectAnalysisError(failed.error).code).toBe("cleanup_incomplete"); const owned = state.handles.at(-1); if (owned === undefined) throw new Error("missing descriptor"); @@ -285,3 +310,207 @@ it("clears a combined Hopper close only after the descriptor and session release await rm(root, { recursive: true, force: true }); } }); + +it.each([false, true])( + "retains a source metadata failure when cancellation arrives during close (close failure: %s)", + async (closeFails) => { + const root = await mkdtemp( + join(tmpdir(), "rea-hopper-file-offset-primary-"), + ); + const bytes = Buffer.alloc(64); + state.path = join(root, "sample"); + await writeFile(state.path, bytes); + const client = await clientFor(state.path, bytes); + const call = vi + .spyOn(HopperClient.prototype, "callTool") + .mockResolvedValue(ok(observation)); + const controller = new AbortController(); + const primary = Object.assign( + new Error("controlled source metadata denial"), + { code: "EACCES" }, + ); + state.statFailure = primary; + state.abortOnClose = controller; + state.fail = closeFails; + try { + const result = await client.execute( + "address_to_file_offset", + { address: "0x1000" }, + { signal: controller.signal }, + ); + if (result.ok) throw new Error("expected source denial"); + expect(controller.signal.aborted).toBe(true); + const failure = closeFails ? result.error.cleanupPrimary : result.error; + expect(failure).toBeInstanceOf(AnalysisAccessDeniedError); + expect(failure?.cause).toBe(primary); + expect(failure?.partialObservation).toMatchObject(observation); + expect(result.error.cleanupIncomplete).toBe(closeFails); + const owned = state.handles.at(-1); + if (owned === undefined) throw new Error("missing source descriptor"); + expect(owned.handle.fd >= 0).toBe(closeFails); + state.fail = false; + expect((await client.close()).ok).toBe(true); + expect(owned.handle.fd).toBe(-1); + expect(result.error.cleanupIncomplete).toBe(false); + } finally { + state.fail = false; + state.statFailure = undefined; + state.abortOnClose = undefined; + call.mockRestore(); + await client.close(); + await rm(root, { recursive: true, force: true }); + } + }, +); + +it.each([false, true])( + "retains verified source coordinates when cancellation arrives during close (close failure: %s)", + async (closeFails) => { + const root = await mkdtemp( + join(tmpdir(), "rea-hopper-file-offset-cancel-"), + ); + const bytes = Buffer.alloc(64); + state.path = join(root, "sample"); + await writeFile(state.path, bytes); + const sha256 = createHash("sha256").update(bytes).digest("hex"); + const client = await clientFor(state.path, bytes); + const call = vi + .spyOn(HopperClient.prototype, "callTool") + .mockResolvedValue(ok(observation)); + const controller = new AbortController(); + state.abortOnClose = controller; + state.fail = closeFails; + try { + const result = await client.execute( + "address_to_file_offset", + { address: "0x1000" }, + { signal: controller.signal }, + ); + if (result.ok) throw new Error("expected cancellation"); + const cancellation = closeFails + ? result.error.cleanupPrimary + : result.error; + expect(cancellation).toBeInstanceOf(AnalysisCancelledError); + expect(cancellation?.cause).toBe(controller.signal.reason); + const partialObservation = { + ...observation, + original_file_offset: { + available: true, + file_offset: 8, + source_sha256: sha256, + }, + }; + expect(cancellation?.partialObservation).toMatchObject( + partialObservation, + ); + expect(projectAnalysisError(result.error)).toMatchObject({ + details: { partial_observation: partialObservation }, + }); + expect(cancellation?.diagnostics).toMatchObject({ + source_sha256: sha256, + file_offset: 8, + source_identity: "verified", + }); + expect(result.error.cleanupIncomplete).toBe(closeFails); + const owned = state.handles.at(-1); + if (owned === undefined) throw new Error("missing source descriptor"); + expect(owned.handle.fd >= 0).toBe(closeFails); + state.fail = false; + expect((await client.close()).ok).toBe(true); + expect(owned.handle.fd).toBe(-1); + expect(result.error.cleanupIncomplete).toBe(false); + } finally { + state.fail = false; + state.abortOnClose = undefined; + call.mockRestore(); + await client.close(); + await rm(root, { recursive: true, force: true }); + } + }, +); + +it.each([false, true])( + "uses the descriptor read outcome when cancellation and an I/O result coincide (read failure: %s)", + async (readFails) => { + const root = await mkdtemp(join(tmpdir(), "rea-hopper-file-offset-read-")); + const bytes = Buffer.alloc(64); + state.path = join(root, "sample"); + await writeFile(state.path, bytes); + const client = await clientFor(state.path, bytes); + const call = vi + .spyOn(HopperClient.prototype, "callTool") + .mockResolvedValue(ok(observation)); + const controller = new AbortController(); + const reason = new Error("caller cancelled during source read"); + const primary = Object.assign( + new Error("controlled descriptor read failure"), + { code: "EIO" }, + ); + let readObserved = false; + state.afterRead = () => { + readObserved = true; + controller.abort(reason); + if (readFails) throw primary; + }; + try { + const result = await client.execute( + "address_to_file_offset", + { address: "0x1000" }, + { signal: controller.signal }, + ); + if (result.ok) + throw new Error("expected interrupted source verification"); + expect(readObserved).toBe(true); + expect(result.error).toBeInstanceOf( + readFails ? BinaryTargetError : AnalysisCancelledError, + ); + expect(result.error.cause).toBe(readFails ? primary : reason); + expect(result.error.partialObservation).toMatchObject(observation); + expect(result.error.cleanupIncomplete).toBe(false); + expect(state.handles.at(-1)?.handle.fd).toBe(-1); + } finally { + state.afterRead = undefined; + call.mockRestore(); + await client.close(); + await rm(root, { recursive: true, force: true }); + } + }, +); + +it("retains a changed source digest without publishing verified coordinates after late cancellation", async () => { + const root = await mkdtemp(join(tmpdir(), "rea-hopper-file-offset-changed-")); + const bytes = Buffer.alloc(64); + state.path = join(root, "sample"); + await writeFile(state.path, bytes); + const client = await clientFor(state.path, bytes); + const changed = Buffer.alloc(64, 1); + await writeFile(state.path, changed); + const call = vi + .spyOn(HopperClient.prototype, "callTool") + .mockResolvedValue(ok(observation)); + const controller = new AbortController(); + state.abortOnClose = controller; + try { + const result = await client.execute( + "address_to_file_offset", + { address: "0x1000" }, + { signal: controller.signal }, + ); + if (result.ok) throw new Error("expected changed source identity"); + expect(controller.signal.aborted).toBe(true); + expect(result.error).toBeInstanceOf(AnalysisArtifactChangedError); + expect(result.error.partialObservation).toMatchObject({ + ...observation, + original_file_offset: { available: false }, + }); + expect(result.error.diagnostics).toEqual({ + source_sha256: createHash("sha256").update(changed).digest("hex"), + }); + expect(state.handles.at(-1)?.handle.fd).toBe(-1); + } finally { + state.abortOnClose = undefined; + call.mockRestore(); + await client.close(); + await rm(root, { recursive: true, force: true }); + } +});