From b43daa166f5f55ce6649f00927bbd5343daaadc2 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Fri, 23 Oct 2026 00:59:51 +0800 Subject: [PATCH] fix(analysis): retain typed outcomes through final cleanup --- src/application/DirectAnalysis.ts | 355 ++++++++++-------- src/application/DirectAnalysisCleanup.ts | 48 +-- .../FunctionAnalysisEvidence.test.ts | 149 ++++++++ src/application/FunctionAnalysisEvidence.ts | 13 +- .../WasmInterfaceComparisonService.test.ts | 59 ++- .../wasm/WasmInterfaceComparisonService.ts | 53 +-- src/server/analysisAdmission.ts | 6 +- .../mcp/functionAnalysisAdmission.test.ts | 40 ++ .../composition/directAnalysisCleanup.test.ts | 274 +++++++++++++- 9 files changed, 753 insertions(+), 244 deletions(-) create mode 100644 src/application/FunctionAnalysisEvidence.test.ts create mode 100644 tests/boundary/mcp/functionAnalysisAdmission.test.ts diff --git a/src/application/DirectAnalysis.ts b/src/application/DirectAnalysis.ts index bc01644eda..f3161896f7 100644 --- a/src/application/DirectAnalysis.ts +++ b/src/application/DirectAnalysis.ts @@ -22,6 +22,7 @@ import { import { EvidenceFileError, EvidenceIntegrityError, + type PublishedEvidenceFile, } from "../domain/evidenceErrors.js"; import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; @@ -71,6 +72,7 @@ import { managedAnalysisCleanupFailure, withSessionCleanup, } from "./DirectAnalysisCleanup.js"; +import { completeProviderEvidence } from "./ProviderOperationExecution.js"; import { BUNDLE_RESOURCE_OPERATIONS } from "./binary/BundleResourceSession.js"; type DirectAnalysisTool = @@ -318,31 +320,49 @@ const runAnalysis = async ( : dependencies.createBinarySession )(config.value, logger); } - return withSessionCleanup( - session, - async () => { - let isDirectory = false; - if (BUNDLE_RESOURCE_OPERATIONS.has(tool)) { - const inspected = await inspectBundleResourceInput(path); - if (!inspected.ok) return cliError(inspected.error); - isDirectory = inspected.value; - } - if (BUNDLE_RESOURCE_OPERATIONS.has(tool) && isDirectory) { - const opened = await session.openBundleResources(path, { signal }); - if (!opened.ok) return cliError(opened.error); - const execution = await session.execute(tool, arguments_, { signal }); - if (!execution.ok) return cliError(execution.error); - return createAnalysisExecutionEvidence( + let publishedSnapshot: PublishedEvidenceFile | undefined; + const execute = async (): Promise> => { + let isDirectory = false; + if (BUNDLE_RESOURCE_OPERATIONS.has(tool)) { + const inspected = await inspectBundleResourceInput(path); + if (!inspected.ok) return err(inspected.error); + isDirectory = inspected.value; + } + if (BUNDLE_RESOURCE_OPERATIONS.has(tool) && isDirectory) { + const opened = await session.openBundleResources(path, { signal }); + if (!opened.ok) return err(opened.error); + const execution = await session.execute(tool, arguments_, { signal }); + if (!execution.ok) return err(execution.error); + return ok( + createAnalysisExecutionEvidence( tool, arguments_, execution.value, "observed", - ); - } - const prepared = await prepareSnapshot({ - path, - snapshotPath, + ), + ); + } + const prepared = await prepareSnapshot({ + path, + snapshotPath, + signal, + ...(options.formatHint === undefined + ? {} + : { formatHint: options.formatHint }), + ...(options.architecture === undefined + ? {} + : { architecture: options.architecture }), + ...(options.entryPoint === undefined + ? {} + : { entryPoint: options.entryPoint }), + }); + if (!prepared.ok) return err(prepared.error); + const { snapshot } = prepared.value; + let resolvedTarget: ResolvedSessionOpen | undefined; + if (snapshot !== undefined && prepared.value.target !== undefined) { + const preview = await session.previewTarget(prepared.value.target, { signal, + snapshot, ...(options.formatHint === undefined ? {} : { formatHint: options.formatHint }), @@ -352,146 +372,150 @@ const runAnalysis = async ( ...(options.entryPoint === undefined ? {} : { entryPoint: options.entryPoint }), - }); - if (!prepared.ok) return cliError(prepared.error); - const { snapshot } = prepared.value; - let resolvedTarget: ResolvedSessionOpen | undefined; - if (snapshot !== undefined && prepared.value.target !== undefined) { - const preview = await session.previewTarget(prepared.value.target, { - signal, - snapshot, - ...(options.formatHint === undefined - ? {} - : { formatHint: options.formatHint }), - ...(options.architecture === undefined - ? {} - : { architecture: options.architecture }), - ...(options.entryPoint === undefined - ? {} - : { entryPoint: options.entryPoint }), - ...(options.providerId === undefined - ? {} - : { providerId: options.providerId }), - }); - if (!preview.ok) return cliError(preview.error); - resolvedTarget = preview.value; - const route = preview.value.route; - const bindingProfile = - route.profile ?? nativeDispatchBindingProfile(route, tool); - const evidenceProfile = analysisProfileForRoute(route, tool); - // A parsed Windows path does not prove Ghidra's native snapshot owner - // admitted the selected source; CLI replay must wait for that client. - if ( - bindingProfile !== undefined && - evidenceProfile !== undefined && - !requiresNativeSourceAdmissionBeforeReplay( - route.binding?.identity.id, - ) && - allowsSnapshotReplay(route, tool) - ) { - const cached = snapshotEvidenceForQuery(snapshot, { - target: preview.value.target, - bindingProfile, - operation: tool, - parameters: arguments_, - provider: isWorkflowEvidenceTool(tool) - ? REA_WORKFLOW_PROVIDER - : providerIdentityForRoute(route, tool), - evidenceProfile, - }); - if (cached !== undefined) return cached; - } - } - const openOptions = { - signal, - ...(options.formatHint === undefined - ? {} - : { formatHint: options.formatHint }), - ...(options.architecture === undefined - ? {} - : { architecture: options.architecture }), - ...(options.entryPoint === undefined - ? {} - : { entryPoint: options.entryPoint }), - ...(snapshot === undefined ? {} : { snapshot }), ...(options.providerId === undefined ? {} : { providerId: options.providerId }), - }; - const opened = - resolvedTarget === undefined - ? await session.open(path, openOptions) - : await session.openResolvedTarget(resolvedTarget, openOptions); - if (!opened.ok) return cliError(opened.error); - const evidenceProfile = analysisProfileForEvidence(session, tool); - const { output, evidence, cachePolicy } = await executeAnalysisTool({ - session, - openedTarget: opened.value, - tool, - arguments: arguments_, - signal, - evidenceProfile, }); - if (evidence !== undefined) { - const recorded = session.recordEvidence(evidence); - if (!recorded.ok) - return cliError(recorded.error.retainPartialObservation(evidence)); - if (isWorkflowEvidenceTool(tool)) { - const unknowns = recordWorkflowUnknowns({ - name: tool, - result: evidence.normalized_result, - evidenceId: evidence.evidence_id, - recordUnknown: (unknown) => session.recordUnknown(unknown), - }); - if (!unknowns.ok) - return cliError(unknowns.error.retainPartialObservation(evidence)); - } - } + if (!preview.ok) return err(preview.error); + resolvedTarget = preview.value; + const route = preview.value.route; + const bindingProfile = + route.profile ?? nativeDispatchBindingProfile(route, tool); + const evidenceProfile = analysisProfileForRoute(route, tool); + // A parsed Windows path does not prove Ghidra's native snapshot owner + // admitted the selected source; CLI replay must wait for that client. if ( - isWorkflowEvidenceTool(tool) && - tool !== "trace_native_ui_action" && - snapshotPath !== undefined && - evidence !== undefined && - session.allowsSnapshotReplay(tool) + bindingProfile !== undefined && + evidenceProfile !== undefined && + !requiresNativeSourceAdmissionBeforeReplay( + route.binding?.identity.id, + ) && + allowsSnapshotReplay(route, tool) ) { - const workflowRecord = workflowSnapshotRecord( - evidence, - tool, + const cached = snapshotEvidenceForQuery(snapshot, { + target: preview.value.target, + bindingProfile, + operation: tool, + parameters: arguments_, + provider: isWorkflowEvidenceTool(tool) + ? REA_WORKFLOW_PROVIDER + : providerIdentityForRoute(route, tool), evidenceProfile, - cachePolicy, - ); - if (workflowRecord !== undefined) { - const recorded = session.recordWorkflowSnapshot(workflowRecord); - if (!recorded.ok) - return cliError(recorded.error.retainPartialObservation(evidence)); - } + }); + if (cached !== undefined) return ok(cached); } - if ( - tool !== "trace_native_ui_action" && - snapshotPath !== undefined && - evidence !== undefined - ) { - const snapshot = session.exportAnalysisSnapshot(); - if (!snapshot.ok) - return cliError(snapshot.error.retainPartialObservation(evidence)); - const written = await session.writeAnalysisSnapshot( - snapshot.value, - snapshotPath, - true, - ); - if (!written.ok) { - const recovered = - written.error instanceof EvidenceFileError && - written.error.published !== undefined && - (await session.closeWriteResources()).ok; - if (!recovered) - return cliError(written.error.retainPartialObservation(evidence)); - } + } + const openOptions = { + signal, + ...(options.formatHint === undefined + ? {} + : { formatHint: options.formatHint }), + ...(options.architecture === undefined + ? {} + : { architecture: options.architecture }), + ...(options.entryPoint === undefined + ? {} + : { entryPoint: options.entryPoint }), + ...(snapshot === undefined ? {} : { snapshot }), + ...(options.providerId === undefined + ? {} + : { providerId: options.providerId }), + }; + const opened = + resolvedTarget === undefined + ? await session.open(path, openOptions) + : await session.openResolvedTarget(resolvedTarget, openOptions); + if (!opened.ok) return err(opened.error); + const evidenceProfile = analysisProfileForEvidence(session, tool); + const executed = await executeAnalysisTool({ + session, + openedTarget: opened.value, + tool, + arguments: arguments_, + signal, + evidenceProfile, + }); + if (!executed.ok) return executed; + const { evidence, cachePolicy } = executed.value; + const recorded = session.recordEvidence(evidence); + if (!recorded.ok) + return err(recorded.error.retainPartialObservation(evidence)); + if (isWorkflowEvidenceTool(tool)) { + const unknowns = recordWorkflowUnknowns({ + name: tool, + result: evidence.normalized_result, + evidenceId: evidence.evidence_id, + recordUnknown: (unknown) => session.recordUnknown(unknown), + }); + if (!unknowns.ok) + return err(unknowns.error.retainPartialObservation(evidence)); + } + if ( + isWorkflowEvidenceTool(tool) && + tool !== "trace_native_ui_action" && + snapshotPath !== undefined && + session.allowsSnapshotReplay(tool) + ) { + const workflowRecord = workflowSnapshotRecord( + evidence, + tool, + evidenceProfile, + cachePolicy, + ); + if (workflowRecord !== undefined) { + const recorded = session.recordWorkflowSnapshot(workflowRecord); + if (!recorded.ok) + return err(recorded.error.retainPartialObservation(evidence)); } - return output; - }, - directAnalysisCleanupFailure, + } + if (tool !== "trace_native_ui_action" && snapshotPath !== undefined) { + const snapshot = session.exportAnalysisSnapshot(); + if (!snapshot.ok) + return err(snapshot.error.retainPartialObservation(evidence)); + const written = await session.writeAnalysisSnapshot( + snapshot.value, + snapshotPath, + true, + ); + if (written.ok) publishedSnapshot = written.value; + else { + if (written.error instanceof EvidenceFileError) + publishedSnapshot = written.error.published; + const recovered = + written.error instanceof EvidenceFileError && + written.error.published !== undefined && + (await session.closeWriteResources()).ok; + if (!recovered) + return err(written.error.retainPartialObservation(evidence)); + } + } + return ok(evidence); + }; + const outcome = await withSessionCleanup<{ + readonly result: Result; + readonly cleanupError?: AnalysisError; + }>( + session, + async () => ({ result: await execute() }), + (value, cleanupError) => ({ ...value, cleanupError }), ); + await dispatchQueuedInterrupts(); + const result = outcome.result.ok + ? completeProviderEvidence(outcome.result.value, signal) + : outcome.result; + if (!result.ok && publishedSnapshot !== undefined) + result.error.retainDiagnostics({ + analysis_snapshot: { ...publishedSnapshot }, + }); + if (outcome.cleanupError !== undefined) { + // A committed snapshot remains published even when final close fails. + if (result.ok && publishedSnapshot !== undefined) + outcome.cleanupError.retainDiagnostics({ + analysis_snapshot: { ...publishedSnapshot }, + }); + return directAnalysisCleanupFailure(result, outcome.cleanupError); + } + return result.ok ? result.value : cliError(result.error); }; const executeAnalysisTool = async (input: { @@ -505,11 +529,15 @@ const executeAnalysisTool = async (input: { readonly arguments: Readonly>; readonly signal: AbortSignal; readonly evidenceProfile: AnalysisProfileCommitment | undefined; -}): Promise<{ - readonly output: JsonValue; - readonly evidence?: Evidence; - readonly cachePolicy?: "live"; -}> => { +}): Promise< + Result< + { + readonly evidence: Evidence; + readonly cachePolicy?: "live"; + }, + AnalysisError + > +> => { const { session, tool, signal, evidenceProfile } = input; if (tool === "analyze_function") { const { executeFunctionAnalysisEvidence } = @@ -520,9 +548,7 @@ const executeAnalysisTool = async (input: { input.openedTarget, { signal }, ); - return result.ok - ? { output: result.value, evidence: result.value } - : { output: cliError(result.error) }; + return result.ok ? ok({ evidence: result.value }) : result; } if ( tool === "binary_overview" || @@ -536,7 +562,7 @@ const executeAnalysisTool = async (input: { const result = await executeComposedWorkflow(session, (observed) => new EnhancedTools(observed).execute(tool, input.arguments, signal), ); - if (!result.ok) return { output: cliError(result.error) }; + if (!result.ok) return result; const evidence = createWorkflowEvidence({ target: input.openedTarget, operation: tool, @@ -545,13 +571,12 @@ const executeAnalysisTool = async (input: { upstreamProfile: evidenceProfile, upstreamLimitations: result.value.limitations, }); - return { - output: evidence, + return ok({ evidence, ...(result.value.cachePolicy === undefined ? {} : { cachePolicy: result.value.cachePolicy }), - }; + }); } let result = await session.execute(tool, input.arguments, { signal }); if ( @@ -573,7 +598,7 @@ const executeAnalysisTool = async (input: { input.arguments.address, signal, ); - if (!address.ok) return { output: cliError(address.error) }; + if (!address.ok) return address; if (address.value !== input.arguments.address) result = await session.execute( tool, @@ -581,7 +606,7 @@ const executeAnalysisTool = async (input: { { signal }, ); } - if (!result.ok) return { output: cliError(result.error) }; + if (!result.ok) return result; const evidence = createEvidence( result.value.subject === undefined ? input.openedTarget @@ -607,7 +632,7 @@ const executeAnalysisTool = async (input: { : [], }, ); - return { output: evidence, evidence }; + return ok({ evidence }); }; const withProcessCancellation = async ( diff --git a/src/application/DirectAnalysisCleanup.ts b/src/application/DirectAnalysisCleanup.ts index 84adb05094..0be6d875c6 100644 --- a/src/application/DirectAnalysisCleanup.ts +++ b/src/application/DirectAnalysisCleanup.ts @@ -1,4 +1,4 @@ -import { analysisErrorProjectionSchema } from "../contracts/errorSchemas.js"; +import type { Evidence } from "../domain/evidence.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; import { jsonValueSchema, type JsonValue } from "../domain/jsonValue.js"; @@ -31,36 +31,28 @@ export const withSessionCleanup = async ( return closed.ok ? value : onCleanupFailure(value, closed.error); }; -/** Keep the primary CLI failure or completed Evidence alongside cleanup failure. */ +/** Project typed one-shot outcomes only after execution and cleanup settle. */ export const directAnalysisCleanupFailure = ( - output: JsonValue, + result: Result, error: AnalysisError, ): JsonValue => { const cleanup = projectAnalysisError(error); - if (typeof output === "object" && output !== null && !Array.isArray(output)) { - const { error: label, ...projection } = output; - const primary = analysisErrorProjectionSchema.safeParse(projection); - if (typeof label === "string" && primary.success) - return { - error: label, - ...primary.data, - code: "cleanup_incomplete", - retryable: false, - remediation: cleanup.remediation, - details: { - // Primary fields describe the original failure; the later close - // assessment, including its resources, belongs to cleanup_error. - resources: error.cleanupResources.slice(), - ...primary.data.details, - cleanup: "incomplete", - execution_failure: - primary.data.details !== undefined && - "execution_failure" in primary.data.details - ? (primary.data.details.execution_failure ?? primary.data.code) - : primary.data.code, - cleanup_error: cleanup, - }, - }; + if (!result.ok) { + const primary = projectAnalysisError(result.error); + return { + error: "Analysis failed", + ...primary, + code: "cleanup_incomplete", + retryable: false, + remediation: cleanup.remediation, + details: { + resources: error.cleanupResources.slice(), + ...primary.details, + cleanup: "incomplete", + execution_failure: primary.details?.execution_failure ?? primary.code, + cleanup_error: cleanup, + }, + }; } return { error: "Analysis failed", @@ -70,7 +62,7 @@ export const directAnalysisCleanupFailure = ( details: { ...cleanup.details, cleanup: "incomplete", - partial_observation: output, + partial_observation: result.value, }, }; }; diff --git a/src/application/FunctionAnalysisEvidence.test.ts b/src/application/FunctionAnalysisEvidence.test.ts new file mode 100644 index 0000000000..d38b0938ec --- /dev/null +++ b/src/application/FunctionAnalysisEvidence.test.ts @@ -0,0 +1,149 @@ +import { expect, it } from "vitest"; +import { createAnalysisExecution } from "./AnalysisProvider.js"; +import { executeFunctionAnalysisEvidence } from "./FunctionAnalysisEvidence.js"; +import { AnalysisOutputError } from "../domain/analysisErrorCore.js"; +import { parseEvidence } from "../domain/evidence.js"; +import { createAnalysisProfile } from "../domain/analysisProfile.js"; +import { ghidraFunctionDossier } from "../domain/ghidraValues.fixture.js"; +import type { BinaryTarget } from "../domain/binaryTargetTypes.js"; +import { err, ok } from "../domain/result.js"; + +const provider = { + id: "dossier-fixture", + name: "Synthetic dossier", + version: "1", +}; +const target: BinaryTarget = { + path: "/selected/application.elf", + sha256: "a".repeat(64), + kind: "executable", + format: "elf", + architecture: "x86_64", + availableArchitectures: ["x86_64"], +}; +const parameters = { procedure: "fixture_main" }; + +it("does not start function work when admission is already cancelled", async () => { + const reason = { request: "cancelled before provider admission" }; + let calls = 0; + const analysis = { + execute: async () => { + calls++; + return ok(createAnalysisExecution(ghidraFunctionDossier(), provider)); + }, + }; + const result = await executeFunctionAnalysisEvidence( + analysis, + parameters, + target, + { signal: AbortSignal.abort(reason) }, + ); + if (result.ok) throw new Error("Expected pre-admission cancellation"); + expect(result.error).toMatchObject({ _tag: "AnalysisCancelledError" }); + expect(result.error.cause).toBe(reason); + expect(calls).toBe(0); +}); + +it.each(["selected-target", "no-subject", "provider-subject"] as const)( + "retains the completed dossier and its %s binding after cancellation", + async (binding) => { + const dossier = ghidraFunctionDossier(); + const reason = { request: "cancelled after provider observation" }; + const analysisProfile = createAnalysisProfile(provider, { + language: "x86:LE:64", + }); + const relationshipCoverage = { + status: "partial" as const, + limitations: ["Indirect callers remain unknown"], + limited_directions: ["callers"] as ["callers"], + }; + const subject = { + ...target, + path: "/observed/provider.elf", + sha256: "b".repeat(64), + }; + for (const aborted of [false, true]) { + const controller = new AbortController(); + const execution = createAnalysisExecution(dossier, provider, { + ...(binding === "selected-target" + ? {} + : { subject: binding === "no-subject" ? null : subject }), + analysisProfile, + relationshipCoverage, + rawResult: { provider_text: "preserved" }, + limitations: ["Synthetic provider observation"], + locations: [{ kind: "address", address: "0x401000" }], + }); + const result = await executeFunctionAnalysisEvidence( + { + execute: async () => { + if (aborted) controller.abort(reason); + return ok(execution); + }, + }, + parameters, + target, + { signal: controller.signal }, + ); + if (aborted) { + if (result.ok) throw new Error("Expected late cancellation"); + expect(result.error).toMatchObject({ _tag: "AnalysisCancelledError" }); + expect(result.error.cause).toBe(reason); + } else if (!result.ok) throw result.error; + const evidence = parseEvidence( + result.ok ? result.value : result.error.partialObservation, + ); + expect(evidence.operation).toBe("analyze_function"); + expect(evidence.parameters).toEqual(parameters); + expect(evidence.normalized_result).toEqual(dossier); + expect(evidence.raw_result).toEqual({ provider_text: "preserved" }); + expect(evidence.provider).toEqual(provider); + expect(evidence.analysis_profile).toEqual(analysisProfile); + expect(evidence.relationship_coverage).toEqual(relationshipCoverage); + expect(evidence.limitations).toContain("Synthetic provider observation"); + expect(evidence.locations).toContainEqual({ + kind: "address", + address: "0x401000", + }); + if (binding === "no-subject") expect(evidence.subject).toBeNull(); + else + expect(evidence.subject?.local_path).toBe( + binding === "selected-target" ? target.path : subject.path, + ); + } + }, +); + +it.each(["provider-failure", "invalid-dossier"] as const)( + "keeps %s primary if cancellation arrives before result consumption", + async (kind) => { + const controller = new AbortController(); + const failure = new AnalysisOutputError( + "analyze_function", + "Provider failure", + ); + const result = await executeFunctionAnalysisEvidence( + { + execute: async () => { + controller.abort({ request: "unrelated late cancellation" }); + return kind === "provider-failure" + ? err(failure) + : ok( + createAnalysisExecution({ procedure: "incomplete" }, provider), + ); + }, + }, + parameters, + target, + { signal: controller.signal }, + ); + if (result.ok) throw new Error("Expected the producer/output failure"); + if (kind === "provider-failure") expect(result.error).toBe(failure); + else + expect(result.error).toMatchObject({ + _tag: "AnalysisOutputError", + operation: "analyze_function", + }); + expect(result.error.partialObservation).toBeUndefined(); + }, +); diff --git a/src/application/FunctionAnalysisEvidence.ts b/src/application/FunctionAnalysisEvidence.ts index 9042082137..e12b0831e8 100644 --- a/src/application/FunctionAnalysisEvidence.ts +++ b/src/application/FunctionAnalysisEvidence.ts @@ -7,8 +7,10 @@ import type { BinaryTarget } from "../domain/binaryTargetTypes.js"; import { createEvidence } from "../domain/evidence.js"; import { parseFunctionDossier } from "../domain/hopperValues.js"; import { jsonObjectSchema, type JsonValue } from "../domain/jsonValue.js"; -import { err, ok } from "../domain/result.js"; +import { err } from "../domain/result.js"; import { analysisInputErrorFromIssues } from "../domain/inputIssueProjection.js"; +import { AnalysisCancelledError } from "../domain/analysisErrorCore.js"; +import { completeProviderEvidence } from "./ProviderOperationExecution.js"; /** Preserve the provider's observations when presenting a validated function dossier. */ export const executeFunctionAnalysisEvidence = async ( @@ -17,6 +19,12 @@ export const executeFunctionAnalysisEvidence = async ( target: BinaryTarget | undefined, options?: ExecutionOptions, ) => { + if (options?.signal?.aborted) + return err( + new AnalysisCancelledError("analyze_function", { + cause: options.signal.reason, + }), + ); const input = toolContract("analyze_function").inputSchema.safeParse(parameters); if (!input.success) @@ -38,7 +46,7 @@ export const executeFunctionAnalysisEvidence = async ( const dossier = parseFunctionDossier(execution.value.result); if (!dossier.ok) return dossier; const observation = execution.value; - return ok( + return completeProviderEvidence( createEvidence( observation.subject === undefined ? target @@ -59,5 +67,6 @@ export const executeFunctionAnalysisEvidence = async ( : { analysisProfile: observation.analysisProfile }), }, ), + options?.signal, ); }; diff --git a/src/application/wasm/WasmInterfaceComparisonService.test.ts b/src/application/wasm/WasmInterfaceComparisonService.test.ts index 85fee9985f..2c427418c0 100644 --- a/src/application/wasm/WasmInterfaceComparisonService.test.ts +++ b/src/application/wasm/WasmInterfaceComparisonService.test.ts @@ -1,5 +1,6 @@ import { expect, it } from "vitest"; -import { createEvidence } from "../../domain/evidence.js"; +import { setImmediate as nextTurn } from "node:timers/promises"; +import { createEvidence, parseEvidence } from "../../domain/evidence.js"; import { projectAnalysisError } from "../../domain/analysisErrorProjection.js"; import { wasmInterfaceComparisonSchema } from "../../contracts/wasm/wasmComparisonContracts.js"; import { compareWasmInterfaces } from "./WasmInterfaceComparisonService.js"; @@ -173,15 +174,17 @@ it("resolves exact retained references and permits cancellation between comparis ...Array.from({ length: 2000 }, (_, i) => facts(`f${i}`).entries[0]!), ); const controller = new AbortController(); + const reason = { request: "cancel between interface batches" }; const pending = compareWasmInterfaces( { left: evidence(large), right: evidence(large), scope: "both" }, { signal: controller.signal }, ); - setImmediate(() => controller.abort()); - expect(await pending).toMatchObject({ - ok: false, - error: { _tag: "AnalysisCancelledError" }, - }); + setImmediate(() => controller.abort(reason)); + const cancelled = await pending; + if (cancelled.ok) throw new Error("Expected cancellation between batches"); + expect(cancelled.error).toMatchObject({ _tag: "AnalysisCancelledError" }); + expect(cancelled.error.cause).toBe(reason); + expect(cancelled.error.partialObservation).toBeUndefined(); }); /** One inventory under the comparison cap; two of them exceed it together. */ @@ -267,3 +270,47 @@ it.each([false, true])( ); }, ); + +it("preserves the caller reason when comparison admission is already cancelled", async () => { + const left = evidence(facts("f")); + const right = evidence(facts("f")); + const reason = { request: "cancel before comparison" }; + const preAborted = await compareWasmInterfaces( + { left, right, scope: "both" }, + { signal: AbortSignal.abort(reason) }, + ); + if (preAborted.ok) throw new Error("Expected cancellation before comparison"); + expect(preAborted.error.cause).toBe(reason); +}); + +it("retains the complete comparison when cancelled at the final handoff", async () => { + const left = evidence(facts("changed")); + const right = evidence(facts("changed", "i64")); + const reason = { request: "cancel completed comparison" }; + const controller = new AbortController(); + const pending = compareWasmInterfaces( + { left, right, scope: "both" }, + { signal: controller.signal }, + ); + // One entry per side needs no batched yields. The initial turn resumes the + // comparison and queues its final handoff before this control turn resumes. + await nextTurn(); + controller.abort(reason); + const result = await pending; + if (result.ok) throw new Error("Expected cancellation during final handoff"); + expect(result.error).toMatchObject({ _tag: "AnalysisCancelledError" }); + expect(result.error.cause).toBe(reason); + const partial = parseEvidence(result.error.partialObservation); + expect(partial).toMatchObject({ + operation: "compare_wasm_interfaces", + confidence: "derived", + authority: "analyst-inference", + evidence_links: [left.evidence_id, right.evidence_id], + }); + expect( + wasmInterfaceComparisonSchema.parse(partial.normalized_result), + ).toMatchObject({ + coverage: "complete-within-inputs", + summary: { changed: 1, added: 0, removed: 0, unchanged: 0, unknown: 0 }, + }); +}); diff --git a/src/application/wasm/WasmInterfaceComparisonService.ts b/src/application/wasm/WasmInterfaceComparisonService.ts index d25a9f1edf..a3800505d2 100644 --- a/src/application/wasm/WasmInterfaceComparisonService.ts +++ b/src/application/wasm/WasmInterfaceComparisonService.ts @@ -26,13 +26,14 @@ import { import { fixedAllowanceOptions } from "../../domain/fixedAllowanceRemediation.js"; import { AnalysisError } from "../../domain/analysisErrorBase.js"; import { analysisInputErrorFromIssues } from "../../domain/inputIssueProjection.js"; -import { err, ok, type Result } from "../../domain/result.js"; +import { err, type Result } from "../../domain/result.js"; import { resolvePairedEvidenceRequest, type EvidenceLookup, } from "../EvidenceInputResolver.js"; import type { ExecutionOptions } from "../AnalysisProvider.js"; import type { z } from "zod"; +import { completeProviderEvidence } from "../ProviderOperationExecution.js"; const OPERATION = "compare_wasm_interfaces"; const parseInspection = ( @@ -60,7 +61,9 @@ export const compareWasmInterfaces = async ( lookup?: EvidenceLookup, ): Promise> => { if (options?.signal?.aborted) - return err(new AnalysisCancelledError(OPERATION)); + return err( + new AnalysisCancelledError(OPERATION, { cause: options.signal.reason }), + ); const parsed = compareWasmInterfacesInputSchema.safeParse(rawInput); if (!parsed.success) return err( @@ -68,9 +71,15 @@ export const compareWasmInterfaces = async ( ); const resolved = resolvePairedEvidenceRequest(parsed.data, lookup); if (!resolved.ok) return resolved; + const assertActive = () => { + if (options?.signal?.aborted) + throw new AnalysisCancelledError(OPERATION, { + cause: options.signal.reason, + }); + }; try { await setImmediate(); - options?.signal?.throwIfAborted(); + assertActive(); const left = parseInspection(resolved.value.left); const right = parseInspection(resolved.value.right); const scope = parsed.data.scope; @@ -93,7 +102,7 @@ export const compareWasmInterfaces = async ( for (const entry of inspection.result.interface?.entries ?? []) { if (++work % 256 === 0) { await setImmediate(); - options?.signal?.throwIfAborted(); + assertActive(); } if (!selected(entry)) continue; const key = JSON.stringify([entry.direction, entry.module, entry.name]); @@ -141,7 +150,7 @@ export const compareWasmInterfaces = async ( ); if (++work % 256 === 0) { await setImmediate(); - options?.signal?.throwIfAborted(); + assertActive(); } let state: keyof typeof summary; let reason: string | null = null; @@ -220,27 +229,23 @@ export const compareWasmInterfaces = async ( summary, limitations, }); + const evidence = createEvidence(undefined, ARTIFACT_COMPARISON_PROVIDER, { + operation: OPERATION, + predicateType: "rea.wasm-interface-comparison", + parameters: { + left_evidence_id: left.evidence.evidence_id, + right_evidence_id: right.evidence.evidence_id, + scope, + }, + result: report, + confidence: "derived", + authority: "analyst-inference", + limitations, + evidenceLinks: [left.evidence.evidence_id, right.evidence.evidence_id], + }); await setImmediate(); - options?.signal?.throwIfAborted(); - return ok( - createEvidence(undefined, ARTIFACT_COMPARISON_PROVIDER, { - operation: OPERATION, - predicateType: "rea.wasm-interface-comparison", - parameters: { - left_evidence_id: left.evidence.evidence_id, - right_evidence_id: right.evidence.evidence_id, - scope, - }, - result: report, - confidence: "derived", - authority: "analyst-inference", - limitations, - evidenceLinks: [left.evidence.evidence_id, right.evidence.evidence_id], - }), - ); + return completeProviderEvidence(evidence, options?.signal); } catch (cause: unknown) { - if (options?.signal?.aborted) - return err(new AnalysisCancelledError(OPERATION)); if (cause instanceof AnalysisError) return err(cause); return err( new AnalysisInputError(OPERATION, { cause }, [ diff --git a/src/server/analysisAdmission.ts b/src/server/analysisAdmission.ts index 93e79e2b11..c9f55a3521 100644 --- a/src/server/analysisAdmission.ts +++ b/src/server/analysisAdmission.ts @@ -25,5 +25,9 @@ export const withAdmittedAnalysis = ( source.session.withAdmittedAnalysis(operationName, signal, operation) : async (operationName, signal, operation) => signal?.aborted === true - ? err(new AnalysisCancelledError(operationName)) + ? err( + new AnalysisCancelledError(operationName, { + cause: signal.reason, + }), + ) : ok(await operation(source.analysis)); diff --git a/tests/boundary/mcp/functionAnalysisAdmission.test.ts b/tests/boundary/mcp/functionAnalysisAdmission.test.ts new file mode 100644 index 0000000000..f4625bf030 --- /dev/null +++ b/tests/boundary/mcp/functionAnalysisAdmission.test.ts @@ -0,0 +1,40 @@ +import { expect, it } from "vitest"; +import { withAdmittedAnalysis } from "../../../src/server/analysisAdmission.js"; +import { executeFunctionAnalysisEvidence } from "../../../src/application/FunctionAnalysisEvidence.js"; +import { createAnalysisExecution } from "../../../src/application/AnalysisProvider.js"; +import { ghidraFunctionDossier } from "../../../src/domain/ghidraValues.fixture.js"; +import { ok } from "../../../src/domain/result.js"; + +it("retains the caller reason when fixed MCP admission prevents function work", async () => { + const reason = { request: "cancelled before provider admission" }; + const signal = AbortSignal.abort(reason); + let calls = 0; + const analysis = { + execute: async () => { + calls++; + return ok( + createAnalysisExecution(ghidraFunctionDossier(), { + id: "fixture", + name: "Synthetic dossier", + version: "1", + }), + ); + }, + }; + const admitted = await withAdmittedAnalysis({ kind: "fixed", analysis })( + "analyze_function", + signal, + () => + executeFunctionAnalysisEvidence( + analysis, + { procedure: "fixture" }, + undefined, + { signal }, + ), + ); + const result = admitted.ok ? admitted.value : admitted; + if (result.ok) throw new Error("Expected pre-admission cancellation"); + expect(result.error).toMatchObject({ _tag: "AnalysisCancelledError" }); + expect(result.error.cause).toBe(reason); + expect(calls).toBe(0); +}); diff --git a/tests/composition/directAnalysisCleanup.test.ts b/tests/composition/directAnalysisCleanup.test.ts index 36f1bf1104..10389c2399 100644 --- a/tests/composition/directAnalysisCleanup.test.ts +++ b/tests/composition/directAnalysisCleanup.test.ts @@ -1,6 +1,6 @@ -import { readdir } from "node:fs/promises"; +import { readFile, readdir, stat } from "node:fs/promises"; import { dirname, join } from "node:path"; -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { runDirectAnalysis, runManagedProviderExecution, @@ -21,6 +21,14 @@ import { } from "../../src/domain/analysisErrorProjection.js"; import { parseEvidence } from "../../src/domain/evidence.js"; import { ProviderCleanupError } from "../../src/domain/providerCleanupError.js"; +import { ghidraFunctionDossier } from "../../src/domain/ghidraValues.fixture.js"; +import { + createAnalysisProfile, + type AnalysisProfileCommitment, +} from "../../src/domain/analysisProfile.js"; +import { OwnedStagedOutput } from "../../src/filesystem/OwnedStagedOutput.js"; +import { readAnalysisSnapshot } from "../../src/application/binary/AnalysisSnapshotFiles.js"; +import { EvidenceFileError } from "../../src/domain/evidenceErrors.js"; import { err, ok } from "../../src/domain/result.js"; import { isCliOperationFailure } from "../../src/cliLogging.js"; import { observed } from "../fixtures/analysisExecution.js"; @@ -46,6 +54,7 @@ const primaryError = new AnalysisTimeoutError("read_bytes", 100, { const factories = ( options: { + readonly profile?: AnalysisProfileCommitment; readonly executionError?: AnalysisError; readonly thrownExecution?: Error; readonly cleanupError?: AnalysisError; @@ -53,22 +62,33 @@ const factories = ( readonly onClose?: () => void; } = {}, ) => { - const session = createTestBinarySession(() => ({ - execute: async (operation) => { - if (operation === "health") return observed(null); - if (options.thrownExecution !== undefined) throw options.thrownExecution; - return options.executionError === undefined - ? observed({ operation, observation: "retained result" }) - : err(options.executionError); - }, - close: async () => { - if (options.thrownCleanup !== undefined) throw options.thrownCleanup; - options.onClose?.(); - return options.cleanupError === undefined - ? ok(null) - : err(options.cleanupError); - }, - })); + const profile = options.profile; + const session = createTestBinarySession( + () => ({ + execute: async (operation) => { + if (operation === "health") return observed(null); + if (options.thrownExecution !== undefined) + throw options.thrownExecution; + return options.executionError === undefined + ? observed( + operation === "analyze_function" + ? ghidraFunctionDossier() + : { operation, observation: "retained result" }, + ) + : err(options.executionError); + }, + close: async () => { + if (options.thrownCleanup !== undefined) throw options.thrownCleanup; + options.onClose?.(); + return options.cleanupError === undefined + ? ok(null) + : err(options.cleanupError); + }, + }), + profile === undefined + ? {} + : { resolveAnalysisProfile: async () => ok({ profile }) }, + ); const dependencies: DirectAnalysisDependencies = { readConfiguration: () => parseConfig({}), createBinarySession: () => session, @@ -80,6 +100,224 @@ const factories = ( return { dependencies, session }; }; +describe("one-shot finalization cancellation", () => { + it.each(["read_bytes", "analyze_function"] as const)( + "retains completed %s Evidence when cancellation arrives during final close", + async (tool) => { + const [path] = await createBinarySessionTargets(); + const parameters = + tool === "analyze_function" ? { procedure: "fixture" } : {}; + const original = parseEvidence( + await runDirectAnalysis( + factories().dependencies, + path, + tool, + parameters, + ), + ); + const controller = new AbortController(); + const reason = { request: "cancel during native close" }; + const { dependencies, session } = factories({ + onClose: () => controller.abort(reason), + }); + const result = await runDirectAnalysis( + dependencies, + path, + tool, + parameters, + { signal: controller.signal }, + ); + expect(result).toEqual({ + error: "Analysis failed", + ...projectAnalysisError( + new AnalysisCancelledError(tool, { + cause: reason, + partialObservation: original, + }), + ), + }); + expect(session.activeTarget()).toBeUndefined(); + }, + ); + + it.each([false, true])( + "preserves the execution failure during close cancellation (cleanup fails: %s)", + async (closeFails) => { + const [path] = await createBinarySessionTargets(); + const controller = new AbortController(); + const { dependencies } = factories({ + executionError: primaryError, + onClose: () => controller.abort({ later: true }), + ...(closeFails ? { cleanupError } : {}), + }); + const result = await runDirectAnalysis( + dependencies, + path, + "read_bytes", + {}, + { signal: controller.signal }, + ); + expect(result).toMatchObject({ + code: closeFails ? "cleanup_incomplete" : "provider_timeout", + message: projectAnalysisError(primaryError).message, + details: { + captured_output: primaryError.capturedOutput, + ...(closeFails + ? { cleanup_error: projectAnalysisError(cleanupError) } + : {}), + }, + }); + }, + ); + + it("retains completed Evidence and cancellation alongside unsuccessful cleanup", async () => { + const [path] = await createBinarySessionTargets(); + const controller = new AbortController(); + const { dependencies } = factories({ + cleanupError, + onClose: () => controller.abort({ during: "close" }), + }); + const result = await runDirectAnalysis( + dependencies, + path, + "read_bytes", + {}, + { signal: controller.signal }, + ); + expect(result).toMatchObject({ + code: "cleanup_incomplete", + category: "cancelled", + details: { + execution_failure: "cancelled", + resources: ["fixture-provider-process"], + partial_observation: { + operation: "read_bytes", + normalized_result: { observation: "retained result" }, + }, + cleanup_error: projectAnalysisError(cleanupError), + }, + }); + }); +}); + +describe("snapshot publication finalization", () => { + it.each([ + "success", + "cancel", + "recovered-write-cancel", + "close-failure", + "cancel-close-failure", + ] as const)( + "retains the actual snapshot publication through finalization: %s", + async (mode) => { + const [path] = await createBinarySessionTargets(); + const snapshotPath = join(dirname(path), "snapshot.json"); + const controller = new AbortController(); + const profile = createAnalysisProfile( + { id: "fixture", name: "Fixture analysis provider", version: "1" }, + { fixture: true }, + ); + const cancelled = mode.includes("cancel"); + const { dependencies, session } = factories({ + profile, + onClose: () => { + if (cancelled) controller.abort({ during: "snapshot finalization" }); + }, + ...(mode.includes("close-failure") ? { cleanupError } : {}), + }); + const stagingCleanup = + mode === "recovered-write-cancel" + ? vi + .spyOn(OwnedStagedOutput.prototype, "close") + .mockRejectedValueOnce( + new Error("transient staging cleanup failure"), + ) + : undefined; + try { + const result = await runDirectAnalysis( + dependencies, + path, + "read_bytes", + {}, + { signal: controller.signal, snapshotPath }, + ); + const loaded = await readAnalysisSnapshot(snapshotPath); + if (!loaded.ok) throw loaded.error; + const evidence = loaded.value.evidence_bundle.records.find( + (record) => record.operation === "read_bytes", + ); + expect(evidence).toBeDefined(); + const receipt = { + path: snapshotPath, + bytes: (await stat(snapshotPath)).size, + }; + expect((await readFile(snapshotPath)).byteLength).toBe(receipt.bytes); + if (mode === "success") expect(result).toEqual(evidence); + else { + const failure = analysisCliErrorEnvelopeSchema.parse(result); + expect(failure.code).toBe( + mode.includes("close-failure") ? "cleanup_incomplete" : "cancelled", + ); + expect(failure.details).toMatchObject({ + partial_observation: evidence, + diagnostics: { analysis_snapshot: receipt }, + }); + } + expect(session.activeTarget()).toBeUndefined(); + expect((await readdir(dirname(path))).toSorted()).toEqual([ + "first.hop", + "second.hop", + "snapshot.json", + ]); + if (stagingCleanup !== undefined) + expect(stagingCleanup).toHaveBeenCalledTimes(2); + } finally { + stagingCleanup?.mockRestore(); + } + }, + ); + + it("preserves a snapshot write failure over later cancellation without a publication receipt", async () => { + const [path] = await createBinarySessionTargets(); + const controller = new AbortController(); + const profile = createAnalysisProfile( + { id: "fixture", name: "Fixture analysis provider", version: "1" }, + { fixture: true }, + ); + const { dependencies, session } = factories({ + profile, + onClose: () => controller.abort({ later: true }), + }); + const failure = new EvidenceFileError("write", "io", { + path: join(dirname(path), "failed.json"), + }); + const write = vi + .spyOn(session, "writeAnalysisSnapshot") + .mockResolvedValue(err(failure)); + try { + const result = analysisCliErrorEnvelopeSchema.parse( + await runDirectAnalysis( + dependencies, + path, + "read_bytes", + {}, + { signal: controller.signal, snapshotPath: failure.path }, + ), + ); + expect(result.code).toBe(projectAnalysisError(failure).code); + expect(result.details).toMatchObject({ + partial_observation: { operation: "read_bytes" }, + }); + expect(result.details).not.toHaveProperty( + "diagnostics.analysis_snapshot", + ); + expect(await readdir(dirname(path))).not.toContain("failed.json"); + } finally { + write.mockRestore(); + } + }); +}); + describe("one-shot analysis cleanup outcomes", () => { it("retains successful evidence and reports CLI failure when close fails", async () => { const [path] = await createBinarySessionTargets();