diff --git a/src/application/process/ProcessCli.ts b/src/application/process/ProcessCli.ts index 76d75a73d4..1a600abe3e 100644 --- a/src/application/process/ProcessCli.ts +++ b/src/application/process/ProcessCli.ts @@ -1,4 +1,5 @@ import { AnalysisError } from "../../domain/analysisErrorBase.js"; +import { err, ok, type Result } from "../../domain/result.js"; import { analysisErrorWithCleanupFailure } from "../../domain/analysisErrorCleanup.js"; import { AnalysisCancelledError, @@ -25,7 +26,8 @@ import { parseProcessCaptureEvidence, } from "./ProcessEvidence.js"; import { readJsonFile } from "../JsonFiles.js"; -import { RegularFileCleanupFailure } from "../../filesystem/OwnedRegularFile.js"; +import { ArtifactReaderFailure } from "../../artifacts/ArtifactReader.js"; +import { ArtifactResourceScope } from "../../artifacts/ArtifactResourceScope.js"; /** Safe process-command failure returned to the CLI adapter. */ export interface ProcessCliErrorOutput extends AnalysisErrorProjection { @@ -59,9 +61,14 @@ export const captureProcessScenarioFile = async ( path: string, environment: Readonly> = process.env, signal?: AbortSignal, -) => { - try { - const input = await readJson(path, "capture_process_scenario", signal); +) => + withProcessInputResources("capture_process_scenario", async (resources) => { + const input = await readJson( + path, + "capture_process_scenario", + signal, + resources, + ); const parsed = processScenarioSchema.safeParse(input); if (!parsed.success) throw analysisInputErrorFromIssues( @@ -104,11 +111,7 @@ export const captureProcessScenarioFile = async ( } if (!captured.ok) return cliAnalysisError(captured.error); return createProcessCaptureEvidence(parsed.data, captured.value); - } catch (cause: unknown) { - if (cause instanceof RegularFileCleanupFailure) throw cause; - return projectProcessCliError(cause, "capture_process_scenario"); - } -}; + }); /** Compare two capture Evidence files and emit derived comparison Evidence. */ export const compareProcessEvidenceFiles = async ( @@ -116,32 +119,33 @@ export const compareProcessEvidenceFiles = async ( rightPath: string, traceSpecPath?: string, signal?: AbortSignal, -) => { - try { +) => + withProcessInputResources("compare_process_captures", async (resources) => { const left = parseCaptureEvidence( - await readJson(leftPath, "compare_process_captures", signal), + await readJson(leftPath, "compare_process_captures", signal, resources), "left", ); const right = parseCaptureEvidence( - await readJson(rightPath, "compare_process_captures", signal), + await readJson(rightPath, "compare_process_captures", signal, resources), "right", ); const traceSpecification = traceSpecPath === undefined ? undefined : parseTraceSpecification( - await readJson(traceSpecPath, "compare_process_captures", signal), + await readJson( + traceSpecPath, + "compare_process_captures", + signal, + resources, + ), ); return compareProcessCaptureEvidence( left, right, traceSpecification === undefined ? {} : { traceSpecification }, ).evidence; - } catch (cause: unknown) { - if (cause instanceof RegularFileCleanupFailure) throw cause; - return projectProcessCliError(cause, "compare_process_captures"); - } -}; + }); const parseCaptureEvidence = (input: unknown, side: "left" | "right") => { const parsed = parseProcessCaptureEvidence(input); @@ -200,11 +204,12 @@ const invalidCaptureEvidence = (cause: unknown, side: "left" | "right") => const readJson = async ( path: string, operation: "capture_process_scenario" | "compare_process_captures", - signal?: AbortSignal, + signal: AbortSignal | undefined, + resources: ArtifactResourceScope, ): Promise => { if (signal?.aborted) throw new AnalysisCancelledError(operation, { cause: signal.reason }); - const loaded = await readJsonFile(path, signal); + const loaded = await readJsonFile(path, signal, resources, operation); if (!loaded.ok && loaded.error instanceof AnalysisCancelledError) throw new AnalysisCancelledError(operation, { cause: loaded.error.cause }); if (loaded.ok) { @@ -262,3 +267,34 @@ const projectProcessCliError = ( }), ); }; + +/** One invocation owns every input descriptor through final cleanup and retry. */ +const withProcessInputResources = async ( + operation: "capture_process_scenario" | "compare_process_captures", + run: (resources: ArtifactResourceScope) => Promise, +): Promise => { + const resources = new ArtifactResourceScope(); + let outcome: Result; + try { + outcome = ok(await run(resources)); + } catch (cause: unknown) { + outcome = err(cause); + } + try { + await resources.close(); + } catch (cause: unknown) { + const cleanup = ArtifactReaderFailure.cleanupObservation( + cause, + resources.cleanupResources, + ); + throw ArtifactReaderFailure.withCleanup( + outcome.ok ? cause : outcome.error, + cleanup, + undefined, + cause instanceof ArtifactReaderFailure ? cause.cleanupLeases : undefined, + ); + } + return outcome.ok + ? outcome.value + : projectProcessCliError(outcome.error, operation); +}; diff --git a/tests/boundary/filesystem/processCliFileInput.test.ts b/tests/boundary/filesystem/processCliFileInput.test.ts index ac204ddfa4..db3935b399 100644 --- a/tests/boundary/filesystem/processCliFileInput.test.ts +++ b/tests/boundary/filesystem/processCliFileInput.test.ts @@ -11,7 +11,7 @@ import { createEvidence } from "../../../src/domain/evidence.js"; import { PROCESS_PROVIDER } from "../../../src/domain/process/processEvidenceProvider.js"; import { EMPTY_PROCESS_CAPTURE_EXAMPLE } from "../../../src/domain/process/processCaptureExample.js"; import { OwnedFileHandle } from "../../../src/filesystem/OwnedFileHandle.js"; -import { RegularFileCleanupFailure } from "../../../src/filesystem/OwnedRegularFile.js"; +import { ArtifactReaderFailure } from "../../../src/artifacts/ArtifactReader.js"; import { analysisCliErrorEnvelopeSchema } from "../../../src/contracts/errorSchemas.js"; import { EvidenceFileError } from "../../../src/domain/evidenceErrors.js"; import { err } from "../../../src/domain/result.js"; @@ -401,40 +401,79 @@ describe("process CLI input diagnostics", () => { }); describe("process CLI file cleanup", () => { - it.each([false, true])( - "preserves the open-file owner when close fails (cancelled: %s)", - async (cancelDuringClose) => { + it("retries a transient input close refusal before validating the scenario", async () => { + const root = await createTestTempDirectory( + "rea-process-cli-cleanup-retry-", + ); + const path = join(root, "scenario.json"); + await writeFile(path, "{}"); + let owner: OwnedFileHandle | undefined; + const close = vi + .spyOn(OwnedFileHandle.prototype, "close") + .mockImplementationOnce(async function (this: OwnedFileHandle) { + owner = this; + throw new Error("transient close refusal"); + }); + try { + expect(await captureProcessScenarioFile(path)).toMatchObject({ + code: "invalid_request", + }); + expect(close).toHaveBeenCalledTimes(2); + expect(owner?.handle.fd).toBe(-1); + } finally { + close.mockRestore(); + await owner?.close(); + } + }); + + it.each([ + { operation: "capture", cancelDuringClose: false }, + { operation: "capture", cancelDuringClose: true }, + { operation: "compare", cancelDuringClose: false }, + { operation: "compare", cancelDuringClose: true }, + ] as const)( + "preserves the open-file owner when $operation close fails (cancelled: $cancelDuringClose)", + async ({ operation, cancelDuringClose }) => { const root = await createTestTempDirectory("rea-process-cli-cleanup-"); const path = join(root, "scenario.json"); await writeFile(path, JSON.stringify({ executable: process.execPath })); const controller = new AbortController(); const cleanupCause = new Error("injected file close failure"); + let owner: OwnedFileHandle | undefined; const close = vi .spyOn(OwnedFileHandle.prototype, "close") - .mockImplementationOnce(async () => { + .mockImplementation(async function (this: OwnedFileHandle) { + owner = this; if (cancelDuringClose) controller.abort(); throw cleanupCause; }); let failure: unknown; try { try { - await captureProcessScenarioFile( - path, - process.env, - cancelDuringClose ? controller.signal : undefined, - ); + const signal = cancelDuringClose ? controller.signal : undefined; + if (operation === "capture") + await captureProcessScenarioFile(path, process.env, signal); + else await compareProcessEvidenceFiles(path, path, undefined, signal); } catch (cause: unknown) { failure = cause; } - expect(failure).toBeInstanceOf(RegularFileCleanupFailure); - if (!(failure instanceof RegularFileCleanupFailure)) + expect(failure).toBeInstanceOf(ArtifactReaderFailure); + if (!(failure instanceof ArtifactReaderFailure)) throw new Error("Expected the retained file handle owner"); - expect(failure.cleanupCause).toBe(cleanupCause); - expect(failure.owner.handle.fd).toBeGreaterThanOrEqual(0); + expect(failure.cleanup).toMatchObject({ + reason: expect.stringContaining(cleanupCause.message), + resources: [path], + }); + expect(owner?.handle.fd).toBeGreaterThanOrEqual(0); + expect(failure.cleanupLeases).toHaveLength(1); + expect(close.mock.calls.length).toBeGreaterThanOrEqual(2); } finally { close.mockRestore(); - if (failure instanceof RegularFileCleanupFailure) - await failure.owner.close(); + if (failure instanceof ArtifactReaderFailure) + for (const lease of failure.cleanupLeases ?? []) + await lease.release(); + else await owner?.close(); + expect(owner?.handle.fd).toBe(-1); } }, );