fix(sql): validate Mongo reads and explain classification failures
Issue for this PR
Stacked on feat/analyst-harness-dab-2026-10 (#1441)
Type of change
- Bug fix
- New feature
- Refactor / code improvement
- Documentation
What does this PR do?
Why: read-only analyst sessions were denied for valid reads. MongoDB shorthand commands and metadata reads were treated as writes or rendered as "undefined", and any SQL the parser could not parse was silently classified as a write and sent to a permission prompt the analyst cannot satisfy, with no explanation.
Root cause:
- Mongo: only string-valued native command keys were normalized, so
{collection, find: {...}}/{collection, aggregate: [...]}had no command;listCollectionsandestimatedDocumentCountwere neither classified as reads nor implemented; nonexistent databases/collections looked like legitimate empty results. - SQL: the classifier collapsed parser exceptions into "write" and dropped the diagnostic; the scratch-write exemption required every statement to be a local write, rejecting
CREATE TABLE ... AS SELECT ...; SELECT ...batches.
Before -> after:
- Mongo shorthand commands normalize and run as reads;
listCollections/estimatedDocumentCountare supported reads; wrong database/collection names return an error listing available names instead of an empty result; malformed or unknown Mongo commands and SQL text sent to a Mongo connection fail up front with JSON-command guidance (before permissions/dispatch). - Unparseable SQL now fails with
Could not prove this SQL is read-only: <parser error>. Rewrite or split the statement.instead of a permission denial. Hard-deny checks still run first and classification remains fail-closed. - Scratch batches mixing local writes with parser-verified reads run without write permission and return the last result.
Remaining (not fixed here; belongs in the altimate-core-internal parser, so both issues stay open as "Part of"):
- Parser rejects DuckDB
//integer division, SQLiteNOT GLOB, DuckDB typedTIMESTAMPTZ '...'literals, and an unquoted table aliasmatch; these now explain the failure but still cannot be proven read-only. - Read-only
string_aggqueries that build SQL text are classified as reads in DuckDB, SQLite and Postgres (pinned by tests); the refusals seen in practice came from DuckDB//integer division inside such queries, i.e. the parser gap above (skipped regression test added). - Top-level DuckDB
PIVOTis returned as UNKNOWN by core and still classified write. - A lexical SELECT/WITH fallback was deliberately not added: enforced read-only connections exist only for PostgreSQL.
Review follow-ups (P2, not addressed):
- Valid commands the parser rejects (e.g.
LOAD httpfs) no longer reach the write-approval path. - With
warehouseomitted and a Mongo connection first in the registry, plain SQL is rejected before the dbt connection fallback is considered. - Mongo collection-name metadata is cached per connection and not refreshed on a miss, so collections created externally are rejected until reconnect or a write through the connector.
How did you verify your code works?
- Driver suite (
packages/drivers,bun test): 655 pass, 0 fail. - Targeted tests (Mongo unit, classify, execute, materialize, scratch-write, read-safety, comment-safety): all pass;
sql-comment-safetyexact-object assertions were updated for the new diagnostic field. - Wider
packages/opencoderuns (test/altimate, plus tests outside it that reference the changed modules): no failures in any SQL/Mongo file; the only failures are 5s/60s timeouts in unrelated learn/datamate/agent-init/debug-report tests under load (agent-safety and debug-report pass when rerun alone). bun run typecheckinpackages/opencode: pass. Driverstsgo --noEmit: pass.git diff --check: clean.- Marker check: changed
packages/opencode/srcfiles are all undersrc/altimate/**and do not exist in the v1.4.9 upstream baseline tag, so no markers are required. - No live MongoDB server was available; driver behavior is covered by synthetic client fixtures.
Screenshots / recordings
N/A (no UI change).
Checklist
- I have tested my changes locally
- I have not included unrelated changes in this PR
๐ค Generated with Claude Code
[!NOTE]
Medium Risk
Changes query classification, permission gating, and Mongo execution preflight (metadata cache and namespace checks) on paths used by read-only agents; behavior is fail-closed but alters when writes prompt vs hard-error.
Overview
Fixes read-only analyst sessions wrongly blocking valid Mongo reads and opaque SQL parser failures.
MongoDB driver and tooling now normalize shorthand command documents ({collection, find}, {collection, aggregate}, native listCollections / estimatedDocumentCount), treat those as reads in classifyMongoCommand, and implement listCollections plus estimatedDocumentCount in the driver. Before opening cursors, reads validate database/collection names via cached listCollections (with one re-list on miss) and return errors listing available names instead of empty results; malformed non-object JSON and invalid commands fail early. Mongo warehouses reject non-JSON or unrecognized command shapes before permission checks; materialize validates normalized commands the same way.
SQL classification adds an optional parseError on classifyAndCheck when lexing/parser proof fails (still fail-closed as write). sql_execute attaches that diagnostic to write-approval metadata for unprovable SQL. isScratchSafeWrite no longer treats batches that include reads as scratch-safe, so mixed write+read scratch batches still require write permission.
Extensive driver and opencode tests cover the new Mongo paths, namespace validation, and parser-edge cases.
Reviewed by Cursor Bugbot for commit 7aa9f47e58e9cf1828d1034ca4b33613cd431200. Bugbot is set up for automated code reviews on this repo. Configure here.