fix(mcp): drop previous-transport keys on Claude server redeclaration

[Eden (edenfunf)]

What

A Claude Code MCP entry could describe two transports at once. When a server that APM previously wrote as remote (url, headers) is redeclared as stdio under the same name, the entry now drops the remote keys instead of keeping them, and vice versa.

Before (after http -> stdio, via a real apm install -g -t claude):

"example": {
  "type": "local",
  "url": "http://127.0.0.1:4521/mcp",
  "headers": { "Authorization": "Bearer ${TOKEN}" },
  "command": "npx",
  "args": ["-y", "example-mcp"]
}

After:

"example": {
  "type": "stdio",
  "command": "npx",
  "args": ["-y", "example-mcp"]
}

Why

_merge_mcp_server_dicts shallow-merges each entry as {**old, **new} so hand-authored keys survive an update that omits them. Transport-specific keys survived with them, which is two defects rather than one:

1. The stale Authorization header stays in ~/.claude.json after the server stops being remote, which is the reported behavior in [BUG] Claude MCP entry keeps the old url/headers when a server's transport changes from http to stdio #2994.
2. The surviving url also re-classifies the entry as remote in _normalize_mcp_entry_for_claude_code, so the stdio branch never runs and type stays at Copilot's local rather than becoming Claude Code's stdio. That second effect is not in the report; it falls out of the same root cause and is fixed by the same change.

APM's install promise is to reproduce the declared server, so the entry must describe exactly the declared transport. This is not preservation of user intent: APM wrote those keys itself in an earlier install.

How

Follows the direction in the triage advisory -- drop previous-transport keys on a transport change, keep unmanaged keys.

• _is_remote_mcp_entry lifts the remote/stdio test that was inline in _normalize_mcp_entry_for_claude_code, so the merge and the normalizer classify an entry the same way rather than each carrying its own copy of the rule.
• _retained_previous_entry returns the previous entry minus _REMOTE_TRANSPORT_KEYS or _STDIO_TRANSPORT_KEYS, and only when the transport actually changed. type is in both sets because it names the transport itself; _format_server_config always restates it (local for stdio, http for remote), so nothing is left without one.
• _merge_mcp_server_dicts merges over that retained entry. The shallow-merge contract is otherwise untouched: with no transport change the behavior is byte-identical to before.

Scope stays inside the Claude adapter. The per-entry shallow merge is unique to it -- CopilotClientAdapter.update_config replaces whole entries via dict.update, so the sibling adapters never had this defect. Cleanup, deletion gates, and registry formatting are unchanged.

Test

Live verification (apm install -g -t claude, sandboxed HOME, self-defined MCP dep), on main before the change and on this branch after:

Step	Before	After
Declare transport: http with an Authorization header	correct	correct
Redeclare same name as transport: stdio	keeps url + headers, type: "local"	type: "stdio", no url/headers
Redeclare back as transport: http	keeps command + args	type: "http", no command/args
Hand-authored oauthAccount across a transport change	preserved	preserved
Reinstall with no manifest change	unchanged	unchanged (byte-equal)

Both scopes exercised: user scope (~/.claude.json) and project scope (.mcp.json), which share _merge_and_normalize_updates.

Unit tests -- new TestClaudeTransportChange in tests/unit/test_claude_mcp.py, driven through update_config so the assertions read the file APM actually writes:

• test_remote_to_stdio_drops_url_and_headers
• test_stdio_to_remote_drops_command_args_env_cwd
• test_transport_change_preserves_unmanaged_keys
• test_unchanged_transport_still_shallow_merges -- regression guard for the OAuth-preservation contract the merge exists for

The first three fail on main and pass here; the fourth passes both ways by design.

Suites -- tests/unit + tests/test_console.py (22618 passed), tests/integration/test_wave2_adapters_coverage.py and tests/integration/test_core_smoke.py (420 passed), and the CI quality ratchets (test_check_test_assertions, test_check_exact_test_duplicates, test_test_taxonomy, test_quality_baselines, test_lifecycle_bug_ledger, test_ci_topology, 90 passed). ruff check and ruff format --check clean on both changed files. Three failures in tests/unit (test_install_safety.py::TestAbsolutePathGuard::test_accepts_unix_absolute, TestSuffixGuard::test_accepts_lib_apm_suffix, test_view_command.py::test_view_versions_bare_registry_flag_forces_registry) reproduce identically on an unmodified main checkout and are unrelated to this change.

