From e84468c0bb0476412ac0c8161f7c2ecc9a6864ca Mon Sep 17 00:00:00 2001 From: ShuaixinHuang <1836656514@qq.com> Date: Tue, 20 Oct 2026 17:25:51 +0800 Subject: [PATCH] fix(setup): preserve Windows environment override identity (#4946) * fix(setup): preserve Windows environment override identity * fix(setup): preserve selected platform and YAML alias metadata * fix(setup): reconcile layered environment and projected YAML identities --- src/application/ClientConfigurationFiles.ts | 57 ++++- .../ClientRegistrationEnvironment.ts | 31 ++- src/application/ClientRegistrationStatus.ts | 9 +- src/application/DshCordisPatch.ts | 49 +++- src/application/DshCordisYaml.ts | 39 ++- src/application/SetupClientConfiguration.ts | 4 +- src/process/snapshotEnvironment.test.ts | 29 ++- src/process/snapshotEnvironment.ts | 41 ++- .../registrationRuntimeEnvironmentCli.test.ts | 64 +++++ .../clientConfigurationEnvironment.test.ts | 237 ++++++++++++++++++ ...entConfigurationLayeredEnvironment.test.ts | 46 ++++ 11 files changed, 562 insertions(+), 44 deletions(-) diff --git a/src/application/ClientConfigurationFiles.ts b/src/application/ClientConfigurationFiles.ts index 4781e1490d..35e02bcd9b 100644 --- a/src/application/ClientConfigurationFiles.ts +++ b/src/application/ClientConfigurationFiles.ts @@ -12,6 +12,8 @@ import type { SetupClient } from "./SupportedClients.js"; import { clientRegistrationEnvironment } from "./ClientRegistrationEnvironment.js"; import type { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; import type { ConfigurationError } from "../domain/configurationErrors.js"; +import { mergeEnvironmentOverrides } from "../process/snapshotEnvironment.js"; +import { PRODUCT_IDENTITY } from "../identity.js"; /** A parsed layer retains its path so rereading the write target replaces only that layer. */ export interface ClientConfigurationSource extends ClientConfigurationDocument { @@ -88,6 +90,7 @@ export const readConfigurationFile = async ( export const readClientConfigurationFiles = async ( client: SetupClient, resources?: ArtifactResourceScope, + platform: NodeJS.Platform = process.platform, ): Promise< Result > => { @@ -104,6 +107,7 @@ export const readClientConfigurationFiles = async ( document.value, {}, "fragment", + platform, ); if (!environment.ok) return err(invalidEnvironment(path, environment.error)); @@ -111,7 +115,10 @@ export const readClientConfigurationFiles = async ( } } const environment = clientRegistrationEnvironment( - effectiveClientConfiguration(documents), + effectiveClientConfiguration(documents, undefined, platform), + {}, + "effective", + platform, ); if (!environment.ok) return err(invalidEnvironment(client.configPath, environment.error)); @@ -142,6 +149,7 @@ export const effectiveClientConfiguration = ( readonly client: Pick; readonly document: ClientConfigurationDocument | undefined; }, + platform: NodeJS.Platform = process.platform, ): ClientConfigurationDocument | undefined => { const documents = target === undefined @@ -164,11 +172,28 @@ export const effectiveClientConfiguration = ( ...Object.keys(document.legacyServers), ...Object.keys(document.servers), ]); - for (const name of names) - servers[name] = mergeConfigurationValue( - servers[name], - effectiveClientServer(document, name), - ); + for (const name of names) { + const previous = servers[name]; + const next = effectiveClientServer(document, name); + const merged = mergeConfigurationValue(previous, next); + const key = + document.dialect === "opencode" || document.dialect === "opencode_v2" + ? "environment" + : "env"; + if ( + name === PRODUCT_IDENTITY.mcpServerKey && + isObject(merged) && + isObject(merged[key]) + ) { + const before = + isObject(previous) && isObject(previous[key]) ? previous[key] : {}; + const after = isObject(next) && isObject(next[key]) ? next[key] : {}; + servers[name] = { + ...merged, + [key]: mergeEnvironmentOverrides(before, after, platform), + }; + } else servers[name] = merged; + } } return { ...last, servers, legacyServers: {} }; }; @@ -177,6 +202,7 @@ export const effectiveClientConfiguration = ( export const readEffectiveClientRegistrationConfiguration = async ( client: SetupClient, resources?: ArtifactResourceScope, + platform: NodeJS.Platform = process.platform, ): Promise< Result< { @@ -186,9 +212,17 @@ export const readEffectiveClientRegistrationConfiguration = async ( ClientConfigurationFileError > > => { - const nativeSources = await readClientConfigurationFiles(client, resources); + const nativeSources = await readClientConfigurationFiles( + client, + resources, + platform, + ); if (!nativeSources.ok) return nativeSources; - const nativeDocument = effectiveClientConfiguration(nativeSources.value); + const nativeDocument = effectiveClientConfiguration( + nativeSources.value, + undefined, + platform, + ); const fallbackPath = client.format === "zcode" ? client.zcodeFallbackPath : undefined; if ( @@ -207,9 +241,14 @@ export const readEffectiveClientRegistrationConfiguration = async ( const fallbackSources = await readClientConfigurationFiles( fallbackClient, resources, + platform, ); if (!fallbackSources.ok) return fallbackSources; - const fallbackDocument = effectiveClientConfiguration(fallbackSources.value); + const fallbackDocument = effectiveClientConfiguration( + fallbackSources.value, + undefined, + platform, + ); return ok( fallbackDocument === undefined ? { document: nativeDocument, configPath: client.configPath } diff --git a/src/application/ClientRegistrationEnvironment.ts b/src/application/ClientRegistrationEnvironment.ts index 7bbfc7aafb..979a867ff1 100644 --- a/src/application/ClientRegistrationEnvironment.ts +++ b/src/application/ClientRegistrationEnvironment.ts @@ -12,6 +12,10 @@ import { } from "./ClientConfigurationDocument.js"; import { PRODUCT_IDENTITY } from "../identity.js"; import { compareUnicodeCodePoints } from "../domain/unicodeCodePointOrder.js"; +import { + mergeEnvironmentOverrides, + snapshotEnvironment, +} from "../process/snapshotEnvironment.js"; import { dshDocument } from "./DshCordisPatch.js"; import type { McpToolTimeoutInputs } from "../mcpStartupPolicy.js"; import type { AnalysisProviderSelector } from "../contracts/providerSelection.js"; @@ -31,13 +35,20 @@ export const clientRegistrationEnvironment = ( parsed: ClientConfigurationDocument | undefined, overrides: Readonly> = {}, validation: "fragment" | "effective" = "effective", + platform: NodeJS.Platform = process.platform, ): Result>, ConfigurationError> => { if (parsed?.dialect === "dsh") - return dshDocument(parsed.document).environment(overrides, validation); + return dshDocument(parsed.document).environment( + overrides, + validation, + platform, + ); const details = clientRegistrationEnvironmentDetails( parsed, overrides, validation, + undefined, + platform, ); return details.ok ? ok(details.value.values) : details; }; @@ -48,17 +59,20 @@ export const clientRegistrationEnvironmentDetails = ( overrides: Readonly> = {}, validation: "fragment" | "effective" = "effective", resolveProviderRequestBudget?: ProviderRequestBudgetResolver, + platform: NodeJS.Platform = process.platform, ): Result => { if (parsed?.dialect === "dsh") { const environment = dshDocument(parsed.document).environment( overrides, validation, + platform, ); if (!environment.ok) return environment; return validatedDetails( environment.value, validation, resolveProviderRequestBudget, + platform, ); } const key = @@ -88,11 +102,16 @@ export const clientRegistrationEnvironmentDetails = ( ); // Validate known settings with the runtime's parser, but retain the original // string overrides, including unknown client-specific keys and no defaults. - const environment = { ...existing.data, ...overrides }; + const environment = mergeEnvironmentOverrides( + existing.data, + overrides, + platform, + ); return validatedDetails( environment, validation, resolveProviderRequestBudget, + platform, ); }; @@ -100,16 +119,18 @@ const validatedDetails = ( environment: Readonly>, validation: "fragment" | "effective", resolveProviderRequestBudget?: ProviderRequestBudgetResolver, + platform: NodeJS.Platform = process.platform, ): Result => { + const runtimeEnvironment = snapshotEnvironment(environment, platform); const validated = validation === "fragment" - ? parseEnvironmentValues(environment) - : parseEnvironment(environment); + ? parseEnvironmentValues(runtimeEnvironment) + : parseEnvironment(runtimeEnvironment); if (!validated.ok) return validated; const providerRequestBudget = resolveProviderRequestBudget?.( validated.value.REA_ANALYSIS_PROVIDER, - environment, + runtimeEnvironment, ) ?? ok(undefined); if (!providerRequestBudget.ok) return providerRequestBudget; return ok({ diff --git a/src/application/ClientRegistrationStatus.ts b/src/application/ClientRegistrationStatus.ts index 68fc3c37a3..28643628cc 100644 --- a/src/application/ClientRegistrationStatus.ts +++ b/src/application/ClientRegistrationStatus.ts @@ -15,6 +15,7 @@ import { valid } from "semver"; import { RegularFileCleanupFailure } from "./RegularFileRead.js"; import { PRODUCT_IDENTITY } from "../identity.js"; +import { snapshotEnvironment } from "../process/snapshotEnvironment.js"; import { CURRENT_ENTRYPOINT_PATH } from "../process/CurrentEntrypointPath.js"; import { MCP_STARTUP_POLICY, @@ -185,6 +186,7 @@ export const readClientRegistrationInspections = async ( const configuration = await readEffectiveClientRegistrationConfiguration( client, options.resources, + options.platform, ); if (!configuration.ok) { if ( @@ -265,6 +267,7 @@ export const readClientRegistrationInspections = async ( {}, "effective", options.resolveProviderRequestBudget, + options.platform, ) : undefined; if ( @@ -297,8 +300,10 @@ export const readClientRegistrationInspections = async ( platform: options.platform ?? process.platform, kimiInputSchemaProfileSelected: registrationEnvironment?.ok === true && - registrationEnvironment.value.values.REA_MCP_INPUT_SCHEMA_PROFILE !== - undefined, + snapshotEnvironment( + registrationEnvironment.value.values, + options.platform, + ).REA_MCP_INPUT_SCHEMA_PROFILE !== undefined, }); record( configuredStatus( diff --git a/src/application/DshCordisPatch.ts b/src/application/DshCordisPatch.ts index a7d2458b87..630f7cce9f 100644 --- a/src/application/DshCordisPatch.ts +++ b/src/application/DshCordisPatch.ts @@ -7,6 +7,10 @@ import { ConfigurationError } from "../domain/configurationErrors.js"; import { err, ok, type Result } from "../domain/result.js"; import type { ClientConfigurationDocument } from "./ClientConfigurationDocument.js"; import { CordisExpression, DshCordisYaml } from "./DshCordisYaml.js"; +import { + mergeEnvironmentOverrides, + snapshotEnvironment, +} from "../process/snapshotEnvironment.js"; export const DSH_ENTRY_ID = "rea-mcp"; export const DSH_CLIENT_PACKAGE = "@deepseek-ai/dsh-mcp-client"; @@ -185,6 +189,7 @@ export class DshCordisDocument { environment( overrides: Readonly> = {}, validation: "fragment" | "effective" = "effective", + platform: NodeJS.Platform = process.platform, ): Result>, ConfigurationError> { const config = this.#field("config"); if (config instanceof CordisExpression) @@ -220,9 +225,11 @@ export class DshCordisDocument { ); const literals: Record = {}; const unresolvedSettings = new Set(); - for (const [key, value] of Object.entries(raw ?? {})) { + for (const [key, value] of Object.entries( + mergeEnvironmentOverrides(raw ?? {}, overrides, platform), + )) { if (value instanceof CordisExpression) { - if (overrides[key] === undefined) unresolvedSettings.add(key); + unresolvedSettings.add(key); continue; } if (typeof value !== "string") @@ -239,11 +246,20 @@ export class DshCordisDocument { ); literals[key] = value; } - const environment = { ...literals, ...overrides }; + const environment = literals; + const runtimeEnvironment = snapshotEnvironment(environment, platform); + const unresolved = new Set( + Object.keys( + snapshotEnvironment( + Object.fromEntries([...unresolvedSettings].map((key) => [key, ""])), + platform, + ), + ), + ); const validated = validation === "fragment" - ? parseEnvironmentValues(environment, unresolvedSettings) - : parseEnvironment(environment, unresolvedSettings); + ? parseEnvironmentValues(runtimeEnvironment, unresolved) + : parseEnvironment(runtimeEnvironment, unresolved); return validated.ok ? ok(environment) : validated; } @@ -255,10 +271,10 @@ export class DshCordisDocument { return err(this.#editabilityError); const environment = this.environment(overrides); if (!environment.ok) return environment; - const env = { - ...(isRecord(this.config?.env) ? this.config.env : {}), - ...overrides, - }; + const env = mergeEnvironmentOverrides( + isRecord(this.config?.env) ? this.config.env : {}, + overrides, + ); const desired = { ...this.config, ...dshClientRegistrationEntry(command) }; delete desired.env; if (Object.keys(env).length > 0) desired.env = env; @@ -373,6 +389,21 @@ export class DshCordisDocument { : this.#yaml.detach(env, invalidated); if (!isMap(nextEnv)) throw new ConfigurationError("Cordis environment is not a mapping"); + for (const pair of [...nextEnv.items]) { + const key = this.#yaml.key(pair.key); + if (Object.hasOwn(desiredEnv, key)) { + this.#yaml.renameKey(nextEnv, pair, key); + continue; + } + const alias = + process.platform === "win32" + ? Object.keys(desiredEnv).find( + (name) => name.toUpperCase() === key.toUpperCase(), + ) + : undefined; + if (alias === undefined) nextEnv.delete(pair.key); + else this.#yaml.renameKey(nextEnv, pair, alias); + } for (const [key, value] of Object.entries(desiredEnv)) { this.#yaml.setValue(nextEnv, key, value); } diff --git a/src/application/DshCordisYaml.ts b/src/application/DshCordisYaml.ts index ea84e5e65d..c7e88958f5 100644 --- a/src/application/DshCordisYaml.ts +++ b/src/application/DshCordisYaml.ts @@ -10,6 +10,7 @@ import { type Document, type Node, type YAMLMap, + type Pair, } from "yaml"; import { ConfigurationError } from "../domain/configurationErrors.js"; @@ -82,7 +83,7 @@ export class DshCordisYaml { else if (isMap(resolved)) { const entries: [string, unknown][] = []; for (const pair of resolved.items) { - entries.push([this.#key(pair.key), this.value(pair.value)]); + entries.push([this.key(pair.key), this.value(pair.value)]); } value = Object.fromEntries(entries); } @@ -91,7 +92,7 @@ export class DshCordisYaml { return value; } - #key(node: unknown): string { + key(node: unknown): string { const key = this.value(node); if (typeof key === "object" && key !== null) throw new ConfigurationError( @@ -101,13 +102,40 @@ export class DshCordisYaml { return key === null ? "" : String(key); } + /** A case-only rename retains source key/value comments on the chosen alias. */ + renameKey(map: YAMLMap, pair: Pair, key: string): void { + const existing = map.items.find( + (item) => item !== pair && this.key(item.key) === key, + ); + const replacement = existing ?? pair; + const carry = (source: unknown, target: unknown): void => { + if (!isNode(source) || !isNode(target) || source === target) return; + for (const name of ["comment", "commentBefore"] as const) { + const values = [target[name], source[name]].filter( + (value): value is string => value !== undefined, + ); + if (values.length > 0) target[name] = [...new Set(values)].join("\n"); + } + }; + if (existing !== undefined) { + carry(pair.key, existing.key); + carry(pair.value, existing.value); + map.items = map.items.filter((item) => item !== pair); + } else if (this.key(pair.key) !== key) { + const renamed = this.document.createNode(key); + carry(pair.key, renamed); + replacement.key = renamed; + } + } + field(map: YAMLMap | undefined, key: string): unknown { - return map?.items.findLast((pair) => this.#key(pair.key) === key)?.value; + return map?.items.findLast((pair) => this.key(pair.key) === key)?.value; } /** Preserve unchanged nodes and carry their comments when replacing values. */ setValue(map: YAMLMap, key: string, value: unknown): void { - const original = this.field(map, key); + const pair = map.items.findLast((item) => this.key(item.key) === key); + const original = pair?.value; if (isDeepStrictEqual(this.value(original), value)) return; const replacement = this.document.createNode(value); if (isNode(original) && isNode(replacement)) { @@ -116,7 +144,8 @@ export class DshCordisYaml { if (original.commentBefore !== undefined) replacement.commentBefore = original.commentBefore; } - map.set(key, replacement); + if (pair === undefined) map.set(key, replacement); + else pair.value = replacement; } /** Copy from the source before edits; materialized nodes own no anchors. */ diff --git a/src/application/SetupClientConfiguration.ts b/src/application/SetupClientConfiguration.ts index a2d7dfc769..e0a5a012d7 100644 --- a/src/application/SetupClientConfiguration.ts +++ b/src/application/SetupClientConfiguration.ts @@ -32,6 +32,7 @@ import { } from "./ClientRegistrationEnvironment.js"; import { PRODUCT_IDENTITY } from "../identity.js"; +import { snapshotEnvironment } from "../process/snapshotEnvironment.js"; import { npxRegistrationCommand } from "./ClientRegistrationIdentity.js"; import { MCP_STARTUP_POLICY, @@ -761,7 +762,8 @@ const clientConfigurationDesired = ( if (!environment.ok) throw environment.error; const registrationEnvironment = client.name === "kimi" && - environment.value.REA_MCP_INPUT_SCHEMA_PROFILE === undefined + snapshotEnvironment(environment.value).REA_MCP_INPUT_SCHEMA_PROFILE === + undefined ? { ...environment.value, REA_MCP_INPUT_SCHEMA_PROFILE: "compact", diff --git a/src/process/snapshotEnvironment.test.ts b/src/process/snapshotEnvironment.test.ts index 4012262993..d41f8fd8ab 100644 --- a/src/process/snapshotEnvironment.test.ts +++ b/src/process/snapshotEnvironment.test.ts @@ -1,6 +1,33 @@ import { expect, it } from "vitest"; -import { snapshotEnvironment } from "./snapshotEnvironment.js"; +import { + mergeEnvironmentOverrides, + snapshotEnvironment, +} from "./snapshotEnvironment.js"; + +it.each(["win32", "linux", "darwin"] as const)( + "merges explicit environment layers using %s key identity before sorting", + (platform) => { + const existing = { + rea_analysis_provider: "hopper", + CallerSetting: "retain", + }; + const overrides = { REA_ANALYSIS_PROVIDER: "ghidra", Path: "chosen" }; + const merged = mergeEnvironmentOverrides(existing, overrides, platform); + expect(merged).toEqual( + platform === "win32" + ? { + REA_ANALYSIS_PROVIDER: "ghidra", + CallerSetting: "retain", + Path: "chosen", + } + : { ...existing, ...overrides }, + ); + expect(Object.isFrozen(merged)).toBe(true); + expect(existing.rea_analysis_provider).toBe("hopper"); + expect(overrides.REA_ANALYSIS_PROVIDER).toBe("ghidra"); + }, +); it.each(["linux", "darwin"] as const)( "preserves case-sensitive %s variables in an immutable snapshot", diff --git a/src/process/snapshotEnvironment.ts b/src/process/snapshotEnvironment.ts index bfb57cd7d6..654328fff7 100644 --- a/src/process/snapshotEnvironment.ts +++ b/src/process/snapshotEnvironment.ts @@ -1,16 +1,33 @@ -/** Snapshot selected variables while preserving platform and caller precedence. */ -export const snapshotEnvironment = ( - environment: Readonly, +/** Merge explicit layers by platform identity while retaining the winning spelling/value. */ +export const mergeEnvironmentOverrides = ( + existing: Readonly>, + overrides: Readonly>, platform: NodeJS.Platform = process.platform, -): Readonly => { - if (platform !== "win32") return Object.freeze({ ...environment }); - const selected = new Map(); +): Readonly> => { + const selected = new Map(); + for (const layer of [existing, overrides]) + for (const [key, value] of Object.entries(layer)) + selected.set(platform === "win32" ? key.toUpperCase() : key, [ + key, + value, + ]); + return Object.freeze(Object.fromEntries(selected.values())); +}; + +/** Snapshot selected variables while preserving platform and caller precedence. */ +export const snapshotEnvironment = ( + environment: Readonly>, + platform: NodeJS.Platform = process.platform, +): Readonly> => { + const selected = mergeEnvironmentOverrides({}, environment, platform); + if (platform !== "win32") return selected; // Fold aliases in caller order so an explicit later override wins before // this canonical environment crosses a Windows process boundary. - for (const [key, value] of Object.entries(environment)) - selected.set(key.toUpperCase(), value); - const snapshot: NodeJS.ProcessEnv = {}; - for (const key of [...selected.keys()].sort()) - snapshot[key] = selected.get(key); - return Object.freeze(snapshot); + return Object.freeze( + Object.fromEntries( + Object.entries(selected) + .map(([key, value]) => [key.toUpperCase(), value] as const) + .sort(([left], [right]) => (left < right ? -1 : left > right ? 1 : 0)), + ), + ); }; diff --git a/tests/acceptance/setup/registrationRuntimeEnvironmentCli.test.ts b/tests/acceptance/setup/registrationRuntimeEnvironmentCli.test.ts index 7af5a60a7e..9971626460 100644 --- a/tests/acceptance/setup/registrationRuntimeEnvironmentCli.test.ts +++ b/tests/acceptance/setup/registrationRuntimeEnvironmentCli.test.ts @@ -19,6 +19,70 @@ const missingHopperAndGhidraEnvironment = (home: string) => ({ JAVA_HOME: join(home, "missing-java"), }); +cliTest.skipIf(process.platform !== "win32")( + "rejects lowercase Windows provider settings in doctor and setup without writing", + async ({ cli }) => { + const home = await createTestTempDirectory("rea-registration-case-"); + const config = join(home, ".cursor/mcp.json"); + await mkdir(dirname(config), { recursive: true }); + const original = JSON.stringify({ + mcpServers: { + rea: clientRegistrationEntry( + "json", + [process.execPath, resolve("scripts/rea.mjs"), "mcp"], + { rea_analysis_provider: "invalid provider" }, + ), + }, + }); + await writeFile(config, original); + const environment = { + HOME: home, + USERPROFILE: home, + XDG_CONFIG_HOME: join(home, ".config"), + ...missingHopperAndGhidraEnvironment(home), + }; + const doctor = await cli.run({ + arguments: ["doctor", "--client", "cursor", "--skill=false", "--json"], + cwd: home, + environment, + }); + expect(doctor.exitCode).toBe(1); + expect(doctor.json).toMatchObject({ + scope_checks: expect.arrayContaining([ + expect.objectContaining({ + name: "registration:cursor", + ok: false, + classification: "config_drift", + remediation: expect.stringContaining("REA_ANALYSIS_PROVIDER"), + }), + ]), + }); + const setup = await cli.run({ + arguments: [ + "setup", + "--client", + "cursor", + "--skill=false", + "--yes", + "--json", + ], + cwd: home, + environment, + }); + expect(setup.exitCode).toBe(1); + expect(setup.json).toMatchObject({ + status: "needs_human", + plannedActions: [], + appliedActions: [], + remediation: expect.stringContaining("REA_ANALYSIS_PROVIDER"), + }); + expect(await readFile(config, "utf8")).toBe(original); + await expect(access(`${config}.rea.backup`)).rejects.toMatchObject({ + code: "ENOENT", + }); + }, +); + const scenarios = [ { client: "cursor", diff --git a/tests/boundary/filesystem/clientConfigurationEnvironment.test.ts b/tests/boundary/filesystem/clientConfigurationEnvironment.test.ts index 10abf90325..bfcfe5c9d5 100644 --- a/tests/boundary/filesystem/clientConfigurationEnvironment.test.ts +++ b/tests/boundary/filesystem/clientConfigurationEnvironment.test.ts @@ -1,4 +1,5 @@ import { access, mkdir, readFile, writeFile } from "node:fs/promises"; +import { spawnSync } from "node:child_process"; import { join } from "node:path"; import { expect, it } from "vitest"; @@ -15,6 +16,242 @@ import { } from "../../../src/application/SetupClientConfiguration.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { readClientRegistrationStatuses } from "../../../src/application/ClientRegistrationStatus.js"; +import { clientRegistrationEnvironmentDetails } from "../../../src/application/ClientRegistrationEnvironment.js"; +import { snapshotEnvironment } from "../../../src/process/snapshotEnvironment.js"; +import { npxRegistrationCommand } from "../../../src/application/ClientRegistrationIdentity.js"; + +it.each(["win32", "linux"] as const)( + "uses the selected %s identity when inspecting lowercase registration settings", + async (platform) => { + const home = await createTestTempDirectory("rea-selected-platform-env-"); + const path = join(home, ".cursor/mcp.json"); + await mkdir(join(home, ".cursor")); + await writeFile( + path, + JSON.stringify({ + mcpServers: { + rea: clientRegistrationEntry( + "json", + npxRegistrationCommand(platform), + { rea_analysis_provider: "invalid provider" }, + ), + }, + }), + ); + const statuses = await readClientRegistrationStatuses(home, "rea", { + platform, + environment: {}, + }); + expect(statuses.find(({ client }) => client === "cursor")).toMatchObject({ + state: platform === "win32" ? "invalid" : "aligned", + }); + }, +); + +it.skipIf(process.platform !== "win32").each([ + { + key: "rea_analysis_provider", + override: "REA_ANALYSIS_PROVIDER", + value: "ghidra", + duplicate: false, + }, + { key: "true", override: "TRUE", value: "chosen", duplicate: false }, + { key: '"true"', override: "true", value: "chosen", duplicate: true }, + { + key: "rea_analysis_provider", + override: "REA_ANALYSIS_PROVIDER", + value: "ghidra", + duplicate: true, + }, +])( + "preserves DSH scalar-key identity and comments when replacing $key", + async ({ key, override, value, duplicate }) => { + const root = await createTestTempDirectory("rea-dsh-alias-comments-"); + const configPath = join(root, "cordis.patch.yml"); + const source = `- insert:\n - id: rea-mcp\n name: "@deepseek-ai/dsh-mcp-client"\n config:\n serverName: rea\n transport: stdio\n command: old-rea\n args: [mcp]\n env:\n # retain preceding rationale\n ${key}: "hopper" # retain inline rationale\n${duplicate ? ` ${override}: "ghidra" # retain winning rationale\n` : ""} CUSTOM: !!js caller.custom\n`; + await writeFile(configPath, source); + const client = { name: "dsh", format: "dsh", configPath } as const; + const result = await configureClientConfiguration( + client, + { [override]: value }, + ["rea", "mcp"], + ); + expect(result, JSON.stringify(result)).toMatchObject({ + status: "configured", + }); + const text = await readFile(configPath, "utf8"); + expect(text).toContain("# retain preceding rationale"); + expect(text).toContain("# retain inline rationale"); + if (duplicate) expect(text).toContain("# retain winning rationale"); + expect(text).toContain("!!js caller.custom"); + const env = clientRegistrationEnvironmentDetails( + parseClientConfiguration(text, "dsh"), + ); + expect(env.ok).toBe(true); + if (!env.ok) throw env.error; + expect(env.value.values).toHaveProperty(override, value); + expect( + Object.keys(env.value.values).filter( + (name) => name.toUpperCase() === override.toUpperCase(), + ), + ).toEqual([override]); + expect(await readFile(`${configPath}.rea.backup`, "utf8")).toBe(source); + expect( + await configureClientConfiguration(client, { [override]: value }, [ + "rea", + "mcp", + ]), + ).toEqual({ status: "unchanged" }); + }, +); + +it.skipIf(process.platform !== "win32")( + "preserves a lowercase explicit Kimi schema profile instead of inserting a conflicting default", + async () => { + const home = await createTestTempDirectory("rea-kimi-case-"); + await mkdir(join(home, ".kimi-code")); + const configPath = join(home, ".kimi-code/mcp.json"); + await writeFile( + configPath, + JSON.stringify({ + mcpServers: { + rea: { + command: "old-rea", + args: ["mcp"], + env: { rea_mcp_input_schema_profile: "full" }, + }, + }, + }), + ); + const client = { name: "kimi", format: "kimi", configPath } as const; + expect( + await configureClientConfiguration( + client, + {}, + npxRegistrationCommand(process.platform), + ), + ).toMatchObject({ status: "configured" }); + const parsed = parseClientConfiguration( + await readFile(configPath, "utf8"), + "kimi", + ); + expect(effectiveClientServer(parsed, "rea")).toHaveProperty("env", { + rea_mcp_input_schema_profile: "full", + }); + const statuses = await readClientRegistrationStatuses(home, "rea", { + environment: {}, + }); + expect(statuses.find(({ client }) => client === "kimi")).toMatchObject({ + state: "aligned", + }); + }, +); + +it + .skipIf(process.platform !== "win32") + .each(["hopper", "!!js caller.provider"])( + "replaces a Windows DSH environment alias without evaluating %s", + async (stored) => { + const root = await createTestTempDirectory("rea-dsh-case-"); + const configPath = join(root, "cordis.patch.yml"); + const source = `- insert:\n - id: rea-mcp\n name: "@deepseek-ai/dsh-mcp-client"\n config:\n serverName: rea\n transport: stdio\n command: old-rea\n args: [mcp]\n env:\n rea_analysis_provider: ${stored}\n CUSTOM: !!js caller.custom\n`; + await writeFile(configPath, source); + const client = { name: "dsh", format: "dsh", configPath } as const; + const overrides = { REA_ANALYSIS_PROVIDER: "ghidra" }; + const configured = await configureClientConfiguration(client, overrides, [ + "rea", + "mcp", + ]); + expect(configured, JSON.stringify(configured)).toMatchObject({ + status: "configured", + }); + const text = await readFile(configPath, "utf8"); + expect(text).not.toContain("rea_analysis_provider:"); + expect(text).toContain("REA_ANALYSIS_PROVIDER: ghidra"); + expect(text).toContain("!!js caller.custom"); + const details = clientRegistrationEnvironmentDetails( + parseClientConfiguration(text, "dsh"), + ); + expect(details.ok).toBe(true); + if (!details.ok) throw details.error; + expect(details.value.timeoutInputs.analysisProvider).toBe("ghidra"); + expect( + await configureClientConfiguration(client, overrides, ["rea", "mcp"]), + ).toEqual({ status: "unchanged" }); + }, +); + +it.skipIf(process.platform !== "win32").each([ + { + existing: { rea_analysis_provider: "hopper" }, + overrides: { REA_ANALYSIS_PROVIDER: "ghidra" }, + expected: "ghidra", + }, + { + existing: { REA_ANALYSIS_PROVIDER: "ghidra" }, + overrides: { rea_analysis_provider: "hopper" }, + expected: "hopper", + }, +])( + "keeps Windows override precedence consistent with the launched environment: $expected", + ({ existing, overrides, expected }) => { + const parsed = parseClientConfiguration( + JSON.stringify({ + mcpServers: { rea: { command: "rea", args: ["mcp"], env: existing } }, + }), + "json", + ); + const result = clientRegistrationEnvironmentDetails(parsed, overrides); + expect(result.ok).toBe(true); + if (!result.ok) throw result.error; + expect(result.value.timeoutInputs.analysisProvider).toBe(expected); + expect(snapshotEnvironment(result.value.values).REA_ANALYSIS_PROVIDER).toBe( + expected, + ); + expect(Object.keys(result.value.values)).toHaveLength(1); + const child = spawnSync( + process.execPath, + ["-e", "process.stdout.write(process.env.REA_ANALYSIS_PROVIDER ?? '')"], + { + env: result.value.values, + encoding: "utf8", + timeout: 5_000, + }, + ); + expect(child.status, child.stderr).toBe(0); + expect(child.stdout).toBe(expected); + }, +); + +it.skipIf(process.platform !== "win32")( + "rejects invalid lowercase known settings before writing a Windows registration", + async () => { + const root = await createTestTempDirectory( + "rea-invalid-lowercase-environment-", + ); + const configPath = join(root, "config.json"); + const original = JSON.stringify({ + mcpServers: { + rea: { + command: "rea", + args: ["mcp"], + env: { rea_analysis_provider: "invalid provider" }, + }, + }, + }); + await writeFile(configPath, original); + const result = await configureClientConfiguration( + { name: "cursor", format: "json", configPath }, + {}, + ["rea", "mcp"], + ); + expect(result).toMatchObject({ status: "failed" }); + expect(await readFile(configPath, "utf8")).toBe(original); + await expect(access(`${configPath}.rea.backup`)).rejects.toMatchObject({ + code: "ENOENT", + }); + }, +); it.each([ ["cursor", "json", "mcpServers", "env"], diff --git a/tests/boundary/filesystem/clientConfigurationLayeredEnvironment.test.ts b/tests/boundary/filesystem/clientConfigurationLayeredEnvironment.test.ts index 41f0b984c2..9d84f66679 100644 --- a/tests/boundary/filesystem/clientConfigurationLayeredEnvironment.test.ts +++ b/tests/boundary/filesystem/clientConfigurationLayeredEnvironment.test.ts @@ -14,6 +14,11 @@ import { import { readClientRegistrationStatuses } from "../../../src/application/ClientRegistrationStatus.js"; import { npxRegistrationCommand } from "../../../src/application/ClientRegistrationIdentity.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { + effectiveClientConfiguration, + readClientConfigurationFiles, +} from "../../../src/application/ClientConfigurationFiles.js"; +import { clientRegistrationEnvironmentDetails } from "../../../src/application/ClientRegistrationEnvironment.js"; const command = npxRegistrationCommand(process.platform); const language = "x86:LE:32:default"; @@ -62,6 +67,47 @@ const layered = async (lower: unknown, upper: unknown) => { return { client, paths, sources, status, unchanged }; }; +it.skipIf(process.platform !== "win32").each([ + { REA_ANALYSIS_PROVIDER: "ghidra", rea_analysis_provider: "hopper" }, + { rea_analysis_provider: "hopper", REA_ANALYSIS_PROVIDER: "ghidra" }, +])( + "keeps explicit overrides above inherited Windows aliases: %j", + async (lower) => { + const fixture = await layered(lower, {}); + expect( + await configureClientConfiguration( + fixture.client, + { REA_ANALYSIS_PROVIDER: "ghidra" }, + command, + ), + ).toMatchObject({ status: "configured" }); + const sources = await readClientConfigurationFiles(fixture.client); + expect(sources.ok).toBe(true); + if (!sources.ok) throw new Error(sources.error.detail); + const environment = clientRegistrationEnvironmentDetails( + effectiveClientConfiguration(sources.value), + ); + expect(environment.ok).toBe(true); + if (!environment.ok) throw environment.error; + expect(environment.value.timeoutInputs.analysisProvider).toBe("ghidra"); + expect(environment.value.values).toHaveProperty( + "REA_ANALYSIS_PROVIDER", + "ghidra", + ); + expect(environment.value.values).not.toHaveProperty( + "rea_analysis_provider", + ); + expect(await readFile(fixture.paths[0]!, "utf8")).toBe(fixture.sources[0]); + expect( + await configureClientConfiguration( + fixture.client, + { REA_ANALYSIS_PROVIDER: "ghidra" }, + command, + ), + ).toEqual({ status: "unchanged" }); + }, +); + it("validates language/compiler settings in the effective OpenCode environment", async () => { const fixture = await layered( { REA_GHIDRA_LANGUAGE_ID: language },