From 2263055fe718e7c0cb82fe610c32cf1926da9c83 Mon Sep 17 00:00:00 2001 From: dyoshikawa Date: Tue, 28 Jul 2026 21:27:32 -0700 Subject: [PATCH 1/2] feat(codexcli): follow the SessionEnd hook, Windows command override, and MCP environment targeting Adds the SessionEnd hook event (Codex CLI 0.145.0) and the per-hook commandWindows/statusMessage fields (0.131.0), accepts the { name, source } object form of MCP env_vars, and models experimental_environment as a codex-scoped canonical field so it reaches Codex without leaking into every other tool's MCP config. --- docs/reference/file-formats.md | 12 +++- skills/rulesync/file-formats.md | 12 +++- src/features/hooks/codexcli-hooks.test.ts | 69 ++++++++++++++++++- src/features/hooks/codexcli-hooks.ts | 8 +++ src/features/hooks/tool-hooks-converter.ts | 2 +- src/features/mcp/codexcli-mcp.test.ts | 80 ++++++++++++++++++++++ src/features/mcp/codexcli-mcp.ts | 70 +++++++++++++++++-- src/features/mcp/rulesync-mcp.test.ts | 23 +++++++ src/features/mcp/rulesync-mcp.ts | 23 +++++-- src/types/hooks.ts | 12 ++++ src/types/mcp.ts | 23 ++++++- 11 files changed, 317 insertions(+), 17 deletions(-) diff --git a/docs/reference/file-formats.md b/docs/reference/file-formats.md index ba49fca35..b90afc195 100644 --- a/docs/reference/file-formats.md +++ b/docs/reference/file-formats.md @@ -164,6 +164,8 @@ Example: - `url` / `headers` / `allowedEnvVars` (optional, `http` hooks): the POST target URL, request headers (values support `$VAR` interpolation), and the env-var allowlist for that interpolation. Forwarded to Claude Code and Qwen Code http hooks. - `server` / `tool` / `input` (optional, `mcp_tool` hooks): the configured MCP server name, the tool to call on it, and the (arbitrary JSON) arguments, whose string values support `${path}` substitution from the hook input. Forwarded to Claude Code mcp_tool hooks. - `model` (optional, `prompt` / `agent` hooks): the model used for evaluation (defaults to a fast model). Forwarded to Claude Code prompt/agent hooks. +- `commandWindows` (optional): a Windows-only override for `command`, so one hook set can be cross-platform. Forwarded to Codex CLI command hooks (`.codex/hooks.json`), which is the only tool that accepts it. +- `statusMessage` (optional): the progress text shown while the hook runs. Forwarded to Qwen Code (command and http hooks) and to Codex CLI command hooks. - `if` (optional): a single permission rule (same syntax as `settings.json` permission rules, e.g. `"Bash(rm *)"`) that filters a hook by tool arguments in addition to the tool name. Forwarded to Claude Code, where it is evaluated only on tool events (`preToolUse`, `postToolUse`, `postToolUseFailure`, `permissionRequest`, `permissionDenied`); it round-trips as an opaque string. Top-level `hooks` keys must be canonical event names; unknown event names are rejected at parse time. Tool-specific override blocks (e.g. `kiro-ide.hooks`) additionally accept tool-native event keys, which pass through verbatim. @@ -175,7 +177,7 @@ Events present in the shared `hooks` block but unsupported by a given tool are s | Event | Cursor | Claude Code | OpenCode | Kilo | Copilot | Copilot CLI | Factory Droid | Gemini CLI | Codex CLI | deepagents | Kiro | Antigravity IDE | Antigravity CLI | Devin | AugmentCode | Goose | | ---------------------- | :----: | :---------: | :------: | :--: | :-----: | :---------: | :-----------: | :--------: | :-------: | :--------: | :--: | :-------------: | :-------------: | :---: | :---------: | :---: | | `sessionStart` | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | — | — | — | ✅ | ✅ | -| `sessionEnd` | ✅ | ✅ | — | — | ✅ | ✅ | ✅ | ✅ | — | ✅ | ✅ | — | — | — | ✅ | ✅ | +| `sessionEnd` | ✅ | ✅ | — | — | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | — | — | — | ✅ | ✅ | | `beforeSubmitPrompt` | ✅ | ✅ | — | — | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | — | — | ✅ | ✅ | ✅ | | `preToolUse` | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | — | ✅ | ✅ | | `postToolUse` | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | — | ✅ | ✅ | @@ -926,6 +928,14 @@ args = ["--from", "git+https://github.com/BeehiveInnovations/pal-mcp-server.git" env_vars = ["OPENAI_API_KEY", "OPENROUTER_API_KEY", "GEMINI_API_KEY"] ``` +An entry may also be an object naming the environment to read the variable from: `{ "name": "REMOTE_TOKEN", "source": "remote" }` reads it from the remote executor environment (and requires remote MCP stdio support), while a bare name and `"source": "local"` read from Codex's own environment. The object form is written to `config.toml` as an inline table, matching Codex's documented shape. + +### Codex-specific: run a stdio server remotely (`experimentalEnvironment`) + +For stdio servers, `experimentalEnvironment: "remote"` starts the server through a remote executor environment when one is available. It is written as `experimental_environment` in `config.toml`. Like `envVars`, it is stripped before every other tool's MCP config is written, so it cannot leak into a config that would not understand it — and for the same reason, a server config copied straight out of a `config.toml` may spell it `experimental_environment`, which is accepted and normalized on the way to Codex. + +See the [Codex MCP reference](https://learn.chatgpt.com/docs/extend/mcp) for both fields. + - Emitted only into the codex CLI output. Stripped from `RulesyncMcp.getMcpServers()` so it does not appear in other tools' generated configs (Claude Code, Kilo, OpenCode, Gemini CLI, Cursor, Cline, Junie, Factorydroid, Rovodev, etc.). - Use this for secrets and API keys you do not want literal-encoded into a committed `mcp.json`. - Precedence: codex CLI resolves these names from the user's runtime shell environment. If a name is also set in `env` (literal value), the codex CLI behavior is upstream-defined; see the [Codex configuration reference](https://developers.openai.com/codex/config-reference#mcp_serversid-env_vars) (last checked 2026-05-13) for the exact resolution rule. diff --git a/skills/rulesync/file-formats.md b/skills/rulesync/file-formats.md index a7eb896d8..b94a04eec 100644 --- a/skills/rulesync/file-formats.md +++ b/skills/rulesync/file-formats.md @@ -164,6 +164,8 @@ Example: - `url` / `headers` / `allowedEnvVars` (optional, `http` hooks): the POST target URL, request headers (values support `$VAR` interpolation), and the env-var allowlist for that interpolation. Forwarded to Claude Code and Qwen Code http hooks. - `server` / `tool` / `input` (optional, `mcp_tool` hooks): the configured MCP server name, the tool to call on it, and the (arbitrary JSON) arguments, whose string values support `${path}` substitution from the hook input. Forwarded to Claude Code mcp_tool hooks. - `model` (optional, `prompt` / `agent` hooks): the model used for evaluation (defaults to a fast model). Forwarded to Claude Code prompt/agent hooks. +- `commandWindows` (optional): a Windows-only override for `command`, so one hook set can be cross-platform. Forwarded to Codex CLI command hooks (`.codex/hooks.json`), which is the only tool that accepts it. +- `statusMessage` (optional): the progress text shown while the hook runs. Forwarded to Qwen Code (command and http hooks) and to Codex CLI command hooks. - `if` (optional): a single permission rule (same syntax as `settings.json` permission rules, e.g. `"Bash(rm *)"`) that filters a hook by tool arguments in addition to the tool name. Forwarded to Claude Code, where it is evaluated only on tool events (`preToolUse`, `postToolUse`, `postToolUseFailure`, `permissionRequest`, `permissionDenied`); it round-trips as an opaque string. Top-level `hooks` keys must be canonical event names; unknown event names are rejected at parse time. Tool-specific override blocks (e.g. `kiro-ide.hooks`) additionally accept tool-native event keys, which pass through verbatim. @@ -175,7 +177,7 @@ Events present in the shared `hooks` block but unsupported by a given tool are s | Event | Cursor | Claude Code | OpenCode | Kilo | Copilot | Copilot CLI | Factory Droid | Gemini CLI | Codex CLI | deepagents | Kiro | Antigravity IDE | Antigravity CLI | Devin | AugmentCode | Goose | | ---------------------- | :----: | :---------: | :------: | :--: | :-----: | :---------: | :-----------: | :--------: | :-------: | :--------: | :--: | :-------------: | :-------------: | :---: | :---------: | :---: | | `sessionStart` | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | — | — | — | ✅ | ✅ | -| `sessionEnd` | ✅ | ✅ | — | — | ✅ | ✅ | ✅ | ✅ | — | ✅ | ✅ | — | — | — | ✅ | ✅ | +| `sessionEnd` | ✅ | ✅ | — | — | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | — | — | — | ✅ | ✅ | | `beforeSubmitPrompt` | ✅ | ✅ | — | — | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | — | — | ✅ | ✅ | ✅ | | `preToolUse` | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | — | ✅ | ✅ | | `postToolUse` | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | — | ✅ | ✅ | @@ -926,6 +928,14 @@ args = ["--from", "git+https://github.com/BeehiveInnovations/pal-mcp-server.git" env_vars = ["OPENAI_API_KEY", "OPENROUTER_API_KEY", "GEMINI_API_KEY"] ``` +An entry may also be an object naming the environment to read the variable from: `{ "name": "REMOTE_TOKEN", "source": "remote" }` reads it from the remote executor environment (and requires remote MCP stdio support), while a bare name and `"source": "local"` read from Codex's own environment. The object form is written to `config.toml` as an inline table, matching Codex's documented shape. + +### Codex-specific: run a stdio server remotely (`experimentalEnvironment`) + +For stdio servers, `experimentalEnvironment: "remote"` starts the server through a remote executor environment when one is available. It is written as `experimental_environment` in `config.toml`. Like `envVars`, it is stripped before every other tool's MCP config is written, so it cannot leak into a config that would not understand it — and for the same reason, a server config copied straight out of a `config.toml` may spell it `experimental_environment`, which is accepted and normalized on the way to Codex. + +See the [Codex MCP reference](https://learn.chatgpt.com/docs/extend/mcp) for both fields. + - Emitted only into the codex CLI output. Stripped from `RulesyncMcp.getMcpServers()` so it does not appear in other tools' generated configs (Claude Code, Kilo, OpenCode, Gemini CLI, Cursor, Cline, Junie, Factorydroid, Rovodev, etc.). - Use this for secrets and API keys you do not want literal-encoded into a committed `mcp.json`. - Precedence: codex CLI resolves these names from the user's runtime shell environment. If a name is also set in `env` (literal value), the codex CLI behavior is upstream-defined; see the [Codex configuration reference](https://developers.openai.com/codex/config-reference#mcp_serversid-env_vars) (last checked 2026-05-13) for the exact resolution rule. diff --git a/src/features/hooks/codexcli-hooks.test.ts b/src/features/hooks/codexcli-hooks.test.ts index 96432e316..cd2491064 100644 --- a/src/features/hooks/codexcli-hooks.test.ts +++ b/src/features/hooks/codexcli-hooks.test.ts @@ -67,7 +67,7 @@ describe("CodexcliHooks", () => { fileContent: JSON.stringify({ hooks: { sessionStart: [{ command: "echo start" }], - sessionEnd: [{ command: "echo end" }], + preModelInvocation: [{ command: "echo before-model" }], subagentStop: [{ command: "echo sub" }], }, }), @@ -82,10 +82,43 @@ describe("CodexcliHooks", () => { const parsed = JSON.parse(codexHooks.getFileContent()); expect(parsed.hooks.SessionStart).toBeDefined(); - expect(parsed.hooks.SessionEnd).toBeUndefined(); + // Codex CLI has no equivalent of the canonical preModelInvocation event. + expect(parsed.hooks.PreModelInvocation).toBeUndefined(); expect(parsed.hooks.SubagentStop).toBeDefined(); }); + it("should emit sessionEnd, commandWindows and statusMessage", async () => { + // SessionEnd landed in Codex CLI 0.145.0; commandWindows (0.131.0) is the + // Windows-only override for `command`, and statusMessage is the TUI label + // shown while the hook runs. https://learn.chatgpt.com/docs/hooks + const rulesyncHooks = new RulesyncHooks( + createMockAiFileParams({ + fileContent: JSON.stringify({ + hooks: { + sessionEnd: [ + { + command: "./scripts/teardown.sh", + commandWindows: "powershell -File ./scripts/teardown.ps1", + statusMessage: "Saving notes", + }, + ], + }, + }), + }), + ); + + const codexHooks = await CodexcliHooks.fromRulesyncHooks({ + outputRoot: testDir, + rulesyncHooks, + validate: true, + }); + + const hook = JSON.parse(codexHooks.getFileContent()).hooks.SessionEnd[0].hooks[0]; + expect(hook.command).toBe("./scripts/teardown.sh"); + expect(hook.commandWindows).toBe("powershell -File ./scripts/teardown.ps1"); + expect(hook.statusMessage).toBe("Saving notes"); + }); + it("should convert subagentStart, subagentStop, and preCompact to PascalCase", async () => { const rulesyncHooks = new RulesyncHooks( createMockAiFileParams({ @@ -297,6 +330,38 @@ describe("CodexcliHooks", () => { }); describe("toRulesyncHooks", () => { + it("should import SessionEnd along with commandWindows and statusMessage", () => { + const codexHooks = new CodexcliHooks( + createMockAiFileParams({ + relativeDirPath: ".codex", + relativeFilePath: "hooks.json", + fileContent: JSON.stringify({ + hooks: { + SessionEnd: [ + { + hooks: [ + { + type: "command", + command: "./scripts/teardown.sh", + commandWindows: "powershell -File ./scripts/teardown.ps1", + statusMessage: "Saving notes", + }, + ], + }, + ], + }, + }), + }), + ); + + expect(codexHooks.toRulesyncHooks().getJson().hooks.sessionEnd?.[0]).toEqual({ + type: "command", + command: "./scripts/teardown.sh", + commandWindows: "powershell -File ./scripts/teardown.ps1", + statusMessage: "Saving notes", + }); + }); + it("should convert Codex CLI format to canonical format", () => { const codexHooks = new CodexcliHooks( createMockAiFileParams({ diff --git a/src/features/hooks/codexcli-hooks.ts b/src/features/hooks/codexcli-hooks.ts index 9a3f4f5e6..60a34a1f8 100644 --- a/src/features/hooks/codexcli-hooks.ts +++ b/src/features/hooks/codexcli-hooks.ts @@ -40,6 +40,14 @@ const CODEXCLI_CONVERTER_CONFIG: ToolHooksConverterConfig = { projectDirVar: "", supportedHookTypes: new Set(["command"]), passthroughFields: ["name", "description"], + // `commandWindows` overrides `command` on Windows; `statusMessage` is the + // label the TUI shows while the hook runs. Both are per-handler fields of + // `.codex/hooks.json` and carry the same name on either side. + // https://learn.chatgpt.com/docs/hooks + stringPassthroughFields: [ + { canonical: "commandWindows", tool: "commandWindows" }, + { canonical: "statusMessage", tool: "statusMessage" }, + ], }; /** diff --git a/src/features/hooks/tool-hooks-converter.ts b/src/features/hooks/tool-hooks-converter.ts index 62f605b12..74755cb69 100644 --- a/src/features/hooks/tool-hooks-converter.ts +++ b/src/features/hooks/tool-hooks-converter.ts @@ -55,7 +55,7 @@ export type ToolHooksConverterConfig = { * for tool-specific opaque strings such as Claude Code's `if` condition. */ stringPassthroughFields?: ReadonlyArray<{ - readonly canonical: "if"; + readonly canonical: "if" | "statusMessage" | "commandWindows"; readonly tool: string; }>; /** diff --git a/src/features/mcp/codexcli-mcp.test.ts b/src/features/mcp/codexcli-mcp.test.ts index 001c9f727..a732269b4 100644 --- a/src/features/mcp/codexcli-mcp.test.ts +++ b/src/features/mcp/codexcli-mcp.test.ts @@ -1090,6 +1090,86 @@ args = ["server.js"] expect(mcpServers.pal.args).toEqual(["pal-mcp-server"]); }); + it("should carry env_vars object entries that name a source environment", async () => { + // Codex accepts `{ name, source }` entries alongside bare names, where + // `source = "remote"` reads the variable from the remote executor + // environment. https://learn.chatgpt.com/docs/extend/mcp + const jsonData = { + mcpServers: { + pal: { + type: "stdio", + command: "uvx", + envVars: ["LOCAL_TOKEN", { name: "REMOTE_TOKEN", source: "remote" }], + }, + }, + }; + const rulesyncMcp = new RulesyncMcp({ + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: ".mcp.json", + fileContent: JSON.stringify(jsonData), + }); + + const codexcliMcp = await CodexcliMcp.fromRulesyncMcp({ + outputRoot: testDir, + rulesyncMcp, + global: true, + }); + + const mcpServers = codexcliMcp.getToml().mcp_servers as any; + expect(mcpServers.pal.env_vars).toEqual([ + "LOCAL_TOKEN", + { name: "REMOTE_TOKEN", source: "remote" }, + ]); + }); + + it("should write experimentalEnvironment as experimental_environment", async () => { + const jsonData = { + mcpServers: { + pal: { type: "stdio", command: "uvx", experimentalEnvironment: "remote" }, + }, + }; + const rulesyncMcp = new RulesyncMcp({ + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: ".mcp.json", + fileContent: JSON.stringify(jsonData), + }); + + const codexcliMcp = await CodexcliMcp.fromRulesyncMcp({ + outputRoot: testDir, + rulesyncMcp, + global: true, + }); + + const mcpServers = codexcliMcp.getToml().mcp_servers as any; + expect(mcpServers.pal.experimental_environment).toBe("remote"); + expect(mcpServers.pal.experimentalEnvironment).toBeUndefined(); + }); + + it("should accept the raw snake_case spelling of experimental_environment", async () => { + // What someone copying a server out of a codex config.toml writes. It is + // stripped for every other tool, so it has to be re-read here or it would + // reach nothing at all. + const jsonData = { + mcpServers: { + pal: { type: "stdio", command: "uvx", experimental_environment: "remote" }, + }, + }; + const rulesyncMcp = new RulesyncMcp({ + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: ".mcp.json", + fileContent: JSON.stringify(jsonData), + }); + + const codexcliMcp = await CodexcliMcp.fromRulesyncMcp({ + outputRoot: testDir, + rulesyncMcp, + global: true, + }); + + const mcpServers = codexcliMcp.getToml().mcp_servers as any; + expect(mcpServers.pal.experimental_environment).toBe("remote"); + }); + it("should coexist envVars and env on the same server", async () => { // `envVars` (list of names inherited from shell) and `env` (literal // name→value map) are distinct concepts. Both must serialize correctly diff --git a/src/features/mcp/codexcli-mcp.ts b/src/features/mcp/codexcli-mcp.ts index 169f00942..e3fd58cf1 100644 --- a/src/features/mcp/codexcli-mcp.ts +++ b/src/features/mcp/codexcli-mcp.ts @@ -37,8 +37,34 @@ const RULESYNC_TO_CODEX_FIELD_MAP: Record = { envVars: "env_vars", }; +const CODEX_TO_RULESYNC_SCALAR_FIELD_MAP: Record = { + experimental_environment: "experimentalEnvironment", +}; + +const RULESYNC_TO_CODEX_SCALAR_FIELD_MAP: Record = { + experimentalEnvironment: "experimental_environment", +}; + const MAX_REMOVE_EMPTY_ENTRIES_DEPTH = 32; +/** + * `env_vars` entries are either a bare variable name or `{ name, source }`, + * where `source = "remote"` reads the variable from the remote executor + * environment. The other renamed keys (`enabled_tools`, `disabled_tools`) stay + * plain string arrays, so this guard is applied to `env_vars` only. + * @see https://learn.chatgpt.com/docs/extend/mcp + */ +function isEnvVarsArray(value: unknown): value is (string | Record)[] { + return ( + Array.isArray(value) && + value.every((entry) => typeof entry === "string" || (isRecord(entry) && "name" in entry)) + ); +} + +function isValidRenamedArray(key: string, value: unknown): boolean { + return key === "env_vars" || key === "envVars" ? isEnvVarsArray(value) : isStringArray(value); +} + /** * Translate a server's `oauth` table from the canonical rulesync shape (Claude * Code style camelCase) into the shape Codex CLI understands. Codex expects the @@ -119,12 +145,24 @@ function convertFromCodexFormat(codexMcp: Record): McpServers { } else if (Object.hasOwn(CODEX_TO_RULESYNC_FIELD_MAP, key)) { const mappedKey = CODEX_TO_RULESYNC_FIELD_MAP[key]; if (mappedKey) { - if (isStringArray(value)) { + if (isValidRenamedArray(key, value)) { converted[mappedKey] = value; } else { warnWithFallback(undefined, `Ignored malformed array for ${key} in MCP server ${name}`); } } + } else if (Object.hasOwn(CODEX_TO_RULESYNC_SCALAR_FIELD_MAP, key)) { + const mappedKey = CODEX_TO_RULESYNC_SCALAR_FIELD_MAP[key]; + if (mappedKey) { + if (typeof value === "string") { + converted[mappedKey] = value; + } else { + warnWithFallback( + undefined, + `Ignored malformed value for ${key} in MCP server ${name}: expected a string`, + ); + } + } } else { converted[key] = value; } @@ -161,7 +199,7 @@ function convertToCodexFormat(mcpServers: McpServers): Record { } else if (Object.hasOwn(RULESYNC_TO_CODEX_FIELD_MAP, key)) { const mappedKey = RULESYNC_TO_CODEX_FIELD_MAP[key]; if (mappedKey) { - if (isStringArray(value)) { + if (isValidRenamedArray(key, value)) { converted[mappedKey] = value; } else { warnWithFallback( @@ -170,6 +208,18 @@ function convertToCodexFormat(mcpServers: McpServers): Record { ); } } + } else if (Object.hasOwn(RULESYNC_TO_CODEX_SCALAR_FIELD_MAP, key)) { + const mappedKey = RULESYNC_TO_CODEX_SCALAR_FIELD_MAP[key]; + if (mappedKey) { + if (typeof value === "string") { + converted[mappedKey] = value; + } else { + warnWithFallback( + undefined, + `[CodexCliMcp] Skipping invalid value type for mapped key '${key}': expected string, got ${typeof value}`, + ); + } + } } else { converted[key] = value; } @@ -290,11 +340,21 @@ export class CodexcliMcp extends ToolMcp { serverName, { ...serverConfig, - // Only envVars needs manual re-merging here. Other codex-specific fields - // (like disabledTools) are preserved by RulesyncMcp's filtering natively. - ...(isRecord(rawServer) && isStringArray(rawServer.envVars) + // Only the codex-only fields stripped by `getMcpServers()` need + // manual re-merging here. Other codex-specific fields (like + // disabledTools) are preserved by RulesyncMcp's filtering natively. + ...(isRecord(rawServer) && isEnvVarsArray(rawServer.envVars) ? { envVars: rawServer.envVars } : {}), + // Both spellings are accepted, so a server config copied straight + // out of a codex `config.toml` keeps working; the canonical + // camelCase form wins when someone wrote both. + ...(isRecord(rawServer) && typeof rawServer.experimental_environment === "string" + ? { experimentalEnvironment: rawServer.experimental_environment } + : {}), + ...(isRecord(rawServer) && typeof rawServer.experimentalEnvironment === "string" + ? { experimentalEnvironment: rawServer.experimentalEnvironment } + : {}), }, ]; }), diff --git a/src/features/mcp/rulesync-mcp.test.ts b/src/features/mcp/rulesync-mcp.test.ts index b5ad5b134..8f3b31d0a 100644 --- a/src/features/mcp/rulesync-mcp.test.ts +++ b/src/features/mcp/rulesync-mcp.test.ts @@ -1321,6 +1321,29 @@ describe("RulesyncMcp", () => { expect((servers.pal as any).envVars).toBeUndefined(); }); + it("should strip both spellings of the codex-only experimental_environment", () => { + // Codex-only, and meaningless to every other tool. The camelCase form is + // canonical; the snake_case one is what someone copying a codex config + // writes, and it leaked into every other tool's config before. + const rulesyncMcp = new RulesyncMcp({ + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: "mcp.json", + fileContent: JSON.stringify({ + mcpServers: { + canonical: { command: "uvx", experimentalEnvironment: "remote" }, + raw: { command: "uvx", experimental_environment: "remote" }, + }, + }), + }); + + const servers = rulesyncMcp.getMcpServers(); + + expect((servers.canonical as any).experimentalEnvironment).toBeUndefined(); + expect((servers.raw as any).experimental_environment).toBeUndefined(); + expect((servers.canonical as any).command).toBe("uvx"); + expect((servers.raw as any).command).toBe("uvx"); + }); + it("should still expose envVars via getJson() for the codex generator", () => { const rulesyncMcp = new RulesyncMcp({ relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, diff --git a/src/features/mcp/rulesync-mcp.ts b/src/features/mcp/rulesync-mcp.ts index a488c34fa..0db3e7822 100644 --- a/src/features/mcp/rulesync-mcp.ts +++ b/src/features/mcp/rulesync-mcp.ts @@ -227,14 +227,25 @@ export class RulesyncMcp extends RulesyncFile { // `forTarget()` merges into the shared map before this runs. .filter(([, serverConfig]) => serverConfig.enabled !== false) .map(([serverName, serverConfig]) => { - // `envVars` is codex-specific: the codex generator reads it directly - // from the unfiltered source JSON. Strip here so it does not leak - // into other tools' outputs. `enabled` is stripped because OpenCode, - // Kilo, Grok CLI and Goose have a NATIVE `enabled` field with - // different semantics a leaked value would silently collide with. + // `envVars` and `experimentalEnvironment` are codex-specific: the + // codex generator reads them directly from the unfiltered source + // JSON. Strip here so they do not leak into other tools' outputs — + // including the raw `experimental_environment` spelling, which is + // what someone copying a codex config writes and which no other tool + // understands. `enabled` is stripped because OpenCode, Kilo, Grok CLI + // and Goose have a NATIVE `enabled` field with different semantics a + // leaked value would silently collide with. return [ serverName, - omit(serverConfig, ["targets", "description", "exposed", "envVars", "enabled"]), + omit(serverConfig, [ + "targets", + "description", + "exposed", + "envVars", + "experimentalEnvironment", + "experimental_environment", + "enabled", + ]), ]; }), ); diff --git a/src/types/hooks.ts b/src/types/hooks.ts index 62002aaf8..f496a9715 100644 --- a/src/types/hooks.ts +++ b/src/types/hooks.ts @@ -114,6 +114,12 @@ export const HookDefinitionSchema = z.looseObject({ // (`&&`/`||`/list) syntax, so it round-trips as an opaque string. // https://code.claude.com/docs/en/hooks if: z.optional(safeString), + // Codex CLI command hooks: a Windows-only override for `command`, so one hook + // set can be cross-platform. Added in Codex CLI 0.131.0 (PR #22159); spelled + // `command_windows` in the inline TOML `[hooks]` form and `commandWindows` in + // `.codex/hooks.json`, which is the file rulesync writes. + // https://learn.chatgpt.com/docs/hooks + commandWindows: z.optional(safeString), }); export type HookDefinition = z.infer; @@ -446,6 +452,11 @@ export const DEEPAGENTS_HOOK_EVENTS: readonly HookEvent[] = [ /** Hook events supported by Codex CLI. */ export const CODEXCLI_HOOK_EVENTS: readonly HookEvent[] = [ "sessionStart", + // Added in Codex CLI 0.145.0 (PR #33895). Its matcher is the end reason and + // its timeout is capped at 3s, but neither is modelled differently here: + // the matcher is already a free string and the timeout is the tool's to + // enforce. https://github.com/openai/codex/releases/tag/rust-v0.145.0 + "sessionEnd", "preToolUse", "postToolUse", "beforeSubmitPrompt", @@ -1181,6 +1192,7 @@ export const COPILOTCLI_TO_CANONICAL_EVENT_NAMES: Record = Objec */ export const CANONICAL_TO_CODEXCLI_EVENT_NAMES: Record = { sessionStart: "SessionStart", + sessionEnd: "SessionEnd", preToolUse: "PreToolUse", postToolUse: "PostToolUse", beforeSubmitPrompt: "UserPromptSubmit", diff --git a/src/types/mcp.ts b/src/types/mcp.ts index 94d348dd2..7e15aec5f 100644 --- a/src/types/mcp.ts +++ b/src/types/mcp.ts @@ -9,6 +9,20 @@ const EnvVarNameSchema = z ), ); +/** + * One `envVars` entry. A bare name reads the variable from Codex's own + * environment; the object form names the environment to read it from, where + * `source = "remote"` reads from the remote executor environment. + * @see https://learn.chatgpt.com/docs/extend/mcp + */ +const EnvVarEntrySchema = z.union([ + EnvVarNameSchema, + z.looseObject({ + name: EnvVarNameSchema, + source: z.optional(z.enum(["local", "remote"])), + }), +]); + export const McpServerSchema = z.looseObject({ // `streamable-http` is the MCP spec's transport name and an accepted alias for // `http` (Claude Code), so configs copied from server docs work unchanged. @@ -28,7 +42,14 @@ export const McpServerSchema = z.looseObject({ // `enabledTools`→`enabled_tools` precedent in `codexcli-mcp.ts`). // Stripped by `RulesyncMcp.getMcpServers()` so it does not leak into // other tools' configs. - envVars: z.optional(z.array(EnvVarNameSchema)), + envVars: z.optional(z.array(EnvVarEntrySchema)), + // Codex CLI-specific (stdio servers): set to `remote` to start the server + // through a remote executor environment when one is available. Kept as a + // plain string rather than an enum so a value Codex adds later is not + // rejected outright. Written as `experimental_environment` in codex TOML and + // stripped by `RulesyncMcp.getMcpServers()`, like `envVars`. + // https://learn.chatgpt.com/docs/extend/mcp + experimentalEnvironment: z.optional(z.string()), disabled: z.optional(z.boolean()), networkTimeout: z.optional(z.number()), timeout: z.optional(z.number()), From 4d5503b5127770e2dccaa148b300d2609208189a Mon Sep 17 00:00:00 2001 From: dyoshikawa Date: Tue, 28 Jul 2026 21:41:50 -0700 Subject: [PATCH 2/2] fix(codexcli): reject env_vars entries Codex would refuse, in both directions Review follow-ups: the env_vars object entry is a strict schema, since upstream denies unknown fields there and one stray key rejects the whole config.toml. The import guard now shares that schema, so a config.toml entry of some other shape is dropped with a warning instead of being written into a .rulesync/mcp.jsonc the next generate would refuse to parse. Also moves the experimentalEnvironment docs section out of the middle of the envVars one and derives the reverse scalar field map. --- docs/reference/file-formats.md | 10 ++--- skills/rulesync/file-formats.md | 10 ++--- src/features/mcp/codexcli-mcp.test.ts | 54 +++++++++++++++++++++++++++ src/features/mcp/codexcli-mcp.ts | 26 ++++++------- src/types/mcp.test.ts | 43 ++++++++++++++++++++- src/types/mcp.ts | 18 ++++++++- 6 files changed, 133 insertions(+), 28 deletions(-) diff --git a/docs/reference/file-formats.md b/docs/reference/file-formats.md index b90afc195..19ab383d6 100644 --- a/docs/reference/file-formats.md +++ b/docs/reference/file-formats.md @@ -928,7 +928,11 @@ args = ["--from", "git+https://github.com/BeehiveInnovations/pal-mcp-server.git" env_vars = ["OPENAI_API_KEY", "OPENROUTER_API_KEY", "GEMINI_API_KEY"] ``` -An entry may also be an object naming the environment to read the variable from: `{ "name": "REMOTE_TOKEN", "source": "remote" }` reads it from the remote executor environment (and requires remote MCP stdio support), while a bare name and `"source": "local"` read from Codex's own environment. The object form is written to `config.toml` as an inline table, matching Codex's documented shape. +An entry may also be an object naming the environment to read the variable from: `{ "name": "REMOTE_TOKEN", "source": "remote" }` reads it from the remote executor environment (and requires remote MCP stdio support), while a bare name and `"source": "local"` read from Codex's own environment. The object form is written to `config.toml` as an inline table, matching Codex's documented shape. Only `name` and `source` are accepted in that object — Codex rejects an unknown key there, and rejecting one server's entry would take the whole `config.toml` down with it, so Rulesync fails on the canonical file instead. For the same reason an entry that a `config.toml` already holds in some other shape is dropped with a warning on import rather than written into a `.rulesync/mcp.jsonc` the next generate would refuse. + +- Emitted only into the codex CLI output. Stripped from `RulesyncMcp.getMcpServers()` so it does not appear in other tools' generated configs (Claude Code, Kilo, OpenCode, Gemini CLI, Cursor, Cline, Junie, Factorydroid, Rovodev, etc.). +- Use this for secrets and API keys you do not want literal-encoded into a committed `mcp.json`. +- Precedence: codex CLI resolves these names from the user's runtime shell environment. If a name is also set in `env` (literal value), the codex CLI behavior is upstream-defined; see the [Codex configuration reference](https://developers.openai.com/codex/config-reference#mcp_serversid-env_vars) (last checked 2026-05-13) for the exact resolution rule. ### Codex-specific: run a stdio server remotely (`experimentalEnvironment`) @@ -936,10 +940,6 @@ For stdio servers, `experimentalEnvironment: "remote"` starts the server through See the [Codex MCP reference](https://learn.chatgpt.com/docs/extend/mcp) for both fields. -- Emitted only into the codex CLI output. Stripped from `RulesyncMcp.getMcpServers()` so it does not appear in other tools' generated configs (Claude Code, Kilo, OpenCode, Gemini CLI, Cursor, Cline, Junie, Factorydroid, Rovodev, etc.). -- Use this for secrets and API keys you do not want literal-encoded into a committed `mcp.json`. -- Precedence: codex CLI resolves these names from the user's runtime shell environment. If a name is also set in `env` (literal value), the codex CLI behavior is upstream-defined; see the [Codex configuration reference](https://developers.openai.com/codex/config-reference#mcp_serversid-env_vars) (last checked 2026-05-13) for the exact resolution rule. - #### Codex-specific: OAuth client id (`oauth.clientId` → `client_id`) A server's `oauth` block is preserved in the canonical Claude Code shape (camelCase `clientId`), but Codex CLI reads the OAuth client id from snake_case `oauth.client_id`. Without it, `codex mcp login ` falls back to dynamic client registration and fails for providers that do not support it (e.g. Slack). The codex generator therefore **duplicates** `clientId` into a sibling `client_id`, keeping the camelCase key so tools that expect it keep working: diff --git a/skills/rulesync/file-formats.md b/skills/rulesync/file-formats.md index b94a04eec..0018a6b3a 100644 --- a/skills/rulesync/file-formats.md +++ b/skills/rulesync/file-formats.md @@ -928,7 +928,11 @@ args = ["--from", "git+https://github.com/BeehiveInnovations/pal-mcp-server.git" env_vars = ["OPENAI_API_KEY", "OPENROUTER_API_KEY", "GEMINI_API_KEY"] ``` -An entry may also be an object naming the environment to read the variable from: `{ "name": "REMOTE_TOKEN", "source": "remote" }` reads it from the remote executor environment (and requires remote MCP stdio support), while a bare name and `"source": "local"` read from Codex's own environment. The object form is written to `config.toml` as an inline table, matching Codex's documented shape. +An entry may also be an object naming the environment to read the variable from: `{ "name": "REMOTE_TOKEN", "source": "remote" }` reads it from the remote executor environment (and requires remote MCP stdio support), while a bare name and `"source": "local"` read from Codex's own environment. The object form is written to `config.toml` as an inline table, matching Codex's documented shape. Only `name` and `source` are accepted in that object — Codex rejects an unknown key there, and rejecting one server's entry would take the whole `config.toml` down with it, so Rulesync fails on the canonical file instead. For the same reason an entry that a `config.toml` already holds in some other shape is dropped with a warning on import rather than written into a `.rulesync/mcp.jsonc` the next generate would refuse. + +- Emitted only into the codex CLI output. Stripped from `RulesyncMcp.getMcpServers()` so it does not appear in other tools' generated configs (Claude Code, Kilo, OpenCode, Gemini CLI, Cursor, Cline, Junie, Factorydroid, Rovodev, etc.). +- Use this for secrets and API keys you do not want literal-encoded into a committed `mcp.json`. +- Precedence: codex CLI resolves these names from the user's runtime shell environment. If a name is also set in `env` (literal value), the codex CLI behavior is upstream-defined; see the [Codex configuration reference](https://developers.openai.com/codex/config-reference#mcp_serversid-env_vars) (last checked 2026-05-13) for the exact resolution rule. ### Codex-specific: run a stdio server remotely (`experimentalEnvironment`) @@ -936,10 +940,6 @@ For stdio servers, `experimentalEnvironment: "remote"` starts the server through See the [Codex MCP reference](https://learn.chatgpt.com/docs/extend/mcp) for both fields. -- Emitted only into the codex CLI output. Stripped from `RulesyncMcp.getMcpServers()` so it does not appear in other tools' generated configs (Claude Code, Kilo, OpenCode, Gemini CLI, Cursor, Cline, Junie, Factorydroid, Rovodev, etc.). -- Use this for secrets and API keys you do not want literal-encoded into a committed `mcp.json`. -- Precedence: codex CLI resolves these names from the user's runtime shell environment. If a name is also set in `env` (literal value), the codex CLI behavior is upstream-defined; see the [Codex configuration reference](https://developers.openai.com/codex/config-reference#mcp_serversid-env_vars) (last checked 2026-05-13) for the exact resolution rule. - #### Codex-specific: OAuth client id (`oauth.clientId` → `client_id`) A server's `oauth` block is preserved in the canonical Claude Code shape (camelCase `clientId`), but Codex CLI reads the OAuth client id from snake_case `oauth.client_id`. Without it, `codex mcp login ` falls back to dynamic client registration and fails for providers that do not support it (e.g. Slack). The codex generator therefore **duplicates** `clientId` into a sibling `client_id`, keeping the camelCase key so tools that expect it keep working: diff --git a/src/features/mcp/codexcli-mcp.test.ts b/src/features/mcp/codexcli-mcp.test.ts index a732269b4..26bc1497b 100644 --- a/src/features/mcp/codexcli-mcp.test.ts +++ b/src/features/mcp/codexcli-mcp.test.ts @@ -1170,6 +1170,60 @@ args = ["server.js"] expect(mcpServers.pal.experimental_environment).toBe("remote"); }); + it("should drop an env_vars entry carrying a key Codex would reject", async () => { + // Upstream's McpServerEnvVar is deny_unknown_fields, so one stray key + // makes Codex reject the whole config.toml rather than this one entry. + const jsonData = { + mcpServers: { + pal: { + command: "uvx", + envVars: [{ name: "A", unexpected: true }], + }, + }, + }; + const rulesyncMcp = new RulesyncMcp({ + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: ".mcp.json", + fileContent: JSON.stringify(jsonData), + }); + + const codexcliMcp = await CodexcliMcp.fromRulesyncMcp({ + outputRoot: testDir, + rulesyncMcp, + global: true, + }); + + const mcpServers = codexcliMcp.getToml().mcp_servers as any; + expect(mcpServers.pal.env_vars).toBeUndefined(); + expect(mcpServers.pal.command).toBe("uvx"); + }); + + it("should let the canonical experimentalEnvironment win over the raw spelling", async () => { + const jsonData = { + mcpServers: { + pal: { + command: "uvx", + experimentalEnvironment: "remote", + experimental_environment: "local", + }, + }, + }; + const rulesyncMcp = new RulesyncMcp({ + relativeDirPath: RULESYNC_RELATIVE_DIR_PATH, + relativeFilePath: ".mcp.json", + fileContent: JSON.stringify(jsonData), + }); + + const codexcliMcp = await CodexcliMcp.fromRulesyncMcp({ + outputRoot: testDir, + rulesyncMcp, + global: true, + }); + + const mcpServers = codexcliMcp.getToml().mcp_servers as any; + expect(mcpServers.pal.experimental_environment).toBe("remote"); + }); + it("should coexist envVars and env on the same server", async () => { // `envVars` (list of names inherited from shell) and `env` (literal // name→value map) are distinct concepts. Both must serialize correctly diff --git a/src/features/mcp/codexcli-mcp.ts b/src/features/mcp/codexcli-mcp.ts index e3fd58cf1..e617e46f3 100644 --- a/src/features/mcp/codexcli-mcp.ts +++ b/src/features/mcp/codexcli-mcp.ts @@ -5,7 +5,7 @@ import * as smolToml from "smol-toml"; import { CODEXCLI_DIR, CODEXCLI_MCP_FILE_NAME } from "../../constants/codexcli-paths.js"; import { ValidationResult } from "../../types/ai-file.js"; -import { McpServers } from "../../types/mcp.js"; +import { isEnvVarEntryArray, McpServers } from "../../types/mcp.js"; import { formatError } from "../../utils/error.js"; import { readFileContentOrNull } from "../../utils/file.js"; import { warnWithFallback } from "../../utils/logger.js"; @@ -37,32 +37,28 @@ const RULESYNC_TO_CODEX_FIELD_MAP: Record = { envVars: "env_vars", }; -const CODEX_TO_RULESYNC_SCALAR_FIELD_MAP: Record = { - experimental_environment: "experimentalEnvironment", -}; - const RULESYNC_TO_CODEX_SCALAR_FIELD_MAP: Record = { experimentalEnvironment: "experimental_environment", }; +const CODEX_TO_RULESYNC_SCALAR_FIELD_MAP: Record = Object.fromEntries( + Object.entries(RULESYNC_TO_CODEX_SCALAR_FIELD_MAP).map(([canonical, codex]) => [ + codex, + canonical, + ]), +); + const MAX_REMOVE_EMPTY_ENTRIES_DEPTH = 32; /** * `env_vars` entries are either a bare variable name or `{ name, source }`, * where `source = "remote"` reads the variable from the remote executor * environment. The other renamed keys (`enabled_tools`, `disabled_tools`) stay - * plain string arrays, so this guard is applied to `env_vars` only. + * plain string arrays, so the widened check applies to `env_vars` only. * @see https://learn.chatgpt.com/docs/extend/mcp */ -function isEnvVarsArray(value: unknown): value is (string | Record)[] { - return ( - Array.isArray(value) && - value.every((entry) => typeof entry === "string" || (isRecord(entry) && "name" in entry)) - ); -} - function isValidRenamedArray(key: string, value: unknown): boolean { - return key === "env_vars" || key === "envVars" ? isEnvVarsArray(value) : isStringArray(value); + return key === "env_vars" || key === "envVars" ? isEnvVarEntryArray(value) : isStringArray(value); } /** @@ -343,7 +339,7 @@ export class CodexcliMcp extends ToolMcp { // Only the codex-only fields stripped by `getMcpServers()` need // manual re-merging here. Other codex-specific fields (like // disabledTools) are preserved by RulesyncMcp's filtering natively. - ...(isRecord(rawServer) && isEnvVarsArray(rawServer.envVars) + ...(isRecord(rawServer) && isEnvVarEntryArray(rawServer.envVars) ? { envVars: rawServer.envVars } : {}), // Both spellings are accepted, so a server config copied straight diff --git a/src/types/mcp.test.ts b/src/types/mcp.test.ts index b3f8400a6..71a7fa704 100644 --- a/src/types/mcp.test.ts +++ b/src/types/mcp.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; -import { McpServerSchema, isMcpServers } from "./mcp.js"; +import { McpServerSchema, isEnvVarEntryArray, isMcpServers } from "./mcp.js"; describe("isMcpServers", () => { it("should return true for a plain object", () => { @@ -93,3 +93,44 @@ describe("McpServerSchema", () => { expect(result.success).toBe(false); }); }); + +describe("envVars entries", () => { + it("should accept bare names and the object form naming a source", () => { + const result = McpServerSchema.safeParse({ + command: "uvx", + envVars: [ + "LOCAL_TOKEN", + { name: "REMOTE_TOKEN", source: "remote" }, + { name: "L", source: "local" }, + ], + }); + expect(result.success).toBe(true); + }); + + it("should reject an object entry carrying a key Codex would not accept", () => { + // Upstream deny_unknown_fields: one stray key rejects the whole config.toml. + expect( + McpServerSchema.safeParse({ command: "uvx", envVars: [{ name: "A", extra: 1 }] }).success, + ).toBe(false); + }); + + it("should reject an unknown source and a non-name entry", () => { + expect( + McpServerSchema.safeParse({ command: "uvx", envVars: [{ name: "A", source: "bogus" }] }) + .success, + ).toBe(false); + expect( + McpServerSchema.safeParse({ command: "uvx", envVars: [{ source: "remote" }] }).success, + ).toBe(false); + expect(McpServerSchema.safeParse({ command: "uvx", envVars: ["1BAD"] }).success).toBe(false); + }); +}); + +describe("isEnvVarEntryArray", () => { + it("should agree with the schema, so import cannot write what generate rejects", () => { + expect(isEnvVarEntryArray(["A", { name: "B", source: "remote" }])).toBe(true); + expect(isEnvVarEntryArray([{ name: "A", source: "bogus" }])).toBe(false); + expect(isEnvVarEntryArray([{ name: "A", extra: 1 }])).toBe(false); + expect(isEnvVarEntryArray("A")).toBe(false); + }); +}); diff --git a/src/types/mcp.ts b/src/types/mcp.ts index 7e15aec5f..6315541d7 100644 --- a/src/types/mcp.ts +++ b/src/types/mcp.ts @@ -15,14 +15,28 @@ const EnvVarNameSchema = z * `source = "remote"` reads from the remote executor environment. * @see https://learn.chatgpt.com/docs/extend/mcp */ -const EnvVarEntrySchema = z.union([ +export const EnvVarEntrySchema = z.union([ EnvVarNameSchema, - z.looseObject({ + // Strict, unlike the loose objects elsewhere in this file: upstream's + // `McpServerEnvVar` denies unknown fields, so one stray key here + // makes Codex reject the whole `config.toml` — every MCP server with it, not + // just this entry. Failing on the rulesync side names the offending file. + z.strictObject({ name: EnvVarNameSchema, source: z.optional(z.enum(["local", "remote"])), }), ]); +/** + * Whether a value is usable as `envVars`. Applied in both directions by the + * codex adapter, so an entry read out of somebody's `config.toml` can never be + * imported into a `.rulesync/mcp.jsonc` that the next generate would refuse to + * parse. + */ +export function isEnvVarEntryArray(value: unknown): value is (string | { name: string })[] { + return Array.isArray(value) && value.every((entry) => EnvVarEntrySchema.safeParse(entry).success); +} + export const McpServerSchema = z.looseObject({ // `streamable-http` is the MCP spec's transport name and an accepted alias for // `http` (Claude Code), so configs copied from server docs work unchanged.