Skip to content

Copilot adapter: add symmetric transport_type validation (mirror #656) #791

@danielmeppiel

Description

@danielmeppiel

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:

  1. Default to "http" when transport_type is missing, empty, or whitespace-only (already implicitly the case, but make it explicit and shared).
  2. 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.

  3. 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

  • copilot.py defaults missing/empty/whitespace transport_type to "http".
  • copilot.py raises ValueError with a clear message for unrecognized non-empty transport_type.
  • Copilot adapter selects the first remote with a usable URL (matching VS Code's _select_remote_with_url behavior).
  • Mirror the 8 test cases from tests/unit/test_vscode_adapter.py in tests/unit/test_copilot_adapter.py.
  • CHANGELOG entry under Fixed.
  • Bonus (optional, separate PR ok): hoist the defaulting/validation/selection logic to a shared base or helper to prevent future drift.

Surfaced by the apm-review-panel during review of #656.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions