fix(control-plane): make no_destructive_sql robust to sqlglot version drift

[imran-siddique]

Summary

The no_destructive_sql validator in the agent-control-plane policy engine references both exp.AlterTable and exp.Grant (sqlglot expression classes) in a single function. No single installable sqlglot version exposes both symbols:

• AlterTable was renamed to Alter in a later release.
• Grant was only added later still.

On the pinned range in pyproject.toml (sqlglot>=23.0.0,<24.0), exp.Grant does not exist. Because the checks are sequential if isinstance(...) statements, the isinstance(statement, exp.Grant) line raises AttributeError for every query, including safe SELECTs. The destructive-SQL validator is therefore completely non-functional on the pinned version.

Reproduction on the pinned sqlglot (23.17.0):

from sqlglot import exp
exp.Grant       # AttributeError: module 'sqlglot.expressions' has no attribute 'Grant'
exp.Alter       # AttributeError
exp.AlterTable  # OK

The no_destructive_sql validator raises AttributeError on a plain SELECT * FROM users.

Fix

Resolve the destructive-statement expression classes in a version-robust way and filter out any that are absent:

• alter_types = (getattr(exp, "Alter", None), getattr(exp, "AlterTable", None)) with None filtered out.
• grant_types = (getattr(exp, "Grant", None),) with None filtered out.
• truncate_types = (getattr(exp, "TruncateTable", None),) with None filtered out.

No destructive-statement coverage is dropped:

• GRANT/REVOKE still fall through to the existing Command-node handling on versions that lack a dedicated Grant node.
• The TRUNCATE check now matches the dedicated TruncateTable node used by current sqlglot (it previously only matched a Command node, which does not occur on the pinned version).

Verification

• The module imports without error and the validator runs on the installed sqlglot (23.17.0) without AttributeError.
• tests/test_sql_policy.py: 43 passed (was 16 failing due to the AttributeError cascade, plus one TRUNCATE failure, before this change).
• Added a regression test test_no_attributeerror_on_installed_sqlglot that fails if the validator ever again references a sqlglot symbol missing on the installed version.

Provenance

Found via a paper-reproduction audit of the shipped control-plane code.

🤖 Generated with Claude Code

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[github-actions]

🤖 AI Agent: test-generator — `agent_control_plane/policy_engine.py`

AI-generated review output. Treat it as untrusted analysis and verify before acting.

agent_control_plane/policy_engine.py

• test_no_attributeerror_on_installed_sqlglot -- Test coverage for the new version-robust getattr logic is limited to ensuring no AttributeError occurs. Add tests to validate behavior for different sqlglot versions (e.g., with/without Grant, Alter, TruncateTable).

agent_os/integrations/escalation.py

• test_approve_with_empty_approver -- Test approve method with an empty approver to ensure it returns False.
• test_deny_with_empty_approver -- Test deny method with an empty approver to ensure it returns False.
• test_duplicate_vote_handling -- Test approve and deny methods to ensure duplicate votes from the same approver are rejected.
• test_vote_recording_on_resolved_request -- Test that votes are not recorded if the request is already resolved.

[github-actions]

🤖 AI Agent: security-scanner — View details

AI-generated review output. Treat it as untrusted analysis and verify before acting.

No security issues found.

[github-actions]

🤖 AI Agent: breaking-change-detector — API Compatibility

AI-generated review output. Treat it as untrusted analysis and verify before acting.

API Compatibility

Severity	Change	Impact
High	no_destructive_sql function now dynamically resolves SQLGlot expression classes (Alter, AlterTable, Grant, TruncateTable) using getattr with fallback to None.	This change may affect users relying on the previous behavior where specific classes were directly referenced. If users have custom extensions or tests that depend on the old behavior, they may encounter issues.
Medium	approve and deny methods in InMemoryApprovalQueue now require a non-empty approver argument and reject duplicate votes from the same approver.	This enforces stricter validation, which could break existing integrations that do not provide an approver or allow duplicate votes.
Low	truncate_types in no_destructive_sql now includes TruncateTable if available, in addition to the previous Command node handling.	This change may alter behavior for users relying on older versions of SQLGlot where TruncateTable was not present.

[github-actions]

🤖 AI Agent: code-reviewer — Action Items:

AI-generated review output. Treat it as untrusted analysis and verify before acting.

TL;DR: 0 blockers, 2 warnings. The changes improve robustness and correctness but introduce potential performance concerns and require further testing for edge cases.

#	Sev	Issue	Where
1	Warn	Potential performance impact due to multiple getattr calls in a loop.	policy_engine.py
2	Warn	Insufficient test coverage for edge cases in approve and deny methods.	escalation.py

Action Items:

1. None.

Warnings (fine as follow-up PRs):

#	Warning	Suggested Action
1	Multiple getattr calls in a loop may impact performance.	Cache resolved attributes outside the loop to avoid repeated lookups.
2	Edge cases in approve and deny methods (e.g., invalid approver formats, concurrent modifications) are not fully tested.	Add tests for edge cases like empty or invalid approver values and concurrent access scenarios.

[github-actions]

🤖 AI Agent: docs-sync-checker — Docs Sync

AI-generated review output. Treat it as untrusted analysis and verify before acting.

Docs Sync

• no_destructive_sql in agent_control_plane/policy_engine.py -- missing docstring
• README.md -- no updates found for changes to no_destructive_sql behavior
• CHANGELOG.md -- missing entry for changes to no_destructive_sql behavior and approve/deny methods in escalation.py

[github-actions]

PR Review Summary

Check	Status	Details
🔍 Code Review	⚠️ Missing	No current-run comment
🛡️ Security Scan	⚠️ Missing	No current-run comment
🔄 Breaking Changes	⚠️ Missing	No current-run comment
📝 Docs Sync	⚠️ Missing	No current-run comment
🧪 Test Coverage	⚠️ Missing	No current-run comment

Verdict: ⚠️ AI review incomplete; ready for human review

AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims.

[github-actions]

🟡 Contributor Check: MEDIUM

Check	Result
Profile	MEDIUM
Credential	LOW
Overall	MEDIUM

Automated check by AGT Contributor Check.