fix: resolve test failures in edit and client commands

- Fix RemoteServerConfig env field access issue in edit command
- Update client edit tests to use add_server instead of update_servers
- Fix profile test assertions for correct output messages

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>

Author: niechen

src/mcpm/commands/edit.py

@@ -339,7 +339,6 @@ def _edit_server_non_interactive(
                 "type": "remote",
                 "url": server_config.url,
                 "headers": server_config.headers,
-                "env": server_config.env,
             }
         else:
             print_error("Unknown server type", f"Server '{server_name}' has unknown type")
@@ -421,7 +420,6 @@ def _edit_server_non_interactive(
                 name=updated_config["name"],
                 url=updated_config["url"],
                 headers=updated_config.get("headers", {}),
-                env=updated_config.get("env", {}),
                 profile_tags=server_config.profile_tags,
             )
 
@@ -738,7 +736,6 @@ def _edit_server_non_interactive(
                 "type": "remote",
                 "url": server_config.url,
                 "headers": server_config.headers,
-                "env": server_config.env,
             }
         else:
             print_error("Unknown server type", f"Server '{server_name}' has unknown type")
@@ -820,7 +817,6 @@ def _edit_server_non_interactive(
                 name=updated_config["name"],
                 url=updated_config["url"],
                 headers=updated_config.get("headers", {}),
-                env=updated_config.get("env", {}),
                 profile_tags=server_config.profile_tags,
             )
 

tests/test_client.py

@@ -470,7 +470,8 @@ def test_client_edit_non_interactive_set_servers(monkeypatch):
     mock_client_manager.is_client_installed = Mock(return_value=True)
     mock_client_manager.config_path = "/path/to/config.json"
     mock_client_manager.get_servers.return_value = {"old-server": Mock()}
-    mock_client_manager.update_servers.return_value = None
+    mock_client_manager.add_server.return_value = None
+    mock_client_manager.remove_server.return_value = None
 
     monkeypatch.setattr("mcpm.commands.client.ClientRegistry.get_client_manager", Mock(return_value=mock_client_manager))
     monkeypatch.setattr("mcpm.commands.client.ClientRegistry.get_client_info", Mock(return_value={"name": "Cursor"}))
@@ -495,7 +496,8 @@ def test_client_edit_non_interactive_set_servers(monkeypatch):
 
     assert result.exit_code == 0
     assert "Successfully updated" in result.output
-    mock_client_manager.update_servers.assert_called_once()
+    # Verify that add_server was called for the new servers
+    assert mock_client_manager.add_server.call_count == 2
 
 
 def test_client_edit_non_interactive_add_profile(monkeypatch):
@@ -505,7 +507,7 @@ def test_client_edit_non_interactive_add_profile(monkeypatch):
     mock_client_manager.is_client_installed = Mock(return_value=True)
     mock_client_manager.config_path = "/path/to/config.json"
     mock_client_manager.get_servers.return_value = {}
-    mock_client_manager.update_servers.return_value = None
+    mock_client_manager.add_server.return_value = None
 
     monkeypatch.setattr("mcpm.commands.client.ClientRegistry.get_client_manager", Mock(return_value=mock_client_manager))
     monkeypatch.setattr("mcpm.commands.client.ClientRegistry.get_client_info", Mock(return_value={"name": "Cursor"}))
@@ -519,7 +521,7 @@ def test_client_edit_non_interactive_add_profile(monkeypatch):
     mock_profile_config = Mock()
     mock_profile_config.list_profiles.return_value = {"test-profile": [Mock(name="test-server")]}
     mock_profile_config.get_profile.return_value = [Mock(name="test-server")]
-    monkeypatch.setattr("mcpm.commands.client.profile_config_manager", mock_profile_config)
+    monkeypatch.setattr("mcpm.profile.profile_config.ProfileConfigManager", lambda: mock_profile_config)
 
     # Force non-interactive mode
     monkeypatch.setattr("mcpm.commands.client.is_non_interactive", lambda: True)
@@ -532,7 +534,8 @@ def test_client_edit_non_interactive_add_profile(monkeypatch):
 
     assert result.exit_code == 0
     assert "Successfully updated" in result.output
-    mock_client_manager.update_servers.assert_called_once()
+    # Verify that add_server was called for the profile
+    assert mock_client_manager.add_server.called
 
 
 def test_client_edit_non_interactive_server_not_found(monkeypatch):
@@ -571,7 +574,7 @@ def test_client_edit_with_force_flag(monkeypatch):
     mock_client_manager.is_client_installed = Mock(return_value=True)
     mock_client_manager.config_path = "/path/to/config.json"
     mock_client_manager.get_servers.return_value = {}
-    mock_client_manager.update_servers.return_value = None
+    mock_client_manager.add_server.return_value = None
 
     monkeypatch.setattr("mcpm.commands.client.ClientRegistry.get_client_manager", Mock(return_value=mock_client_manager))
     monkeypatch.setattr("mcpm.commands.client.ClientRegistry.get_client_info", Mock(return_value={"name": "Cursor"}))
@@ -591,7 +594,8 @@ def test_client_edit_with_force_flag(monkeypatch):
 
     assert result.exit_code == 0
     assert "Successfully updated" in result.output
-    mock_client_manager.update_servers.assert_called_once()
+    # Verify add_server was called for the new server
+    assert mock_client_manager.add_server.called
 
 
 def test_client_edit_command_help():

tests/test_edit.py

@@ -146,7 +146,8 @@ def test_edit_stdio_server_non_interactive(monkeypatch):
 
     mock_global_config = Mock()
     mock_global_config.get_server.return_value = test_server
