terrymanu commented on PR #39029:
URL: https://github.com/apache/shardingsphere/pull/39029#issuecomment-4900856399
### Summary
Review Result: Mergeable
Reason: The PR consistently refines MCP HTTP auth-boundary wording across
the production model-facing payload, the guidance baseline contract, and the
MCP builder evaluation artifact. I did not find a correctness regression or
contract mismatch.
### Evidence
- `MCPClientSafetyPolicy.createModelFacingPayload()` now states that the MCP
runtime does not provide built-in authentication or authorization while
preserving the existing trusted session attribution and STDIO boundary wording.
- `guidance.yaml` was updated with the same
`security_hints.client_safety_policy.transport_scope` wording, and the baseline
projection test compares the generated guidance payload against this normalized
baseline.
- `mcp-builder-evaluation.xml` q10 was updated to evaluate both
authentication and authorization absence, matching the refined boundary.
- Search on the latest PR head found no remaining old `in this release`
auth-boundary wording under `mcp` or `test/e2e/mcp`.
### Review Details
- Review Focus: Code Correctness Review; CI not reviewed by request.
- Reviewed scope: latest PR head `f939a19d46f30e0d3cfcfe887b85940c8f7578c5`
for apache/shardingsphere#39029.
- Local merge-base SHA: `c46e5a9de895875b346b6249bc2e0acba7d90121`.
- File list match status: matched. GitHub PR files API reported 3 files, and
local three-dot diff reported the same 3 files:
-
`mcp/support/src/main/java/org/apache/shardingsphere/mcp/support/security/MCPClientSafetyPolicy.java`
-
`test/e2e/mcp/src/test/resources/baseline-contract/model-contract/guidance.yaml`
-
`test/e2e/mcp/src/test/resources/llm/evaluation/mcp-builder-evaluation.xml`
- Not reviewed scope: CI/check runs/workflow logs, runtime HTTP deployment
behavior beyond the changed wording, unrelated MCP modules, and files outside
the PR diff.
- Verification:
- `git diff --check
c46e5a9de895875b346b6249bc2e0acba7d90121...f939a19d46f30e0d3cfcfe887b85940c8f7578c5
-- <changed files>`: exit 0.
- `./mvnw -pl mcp/support
-Dtest=MCPClientSafetyPolicyTest,MCPGuidancePayloadBuilderTest test -DskipITs
-B -ntp`: exit 0.
- `./mvnw -pl test/e2e/mcp -am
-Dtest=HttpTransportBaselineContractE2ETest#assertGuidanceBaselineContractProjection,MCPBuilderEvaluationArtifactTest
-Dsurefire.failIfNoSpecifiedTests=false test -DskipITs -B -ntp`: exit 0.
- Release Note / User Docs: No release note or user docs update required;
this is wording clarification for model-facing guidance/evaluation text and
does not change MCP configuration keys, runtime behavior, or public API.
--
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]
