diff --git a/README.md b/README.md index 394682122..595f9315a 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/src/apm_cli/adapters/client/codex.py b/src/apm_cli/adapters/client/codex.py index ea8f2eda7..8ed9f2d94 100644 --- a/src/apm_cli/adapters/client/codex.py +++ b/src/apm_cli/adapters/client/codex.py @@ -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 @@ -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 + # Update configuration using the chosen key if not self.update_config({config_key: server_config}): return False @@ -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 = { @@ -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 " + "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", ""), + } + 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 @@ -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. diff --git a/src/apm_cli/core/conflict_detector.py b/src/apm_cli/core/conflict_detector.py index 07523f255..582ddbf7f 100644 --- a/src/apm_cli/core/conflict_detector.py +++ b/src/apm_cli/core/conflict_detector.py @@ -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 diff --git a/tests/unit/integration/test_mcp_integrator.py b/tests/unit/integration/test_mcp_integrator.py index 94d63866a..c0585581c 100644 --- a/tests/unit/integration/test_mcp_integrator.py +++ b/tests/unit/integration/test_mcp_integrator.py @@ -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", diff --git a/tests/unit/test_conflict_detection.py b/tests/unit/test_conflict_detection.py index 9dd52a53b..9c6ccbf46 100644 --- a/tests/unit/test_conflict_detection.py +++ b/tests/unit/test_conflict_detection.py @@ -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") diff --git a/tests/unit/test_mcp_client_factory.py b/tests/unit/test_mcp_client_factory.py index d049f7a8d..1e3164f7f 100644 --- a/tests/unit/test_mcp_client_factory.py +++ b/tests/unit/test_mcp_client_factory.py @@ -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() @@ -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"}, + ], + } + ], + } + 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()