From 7fe600703adfbb90b376769127a4f2199031de8b Mon Sep 17 00:00:00 2001 From: Masamune William Ishihara Date: Sun, 10 May 2026 18:18:06 -0700 Subject: [PATCH 1/8] test(conflict_detection): add test for codex flat-key remote URL entries detection --- tests/unit/test_conflict_detection.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) 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") From 6a44d7465a7fd25c0c63357980baa2e889f01387 Mon Sep 17 00:00:00 2001 From: Masamune William Ishihara Date: Sun, 10 May 2026 18:18:18 -0700 Subject: [PATCH 2/8] fix(conflict_detector): include 'url' in server configuration checks --- src/apm_cli/core/conflict_detector.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 From f2f6fbced3814214f316246fbf3e7fc4851fb3e7 Mon Sep 17 00:00:00 2001 From: Masamune William Ishihara Date: Sun, 10 May 2026 18:35:20 -0700 Subject: [PATCH 3/8] test(mcp_client_factory): add tests for streamable-http server configuration and handling --- tests/unit/integration/test_mcp_integrator.py | 9 ++ tests/unit/test_mcp_client_factory.py | 106 +++++++++++++++--- 2 files changed, 100 insertions(+), 15 deletions(-) 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_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() From e068d11a7a645419ef11224b1aa1ef82f470853b Mon Sep 17 00:00:00 2001 From: Masamune William Ishihara Date: Sun, 10 May 2026 18:35:30 -0700 Subject: [PATCH 4/8] fix(codex_client_adapter): improve handling of remote servers and SSE transport support --- src/apm_cli/adapters/client/codex.py | 63 +++++++++++++++++++++------- 1 file changed, 47 insertions(+), 16 deletions(-) diff --git a/src/apm_cli/adapters/client/codex.py b/src/apm_cli/adapters/client/codex.py index ea8f2eda7..6db046c49 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 @@ -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. From 4d7b0175b767d0f5034a417689db4e08b70fb9e1 Mon Sep 17 00:00:00 2001 From: Masamune William Ishihara Date: Sun, 10 May 2026 18:35:37 -0700 Subject: [PATCH 5/8] docs: update README to clarify Codex CLI limitations with remote MCP servers --- README.md | 2 -- 1 file changed, 2 deletions(-) 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 From 1a75308762186567c182b61a708c0f30db66c5f0 Mon Sep 17 00:00:00 2001 From: Masamune William Ishihara Date: Sun, 10 May 2026 20:40:17 -0700 Subject: [PATCH 6/8] fix(codex_client_adapter): update return type in _format_server_config docstring to include None --- src/apm_cli/adapters/client/codex.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/apm_cli/adapters/client/codex.py b/src/apm_cli/adapters/client/codex.py index 6db046c49..8ed9f2d94 100644 --- a/src/apm_cli/adapters/client/codex.py +++ b/src/apm_cli/adapters/client/codex.py @@ -196,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 = { From 9caa34ca3e0ee42fa3621d0dd52ef66d6ba6a45d Mon Sep 17 00:00:00 2001 From: Masamune William Ishihara Date: Fri, 15 May 2026 13:39:19 -0700 Subject: [PATCH 7/8] fix(codex): reject non-HTTPS remote URLs to prevent cleartext header leakage --- src/apm_cli/adapters/client/codex.py | 14 +++++++- tests/unit/test_mcp_client_factory.py | 52 +++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/src/apm_cli/adapters/client/codex.py b/src/apm_cli/adapters/client/codex.py index 8ed9f2d94..402639292 100644 --- a/src/apm_cli/adapters/client/codex.py +++ b/src/apm_cli/adapters/client/codex.py @@ -3,6 +3,7 @@ import logging import os from pathlib import Path +from urllib.parse import urlparse import toml @@ -231,8 +232,19 @@ def _format_server_config(self, server_info, env_overrides=None, runtime_vars=No ) return None + remote_url = (remote.get("url") or "").strip() + + scheme = urlparse(remote_url).scheme.lower() + if scheme != "https": + _rich_warning( + f"Skipping MCP server '{server_name}' for Codex CLI: remote URL " + f"must use https:// (got {scheme or 'no scheme'}).", + symbol="warning", + ) + return None + remote_config = { - "url": (remote.get("url") or "").strip(), + "url": remote_url, "id": server_info.get("id", ""), } http_headers: dict[str, str] = {} diff --git a/tests/unit/test_mcp_client_factory.py b/tests/unit/test_mcp_client_factory.py index 1e3164f7f..54173057d 100644 --- a/tests/unit/test_mcp_client_factory.py +++ b/tests/unit/test_mcp_client_factory.py @@ -326,6 +326,58 @@ def test_format_server_config_streamable_http_self_defined(self): self.assertEqual(config["url"], "https://example.com/mcp") self.assertEqual(config["http_headers"], {"Authorization": "Bearer xyz"}) + @patch("apm_cli.adapters.client.codex._rich_warning") + def test_format_server_config_streamable_http_rejects_non_https(self, mock_warn): + """Non-HTTPS remote URLs are rejected to prevent cleartext header leakage.""" + server_info = { + "name": "evil-remote", + "id": "evil-id", + "remotes": [ + { + "url": "http://mcp.example.com/mcp", + "transport_type": "streamable-http", + "headers": [{"name": "Authorization", "value": "Bearer secret"}], + } + ], + } + + result = self.adapter._format_server_config(server_info) + + self.assertIsNone(result) + mock_warn.assert_called_once() + warn_message = mock_warn.call_args[0][0] + self.assertIn("evil-remote", warn_message) + self.assertIn("https://", warn_message) + + @patch("apm_cli.adapters.client.codex._rich_warning") + @patch("apm_cli.registry.client.SimpleRegistryClient.find_server_by_reference") + def test_configure_mcp_server_http_remote_rejected(self, mock_find_server, mock_warn): + """End-to-end: an http:// remote URL never lands in the Codex config.""" + mock_find_server.return_value = { + "name": "evil-remote", + "id": "evil-id", + "remotes": [ + { + "url": "http://mcp.example.com/mcp", + "transport_type": "streamable-http", + "headers": [{"name": "Authorization", "value": "Bearer secret"}], + } + ], + "packages": [], + } + + result = self.adapter.configure_mcp_server("evil-remote") + + self.assertFalse(result) + mock_warn.assert_called_once() + warn_message = mock_warn.call_args[0][0] + self.assertIn("evil-remote", warn_message) + self.assertIn("https://", warn_message) + + # Verify no config was written + config = self.adapter.get_current_config() + self.assertNotIn("mcp_servers", config) + @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.""" From b5235d63506b7fcd5a80753f59727c0af357cd2b Mon Sep 17 00:00:00 2001 From: Daniel Meppiel Date: Tue, 19 May 2026 15:12:56 +0200 Subject: [PATCH 8/8] fix(codex): polish streamable-http MCP support (follow-ups for #1262) Six non-blocking follow-ups from PR #1262 shepherd review: 1. Auth-header disclosure (audit): no leak path found. _warn_input_variables in base.py logs only the ${input:VAR_ID} identifier, never the header value. Header values flow value -> http_headers dict -> toml.dump only. No code change required. 2. _rich_success on success: replace bare print() with green/bold _rich_success() to mirror Claude adapter pattern. Covers both stdio and streamable-http registration paths. 3. Empty-URL guard: reject remote entries whose url is empty / whitespace with an explicit 'has an empty url' warning, instead of falling through to a misleading 'no scheme' urlparse warning. 4. Hybrid (remotes + packages) precedence: add a code comment + _log.debug line documenting that Codex prefers the stdio package when a registry server publishes both. Behavior unchanged; the existing test_configure_mcp_server_hybrid_accepted test already asserts it. 5. CHANGELOG: add a [Unreleased] / Fixed bullet describing Codex streamable-http MCP support, with refs (#1260, #1262). 6. Docs sync: audited docs/src/content/docs/. No stale 'Codex does not support remote MCP' claim remained; README L155 was already removed in commit 4d7b017. No docs edit required. New tests in tests/unit/test_mcp_client_factory.py: - test_format_server_config_streamable_http_rejects_empty_url (covers '', ' ', None via subTest) - test_configure_mcp_server_streamable_http_emits_rich_success - test_configure_mcp_server_stdio_emits_rich_success - test_configure_mcp_server_hybrid_logs_precedence Verification: uv run --extra dev ruff check src/ tests/ # silent uv run --extra dev ruff format --check src/ tests/ # silent uv run --extra dev pytest tests/unit/test_mcp_client_factory.py tests/test_codex_empty_string_and_defaults.py tests/test_codex_docker_args_fix.py tests/unit/test_codex_runtime.py -> 45 passed, 6 subtests passed Closes follow-ups raised in https://github.com/microsoft/apm/pull/1262#issuecomment-4487961335 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 1 + src/apm_cli/adapters/client/codex.py | 27 ++++++- tests/unit/test_mcp_client_factory.py | 100 ++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5cc53e42..35c6f8d51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Codex CLI now supports remote MCP servers using the `streamable-http` transport. Previously the Codex adapter rejected ALL remote MCP entries with "SSE not supported" even when the registry or manifest declared `transport: streamable-http`, leaving Codex users without any remote MCP access. The adapter now writes `url` + optional `http_headers` to `[mcp_servers.NAME]` for `streamable-http` remotes; `sse` remotes are still rejected (deprecated by the MCP spec) with a fix-it pointing at `transport: streamable-http`; non-`https://` URLs and empty URLs are rejected with clear warnings to prevent cleartext header leakage and silent misconfiguration. Hybrid registry servers (`remotes` + `packages`) continue to prefer the stdio package on Codex, with a debug log documenting the precedence. (#1260, #1262) - MCP server installation now respects the `targets:` whitelist exactly like `apm install`: drop a non-listed runtime even when its `.cursor/`, `.codex/`, or other on-disk signal exists. Previously the MCP install path called `active_targets()` reading the singular `target:` key only, so projects whitelisting `targets: [copilot]` could still receive `~/.codex/config.toml` and `.cursor/mcp.json` writes from foreign signals. The fix audits both paths: (a) the call site at `local_bundle_handler.py` now forwards the canonical plural list; (b) the gate now delegates to the same `resolve_targets` resolver that backs `apm install` skills, so a malformed `targets:` field (conflicting `target:` + `targets:`, `targets: []`, or unknown target name) fails closed with the same `[x]` red error voice + remediation block. The same delegation closes a related asymmetry: a greenfield project (no `targets:`, no `--target` flag, no detected signals) used to silently fall back to `[copilot]` for MCP-only invocations, while `apm install` raised `NoHarnessError` on the same input -- both surfaces now error consistently. Drop lines now use the `[i] Skipped MCP config for X (active targets: Y)` format mirroring the canonical `Targets: X (source: Y)` provenance line. The `-g`/`--global` carve-out is unchanged: `apm install -g --mcp NAME` writes to user-scope (`~/.config/...`, `~/.codex/`, etc.) bypassing the project-scope gate by design. (#1335) - Gemini CLI: `apm install -g --mcp NAME` now correctly writes to `~/.gemini/settings.json` (user scope) and `apm install` from outside the target project writes to `/.gemini/settings.json` instead of `cwd`. Previously `--global` had no effect on Gemini and project-scope writes silently landed in the wrong directory. The matching opt-in gate and cleanup paths in `MCPIntegrator` are aligned in the same change. (#1299) - `apm install --target claude` now preserves self-defined stdio MCP `env` values from `apm.yml` and writes non-string values such as `PORT: 3000` and `DEBUG: false` as MCP-compatible strings. (#1222) diff --git a/src/apm_cli/adapters/client/codex.py b/src/apm_cli/adapters/client/codex.py index 402639292..1c84e3ece 100644 --- a/src/apm_cli/adapters/client/codex.py +++ b/src/apm_cli/adapters/client/codex.py @@ -9,7 +9,7 @@ from ...registry.client import SimpleRegistryClient from ...registry.integration import RegistryIntegration -from ...utils.console import _rich_warning +from ...utils.console import _rich_success, _rich_warning from .base import MCPClientAdapter _log = logging.getLogger(__name__) @@ -181,7 +181,10 @@ def configure_mcp_server( if not self.update_config({config_key: server_config}): return False - print(f"Successfully configured MCP server '{config_key}' for Codex CLI") + _rich_success( + f"Configured MCP server '{config_key}' for Codex CLI", + symbol="success", + ) return True except Exception as e: @@ -217,7 +220,11 @@ 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 - # Remote MCP: SSE is unsupported by Codex -- return None to skip. + # Remote MCP handling. + # Precedence on Codex when a server publishes BOTH a remote and a stdio + # package: prefer the stdio package (falls through to the packages branch + # below). The remote-only branch here handles the streamable-http path + # and rejects SSE / non-https / empty-url remotes with explicit warnings. remotes = server_info.get("remotes", []) packages = server_info.get("packages", []) if remotes and not packages: @@ -233,6 +240,13 @@ def _format_server_config(self, server_info, env_overrides=None, runtime_vars=No return None remote_url = (remote.get("url") or "").strip() + if not remote_url: + _rich_warning( + f"Skipping MCP server '{server_name}' for Codex CLI: remote entry " + "has an empty url. Set `url:` to the server's streamable-http endpoint.", + symbol="warning", + ) + return None scheme = urlparse(remote_url).scheme.lower() if scheme != "https": @@ -270,6 +284,13 @@ def _format_server_config(self, server_info, env_overrides=None, runtime_vars=No ) if packages: + if remotes: + # Hybrid registry server: log that Codex prefers the stdio package + # over the remote endpoint so the precedence is auditable. + _log.debug( + "Codex hybrid server '%s': preferring stdio package over remote endpoint", + server_info.get("name", "unknown"), + ) # Use the first package for configuration (prioritize npm, then docker, then others) package = self._select_best_package(packages) diff --git a/tests/unit/test_mcp_client_factory.py b/tests/unit/test_mcp_client_factory.py index 54173057d..a9488c132 100644 --- a/tests/unit/test_mcp_client_factory.py +++ b/tests/unit/test_mcp_client_factory.py @@ -402,6 +402,106 @@ def test_configure_mcp_server_streamable_http_writes_toml_entry(self, mock_find_ self.assertEqual(figma["id"], "ab12cd34-0000-0000-0000-000000000000") self.assertEqual(figma["http_headers"], {"Authorization": "Bearer ghp_xxx"}) + @patch("apm_cli.adapters.client.codex._rich_warning") + def test_format_server_config_streamable_http_rejects_empty_url(self, mock_warn): + """Empty / whitespace-only remote URLs are rejected with a clear message.""" + for empty_value in ("", " ", None): + with self.subTest(url=empty_value): + mock_warn.reset_mock() + server_info = { + "name": "broken-remote", + "id": "broken-id", + "remotes": [ + { + "url": empty_value, + "transport_type": "streamable-http", + } + ], + } + result = self.adapter._format_server_config(server_info) + self.assertIsNone(result) + mock_warn.assert_called_once() + msg = mock_warn.call_args[0][0] + self.assertIn("broken-remote", msg) + # Message must explicitly mention that the URL is empty/missing + # rather than the misleading "no scheme" wording urlparse would + # produce for an empty string. + self.assertTrue( + "empty" in msg.lower() or "missing" in msg.lower() or "no url" in msg.lower(), + f"Expected empty/missing URL wording; got: {msg!r}", + ) + + @patch("apm_cli.adapters.client.codex._rich_success") + @patch("apm_cli.registry.client.SimpleRegistryClient.find_server_by_reference") + def test_configure_mcp_server_streamable_http_emits_rich_success( + self, mock_find_server, mock_success + ): + """Successful streamable-HTTP registration emits a green _rich_success line.""" + mock_find_server.return_value = { + "name": "figma", + "id": "ab12cd34-0000-0000-0000-000000000000", + "remotes": [ + { + "url": "https://mcp.figma.com/mcp", + "transport_type": "streamable-http", + } + ], + } + result = self.adapter.configure_mcp_server("figma/figma") + self.assertTrue(result) + mock_success.assert_called_once() + msg = mock_success.call_args[0][0] + self.assertIn("figma", msg) + self.assertIn("Codex CLI", msg) + + @patch("apm_cli.adapters.client.codex._rich_success") + @patch("apm_cli.registry.client.SimpleRegistryClient.find_server_by_reference") + def test_configure_mcp_server_stdio_emits_rich_success(self, mock_find_server, mock_success): + """stdio registrations also emit _rich_success (not bare print).""" + mock_find_server.return_value = { + "id": "test-id", + "name": "test-server", + "packages": [ + { + "registry_name": "npm", + "name": "test-package", + "version": "1.0.0", + "arguments": [], + } + ], + "environment_variables": [], + } + result = self.adapter.configure_mcp_server("test-server", "my_server") + self.assertTrue(result) + mock_success.assert_called_once() + + @patch("apm_cli.adapters.client.codex._log") + @patch("apm_cli.registry.client.SimpleRegistryClient.find_server_by_reference") + def test_configure_mcp_server_hybrid_logs_precedence(self, mock_find_server, mock_log): + """Hybrid servers (remotes + packages) log that packages win for Codex.""" + mock_find_server.return_value = { + "id": "hybrid-server-id", + "name": "hybrid-server", + "remotes": [{"transport_type": "streamable-http", "url": "https://example.com/mcp"}], + "packages": [ + { + "registry_name": "npm", + "name": "hybrid-package", + "version": "1.0.0", + "arguments": [], + } + ], + "environment_variables": [], + } + result = self.adapter.configure_mcp_server("hybrid-server", "hybrid") + self.assertTrue(result) + # At least one debug call must mention the precedence decision. + debug_messages = [str(call) for call in mock_log.debug.call_args_list] + self.assertTrue( + any("hybrid" in m and "package" in m.lower() for m in debug_messages), + f"Expected a debug log about packages-win precedence; got: {debug_messages}", + ) + if __name__ == "__main__": unittest.main()