terrymanu commented on PR #39030:
URL: https://github.com/apache/shardingsphere/pull/39030#issuecomment-4900950503
### Summary
**Review Result: Mergeable**
Reason: Code-scope only; CI was not reviewed by request. The latest PR scope
is small and coherent: it stabilizes one MCP model-facing fallback diagnostic
and expands the MCP unit-test workflow module list to cover all current MCP
feature modules, with focused test coverage and local verification passing.
### Evidence
- The model-facing elicitation fallback reason now avoids release-scoped
wording and states the stable runtime boundary: URL mode is not implemented by
the MCP runtime. The fallback reason code remains unchanged, so clients still
receive the existing structured `fallback_reason` values.
- `MCPToolElicitationFallbackResponseFactoryTest` now directly asserts the
emitted `next_actions.reason`, so the changed diagnostic is covered on the
sensitive fallback path.
- `ci-mcp.yml` now lists all six MCP feature modules present under
`mcp/features`: `broadcast`, `encrypt`, `mask`, `readwrite-splitting`,
`shadow`, and `sharding`.
- Local verification of the workflow Maven command showed all six MCP
feature modules entering the reactor and the build completed successfully.
### Review Details
- Review Focus: Code Correctness Review; CI not reviewed by request.
- Reviewed Scope: apache/shardingsphere#39030 latest head
`30e30bb2fee3e059d58a4954cd8994edfca87226`, base ref `master` at
`aef7d13c0a39570e31b6b38a70239df86ec4636d`, local merge-base
`c46e5a9de895875b346b6249bc2e0acba7d90121`.
- File list match status: matched. GitHub `/pulls/39030/files` reported 3
files, and local triple-dot diff reported the same 3 files:
- `.github/workflows/ci-mcp.yml`
-
`mcp/bootstrap/src/main/java/org/apache/shardingsphere/mcp/bootstrap/transport/capability/tool/MCPToolElicitationFallbackResponseFactory.java`
-
`mcp/bootstrap/src/test/java/org/apache/shardingsphere/mcp/bootstrap/transport/capability/tool/MCPToolElicitationFallbackResponseFactoryTest.java`
- Not Reviewed Scope: GitHub Actions/check-run status and logs, because this
was a Code Correctness Review; broader MCP runtime behavior outside the touched
elicitation fallback path; unrelated workflows.
- Verification:
- `./mvnw -pl mcp/bootstrap
-Dtest=MCPToolElicitationFallbackResponseFactoryTest,MCPToolElicitationFlowTest,MCPToolElicitationFallbackReasonTest
test -DskipITs -B -ntp`: exit 0.
- `./mvnw -pl
mcp/api,mcp/support,mcp/core,mcp/features/broadcast,mcp/features/encrypt,mcp/features/mask,mcp/features/readwrite-splitting,mcp/features/shadow,mcp/features/sharding,mcp/bootstrap,mcp/registry
-am install -DskipITs -Dspotless.skip=true
-Dsurefire.failIfNoSpecifiedTests=false -B -ntp`: exit 0.
- Release Note / User Docs: not required. The PR changes an
internal/model-facing diagnostic wording and PR unit-test workflow coverage; it
does not change user configuration, runtime behavior, public API, migration
requirements, or release-facing feature semantics.
--
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]
