diff --git a/docs/installation.md b/docs/installation.md index d18a9b6405..cbfb3e4db5 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -340,6 +340,13 @@ A scoped report has `scope.mode: "explicit"`. Its `scope_checks` determine `environment_healthy` summarizes the full audit. `--target` adds a target check, but the report remains audit-wide unless a scope option is also supplied. +An invalid `REA_LOG_LEVEL` fails every scope, including one client or the +installed skill, because the executable rejects it before any command. A +selected client also fails for every other setting that MCP startup rejects, +including `REA_GHIDRA_STARTUP_TIMEOUT_MS` and `REA_HOPPER_STARTUP_TIMEOUT_MS`. +Those settings still fail a selected provider and the audit-wide result. A +skill-only report leaves them informational. + Follow the remediation for the failed check. If a native target has several available providers, choose one with `--provider` on the CLI or `provider_id` on `open_binary`. A `capability_unavailable` failure carries diff --git a/scripts/rea.mjs b/scripts/rea.mjs index 326676a976..ab16851541 100755 --- a/scripts/rea.mjs +++ b/scripts/rea.mjs @@ -94,7 +94,9 @@ if (!(await compiledRuntimeExists(runtimeFiles))) { const cliArgs = normalizeCliArguments(args); const outputArguments = validateCliOutputArguments(cliArgs); const configurationError = cliConfigurationError(process.env); - if (configurationError !== undefined) { + // Doctor is the readiness report for this failure. Other commands still stop + // here so they do not run with an unusable log level. + if (configurationError !== undefined && !isDoctorCommand(cliArgs)) { process.stderr.write(`${configurationError}\n`); process.exitCode = 1; } else if (!outputArguments.ok) { @@ -135,6 +137,27 @@ if (!(await compiledRuntimeExists(runtimeFiles))) { } } +/** True when the first positional command is `doctor`, after global output flags. */ +function isDoctorCommand(arguments_) { + const valuedFlags = new Set([ + "--format", + "--filter-output", + "--token-limit", + "--token-offset", + ]); + for (let index = 0; index < arguments_.length; index += 1) { + const argument = arguments_[index]; + if (argument === "--") return arguments_[index + 1] === "doctor"; + if (valuedFlags.has(argument)) { + index += 1; + continue; + } + if (argument.startsWith("-")) continue; + return argument === "doctor"; + } + return false; +} + /** * A reader that closes the pipe early, as `rea ... | head` does, has declined * the rest of the output. Stop writing without reporting a failure, and exit diff --git a/src/application/Doctor.test.ts b/src/application/Doctor.test.ts index b8aad49859..a7d57143d0 100644 --- a/src/application/Doctor.test.ts +++ b/src/application/Doctor.test.ts @@ -89,7 +89,7 @@ describe("doctor Hopper startup timeout", () => { }, ); - it("keeps an invalid Hopper timeout informational outside the Hopper scope", async () => { + it("keeps an invalid Hopper timeout informational when no provider is selected", async () => { const selectedHost = timeoutHost("0"); const result = await runDoctor(undefined, selectedHost, { providers: [] }); expect(result.healthy).toBe(true); @@ -100,6 +100,273 @@ describe("doctor Hopper startup timeout", () => { }), ); }); + + it("requires an invalid Hopper timeout for a ready non-Hopper provider", async () => { + const ghidra = { + providerInspections: () => + Promise.resolve([ + { + id: "ghidra" as const, + configured: true, + available: true, + providerVersion: "12.1.4", + registrationEnvironment: { GHIDRA_INSTALL_DIR: "/tools/ghidra" }, + checks: [ + { + name: "configuration", + ok: true, + code: null, + detail: "/tools/ghidra", + remediation: null, + classification: "config_drift" as const, + }, + ], + }, + ]), + }; + const blocked = await runDoctor(undefined, timeoutHost("abc", ghidra), { + providers: ["ghidra"], + }); + expect(blocked.healthy).toBe(false); + expect(blocked.scope_checks).toContainEqual( + expect.objectContaining({ + name: "hopper-startup-timeout", + ok: false, + classification: "config_drift", + }), + ); + expect( + blocked.scope_checks.filter( + ({ name }) => name === "hopper-startup-timeout", + ), + ).toHaveLength(1); + + const ready = await runDoctor(undefined, timeoutHost("120000", ghidra), { + providers: ["ghidra"], + }); + expect(ready.healthy).toBe(true); + expect(ready.scope_checks).toContainEqual( + expect.objectContaining({ name: "hopper-startup-timeout", ok: true }), + ); + }); +}); + +describe("doctor native analysis configuration", () => { + const configurationHost = ( + environment: Readonly, + ): DoctorHost => ({ + ...systemDoctorHost({ environment: { ...environment } }), + ...host({ + installationPaths: () => Promise.resolve([]), + clientRegistrations: () => Promise.resolve([]), + }), + }); + + it.each([ + ["REA_ANALYSIS_PROVIDER", "Hopper"], + ["REA_GHIDRA_STARTUP_TIMEOUT_MS", "abc"], + ["REA_SWIFTC_COMPILE_TIMEOUT_MS", "0"], + ["REA_LOG_LEVEL", "verbose"], + ["REA_GHIDRA_COMPILER_SPEC_ID", "gcc"], + ] as const)( + "rejects %s before scoped Hopper readiness reports success", + async (setting, value) => { + const result = await runDoctor( + undefined, + configurationHost({ [setting]: value }), + { providers: ["hopper"] }, + ); + + expect(result.healthy).toBe(false); + expect(result.environment_healthy).toBe(false); + expect(result.scope_checks).toContainEqual( + expect.objectContaining({ + name: `runtime-configuration:${setting}`, + ok: false, + classification: "config_drift", + detail: expect.stringContaining(setting), + remediation: expect.stringContaining(setting), + details: { setting }, + }), + ); + }, + ); + + it("keeps a lowercase provider preference ready for the selected engine", async () => { + const result = await runDoctor( + undefined, + configurationHost({ REA_ANALYSIS_PROVIDER: "hopper" }), + { providers: ["hopper"] }, + ); + + expect(result.healthy).toBe(true); + expect( + result.checks.some(({ name }) => + name.startsWith("runtime-configuration:"), + ), + ).toBe(false); + }); + + it("keeps an invalid native setting informational when nothing starts MCP", async () => { + const result = await runDoctor( + undefined, + configurationHost({ REA_GHIDRA_STARTUP_TIMEOUT_MS: "abc" }), + { providers: [] }, + ); + + expect(result.healthy).toBe(true); + expect(result.environment_healthy).toBe(false); + expect(result.scope_checks.map(({ name }) => name)).not.toContain( + "runtime-configuration:REA_GHIDRA_STARTUP_TIMEOUT_MS", + ); + expect(result.informational_checks).toContainEqual( + expect.objectContaining({ + name: "runtime-configuration:REA_GHIDRA_STARTUP_TIMEOUT_MS", + ok: false, + }), + ); + }); + + it("does not treat a JEB-only timeout as native analysis configuration", async () => { + const result = await runDoctor( + undefined, + configurationHost({ REA_JEB_MCP_TIMEOUT_MS: "abc" }), + { providers: ["hopper"] }, + ); + + expect(result.healthy).toBe(true); + expect( + result.checks.some(({ name }) => + name.startsWith("runtime-configuration:"), + ), + ).toBe(false); + }); +}); + +describe("doctor MCP startup readiness", () => { + const configurationHost = ( + environment: Readonly, + ): DoctorHost => ({ + ...systemDoctorHost({ environment: { ...environment } }), + ...host({ + installationPaths: () => Promise.resolve([]), + clientRegistrations: () => Promise.resolve([]), + }), + }); + const alignedCodex: ClientRegistrationStatus = { + client: "codex", + config_path: "/home/user/.codex/config.toml", + command: ["/current/rea", "mcp"], + state: "aligned", + remediation: null, + }; + const selectedHost = ( + environment: Readonly, + ): DoctorHost => ({ + ...configurationHost(environment), + clientRegistrations: () => Promise.resolve([alignedCodex]), + }); + + it("requires an invalid log level for a client and for the skill", async () => { + const client = await runDoctor( + undefined, + selectedHost({ + REA_LOG_LEVEL: "verbose", + REA_GHIDRA_STARTUP_TIMEOUT_MS: "abc", + }), + { clients: ["codex"] }, + ); + expect(client.healthy).toBe(false); + expect(client.scope_checks).toContainEqual( + expect.objectContaining({ + name: "runtime-configuration:REA_LOG_LEVEL", + ok: false, + details: { setting: "REA_LOG_LEVEL" }, + }), + ); + expect(client.scope_checks).toContainEqual( + expect.objectContaining({ + name: "runtime-configuration:REA_GHIDRA_STARTUP_TIMEOUT_MS", + ok: false, + }), + ); + expect(client.scope_checks).toContainEqual( + expect.objectContaining({ name: "registration:codex", ok: true }), + ); + + const skill = await runDoctor( + undefined, + selectedHost({ + REA_LOG_LEVEL: "verbose", + REA_GHIDRA_STARTUP_TIMEOUT_MS: "abc", + }), + { skill: true }, + ); + expect(skill.healthy).toBe(false); + expect(skill.scope_checks).toContainEqual( + expect.objectContaining({ + name: "runtime-configuration:REA_LOG_LEVEL", + ok: false, + }), + ); + expect(skill.scope_checks.map(({ name }) => name)).not.toContain( + "runtime-configuration:REA_GHIDRA_STARTUP_TIMEOUT_MS", + ); + }); + + it("requires every MCP startup failure for an aligned client", async () => { + const ghidraTimeout = await runDoctor( + undefined, + selectedHost({ REA_GHIDRA_STARTUP_TIMEOUT_MS: "abc" }), + { clients: ["codex"] }, + ); + expect(ghidraTimeout.healthy).toBe(false); + expect(ghidraTimeout.scope_checks).toContainEqual( + expect.objectContaining({ + name: "runtime-configuration:REA_GHIDRA_STARTUP_TIMEOUT_MS", + ok: false, + }), + ); + expect(ghidraTimeout.scope_checks).toContainEqual( + expect.objectContaining({ name: "registration:codex", ok: true }), + ); + + const hopperTimeout = await runDoctor( + undefined, + selectedHost({ REA_HOPPER_STARTUP_TIMEOUT_MS: "abc" }), + { clients: ["codex"] }, + ); + expect(hopperTimeout.healthy).toBe(false); + expect( + hopperTimeout.scope_checks.filter( + ({ name }) => name === "hopper-startup-timeout", + ), + ).toHaveLength(1); + + const hopperAndClient = await runDoctor( + undefined, + selectedHost({ REA_HOPPER_STARTUP_TIMEOUT_MS: "abc" }), + { clients: ["codex"], providers: ["hopper"] }, + ); + expect( + hopperAndClient.scope_checks.filter( + ({ name }) => name === "hopper-startup-timeout", + ), + ).toHaveLength(1); + + const skill = await runDoctor( + undefined, + selectedHost({ REA_GHIDRA_STARTUP_TIMEOUT_MS: "abc" }), + { skill: true }, + ); + expect(skill.healthy).toBe(true); + expect(skill.informational_checks).toContainEqual( + expect.objectContaining({ + name: "runtime-configuration:REA_GHIDRA_STARTUP_TIMEOUT_MS", + ok: false, + }), + ); + }); }); describe("doctor runtime check", () => { diff --git a/src/application/Doctor.ts b/src/application/Doctor.ts index f3727ac107..588c5d4acd 100644 --- a/src/application/Doctor.ts +++ b/src/application/Doctor.ts @@ -11,7 +11,10 @@ import { constants } from "node:fs"; import { access, readdir, stat } from "node:fs/promises"; import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; import type { RetainedResourceScope } from "./AnalysisProvider.js"; +import { nativeAnalysisConfigurationError } from "../config/parseConfig.js"; import { homeDirectoryFromEnvironment } from "../config/homeDirectory.js"; +import type { ConfigurationSettingIssue } from "../domain/configurationErrors.js"; +import { snapshotEnvironment } from "../process/snapshotEnvironment.js"; import { join } from "node:path"; import { analysisErrorRemediationAction } from "../domain/analysisErrorPresentation.js"; @@ -109,6 +112,11 @@ export interface DoctorHost { readonly homeDirectory: string; readonly configuredHopperPath?: string; readonly configuredHopperStartupTimeout?: string; + /** + * Settings `parseConfig` rejects before native analysis starts. + * Undefined means this host did not run that admission. + */ + readonly configurationIssues?: readonly ConfigurationSettingIssue[]; readonly configuredIlspyCmdPath?: string; macosVersion(): Promise; linuxDistribution(): Promise; @@ -508,6 +516,9 @@ export const systemDoctorHost = ( const platform = options.platform ?? process.platform; const architecture = options.architecture ?? process.arch; const environment = options.environment ?? process.env; + const configurationError = nativeAnalysisConfigurationError( + snapshotEnvironment(environment, platform), + ); const hostExecFileOutput = options.execFileOutput ?? execFileOutput; const homeDirectory = homeDirectoryFromEnvironment(environment, platform); const commandEnvironment = { env: environment }; @@ -548,6 +559,7 @@ export const systemDoctorHost = ( configuredHopperStartupTimeout: environment.REA_HOPPER_STARTUP_TIMEOUT_MS, }), + configurationIssues: configurationError?.settings ?? [], ...(environment.REA_ILSPY_CMD_PATH === undefined ? {} : { configuredIlspyCmdPath: environment.REA_ILSPY_CMD_PATH }), diff --git a/src/application/DoctorDiagnostics.ts b/src/application/DoctorDiagnostics.ts index 9f9526ad8f..f09c17889a 100644 --- a/src/application/DoctorDiagnostics.ts +++ b/src/application/DoctorDiagnostics.ts @@ -37,6 +37,7 @@ export const collectDoctorDiagnostics = async ( checks.push(hopperCheck(host, hopperPath)); const hopperTimeoutCheck = optionalHopperStartupTimeoutCheck(host); if (hopperTimeoutCheck !== undefined) checks.push(hopperTimeoutCheck); + checks.push(...runtimeConfigurationChecks(host)); const hopperVersionCheck = await optionalHopperVersionCheck(host, hopperPath); if (hopperVersionCheck !== undefined) checks.push(hopperVersionCheck); @@ -230,6 +231,44 @@ const optionalHopperStartupTimeoutCheck = ( ); }; +/** + * Project every native-analysis admission failure except the Hopper timeout, + * which already has its own check. Provider scopes include engine settings. + * Settings that block process start are marked for every doctor scope. + */ +const runtimeConfigurationChecks = ( + host: DoctorHost, +): readonly DoctorCheck[] => { + const issues = host.configurationIssues; + if (issues === undefined || issues.length === 0) return []; + const grouped: { setting: string; constraints: string[] }[] = []; + for (const issue of issues) { + if (issue.setting === "REA_HOPPER_STARTUP_TIMEOUT_MS") continue; + const existing = grouped.find((item) => item.setting === issue.setting); + if (existing === undefined) + grouped.push({ setting: issue.setting, constraints: [issue.constraint] }); + else existing.constraints.push(issue.constraint); + } + return grouped.map(({ setting, constraints }) => ({ + ...check( + `runtime-configuration:${setting}`, + false, + constraints + .map((constraint) => + constraint.includes(setting) + ? constraint.replace(/\.$/u, "") + : `${setting}: ${constraint}`, + ) + .join("; "), + { + classification: "config_drift", + remediation: `Correct ${setting} in the environment that starts REA (your shell or the MCP client's registration), then rerun.`, + }, + ), + details: { setting }, + })); +}; + const optionalHopperVersionCheck = async ( host: DoctorHost, hopperPath: string | undefined, diff --git a/src/application/DoctorScope.ts b/src/application/DoctorScope.ts index da22d43b41..49df606bab 100644 --- a/src/application/DoctorScope.ts +++ b/src/application/DoctorScope.ts @@ -1,4 +1,5 @@ import { uniqueSorted } from "../domain/canonicalOrdering.js"; +import { blocksProcessStart } from "../logger.js"; import type { ClientRegistrationStatus, UnhealthyClientRegistrationStatus, @@ -80,8 +81,33 @@ export const scopeDoctorChecks = ( const hopperHostChecks = input.scope.providers.includes("hopper") ? input.checks.filter(({ name }) => name === "host") : []; + // parseConfig rejects these before the MCP server starts. Hopper's own + // scope already includes hopper-startup-timeout through its provider checks. + const blocksMcpStartup = (check: DoctorCheck): boolean => + check.name.startsWith("runtime-configuration:") || + check.name === "hopper-startup-timeout"; + const blocksProcess = (check: DoctorCheck): boolean => { + const setting = check.details?.setting; + return ( + check.name.startsWith("runtime-configuration:") && + typeof setting === "string" && + blocksProcessStart(setting) + ); + }; + const exercisesMcpStartup = + input.scope.clients.length > 0 || input.scope.providers.length > 0; + const startupChecks = ( + exercisesMcpStartup + ? input.checks.filter(blocksMcpStartup) + : input.checks.filter(blocksProcess) + ).filter( + (check) => + check.name !== "hopper-startup-timeout" || + !input.scope.providers.includes("hopper"), + ); const scopeChecks = [ ...coreChecks, + ...startupChecks, ...hopperHostChecks, ...selectedProviderChecks, ...clientChecks, @@ -91,6 +117,7 @@ export const scopeDoctorChecks = ( return { healthy: coreChecks.every(({ ok }) => ok) && + startupChecks.every(({ ok }) => ok) && hopperHostChecks.every(({ ok }) => ok) && clientChecks.every(({ ok }) => ok) && skillChecks.every(({ ok }) => ok) && diff --git a/src/cli.ts b/src/cli.ts index df2c15c15f..d7e9e72ad9 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -58,7 +58,10 @@ import { import { registerReverseEngineeringCommands } from "./cli/reverseEngineeringCommands.js"; import { registerCutterCommands } from "./cli/cutterCommands.js"; -/** Describe CLI settings that are invalid before any command runs, as MCP does. */ +/** + * Describe CLI settings that are invalid before any command runs, as MCP does. + * Keep this aligned with PROCESS_START_CONFIGURATION_SETTINGS. + */ export const cliConfigurationError = ( selectedEnvironment: Readonly>, ): string | undefined => { @@ -82,7 +85,8 @@ export const createCli = ( ): CliInstance => { const environment = snapshotEnvironment(selectedEnvironment); const analysis = createDirectAnalysis(environment); - // The executable reports an invalid level through cliConfigurationError. + // The executable reports an invalid level before every command except doctor. + // Doctor keeps going so its readiness report can name REA_LOG_LEVEL. const level = environment.REA_LOG_LEVEL === undefined ? undefined diff --git a/src/config.test.ts b/src/config.test.ts index d85ecb76cb..1c2017017a 100644 --- a/src/config.test.ts +++ b/src/config.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import { defaultHopperLauncherPath, + nativeAnalysisConfigurationError, parseConfig, } from "./config/parseConfig.js"; import { @@ -98,6 +99,28 @@ describe("runtime configuration", () => { expect(parseConfig({ REA_ANALYSIS_PROVIDER: invalid }).ok).toBe(false); }); + it("shares native admission with doctor and ignores the JEB-only timeout", () => { + expect(nativeAnalysisConfigurationError({})).toBeUndefined(); + expect( + nativeAnalysisConfigurationError({ REA_JEB_MCP_TIMEOUT_MS: "abc" }), + ).toBeUndefined(); + expect(parseConfig({ REA_JEB_MCP_TIMEOUT_MS: "abc" }).ok).toBe(true); + const rejected = nativeAnalysisConfigurationError({ + REA_ANALYSIS_PROVIDER: "Hopper", + REA_GHIDRA_STARTUP_TIMEOUT_MS: "abc", + }); + expect(rejected?.settings.map(({ setting }) => setting)).toEqual([ + "REA_ANALYSIS_PROVIDER", + "REA_GHIDRA_STARTUP_TIMEOUT_MS", + ]); + expect( + parseConfig({ + REA_ANALYSIS_PROVIDER: "Hopper", + REA_GHIDRA_STARTUP_TIMEOUT_MS: "abc", + }).ok, + ).toBe(false); + }); + it("parses absolute BYO Ghidra and optional Java paths", () => { expect( parseConfig({ diff --git a/src/config/parseConfig.ts b/src/config/parseConfig.ts index c00f787a8d..8fa259d682 100644 --- a/src/config/parseConfig.ts +++ b/src/config/parseConfig.ts @@ -39,16 +39,35 @@ export const defaultHopperLauncherPath = ( ); }; +/** + * Environment native analysis admits. JEB validates its own timeout during + * optional provider composition; that setting must not block unrelated + * CLI/MCP startup. Registration validation still checks it directly. + */ +const nativeAnalysisEnvironment = ( + environment: Readonly>, +): Readonly> => { + const { REA_JEB_MCP_TIMEOUT_MS: _jebTimeout, ...appEnvironment } = + environment; + return appEnvironment; +}; + +/** + * The configuration error native commands return, without probing launchers. + * Doctor uses this so readiness and analysis admit the same settings. + */ +export const nativeAnalysisConfigurationError = ( + environment: Readonly>, +): ConfigurationError | undefined => { + const parsed = parseEnvironment(nativeAnalysisEnvironment(environment)); + return parsed.ok ? undefined : parsed.error; +}; + /** Parse provider configuration and runtime prerequisites at the composition root. */ export const parseConfig = ( environment: Readonly>, ): Result => { - // JEB validates its own timeout during optional provider composition. It is - // not an AppConfig setting and must not prevent unrelated CLI/MCP startup. - // Registration validation still checks the complete environment directly. - const { REA_JEB_MCP_TIMEOUT_MS: _jebTimeout, ...appEnvironment } = - environment; - const parsed = parseEnvironment(appEnvironment); + const parsed = parseEnvironment(nativeAnalysisEnvironment(environment)); if (!parsed.ok) return parsed; const env = parsed.value; return ok({ diff --git a/src/kuna/KunaDoctor.ts b/src/kuna/KunaDoctor.ts index b5fbfe43a6..d4a533ab72 100644 --- a/src/kuna/KunaDoctor.ts +++ b/src/kuna/KunaDoctor.ts @@ -1,6 +1,6 @@ import { constants } from "node:fs"; -import { access, stat } from "node:fs/promises"; -import { dirname } from "node:path"; +import { access, lstat, stat } from "node:fs/promises"; +import { dirname, posix, win32 } from "node:path"; import type { DoctorProviderCheck, @@ -151,10 +151,11 @@ export const inspectKunaProvider = async ( }; } const { value } = installation; + const cache = await cacheCheck(environment, platform); return { id: KUNA_PROVIDER_ID, configured, - available: true, + available: cache.ok, providerVersion: value.version, registrationEnvironment: Object.fromEntries( REGISTRATION_SETTINGS.flatMap((name) => { @@ -181,7 +182,7 @@ export const inspectKunaProvider = async ( sha256: value.sha256, }, }, - await cacheCheck(environment, platform), + cache, ], }; }; @@ -226,8 +227,10 @@ const cacheCheck = async ( }; const { cacheRoot, cacheMaxBytes } = configuration.value; const details = { cache_root: cacheRoot, cache_max_bytes: cacheMaxBytes }; + // A trailing separator makes lstat follow a dangling link and report ENOENT. + const inspected = cacheRootWithoutTrailingSeparator(cacheRoot, platform); try { - if (!(await stat(cacheRoot)).isDirectory()) + if (!(await stat(inspected)).isDirectory()) return { name: "cache", ok: false, @@ -238,10 +241,10 @@ const cacheCheck = async ( classification: "config_drift", details, }; - await access(cacheRoot, constants.W_OK | constants.X_OK); + await access(inspected, constants.W_OK | constants.X_OK); } catch (cause: unknown) { - if (cause instanceof Error && "code" in cause && cause.code === "ENOENT") { - const creatable = await nearestDirectoryWritable(cacheRoot); + if (isMissing(cause)) { + const creatable = await nearestDirectoryWritable(inspected); return creatable.ok ? { name: "cache", @@ -285,13 +288,18 @@ const cacheCheck = async ( }; }; -/** Whether the nearest existing ancestor is a directory REA may create entries in. */ +/** + * Whether the nearest existing entry on `path` is a directory REA may create + * entries in. A missing target is only absent when no entry occupies its name; + * a dangling symbolic link blocks creation just as a regular file does. + * `path` must not end in a separator, or lstat follows a dangling link. + */ const nearestDirectoryWritable = async ( path: string, ): Promise< { readonly ok: true } | { readonly ok: false; readonly reason: string } > => { - let candidate = dirname(path); + let candidate = path; for (;;) { try { if (!(await stat(candidate)).isDirectory()) @@ -299,15 +307,50 @@ const nearestDirectoryWritable = async ( await access(candidate, constants.W_OK | constants.X_OK); return { ok: true }; } catch (cause: unknown) { - const missing = - cause instanceof Error && "code" in cause && cause.code === "ENOENT"; const parent = dirname(candidate); - if (!missing || parent === candidate) + const absent = isMissing(cause) ? await entryAbsent(candidate) : false; + if (absent !== true || parent === candidate) return { ok: false, - reason: `${candidate} is not writable: ${cause instanceof Error ? cause.message : String(cause)}`, + reason: + typeof absent === "string" + ? absent + : `${candidate} is not writable: ${cause instanceof Error ? cause.message : String(cause)}`, }; candidate = parent; } } }; + +const isMissing = (cause: unknown): boolean => + cause instanceof Error && "code" in cause && cause.code === "ENOENT"; + +/** True when nothing occupies `path`; otherwise why its name is unusable. */ +const entryAbsent = async (path: string): Promise => { + try { + return (await lstat(path)).isSymbolicLink() + ? `${path} is a symbolic link whose target does not exist.` + : `${path} exists but cannot be resolved.`; + } catch (cause: unknown) { + return isMissing(cause) + ? true + : `${path} cannot be inspected: ${cause instanceof Error ? cause.message : String(cause)}`; + } +}; + +/** + * Drop a terminal separator without turning a filesystem root into its parent. + * On Darwin and Linux, `\` is a filename character, so only `/` is removed. + */ +const cacheRootWithoutTrailingSeparator = ( + cacheRoot: string, + platform: NodeJS.Platform, +): string => { + const pathApi = platform === "win32" ? win32 : posix; + const root = pathApi.parse(cacheRoot).root; + const separator = pathApi.sep.charCodeAt(0); + let end = cacheRoot.length; + while (end > root.length && cacheRoot.charCodeAt(end - 1) === separator) + end -= 1; + return end === cacheRoot.length ? cacheRoot : cacheRoot.slice(0, end); +}; diff --git a/src/logger.ts b/src/logger.ts index 05fc3822fe..6e5280cd8e 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -4,6 +4,18 @@ import { z } from "zod"; import { ConfigurationError } from "./domain/configurationErrors.js"; import { err, ok, type Result } from "./domain/result.js"; +/** + * Settings the executable rejects before a command starts. MCP startup rejects + * the same values. Doctor requires them in every readiness scope. + */ +export const PROCESS_START_CONFIGURATION_SETTINGS = ["REA_LOG_LEVEL"] as const; + +/** True when `setting` prevents the REA process from starting. */ +export const blocksProcessStart = (setting: string): boolean => + PROCESS_START_CONFIGURATION_SETTINGS.some( + (candidate) => candidate === setting, + ); + /** Supported structured log levels at the process boundary. */ export type LogLevel = z.infer; diff --git a/tests/boundary/cli/cliLogLevelConfiguration.test.ts b/tests/boundary/cli/cliLogLevelConfiguration.test.ts index cc09a3b944..fb6490091f 100644 --- a/tests/boundary/cli/cliLogLevelConfiguration.test.ts +++ b/tests/boundary/cli/cliLogLevelConfiguration.test.ts @@ -2,20 +2,59 @@ import { expect } from "vitest"; import { cliTest } from "../../support/cli/cliFixture.js"; +const invalidLogLevelMessage = + 'REA configuration is invalid: REA_LOG_LEVEL: Invalid option: expected one of "trace"|"debug"|"info"|"warn"|"error"|"fatal"|"silent".\n'; + for (const level of ["loud", "INFO"]) cliTest( `reports REA_LOG_LEVEL=${level} as invalid configuration`, async ({ cli }) => { - for (const arguments_ of [["doctor", "--json"], ["--help"]]) { - const result = await cli.run({ - arguments: arguments_, - environment: { REA_LOG_LEVEL: level }, - }); - expect(result.exitCode).toBe(1); - expect(result.stdout).toBe(""); - expect(result.stderr).toBe( - 'REA configuration is invalid: REA_LOG_LEVEL: Invalid option: expected one of "trace"|"debug"|"info"|"warn"|"error"|"fatal"|"silent".\n', - ); - } + const help = await cli.run({ + arguments: ["--help"], + environment: { REA_LOG_LEVEL: level }, + }); + expect(help.exitCode).toBe(1); + expect(help.stdout).toBe(""); + expect(help.stderr).toBe(invalidLogLevelMessage); + + const doctor = await cli.run({ + arguments: ["--json", "doctor", "--provider", "hopper"], + environment: { REA_LOG_LEVEL: level }, + }); + expect(doctor.exitCode, doctor.stderr).toBe(1); + expect(doctor.stderr).toBe(""); + expect(doctor.json).toMatchObject({ + healthy: false, + scope_checks: expect.arrayContaining([ + expect.objectContaining({ + name: "runtime-configuration:REA_LOG_LEVEL", + ok: false, + classification: "config_drift", + detail: expect.stringContaining("REA_LOG_LEVEL"), + }), + ]), + }); + + const client = await cli.run({ + arguments: ["--json", "doctor", "--client", "codex"], + environment: { REA_LOG_LEVEL: level }, + }); + expect(client.exitCode, client.stderr).toBe(1); + expect(client.stderr).toBe(""); + expect(client.json).toMatchObject({ + healthy: false, + scope: { mode: "explicit", clients: ["codex"], providers: [] }, + scope_checks: expect.arrayContaining([ + expect.objectContaining({ + name: "runtime-configuration:REA_LOG_LEVEL", + ok: false, + }), + ]), + informational_checks: expect.not.arrayContaining([ + expect.objectContaining({ + name: "runtime-configuration:REA_LOG_LEVEL", + }), + ]), + }); }, ); diff --git a/tests/boundary/cli/doctorHopperDiscovery.test.ts b/tests/boundary/cli/doctorHopperDiscovery.test.ts index 1a33db49cd..c613dfd2d8 100644 --- a/tests/boundary/cli/doctorHopperDiscovery.test.ts +++ b/tests/boundary/cli/doctorHopperDiscovery.test.ts @@ -60,6 +60,75 @@ cliTest.skipIf(!["darwin", "linux"].includes(process.platform))( }, ); +cliTest.skipIf(!["darwin", "linux"].includes(process.platform))( + "reports native configuration that blocks the selected engine", + async ({ cli }) => { + const home = await createTestTempDirectory("rea-doctor-native-config-"); + const shared = { + HOME: home, + USERPROFILE: home, + GHIDRA_INSTALL_DIR: join(home, "missing-ghidra"), + HOPPER_LAUNCHER_PATH: join(home, "missing-hopper"), + }; + const invalid = await cli.run({ + arguments: ["doctor", "--provider", "hopper", "--json"], + environment: { + ...shared, + REA_ANALYSIS_PROVIDER: "Hopper", + REA_GHIDRA_STARTUP_TIMEOUT_MS: "abc", + }, + }); + expect(invalid.exitCode, invalid.stderr).toBe(1); + expect(invalid.json).toMatchObject({ + healthy: false, + scope_checks: expect.arrayContaining([ + expect.objectContaining({ + name: "runtime-configuration:REA_ANALYSIS_PROVIDER", + ok: false, + classification: "config_drift", + }), + expect.objectContaining({ + name: "runtime-configuration:REA_GHIDRA_STARTUP_TIMEOUT_MS", + ok: false, + classification: "config_drift", + }), + ]), + }); + + const jebOnly = await cli.run({ + arguments: ["doctor", "--provider", "hopper", "--json"], + environment: { ...shared, REA_JEB_MCP_TIMEOUT_MS: "abc" }, + }); + const checks = (jebOnly.json as { checks?: { name: string }[] } | undefined) + ?.checks; + expect( + checks?.some((check) => check.name.startsWith("runtime-configuration:")), + ).toBe(false); + }, +); + +cliTest( + "reports MCP startup failures for a selected client", + async ({ cli }) => { + const doctor = await cli.run({ + arguments: ["--json", "doctor", "--client", "codex"], + environment: { REA_GHIDRA_STARTUP_TIMEOUT_MS: "abc" }, + }); + expect(doctor.exitCode, doctor.stderr).toBe(1); + expect(doctor.stderr).toBe(""); + expect(doctor.json).toMatchObject({ + healthy: false, + scope: { mode: "explicit", clients: ["codex"], providers: [] }, + scope_checks: expect.arrayContaining([ + expect.objectContaining({ + name: "runtime-configuration:REA_GHIDRA_STARTUP_TIMEOUT_MS", + ok: false, + }), + ]), + }); + }, +); + cliTest.skipIf(!["darwin", "linux"].includes(process.platform))( "retains a configured Hopper path in host diagnostics without launching it", async ({ cli }) => { diff --git a/tests/boundary/providers/kuna/kunaDoctor.test.ts b/tests/boundary/providers/kuna/kunaDoctor.test.ts index 758b94d570..5a3acd0aef 100644 --- a/tests/boundary/providers/kuna/kunaDoctor.test.ts +++ b/tests/boundary/providers/kuna/kunaDoctor.test.ts @@ -1,5 +1,5 @@ -import { mkdir, writeFile } from "node:fs/promises"; -import { join } from "node:path"; +import { lstat, mkdir, readlink, symlink, writeFile } from "node:fs/promises"; +import { join, sep as separator } from "node:path"; import { expect, it as test } from "vitest"; @@ -97,11 +97,17 @@ it("reports a cache root REA cannot use without creating anything", async () => REA_KUNA_CACHE_DIR: existing, }); + expect(blocked.available).toBe(false); + expect(blocked.checks[0]).toMatchObject({ name: "installation", ok: true }); expect(blocked.checks[1]).toMatchObject({ name: "cache", ok: false, code: "open_options_invalid", }); + expect(blocked.registrationEnvironment).toMatchObject({ + REA_KUNA_COMMAND: kuna, + REA_KUNA_CACHE_DIR: occupied, + }); expect(ready.checks[1]).toMatchObject({ name: "cache", ok: true, @@ -139,6 +145,93 @@ it("reports a cache root REA cannot create under its nearest existing parent", a }); }); +it("reports a dangling cache link as unusable and accepts a resolving link", async () => { + const root = await createTestTempDirectory("rea-kuna-doctor-link-"); + const kuna = await writeFakeKuna(root); + const dangling = join(root, "dangling"); + const missing = join(root, "missing-target"); + await symlink(missing, dangling); + const target = join(root, "target"); + await mkdir(target); + const resolving = join(root, "resolving"); + await symlink(target, resolving); + + const direct = await inspectKunaProvider({ + REA_KUNA_COMMAND: kuna, + REA_KUNA_CACHE_DIR: dangling, + }); + const trailing = await inspectKunaProvider({ + REA_KUNA_COMMAND: kuna, + REA_KUNA_CACHE_DIR: `${dangling}${separator}`, + }); + const beneath = await inspectKunaProvider({ + REA_KUNA_COMMAND: kuna, + REA_KUNA_CACHE_DIR: join(dangling, "nested", "cache"), + }); + const ready = await inspectKunaProvider({ + REA_KUNA_COMMAND: kuna, + REA_KUNA_CACHE_DIR: resolving, + }); + const readyTrailing = await inspectKunaProvider({ + REA_KUNA_COMMAND: kuna, + REA_KUNA_CACHE_DIR: `${resolving}${separator}`, + }); + + for (const blocked of [direct, trailing, beneath]) { + expect(blocked.available).toBe(false); + expect(blocked.checks[1]).toMatchObject({ + name: "cache", + ok: false, + code: "open_options_invalid", + detail: expect.stringContaining( + `${dangling} is a symbolic link whose target does not exist.`, + ), + }); + } + expect(trailing.checks[1]).toMatchObject({ + detail: expect.stringContaining(`${dangling}${separator}`), + details: { cache_root: `${dangling}${separator}` }, + }); + for (const usable of [ready, readyTrailing]) { + expect(usable.available).toBe(true); + expect(usable.checks[1]).toMatchObject({ + name: "cache", + ok: true, + detail: expect.stringContaining("cached in"), + }); + } + expect(ready.checks[1]?.detail).toContain(resolving); + // Diagnosis neither creates the cache nor repairs the link. + expect(await readlink(dangling)).toBe(missing); + await expect(lstat(missing)).rejects.toMatchObject({ code: "ENOENT" }); + await expect(lstat(join(dangling, "nested"))).rejects.toThrow(); +}); + +it("keeps a trailing backslash as part of a POSIX cache path", async () => { + const root = await createTestTempDirectory("rea-kuna-doctor-backslash-"); + const kuna = await writeFakeKuna(root); + const directory = join(root, "cache"); + await mkdir(directory); + const named = `${directory}\\`; + await writeFile(named, "not-a-directory"); + + const inspection = await inspectKunaProvider({ + REA_KUNA_COMMAND: kuna, + REA_KUNA_CACHE_DIR: named, + }); + + expect(inspection.available).toBe(false); + expect(inspection.checks[1]).toMatchObject({ + name: "cache", + ok: false, + code: "open_options_invalid", + detail: expect.stringContaining(`${named} exists but is not a directory.`), + details: expect.objectContaining({ cache_root: named }), + }); + expect((await lstat(directory)).isDirectory()).toBe(true); + expect((await lstat(named)).isFile()).toBe(true); +}); + const doctorEnvironment = (root: string, kuna: string): NodeJS.ProcessEnv => ({ HOME: root, PATH: "", diff --git a/tests/boundary/providers/kuna/kunaSetupReadiness.test.ts b/tests/boundary/providers/kuna/kunaSetupReadiness.test.ts index 95ca8eb2e6..9367c6e987 100644 --- a/tests/boundary/providers/kuna/kunaSetupReadiness.test.ts +++ b/tests/boundary/providers/kuna/kunaSetupReadiness.test.ts @@ -34,7 +34,11 @@ test.each([ REA_KUNA_COMMAND: command, REA_KUNA_CACHE_DIR: cachePath, }); - expect(inspection.available).toBe(true); + expect(inspection.available).toBe(cache === "ready"); + expect(inspection.checks[0]).toMatchObject({ + name: "installation", + ok: true, + }); expect(inspection.checks[1]).toMatchObject({ name: "cache", ok: cache === "ready",