From 85ac14e9c263aeb3441fd6e9e8a8c87f598db4ef Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 25 Sep 2026 21:37:54 -0700 Subject: [PATCH] fix(codex): retain runtime MCP entries without losing revocation (#22426) * Retain runtime-only MCP entries Adapted from the investigation and proposal by @mmarabel. Co-authored-by: mmarabel * fix(codex): respect inline and dotted canonical MCP ownership * fix(codex): retain canonical MCP removal across upgrades * fix(types): include MCP ownership in CLI project * Keep unrelated main test formatting unchanged --------- Co-authored-by: mmarabel --- config/tsconfig.cli.json | 1 + .../codex-config-mirror-mcp-ownership.test.ts | 159 ++++++++++++++++++ src/main/codex/codex-config-mirror.test.ts | 94 +++++++++++ src/main/codex/codex-config-mirror.ts | 81 +++++++-- src/main/codex/config-settings-baseline.ts | 24 ++- src/main/codex/config-settings-promotion.ts | 27 ++- src/main/codex/config-toml-line-scan.ts | 14 +- .../codex/config-toml-mcp-servers.test.ts | 67 ++++++++ src/main/codex/config-toml-mcp-servers.ts | 42 +++++ .../config-toml-runtime-owned-sections.ts | 12 ++ 10 files changed, 496 insertions(+), 25 deletions(-) create mode 100644 src/main/codex/codex-config-mirror-mcp-ownership.test.ts create mode 100644 src/main/codex/config-toml-mcp-servers.test.ts create mode 100644 src/main/codex/config-toml-mcp-servers.ts diff --git a/config/tsconfig.cli.json b/config/tsconfig.cli.json index f9bcf82c52d..12d4d2fe785 100644 --- a/config/tsconfig.cli.json +++ b/config/tsconfig.cli.json @@ -92,6 +92,7 @@ "../src/main/codex/config-toml-hook-trust-read.ts", "../src/main/codex/config-toml-key-path.ts", "../src/main/codex/config-toml-line-scan.ts", + "../src/main/codex/config-toml-mcp-servers.ts", "../src/main/codex/config-toml-project-trust.ts", "../src/main/codex/config-toml-runtime-owned-sections.ts", "../src/main/codex/config-toml-syntax.ts", diff --git a/src/main/codex/codex-config-mirror-mcp-ownership.test.ts b/src/main/codex/codex-config-mirror-mcp-ownership.test.ts new file mode 100644 index 00000000000..85121996a06 --- /dev/null +++ b/src/main/codex/codex-config-mirror-mcp-ownership.test.ts @@ -0,0 +1,159 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { + syncSystemConfigIntoManagedCodexHome, + syncSystemConfigIntoLegacySharedCodexHome +} from './codex-config-mirror' + +let root: string +let runtimeHomePath: string +let systemHomePath: string + +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'orca-mcp-ownership-')) + runtimeHomePath = join(root, 'runtime') + systemHomePath = join(root, 'system') + mkdirSync(runtimeHomePath) + mkdirSync(systemHomePath) +}) + +afterEach(() => rmSync(root, { recursive: true, force: true })) + +describe('canonical MCP ownership during config mirroring', () => { + it('does not duplicate a server defined inline in the canonical MCP table', () => { + writeFileSync( + join(runtimeHomePath, 'config.toml'), + '[mcp_servers.shared]\ncommand = "runtime"\n' + ) + writeFileSync( + join(systemHomePath, 'config.toml'), + '[mcp_servers]\nshared = { command = "system" }\n' + ) + + syncSystemConfigIntoManagedCodexHome({ runtimeHomePath, systemHomePath }) + + const runtimeConfig = readFileSync(join(runtimeHomePath, 'config.toml'), 'utf-8') + expect(runtimeConfig).toContain('shared = { command = "system" }') + expect(runtimeConfig).not.toContain('[mcp_servers.shared]') + }) + + it('preserves deletion after a canonical inline server is rewritten by the runtime', () => { + writeFileSync( + join(runtimeHomePath, 'config.toml'), + '[mcp_servers.shared]\ncommand = "runtime"\n' + ) + writeFileSync( + join(systemHomePath, 'config.toml'), + '[mcp_servers]\nshared = { command = "system" }\n' + ) + syncSystemConfigIntoManagedCodexHome({ runtimeHomePath, systemHomePath }) + writeFileSync( + join(runtimeHomePath, 'config.toml'), + '[mcp_servers.shared]\ncommand = "system"\n[mcp_servers.runtime_only]\ncommand = "runtime"\n' + ) + writeFileSync(join(systemHomePath, 'config.toml'), 'model = "system"\n') + + syncSystemConfigIntoManagedCodexHome({ runtimeHomePath, systemHomePath }) + + const runtimeConfig = readFileSync(join(runtimeHomePath, 'config.toml'), 'utf-8') + expect(runtimeConfig).not.toContain('[mcp_servers.shared]') + expect(runtimeConfig).toContain('[mcp_servers.runtime_only]') + }) + + it('honors a closed canonical MCP root and its later removal', () => { + writeFileSync( + join(runtimeHomePath, 'config.toml'), + '[mcp_servers.runtime_only]\ncommand = "runtime"\n' + ) + writeFileSync( + join(systemHomePath, 'config.toml'), + 'mcp_servers = { shared = { enabled = false } }\n' + ) + syncSystemConfigIntoManagedCodexHome({ runtimeHomePath, systemHomePath }) + expect(readFileSync(join(runtimeHomePath, 'config.toml'), 'utf-8')).not.toContain( + '[mcp_servers.' + ) + expect( + JSON.parse( + readFileSync(join(runtimeHomePath, '.orca-config-settings-baseline.json'), 'utf-8') + ) + ).toMatchObject({ mcpServerRoot: true }) + writeFileSync(join(runtimeHomePath, 'config.toml'), '[mcp_servers.shared]\nenabled = false\n') + writeFileSync(join(systemHomePath, 'config.toml'), 'model = "system"\n') + + syncSystemConfigIntoManagedCodexHome({ runtimeHomePath, systemHomePath }) + + expect(readFileSync(join(runtimeHomePath, 'config.toml'), 'utf-8')).not.toContain( + '[mcp_servers.' + ) + }) +}) + +describe('MCP ownership migration', () => { + it.each([1, 2, 3])( + 'keeps pre-ownership baseline version %s canonical for one pass', + (version) => { + writeFileSync( + join(runtimeHomePath, 'config.toml'), + '[mcp_servers.removed]\ncommand = "old"\n' + ) + writeFileSync(join(systemHomePath, 'config.toml'), 'model = "system"\n') + writeFileSync( + join(runtimeHomePath, '.orca-config-settings-baseline.json'), + JSON.stringify({ version, settings: {} }) + ) + + syncSystemConfigIntoManagedCodexHome({ runtimeHomePath, systemHomePath }) + + expect(readFileSync(join(runtimeHomePath, 'config.toml'), 'utf-8')).not.toContain( + '[mcp_servers.' + ) + expect( + JSON.parse( + readFileSync(join(runtimeHomePath, '.orca-config-settings-baseline.json'), 'utf-8') + ) + ).toMatchObject({ mcpServers: [] }) + writeFileSync(join(runtimeHomePath, 'config.toml'), '[mcp_servers.added]\ncommand = "new"\n') + + syncSystemConfigIntoManagedCodexHome({ runtimeHomePath, systemHomePath }) + + expect(readFileSync(join(runtimeHomePath, 'config.toml'), 'utf-8')).toContain( + '[mcp_servers.added]' + ) + } + ) + + it('keeps the retained shared home one-way without an ownership baseline', () => { + writeFileSync(join(runtimeHomePath, 'config.toml'), '[mcp_servers.removed]\ncommand = "old"\n') + writeFileSync(join(systemHomePath, 'config.toml'), 'model = "system"\n') + + syncSystemConfigIntoLegacySharedCodexHome({ runtimeHomePath, systemHomePath }) + + expect(readFileSync(join(runtimeHomePath, 'config.toml'), 'utf-8')).not.toContain( + '[mcp_servers.' + ) + }) + + it('tracks commented CRLF names so their later removal remains authoritative', () => { + writeFileSync( + join(runtimeHomePath, 'config.toml'), + '[mcp_servers.shared]\ncommand = "runtime"\n' + ) + writeFileSync( + join(systemHomePath, 'config.toml'), + '[mcp_servers.shared] # see [docs]\r\ncommand = "system"\r\n' + ) + + syncSystemConfigIntoManagedCodexHome({ runtimeHomePath, systemHomePath }) + expect(readFileSync(join(runtimeHomePath, 'config.toml'), 'utf-8')).not.toContain('"runtime"') + writeFileSync(join(systemHomePath, 'config.toml'), 'model = "system"\n') + + syncSystemConfigIntoManagedCodexHome({ runtimeHomePath, systemHomePath }) + + expect(readFileSync(join(runtimeHomePath, 'config.toml'), 'utf-8')).not.toContain( + '[mcp_servers.shared]' + ) + }) +}) diff --git a/src/main/codex/codex-config-mirror.test.ts b/src/main/codex/codex-config-mirror.test.ts index 2469a2be724..74db1cd9c05 100644 --- a/src/main/codex/codex-config-mirror.test.ts +++ b/src/main/codex/codex-config-mirror.test.ts @@ -52,6 +52,10 @@ function getRuntimeConfigPath(): string { return join(userDataDir, 'codex-runtime-home', 'home', 'config.toml') } +function getRuntimeBaselinePath(): string { + return join(userDataDir, 'codex-runtime-home', 'home', '.orca-config-settings-baseline.json') +} + beforeEach(() => { fakeHomeDir = mkdtempSync(join(tmpdir(), 'orca-codex-config-home-')) userDataDir = mkdtempSync(join(tmpdir(), 'orca-codex-config-user-data-')) @@ -364,6 +368,96 @@ describe('syncSystemConfigIntoManagedCodexHome', () => { expect(runtimeConfig.match(/\[projects\."\/repo"\]/g)?.length).toBe(1) }) + it('preserves runtime-only MCP servers and nested descendants with system precedence', () => { + mkdirSync(join(userDataDir, 'codex-runtime-home', 'home'), { recursive: true }) + writeFileSync( + getRuntimeConfigPath(), + [ + '[mcp_servers.runtime_only]', + 'command = "runtime-command"', + '', + '[mcp_servers.runtime_only.env]', + 'MODE = "runtime"', + '', + '[mcp_servers.shared]', + 'command = "runtime-shared"', + '' + ].join('\n'), + 'utf-8' + ) + writeFileSync( + getSystemConfigPath(), + [ + '[mcp_servers."shared"]', + 'command = "system-shared"', + '', + '[mcp_servers.system_only]', + 'command = "system-only"', + '' + ].join('\n'), + 'utf-8' + ) + + syncSystemConfigIntoManagedCodexHome() + + const runtimeConfig = readFileSync(getRuntimeConfigPath(), 'utf-8') + expect(runtimeConfig).toContain('[mcp_servers.runtime_only]') + expect(runtimeConfig).toContain('[mcp_servers.runtime_only.env]') + expect(runtimeConfig).toContain('MODE = "runtime"') + expect(runtimeConfig).toContain('command = "system-shared"') + expect(runtimeConfig).not.toContain('runtime-shared') + expect(runtimeConfig.match(/\[mcp_servers\.(?:shared|"shared")\]/g)).toHaveLength(1) + expect(runtimeConfig).toContain('[mcp_servers.system_only]') + expect(JSON.parse(readFileSync(getRuntimeBaselinePath(), 'utf-8'))).toMatchObject({ + mcpServers: ['shared', 'system_only'] + }) + }) + + it('revokes a previously mirrored MCP server when the system source deletes it', () => { + mkdirSync(join(userDataDir, 'codex-runtime-home', 'home'), { recursive: true }) + writeFileSync(getRuntimeConfigPath(), '[mcp_servers.revoked]\ncommand = "run"\n', 'utf-8') + writeFileSync(getSystemConfigPath(), '[mcp_servers.revoked]\ncommand = "run"\n', 'utf-8') + + syncSystemConfigIntoManagedCodexHome() + writeFileSync(getSystemConfigPath(), 'model = "system"\n', 'utf-8') + syncSystemConfigIntoManagedCodexHome() + + expect(readFileSync(getRuntimeConfigPath(), 'utf-8')).not.toContain('[mcp_servers.revoked]') + }) + + it('keeps runtime-only MCP additions after a source refresh and remains byte-idempotent', () => { + mkdirSync(join(userDataDir, 'codex-runtime-home', 'home'), { recursive: true }) + writeFileSync(getRuntimeConfigPath(), '[mcp_servers.runtime_only]\ncommand = "run"\n', 'utf-8') + writeFileSync(getSystemConfigPath(), 'model = "system"\n', 'utf-8') + + syncSystemConfigIntoManagedCodexHome() + const first = readFileSync(getRuntimeConfigPath(), 'utf-8') + syncSystemConfigIntoManagedCodexHome() + + expect(readFileSync(getRuntimeConfigPath(), 'utf-8')).toBe(first) + expect(readFileSync(getRuntimeConfigPath(), 'utf-8')).toContain('[mcp_servers.runtime_only]') + }) + + it('keeps an explicit system MCP disable canonical', () => { + mkdirSync(join(userDataDir, 'codex-runtime-home', 'home'), { recursive: true }) + writeFileSync( + getRuntimeConfigPath(), + '[mcp_servers.blocked]\ncommand = "runtime"\nenabled = true\n', + 'utf-8' + ) + writeFileSync( + getSystemConfigPath(), + '[mcp_servers."blocked"]\ncommand = "system"\nenabled = false\n', + 'utf-8' + ) + + syncSystemConfigIntoManagedCodexHome() + + const runtimeConfig = readFileSync(getRuntimeConfigPath(), 'utf-8') + expect(runtimeConfig).toContain('enabled = false') + expect(runtimeConfig).not.toContain('enabled = true') + }) + it('deduplicates basic and literal project headers by decoded Windows path', () => { mkdirSync(join(userDataDir, 'codex-runtime-home', 'home'), { recursive: true }) writeFileSync( diff --git a/src/main/codex/codex-config-mirror.ts b/src/main/codex/codex-config-mirror.ts index af6f010da18..275e350328a 100644 --- a/src/main/codex/codex-config-mirror.ts +++ b/src/main/codex/codex-config-mirror.ts @@ -1,3 +1,4 @@ +import { readMcpServerTomlOwnership } from './config-toml-mcp-servers' import { dirname, join } from 'node:path' import { observeAgentStateFile } from './codex-path-observation' import { @@ -21,6 +22,7 @@ import { preserveRuntimeConflictValues } from './codex-config-settings-preservat import { applyCodexDaemonSocketGuard } from './codex-daemon-socket-path-guard' import { deduplicateProjectTomlSections, + getMcpServerTomlSectionName, getProjectTrustLevel, getRevocationTomlSectionHeaderKey, getTomlSectionHeaderKey, @@ -109,7 +111,9 @@ function mirrorSystemConfigIntoManagedCodexHome(homes: CodexSettingsPromotionHom ), // Why: this pass made the runtime's marketplace and plugin tables canonical, // so a later source config that lacks one is a removal, not an addition. - mirroredRegistrations: true + mirroredRegistrations: true, + mirroredMcpServers: mirrorResult.mirroredMcpServerNames, + mirroredMcpServerRoot: mirrorResult.mirroredMcpServerRoot }) return true } @@ -167,11 +171,14 @@ export function syncSystemConfigIntoLegacySharedCodexHome( let mirroredRuntimeConfig = runtimeConfigBeforeMirror ?? '' if (rawSystemConfig.trim() !== '') { const sourceConfigDir = resolveCodexConfigMirrorSourceDirectory(homes.systemHomePath) + // The retired home has no ownership baseline; its entire MCP root stays canonical. mirroredRuntimeConfig = runtimeConfigBeforeMirror !== null ? mergeSystemCodexConfigIntoRuntime( runtimeConfigBeforeMirror, - prepareSystemConfigForRuntimeMirror(rawSystemConfig, sourceConfigDir) + prepareSystemConfigForRuntimeMirror(rawSystemConfig, sourceConfigDir), + new Set(), + true ) : prepareSystemConfigForFreshRuntimeMirror(rawSystemConfig, sourceConfigDir) } @@ -191,7 +198,12 @@ export function syncSystemConfigIntoLegacySharedCodexHome( type CodexConfigMirrorResult = | { status: 'skipped-missing-source' } | { status: 'refused-indeterminate'; error: unknown } - | { status: 'mirrored'; preservedConflictKeys: ReadonlySet } + | { + status: 'mirrored' + preservedConflictKeys: ReadonlySet + mirroredMcpServerNames: ReadonlySet + mirroredMcpServerRoot: boolean + } function syncSystemConfigIntoManagedCodexHomeUnsafe( { runtimeHomePath, systemHomePath, systemConfigDir }: CodexSettingsPromotionHomes, @@ -225,34 +237,55 @@ function syncSystemConfigIntoManagedCodexHomeUnsafe( ) return runtimeConfigExists ? { status: 'skipped-missing-source' } - : { status: 'mirrored', preservedConflictKeys: new Set() } + : { + status: 'mirrored', + preservedConflictKeys: new Set(), + mirroredMcpServerNames: new Set(), + mirroredMcpServerRoot: false + } } const sourceConfigDir = resolveCodexConfigMirrorSourceDirectory(systemHomePath, systemConfigDir) if (!runtimeConfigExists) { - writeFileAtomically( - runtimeConfigPath, - applyCodexDaemonSocketGuard( - prepareSystemConfigForFreshRuntimeMirror(rawSystemConfig, sourceConfigDir), - runtimeHomePath - ) + const freshRuntimeConfig = applyCodexDaemonSocketGuard( + prepareSystemConfigForFreshRuntimeMirror(rawSystemConfig, sourceConfigDir), + runtimeHomePath ) - return { status: 'mirrored', preservedConflictKeys: new Set() } + const ownership = readMcpServerTomlOwnership(freshRuntimeConfig) + writeFileAtomically(runtimeConfigPath, freshRuntimeConfig) + return { + status: 'mirrored', + preservedConflictKeys: new Set(), + mirroredMcpServerNames: ownership.names, + mirroredMcpServerRoot: ownership.ownsRoot + } } const systemConfig = prepareSystemConfigForRuntimeMirror(rawSystemConfig, sourceConfigDir) + const { names: mirroredMcpServerNames, ownsRoot: mirroredMcpServerRoot } = + readMcpServerTomlOwnership(systemConfig) // Why: reuse the bytes already observed above rather than re-reading. A second // read could succeed where the first failed and re-open the gap this closes. const runtimeConfig = runtimeConfigObservation.value const preserved = preserveRuntimeConflictValues( - mergeSystemCodexConfigIntoRuntime(runtimeConfig, systemConfig), + mergeSystemCodexConfigIntoRuntime( + runtimeConfig, + systemConfig, + promotionPlan.mirroredMcpServers, + promotionPlan.mirroredMcpServerRoot + ), promotionPlan.runtimeValuesToPreserve ) const nextRuntimeConfig = applyCodexDaemonSocketGuard(preserved.content, runtimeHomePath) if (nextRuntimeConfig !== runtimeConfig) { writeFileAtomically(runtimeConfigPath, nextRuntimeConfig) } - return { status: 'mirrored', preservedConflictKeys: preserved.keys } + return { + status: 'mirrored', + preservedConflictKeys: preserved.keys, + mirroredMcpServerNames, + mirroredMcpServerRoot + } } export function resolveCodexConfigMirrorSourceDirectory( @@ -284,7 +317,12 @@ export function prepareSystemConfigForFreshRuntimeMirror( return stripRuntimeOwnedTomlSections(prepareSystemConfigForRuntimeMirror(config, systemConfigDir)) } -function mergeSystemCodexConfigIntoRuntime(runtimeConfig: string, systemConfig: string): string { +function mergeSystemCodexConfigIntoRuntime( + runtimeConfig: string, + systemConfig: string, + mirroredMcpServerNames: ReadonlySet = new Set(), + mirroredMcpServerRoot = false +): string { const runtimeSections = deduplicateProjectTomlSections(getTomlSections(runtimeConfig)) const runtimeProjectHeaders = new Set( runtimeSections @@ -307,6 +345,7 @@ function mergeSystemCodexConfigIntoRuntime(runtimeConfig: string, systemConfig: .filter((section) => getProjectTrustLevel(section.block) === 'trusted') .map((section) => getTomlSectionHeaderKey(section.header)) ) + const systemMcpServers = readMcpServerTomlOwnership(systemConfig) // Why: ordinary Codex settings should mirror ~/.codex exactly; runtime hook // trust and project trust are written under Orca's managed CODEX_HOME and // must survive the copy unless the user explicitly revoked project trust in @@ -314,7 +353,19 @@ function mergeSystemCodexConfigIntoRuntime(runtimeConfig: string, systemConfig: return joinTomlBlocks([ stripRuntimeOwnedTomlSections(systemConfig, runtimeProjectHeaders), ...runtimeSections - .filter((section) => isRuntimePreservedTomlSection(section.header)) + .filter((section) => { + if (isRuntimePreservedTomlSection(section.header)) { + return true + } + const mcpServerName = getMcpServerTomlSectionName(section.header) + return ( + mcpServerName !== null && + !systemMcpServers.ownsRoot && + !mirroredMcpServerRoot && + !systemMcpServers.names.has(mcpServerName) && + !mirroredMcpServerNames.has(mcpServerName) + ) + }) .filter( (section) => !isRuntimeProjectTomlSection(section.header) || diff --git a/src/main/codex/config-settings-baseline.ts b/src/main/codex/config-settings-baseline.ts index 5cc26e361b9..d6c44909ef8 100644 --- a/src/main/codex/config-settings-baseline.ts +++ b/src/main/codex/config-settings-baseline.ts @@ -21,6 +21,9 @@ export type CodexSettingsBaseline = { * table reads as an addition rather than as a canonical removal. */ registrations: ReadonlyMap> + /** MCP server names the last mirror copied from the canonical source. */ + mcpServers: ReadonlySet + mcpServerRoot: boolean } type StoredSettingsBaseline = { @@ -28,6 +31,8 @@ type StoredSettingsBaseline = { settings: Record conflicts?: Record registrations?: Record> + mcpServers?: string[] + mcpServerRoot?: boolean } /** @@ -81,7 +86,14 @@ function readParsedCodexSettingsBaseline( conflicts.set(key, conflict) } } - return { settings, conflicts, registrations: readStoredRegistrations(parsed.registrations) } + return { + settings, + conflicts, + registrations: readStoredRegistrations(parsed.registrations), + mcpServers: readStoredMcpServers(parsed.mcpServers), + // Older mirrors owned the whole MCP root; retain that removal policy for one pass. + mcpServerRoot: parsed.mcpServers === undefined || parsed.mcpServerRoot === true + } } catch (error) { // Why: invalid baseline state is still `null` — resetting it is the intent, // and only a read that FAILED must be preserved. @@ -89,6 +101,10 @@ function readParsedCodexSettingsBaseline( } } +function readStoredMcpServers(stored: string[] | undefined): ReadonlySet { + return new Set((stored ?? []).filter((name): name is string => typeof name === 'string')) +} + function readStoredRegistrations( stored: Record> | undefined ): Map> { @@ -124,7 +140,8 @@ export function writeCodexSettingsBaseline( ): void { const file: StoredSettingsBaseline = { version: 3, - settings: Object.fromEntries(baseline.settings) + settings: Object.fromEntries(baseline.settings), + mcpServers: [...baseline.mcpServers] } if (baseline.conflicts.size > 0) { file.conflicts = Object.fromEntries(baseline.conflicts) @@ -134,6 +151,9 @@ export function writeCodexSettingsBaseline( [...baseline.registrations].map(([key, fields]) => [key, Object.fromEntries(fields)]) ) } + if (baseline.mcpServerRoot) { + file.mcpServerRoot = true + } const baselinePath = getCodexSettingsBaselinePath(runtimeHomePath) const serialized = `${JSON.stringify(file, null, 2)}\n` let existing: string | null = null diff --git a/src/main/codex/config-settings-promotion.ts b/src/main/codex/config-settings-promotion.ts index 52f4d6eefcd..56e499a6c18 100644 --- a/src/main/codex/config-settings-promotion.ts +++ b/src/main/codex/config-settings-promotion.ts @@ -37,6 +37,9 @@ export type CodexSettingsBaselineSnapshotOptions = { * mirrored would read a source config that never had them as a removal. */ mirroredRegistrations?: boolean + /** Names copied from the canonical source in this mirror pass. */ + mirroredMcpServers?: ReadonlySet + mirroredMcpServerRoot?: boolean } /** @@ -71,7 +74,9 @@ export function snapshotCodexRuntimeSettingsBaseline( conflicts, registrations: options.mirroredRegistrations ? readCodexRegistrationBaseline(runtimeConfig) - : new Map() + : new Map(), + mcpServers: options.mirroredMcpServers ?? new Set(), + mcpServerRoot: options.mirroredMcpServerRoot ?? false }) } catch (error) { console.warn('[codex-settings-promotion] failed to snapshot settings baseline', error) @@ -88,6 +93,9 @@ export type CodexSettingsPromotionHomes = { export type CodexSettingsPromotionPlan = { conflicts: ReadonlyMap runtimeValuesToPreserve: ReadonlyMap + /** MCP names the previous mirror copied from the canonical source. */ + mirroredMcpServers: ReadonlySet + mirroredMcpServerRoot: boolean } function getHostPromotionHomes(): CodexSettingsPromotionHomes { @@ -142,6 +150,8 @@ function promoteCodexRuntimeSettingsToSystemUnsafe( throw new Error('Codex settings baseline could not be read') } const baseline = baselineObservation.kind === 'present' ? baselineObservation.baseline : null + const mirroredMcpServers = baseline?.mcpServers ?? new Set() + const mirroredMcpServerRoot = baseline?.mcpServerRoot ?? false const updates = new Map() const conflicts = new Map() const runtimeValuesToPreserve = new Map() @@ -160,7 +170,7 @@ function promoteCodexRuntimeSettingsToSystemUnsafe( // canonical is an addition, never a removal it must honor. Scalars still need a // real baseline, so they stay gated above. if (updates.size === 0 && !hasCodexRegistrationEntries(runtimeTomlObservation.value)) { - return { conflicts, runtimeValuesToPreserve } + return { conflicts, runtimeValuesToPreserve, mirroredMcpServers, mirroredMcpServerRoot } } // Why: a fresh host has no ~/.codex; create it owner-only (holds auth.json) or the atomic write ENOENTs and the mirror wipes it. mkdirSync(systemHomePath, { recursive: true, mode: 0o700 }) @@ -202,17 +212,17 @@ function promoteCodexRuntimeSettingsToSystemUnsafe( ) ) if (nextContent === systemContent) { - return { conflicts, runtimeValuesToPreserve } + return { conflicts, runtimeValuesToPreserve, mirroredMcpServers, mirroredMcpServerRoot } } if (targetExists && parseWslUncPath(writeTarget.path)) { // Why: \\wsl$ 9P symlink metadata is unreliable; write through the existing file to preserve the WSL-side inode. writeFileSync(writeTarget.path, nextContent, 'utf-8') - return { conflicts, runtimeValuesToPreserve } + return { conflicts, runtimeValuesToPreserve, mirroredMcpServers, mirroredMcpServerRoot } } writeFileAtomically(writeTarget.path, nextContent, { mode: writeTarget.mode }) - return { conflicts, runtimeValuesToPreserve } + return { conflicts, runtimeValuesToPreserve, mirroredMcpServers, mirroredMcpServerRoot } } type PromotionCollectionContext = { @@ -264,7 +274,12 @@ function getComparableRaw(value: TopLevelSettingValue | undefined): string | nul } function emptyPromotionPlan(): CodexSettingsPromotionPlan { - return { conflicts: new Map(), runtimeValuesToPreserve: new Map() } + return { + conflicts: new Map(), + runtimeValuesToPreserve: new Map(), + mirroredMcpServers: new Set(), + mirroredMcpServerRoot: false + } } // Why: follow an existing dotfile-manager symlink and carry its mode forward so an atomic write can't widen a 0600 config. diff --git a/src/main/codex/config-toml-line-scan.ts b/src/main/codex/config-toml-line-scan.ts index 98d096167fe..0e983aac2dd 100644 --- a/src/main/codex/config-toml-line-scan.ts +++ b/src/main/codex/config-toml-line-scan.ts @@ -95,8 +95,18 @@ export function updateTomlLineScanState(state: TomlLineScanState, line: string): } export function getTomlTableHeader(line: string): string | null { - const match = /^(\s*\[\[?.+\]\]?\s*)(?:#.*)?$/.exec(line) - return match?.[1] ?? null + let index = 0 + while (index < line.length && line[index] !== '#') { + if (line[index] === '"') { + index = skipTomlBasicString(line, index + 1) + } else if (line[index] === "'") { + index = skipTomlLiteralString(line, index + 1) + } else { + index++ + } + } + const header = line.slice(0, index).trimEnd() + return /^\s*\[\[?.+\]\]?$/.test(header) ? header : null } export function parseTomlSingleLineStringValue( diff --git a/src/main/codex/config-toml-mcp-servers.test.ts b/src/main/codex/config-toml-mcp-servers.test.ts new file mode 100644 index 00000000000..e1d57770074 --- /dev/null +++ b/src/main/codex/config-toml-mcp-servers.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, it } from 'vitest' +import { readMcpServerTomlOwnership } from './config-toml-mcp-servers' +import { getTomlTableHeader } from './config-toml-line-scan' + +describe('MCP server TOML ownership', () => { + it.each([ + '[mcp_servers."server.with.dot"]\ncommand = "agent"', + '[mcp_servers]\n"server.with.dot" = { command = "agent" }', + 'mcp_servers."server.with.dot".command = "agent"', + '[mcp_servers."server.with.dot".env]\nMODE = "fixture"', + '[mcp_servers."server.with.dot"] # see [docs]\ncommand = "agent"', + '[mcp_servers."server.with.dot"] # server\r\ncommand = "agent"\r\n', + '[[mcp_servers."server.with.dot"]] # array\r\ncommand = "agent"' + ])('recognizes the decoded server name in %s', (config) => { + expect(readMcpServerTomlOwnership(config)).toEqual({ + names: new Set(['server.with.dot']), + ownsRoot: false + }) + }) + + it('treats a root assignment as ownership of the whole closed table', () => { + expect(readMcpServerTomlOwnership('"mcp_servers" = { shared = { enabled = false } }')).toEqual({ + names: new Set(), + ownsRoot: true + }) + }) + + it.each([ + '[profile] # comment\r\nmcp_servers = { foo = {} }\r\n', + '[profile] # see [docs]\nmcp_servers = { foo = {} }\n', + '[not valid]\nmcp_servers = { foo = {} }\n' + ])('does not treat a nested assignment as a canonical root in %s', (config) => { + expect(readMcpServerTomlOwnership(config)).toEqual({ names: new Set(), ownsRoot: false }) + }) + + it('ignores apparent keys in strings, arrays and unrelated tables', () => { + const config = [ + 'description = """', + '[mcp_servers.fake]', + 'mcp_servers = {}', + '"""', + 'args = [', + '"mcp_servers.quoted = {}",', + ']', + '[profile]', + 'mcp_servers = {}', + '[mcp_servers.real]', + 'command = "agent"' + ].join('\n') + expect(readMcpServerTomlOwnership(config)).toEqual({ + names: new Set(['real']), + ownsRoot: false + }) + }) +}) + +describe('commented TOML headers', () => { + it.each<[string, string | null]>([ + ['[mcp_servers."name#with]bracket"] # see [docs]\r', '[mcp_servers."name#with]bracket"]'], + ['[[mcp_servers.name]] # comment\r', '[[mcp_servers.name]]'], + ["[mcp_servers.'literal#name'] # comment", "[mcp_servers.'literal#name']"], + ['# [mcp_servers.fake]', null], + ['command = "[mcp_servers.fake]"', null] + ])('recognizes the structural header in %s', (line, header) => { + expect(getTomlTableHeader(line)).toBe(header) + }) +}) diff --git a/src/main/codex/config-toml-mcp-servers.ts b/src/main/codex/config-toml-mcp-servers.ts new file mode 100644 index 00000000000..12a492a7c88 --- /dev/null +++ b/src/main/codex/config-toml-mcp-servers.ts @@ -0,0 +1,42 @@ +import { parseTomlKeyPath, parseTomlTableHeaderPath } from './config-toml-key-path' +import { + createTomlLineScanState, + getTomlTableHeader, + isTomlStructuralLine, + updateTomlLineScanState +} from './config-toml-line-scan' + +/** Canonical inline root assignments own the whole table, which TOML forbids extending. */ +export function readMcpServerTomlOwnership(config: string): { + names: ReadonlySet + ownsRoot: boolean +} { + const names = new Set() + let ownsRoot = false + let tablePath: string[] | null = [] + let state = createTomlLineScanState() + for (const line of config.split('\n')) { + if (isTomlStructuralLine(state)) { + const header = getTomlTableHeader(line) + if (header) { + tablePath = parseTomlTableHeaderPath(header)?.segments ?? null + if (tablePath?.[0] === 'mcp_servers' && tablePath[1] !== undefined) { + names.add(tablePath[1]) + } + } else { + const key = parseTomlKeyPath(line) + const path = + tablePath && key && line[key.end] === '=' ? [...tablePath, ...key.segments] : [] + if (path[0] === 'mcp_servers') { + if (path[1] === undefined) { + ownsRoot = true + } else { + names.add(path[1]) + } + } + } + } + state = updateTomlLineScanState(state, line) + } + return { names, ownsRoot } +} diff --git a/src/main/codex/config-toml-runtime-owned-sections.ts b/src/main/codex/config-toml-runtime-owned-sections.ts index 0ff9ab19f6e..fee0cfdf643 100644 --- a/src/main/codex/config-toml-runtime-owned-sections.ts +++ b/src/main/codex/config-toml-runtime-owned-sections.ts @@ -5,6 +5,7 @@ import { isTomlStructuralLine, updateTomlLineScanState } from './config-toml-line-scan' +import { parseTomlTableHeaderPath } from './config-toml-key-path' import { normalizeCodexProjectPathForLookup, normalizeCodexProjectPathForRevocationLookup, @@ -91,6 +92,17 @@ export function isRuntimeProjectTomlSection(header: string): boolean { return parseCodexProjectHeaderPath(header) !== null } +const CODEX_MCP_SERVER_TABLE_ROOT = 'mcp_servers' + +/** Returns the decoded MCP server name for an owner table or nested descendant. */ +export function getMcpServerTomlSectionName(header: string): string | null { + const table = parseTomlTableHeaderPath(header) + if (!table || table.isArray || table.segments[0] !== CODEX_MCP_SERVER_TABLE_ROOT) { + return null + } + return table.segments[1] ?? null +} + export function getTomlSectionHeaderKey(header: string): string { const projectPath = parseCodexProjectHeaderPath(header) return projectPath === null