Skip to content
2 changes: 0 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,6 @@ Or add an MCP server (wired into Copilot, Claude, Cursor, Codex, OpenCode, Gemin
apm install --mcp io.github.github/github-mcp-server --transport http # connects over HTTPS
```

> *Codex CLI currently does not support remote MCP servers; the install will skip Codex with a notice. Omit `--transport http` to use the local Docker variant on Codex (requires `GITHUB_PERSONAL_ACCESS_TOKEN`).*

See the **[Getting Started guide](https://microsoft.github.io/apm/getting-started/quick-start/)** for the full walkthrough.

## Works with agentrc
Expand Down
65 changes: 48 additions & 17 deletions src/apm_cli/adapters/client/codex.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,18 +155,6 @@ def configure_mcp_server(
print(f"Error: MCP server '{server_url}' not found in registry")
return False

# Check for remote servers early - Codex doesn't support remote/SSE servers
remotes = server_info.get("remotes", [])
packages = server_info.get("packages", [])

# If server has only remote endpoints and no packages, it's a remote-only server
if remotes and not packages:
print(f"[!] Warning: MCP server '{server_url}' is a remote server (SSE type)")
print(" Codex CLI only supports local servers with command/args configuration")
print(" Remote servers are not supported by Codex CLI")
print(" Skipping installation for Codex CLI")
return False

# Determine the server name for configuration key
if server_name:
# Use explicitly provided server name
Expand All @@ -184,6 +172,10 @@ def configure_mcp_server(
# 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
Comment on lines 172 to +177
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix it
1a75308


# Update configuration using the chosen key
if not self.update_config({config_key: server_config}):
return False
Expand All @@ -204,7 +196,7 @@ def _format_server_config(self, server_info, env_overrides=None, runtime_vars=No
runtime_vars (dict, optional): Runtime variable values.

Returns:
dict: Formatted server configuration for Codex CLI.
dict | None: Formatted server configuration for Codex CLI, or None if unsupported (e.g. SSE remote).
"""
# Default configuration structure with registry ID for conflict detection
config = {
Expand All @@ -224,11 +216,37 @@ def _format_server_config(self, server_info, env_overrides=None, runtime_vars=No
self._warn_input_variables(raw["env"], server_info.get("name", ""), "Codex CLI")
return config

# Note: Remote servers (SSE type) are handled in configure_mcp_server and rejected early
# This method only handles local servers with packages

# Get packages from server info
# Remote MCP: SSE is unsupported by Codex -- return None to skip.
remotes = server_info.get("remotes", [])
packages = server_info.get("packages", [])
if remotes and not packages:
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 "
Comment on lines +223 to +227
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:541
  • gemini.py:131
  • vscode.py:362

The recommendation to “mirror the other adapters’ validation” is therefore self-defeating, because the referenced implementations use the exact same logic.

"is deprecated by the MCP spec and not supported by Codex. "
"Switch to `transport: streamable-http`.",
symbol="warning",
)
return None

remote_config = {
"url": (remote.get("url") or "").strip(),
"id": server_info.get("id", ""),
}
Comment on lines +234 to +237
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

http_headers: dict[str, str] = {}
for header in remote.get("headers", []):
h_name = header.get("name", "")
h_value = header.get("value", "")
if h_name and h_value:
http_headers[h_name] = self._resolve_variable_placeholders(
h_value, env_overrides or {}, runtime_vars or {}
)
if http_headers:
remote_config["http_headers"] = http_headers
self._warn_input_variables(http_headers, server_name, "Codex CLI")
return remote_config

if not packages:
# If no packages are available, this indicates incomplete server configuration
Expand Down Expand Up @@ -595,6 +613,19 @@ def _inject_docker_env_vars(self, args, env_vars):

return result

@staticmethod
def _select_remote_with_url(remotes):
"""Return the first remote entry that has a non-empty URL.

Returns:
dict or None: The first usable remote, or None if none qualify.
"""
for remote in remotes:
url = (remote.get("url") or "").strip()
if url:
return remote
return None

def _select_best_package(self, packages):
"""Select the best package for installation from available packages.

Expand Down
4 changes: 3 additions & 1 deletion src/apm_cli/core/conflict_detector.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,9 @@ def get_existing_server_configs(self) -> dict[str, Any]:
server_name = raw_key[len("mcp_servers.") :]
if server_name.startswith('"') and server_name.endswith('"'):
server_name = server_name[1:-1]
if isinstance(value, dict) and ("command" in value or "args" in value):
if isinstance(value, dict) and (
"command" in value or "args" in value or "url" in value
):
servers[server_name] = value

return servers
Expand Down
9 changes: 9 additions & 0 deletions tests/unit/integration/test_mcp_integrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,15 @@ def test_sse_transport_builds_remote(self):
info = MCPIntegrator._build_self_defined_info(dep)
assert "remotes" in info

def test_streamable_http_transport_builds_remote(self):
dep = _make_self_defined(
"stream-svc", transport="streamable-http", url="https://example.com/mcp"
)
info = MCPIntegrator._build_self_defined_info(dep)
assert info["remotes"][0]["transport_type"] == "streamable-http"
assert info["remotes"][0]["url"] == "https://example.com/mcp"
assert "packages" not in info

def test_http_with_headers(self):
dep = _make_self_defined(
"headered-svc",
Expand Down
16 changes: 16 additions & 0 deletions tests/unit/test_conflict_detection.py
Original file line number Diff line number Diff line change
Expand Up @@ -295,3 +295,19 @@ def test_codex_flat_keys_combine_with_nested_table(self):
)
configs = detector.get_existing_server_configs()
self.assertEqual(set(configs), {"nested", "flat", "quoted-name"})

def test_codex_flat_keys_detect_remote_url_entries(self):
"""Codex flat-key remote entries (url-only) are picked up alongside stdio ones."""
detector = self._make_detector(
"codex",
"mcp_servers",
{
"mcp_servers.foo": {
"url": "https://mcp.example.com/mcp",
"id": "ab12cd34-0000-0000-0000-000000000000",
},
},
)
configs = detector.get_existing_server_configs()
self.assertIn("foo", configs)
self.assertEqual(configs["foo"]["url"], "https://mcp.example.com/mcp")
106 changes: 91 additions & 15 deletions tests/unit/test_mcp_client_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,33 +159,28 @@ def test_configure_mcp_server_basic(self, mock_find_server):
server_config = config["mcp_servers"]["my_server"]
self.assertEqual(server_config["command"], "npx")

@patch("apm_cli.adapters.client.codex._rich_warning")
@patch("apm_cli.registry.client.SimpleRegistryClient.find_server_by_reference")
def test_configure_mcp_server_remote_rejected(self, mock_find_server):
"""Test that remote servers (SSE type) are rejected by Codex adapter."""
# Mock registry response for remote-only server
def test_configure_mcp_server_sse_remote_rejected(self, mock_find_server, mock_warn):
"""SSE remotes are rejected with a warning that points to streamable-http."""
mock_server_info = {
"id": "remote-server-id",
"name": "remote-server",
"remotes": [{"transport_type": "sse", "url": "https://example.com/mcp"}],
"packages": [], # No packages, only remote endpoints
"packages": [],
}
mock_find_server.return_value = mock_server_info

# Capture printed output
with patch("builtins.print") as mock_print:
result = self.adapter.configure_mcp_server("remote-server")
result = self.adapter.configure_mcp_server("remote-server")

# Should return False (rejected)
self.assertFalse(result)
mock_find_server.assert_called_once_with("remote-server")

# Verify warning message was printed
mock_print.assert_any_call(
"[!] Warning: MCP server 'remote-server' is a remote server (SSE type)"
)
mock_print.assert_any_call(
" Codex CLI only supports local servers with command/args configuration"
)
mock_warn.assert_called_once()
warn_message = mock_warn.call_args[0][0]
self.assertIn("remote-server", warn_message)
self.assertIn("SSE", warn_message)
self.assertIn("streamable-http", warn_message)

# Verify no config was updated
config = self.adapter.get_current_config()
Expand Down Expand Up @@ -274,6 +269,87 @@ def test_self_defined_stdio_normalizes_project_placeholders(self):
["-y", "@modelcontextprotocol/server-filesystem", ".", "."],
)

def test_format_server_config_streamable_http_writes_url_and_id(self):
"""Streamable-HTTP remote produces url + id (no http_headers when none)."""
server_info = {
"name": "figma",
"id": "ab12cd34-0000-0000-0000-000000000000",
"remotes": [
{
"url": "https://mcp.figma.com/mcp",
"transport_type": "streamable-http",
}
],
}
config = self.adapter._format_server_config(server_info)
self.assertEqual(config["url"], "https://mcp.figma.com/mcp")
self.assertEqual(config["id"], "ab12cd34-0000-0000-0000-000000000000")
self.assertNotIn("http_headers", config)

def test_format_server_config_streamable_http_writes_headers(self):
"""Registry-supplied headers land under ``http_headers``."""
server_info = {
"name": "figma",
"remotes": [
{
"url": "https://mcp.figma.com/mcp",
"transport_type": "streamable-http",
"headers": [
{"name": "Authorization", "value": "Bearer ghp_xxx"},
{"name": "X-Figma-Region", "value": "us-east-1"},
],
}
Comment on lines +297 to +301
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHub’s PAT format requires:

ghp_ + 36 alphanumeric characters

],
}
config = self.adapter._format_server_config(server_info)
self.assertEqual(
config["http_headers"],
{
"Authorization": "Bearer ghp_xxx",
"X-Figma-Region": "us-east-1",
},
)

def test_format_server_config_streamable_http_self_defined(self):
"""Self-defined streamable-http info produces a remote config."""
server_info = {
"name": "my-remote",
"remotes": [
{
"transport_type": "streamable-http",
"url": "https://example.com/mcp",
"headers": [{"name": "Authorization", "value": "Bearer xyz"}],
}
],
}
config = self.adapter._format_server_config(server_info)
self.assertEqual(config["url"], "https://example.com/mcp")
self.assertEqual(config["http_headers"], {"Authorization": "Bearer xyz"})

@patch("apm_cli.registry.client.SimpleRegistryClient.find_server_by_reference")
def test_configure_mcp_server_streamable_http_writes_toml_entry(self, mock_find_server):
"""End-to-end install of a streamable-HTTP server writes a parseable TOML entry."""
mock_find_server.return_value = {
"name": "figma",
"id": "ab12cd34-0000-0000-0000-000000000000",
"remotes": [
{
"url": "https://mcp.figma.com/mcp",
"transport_type": "streamable-http",
"headers": [{"name": "Authorization", "value": "Bearer ghp_xxx"}],
}
],
}

result = self.adapter.configure_mcp_server("figma/figma")

self.assertTrue(result)
config = self.adapter.get_current_config()
figma = config["mcp_servers"]["figma"]
self.assertEqual(figma["url"], "https://mcp.figma.com/mcp")
self.assertEqual(figma["id"], "ab12cd34-0000-0000-0000-000000000000")
self.assertEqual(figma["http_headers"], {"Authorization": "Bearer ghp_xxx"})


if __name__ == "__main__":
unittest.main()