fix(setup): preserve launcher identity during registration repair
This commit is contained in:
1 parent
850abe5e6f
commit
915e56e3e5
5 files changed
+361
-39
No files matched your search
@@ -21,22 +21,36 @@ const tomlTimeoutSchema = z.union([
|
||||
z.bigint().positive(),
|
||||
]);
|
||||
|
||||
/** Common launcher facts, without assigning a shared meaning to client policy. */
|
||||
const registrationSchema = z
|
||||
const launcherSchema = z
|
||||
.object({
|
||||
command: z.string().min(1),
|
||||
args: z.array(z.string()).default([]),
|
||||
startup_timeout_sec: tomlTimeoutSchema.optional(),
|
||||
tool_timeout_sec: tomlTimeoutSchema.optional(),
|
||||
startupTimeoutMs: z.number().positive().optional(),
|
||||
toolTimeoutMs: z.number().positive().optional(),
|
||||
type: z.string().optional(),
|
||||
transport: z.string().optional(),
|
||||
serverName: z.string().optional(),
|
||||
disabled: z.boolean().optional(),
|
||||
enabled: z.boolean().optional(),
|
||||
})
|
||||
.passthrough();
|
||||
const opencodeLauncherSchema = z
|
||||
.object({
|
||||
type: z.literal("local"),
|
||||
command: z.array(z.string()).min(1),
|
||||
})
|
||||
.passthrough();
|
||||
const normalizeOpencodeLauncher = (
|
||||
entry: z.output<typeof opencodeLauncherSchema>,
|
||||
): z.output<typeof launcherSchema> => {
|
||||
const [command = "", ...args] = entry.command;
|
||||
return { ...entry, command, args };
|
||||
};
|
||||
|
||||
const registrationSchema = launcherSchema.extend({
|
||||
startup_timeout_sec: tomlTimeoutSchema.optional(),
|
||||
tool_timeout_sec: tomlTimeoutSchema.optional(),
|
||||
startupTimeoutMs: z.number().positive().optional(),
|
||||
toolTimeoutMs: z.number().positive().optional(),
|
||||
type: z.string().optional(),
|
||||
transport: z.string().optional(),
|
||||
serverName: z.string().optional(),
|
||||
disabled: z.boolean().optional(),
|
||||
enabled: z.boolean().optional(),
|
||||
});
|
||||
|
||||
export type ClientRegistration = z.output<typeof registrationSchema>;
|
||||
|
||||
@@ -76,6 +90,22 @@ const registrationError = (error: z.ZodError): ConfigurationError =>
|
||||
})),
|
||||
});
|
||||
|
||||
/** Launcher identity remains usable when unrelated readiness settings are invalid. */
|
||||
export const parseClientRegistrationLauncher = (
|
||||
value: unknown,
|
||||
client: SetupClient,
|
||||
): Result<{ command: string; args: string[] }, ConfigurationError> => {
|
||||
const parsed =
|
||||
client.format === "opencode"
|
||||
? opencodeLauncherSchema
|
||||
.transform(normalizeOpencodeLauncher)
|
||||
.safeParse(value)
|
||||
: launcherSchema.safeParse(value);
|
||||
return parsed.success
|
||||
? ok({ command: parsed.data.command, args: parsed.data.args })
|
||||
: err(registrationError(parsed.error));
|
||||
};
|
||||
|
||||
/** Policy belongs to the client dialect, and is validated before refresh writes. */
|
||||
const validateClientPolicy = (
|
||||
value: unknown,
|
||||
@@ -109,17 +139,14 @@ export const parseClientRegistration = (
|
||||
: err(registrationError(parsed.error));
|
||||
}
|
||||
if (client.format === "opencode") {
|
||||
const entry = z
|
||||
.object({
|
||||
type: z.literal("local"),
|
||||
command: z.array(z.string()).min(1),
|
||||
const entry = opencodeLauncherSchema
|
||||
.extend({
|
||||
environment: z.record(z.string(), z.string()).optional(),
|
||||
})
|
||||
.passthrough()
|
||||
.transform(normalizeOpencodeLauncher)
|
||||
.safeParse(value);
|
||||
if (!entry.success) return err(registrationError(entry.error));
|
||||
const [command = "", ...args] = entry.data.command;
|
||||
value = { ...entry.data, command, args };
|
||||
value = entry.data;
|
||||
}
|
||||
const parsed = registrationSchema.safeParse(value);
|
||||
if (!parsed.success) return err(registrationError(parsed.error));
|
||||
@@ -182,7 +209,7 @@ export const refreshClientRegistration = (
|
||||
const existing = source.success ? source.data : {};
|
||||
const policy = validateClientPolicy(existing, client);
|
||||
if (!policy.ok) return policy;
|
||||
const prior = parseClientRegistration(existing, client);
|
||||
const prior = parseClientRegistrationLauncher(existing, client);
|
||||
const priorCommand = prior.ok
|
||||
? [prior.value.command, ...prior.value.args]
|
||||
: undefined;
|
||||
|
||||
@@ -22,7 +22,7 @@ import { defaultKunaCacheDirectory } from "../config/kunaCacheDirectory.js";
|
||||
import { linuxHopperReleaseCacheDirectory } from "./LinuxHopperCacheDirectory.js";
|
||||
|
||||
import { ClientConfigurationPublication } from "./ClientConfigurationPublication.js";
|
||||
import { z } from "zod";
|
||||
import { parseClientRegistrationLauncher } from "./ClientRegistration.js";
|
||||
|
||||
import { PRODUCT_IDENTITY } from "../identity.js";
|
||||
import {
|
||||
@@ -734,21 +734,9 @@ const resolveUninstallConfigPath = async (
|
||||
|
||||
/** Ownership rule for persistent MCP registrations created by REA setup. */
|
||||
const isOwnedRegistration = (value: unknown, client: SetupClient): boolean => {
|
||||
if (client.format === "opencode") {
|
||||
const parsed = z
|
||||
.object({ type: z.literal("local"), command: z.array(z.string()).min(1) })
|
||||
.passthrough()
|
||||
.safeParse(value);
|
||||
if (!parsed.success) return false;
|
||||
const command = parsed.data.command;
|
||||
return (
|
||||
classifyClientRegistrationCommand(command, CURRENT_ENTRYPOINT_PATH) ===
|
||||
"owned"
|
||||
);
|
||||
}
|
||||
const parsed = registrationSchema.safeParse(value);
|
||||
if (!parsed.success) return false;
|
||||
const { command, args } = parsed.data;
|
||||
const parsed = parseClientRegistrationLauncher(value, client);
|
||||
if (!parsed.ok) return false;
|
||||
const { command, args } = parsed.value;
|
||||
return (
|
||||
classifyClientRegistrationCommand(
|
||||
[command, ...args],
|
||||
@@ -1183,8 +1171,5 @@ const uninstallStatusPriority = (status: UninstallItem["status"]): number => {
|
||||
}
|
||||
}
|
||||
};
|
||||
const registrationSchema = z
|
||||
.object({ command: z.string(), args: z.array(z.string()) })
|
||||
.passthrough();
|
||||
const isMissing = (cause: unknown): boolean =>
|
||||
cause instanceof Error && "code" in cause && cause.code === "ENOENT";
|
||||
@@ -9,14 +9,16 @@ import {
|
||||
symlink,
|
||||
writeFile,
|
||||
} from "node:fs/promises";
|
||||
import { dirname, join } from "node:path";
|
||||
import { dirname, join, resolve } from "node:path";
|
||||
import { promisify } from "node:util";
|
||||
|
||||
import { spawn } from "@lydell/node-pty";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { z } from "zod";
|
||||
import { parse as parseToml } from "smol-toml";
|
||||
|
||||
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
|
||||
import { cliTest } from "../../support/cli/cliFixture.js";
|
||||
|
||||
const execute = promisify(execFile);
|
||||
const decisionMarker = "REA_SETUP_DECISION:";
|
||||
@@ -176,6 +178,107 @@ describe("TTY setup dry run", () => {
|
||||
});
|
||||
});
|
||||
|
||||
cliTest.skipIf(process.platform === "win32")(
|
||||
"repairs a Codex timeout without replacing an equivalent caller-selected launcher",
|
||||
async ({ cli }) => {
|
||||
const home = await createTestTempDirectory("rea-codex-alias-repair-");
|
||||
const codexHome = join(home, ".codex");
|
||||
const config = join(codexHome, "config.toml");
|
||||
const backup = `${config}.rea.backup`;
|
||||
const alias = join(home, "rea-launcher-alias.mjs");
|
||||
await mkdir(codexHome);
|
||||
await symlink(resolve("scripts/rea.mjs"), alias, "file");
|
||||
const original = [
|
||||
"# Caller-owned Codex approval policy",
|
||||
'approval_policy = "on-request"',
|
||||
'sandbox_mode = "read-only"',
|
||||
"",
|
||||
"[mcp_servers.rea]",
|
||||
`command = ${JSON.stringify(process.execPath)}`,
|
||||
`args = ${JSON.stringify([alias, "mcp"])}`,
|
||||
"startup_timeout_sec = 0",
|
||||
"tool_timeout_sec = 900",
|
||||
'default_tools_approval_mode = "prompt"',
|
||||
"",
|
||||
"[mcp_servers.rea.tools.open_binary]",
|
||||
'approval_mode = "approve"',
|
||||
"output_token_limit = 2048",
|
||||
"",
|
||||
].join("\n");
|
||||
await writeFile(config, original);
|
||||
const setup = (approval: "--dry-run" | "--yes") =>
|
||||
cli.run({
|
||||
arguments: [
|
||||
"setup",
|
||||
"--client",
|
||||
"codex",
|
||||
"--no-skill",
|
||||
approval,
|
||||
"--json",
|
||||
],
|
||||
cwd: home,
|
||||
environment: {
|
||||
HOME: home,
|
||||
USERPROFILE: home,
|
||||
CODEX_HOME: codexHome,
|
||||
XDG_CONFIG_HOME: join(home, ".config"),
|
||||
XDG_DATA_HOME: join(home, ".local", "share"),
|
||||
XDG_CACHE_HOME: join(home, ".cache"),
|
||||
},
|
||||
timeoutMs: 20_000,
|
||||
});
|
||||
|
||||
const planned = await setup("--dry-run");
|
||||
expect(planned.exitCode, planned.stderr).toBe(0);
|
||||
expect(planned.json).toMatchObject({
|
||||
status: "planned",
|
||||
plannedActions: [
|
||||
expect.objectContaining({
|
||||
id: "configure_client:codex",
|
||||
operation: "update",
|
||||
target: config,
|
||||
backupPath: backup,
|
||||
}),
|
||||
],
|
||||
});
|
||||
expect(await readFile(config, "utf8")).toBe(original);
|
||||
await expect(access(backup)).rejects.toMatchObject({ code: "ENOENT" });
|
||||
|
||||
const applied = await setup("--yes");
|
||||
expect(applied.exitCode, applied.stderr).toBe(0);
|
||||
expect(applied.json).toMatchObject({ status: "ready" });
|
||||
const repaired = await readFile(config, "utf8");
|
||||
expect(parseToml(repaired)).toMatchObject({
|
||||
approval_policy: "on-request",
|
||||
sandbox_mode: "read-only",
|
||||
mcp_servers: {
|
||||
rea: {
|
||||
command: process.execPath,
|
||||
args: [alias, "mcp"],
|
||||
startup_timeout_sec: 30,
|
||||
tool_timeout_sec: 900,
|
||||
default_tools_approval_mode: "prompt",
|
||||
tools: {
|
||||
open_binary: { approval_mode: "approve", output_token_limit: 2048 },
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
expect(await readFile(backup, "utf8")).toBe(original);
|
||||
expect((await lstat(alias)).isSymbolicLink()).toBe(true);
|
||||
|
||||
const repeated = await setup("--yes");
|
||||
expect(repeated.exitCode, repeated.stderr).toBe(0);
|
||||
expect(repeated.json).toMatchObject({
|
||||
status: "ready",
|
||||
plannedActions: [],
|
||||
appliedActions: [],
|
||||
});
|
||||
expect(await readFile(config, "utf8")).toBe(repaired);
|
||||
expect(await readFile(backup, "utf8")).toBe(original);
|
||||
},
|
||||
);
|
||||
|
||||
describe("interactive setup journey", () => {
|
||||
it("shows detected and absent clients without preselecting detected clients", async () => {
|
||||
const result = await runJourney(
|
||||
|
||||
@@ -0,0 +1,155 @@
|
||||
import { mkdir, readFile, readdir, writeFile } from "node:fs/promises";
|
||||
import { dirname, join } from "node:path";
|
||||
|
||||
import { parse as parseToml } from "smol-toml";
|
||||
import { expect, it } from "vitest";
|
||||
|
||||
import { readClientRegistrationStatuses } from "../../../src/application/ClientRegistrationStatus.js";
|
||||
import { systemUninstallHost } from "../../../src/application/Uninstall.js";
|
||||
import { PRODUCT_IDENTITY } from "../../../src/identity.js";
|
||||
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
|
||||
|
||||
const codexSource = (invalid: string): string =>
|
||||
[
|
||||
'caller_root = "keep"',
|
||||
"[mcp_servers.rea]",
|
||||
`command = "${PRODUCT_IDENTITY.cliBinary}"`,
|
||||
'args = ["mcp"]',
|
||||
invalid,
|
||||
"[mcp_servers.peer]",
|
||||
'command = "peer"',
|
||||
"args = []",
|
||||
"",
|
||||
].join("\n");
|
||||
|
||||
it.each([
|
||||
{
|
||||
client: "codex",
|
||||
path: ".codex/config.toml",
|
||||
invalid: "invalid tool policy",
|
||||
source: codexSource('enabled_tools = "not-an-array"'),
|
||||
parse: (text: string): unknown => parseToml(text),
|
||||
remaining: {
|
||||
caller_root: "keep",
|
||||
mcp_servers: { peer: { command: "peer", args: [] } },
|
||||
},
|
||||
},
|
||||
{
|
||||
client: "codex",
|
||||
path: ".codex/config.toml",
|
||||
invalid: "a nonpositive startup timeout",
|
||||
source: codexSource("startup_timeout_sec = 0"),
|
||||
parse: (text: string): unknown => parseToml(text),
|
||||
remaining: {
|
||||
caller_root: "keep",
|
||||
mcp_servers: { peer: { command: "peer", args: [] } },
|
||||
},
|
||||
},
|
||||
{
|
||||
client: "opencode",
|
||||
path: ".config/opencode/opencode.json",
|
||||
invalid: "a non-string environment value",
|
||||
source: JSON.stringify({
|
||||
caller_root: "keep",
|
||||
mcp: {
|
||||
servers: {
|
||||
rea: {
|
||||
type: "local",
|
||||
command: [PRODUCT_IDENTITY.cliBinary, "mcp"],
|
||||
environment: { CALLER_SETTING: 42 },
|
||||
},
|
||||
peer: { type: "local", command: ["peer"] },
|
||||
},
|
||||
},
|
||||
}),
|
||||
parse: (text: string): unknown => JSON.parse(text),
|
||||
remaining: {
|
||||
caller_root: "keep",
|
||||
mcp: { servers: { peer: { type: "local", command: ["peer"] } } },
|
||||
},
|
||||
},
|
||||
])(
|
||||
"removes the owned $client registration despite $invalid",
|
||||
async (sample) => {
|
||||
const home = await createTestTempDirectory("rea-registration-ownership-");
|
||||
const configPath = join(home, sample.path);
|
||||
const environment = {
|
||||
CODEX_HOME: join(home, ".codex"),
|
||||
XDG_CONFIG_HOME: join(home, ".config"),
|
||||
};
|
||||
await mkdir(dirname(configPath), { recursive: true });
|
||||
await writeFile(configPath, sample.source);
|
||||
const statuses = await readClientRegistrationStatuses(home, undefined, {
|
||||
environment,
|
||||
});
|
||||
expect(
|
||||
statuses.find(({ client }) => client === sample.client),
|
||||
).toMatchObject({
|
||||
state: "invalid",
|
||||
});
|
||||
const host = systemUninstallHost(home, undefined, environment);
|
||||
try {
|
||||
const client = (await host.clients()).find(
|
||||
(client) =>
|
||||
client.name === sample.client && client.configPath === configPath,
|
||||
);
|
||||
if (client === undefined)
|
||||
throw new Error("Expected the configured client");
|
||||
expect(await host.removeClient(client)).toMatchObject({
|
||||
name: sample.client,
|
||||
status: "removed",
|
||||
});
|
||||
expect(sample.parse(await readFile(configPath, "utf8"))).toEqual(
|
||||
sample.remaining,
|
||||
);
|
||||
expect(await readFile(`${configPath}.rea.backup`, "utf8")).toBe(
|
||||
sample.source,
|
||||
);
|
||||
} finally {
|
||||
expect(await host.close?.()).toBeUndefined();
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it("retains an OpenCode remote registration with a REA-like command", async () => {
|
||||
const home = await createTestTempDirectory("rea-registration-remote-owner-");
|
||||
const configPath = join(home, ".config/opencode/opencode.json");
|
||||
const environment = { XDG_CONFIG_HOME: join(home, ".config") };
|
||||
const source = JSON.stringify({
|
||||
caller_root: "keep",
|
||||
mcp: {
|
||||
servers: {
|
||||
rea: {
|
||||
type: "remote",
|
||||
url: "https://caller.example/mcp",
|
||||
command: [PRODUCT_IDENTITY.cliBinary, "mcp"],
|
||||
},
|
||||
peer: { type: "local", command: ["peer"] },
|
||||
},
|
||||
},
|
||||
});
|
||||
await mkdir(dirname(configPath), { recursive: true });
|
||||
await writeFile(configPath, source);
|
||||
const statuses = await readClientRegistrationStatuses(home, undefined, {
|
||||
environment,
|
||||
});
|
||||
expect(statuses.find(({ client }) => client === "opencode")).toMatchObject({
|
||||
state: "invalid",
|
||||
});
|
||||
const host = systemUninstallHost(home, undefined, environment);
|
||||
try {
|
||||
const client = (await host.clients()).find(
|
||||
(client) =>
|
||||
client.name === "opencode" && client.configPath === configPath,
|
||||
);
|
||||
if (client === undefined) throw new Error("Expected the configured client");
|
||||
expect(await host.removeClient(client)).toMatchObject({
|
||||
name: "opencode",
|
||||
status: "retained",
|
||||
});
|
||||
expect(await readFile(configPath, "utf8")).toBe(source);
|
||||
expect(await readdir(dirname(configPath))).toEqual(["opencode.json"]);
|
||||
} finally {
|
||||
expect(await host.close?.()).toBeUndefined();
|
||||
}
|
||||
});
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
} from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { parse as parseToml } from "smol-toml";
|
||||
|
||||
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
|
||||
|
||||
@@ -69,6 +70,57 @@ it.skipIf(process.platform === "win32")(
|
||||
},
|
||||
);
|
||||
|
||||
it.skipIf(process.platform === "win32")(
|
||||
"repairs a Codex timeout without replacing an equivalent caller launcher alias",
|
||||
async () => {
|
||||
const home = await createTestTempDirectory(
|
||||
"rea-config-timeout-launcher-alias-",
|
||||
);
|
||||
const real = join(home, "rea-current");
|
||||
const alias = join(home, "rea-alias");
|
||||
await writeFile(real, "current launcher\n");
|
||||
await symlink(real, alias);
|
||||
await mkdir(join(home, ".codex"));
|
||||
const configPath = join(home, ".codex", "config.toml");
|
||||
const original = [
|
||||
"[mcp_servers.rea]",
|
||||
`command = ${JSON.stringify(alias)}`,
|
||||
'args = ["mcp"]',
|
||||
"startup_timeout_sec = 0",
|
||||
"tool_timeout_sec = 450",
|
||||
"future_policy = { retain = true }",
|
||||
"",
|
||||
"[mcp_servers.rea.env]",
|
||||
'CALLER_SETTING = "keep exactly"',
|
||||
"",
|
||||
].join("\n");
|
||||
await writeFile(configPath, original);
|
||||
const client = { name: "codex", format: "toml", configPath } as const;
|
||||
expect(
|
||||
await configureClientConfiguration(client, {}, [real, "mcp"]),
|
||||
).toMatchObject({ status: "configured" });
|
||||
const configured = await readFile(configPath, "utf8");
|
||||
expect(parseToml(configured)).toMatchObject({
|
||||
mcp_servers: {
|
||||
rea: {
|
||||
command: alias,
|
||||
args: ["mcp"],
|
||||
startup_timeout_sec: 30,
|
||||
tool_timeout_sec: 450,
|
||||
future_policy: { retain: true },
|
||||
env: { CALLER_SETTING: "keep exactly" },
|
||||
},
|
||||
},
|
||||
});
|
||||
expect(await readFile(`${configPath}.rea.backup`, "utf8")).toBe(original);
|
||||
expect(
|
||||
await configureClientConfiguration(client, {}, [real, "mcp"]),
|
||||
).toEqual({ status: "unchanged" });
|
||||
expect(await readFile(configPath, "utf8")).toBe(configured);
|
||||
expect(await readFile(`${configPath}.rea.backup`, "utf8")).toBe(original);
|
||||
},
|
||||
);
|
||||
|
||||
describe("JSON client configuration transaction: writing configuration through safe paths", () => {
|
||||
it.skipIf(typeof process.getuid !== "function")(
|
||||
"rejects a symlink target not owned by the current user",
|
||||
|
||||
Reference in new issue
Block a user