Repository navigation
feat(mcp): inject MCP server InitializeResult.instructions into system prompt - #4262
luyao10282 wants to merge 1 commit into
Conversation
…m prompt - Expose getInstructions() on stdio, SSE, and HTTP MCP clients and store handshake instructions on McpConnectionManager entries. - Append connected baseline MCP server instructions under '# MCP Server Instructions' in AgentMcpService before each step begins.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟡 Changes recommended
Prompt synchronization must account for turn snapshots, remove obsolete instructions, and preserve user-authored guidance.
3 open issues
What changed in this PR
Adds MCP server handshake instructions to agent system prompts so the model receives server-provided usage guidance.
Changes:
- Exposes instructions through all three MCP transports and stores non-empty values.
- Adds per-step prompt synchronization for connected baseline servers.
- Adds a prompt-injection unit test.
| File | Description |
|---|---|
packages/agent-core-v2/test/agent/mcp/mcp.test.ts |
Tests instruction injection into profile state. |
packages/agent-core-v2/src/mcpCore/types.ts |
Adds an optional instruction accessor. |
packages/agent-core-v2/src/mcpCore/connection-manager.ts |
Stores and exposes handshake instructions. |
packages/agent-core-v2/src/mcpCore/client-stdio.ts |
Exposes stdio server instructions. |
packages/agent-core-v2/src/mcpCore/client-sse.ts |
Exposes SSE server instructions. |
packages/agent-core-v2/src/mcpCore/client-http.ts |
Exposes HTTP server instructions. |
packages/agent-core-v2/src/agent/mcp/mcpService.ts |
Synchronizes instructions before each step. |
Review effort: Balanced
| this.attachMcpTools(); | ||
| loop.hooks.onWillBeginStep.register('mcp', async (ctx, next) => { | ||
| await this.waitForInitialLoad(ctx.signal); | ||
| this.syncMcpInstructions(); |
| sections.push(`## ${entry.name}\n${instructions}`); | ||
| } | ||
| } | ||
| if (sections.length === 0) return; |
| const index = current.indexOf(marker); | ||
| const base = index === -1 ? current : current.slice(0, index); |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2053be564d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| this.attachMcpTools(); | ||
| loop.hooks.onWillBeginStep.register('mcp', async (ctx, next) => { | ||
| await this.waitForInitialLoad(ctx.signal); | ||
| this.syncMcpInstructions(); |
There was a problem hiding this comment.
beginActiveTurn() calls prepareTurnConfig() at loopService.ts:1196, and getOrCreateTurnConfig() snapshots profile.getSystemPrompt() at llmRequesterService.ts:765-770; this hook is only run later by gate() at loopService.ts:1024. Consequently, the first turn sends no MCP instructions at all, and any later instruction change is delayed until the following turn because every request in the current turn uses the frozen prompt. Perform this synchronization before prepareTurnConfig() or include the contribution while resolving the effective request.
AGENTS.md reference: AGENTS.md:L122-L124
Useful? React with 👍 / 👎.
| sections.push(`## ${entry.name}\n${instructions}`); | ||
| } | ||
| } | ||
| if (sections.length === 0) return; |
There was a problem hiding this comment.
When the last instruction-bearing server is disabled, removed, fails, or reconnects without instructions, sections is empty while the profile still contains the block injected on an earlier step. Returning here leaves those obsolete directives in the system prompt indefinitely, so subsequent turns can be told to use a server or tools that are no longer available. Strip the managed block when the current section set is empty.
AGENTS.md reference: AGENTS.md:L122-L124
Useful? React with 👍 / 👎.
| 'The following MCP servers have provided instructions for how to use their tools and resources:\n\n' + | ||
| sections.join('\n\n'); | ||
| const current = this.profile.getSystemPrompt(); | ||
| if (current.length === 0) return; |
There was a problem hiding this comment.
A profile is allowed to render an empty system prompt, but with such a profile this return prevents the generated MCP block from ever being installed even when connected servers provide instructions. In that configuration every turn silently omits the only MCP guidance; use the generated block itself when the base prompt is empty instead of returning.
AGENTS.md reference: AGENTS.md:L102-L104
Useful? React with 👍 / 👎.
| } | ||
| if (sections.length === 0) return; | ||
| const block = | ||
| '# MCP Server Instructions\n\n' + |
There was a problem hiding this comment.
This commit introduces user-visible system-prompt behavior, but its seven changed paths contain no .changeset entry and the repository's existing changesets contain no matching MCP-instructions release note. Add the required short user-facing changeset so the release workflow records this behavior change.
AGENTS.md reference: AGENTS.md:L89-L90
Useful? React with 👍 / 👎.
Summary
MCP servers can return server-level usage guidance via
InitializeResult.instructionsduring handshake (Client.getInstructions()in@modelcontextprotocol/sdk), which was previously ignored after connection.getInstructions()onStdioMcpClient,SseMcpClient, andHttpMcpClientand store non-empty handshakeinstructionsonMcpConnectionManagerentries.# MCP Server InstructionsinAgentMcpServicebefore each step begins.test/agent/mcp/mcp.test.ts.Verification
node scripts/check-no-comments.mjspnpm --filter @moonshot-ai/agent-core-v2 exec vitest run test/agent/mcp/mcp.test.ts