Skip to content
Open
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
14 changes: 14 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,20 @@ Codex can keep a shell command running after a turn continues. AIR clients can s

See [AIR extensions](docs/air-extensions.md#async-tasks) for the capability, lifecycle events, and stop request.

### Client-only MCP servers

By default, a session gets the MCP servers in the user's Codex configuration plus the client's `mcpServers`. A client that needs a session to have exactly its own servers sets `strictMcpConfig` in the request's `_meta`, on `session/new`, `session/load`, `session/resume` and `session/fork`:

```json
{
"cwd": "/path/to/project",
"mcpServers": [],
"_meta": { "codex": { "strictMcpConfig": true } }
}
```

The adapter then disables every MCP server that Codex's configuration would load for `cwd`, and turns off the `apps`, `plugins` and `skill_mcp_dependency_install` features, which add MCP servers of their own. The rest of the configuration still applies. A client server whose name matches a configured one is rejected with `invalid_params`, because Codex would merge the two entries. Pass it under another name.

## License

By contributing, you agree that your contributions will be licensed under the Apache 2.0 License.
92 changes: 86 additions & 6 deletions src/CodexAcpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {isDeepStrictEqual} from "node:util";
import {logger} from "./Logger";
import {isAccountReadAuthFailureError, isAccountReadUnavailableError} from "./CodexThreadErrors";
import {listedSessionTitle, normalizeSessionTitle} from "./SessionTitle";
import {type AcpMcpServer, type WithAcpMcpServers, normalizeMcpServers, toCodexMcpServerConfig} from "./McpServerConfig";
import {type AcpMcpServer, type NormalizedMcpServer, type WithAcpMcpServers, normalizeMcpServers, toCodexMcpServerConfig} from "./McpServerConfig";
import type {
AccountUpdatedNotification,
ApprovalsReviewer,
Expand Down Expand Up @@ -694,7 +694,7 @@ export class CodexAcpClient {
await this.refreshSkills(request.cwd, additionalDirectories);

const {config, skippedMcpServers, defaultPermissionProfileId, ...permissionProfileMetadata} =
await this.createSessionConfig(request.cwd, additionalDirectories, request.mcpServers ?? []);
await this.createSessionConfig(request.cwd, additionalDirectories, request.mcpServers ?? [], readStrictMcpConfig(request._meta));
const response = await this.resumeThreadWithInstructions({
excludeTurns: true,
config,
Expand Down Expand Up @@ -732,11 +732,12 @@ export class CodexAcpClient {
): Promise<SessionMetadata> {
const instructions = await this.resolveSessionInstructions(request, previousInstructions);
const additionalDirectories = readAdditionalDirectories(request.cwd, request.additionalDirectories, request._meta);
const strictMcpConfig = readStrictMcpConfig(request._meta);
return await runForkSession(request, additionalDirectories, {
codexClient: this.codexClient,
refreshSkills: (cwd, directories) => this.refreshSkills(cwd, directories),
createSessionConfig: (cwd, directories, mcpServers) =>
this.createSessionConfig(cwd, directories, mcpServers),
this.createSessionConfig(cwd, directories, mcpServers, strictMcpConfig),
getResumeModelProvider: () => this.getResumeModelProvider(),
fetchAvailableModels: () => this.fetchAvailableModels(),
createCurrentModelId: (models, model, reasoningEffort) =>
Expand All @@ -757,7 +758,7 @@ export class CodexAcpClient {
await this.refreshSkills(request.cwd, additionalDirectories);

const {config, skippedMcpServers, defaultPermissionProfileId, ...permissionProfileMetadata} =
await this.createSessionConfig(request.cwd, additionalDirectories, request.mcpServers ?? []);
await this.createSessionConfig(request.cwd, additionalDirectories, request.mcpServers ?? [], readStrictMcpConfig(request._meta));
const response = await this.resumeThreadWithInstructions({
excludeTurns: true,
config,
Expand Down Expand Up @@ -864,7 +865,7 @@ export class CodexAcpClient {
await this.refreshSkills(request.cwd, additionalDirectories);

const {config, skippedMcpServers, defaultPermissionProfileId: _defaultPermissionProfileId, ...permissionProfileMetadata} =
await this.createSessionConfig(request.cwd, additionalDirectories, request.mcpServers ?? []);
await this.createSessionConfig(request.cwd, additionalDirectories, request.mcpServers ?? [], readStrictMcpConfig(request._meta));
const response = await this.codexClient.threadStart({
config,
modelProvider: this.getModelProvider(),
Expand Down Expand Up @@ -1139,7 +1140,8 @@ export class CodexAcpClient {
private async createSessionConfig(
projectPath: string,
additionalDirectories: string[],
mcpServers: Array<AcpMcpServer>
mcpServers: Array<AcpMcpServer>,
strictMcpConfig: boolean,
): Promise<SessionConfig> {
const sessionRoots = [projectPath, ...additionalDirectories];
const activeProvider = this.gatewayConfig
Expand Down Expand Up @@ -1172,6 +1174,14 @@ export class CodexAcpClient {
...(effectiveConfig?.config?.approval_policy != null && {approvalPolicy: effectiveConfig.config.approval_policy}),
...(effectiveConfig?.config?.approvals_reviewer != null && {approvalsReviewer: effectiveConfig.config.approvals_reviewer}),
};
if (strictMcpConfig) {
const requested = normalizeMcpServers(mcpServers).map(server => ({name: server.name, server}));
return {
config: this.createStrictMcpSessionConfig(effectiveConfig, configWithWorkspaceRoots, requested),
skippedMcpServers: [],
...permissionProfileMetadata,
};
}
if (mcpServers.length === 0) {
return {config: configWithWorkspaceRoots, skippedMcpServers: [], ...permissionProfileMetadata};
}
Expand Down Expand Up @@ -1206,6 +1216,43 @@ export class CodexAcpClient {
};
}

/**
* Session config for `_meta.codex.strictMcpConfig`: the session's MCP servers are
* exactly the client's. Every server Codex has configured is disabled by name, and
* the features that add MCP servers of their own are turned off.
*/
private createStrictMcpSessionConfig(
effectiveConfig: ConfigReadResponse,
config: JsonObject,
requestedServers: Array<{ name: string, server: NormalizedMcpServer }>,
): JsonObject {
const configuredNames = getEffectiveMcpServerNames(effectiveConfig);
// A client server cannot replace a configured one: Codex deep-merges the two
// entries, so fields of the configured server would leak into the client's.
const conflicts = requestedServers
.map(mcp => mcp.name)
.filter(name => configuredNames.has(name));
if (conflicts.length > 0) {
throw RequestError.invalidParams(
undefined,
`_meta.codex.strictMcpConfig: MCP server name(s) ${conflicts.join(", ")} are also configured in Codex; pass them under different names`,
);
}

const features = isJsonObject(config["features"]) ? config["features"] : {};
return {
...config,
features: {
...features,
...STRICT_MCP_DISABLED_FEATURES,
},
"mcp_servers": {
...Object.fromEntries([...configuredNames].map(name => [name, {enabled: false}])),
...Object.fromEntries(requestedServers.map(mcp => [mcp.name, toCodexMcpServerConfig(mcp.server)])),
},
};
}

private getConfigMcpServerNames(response: ConfigReadResponse): Set<string> {
const effectiveMcpServers = response?.config?.["mcp_servers"];
const configLayers = response?.layers ?? [];
Expand Down Expand Up @@ -1738,6 +1785,39 @@ function sameProviderConfig(left: GatewayConfig, right: GatewayConfig): boolean
return left.modelProvider === right.modelProvider && isDeepStrictEqual(left.config, right.config);
}

/**
* Features that add MCP servers of their own. `strictMcpConfig` turns them off so a
* session's servers are exactly the ones the client passed.
*/
const STRICT_MCP_DISABLED_FEATURES = {
apps: false,
plugins: false,
skill_mcp_dependency_install: false,
} as const;

/**
* Names of the MCP servers Codex will load: the effective config only. Servers in
* disabled layers (an untrusted project, for example) never load, and naming them in
* the session config would create entries with no transport.
*/
function getEffectiveMcpServerNames(response: ConfigReadResponse): Set<string> {
const effectiveMcpServers = response?.config?.["mcp_servers"];
return new Set(isJsonObject(effectiveMcpServers) ? Object.keys(effectiveMcpServers) : []);
}

/** Reads `_meta.codex.strictMcpConfig`. Absent means false; any non-boolean is rejected. */
function readStrictMcpConfig(meta?: Record<string, unknown> | null): boolean {
const codexMeta = meta?.["codex"] as JsonValue | undefined;
if (!isJsonObject(codexMeta) || !("strictMcpConfig" in codexMeta)) {
return false;
}
const value = codexMeta["strictMcpConfig"];
if (typeof value !== "boolean") {
throw RequestError.invalidParams(undefined, "_meta.codex.strictMcpConfig must be a boolean");
}
return value;
}

function readMetaAdditionalRoots(meta?: Record<string, unknown> | null): string[] | undefined {
const rawRoots = meta?.["additionalRoots"];
if (!Array.isArray(rawRoots)) {
Expand Down
69 changes: 69 additions & 0 deletions src/__tests__/CodexACPAgent/mcp-config-merge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,75 @@ url = "https://example.com/mcp"
})).resolves.toBeDefined();
});

const clientMcp: McpServerStdio = {
name: "client-mcp",
command: path.resolve("node_modules/.bin/mcp-hello-world"),
args: [],
env: [],
};

/** MCP servers the session will run: every server its thread reports, less the disabled ones. */
async function sessionMcpServerNames(meta?: Record<string, unknown>): Promise<string[]> {
const codexAcpAgent = fixture.getCodexAcpAgent();
const newSessionResponse = await codexAcpAgent.newSession({
cwd: projectPath,
mcpServers: [clientMcp],
...(meta && {_meta: meta}),
});
const status = await fixture.getCodexAcpClient().appServerClient.listMcpServerStatus({
threadId: newSessionResponse.sessionId,
});
return status.data
.filter(server => server.runtimeStatus !== "disabled")
.map(server => server.name)
.sort();
}

it('should give a strictMcpConfig session only the ACP MCP servers', async () => {
const codexAcpAgent = fixture.getCodexAcpAgent();
await codexAcpAgent.initialize({protocolVersion: 1});
fixture.getCodexAcpClient().readAuthRequirement = vi.fn().mockResolvedValue({required: false, account: null});

expect(await sessionMcpServerNames()).toEqual(["client-mcp", "shared-mcp"]);
expect(await sessionMcpServerNames({codex: {strictMcpConfig: true}})).toEqual(["client-mcp"]);
});

it('should disable a trusted project MCP server in a strictMcpConfig session', async () => {
fs.appendFileSync(
path.join(codexHome, "config.toml"),
`\n[projects.${JSON.stringify(fs.realpathSync(projectPath))}]\ntrust_level = "trusted"\n`,
"utf8",
);
const codexAcpAgent = fixture.getCodexAcpAgent();
await codexAcpAgent.initialize({protocolVersion: 1});
fixture.getCodexAcpClient().readAuthRequirement = vi.fn().mockResolvedValue({required: false, account: null});

expect(await sessionMcpServerNames()).toEqual(["client-mcp", "project-mcp", "shared-mcp"]);
expect(await sessionMcpServerNames({codex: {strictMcpConfig: true}})).toEqual(["client-mcp"]);
});

it('should reject a strictMcpConfig session whose ACP MCP server shares a configured name', async () => {
const codexAcpAgent = fixture.getCodexAcpAgent();
await codexAcpAgent.initialize({protocolVersion: 1});
fixture.getCodexAcpClient().readAuthRequirement = vi.fn().mockResolvedValue({required: false, account: null});

const conflictingMcp: McpServerStdio = {
name: "shared-mcp",
command: "./node_modules/.bin/mcp-hello-world",
args: [],
env: [],
};

await expect(codexAcpAgent.newSession({
cwd: projectPath,
mcpServers: [conflictingMcp],
_meta: {codex: {strictMcpConfig: true}},
})).rejects.toMatchObject({
code: -32602,
message: expect.stringContaining("shared-mcp"),
});
});

it('should not wait for an ACP MCP that a disabled MCP of the same name in the config replaces', async () => {
const codexAcpAgent = fixture.getCodexAcpAgent();
await codexAcpAgent.initialize({protocolVersion: 1});
Expand Down
Loading