fix(setup): match retained fields to the client that writes them
Codex drops a tools array that its stdio config rejects, and the Harness approval plan lists the Cordis launcher keys the writer replaces.
This commit is contained in:
1 parent
4eb3b5335a
commit
55246a6425
6 files changed
+93
-9
No files matched your search
@@ -68,9 +68,12 @@ client-specific keys. It removes remote-server keys such as `url`, `headers`,
|
||||
`enabled: false` or (outside Grok Build) `disabled: true`, so the registration
|
||||
is active. A registration field that does not parse, including a non-boolean
|
||||
`enabled` or `disabled` and a malformed `startup_timeout_sec` or `tools`, is
|
||||
removed too. An existing entry without a
|
||||
`command` is a remote server and is replaced whole. A legacy OpenCode entry
|
||||
moved into the native `mcp.servers` table keeps only its environment.
|
||||
removed too. A `tools` value is removed except on GitHub Copilot CLI, which
|
||||
replaces it with `["*"]`; Codex rejects a `tools` sequence. An existing entry
|
||||
without a `command` is a remote server and is replaced whole. DeepSeek Harness
|
||||
keeps other fields on the existing Cordis server and replaces `serverName`,
|
||||
`transport`, `command`, and `args`. A legacy OpenCode entry moved into the
|
||||
native `mcp.servers` table keeps only its environment.
|
||||
Setup and doctor report a malformed REA environment map or invalid runtime
|
||||
setting before setup writes files. Repair the named setting in the client
|
||||
configuration; setup does not silently discard the rest of the map.
|
||||
|
||||
@@ -1536,6 +1536,13 @@ const REGISTRATION_REMOTE_KEYS: Readonly<Record<string, true>> = {
|
||||
const keepsGrokDisabledFlag = (dialect: ClientRegistrationDialect): boolean =>
|
||||
dialect === "grok";
|
||||
|
||||
/**
|
||||
* Copilot rewrites `tools` as a string array. Other clients do not: Codex
|
||||
* expects a map and rejects a sequence (`invalid type: sequence, expected a map`).
|
||||
*/
|
||||
const keepsCopilotTools = (dialect: ClientRegistrationDialect): boolean =>
|
||||
dialect === "copilot_cli";
|
||||
|
||||
/**
|
||||
* Whether setup replaces or removes an existing entry key. Setup registers an
|
||||
* active server, so entry-level switches that turn it off are removed too.
|
||||
@@ -1551,6 +1558,7 @@ const registrationManagedKey = (
|
||||
Object.hasOwn(REGISTRATION_LAUNCHER_KEYS, key) ||
|
||||
Object.hasOwn(REGISTRATION_REMOTE_KEYS, key) ||
|
||||
!registrationFieldAccepted(key, value) ||
|
||||
(key === "tools" && !keepsCopilotTools(dialect)) ||
|
||||
(key === "enabled" && value === false) ||
|
||||
(key === "disabled" && value === true && !keepsGrokDisabledFlag(dialect));
|
||||
|
||||
@@ -1613,10 +1621,11 @@ export const describeClientRegistrationRetention = (
|
||||
const removed = keepsGrokDisabledFlag(dialect)
|
||||
? "Remote-server keys, an enabled: false switch, and registration fields that do not parse are removed. A boolean disabled value is kept."
|
||||
: "Remote-server keys, switches that turn the server off, and registration fields that do not parse are removed.";
|
||||
const tools = keepsCopilotTools(dialect) ? "" : " A tools value is removed.";
|
||||
const replacement =
|
||||
replaced.length === 0
|
||||
? removed
|
||||
: `Setup replaces ${replaced.join(" and ")}. ${removed}`;
|
||||
? `${removed}${tools}`
|
||||
: `Setup replaces ${replaced.join(" and ")}. ${removed}${tools}`;
|
||||
const legacy =
|
||||
dialect === "opencode_v2"
|
||||
? " A legacy OpenCode entry moved into the native server table keeps only its environment."
|
||||
|
||||
@@ -404,6 +404,18 @@ export const serializeDshDocument = (
|
||||
throw new ConfigurationError("Missing Cordis edit plan");
|
||||
return document.cordis.source.serialize(document.cordis.desired);
|
||||
};
|
||||
/** Approval-plan text for the launcher keys `DshCordisDocument` replaces. */
|
||||
export const describeDshRegistrationRetention = (
|
||||
command: readonly string[],
|
||||
): string => {
|
||||
const entry = dshClientRegistrationEntry(command);
|
||||
const replaced = [...LAUNCHER_KEYS]
|
||||
.filter((key) => Object.hasOwn(entry, key))
|
||||
.sort((left, right) => left.localeCompare(right))
|
||||
.map((key) => `${key} with ${JSON.stringify(entry[key])}`);
|
||||
return `Other fields on the existing Cordis server are kept. Setup replaces ${replaced.join(", ")}. Environment overrides are merged.`;
|
||||
};
|
||||
|
||||
export const dshClientRegistrationEntry = (
|
||||
command: readonly string[],
|
||||
environment: Readonly<Record<string, string>> = {},
|
||||
|
||||
@@ -159,6 +159,9 @@ describe("setup workflow", () => {
|
||||
expect(result.plannedActions[0]?.detail).toContain(
|
||||
"registration fields that do not parse are removed",
|
||||
);
|
||||
expect(result.plannedActions[0]?.detail).toContain(
|
||||
"A tools value is removed.",
|
||||
);
|
||||
expect(host.hopperInstalls).toBe(0);
|
||||
expect(host.configurations).toBe(0);
|
||||
});
|
||||
@@ -270,6 +273,36 @@ describe("setup approval disclosure", () => {
|
||||
);
|
||||
expect(detail).not.toContain(`enabled with ${JSON.stringify(true)}`);
|
||||
});
|
||||
|
||||
it("describes Cordis fields the Harness writer replaces", async () => {
|
||||
const host = new FakeSetupHost();
|
||||
host.hopper = "/Applications/Hopper";
|
||||
host.skill = "unchanged";
|
||||
host.clients = [
|
||||
{
|
||||
name: "dsh",
|
||||
displayName: "DeepSeek Harness",
|
||||
configPath: "/cordis.patch.yml",
|
||||
format: "dsh",
|
||||
},
|
||||
];
|
||||
|
||||
const result = await runSetup(
|
||||
{ ...options(false), dryRun: true, clientIds: ["dsh"] },
|
||||
host,
|
||||
);
|
||||
const detail = result.plannedActions[0]?.detail ?? "";
|
||||
const [executable = "rea", ...args] = host.registrationCommand;
|
||||
|
||||
expect(detail).toContain(
|
||||
"Other fields on the existing Cordis server are kept.",
|
||||
);
|
||||
expect(detail).toContain(`serverName with ${JSON.stringify("rea")}`);
|
||||
expect(detail).toContain(`transport with ${JSON.stringify("stdio")}`);
|
||||
expect(detail).toContain(`command with ${JSON.stringify(executable)}`);
|
||||
expect(detail).toContain(`args with ${JSON.stringify(args)}`);
|
||||
expect(detail).not.toContain("Remote-server keys");
|
||||
});
|
||||
});
|
||||
|
||||
describe("setup workflow lifecycle and provider planning", () => {
|
||||
|
||||
@@ -4,6 +4,7 @@ import {
|
||||
describeClientRegistrationRetention,
|
||||
type ClientRegistrationDialect,
|
||||
} from "./ClientConfigurationDocument.js";
|
||||
import { describeDshRegistrationRetention } from "./DshCordisPatch.js";
|
||||
import { isOwnedClientRegistrationCommand } from "./ClientRegistrationIdentity.js";
|
||||
import {
|
||||
linuxHopperInstallDisclosure,
|
||||
@@ -334,10 +335,10 @@ const clientConfigurationDetail = (
|
||||
(client.format === undefined || client.format === "unsupported"
|
||||
? "json"
|
||||
: client.format);
|
||||
const retainedFields = describeClientRegistrationRetention(
|
||||
client.name,
|
||||
dialect,
|
||||
);
|
||||
const retainedFields =
|
||||
dialect === "dsh"
|
||||
? describeDshRegistrationRetention(command)
|
||||
: describeClientRegistrationRetention(client.name, dialect);
|
||||
const environmentDetail =
|
||||
entries.length === 0
|
||||
? ` No additional provider environment variables. Existing server environment overrides are retained. ${retainedFields}`
|
||||
|
||||
@@ -139,6 +139,32 @@ it("drops Codex oauth_resource from a local command entry", async () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("drops a Codex tools array from a local command entry", async () => {
|
||||
const root = await createTestTempDirectory("rea-registration-codex-tools-");
|
||||
const configPath = join(root, "config.toml");
|
||||
const client: SetupClient = { name: "codex", format: "toml", configPath };
|
||||
const existing = {
|
||||
...clientRegistrationEntry("toml", ["rea-old", "mcp"], environment),
|
||||
tool_timeout_sec: 900,
|
||||
tools: ["*"],
|
||||
};
|
||||
await writeFile(
|
||||
configPath,
|
||||
serializeClientConfiguration(configurationWith("toml", existing), "toml"),
|
||||
);
|
||||
expect(await configureClientConfiguration(client, {}, command)).toMatchObject(
|
||||
{ status: "configured" },
|
||||
);
|
||||
expect(
|
||||
parseClientConfiguration(await readFile(configPath, "utf8"), "toml").servers
|
||||
.rea,
|
||||
).toEqual({
|
||||
...clientRegistrationEntry("toml", command, environment),
|
||||
startup_timeout_sec: MCP_STARTUP_POLICY.codexStartupTimeoutSeconds,
|
||||
tool_timeout_sec: 900,
|
||||
});
|
||||
});
|
||||
|
||||
it("drops malformed registration fields from a local command entry", async () => {
|
||||
const root = await createTestTempDirectory("rea-registration-schema-");
|
||||
const configPath = join(root, "mcp.json");
|
||||
|
||||
Reference in new issue
Block a user