mirror of
https://github.com/morluto/rea.git
synced 2026-10-26 17:35:11 +00:00
fix(process): retain CLI input owners through final cleanup
This commit is contained in:
1 parent
d421e3eeef
commit
4f6166e04d
2 files changed
+112
-37
No files matched your search
@@ -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<Record<string, string | undefined>> = 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<unknown> => {
|
||||
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 <Value>(
|
||||
operation: "capture_process_scenario" | "compare_process_captures",
|
||||
run: (resources: ArtifactResourceScope) => Promise<Value>,
|
||||
): Promise<Value | ProcessCliErrorOutput> => {
|
||||
const resources = new ArtifactResourceScope();
|
||||
let outcome: Result<Value, unknown>;
|
||||
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);
|
||||
};
|
||||
@@ -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);
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
Reference in new issue
Block a user