fix: wait for MCP server startup before starting an agent turn - #453
Open
bgeisberger wants to merge 2 commits into
Open
fix: wait for MCP server startup before starting an agent turn#453bgeisberger wants to merge 2 commits into
bgeisberger wants to merge 2 commits into
Conversation
Sessions created with MCP servers registered a pending startup entry and kicked off the status publisher as fire-and-forget, so newSession and loadSession returned before Codex had started the servers. A prompt sent right after could start a turn without the session's MCP tools. Retain the startup promise and await it in prompt() before dispatching a turn. Only prompts that actually run an agent turn wait: commands the adapter answers itself, and Codex requests that never run a turn, are classified by CodexCommands.startsAgentTurn() and dispatch immediately. The wait is bounded by MCP_STARTUP_PROMPT_TIMEOUT_MS (default 30s), because the startup result only settles once every requested server reports a status newer than the snapshot version, which Codex may never send. On timeout the turn starts without those tools and later prompts are not delayed again. Cancel during the wait is handled explicitly: no turn exists yet, so cancel() would otherwise have found nothing to interrupt.
There was a problem hiding this comment.
🟡 Changes recommended
startsAgentTurn() currently gates some /goal variants that are handled locally (/goal, /goal pause, /goal clear, and oversized goals), causing unnecessary MCP-startup waiting that contradicts the intended bypass behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds a prompt-time gate in the ACP adapter to ensure a session’s configured MCP servers reach a terminal startup state (or a bounded timeout) before dispatching an agent turn that could rely on MCP tools, while still allowing adapter-handled “local commands” to respond immediately.
Changes:
- Track MCP startup per session and await it (with cancellation + timeout) before starting turn-driving prompts.
- Add a
startsAgentTurn()classifier to avoid gating prompts that the adapter handles locally. - Add Vitest regression coverage for gating behavior, cancellation, local-command bypass, and timeout fallback; document
MCP_STARTUP_PROMPT_TIMEOUT_MS.
File summaries
| File | Description |
|---|---|
| src/CodexCommands.ts | Adds startsAgentTurn() to decide whether a prompt should be gated on MCP startup. |
| src/CodexAcpServer.ts | Implements MCP startup tracking + bounded await before starting turns; handles cancel while waiting; adds env-configured timeout. |
| src/tests/CodexACPAgent/mcp-startup-gate.test.ts | Adds regression tests for the startup gate, bypass, cancellation, and timeout behavior. |
| readme-dev.md | Documents MCP_STARTUP_PROMPT_TIMEOUT_MS. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
MCP_STARTUP_PROMPT_TIMEOUT_MS(default 30s) and skip it on later prompts once it expiresRoot cause
session/newregisters the session's pending MCP startup and publishes its status fire-and-forget, then returns. Nothing ordersturn/startagainst that startup, so a prompt sent immediately after runs without the server's tools.Measured against a stdio MCP server that delays
initializeby 6s:turn/startgoes out ~0.2s after session creation, about six seconds before the server reportsready, and the model has no access to the tool. Codex does not await MCP startup on the app-server path, so the adapter has to order it.Earlier versions awaited MCP startup during session creation. That await was removed in #83 and #112 when the startup tracking was reworked, presumably for unrelated reasons.
The wait is bounded because it only settles once every requested server reports a status newer than the version snapshot taken at session start. If Codex ever reuses an already-running server without re-announcing it, no such status arrives. On timeout the turn starts without those tools rather than hanging.
Upstream
Codex's CLI path does await MCP startup before listing tools, tracked upstream as a first-turn stall (openai/codex#19556). The app-server path we drive does not, so this change adds the ordering. openai/codex#21318 and openai/codex#29321 propose building turns from only-ready MCP tools in the CLI as well, so this gate stays necessary rather than becoming redundant if they land.
Testing
npm run typechecknpm test: 492 passed, 26 skippednpm run bundle:all: all six targets compile, andcodex-acp-x64-linux --versionrunsend_turn) and that the local-command test fails if/statusis misclassified as turn-startingEnd-to-end against a real MCP server
Setup: an ACP session configured with one stdio MCP server that sleeps 6s before answering
initialize, and exposes a single tool returning a fixed magic string. The prompt instructs the model to call that tool and echo the string back, or to answerNO_TOOLif no such tool is available to it. Three runs per case, and all timings are relative to session creation.turn/startreadyRow 1 is the bug: the turn starts six seconds before the server is ready and the model cannot see the tool. Rows 2 and 3 are controls showing the same setup works whenever the turn happens to start after readiness. Row 4 is the fix, with the turn now starting 15ms after readiness instead of six seconds before it.