mirror of
https://github.com/morluto/rea
synced 2026-10-25 22:08:11 -04:00
refactor(doctor): retain parsed configuration diagnostics
Return native admission issues and an explicit Hopper timeout result from the configuration owner. Doctor consumes those parsed facts instead of raw timeout text plus a separate issue list. Reuse a successful aggregate parse; validate the individual timeout independently when other settings reject it. Preserve aggregate error precedence, JEB isolation, omission, constraints, causes and provider scope. Keep partial diagnostics when unrelated NUL, loader arguments, language selection or timeout settings are invalid. Validation: configuration and Doctor workflow regressions, mixed-invalid settings, full static/docs checks. No installed provider was exercised.
This commit is contained in:
@@ -18,8 +18,10 @@ default, any floor, the maximum, the schema, the parser, and the recovery
|
||||
text. Register that schema on `environmentSchema` in
|
||||
[`environment.ts`](https://github.com/morluto/rea/blob/main/src/config/environment.ts).
|
||||
`parseConfig` calls `parseEnvironment`. Doctor uses that same parser through
|
||||
`nativeAnalysisConfigurationError`, so startup and `rea doctor` accept the
|
||||
same values. Add the name to `SELECTED_REGISTRATION_SETTINGS` so `rea setup`
|
||||
`inspectNativeAnalysisConfiguration`, which also preserves a supplied Hopper
|
||||
timeout's parsed diagnostic when another setting is invalid. Startup and
|
||||
`rea doctor` accept the same values. Add the name to
|
||||
`SELECTED_REGISTRATION_SETTINGS` so `rea setup`
|
||||
copies it into the client registration. Unset keeps today's behavior. An
|
||||
invalid value is a `ConfigurationError` and projects as `configuration_invalid`.
|
||||
Do not fall back to the default.
|
||||
|
||||
@@ -12,6 +12,16 @@ import { HopperReleaseError } from "./HopperReleaseFeed.js";
|
||||
import { MAC_HOPPER_SYSTEM_LAUNCHER } from "./MacHopperPaths.js";
|
||||
import { createTestTempDirectory } from "../../tests/fixtures/temporaryDirectory.js";
|
||||
|
||||
const configurationHost = (
|
||||
environment: Readonly<NodeJS.ProcessEnv>,
|
||||
): DoctorHost => ({
|
||||
...systemDoctorHost({ environment: { ...environment } }),
|
||||
...host({
|
||||
installationPaths: () => Promise.resolve([]),
|
||||
clientRegistrations: () => Promise.resolve([]),
|
||||
}),
|
||||
});
|
||||
|
||||
describe("doctor Hopper startup timeout", () => {
|
||||
const timeoutHost = (
|
||||
value: string | undefined,
|
||||
@@ -157,16 +167,6 @@ describe("doctor Hopper startup timeout", () => {
|
||||
});
|
||||
|
||||
describe("doctor native analysis configuration", () => {
|
||||
const configurationHost = (
|
||||
environment: Readonly<NodeJS.ProcessEnv>,
|
||||
): DoctorHost => ({
|
||||
...systemDoctorHost({ environment: { ...environment } }),
|
||||
...host({
|
||||
installationPaths: () => Promise.resolve([]),
|
||||
clientRegistrations: () => Promise.resolve([]),
|
||||
}),
|
||||
});
|
||||
|
||||
it.each([
|
||||
["REA_ANALYSIS_PROVIDER", "Hopper"],
|
||||
["REA_REGEX_MATCHING_DEADLINE_MS", "0"],
|
||||
@@ -202,9 +202,78 @@ describe("doctor native analysis configuration", () => {
|
||||
details: { setting },
|
||||
}),
|
||||
);
|
||||
expect(
|
||||
result.checks.some(({ name }) => name === "hopper-startup-timeout"),
|
||||
).toBe(false);
|
||||
},
|
||||
);
|
||||
|
||||
it.each([
|
||||
["REA_GHIDRA_STARTUP_TIMEOUT_MS", "abc"],
|
||||
["HOPPER_LOADER_ARGS_JSON", "{"],
|
||||
["REA_GHIDRA_COMPILER_SPEC_ID", "gcc"],
|
||||
["CALLER_SETTING", "private\0value"],
|
||||
] as const)(
|
||||
"keeps a valid supplied Hopper timeout when %s rejects native admission",
|
||||
async (setting, value) => {
|
||||
const result = await runDoctor(
|
||||
undefined,
|
||||
configurationHost({
|
||||
[setting]: value,
|
||||
REA_HOPPER_STARTUP_TIMEOUT_MS: "120001",
|
||||
}),
|
||||
{ providers: ["hopper"] },
|
||||
);
|
||||
expect(result.healthy).toBe(false);
|
||||
expect(result.scope_checks).toContainEqual(
|
||||
expect.objectContaining({
|
||||
name: "hopper-startup-timeout",
|
||||
ok: true,
|
||||
detail: "REA_HOPPER_STARTUP_TIMEOUT_MS=120001",
|
||||
}),
|
||||
);
|
||||
expect(result.scope_checks).toContainEqual(
|
||||
expect.objectContaining({
|
||||
name: `runtime-configuration:${setting}`,
|
||||
ok: false,
|
||||
}),
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it("reports an invalid Hopper timeout alongside an unrelated NUL admission refusal", async () => {
|
||||
const result = await runDoctor(
|
||||
undefined,
|
||||
configurationHost({
|
||||
CALLER_SETTING: "private\0value",
|
||||
REA_HOPPER_STARTUP_TIMEOUT_MS: "0",
|
||||
}),
|
||||
{ providers: ["hopper"] },
|
||||
);
|
||||
expect(result.healthy).toBe(false);
|
||||
expect(result.scope_checks).toContainEqual(
|
||||
expect.objectContaining({
|
||||
name: "hopper-startup-timeout",
|
||||
ok: false,
|
||||
detail:
|
||||
"REA_HOPPER_STARTUP_TIMEOUT_MS: must be a positive decimal integer",
|
||||
}),
|
||||
);
|
||||
expect(result.scope_checks).toContainEqual(
|
||||
expect.objectContaining({
|
||||
name: "runtime-configuration:CALLER_SETTING",
|
||||
ok: false,
|
||||
detail: expect.stringContaining("NUL"),
|
||||
}),
|
||||
);
|
||||
expect(
|
||||
result.scope_checks.filter(
|
||||
({ name }) => name === "hopper-startup-timeout",
|
||||
),
|
||||
).toHaveLength(1);
|
||||
expect(JSON.stringify(result.checks)).not.toContain("private");
|
||||
});
|
||||
|
||||
it("keeps a lowercase provider preference ready for the selected engine", async () => {
|
||||
const result = await runDoctor(
|
||||
undefined,
|
||||
@@ -257,15 +326,6 @@ describe("doctor native analysis configuration", () => {
|
||||
});
|
||||
|
||||
describe("doctor MCP startup readiness", () => {
|
||||
const configurationHost = (
|
||||
environment: Readonly<NodeJS.ProcessEnv>,
|
||||
): DoctorHost => ({
|
||||
...systemDoctorHost({ environment: { ...environment } }),
|
||||
...host({
|
||||
installationPaths: () => Promise.resolve([]),
|
||||
clientRegistrations: () => Promise.resolve([]),
|
||||
}),
|
||||
});
|
||||
const alignedCodex: ClientRegistrationStatus = {
|
||||
client: "codex",
|
||||
config_path: "/home/user/.codex/config.toml",
|
||||
|
||||
@@ -11,9 +11,11 @@ import { constants } from "node:fs";
|
||||
import { access, stat } from "node:fs/promises";
|
||||
import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js";
|
||||
import type { RetainedResourceScope } from "./AnalysisProvider.js";
|
||||
import { nativeAnalysisConfigurationError } from "../config/parseConfig.js";
|
||||
import {
|
||||
inspectNativeAnalysisConfiguration,
|
||||
type NativeAnalysisConfigurationInspection,
|
||||
} 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";
|
||||
|
||||
@@ -108,12 +110,11 @@ export interface DoctorHost {
|
||||
readonly runtime: JavaScriptRuntime;
|
||||
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 nativeConfiguration?: NativeAnalysisConfigurationInspection;
|
||||
readonly configuredIlspyCmdPath?: string;
|
||||
macosVersion(): Promise<string | undefined>;
|
||||
linuxDistribution(): Promise<LinuxDistribution | undefined>;
|
||||
@@ -539,7 +540,7 @@ export const systemDoctorHost = (
|
||||
const platform = options.platform ?? process.platform;
|
||||
const architecture = options.architecture ?? process.arch;
|
||||
const environment = options.environment ?? process.env;
|
||||
const configurationError = nativeAnalysisConfigurationError(
|
||||
const nativeConfiguration = inspectNativeAnalysisConfiguration(
|
||||
snapshotEnvironment(environment, platform),
|
||||
);
|
||||
const hostExecFileOutput = options.execFileOutput ?? execFileOutput;
|
||||
@@ -576,13 +577,7 @@ export const systemDoctorHost = (
|
||||
...(environment.HOPPER_LAUNCHER_PATH === undefined
|
||||
? {}
|
||||
: { configuredHopperPath: environment.HOPPER_LAUNCHER_PATH }),
|
||||
...(environment.REA_HOPPER_STARTUP_TIMEOUT_MS === undefined
|
||||
? {}
|
||||
: {
|
||||
configuredHopperStartupTimeout:
|
||||
environment.REA_HOPPER_STARTUP_TIMEOUT_MS,
|
||||
}),
|
||||
configurationIssues: configurationError?.settings ?? [],
|
||||
nativeConfiguration,
|
||||
...(environment.REA_ILSPY_CMD_PATH === undefined
|
||||
? {}
|
||||
: { configuredIlspyCmdPath: environment.REA_ILSPY_CMD_PATH }),
|
||||
|
||||
@@ -2,7 +2,6 @@ import {
|
||||
SUPPORTED_NODE_VERSION_PROSE,
|
||||
supportsJavaScriptRuntime,
|
||||
} from "../domain/runtimeVersion.js";
|
||||
import { providerStartupTimeoutSchema } from "../config/providerStartupTimeout.js";
|
||||
import { hopperLauncherDiscovery } from "./HopperLauncherDiscovery.js";
|
||||
import { LINUX_HOPPER_SETUP_HOSTS, supportsReaHost } from "./LinuxHopper.js";
|
||||
import type {
|
||||
@@ -193,16 +192,14 @@ const hopperCheck = (
|
||||
const optionalHopperStartupTimeoutCheck = (
|
||||
host: DoctorHost,
|
||||
): DoctorCheck | undefined => {
|
||||
if (host.configuredHopperStartupTimeout === undefined) return undefined;
|
||||
const parsed = providerStartupTimeoutSchema.safeParse(
|
||||
host.configuredHopperStartupTimeout,
|
||||
);
|
||||
const parsed = host.nativeConfiguration?.configuredHopperStartupTimeout;
|
||||
if (parsed === undefined) return undefined;
|
||||
return check(
|
||||
"hopper-startup-timeout",
|
||||
parsed.success,
|
||||
parsed.success
|
||||
? `REA_HOPPER_STARTUP_TIMEOUT_MS=${String(parsed.data)}`
|
||||
: `REA_HOPPER_STARTUP_TIMEOUT_MS: ${parsed.error.issues.map(({ message }) => message).join("; ")}`,
|
||||
parsed.ok,
|
||||
parsed.ok
|
||||
? `REA_HOPPER_STARTUP_TIMEOUT_MS=${String(parsed.value)}`
|
||||
: `REA_HOPPER_STARTUP_TIMEOUT_MS: ${parsed.error.settings.map(({ constraint }) => constraint).join("; ")}`,
|
||||
{
|
||||
classification: "config_drift",
|
||||
remediation:
|
||||
@@ -219,7 +216,7 @@ const optionalHopperStartupTimeoutCheck = (
|
||||
const runtimeConfigurationChecks = (
|
||||
host: DoctorHost,
|
||||
): readonly DoctorCheck[] => {
|
||||
const issues = host.configurationIssues;
|
||||
const issues = host.nativeConfiguration?.issues;
|
||||
if (issues === undefined || issues.length === 0) return [];
|
||||
const grouped: { setting: string; constraints: string[] }[] = [];
|
||||
for (const issue of issues) {
|
||||
|
||||
+33
-6
@@ -2,7 +2,7 @@ import { describe, expect, it } from "vitest";
|
||||
|
||||
import {
|
||||
defaultHopperLauncherPath,
|
||||
nativeAnalysisConfigurationError,
|
||||
inspectNativeAnalysisConfiguration,
|
||||
parseConfig,
|
||||
} from "./config/parseConfig.js";
|
||||
import {
|
||||
@@ -100,16 +100,16 @@ describe("runtime configuration", () => {
|
||||
});
|
||||
|
||||
it("shares native admission with doctor and ignores the JEB-only timeout", () => {
|
||||
expect(nativeAnalysisConfigurationError({})).toBeUndefined();
|
||||
expect(inspectNativeAnalysisConfiguration({})).toEqual({ issues: [] });
|
||||
expect(
|
||||
nativeAnalysisConfigurationError({ REA_JEB_MCP_TIMEOUT_MS: "abc" }),
|
||||
).toBeUndefined();
|
||||
inspectNativeAnalysisConfiguration({ REA_JEB_MCP_TIMEOUT_MS: "abc" }),
|
||||
).toEqual({ issues: [] });
|
||||
expect(parseConfig({ REA_JEB_MCP_TIMEOUT_MS: "abc" }).ok).toBe(true);
|
||||
const rejected = nativeAnalysisConfigurationError({
|
||||
const rejected = inspectNativeAnalysisConfiguration({
|
||||
REA_ANALYSIS_PROVIDER: "Hopper",
|
||||
REA_GHIDRA_STARTUP_TIMEOUT_MS: "abc",
|
||||
});
|
||||
expect(rejected?.settings.map(({ setting }) => setting)).toEqual([
|
||||
expect(rejected.issues.map(({ setting }) => setting)).toEqual([
|
||||
"REA_ANALYSIS_PROVIDER",
|
||||
"REA_GHIDRA_STARTUP_TIMEOUT_MS",
|
||||
]);
|
||||
@@ -121,6 +121,33 @@ describe("runtime configuration", () => {
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("preserves the timeout's schema cause when aggregate admission stops at NUL", () => {
|
||||
const inspected = inspectNativeAnalysisConfiguration({
|
||||
CALLER_SETTING: "private\0value",
|
||||
REA_HOPPER_STARTUP_TIMEOUT_MS: "0",
|
||||
});
|
||||
expect(inspected.issues).toEqual([
|
||||
{
|
||||
setting: "CALLER_SETTING",
|
||||
constraint:
|
||||
"Environment variable names and values must not contain NUL bytes",
|
||||
},
|
||||
]);
|
||||
const timeout = inspected.configuredHopperStartupTimeout;
|
||||
if (timeout === undefined || timeout.ok)
|
||||
throw new Error("Expected the configured timeout's independent refusal");
|
||||
expect(timeout.error.cause).toMatchObject({
|
||||
name: "ZodError",
|
||||
issues: [{ message: "must be a positive decimal integer" }],
|
||||
});
|
||||
expect(timeout.error.settings).toEqual([
|
||||
{
|
||||
setting: "REA_HOPPER_STARTUP_TIMEOUT_MS",
|
||||
constraint: "must be a positive decimal integer",
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("parses absolute BYO Ghidra and optional Java paths", () => {
|
||||
expect(
|
||||
parseConfig({
|
||||
|
||||
@@ -1,8 +1,12 @@
|
||||
import { accessSync, constants, statSync } from "node:fs";
|
||||
|
||||
import type { ConfigurationError } from "../domain/configurationErrors.js";
|
||||
import { ok, type Result } from "../domain/result.js";
|
||||
import {
|
||||
ConfigurationError,
|
||||
type ConfigurationSettingIssue,
|
||||
} from "../domain/configurationErrors.js";
|
||||
import { err, ok, type Result } from "../domain/result.js";
|
||||
import { parseEnvironment } from "./environment.js";
|
||||
import { providerStartupTimeoutSchema } from "./providerStartupTimeout.js";
|
||||
import { homeDirectoryFromEnvironment } from "./homeDirectory.js";
|
||||
import { DEFAULT_BRIDGE_MAX_REQUEST } from "./bridgeMaxRequestBytes.js";
|
||||
import { DEFAULT_REGEX_MATCHING_DEADLINE } from "./regexMatchingDeadline.js";
|
||||
@@ -48,15 +52,44 @@ const nativeAnalysisEnvironment = (
|
||||
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 = (
|
||||
/** Native admission issues and an explicitly configured timeout's independent facts. */
|
||||
export interface NativeAnalysisConfigurationInspection {
|
||||
readonly issues: readonly ConfigurationSettingIssue[];
|
||||
readonly configuredHopperStartupTimeout?: Result<number, ConfigurationError>;
|
||||
}
|
||||
|
||||
/** Inspect native admission without probing launchers or losing mixed-invalid diagnostics. */
|
||||
export const inspectNativeAnalysisConfiguration = (
|
||||
environment: Readonly<Record<string, string | undefined>>,
|
||||
): ConfigurationError | undefined => {
|
||||
): NativeAnalysisConfigurationInspection => {
|
||||
const parsed = parseEnvironment(nativeAnalysisEnvironment(environment));
|
||||
return parsed.ok ? undefined : parsed.error;
|
||||
const issues = parsed.ok ? [] : parsed.error.settings;
|
||||
const configuredTimeout = environment.REA_HOPPER_STARTUP_TIMEOUT_MS;
|
||||
if (configuredTimeout === undefined) return { issues };
|
||||
if (parsed.ok)
|
||||
return {
|
||||
issues,
|
||||
configuredHopperStartupTimeout: ok(
|
||||
parsed.value.REA_HOPPER_STARTUP_TIMEOUT_MS,
|
||||
),
|
||||
};
|
||||
// Another setting may reject the aggregate before this supplied timeout's
|
||||
// facts are available. Its dedicated diagnostic still uses the same schema.
|
||||
const timeout = providerStartupTimeoutSchema.safeParse(configuredTimeout);
|
||||
return {
|
||||
issues,
|
||||
configuredHopperStartupTimeout: timeout.success
|
||||
? ok(timeout.data)
|
||||
: err(
|
||||
new ConfigurationError("Invalid REA environment configuration", {
|
||||
cause: timeout.error,
|
||||
settings: timeout.error.issues.map(({ message }) => ({
|
||||
setting: "REA_HOPPER_STARTUP_TIMEOUT_MS",
|
||||
constraint: message,
|
||||
})),
|
||||
}),
|
||||
),
|
||||
};
|
||||
};
|
||||
|
||||
/** Parse provider configuration and runtime prerequisites at the composition root. */
|
||||
|
||||
Reference in New Issue
Block a user