terrymanu commented on PR #38789:
URL: https://github.com/apache/shardingsphere/pull/38789#issuecomment-4611286703
### Decision
- **Merge Verdict: Mergeable**
- **Reviewed Scope:** PR #38789 latest head
`5485df23aaab832185c7f3d60133e2b2fdae3309`; base/merge-base
`a31579b10833d5714d234f358686e4fb222ea8e2`. GitHub `/pulls/38789/files` matched
local triple-dot diff: 16 modified ShardingSphere-MCP user docs files under
`docs/document/content/user-manual/shardingsphere-mcp/`, including
`capabilities`, `configuration`, `deployment`, `developer-appendix`,
`troubleshooting`, `client-integration/_index`, and `features/encrypt` /
`features/mask` in both English and Chinese.
- **Not Reviewed Scope:** CI/check-run status, full Hugo docs rendering,
non-MCP documentation, and live runtime execution against Proxy or direct
databases.
- **Need Expert Review:** No dedicated expert review required. No production
code, parser, dependency, protocol contract, or runtime security policy changed.
### Basis
- The docs now consistently separate ShardingSphere-Proxy from direct
database connections and preserve the Proxy-only rule boundaries for
encryption/masking. This matches the PR goal and the MCP feature behavior
(`docs/document/content/user-manual/shardingsphere-mcp/capabilities.en.md:28`,
`docs/document/content/user-manual/shardingsphere-mcp/configuration.en.md:109`,
`docs/document/content/user-manual/shardingsphere-mcp/features/encrypt.en.md:89`,
`docs/document/content/user-manual/shardingsphere-mcp/features/mask.en.md:77`).
- Runtime protection limits in the docs match the implementation: default
`max_rows=100`, maximum `5000`, timeout maximum `300000`, and session-level
tool-call quota
(`docs/document/content/user-manual/shardingsphere-mcp/capabilities.en.md:74`,
`mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/security/MCPRuntimeProtectionPolicy.java:30`).
- HTTP session attribution wording is accurate: the YAML fields and default
trusted-header names match the swapper, omitted configuration disables
attribution binding, and mismatch rejection only applies when attribution is
enabled and supplied for an existing session
(`docs/document/content/user-manual/shardingsphere-mcp/configuration.en.md:39`,
`mcp/bootstrap/src/main/java/org/apache/shardingsphere/mcp/bootstrap/config/yaml/swapper/YamlHttpTransportConfigurationSwapper.java:39`,
`mcp/bootstrap/src/main/java/org/apache/shardingsphere/mcp/bootstrap/transport/server/http/validator/ShardingSphereServerTransportSecurityValidator.java:61`).
- The new “list algorithms available from the current Proxy” examples are
backed by MCP resources that query Proxy DistSQL plugin lists for encrypt and
mask algorithms
(`docs/document/content/user-manual/shardingsphere-mcp/features/encrypt.en.md:23`,
`mcp/features/encrypt/src/main/java/org/apache/shardingsphere/mcp/feature/encrypt/resource/handler/EncryptAlgorithmsHandler.java:43`,
`mcp/features/mask/src/main/java/org/apache/shardingsphere/mcp/feature/mask/resource/handler/MaskAlgorithmsHandler.java:43`).
- Docker guidance aligns with the entrypoint and HTTP bind defaults: custom
config is supported by `SHARDINGSPHERE_MCP_CONFIG`, and container HTTP examples
correctly require an exposable bind host such as `0.0.0.0`
(`docs/document/content/user-manual/shardingsphere-mcp/deployment.en.md:34`,
`distribution/mcp/src/main/bin/docker-entrypoint.sh:28`,
`mcp/bootstrap/src/main/java/org/apache/shardingsphere/mcp/bootstrap/config/yaml/swapper/YamlHttpTransportConfigurationSwapper.java:33`).
- Risk scan found no substantive unrelated changes, no parser or SQL dialect
changes, no API/SPI or dependency changes, and no Proxy/JDBC high-frequency
execution-path change.
### Verification
- Reviewer-run boundary checks:
- `git fetch apache master:refs/remotes/apache/master
pull/38789/head:refs/remotes/apache/pr/38789` exited `0`.
- GitHub `/pulls/38789/files` and local `git diff --name-status
a31579b10833d5714d234f358686e4fb222ea8e2..5485df23aaab832185c7f3d60133e2b2fdae3309`
both showed the same 16 modified files.
- Reviewer-run review checks:
- GitHub issue comments, PR review comments, and PR reviews endpoints
returned no previous visible feedback, so `Multi-Round Comparison` is not
applicable.
- `rg` source/doc consistency checks covered session attribution, runtime
limits, algorithm resources, Docker config handling, Proxy-only feature
boundaries, and leftover misleading “regular/direct physical database” wording.
- `git diff --check
a31579b10833d5714d234f358686e4fb222ea8e2..5485df23aaab832185c7f3d60133e2b2fdae3309`
exited `0`.
- Maven, Spotless, and Checkstyle were not run locally because this was a
review-only docs change and no files were modified during review. CI/check-run
status was not used for the verdict.
--
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]
