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/14 05:16:23 UTC
[PR] Refactor MCP execution result and response boundaries [shardingsphere]
terrymanu opened a new pull request, #39094:
URL: https://github.com/apache/shardingsphere/pull/39094
- separate runtime validation and SQL execution facts from MCP payload mapping
- clarify MCPMapResponse usage and execute-update ResultSet schema
- prevent automatic replay of truncated side-effecting SQL
--
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] Refactor MCP execution result and response boundaries [shardingsphere]
Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu commented on PR #39094:
URL: https://github.com/apache/shardingsphere/pull/39094#issuecomment-4966230392
### Summary
**Review Result: Mergeable**
Reason: This is a code-scope-only result. No P0, P1, or P2 code issues were found at the latest head; CI not reviewed by request.
### Evidence
- The root cause is addressed: the support layer now contains runtime-validation and SQL-execution facts, while the core layer owns MCP responses, recovery, and payload mapping. This removes the previous mixed responsibilities and reverse callback dependency.
- `RuntimeDatabaseValidationResponse` now has consistent naming and layering. Validation diagnostics remain `isError=false`, while actual runtime exceptions remain `isError=true`.
- `SQLExecutionResponse` now contains only protocol-boundary behavior. Execution data and construction logic moved to `SQLExecutionResult`, while query, explain, DML, DDL/DCL, transaction, and savepoint behavior remains consistent.
- The `execute_update` ResultSet payload, descriptor, and structured-content validation are aligned. Truncated side-effecting statements now return a terminal action instead of suggesting automatic replay.
- `MCPMapResponse` behavior is unchanged, and its boundary as a terminal payload adapter that does not copy, merge, or validate payloads is now explicit.
- All in-repository consumers of the shared SPI were migrated and compiled successfully. The MCP modules were introduced after the 5.5.3 release, so this does not break a released API.
- No stale references remain to the old response package, result types, response wither methods, or runtime-validation sentinel.
### Review Details
- Review Focus: Code Correctness Review — CI not reviewed by request.
- Reviewed Scope: Latest head `b8183be1935be52e6e38d3072d7d9938f6ba12e8`; merge-base `0b523bd47978445cd759aab596f8349d59646a06`. The local triple-dot diff exactly matched the 49 files returned by GitHub `/pulls/39094/files`. All 48 MCP files were reviewed; `AGENTS.md` was excluded as requested.
- Not Reviewed Scope: `AGENTS.md`; GitHub Actions, check runs, and workflow logs; full MCP E2E and external database execution.
- Verification:
- 21 focused test classes across support, core, and bootstrap: exit 0; 234 tests, 0 failures, 0 errors.
- `./mvnw -f mcp/pom.xml -DskipTests -Dspotless.skip=true -Dcheckstyle.skip=true package`: exit 0; the complete 13-module MCP reactor and all six feature modules compiled successfully.
- GitHub metadata was refreshed before completion. The head SHA was unchanged, with no public reviews, review comments, or issue comments.
- Release Note / User Docs: No additional release note or migration documentation is required. This refactors the unreleased 5.5.4-SNAPSHOT MCP implementation, and the model-visible contract is updated in the MCP descriptor.
--
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] Refactor MCP execution result and response boundaries [shardingsphere]
Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu merged PR #39094:
URL: https://github.com/apache/shardingsphere/pull/39094
--
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