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
9 changes: 6 additions & 3 deletions packages/agent/src/adapters/codex/codex-agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,15 @@ export class CodexAcpAgent extends BaseAcpAgent {
super(client);
this.logger = new Logger({ debug: true, prefix: "[CodexAcpAgent]" });

// Load user codex settings before spawning so spawnCodexProcess can
// filter out any [mcp_servers.*] entries from ~/.codex/config.toml.
const cwd = options.codexProcessOptions.cwd ?? process.cwd();
const settingsManager = new CodexSettingsManager(cwd);

// Spawn the codex-acp subprocess
this.codexProcess = spawnCodexProcess({
...options.codexProcessOptions,
settings: settingsManager.getSettings(),
logger: this.logger,
processCallbacks: options.processCallbacks,
});
Expand All @@ -120,9 +126,6 @@ export class CodexAcpAgent extends BaseAcpAgent {
const codexWritable = nodeWritableToWebWritable(this.codexProcess.stdin);
const codexStream = ndJsonStream(codexWritable, codexReadable);

// Set up session with CodexSettingsManager
const cwd = options.codexProcessOptions.cwd ?? process.cwd();
const settingsManager = new CodexSettingsManager(cwd);
const abortController = new AbortController();
this.session = {
abortController,
Expand Down
36 changes: 18 additions & 18 deletions packages/agent/src/adapters/codex/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ export interface CodexSettings {
personality?: string;
modelReasoningEffort?: string;
trustLevel?: string;
// Names of every `[mcp_servers.<name>]` section declared in the user's config.toml
mcpServerNames: string[];
}

/**
Expand All @@ -24,32 +26,29 @@ export interface CodexSettings {
*/
export class CodexSettingsManager {
private cwd: string;
private settings: CodexSettings = {};
private initialized = false;
private settings: CodexSettings = { mcpServerNames: [] };

constructor(cwd: string) {
this.cwd = cwd;
this.loadSettings();
}

async initialize(): Promise<void> {
if (this.initialized) {
return;
}
await this.loadSettings();
this.initialized = true;
// No-op: settings are loaded in the constructor. Kept async to
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this, but I couldn't find an elegant solution without jacking up the BaseSettingsManager interface (and subsequently, Claude adapter)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fine for now

// satisfy the BaseSettingsManager interface.
}

private getConfigPath(): string {
return path.join(os.homedir(), ".codex", "config.toml");
}

private async loadSettings(): Promise<void> {
private loadSettings(): void {
const configPath = this.getConfigPath();
try {
const content = await fs.promises.readFile(configPath, "utf-8");
const content = fs.readFileSync(configPath, "utf-8");
this.settings = parseCodexToml(content, this.cwd);
} catch {
this.settings = {};
this.settings = { mcpServerNames: [] };
}
}

Expand All @@ -62,17 +61,13 @@ export class CodexSettingsManager {
}

async setCwd(cwd: string): Promise<void> {
if (this.cwd === cwd) {
return;
}
this.dispose();
if (this.cwd === cwd) return;
this.cwd = cwd;
this.initialized = false;
await this.initialize();
this.loadSettings();
}

dispose(): void {
this.initialized = false;
// No-op: no resources to release. Kept to satisfy the BaseSettingsManager interface.
}
}

Expand All @@ -82,7 +77,8 @@ export class CodexSettingsManager {
* Does NOT handle full TOML spec — only what codex config uses.
*/
function parseCodexToml(content: string, cwd: string): CodexSettings {
const settings: CodexSettings = {};
const settings: CodexSettings = { mcpServerNames: [] };
const mcpServerNames = new Set<string>();
let currentSection = "";

for (const line of content.split("\n")) {
Expand All @@ -93,6 +89,9 @@ function parseCodexToml(content: string, cwd: string): CodexSettings {
const sectionMatch = trimmed.match(/^\[(.+)\]$/);
if (sectionMatch) {
currentSection = sectionMatch[1] ?? "";
if (currentSection.startsWith("mcp_servers.")) {
mcpServerNames.add(currentSection.slice("mcp_servers.".length));
}
continue;
}

Expand Down Expand Up @@ -123,5 +122,6 @@ function parseCodexToml(content: string, cwd: string): CodexSettings {
}
}

settings.mcpServerNames = Array.from(mcpServerNames);
return settings;
}
9 changes: 9 additions & 0 deletions packages/agent/src/adapters/codex/spawn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { delimiter, dirname } from "node:path";
import type { Readable, Writable } from "node:stream";
import type { ProcessSpawnedCallback } from "../../types";
import { Logger } from "../../utils/logger";
import type { CodexSettings } from "./settings";

export interface CodexProcessOptions {
cwd?: string;
Expand All @@ -14,6 +15,7 @@ export interface CodexProcessOptions {
binaryPath?: string;
logger?: Logger;
processCallbacks?: ProcessSpawnedCallback;
settings?: CodexSettings;
}

export interface CodexProcess {
Expand All @@ -28,6 +30,13 @@ function buildConfigArgs(options: CodexProcessOptions): string[] {

args.push("-c", `features.remote_models=false`);

// Disable the user's local MCPs one-by-one so Codex only uses the MCPs we
// provide via ACP. We can't use `-c mcp_servers={}` because that makes Codex
// ignore MCPs entirely, including the ones we inject later.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm - im not sure we want to remove all locals right? only if they conflict we should favor ours

for (const name of options.settings?.mcpServerNames ?? []) {
args.push("-c", `mcp_servers.${name}.enabled=false`);
}

if (options.apiBaseUrl) {
args.push("-c", `model_provider="posthog"`);
args.push("-c", `model_providers.posthog.name="PostHog Gateway"`);
Expand Down
Loading