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
78 changes: 78 additions & 0 deletions .zcf/plan/current/fix-issue-259-mcp-config-corruption.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
# Fix Issue #259: MCP Configuration Corruption

**Created**: 2026-01-09 19:02:26
**Issue**: https://github.com/UfoMiao/zcf/issues/259
**Branch**: refactor/taplo-toml

## Problem Summary

When configuring Codex API, ZCF inadvertently modifies MCP server configurations by adding `command` and `args` fields to sections that should only contain `url` field (SSE protocol).

**Original Config**:
```toml
[mcp_servers.mcpHub]
url = "http:/xxxx:3010/mcp/codex"
```

**After ZCF Modification** (broken):
```toml
[mcp_servers.mcpHub]
command = "mcpHub"
args = []
url = "http:/xxxx:3010/mcp/codex"
```

## Root Cause

Current code uses `writeCodexConfig` which re-renders the entire TOML file, causing:
1. API modifications to affect MCP configurations
2. MCP configurations to be "polluted" with stdio protocol fields

## Solution

Use `@rainbowatcher/toml-edit-js` for targeted modifications:
- API changes only modify API-related fields
- MCP changes only modify MCP-related fields

## Implementation Steps

### Step 1: Create codex-toml-updater.ts

New utility functions for targeted TOML updates:
- `updateTopLevelApiFields()` - Update model, model_provider
- `upsertProviderSection()` - Add/update provider
- `deleteProviderSection()` - Remove provider
- `upsertMcpSection()` - Add/update MCP service
- `deleteMcpSection()` - Remove MCP service

### Step 2: Modify API Call Sites (8 locations)

| File | Line | Function |
|------|------|----------|
| features.ts | 706 | updateCodexModelProvider |
| codex-provider-manager.ts | 95 | addProviderToExisting |
| codex-provider-manager.ts | 174 | editExistingProvider |
| codex-provider-manager.ts | 271 | deleteProviders |
| codex.ts | 1508 | applyCustomApiConfig |
| codex.ts | 1810 | configureCodexApi |
| codex.ts | 2117 | switchCodexProvider |
| codex.ts | 2171 | switchToOfficialLogin |
| codex.ts | 2240 | switchToProvider |

### Step 3: Modify MCP Call Sites (3 locations)

| File | Line | Function |
|------|------|----------|
| codex-configure.ts | 107 | configureCodexMcp (skipPrompt) |
| codex-configure.ts | 148 | configureCodexMcp (empty) |
| codex-configure.ts | 239 | configureCodexMcp (interactive) |

### Step 4: Verification

- Test API modification doesn't affect MCP config
- Test MCP modification doesn't affect API config
- Test preservation of `url`-type MCP services

## Expected Outcome

After fix, modifying API configuration will NOT touch MCP sections, preserving user's custom MCP configurations including SSE-type services with `url` field.
57 changes: 57 additions & 0 deletions .zcf/plan/history/2026-01-09_001414_fix-toml-top-level-fields.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# Fix TOML Top-Level Fields Bug

**Task**: Fix bugs in `updateTopLevelTomlFields()` function identified by Cursor Bugbot in PR #277

**Created**: 2026-01-08

## Context

Cursor Bugbot identified 3 bugs in `src/utils/zcf-config.ts`:

1. **Bug 2 (High)**: `lastUpdated` concatenated on same line as `version` field
2. **Bug 3 (High)**: New `version` field incorrectly inserted inside TOML section
3. **Bug 4 (Medium)**: Regex may corrupt section-level version instead of top-level

## Solution

Use Section-based precise positioning approach:
- Find first `[section]` position to determine top-level boundary
- Only operate on `version` and `lastUpdated` within top-level area
- Properly handle newline characters

## Execution Steps

### Step 1: Refactor `updateTopLevelTomlFields()` function
- File: `src/utils/zcf-config.ts`
- Location: Lines 75-136
- Changes:
- Find first section boundary
- Split content into top-level and rest
- Update/add version only in top-level area
- Update/add lastUpdated after version
- Properly handle newlines

### Step 2: Add helper functions
- `insertAtTopLevel()`: Insert content at top-level start (skip comments)
- `insertAfterVersion()`: Insert content after version field

### Step 3: Write/update test cases
- File: `tests/utils/zcf-config.test.ts` or new file
- Test scenarios:
1. Update existing top-level version and lastUpdated
2. Add missing top-level version (file starts with section)
3. Don't modify version under [claudeCode] section
4. lastUpdated properly on new line
5. Preserve top-level comments
6. Handle empty file

### Step 4: Run tests to verify
```bash
pnpm test:run -- zcf-config
```

## Expected Outcome

