Repository navigation
fix(install): adopt matching MCP entries when regenerating the lockfile from scratch - #3091
Open
Sebastian Alpers (salpers) wants to merge 2 commits into
Open
Sebastian Alpers (salpers) wants to merge 2 commits into
Sebastian Alpers (salpers) wants to merge 2 commits into
Conversation
Sebastian Alpers (salpers)
requested review from
Daniel Meppiel (danielmeppiel) and
Sergio Sisternes (sergio-sisternes-epam)
as code owners
September 28, 2026 07:37
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Update-path handling and fallback ownership issues remain, with related test and documentation gaps.
Review effort: Lite
Findings: 1
Open (3)
What changed in this PR
Fixes MCP ownership loss when regenerating lockfiles, including Claude Code adoption.
Changes:
- Enables adoption without a prior lockfile.
- Uses current MCP configs as the fallback baseline.
- Normalizes Claude Code entries for exact matching.
- Documents the fix in the changelog.
| File | Summary |
|---|---|
src/apm_cli/install/mcp/integration.py |
Adds fallback MCP adoption handling. |
src/apm_cli/commands/install.py |
Corrects ownership state for missing lockfiles. |
src/apm_cli/adapters/client/claude.py |
Matches Claude’s on-disk configuration shape. |
CHANGELOG.md |
Documents the MCP ownership fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Sebastian Alpers (salpers)
force-pushed
the
fix/fresh-lock-mcp-adoption
branch
3 times, most recently
from
September 30, 2026 18:56
1b80d04 to
fbf7476
Compare
…le from scratch
Without a lockfile, install treated MCP ownership as already recorded and
skipped adoption, so self-defined servers whose native configs were already
committed were logged as already configured and recorded for no runtime.
The regenerated lock then wrote mcp_target_servers: {}, which blocked
adoption on every later run.
Default ownership to absent when there is no lockfile, fall back to the
current resolved configs as the exact-match adoption baseline, and have the
Claude adapter render its normalised on-disk shape so .mcp.json entries can
be adopted at all.
Refs microsoft#3090
Review follow-ups on the fresh-lockfile adoption path. A lock that lists mcp_servers but carries no mcp_configs fell back to the current configs for baselines yet kept only the old names as candidates, so a current self-defined server present natively was never adopted. Candidates are now the union of both. Also cover the two contracts the fix rests on: * Claude's render_server_config equals the entry configure_mcp_server writes to .mcp.json, for stdio and remote shapes, so exact-match adoption cannot silently break on an adapter change. * A lockfile-wipe install (no ownership recorded) restores per-runtime mcp_target_servers for an exact native match and leaves a user-edited entry unowned.
Sebastian Alpers (salpers)
force-pushed
the
fix/fresh-lock-mcp-adoption
branch
from
September 30, 2026 20:46
fbf7476 to
b86a3b4
Compare
Mohammed Alkindi (MohammedAlkindi)
left a comment
Contributor
There was a problem hiding this comment.
The apm install --mcp path has the same trap. install/mcp/command.py still sets target_servers_present to True when there is no lockfile and only calls resolve_mcp_target_servers when one exists. On this branch, with acme-local committed in .mcp.json and .github/mcp.json: rm apm.lock.yaml && apm install --mcp acme-local -- node srv.js writes mcp_target_servers: {}, and a following apm install --force keeps it empty. Plain apm install --force after the same wipe restores both owners.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Regenerating
apm.lock.yamlfrom scratch, over native MCP configs that are already committed, loses MCP ownership. The self-defined server is logged as(already configured), nokind: uri / target: mcpdeployments are recorded, and the lock getsmcp_target_servers: {}, which blocks adoption on every later run. Full analysis and repro are in #3090.Three small changes:
commands/install.py: defaultold_mcp_target_servers_presenttoFalsewhen there is no lockfile. Previously it stayedTrue, soresolve_mcp_target_serversreturned early and adoption never ran.install/mcp/integration.py: when the old lock has nomcp_configs, use the current resolved configs as the adoption baseline. Adoption stays exact-match only, so user-authored entries that differ are never claimed.adapters/client/claude.py:render_server_configreturns the shape normalised by_normalize_mcp_entry_for_claude_code(type: stdio, notools/id), which is what the adapter actually writes to.mcp.json. Without this, exact-match adoption can never match a Claude Code entry. That also affects the existing legacy-lockfile adoption path (Adopt legacy lockfiles predating mcp_target_servers on upgrade #2158).Issue and approved scope
Issue: #3090
Human scope-approval comment: pending. Opened alongside the issue; happy to adjust or hold for scope approval.
Does this PR complete the issue, or what remains? Completes it.
Type of change
Testing
On a real repo (four targets, one self-defined stdio server declared in a local sub-package, all four native configs committed):
rm apm.lock.yaml && apm install --forcemcp_target_servers: {}generated_at/apm_versionapm install --forceapm audit --ciI haven't run the full test suite locally; CI will. If you want the change in, I'll add unit coverage for the no-lockfile adoption path and the Claude render shape.
Spec conformance (OpenAPM v0.1)