-
Notifications
You must be signed in to change notification settings - Fork 156
vMCP: Composite Tools supports non-string Arguments #2971
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
base: main
Are you sure you want to change the base?
Conversation
9da5fb9 to
5cd8e58
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2971 +/- ##
==========================================
+ Coverage 56.00% 56.01% +0.01%
==========================================
Files 328 328
Lines 32314 32337 +23
==========================================
+ Hits 18096 18115 +19
- Misses 12673 12675 +2
- Partials 1545 1547 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR updates the VirtualMCPServer and VirtualMCPCompositeToolDefinition CRDs to support non-string argument types in composite tool workflow steps. Previously, the WorkflowStep.Arguments field was defined as map[string]string, which forced all argument values to be strings even when the backend MCP tools expected typed values (integers, booleans, arrays, objects). The PR changes the field type to *runtime.RawExtension, the Kubernetes-native way to preserve arbitrary JSON types in CRDs.
Key Changes:
- Changed
WorkflowStep.Argumentsfrommap[string]stringto*runtime.RawExtensionto preserve value types - Updated converter logic to unmarshal RawExtension into
map[string]anywith proper error handling - Updated webhook validation in
VirtualMCPCompositeToolDefinitionto validate Arguments JSON and template syntax - Added comprehensive unit and integration tests for non-string argument types
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
cmd/thv-operator/api/v1alpha1/virtualmcpserver_types.go |
Updated WorkflowStep.Arguments field type to *runtime.RawExtension with kubebuilder markers |
cmd/thv-operator/api/v1alpha1/zz_generated.deepcopy.go |
Auto-generated deepcopy method updated to handle RawExtension |
cmd/thv-operator/api/v1alpha1/virtualmcpcompositetooldefinition_webhook.go |
Updated template validation to unmarshal Arguments JSON and validate string values |
cmd/thv-operator/api/v1alpha1/virtualmcpcompositetooldefinition_webhook_test.go |
Updated tests to use RawExtension for Arguments |
cmd/thv-operator/pkg/vmcpconfig/converter.go |
Updated convertArguments to unmarshal RawExtension to map[string]any with error handling |
cmd/thv-operator/pkg/vmcpconfig/converter_test.go |
Added comprehensive tests for non-string arguments including error cases |
cmd/thv-operator/controllers/virtualmcpserver_vmcpconfig_test.go |
Updated controller test to use RawExtension for Arguments |
test/integration/vmcp/vmcp_integration_test.go |
Added integration test verifying non-string arguments flow through to backend tools |
test/e2e/thv-operator/virtualmcp/virtualmcp_composite_sequential_test.go |
Updated E2E test to marshal Arguments as JSON for RawExtension |
test/e2e/thv-operator/virtualmcp/virtualmcp_composite_parallel_test.go |
Updated E2E test to marshal Arguments as JSON for RawExtension |
deploy/charts/operator-crds/crds/toolhive.stacklok.dev_virtualmcpservers.yaml |
Updated CRD schema to use object type with preserve-unknown-fields |
deploy/charts/operator-crds/crds/toolhive.stacklok.dev_virtualmcpcompositetooldefinitions.yaml |
Updated CRD schema to use object type with preserve-unknown-fields |
deploy/charts/operator-crds/Chart.yaml |
Bumped chart version to 0.0.78 |
deploy/charts/operator-crds/README.md |
Updated version badge to 0.0.78 |
docs/operator/crd-api.md |
Updated API documentation to describe RawExtension type and template support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
cmd/thv-operator/api/v1alpha1/virtualmcpcompositetooldefinition_webhook.go
Show resolved
Hide resolved
Signed-off-by: Jeremy Drouillard <jeremy@stacklok.com>
Signed-off-by: Jeremy Drouillard <jeremy@stacklok.com>
Signed-off-by: Jeremy Drouillard <jeremy@stacklok.com>
Signed-off-by: Jeremy Drouillard <jeremy@stacklok.com>
5dac0dc to
b4e3b6e
Compare
Summary
Fixes #2921
This PR updates the
VirtualMCPServerCRD'sWorkflowStep.Argumentsfield to support non-string value types (integers, booleans, arrays, objects). Previously, the field was defined asmap[string]string, which prevented passing typed values to backend MCP tools even though the internalvmcpruntime expectsmap[string]any.Problem
When defining composite tools in a
VirtualMCPServer, users could not specify non-string arguments:The type mismatch between the CRD (
map[string]string) and the runtime (map[string]any) caused numeric and boolean values to be incorrectly converted to strings.Solution
Changed
WorkflowStep.Argumentsfrommap[string]stringto*runtime.RawExtension, which is the Kubernetes-native way to store arbitrary JSON in CRDs. This preserves the original types through the entire pipeline.Changes
CRD Type Definition (
cmd/thv-operator/api/v1alpha1/virtualmcpserver_types.go):Converter (
cmd/thv-operator/pkg/vmcpconfig/converter.go):convertArgumentsto unmarshalRawExtensionintomap[string]anyWebhook Validation:
virtualmcpcompositetooldefinition_webhook.go)virtualmcpserver_webhook.goby factoring out a reusable function from the step above.Tests:
TestConvertCompositeTools_NonStringArgumentsunit test for the converterTestVMCPServer_CompositeToolNonStringArgumentsintegration test