You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by "terrymanu (via GitHub)" <gi...@apache.org> on 2026/07/08 07:40:19 UTC
[PR] Refine MCP workflow planning boundaries [shardingsphere]
terrymanu opened a new pull request, #39054:
URL: https://github.com/apache/shardingsphere/pull/39054
- extract planning context validation and algorithm requirement collection
- validate nested MCP input schema fields
- add focused workflow and sharding kernel tests
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine MCP workflow planning boundaries [shardingsphere]
Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu merged PR #39054:
URL: https://github.com/apache/shardingsphere/pull/39054
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Refine MCP workflow planning boundaries [shardingsphere]
Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu commented on PR #39054:
URL: https://github.com/apache/shardingsphere/pull/39054#issuecomment-4912776009
### Summary
**Review Result: Mergeable**
Reason: Code-scope review only. The latest PR head keeps the workflow-planning refactor within the MCP support/sharding boundary, preserves the existing `WorkflowPlanningSupport` behavior through focused delegate classes, adds direct tests for the new public helpers, and passes focused local verification. CI not reviewed by request.
### Evidence
- The PR splits the previous `WorkflowPlanningSupport` responsibilities without changing the public support entry points: `WorkflowPlanningSupport` now delegates planning-context checks and algorithm requirement collection through `WorkflowPlanningContextValidator` and `WorkflowAlgorithmRequirementCollector`, while keeping the same externally used methods in place (`mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/workflow/service/WorkflowPlanningSupport.java`).
- The extracted context validator still covers the same planning path: missing database, identifier validation, schema inference, table lookup, and column lookup are preserved in `WorkflowPlanningContextValidator.ensurePlanningContext` (`mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/workflow/service/WorkflowPlanningContextValidator.java`).
- The extracted algorithm requirement collector preserves the prior behavior for blocking algorithm issues, default property application, missing required properties, and readiness gating (`mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/workflow/service/WorkflowAlgorithmRequirementCollector.java`).
- The nested MCP input-schema validation is covered by a focused negative test for unsupported nested fields, and the descriptor catalog tests still run successfully (`mcp/support/src/test/java/org/apache/shardingsphere/mcp/support/descriptor/MCPDescriptorCatalogValidatorTest.java`).
- The new public helper classes have direct focused tests, and existing wrapper-level `WorkflowPlanningSupportTest` still exercises the production-facing support API. The added sharding kernel test covers the missing-database clarification path and cleanup drop-only validation (`mcp/support/src/test/java/org/apache/shardingsphere/mcp/support/workflow/service/WorkflowPlanningContextValidatorTest.java`, `mcp/support/src/test/java/org/apache/shardingsphere/mcp/support/workflow/service/WorkflowAlgorithmRequirementCollectorTest.java`, `mcp/features/sharding/src/test/java/org/apache/shardingsphere/mcp/feature/sharding/tool/service/ShardingWorkflowPlanningKernelTest.java`).
- No substantive unrelated changes were found in the GitHub changed-file scope. No release note or user documentation is required because this is an internal MCP workflow-planning boundary/refactor plus validation/test coverage change, with no user-facing configuration, protocol, SQL syntax, migration, or compatibility behavior change identified.
### Review Details
- Review Focus: Code Correctness Review. CI not reviewed by request.
- Reviewed Scope: PR `apache/shardingsphere#39054`, latest head SHA `3d180810e60fbf0b1004c2bf1deaec25eaeda402`, base/merge-base SHA `2e4bc504ed792cee62ae56fbb511c11c5bbbf470`. GitHub `/pulls/39054/files` listed 8 files, and the local triple-dot file list matched GitHub.
- Reviewed Files:
- `mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/descriptor/MCPToolDescriptorCatalogValidator.java`
- `mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/workflow/service/WorkflowAlgorithmRequirementCollector.java`
- `mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/workflow/service/WorkflowPlanningContextValidator.java`
- `mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/workflow/service/WorkflowPlanningSupport.java`
- `mcp/support/src/test/java/org/apache/shardingsphere/mcp/support/descriptor/MCPDescriptorCatalogValidatorTest.java`
- `mcp/support/src/test/java/org/apache/shardingsphere/mcp/support/workflow/service/WorkflowAlgorithmRequirementCollectorTest.java`
- `mcp/support/src/test/java/org/apache/shardingsphere/mcp/support/workflow/service/WorkflowPlanningContextValidatorTest.java`
- `mcp/features/sharding/src/test/java/org/apache/shardingsphere/mcp/feature/sharding/tool/service/ShardingWorkflowPlanningKernelTest.java`
- Not Reviewed Scope: GitHub Actions/check-runs/logs were not reviewed by request. Full MCP CI, production Proxy/container E2E, and unrelated local untracked files were not reviewed.
- Verification:
- `./mvnw -pl mcp/support,mcp/features/sharding -am -DskipITs -Dspotless.skip=true -Dsurefire.failIfNoSpecifiedTests=false -B -ntp test` exited `0`; `mcp-support` reported 573 tests with 0 failures/errors/skips, and `mcp-feature-sharding` reported 124 tests with 0 failures/errors/skips.
- `./mvnw -pl mcp/support,mcp/features/sharding -am -Pcheck -DskipTests -DskipITs -Dspotless.skip=true -B -ntp checkstyle:check` exited `0`.
- `./mvnw -pl mcp/support,mcp/features/sharding -Pcheck -DskipTests -DskipITs -B -ntp spotless:check` exited `0`.
- GitHub PR metadata, changed files, comments, and reviews were accessible through the public GitHub REST API. No review comments or reviews were present at the time of review. No inaccessible GitHub endpoint affects this code-correctness result.
- Release Note / User Docs: Not required; the PR is internal MCP workflow-planning refactor/validation/test work and does not introduce a user-facing feature, configuration change, compatibility requirement, migration path, or troubleshooting behavior requiring release documentation.
--
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: notifications-unsubscribe@shardingsphere.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org