terrymanu commented on PR #38714: URL: https://github.com/apache/shardingsphere/pull/38714#issuecomment-4532375273
Decision Merge Verdict: Mergeable Reviewed Scope: PR latest head 62de2f3f6b270dbf1968bc817577dbea55a063c2, mainly mcp/core SQL classification and cross-schema guard, mcp/support workflow/descriptor updates, MCP E2E test refactoring, and docs/README contract updates. Not Reviewed Scope: I did not line-by-line review every detail across all 56 changed files; the review focused on the main runtime and regression-risk paths. Need Expert Review: Not required. If the community wants product/security ownership confirmation for renaming “audit” to “SQL execution trace”, that can be requested separately, but the current code and docs are internally consistent. Basis The previous blocker is closed: MCP E2E - Default Reactor now passes on the latest PR head, and I also reproduced the same CI command locally with success. Key checks: MCPSQLExecutionFacade still preserves session locking, capability checks, cross-schema validation, and success/failure trace recording order. SQLStatementSafetyValidator centralizes banned SQL, metadata introspection, locking reads, side-effecting functions, and recursive CTE validation. SQLStatementTargetResolver covers multi-object references, CTEs, subqueries, DDL/DCL targets, and qualified functions with tests. ReadinessProbe covers ready, timeout, interrupt, and non-retryable readiness paths. Adversarial pass: Cross-dialect/adjacent feature path checked: DATABASE_AS_SCHEMA dialects with supportsCrossSchemaSql=false are still guarded; native-schema dialects with support enabled are not incorrectly blocked. Config-disabled path checked: supportsCrossSchemaSql=false has query/DML/DDL/DCL counterexamples covered; enabled path is also tested. Original symptom path checked: the previously failing MCP E2E job now succeeds on latest head. -- 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]
