fix(codex): add startup_timeout_sec to MCP servers to prevent first-run timeouts#971
fix(codex): add startup_timeout_sec to MCP servers to prevent first-run timeouts#971Lidang-Jiang wants to merge 4 commits intoaffaan-m:mainfrom
Conversation
…un timeouts On first startup, npx-based MCP servers need to download packages before they can respond. The default timeout is too short for this, causing frequent "timed out after 10 seconds" errors reported in affaan-m#544. Add startup_timeout_sec = 30 to all five command-based MCP servers (github, context7, memory, playwright, sequential-thinking). The URL-based exa server is unaffected and left unchanged. 30 seconds was chosen over the 20s precedent in merge-mcp-config.js to give extra headroom for slow networks on first run. Fixes affaan-m#544 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Lidang-Jiang <lidangjiang@gmail.com>
📝 WalkthroughWalkthroughAdds Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes first-run MCP server timeout errors by adding
Confidence Score: 5/5Safe to merge — the fix is straightforward and the previously-flagged regression path (merge script reverting the timeout) has been correctly addressed. No P0 or P1 issues remain. The only finding is a P2 style suggestion about integer vs float literal consistency that is unlikely to cause a real runtime problem. The prior concern about No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Codex starts MCP server via npx] --> B{Packages cached?}
B -- Yes --> C[Server responds quickly]
B -- No --> D[npx downloads package]
D --> E{Startup timeout elapsed?}
E -- Before fix: 10s default --> F[❌ Timeout error]
E -- After fix: 30s --> G[✅ Server ready]
C --> G
subgraph merge-mcp-config.js
H[--update-mcp flag] --> I[removeServerFromText]
I --> J[Append ECC_SERVERS spec]
J --> K[startup_timeout_sec = 30 preserved]
end
Reviews (3): Last reviewed commit: "fix(codex): align context7-mcp package s..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| [mcp_servers.github] | ||
| command = "npx" | ||
| args = ["-y", "@modelcontextprotocol/server-github"] | ||
| startup_timeout_sec = 30 | ||
|
|
||
| [mcp_servers.context7] | ||
| command = "npx" | ||
| args = ["-y", "@upstash/context7-mcp@latest"] | ||
| startup_timeout_sec = 30 | ||
|
|
||
| [mcp_servers.exa] | ||
| url = "https://mcp.exa.ai/mcp" | ||
|
|
||
| [mcp_servers.memory] | ||
| command = "npx" | ||
| args = ["-y", "@modelcontextprotocol/server-memory"] | ||
| startup_timeout_sec = 30 | ||
|
|
||
| [mcp_servers.playwright] | ||
| command = "npx" | ||
| args = ["-y", "@playwright/mcp@latest", "--extension"] | ||
| startup_timeout_sec = 30 | ||
|
|
||
| [mcp_servers.sequential-thinking] | ||
| command = "npx" | ||
| args = ["-y", "@modelcontextprotocol/server-sequential-thinking"] | ||
| startup_timeout_sec = 30 |
There was a problem hiding this comment.
startup_timeout_sec not added to merge-mcp-config.js — fix will be reverted on next update
The companion script scripts/codex/merge-mcp-config.js maintains its own ECC_SERVERS definitions (lines 86–100) that are used to add or update MCP server configs. None of the dlxServer(...) calls for playwright, context7-mcp, memory, sequential-thinking, or github include startup_timeout_sec:
playwright: dlxServer('playwright', '@playwright/mcp@latest'),
'context7-mcp': dlxServer('context7-mcp', '@upstash/context7-mcp'),
memory: dlxServer('memory', '@modelcontextprotocol/server-memory'),
'sequential-thinking': dlxServer('sequential-thinking', ...),
github: { fields: { command: 'bash', ... }, toml: ... }This creates two problems:
-
--update-mcpreverts the fix: When users runnode merge-mcp-config.js <config.toml> --update-mcp,removeServerFromTextstrips each section and re-appends it fromECC_SERVERS, which has nostartup_timeout_sec. The timeout fix is silently lost. -
Add-only mode emits drift warnings:
configDiffers()compares the user's existing config againstspec.fields. Becausestartup_timeout_secis now inconfig.tomlbut not inspec.fields, every affected server will trigger aWARNING: mcp_servers.<name> differs from ECC recommendationmessage, even though the user's config is intentionally better.
merge-mcp-config.js should be updated to include startup_timeout_sec in each relevant server's extraFields and extraToml, similar to how supabase already does it (line 87):
startup_timeout_sec: 30,![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.codex/config.toml:
- Around line 41-64: The new startup_timeout_sec = 30 entries were added to the
MCP server blocks but the generator/spec (ECC_SERVERS in
scripts/codex/merge-mcp-config.js) wasn’t updated, which will cause merge drift;
update the ECC_SERVERS definition in scripts/codex/merge-mcp-config.js to
include startup_timeout_sec: 30 for the matching server entries (context7,
memory, playwright, sequential-thinking, etc.) so the generated merge spec
matches .codex/config.toml and prevents --update-mcp from removing these values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
…VERS Reviewers identified that merge-mcp-config.js --update-mcp would silently strip the startup_timeout_sec from config.toml because the ECC_SERVERS spec did not include it. Add startup_timeout_sec = 30 to playwright, context7-mcp, github, memory, and sequential-thinking. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Lidang-Jiang <lidangjiang@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/codex/merge-mcp-config.js (1)
88-99: Extract repeatedstartup_timeout_sec = 30into a shared constant.This block duplicates the same timeout literal in multiple places. Centralizing it prevents accidental divergence and makes future tuning one-line.
♻️ Proposed refactor
+const DEFAULT_MCP_STARTUP_TIMEOUT_SEC = 30; +const DEFAULT_MCP_STARTUP_TIMEOUT_TOML = `startup_timeout_sec = ${DEFAULT_MCP_STARTUP_TIMEOUT_SEC}`; + const ECC_SERVERS = { supabase: dlxServer('supabase', '@supabase/mcp-server-supabase@latest', { startup_timeout_sec: 20.0, tool_timeout_sec: 120.0 }, 'startup_timeout_sec = 20.0\ntool_timeout_sec = 120.0'), - playwright: dlxServer('playwright', '@playwright/mcp@latest', { startup_timeout_sec: 30 }, 'startup_timeout_sec = 30'), - 'context7-mcp': dlxServer('context7-mcp', '@upstash/context7-mcp', { startup_timeout_sec: 30 }, 'startup_timeout_sec = 30'), + playwright: dlxServer('playwright', '@playwright/mcp@latest', { startup_timeout_sec: DEFAULT_MCP_STARTUP_TIMEOUT_SEC }, DEFAULT_MCP_STARTUP_TIMEOUT_TOML), + 'context7-mcp': dlxServer('context7-mcp', '@upstash/context7-mcp@latest', { startup_timeout_sec: DEFAULT_MCP_STARTUP_TIMEOUT_SEC }, DEFAULT_MCP_STARTUP_TIMEOUT_TOML), exa: { fields: { url: 'https://mcp.exa.ai/mcp' }, toml: `[mcp_servers.exa]\nurl = "https://mcp.exa.ai/mcp"` }, github: { - fields: { command: 'bash', args: ['-lc', GH_BOOTSTRAP], startup_timeout_sec: 30 }, - toml: `[mcp_servers.github]\ncommand = "bash"\nargs = ["-lc", ${JSON.stringify(GH_BOOTSTRAP)}]\nstartup_timeout_sec = 30` + fields: { command: 'bash', args: ['-lc', GH_BOOTSTRAP], startup_timeout_sec: DEFAULT_MCP_STARTUP_TIMEOUT_SEC }, + toml: `[mcp_servers.github]\ncommand = "bash"\nargs = ["-lc", ${JSON.stringify(GH_BOOTSTRAP)}]\n${DEFAULT_MCP_STARTUP_TIMEOUT_TOML}` }, - memory: dlxServer('memory', '@modelcontextprotocol/server-memory', { startup_timeout_sec: 30 }, 'startup_timeout_sec = 30'), - 'sequential-thinking': dlxServer('sequential-thinking', '@modelcontextprotocol/server-sequential-thinking', { startup_timeout_sec: 30 }, 'startup_timeout_sec = 30') + memory: dlxServer('memory', '@modelcontextprotocol/server-memory', { startup_timeout_sec: DEFAULT_MCP_STARTUP_TIMEOUT_SEC }, DEFAULT_MCP_STARTUP_TIMEOUT_TOML), + 'sequential-thinking': dlxServer('sequential-thinking', '@modelcontextprotocol/server-sequential-thinking', { startup_timeout_sec: DEFAULT_MCP_STARTUP_TIMEOUT_SEC }, DEFAULT_MCP_STARTUP_TIMEOUT_TOML) };As per coding guidelines: "Do not use hardcoded values; use constants or configuration instead."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/codex/merge-mcp-config.js` around lines 88 - 99, Introduce a single constant (e.g., STARTUP_TIMEOUT_SEC = 30) and replace all hardcoded 30 occurrences used for startup timeouts: update object literals like the second argument to dlxServer calls (currently { startup_timeout_sec: 30 }) and replace the repeated string fragments ('startup_timeout_sec = 30') used in toml/string args with a template built from that constant; ensure places referenced in the diff—keys for 'playwright', 'context7-mcp', 'github', 'memory', 'sequential-thinking', and any dlxServer(...) invocations—use the new STARTUP_TIMEOUT_SEC variable so both the JS objects and the toml strings remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/codex/merge-mcp-config.js`:
- Line 89: The package specifier for the 'context7-mcp' entry is inconsistent
and causes configDiffers to flag drift; update the dlxServer call for
'context7-mcp' (the line invoking dlxServer('context7-mcp',
'@upstash/context7-mcp', ...)) to use the exact same package string
'@upstash/context7-mcp@latest' as in .codex/config.toml so configDiffers
performs an exact match; ensure the change is applied where dlxServer is called
so future comparisons won't trigger needless rewrites.
---
Nitpick comments:
In `@scripts/codex/merge-mcp-config.js`:
- Around line 88-99: Introduce a single constant (e.g., STARTUP_TIMEOUT_SEC =
30) and replace all hardcoded 30 occurrences used for startup timeouts: update
object literals like the second argument to dlxServer calls (currently {
startup_timeout_sec: 30 }) and replace the repeated string fragments
('startup_timeout_sec = 30') used in toml/string args with a template built from
that constant; ensure places referenced in the diff—keys for 'playwright',
'context7-mcp', 'github', 'memory', 'sequential-thinking', and any
dlxServer(...) invocations—use the new STARTUP_TIMEOUT_SEC variable so both the
JS objects and the toml strings remain consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8ac7c3d6-85e6-40a0-8ce4-ff1d77429277
📒 Files selected for processing (1)
scripts/codex/merge-mcp-config.js
Add @latest suffix to '@upstash/context7-mcp' in ECC_SERVERS so the generated merge spec matches .codex/config.toml exactly, preventing configDiffers from flagging false drift on --update-mcp runs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Lidang-Jiang <lidangjiang@gmail.com>
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
2 participants
Summary
startup_timeout_sec = 30to all five command-based MCP servers in.codex/config.tomlstartup_timeout_secintomerge-mcp-config.jsECC_SERVERS to prevent--update-mcpdrift@upstash/context7-mcp@latestpackage specifier between config.toml and merge scriptDEFAULT_MCP_STARTUP_TIMEOUT_SECconstant to reduce duplication (CodeRabbit nitpick)Problem
When Codex starts for the first time (or after clearing npm cache), MCP servers launched via
npxneed to download their packages before responding. The default startup timeout is too short, causing errors like:Changes
githubstartup_timeout_sec = 30context7startup_timeout_sec = 30memorystartup_timeout_sec = 30playwrightstartup_timeout_sec = 30sequential-thinkingstartup_timeout_sec = 30exa30 seconds was chosen over the 20s precedent in
merge-mcp-config.jsto give extra headroom for slow networks on first run.Additionally, per CodeRabbit reviewer feedback, the hardcoded
30values inmerge-mcp-config.jshave been extracted into shared constantsDEFAULT_MCP_STARTUP_TIMEOUT_SECandDEFAULT_MCP_STARTUP_TIMEOUT_TOMLto prevent divergence and simplify future tuning.Before (TOML syntax validation + test suite — prior to constant extraction)
After (TOML syntax validation + constant verification + test suite — after constant extraction)
Reviewer feedback addressed
startup_timeout_secintomerge-mcp-config.jsECC_SERVERS to prevent--update-mcpdrift ✅@upstash/context7-mcp@latestpackage specifier ✅DEFAULT_MCP_STARTUP_TIMEOUT_SECconstant ✅Test plan
python3 -c "import tomllib; tomllib.load(open('.codex/config.toml','rb'))"Fixes #544