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/06/30 15:25:31 UTC
[PR] Refine MCP collection copy rules and usages [shardingsphere]
terrymanu opened a new pull request, #38972:
URL: https://github.com/apache/shardingsphere/pull/38972
- Remove redundant collection copies in MCP validators, capability provider, guidance payload builder, and tests
- Tighten AGENTS.md guidance against semantics-free collection copies
- Trim low-signal agent guidance sections
For #35294
--
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] Refine MCP collection copy rules and usages [shardingsphere]
Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu commented on PR #38972:
URL: https://github.com/apache/shardingsphere/pull/38972#issuecomment-4845296413
### Summary
**Review Result: Mergeable**
Reason: Code-scope review only. The latest PR head removes redundant MCP collection copies without changing ownership or mutation semantics, tightens the agent rule that prevents the same pattern from recurring, and keeps the remaining literal-backed mutable collections only where the tests mutate those instances. CI not reviewed by request.
### Evidence
- The PR scope is 8 files and matches GitHub `/pulls/38972/files`: `AGENTS.md`, 3 MCP production files, and 4 MCP test files.
- `MCPDescriptorCatalogValidator` now reuses the already-populated `registeredTemplateVariables` set for membership checks, and uses the URI template variable list directly where only `contains` is needed.
- `MCPDatabaseCapabilityProvider` now uses the owned `Map` returned by `MCPJdbcDatabaseProfileLoader#load`; that loader constructs and returns a fresh `LinkedHashMap`.
- `WorkflowGuidancePayloadBuilder#addSequencing` is private and is called with a local `nextActions` list created in `appendApplyGuidance`, so removing the shallow list copy preserves behavior.
- Test changes pass immutable literals only through paths that copy or iterate inputs before mutation; the mutable fixture collections that are later modified remain unchanged.
- Local verification passed:
- `./mvnw -pl mcp/support,mcp/core -DskipITs -Dspotless.skip=true -Dtest=MCPDescriptorCatalogValidatorTest,MCPDatabaseCapabilityProviderTest,WorkflowGuidancePayloadBuilderTest,WorkflowContextSnapshotTest,WorkflowProxyQueryServiceTest,MCPCompletionServiceTest,SearchMetadataToolServiceTest -Dsurefire.failIfNoSpecifiedTests=false test` exited 0.
- `./mvnw spotless:check -Pcheck -T1C` exited 0.
- `./mvnw checkstyle:check -Pcheck -T1C` exited 0.
### Review Details
- Review Focus: Code Correctness Review; CI not reviewed by request.
- Reviewed Scope: PR #38972, latest head `742a97ef4a4fe0689c0e4b123ea6b93269989167`, base `f6bcc6691090a4c7f5b43ba272497b5cc88d3444`, merge-base `f6bcc6691090a4c7f5b43ba272497b5cc88d3444`. Local triple-dot file list matched GitHub `/pulls/38972/files`.
- Not Reviewed Scope: GitHub Actions/check-runs, unrelated MCP modules outside the changed files, and broader AGENTS.md policy redesign beyond the PR diff.
- Verification: Local targeted MCP tests, Spotless check, and Checkstyle check all passed with exit code 0.
- Release Note / User Docs: Not required. The PR is internal guidance cleanup plus behavior-preserving MCP/test simplification; it does not change user-facing runtime behavior or configuration.
--
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] Refine MCP collection copy rules and usages [shardingsphere]
Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu merged PR #38972:
URL: https://github.com/apache/shardingsphere/pull/38972
--
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