Skip to content

Commit d59fc6a

Browse files
author
LittleCoinCoin
committed
feat: integrate Pydantic model hierarchy into CLI handlers
- Update handle_mcp_configure() to use MCPServerConfigOmni and HOST_MODEL_REGISTRY - Fix bug: change 'args or []' to 'args' to properly handle None for remote servers - Integrate from_omni() conversion for host-specific model creation - Add conversion report generation and display for user feedback - Add URL validation to MCPServerConfigOmni model - Update test expectations to use host-specific models (MCPServerConfigClaude, etc.) - Maintain backward compatibility with dry-run output format All tests passing (248/248, 100% success rate)
1 parent a1e3c21 commit d59fc6a

File tree

3 files changed

+80
-49
lines changed

3 files changed

+80
-49
lines changed

hatch/cli_hatch.py

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
from hatch_validator.package.package_service import PackageService
2121
from hatch.template_generator import create_package_template
2222
from hatch.mcp_host_config import MCPHostConfigurationManager, MCPHostType, MCPHostRegistry, MCPServerConfig
23+
from hatch.mcp_host_config.models import MCPServerConfigOmni, HOST_MODEL_REGISTRY
24+
from hatch.mcp_host_config.reporting import generate_conversion_report, display_report
2325

2426

2527
def get_hatch_version() -> str:
@@ -606,21 +608,41 @@ def handle_mcp_configure(host: str, server_name: str, command: str, args: list,
606608
env_dict = parse_env_vars(env)
607609
headers_dict = parse_headers(headers)
608610

609-
# Create server configuration (only include headers if URL is provided)
610-
config_data = {
611+
# Create Omni configuration (universal model)
612+
omni_config_data = {
611613
'name': server_name,
612614
'command': command,
613-
'args': args or [],
615+
'args': args, # Fixed: Don't convert None to [] - let Pydantic handle it
614616
'env': env_dict,
615617
'url': url
616618
}
617619

618-
# Only add headers if URL is provided (per MCPServerConfig validation)
620+
# Only add headers if URL is provided
619621
if url and headers_dict:
620-
config_data['headers'] = headers_dict
622+
omni_config_data['headers'] = headers_dict
621623

622-
server_config = MCPServerConfig(**config_data)
624+
# Create Omni model
625+
omni_config = MCPServerConfigOmni(**omni_config_data)
623626

627+
# Convert to host-specific model using HOST_MODEL_REGISTRY
628+
host_model_class = HOST_MODEL_REGISTRY.get(host_type)
629+
if not host_model_class:
630+
print(f"Error: No model registered for host '{host}'")
631+
return 1
632+
633+
# Convert Omni to host-specific model
634+
server_config = host_model_class.from_omni(omni_config)
635+
636+
# Generate conversion report
637+
report = generate_conversion_report(
638+
operation='create',
639+
server_name=server_name,
640+
target_host=host_type,
641+
omni=omni_config,
642+
dry_run=dry_run
643+
)
644+
645+
# Display conversion report
624646
if dry_run:
625647
print(f"[DRY RUN] Would configure MCP server '{server_name}' on host '{host}':")
626648
print(f"[DRY RUN] Command: {command}")
@@ -633,8 +655,13 @@ def handle_mcp_configure(host: str, server_name: str, command: str, args: list,
633655
if headers_dict:
634656
print(f"[DRY RUN] Headers: {headers_dict}")
635657
print(f"[DRY RUN] Backup: {'Disabled' if no_backup else 'Enabled'}")
658+
# Display report in dry-run mode
659+
display_report(report)
636660
return 0
637661

662+
# Display report before confirmation
663+
display_report(report)
664+
638665
# Confirm operation unless auto-approved
639666
if not request_confirmation(
640667
f"Configure MCP server '{server_name}' on host '{host}'?",

hatch/mcp_host_config/models.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,15 @@ class MCPServerConfigOmni(BaseModel):
535535
envFile: Optional[str] = None
536536
inputs: Optional[List[Dict]] = None
537537

538+
@field_validator('url')
539+
@classmethod
540+
def validate_url_format(cls, v):
541+
"""Validate URL format when provided."""
542+
if v is not None:
543+
if not v.startswith(('http://', 'https://')):
544+
raise ValueError("URL must start with http:// or https://")
545+
return v
546+
538547

539548
# HOST_MODEL_REGISTRY: Dictionary dispatch for host-specific models
540549
HOST_MODEL_REGISTRY: Dict[MCPHostType, type[MCPServerConfigBase]] = {

tests/test_mcp_cli_host_config_integration.py

Lines changed: 38 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -102,50 +102,44 @@ def test_configure_creates_omni_with_env_vars(self):
102102
@regression_test
103103
def test_configure_creates_omni_with_headers(self):
104104
"""Test that headers are parsed correctly into Omni model."""
105-
# NOTE: This test currently fails due to bug in handle_mcp_configure (line 613)
106-
# The implementation uses `args or []` which converts None to empty list,
107-
# causing validation error. This will be fixed in Phase 4.
108-
# For now, test that the error is caught properly.
109-
result = handle_mcp_configure(
110-
host='claude-desktop',
111-
server_name='test-server',
112-
command=None,
113-
args=None, # Will be converted to [] by current implementation (bug)
114-
env=None,
115-
url='https://api.example.com',
116-
headers=['Authorization=Bearer token', 'Content-Type=application/json'],
117-
no_backup=True,
118-
dry_run=False,
119-
auto_approve=False
120-
)
105+
with patch('hatch.cli_hatch.MCPHostConfigurationManager') as mock_manager:
106+
with patch('hatch.cli_hatch.request_confirmation', return_value=False):
107+
result = handle_mcp_configure(
108+
host='claude-desktop',
109+
server_name='test-server',
110+
command=None,
111+
args=None,
112+
env=None,
113+
url='https://api.example.com',
114+
headers=['Authorization=Bearer token', 'Content-Type=application/json'],
115+
no_backup=True,
116+
dry_run=False,
117+
auto_approve=False
118+
)
121119

122-
# Current implementation returns error due to args validation bug
123-
# This will be fixed in Phase 4 to return 0
124-
self.assertEqual(result, 1)
120+
# Verify the function executed without errors (bug fixed in Phase 4)
121+
self.assertEqual(result, 0)
125122

126123
@regression_test
127124
def test_configure_creates_omni_remote_server(self):
128125
"""Test that remote server arguments create correct Omni model."""
129-
# NOTE: This test currently fails due to bug in handle_mcp_configure (line 613)
130-
# The implementation uses `args or []` which converts None to empty list,
131-
# causing validation error. This will be fixed in Phase 4.
132-
# For now, test that the error is caught properly.
133-
result = handle_mcp_configure(
134-
host='claude-desktop',
135-
server_name='remote-server',
136-
command=None,
137-
args=None, # Will be converted to [] by current implementation (bug)
138-
env=None,
139-
url='https://api.example.com',
140-
headers=['Auth=token'],
141-
no_backup=True,
142-
dry_run=False,
143-
auto_approve=False
144-
)
126+
with patch('hatch.cli_hatch.MCPHostConfigurationManager') as mock_manager:
127+
with patch('hatch.cli_hatch.request_confirmation', return_value=False):
128+
result = handle_mcp_configure(
129+
host='claude-desktop',
130+
server_name='remote-server',
131+
command=None,
132+
args=None,
133+
env=None,
134+
url='https://api.example.com',
135+
headers=['Auth=token'],
136+
no_backup=True,
137+
dry_run=False,
138+
auto_approve=False
139+
)
145140

146-
# Current implementation returns error due to args validation bug
147-
# This will be fixed in Phase 4 to return 0
148-
self.assertEqual(result, 1)
141+
# Verify the function executed without errors (bug fixed in Phase 4)
142+
self.assertEqual(result, 0)
149143

150144
@regression_test
151145
def test_configure_omni_with_all_universal_fields(self):
@@ -246,7 +240,7 @@ def test_configure_passes_host_specific_model_to_manager(self):
246240
mock_manager = MagicMock()
247241
mock_manager_class.return_value = mock_manager
248242
mock_manager.configure_server.return_value = MagicMock(success=True, backup_path=None)
249-
243+
250244
with patch('hatch.cli_hatch.request_confirmation', return_value=True):
251245
# Call configure command
252246
result = handle_mcp_configure(
@@ -261,15 +255,16 @@ def test_configure_passes_host_specific_model_to_manager(self):
261255
dry_run=False,
262256
auto_approve=False
263257
)
264-
258+
265259
# Verify configure_server was called
266260
self.assertEqual(result, 0)
267261
mock_manager.configure_server.assert_called_once()
268-
269-
# Verify the server_config argument is an MCPServerConfig instance
262+
263+
# Verify the server_config argument is a host-specific model instance
264+
# (MCPServerConfigClaude for claude-desktop host)
270265
call_args = mock_manager.configure_server.call_args
271266
server_config = call_args.kwargs['server_config']
272-
self.assertIsInstance(server_config, MCPServerConfig)
267+
self.assertIsInstance(server_config, MCPServerConfigClaude)
273268

274269

275270
class TestReportingIntegration(unittest.TestCase):

0 commit comments

Comments
 (0)