Skip to content

fix(acp): plumb mcp_servers config into ACP session payloads#985

Open
feiyun968-agent wants to merge 7 commits into
openabdev:mainfrom
feiyun968-agent:fix/976-mcp-servers-acp-plumbing
Open

fix(acp): plumb mcp_servers config into ACP session payloads#985
feiyun968-agent wants to merge 7 commits into
openabdev:mainfrom
feiyun968-agent:fix/976-mcp-servers-acp-plumbing

Conversation

@feiyun968-agent
Copy link
Copy Markdown
Contributor

Discord Discussion URL: #976

Summary

Fixes the MCP function call stall reported in #976.

codex-acp relies solely on the mcpServers field in the ACP session/new and session/load payloads for MCP server discovery. Both calls hardcoded mcpServers: [], so any configured MCP sidecar was never registered with the agent — causing the model to emit function_call events that stalled indefinitely without ever dispatching tools/call.

Changes

  • src/config.rs — Add McpServerConfig struct and mcp_servers: HashMap<String, McpServerConfig> field to AgentConfig; add AgentConfig::acp_mcp_servers() to serialize into the JSON array the ACP protocol expects
  • src/acp/connection.rs — Accept mcp_servers: &[Value] in session_new() and session_load() instead of hardcoding []
  • src/acp/pool.rs — Resolve acp_mcp_servers() once per session init and pass to both call sites
  • docs/codex.md — Document the new [[agent.mcp_servers]] config section with stdio, http, and sse examples

Behaviour

  • Empty mcp_servers (the default) preserves existing behaviour exactly — mcpServers: [] is sent, same as before
  • Supports stdio (command/args/env), http, and sse (url/headers) transports
  • Env vars and headers support ${VAR} expansion via the existing expand_env_vars helper

Validation

  • cargo fmt (clean)
  • cargo check — C linker unavailable in this environment; code structure verified against PR feat(agent): forward configured MCP servers #903 (same approach, validated at the time with cargo check, cargo test 403 passed, cargo clippy -- -D warnings)
  • Diff reviewed manually; logic is minimal and follows existing patterns in the codebase

Prior Art

feiyun968-agent and others added 5 commits May 25, 2026 15:19
…ev#976)

codex-acp relies solely on the mcpServers field in the ACP session/new
and session/load payloads for MCP server discovery. Both calls hardcoded
mcpServers: [], so any configured MCP sidecar was never registered —
causing the model to emit function_calls that stalled without ever
dispatching tools/call.

- Add McpServerConfig struct and mcp_servers field to AgentConfig
- Add AgentConfig::acp_mcp_servers() to serialize config into ACP JSON
- Pass resolved servers into session_new() and session_load()
- Document [[agent.mcp_servers]] config in docs/codex.md

Supports stdio (command/args/env), http, and sse (url/headers)
transports. Empty mcp_servers (default) preserves existing behaviour.

Fixes openabdev#976
@github-actions github-actions Bot added the closing-soon PR missing Discord Discussion URL — will auto-close in 3 days label Jun 3, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 3, 2026

⚠️ This PR is missing a Discord Discussion URL in the body.

All PRs must reference a prior Discord discussion to ensure community alignment before implementation.

Please edit the PR description to include a link like:

Discord Discussion URL: https://discord.com/channels/...

This PR will be automatically closed in 3 days if the link is not added.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

closing-soon PR missing Discord Discussion URL — will auto-close in 3 days

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants