terrymanu commented on PR #39017: URL: https://github.com/apache/shardingsphere/pull/39017#issuecomment-4888670337
### Summary **Review Result: Mergeable** Reason: This is a code-scope result only; CI not reviewed by request. I did not find a code-level blocker in the latest PR scope. ### Evidence - `mcp/core/src/main/java/org/apache/shardingsphere/mcp/core/completion/provider/MetadataCompletionProvider.java` now treats `storageUnit` as a supported completion argument, requires only `database` context for it, reuses single-database defaulting, and points empty/missing cases at the nearest storage-unit resource path. - The storage-unit completion path reuses `GovernanceMetadataQueryService.queryStorageUnits(...)`, so it stays aligned with the existing resource/search DistSQL path and existing storage-unit redaction behavior. - `MetadataCompletionProviderTest` covers direct `storageUnit` completion, single-database defaulting, and missing database context. `GovernanceMetadataQueryServiceTest` remains focused on the governance metadata query/redaction contract after the package move. - Sharding and Shadow descriptor annotation titles are exposed through descriptor catalog loading and covered by new catalog-level contract tests. - The protocol docs and `mcp/server.json` changes match the implemented public MCP surface: `storage_unit` is documented for metadata search, storage unit completion is documented, and registry wording now reflects controlled SQL plus rule workflows. ### Review Details - Review Focus: Code Correctness Review. CI not reviewed by request. - Reviewed Scope: PR #39017 latest head `b509bb4b1d3115d1cc27203cc9293717b5e1a8b2` against base `515df2adbf78bd72c6531617bb2bf153d68576b4`; local merge-base `515df2adbf78bd72c6531617bb2bf153d68576b4`. The local triple-dot file list matched GitHub `/pulls/39017/files` for all 14 changed files. - Not Reviewed Scope: GitHub Actions, check-runs, workflow logs, and full repository runtime behavior outside the changed MCP completion/resource/search descriptor surface. - Verification: Fetched PR metadata/files/comments/reviews from public GitHub data; issue comments, review comments, and reviews were all empty at the reviewed head. Fetched the PR ref locally successfully and reviewed the latest diff plus related call paths. I did not run local Maven tests; this result is based on static code/test/descriptor review of the latest PR head. - Release Note / User Docs: User-facing MCP protocol documentation and server registry description are updated in this PR. No separate release note is required for this scoped MCP contract alignment. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
