terrymanu commented on PR #39164: URL: https://github.com/apache/shardingsphere/pull/39164#issuecomment-5000035989
### Summary **Review Result: Mergeable** **Reason:** Excluding `AGENTS.md` as requested, no merge-blocking issues were found within the MCP code scope. The Cluster-mode precondition check, failure guidance, descriptors, unit tests, and Proxy E2E contract form a complete loop. This conclusion is limited to code scope; CI not reviewed by request. ### Evidence - `AlterReadwriteSplittingStorageUnitStatusExecutor` explicitly requires Cluster mode, and the planning-stage check is consistent with the underlying execution constraint. - Standalone or unknown modes return `WF-MODE-002`, generate no artifacts, and return a terminal next action. - Cluster, Standalone, empty-query-result, and guidance branches are covered by tests. - All tests in the `mcp/support` and `readwrite-splitting` modules pass, with no Checkstyle violations. - No blocking issues were found in public APIs, dependencies, performance, state cleanup, or documentation contracts. ### Suggestions The following are non-blocking suggestions and can be considered later: - Add a mixed-case `mode_type` test to explicitly protect the `equalsIgnoreCase` behavior. - Add a test that reuses a `plan_id` after switching from Cluster to Standalone, verifying that old artifacts are cleared; the existing `clearPlanningState()` implementation already provides this protection. - The current `CLUSTER_MODE` constant and `queryProxyMode` placement are simple and clear. No additional abstraction is recommended. ### Review Details - **Review Focus:** Code Correctness Review. CI not reviewed by request. - **Reviewed Scope:** MCP readwrite-splitting, MCP support, and MCP E2E code. - **Not Reviewed Scope:** `AGENTS.md`, GitHub Actions, CI checks, and workflow logs. - **Verification:** MCP support and readwrite-splitting tests passed; Checkstyle passed; the corresponding Proxy E2E scenario passed. - **Release Note / User Docs:** No release note is required; the MCP descriptor and prompt were updated consistently. -- 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]
