From dc192851f0abfd812ffadc5b3eb1c5c152171cc5 Mon Sep 17 00:00:00 2001 From: Bryce Bjork Date: Mon, 12 Oct 2026 23:02:15 -0400 Subject: [PATCH] fix(server): Codex titles and branch names no longer start the user's MCP servers (#17058) Co-authored-by: Claude Opus 5.5 Co-authored-by: Yash Singh --- .../CodexTextGeneration.test.ts | 72 ++++++++++ .../src/textGeneration/CodexTextGeneration.ts | 126 ++++++++++++++++-- 2 files changed, 185 insertions(+), 13 deletions(-) diff --git a/apps/server/src/textGeneration/CodexTextGeneration.test.ts b/apps/server/src/textGeneration/CodexTextGeneration.test.ts index 0c245537108c23b2ee421b0383a57372c31e4600..bd8786b8727ce6e2e80ab523560c7774b4350ef3 100644 --- a/apps/server/src/textGeneration/CodexTextGeneration.test.ts +++ b/apps/server/src/textGeneration/CodexTextGeneration.test.ts @@ -40,6 +40,8 @@ interface FakeCodexInput { forbidArg?: string; stdinMustContain?: string; stdinMustNotContain?: string; + /** What `codex mcp list --json` answers; a number exits with that code instead. */ + mcpServers?: ReadonlyArray<{ name: string; enabled: boolean }> | number; } // The stub walks argv the way the shell script it replaced did: `--image`, @@ -56,6 +58,7 @@ function makeFakeCodexBinary(dir: string, input: FakeCodexInput) { forbidArg: input.forbidArg ?? null, stdinMustContain: input.stdinMustContain ?? null, stdinMustNotContain: input.stdinMustNotContain ?? null, + mcpServers: input.mcpServers ?? [], stderr: input.stderr ?? null, output: input.output, exitCode: input.exitCode ?? 0, @@ -69,6 +72,11 @@ function makeFakeCodexBinary(dir: string, input: FakeCodexInput) { 'import * as NodeFS from "node:fs";', `const check = ${check};`, "const args = process.argv.slice(2);", + 'if (args.includes("mcp") && args.includes("list")) {', + ' if (typeof check.mcpServers === "number") process.exit(check.mcpServers);', + " process.stdout.write(JSON.stringify(check.mcpServers));", + " process.exit(0);", + "}", 'const originalArgs = ` ${args.join(" ")} `;', "let outputPath = null;", "let seenImage = false;", @@ -266,6 +274,70 @@ it.layer(layerCodexTextGenerationTest)("CodexTextGeneration", (it) => { ), ); + it.effect("generates titles with the user's MCP servers and plugins off", () => + withFakeCodexEnv( + { + output: JSON.stringify({ title: "Remove default shapes" }), + mcpServers: [ + { name: "augmenter", enabled: true }, + { name: "my.server", enabled: true }, + { name: "already-off", enabled: false }, + ], + requireArg: + '--config features.plugins=false --config features.apps=false --config mcp_servers={"augmenter"={enabled=false},"my.server"={enabled=false}}', + }, + (textGeneration) => + Effect.gen(function* () { + const result = yield* textGeneration.generateThreadTitle({ + cwd: process.cwd(), + message: "Augmenter task 1. Use the Augmenter MCP: link_thread when you start.", + modelSelection: DEFAULT_TEST_MODEL_SELECTION, + }); + expect(result.title).toBe("Remove default shapes"); + }), + ), + ); + + it.effect("still generates when Codex cannot list its MCP servers", () => + withFakeCodexEnv( + { + output: JSON.stringify({ branch: "remove-default-shapes" }), + mcpServers: 2, + requireArg: "--config features.plugins=false --config features.apps=false --ephemeral", + forbidArg: "mcp_servers=", + }, + (textGeneration) => + Effect.gen(function* () { + const result = yield* textGeneration.generateBranchName({ + cwd: process.cwd(), + message: "Remove default shapes from characters", + modelSelection: DEFAULT_TEST_MODEL_SELECTION, + }); + expect(result.branch).toBe("remove-default-shapes"); + }), + ), + ); + + it.effect("keeps plugins and apps off over launch args that turn them on", () => + withFakeCodexEnv( + { + output: JSON.stringify({ title: "Remove default shapes" }), + launchArgs: "--config features.plugins=true --config features.apps=true", + requireArg: + "--config features.plugins=true --config features.apps=true --config features.plugins=false --config features.apps=false --ephemeral", + }, + (textGeneration) => + Effect.gen(function* () { + const result = yield* textGeneration.generateThreadTitle({ + cwd: process.cwd(), + message: "Remove default shapes from characters", + modelSelection: DEFAULT_TEST_MODEL_SELECTION, + }); + expect(result.title).toBe("Remove default shapes"); + }), + ), + ); + it.effect("passes exec-safe launch args into codex exec", () => withFakeCodexEnv( { diff --git a/apps/server/src/textGeneration/CodexTextGeneration.ts b/apps/server/src/textGeneration/CodexTextGeneration.ts index 16d8bf9a794ed47f2bb80000dee36dec323e2f36..3ff07c04705ac75b3a5c445bf45db9e2e10c972f 100644 --- a/apps/server/src/textGeneration/CodexTextGeneration.ts +++ b/apps/server/src/textGeneration/CodexTextGeneration.ts @@ -29,6 +29,30 @@ import { getCodexServiceTierOptionValue } from "../codexModelOptions.ts"; import * as HostProcess from "@t3tools/shared/HostProcess"; const CODEX_TIMEOUT_MS = 180_000; +const CODEX_MCP_LIST_TIMEOUT_MS = 10_000; +// Plugins and apps bring MCP servers of their own; the user's are listed and +// turned off by name below. These follow the user's launch args, so they win +// over a `-c features.plugins=true` there, which `--disable plugins` does not. +const CODEX_NO_PLUGIN_ARGS = [ + "--config", + "features.plugins=false", + "--config", + "features.apps=false", +] as const; +const CodexMcpServers = Schema.fromJsonString( + Schema.Array(Schema.Struct({ name: Schema.String, enabled: Schema.Boolean })), +); +const decodeCodexMcpServers = Schema.decodeEffect(CodexMcpServers); + +/** + * The `--config` value that turns off each MCP server Codex would start. + * `mcp_servers={}` merges into the user's servers rather than replacing them, + * so each is named; quoting the name keeps one with a dot a single key. + */ +const codexMcpServersOff = (names: ReadonlyArray): string | null => + names.length === 0 + ? null + : `mcp_servers={${names.map((name) => `${JSON.stringify(name)}={enabled=false}`).join(",")}}`; const encodeJsonString = Schema.encodeEffect(Schema.fromJsonString(Schema.Unknown)); /** * Build a Codex text-generation closure bound to a specific `CodexSettings` @@ -143,6 +167,67 @@ export const makeCodexTextGeneration = Effect.fn("makeCodexTextGeneration")(func return imagePaths; }); + /** + * The MCP servers Codex would start for this configuration, by name. Text + * generation goes ahead with them listed as none if Codex cannot list them. + */ + const listEnabledMcpServers = Effect.fn("listEnabledMcpServers")( + function* (input: { + readonly binary: string; + readonly cwd: string; + readonly env: NodeJS.ProcessEnv; + readonly launchArgs: ReadonlyArray; + }) { + const spawnCommand = yield* resolveSpawnCommand( + input.binary, + [...input.launchArgs, ...CODEX_NO_PLUGIN_ARGS, "mcp", "list", "--json"], + { env: input.env }, + ); + const child = yield* commandSpawner.spawn( + ChildProcess.make(spawnCommand.command, spawnCommand.args, { + env: input.env, + cwd: input.cwd, + shell: spawnCommand.shell, + }), + ); + const [stdout, , exitCode] = yield* Effect.all( + [ + child.stdout.pipe(Stream.decodeText(), Stream.mkString), + child.stderr.pipe(Stream.runDrain), + child.exitCode, + ], + { concurrency: "unbounded" }, + ); + if (exitCode !== 0) { + yield* Effect.logWarning("codex mcp list failed; text generation keeps its MCP servers", { + reason: "exit", + exitCode, + }); + return []; + } + const servers = yield* decodeCodexMcpServers(stdout).pipe( + Effect.tapError(() => + Effect.logWarning("codex mcp list failed; text generation keeps its MCP servers", { + reason: "decode", + }), + ), + ); + return servers.filter((server) => server.enabled).map((server) => server.name); + }, + Effect.scoped, + Effect.timeoutOption(CODEX_MCP_LIST_TIMEOUT_MS), + Effect.flatMap( + Option.match({ + onNone: () => + Effect.logWarning("codex mcp list failed; text generation keeps its MCP servers", { + reason: "timeout", + }).pipe(Effect.as>([])), + onSome: (names) => Effect.succeed(names), + }), + ), + Effect.orElseSucceed((): ReadonlyArray => []), + ); + const runCodexJson = Effect.fn("runCodexJson")(function* ({ operation, cwd, @@ -189,11 +274,36 @@ export const makeCodexTextGeneration = Effect.fn("makeCodexTextGeneration")(func getModelSelectionStringOptionValue(modelSelection, "reasoningEffort") ?? DEFAULT_TEXT_GENERATION_REASONING_EFFORT; const serviceTier = resolved ? undefined : getCodexServiceTierOptionValue(modelSelection); + const binary = effectiveConfig.binaryPath || "codex"; + const env = { + ...effectiveEnvironment, + ...(effectiveConfig.homePath + ? { + CODEX_HOME: expandHomePath( + effectiveConfig.homePath, + yield* HostProcess.HomeDirectory, + ), + } + : {}), + }; + const execLaunchArgs = codexExecLaunchArgs(launchArgs); + const mcpServersOff = yield* listEnabledMcpServers({ + binary, + cwd, + env, + // `codex mcp` rejects --strict-config; the overrides still apply. + launchArgs: execLaunchArgs.filter((arg) => arg !== "--strict-config"), + }).pipe(Effect.map(codexMcpServersOff)); const spawnCommand = yield* resolveSpawnCommand( - effectiveConfig.binaryPath || "codex", + binary, [ "exec", - ...codexExecLaunchArgs(launchArgs), + ...execLaunchArgs, + // Text generation needs only the prompt. A user's MCP server would + // otherwise start and could act on it, as Claude's --strict-mcp-config + // prevents for Claude. + ...CODEX_NO_PLUGIN_ARGS, + ...(mcpServersOff ? ["--config", mcpServersOff] : []), "--ephemeral", "--skip-git-repo-check", "-s", @@ -213,17 +323,7 @@ export const makeCodexTextGeneration = Effect.fn("makeCodexTextGeneration")(func { env: effectiveEnvironment }, ); const command = ChildProcess.make(spawnCommand.command, spawnCommand.args, { - env: { - ...effectiveEnvironment, - ...(effectiveConfig.homePath - ? { - CODEX_HOME: expandHomePath( - effectiveConfig.homePath, - yield* HostProcess.HomeDirectory, - ), - } - : {}), - }, + env, cwd, shell: spawnCommand.shell, stdin: { -- 2.54.0