Repository navigation
Conversation
…-operation-contract
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The review found an issue that should be addressed before approval.
17 open issues
CompareAndSwap,CompareAndDeleteandCompareAndDeleteWithDependents,AddWithParent… · NewVersionedResource.SupportsConditionalMutations()is declared as an opt-in switch, but nothing… · NewParseUint… · NewPropertiesis a non-nil empty map, but… · Newencoding/jsonhashes a nilParameterTypes… · NewItemsandEnumsslices before hashing. Provider metadata withitems: []and… · Newfloatparameter or nested field accepts values such as1e100under this unbounded number… · Newbool,int64, andfloat64fall through this switch. Consequently, a Go… · NewCreateaccepts any futureexpiresAt, including fractional seconds, but RFC3339 formatting… · NewBuildResourceKeyconcatenates mesh and name with/, but neither identifier is restricted and… · Newpositionfields… · NewCompareAndDeleteWithDependentsaccepts a genericconditions []index.IndexCondition, but the… · Newstore.IsVersionedResourceKind(gs.kind)is evaluated on everyGetByKey(and also in… · New
1 file not reviewed
api/mesh/v1alpha1/mcp_server.pb.go: Generated file
What changed in this PR
Introduces contract and persistence foundations for exposing Dubbo operations as MCP Tools, supporting issue #1576.
Changes:
- Adds exact operation resolution, recursive input schemas, directional fingerprints, and position-based argument restoration.
- Introduces MCP server snapshots and credential lifecycle repositories.
- Adds conditional mutations and dependent-resource operations to memory/GORM stores, with supporting tests.
Validation: Reviewed all 25 changed files and verified empty-map JSON round-trip behavior with a standalone Go check. Project tests were not run.
| File | Description |
|---|---|
| pkg/store/memory/store.go | Updated as part of this pull request. |
| pkg/store/memory/conditional_test.go | Updated as part of this pull request. |
| pkg/store/dbcommon/model.go | Updated as part of this pull request. |
| pkg/store/dbcommon/gorm_store.go | Updated as part of this pull request. |
| pkg/store/dbcommon/conditional_test.go | Updated as part of this pull request. |
| pkg/mcp/repository/server.go | Updated as part of this pull request. |
| pkg/mcp/repository/server_test.go | Updated as part of this pull request. |
| pkg/mcp/repository/credential.go | Updated as part of this pull request. |
| pkg/mcp/repository/credential_test.go | Updated as part of this pull request. |
| pkg/mcp/business/schema.go | Updated as part of this pull request. |
| pkg/mcp/business/contract.go | Updated as part of this pull request. |
| pkg/mcp/business/contract_test.go | Updated as part of this pull request. |
| pkg/mcp/business/arguments.go | Updated as part of this pull request. |
| pkg/core/store/store.go | Updated as part of this pull request. |
| pkg/core/store/index/mcp.go | Updated as part of this pull request. |
| pkg/core/resource/apis/mesh/v1alpha1/mcpserver_types.go | Updated as part of this pull request. |
| pkg/core/resource/apis/mesh/v1alpha1/mcpcredential_types.go | Updated as part of this pull request. |
| pkg/core/resource/apis/mesh/v1alpha1/mcp_helpers.go | Updated as part of this pull request. |
| pkg/core/manager/manager.go | Updated as part of this pull request. |
| pkg/console/service/service_generic_invoke_decode.go | Updated as part of this pull request. |
| pkg/common/util/genericinvoke/decode.go | Updated as part of this pull request. |
| go.sum | Updated as part of this pull request. |
| go.mod | Updated as part of this pull request. |
| api/mesh/v1alpha1/mcp_server.proto | Updated as part of this pull request. |
| api/mesh/v1alpha1/mcp_server.pb.go | Updated as part of this pull request. |
Review effort: Max
| string type = 1; | ||
| repeated string items = 2; | ||
| repeated string enums = 3; | ||
| map<string, string> properties = 4; |
Comment on lines
+253
to
+257
| func (rm *resourcesManager) AddWithParent(parent model.Resource, child model.Resource) error { | ||
| parentStore, err := rm.storeRouter.ResourceRoute(parent) | ||
| if err != nil { | ||
| return err | ||
| } |
Comment on lines
+86
to
+87
| _, ok := newResource().(VersionedResource) | ||
| return ok |
| return "", &PreconditionError{Reason: "resource does not support conditional mutations"} | ||
| } | ||
| expected, err := strconv.ParseUint(expectedVersion, 10, 64) | ||
| if err != nil || expected == 0 { |
| decoded[key] = converted | ||
| } | ||
| return decoded, nil | ||
| case definition.Properties != nil: |
Comment on lines
+65
to
+67
| func (r *Servers) Get(mesh, name string) (*meshresource.MCPServerResource, error) { | ||
| server, exists, err := manager.GetByKey[*meshresource.MCPServerResource]( | ||
| r.resources, meshresource.MCPServerKind, coremodel.BuildResourceKey(mesh, name)) |
Comment on lines
+85
to
+86
| updated := server.DeepCopyObject().(*meshresource.MCPServerResource) | ||
| updated.Spec.Draft = proto.Clone(draft).(*meshproto.MCPServerSnapshot) |
Comment on lines
+107
to
+110
| snapshot := proto.Clone(server.Spec.Draft).(*meshproto.MCPServerSnapshot) | ||
| if err := validate(snapshot); err != nil { | ||
| return nil, err | ||
| } |
Comment on lines
+423
to
+425
| if len(conditions) != 2 || conditions[0].IndexName != index.ByMeshIndex || conditions[0].Value != obj.ResourceMesh() || conditions[1].IndexName != index.ByMCPCredentialServerID || conditions[1].Value != obj.ResourceMeta().Name { | ||
| return &store.PreconditionError{Reason: "dependent query must be scoped to server mesh and ID"} | ||
| } |
|
|
||
| // GetByKey retrieves a resource by its unique key | ||
| func (gs *GormStore) GetByKey(key string) (item interface{}, exists bool, err error) { | ||
| if store.IsVersionedResourceKind(gs.kind) { |
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
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.
close: #1576
feat(mcp): add operation contracts and parameter binding