mirror of
https://github.com/stablyai/orca.git
synced 2026-09-28 00:02:41 +00:00
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 <mmarabel@users.noreply.github.com> * 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 <mmarabel@users.noreply.github.com>
This commit is contained in:
@@ -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",
|
||||
|
||||
@@ -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]'
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -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(
|
||||
|
||||
@@ -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<string> }
|
||||
| {
|
||||
status: 'mirrored'
|
||||
preservedConflictKeys: ReadonlySet<string>
|
||||
mirroredMcpServerNames: ReadonlySet<string>
|
||||
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<string> = 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) ||
|
||||
|
||||
@@ -21,6 +21,9 @@ export type CodexSettingsBaseline = {
|
||||
* table reads as an addition rather than as a canonical removal.
|
||||
*/
|
||||
registrations: ReadonlyMap<string, ReadonlyMap<string, string>>
|
||||
/** MCP server names the last mirror copied from the canonical source. */
|
||||
mcpServers: ReadonlySet<string>
|
||||
mcpServerRoot: boolean
|
||||
}
|
||||
|
||||
type StoredSettingsBaseline = {
|
||||
@@ -28,6 +31,8 @@ type StoredSettingsBaseline = {
|
||||
settings: Record<string, string | null>
|
||||
conflicts?: Record<string, CodexSettingsConflict>
|
||||
registrations?: Record<string, Record<string, string>>
|
||||
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<string> {
|
||||
return new Set((stored ?? []).filter((name): name is string => typeof name === 'string'))
|
||||
}
|
||||
|
||||
function readStoredRegistrations(
|
||||
stored: Record<string, Record<string, string>> | undefined
|
||||
): Map<string, ReadonlyMap<string, string>> {
|
||||
@@ -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
|
||||
|
||||
@@ -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<string>
|
||||
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<string, CodexSettingsConflict>
|
||||
runtimeValuesToPreserve: ReadonlyMap<string, string | null>
|
||||
/** MCP names the previous mirror copied from the canonical source. */
|
||||
mirroredMcpServers: ReadonlySet<string>
|
||||
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<string>()
|
||||
const mirroredMcpServerRoot = baseline?.mcpServerRoot ?? false
|
||||
const updates = new Map<string, string>()
|
||||
const conflicts = new Map<string, CodexSettingsConflict>()
|
||||
const runtimeValuesToPreserve = new Map<string, string | null>()
|
||||
@@ -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.
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
@@ -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<string>
|
||||
ownsRoot: boolean
|
||||
} {
|
||||
const names = new Set<string>()
|
||||
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 }
|
||||
}
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user