terrymanu commented on PR #39142: URL: https://github.com/apache/shardingsphere/pull/39142#issuecomment-4985279557
### Summary **Review Result: Mergeable** Reason: No code-level blockers were found in this review. The PR correctly completes the MCP request context hierarchy refactor. This result applies only to code scope; CI was not reviewed by request. ### Evidence - [MCPFeatureRequestContext.java#L35](https://github.com/apache/shardingsphere/blob/8cd357f76d4d428ce71011eaa892f83bfd56a234/mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/MCPFeatureRequestContext.java#L35) merges the former database and workflow extension contracts while continuing to extend the API-level `MCPRequestContext`. - [MCPFeatureRuntimeRequestContext.java#L44](https://github.com/apache/shardingsphere/blob/8cd357f76d4d428ce71011eaa892f83bfd56a234/mcp/core/src/main/java/org/apache/shardingsphere/mcp/core/context/MCPFeatureRuntimeRequestContext.java#L44) preserves the original request-level session, transport, database, metadata, execution, query, and workflow state while renaming the implementation type. - [MCPRequestContextTypes.java#L44](https://github.com/apache/shardingsphere/blob/8cd357f76d4d428ce71011eaa892f83bfd56a234/mcp/core/src/main/java/org/apache/shardingsphere/mcp/core/handler/MCPRequestContextTypes.java#L44) correctly narrows the runtime-supported context types to the API base contract and the support extension contract. - Resource/tool registries still create and pass the same runtime context from core. Handler SPIs, descriptors, transports, session lifecycle, and protocol error paths are unchanged in substance. - Feature modules already depended on `mcp/support`; the PR does not modify the POMs, and the MCP SDK remains isolated in bootstrap. - The Chinese and English MCP developer documentation was updated consistently: [mcp.en.md#L83](https://github.com/apache/shardingsphere/blob/8cd357f76d4d428ce71011eaa892f83bfd56a234/docs/document/content/dev-manual/mcp.en.md#L83). - The GitHub file list matches the local `base...head` scope: 135 files, head `8cd357f76d4d428ce71011eaa892f83bfd56a234`, merge-base `26cd31495edabefd41b7b47365374ace2e61fda3`. ### Review Details - Review Focus: Code Correctness Review; CI not reviewed by request. - Reviewed Scope: All 135 files changed by PR #39142, covering MCP core, support, features, bootstrap tests, and the Chinese and English MCP developer documentation. - Not Reviewed Scope: GitHub Actions, check runs, and CI status. - Verification: - `./mvnw -f mcp/pom.xml -T1C test`: exit 0, `BUILD SUCCESS` - `./mvnw -f mcp/pom.xml spotless:check checkstyle:check -Pcheck -T1C`: exit 0, 0 Checkstyle violations - Legacy context type reference scan: no remaining references - POM changes: none - Release Note / User Docs: User documentation was updated. No additional migration note is required because this change does not alter the protocol, configuration, or runtime behavior. -- 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]
