Summary
PR #656 (#656) fixed the VS Code adapter to default transport_type to http and to raise ValueError for unrecognized transports. While reviewing it, the panel surfaced an inverse bug in the Copilot adapter: it performs no validation at all on transport_type from registry remotes.
Current behavior (Copilot adapter)
src/apm_cli/adapters/client/copilot.py around lines 188-196 hardcodes the transport branch and accepts any transport_type value (or none) from the registry without validation. If a registry entry returns transport_type: "grpc" (or any future unsupported value), Copilot will silently write a garbage config rather than failing loud or defaulting to a known-good value.
Expected behavior
Mirror PR #656's pattern in the Copilot adapter:
- Default to
"http" when transport_type is missing, empty, or whitespace-only (already implicitly the case, but make it explicit and shared).
- Raise a clear
ValueError for non-empty, unrecognized transport types — same message shape as VS Code:
Unsupported remote transport '{transport}' for Copilot. Server: {name}. Supported transports: http, sse, streamable-http.
- Ideally promote the selection helper (
_select_remote_with_url) and the defaulting/validation logic to a shared location (MCPClientAdapter base, or a free function in apm_cli/adapters/client/) so all adapters stay in sync. There's a related architecture nit tracked separately for de-duping remotes[0] selection.
Why this matters
This is a cross-runtime parity fix. With #656 merged, VS Code is correctly stricter than Copilot. Same APM package, same registry data, same runtime expectation — both adapters should behave identically on bad input.
Reference implementation
PR #656 is a perfect template: tight (+144/-6, 3 files), 8 tests covering all transport_type variants, CHANGELOG entry. Copying the same approach into copilot.py should be mechanical.
Good first issue
Tagging good first issue — the scope is small, the pattern is already proven, and the test cases mirror what's already in tests/unit/test_vscode_adapter.py (test_format_server_config_remote_*).
Acceptance criteria
Surfaced by the apm-review-panel during review of #656.
Summary
PR #656 (#656) fixed the VS Code adapter to default
transport_typetohttpand to raiseValueErrorfor unrecognized transports. While reviewing it, the panel surfaced an inverse bug in the Copilot adapter: it performs no validation at all ontransport_typefrom registry remotes.Current behavior (Copilot adapter)
src/apm_cli/adapters/client/copilot.pyaround lines 188-196 hardcodes the transport branch and accepts anytransport_typevalue (or none) from the registry without validation. If a registry entry returnstransport_type: "grpc"(or any future unsupported value), Copilot will silently write a garbage config rather than failing loud or defaulting to a known-good value.Expected behavior
Mirror PR #656's pattern in the Copilot adapter:
"http"whentransport_typeis missing, empty, or whitespace-only (already implicitly the case, but make it explicit and shared).ValueErrorfor non-empty, unrecognized transport types — same message shape as VS Code:_select_remote_with_url) and the defaulting/validation logic to a shared location (MCPClientAdapterbase, or a free function inapm_cli/adapters/client/) so all adapters stay in sync. There's a related architecture nit tracked separately for de-dupingremotes[0]selection.Why this matters
This is a cross-runtime parity fix. With #656 merged, VS Code is correctly stricter than Copilot. Same APM package, same registry data, same runtime expectation — both adapters should behave identically on bad input.
Reference implementation
PR #656 is a perfect template: tight (+144/-6, 3 files), 8 tests covering all
transport_typevariants, CHANGELOG entry. Copying the same approach intocopilot.pyshould be mechanical.Good first issue
Tagging
good first issue— the scope is small, the pattern is already proven, and the test cases mirror what's already intests/unit/test_vscode_adapter.py(test_format_server_config_remote_*).Acceptance criteria
copilot.pydefaults missing/empty/whitespacetransport_typeto"http".copilot.pyraisesValueErrorwith a clear message for unrecognized non-emptytransport_type._select_remote_with_urlbehavior).tests/unit/test_vscode_adapter.pyintests/unit/test_copilot_adapter.py.Fixed.Surfaced by the apm-review-panel during review of #656.