terrymanu commented on PR #39067: URL: https://github.com/apache/shardingsphere/pull/39067#issuecomment-4932384557
### Correction Previous Finding: trailing whitespace reported from `git diff --check`. Current Status: Withdrawn. Reason: The repository formatting/style gates are Spotless and Checkstyle. `./mvnw spotless:apply -Pcheck -T1C` passes and leaves the blank-line indentation unchanged, and `./mvnw checkstyle:check -Pcheck -T1C` also passes. Therefore the earlier `git diff --check` whitespace request is not a valid merge blocker for this PR. ### Summary **Review Result: Mergeable** Reason: This is code-scope only; CI not reviewed by request. After rechecking the latest PR head and withdrawing the Spotless-stable whitespace concern, I found no remaining code-level blocker in the reviewed scope. ### Evidence - `mcp/support` now validates planning-tool annotations and has focused tests for invalid read-only, destructive, and idempotent hints. - `MCPNextActionUtils.ordered(Collection<...>)` preserves copy/order semantics, and `SearchMetadataPayloadBuilder` reuses it without changing payload behavior. - `approved_steps` is centralized through `WorkflowFieldNames.APPROVED_STEPS` while preserving the external protocol value. - MCP handler/resource tests keep collaborator replacement inside tests instead of adding production-only test constructors. - LLM usability failure summaries now include artifact directories, with focused test coverage. ### Review Details - Review Focus: Code Correctness Review. CI not reviewed by request. - Reviewed Scope: all 33 files from GitHub `/pulls/39067/files`, covering `AGENTS.md`, `mcp/support`, `mcp/core`, `mcp/bootstrap` tests, `mcp/features/*` tests, and `test/e2e/mcp`. - Latest reviewed head: `fac01113f57b1853f57b36beb89eec2953b900bf`; base/merge-base: `74ed32c6731878e6f1bddf6f14113b1bb7da7e14`; local file list matched GitHub. - Not Reviewed Scope: CI/check-runs, full `clean install`, runtime E2E against a real MCP server/proxy. - Verification: `./mvnw spotless:apply -Pcheck -T1C` exit 0; `./mvnw checkstyle:check -Pcheck -T1C` exit 0; scoped Maven tests for the changed MCP modules exit 0. GitHub metadata/files/comments/reviews were fetched via public REST; no required code-review endpoint was inaccessible. - Release Note / User Docs: not required; changes are internal MCP workflow/test-boundary and diagnostic refinements. -- 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]
