diff --git a/src/artifacts/MachOSliceArtifactReader.test.ts b/src/artifacts/MachOSliceArtifactReader.test.ts index 8a3cb2830..9e0d63b13 100644 --- a/src/artifacts/MachOSliceArtifactReader.test.ts +++ b/src/artifacts/MachOSliceArtifactReader.test.ts @@ -1,3 +1,5 @@ +import { EventEmitter } from "node:events"; +import { PassThrough } from "node:stream"; import { writeFile } from "node:fs/promises"; import { join } from "node:path"; @@ -6,9 +8,151 @@ import { describe, expect, it, onTestFinished } from "vitest"; import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js"; import { CPU, fatImage, machoImage } from "./apple/MachoImage.fixture.js"; import { MachOSliceArtifactReader } from "./MachOSliceArtifactReader.js"; -import { ok } from "../domain/result.js"; +import { err, ok } from "../domain/result.js"; import { openRegularFile } from "../filesystem/RegularFile.js"; +import { ProviderProcessSupervisor } from "../process/ProviderProcess.js"; import type { NativeCommandRunner } from "../native/CommandRunner.js"; +import { NativeCommandFailure } from "../native/CommandRunner.js"; + +const cleanupOwner = (runId: string): ProviderProcessSupervisor => { + const child = Object.assign(new EventEmitter(), { + stdout: new PassThrough(), + stderr: new PassThrough(), + exitCode: null as number | null, + signalCode: null as NodeJS.Signals | null, + kill: () => true, + }); + return new ProviderProcessSupervisor({ + process: child, + ownsProcessLifetime: true, + ownership: { + runId, + leaderPid: 45_000, + processGroupId: 45_000, + expectedParentPid: process.pid, + }, + cleanup: async () => ({ cleaned: false, reason: "still running" }), + }); +}; + +describe("Mach-O cleanup exceptions", () => { + it("retains an undefined cleanup rejection as an explicit failure", async () => { + const reader = new MachOSliceArtifactReader( + "/unused/macho", + {}, + { + run: async () => { + throw new Error("Unexpected command"); + }, + retryCleanup: async () => { + throw undefined; + }, + }, + ); + await expect(reader.close()).rejects.toMatchObject({ + cleanup: { + reason: "undefined", + resources: ["native-command-process-cleanup"], + }, + }); + }); +}); + +describe("Mach-O command cleanup lifecycle", () => { + it("waits for active lipo capture before cleanup and preserves its failure", async () => { + const root = await createTestTempDirectory("rea-slices-close-race-"); + const binary = join(root, "input"); + await writeFile(binary, Buffer.from("fixture")); + let finishCommand!: ( + result: Awaited>, + ) => void; + let commandStarted!: () => void; + const started = new Promise((resolve) => { + commandStarted = resolve; + }); + const command = new Promise< + Awaited> + >((resolve) => { + finishCommand = resolve; + }); + let cleanupRetries = 0; + const runner: NativeCommandRunner = { + run: () => { + commandStarted(); + return command; + }, + retryCleanup: async () => { + cleanupRetries += 1; + if (cleanupRetries > 1) return []; + return [ + { + owner: cleanupOwner("run:first"), + reason: "first process group remains live", + resources: ["run:first", "process-group:1234"], + }, + { + owner: cleanupOwner("run:second"), + reason: "second process group remains live", + resources: ["run:second", "process-group:5678"], + }, + ]; + }, + }; + const reader = new MachOSliceArtifactReader(binary, {}, runner); + const iteration = reader.entries(); + const firstEntry = iteration[Symbol.asyncIterator]().next(); + await started; + const closing = reader.close(); + await Promise.resolve(); + expect(cleanupRetries).toBe(0); + finishCommand( + err( + new NativeCommandFailure( + "lipo", + "timeout", + null, + undefined, + { + stdout: "partial lipo output", + stderr: "partial lipo diagnostic", + stdoutBytes: 19, + stderrBytes: 23, + exitCode: null, + signal: null, + truncated: false, + }, + "capture process group remains unverified", + ["capture-group:9999"], + ), + ), + ); + + await expect(firstEntry).rejects.toMatchObject({ + reason: "unavailable", + cause: expect.objectContaining({ + cleanupFailure: "capture process group remains unverified", + }), + cleanup: { + reason: "capture process group remains unverified", + resources: ["capture-group:9999"], + }, + }); + await expect(closing).rejects.toMatchObject({ + cleanup: { + reason: + "run:first, process-group:1234: first process group remains live; run:second, process-group:5678: second process group remains live", + resources: [ + "run:first", + "process-group:1234", + "run:second", + "process-group:5678", + ], + }, + }); + await reader.close(); + expect(cleanupRetries).toBe(2); + }); +}); describe("MachOSliceArtifactReader", () => { it("keeps a borrowed source usable after a cancelled slice read", async () => { diff --git a/src/artifacts/MachOSliceArtifactReader.ts b/src/artifacts/MachOSliceArtifactReader.ts index e21e97554..115ffae79 100644 --- a/src/artifacts/MachOSliceArtifactReader.ts +++ b/src/artifacts/MachOSliceArtifactReader.ts @@ -39,6 +39,7 @@ export class MachOSliceArtifactReader implements ArtifactReader { #ownedSource: OwnedFileHandle | undefined; #pendingSource: Promise | undefined; readonly #entries = new Map(); + readonly #pendingCommands = new Set>(); readonly #ownsSource: boolean; #closed = false; #closePromise: Promise | undefined; @@ -59,16 +60,46 @@ export class MachOSliceArtifactReader implements ArtifactReader { this.#entries.clear(); const source = await this.#ensureSource(); await this.#verifySource(source); - const captured = await this.runner.run( + if (this.#closed) + throw new ArtifactReaderFailure( + "unavailable", + "Universal Mach-O reader is closed", + ); + const command = this.runner.run( "lipo", ["-detailed_info", this.path], signal === undefined ? {} : { signal }, ); + this.#pendingCommands.add(command); + let captured: Awaited; + try { + captured = await command; + } finally { + this.#pendingCommands.delete(command); + } if (!captured.ok) throw new ArtifactReaderFailure( captured.error.reason === "cancelled" ? "cancelled" : "unavailable", "lipo could not enumerate universal Mach-O slices", - { cause: captured.error }, + { + cause: captured.error, + ...(captured.error.cleanupFailure === null && + captured.error.cleanupResources.length === 0 + ? {} + : { + cleanup: { + reason: + captured.error.cleanupFailure ?? + "Native command cleanup remains unverified", + resources: [...captured.error.cleanupResources], + }, + }), + }, + ); + if (this.#closed) + throw new ArtifactReaderFailure( + "unavailable", + "Universal Mach-O reader closed during slice discovery", ); this.#command = { tool: captured.value.tool, @@ -262,13 +293,49 @@ export class MachOSliceArtifactReader implements ArtifactReader { async close(): Promise { this.#closed = true; - this.#closePromise ??= this.#closeSource().catch((cause: unknown) => { + this.#closePromise ??= this.#releaseResources().catch((cause: unknown) => { this.#closePromise = undefined; throw cause; }); return this.#closePromise; } + async #releaseResources(): Promise { + await Promise.allSettled([...this.#pendingCommands]); + let cleanup: ArtifactReaderFailure["cleanup"]; + try { + const failures = (await this.runner.retryCleanup?.()) ?? []; + if (failures.length > 0) + cleanup = { + reason: failures + .map( + ({ reason, resources }) => `${resources.join(", ")}: ${reason}`, + ) + .join("; "), + resources: [ + ...new Set(failures.flatMap(({ resources }) => resources)), + ], + }; + } catch (cause: unknown) { + cleanup = ArtifactReaderFailure.cleanupObservation( + cause, + "native-command-process-cleanup", + ); + } + try { + await this.#closeSource(); + } catch (cause: unknown) { + if (cleanup === undefined) throw cause; + throw ArtifactReaderFailure.withCleanup(cause, cleanup); + } + if (cleanup !== undefined) + throw new ArtifactReaderFailure( + "unavailable", + "Native command process cleanup remains incomplete", + { cleanup }, + ); + } + async #ensureSource(): Promise { if (this.#closed) throw new ArtifactReaderFailure( diff --git a/src/native/CommandRunner.ts b/src/native/CommandRunner.ts index 7266484d6..e9775d1b7 100644 --- a/src/native/CommandRunner.ts +++ b/src/native/CommandRunner.ts @@ -8,7 +8,12 @@ import { OwnedCommandFailure, runOwnedCommand, } from "../process/OwnedCommand.js"; -import type { ProviderProcessSnapshot } from "../process/ProviderProcess.js"; +import type { + OwnedProviderProcessSpawnOptions, + ProviderProcessSnapshot, + ProviderProcessSupervisor, + SpawnedOwnedProviderProcess, +} from "../process/ProviderProcess.js"; /** Maximum wall-clock duration for one native command. */ export const NATIVE_COMMAND_TIMEOUT_MS = 60_000; @@ -32,6 +37,9 @@ export interface NativeCommandCapture { /** Native command failure with retained partial output and cleanup status. */ export class NativeCommandFailure extends Error { + readonly cleanupOwner: ProviderProcessSupervisor | undefined; + readonly cleanupFailures: readonly NativeCommandCleanupFailure[]; + constructor( readonly tool: string, readonly reason: @@ -42,15 +50,32 @@ export class NativeCommandFailure extends Error { | "nonzero-exit" | "io", readonly exitCode: number | null = null, - options?: ErrorOptions, + options?: NativeCommandFailureOptions, readonly capture: NativeCommandFailureCapture | null = null, readonly cleanupFailure: string | null = null, readonly cleanupResources: readonly string[] = [], ) { super(`Native command ${tool} failed: ${reason}`, options); + this.cleanupOwner = options?.cleanupOwner; + this.cleanupFailures = options?.cleanupFailures ?? []; } } +/** Named cleanup ownership carried alongside the ordinary error cause. */ +export interface NativeCommandFailureOptions extends ErrorOptions { + /** Typed owner retained when command process cleanup was incomplete. */ + readonly cleanupOwner?: ProviderProcessSupervisor; + /** All owners still incomplete after a preflight retry blocked this command. */ + readonly cleanupFailures?: readonly NativeCommandCleanupFailure[]; +} + +/** One failed cleanup attempt; the runner retains `owner` until it is verified stopped. */ +export interface NativeCommandCleanupFailure { + readonly owner: ProviderProcessSupervisor; + readonly reason: string; + readonly resources: readonly string[]; +} + /** Partial output and lifecycle details retained when a native command fails. */ export interface NativeCommandFailureCapture { readonly stdout: string; @@ -74,21 +99,32 @@ export interface NativeCommandRunner { arguments_: readonly string[], options: NativeCommandOptions, ): Promise>; + /** Retry only owners from completed commands whose cleanup was incomplete. */ + retryCleanup?(): Promise; } +/** Optional launch seam for production-boundary lifecycle verification. */ +export type NativeCommandLauncher = ( + options: OwnedProviderProcessSpawnOptions, +) => Promise; + /** Resolve one allowlisted native tool to an immutable executable identity. */ export type NativeToolResolver = ( tool: string, signal?: AbortSignal, + launcher?: NativeCommandLauncher, ) => Promise>; /** Run allowlisted Xcode tools directly without a shell. */ export class XcrunCommandRunner implements NativeCommandRunner { private readonly environment: NodeJS.ProcessEnv; + readonly #pendingCleanup = new Set(); + #cleanupAttempt: Promise | undefined; constructor( environment: Readonly, private readonly resolveTool: NativeToolResolver = (tool, signal) => - resolveXcrunTool(tool, this.environment, signal), + resolveXcrunTool(tool, this.environment, signal, this.launcher), + private readonly launcher?: NativeCommandLauncher, ) { this.environment = snapshotEnvironment(environment); } @@ -102,31 +138,126 @@ export class XcrunCommandRunner implements NativeCommandRunner { return Promise.resolve( err(new NativeCommandFailure(tool, "unavailable")), ); - return this.resolveTool(tool, options.signal).then(async (resolved) => { - if (!resolved.ok) return resolved; - const captured = await captureProcess( - resolved.value.path, - arguments_, - tool, - options, - this.environment, - ); - return captured.ok - ? ok({ - ...captured.value, - tool, - executable: resolved.value.path, - executableSha256: resolved.value.sha256, - toolVersion: null, - versionReason: - "Tool exposes no uniform stable version flag; executable SHA-256 is a prelaunch file sample and does not prove atomic OS image binding.", - arguments: [...arguments_], - }) - : captured; + return this.#runAfterCleanup(tool, arguments_, options); + } + + /** Retry every retained owner; a failed retry prevents another command launch. */ + retryCleanup(): Promise { + if (this.#pendingCleanup.size === 0) return Promise.resolve([]); + if (this.#cleanupAttempt !== undefined) return this.#cleanupAttempt; + this.#cleanupAttempt = this.#retryCleanup().finally(() => { + this.#cleanupAttempt = undefined; }); + return this.#cleanupAttempt; + } + + async #runAfterCleanup( + tool: string, + arguments_: readonly string[], + options: NativeCommandOptions, + ): Promise> { + const pendingFailures = await this.retryCleanup(); + if (pendingFailures.length > 0) + return err(commandCleanupFailure(tool, pendingFailures)); + const resolved = await this.resolveTool( + tool, + options.signal, + this.launcher, + ); + if (!resolved.ok) { + if (resolved.error.cleanupOwner !== undefined) + this.#pendingCleanup.add(resolved.error.cleanupOwner); + return resolved; + } + const captured = await captureProcess( + resolved.value.path, + arguments_, + tool, + options, + this.environment, + this.launcher, + ); + if (!captured.ok && captured.error.cleanupOwner !== undefined) + this.#pendingCleanup.add(captured.error.cleanupOwner); + return captured.ok + ? ok({ + ...captured.value, + tool, + executable: resolved.value.path, + executableSha256: resolved.value.sha256, + toolVersion: null, + versionReason: + "Tool exposes no uniform stable version flag; executable SHA-256 is a prelaunch file sample and does not prove atomic OS image binding.", + arguments: [...arguments_], + }) + : captured; + } + + async #retryCleanup(): Promise { + const failures: NativeCommandCleanupFailure[] = []; + for (const owner of this.#pendingCleanup) { + let reason: string | undefined; + try { + const stopped = await owner.stop(); + if (stopped.status !== "incomplete") { + this.#pendingCleanup.delete(owner); + continue; + } + reason = stopped.reason || "native command cleanup remains unverified"; + } catch (cause: unknown) { + reason = + cause instanceof Error + ? cause.message || cause.name + : String(cause) || "native command cleanup failed unexpectedly"; + } + failures.push({ + owner, + reason: reason ?? "native command cleanup remains unverified", + resources: commandProcessResources(owner), + }); + } + return failures; } } +const commandProcessResources = ( + owner: ProviderProcessSupervisor, +): readonly string[] => { + const ownership = owner.launch.ownership; + return ownership === undefined + ? ["native-command-process"] + : [ + ownership.runId, + `pid:${String(ownership.leaderPid)}`, + `process-group:${String(ownership.processGroupId)}`, + ]; +}; + +const commandCleanupFailure = ( + tool: string, + failures: readonly NativeCommandCleanupFailure[], +): NativeCommandFailure => + new NativeCommandFailure( + tool, + "io", + null, + { + cleanupFailures: failures, + cause: new AggregateError( + failures.map( + ({ reason, resources }) => + new Error(`${reason} (${resources.join(", ")})`), + ), + "Previously owned native command cleanup remains incomplete", + ), + }, + null, + failures + .map(({ reason, resources }) => `${resources.join(", ")}: ${reason}`) + .join("; "), + [...new Set(failures.flatMap(({ resources }) => resources))], + ); + /** Executable path and SHA-256 sampled during one tool resolution. */ export interface ResolvedTool { readonly path: string; @@ -152,6 +283,7 @@ export const resolveXcrunTool = async ( tool: string, environment: Readonly, signal?: AbortSignal, + launcher?: NativeCommandLauncher, ): Promise> => { const found = await captureProcess( "/usr/bin/xcrun", @@ -159,6 +291,7 @@ export const resolveXcrunTool = async ( "xcrun", signal === undefined ? {} : { signal }, environment, + launcher, ); if (!found.ok) { return err( @@ -169,7 +302,15 @@ export const resolveXcrunTool = async ( ? "unavailable" : found.error.reason, found.error.exitCode, - { cause: found.error }, + { + cause: found.error, + ...(found.error.cleanupOwner === undefined + ? {} + : { cleanupOwner: found.error.cleanupOwner }), + ...(found.error.cleanupFailures.length === 0 + ? {} + : { cleanupFailures: found.error.cleanupFailures }), + }, found.error.capture, found.error.cleanupFailure, found.error.cleanupResources, @@ -256,6 +397,7 @@ const captureProcess = async ( readonly acceptNonZero?: boolean; }, environment: Readonly, + launcher?: NativeCommandLauncher, ): Promise> => { const runId = `rea-native-command-${randomUUID()}`; try { @@ -276,6 +418,7 @@ const captureProcess = async ( diagnosticBytes: NATIVE_COMMAND_OUTPUT_BUDGET_BYTES, acceptNonZeroExit: options.acceptNonZero === true, }, + launcher === undefined ? {} : { launcher }, ); return ok(processCaptureFromSnapshot(snapshot)); } catch (cause: unknown) { @@ -294,7 +437,12 @@ const captureProcess = async ( ? processFailureReason(capture) : cause.reason, capture?.exitCode ?? null, - { cause }, + { + cause, + ...(cause.cleanupOwner === undefined + ? {} + : { cleanupOwner: cause.cleanupOwner }), + }, capture, cause.cleanupFailure, cause.resources, diff --git a/src/native/NativeMacOSProvider.ts b/src/native/NativeMacOSProvider.ts index d44409984..7535e5bbe 100644 --- a/src/native/NativeMacOSProvider.ts +++ b/src/native/NativeMacOSProvider.ts @@ -60,6 +60,7 @@ import { NATIVE_COMMAND_OUTPUT_BUDGET_BYTES, NativeCommandFailure, XcrunCommandRunner, + type NativeCommandCleanupFailure, type NativeCommandCapture, type NativeCommandRunner, } from "./CommandRunner.js"; @@ -126,6 +127,7 @@ export class NativeMacOSProvider implements AnalysisProvider { class NativeMacOSClient implements AnalysisClient { readonly #pendingNativeUiRuntimes = new Set(); + readonly #pendingCommands = new Set>(); #nativeUiTail: Promise = Promise.resolve(); #closed = false; @@ -259,18 +261,23 @@ class NativeMacOSClient implements AnalysisClient { async close(): Promise> { this.#closed = true; + await Promise.allSettled([...this.#pendingCommands]); const uiCleanupFailures = await this.#serializeNativeUi(() => this.#retryNativeUiCleanup(), ); - const tracer = await this.tracer.close(); - if (uiCleanupFailures.length === 0) return tracer; - const uiCleanup = this.#nativeUiCleanupFailure( - uiCleanupFailures, - "close_binary", - ); - return tracer.ok - ? err(uiCleanup) - : err(analysisErrorWithCleanupFailure(tracer.error, uiCleanup)); + const commandCleanup = await this.#retryNativeCommandCleanup(); + let result = await this.tracer.close(); + const cleanupErrors = [ + ...(uiCleanupFailures.length === 0 + ? [] + : [this.#nativeUiCleanupFailure(uiCleanupFailures, "close_binary")]), + ...(commandCleanup === undefined ? [] : [commandCleanup]), + ]; + for (const cleanupError of cleanupErrors) + result = result.ok + ? err(cleanupError) + : err(analysisErrorWithCleanupFailure(result.error, cleanupError)); + return result; } #observeNativeUi( @@ -360,6 +367,28 @@ class NativeMacOSClient implements AnalysisClient { return failures; } + async #retryNativeCommandCleanup(): Promise { + if (this.runner.retryCleanup === undefined) return undefined; + try { + const failures = await this.runner.retryCleanup(); + return failures.length === 0 + ? undefined + : nativeCommandCleanupFailure(failures); + } catch (cause: unknown) { + return new ProviderCleanupError( + IDENTITY.id, + ["native-command-process-cleanup"], + { + reason: + cause instanceof Error + ? cause.message || cause.name + : String(cause) || "native command cleanup failed unexpectedly", + }, + { cause }, + ); + } + } + #nativeUiCleanupFailure( failures: readonly { readonly runtime: NativeUiHelperRuntime; @@ -782,15 +811,45 @@ class NativeMacOSClient implements AnalysisClient { readonly acceptNonZero?: boolean; } = {}, ): Promise> { - const captured = await this.runner.run(tool, arguments_, { + if (this.#closed) + return err( + new AnalysisCapabilityUnavailableError( + IDENTITY.id, + operation, + "Native provider client is closed", + ), + ); + const command = this.runner.run(tool, arguments_, { ...(options.signal === undefined ? {} : { signal: options.signal }), acceptNonZero: options.acceptNonZero ?? false, }); + this.#pendingCommands.add(command); + let captured: Awaited; + try { + captured = await command; + } finally { + this.#pendingCommands.delete(command); + } if (captured.ok) return captured; return err(translateCommandFailure(operation, captured.error)); } } +const nativeCommandCleanupFailure = ( + failures: readonly NativeCommandCleanupFailure[], +): ProviderCleanupError => + new ProviderCleanupError( + IDENTITY.id, + [...new Set(failures.flatMap(({ resources }) => resources))], + { + reason: "Native command process cleanup remains incomplete", + failures: failures.map(({ reason, resources }) => ({ + reason, + resources: [...resources], + })), + }, + ); + interface NativeObservation { readonly result: JsonValue; readonly limitations: readonly string[]; diff --git a/tests/boundary/providers/native/nativeCommandTermination.test.ts b/tests/boundary/providers/native/nativeCommandTermination.test.ts index 8d3a7ed2c..233ebe85e 100644 --- a/tests/boundary/providers/native/nativeCommandTermination.test.ts +++ b/tests/boundary/providers/native/nativeCommandTermination.test.ts @@ -1,11 +1,70 @@ import { createHash } from "node:crypto"; import { readFile } from "node:fs/promises"; +import { EventEmitter } from "node:events"; +import { PassThrough } from "node:stream"; import { afterEach, expect, it, vi } from "vitest"; -import { XcrunCommandRunner } from "../../../../src/native/CommandRunner.js"; +import { + XcrunCommandRunner, + type NativeCommandLauncher, +} from "../../../../src/native/CommandRunner.js"; import { ok } from "../../../../src/domain/result.js"; afterEach(() => vi.unstubAllEnvs()); +it.skipIf(process.platform === "win32")( + "retains an xcrun resolver's failed owner and retries it before resolving again", + async () => { + let launches = 0; + const launcher: NativeCommandLauncher = async (options) => { + launches += 1; + const child = Object.assign(new EventEmitter(), { + stdout: new PassThrough(), + stderr: new PassThrough(), + exitCode: null as number | null, + signalCode: null as NodeJS.Signals | null, + kill: () => true, + }); + setImmediate(() => { + child.stdout.end("partial xcrun output"); + child.stderr.end("resolver failed"); + child.exitCode = 1; + child.emit("exit", 1, null); + setImmediate(() => child.emit("close", 1, null)); + }); + return { + process: child, + ownership: { + runId: options.runId, + leaderPid: 43_000 + launches, + processGroupId: 43_000 + launches, + expectedParentPid: process.pid, + }, + cleanup: async () => ({ + cleaned: false, + reason: "resolver owner remains unverified", + }), + }; + }; + const runner = new XcrunCommandRunner({}, undefined, launcher); + + const resolution = await runner.run("file", [], {}); + + expect(resolution.ok).toBe(false); + if (resolution.ok) return; + expect(resolution.error.cleanupOwner).toBeDefined(); + expect(resolution.error.capture).toMatchObject({ + stdout: "partial xcrun output", + stderr: "resolver failed", + }); + expect(launches).toBe(1); + + const retry = await runner.run("file", [], {}); + expect(retry.ok).toBe(false); + if (!retry.ok) expect(retry.error.cleanupFailures).toHaveLength(1); + expect(launches).toBe(1); + }, +); + it.skipIf(process.platform === "win32")( "executes native commands with the selected environment snapshot instead of ambient values", async () => { @@ -37,6 +96,97 @@ it.skipIf(process.platform === "win32")( }, ); +it.skipIf(process.platform === "win32")( + "retains and retries every failed native command cleanup owner before launching another command", + async () => { + const cleanupCalls = new Map(); + let launches = 0; + const launcher: NativeCommandLauncher = async (options) => { + const name = options.arguments[0] ?? "unnamed"; + launches += 1; + const child = Object.assign(new EventEmitter(), { + stdout: new PassThrough(), + stderr: new PassThrough(), + exitCode: null as number | null, + signalCode: null as NodeJS.Signals | null, + kill: () => true, + }); + setImmediate(() => { + child.stdout.end(`partial-${name}`); + child.stderr.end(`diagnostic-${name}`); + child.exitCode = 0; + child.emit("exit", 0, null); + setImmediate(() => child.emit("close", 0, null)); + }); + return { + process: child, + ownership: { + runId: options.runId, + leaderPid: 42_000 + launches, + processGroupId: 42_000 + launches, + expectedParentPid: process.pid, + }, + cleanup: async () => { + const calls = (cleanupCalls.get(name) ?? 0) + 1; + cleanupCalls.set(name, calls); + return calls < 3 + ? { + cleaned: false, + reason: `${name} cleanup attempt ${String(calls)}`, + } + : { cleaned: true, signaled: false }; + }, + }; + }; + const runner = new XcrunCommandRunner( + {}, + async () => ok({ path: process.execPath, sha256: "a".repeat(64) }), + launcher, + ); + + const [first, second] = await Promise.all([ + runner.run("file", ["first"], {}), + runner.run("file", ["second"], {}), + ]); + expect(first.ok).toBe(false); + expect(second.ok).toBe(false); + if (first.ok || second.ok) return; + expect(first.error.capture).toMatchObject({ + stdout: "partial-first", + stderr: "diagnostic-first", + }); + expect(first.error.cleanupOwner).toBeDefined(); + expect(second.error.cleanupOwner).toBeDefined(); + expect(cleanupCalls).toEqual( + new Map([ + ["first", 1], + ["second", 1], + ]), + ); + + const blocked = await runner.run("file", ["must-not-launch"], {}); + expect(blocked.ok).toBe(false); + if (blocked.ok) return; + expect(blocked.error.cleanupFailures).toHaveLength(2); + expect(blocked.error.cleanupResources).toEqual( + expect.arrayContaining([ + first.error.cleanupOwner?.launch.ownership?.runId, + second.error.cleanupOwner?.launch.ownership?.runId, + ]), + ); + expect(launches).toBe(2); + + const cleared = await runner.retryCleanup?.(); + expect(cleared).toEqual([]); + expect(cleanupCalls).toEqual( + new Map([ + ["first", 3], + ["second", 3], + ]), + ); + }, +); + it.skipIf(process.platform === "win32")( "does not accept signal termination as an allowed nonzero exit", async () => { diff --git a/tests/boundary/providers/native/nativeProvider.test.ts b/tests/boundary/providers/native/nativeProvider.test.ts index 70a3b06ad..b4a713204 100644 --- a/tests/boundary/providers/native/nativeProvider.test.ts +++ b/tests/boundary/providers/native/nativeProvider.test.ts @@ -1,6 +1,8 @@ import { createHash } from "node:crypto"; +import { EventEmitter } from "node:events"; import { chmod, mkdir, readFile, rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; +import { PassThrough } from "node:stream"; import { afterEach, beforeAll, describe, expect, it } from "vitest"; @@ -18,6 +20,7 @@ import { parseLipoArchitectures } 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 { ProviderProcessSupervisor } from "../../../../src/process/ProviderProcess.js"; import { NativeFixtureRunner as FixtureRunner, @@ -26,6 +29,27 @@ import { nativeMachoTargetForFile, } from "../../../fixtures/nativeCommands.js"; +const cleanupOwner = (runId: string): ProviderProcessSupervisor => { + const child = Object.assign(new EventEmitter(), { + stdout: new PassThrough(), + stderr: new PassThrough(), + exitCode: null as number | null, + signalCode: null as NodeJS.Signals | null, + kill: () => true, + }); + return new ProviderProcessSupervisor({ + process: child, + ownsProcessLifetime: true, + ownership: { + runId, + leaderPid: 46_000, + processGroupId: 46_000, + expectedParentPid: process.pid, + }, + cleanup: async () => ({ cleaned: false, reason: "still running" }), + }); +}; + beforeAll(() => prepareProcessOwnershipInspection()); let directory: string | undefined; @@ -315,6 +339,97 @@ describe("native command failure capture and limits", () => { }); }); +describe("native command cleanup lifecycle", () => { + it("waits for an active command before retrying its failed owner at close", async () => { + let finishCommand!: ( + result: Awaited>, + ) => void; + let commandStarted!: () => void; + const started = new Promise((resolve) => { + commandStarted = resolve; + }); + const command = new Promise< + Awaited> + >((resolve) => { + finishCommand = resolve; + }); + let cleanupRetries = 0; + const runner: NativeCommandRunner = { + run: () => { + commandStarted(); + return command; + }, + retryCleanup: async () => { + cleanupRetries += 1; + return cleanupRetries === 1 + ? [ + { + owner: cleanupOwner("run:first"), + reason: "first process group remains live", + resources: ["run:first", "process-group:1234"], + }, + { + owner: cleanupOwner("run:second"), + reason: "second process group remains live", + resources: ["run:second", "process-group:5678"], + }, + ] + : []; + }, + }; + const client = new NativeMacOSProvider({}, runner, "darwin").createClient( + machoTarget("/private/fixture"), + ); + + const execution = client.execute("list_architectures", {}); + await started; + const closing = client.close(); + await Promise.resolve(); + expect(cleanupRetries).toBe(0); + finishCommand( + err( + new NativeCommandFailure("lipo", "timeout", null, undefined, { + stdout: "partial architecture output", + stderr: "partial diagnostic", + stdoutBytes: 27, + stderrBytes: 19, + exitCode: null, + signal: null, + truncated: false, + }), + ), + ); + + const [observed, closed] = await Promise.all([execution, closing]); + expect(observed.ok).toBe(false); + expect(closed.ok).toBe(false); + expect(cleanupRetries).toBe(1); + if (!closed.ok) { + expect(closed.error.cleanupResources).toEqual([ + "run:first", + "process-group:1234", + "run:second", + "process-group:5678", + ]); + expect(closed.error).toHaveProperty("diagnostics", { + reason: "Native command process cleanup remains incomplete", + failures: [ + { + reason: "first process group remains live", + resources: ["run:first", "process-group:1234"], + }, + { + reason: "second process group remains live", + resources: ["run:second", "process-group:5678"], + }, + ], + }); + } + await expect(client.close()).resolves.toMatchObject({ ok: true }); + expect(cleanupRetries).toBe(2); + }); +}); + describe("native command failure projection", () => { it("projects native cancellation output and incomplete cleanup to callers", async () => { const capture = {