-
Notifications
You must be signed in to change notification settings - Fork 3
Add Conversion Report System and Complete Host-Specific CLI Arguments Support #40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
7e3961f to
7df305d
Compare
The test_remove_host_successful test was failing because it mocked MCPHostConfigurationManager but didn't mock the env_manager method clear_host_from_all_packages_all_envs() which is called during host removal. Added mock to return value 2 (number of packages cleared) to allow the test to complete successfully.
The MCP configure command had a critical argparse naming conflict where the --command flag (for server command) was overwriting the top-level 'command' dest (which should be 'mcp'). This caused the CLI routing logic to fail because args.command would be 'python' instead of 'mcp'. Changes: - Added dest='server_command' to --command argument in configure parser - Updated handler call to use args.server_command instead of args.command - Fixed test_configure_argument_parsing_basic to handle SystemExit properly - Fixed test_configure_argument_parsing_with_options CLI arguments This resolves the issue where 'hatch mcp configure' commands would display help instead of executing the configuration.
The test registry creation was looking for 'hatch_dependencies' and
'python_dependencies' keys, but the actual metadata structure uses
'dependencies.hatch' and 'dependencies.python' (nested structure).
This caused the test registry to be created without dependency information,
preventing dependency resolution from working correctly in tests.
Updated to use correct nested structure:
- metadata.get('dependencies', {}).get('hatch', [])
- metadata.get('dependencies', {}).get('python', [])
Changed dependency from registry-based 'base_pkg' to local path '../../basic/base_pkg' to properly test local dependency resolution in test_add_package_with_dependencies_non_tty. This aligns with the v1.2.0+ schema where dependencies can be specified as relative paths.
Add comprehensive Pydantic model hierarchy for MCP host configuration: - Add type field to MCPServerConfig for transport discrimination - Implement MCPServerConfigBase with universal fields and validation - Add host-specific models: Gemini, VS Code, Cursor, Claude - Add MCPServerConfigOmni as primary API interface - Implement HOST_MODEL_REGISTRY for dictionary dispatch - Add from_omni() conversion methods with dynamic field derivation Key features: - Type field enables explicit transport specification (stdio/sse/http) - Dynamic field derivation using cls.model_fields.keys() - Pydantic-native APIs (model_dump, model_validate) - Backward compatibility maintained for existing code
Add test suites for type field validation and model hierarchy: Phase 3A Tests (test_mcp_server_config_type_field.py): - 13 regression tests for type field validation - Test stdio/sse/http type with command/url validation - Test backward compatibility without type field - Test type field serialization and roundtrip Phase 3B Tests (test_mcp_pydantic_architecture_v4.py): - 28 regression tests for model hierarchy - Test base model validation and type inference - Test host-specific models (Gemini, VS Code, Cursor, Claude) - Test MCPServerConfigOmni with mixed host fields - Test HOST_MODEL_REGISTRY dictionary dispatch - Test from_omni() conversion with field filtering Updated existing tests: - Update test_mcp_server_config_no_future_extension_fields to reflect extra='allow' design for host-specific fields Test results: 55/55 tests pass (100% pass rate)
Add comprehensive reporting system for MCP configuration operations: - Add FieldOperation model with __str__() for console output - Add ConversionReport model for operation metadata - Implement generate_conversion_report() with dynamic field derivation - Implement display_report() for formatted console output Key features: - Three operation types: UPDATED, UNSUPPORTED, UNCHANGED - Dynamic field derivation using HOST_MODEL_REGISTRY - ASCII arrow (-->) for terminal compatibility - Supports create, update, delete, migrate operations - Dry-run mode with clear preview messaging Benefits: - No hardcoded SUPPORTED_FIELDS constants - Single source of truth (Pydantic model definitions) - Clean separation from models module - User-friendly console output
Add test suite for MCP user feedback reporting system: Test Classes (14 regression tests total): - TestFieldOperation (4 tests) * Test __str__() for UPDATED, UNSUPPORTED, UNCHANGED operations * Test ASCII arrow usage (not Unicode) * Test None old_value handling - TestConversionReport (3 tests) * Test create, update, migrate operations * Test model field validation - TestGenerateConversionReport (4 tests) * Test create with all supported fields * Test create with unsupported fields * Test update with changed/unchanged fields * Test dynamic field derivation from HOST_MODEL_REGISTRY - TestDisplayReport (3 tests) * Test console output for create/update operations * Test dry-run mode with preview messaging Test results: 14/14 tests pass (100% pass rate) Total MCP tests: 221/221 pass (100% pass rate)
- Add 27 regression and integration tests for CLI integration - Create test data directory with mock MCP host config files - Test CLI argument parsing to Omni model creation - Test model integration with HOST_MODEL_REGISTRY - Test reporting integration in CLI commands - Test host-specific argument handling - Test error handling and validation - Test backward compatibility - Verify readiness for Phase 4 CLI integration implementation All tests passing (27/27, 100% success rate)
- 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)
- Document new conversion report output showing field-by-field changes - Add examples demonstrating user feedback display for local/remote servers - Document dry-run mode with conversion report preview - Explain host-specific field support and automatic filtering - Add detailed behavior section explaining UPDATED/UNSUPPORTED/UNCHANGED fields All documentation changes reflect Phase 4 implementation
- Integrate MCPServerConfigOmni and HOST_MODEL_REGISTRY into package add handler - Integrate MCPServerConfigOmni and HOST_MODEL_REGISTRY into package sync handler - Generate and display conversion reports for each package-host combination - Add dry-run mode conversion report preview for package sync - Convert MCPServerConfig to Omni model before host-specific conversion - Pass host-specific models to MCPHostConfigurationManager - Maintain backward compatibility with existing package management workflow All tests passing (248/248, 100% success rate)
- Only include non-None fields in Omni model creation - Fix handle_mcp_configure to conditionally add fields - Fix handle_package_add to conditionally add fields - Fix handle_package_sync to conditionally add fields (dry-run and actual) - Reports now correctly show only set fields, not 'UPDATED None --> None' This fixes the bug where fields with None values were showing as 'UPDATED None --> None' instead of being excluded from the report. The fix ensures model_dump(exclude_unset=True) works correctly by only including fields that have actual values when creating Omni models. All tests passing (248/248, 100% success rate)
- Add --timeout argument for Gemini (request timeout in milliseconds) - Add --trust argument for Gemini (bypass tool call confirmations) - Add --cwd argument for Gemini (working directory for stdio transport) - Add --env-file argument for Cursor/VS Code/LM Studio (path to environment file) Implementation details: - Updated handle_mcp_configure() signature with 4 new parameters - Added host-specific argument validation (error if used with incompatible hosts) - Integrated new fields into MCPServerConfigOmni creation - Updated argument parser with new CLI flags - Updated main() function call to pass new parameters Testing: - Created test_mcp_cli_host_specific_args.py with 12 comprehensive tests - Updated test_mcp_cli_direct_management.py to match new function signature - All 260 MCP tests passing (100% success rate) This implements Phase 2 of the original design specifications for commonly-used host-specific fields across Gemini, Cursor, and VS Code hosts.
- Remove incorrect pre-validation logic that rejected host-specific arguments
- Add ALL missing CLI arguments:
* --http-url (Gemini HTTP streaming endpoint)
* --include-tools (Gemini tool allowlist)
* --exclude-tools (Gemini tool blocklist)
* --inputs (VS Code input variable definitions)
- Implement parse_inputs() helper for VS Code inputs parsing
- Update handle_mcp_configure() signature with all new parameters
- Update argument parser with all host-specific arguments
- Update function call in main() to pass all new parameters
- Replace validation tests with UNSUPPORTED reporting tests
- All 260 MCP tests passing (100% success rate)
Design principle: Accept all host-specific arguments for all hosts.
The reporting system shows unsupported fields as 'UNSUPPORTED' in
conversion reports rather than rejecting them upfront with errors.
Example:
hatch mcp configure demo --host vscode --command python --timeout 30000
Output:
name: UPDATED None --> 'demo'
command: UPDATED None --> 'python'
timeout: UNSUPPORTED (not supported by vscode)
7df305d to
75943b9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements a comprehensive conversion report system that provides transparent feedback when configuring MCP servers across different hosts, along with complete support for all host-specific CLI arguments.
Key Features
Intelligent Field Reporting
UPDATED,UNCHANGED, orUNSUPPORTEDfor the target hostComplete Host-Specific Arguments
--timeout,--trust,--cwd,--http-url,--include-tools,--exclude-tools--env-file,--inputs(with structured parsing)--env-fileArchitecture Improvements
MCPServerConfigOmniuniversal model for accepting all configuration fieldsHOST_MODEL_REGISTRYfor dynamic host-specific model conversiongenerate_conversion_report()anddisplay_report()for consistent user feedbackconfigure,package add,package syncUser Experience
Example of new configuration report:
Testing
Documentation