Skip to content

Commit 5994ae7

Browse files
committed
fix: address review feedback on OpenCode integration
- Break long ternary in opencode.py into if/else block - Use client manager's _get_empty_config() in _create_basic_config (fixes hardcoded 'mcpServers' key when creating OpenCode config with -e flag) - Remove dead elif branch in _get_current_client_mcpm_state (unreachable after command=='mcpm' already matched) - Handle array command format in mcpm_servers detection block (same array normalization as _get_current_client_mcpm_state)
1 parent 178c0d2 commit 5994ae7

2 files changed

Lines changed: 40 additions & 25 deletions

File tree

src/mcpm/clients/managers/opencode.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,10 @@ def to_client_format(self, server_config: ServerConfig) -> Dict[str, Any]:
109109
entry["headers"] = server_config.headers
110110

111111
else:
112-
# CustomServerConfig — pass through raw config
113-
entry = server_config.config if isinstance(server_config, CustomServerConfig) else server_config.to_dict()
112+
if isinstance(server_config, CustomServerConfig):
113+
entry = server_config.config
114+
else:
115+
entry = server_config.to_dict()
114116

115117
return entry
116118

src/mcpm/commands/client.py

Lines changed: 36 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,18 @@ def list_clients(verbose):
226226
@click.option("--remove-profile", help="Comma-separated list of profile names to remove")
227227
@click.option("--set-profiles", help="Comma-separated list of profile names to set (replaces all)")
228228
@click.option("--force", is_flag=True, help="Skip confirmation prompts")
229-
def edit_client(client_name, external, config_path_override, add_server, remove_server, set_servers, add_profile, remove_profile, set_profiles, force):
229+
def edit_client(
230+
client_name,
231+
external,
232+
config_path_override,
233+
add_server,
234+
remove_server,
235+
set_servers,
236+
add_profile,
237+
remove_profile,
238+
set_profiles,
239+
force,
240+
):
230241
"""Enable/disable MCPM-managed servers in the specified client configuration.
231242
232243
Interactive by default, or use CLI parameters for automation.
@@ -286,7 +297,7 @@ def edit_client(client_name, external, config_path_override, add_server, remove_
286297
# Ensure config file exists before opening
287298
if not config_exists:
288299
console.print("[yellow]Config file does not exist. Creating basic config...[/]")
289-
_create_basic_config(config_path)
300+
_create_basic_config(config_path, client_manager)
290301

291302
_open_in_editor(config_path, display_name)
292303
return
@@ -300,12 +311,16 @@ def edit_client(client_name, external, config_path_override, add_server, remove_
300311
for client_server_name, server_config in mcp_servers.items():
301312
if not isinstance(server_config, dict):
302313
continue
303-
command = server_config.get("command", "")
304-
args = server_config.get("args", [])
314+
raw_cmd = server_config.get("command", "")
315+
if isinstance(raw_cmd, list):
316+
command = raw_cmd[0] if raw_cmd else ""
317+
args = raw_cmd[1:] if len(raw_cmd) > 1 else []
318+
else:
319+
command = raw_cmd
320+
args = server_config.get("args", [])
305321

306322
if client_server_name.startswith("mcpm_") and command == "mcpm" and len(args) >= 2 and args[0] == "run":
307-
actual_server_name = args[1]
308-
mcpm_servers.add(actual_server_name)
323+
mcpm_servers.add(args[1])
309324

310325
# Get all MCPM global servers
311326
global_servers = global_config_manager.list_servers()
@@ -403,11 +418,6 @@ def _get_current_client_mcpm_state(client_manager, global_server_names=None):
403418
profiles.append(args[2])
404419
elif len(args) >= 2 and args[0] == "run":
405420
individual_servers.append(args[1])
406-
elif server_name.startswith("mcpm_") and command == "mcpm":
407-
if len(args) >= 3 and args[0] == "profile" and args[1] == "run":
408-
profiles.append(args[2])
409-
elif len(args) >= 2 and args[0] == "run":
410-
individual_servers.append(args[1])
411421
elif server_name in global_server_names:
412422
individual_servers.append(server_name)
413423
except Exception:
@@ -728,9 +738,13 @@ def _save_config_with_mcpm_servers(client_manager, config_path, current_config,
728738
print_error("Error saving configuration", str(e))
729739

730740

731-
def _create_basic_config(config_path):
741+
def _create_basic_config(config_path, client_manager=None):
732742
"""Create a basic MCP client config file."""
733-
basic_config = {"mcpServers": {}}
743+
if client_manager and hasattr(client_manager, "_get_empty_config"):
744+
basic_config = client_manager._get_empty_config()
745+
else:
746+
configure_key = getattr(client_manager, "configure_key_name", "mcpServers") if client_manager else "mcpServers"
747+
basic_config = {configure_key: {}}
734748

735749
# Create the directory if it doesn't exist
736750
os.makedirs(os.path.dirname(config_path), exist_ok=True)
@@ -1176,6 +1190,7 @@ def _edit_client_non_interactive(
11761190
return 1
11771191

11781192
from mcpm.profile.profile_config import ProfileConfigManager
1193+
11791194
profile_manager = ProfileConfigManager()
11801195
available_profiles = profile_manager.list_profiles()
11811196

@@ -1261,7 +1276,9 @@ def _edit_client_non_interactive(
12611276

12621277
# Show profile changes
12631278
if final_profiles != set(current_profiles):
1264-
console.print(f"Profiles: [dim]{len(current_profiles)} profiles[/] → [cyan]{len(final_profiles)} profiles[/]")
1279+
console.print(
1280+
f"Profiles: [dim]{len(current_profiles)} profiles[/] → [cyan]{len(final_profiles)} profiles[/]"
1281+
)
12651282

12661283
added_profiles = final_profiles - set(current_profiles)
12671284
if added_profiles:
@@ -1275,7 +1292,9 @@ def _edit_client_non_interactive(
12751292

12761293
# Show server changes
12771294
if final_servers != set(current_individual_servers):
1278-
console.print(f"Servers: [dim]{len(current_individual_servers)} servers[/] → [cyan]{len(final_servers)} servers[/]")
1295+
console.print(
1296+
f"Servers: [dim]{len(current_individual_servers)} servers[/] → [cyan]{len(final_servers)} servers[/]"
1297+
)
12791298

12801299
added_servers = final_servers - set(current_individual_servers)
12811300
if added_servers:
@@ -1310,9 +1329,7 @@ def _edit_client_non_interactive(
13101329
try:
13111330
profile_server_name = f"mcpm_profile_{profile_name}"
13121331
server_config = STDIOServerConfig(
1313-
name=profile_server_name,
1314-
command="mcpm",
1315-
args=["profile", "run", profile_name]
1332+
name=profile_server_name, command="mcpm", args=["profile", "run", profile_name]
13161333
)
13171334
client_manager.add_server(server_config)
13181335
except Exception as e:
@@ -1331,11 +1348,7 @@ def _edit_client_non_interactive(
13311348
for server_name in final_servers - set(current_individual_servers):
13321349
try:
13331350
prefixed_name = f"mcpm_{server_name}"
1334-
server_config = STDIOServerConfig(
1335-
name=prefixed_name,
1336-
command="mcpm",
1337-
args=["run", server_name]
1338-
)
1351+
server_config = STDIOServerConfig(name=prefixed_name, command="mcpm", args=["run", server_name])
13391352
client_manager.add_server(server_config)
13401353
except Exception as e:
13411354
console.print(f"[red]Error adding server {server_name}: {e}[/]")

0 commit comments

Comments
 (0)