From 0f61261e011ab520773fb51c07a494c98b6a73e5 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sat, 17 Oct 2026 02:44:35 +0800 Subject: [PATCH] fix(native): retain owned preparation failures through command adapters Refs #4522, #4526, #4555. Retain producer-owned compiler output and cleanup markers through xcrun resolution and native failure projection. Keep physical preparation failures primary and preserve separate command deadline classification for interrupted preparation. Omitted output facts stay unknown; client close does not resolve foreign helper cleanup. Include CLI failure output in native regression reports. Validation: 87 focused tests passed. Broader 140-file run passed 1483 tests, skipped 109, and exposed three pre-existing deep-provider selection failures tracked in #4555. Static checks passed. Compiler faults use controlled Node fixtures; native Swift compiler faults and Windows execution were not verified. Other command consumers still need preparation-fact propagation. --- src/native/CommandRunner.ts | 29 +++-- src/native/NativeMacOSProvider.ts | 52 +++++++++ src/process/OwnedCommand.ts | 23 +++- .../cli/nativeMachoDylibIdentityCli.test.ts | 2 +- .../cli/nativeMachoLinkerOptionsCli.test.ts | 2 +- .../cli/nativeMachoObjectSegmentCli.test.ts | 2 +- tests/boundary/process/ownedCommand.test.ts | 66 +++++++++++ .../native/nativeCommandTermination.test.ts | 2 + .../providers/native/nativeProvider.test.ts | 89 ++++++++++++++- .../darwinCompilerCleanup.test.ts | 103 ++++++++++++++++++ 10 files changed, 356 insertions(+), 14 deletions(-) diff --git a/src/native/CommandRunner.ts b/src/native/CommandRunner.ts index b5393190cb..cf7458f445 100644 --- a/src/native/CommandRunner.ts +++ b/src/native/CommandRunner.ts @@ -48,6 +48,9 @@ export class NativeCommandFailure extends Error { readonly cleanupOwner: ProviderProcessSupervisor | undefined; readonly cleanupFailures: readonly NativeCommandCleanupFailure[]; readonly detail: string | null; + readonly preparationFailure: + | DarwinProcessOwnershipInspectionError + | undefined; constructor( readonly tool: string, @@ -67,7 +70,14 @@ export class NativeCommandFailure extends Error { super(`Native command ${tool} failed: ${reason}`, options); this.cleanupOwner = options?.cleanupOwner; this.cleanupFailures = options?.cleanupFailures ?? []; - this.detail = options?.detail ?? null; + this.preparationFailure = + options?.cause instanceof DarwinProcessOwnershipInspectionError + ? options.cause + : options?.cause instanceof NativeCommandFailure || + options?.cause instanceof OwnedCommandFailure + ? options.cause.preparationFailure + : undefined; + this.detail = options?.detail ?? this.preparationFailure?.message ?? null; } } @@ -477,14 +487,15 @@ const captureProcess = async ( ); } return err( - new NativeCommandFailure(tool, "io", null, { - cause, - // Process supervision needs the Darwin ownership helper; its typed - // message names the compiler or SDK prerequisite to fix. - ...(cause instanceof DarwinProcessOwnershipInspectionError - ? { detail: cause.message } - : {}), - }), + new NativeCommandFailure( + tool, + cause instanceof DarwinProcessOwnershipInspectionError && + cause.cancellation !== undefined + ? "cancelled" + : "io", + null, + { cause }, + ), ); } }; diff --git a/src/native/NativeMacOSProvider.ts b/src/native/NativeMacOSProvider.ts index 4fb002a92a..f2d76fd7e0 100644 --- a/src/native/NativeMacOSProvider.ts +++ b/src/native/NativeMacOSProvider.ts @@ -925,6 +925,58 @@ const translateCommandFailure = ( operation: NativeToolName, failure: NativeCommandFailure, ): AnalysisError => { + const preparation = failure.preparationFailure; + if (preparation !== undefined) { + const marker = preparation.cleanupFailure; + const compiler = preparation.compilerOutcome; + const options = { + cause: failure, + ...(preparation.capturedOutput === undefined + ? {} + : { capturedOutput: preparation.capturedOutput }), + ...(marker === undefined + ? {} + : { + cleanup: { + reason: marker.message, + resources: marker.cleanupResources, + }, + }), + diagnostics: { + phase: "process-preparation", + tool: failure.tool, + reason: failure.reason, + detail: preparation.message, + ...(compiler === undefined + ? {} + : { + compiler_outcome: { + outcome: compiler.outcome, + code: compiler.code, + signal: compiler.signal, + ...(compiler.timeoutMs === undefined + ? {} + : { timeout_ms: compiler.timeoutMs }), + }, + }), + }, + }; + const error = + preparation.cancellation === undefined + ? new ProviderAdapterError(IDENTITY.id, operation, options) + : failure.reason === "timeout" + ? new AnalysisTimeoutError(operation, NATIVE_COMMAND_TIMEOUT_MS, { + ...options, + cause: preparation.cancellation.reason, + }) + : new AnalysisCancelledError(operation, { + ...options, + cause: preparation.cancellation.reason, + }); + return marker === undefined + ? error + : error.bindCleanupDependencies([marker]); + } const capturedOutput = failure.capture === null ? undefined diff --git a/src/process/OwnedCommand.ts b/src/process/OwnedCommand.ts index 13acb77288..baf22c2b4a 100644 --- a/src/process/OwnedCommand.ts +++ b/src/process/OwnedCommand.ts @@ -1,3 +1,4 @@ +import { DarwinProcessOwnershipInspectionError } from "./DarwinProcessRunTokenReader.js"; import type { Readable, Writable } from "node:stream"; import { pipeline } from "node:stream/promises"; @@ -26,6 +27,9 @@ import { export class OwnedCommandFailure extends Error { readonly cleanupResult: ProcessCleanupFailure | undefined; readonly collectionFailure: boolean; + readonly preparationFailure: + | DarwinProcessOwnershipInspectionError + | undefined; constructor( readonly reason: "cancelled" | "timeout" | "output-limit" | "process", @@ -35,6 +39,7 @@ export class OwnedCommandFailure extends Error { options?: ErrorOptions & { readonly cleanupResult?: ProcessCleanupFailure; readonly collectionFailure?: boolean; + readonly preparationFailure?: DarwinProcessOwnershipInspectionError; }, readonly resources: readonly string[] = [], readonly cleanupOwner?: ProviderProcessSupervisor, @@ -42,6 +47,7 @@ export class OwnedCommandFailure extends Error { super(message, options); this.cleanupResult = options?.cleanupResult; this.collectionFailure = options?.collectionFailure ?? false; + this.preparationFailure = options?.preparationFailure; } } @@ -129,6 +135,18 @@ const collectOwnedCommand = async ( signal, }); } catch (cause: unknown) { + // A caller's raw abort reason remains opaque even if it is a typed error. + const preparationFailure = + cause instanceof DarwinProcessOwnershipInspectionError && + !(signal.aborted && cause === signal.reason) + ? cause + : undefined; + // Preparation owns its physical outcome and any retained helper resources. + if ( + preparationFailure !== undefined && + preparationFailure.cancellation === undefined + ) + throw preparationFailure; const interruption = deadline.interruption; if (interruption !== undefined) throw new OwnedCommandFailure( @@ -138,7 +156,10 @@ const collectOwnedCommand = async ( : "Command deadline elapsed during ownership preparation.", null, null, - { cause }, + { + cause, + ...(preparationFailure === undefined ? {} : { preparationFailure }), + }, ); throw cause; } diff --git a/tests/boundary/cli/nativeMachoDylibIdentityCli.test.ts b/tests/boundary/cli/nativeMachoDylibIdentityCli.test.ts index d5a51c01a9..8104786549 100644 --- a/tests/boundary/cli/nativeMachoDylibIdentityCli.test.ts +++ b/tests/boundary/cli/nativeMachoDylibIdentityCli.test.ts @@ -37,7 +37,7 @@ describe.skipIf(process.platform !== "darwin")( arguments: ["inspect-macho", library, "--json"], environment: { REA_LOG_LEVEL: "silent" }, }); - expect(result.exitCode).toBe(0); + expect(result.exitCode, result.stdout + result.stderr).toBe(0); const inspected = evidenceSchema.parse(result.json).normalized_result; expect(inspected.dependencies).toMatchObject({ items: [ diff --git a/tests/boundary/cli/nativeMachoLinkerOptionsCli.test.ts b/tests/boundary/cli/nativeMachoLinkerOptionsCli.test.ts index dd16318870..2b24a97582 100644 --- a/tests/boundary/cli/nativeMachoLinkerOptionsCli.test.ts +++ b/tests/boundary/cli/nativeMachoLinkerOptionsCli.test.ts @@ -38,7 +38,7 @@ describe.skipIf(process.platform !== "darwin")( arguments: ["inspect-macho", object, "--json"], environment: { REA_LOG_LEVEL: "silent" }, }); - expect(result.exitCode).toBe(0); + expect(result.exitCode, result.stdout + result.stderr).toBe(0); const { load_commands: loadCommands } = evidenceSchema.parse( result.json, ).normalized_result; diff --git a/tests/boundary/cli/nativeMachoObjectSegmentCli.test.ts b/tests/boundary/cli/nativeMachoObjectSegmentCli.test.ts index 4d68b30bda..50d720cab9 100644 --- a/tests/boundary/cli/nativeMachoObjectSegmentCli.test.ts +++ b/tests/boundary/cli/nativeMachoObjectSegmentCli.test.ts @@ -30,7 +30,7 @@ describe.skipIf(process.platform !== "darwin")( arguments: ["inspect-macho", object, "--json"], environment: { REA_LOG_LEVEL: "silent" }, }); - expect(result.exitCode).toBe(0); + expect(result.exitCode, result.stdout + result.stderr).toBe(0); const inspected = evidenceSchema.parse(result.json).normalized_result; expect(inspected.load_commands.items[0]?.fields.segname).toBe(""); expect(inspected.segments).toMatchObject({ diff --git a/tests/boundary/process/ownedCommand.test.ts b/tests/boundary/process/ownedCommand.test.ts index 8eda618907..faa4d7a299 100644 --- a/tests/boundary/process/ownedCommand.test.ts +++ b/tests/boundary/process/ownedCommand.test.ts @@ -1,3 +1,5 @@ +import { DarwinProcessOwnershipInspectionError } from "../../../src/process/DarwinProcessRunTokenReader.js"; +import { ProviderCleanupError } from "../../../src/domain/providerCleanupError.js"; import { randomUUID } from "node:crypto"; import { EventEmitter } from "node:events"; import { PassThrough, Writable } from "node:stream"; @@ -536,3 +538,67 @@ it("preserves original command failure together with failed cleanup resource ide snapshot: { exitCode: 2 }, }); }); + +it("retains the preparation owner alongside an independent command deadline", async () => { + const marker = new ProviderCleanupError( + "process-ownership", + ["owned helper root"], + { reason: "controlled cleanup failure" }, + ); + let preparation: DarwinProcessOwnershipInspectionError | undefined; + const failure = await runOwnedCommand( + command("process.exit(0)"), + { timeoutMs: 20, diagnosticBytes: 1024 }, + { + launcher: async (input) => { + try { + await delay(80, undefined, { signal: input.signal }); + } catch { + preparation = new DarwinProcessOwnershipInspectionError( + "cancelled preparation", + { + cancellation: { reason: input.signal?.reason }, + cleanupFailure: marker, + }, + ); + throw preparation; + } + throw new Error("Unexpected completed preparation"); + }, + }, + ).catch((cause: unknown) => cause); + expect(failure).toBeInstanceOf(OwnedCommandFailure); + if (!(failure instanceof OwnedCommandFailure)) + throw new Error("Expected command deadline failure"); + expect(failure.reason).toBe("timeout"); + expect(failure.preparationFailure).toBe(preparation); + expect(failure.preparationFailure?.cleanupFailure).toBe(marker); +}); + +it("keeps a caller-supplied preparation error opaque when it is the exact abort reason", async () => { + const controller = new AbortController(); + const marker = new ProviderCleanupError("caller", ["caller metadata"], { + reason: "opaque caller value", + }); + const reason = new DarwinProcessOwnershipInspectionError("caller value", { + cleanupFailure: marker, + }); + const failure = await runOwnedCommand( + command("process.exit(0)"), + { timeoutMs: 2000, diagnosticBytes: 1024 }, + { + signal: controller.signal, + launcher: async (input) => { + controller.abort(reason); + input.signal?.throwIfAborted(); + throw new Error("Unexpected completed preparation"); + }, + }, + ).catch((cause: unknown) => cause); + expect(failure).toBeInstanceOf(OwnedCommandFailure); + if (!(failure instanceof OwnedCommandFailure)) + throw new Error("Expected command cancellation"); + expect(failure.reason).toBe("cancelled"); + expect(failure.cause).toBe(reason); + expect(failure.preparationFailure).toBeUndefined(); +}); diff --git a/tests/boundary/providers/native/nativeCommandTermination.test.ts b/tests/boundary/providers/native/nativeCommandTermination.test.ts index 5d23e6b228..7bda530080 100644 --- a/tests/boundary/providers/native/nativeCommandTermination.test.ts +++ b/tests/boundary/providers/native/nativeCommandTermination.test.ts @@ -224,9 +224,11 @@ it("keeps the ownership helper prerequisite when xcrun cannot be supervised", as expect(result.ok).toBe(false); if (result.ok) return; + expect(result.error.preparationFailure).toBe(inspection); expect(result.error).toMatchObject({ tool: "file", reason: "io", + preparationFailure: inspection, detail: inspection.message, cause: { tool: "xcrun", detail: inspection.message, cause: inspection }, }); diff --git a/tests/boundary/providers/native/nativeProvider.test.ts b/tests/boundary/providers/native/nativeProvider.test.ts index 426b2f9f3c..62966c949d 100644 --- a/tests/boundary/providers/native/nativeProvider.test.ts +++ b/tests/boundary/providers/native/nativeProvider.test.ts @@ -1,3 +1,4 @@ +import { OwnedCommandFailure } from "../../../../src/process/OwnedCommand.js"; import { createHash } from "node:crypto"; import { DarwinProcessOwnershipInspectionError } from "../../../../src/process/DarwinProcessRunTokenReader.js"; import { EventEmitter } from "node:events"; @@ -22,6 +23,7 @@ import { parseLipoOutput } from "../../../../src/native/parsers/lipo.js"; import { parseCodeSignature } from "../../../../src/native/parsers/codesign.js"; import { prepareProcessOwnershipInspection } from "../../../../src/process/ProcessOwnershipObservation.js"; import { projectAnalysisError } from "../../../../src/domain/analysisErrorProjection.js"; +import { ProviderCleanupError } from "../../../../src/domain/providerCleanupError.js"; import { ProviderProcessSupervisor } from "../../../../src/process/ProviderProcess.js"; import { @@ -347,8 +349,13 @@ describe("native command failure capture and limits", () => { expect(result.ok).toBe(false); if (result.ok) return; expect(projectAnalysisError(result.error).details).toMatchObject({ - diagnostics: { reason: "io", detail: inspection.message }, + diagnostics: { + phase: "process-preparation", + reason: "io", + detail: inspection.message, + }, }); + expect(result.error.capturedOutput).toBeUndefined(); }); it("retains output written before cancellation and reaps the stalled command", async () => { @@ -1159,3 +1166,83 @@ class CountingRunner implements NativeCommandRunner { return Promise.resolve(err(new NativeCommandFailure(tool, "unavailable"))); } } + +it("does not reopen a resolved preparation cleanup marker or invent absent compiler output facts", async () => { + const marker = new ProviderCleanupError( + "process-ownership", + ["owned helper root"], + { reason: "controlled close failure" }, + ); + const capturedOutput = { + stdout: "compiler output", + stderr: "�", + stderr_bytes: 1, + truncated: false, + }; + const inspection = new DarwinProcessOwnershipInspectionError( + "controlled preparation failure", + { capturedOutput, cleanupFailure: marker }, + ); + marker.resolveCleanup(); + const failure = new NativeCommandFailure("swift-demangle", "io", null, { + cause: inspection, + }); + const runner: NativeCommandRunner = { run: async () => err(failure) }; + const client = new NativeMacOSProvider({}, runner, "darwin").createClient( + machoTarget("/fixture"), + ); + try { + const result = await client.execute("demangle_swift", { + symbols: ["fixture"], + }); + if (result.ok) throw new Error("Expected preparation failure"); + expect(result.error.cleanupIncomplete).toBe(false); + expect(result.error.capturedOutput).toEqual(capturedOutput); + expect(result.error.capturedOutput).not.toHaveProperty("stdout_bytes"); + expect(result.error.capturedOutput).not.toHaveProperty("exit_code"); + } finally { + await client.close(); + } +}); + +it("retains an owned preparation cleanup marker when the command deadline expires", async () => { + const marker = new ProviderCleanupError( + "process-ownership", + ["owned helper root"], + { reason: "controlled cleanup failure" }, + ); + const reason = new Error("command deadline"); + const inspection = new DarwinProcessOwnershipInspectionError( + "cancelled preparation", + { cancellation: { reason }, cleanupFailure: marker }, + ); + const owned = new OwnedCommandFailure( + "timeout", + "command preparation timed out", + null, + null, + { cause: inspection, preparationFailure: inspection }, + ); + const failure = new NativeCommandFailure("swift-demangle", "timeout", null, { + cause: owned, + }); + const runner: NativeCommandRunner = { run: async () => err(failure) }; + const client = new NativeMacOSProvider({}, runner, "darwin").createClient( + machoTarget("/fixture"), + ); + try { + const result = await client.execute("demangle_swift", { + symbols: ["fixture"], + }); + if (result.ok) throw new Error("Expected preparation timeout"); + expect(result.error).toMatchObject({ + _tag: "AnalysisTimeoutError", + cleanupIncomplete: true, + }); + expect(result.error.cause).toBe(reason); + marker.resolveCleanup(); + expect(result.error.cleanupIncomplete).toBe(false); + } finally { + await client.close(); + } +}); diff --git a/tests/process-global/darwinCompilerCleanup.test.ts b/tests/process-global/darwinCompilerCleanup.test.ts index 667d1c71c7..3cc9abd6eb 100644 --- a/tests/process-global/darwinCompilerCleanup.test.ts +++ b/tests/process-global/darwinCompilerCleanup.test.ts @@ -41,6 +41,9 @@ import { createDarwinProcessRunTokenReader, DarwinProcessOwnershipInspectionError, } from "../../src/process/DarwinProcessRunTokenReader.js"; +import { NativeMacOSProvider } from "../../src/native/NativeMacOSProvider.js"; +import { XcrunCommandRunner } from "../../src/native/CommandRunner.js"; +import { nativeMachoTarget } from "../fixtures/nativeCommands.js"; import { GhidraClient } from "../../src/ghidra/GhidraClient.js"; import { GhidraHeadlessLauncher } from "../../src/ghidra/GhidraLauncher.js"; import { captureProcessScenario } from "../../src/process/capture/ProcessHarness.js"; @@ -286,3 +289,103 @@ it.skipIf(process.platform === "win32")( expect(result.error.capturedOutput?.exit_code).toBe(7); }, ); + +it.skipIf(process.platform !== "darwin").each([false, true])( + "native command preparation preserves compiler output and its foreign cleanup owner with late cancellation %s", + async (lateCancellation) => { + const { reader } = await fixture(); + const controller = new AbortController(); + if (lateCancellation) fault.abort = controller; + vi.spyOn(systemProcessOwnershipHost, "prepare").mockImplementation( + async (signal) => { + await reader.prepare(signal); + }, + ); + const client = new NativeMacOSProvider( + {}, + new XcrunCommandRunner({}), + "darwin", + ).createClient(nativeMachoTarget(ghidraFixturePath)); + try { + const result = await client.execute( + "list_architectures", + {}, + { signal: controller.signal }, + ); + if (result.ok) throw new Error("Expected failed process preparation"); + expect(result.error._tag).toBe("ProviderAdapterError"); + expect(result.error).toMatchObject({ + cleanupIncomplete: true, + capturedOutput: { + stdout: "owned compiler stdout", + stdout_bytes: 21, + stderr: "controlled compiler failure\n", + stderr_bytes: 28, + exit_code: 7, + }, + diagnostics: { phase: "process-preparation", tool: "lipo" }, + }); + expect(result.error.cleanupResources).toEqual([...fault.roots]); + expect((await client.close()).ok).toBe(true); + expect(result.error.cleanupIncomplete).toBe(true); + fault.blocked = false; + await reader.close(); + expect(result.error.cleanupIncomplete).toBe(false); + expect(result.error.capturedOutput?.exit_code).toBe(7); + } finally { + await client.close(); + } + }, +); + +it + .skipIf(process.platform !== "darwin") + .each([ + null, + "stop native preparation", + new Error("stop native preparation"), + ] as const)( + "native preparation preserves exact cancellation %s with retained helper cleanup", + async (reason) => { + const { reader, readyPath } = await fixture("active"); + vi.spyOn(systemProcessOwnershipHost, "prepare").mockImplementation( + async (signal) => { + await reader.prepare(signal); + }, + ); + const controller = new AbortController(); + const client = new NativeMacOSProvider( + {}, + new XcrunCommandRunner({}), + "darwin", + ).createClient(nativeMachoTarget(ghidraFixturePath)); + const pending = client.execute( + "list_architectures", + {}, + { signal: controller.signal }, + ); + try { + await vi.waitFor(async () => + expect(await readFile(readyPath, "utf8")).toBe("ready"), + ); + controller.abort(reason); + const result = await pending; + if (result.ok) throw new Error("Expected cancelled preparation"); + expect(result.error).toMatchObject({ + _tag: "AnalysisCancelledError", + cleanupIncomplete: true, + diagnostics: { phase: "process-preparation", reason: "cancelled" }, + }); + expect(result.error.cause).toBe(reason); + expect((await client.close()).ok).toBe(true); + expect(result.error.cleanupIncomplete).toBe(true); + fault.blocked = false; + await reader.close(); + expect(result.error.cleanupIncomplete).toBe(false); + } finally { + controller.abort(reason); + await pending; + await client.close(); + } + }, +);