Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions apps/server/src/textGeneration/CodexTextGeneration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
Expand All @@ -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,
Expand All @@ -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;",
Expand Down Expand Up @@ -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(
{
Expand Down
126 changes: 113 additions & 13 deletions apps/server/src/textGeneration/CodexTextGeneration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>): 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`
Expand Down Expand Up @@ -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<string>;
}) {
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<ReadonlyArray<string>>([])),
onSome: (names) => Effect.succeed(names),
}),
),
Effect.orElseSucceed((): ReadonlyArray<string> => []),
);

const runCodexJson = Effect.fn("runCodexJson")(function* <S extends Schema.Top>({
operation,
cwd,
Expand Down Expand Up @@ -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,
Comment thread
brycedbjork marked this conversation as resolved.
Comment thread
Yash-Singh1 marked this conversation as resolved.
...(mcpServersOff ? ["--config", mcpServersOff] : []),
"--ephemeral",
"--skip-git-repo-check",
"-s",
Expand All @@ -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: {
Expand Down
Loading