diff --git a/docs/mcp-contracts.md b/docs/mcp-contracts.md index 677cd44371..c541c46977 100644 --- a/docs/mcp-contracts.md +++ b/docs/mcp-contracts.md @@ -435,6 +435,9 @@ Provider calls receive the request cancellation signal. Artifact traversal, hashing, version comparisons, Hopper requests, and process capture check the same signal. Cancellation is distinct from timeout. A cleanup failure uses `cleanup_incomplete` and lists only the owned resource kinds that remain. +Once those owners confirm cleanup, composed failures recover their original +typed primary outcome and recovery guidance. Historical composition diagnostics +remain in `details.cleanup_history`, separate from the primary diagnostics. An atomic Evidence bundle or snapshot write can publish its complete output before staging cleanup fails. In that case, `details.published: true`, `details.published_path`, and `details.bytes` retain the publication receipt. diff --git a/src/domain/analysisErrorBase.ts b/src/domain/analysisErrorBase.ts index c62ba74117..593c943a60 100644 --- a/src/domain/analysisErrorBase.ts +++ b/src/domain/analysisErrorBase.ts @@ -53,6 +53,8 @@ export interface AnalysisCleanupObservation { /** Optional provider-neutral context that must survive typed error projection. */ export interface AnalysisErrorOptions extends ErrorOptions { + /** Typed primary retained when a later lifecycle failure composes this error. */ + readonly cleanupPrimary?: AnalysisError | undefined; /** Producer-owned, caller-safe facts retained through every typed projection. */ readonly diagnostics?: Readonly> | undefined; readonly capturedOutput?: AnalysisCapturedOutput | undefined; @@ -67,6 +69,7 @@ export interface AnalysisCleanupResourceOwner { /** Base class for expected analysis, provider, and session failures. */ export abstract class AnalysisError extends Error { + readonly #cleanupPrimary: AnalysisError | undefined; readonly #cleanupDependencies = new Set(); #cleanupDependents: WeakRef[] = []; readonly diagnostics: Readonly> | undefined; @@ -77,6 +80,7 @@ export abstract class AnalysisError extends Error { #cleanupResourceOwner: AnalysisCleanupResourceOwner | undefined; constructor(message: string, options?: AnalysisErrorOptions) { super(message, options); + this.#cleanupPrimary = options?.cleanupPrimary; this.diagnostics = options?.diagnostics === undefined ? undefined @@ -95,6 +99,10 @@ export abstract class AnalysisError extends Error { readonly userCategory: "cancelled" | undefined = undefined; readonly executionFailure: string | undefined = undefined; partialObservation: AnalysisPartialObservation | undefined; + + get cleanupPrimary(): AnalysisError | undefined { + return this.#cleanupPrimary; + } readonly cleanupReport: ProcessCaptureCleanupReport | undefined = undefined; /** Live dependencies own current resource attribution; cleanup keeps history. */ @@ -184,6 +192,13 @@ export abstract class AnalysisError extends Error { return this; } } + +/** Confirmed cleanup reveals the typed primary; unresolved ownership stays visible. */ +export const resolvedAnalysisError = (error: AnalysisError): AnalysisError => { + while (!error.cleanupIncomplete && error.cleanupPrimary !== undefined) + error = error.cleanupPrimary; + return error; +}; import type { PartialProcessCaptureObservation, ProcessCaptureCleanupReport, diff --git a/src/domain/analysisErrorCleanup.test.ts b/src/domain/analysisErrorCleanup.test.ts new file mode 100644 index 0000000000..481fc77df1 --- /dev/null +++ b/src/domain/analysisErrorCleanup.test.ts @@ -0,0 +1,172 @@ +import { expect, it } from "vitest"; +import { + AnalysisCancelledError, + AnalysisInputError, + AnalysisOutputError, + AnalysisResourceConstraintError, + AnalysisTimeoutError, +} from "./analysisErrorCore.js"; +import { analysisErrorWithCleanupFailure } from "./analysisErrorCleanup.js"; +import { projectAnalysisError } from "./analysisErrorProjection.js"; +import { + analysisErrorCategory, + analysisErrorRemediationAction, + analysisErrorUserMessage, +} from "./analysisErrorPresentation.js"; +import { ProviderCleanupError } from "./providerCleanupError.js"; +import { createEvidence } from "./evidence.js"; + +it("keeps primary diagnostics separate from historical cleanup diagnostics with the same keys", () => { + const primary = new AnalysisInputError("inspect", { + diagnostics: { + primary_error: "source observation", + cleanup_error: "another source observation", + }, + }); + const cleanup = new ProviderCleanupError("fixture", ["owned-input"], {}); + const composed = analysisErrorWithCleanupFailure( + primary, + cleanup, + ).bindCleanupDependencies([primary, cleanup]); + cleanup.resolveCleanup(); + const projected = projectAnalysisError(composed); + expect(projected.details?.diagnostics).toEqual(primary.diagnostics); + expect(projected.details?.cleanup_history).toMatchObject({ + primary_error: projectAnalysisError(primary), + cleanup_error: { code: "cleanup_incomplete" }, + }); + expect(composed.cleanupPrimary).toBe(primary); + expect(Object.hasOwn(composed, "cleanupPrimary")).toBe(false); +}); + +it.each([ + new AnalysisInputError("inspect", { diagnostics: { input: "original" } }, [ + { + path: ["path"], + reason: "out_of_range", + message: "fixed input allowance", + }, + ]), + new AnalysisCancelledError("inspect", { cause: "caller reason" }), + new AnalysisTimeoutError("inspect", 1234), + new AnalysisResourceConstraintError( + "inspect", + "file-size", + "fixed input allowance", + { maximum_input_bytes: 123 }, + { retryable: false, remediationAction: "Select a smaller suitable input." }, + ), + new AnalysisOutputError("inspect", "invalid output", { + capturedOutput: { + stdout: "original output", + stderr: "original warning", + truncated: true, + }, + partialObservation: createEvidence( + undefined, + { id: "fixture", name: "Fixture", version: null }, + { + operation: "inspect", + parameters: {}, + result: { original_fact: true }, + confidence: "observed", + locations: [], + limitations: [], + }, + ), + }), +])("restores the typed $_tag primary after confirmed cleanup", (primary) => { + const original = projectAnalysisError(primary); + const cleanup = new ProviderCleanupError( + "fixture", + ["owned-input"], + { reason: "close denied" }, + { operation: "inspect" }, + ); + const composed = analysisErrorWithCleanupFailure( + primary, + cleanup, + "inspect", + ).bindCleanupDependencies([primary, cleanup]); + expect(projectAnalysisError(composed).code).toBe("cleanup_incomplete"); + cleanup.resolveCleanup(); + const projected = projectAnalysisError(composed); + expect(projected).toMatchObject(original); + expect(projected.details?.cleanup_history).toMatchObject({ + primary_error: original, + cleanup_error: { code: "cleanup_incomplete" }, + }); + expect(projected.details).not.toHaveProperty("cleanup", "incomplete"); + expect(projected.details).not.toHaveProperty("resources"); + expect(analysisErrorCategory(composed)).toBe(original.category); + expect(analysisErrorUserMessage(composed)).toBe(original.message); + expect(analysisErrorRemediationAction(composed)).toBe( + original.remediation.action, + ); + expect(composed.cause).toBeInstanceOf(AggregateError); + expect((composed.cause as AggregateError).errors[0]).toBe(primary); +}); + +it.each(["inner", "outer"] as const)( + "waits for both nested cleanup owners when %s resolves first", + (first) => { + const primary = new AnalysisInputError("inspect"); + const innerCleanup = new ProviderCleanupError("fixture", ["input"], {}); + const outerCleanup = new ProviderCleanupError("fixture", ["workspace"], {}); + const inner = analysisErrorWithCleanupFailure( + primary, + innerCleanup, + ).bindCleanupDependencies([primary, innerCleanup]); + const outer = analysisErrorWithCleanupFailure( + inner, + outerCleanup, + ).bindCleanupDependencies([inner, outerCleanup]); + const released = first === "inner" ? innerCleanup : outerCleanup; + const pending = first === "inner" ? outerCleanup : innerCleanup; + released.resolveCleanup(); + expect(outer.cleanupIncomplete).toBe(true); + expect(projectAnalysisError(outer).code).toBe("cleanup_incomplete"); + pending.resolveCleanup(); + expect(outer.cleanupIncomplete).toBe(false); + expect(projectAnalysisError(outer)).toMatchObject( + projectAnalysisError(primary), + ); + }, +); + +it("does not infer cleanup success when a composition has no live owner binding", () => { + const primary = new AnalysisInputError("inspect"); + const cleanup = new ProviderCleanupError("fixture", ["input"], {}); + const composed = analysisErrorWithCleanupFailure(primary, cleanup); + cleanup.resolveCleanup(); + expect(projectAnalysisError(composed).code).toBe("cleanup_incomplete"); +}); + +it("retains an observation attached to the composite after the primary failed", () => { + const primary = new AnalysisInputError("inspect"); + const cleanup = new ProviderCleanupError("fixture", ["input"], {}); + const composed = analysisErrorWithCleanupFailure( + primary, + cleanup, + ).bindCleanupDependencies([primary, cleanup]); + const observation = createEvidence( + undefined, + { id: "fixture", name: "Fixture", version: null }, + { + operation: "inspect", + parameters: {}, + result: { late_fact: true }, + confidence: "observed", + locations: [], + limitations: [], + }, + ); + composed.retainPartialObservation(observation); + composed.resolveCleanup(); + expect(projectAnalysisError(composed).code).toBe("cleanup_incomplete"); + cleanup.resolveCleanup(); + expect(projectAnalysisError(composed)).toMatchObject({ + code: "invalid_request", + details: { partial_observation: observation }, + }); +}); diff --git a/src/domain/analysisErrorCleanup.ts b/src/domain/analysisErrorCleanup.ts index 3dc8ed6f76..ab7b6d43ea 100644 --- a/src/domain/analysisErrorCleanup.ts +++ b/src/domain/analysisErrorCleanup.ts @@ -13,6 +13,7 @@ export const analysisErrorWithCleanupFailure = ( cleanup instanceof ProviderAdapterError ? cleanup.providerId : "rea", operation, { + cleanupPrimary: primary, cause: new AggregateError( [primary, cleanup], "Analysis failed and cleanup remains incomplete.", diff --git a/src/domain/analysisErrorPresentation.ts b/src/domain/analysisErrorPresentation.ts index 410dfae2a4..75a2d0c285 100644 --- a/src/domain/analysisErrorPresentation.ts +++ b/src/domain/analysisErrorPresentation.ts @@ -31,12 +31,17 @@ import { ProviderSelectionError } from "./providerSelectionError.js"; import { ProviderAdapterError } from "./providerAdapterError.js"; import { UnknownRegistryError } from "./unknownRegistryError.js"; import { providerRetryAction } from "./providerOperationHealth.js"; -import type { AnalysisError, AnalysisErrorTag } from "./analysisErrorBase.js"; +import { + resolvedAnalysisError, + type AnalysisError, + type AnalysisErrorTag, +} from "./analysisErrorBase.js"; import type { AnalysisErrorProjection } from "./analysisErrorProjection.js"; export const analysisErrorRemediationAction = ( error: AnalysisError, ): string => { + error = resolvedAnalysisError(error); if ( error instanceof ProviderAdapterError && error.remediationAction !== undefined @@ -138,6 +143,7 @@ export const analysisErrorRemediationAction = ( export const analysisErrorCategory = ( error: AnalysisError, ): AnalysisErrorProjection["category"] => { + error = resolvedAnalysisError(error); if (error instanceof AnalysisCapabilityUnavailableError && error.retryable) return "unavailable"; if ( @@ -234,6 +240,7 @@ const STATIC_ERROR_CATEGORIES: Readonly< }; export const analysisErrorUserMessage = (error: AnalysisError): string => { + error = resolvedAnalysisError(error); if (error instanceof AnalysisSnapshotMismatchError) return error.message; if (error instanceof AnalysisUnsupportedTargetError) return error.message; if (error instanceof AnalysisResourceConstraintError) return error.reason; diff --git a/src/domain/analysisErrorProjection.ts b/src/domain/analysisErrorProjection.ts index a89f1b4138..f0d94b8815 100644 --- a/src/domain/analysisErrorProjection.ts +++ b/src/domain/analysisErrorProjection.ts @@ -40,21 +40,32 @@ import { ProviderAdapterError } from "./providerAdapterError.js"; import { ProviderSelectionError } from "./providerSelectionError.js"; import { UnknownRegistryError } from "./unknownRegistryError.js"; import { providerRetryAction } from "./providerOperationHealth.js"; -import type { AnalysisError, AnalysisErrorTag } from "./analysisErrorBase.js"; +import { + resolvedAnalysisError, + type AnalysisError, + type AnalysisErrorTag, +} from "./analysisErrorBase.js"; /** Project expected failures into exhaustive, secret-safe caller fields. */ export const projectAnalysisError = ( error: AnalysisError, ): AnalysisErrorProjection => { + const composed = error; + error = resolvedAnalysisError(error); + const diagnostics = error.diagnostics; + const capturedOutput = composed.capturedOutput ?? error.capturedOutput; + const partialObservation = + composed.partialObservation ?? error.partialObservation; assertKnownAnalysisErrorTag(error._tag); const underlyingCode = underlyingErrorCode(error); const code = error.cleanupIncomplete ? "cleanup_incomplete" : underlyingCode; const primaryDetails = errorDetails(error); const details = { ...primaryDetails, - ...(error.diagnostics === undefined + ...(diagnostics === undefined ? {} : { diagnostics }), + ...(composed === error || composed.diagnostics === undefined ? {} - : { diagnostics: error.diagnostics }), + : { cleanup_history: composed.diagnostics }), ...(error.cleanupIncomplete ? { cleanup: "incomplete", @@ -66,15 +77,19 @@ export const projectAnalysisError = ( primaryDetails?.execution_failure ?? underlyingCode, } : {}), - ...(error.capturedOutput === undefined - ? {} - : { captured_output: { ...error.capturedOutput } }), - ...(error.partialObservation === undefined + ...(capturedOutput === undefined ? {} : { - partial_observation: isImmutableJsonSnapshot(error.partialObservation) - ? error.partialObservation - : jsonValueSchema.parse(error.partialObservation), + captured_output: { + ...capturedOutput, + }, + }), + ...(partialObservation === undefined + ? {} + : { + partial_observation: isImmutableJsonSnapshot(partialObservation) + ? partialObservation + : jsonValueSchema.parse(partialObservation), }), }; return { diff --git a/tests/boundary/mcp/cleanupPrimaryProjection.test.ts b/tests/boundary/mcp/cleanupPrimaryProjection.test.ts new file mode 100644 index 0000000000..a6113bdc09 --- /dev/null +++ b/tests/boundary/mcp/cleanupPrimaryProjection.test.ts @@ -0,0 +1,80 @@ +import { expect, it, onTestFinished } from "vitest"; +import { AnalysisCleanupLease } from "../../../src/artifacts/AnalysisCleanupLease.js"; +import { AnalysisInputError } from "../../../src/domain/analysisErrorCore.js"; +import { analysisErrorWithCleanupFailure } from "../../../src/domain/analysisErrorCleanup.js"; +import { ProviderCleanupError } from "../../../src/domain/providerCleanupError.js"; +import { err } from "../../../src/domain/result.js"; +import { createApplicationMcpHarness } from "../../fixtures/applicationMcpHarness.js"; +import { advertisedMcpToolSchemas } from "../../fixtures/advertisedMcpTool.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; + +it("restores the primary error after exact cleanup retry through the same MCP connection", async () => { + const primary = new AnalysisInputError("inspect_adb_window", undefined, [ + { + path: ["serial"], + reason: "invalid_value", + message: "fixture input refused", + }, + ]); + const cleanup = new ProviderCleanupError( + "fixture", + ["selected-input"], + { reason: "close denied" }, + { operation: "inspect_adb_window" }, + ); + let permitRelease = false; + const lease = new AnalysisCleanupLease(async () => { + if (!permitRelease) throw new Error("close denied"); + }, ["selected-input"]); + lease.bindCleanupError(cleanup); + const composed = analysisErrorWithCleanupFailure( + primary, + cleanup, + "inspect_adb_window", + ).bindCleanupDependencies([primary, cleanup]); + const harness = await createApplicationMcpHarness({ + serverOptions: { + adbDeviceAnalysis: { + inspectAvailability: async () => ({ + status: "available", + code: null, + reason: null, + diagnostics: {}, + }), + execute: async () => err(composed), + close: async () => { + permitRelease = true; + await lease.release(); + }, + }, + }, + }); + onTestFinished(() => harness.close()); + const { name, ajv, inputSchema } = await advertisedMcpToolSchemas( + harness.client, + "inspect_adb_window", + ); + const args = { serial: "fixture" }; + expect(ajv.validate(inputSchema, args)).toBe(true); + const call = async () => + parseMcpToolError(await harness.client.callTool({ name, arguments: args })) + .error; + expect((await call()).code).toBe("cleanup_incomplete"); + await expect(lease.release()).rejects.toThrow("close denied"); + expect((await call()).code).toBe("cleanup_incomplete"); + permitRelease = true; + await lease.release(); + expect(await call()).toMatchObject({ + code: "invalid_request", + category: "invalid_input", + retryable: true, + details: { + issues: [{ path: ["serial"], reason: "invalid_value" }], + cleanup_history: { + primary_error: { code: "invalid_request" }, + cleanup_error: { code: "cleanup_incomplete" }, + }, + }, + }); + await harness.client.ping(); +}); diff --git a/tests/process-global/binaryDiagnosticsCliCleanup.test.ts b/tests/process-global/binaryDiagnosticsCliCleanup.test.ts index d23ef2a493..29a5c1d23d 100644 --- a/tests/process-global/binaryDiagnosticsCliCleanup.test.ts +++ b/tests/process-global/binaryDiagnosticsCliCleanup.test.ts @@ -16,9 +16,14 @@ afterEach(() => { process.exitCode = exitCode; }); -it.each(["layout", "recorded"] as const)( - "projects the final owned outcome through the public CLI: %s", - async (scenario) => { +it.each( + (["layout", "recorded"] as const).flatMap((scenario) => [ + { scenario, recovered: false }, + { scenario, recovered: true }, + ]), +)( + "projects the final owned outcome through the public CLI: $scenario recovered=$recovered", + async ({ scenario, recovered }) => { const primary = new AnalysisOutputError( "inspect_binary_layout", "fixture decoder output overflow", @@ -35,15 +40,13 @@ it.each(["layout", "recorded"] as const)( ["owned-process", "/retained/runtime"], { reason: "fixture process inspection still denied" }, ); - const inspectAndClose = vi.fn(async () => - err( - analysisErrorWithCleanupFailure( - primary, - cleanup, - "inspect_binary_layout", - ), - ), - ); + const composed = analysisErrorWithCleanupFailure( + primary, + cleanup, + "inspect_binary_layout", + ).bindCleanupDependencies([primary, cleanup]); + if (recovered) cleanup.resolveCleanup(); + const inspectAndClose = vi.fn(async () => err(composed)); const identity = { id: "fixture-decoder", name: "Fixture decoder", @@ -90,21 +93,29 @@ it.each(["layout", "recorded"] as const)( ); const output: unknown = JSON.parse(stdout); expect(output).toMatchObject({ - code: "cleanup_incomplete", + code: recovered ? "unreadable_output" : "cleanup_incomplete", details: { - cleanup: "incomplete", - resources: ["owned-process", "/retained/runtime"], + ...(recovered + ? {} + : { + cleanup: "incomplete", + resources: ["owned-process", "/retained/runtime"], + }), captured_output: { stdout: "partial original output", stderr: "warning", truncated: true, }, - diagnostics: { + [recovered ? "cleanup_history" : "diagnostics"]: { primary_error: { code: "unreadable_output" }, cleanup_error: { code: "cleanup_incomplete" }, }, }, }); + if (recovered) { + expect(output).not.toHaveProperty("details.cleanup"); + expect(output).not.toHaveProperty("details.resources"); + } expect(inspectAndClose).toHaveBeenCalledOnce(); expect(process.exitCode).toBe(1); }, diff --git a/tests/process-global/firmwareFileLifetime.test.ts b/tests/process-global/firmwareFileLifetime.test.ts index 6e4f783bc9..00c3e53cdb 100644 --- a/tests/process-global/firmwareFileLifetime.test.ts +++ b/tests/process-global/firmwareFileLifetime.test.ts @@ -310,6 +310,7 @@ it.each( expect(error.diagnostics?.primary_error).toMatchObject({ code: "invalid_request", }); + expect(projectAnalysisError(error).code).toBe("invalid_request"); } expect(fault.handles[0]?.fd).toBe(-1); } finally {