From 4aefe2daec12bbf7e9e4ed7e994d92ac245dea28 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Sun, 15 Nov 2026 18:09:40 +0800 Subject: [PATCH] fix(errors): name rea doctor only for providers it can check (#10565) * fix(errors): name rea doctor only for providers it can check An unclassified ProviderAdapterError, an unreadable result and the final presentation fallback all told the caller to run rea doctor, although doctor checks only hopper, ghidra, ida, jadx, kuna and desktop_build. The doctor scope list moves into the domain so presentation can consult it: covered providers get `rea doctor --provider ID`, others are told plainly that the failure is unclassified and where its reason is reported. Producers whose actionable reason lived only in details.diagnostics now author it: Ghidra's committed-profile conflicts share one owner that says to reopen the target, closed analysis clients share one owner, and a jadx-reported request failure no longer points at provider readiness. The deep-analysis profile rejection names doctor only for doctor-checkable providers. Dead presentation branches for tags handled earlier are removed. * fix(errors): make provider recovery actionable * test(android): remove brittle recovery wording check * test: wait for observed child exit before cancellation * test(errors): check that doctor is not offered instead of matching recovery prose --- docs/user-visible-errors.csv | 8 +- src/android/JadxSession.ts | 7 + src/application/DesktopBuildDoctor.ts | 2 +- src/application/DoctorDiagnostics.ts | 2 +- src/application/DoctorScope.ts | 2 +- src/application/Setup.ts | 2 +- src/application/SetupDesktopBuild.ts | 2 +- .../binary/AnalysisProviderEvaluation.ts | 18 ++- .../binary/AnalysisProviderRegistry.test.ts | 5 +- src/application/binary/CompositeProvider.ts | 11 +- src/cli/setupCommands.ts | 2 +- src/doctorRuntime.test.ts | 2 +- src/domain/analysisErrorPresentation.ts | 38 ++--- src/domain/analysisErrorProjection.test.ts | 55 +++++++ .../doctorProviderIds.ts} | 12 ++ src/domain/providerAdapterError.ts | 12 ++ src/ghidra/GhidraProvider.behavior.test.ts | 8 + src/ghidra/GhidraProviderClient.ts | 148 ++++++++---------- src/javascript/recovery/WakaruReport.test.ts | 8 +- src/providers/AnalysisClientCreation.ts | 11 +- src/providers/DeferredAnalysisClient.ts | 11 +- .../enhancedTools.callPathsAndBatches.test.ts | 4 +- .../mcp/nativeUiPartialCallRoutesMcp.test.ts | 4 +- .../boundary/process/jadxIntegration.test.ts | 26 +++ 24 files changed, 257 insertions(+), 143 deletions(-) rename src/{application/DoctorProviderIds.ts => domain/doctorProviderIds.ts} (56%) diff --git a/docs/user-visible-errors.csv b/docs/user-visible-errors.csv index 2583d15152..53b44d301b 100644 --- a/docs/user-visible-errors.csv +++ b/docs/user-visible-errors.csv @@ -3,11 +3,11 @@ harm_rank,location,trigger,current_copy,user_risk,proposed_replacement,implement 5,src/domain/native/bytePattern.ts,Unequal mask length,Mask length must equal pattern length,Low risk; identifies input correction,Keep current copy,implemented,parser and MCP validation tests 5,src/ghidra/GhidraByteSearchCursorFailure.ts:ghidraByteSearchCursorMismatch,Byte-search cursor does not match the current binding,"Byte search cursor does not match this search. It is bound to the target, analysis profile (including a selected Ghidra project), pattern, mask, bounds, and document, and does not identify which of those differs.",Previous copy named only the target and query and directed schema repair for a schema-valid cursor,"Name the bound values including the analysis profile, state that the differing component is unknown, attribute the issue to cursor, and omit recovery.kind=inspect_input_contract",implemented,provider projection test; real Ghidra not run 1,src/domain/errors.ts:AnalysisCapabilityUnavailableError,Requested analysis unavailable,"Provider {providerId} cannot execute {operation}: {reason}",Exposes provider and internal operation/reason,"This analysis is unavailable for the current target. Choose another analysis or target.",implemented,focused assertions passed; full `npm run check` passed -2,src/domain/errors.ts:ProviderAdapterError,Provider adapter fails,"Provider {providerId} adapter failed during {operation}",Exposes provider and implementation boundary,"Analysis could not complete. Run `rea doctor` and review the reported details; repeating the unchanged request is not expected to succeed.",implemented,focused assertions passed; full `npm run check` passed +2,src/domain/errors.ts:ProviderAdapterError,Provider adapter fails,"Provider {providerId} adapter failed during {operation}",Exposes provider and implementation boundary,"The {providerId} provider could not complete {operation}, and REA did not classify the failure. Any reason the provider reported is in details.diagnostics. Recovery names `rea doctor --provider {providerId}` only when doctor can check that provider (hopper, ghidra, ida, jadx, kuna); otherwise it states that no specific recovery is known.",implemented,focused assertions passed; full `npm run check` passed 3,src/domain/errors.ts:HopperRemoteError,Remote analysis request fails,"Hopper request failed ({code}): {safeMessage}",Exposes provider name code and upstream text,"Analysis could not complete. Run `rea doctor` and review the reported details; repeating the unchanged request is not expected to succeed.",implemented,focused assertions passed; full `npm run check` passed -4,src/domain/errors.ts:HopperProtocolError,Malformed provider protocol result,Raw constructor message,Can expose provider protocol and implementation details,"Analysis returned an unreadable result. Run `rea doctor` and review the reported details; repeating the unchanged request is not expected to succeed.",implemented,focused assertions passed; full `npm run check` passed -5,src/domain/errors.ts:AnalysisOutputError,Analysis output fails parsing,"Invalid analysis output for {operation}: {reason}",Exposes internal operation and parser reason,"Analysis returned an unreadable result. Run `rea doctor` and review the reported details; repeating the unchanged request is not expected to succeed.",implemented,focused assertions passed; full `npm run check` passed -6,src/domain/errors.ts:AnalysisProtocolError,Application protocol fails,Raw constructor message,Can expose protocol internals,"Analysis returned an unreadable result. Run `rea doctor` and review the reported details; repeating the unchanged request is not expected to succeed.",implemented,focused assertions passed; full `npm run check` passed +4,src/domain/errors.ts:HopperProtocolError,Malformed provider protocol result,Raw constructor message,Can expose provider protocol and implementation details,"Analysis returned a result REA could not read. Review the reported details; repeating the unchanged request is not expected to succeed.",implemented,focused assertions passed; full `npm run check` passed +5,src/domain/errors.ts:AnalysisOutputError,Analysis output fails parsing,"Invalid analysis output for {operation}: {reason}",Exposes internal operation and parser reason,"Analysis returned a result REA could not read. Review the reported details; repeating the unchanged request is not expected to succeed.",implemented,focused assertions passed; full `npm run check` passed +6,src/domain/errors.ts:AnalysisProtocolError,Application protocol fails,Raw constructor message,Can expose protocol internals,"Analysis returned a result REA could not read. Review the reported details; repeating the unchanged request is not expected to succeed.",implemented,focused assertions passed; full `npm run check` passed 7,src/domain/errors.ts:HopperProcessError,Analysis subprocess stops,"Hopper bridge stopped unexpectedly with code {exitCode}",Exposes provider bridge and process code,"Analysis could not start or stopped unexpectedly. Run `rea doctor`, then try again.",implemented,focused assertions passed; full `npm run check` passed 8,src/domain/errors.ts:HopperStartError,Analysis subprocess cannot start,Hopper application bridge could not be started,Exposes provider and bridge implementation,"Analysis could not start or stopped unexpectedly. Run `rea doctor`, then try again.",implemented,focused assertions passed; full `npm run check` passed 9,src/domain/errors.ts:BinaryTargetError,Target cannot open,"Cannot open artifact: {reason}",May expose raw reason and lacks checks,"Preserve the target admission reason; directories name analyze_javascript_application and rea analyze as available alternatives.",implemented,focused assertions passed; full `npm run check` passed diff --git a/src/android/JadxSession.ts b/src/android/JadxSession.ts index 07e8e902bb..33903b416d 100644 --- a/src/android/JadxSession.ts +++ b/src/android/JadxSession.ts @@ -52,6 +52,9 @@ import { JADX_HEAP_REMEDIATION, } from "./JadxRelease.js"; +const JADX_REPORTED_FAILURE_REMEDIATION = + "jadx is running and reported this failure for the selected input, so provider readiness checks cannot repair it. Review details.diagnostics.reason and adjust the input or request if applicable. Repeating the unchanged request is not expected to succeed."; + type JadxCall = { readonly operation: string; readonly input: Readonly>; @@ -431,7 +434,11 @@ export class JadxSession { }, ]); } + // jadx answered, so its own reply, not provider readiness, explains + // the failure; doctor cannot repair a reported per-request error. throw new ProviderAdapterError("jadx", request.operation, { + userMessage: `jadx reported an error for ${name} that REA did not classify; its reason is in details.diagnostics.reason.`, + remediationAction: JADX_REPORTED_FAILURE_REMEDIATION, diagnostics: { upstream_operation: name, reason: text, diff --git a/src/application/DesktopBuildDoctor.ts b/src/application/DesktopBuildDoctor.ts index 272f2f2334..69fe18c102 100644 --- a/src/application/DesktopBuildDoctor.ts +++ b/src/application/DesktopBuildDoctor.ts @@ -4,7 +4,7 @@ import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.j import type { ClientRegistrationStatus } from "./ClientRegistrationStatus.js"; import type { DoctorCheck, DoctorHost, DoctorSkillState } from "./Doctor.js"; import { nodeCheck } from "./DoctorDiagnostics.js"; -import { DESKTOP_BUILD_PROVIDER_ID } from "./DoctorProviderIds.js"; +import { DESKTOP_BUILD_PROVIDER_ID } from "../domain/doctorProviderIds.js"; import type { DoctorScopeReport } from "./DoctorScope.js"; /** Free space agentenv's images need where Docker keeps them. */ diff --git a/src/application/DoctorDiagnostics.ts b/src/application/DoctorDiagnostics.ts index fe6595149a..c0897590ce 100644 --- a/src/application/DoctorDiagnostics.ts +++ b/src/application/DoctorDiagnostics.ts @@ -6,7 +6,7 @@ import { import { hopperLauncherDiscovery } from "./HopperLauncherDiscovery.js"; import { macosMajorVersion } from "../domain/macosVersion.js"; import { LINUX_HOPPER_SETUP_HOSTS, supportsReaHost } from "./LinuxHopper.js"; -import { isDesktopBuildCheck } from "./DoctorProviderIds.js"; +import { isDesktopBuildCheck } from "../domain/doctorProviderIds.js"; import type { DoctorCheck, DoctorHost, diff --git a/src/application/DoctorScope.ts b/src/application/DoctorScope.ts index 9b5c204caa..eb0a85eacf 100644 --- a/src/application/DoctorScope.ts +++ b/src/application/DoctorScope.ts @@ -9,7 +9,7 @@ import { isDesktopBuildInformationalCheck } from "./DesktopBuildDoctor.js"; import { DESKTOP_BUILD_PROVIDER_ID, isDesktopBuildCheck, -} from "./DoctorProviderIds.js"; +} from "../domain/doctorProviderIds.js"; import type { DoctorCheck, DoctorProviderInspection, diff --git a/src/application/Setup.ts b/src/application/Setup.ts index 9c4ffdcf5f..7e918af3f9 100644 --- a/src/application/Setup.ts +++ b/src/application/Setup.ts @@ -41,7 +41,7 @@ import type { SkillInstallResult, } from "./SetupSkill.js"; import { planDesktopBuild } from "./SetupDesktopBuild.js"; -import { DESKTOP_BUILD_PROVIDER_ID } from "./DoctorProviderIds.js"; +import { DESKTOP_BUILD_PROVIDER_ID } from "../domain/doctorProviderIds.js"; import type { SetupHopperInstallFailure, SetupHopperInstallResult, diff --git a/src/application/SetupDesktopBuild.ts b/src/application/SetupDesktopBuild.ts index 47b4948a3c..d580fbc90a 100644 --- a/src/application/SetupDesktopBuild.ts +++ b/src/application/SetupDesktopBuild.ts @@ -1,6 +1,6 @@ import { DESKTOP_EXPORT_FRAMEWORK_VERSION } from "../domain/desktop/appExport.js"; import { AGENTENV_INSTALL_COMMAND } from "./DesktopBuildDoctor.js"; -import { DESKTOP_BUILD_PROVIDER_ID } from "./DoctorProviderIds.js"; +import { DESKTOP_BUILD_PROVIDER_ID } from "../domain/doctorProviderIds.js"; import { assertSetupActive } from "./SetupCancellation.js"; import type { SetupAction, diff --git a/src/application/binary/AnalysisProviderEvaluation.ts b/src/application/binary/AnalysisProviderEvaluation.ts index 63ac195ff5..ec150e5e59 100644 --- a/src/application/binary/AnalysisProviderEvaluation.ts +++ b/src/application/binary/AnalysisProviderEvaluation.ts @@ -18,6 +18,7 @@ import type { import type { AnalysisProviderCandidateStatus } from "./AnalysisProviderRegistry.js"; import { withCandidateRejections } from "./AnalysisProviderCandidateDiagnostics.js"; import { ProviderAdapterError } from "../../domain/providerAdapterError.js"; +import { isDoctorProviderId } from "../../domain/doctorProviderIds.js"; import { providerReleaseVerificationSchema, type ProviderReleaseVerification, @@ -105,7 +106,7 @@ export const evaluateAnalysisProviderCandidate = async ( return ok( rejectedProfile(candidate, status, { reason: "Provider version or analysis profile could not be resolved", - remediation: PROFILE_REMEDIATION, + remediation: profileRemediation(candidate), }), ); const profile = analysisProfileSchema.parse(resolved.value.profile); @@ -119,7 +120,7 @@ export const evaluateAnalysisProviderCandidate = async ( return ok( rejectedProfile(candidate, status, { reason: "Resolved analysis profile has mismatched provider identity", - remediation: PROFILE_REMEDIATION, + remediation: profileRemediation(candidate), }), ); const verification = @@ -132,7 +133,7 @@ export const evaluateAnalysisProviderCandidate = async ( return ok( rejectedProfile(candidate, status, { reason: "Provider returned invalid release verification", - remediation: PROFILE_REMEDIATION, + remediation: profileRemediation(candidate), }), ); return ok({ @@ -157,7 +158,7 @@ export const evaluateAnalysisProviderCandidate = async ( return ok( rejectedProfile(candidate, status, { reason: "Provider returned an invalid analysis profile", - remediation: PROFILE_REMEDIATION, + remediation: profileRemediation(candidate), }), ); } @@ -166,8 +167,13 @@ export const evaluateAnalysisProviderCandidate = async ( const signalIsAborted = (signal: AbortSignal | undefined): boolean => signal?.aborted === true; -const PROFILE_REMEDIATION = - "Run rea doctor to check this provider's installation and version, then reopen the target, or select another analysis provider."; +/** Name doctor only for a provider whose installation it can check. */ +const profileRemediation = (candidate: AnalysisProviderCandidate): string => { + const providerId = candidate.identity().id; + return isDoctorProviderId(providerId) + ? `Run rea doctor --provider ${providerId} to check this provider's installation and version, then reopen the target, or select another analysis provider.` + : `Check the ${providerId} provider's installation and version against its documented requirements, then reopen the target, or select another analysis provider.`; +}; const rejectedProfile = ( candidate: AnalysisProviderCandidate, diff --git a/src/application/binary/AnalysisProviderRegistry.test.ts b/src/application/binary/AnalysisProviderRegistry.test.ts index 9688a9264f..16ea27941d 100644 --- a/src/application/binary/AnalysisProviderRegistry.test.ts +++ b/src/application/binary/AnalysisProviderRegistry.test.ts @@ -669,7 +669,10 @@ describe("analysis provider registry: rejection diagnostics", () => { providerId: "alpha", origin: "availability", code: "version_unresolved", - remediation: expect.stringContaining("rea doctor") as unknown, + // doctor cannot check this provider, so it is not offered. + remediation: expect.not.stringContaining( + "rea doctor", + ) as unknown, }, ], }, diff --git a/src/application/binary/CompositeProvider.ts b/src/application/binary/CompositeProvider.ts index a3572024b9..38098fbaca 100644 --- a/src/application/binary/CompositeProvider.ts +++ b/src/application/binary/CompositeProvider.ts @@ -1,6 +1,9 @@ import { AnalysisCapabilityUnavailableError } from "../../domain/analysisErrorCore.js"; import { ProviderCleanupError } from "../../domain/providerCleanupError.js"; -import { ProviderAdapterError } from "../../domain/providerAdapterError.js"; +import { + closedAnalysisClientError, + ProviderAdapterError, +} from "../../domain/providerAdapterError.js"; import { projectAnalysisError } from "../../domain/analysisErrorProjection.js"; import { err, ok } from "../../domain/result.js"; import type { BinaryTarget } from "../../domain/binaryTargetTypes.js"; @@ -217,11 +220,7 @@ export class CompositeProvider implements AnalysisProvider { }, execute: async (operation, parameters, options) => { if (closed) - return err( - new ProviderAdapterError(this.#identity.id, operation, { - diagnostics: { reason: "client_closed" }, - }), - ); + return err(closedAnalysisClientError(this.#identity.id, operation)); if (operation === "health") return executeCompositeHealth(this.#identity, clients, options); const provider = this.#providerByOperation.get(operation); diff --git a/src/cli/setupCommands.ts b/src/cli/setupCommands.ts index edcd99951e..1eae3a7bc5 100644 --- a/src/cli/setupCommands.ts +++ b/src/cli/setupCommands.ts @@ -5,7 +5,7 @@ import type { DoctorScope } from "../application/Doctor.js"; import { DESKTOP_BUILD_PROVIDER_ID, DOCTOR_PROVIDER_IDS, -} from "../application/DoctorProviderIds.js"; +} from "../domain/doctorProviderIds.js"; import { isSetupFailure, type SetupOptions, diff --git a/src/doctorRuntime.test.ts b/src/doctorRuntime.test.ts index 5e088eb859..f5fa317959 100644 --- a/src/doctorRuntime.test.ts +++ b/src/doctorRuntime.test.ts @@ -10,7 +10,7 @@ import { selectedProviderSettings, settingAffectsProbe, } from "./doctorRuntime.js"; -import { DOCTOR_PROVIDER_IDS } from "./application/DoctorProviderIds.js"; +import { DOCTOR_PROVIDER_IDS } from "./domain/doctorProviderIds.js"; import { providerRegistrationEnvironment } from "./application/SetupRegistrationEnvironment.js"; import { createTestTempDirectory } from "../tests/fixtures/temporaryDirectory.js"; import { writeJadxJarInventory } from "../tests/fixtures/android/jadxJar.js"; diff --git a/src/domain/analysisErrorPresentation.ts b/src/domain/analysisErrorPresentation.ts index cd1c8a96a0..b0bb439ff8 100644 --- a/src/domain/analysisErrorPresentation.ts +++ b/src/domain/analysisErrorPresentation.ts @@ -35,6 +35,7 @@ import { } from "./hopperErrors.js"; import { ProviderSelectionError } from "./providerSelectionError.js"; import { ProviderAdapterError } from "./providerAdapterError.js"; +import { isDoctorProviderId } from "./doctorProviderIds.js"; import { UnknownRegistryError } from "./unknownRegistryError.js"; import { providerRetryAction } from "./providerOperationHealth.js"; import { analysisErrorRefusal } from "./analysisErrorRefusal.js"; @@ -94,6 +95,8 @@ export const analysisErrorRemediationAction = ( error.remediationAction !== undefined ) return error.remediationAction; + if (error instanceof ProviderAdapterError && error.userMessage === undefined) + return unclassifiedProviderRemediation(error.providerId); if ( error instanceof BrowserObservationError && error.remediationAction !== undefined @@ -552,6 +555,8 @@ const outcomeUserMessage = (error: AnalysisError): string => { if (error.userMessage !== undefined) return error.userMessage; if (error instanceof AnalysisCapabilityUnavailableError && error.retryable) return "Required runtime controls are unavailable for this attempt."; + if (error instanceof ProviderAdapterError) + return `The ${error.providerId} provider could not complete ${error.operation}, and REA did not classify the failure. Any reason the provider reported is in details.diagnostics.`; const standardMessage = standardErrorMessage(error._tag); if (standardMessage !== undefined) return standardMessage; if (error instanceof ArtifactOperationError) return artifactMessage(error); @@ -600,11 +605,20 @@ const outcomeUserMessage = (error: AnalysisError): string => { if (error._tag === "ProcessCaptureError") return ( error.userMessage ?? - "Process capture could not complete. Run `rea doctor`, then review capture policy and try again." + "Process capture could not complete. Review the reported details and capture policy, then try again." ); - return "Analysis could not complete. Run `rea doctor`, then try again."; + return "Analysis could not complete, and REA did not classify the failure. Review the reported details."; }; +/** + * Recovery for a provider failure no typed variant classified. Doctor is + * named only for providers whose readiness it can check. + */ +const unclassifiedProviderRemediation = (providerId: string): string => + isDoctorProviderId(providerId) + ? `Review the reported details. If the ${providerId} installation or configuration may be at fault, run \`rea doctor --provider ${providerId}\` and apply its fixes. Repeating the unchanged request is not expected to succeed.` + : "Review the reported details for the provider's reason; REA has no specific recovery for this unclassified failure. Repeating the unchanged request is not expected to succeed."; + const hopperErrorUserMessage = (error: AnalysisError): string | undefined => { if (error instanceof HopperTimeoutError) { if (error.operation === undefined) @@ -691,17 +705,11 @@ const hopperProcessMessage = (error: HopperProcessError): string => { const standardErrorMessage = (tag: AnalysisErrorTag): string | undefined => { if (UNREADABLE_OUTPUT_TAGS.has(tag)) - return "Analysis returned an unreadable result. Run `rea doctor` and review the reported details; repeating the unchanged request is not expected to succeed."; + return "Analysis returned a result REA could not read. Review the reported details; repeating the unchanged request is not expected to succeed."; if (UNSUPPORTED_PROVIDER_TAGS.has(tag)) return "This analysis is unavailable for the current target. Choose another analysis or target."; if (CANCELLED_TAGS.has(tag)) return "Analysis was cancelled. Start it again when ready."; - if (TIMEOUT_TAGS.has(tag)) - return "Analysis took too long. Try a smaller request, then run `rea doctor` if it continues."; - if (ADAPTER_FAILURE_TAGS.has(tag)) - return "Analysis could not complete. Run `rea doctor` and review the reported details; repeating the unchanged request is not expected to succeed."; - if (START_FAILURE_TAGS.has(tag)) - return "Analysis could not start or stopped unexpectedly. Run `rea doctor`, then try again."; return undefined; }; @@ -724,18 +732,6 @@ const CANCELLED_TAGS: ReadonlySet = new Set([ "AnalysisCancelledError", "HopperCancelledError", ]); -const TIMEOUT_TAGS: ReadonlySet = new Set([ - "AnalysisTimeoutError", - "HopperTimeoutError", -]); -const ADAPTER_FAILURE_TAGS: ReadonlySet = new Set([ - "ProviderAdapterError", - "HopperRemoteError", -]); -const START_FAILURE_TAGS: ReadonlySet = new Set([ - "HopperProcessError", - "HopperStartError", -]); /** Operations that accept integrity_policy for declared-integrity mismatches. */ const INTEGRITY_POLICY_OPERATIONS: ReadonlySet< diff --git a/src/domain/analysisErrorProjection.test.ts b/src/domain/analysisErrorProjection.test.ts index d94a78c554..cc862d803f 100644 --- a/src/domain/analysisErrorProjection.test.ts +++ b/src/domain/analysisErrorProjection.test.ts @@ -1038,6 +1038,61 @@ describe("analysis error projection: Hopper startup failures", () => { }); }); +describe("analysis error projection: unclassified failures", () => { + it.each(["hopper", "ghidra", "ida", "jadx", "kuna"])( + "names doctor only with the provider it can check (%s)", + (providerId) => { + const projected = projectAnalysisError( + new ProviderAdapterError(providerId, "open_binary"), + ); + + expect(projected.code).toBe("execution_failure"); + expect(projected.message).toContain( + `${providerId} provider could not complete open_binary, and REA did not classify the failure`, + ); + expect(projected.message).not.toContain("doctor"); + expect(projected.remediation.action).toContain( + `\`rea doctor --provider ${providerId}\``, + ); + }, + ); + + it.each(["firmware", "native-macos", "rizin", "rea-cdp-browser", "rea"])( + "does not send %s failures to a doctor that cannot check them", + (providerId) => { + const projected = projectAnalysisError( + new ProviderAdapterError(providerId, "inspect"), + ); + + expect(projected.message).toContain("did not classify the failure"); + for (const text of [projected.message, projected.remediation.action]) + expect(text).not.toContain("doctor"); + }, + ); + + it("keeps an authored provider message as the recovery", () => { + const projected = projectAnalysisError( + new ProviderAdapterError("firmware", "inspect", { + userMessage: "Select a smaller firmware image.", + }), + ); + + expect(projected.remediation.action).toBe( + "Select a smaller firmware image.", + ); + }); + + it("does not attribute an unreadable result to provider readiness", () => { + const projected = projectAnalysisError( + new AnalysisOutputError("decompile", "failed"), + ); + + expect(projected.code).toBe("unreadable_output"); + for (const text of [projected.message, projected.remediation.action]) + expect(text).not.toContain("doctor"); + }); +}); + describe("analysis error projection: retry guidance", () => { it.each([ [ diff --git a/src/application/DoctorProviderIds.ts b/src/domain/doctorProviderIds.ts similarity index 56% rename from src/application/DoctorProviderIds.ts rename to src/domain/doctorProviderIds.ts index 380e4353ee..cfbfaaeaaf 100644 --- a/src/application/DoctorProviderIds.ts +++ b/src/domain/doctorProviderIds.ts @@ -11,6 +11,18 @@ export const DOCTOR_PROVIDER_IDS = [ DESKTOP_BUILD_PROVIDER_ID, ] as const; +/** A provider id `rea doctor --provider` accepts. */ +export type DoctorProviderId = (typeof DOCTOR_PROVIDER_IDS)[number]; + +const DOCTOR_PROVIDER_ID_SET: ReadonlySet = new Set( + DOCTOR_PROVIDER_IDS, +); + +/** Whether `rea doctor --provider` can check this provider's readiness. */ +export const isDoctorProviderId = ( + providerId: string, +): providerId is DoctorProviderId => DOCTOR_PROVIDER_ID_SET.has(providerId); + /** Whether a check belongs to the desktop build scope, which decides only its own health. */ export const isDesktopBuildCheck = (name: string): boolean => name.startsWith(`${DESKTOP_BUILD_PROVIDER_ID}-`); diff --git a/src/domain/providerAdapterError.ts b/src/domain/providerAdapterError.ts index 966181195f..832d34b48a 100644 --- a/src/domain/providerAdapterError.ts +++ b/src/domain/providerAdapterError.ts @@ -25,3 +25,15 @@ export class ProviderAdapterError extends AnalysisError { this.remediationAction = options.remediationAction; } } + +/** A request reached an analysis client after its session closed that client. */ +export const closedAnalysisClientError = ( + providerId: string, + operation: string, +): ProviderAdapterError => + new ProviderAdapterError(providerId, operation, { + userMessage: `The ${providerId} analysis client for this session was closed before ${operation} ran.`, + remediationAction: + "Open the target again with open_binary, or rerun the CLI command, then send the request to the new session.", + diagnostics: { reason: "client_closed" }, + }); diff --git a/src/ghidra/GhidraProvider.behavior.test.ts b/src/ghidra/GhidraProvider.behavior.test.ts index 8d8409d223..67b8a974e0 100644 --- a/src/ghidra/GhidraProvider.behavior.test.ts +++ b/src/ghidra/GhidraProvider.behavior.test.ts @@ -6586,6 +6586,14 @@ it("refuses a changed installation before creating a target-bound native client" current_installation: { provider_version: "12.1.5" }, }, }); + // The actionable reason reaches the caller, not only details. + const projected = projectAnalysisError(result.error); + expect(projected.message).toContain( + "installation version differs from the selected analysis profile", + ); + expect(projected.remediation.action).toContain("Reopen the target"); + expect(projected.remediation.action).toContain("close_binary"); + expect(projected.remediation.action).not.toContain("doctor"); expect(client.reuseReadiness?.()).toBe("blocked"); expect(nativeCreates).toBe(0); } finally { diff --git a/src/ghidra/GhidraProviderClient.ts b/src/ghidra/GhidraProviderClient.ts index 5fafe59764..54fec833aa 100644 --- a/src/ghidra/GhidraProviderClient.ts +++ b/src/ghidra/GhidraProviderClient.ts @@ -302,6 +302,24 @@ const emptyReferenceCaveat = ( return []; }; +const GHIDRA_PROFILE_CONFLICT_REMEDIATION = + "Reopen the target with open_binary after calling close_binary and resolving any cleanup failure, or rerun the CLI command, so REA resolves a fresh Ghidra analysis profile from the current configuration and target. Repeating the request against the committed profile is not expected to succeed."; + +/** + * The committed analysis profile no longer matches the configuration, + * installation or target this session would open; only a fresh profile + * resolution can reconcile them. + */ +const ghidraProfileConflict = ( + operation: string, + reason: string, + diagnostics: Readonly> = {}, +): ProviderAdapterError => + new ProviderAdapterError("ghidra", operation, { + userMessage: `The committed Ghidra analysis profile does not match the current configuration or target. ${reason}`, + remediationAction: GHIDRA_PROFILE_CONFLICT_REMEDIATION, + diagnostics: { reason, ...diagnostics }, + }); /** * Caveat for a containing-procedure lookup in mapped memory that found no * function: the function inventory lacks the address, which does not establish @@ -346,12 +364,10 @@ export const createGhidraProviderClient = (input: { (checkpoint.data !== undefined) ) return err( - new ProviderAdapterError("ghidra", "open_binary", { - diagnostics: { - reason: - "Resolve REA_GHIDRA_CHECKPOINT_DIR into the analysis profile before opening the session.", - }, - }), + ghidraProfileConflict( + "open_binary", + "Resolve REA_GHIDRA_CHECKPOINT_DIR into the analysis profile before opening the session.", + ), ); const psp = isGhidraPspTarget(target); const pspExtension = psp @@ -366,12 +382,10 @@ export const createGhidraProviderClient = (input: { committedProfile.parameters.loader !== "PspElfLoader") ) return err( - new ProviderAdapterError("ghidra", "open_binary", { - diagnostics: { - reason: - "Resolve the supported PSP profile and installed Allegrex extension before opening the session.", - }, - }), + ghidraProfileConflict( + "open_binary", + "Resolve the supported PSP profile and installed Allegrex extension before opening the session.", + ), ); const softFloat = isGhidraMipsSoftFloatTarget(target); if ( @@ -380,12 +394,10 @@ export const createGhidraProviderClient = (input: { GHIDRA_MIPS_SOFT_FLOAT_STORAGE) ) return err( - new ProviderAdapterError("ghidra", "open_binary", { - diagnostics: { - reason: - "Resolve the soft-float MIPS storage policy into the analysis profile before opening the session.", - }, - }), + ghidraProfileConflict( + "open_binary", + "Resolve the soft-float MIPS storage policy into the analysis profile before opening the session.", + ), ); const riscvCallingConvention = ghidraRiscvCallingConvention(target); if ( @@ -393,12 +405,10 @@ export const createGhidraProviderClient = (input: { committedProfile.parameters.riscv_calling_convention ) return err( - new ProviderAdapterError("ghidra", "open_binary", { - diagnostics: { - reason: - "Resolve the RISC-V calling-convention policy into the analysis profile before opening the session.", - }, - }), + ghidraProfileConflict( + "open_binary", + "Resolve the RISC-V calling-convention policy into the analysis profile before opening the session.", + ), ); const extensionProfile = ghidraExtensionSchema .array() @@ -409,12 +419,10 @@ export const createGhidraProviderClient = (input: { extensionProfile.data.length === 0) ) return err( - new ProviderAdapterError("ghidra", "open_binary", { - diagnostics: { - reason: - "Resolve the configured Ghidra extensions into an analysis profile before opening the session.", - }, - }), + ghidraProfileConflict( + "open_binary", + "Resolve the configured Ghidra extensions into an analysis profile before opening the session.", + ), ); const extensions = extensionProfile.data; const projectProfile = ghidraProjectCommitmentSchema @@ -432,12 +440,10 @@ export const createGhidraProviderClient = (input: { projectProfile.data.program_name))) ) return err( - new ProviderAdapterError("ghidra", "open_binary", { - diagnostics: { - reason: - "Resolve the selected Ghidra project into a fresh analysis profile before opening the session.", - }, - }), + ghidraProfileConflict( + "open_binary", + "Resolve the selected Ghidra project into a fresh analysis profile before opening the session.", + ), ); const project = projectProfile.data; const projection: GhidraProjection = { @@ -458,12 +464,10 @@ export const createGhidraProviderClient = (input: { (config.ghidraSeedFile === undefined) !== (seedProfile.data === undefined) ) return err( - new ProviderAdapterError("ghidra", "open_binary", { - diagnostics: { - reason: - "Resolve REA_GHIDRA_SEED_FILE into an analysis profile before opening the session.", - }, - }), + ghidraProfileConflict( + "open_binary", + "Resolve REA_GHIDRA_SEED_FILE into an analysis profile before opening the session.", + ), ); const seeds = seedProfile.data; const parameterLanguage = ghidraProfileLanguageOverride( @@ -475,11 +479,10 @@ export const createGhidraProviderClient = (input: { const targetLanguage = ghidraPinnedLanguage(target); if (pinnedLanguage !== undefined && targetLanguage === undefined) return err( - new ProviderAdapterError("ghidra", "open_binary", { - diagnostics: { - reason: `The committed profile pins the ${pinnedLanguage.languageId} language, which the current target does not establish; reopen an admitted target to resolve a new profile.`, - }, - }), + ghidraProfileConflict( + "open_binary", + `The committed profile pins the ${pinnedLanguage.languageId} language, which the current target does not establish; reopen an admitted target to resolve a new profile.`, + ), ); // Provider-pinned profiles declare their language through the same // parameter slot caller-configured profiles use, so only an environment @@ -489,11 +492,10 @@ export const createGhidraProviderClient = (input: { config.ghidraLanguageOverride !== undefined ) return err( - new ProviderAdapterError("ghidra", "open_binary", { - diagnostics: { - reason: `The committed profile pins its import language, ${pinnedLanguage.languageId}; REA_GHIDRA_LANGUAGE_ID/REA_GHIDRA_COMPILER_SPEC_ID cannot be combined with it. Reopen the target without the override.`, - }, - }), + ghidraProfileConflict( + "open_binary", + `The committed profile pins its import language, ${pinnedLanguage.languageId}; REA_GHIDRA_LANGUAGE_ID/REA_GHIDRA_COMPILER_SPEC_ID cannot be combined with it. Reopen the target without the override.`, + ), ); if ( !sameGhidraLanguageOverride( @@ -502,12 +504,10 @@ export const createGhidraProviderClient = (input: { ) ) return err( - new ProviderAdapterError("ghidra", "open_binary", { - diagnostics: { - reason: - "The committed analysis profile and REA_GHIDRA_LANGUAGE_ID/REA_GHIDRA_COMPILER_SPEC_ID differ; reopen the target to resolve a new profile.", - }, - }), + ghidraProfileConflict( + "open_binary", + "The committed analysis profile and REA_GHIDRA_LANGUAGE_ID/REA_GHIDRA_COMPILER_SPEC_ID differ; reopen the target to resolve a new profile.", + ), ); // Both caller-selected and provider-pinned languages use the validated // committed slot; the profile cannot supply its own applicability proof. @@ -519,11 +519,7 @@ export const createGhidraProviderClient = (input: { "existing-project-private-copy-v1", ); if (mipsProfileMismatch !== null) - return err( - new ProviderAdapterError("ghidra", "open_binary", { - diagnostics: { reason: mipsProfileMismatch }, - }), - ); + return err(ghidraProfileConflict("open_binary", mipsProfileMismatch)); const importLanguage = parameterLanguage; const disabledAnalyzers = ghidraProfileDisabledAnalyzers( committedProfile.parameters, @@ -535,12 +531,10 @@ export const createGhidraProviderClient = (input: { ) ) return err( - new ProviderAdapterError("ghidra", "open_binary", { - diagnostics: { - reason: - "The committed analysis profile and REA_GHIDRA_DISABLED_ANALYZERS differ; reopen the target to resolve a new profile.", - }, - }), + ghidraProfileConflict( + "open_binary", + "The committed analysis profile and REA_GHIDRA_DISABLED_ANALYZERS differ; reopen the target to resolve a new profile.", + ), ); const invalidProfile = validateGhidraExtensionProfile( @@ -550,11 +544,7 @@ export const createGhidraProviderClient = (input: { installation.platform, ); if (invalidProfile !== null) - return err( - new ProviderAdapterError("ghidra", "open_binary", { - diagnostics: { reason: invalidProfile }, - }), - ); + return err(ghidraProfileConflict("open_binary", invalidProfile)); let extensionFailure: AnalysisError | undefined; // Replaced by the reported outcome once the session handshake is checked. const seedLimitations: string[] = [...ghidraSeedLimitations(seeds)]; @@ -1294,14 +1284,14 @@ const ghidraClientPrerequisites = ( ); if (profile.provider.version !== installation.providerVersion) return err( - new ProviderAdapterError("ghidra", "health", { - diagnostics: { - reason: - "The current installation version differs from the selected analysis profile; resolve the profile again.", + ghidraProfileConflict( + "health", + "The current installation version differs from the selected analysis profile; resolve the profile again.", + { selected_provider_version: profile.provider.version, current_installation: ghidraInstallationDiagnostics(installation), }, - }), + ), ); return ok({ analyzeHeadlessPath: installation.analyzeHeadlessPath, diff --git a/src/javascript/recovery/WakaruReport.test.ts b/src/javascript/recovery/WakaruReport.test.ts index be6d0c4287..473feef5fe 100644 --- a/src/javascript/recovery/WakaruReport.test.ts +++ b/src/javascript/recovery/WakaruReport.test.ts @@ -127,11 +127,13 @@ it("keeps a malformed provenance report distinct from the fixed report allowance if (!(refusal instanceof ProviderAdapterError)) throw new Error("Expected a wrapped Wakaru report refusal"); const projected = projectAnalysisError(refusal); - expect(projected.message).toMatch(/rea doctor/iu); - expect(projected.message).toMatch( + // doctor cannot check Wakaru, so the unclassified failure says so plainly. + expect(projected.message).toMatch(/did not classify the failure/iu); + expect(projected.remediation.action).toMatch( /unchanged request.*not expected to succeed/iu, ); - expect(projected.remediation.action).toBe(projected.message); + for (const text of [projected.message, projected.remediation.action]) + expect(text).not.toMatch(/doctor/iu); expect(JSON.stringify(projected)).not.toContain("report allowance"); expect(JSON.stringify(projected)).not.toContain("no runtime override"); }); diff --git a/src/providers/AnalysisClientCreation.ts b/src/providers/AnalysisClientCreation.ts index 6c87c321cf..32d77eaa3d 100644 --- a/src/providers/AnalysisClientCreation.ts +++ b/src/providers/AnalysisClientCreation.ts @@ -15,7 +15,10 @@ import { cancelledBy, carriedReason, } from "../domain/analysisErrorCore.js"; -import { ProviderAdapterError } from "../domain/providerAdapterError.js"; +import { + closedAnalysisClientError, + ProviderAdapterError, +} from "../domain/providerAdapterError.js"; import { analysisErrorWithCleanupFailure } from "../domain/analysisErrorCleanup.js"; import { err, ok, type Result } from "../domain/result.js"; import { SharedStartupAttempt } from "../process/SharedStartupAttempt.js"; @@ -75,11 +78,7 @@ export class AnalysisClientCreation { ): Promise> { if (signal?.aborted) return err(cancelledBy(operation, signal)); if (this.#closed) - return err( - new ProviderAdapterError(this.providerId, operation, { - diagnostics: { reason: "client_closed" }, - }), - ); + return err(closedAnalysisClientError(this.providerId, operation)); if (this.#failure?.cleanupIncomplete) return err( analysisClientFailureForOperation( diff --git a/src/providers/DeferredAnalysisClient.ts b/src/providers/DeferredAnalysisClient.ts index 40ef8c5805..17b5a29e2a 100644 --- a/src/providers/DeferredAnalysisClient.ts +++ b/src/providers/DeferredAnalysisClient.ts @@ -6,7 +6,10 @@ import type { } from "../application/AnalysisProvider.js"; import type { ProviderOperationHealth } from "../domain/providerOperationHealth.js"; import { AnalysisError } from "../domain/analysisErrorBase.js"; -import { ProviderAdapterError } from "../domain/providerAdapterError.js"; +import { + closedAnalysisClientError, + ProviderAdapterError, +} from "../domain/providerAdapterError.js"; import { err } from "../domain/result.js"; import { AnalysisClientCreation } from "./AnalysisClientCreation.js"; @@ -46,11 +49,7 @@ export class DeferredAnalysisClient implements AnalysisClient { const loaded = await this.#creation.acquire(operation, options?.signal); if (!loaded.ok) return loaded; if (this.#creation.readyClient !== loaded.value) - return err( - new ProviderAdapterError(this.#providerId, operation, { - diagnostics: { reason: "client_closed" }, - }), - ); + return err(closedAnalysisClientError(this.#providerId, operation)); return await loaded.value.execute(operation, parameters, options); } catch (cause: unknown) { if (cause instanceof AnalysisError) return err(cause); diff --git a/tests/boundary/mcp/enhancedTools.callPathsAndBatches.test.ts b/tests/boundary/mcp/enhancedTools.callPathsAndBatches.test.ts index bba99d4650..74544340bc 100644 --- a/tests/boundary/mcp/enhancedTools.callPathsAndBatches.test.ts +++ b/tests/boundary/mcp/enhancedTools.callPathsAndBatches.test.ts @@ -589,12 +589,12 @@ describe("enhanced MCP batches", () => { category: "execution_failure", details: { operation: "decompile", reason: "failed" }, message: expect.stringMatching( - /unreadable result.*rea doctor.*repeating the unchanged request.*not expected to succeed/i, + /result REA could not read.*repeating the unchanged request.*not expected to succeed/i, ), retryable: false, remediation: { action: expect.stringMatching( - /unreadable result.*rea doctor.*repeating the unchanged request.*not expected to succeed/i, + /result REA could not read.*repeating the unchanged request.*not expected to succeed/i, ), }, }, diff --git a/tests/boundary/mcp/nativeUiPartialCallRoutesMcp.test.ts b/tests/boundary/mcp/nativeUiPartialCallRoutesMcp.test.ts index b940feaf95..05ae62e416 100644 --- a/tests/boundary/mcp/nativeUiPartialCallRoutesMcp.test.ts +++ b/tests/boundary/mcp/nativeUiPartialCallRoutesMcp.test.ts @@ -410,7 +410,7 @@ it.each(["unavailable", "output"] as const)( reason: failure === "unavailable" ? expect.stringMatching(/unavailable.*target/iu) - : expect.stringContaining("unreadable result"), + : expect.stringContaining("result REA could not read"), limitations: expect.arrayContaining([ "The provider could not complete callee enumeration.", ]), @@ -767,7 +767,7 @@ it("reports malformed partial CallReference as unreadable route evidence", async expect.objectContaining({ relation: "provider_call", resolution: "unresolved", - reason: expect.stringContaining("unreadable result"), + reason: expect.stringContaining("result REA could not read"), }), ); }); diff --git a/tests/boundary/process/jadxIntegration.test.ts b/tests/boundary/process/jadxIntegration.test.ts index 1660cc09e5..69b8d5d762 100644 --- a/tests/boundary/process/jadxIntegration.test.ts +++ b/tests/boundary/process/jadxIntegration.test.ts @@ -1405,6 +1405,32 @@ it("retains the execution failure while cleanup is pending and resumes after cle } }); +it("does not send a jadx-reported request failure to doctor", async () => { + const { service, apk, launches, provider } = await setup("tool-error"); + try { + const outcome = await service.execute("inspect_android_package", { + path: apk, + }); + if (outcome.ok) throw new Error("Expected the reported tool error"); + const projected = projectAnalysisError(outcome.error); + expect(projected).toMatchObject({ + code: "execution_failure", + retryable: false, + details: { + diagnostics: { + reason: "manifest decoder rejected malformed binary XML", + }, + }, + }); + expect(projected.message).toContain("details.diagnostics.reason"); + for (const text of [projected.message, projected.remediation.action]) + expect(text).not.toContain("doctor"); + await verifyCleanup(launches); + } finally { + await provider.close(); + } +}); + it("accepts exact selected-overload smali from the bridge", async () => { const { service, apk, launches, provider } = await setup( "selected-overload-smali",