Skip to content

Commit a7e21d2

Browse files
author
LittleCoinCoin
committed
fix(cli): implement shlex.split() for --args parsing
Add shlex.split() processing to handle quoted strings in --args parameter. Enables users to pass complex arguments like '-r --name aName' as a single quoted string that gets properly split into individual arguments. Handles edge cases: - Empty strings are filtered out - Invalid quotes trigger warning but don't fail - Multiple quoted strings are all processed Fixes: Issue 4 - --args string splitting for quoted arguments
1 parent dede78e commit a7e21d2

File tree

2 files changed

+141
-5
lines changed

2 files changed

+141
-5
lines changed

hatch/cli_hatch.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import argparse
1111
import json
1212
import logging
13+
import shlex
1314
import sys
1415
from pathlib import Path
1516
from typing import Optional, List
@@ -680,7 +681,19 @@ def handle_mcp_configure(host: str, server_name: str, command: str, args: list,
680681
if command is not None:
681682
omni_config_data['command'] = command
682683
if args is not None:
683-
omni_config_data['args'] = args
684+
# Process args with shlex.split() to handle quoted strings (Issue 4)
685+
processed_args = []
686+
for arg in args:
687+
if arg: # Skip empty strings
688+
try:
689+
# Split quoted strings into individual arguments
690+
split_args = shlex.split(arg)
691+
processed_args.extend(split_args)
692+
except ValueError as e:
693+
# Handle invalid quotes gracefully
694+
print(f"Warning: Invalid quote in argument '{arg}': {e}")
695+
processed_args.append(arg)
696+
omni_config_data['args'] = processed_args if processed_args else None
684697
if env_dict:
685698
omni_config_data['env'] = env_dict
686699
if url is not None:

tests/test_mcp_cli_host_config_integration.py

Lines changed: 127 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ def test_configure_creates_omni_with_headers(self):
105105
with patch('hatch.cli_hatch.MCPHostConfigurationManager') as mock_manager:
106106
with patch('hatch.cli_hatch.request_confirmation', return_value=False):
107107
result = handle_mcp_configure(
108-
host='claude-desktop',
108+
host='gemini', # Use gemini which supports remote servers
109109
server_name='test-server',
110110
command=None,
111111
args=None,
@@ -126,7 +126,7 @@ def test_configure_creates_omni_remote_server(self):
126126
with patch('hatch.cli_hatch.MCPHostConfigurationManager') as mock_manager:
127127
with patch('hatch.cli_hatch.request_confirmation', return_value=False):
128128
result = handle_mcp_configure(
129-
host='claude-desktop',
129+
host='gemini', # Use gemini which supports remote servers
130130
server_name='remote-server',
131131
command=None,
132132
args=None,
@@ -291,8 +291,9 @@ def test_configure_dry_run_displays_report_only(self):
291291
# Verify the function executed without errors
292292
self.assertEqual(result, 0)
293293

294-
# Verify MCPHostConfigurationManager was not instantiated (no actual configuration)
295-
mock_manager.assert_not_called()
294+
# Verify MCPHostConfigurationManager.create_server was NOT called (dry-run doesn't persist)
295+
# Note: get_server_config is called to check if server exists, but create_server is not called
296+
mock_manager.return_value.create_server.assert_not_called()
296297

297298

298299
class TestHostSpecificArguments(unittest.TestCase):
@@ -676,6 +677,128 @@ def test_claude_code_rejects_url_configuration(self):
676677
if 'Error' in str(call) or 'error' in str(call)]
677678
self.assertTrue(len(error_calls) > 0, "Expected error message to be printed")
678679

680+
@regression_test
681+
def test_args_quoted_string_splitting(self):
682+
"""Test that quoted strings in --args are properly split (Issue 4)."""
683+
with patch('hatch.cli_hatch.MCPHostConfigurationManager') as mock_manager:
684+
with patch('hatch.cli_hatch.request_confirmation', return_value=False):
685+
# Simulate user providing: --args "-r --name aName"
686+
# This arrives as a single string element in the args list
687+
result = handle_mcp_configure(
688+
host='claude-desktop',
689+
server_name='test-server',
690+
command='python',
691+
args=['-r --name aName'], # Single string with quoted content
692+
env=None,
693+
url=None,
694+
headers=None,
695+
no_backup=True,
696+
dry_run=False,
697+
auto_approve=False
698+
)
699+
700+
# Verify: Should succeed (return 0)
701+
self.assertEqual(result, 0)
702+
703+
# Verify: MCPServerConfigOmni was created with split args
704+
call_args = mock_manager.return_value.create_server.call_args
705+
if call_args:
706+
omni_config = call_args[1]['omni']
707+
# Args should be split into 3 elements: ['-r', '--name', 'aName']
708+
self.assertEqual(omni_config.args, ['-r', '--name', 'aName'])
709+
710+
@regression_test
711+
def test_args_multiple_quoted_strings(self):
712+
"""Test multiple quoted strings in --args are all split correctly (Issue 4)."""
713+
with patch('hatch.cli_hatch.MCPHostConfigurationManager') as mock_manager:
714+
with patch('hatch.cli_hatch.request_confirmation', return_value=False):
715+
# Simulate: --args "-r" "--name aName"
716+
result = handle_mcp_configure(
717+
host='claude-desktop',
718+
server_name='test-server',
719+
command='python',
720+
args=['-r', '--name aName'], # Two separate args
721+
env=None,
722+
url=None,
723+
headers=None,
724+
no_backup=True,
725+
dry_run=False,
726+
auto_approve=False
727+
)
728+
729+
# Verify: Should succeed
730+
self.assertEqual(result, 0)
731+
732+
# Verify: All args are properly split
733+
call_args = mock_manager.return_value.create_server.call_args
734+
if call_args:
735+
omni_config = call_args[1]['omni']
736+
# Should be split into: ['-r', '--name', 'aName']
737+
self.assertEqual(omni_config.args, ['-r', '--name', 'aName'])
738+
739+
@regression_test
740+
def test_args_empty_string_handling(self):
741+
"""Test that empty strings in --args are filtered out (Issue 4)."""
742+
with patch('hatch.cli_hatch.MCPHostConfigurationManager') as mock_manager:
743+
with patch('hatch.cli_hatch.request_confirmation', return_value=False):
744+
# Simulate: --args "" "server.py"
745+
result = handle_mcp_configure(
746+
host='claude-desktop',
747+
server_name='test-server',
748+
command='python',
749+
args=['', 'server.py'], # Empty string should be filtered
750+
env=None,
751+
url=None,
752+
headers=None,
753+
no_backup=True,
754+
dry_run=False,
755+
auto_approve=False
756+
)
757+
758+
# Verify: Should succeed
759+
self.assertEqual(result, 0)
760+
761+
# Verify: Empty strings are filtered out
762+
call_args = mock_manager.return_value.create_server.call_args
763+
if call_args:
764+
omni_config = call_args[1]['omni']
765+
# Should only contain 'server.py'
766+
self.assertEqual(omni_config.args, ['server.py'])
767+
768+
@regression_test
769+
def test_args_invalid_quote_handling(self):
770+
"""Test that invalid quotes in --args are handled gracefully (Issue 4)."""
771+
with patch('hatch.cli_hatch.MCPHostConfigurationManager') as mock_manager:
772+
with patch('hatch.cli_hatch.request_confirmation', return_value=False):
773+
with patch('hatch.cli_hatch.print') as mock_print:
774+
# Simulate: --args 'unclosed "quote'
775+
result = handle_mcp_configure(
776+
host='claude-desktop',
777+
server_name='test-server',
778+
command='python',
779+
args=['unclosed "quote'], # Invalid quote
780+
env=None,
781+
url=None,
782+
headers=None,
783+
no_backup=True,
784+
dry_run=False,
785+
auto_approve=False
786+
)
787+
788+
# Verify: Should succeed (graceful fallback)
789+
self.assertEqual(result, 0)
790+
791+
# Verify: Warning was printed
792+
warning_calls = [call for call in mock_print.call_args_list
793+
if 'Warning' in str(call)]
794+
self.assertTrue(len(warning_calls) > 0, "Expected warning for invalid quote")
795+
796+
# Verify: Original arg is used as fallback
797+
call_args = mock_manager.return_value.create_server.call_args
798+
if call_args:
799+
omni_config = call_args[1]['omni']
800+
self.assertIn('unclosed "quote', omni_config.args)
801+
679802
@regression_test
680803
def test_cli_handler_signature_compatible(self):
681804
"""Test that handle_mcp_configure signature is compatible with integration."""

0 commit comments

Comments
 (0)