From 4da7e46e9ce90d8a59dfc50dd7a9e400b657461a Mon Sep 17 00:00:00 2001 From: YingNing <59612604+dxqlovestudy@users.noreply.github.com> Date: Mon, 26 Oct 2026 23:49:11 +0800 Subject: [PATCH] fix(provider-selection): name whether a rejection came from availability or target support (#7220) Distinguish local provider readiness failures from target-support refusals in shared CLI/MCP diagnostics, preserving original codes and evidence. Avoid claiming configured providers are unconfigured. 120 focused tests, static/docs checks and real ELF CLI/MCP verification pass. Hosted CI has the same four ADB failures as exact main d9908fd7a; three canceled shards remain unverified. Merge uses the user's explicit authorization to bypass proven main failures. Fresh Codex and Greptile reviews are clear. Fixes #7214 Co-authored-by: gensokyo-dev <342580529+gensokyo-dev@users.noreply.github.com> --- docs/cli.md | 8 +- .../binary/AnalysisProviderRegistry.test.ts | 125 ++++++++++++++++++ .../binary/AnalysisProviderRegistry.ts | 53 +++++--- src/domain/analysisErrorProjection.ts | 1 + src/domain/providerSelectionError.ts | 16 ++- src/native/pwntools/PwntoolsFailures.ts | 1 + 6 files changed, 187 insertions(+), 17 deletions(-) diff --git a/docs/cli.md b/docs/cli.md index 367731119a..eabae901c8 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -443,7 +443,13 @@ The selected provider remains bound to the session until an explicit switch or close. Provider failures are returned with their original reason. For an `ambiguous` selection error, choose from `details.candidate_ids`; for `provider_unavailable`, follow `message`, `remediation.action`, and the original -reasons in `details.rejections`. For Hopper, Ghidra, IDA, JADX, and Kuna, run +reasons in `details.rejections`. Each rejection names its `origin`: `availability` +entries describe local readiness failures, including missing configuration or an +invalid existing registration; they do not establish target support. `target` +entries record a refused kind, format, architecture, or role. An availability-only +message describes the recorded rejections without claiming that every provider +is unconfigured or that target support is known. +For Hopper, Ghidra, IDA, JADX, and Kuna, run `rea doctor --provider ID --json` to diagnose the selected engine. Other adapters, such as `pwntools-elf`, report their configuration requirements directly and are not accepted by `doctor --provider`. See diff --git a/src/application/binary/AnalysisProviderRegistry.test.ts b/src/application/binary/AnalysisProviderRegistry.test.ts index c464390a76..10c440daa0 100644 --- a/src/application/binary/AnalysisProviderRegistry.test.ts +++ b/src/application/binary/AnalysisProviderRegistry.test.ts @@ -433,6 +433,131 @@ describe("analysis provider registry: selection failures", () => { }); }); +describe("analysis provider registry: rejection origins", () => { + it("distinguishes recorded readiness failures from an unobserved target refusal", async () => { + const registry = new AnalysisProviderRegistry([ + candidate("alpha", { available: false, unconfigured: true }).provider, + candidate("gamma", { available: false, unconfigured: true }).provider, + ]); + const selection = await registry.select(DATABASE_TARGET); + expect(selection.ok).toBe(true); + if (!selection.ok) return; + const projected = projectAnalysisError( + registry.unboundOperationError("binary_overview", selection.value), + ); + expect(projected.message).toContain( + "Provider selection failed local readiness checks. No target-support refusal was reported.", + ); + }); + + it("reports a configured provider's readiness failure without hiding a ready alternative", async () => { + const invalid = candidate("alpha", { available: false }); + const ready = candidate("beta"); + const registry = new AnalysisProviderRegistry([ + { + ...invalid.provider, + inspectAvailability: () => ({ + status: "unavailable", + code: "open_options_invalid", + reason: "Configured registration is invalid", + diagnostics: { registration_path: "/configured/alpha.json" }, + }), + }, + ready.provider, + ]); + const selected = await registry.select(DATABASE_TARGET, "alpha"); + if (selected.ok) + throw new Error("Expected the selected provider to refuse"); + const projected = projectAnalysisError(selected.error); + expect(projected.message).toContain("local readiness"); + expect(projected.message).not.toContain( + "No analysis provider is configured", + ); + expect(projected.details).toMatchObject({ + requested_provider_id: "alpha", + candidate_ids: ["alpha", "beta"], + rejections: [ + { + provider_id: "alpha", + origin: "availability", + code: "open_options_invalid", + reason: "Configured registration is invalid", + diagnostics: { registration_path: "/configured/alpha.json" }, + }, + ], + }); + expect((await registry.select(DATABASE_TARGET, "beta")).ok).toBe(true); + expect(invalid.created).toEqual([]); + }); + + it("names whether a rejection came from provider availability or target support", async () => { + const availabilityOnly = new AnalysisProviderRegistry([ + candidate("alpha", { available: false, unconfigured: true }).provider, + candidate("gamma", { available: false, unconfigured: true }).provider, + ]); + const availabilitySelection = + await availabilityOnly.select(DATABASE_TARGET); + expect(availabilitySelection.ok).toBe(true); + if (availabilitySelection.ok) { + const { details } = projectAnalysisError( + availabilityOnly.unboundOperationError( + "binary_overview", + availabilitySelection.value, + ), + ); + const rejections = details?.rejections as unknown as readonly { + origin: string; + }[]; + expect(rejections.map(({ origin }) => origin)).toEqual([ + "availability", + "availability", + ]); + } + + const unconfiguredButAdmitting: AnalysisProviderCandidate = { + ...candidate("alpha", { available: false, unconfigured: true }).provider, + inspectTargetSupport: () => ({ + status: "supported", + code: null, + reason: null, + diagnostics: { target_kind: "archive" }, + }), + }; + const refusingTarget: AnalysisProviderCandidate = { + ...candidate("beta").provider, + inspectTargetSupport: () => ({ + status: "unsupported", + code: "target_format_unsupported", + reason: "beta refuses archives", + diagnostics: { target_kind: "archive" }, + }), + }; + const mixed = new AnalysisProviderRegistry([ + unconfiguredButAdmitting, + refusingTarget, + ]); + const mixedSelection = await mixed.select(ARTIFACT_TARGET); + expect(mixedSelection.ok).toBe(true); + if (mixedSelection.ok) { + const projected = projectAnalysisError( + mixed.unboundOperationError("binary_overview", mixedSelection.value), + ); + expect(projected.message).toContain( + "The selected analysis provider is unavailable", + ); + const { details } = projected; + const rejections = details?.rejections as unknown as readonly { + origin: string; + code: string; + }[]; + expect(rejections).toMatchObject([ + { origin: "availability", code: "not_configured" }, + { origin: "target", code: "target_format_unsupported" }, + ]); + } + }); +}); + describe("analysis provider registry: profile and cleanup failures", () => { it("does not bind a profile with unresolved version or adapter failure", async () => { const unresolved = candidate("unresolved", { profile: "missing" }); diff --git a/src/application/binary/AnalysisProviderRegistry.ts b/src/application/binary/AnalysisProviderRegistry.ts index 752f992116..e07ce2820e 100644 --- a/src/application/binary/AnalysisProviderRegistry.ts +++ b/src/application/binary/AnalysisProviderRegistry.ts @@ -21,6 +21,7 @@ import { import { AnalysisError } from "../../domain/analysisErrorBase.js"; import { ProviderSelectionError, + type ProviderRejectionOrigin, type ProviderSelectionErrorOptions, type ProviderSelectionFailureReason, type ProviderSelectionRejection, @@ -880,21 +881,43 @@ const selectionError = ( const candidateRejections = ( candidates: readonly AnalysisProviderCandidateStatus[], ): readonly ProviderSelectionRejection[] => - candidates.flatMap((candidate) => - [ - ...(candidate.availability.status === "unavailable" - ? [candidate.availability] - : []), - ...(candidate.targetSupport.status === "unsupported" - ? [candidate.targetSupport] - : []), - ].map((rejection) => ({ - providerId: candidate.provider.id, - code: rejection.code, - reason: rejection.reason, - diagnostics: structuredClone(rejection.diagnostics), - })), - ); + candidates.flatMap((candidate) => { + const rejections: ProviderSelectionRejection[] = []; + if (candidate.availability.status === "unavailable") + rejections.push( + selectedRejection(candidate, "availability", candidate.availability), + ); + if (candidate.targetSupport.status === "unsupported") + rejections.push( + selectedRejection(candidate, "target", candidate.targetSupport), + ); + return rejections; + }); + +/** One availability or target-support observation that rejected a candidate. */ +type RejectionObservation = + | Extract< + AnalysisProviderCandidateStatus["availability"], + { status: "unavailable" } + > + | Extract< + AnalysisProviderCandidateStatus["targetSupport"], + { status: "unsupported" } + >; + +/** Name the observation a rejection came from so availability cannot read as a refusal. */ +const selectedRejection = ( + candidate: AnalysisProviderCandidateStatus, + origin: ProviderRejectionOrigin, + rejection: RejectionObservation, +): ProviderSelectionRejection => ({ + providerId: candidate.provider.id, + origin, + code: rejection.code, + reason: rejection.reason, + // ProviderSelectionError owns the single defensive diagnostic copy. + diagnostics: rejection.diagnostics, +}); const cloneAvailability = ( availability: AnalysisProviderCandidateStatus["availability"], diff --git a/src/domain/analysisErrorProjection.ts b/src/domain/analysisErrorProjection.ts index a852e2cd66..369318b374 100644 --- a/src/domain/analysisErrorProjection.ts +++ b/src/domain/analysisErrorProjection.ts @@ -448,6 +448,7 @@ const providerErrorDetails = ( candidate_ids: [...error.candidateIds], rejections: error.rejections.map((rejection) => ({ provider_id: rejection.providerId, + origin: rejection.origin, code: rejection.code, reason: rejection.reason, diagnostics: rejection.diagnostics, diff --git a/src/domain/providerSelectionError.ts b/src/domain/providerSelectionError.ts index e53f6ae685..1ce240946b 100644 --- a/src/domain/providerSelectionError.ts +++ b/src/domain/providerSelectionError.ts @@ -13,9 +13,13 @@ export type ProviderSelectionFailureReason = | "ambiguous" | "invalid_options"; +/** Which observation produced a rejection; availability says nothing about the target. */ +export type ProviderRejectionOrigin = "availability" | "target"; + /** One actionable candidate rejection retained in caller diagnostics. */ export interface ProviderSelectionRejection { readonly providerId: string; + readonly origin: ProviderRejectionOrigin; readonly code: string; readonly reason: string; readonly diagnostics: Readonly>; @@ -80,6 +84,14 @@ const providerSelectionDiagnostic = ( return `Analysis provider selection failed (${options.reason}) for ${options.requestedProviderId}; candidates: ${candidates}`; }; +/** True when every recorded rejection describes availability, not target support. */ +const providerAvailabilityOnly = ( + rejections: readonly ProviderSelectionRejection[] | undefined, +): boolean => + rejections !== undefined && + rejections.length > 0 && + rejections.every(({ origin }) => origin === "availability"); + const providerSelectionUserMessage = ( options: ProviderSelectionErrorOptions, ): string => { @@ -89,7 +101,9 @@ const providerSelectionUserMessage = ( case "unknown_provider": return `The requested analysis provider is unknown. Choose one of: ${options.candidateIds.join(", ") || "no configured providers"}.`; case "provider_unavailable": - return "The selected analysis provider is unavailable. Review the reported local diagnostics or run `rea doctor`, then retry."; + return providerAvailabilityOnly(options.rejections) + ? "Provider selection failed local readiness checks. No target-support refusal was reported. Review the local provider diagnostics or run `rea doctor`, then retry." + : "The selected analysis provider is unavailable. Review the reported local diagnostics or run `rea doctor`, then retry."; case "target_unsupported": return "The selected analysis provider does not support this target. Choose another provider or open a supported extracted binary."; case "invalid_options": diff --git a/src/native/pwntools/PwntoolsFailures.ts b/src/native/pwntools/PwntoolsFailures.ts index 82cb960c7f..1197d1d913 100644 --- a/src/native/pwntools/PwntoolsFailures.ts +++ b/src/native/pwntools/PwntoolsFailures.ts @@ -343,6 +343,7 @@ export const pwntoolsUnavailable = ( rejections: [ { providerId: context.providerId, + origin: "availability", code: "provider_unavailable", reason, diagnostics: {