terrymanu commented on PR #38807: URL: https://github.com/apache/shardingsphere/pull/38807#issuecomment-4622337776
### Decision - **Merge Verdict: Mergeable** - **Reviewed Scope:** Latest PR head `08f5bafeee893048369df4ab612849ed3374ac02`; local merge-base `6dd43c31873c6d1fb15cc1b2ab575e5673702def`; GitHub `/pulls/38807/files` matched the local triple-dot file list after status normalization, 8/8 files. Reviewed `docs/document/content/user-manual/shardingsphere-mcp/_index.en.md`, `_index.cn.md`, `deployment.en.md`, `deployment.cn.md`, `quick-start.en.md`, `quick-start.cn.md`, `troubleshooting.en.md`, and `troubleshooting.cn.md`. - **Not Reviewed Scope:** GitHub Actions/check-runs, live MCP runtime connected to a real AI client/database, and the broader future implementation discussion in #35294 beyond this documentation-focused update. - **Need Expert Review:** Not required. A docs/product-owner wording pass is optional. ### Basis - The PR is docs-only and matches its stated scope: it adds deployment health checks, basic observability entrypoints, and navigation links without changing runtime code, API/SPI contracts, parser behavior, or dependencies. - The health-check sequence in `docs/document/content/user-manual/shardingsphere-mcp/deployment.en.md:97` separates endpoint reachability, MCP protocol readiness, and runtime database readiness, which directly addresses post-startup verification rather than only saying the port is open. - The referenced runtime resource and preflight tool are backed by the current MCP descriptor and handler surface: `mcp/core/src/main/resources/META-INF/shardingsphere-mcp/mcp-descriptor-core.yaml:34`, `mcp/core/src/main/resources/META-INF/shardingsphere-mcp/mcp-descriptor-core.yaml:798`, and `mcp/core/src/main/java/org/apache/shardingsphere/mcp/core/resource/handler/capability/RuntimeStatusHandler.java:46`. - The runtime protection wording is consistent with the existing payload fields for row limits, timeout limits, and session tool-call limits in `mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/security/MCPRuntimeProtectionPolicy.java:74`. - The cross-page links are consistent with the MCP manual structure: the index points users to deployment diagnostics, quick start links forward to deployment health checks, and troubleshooting points users back to the post-deployment checks before symptom diagnosis (`docs/document/content/user-manual/shardingsphere-mcp/_index.en.md:37`, `docs/document/content/user-manual/shardingsphere-mcp/quick-start.en.md:86`, `docs/document/content/user-manual/shardingsphere-mcp/troubleshooting.en.md:9`). - The linked issue #35294 is a broad MCP ideas/design thread. This PR uses `For #35294` rather than claiming to close it, and the implementation scope is appropriately limited to deployment/testing guidance. ### Verification - Fetched PR metadata, linked issue context, GitHub file list, review comments, reviews, and PR conversation comments through the GitHub REST API; no GitHub-visible prior review comments or review rounds were present. - Fetched `apache/master` and `refs/pull/38807/head`, computed the merge base, and verified the local changed-file paths matched GitHub `/pulls/38807/files`. - Ran static reference checks with `rg` for `shardingsphere://runtime`, `database_gateway_validate_proxy_connectivity`, runtime protection, logging, and MCP configuration terms. - Verified linked target pages exist in the PR head for deployment, troubleshooting, and developer appendix pages in both English and Chinese. - Rendered the PR-head documentation with `hugo --source <temp>/docs/document --destination <temp>/public`; exit code 0. The render emitted only preexisting Hugo theme deprecation warnings, not failures from these changes. -- 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]
