fix: support Streamable HTTP MCP servers for Codex CLI (closes #1260)#1262
fix: support Streamable HTTP MCP servers for Codex CLI (closes #1260)#1262masaishi wants to merge 7 commits into
Conversation
…uration and handling
… transport support
masaishi
requested review from
danielmeppiel and
sergio-sisternes-epam
as code owners
|
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
Pull request overview
This PR updates the Codex CLI adapter so apm install can emit Streamable HTTP (remote) MCP server entries into Codex config.toml, while continuing to skip SSE remotes with an updated warning message. It also expands conflict detection to recognize Codex TOML “flat-key” server entries that are configured via url, and removes a README statement claiming Codex cannot use remote MCP servers.
Changes:
- Codex adapter: generate
mcp_servers.<name>TOML entries for remotestreamable-httpMCP servers (includinghttp_headers), and skip SSE remotes via formatter signaling. - Conflict detector: treat
mcp_servers.<name>flat-key configs containingurlas existing servers (not justcommand/args). - Tests + README: add unit coverage for streamable-http + conflict detection, and remove misleading README wording about Codex remote MCP support.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/apm_cli/adapters/client/codex.py |
Adds remote Streamable HTTP config emission for Codex and SSE-skip path via _format_server_config() returning None. |
src/apm_cli/core/conflict_detector.py |
Extends Codex flat-key server detection to include url-based configs. |
tests/unit/test_mcp_client_factory.py |
Updates SSE skip test to assert _rich_warning content; adds streamable-http formatting + install tests. |
tests/unit/test_conflict_detection.py |
Adds coverage for detecting Codex flat-key remote url entries. |
tests/unit/integration/test_mcp_integrator.py |
Adds coverage ensuring self-defined streamable-http builds a remotes entry. |
README.md |
Removes the statement that Codex CLI does not support remote MCP servers. |
| remote = self._select_remote_with_url(remotes) or remotes[0] | ||
| server_name = server_info.get("name", "") | ||
| if (remote.get("transport_type") or "").strip() == "sse": | ||
| _rich_warning( | ||
| f"Skipping MCP server '{server_name}' for Codex CLI: SSE transport " |
There was a problem hiding this comment.
APM always builds a single-element remotes list (mcp_integrator.py:216: info["remotes"] = [remote]), so the proposed “first SSE, later streamable-http” scenario cannot occur.
Additionally, the _select_remote_with_url(remotes) or remotes[0] pattern is already identical across the other adapters:
copilot.py:541gemini.py:131vscode.py:362
The recommendation to “mirror the other adapters’ validation” is therefore self-defeating, because the referenced implementations use the exact same logic.
| remote_config = { | ||
| "url": (remote.get("url") or "").strip(), | ||
| "id": server_info.get("id", ""), | ||
| } |
There was a problem hiding this comment.
codex.py:617-627’s _select_remote_with_url contractually never returns a remote with an empty URL.
Writing id as an empty string is intentional and consistent with copilot.py:565. This follows the conflict_detector schema convention (introduced in commit 6a44d74).
Omitting the key would break conflict detection behavior.
| # Generate server configuration with environment variable resolution | ||
| server_config = self._format_server_config(server_info, env_overrides, runtime_vars) | ||
|
|
||
| # Skip if formatter signaled "unsupported" (e.g. SSE remote on Codex) | ||
| if server_config is None: | ||
| return False |
| "headers": [ | ||
| {"name": "Authorization", "value": "Bearer ghp_xxx"}, | ||
| {"name": "X-Figma-Region", "value": "us-east-1"}, | ||
| ], | ||
| } |
There was a problem hiding this comment.
GitHub’s PAT format requires:
ghp_ + 36 alphanumeric characters
…g docstring to include None
Description
Codex CLI supports Streamable HTTP MCP servers, but
apm installwas skipping Codex for every remote MCP entry, andREADME.mddocumented this as intended behavior.This PR aligns the Codex adapter with the official Codex MCP spec:
~/.codex/config.tomlasmcp_servers.<name>entries withurl = "..."(andhttp_headerswhen provided).transport: streamable-http(SSE is deprecated by the MCP spec and absent from the Codex config schema).mcp_servers.<name>entries that useurl(previously onlycommand/argswere matched).README.md.Fixes #1260
Type of change
Testing
Tested locally
All existing tests pass
Added tests for new functionality (if applicable)
test_mcp_client_factory.pytest_conflict_detection.py_build_self_defined_info(test_mcp_integrator.py)