terrymanu commented on PR #38751: URL: https://github.com/apache/shardingsphere/pull/38751#issuecomment-4571831310
### Decision - **Merge Verdict: Mergeable** - **Reviewed Scope:** Reviewed PR #38751 latest head `104b2bc67c8d8d201facedb43b1f604901c3a9dd`; base `apache/master` `29c79a75903e148944b626038393ad6a10759b83`; local merge-base `29c79a75903e148944b626038393ad6a10759b83`. Local triple-dot diff matched GitHub `/pulls/38751/files` exactly, 24/24 files. Reviewed the ShardingSphere-MCP user manual changes under `docs/document/content/user-manual/shardingsphere-mcp/**`, plus MCP descriptor/runtime evidence from `mcp/**`, `distribution/mcp/**`, `mcp/server.json`, and MCP E2E helpers. - **Not Reviewed Scope:** I did not run a rendered documentation site build or a packaged MCP runtime curl smoke test. I did not inspect GitHub Actions/check-runs. - **Need Expert Review:** No mandatory security, concurrency, performance, SQL parser, or supply-chain expert review is required because this is a docs-only change. Normal MCP docs/runtime maintainer review is enough. ### Basis - The previous HTTP lifecycle gap is fixed. `docs/document/content/user-manual/shardingsphere-mcp/quick-start.en.md:67` now initializes with `protocolVersion`, sends `notifications/initialized` before the first normal MCP call, and documents the expected HTTP `202`; the same flow exists in the Chinese page. `docs/document/content/user-manual/shardingsphere-mcp/client-integration.en.md:21` now also describes the lifecycle as `initialize` -> keep session headers -> `notifications/initialized` -> later MCP calls. This matches the in-repo HTTP client behavior at `test/e2e/mcp/src/test/java/org/apache/shardingsphere/test/e2e/mcp/support/transport/client/MCPHttpInteractionClient.java:69` and `test/e2e/mcp/src/test/java/org/apache/shardingsphere/test/e2e/mcp/support/transport/client/MCPHttpInteractionClient.java:123`, and the initialize params at `test/e2e/mcp/src/test/java/org/apache/shardingsphere/test/e2e/mcp/support/transport/MCPInteractionProtocolSupport.java:41`. - The capability index is consistent with descriptor-backed MCP surface. The static resources and core SQL tools in `docs/document/content/user-manual/shardingsphere-mcp/capabilities.en.md:20` align with core descriptors such as `mcp/core/src/main/resources/META-INF/shardingsphere-mcp/mcp-descriptors/mcp-descriptor-core.yaml:17`, `mcp/core/src/main/resources/META-INF/shardingsphere-mcp/mcp-descriptors/mcp-descriptor-core.yaml:798`, and `mcp/core/src/main/resources/META-INF/shardingsphere-mcp/mcp-descriptors/mcp-descriptor-core.yaml:952`. - The workflow documentation matches the shared workflow descriptor. `docs/document/content/user-manual/shardingsphere-mcp/workflow.en.md:42` documents `preview`, `review-then-execute`, and `manual-only`, matching `mcp/support/src/main/resources/META-INF/shardingsphere-mcp/mcp-descriptors/mcp-descriptor-support.yaml:163`. - The feature plugin pages align with the packaged distribution and feature descriptors. The docs list encrypt/mask as packaged feature plugins at `docs/document/content/user-manual/shardingsphere-mcp/features/_index.en.md:9`, and `distribution/mcp/pom.xml:34` includes bootstrap, encrypt, mask, and MySQL runtime dependencies. Encrypt and mask public surfaces match their descriptor resources/prompts/tools at `mcp/features/encrypt/src/main/resources/META-INF/shardingsphere-mcp/mcp-descriptors/mcp-descriptor-encrypt.yaml:17` and `mcp/features/mask/src/main/resources/META-INF/shardingsphere-mcp/mcp-descriptors/mcp-descriptor-mask.yaml:17`. - Deployment and configuration docs are consistent with runtime assets. `docs/document/content/user-manual/shardingsphere-mcp/configuration.en.md:6` and `docs/document/content/user-manual/shardingsphere-mcp/deployment.en.md:24` match `distribution/mcp/src/main/bin/start.sh:22`, `distribution/mcp/src/main/bin/docker-entrypoint.sh:28`, and `mcp/server.json:13`. - Fresh-pass risk scan found no unresolved risk: no production code, dependency, parser, routing, metadata assembly, API/SPI, or high-frequency execution path changed. The adversarial pass checked adjacent HTTP/STDIO and encrypt/mask documentation paths, config/default transport paths, and the original lifecycle symptom path; all are covered by the latest docs or existing runtime evidence. ### Verification - `git fetch apache master:refs/remotes/apache/master pull/38751/head:refs/remotes/apache/pr/38751`: passed, with only an unrelated local auto-gc warning. - GitHub PR metadata, `/files`, reviews, review comments, and issue comments were checked; GitHub-visible reviews/comments are empty, so no `Multi-Round Comparison` section is needed. - Local/GitHub file-list parity check: passed, 24/24 files matched. - `./mvnw spotless:check -Pcheck -T1C`: passed. - `./mvnw checkstyle:check -Pcheck -T1C`: passed. - `git diff --check refs/remotes/apache/master...refs/remotes/apache/pr/38751`: passed. - Targeted `rg`/`nl` inspections for `protocolVersion`, `notifications/initialized`, descriptor names, runtime config, distribution scripts, and feature plugin surfaces: passed. -- 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]
