Skip to content

Commit 7385763

Browse files
author
LittleCoinCoin
committed
chore(cli): remove useless --no-mcp-config flag
The --no-mcp-config argument in hatch package add was confirmed to be useless as the logic "and not args.no_mcp_config" provided no additional functionality beyond the existing "and args.host" condition. Remove both the argument definition and associated conditional logic to clean up the CLI surface and eliminate dead code.
1 parent a86fa7c commit 7385763

File tree

2 files changed

+116
-78
lines changed

2 files changed

+116
-78
lines changed

hatch/cli_hatch.py

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1764,11 +1764,6 @@ def main():
17641764
"--host",
17651765
help="Comma-separated list of MCP host platforms to configure (e.g., claude-desktop,cursor)",
17661766
)
1767-
pkg_add_parser.add_argument(
1768-
"--no-mcp-config",
1769-
action="store_true",
1770-
help="Skip automatic MCP host configuration even if package has MCP servers",
1771-
)
17721767

17731768
# Remove package command
17741769
pkg_remove_parser = pkg_subparsers.add_parser(
@@ -2154,7 +2149,7 @@ def main():
21542149
print(f"Successfully added package: {args.package_path_or_name}")
21552150

21562151
# Handle MCP host configuration if requested
2157-
if hasattr(args, "host") and args.host and not args.no_mcp_config:
2152+
if hasattr(args, "host") and args.host:
21582153
try:
21592154
hosts = parse_host_list(args.host)
21602155
env_name = args.env or env_manager.get_current_environment()

tests/test_mcp_cli_package_management.py

Lines changed: 115 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -5,27 +5,33 @@
55
configuration integration following CrackingShells testing standards.
66
"""
77

8-
import unittest
98
import sys
9+
import unittest
1010
from pathlib import Path
11-
from unittest.mock import patch, MagicMock, mock_open
11+
from unittest.mock import MagicMock, mock_open, patch
1212

1313
# Add the parent directory to the path to import wobble
1414
sys.path.insert(0, str(Path(__file__).parent.parent))
1515

1616
try:
17-
from wobble.decorators import regression_test, integration_test
17+
from wobble.decorators import integration_test, regression_test
1818
except ImportError:
1919
# Fallback decorators if wobble is not available
2020
def regression_test(func):
2121
return func
22-
22+
2323
def integration_test(scope="component"):
2424
def decorator(func):
2525
return func
26+
2627
return decorator
2728

28-
from hatch.cli_hatch import parse_host_list, request_confirmation, get_package_mcp_server_config
29+
30+
from hatch.cli_hatch import (
31+
get_package_mcp_server_config,
32+
parse_host_list,
33+
request_confirmation,
34+
)
2935
from hatch.mcp_host_config import MCPHostType, MCPServerConfig
3036

3137

@@ -61,7 +67,9 @@ def test_parse_host_list_none(self):
6167
@regression_test
6268
def test_parse_host_list_all(self):
6369
"""Test parsing 'all' host list."""
64-
with patch('hatch.cli_hatch.MCPHostRegistry.detect_available_hosts') as mock_detect:
70+
with patch(
71+
"hatch.cli_hatch.MCPHostRegistry.detect_available_hosts"
72+
) as mock_detect:
6573
mock_detect.return_value = [MCPHostType.CLAUDE_DESKTOP, MCPHostType.CURSOR]
6674
hosts = parse_host_list("all")
6775
expected = [MCPHostType.CLAUDE_DESKTOP, MCPHostType.CURSOR]
@@ -73,7 +81,7 @@ def test_parse_host_list_invalid_host(self):
7381
"""Test parsing invalid host raises ValueError."""
7482
with self.assertRaises(ValueError) as context:
7583
parse_host_list("invalid-host")
76-
84+
7785
self.assertIn("Unknown host 'invalid-host'", str(context.exception))
7886
self.assertIn("Available:", str(context.exception))
7987

@@ -82,7 +90,7 @@ def test_parse_host_list_mixed_valid_invalid(self):
8290
"""Test parsing mixed valid and invalid hosts."""
8391
with self.assertRaises(ValueError) as context:
8492
parse_host_list("claude-desktop,invalid-host,cursor")
85-
93+
8694
self.assertIn("Unknown host 'invalid-host'", str(context.exception))
8795

8896
@regression_test
@@ -101,154 +109,174 @@ def test_request_confirmation_auto_approve(self):
101109
@regression_test
102110
def test_request_confirmation_user_yes(self):
103111
"""Test confirmation with user saying yes."""
104-
with patch('builtins.input', return_value='y'):
112+
with patch("builtins.input", return_value="y"):
105113
result = request_confirmation("Test message?", auto_approve=False)
106114
self.assertTrue(result)
107115

108116
@regression_test
109117
def test_request_confirmation_user_yes_full(self):
110118
"""Test confirmation with user saying 'yes'."""
111-
with patch('builtins.input', return_value='yes'):
119+
with patch("builtins.input", return_value="yes"):
112120
result = request_confirmation("Test message?", auto_approve=False)
113121
self.assertTrue(result)
114122

115123
@regression_test
116124
def test_request_confirmation_user_no(self):
117125
"""Test confirmation with user saying no."""
118-
with patch.dict('os.environ', {'HATCH_AUTO_APPROVE': ''}, clear=False):
119-
with patch('builtins.input', return_value='n'):
126+
with patch.dict("os.environ", {"HATCH_AUTO_APPROVE": ""}, clear=False):
127+
with patch("builtins.input", return_value="n"):
120128
result = request_confirmation("Test message?", auto_approve=False)
121129
self.assertFalse(result)
122130

123131
@regression_test
124132
def test_request_confirmation_user_no_full(self):
125133
"""Test confirmation with user saying 'no'."""
126-
with patch.dict('os.environ', {'HATCH_AUTO_APPROVE': ''}, clear=False):
127-
with patch('builtins.input', return_value='no'):
134+
with patch.dict("os.environ", {"HATCH_AUTO_APPROVE": ""}, clear=False):
135+
with patch("builtins.input", return_value="no"):
128136
result = request_confirmation("Test message?", auto_approve=False)
129137
self.assertFalse(result)
130138

131139
@regression_test
132140
def test_request_confirmation_user_empty(self):
133141
"""Test confirmation with user pressing enter (default no)."""
134-
with patch.dict('os.environ', {'HATCH_AUTO_APPROVE': ''}, clear=False):
135-
with patch('builtins.input', return_value=''):
142+
with patch.dict("os.environ", {"HATCH_AUTO_APPROVE": ""}, clear=False):
143+
with patch("builtins.input", return_value=""):
136144
result = request_confirmation("Test message?", auto_approve=False)
137145
self.assertFalse(result)
138146

139147
@integration_test(scope="component")
140148
def test_package_add_argument_parsing(self):
141149
"""Test package add command argument parsing with MCP flags."""
142-
from hatch.cli_hatch import main
143150
import argparse
144-
151+
152+
from hatch.cli_hatch import main
153+
145154
# Mock argparse to capture parsed arguments
146-
with patch('argparse.ArgumentParser.parse_args') as mock_parse:
155+
with patch("argparse.ArgumentParser.parse_args") as mock_parse:
147156
mock_args = MagicMock()
148-
mock_args.command = 'package'
149-
mock_args.pkg_command = 'add'
150-
mock_args.package_path_or_name = 'test-package'
151-
mock_args.host = 'claude-desktop,cursor'
152-
mock_args.no_mcp_config = False
157+
mock_args.command = "package"
158+
mock_args.pkg_command = "add"
159+
mock_args.package_path_or_name = "test-package"
160+
mock_args.host = "claude-desktop,cursor"
153161
mock_args.env = None
154162
mock_args.version = None
155163
mock_args.force_download = False
156164
mock_args.refresh_registry = False
157165
mock_args.auto_approve = False
158166
mock_parse.return_value = mock_args
159-
167+
160168
# Mock environment manager to avoid actual operations
161-
with patch('hatch.cli_hatch.HatchEnvironmentManager') as mock_env_manager:
169+
with patch("hatch.cli_hatch.HatchEnvironmentManager") as mock_env_manager:
162170
mock_env_manager.return_value.add_package_to_environment.return_value = True
163-
mock_env_manager.return_value.get_current_environment.return_value = "default"
164-
171+
mock_env_manager.return_value.get_current_environment.return_value = (
172+
"default"
173+
)
174+
165175
# Mock MCP manager
166-
with patch('hatch.cli_hatch.MCPHostConfigurationManager'):
167-
with patch('builtins.print') as mock_print:
176+
with patch("hatch.cli_hatch.MCPHostConfigurationManager"):
177+
with patch("builtins.print") as mock_print:
168178
result = main()
169-
179+
170180
# Should succeed
171181
self.assertEqual(result, 0)
172-
182+
173183
# Should print success message
174-
mock_print.assert_any_call("Successfully added package: test-package")
184+
mock_print.assert_any_call(
185+
"Successfully added package: test-package"
186+
)
175187

176188
@integration_test(scope="component")
177189
def test_package_sync_argument_parsing(self):
178190
"""Test package sync command argument parsing."""
179-
from hatch.cli_hatch import main
180191
import argparse
181192

193+
from hatch.cli_hatch import main
194+
182195
# Mock argparse to capture parsed arguments
183-
with patch('argparse.ArgumentParser.parse_args') as mock_parse:
196+
with patch("argparse.ArgumentParser.parse_args") as mock_parse:
184197
mock_args = MagicMock()
185-
mock_args.command = 'package'
186-
mock_args.pkg_command = 'sync'
187-
mock_args.package_name = 'test-package'
188-
mock_args.host = 'claude-desktop,cursor'
198+
mock_args.command = "package"
199+
mock_args.pkg_command = "sync"
200+
mock_args.package_name = "test-package"
201+
mock_args.host = "claude-desktop,cursor"
189202
mock_args.env = None
190203
mock_args.dry_run = True # Use dry run to avoid actual configuration
191204
mock_args.auto_approve = False
192205
mock_args.no_backup = False
193206
mock_parse.return_value = mock_args
194207

195208
# Mock the get_package_mcp_server_config function
196-
with patch('hatch.cli_hatch.get_package_mcp_server_config') as mock_get_config:
209+
with patch(
210+
"hatch.cli_hatch.get_package_mcp_server_config"
211+
) as mock_get_config:
197212
mock_server_config = MagicMock()
198-
mock_server_config.name = 'test-package'
199-
mock_server_config.args = ['/path/to/server.py']
213+
mock_server_config.name = "test-package"
214+
mock_server_config.args = ["/path/to/server.py"]
200215
mock_get_config.return_value = mock_server_config
201216

202217
# Mock environment manager
203-
with patch('hatch.cli_hatch.HatchEnvironmentManager') as mock_env_manager:
218+
with patch(
219+
"hatch.cli_hatch.HatchEnvironmentManager"
220+
) as mock_env_manager:
204221
mock_env_manager.return_value.get_current_environment.return_value = "default"
205222

206223
# Mock MCP manager
207-
with patch('hatch.cli_hatch.MCPHostConfigurationManager'):
208-
with patch('builtins.print') as mock_print:
224+
with patch("hatch.cli_hatch.MCPHostConfigurationManager"):
225+
with patch("builtins.print") as mock_print:
209226
result = main()
210227

211228
# Should succeed
212229
self.assertEqual(result, 0)
213230

214231
# Should print dry run message (new format includes dependency info)
215-
mock_print.assert_any_call("[DRY RUN] Would synchronize MCP servers for 1 package(s) to hosts: ['claude-desktop', 'cursor']")
232+
mock_print.assert_any_call(
233+
"[DRY RUN] Would synchronize MCP servers for 1 package(s) to hosts: ['claude-desktop', 'cursor']"
234+
)
216235

217236
@integration_test(scope="component")
218237
def test_package_sync_package_not_found(self):
219238
"""Test package sync when package doesn't exist."""
220-
from hatch.cli_hatch import main
221239
import argparse
222240

241+
from hatch.cli_hatch import main
242+
223243
# Mock argparse to capture parsed arguments
224-
with patch('argparse.ArgumentParser.parse_args') as mock_parse:
244+
with patch("argparse.ArgumentParser.parse_args") as mock_parse:
225245
mock_args = MagicMock()
226-
mock_args.command = 'package'
227-
mock_args.pkg_command = 'sync'
228-
mock_args.package_name = 'nonexistent-package'
229-
mock_args.host = 'claude-desktop'
246+
mock_args.command = "package"
247+
mock_args.pkg_command = "sync"
248+
mock_args.package_name = "nonexistent-package"
249+
mock_args.host = "claude-desktop"
230250
mock_args.env = None
231251
mock_args.dry_run = False
232252
mock_args.auto_approve = False
233253
mock_args.no_backup = False
234254
mock_parse.return_value = mock_args
235255

236256
# Mock the get_package_mcp_server_config function to raise ValueError
237-
with patch('hatch.cli_hatch.get_package_mcp_server_config') as mock_get_config:
238-
mock_get_config.side_effect = ValueError("Package 'nonexistent-package' not found in environment 'default'")
257+
with patch(
258+
"hatch.cli_hatch.get_package_mcp_server_config"
259+
) as mock_get_config:
260+
mock_get_config.side_effect = ValueError(
261+
"Package 'nonexistent-package' not found in environment 'default'"
262+
)
239263

240264
# Mock environment manager
241-
with patch('hatch.cli_hatch.HatchEnvironmentManager') as mock_env_manager:
265+
with patch(
266+
"hatch.cli_hatch.HatchEnvironmentManager"
267+
) as mock_env_manager:
242268
mock_env_manager.return_value.get_current_environment.return_value = "default"
243269

244-
with patch('builtins.print') as mock_print:
270+
with patch("builtins.print") as mock_print:
245271
result = main()
246272

247273
# Should fail
248274
self.assertEqual(result, 1)
249275

250276
# Should print error message (new format)
251-
mock_print.assert_any_call("Error: No MCP server configurations found for package 'nonexistent-package' or its dependencies")
277+
mock_print.assert_any_call(
278+
"Error: No MCP server configurations found for package 'nonexistent-package' or its dependencies"
279+
)
252280

253281
@regression_test
254282
def test_get_package_mcp_server_config_success(self):
@@ -257,27 +285,38 @@ def test_get_package_mcp_server_config_success(self):
257285
mock_env_manager = MagicMock()
258286
mock_env_manager.list_packages.return_value = [
259287
{
260-
'name': 'test-package',
261-
'version': '1.0.0',
262-
'source': {'path': '/path/to/package'}
288+
"name": "test-package",
289+
"version": "1.0.0",
290+
"source": {"path": "/path/to/package"},
263291
}
264292
]
265293
# Mock the Python executable method to return a proper string
266294
mock_env_manager.get_current_python_executable.return_value = "/path/to/python"
267295

268296
# Mock file system and metadata
269-
with patch('pathlib.Path.exists', return_value=True):
270-
with patch('builtins.open', mock_open(read_data='{"package_schema_version": "1.2.1", "name": "test-package"}')):
271-
with patch('hatch_validator.package.package_service.PackageService') as mock_service_class:
297+
with patch("pathlib.Path.exists", return_value=True):
298+
with patch(
299+
"builtins.open",
300+
mock_open(
301+
read_data='{"package_schema_version": "1.2.1", "name": "test-package"}'
302+
),
303+
):
304+
with patch(
305+
"hatch_validator.package.package_service.PackageService"
306+
) as mock_service_class:
272307
mock_service = MagicMock()
273308
mock_service.get_mcp_entry_point.return_value = "mcp_server.py"
274309
mock_service_class.return_value = mock_service
275310

276-
config = get_package_mcp_server_config(mock_env_manager, "test-env", "test-package")
311+
config = get_package_mcp_server_config(
312+
mock_env_manager, "test-env", "test-package"
313+
)
277314

278315
self.assertIsInstance(config, MCPServerConfig)
279316
self.assertEqual(config.name, "test-package")
280-
self.assertEqual(config.command, "/path/to/python") # Now uses environment-specific Python
317+
self.assertEqual(
318+
config.command, "/path/to/python"
319+
) # Now uses environment-specific Python
281320
self.assertTrue(config.args[0].endswith("mcp_server.py"))
282321

283322
@regression_test
@@ -288,7 +327,9 @@ def test_get_package_mcp_server_config_package_not_found(self):
288327
mock_env_manager.list_packages.return_value = []
289328

290329
with self.assertRaises(ValueError) as context:
291-
get_package_mcp_server_config(mock_env_manager, "test-env", "nonexistent-package")
330+
get_package_mcp_server_config(
331+
mock_env_manager, "test-env", "nonexistent-package"
332+
)
292333

293334
self.assertIn("Package 'nonexistent-package' not found", str(context.exception))
294335

@@ -299,19 +340,21 @@ def test_get_package_mcp_server_config_no_metadata(self):
299340
mock_env_manager = MagicMock()
300341
mock_env_manager.list_packages.return_value = [
301342
{
302-
'name': 'test-package',
303-
'version': '1.0.0',
304-
'source': {'path': '/path/to/package'}
343+
"name": "test-package",
344+
"version": "1.0.0",
345+
"source": {"path": "/path/to/package"},
305346
}
306347
]
307348

308349
# Mock file system - metadata file doesn't exist
309-
with patch('pathlib.Path.exists', return_value=False):
350+
with patch("pathlib.Path.exists", return_value=False):
310351
with self.assertRaises(ValueError) as context:
311-
get_package_mcp_server_config(mock_env_manager, "test-env", "test-package")
352+
get_package_mcp_server_config(
353+
mock_env_manager, "test-env", "test-package"
354+
)
312355

313356
self.assertIn("not a Hatch package", str(context.exception))
314357

315358

316-
if __name__ == '__main__':
359+
if __name__ == "__main__":
317360
unittest.main()

0 commit comments

Comments
 (0)