From 0a0244db4212bc1f59dec4b925fa90b3b473136a Mon Sep 17 00:00:00 2001 From: Kaoru0822 <208281322+kaoru0822-kitauji@users.noreply.github.com> Date: Fri, 9 Oct 2026 04:08:43 +0000 Subject: [PATCH] fix(config): name the rejected setting and its constraint in configuration errors Every configuration failure projected the same message, "REA configuration is invalid. Run `rea doctor` and fix the reported setting.", with no details. The environment parser discarded the issue that names the variable (for example `REA_GHIDRA_NATIVEAOT_JAR must be absolute`), and doctor can report healthy for that environment, so neither the CLI, the MCP startup message, nor doctor told callers which setting to correct. ConfigurationError now carries the rejected settings and their constraints (never their values). The projection names them in its message, lists them in `details.settings`, and asks the caller to correct those settings where REA is started. Errors without a named setting keep the previous message. Fixes #1184 Co-Authored-By: Claude Opus 5.5 --- src/config.test.ts | 43 +++++++++++++++++++++++++ src/config/environment.ts | 6 ++++ src/config/mcpResponseBudget.ts | 13 ++++++-- src/config/parsers.ts | 30 +++++++++++++++-- src/domain/analysisErrorPresentation.ts | 12 ++++++- src/domain/analysisErrorProjection.ts | 12 ++++++- src/domain/configurationErrors.ts | 18 +++++++++++ src/ida/IdaConfiguration.ts | 20 +++++++++--- src/ida/IdaProvider.ts | 5 +-- 9 files changed, 145 insertions(+), 14 deletions(-) diff --git a/src/config.test.ts b/src/config.test.ts index dd228e4a6..6643fbdd1 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { parseConfig } from "./config.js"; +import { projectAnalysisError } from "./domain/analysisErrorProjection.js"; describe("runtime configuration", () => { it("allows target-free startup and applies runtime defaults", () => { @@ -155,3 +156,45 @@ describe("runtime collection configuration", () => { }, ); }); + +describe("configuration failure diagnostics", () => { + it.each([ + ["GHIDRA_INSTALL_DIR", "./fixture"], + ["JAVA_HOME", "./jdk"], + ["REA_GHIDRA_NATIVEAOT_JAR", "./missing.jar"], + ])( + "names the rejected %s and its absolute-path constraint", + (setting, value) => { + const result = parseConfig({ [setting]: value }); + expect(result.ok).toBe(false); + if (result.ok) return; + const constraint = `${setting} must be absolute`; + expect(projectAnalysisError(result.error)).toMatchObject({ + code: "configuration_invalid", + message: `REA configuration is invalid: ${constraint}.`, + remediation: { action: expect.stringContaining(`Correct ${setting} `) }, + details: { settings: [{ setting, constraint }] }, + }); + // The rejected value itself is not echoed. + expect(JSON.stringify(projectAnalysisError(result.error))).not.toContain( + value, + ); + }, + ); + + it("names settings rejected by their own parsers", () => { + const result = parseConfig({ HOPPER_LOADER_ARGS_JSON: "not-json" }); + expect(result.ok).toBe(false); + if (!result.ok) + expect(projectAnalysisError(result.error)).toMatchObject({ + details: { + settings: [ + { + setting: "HOPPER_LOADER_ARGS_JSON", + constraint: "HOPPER_LOADER_ARGS_JSON must be valid JSON", + }, + ], + }, + }); + }); +}); diff --git a/src/config/environment.ts b/src/config/environment.ts index fe7e1245e..f743de576 100644 --- a/src/config/environment.ts +++ b/src/config/environment.ts @@ -53,9 +53,15 @@ export const parseEnvironment = ( ): Result => { const parsed = environmentSchema.safeParse(environment); if (!parsed.success) { + // Zod messages name the constraint, never the rejected value. + const settings = parsed.error.issues.map((issue) => ({ + setting: String(issue.path[0] ?? "environment"), + constraint: issue.message, + })); return err( new ConfigurationError("Invalid REA environment configuration", { cause: parsed.error, + settings, }), ); } diff --git a/src/config/mcpResponseBudget.ts b/src/config/mcpResponseBudget.ts index 01d69db97..27126bf0c 100644 --- a/src/config/mcpResponseBudget.ts +++ b/src/config/mcpResponseBudget.ts @@ -15,6 +15,8 @@ export const mcpResponseBudgetSchema = z .transform(Number) .pipe(z.number().int().safe().min(MINIMUM_RESPONSE_BYTES)); +const budgetConstraint = `REA_MCP_MAX_RESPONSE_BYTES must be a decimal safe integer at least ${String(MINIMUM_RESPONSE_BYTES)} bytes, the default stdio receive-buffer size.`; + /** Parse only the MCP-specific budget without loading unrelated provider settings. */ export const parseMcpResponseBudget = ( value: string | undefined, @@ -24,8 +26,13 @@ export const parseMcpResponseBudget = ( return parsed.success ? ok(parsed.data) : err( - new ConfigurationError( - `REA_MCP_MAX_RESPONSE_BYTES must be a decimal safe integer at least ${String(MINIMUM_RESPONSE_BYTES)} bytes, the default stdio receive-buffer size.`, - ), + new ConfigurationError(budgetConstraint, { + settings: [ + { + setting: "REA_MCP_MAX_RESPONSE_BYTES", + constraint: budgetConstraint, + }, + ], + }), ); }; diff --git a/src/config/parsers.ts b/src/config/parsers.ts index 41637d513..344b31eea 100644 --- a/src/config/parsers.ts +++ b/src/config/parsers.ts @@ -14,10 +14,21 @@ export const parseStringArray = ( : err( new ConfigurationError(`${name} must encode an array of strings`, { cause: parsed.error, + settings: [ + { + setting: name, + constraint: `${name} must encode an array of strings`, + }, + ], }), ); } catch (cause: unknown) { - return err(new ConfigurationError(`${name} must be valid JSON`, { cause })); + return err( + new ConfigurationError(`${name} must be valid JSON`, { + cause, + settings: [{ setting: name, constraint: `${name} must be valid JSON` }], + }), + ); } }; @@ -32,6 +43,9 @@ export const parseLoaderArgs = ( return err( new ConfigurationError("HOPPER_LOADER_ARGS_JSON must be valid JSON", { cause, + settings: [ + loaderArgsIssue("HOPPER_LOADER_ARGS_JSON must be valid JSON"), + ], }), ); } @@ -41,7 +55,19 @@ export const parseLoaderArgs = ( : err( new ConfigurationError( "HOPPER_LOADER_ARGS_JSON must encode an array of strings", - { cause: parsed.error }, + { + cause: parsed.error, + settings: [ + loaderArgsIssue( + "HOPPER_LOADER_ARGS_JSON must encode an array of strings", + ), + ], + }, ), ); }; + +const loaderArgsIssue = (constraint: string) => ({ + setting: "HOPPER_LOADER_ARGS_JSON", + constraint, +}); diff --git a/src/domain/analysisErrorPresentation.ts b/src/domain/analysisErrorPresentation.ts index cc3dc7e40..7676b2b39 100644 --- a/src/domain/analysisErrorPresentation.ts +++ b/src/domain/analysisErrorPresentation.ts @@ -78,6 +78,8 @@ export const analysisErrorRemediationAction = ( return "Check the current process's read access to the selected path. Retry with a readable local file."; if (error instanceof AnalysisArtifactChangedError) return "Wait until the selected file is stable. For an active binary session, reopen the target with open_binary before retrying so REA acquires its current identity; for a CLI command or target-free tool, rerun the operation."; + if (error instanceof ConfigurationError && error.settings.length > 0) + return `Correct ${[...new Set(error.settings.map(({ setting }) => setting))].join(", ")} in the environment that starts REA (your shell or the MCP client's registration), then rerun.`; if (error instanceof AnalysisInputError) return "Correct the listed arguments and retry."; if (error instanceof UnknownRegistryError && error.reason === "not-found") @@ -189,7 +191,15 @@ export const analysisErrorUserMessage = (error: AnalysisError): string => { if (error instanceof UnknownRegistryError) return "Evidence state changed before the update completed. Refresh the current state and try again."; if (error instanceof ConfigurationError) - return "REA configuration is invalid. Run `rea doctor` and fix the reported setting."; + return error.settings.length === 0 + ? "REA configuration is invalid. Run `rea doctor` and fix the reported setting." + : `REA configuration is invalid: ${error.settings + .map(({ setting, constraint }) => + constraint.includes(setting) + ? constraint.replace(/\.$/u, "") + : `${setting}: ${constraint}`, + ) + .join("; ")}.`; if (error instanceof NoBinaryOpenError) return error.message; if (error instanceof BinaryTargetError) return error.constraint === "directory_requires_file" diff --git a/src/domain/analysisErrorProjection.ts b/src/domain/analysisErrorProjection.ts index 66e5c77e9..7fd0241ca 100644 --- a/src/domain/analysisErrorProjection.ts +++ b/src/domain/analysisErrorProjection.ts @@ -18,7 +18,10 @@ import { AnalysisResourceConstraintError, } from "./analysisErrorCore.js"; import { ArtifactOperationError } from "./artifactOperationError.js"; -import { BinaryTargetError } from "./configurationErrors.js"; +import { + BinaryTargetError, + ConfigurationError, +} from "./configurationErrors.js"; import { BrowserObservationError } from "./browserObservationError.js"; import { EvidenceFileError, @@ -482,6 +485,13 @@ const lifecycleErrorDetails = ( }; return Object.keys(details).length === 0 ? undefined : details; } + if (error instanceof ConfigurationError && error.settings.length > 0) + return { + settings: error.settings.map(({ setting, constraint }) => ({ + setting, + constraint, + })), + }; if (error instanceof BinaryTargetError) return { path: error.path, diff --git a/src/domain/configurationErrors.ts b/src/domain/configurationErrors.ts index c7c07f75e..5820db7ff 100644 --- a/src/domain/configurationErrors.ts +++ b/src/domain/configurationErrors.ts @@ -3,9 +3,27 @@ import { type AnalysisErrorOptions, } from "./analysisErrorBase.js"; +/** One rejected configuration setting and the constraint it failed. */ +export interface ConfigurationSettingIssue { + readonly setting: string; + readonly constraint: string; +} + +/** Rejected settings retained for diagnostics; their values are not. */ +export interface ConfigurationErrorOptions extends AnalysisErrorOptions { + readonly settings?: readonly ConfigurationSettingIssue[]; +} + /** Runtime configuration could not be parsed safely. */ export class ConfigurationError extends AnalysisError { readonly _tag = "ConfigurationError"; + readonly settings: readonly ConfigurationSettingIssue[]; + constructor(message: string, options?: ConfigurationErrorOptions) { + super(message, options); + this.settings = (options?.settings ?? []).map( + ({ setting, constraint }) => ({ setting, constraint }), + ); + } } /** No app or binary session exists for an analysis request. */ diff --git a/src/ida/IdaConfiguration.ts b/src/ida/IdaConfiguration.ts index 051ee8d4d..26084b734 100644 --- a/src/ida/IdaConfiguration.ts +++ b/src/ida/IdaConfiguration.ts @@ -50,7 +50,7 @@ export const readIdaConfiguration = ( try { if (statSync(path).size > 65_536) return err( - new ConfigurationError( + idaConfigurationError( "REA_IDA_MCP_CONFIG exceeds the 64 KiB registration size.", ), ); @@ -64,17 +64,27 @@ export const readIdaConfiguration = ( return parsed.success ? ok(parsed.data) : err( - new ConfigurationError( + idaConfigurationError( "REA_IDA_MCP_CONFIG must contain an IDA MCP command/args or local url registration; see docs/ida-provider.md.", - { cause: parsed.error }, + parsed.error, ), ); } catch (cause: unknown) { return err( - new ConfigurationError( + idaConfigurationError( `Cannot read IDA MCP registration ${path}; provide a readable JSON configuration.`, - { cause }, + cause, ), ); } }; + +/** A REA_IDA_MCP_CONFIG rejection that names the setting it concerns. */ +export const idaConfigurationError = ( + constraint: string, + cause?: unknown, +): ConfigurationError => + new ConfigurationError(constraint, { + ...(cause === undefined ? {} : { cause }), + settings: [{ setting: "REA_IDA_MCP_CONFIG", constraint }], + }); diff --git a/src/ida/IdaProvider.ts b/src/ida/IdaProvider.ts index 6f195cb61..ff8a46fbb 100644 --- a/src/ida/IdaProvider.ts +++ b/src/ida/IdaProvider.ts @@ -11,13 +11,14 @@ import type { ProviderTargetSupport, } from "../application/AnalysisProvider.js"; import type { AppConfig } from "../config.js"; -import { ConfigurationError } from "../domain/configurationErrors.js"; +import type { ConfigurationError } from "../domain/configurationErrors.js"; import { createHash } from "node:crypto"; import canonicalize from "canonicalize"; import type { Result } from "../domain/result.js"; import { readIdaConfiguration, type IdaConfiguration, + idaConfigurationError, } from "./IdaConfiguration.js"; import { createIdaMcpConnection, @@ -165,7 +166,7 @@ export class IdaProvider implements AnalysisProviderCandidate { const readIdaConfigurationNotConfigured = () => err( - new ConfigurationError( + idaConfigurationError( "Set REA_IDA_MCP_CONFIG to an upstream IDA MCP JSON registration; see docs/ida-provider.md.", ), );