Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

<<<<<<< fix-copilot-transport-validation-791
- VS Code adapter now defaults to `http` transport when `transport_type` is missing from remote registry data, matching Copilot adapter behavior (#654)
- Copilot adapter now validates remote `transport_type`, defaulting to `http` when missing/empty/whitespace and raising a clear `ValueError` for unrecognized transports, mirroring the VS Code adapter so both refuse bad registry data instead of silently writing garbage config (#791)
- `apm install` no longer silently drops skills, agents, and commands when a Claude Code plugin also ships `hooks/*.json`. The package-type detection cascade now classifies plugin-shaped packages as `MARKETPLACE_PLUGIN` (which already maps hooks via the plugin synthesizer) before falling back to the hook-only classification, and emits a default-visibility `[!]` warning when a hook-only classification disagrees with the package's directory contents (#780)
- Preserve custom git ports across protocols: non-default ports on `ssh://` and `https://` dependency URLs (e.g. Bitbucket Datacenter on SSH port 7999, self-hosted GitLab on HTTPS port 8443) are now captured as a first-class `port` field on `DependencyReference` and threaded through all clone URL builders. When the SSH clone fails, the HTTPS fallback reuses the same port instead of silently dropping it (#661, #731)
- Detect port-like first path segment in SCP shorthand (`git@host:7999/path`) and raise an actionable error suggesting the `ssh://` URL form, instead of silently misparsing the port as part of the repository path (#784)
- `apm install --global` now installs MCP servers to global-capable runtimes (Copilot CLI, Codex CLI) instead of blanket-skipping all MCP installation at user scope. Note: lockfile-path behavior at `--global` tracked in #794 (#638)
- `--trust-transitive-mcp` no longer silently ignored when combined with `--global` (#638)
- Token resolution now discriminates by port, fixing credential collisions across multiple self-hosted Git instances on the same host. Thanks @edenfunf! (#785)
=======
- `apm install` surfaces the custom port in clone / `ls-remote` error messages for generic git hosts. (#804)

## [0.9.1] - 2026-04-22
Expand Down Expand Up @@ -119,6 +129,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- `MCP_REGISTRY_URL` validated at startup (schemeless / unsupported schemes rejected; `http://` rejected by default, opt in via `MCP_REGISTRY_ALLOW_HTTP=1`); APM fails closed when a custom registry is unreachable during install pre-flight, instead of silently approving every MCP dep. Default registry keeps assume-valid for transient errors. (#814)
- `apm install --mcp` defense-in-depth: rejects embedded `..` in dep names with a valid positive example, redacts URL credentials in diagnostic output (`https://user:token@host/` -> `https://host/`), warns on `--registry` / `MCP_REGISTRY_URL` pointing at loopback / link-local / RFC1918 / cloud-metadata hosts (including decimal-encoded loopback). (#810)
- `SimpleRegistryClient` applies a `(connect=10s, read=30s)` timeout on every registry HTTP call, removing the unbounded-hang failure mode. Tunable via `MCP_REGISTRY_CONNECT_TIMEOUT` / `MCP_REGISTRY_READ_TIMEOUT`. (#810)
>>>>>>> main

## [0.8.12] - 2026-04-19

Expand Down
47 changes: 41 additions & 6 deletions src/apm_cli/adapters/client/copilot.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,17 +186,36 @@ def _format_server_config(self, server_info, env_overrides=None, runtime_vars=No
# Check for remote endpoints first (registry-defined priority)
remotes = server_info.get("remotes", [])
if remotes:
# Use remote endpoint if available
remote = remotes[0] # Take the first remote

# All remote servers use http type for proper authentication support
# Select the first remote with a non-empty URL; fall back to the
# first entry so downstream code still emits the historical empty
# URL error path when no remote is usable.
remote = self._select_remote_with_url(remotes) or remotes[0]

# Validate transport_type from registry: default to "http" when
# missing/empty, raise ValueError for unsupported values. Mirrors
# the VS Code adapter check introduced in PR #656 so registry data
# with, e.g. transport_type="grpc" fails loudly instead of silently
# producing a garbage config.
transport = (remote.get("transport_type") or "").strip()
if not transport:
transport = "http"
elif transport not in ("sse", "http", "streamable-http"):
raise ValueError(
f"Unsupported remote transport '{transport}' for Copilot. "
f"Server: {server_info.get('name', 'unknown')}. "
f"Supported transports: http, sse, streamable-http."
)

# Copilot CLI writes "type": "http" for all remote servers so
# authentication flows (headers) are consistent regardless of the
# underlying transport advertised by the registry.
config = {
"type": "http",
"url": remote.get("url", ""),
"url": (remote.get("url") or "").strip(),
"tools": ["*"], # Required by Copilot CLI specification
"id": server_info.get("id", "") # Add registry UUID for conflict detection
}

# Add authentication headers for GitHub MCP server
server_name = server_info.get("name", "")
is_github_server = self._is_github_server(server_name, remote.get("url", ""))
Expand Down Expand Up @@ -649,6 +668,22 @@ def _resolve_env_placeholders(self, value, resolved_env):
"""Legacy method for backward compatibility. Use _resolve_variable_placeholders instead."""
return self._resolve_variable_placeholders(value, resolved_env, {})

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

Args:
remotes (list): Candidate remote entries from the registry.

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
179 changes: 179 additions & 0 deletions tests/unit/test_copilot_adapter.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
"""Unit tests for the Copilot client adapter transport validation (issue #791)."""

import json
import os
import shutil
import tempfile
import unittest
from unittest.mock import MagicMock, patch

from apm_cli.adapters.client.copilot import CopilotClientAdapter


class TestCopilotRemoteTransportValidation(unittest.TestCase):
"""Validation of ``transport_type`` mirrors PR #656 (VS Code adapter)."""

def setUp(self):
self.temp_dir = tempfile.mkdtemp()
self.temp_path = os.path.join(self.temp_dir, "mcp-config.json")
with open(self.temp_path, "w") as f:
json.dump({"mcpServers": {}}, f)

self.mock_registry_patcher = patch(
"apm_cli.adapters.client.copilot.SimpleRegistryClient"
)
self.mock_registry_class = self.mock_registry_patcher.start()
self.mock_registry_class.return_value = MagicMock()

self.mock_integration_patcher = patch(
"apm_cli.adapters.client.copilot.RegistryIntegration"
)
self.mock_integration_class = self.mock_integration_patcher.start()
self.mock_integration_class.return_value = MagicMock()

self.get_path_patcher = patch(
"apm_cli.adapters.client.copilot.CopilotClientAdapter.get_config_path",
return_value=self.temp_path,
)
self.get_path_patcher.start()

def tearDown(self):
self.get_path_patcher.stop()
self.mock_integration_patcher.stop()
self.mock_registry_patcher.stop()
shutil.rmtree(self.temp_dir, ignore_errors=True)

def test_remote_missing_transport_type_defaults_to_http(self):
"""Remote with no transport_type produces a type=http config (issue #791)."""
adapter = CopilotClientAdapter()

server_info = {
"id": "remote-1",
"name": "atlassian-mcp-server",
"remotes": [{"url": "https://mcp.atlassian.com/v1/mcp"}],
}

config = adapter._format_server_config(server_info)

self.assertEqual(config["type"], "http")
self.assertEqual(config["url"], "https://mcp.atlassian.com/v1/mcp")

def test_remote_empty_transport_type_defaults_to_http(self):
"""Empty string transport_type is treated as missing."""
adapter = CopilotClientAdapter()

server_info = {
"id": "remote-2",
"name": "remote-srv",
"remotes": [{"transport_type": "", "url": "https://example.com/mcp"}],
}

config = adapter._format_server_config(server_info)

self.assertEqual(config["type"], "http")
self.assertEqual(config["url"], "https://example.com/mcp")

def test_remote_none_transport_type_defaults_to_http(self):
"""Null transport_type is treated as missing."""
adapter = CopilotClientAdapter()

server_info = {
"id": "remote-3",
"name": "remote-srv",
"remotes": [{"transport_type": None, "url": "https://example.com/mcp"}],
}

config = adapter._format_server_config(server_info)

self.assertEqual(config["type"], "http")

def test_remote_whitespace_transport_type_defaults_to_http(self):
"""Whitespace-only transport_type is treated as missing."""
adapter = CopilotClientAdapter()

server_info = {
"id": "remote-4",
"name": "remote-srv",
"remotes": [{"transport_type": " ", "url": "https://example.com/mcp"}],
}

config = adapter._format_server_config(server_info)

self.assertEqual(config["type"], "http")

def test_remote_unsupported_transport_raises(self):
"""Unrecognized transport_type raises ValueError with server name."""
adapter = CopilotClientAdapter()

server_info = {
"id": "remote-5",
"name": "future-srv",
"remotes": [{"transport_type": "grpc", "url": "https://example.com/mcp"}],
}

with self.assertRaises(ValueError) as ctx:
adapter._format_server_config(server_info)

message = str(ctx.exception)
self.assertIn("Unsupported remote transport", message)
self.assertIn("grpc", message)
self.assertIn("future-srv", message)
self.assertIn("Copilot", message)

def test_remote_supported_transports_do_not_raise(self):
"""'sse' and 'streamable-http' transports pass validation."""
adapter = CopilotClientAdapter()

for transport in ("http", "sse", "streamable-http"):
server_info = {
"id": f"remote-{transport}",
"name": f"srv-{transport}",
"remotes": [{"transport_type": transport, "url": "https://example.com/mcp"}],
}

config = adapter._format_server_config(server_info)
# Copilot CLI always emits type="http" for auth compatibility.
self.assertEqual(config["type"], "http")
self.assertEqual(config["url"], "https://example.com/mcp")

def test_remote_skips_entries_without_url(self):
"""Remotes with empty URLs are skipped; first usable remote wins."""
adapter = CopilotClientAdapter()

server_info = {
"id": "remote-multi",
"name": "multi-remote",
"remotes": [
{"transport_type": "http", "url": ""},
{"transport_type": "sse", "url": "https://good.example.com/sse"},
],
}

config = adapter._format_server_config(server_info)
self.assertEqual(config["url"], "https://good.example.com/sse")


class TestCopilotSelectRemoteWithUrl(unittest.TestCase):
"""Direct unit tests for the ``_select_remote_with_url`` helper."""

def test_returns_first_remote_with_url(self):
remotes = [
{"url": ""},
{"url": "https://example.com/a"},
{"url": "https://example.com/b"},
]
self.assertEqual(
CopilotClientAdapter._select_remote_with_url(remotes)["url"],
"https://example.com/a",
)

def test_returns_none_when_no_url(self):
remotes = [{"url": ""}, {"url": " "}, {"url": None}]
self.assertIsNone(CopilotClientAdapter._select_remote_with_url(remotes))

def test_handles_empty_list(self):
self.assertIsNone(CopilotClientAdapter._select_remote_with_url([]))


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