From 02e2c9cd7f07602ac7e247bc383c30fa654670da Mon Sep 17 00:00:00 2001 From: pablodiazjorge <102741971+pablodiazjorge@users.noreply.github.com> Date: Thu, 22 Oct 2026 20:31:23 +0200 Subject: [PATCH] fix(providers): report provider-selection diagnostics when the selected provider refuses its profile (#4103) * fix(providers): report provider-selection diagnostics when the selected provider refuses its profile An explicitly selected provider that refuses the target during profile resolution now yields the target_unsupported selection error with its candidates and rejections, keeping the refusal's message and, when present, its specific recovery. Fixes #3930 * fix(providers): keep refusal evidence and cleanup ownership on selection failures A refusal with unconfirmed cleanup is returned unchanged; otherwise the selection failure carries the refusal's captured output, diagnostics, partial observation and cause. Registry tests cover the copied recovery and the cleanup guard. --- .../binary/AnalysisProviderRegistry.test.ts | 68 +++++++++++++ .../binary/AnalysisProviderRegistry.ts | 40 +++++++- src/domain/analysisErrorPresentation.ts | 5 + src/domain/analysisErrorProjection.test.ts | 26 +++++ src/domain/providerSelectionError.ts | 24 +++-- .../mcp/ghidraProfileRefusalMcp.test.ts | 96 +++++++++++++++++++ 6 files changed, 248 insertions(+), 11 deletions(-) create mode 100644 tests/boundary/mcp/ghidraProfileRefusalMcp.test.ts diff --git a/src/application/binary/AnalysisProviderRegistry.test.ts b/src/application/binary/AnalysisProviderRegistry.test.ts index a09112ff2f..900838dc14 100644 --- a/src/application/binary/AnalysisProviderRegistry.test.ts +++ b/src/application/binary/AnalysisProviderRegistry.test.ts @@ -15,6 +15,7 @@ import { createAnalysisProfile } from "../../domain/analysisProfile.js"; import type { BinaryTarget } from "../../domain/binaryTargetTypes.js"; import { ProviderAdapterError } from "../../domain/providerAdapterError.js"; import { ProviderCleanupError } from "../../domain/providerCleanupError.js"; +import { AnalysisUnsupportedTargetError } from "../../domain/analysisErrorCore.js"; import { ProviderSelectionError } from "../../domain/providerSelectionError.js"; import { projectAnalysisError } from "../../domain/analysisErrorProjection.js"; import { AnalysisCancelledError } from "../../domain/analysisErrorCore.js"; @@ -607,6 +608,73 @@ const capability = (provider: ProviderIdentity): CapabilityDescriptor => ({ limitations: [], }); +describe("analysis provider registry: explicit profile refusals", () => { + const refusing = (refusal: AnalysisUnsupportedTargetError) => ({ + ...candidate("ghidra").provider, + resolveAnalysisProfile: () => Promise.resolve(err(refusal)), + }); + + it("keeps a refusing provider's reason, recovery and evidence in the selection failure", async () => { + const capturedOutput = { + stdout: "", + stderr: "x86-64 only", + truncated: false, + }; + const refusal = new AnalysisUnsupportedTargetError( + "resolve_analysis_profile", + DATABASE_TARGET.path, + "The configured extension supports x86-64 only.", + { + remediationAction: "Omit the extension setting for this target.", + capturedOutput, + diagnostics: { extension: "fixture" }, + }, + ); + const selected = await new AnalysisProviderRegistry([ + refusing(refusal), + ]).select(DATABASE_TARGET, "ghidra"); + if (selected.ok) throw new Error("expected a profile refusal"); + expect(selected.error).toBeInstanceOf(ProviderSelectionError); + expect(selected.error.cause).toBe(refusal); + expect(projectAnalysisError(selected.error)).toMatchObject({ + code: "capability_unavailable", + message: refusal.message, + remediation: { action: "Omit the extension setting for this target." }, + details: { + selection_reason: "target_unsupported", + requested_provider_id: "ghidra", + candidate_ids: ["ghidra"], + rejections: [ + { + provider_id: "ghidra", + code: "target_format_unsupported", + reason: "The configured extension supports x86-64 only.", + }, + ], + captured_output: capturedOutput, + diagnostics: { extension: "fixture" }, + }, + }); + }); + + it("returns a refusal with unconfirmed cleanup unchanged", async () => { + const refusal = new AnalysisUnsupportedTargetError( + "resolve_analysis_profile", + DATABASE_TARGET.path, + "The configured extension supports x86-64 only.", + { cleanup: { reason: "probe close failed", resources: ["/tmp/probe"] } }, + ); + const selected = await new AnalysisProviderRegistry([ + refusing(refusal), + ]).select(DATABASE_TARGET, "ghidra"); + if (selected.ok) throw new Error("expected a profile refusal"); + expect(selected.error).toBe(refusal); + expect(projectAnalysisError(selected.error)).toMatchObject({ + code: "cleanup_incomplete", + details: { cleanup: "incomplete", resources: ["/tmp/probe"] }, + }); + }); +}); describe("profile resolution terminal facts", () => { it.each([false, true])( "preserves a physical profile failure across late caller abort (throws: %s)", diff --git a/src/application/binary/AnalysisProviderRegistry.ts b/src/application/binary/AnalysisProviderRegistry.ts index b5172ad35a..cf1de645a0 100644 --- a/src/application/binary/AnalysisProviderRegistry.ts +++ b/src/application/binary/AnalysisProviderRegistry.ts @@ -14,10 +14,14 @@ import { } from "../../contracts/providerSelection.js"; import type { AnalysisProfileCommitment } from "../../domain/analysisProfile.js"; import type { BinaryTarget } from "../../domain/binaryTargetTypes.js"; -import { AnalysisCancelledError } from "../../domain/analysisErrorCore.js"; +import { + AnalysisCancelledError, + AnalysisUnsupportedTargetError, +} from "../../domain/analysisErrorCore.js"; import { AnalysisError } from "../../domain/analysisErrorBase.js"; import { ProviderSelectionError, + type ProviderSelectionErrorOptions, type ProviderSelectionFailureReason, type ProviderSelectionRejection, } from "../../domain/providerSelectionError.js"; @@ -593,9 +597,29 @@ export class AnalysisProviderRegistry { ); if (!evaluation.ok) return evaluation; const evaluated = evaluation.value; - if (evaluated.profileError !== undefined) - return err(evaluated.profileError); const statuses = replaceCandidate(baseline, evaluated.status); + const refusal = evaluated.profileError; + // A selected provider refusing the target is a selection outcome that keeps the + // refusal's reason, recovery and evidence; unconfirmed cleanup stays with the refusal. + if ( + refusal instanceof AnalysisUnsupportedTargetError && + !refusal.cleanupIncomplete + ) + return err( + selectionError("target_unsupported", providerId, statuses, { + refusal: { + userMessage: refusal.message, + ...(refusal.remediationAction === undefined + ? {} + : { remediationAction: refusal.remediationAction }), + capturedOutput: refusal.capturedOutput, + diagnostics: refusal.diagnostics, + partialObservation: refusal.partialObservation, + cause: refusal, + }, + }), + ); + if (refusal !== undefined) return err(refusal); if (evaluated.profile === undefined) return err(selectionError("provider_unavailable", providerId, statuses)); return ok( @@ -818,9 +842,19 @@ const selectionError = ( options: { readonly operation?: string; readonly rejectionCandidates?: readonly AnalysisProviderCandidateStatus[]; + readonly refusal?: Pick< + ProviderSelectionErrorOptions, + | "userMessage" + | "remediationAction" + | "capturedOutput" + | "diagnostics" + | "partialObservation" + | "cause" + >; } = {}, ): ProviderSelectionError => new ProviderSelectionError({ + ...options.refusal, operation: options.operation ?? "open_binary", reason, requestedProviderId, diff --git a/src/domain/analysisErrorPresentation.ts b/src/domain/analysisErrorPresentation.ts index 173b7189bd..78a70d155d 100644 --- a/src/domain/analysisErrorPresentation.ts +++ b/src/domain/analysisErrorPresentation.ts @@ -70,6 +70,11 @@ export const analysisErrorRemediationAction = ( return "Review details.reason, then retry the same analysis after the condition clears."; if (error instanceof EvidenceFileError && error.published !== undefined) return "Use the published file at details.published_path and resolve the reported staging cleanup failure."; + if ( + error instanceof ProviderSelectionError && + error.remediationAction !== undefined + ) + return error.remediationAction; if (error instanceof AnalysisSnapshotMismatchError) return "Run analysis without this snapshot, then save a fresh snapshot using the intended artifact, provider, and analysis profile."; if (error instanceof AnalysisUnsupportedTargetError) diff --git a/src/domain/analysisErrorProjection.test.ts b/src/domain/analysisErrorProjection.test.ts index 872e23e54a..224f34511e 100644 --- a/src/domain/analysisErrorProjection.test.ts +++ b/src/domain/analysisErrorProjection.test.ts @@ -282,6 +282,32 @@ it("distinguishes unsupported target formats from unavailable provider capabilit expect(analysisErrorProjectionSchema.safeParse(projected).success).toBe(true); }); +it("keeps a refusing provider's recovery on a target selection failure", () => { + const selection = { + reason: "target_unsupported" as const, + requestedProviderId: "ghidra", + candidateIds: ["ghidra"], + userMessage: "Unsupported target: the configured extension is x86-64 only.", + }; + expect( + projectAnalysisError( + new ProviderSelectionError({ + ...selection, + remediationAction: "Omit the extension setting for this target.", + }), + ), + ).toMatchObject({ + code: "capability_unavailable", + message: selection.userMessage, + remediation: { action: "Omit the extension setting for this target." }, + details: { selection_reason: "target_unsupported" }, + }); + expect( + projectAnalysisError(new ProviderSelectionError(selection)).remediation + .action, + ).toBe(selection.userMessage); +}); + it.each([false, true])( "marks occupied capacity retryable only while cleanup is confirmed: %s", (cleanupIncomplete) => { diff --git a/src/domain/providerSelectionError.ts b/src/domain/providerSelectionError.ts index fb7a262c3f..e53f6ae685 100644 --- a/src/domain/providerSelectionError.ts +++ b/src/domain/providerSelectionError.ts @@ -2,6 +2,7 @@ import type { JsonValue } from "./jsonValue.js"; import { AnalysisError, type AnalysisCapturedOutput, + type AnalysisErrorOptions, } from "./analysisErrorBase.js"; /** Stable failure classes for target-bound deep-provider selection. */ @@ -21,9 +22,14 @@ export interface ProviderSelectionRejection { } /** Complete typed context for one failed deep-provider selection. */ -export interface ProviderSelectionErrorOptions { - readonly capturedOutput?: AnalysisCapturedOutput; +export interface ProviderSelectionErrorOptions extends Pick< + AnalysisErrorOptions, + "cause" | "diagnostics" | "partialObservation" +> { + readonly capturedOutput?: AnalysisCapturedOutput | undefined; readonly userMessage?: string; + /** Recovery known to the provider that refused the target, when it has one. */ + readonly remediationAction?: string; readonly operation?: string; readonly reason: ProviderSelectionFailureReason; readonly requestedProviderId: string; @@ -39,16 +45,17 @@ export class ProviderSelectionError extends AnalysisError { readonly requestedProviderId: string; readonly candidateIds: readonly string[]; readonly rejections: readonly ProviderSelectionRejection[]; + readonly remediationAction: string | undefined; override readonly userMessage: string; constructor(options: ProviderSelectionErrorOptions) { const operation = options.operation ?? "open_binary"; - super( - providerSelectionDiagnostic(options), - options.capturedOutput === undefined - ? undefined - : { capturedOutput: options.capturedOutput }, - ); + super(providerSelectionDiagnostic(options), { + capturedOutput: options.capturedOutput, + diagnostics: options.diagnostics, + partialObservation: options.partialObservation, + ...(options.cause === undefined ? {} : { cause: options.cause }), + }); this.operation = operation; this.reason = options.reason; this.requestedProviderId = options.requestedProviderId; @@ -57,6 +64,7 @@ export class ProviderSelectionError extends AnalysisError { ...rejection, diagnostics: structuredClone(rejection.diagnostics), })); + this.remediationAction = options.remediationAction; this.userMessage = options.userMessage ?? providerSelectionUserMessage(options); } diff --git a/tests/boundary/mcp/ghidraProfileRefusalMcp.test.ts b/tests/boundary/mcp/ghidraProfileRefusalMcp.test.ts new file mode 100644 index 0000000000..fd32c4051d --- /dev/null +++ b/tests/boundary/mcp/ghidraProfileRefusalMcp.test.ts @@ -0,0 +1,96 @@ +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { Client, InMemoryTransport } from "@modelcontextprotocol/client"; +import { expect, it } from "vitest"; + +import { AnalysisProviderRegistry } from "../../../src/application/binary/AnalysisProviderRegistry.js"; +import { composeBinarySession } from "../../../src/application/binary/BinarySessionComposition.js"; +import { parseConfig } from "../../../src/config/parseConfig.js"; +import { GhidraProvider } from "../../../src/ghidra/GhidraProvider.js"; +import { silentLogger } from "../../../src/logger.js"; +import { createServer } from "../../../src/server/createServer.js"; +import { parseMcpToolError } from "../../fixtures/mcpToolError.js"; +import { ghidraInstallationHost } from "./ghidraMcpHarness.js"; + +const REFUSAL = + "REA_GHIDRA_LANGUAGE_ID cannot override the admitted raw-binary import."; + +const openRawArm = async (languageId: string | undefined) => { + const directory = await mkdtemp(join(tmpdir(), "rea-profile-refusal-")); + const path = join(directory, "image.bin"); + await writeFile(path, Buffer.from([0, 0, 160, 225])); + const config = parseConfig({ + GHIDRA_INSTALL_DIR: "/opt/ghidra_12.1.4_PUBLIC", + ...(languageId === undefined ? {} : { REA_GHIDRA_LANGUAGE_ID: languageId }), + }); + if (!config.ok) throw config.error; + let clients = 0; + const provider = new GhidraProvider( + config.value, + silentLogger, + {}, + ghidraInstallationHost(), + () => { + clients += 1; + throw new Error("Profile resolution must not start an engine"); + }, + ); + const session = composeBinarySession( + new AnalysisProviderRegistry([provider]), + ); + const server = createServer( + { kind: "session", session }, + { logger: silentLogger }, + ); + const mcp = new Client({ name: "profile-refusal", version: "1.0.0" }); + const [clientTransport, serverTransport] = + InMemoryTransport.createLinkedPair(); + try { + await server.connect(serverTransport); + await mcp.connect(clientTransport); + const result = await mcp.callTool({ + name: "open_binary", + arguments: { + path, + format: "raw-binary", + architecture: "arm", + entry_point: "0x8000000", + provider_id: "ghidra", + }, + }); + return { result, clients }; + } finally { + await Promise.allSettled([mcp.close(), server.close()]); + await session.close(); + await rm(directory, { recursive: true, force: true }); + } +}; + +it("returns provider-selection diagnostics when the selected provider refuses its profile", async () => { + const { result, clients } = await openRawArm("ARM:LE:32:v7"); + expect(clients).toBe(0); + const { error } = parseMcpToolError(result); + expect(error).toMatchObject({ + code: "capability_unavailable", + message: expect.stringContaining(REFUSAL), + details: { + selection_reason: "target_unsupported", + requested_provider_id: "ghidra", + candidate_ids: ["ghidra"], + rejections: [ + expect.objectContaining({ + provider_id: "ghidra", + code: "target_format_unsupported", + reason: REFUSAL, + }), + ], + }, + }); +}); + +it("still opens the same raw target when no language override conflicts", async () => { + const { result, clients } = await openRawArm(undefined); + expect(result.isError, JSON.stringify(result)).not.toBe(true); + expect(clients).toBe(0); +});