terrymanu commented on PR #39001:
URL: https://github.com/apache/shardingsphere/pull/39001#issuecomment-4884609178
### Summary
**Review Result: Mergeable**
Reason: Code-scope review only. The PR strengthens `next_actions` example
validation by aligning required example fields with the existing next-action
contract, updates the affected test fixture, and adds focused regression
coverage. CI not reviewed by request.
### Evidence
-
`mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/descriptor/MCPToolOutputSchemaValidator.java`
now defines required fields per next action type and validates each concrete
`next_actions` example before accepting the descriptor.
- The required-field set matches the existing guidance contract in
`MCPGuidancePayloadBuilder`: `tool_call` requires `tool_name` and `arguments`;
`completion` requires `ref` and `argument`; resume fields remain optional.
-
`mcp/support/src/test/java/org/apache/shardingsphere/mcp/support/descriptor/MCPDescriptorCatalogValidatorTest.java`
adds a direct regression case for a `tool_call` example missing `arguments`.
-
`mcp/support/src/test/resources/META-INF/shardingsphere-mcp/mcp-descriptors/mcp-descriptor-test-planning.yaml`
updates the affected planning example to include the required `tool_name` and
`arguments` fields.
### Review Details
- Review Focus: Code Correctness Review. CI not reviewed by request.
- Reviewed Scope: `mcp/support` descriptor validation changes in 3 files.
Latest PR head `f8d8df40b7496f1b90e71a0a3df3d3f6751c40cd`; base ref `master` at
`f73b57cae98c8c9d3aa80b70149a87c6c6caaa58`; local merge-base
`9058dfdeed5dde51504f768a5dd8f5cc64f2a2a5`. Local triple-dot file list matched
GitHub `/pulls/39001/files`.
- Not Reviewed Scope: GitHub Actions/check-runs, full repository build, and
unrelated MCP runtime behavior outside the changed validator/resource fixture.
- Verification:
- `./mvnw -pl mcp/support -Dtest=MCPDescriptorCatalogValidatorTest
-DskipITs -Djacoco.skip=true test -Pcheck -T1C` exited 0.
- `./mvnw -pl mcp/support -Dtest=MCPDescriptorCatalogLoaderTest -DskipITs
-Djacoco.skip=true test -Pcheck -T1C` exited 0.
- `./mvnw -pl mcp/support checkstyle:check -Pcheck -T1C` exited 0.
- `./mvnw -pl mcp/support spotless:check -Pcheck -T1C` exited 0.
- GitHub PR metadata, changed files, commits, issue comments, review
comments, and reviews were accessible through public REST data; no public PR
comments or reviews were present.
- Release Note / User Docs: Not required. This is an internal descriptor
validation hardening change with focused test/resource updates and no
user-facing configuration, migration, or compatibility contract change.
--
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]
