terrymanu commented on PR #38851: URL: https://github.com/apache/shardingsphere/pull/38851#issuecomment-4705245460
### Summary Thanks for the update. I re-reviewed the latest PR head `ef99bee28c0365655b450b2f477ac6efb0a6ec7c`. **Merge Decision: Mergeable** ### Reason The previous model-facing contract issue is fixed: `mcp/core/src/main/resources/META-INF/shardingsphere-mcp/mcp-descriptors/mcp-descriptor-core.yaml` now includes `storage_unit` in the `objectType` output description, matching the actual search result behavior covered by `SearchMetadataToolHandlerTest`. The PR keeps the implementation scoped to MCP metadata exposure. It reuses existing DistSQL for Proxy-backed storage unit and single-table metadata, avoids expanding storage unit search by default, redacts sensitive storage unit properties, and includes focused unit tests plus Proxy-backed E2E contract coverage. ### Reviewed Scope Reviewed latest PR head `ef99bee28c0365655b450b2f477ac6efb0a6ec7c` against `apache/master` with merge base `f28d59023c301d8647d74c034dd2cfe0166daacb`. GitHub changed files and local triple-dot diff both contain 29 files. Reviewed MCP core/support/bootstrap changes, MCP E2E contract updates, and MCP user docs. ### Not Reviewed Scope I did not use GitHub Actions/check-run status as merge evidence. I also did not run a full Docker/Proxy E2E environment manually. ### Verification Local verification passed: ```shell ./mvnw -pl mcp/api,mcp/support,mcp/core,mcp/features/encrypt,mcp/features/mask,mcp/bootstrap,mcp/registry -am install -DskipITs -Dspotless.skip=true -Dsurefire.failIfNoSpecifiedTests=false -B -ntp -- 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]