-    mock_global_config.update_server.return_value = None
+    mock_global_config.remove_server.return_value = None
+    mock_global_config.add_server.return_value = None
     monkeypatch.setattr("mcpm.commands.edit.global_config_manager", mock_global_config)
 
     # Force non-interactive mode
@@ -163,7 +164,8 @@ def test_edit_stdio_server_non_interactive(monkeypatch):
 
     assert result.exit_code == 0
     assert "Successfully updated server" in result.output
-    mock_global_config.update_server.assert_called_once()
+    mock_global_config.remove_server.assert_called_once_with("test-server")
+    mock_global_config.add_server.assert_called_once()
 
 
 def test_edit_remote_server_non_interactive(monkeypatch):
@@ -178,7 +180,8 @@ def test_edit_remote_server_non_interactive(monkeypatch):
 
     mock_global_config = Mock()
     mock_global_config.get_server.return_value = test_server
-    mock_global_config.update_server.return_value = None
+    mock_global_config.remove_server.return_value = None
+    mock_global_config.add_server.return_value = None
     monkeypatch.setattr("mcpm.commands.edit.global_config_manager", mock_global_config)
 
     # Force non-interactive mode
@@ -194,7 +197,8 @@ def test_edit_remote_server_non_interactive(monkeypatch):
 
     assert result.exit_code == 0
     assert "Successfully updated server" in result.output
-    mock_global_config.update_server.assert_called_once()
+    mock_global_config.remove_server.assert_called_once_with("api-server")
+    mock_global_config.add_server.assert_called_once()
 
 
 def test_edit_server_partial_update_non_interactive(monkeypatch):
@@ -209,7 +213,8 @@ def test_edit_server_partial_update_non_interactive(monkeypatch):
 
     mock_global_config = Mock()
     mock_global_config.get_server.return_value = test_server
-    mock_global_config.update_server.return_value = None
+    mock_global_config.remove_server.return_value = None
+    mock_global_config.add_server.return_value = None
     monkeypatch.setattr("mcpm.commands.edit.global_config_manager", mock_global_config)
 
     # Force non-interactive mode
@@ -224,7 +229,8 @@ def test_edit_server_partial_update_non_interactive(monkeypatch):
 
     assert result.exit_code == 0
     assert "Successfully updated server" in result.output
-    mock_global_config.update_server.assert_called_once()
+    mock_global_config.remove_server.assert_called_once_with("test-server")
+    mock_global_config.add_server.assert_called_once()
 
 
 def test_edit_server_invalid_env_format(monkeypatch):
@@ -264,7 +270,8 @@ def test_edit_server_with_force_flag(monkeypatch):
 
     mock_global_config = Mock()
     mock_global_config.get_server.return_value = test_server
-    mock_global_config.update_server.return_value = None
+    mock_global_config.remove_server.return_value = None
+    mock_global_config.add_server.return_value = None
     monkeypatch.setattr("mcpm.commands.edit.global_config_manager", mock_global_config)
 
     runner = CliRunner()
@@ -276,7 +283,8 @@ def test_edit_server_with_force_flag(monkeypatch):
 
     assert result.exit_code == 0
     assert "Successfully updated server" in result.output
-    mock_global_config.update_server.assert_called_once()
+    mock_global_config.remove_server.assert_called_once_with("test-server")
+    mock_global_config.add_server.assert_called_once()
 
 
 def test_shlex_argument_parsing():

tests/test_profile_commands.py

@@ -24,7 +24,10 @@ def test_profile_edit_non_interactive_add_server(monkeypatch):
 
     # Mock GlobalConfigManager
     mock_global_config = Mock()
-    mock_global_config.get_server.return_value = STDIOServerConfig(name="new-server", command="echo new")
+    mock_global_config.list_servers.return_value = {
+        "new-server": STDIOServerConfig(name="new-server", command="echo new"),
+        "another-server": STDIOServerConfig(name="another-server", command="echo another")
+    }
     monkeypatch.setattr("mcpm.commands.profile.edit.global_config_manager", mock_global_config)
 
     # Force non-interactive mode
@@ -37,7 +40,7 @@ def test_profile_edit_non_interactive_add_server(monkeypatch):
     ])
 
     assert result.exit_code == 0
-    assert "Successfully updated profile" in result.output
+    assert "Profile 'test-profile' updated" in result.output
     # Should be called for each server being added
     assert mock_profile_config.add_server_to_profile.call_count >= 1
 
@@ -64,7 +67,7 @@ def test_profile_edit_non_interactive_remove_server(monkeypatch):
     ])
 
     assert result.exit_code == 0
-    assert "Successfully updated profile" in result.output
+    assert "Profile 'test-profile' updated" in result.output
     mock_profile_config.remove_server.assert_called_with("test-profile", "server1")
 
 
@@ -95,7 +98,7 @@ def test_profile_edit_non_interactive_set_servers(monkeypatch):
     ])
 
     assert result.exit_code == 0
-    assert "Successfully updated profile" in result.output
+    assert "Profile 'test-profile' updated" in result.output
     # Should clear existing servers then add new ones
     mock_profile_config.clear_profile.assert_called_with("test-profile")
 
@@ -121,7 +124,7 @@ def test_profile_edit_non_interactive_rename(monkeypatch):
     ])
 
     assert result.exit_code == 0
-    assert "Successfully updated profile" in result.output
+    assert "Profile 'test-profile' updated" in result.output
     mock_profile_config.rename_profile.assert_called_with("old-profile-name", "new-profile-name")
 
 
@@ -194,7 +197,7 @@ def test_profile_edit_with_force_flag(monkeypatch):
     ])
 
     assert result.exit_code == 0
-    assert "Successfully updated profile" in result.output
+    assert "Profile 'test-profile' updated" in result.output
 
 
 def test_profile_edit_interactive_fallback(monkeypatch):