Issue: #2994 (status/accepted, claimed in this comment).

[Copilot]

Copilot review overview

🟡 Changes recommended

Existing mixed configurations can retain stale stdio keys, and the new runtime-config behavior is not documented.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 1

Open (2)

• Clean stale transport keys in unchanged mixed entries · New
• Document transport-key cleanup for same-name redeclarations · New

What changed in this PR

Fixes Claude MCP transport redeclarations so stale transport-specific fields are removed.

Changes:

• Adds transport-aware merge cleanup and shared classification.
• Adds regression tests for transport switches and key preservation.
• Records the fix in the changelog.

File	Description
src/​apm_cli/​adapters/​client/​claude.py	Implements transport-aware merging and normalization.
tests/​unit/​test_claude_mcp.py	Tests transport changes and shallow-merge behavior.
CHANGELOG.md	Documents the fix.

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Eden (edenfunf)]

Confirmed and fixed in 92e0c81 -- this was a real gap, thanks.

Reproduced it on the branch before changing anything: seeded ~/.claude.json with an entry a pre-fix release would leave behind (type: "http", url, plus stale command/args/env/cwd), declared the same remote server, ran apm install -g -t claude. Every stale stdio key survived, including a leftover secret in env.

Rather than add a migration branch, I removed the comparison: the stale set is now selected from the update alone.

stale = (
    cls._STDIO_TRANSPORT_KEYS
    if cls._is_remote_mcp_entry(new_cfg)
    else cls._REMOTE_TRANSPORT_KEYS
)
return {key: value for key, value in prev.items() if key not in stale}

An entry is rewritten to carry only the transport its declaration names, whatever shape it had before, so the mixed case is not a special case at all. The fast path is gone, and the unchanged-transport shallow-merge contract is unaffected -- for a stdio update the remote keys are stripped, which a stdio entry never had, so keys like cwd that the update omits still survive. Two tests cover the repair in both directions, and test_unchanged_transport_still_shallow_merges still guards the preservation contract.

One residual I want to flag rather than quietly expand scope into. The adapter repairs the entry whenever it writes it, but a user who upgrades APM and reruns apm install with an unchanged declaration never reaches the adapter: _check_self_defined_servers_needing_installation gates on the server name being present in the target config, and _detect_mcp_config_drift compares the declaration against the lockfile, not against the shape of the entry on disk. Both are shared across every target. So a mixed entry is repaired on the next install that writes that server, not by a no-op reinstall. Closing that would mean teaching a cross-cutting drift check to inspect the written entry, which is outside the scope recorded on #2994 -- happy to open a separate issue for it if you agree it is worth having.

[Eden (edenfunf)]

Agreed that the behaviour was discoverable only from the implementation -- documented in 92e0c81.

I put it in What apm install writes to disk rather than Updating and replacing a server. That latter section is about the manifest: it tabulates what re-running apm install --mcp NAME does to dependencies.mcp (appended / no-op / prompts / refuses with exit 2). The transport-cleanup rule is about the runtime config file, which is what the earlier section covers, alongside the other per-target write rules.

I also scoped the sentence to Claude Code. The preservation half of this contract is not general: _merge_mcp_server_dicts is only on the Claude adapter, while CopilotClientAdapter.update_config replaces whole entries via dict.update, so on every other target an unmanaged key does not survive a reinstall in the first place. Documenting it unscoped would have been false for the rest of the matrix.

Added text:

Claude Code is the one target whose entries are merged key by key rather than replaced, so keys APM does not manage (a hand-authored OAuth block, for example) survive a reinstall. The keys describing a transport are not among them: an entry is rewritten to carry only the transport its declaration names, so redeclaring a server from http to stdio drops the previous url and headers instead of leaving both transports on one entry.

[Sergio Sisternes (sergio-sisternes-epam)]

Thank you for this pull request. It implements the accepted scope on #2994. CODEOWNERS review is already requested of danielmeppiel and sergio-sisternes-epam; that request is unchanged. This comment is advisory only and is not merge approval. Please wait for a human maintainer to review.

---

Generated by autopilot-pr-triage-worker. This comment is AI-generated and may contain errors.