Skip to content

Commit a43cedf

Browse files
committed
fix(#47,#44): fix MCP source attribution and cross-tool broadcast
Issue #47 — Misattributed source_tool after dedup: The MCP cache dedup key was name-only, so whichever tool was collected last would overwrite earlier entries' source_tool. Changed _key_mcp() to use (source_tool, name) so each tool's servers are preserved as distinct entries with correct attribution. Issue #44 — apc mcp sync broadcasts ALL servers to ALL tools: Added _filter_mcp_for_tool() in sync_helpers.py that applies a server to a tool only when source_tool matches or the server's targets list explicitly includes the tool. sync_all() and sync_mcp() now use this filter by default. Added --all-sources / --all-sources-mcp flags to restore the old broadcast behaviour when explicitly requested. Tests updated to reflect new source-aware default behaviour and added test_collect_then_sync_claude_all_sources for the opt-in path.
1 parent 9fd0364 commit a43cedf

6 files changed

Lines changed: 590 additions & 27 deletions

File tree

src/cache.py

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,13 @@ def merge_skills(existing: List[Dict], new: List[Dict]) -> List[Dict]:
8383

8484

8585
def merge_mcp_servers(existing: List[Dict], new: List[Dict]) -> List[Dict]:
86-
"""Merge new MCP servers into existing by (source_tool, name) key. Upsert only."""
86+
"""Merge new MCP servers into existing by (source_tool, name) key. Upsert only.
87+
88+
Keying by both source_tool and name preserves source attribution when the
89+
same server name is configured in multiple tools (#47). This also ensures
90+
that each tool's servers remain distinct in the cache, enabling source-aware
91+
filtering during sync (#44).
92+
"""
8793
index = {(_key_mcp(s)): s for s in existing}
8894
for s in new:
8995
index[_key_mcp(s)] = s
@@ -127,11 +133,13 @@ def _key(e: Dict) -> str:
127133

128134

129135
def _key_mcp(s: Dict) -> str:
130-
"""Deduplicate MCP servers by name only.
136+
"""Deduplicate MCP servers by (source_tool, name).
137+
138+
Keying by name alone caused source_tool misattribution when the same
139+
server name appeared in multiple tools — whichever was collected last
140+
would overwrite the earlier entry's source_tool (#47).
131141
132-
The same logical MCP server can be collected from multiple tools
133-
(e.g. a shared server configured in both Claude and Cursor). Keying
134-
by name alone ensures we keep one canonical entry (last collected wins)
135-
instead of accumulating one entry per source tool.
142+
Keying by (source_tool, name) preserves each tool's entry separately with
143+
correct attribution, and enables source-aware filtering during sync (#44).
136144
"""
137-
return s.get("name", "")
145+
return f"{s.get('source_tool', '')}:{s.get('name', '')}"

src/main.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,15 @@ def cli():
8080
@click.option(
8181
"--override-mcp", is_flag=True, help="Replace existing MCP servers instead of merging"
8282
)
83+
@click.option(
84+
"--all-sources-mcp",
85+
"all_sources_mcp",
86+
is_flag=True,
87+
help="Broadcast ALL cached MCP servers to every tool, ignoring source_tool origin.",
88+
)
8389
@click.option("--dry-run", is_flag=True, help="Show what would be applied without writing")
8490
@click.option("--yes", "-y", is_flag=True, help="Skip confirmation prompt")
85-
def sync(tools, apply_all, no_memory, override_mcp, dry_run, yes):
91+
def sync(tools, apply_all, no_memory, override_mcp, all_sources_mcp, dry_run, yes):
8692
"""Sync local cache contents to target AI tools.
8793
8894
No login or network required.
@@ -127,7 +133,9 @@ def sync(tools, apply_all, no_memory, override_mcp, dry_run, yes):
127133
info("Cancelled.")
128134
return
129135

130-
ok = sync_all(tool_list, no_memory=no_memory, override_mcp=override_mcp)
136+
ok = sync_all(
137+
tool_list, no_memory=no_memory, override_mcp=override_mcp, all_sources_mcp=all_sources_mcp
138+
)
131139
if not ok:
132140
raise SystemExit(1)
133141

src/mcp.py

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,20 @@ def mcp_list_cmd():
2727
@click.option("--tools", default=None, help="Comma-separated list of target tools")
2828
@click.option("--all", "apply_all", is_flag=True, help="Apply to all detected tools")
2929
@click.option("--override", is_flag=True, help="Replace existing MCP servers instead of merging")
30+
@click.option(
31+
"--all-sources",
32+
"all_sources",
33+
is_flag=True,
34+
help="Sync ALL cached servers to every tool, ignoring source_tool origin.",
35+
)
3036
@click.option("--yes", "-y", is_flag=True, help="Skip confirmation prompt")
31-
def mcp_sync(tools, apply_all, override, yes):
32-
"""Sync MCP servers to target tools."""
37+
def mcp_sync(tools, apply_all, override, all_sources, yes):
38+
"""Sync MCP servers to target tools.
39+
40+
By default each server is only applied to the tool it was originally
41+
collected from (source_tool). Use --all-sources to broadcast every
42+
cached server to every target tool regardless of origin.
43+
"""
3344
header("MCP Sync")
3445

3546
tool_list = resolve_target_tools(tools, apply_all)
@@ -45,7 +56,7 @@ def mcp_sync(tools, apply_all, override, yes):
4556
click.echo("Cancelled.")
4657
return
4758

48-
sync_mcp(tool_list, override=override)
59+
sync_mcp(tool_list, override=override, all_sources=all_sources)
4960

5061

5162
@mcp.command("remove")

src/sync_helpers.py

Lines changed: 48 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,38 @@ def sync_skills(tool_list: List[str]) -> Tuple[int, int]:
118118
return total_copy, total_link
119119

120120

121-
def sync_mcp(tool_list: List[str], override: bool = False) -> int:
122-
"""Apply MCP servers from cache to tools. Returns count."""
121+
def _filter_mcp_for_tool(servers: List[Dict], tool_name: str, all_sources: bool) -> List[Dict]:
122+
"""Return only the servers that should be applied to a given tool.
123+
124+
By default (all_sources=False) we apply a server to a tool only when:
125+
- source_tool matches tool_name (server originated from this tool), OR
126+
- targets is a non-empty list that explicitly includes this tool.
127+
128+
When all_sources=True every server is applied regardless of origin —
129+
this replicates the previous (pre-#44) broadcast behaviour and can be
130+
requested via ``apc mcp sync --all-sources``.
131+
"""
132+
if all_sources:
133+
return servers
134+
135+
result = []
136+
for s in servers:
137+
source = s.get("source_tool", "")
138+
targets = s.get("targets", [])
139+
if source == tool_name:
140+
result.append(s)
141+
elif targets and tool_name in targets:
142+
result.append(s)
143+
return result
144+
145+
146+
def sync_mcp(tool_list: List[str], override: bool = False, all_sources: bool = False) -> int:
147+
"""Apply MCP servers from cache to tools. Returns count.
148+
149+
By default only syncs each server back to the tool it originated from
150+
(source_tool == tool_name) or tools explicitly listed in its targets.
151+
Pass all_sources=True to restore the old broadcast behaviour (#44).
152+
"""
123153
mcp_servers = load_mcp_servers()
124154

125155
if not mcp_servers:
@@ -135,16 +165,17 @@ def sync_mcp(tool_list: List[str], override: bool = False) -> int:
135165
"Ensure those files are excluded from version control."
136166
)
137167

138-
current_mcp_names = [s.get("name", "unnamed") for s in mcp_servers]
139168
total = 0
140169

141170
for tool_name in tool_list:
142171
try:
143172
applier = get_applier(tool_name)
144173
manifest = applier.get_manifest()
145174

146-
secrets = _resolve_all_mcp_secrets(mcp_servers)
147-
m = applier.apply_mcp_servers(mcp_servers, secrets, manifest, override=override)
175+
tool_servers = _filter_mcp_for_tool(mcp_servers, tool_name, all_sources)
176+
current_mcp_names = [s.get("name", "unnamed") for s in tool_servers]
177+
secrets = _resolve_all_mcp_secrets(tool_servers)
178+
m = applier.apply_mcp_servers(tool_servers, secrets, manifest, override=override)
148179
# Prune orphaned MCP servers (keep skill names empty — not our concern)
149180
applier.prune([], current_mcp_names, manifest)
150181
manifest.save()
@@ -184,7 +215,12 @@ def sync_memory(tool_list: List[str]) -> int:
184215
return total
185216

186217

187-
def sync_all(tool_list: List[str], no_memory: bool = False, override_mcp: bool = False) -> bool:
218+
def sync_all(
219+
tool_list: List[str],
220+
no_memory: bool = False,
221+
override_mcp: bool = False,
222+
all_sources_mcp: bool = False,
223+
) -> bool:
188224
"""Apply everything (skills + MCP + memory). Used by `apc sync`.
189225
190226
Returns True if at least one tool was synced successfully, False otherwise.
@@ -197,12 +233,11 @@ def sync_all(tool_list: List[str], no_memory: bool = False, override_mcp: bool =
197233
skills_dir = get_skills_dir()
198234
installed_skills = _discover_installed_skills()
199235

200-
# Build combined name lists for pruning
236+
# Build combined name list for pruning (skills only)
201237
all_skill_names = list(
202238
{s.get("name", "unnamed") for s in collected_skills}
203239
| {s.get("name", "unnamed") for s in installed_skills}
204240
)
205-
current_mcp_names = [s.get("name", "unnamed") for s in mcp_servers]
206241

207242
total_skills = 0
208243
total_mcp = 0
@@ -220,9 +255,11 @@ def sync_all(tool_list: List[str], no_memory: bool = False, override_mcp: bool =
220255
# Link installed skills
221256
lk = applier.link_skills(installed_skills, skills_dir, manifest)
222257

223-
# MCP servers
224-
secrets = _resolve_all_mcp_secrets(mcp_servers)
225-
m = applier.apply_mcp_servers(mcp_servers, secrets, manifest, override=override_mcp)
258+
# MCP servers — filter to this tool's servers by default (#44)
259+
tool_mcp = _filter_mcp_for_tool(mcp_servers, tool_name, all_sources_mcp)
260+
current_mcp_names = [sv.get("name", "unnamed") for sv in tool_mcp]
261+
secrets = _resolve_all_mcp_secrets(tool_mcp)
262+
m = applier.apply_mcp_servers(tool_mcp, secrets, manifest, override=override_mcp)
226263

227264
# Memory
228265
mem = 0

tests/test_docker_integration.py

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1133,12 +1133,29 @@ def test_collect_then_sync_claude(self, runner, cli):
11331133
r = runner.invoke(cli, ["sync", "--tools", "claude-code", "--yes", "--no-memory"])
11341134
assert r.exit_code == 0
11351135

1136-
# Verify MCP servers in claude.json include servers from other tools
1136+
# By default only claude-code's own servers are synced back (#44)
11371137
data = json.loads((HOME / ".claude.json").read_text())
11381138
mcp_names = set(data.get("mcpServers", {}).keys())
1139-
# Should have cursor, gemini, copilot, windsurf MCP servers synced
1140-
for expected in ["test-cursor-mcp", "test-gemini-mcp", "test-windsurf-mcp"]:
1141-
assert expected in mcp_names, f"{expected} not found in claude.json mcpServers"
1139+
assert "test-claude-mcp" in mcp_names, "claude's own server should be synced"
1140+
# Cross-tool servers are NOT synced by default (require --all-sources-mcp)
1141+
for cross_tool in ["test-cursor-mcp", "test-gemini-mcp", "test-windsurf-mcp"]:
1142+
assert cross_tool not in mcp_names, f"{cross_tool} should not be in claude.json"
1143+
1144+
def test_collect_then_sync_claude_all_sources(self, runner, cli):
1145+
"""With --all-sources-mcp, all cached MCP servers are synced to every tool."""
1146+
r = runner.invoke(cli, ["collect", "--yes"])
1147+
assert r.exit_code == 0
1148+
1149+
r = runner.invoke(
1150+
cli, ["sync", "--tools", "claude-code", "--yes", "--no-memory", "--all-sources-mcp"]
1151+
)
1152+
assert r.exit_code == 0
1153+
1154+
data = json.loads((HOME / ".claude.json").read_text())
1155+
mcp_names = set(data.get("mcpServers", {}).keys())
1156+
# All servers from all tools should be present with --all-sources-mcp
1157+
for expected in ["test-claude-mcp", "test-cursor-mcp", "test-gemini-mcp"]:
1158+
assert expected in mcp_names, f"{expected} not found with --all-sources-mcp"
11421159

11431160
def test_collect_then_sync_cursor_override(self, runner, cli):
11441161
"""Collect, sync to cursor with --override-mcp, verify only cache servers remain."""

0 commit comments

Comments
 (0)