fix mcp install for codex#263
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
🤖 My Senior Dev — Analysis Complete👤 For @khaliqgant📁 Expert in View your contributor analytics → 📊 2 files reviewed • 5 need attention
🚀 Open Interactive Review →The full interface unlocks features not available in GitHub:
💬 Chat here: 📖 View all 12 personas & slash commandsYou can interact with me by mentioning In PR comments or on any line of code:
Slash commands:
AI Personas (mention to get their perspective):
For the best experience, view this PR on myseniordev.com — includes AI chat, file annotations, and interactive reviews. |
codeant-ai
Bot
added
the
size:M
Sequence DiagramThis PR changes MCP installation so that when users install with an MCP editor target, the CLI skips format conversion and writes MCP entries directly into that editor's config file. This prevents MCP packages from being routed through the agents document path for Codex and correctly supports Cursor MCP config writes. sequenceDiagram
participant User
participant CLI
participant Registry
participant CodexConfig
participant CursorConfig
User->>CLI: Install MCP package with as editor
CLI->>Registry: Fetch package metadata and tarball
Registry-->>CLI: Return MCP package files
CLI->>CLI: Detect MCP editor target and skip conversion
alt Target is codex
CLI->>CodexConfig: Write MCP server entry to codex config
else Target is cursor
CLI->>CursorConfig: Write MCP server entry to cursor mcp config
end
CLI-->>User: Complete install without agents file path
Generated by CodeAnt AI |
Nitpicks 🔍
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| mockClient.getPackage.mockResolvedValue(mockPackage); | ||
| mockClient.downloadPackage.mockResolvedValue(await createMCPTarball(mcpServerJson)); | ||
|
|
||
| // The CLI passes both as and editor when --as codex is used |
There was a problem hiding this comment.
Suggestion: This test can produce false positives because .codex/config.toml is not cleaned by the shared beforeEach, so a stale config from earlier MCP Codex tests can satisfy assertions even if this install path regresses. Explicitly remove .codex at the start of this test to guarantee isolation. [logic error]
Severity Level: Major ⚠️
- ⚠️ Codex MCP install regressions may pass CI unnoticed.
- ⚠️ Test isolation broken in install-file-locations suite.| // The CLI passes both as and editor when --as codex is used | |
| // Ensure isolation: .codex is not cleared in the shared beforeEach list | |
| await fs.rm(path.join(testDir, '.codex'), { recursive: true, force: true }); | |
Steps of Reproduction ✅
1. Run this test file's suite
(`packages/cli/src/__tests__/install-file-locations.test.ts`): `beforeEach` at line 77
clears many paths, but its `dirs` list (lines 79-30) does not include `.codex`, so
`.codex/config.toml` can persist between tests.
2. In the same `describe('MCP server packages')` block, earlier tests at lines 870 and 896
call `handleInstall(..., { editor: 'codex' })` and read `.codex/config.toml`, creating
Codex config state on disk; persistence is real because `saveFile` is mocked as
`vi.fn(actual.saveFile)` at line 41.
3. The target test at line 947 calls `handleInstall('@test/mcp-as-codex', { as: 'codex',
editor: 'codex' })` (line 967), then only checks file content from `.codex/config.toml`
(line 970) for expected strings.
4. If Codex MCP writing regresses in runtime install logic
(`packages/cli/src/commands/install.ts` MCP branch around lines 1004-1067), this test can
still pass by reading stale config from previous tests, producing a false positive CI
result.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/cli/src/__tests__/install-file-locations.test.ts
**Line:** 966:966
**Comment:**
*Logic Error: This test can produce false positives because `.codex/config.toml` is not cleaned by the shared `beforeEach`, so a stale config from earlier MCP Codex tests can satisfy assertions even if this install path regresses. Explicitly remove `.codex` at the start of this test to guarantee isolation.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| // Skip conversion for snippets - they're raw content that doesn't need format conversion | ||
| if (options.as && format && format !== pkg.format && effectiveSubtype !== 'snippet') { | ||
| // Skip conversion for MCP server packages targeting an MCP editor — they use the dedicated MCP install path | ||
| const isMCPToEditor = isMCPServerPackage && MCP_EDITORS.includes(format as MCPEditor); |
There was a problem hiding this comment.
Suggestion: MCP editor inference is incomplete: when reinstalling from lockfile, as can be codex/cursor without editor being set, so this path is detected as MCP-to-editor but later defaults to Claude and writes MCP config to the wrong file. Infer and persist options.editor from format when format is an MCP editor to keep installs targeting the expected editor config. [logic error]
Severity Level: Critical 🚨
- ❌ Lockfile reinstall targets wrong MCP config file.
- ❌ Codex/Cursor MCP servers may be missing after reinstall.
- ⚠️ PR's editor-specific MCP install behavior regresses.| const isMCPToEditor = isMCPServerPackage && MCP_EDITORS.includes(format as MCPEditor); | |
| const inferredMcpEditor = isMCPServerPackage && MCP_EDITORS.includes(format as MCPEditor) | |
| ? (format as MCPEditor) | |
| : undefined; | |
| if (!options.editor && inferredMcpEditor) { | |
| options.editor = inferredMcpEditor; | |
| } | |
| const isMCPToEditor = Boolean(inferredMcpEditor); |
Steps of Reproduction ✅
1. Run lockfile reinstall path (`prpm install` with no package), which goes through
`createInstallCommand()` no-package branch at
`packages/cli/src/commands/install.ts:2071-2081` and calls `installFromLockfile({ as:
convertTo, ... })`.
2. `installFromLockfile()` calls `handleInstall()` at
`packages/cli/src/commands/install.ts:1976-1986` with `as: options.as || lockEntry.format`
but does not pass `editor`.
3. For an MCP package previously installed as Codex/Cursor, `handleInstall()` identifies
MCP path (`isMCPToEditor` logic at `install.ts:671-672`, MCP branch at
`install.ts:1006-1007`) but later resolves editor with fallback `const editor =
options.editor || 'claude'` at `install.ts:1029`.
4. MCP merge writes using Claude default path semantics
(`packages/cli/src/core/mcp.ts:338-367`), so config is written to `.mcp.json`/Claude
location instead of intended `.codex/config.toml` or `.cursor/mcp.json`.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** packages/cli/src/commands/install.ts
**Line:** 671:671
**Comment:**
*Logic Error: MCP editor inference is incomplete: when reinstalling from lockfile, `as` can be `codex`/`cursor` without `editor` being set, so this path is detected as MCP-to-editor but later defaults to Claude and writes MCP config to the wrong file. Infer and persist `options.editor` from `format` when `format` is an MCP editor to keep installs targeting the expected editor config.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
|
CodeAnt AI finished reviewing your PR. |
1 participant
CodeAnt-AI Description
Install MCP servers/tools into editor-specific configs when using --as (codex/cursor)
What Changed
Impact
✅ MCP installs land in Codex config instead of AGENTS.md✅ MCP tools install into Cursor's mcp.json✅ Fewer incorrect format conversions when installing MCP into editors💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.