APM Review Panel — PR #656

[+] Verdict: APPROVE — merge as-is, file follow-ups

A tight, focused fix for a real first-impression footgun: installing the official Atlassian MCP server (com.atlassian/atlassian-mcp-server) into VS Code returned "Unable to configure server", while the same package worked fine in Copilot. Root cause was the VS Code adapter requiring an explicit transport_type while the registry returns remotes without one. Same package, different runtime, different result — exactly the class of papercut that erodes trust in a package manager.

What's excellent

• Symptom-first: the bug story (Atlassian MCP server fails silently in VS Code) → the fix (default to http, matching Copilot) is clean and reads in one pass.
• 8 new tests cover every transport_type variant: missing, empty, None, whitespace, unsupported (raises), URL-less remotes (skipped), header normalization on the default-http path. This is the kind of coverage that locks in the contract.
• New _select_remote_with_url helper kills the implicit remotes[0] assumption that caused the silent-empty-config behavior — fail-closed on bad remotes is strictly better than silently producing a broken config.
• CHANGELOG entry present, scoped, and accurate.

Findings

1. [i] ValueError for unrecognized transport — discussed and accepted as-is. Architect raised a forward-compat concern: when a future websocket/grpc transport lands in the registry, this path now hard-crashes instead of degrading gracefully. We verified the new ValueError follows the file's existing convention at vscode.py:187-189 (except ValueError: raise with comment "Re-raise ValueError for registry errors"). Failing loud is the right default for an unknown transport — the old silent-empty-config behavior was the actual bug. We'll revisit if/when a real-world unsupported transport surfaces. Not blocking.

2. [i] ValueError message — minor polish opportunity. The text names the offending transport, the server, and the supported list:

   Unsupported remote transport 'grpc' for VS Code. Server: future-srv. Supported transports: http, sse, streamable-http.
   Both cli-logging and devx flagged that it could include a "what to do" hint (e.g., "Check the server's registry entry or file an issue with the server maintainer / open an issue at github.com/microsoft/apm/issues"). Optional same-PR add if you want to land it in one shot; otherwise a doc/UX sweep follow-up is fine.

3. [i] verbose_detail log on default-to-http fallback — small ergonomic win. When transport_type is empty/missing and the adapter defaults to http, no log fires. A --verbose-only line ("Defaulted transport_type to 'http' for server 'X' (registry field missing)") would help diagnose malformed registry entries fast. Optional.

4. [i] Test gap: all-URL-less remotes. Architect noted there's no test for the case where every remote in the list has an empty/missing URL — _select_remote_with_url returns None, control falls through to if not server_config: and surfaces "Unable to configure server" gracefully. The behavior is correct, but a one-line test would lock it in. Optional.

5. [i] CHANGELOG entry doesn't include the user-visible symptom. Anyone Googling "Unable to configure server" to find this fix won't land here. Consider appending "...previously caused 'Unable to configure server' errors." Optional.

Cross-runtime symmetry — file as follow-up

The DevX agent surfaced an interesting asymmetry that's not introduced by this PR but is exposed by the fix: the Copilot adapter has the inverse bug — it hardcodes "type": "http" and performs no validation at all. It would happily accept "grpc" from the registry and write a garbage config. With #656 merged, VS Code is now correctly stricter than Copilot. This deserves a follow-up issue, ideally tagged good first issue since #656 is a perfect reference implementation:

Symmetric transport validation in Copilot adapter — apm_cli/adapters/client/copilot.py:188-196 accepts any transport_type value (or none) and silently writes it. Mirror PR #656's defaulting-and-raising logic so both adapters share the same contract.

Two more low-priority follow-ups worth tracking:

• De-duplicate _select_remote_with_url — same remotes[0] selection pattern lives in both adapters; promote to MCPClientAdapter or a free function (architect nit).
• Cleartext URL warning at install time — broader hardening; the registry can serve http:// URLs and they pass through unchanged. Out of scope here; file separately if desired (security agent).

Merge call

Ship it. The bug is real and reported by users in the wild, the fix is tight, the tests are thorough, and the contested ValueError follows the file's existing convention. The forward-compat concern is principled future-proofing, not a regression. Sergio has the throughput and the receipts (#638, #656 in flight) — keeping merge latency low on clean fixes is the right call. The remaining signal lives entirely in follow-up issues.

Growth angle

• Lead the release notes with the Atlassian story — apm install com.atlassian/atlassian-mcp-server for VS Code is a recognizable, "I actually wanted this" name. Reframes the changelog from "fixed an adapter bug" to "APM now installs the MCP servers you actually use, in the runtime you actually use."
• Cross-runtime parity is becoming a positioning thread. Together with fix(auth): thread dep_ref.port into credential resolution (#785) #788 (auth port-threading parity across SSH/HTTPS), this is the second PR in a row landing "same APM command, same result, every runtime." Worth amplifying as a wedge against per-runtime configuration drift once a third parity fix lands.
• Contributor funnel: filing the Copilot inverse bug as good first issue with fix: VS Code adapter fails to configure HTTP/SSE remote MCP servers (#654) #656 as the reference implementation is exactly the loop that grows a contributor base — every fix shipped becomes the template for the symmetric one.

Thanks @sergio-sisternes-epam — clean, well-tested, scoped fix with a real user-impact story behind it.

_{Reviewed via the apm-review-panel skill: python-architect, cli-logging-expert, devx-ux-expert, supply-chain-security-expert; synthesized by apm-ceo with oss-growth-hacker side-channel.}