fix(setup): preserve source keys across configuration boundaries
This commit is contained in:
1 parent
99d2d7ad22
commit
98ca6c8270
7 files changed
+172
-27
No files matched your search
@@ -17,7 +17,6 @@ import {
|
||||
createScanner,
|
||||
format as formatJson,
|
||||
getNodeValue,
|
||||
parse as parseJsonc,
|
||||
parseTree,
|
||||
printParseErrorCode,
|
||||
type Edit,
|
||||
@@ -58,7 +57,14 @@ export interface ClientConfigurationDocument {
|
||||
readonly legacyServers: Record<string, unknown>;
|
||||
}
|
||||
|
||||
const objectSchema = z.record(z.string(), z.unknown());
|
||||
/** Admit source mappings without Zod's record clone dropping caller keys. */
|
||||
export const clientConfigurationRecordSchema = z
|
||||
.custom<Record<string, unknown>>((value) => {
|
||||
if (typeof value !== "object" || value === null) return false;
|
||||
const prototype: unknown = Object.getPrototypeOf(value);
|
||||
return prototype === null || prototype === Object.prototype;
|
||||
})
|
||||
.transform((value) => Object.fromEntries(Object.entries(value)));
|
||||
|
||||
/** Compare parsed configuration values without depending on parser prototypes. */
|
||||
export const clientConfigurationValuesEqual = (
|
||||
@@ -171,24 +177,27 @@ const parseDocument = (
|
||||
const errors: ParseError[] = [];
|
||||
// Accept a UTF-8 BOM without shifting diagnostics or editing the original text.
|
||||
const jsonText = text.startsWith("\uFEFF") ? ` ${text.slice(1)}` : text;
|
||||
const document = parseJsonc(jsonText, errors, { allowTrailingComma: true });
|
||||
const root = parseTree(jsonText, errors, { allowTrailingComma: true });
|
||||
const firstError = errors[0];
|
||||
if (firstError !== undefined)
|
||||
throw new SyntaxError(
|
||||
`Invalid JSON/JSONC at offset ${firstError.offset}: ${printParseErrorCode(firstError.error)}`,
|
||||
);
|
||||
return requireObject(document, "The configuration root");
|
||||
return requireObject(
|
||||
root === undefined ? undefined : getNodeValue(root),
|
||||
"The configuration root",
|
||||
);
|
||||
};
|
||||
|
||||
/** A parsed configuration whose structure, not its syntax, cannot hold MCP servers. */
|
||||
export class ClientConfigurationShapeError extends TypeError {}
|
||||
|
||||
/** Validate and copy own properties, excluding non-record YAML values and parser-injected prototypes. */
|
||||
/** Validate source mappings and copy their own properties without rewriting values. */
|
||||
const requireObject = (
|
||||
value: unknown,
|
||||
description: string,
|
||||
): Record<string, unknown> => {
|
||||
const parsed = objectSchema.safeParse(value);
|
||||
const parsed = clientConfigurationRecordSchema.safeParse(value);
|
||||
if (parsed.success) return parsed.data;
|
||||
throw new ClientConfigurationShapeError(`${description} must be an object`, {
|
||||
cause: parsed.error,
|
||||
@@ -283,7 +292,9 @@ export const effectiveClientServer = (
|
||||
): unknown =>
|
||||
Object.hasOwn(parsed.servers, name)
|
||||
? parsed.servers[name]
|
||||
: parsed.legacyServers[name];
|
||||
: Object.hasOwn(parsed.legacyServers, name)
|
||||
? parsed.legacyServers[name]
|
||||
: undefined;
|
||||
|
||||
/**
|
||||
* Return `parsed.document` with `servers` at its server table. A legacy OpenCode
|
||||
@@ -299,7 +310,9 @@ export const withClientServers = (
|
||||
return Object.hasOwn(servers, PRODUCT_IDENTITY.mcpServerKey)
|
||||
? editDshDocument(
|
||||
parsed,
|
||||
objectSchema.parse(servers[PRODUCT_IDENTITY.mcpServerKey]),
|
||||
clientConfigurationRecordSchema.parse(
|
||||
servers[PRODUCT_IDENTITY.mcpServerKey],
|
||||
),
|
||||
)
|
||||
: editDshDocument(parsed);
|
||||
const [key = "", nested] = parsed.serversPath;
|
||||
@@ -308,7 +321,9 @@ export const withClientServers = (
|
||||
// ZCode `config.json` without an `mcp` object), so treat absence as empty.
|
||||
const outerValue = parsed.document[key];
|
||||
const outer =
|
||||
outerValue === undefined ? {} : { ...objectSchema.parse(outerValue) };
|
||||
outerValue === undefined
|
||||
? {}
|
||||
: clientConfigurationRecordSchema.parse(outerValue);
|
||||
if (
|
||||
parsed.dialect === "opencode_v2" &&
|
||||
removeLegacy !== undefined &&
|
||||
@@ -325,7 +340,7 @@ const valueAt = (
|
||||
): unknown => {
|
||||
let current: unknown = document;
|
||||
for (const key of path) {
|
||||
const object = objectSchema.safeParse(current);
|
||||
const object = clientConfigurationRecordSchema.safeParse(current);
|
||||
if (!object.success || !Object.hasOwn(object.data, key)) return undefined;
|
||||
current = object.data[key];
|
||||
}
|
||||
@@ -1181,7 +1196,9 @@ const tomlServerEntry = (
|
||||
document: Record<string, unknown>,
|
||||
serverKey: string,
|
||||
): unknown => {
|
||||
const servers = objectSchema.safeParse(document.mcp_servers);
|
||||
const servers = clientConfigurationRecordSchema.safeParse(
|
||||
document.mcp_servers,
|
||||
);
|
||||
if (!servers.success || !Object.hasOwn(servers.data, serverKey))
|
||||
return undefined;
|
||||
return servers.data[serverKey];
|
||||
@@ -1255,7 +1272,9 @@ const serializeGrokConfiguration = (
|
||||
const withoutEmptyServerTable = (
|
||||
document: Record<string, unknown>,
|
||||
): Record<string, unknown> => {
|
||||
const servers = objectSchema.safeParse(document.mcp_servers);
|
||||
const servers = clientConfigurationRecordSchema.safeParse(
|
||||
document.mcp_servers,
|
||||
);
|
||||
if (!servers.success || Object.keys(servers.data).length > 0) return document;
|
||||
const { mcp_servers: empty, ...rest } = document;
|
||||
void empty;
|
||||
@@ -1291,7 +1310,9 @@ const tomlTextHolds = (
|
||||
): boolean => {
|
||||
try {
|
||||
return clientConfigurationValuesEqual(
|
||||
withoutEmptyServerTable(objectSchema.parse(parseClientToml(text))),
|
||||
withoutEmptyServerTable(
|
||||
clientConfigurationRecordSchema.parse(parseClientToml(text)),
|
||||
),
|
||||
withoutEmptyServerTable(document),
|
||||
);
|
||||
} catch (cause: unknown) {
|
||||
|
||||
@@ -164,7 +164,7 @@ export const effectiveClientConfiguration = (
|
||||
const last = documents.at(-1);
|
||||
if (last === undefined) return undefined;
|
||||
if (documents.length === 1) return last;
|
||||
const servers: Record<string, unknown> = {};
|
||||
const servers: Record<string, unknown> = Object.create(null);
|
||||
for (const document of documents) {
|
||||
const names = new Set([
|
||||
...Object.keys(document.legacyServers),
|
||||
|
||||
@@ -1,5 +1,3 @@
|
||||
import { z } from "zod";
|
||||
|
||||
import {
|
||||
parseEnvironment,
|
||||
parseEnvironmentValues,
|
||||
@@ -8,6 +6,7 @@ import { ConfigurationError } from "../domain/configurationErrors.js";
|
||||
import { err, ok, type Result } from "../domain/result.js";
|
||||
import {
|
||||
effectiveClientServer,
|
||||
clientConfigurationRecordSchema,
|
||||
type ClientConfigurationDocument,
|
||||
} from "./ClientConfigurationDocument.js";
|
||||
import { PRODUCT_IDENTITY } from "../identity.js";
|
||||
@@ -30,6 +29,22 @@ export interface ClientRegistrationEnvironmentDetails {
|
||||
readonly timeoutInputs: McpToolTimeoutInputs;
|
||||
}
|
||||
|
||||
const environmentSchema = clientConfigurationRecordSchema.transform(
|
||||
(values, context) => {
|
||||
const entries: [string, string][] = [];
|
||||
for (const [key, value] of Object.entries(values)) {
|
||||
if (typeof value === "string") entries.push([key, value]);
|
||||
else
|
||||
context.addIssue({
|
||||
code: "custom",
|
||||
path: [key],
|
||||
message: "Environment values must be strings",
|
||||
});
|
||||
}
|
||||
return Object.fromEntries(entries);
|
||||
},
|
||||
);
|
||||
|
||||
/** Read explicit REA overrides without dropping peers when one value is invalid. */
|
||||
export const clientRegistrationEnvironment = (
|
||||
parsed: ClientConfigurationDocument | undefined,
|
||||
@@ -80,9 +95,7 @@ export const clientRegistrationEnvironmentDetails = (
|
||||
typeof registration === "object" && registration !== null
|
||||
? Reflect.get(registration, key)
|
||||
: undefined;
|
||||
const existing = z
|
||||
.record(z.string(), z.string())
|
||||
.safeParse(raw === undefined ? {} : raw);
|
||||
const existing = environmentSchema.safeParse(raw === undefined ? {} : raw);
|
||||
if (!existing.success)
|
||||
return err(
|
||||
new ConfigurationError("Invalid REA server environment", {
|
||||
|
||||
@@ -223,7 +223,7 @@ export class DshCordisDocument {
|
||||
return err(
|
||||
new ConfigurationError("Cordis server environment must be a mapping"),
|
||||
);
|
||||
const literals: Record<string, string> = {};
|
||||
const literals: Record<string, string> = Object.create(null);
|
||||
const unresolvedSettings = new Set<string>();
|
||||
for (const [key, value] of Object.entries(
|
||||
mergeEnvironmentOverrides<unknown>(raw ?? {}, overrides, platform),
|
||||
@@ -246,8 +246,7 @@ export class DshCordisDocument {
|
||||
);
|
||||
literals[key] = value;
|
||||
}
|
||||
const environment = literals;
|
||||
const runtimeEnvironment = snapshotEnvironment(environment, platform);
|
||||
const runtimeEnvironment = snapshotEnvironment(literals, platform);
|
||||
const unresolved = new Set(
|
||||
Object.keys(
|
||||
snapshotEnvironment(
|
||||
@@ -260,7 +259,7 @@ export class DshCordisDocument {
|
||||
validation === "fragment"
|
||||
? parseEnvironmentValues(runtimeEnvironment, unresolved)
|
||||
: parseEnvironment(runtimeEnvironment, unresolved);
|
||||
return validated.ok ? ok(environment) : validated;
|
||||
return validated.ok ? ok(literals) : validated;
|
||||
}
|
||||
|
||||
desired(
|
||||
|
||||
@@ -272,7 +272,11 @@ for (const scenario of scenarios)
|
||||
const server = {
|
||||
command: "stale-launcher",
|
||||
args: ["mcp"],
|
||||
env: { CALLER_SETTING: "keep exactly", ...env },
|
||||
env: {
|
||||
CALLER_SETTING: "keep exactly",
|
||||
["__proto__"]: "retain caller key",
|
||||
...env,
|
||||
},
|
||||
};
|
||||
return scenario.client === "dsh"
|
||||
? stringify([
|
||||
@@ -350,7 +354,11 @@ for (const scenario of scenarios)
|
||||
const text = await readFile(config, "utf8");
|
||||
const parsed = parseClientConfiguration(text, scenario.format);
|
||||
expect(effectiveClientServer(parsed, "rea")).toMatchObject({
|
||||
env: { CALLER_SETTING: "keep exactly", ...scenario.repaired },
|
||||
env: {
|
||||
CALLER_SETTING: "keep exactly",
|
||||
["__proto__"]: "retain caller key",
|
||||
...scenario.repaired,
|
||||
},
|
||||
});
|
||||
expect(await readFile(`${config}.rea.backup`, "utf8")).toBe(repaired);
|
||||
const repeated = await run([
|
||||
|
||||
@@ -363,12 +363,17 @@ it.each([
|
||||
REA_GHIDRA_LANGUAGE_ID: "x86:LE:32:default",
|
||||
REA_GHIDRA_COMPILER_SPEC_ID: "borlandcpp",
|
||||
CALLER_SETTING: "preserve exactly",
|
||||
["__proto__"]: "caller prototype setting",
|
||||
constructor: "caller constructor setting",
|
||||
toString: "caller string setting",
|
||||
};
|
||||
const serialized = serializeClientConfiguration(
|
||||
{
|
||||
["__proto__"]: { caller: "retain root value" },
|
||||
[serversKey]: {
|
||||
rea: clientRegistrationEntry(format, ["rea-old", "mcp"], custom),
|
||||
other: clientRegistrationEntry(format, ["other"], { OTHER: "keep" }),
|
||||
["__proto__"]: clientRegistrationEntry(format, ["peer"], {}),
|
||||
},
|
||||
},
|
||||
format,
|
||||
@@ -386,6 +391,14 @@ it.each([
|
||||
).toMatchObject({ status: "configured" });
|
||||
const configured = await readFile(configPath, "utf8");
|
||||
const parsed = parseClientConfiguration(configured, format);
|
||||
expect(Object.hasOwn(parsed.document, "__proto__")).toBe(true);
|
||||
expect(parsed.document["__proto__"]).toEqual({
|
||||
caller: "retain root value",
|
||||
});
|
||||
expect(Object.hasOwn(parsed.servers, "__proto__")).toBe(true);
|
||||
expect(effectiveClientServer(parsed, "__proto__")).toEqual(
|
||||
clientRegistrationEntry(format, ["peer"], {}),
|
||||
);
|
||||
expect(effectiveClientServer(parsed, "rea")).toHaveProperty(
|
||||
environmentKey,
|
||||
{ ...custom, ...providers },
|
||||
@@ -410,6 +423,63 @@ it.each([
|
||||
},
|
||||
);
|
||||
|
||||
it.each(["linux", "win32"] as const)(
|
||||
"retains every literal Cordis environment key for %s",
|
||||
(platform) => {
|
||||
const source = `- insert:
|
||||
- id: rea-mcp
|
||||
name: "@deepseek-ai/dsh-mcp-client"
|
||||
config:
|
||||
serverName: rea
|
||||
transport: stdio
|
||||
command: rea
|
||||
args: [mcp]
|
||||
env:
|
||||
"__proto__": selected
|
||||
constructor: caller
|
||||
CUSTOM: !!js caller.custom
|
||||
`;
|
||||
const details = clientRegistrationEnvironmentDetails(
|
||||
parseClientConfiguration(source, "dsh"),
|
||||
{},
|
||||
"effective",
|
||||
platform,
|
||||
);
|
||||
expect(details.ok).toBe(true);
|
||||
if (!details.ok) throw details.error;
|
||||
expect(Object.entries(details.value.values)).toEqual([
|
||||
["__proto__", "selected"],
|
||||
["constructor", "caller"],
|
||||
]);
|
||||
},
|
||||
);
|
||||
|
||||
it("rejects a non-string caller environment key before editing configuration", async () => {
|
||||
const root = await createTestTempDirectory("rea-invalid-own-environment-");
|
||||
const configPath = join(root, "config.json");
|
||||
const source =
|
||||
'{"mcpServers":{"rea":{"command":"old-rea","args":["mcp"],"env":{"__proto__":123}}}}';
|
||||
await writeFile(configPath, source);
|
||||
const details = clientRegistrationEnvironmentDetails(
|
||||
parseClientConfiguration(source, "json"),
|
||||
);
|
||||
expect(details).toMatchObject({
|
||||
ok: false,
|
||||
error: { settings: [{ setting: "env.__proto__" }] },
|
||||
});
|
||||
expect(
|
||||
await configureClientConfiguration(
|
||||
{ name: "cursor", format: "json", configPath },
|
||||
{},
|
||||
["rea", "mcp"],
|
||||
),
|
||||
).toMatchObject({ status: "failed" });
|
||||
expect(await readFile(configPath, "utf8")).toBe(source);
|
||||
await expect(access(`${configPath}.rea.backup`)).rejects.toMatchObject({
|
||||
code: "ENOENT",
|
||||
});
|
||||
});
|
||||
|
||||
it("retains legacy OpenCode environment when migrating into a native server table", async () => {
|
||||
const root = await createTestTempDirectory("rea-opencode-environment-");
|
||||
const configPath = join(root, "opencode.json");
|
||||
|
||||
@@ -152,8 +152,16 @@ it("rejects a companion absent from the complete layered environment", async ()
|
||||
|
||||
it("keeps higher-priority values and does not copy inherited settings into the write layer", async () => {
|
||||
const fixture = await layered(
|
||||
{ REA_GHIDRA_LANGUAGE_ID: language, HOPPER_LOADER_ARGS_JSON: '["lower"]' },
|
||||
{ REA_GHIDRA_COMPILER_SPEC_ID: compiler, HOPPER_LOADER_ARGS_JSON: '[""]' },
|
||||
{
|
||||
REA_GHIDRA_LANGUAGE_ID: language,
|
||||
HOPPER_LOADER_ARGS_JSON: '["lower"]',
|
||||
["__proto__"]: "lower caller value",
|
||||
},
|
||||
{
|
||||
REA_GHIDRA_COMPILER_SPEC_ID: compiler,
|
||||
HOPPER_LOADER_ARGS_JSON: '[""]',
|
||||
["__proto__"]: "selected caller value",
|
||||
},
|
||||
);
|
||||
expect(
|
||||
await configureClientConfiguration(
|
||||
@@ -173,6 +181,7 @@ it("keeps higher-priority values and does not copy inherited settings into the w
|
||||
REA_GHIDRA_COMPILER_SPEC_ID: compiler,
|
||||
HOPPER_LOADER_ARGS_JSON: '[""]',
|
||||
REA_LOG_LEVEL: "debug",
|
||||
["__proto__"]: "selected caller value",
|
||||
},
|
||||
);
|
||||
expect(
|
||||
@@ -187,6 +196,31 @@ it("keeps higher-priority values and does not copy inherited settings into the w
|
||||
).toEqual({ status: "unchanged" });
|
||||
});
|
||||
|
||||
it("merges caller server names as own keys without inventing absent registrations", () => {
|
||||
const lower = parseClientConfiguration(
|
||||
'{"mcp":{"__proto__":{"type":"local","command":["peer"],"environment":{"LOWER":"retained"}}}}',
|
||||
"opencode",
|
||||
);
|
||||
const upper = parseClientConfiguration(
|
||||
'{"mcp":{"__proto__":{"environment":{"UPPER":"selected"}}}}',
|
||||
"opencode",
|
||||
);
|
||||
expect(effectiveClientServer(lower, "constructor")).toBeUndefined();
|
||||
const effective = effectiveClientConfiguration([
|
||||
{ ...lower, sourcePath: "lower.json" },
|
||||
{ ...upper, sourcePath: "upper.json" },
|
||||
]);
|
||||
expect(effective).toBeDefined();
|
||||
if (effective === undefined) throw new Error("missing layered configuration");
|
||||
expect(Object.keys(effective.servers)).toEqual(["__proto__"]);
|
||||
expect(effectiveClientServer(effective, "__proto__")).toEqual({
|
||||
type: "local",
|
||||
command: ["peer"],
|
||||
environment: { LOWER: "retained", UPPER: "selected" },
|
||||
});
|
||||
expect(effectiveClientServer(effective, "toString")).toBeUndefined();
|
||||
});
|
||||
|
||||
it.each([
|
||||
["invalid JSON", { HOPPER_LOADER_ARGS_JSON: "{" }],
|
||||
["invalid value type", { HOPPER_LOADER_ARGS_JSON: 42 }],
|
||||
|
||||
Reference in new issue
Block a user