- All 3 bugs fixed
- No regression in existing functionality
- Test coverage for edge cases
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
File renamed without changes.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
"docs:preview": "pnpm -F @zcf/docs preview"
},
"dependencies": {
"@rainbowatcher/toml-edit-js": "catalog:runtime",
"@types/semver": "catalog:types",
"ansis": "catalog:cli",
"cac": "catalog:cli",
Expand All @@ -81,7 +82,6 @@
"ora": "catalog:cli",
"pathe": "catalog:runtime",
"semver": "catalog:runtime",
"smol-toml": "catalog:runtime",
"tinyexec": "catalog:runtime",
"trash": "catalog:runtime"
},
Expand Down
23 changes: 11 additions & 12 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pnpm-workspace.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ catalogs:
vitepress: ^1.6.4
vue: ^3.5.25
runtime:
'@rainbowatcher/toml-edit-js': ^0.6.4
dayjs: ^1.11.18
find-up-simple: ^1.0.1
fs-extra: ^11.3.2
i18next: ^25.5.2
i18next-fs-backend: ^2.6.0
pathe: ^2.0.3
semver: ^7.7.2
smol-toml: ^1.4.2
tinyexec: ^1.0.1
trash: ^10.0.0
testing:
Expand Down
22 changes: 18 additions & 4 deletions src/utils/code-tools/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -375,12 +375,26 @@ const result = await uninstallCodex(options)

### Q: How to handle TOML configuration parsing?

The module uses `smol-toml` for TOML parsing:
The module uses `@rainbowatcher/toml-edit-js` for TOML parsing with format-preserving editing:
```typescript
import { parse, stringify } from 'smol-toml'
import { parseToml, stringifyToml, editToml, batchEditToml } from '../toml-edit'

const config = parse(tomlString)
const tomlString = stringify(config)
// Parse TOML string to object
const config = parseToml<MyConfig>(tomlString)

// Stringify object to TOML (for new files)
const tomlString = stringifyToml(config)

// Edit specific nested paths while preserving formatting and comments
// Note: editToml only works with nested paths (e.g., 'section.key'), not top-level fields
const updatedToml = editToml(originalToml, 'section.key', 'new-value')

// Batch edit multiple paths
const edits: Array<[string, unknown]> = [
['general.lang', 'zh-CN'],
['settings.enabled', true],
]
const result = batchEditToml(originalToml, edits)
```

## Related File List
Expand Down
36 changes: 9 additions & 27 deletions src/utils/code-tools/codex-configure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ import { ensureI18nInitialized, i18n } from '../../i18n'
import { selectMcpServices } from '../mcp-selector'
import { getSystemRoot, isWindows } from '../platform'
import { updateZcfConfig } from '../zcf-config'
import { backupCodexComplete, getBackupMessage, readCodexConfig, writeCodexConfig } from './codex'
import { backupCodexComplete, getBackupMessage, readCodexConfig } from './codex'
import { applyCodexPlatformCommand } from './codex-platform'
import { batchUpdateCodexMcpServices } from './codex-toml-updater'

export async function configureCodexMcp(options?: CodexFullInitOptions): Promise<void> {
ensureI18nInitialized()
Expand Down Expand Up @@ -40,7 +41,6 @@ export async function configureCodexMcp(options?: CodexFullInitOptions): Promise
.filter(service => !service.requiresApiKey)
.map(service => service.id)

const baseProviders = existingConfig?.providers || []
const existingServices = existingConfig?.mcpServices || []
const selection: CodexMcpService[] = []

Expand Down Expand Up @@ -104,14 +104,8 @@ export async function configureCodexMcp(options?: CodexFullInitOptions): Promise
return svc
})

writeCodexConfig({
model: existingConfig?.model || null,
modelProvider: existingConfig?.modelProvider || null,
providers: baseProviders,
mcpServices: finalServices,
managed: true,
otherConfig: existingConfig?.otherConfig || [],
})
// Use targeted MCP updates - preserves existing SSE-type services
batchUpdateCodexMcpServices(finalServices)
updateZcfConfig({ codeToolType: 'codex' })
console.log(ansis.green(i18n.t('codex:mcpConfigured')))
return
Expand All @@ -122,13 +116,13 @@ export async function configureCodexMcp(options?: CodexFullInitOptions): Promise
return

const servicesMeta = await getMcpServices()
const baseProviders = existingConfig?.providers || []
const selection: CodexMcpService[] = []
const existingServices = existingConfig?.mcpServices || []

if (selectedIds.length === 0) {
console.log(ansis.yellow(i18n.t('codex:noMcpConfigured')))

// No new services to add, but ensure Windows SYSTEMROOT is set for existing services
const preserved = (existingServices || []).map((svc) => {
if (isWindows()) {
const systemRoot = getSystemRoot()
Expand All @@ -145,14 +139,8 @@ export async function configureCodexMcp(options?: CodexFullInitOptions): Promise
return svc
})

writeCodexConfig({
model: existingConfig?.model || null,
modelProvider: existingConfig?.modelProvider || null,
providers: baseProviders,
mcpServices: preserved,
managed: true,
otherConfig: existingConfig?.otherConfig || [],
})
// Use targeted MCP updates - preserves existing SSE-type services
batchUpdateCodexMcpServices(preserved)
updateZcfConfig({ codeToolType: 'codex' })
return
}
Expand Down Expand Up @@ -236,14 +224,8 @@ export async function configureCodexMcp(options?: CodexFullInitOptions): Promise
return svc
})

writeCodexConfig({
model: existingConfig?.model || null,
modelProvider: existingConfig?.modelProvider || null,
providers: baseProviders,
mcpServices: finalServices,
managed: true,
otherConfig: existingConfig?.otherConfig || [],
})
// Use targeted MCP updates - preserves existing SSE-type services
batchUpdateCodexMcpServices(finalServices)

updateZcfConfig({ codeToolType: 'codex' })
console.log(ansis.green(i18n.t('codex:mcpConfigured')))
Expand Down
Loading