Feature configuration system #103
Open
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
❌ Code Formatting Check FailedSome files in this PR are not properly formatted according to the project's clang-format rules. To fix this issue: make formatThen commit and push the changes. |
1 similar comment
❌ Code Formatting Check FailedSome files in this PR are not properly formatted according to the project's clang-format rules. To fix this issue: make formatThen commit and push the changes. |
gophergogo
reviewed
Sep 3, 2025
gophergogo
reviewed
Sep 3, 2025
gophergogo
reviewed
Sep 3, 2025
❌ Code Formatting Check FailedSome files in this PR are not properly formatted according to the project's clang-format rules. To fix this issue: make formatThen commit and push the changes. |
1 similar comment
❌ Code Formatting Check FailedSome files in this PR are not properly formatted according to the project's clang-format rules. To fix this issue: make formatThen commit and push the changes. |
edfams
reviewed
Sep 5, 2025
❌ Code Formatting Check FailedSome files in this PR are not properly formatted according to the project's clang-format rules. To fix this issue: make formatThen commit and push the changes. |
2 similar comments
❌ Code Formatting Check FailedSome files in this PR are not properly formatted according to the project's clang-format rules. To fix this issue: make formatThen commit and push the changes. |
❌ Code Formatting Check FailedSome files in this PR are not properly formatted according to the project's clang-format rules. To fix this issue: make formatThen commit and push the changes. |
- Implement core configuration data structures (NodeConfig, AdminConfig, BootstrapConfig) - Add ConfigValidationError exception for detailed validation errors - Support JSON serialization/deserialization for all config types - Include validation logic with comprehensive error messages - Add merge capability for configuration overlays
- Implement ConfigVersion class for semantic versioning - Add FilterConfig, FilterChainConfig, and TLSConfig structures - Add TransportConfig and ServerConfig with full validation - Include ConfigFactory with default configuration templates - Support JSON serialization and configuration merging
- Remove incorrect #endif when using #pragma once - File now compiles correctly
- Implement ConfigField wrapper to track field presence - Fix merge to use presence instead of sentinel values - Support deterministic configuration overlays
- Support duration units (ms, s, m, h, d) in config - Support size units (B, KB, MB, GB, TB) in config - Accept both plain numbers and strings with units
- Implement strict/warn/permissive validation modes - Track and report unknown configuration fields - Add ValidationContext for parsing state management
- Implement detailed error context tracking - Add safe JSON field accessors with type checking - Provide file/line/path information in errors
- Define ConfigSource abstract interface - Add ConfigSnapshot and change event types - Declare thread-safe singleton manager
- Add file, JSON, and environment config sources - Implement atomic configuration updates - Add version history and rollback support
- Remove incorrect PathScope usage with getUnknownFields() - Validation already handled by nested fromJson calls
- Check for duplicate filter chain names during validation - Include precise array indices in error messages - Improve transport reference error paths
- Cast chars to unsigned char for isalnum/isdigit - Prevents UB with non-ASCII or negative char values
- Wrap all j.get<>() calls with try-catch blocks - Provide precise field paths in error messages - Handle nested object parsing errors properly - Add error context for parseJsonSize and parseJsonDuration calls
Remove incorrect dereference operators when assigning to BootstrapConfig's node and admin members which are values, not pointers
- Use WithValidation types to enforce unknown field policy - Add validation context to parseConfiguration method - Check for unknown fields at root level - Report unknown fields based on STRICT/WARN/PERMISSIVE policy
- Copy listeners to local vector before invoking callbacks - Release mutex before calling listener functions - Prevents deadlocks and blocking on slow callbacks
- Test chain creation from JSON configuration - Test typed_config normalization - Test invalid configurations - Add simplified test for basic functionality - Update CMakeLists.txt to build new tests
Store originating dispatcher in AdvancedFilterChain and reuse it when cloning. Prevents null dispatcher error that caused clone to fail. - Add dispatcher member variable to AdvancedFilterChain - Pass dispatcher through constructor - Add getDispatcher() accessor method - Update mcp_chain_clone to use stored dispatcher - Ensure proper JSON cleanup in all error paths
- Update test_chain_from_json to expect clone success - Add ChainClone test case to test_json_chain_creation_simple - Fix include path in test_chain_from_json
Register test_chain_from_json executable in CMakeLists.txt with required link libraries for JSON chain testing.
Wrap all extern C functions with comprehensive exception handling to prevent exceptions from crossing the C boundary which causes undefined behavior. - Add try-catch blocks to all 26+ C API functions - Return appropriate error codes (MCP_ERROR_*) for status functions - Return nullptr/0 for handle-returning functions on exception - Return MCP_CHAIN_STATE_ERROR for state functions on exception - Guard logging statements to prevent throw during error handling - Handle std::bad_alloc separately for out-of-memory conditions
Create comprehensive test suite to verify exception safety in filter chain C API functions. Tests ensure no exceptions cross FFI boundary and appropriate error codes are returned.
viksachdev
force-pushed
the
feature/configuration-system
branch
from
dab9e29 to
3f57e81
Compare
❌ Code Formatting Check FailedSome files in this PR are not properly formatted according to the project's clang-format rules. To fix this issue: make formatThen commit and push the changes. |
- Create wrapper to unify FilterChain and AdvancedFilterChain types - Support both simple and advanced chain implementations - Add to CMake build configuration
- Replace g_chain_manager with g_unified_chain_manager - Wrap AdvancedFilterChain in UnifiedFilterChain - Update all handle operations to use unified system
- Remove g_filter_chain_manager in favor of unified manager - Convert unique_ptr to shared_ptr for UnifiedFilterChain - Update chain operations to use unified system
- Test both simple and advanced chain creation - Verify handle isolation between chain types - Test invalid handle operations - Add to CMake test configuration
Add dispatcher thread checks to prevent race conditions in chain management operations. Operations now return MCP_ERROR_INVALID_STATE when called from wrong thread.
Create comprehensive test suite to verify thread affinity enforcement for all chain operations. Tests ensure operations fail with proper error codes when called from wrong thread.
- Keep single declaration in mcp_c_collections.h where JSON constructors reside - Update documentation to clarify memory ownership - Document that mcp_json_stringify returns string to be freed with mcp_string_free
- Include mcp_c_api_json.h for canonical JSON functions - Replace conflicting declarations with compatibility wrappers - Add mcp_json_parse_mcp_string for mcp_string_t input - Add mcp_json_stringify_buffer stub for legacy API - Add deprecated mcp_json_release wrapper to mcp_json_free
- Use JsonValue::parse() and toString() for JSON operations - Leverage existing convertFromCApi/convertToCApi functions - Implement compatibility wrappers for mcp_string_t - Remove direct nlohmann::json dependency
- Test header compatibility (no conflicts) - Test canonical parse/stringify functions - Test compatibility wrappers - Test memory ownership and cleanup - Add test to CMakeLists.txt build configuration
Clean up dead code
Implement mcp_string_dup and mcp_string_buffer_free
Make mcp_json_release a proper exported function for ABI compatibility
Fix buffer API usage in chain creation test
Export complete chain configuration including name, mode, routing, buffer size, timeouts and all filter properties with metadata. Enables lossless round-trip for chain cloning and external tooling.
Tests that exported chains can be imported back with all settings and filter properties intact.
❌ Code Formatting Check FailedSome files in this PR are not properly formatted according to the project's clang-format rules. To fix this issue: make formatThen commit and push the changes. |
- Implement complete chain property export in mcp_chain_export_to_json - Add public getter methods to AdvancedFilterChain for export access - Wrap logging calls in try-catch blocks to prevent FFI exceptions - Optimize hot paths by reducing verbose logging
- Update API calls to use mcp_filter_chain_release instead of mcp_chain_destroy - Remove references to obsolete mcp_initialize and mcp_cleanup
- Add missing getMetadata() implementation to MockFilterFactory - Fix mcp_protocol_metadata_t structure initialization
- Remove non-existent gopher-mcp-filter-static library reference
❌ Code Formatting Check FailedSome files in this PR are not properly formatted according to the project's clang-format rules. To fix this issue: make formatThen commit and push the changes. |
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.
Implements #102 - File-based configuration system
This PR introduces the foundational configuration infrastructure required for the configuration-driven filter architecture. The implementation provides type-safe configuration structures with comprehensive validation and human-friendly error messages.