From 16cfe30dabd3235cc22a6fa62d8d4c33d0b7e511 Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Thu, 29 Oct 2026 07:34:43 +0800 Subject: [PATCH] fix(setup): identify invalid launch environment overrides (#7899) * fix(setup): identify invalid launch environment overrides * test(setup): cover merged environment recovery --- src/application/ClientConfigurationFiles.ts | 51 ++++++++-- .../launchEnvironmentDiagnostics.test.ts | 88 +++++++++++++++++ .../clientConfigurationOverrideOrigin.test.ts | 94 +++++++++++++++++++ 3 files changed, 226 insertions(+), 7 deletions(-) create mode 100644 tests/acceptance/setup/launchEnvironmentDiagnostics.test.ts create mode 100644 tests/boundary/filesystem/clientConfigurationOverrideOrigin.test.ts diff --git a/src/application/ClientConfigurationFiles.ts b/src/application/ClientConfigurationFiles.ts index a40d069c89..a65ce7674c 100644 --- a/src/application/ClientConfigurationFiles.ts +++ b/src/application/ClientConfigurationFiles.ts @@ -22,11 +22,34 @@ export interface ClientConfigurationSource extends ClientConfigurationDocument { const invalidEnvironment = ( path: string, error: ConfigurationError, -): ClientConfigurationFileError => ({ - kind: "malformed", - path, - detail: `Configuration ${path} has an invalid REA server environment: ${error.settings.map(({ setting, constraint }) => `${setting}: ${constraint}`).join("; ")}. Repair it before rerunning setup.`, -}); + overrides: Readonly> = {}, + platform: NodeJS.Platform = process.platform, +): ClientConfigurationFileError => { + const constraints = error.settings + .map(({ setting, constraint }) => `${setting}: ${constraint}`) + .join("; "); + const overrideNames = new Set( + Object.keys(overrides).map((name) => + platform === "win32" ? name.toUpperCase() : name, + ), + ); + const overrideSettings = error.settings + .map(({ setting }) => setting) + .filter((setting) => + overrideNames.has(platform === "win32" ? setting.toUpperCase() : setting), + ); + const location = + overrideSettings.length > 0 + ? `The proposed registration environment includes launch overrides (${overrideSettings.join(", ")}) that do not satisfy the effective REA configuration constraints` + : overrideNames.size > 0 + ? `The effective REA server environment for ${path} is invalid; check the client configuration and proposed launch overrides` + : `Configuration ${path} has an invalid REA server environment`; + return { + kind: "malformed", + path, + detail: `${location}: ${constraints}${constraints.endsWith(".") ? "" : "."} ${overrideNames.size === 0 ? "Repair it" : "Check the launch environment and any existing registration settings"} before rerunning setup.`, + }; +}; /** A named source failure that setup can explain without losing the bad path. */ export interface ClientConfigurationFileError { @@ -110,7 +133,14 @@ export const readClientConfigurationFiles = async ( platform, ); if (!environment.ok) - return err(invalidEnvironment(path, environment.error)); + return err( + invalidEnvironment( + path, + environment.error, + path === client.configPath ? writeOverrides : undefined, + platform, + ), + ); documents.push({ ...document.value, sourcePath: path }); } } @@ -121,7 +151,14 @@ export const readClientConfigurationFiles = async ( platform, ); if (!environment.ok) - return err(invalidEnvironment(client.configPath, environment.error)); + return err( + invalidEnvironment( + client.configPath, + environment.error, + writeOverrides, + platform, + ), + ); return ok(documents); }; diff --git a/tests/acceptance/setup/launchEnvironmentDiagnostics.test.ts b/tests/acceptance/setup/launchEnvironmentDiagnostics.test.ts new file mode 100644 index 0000000000..2f61d9a55d --- /dev/null +++ b/tests/acceptance/setup/launchEnvironmentDiagnostics.test.ts @@ -0,0 +1,88 @@ +import { access, readFile, writeFile } from "node:fs/promises"; +import { join } from "node:path"; +import { expect } from "vitest"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest } from "../../support/cli/cliFixture.js"; + +cliTest( + "identifies rejected launch overrides without blaming or changing a client file", + async ({ cli }) => { + const home = await createTestTempDirectory( + "rea-setup-override-diagnostic-", + ); + const config = join(home, ".claude.json"); + const environment = { + HOME: home, + USERPROFILE: home, + XDG_CONFIG_HOME: join(home, "config"), + XDG_DATA_HOME: join(home, "data"), + XDG_CACHE_HOME: join(home, "cache"), + XDG_STATE_HOME: join(home, "state"), + }; + const run = (overrides: Record) => + cli.run({ + arguments: [ + "setup", + "--client", + "claude_code", + "--yes", + "--skill=false", + "--json", + ], + cwd: home, + environment: { ...environment, ...overrides }, + }); + + const absent = await run({ REA_ASAR_MAX_HEADER_BYTES: "0" }); + expect(absent.exitCode).toBe(1); + expect(absent.json).toMatchObject({ + status: "needs_human", + plannedActions: [], + appliedActions: [], + remediation: expect.stringContaining("launch overrides"), + }); + expect(absent.stdout).toContain("REA_ASAR_MAX_HEADER_BYTES"); + expect(absent.stdout).not.toContain( + `Configuration ${config} has an invalid`, + ); + await expect(access(config)).rejects.toMatchObject({ code: "ENOENT" }); + + const original = JSON.stringify({ + mcpServers: { + rea: { command: "rea", args: ["mcp"], env: { CALLER_KEY: "keep" } }, + }, + }); + await writeFile(config, original); + const existing = await run({ REA_BRIDGE_MAX_REQUEST_BYTES: "1" }); + expect(existing.exitCode).toBe(1); + expect(existing.json).toMatchObject({ + status: "needs_human", + remediation: expect.stringContaining("launch overrides"), + }); + expect(existing.stdout).toContain("REA_BRIDGE_MAX_REQUEST_BYTES"); + expect(await readFile(config, "utf8")).toBe(original); + await expect(access(`${config}.rea.backup`)).rejects.toMatchObject({ + code: "ENOENT", + }); + + const invalidFile = JSON.stringify({ + mcpServers: { + rea: { + command: "rea", + args: ["mcp"], + env: { HOPPER_LOADER_ARGS_JSON: "not-json" }, + }, + }, + }); + await writeFile(config, invalidFile); + const stored = await run({}); + expect(stored.exitCode).toBe(1); + expect(stored.json).toMatchObject({ + status: "needs_human", + remediation: expect.stringContaining(config), + }); + expect(stored.stdout).toContain("HOPPER_LOADER_ARGS_JSON"); + expect(stored.stdout).not.toContain("not-json"); + expect(await readFile(config, "utf8")).toBe(invalidFile); + }, +); diff --git a/tests/boundary/filesystem/clientConfigurationOverrideOrigin.test.ts b/tests/boundary/filesystem/clientConfigurationOverrideOrigin.test.ts new file mode 100644 index 0000000000..6e83335828 --- /dev/null +++ b/tests/boundary/filesystem/clientConfigurationOverrideOrigin.test.ts @@ -0,0 +1,94 @@ +import { join } from "node:path"; +import { writeFile } from "node:fs/promises"; +import { expect, it } from "vitest"; +import { readClientConfigurationFiles } from "../../../src/application/ClientConfigurationFiles.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; + +it("attributes case-folded Windows overrides without disclosing values", async () => { + const root = await createTestTempDirectory("rea-config-override-origin-"); + const configPath = join(root, "client.json"); + const client = { name: "cursor", format: "json", configPath } as const; + const result = await readClientConfigurationFiles( + client, + undefined, + "win32", + { + rea_asar_max_header_bytes: "0", + }, + ); + expect(result).toMatchObject({ + ok: false, + error: { + detail: expect.stringContaining( + "launch overrides (REA_ASAR_MAX_HEADER_BYTES)", + ), + }, + }); +}); + +it("keeps a malformed stored environment attributed to its file", async () => { + const root = await createTestTempDirectory("rea-config-file-origin-"); + const configPath = join(root, "client.json"); + await writeFile( + configPath, + JSON.stringify({ + mcpServers: { + rea: { + command: "rea", + args: ["mcp"], + env: { HOPPER_LOADER_ARGS_JSON: "not-json" }, + }, + }, + }), + ); + const result = await readClientConfigurationFiles({ + name: "cursor", + format: "json", + configPath, + }); + expect(result).toMatchObject({ + ok: false, + error: { detail: expect.stringContaining(`Configuration ${configPath}`) }, + }); + if (!result.ok) expect(result.error.detail).not.toContain("not-json"); +}); + +it.each([ + { + stored: { REA_GHIDRA_LANGUAGE_ID: "x86:LE:64:default" }, + overrides: { REA_GHIDRA_PROJECT_PATH: "/tmp/selected-project.gpr" }, + reported: "REA_GHIDRA_LANGUAGE_ID", + }, + { + stored: { HOPPER_LOADER_ARGS_JSON: "invalid-json" }, + overrides: { REA_ASAR_MAX_HEADER_BYTES: "1024" }, + reported: "HOPPER_LOADER_ARGS_JSON", + }, +])( + "recommends checking both sources when the reported setting is not an override: $reported", + async ({ stored, overrides, reported }) => { + const root = await createTestTempDirectory("rea-config-merged-origin-"); + const configPath = join(root, "client.json"); + await writeFile( + configPath, + JSON.stringify({ + mcpServers: { rea: { command: "rea", args: ["mcp"], env: stored } }, + }), + ); + const result = await readClientConfigurationFiles( + { name: "cursor", format: "json", configPath }, + undefined, + process.platform, + overrides, + ); + if (result.ok) throw new Error("Expected rejected environment"); + expect(result.error.detail).toContain(reported); + expect(result.error.detail).toContain( + "check the client configuration and proposed launch overrides", + ); + expect(result.error.detail).toContain( + "Check the launch environment and any existing registration settings", + ); + expect(result.error.detail).not.toContain("invalid-json"); + }, +);