command_line/mcp/: server-install pipeline implemented 4x — env/arg prompting triplicated, mcp_servers.json persistence quadruplicated, two of the copies are dead code

Start by tracing the live entry points in install_command.py, catalog_server_installer.py, custom_server_form.py, remove_command.py, and edit_command.py, then compare the dead paths in wizard_utils.py and custom_server_installer.py with their tests. Define the shared config-store, prompting, and installation boundaries before changing callers. Done means dead implementations are removed, active paths share the extracted flows, and required-argument behavior plus mcp_servers.json persistence are consistent.

Summary

There are four parallel implementations of the "collect server config → build ServerConfig → register with manager → persist to mcp_servers.json → offer agent binding" pipeline under command_line/mcp/, plus duplicated example constants. Two of the copies appear to be dead code reachable only from tests.

Clone 1: env-var/cmd-arg prompting loop (56-line exact clone, triplicated semantically)

• install_command.py:145-201 (InstallCommand._install_from_catalog)
• wizard_utils.py:46-100 (run_interactive_install_wizard)
• catalog_server_installer.py:78-130 (prompt_for_server_config) — same loop, third dialect: safe_input instead of emit_prompt, adds ENV_VAR_HINTS and persists values via set_config_value

All three do: prompt for custom name → check find_server_id_by_name → "Override? [y/N]" → loop get_environment_vars() with "Already set" detection → loop get_command_line_args() building arg_prompt with [default] / (optional) suffixes.

Divergence evidence: catalog_server_installer.py:122-126 rejects a missing required arg (emit_warning(...); return None), while install_command.py:189-200 and wizard_utils.py:88-99 silently drop it — so a required argument can be skipped or not depending on which entry point installed the server. catalog_server_installer saves env values to config (set_config_value(var, value), line 96-98); the other two don't.

Clone 2: mcp_servers.json persistence block (quadruplicated)

The read-modify-write block (if os.path.exists(MCP_SERVERS_FILE): json.load ... else: {"mcp_servers": {}} ... save_config["type"] = ... os.makedirs ... json.dump(data, f, indent=2)) appears in:

• wizard_utils.py:299-316 (install_server_from_catalog)
• custom_server_installer.py:171-187
• custom_server_form.py:425-450 (adds rename-handling)
• plus read-side reimplementations in remove_command.py:74-88 and edit_command.py:96-130

Besides DRY, this is a robustness problem: four scattered non-atomic writers of the same JSON file, each silently re-deriving the schema (data.get("mcp_servers", {})).

Clone 3: custom-server install flow duplicated, one copy dead

• custom_server_form.py:31-58 ↔ custom_server_installer.py:13-40: CUSTOM_SERVER_EXAMPLES dict is byte-identical in both files.
• custom_server_installer.prompt_and_install_custom_server (line 43) reimplements the whole validate → ServerConfig → register → persist → prompt_bind_after_install_sync flow that custom_server_form._install_server (line 343) also implements. prompt_and_install_custom_server has no production callers — only tests/command_line/mcp/test_custom_server_installer.py imports it (rg -n "prompt_and_install_custom_server" code_puppy/ → definition only). The menu path uses run_custom_server_form (install_menu.py:675).

Clone 4: dead wizard

wizard_utils.run_interactive_install_wizard (line 18), interactive_server_selection (116), interactive_get_server_name (172), and interactive_configure_server (187) have zero production callers — only install_server_from_catalog (243) is imported (by install_command.py:69 and catalog_server_installer.py:154). The 56-line clone with install_command.py exists because the live code was forked out of the wizard and the wizard was never deleted.

Proposed extraction

1. Delete dead code: run_interactive_install_wizard + its three interactive_* helpers in wizard_utils.py, and custom_server_installer.py entirely (move CUSTOM_SERVER_EXAMPLES to one home, e.g. a mcp/examples.py or keep in custom_server_form.py). Retarget/delete the orphaned tests.
2. mcp/config_store.py (or extend mcp/utils.py):

   def load_servers() -> dict: ...
   def save_server(name: str, server_type: str, config: dict, *, replace_name: str | None = None) -> None: ...
   def remove_server(name: str) -> None: ...
   

   Single atomic writer used by wizard_utils, custom_server_form, remove_command, edit_command.
3. mcp/prompting.py: one collect_server_inputs(server, *, input_fn) -> ServerInputs | None implementing name/override/env-var/cmd-arg collection once, with one documented policy for missing required args; install_command and catalog_server_installer both call it (the emit_prompt vs safe_input difference is an injectable input_fn).
4. One install_server(manager, name, server_type, config_dict, group_id) that owns register → persist → success message → bind prompt, used by both catalog and custom paths.

Net effect: ~250-300 duplicated lines removed from mcp/, one persistence path for mcp_servers.json, consistent required-arg behavior.

Related: #414 (executor boilerplate in command handlers), #409/#436 (file size caps).

Filed by Zen Reviewer B (code-puppy-60635a) — DRY review round