From 2af12a18a36bb80fb7d91ef4c8009212750daa0a Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Thu, 8 Oct 2026 23:29:34 +0800 Subject: [PATCH] fix(process): preserve observations after successful cleanup --- src/domain/analysisErrorProjection.ts | 31 +++-- src/domain/process/processCapture.ts | 16 +-- .../capture/ProcessCaptureLifecycle.ts | 114 +++++++++++------- src/process/capture/ProcessHarness.test.ts | 102 ++++++++++++++++ tests/boundary/mcp/toolResult.test.ts | 48 ++++++++ 5 files changed, 252 insertions(+), 59 deletions(-) diff --git a/src/domain/analysisErrorProjection.ts b/src/domain/analysisErrorProjection.ts index bd8e7e9c9b..3025dbb474 100644 --- a/src/domain/analysisErrorProjection.ts +++ b/src/domain/analysisErrorProjection.ts @@ -393,16 +393,27 @@ const lifecycleErrorDetails = ( ), }), }; - if ( - error._tag === "ProcessCaptureError" && - error.userCategory === "cancelled" - ) - return { operation: "process_capture", cleanup: "complete" }; - if ( - error._tag === "ProcessCaptureError" && - error.executionFailure !== undefined - ) - return { execution_failure: error.executionFailure }; + if (error._tag === "ProcessCaptureError") { + const details = { + ...(error.userCategory === "cancelled" + ? { operation: "process_capture", cleanup: "complete" } + : {}), + ...(error.cleanupReport === undefined + ? {} + : { cleanup_report: jsonValueSchema.parse(error.cleanupReport) }), + ...(error.executionFailure === undefined + ? {} + : { execution_failure: error.executionFailure }), + ...(error.partialObservation === undefined + ? {} + : { + partial_observation: jsonValueSchema.parse( + error.partialObservation, + ), + }), + }; + return Object.keys(details).length === 0 ? undefined : details; + } if (error instanceof BinaryTargetError) return { path: error.path, diff --git a/src/domain/process/processCapture.ts b/src/domain/process/processCapture.ts index 283706b125..9739cba666 100644 --- a/src/domain/process/processCapture.ts +++ b/src/domain/process/processCapture.ts @@ -578,7 +578,7 @@ const incompleteCaptureObservationSchema = z.strictObject({ observations: incompleteProcessCaptureObservationsSchema, }); -/** Validated, non-comparable process observations attached to cleanup errors. */ +/** Validated, non-comparable process observations attached to execution or cleanup errors. */ export const partialProcessCaptureObservationSchema = z .union([ completedCaptureObservationSchema.extend( @@ -589,15 +589,15 @@ export const partialProcessCaptureObservationSchema = z ), ]) .superRefine((partial, context) => { - if ( - !Object.values(partial.cleanup).some( - ({ state }) => state === "failed" || state === "unverified", - ) - ) + const cleanupIncomplete = Object.values(partial.cleanup).some( + ({ state }) => state === "failed" || state === "unverified", + ); + if (!cleanupIncomplete && partial.execution_failure === null) context.addIssue({ code: "custom", - path: ["cleanup"], - message: "partial observations require incomplete resource cleanup", + path: ["execution_failure"], + message: + "partial observations require incomplete cleanup or an execution failure", }); if ("observations" in partial) { diff --git a/src/process/capture/ProcessCaptureLifecycle.ts b/src/process/capture/ProcessCaptureLifecycle.ts index 50f9d8e1d5..d0b22abc9b 100644 --- a/src/process/capture/ProcessCaptureLifecycle.ts +++ b/src/process/capture/ProcessCaptureLifecycle.ts @@ -846,6 +846,52 @@ export const resolveProcessResult = ( }, ): ProcessCapture => { const cleanupFailure = cleanupReportFailure(cleanup); + const executionFailureReason = + describeProcessCaptureExecutionFailure(executionFailure); + let partialObservation: PartialProcessCaptureObservation | undefined; + if ( + (cleanupFailure !== undefined || executionFailure !== undefined) && + (capture !== undefined || observations !== undefined) + ) { + const normalizedObservations = + observations === undefined || partialContext === undefined + ? observations + : normalizePartialProcessObservations(observations, partialContext); + if (capture === undefined) { + if (normalizedObservations !== undefined) { + const partialResult = partialProcessCaptureObservationSchema.safeParse({ + observations: normalizedObservations, + cleanup, + execution_failure: executionFailureReason ?? null, + }); + if (partialResult.success) partialObservation = partialResult.data; + } + } else { + const captureData = + "cleanup" in capture + ? (() => { + const { cleanup: _cleanup, ...data } = capture; + void _cleanup; + return data; + })() + : capture; + const settlement = + cleanupFailure !== undefined + ? { ...capture.settlement, cleanup_outcome: "failed" as const } + : capture.settlement.state === "quiesced" + ? { + ...capture.settlement, + cleanup_outcome: "not_required" as const, + } + : { ...capture.settlement, cleanup_outcome: "cleaned" as const }; + const partialResult = partialProcessCaptureObservationSchema.safeParse({ + capture: { ...captureData, settlement }, + cleanup, + execution_failure: executionFailureReason ?? null, + }); + if (partialResult.success) partialObservation = partialResult.data; + } + } if (cleanupFailure !== undefined) { const cleanupResources = Object.entries(cleanup) .filter( @@ -853,44 +899,6 @@ export const resolveProcessResult = ( outcome.state === "failed" || outcome.state === "unverified", ) .map(([resource]) => resource); - const executionFailureReason = - describeProcessCaptureExecutionFailure(executionFailure); - let partialObservation: PartialProcessCaptureObservation | undefined; - if (capture !== undefined || observations !== undefined) { - const normalizedObservations = - observations === undefined || partialContext === undefined - ? observations - : normalizePartialProcessObservations(observations, partialContext); - const partialPayload = - capture === undefined - ? { observations: normalizedObservations } - : { capture }; - const pendingCapture = - capture === undefined - ? undefined - : { - ...capture, - settlement: { - ...capture.settlement, - cleanup_outcome: "failed" as const, - }, - }; - const partialResult = partialProcessCaptureObservationSchema.safeParse({ - ...partialPayload, - ...(pendingCapture === undefined - ? {} - : { - capture: Object.fromEntries( - Object.entries(pendingCapture).filter( - ([key]) => key !== "cleanup", - ), - ), - }), - cleanup, - execution_failure: executionFailureReason ?? null, - }); - if (partialResult.success) partialObservation = partialResult.data; - } throw new ProcessCaptureError(cleanupFailure, { cause: executionFailure, reason: "cleanup_incomplete", @@ -902,15 +910,39 @@ export const resolveProcessResult = ( cleanupReport: cleanup, }); } - if (executionFailure instanceof ProcessCaptureError) throw executionFailure; + if (executionFailure instanceof ProcessCaptureError) { + const retainedExecutionFailure = + executionFailure.executionFailure ?? executionFailureReason; + const retainedPartialObservation = + executionFailure.partialObservation ?? partialObservation; + throw new ProcessCaptureError(executionFailure.message, { + cause: executionFailure, + ...(executionFailure.userMessage === undefined + ? {} + : { userMessage: executionFailure.userMessage }), + ...(executionFailure.userCategory === undefined + ? {} + : { userCategory: executionFailure.userCategory }), + reason: executionFailure.reason, + cleanupResources: executionFailure.cleanupResources, + ...(retainedExecutionFailure === undefined + ? {} + : { executionFailure: retainedExecutionFailure }), + ...(retainedPartialObservation === undefined + ? {} + : { partialObservation: retainedPartialObservation }), + cleanupReport: executionFailure.cleanupReport ?? cleanup, + }); + } if (executionFailure !== undefined) { - const executionFailureReason = - describeProcessCaptureExecutionFailure(executionFailure); throw new ProcessCaptureError("process capture failed", { cause: executionFailure, + reason: "capture_failed", ...(executionFailureReason === undefined ? {} : { executionFailure: executionFailureReason }), + ...(partialObservation === undefined ? {} : { partialObservation }), + cleanupReport: cleanup, }); } if (capture === undefined) diff --git a/src/process/capture/ProcessHarness.test.ts b/src/process/capture/ProcessHarness.test.ts index 961276809a..78a3c48ee0 100644 --- a/src/process/capture/ProcessHarness.test.ts +++ b/src/process/capture/ProcessHarness.test.ts @@ -16,6 +16,7 @@ import { EMPTY_PROCESS_CAPTURE_EXAMPLE } from "../../domain/process/processCaptu import { projectAnalysisError } from "../../domain/analysisErrorProjection.js"; import { emptyProcessCapture } from "../../domain/process/processCapture.fixture.js"; import { analysisErrorProjectionSchema } from "../../contracts/errorSchemas.js"; +import { processCaptureCancelled } from "./ProcessCaptureError.js"; import { releaseProcessResources, resolveProcessResult, @@ -352,8 +353,20 @@ it("retains observations and both causes when process cleanup is unverifiable", terminal_renderer: { state: "cleaned", reason: null }, temporary_root: { state: "cleaned", reason: null }, }, + execution_failure: null, }).success, ).toBe(false); + expect( + partialProcessCaptureObservationSchema.safeParse({ + ...partialObservation, + cleanup: { + owned_process_group: { state: "cleaned", reason: null }, + terminal_renderer: { state: "cleaned", reason: null }, + temporary_root: { state: "cleaned", reason: null }, + }, + execution_failure: "capture ended after a fixture error", + }).success, + ).toBe(true); if (!("capture" in partialObservation)) throw new Error("expected completed partial capture observations"); const partialCapture = partialObservation.capture; @@ -392,6 +405,95 @@ it("projects execution and cleanup failures when capture never completed", () => }); }); +it("retains completed observations when finalization fails after clean cleanup", () => { + const capture = parseProcessCapture({ + ...emptyProcessCapture(), + frames: [{ sequence: 0, at_ms: 0, data: "observed before finalization" }], + event_journal: [], + }); + const executionFailure = new Error("final filesystem snapshot failed"); + const cleanup = { + owned_process_group: { state: "cleaned" as const, reason: null }, + terminal_renderer: { state: "cleaned" as const, reason: null }, + temporary_root: { state: "cleaned" as const, reason: null }, + }; + + let error: ProcessCaptureError | undefined; + try { + resolveProcessResult(capture, executionFailure, cleanup); + } catch (cause: unknown) { + if (!(cause instanceof ProcessCaptureError)) throw cause; + error = cause; + } + expect(error).toBeDefined(); + if (error === undefined) throw new Error("expected capture failure"); + + expect(error).toMatchObject({ + reason: "capture_failed", + cleanupIncomplete: false, + executionFailure: "final filesystem snapshot failed", + cleanupReport: cleanup, + }); + expect(error.cause).toBe(executionFailure); + expect(error.partialObservation).toMatchObject({ + capture: { + frames: [{ data: "observed before finalization" }], + settlement: { cleanup_outcome: "not_required" }, + }, + cleanup, + execution_failure: "final filesystem snapshot failed", + }); + expect(projectAnalysisError(error)).toMatchObject({ + code: "process_capture_failed", + details: { + cleanup_report: cleanup, + execution_failure: "final filesystem snapshot failed", + partial_observation: { + capture: { frames: [{ data: "observed before finalization" }] }, + }, + }, + }); + expect( + analysisErrorProjectionSchema.safeParse(projectAnalysisError(error)) + .success, + ).toBe(true); +}); + +it("preserves cancellation while projecting observations and successful cleanup", () => { + const capture = parseProcessCapture({ + ...emptyProcessCapture(), + frames: [{ sequence: 0, at_ms: 0, data: "observed before cancellation" }], + event_journal: [], + }); + const cleanup = { + owned_process_group: { state: "cleaned" as const, reason: null }, + terminal_renderer: { state: "cleaned" as const, reason: null }, + temporary_root: { state: "cleaned" as const, reason: null }, + }; + let error: ProcessCaptureError | undefined; + try { + resolveProcessResult(capture, processCaptureCancelled(), cleanup); + } catch (cause: unknown) { + if (!(cause instanceof ProcessCaptureError)) throw cause; + error = cause; + } + expect(error).toBeDefined(); + if (error === undefined) throw new Error("expected cancellation"); + + expect(projectAnalysisError(error)).toMatchObject({ + code: "cancelled", + message: "Process capture was cancelled. Start it again when ready.", + details: { + operation: "process_capture", + cleanup: "complete", + cleanup_report: cleanup, + partial_observation: { + capture: { frames: [{ data: "observed before cancellation" }] }, + }, + }, + }); +}); + it("fails closed on Windows before resolving or launching scenario paths", async () => { const scenario = processScenarioSchema.parse({ executable: "Z:/missing/should-never-be-resolved.exe", diff --git a/tests/boundary/mcp/toolResult.test.ts b/tests/boundary/mcp/toolResult.test.ts index eb00c78743..7a48a8c861 100644 --- a/tests/boundary/mcp/toolResult.test.ts +++ b/tests/boundary/mcp/toolResult.test.ts @@ -47,6 +47,54 @@ const contract: ToolContract = { }; describe("completed partial process capture MCP projection", () => { + it("preserves observations when execution fails after cleanup succeeds", () => { + const cleanup = { + owned_process_group: { state: "cleaned" as const, reason: null }, + terminal_renderer: { state: "cleaned" as const, reason: null }, + temporary_root: { state: "cleaned" as const, reason: null }, + }; + const capture = { + ...emptyProcessCapture(), + frames: [{ sequence: 0, at_ms: 0, data: "observed output" }], + event_journal: [], + }; + let failure: ProcessCaptureError | undefined; + try { + resolveProcessResult( + capture, + new Error("final filesystem snapshot failed"), + cleanup, + ); + } catch (cause: unknown) { + if (!(cause instanceof ProcessCaptureError)) throw cause; + failure = cause; + } + if (failure === undefined) + throw new Error("expected process capture failure"); + + const result = toCallToolResult( + err(failure), + toolContract("capture_process_scenario"), + ); + expect(result.isError).toBe(true); + expect(result.structuredContent).toMatchObject({ + error: { + code: "process_capture_failed", + details: { + cleanup_report: cleanup, + execution_failure: "final filesystem snapshot failed", + partial_observation: { + capture: { + frames: [{ data: "observed output" }], + settlement: { cleanup_outcome: "not_required" }, + }, + execution_failure: "final filesystem snapshot failed", + }, + }, + }, + }); + }); + it("preserves partial capture details over the MCP SDK transport", async () => { const executionFailure = new Error("capture ended after a fixture error"); const cleanup = {