fix(analyst): report tool failures as failures, accept valid read-only syntax, retrieve long text in full

Merged
AN
7d ago
+2512-184

Issue for this PR

Closes #1447

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

Why. Running the read-only analyst against real warehouses showed five classes of harness friction that cost the model turns or hid failures from it: backend errors reported as success, valid read-only SQL refused as writes, Mongo command shapes rejected, ragged spill rows, and long text that could not be read in full. Issue #1447 lists the evidence.

Root causes and what changed.

  1. Failures reported as success. Tool.define's wrapper only checked metadata.success === false, and both SDK resolver boundaries returned every resolved envelope as a successful result. Now a result with an explicit error envelope or backend metadata.error (when the tool has not declared success:true) is returned with status:"error" and rejected at the SDK boundary via Tool.assertSuccess, so the model, lint and telemetry see the failure; the tool's text output is kept. Per review: bash results stay ordinary SDK results (grep no-match and friends are not errors); bash records metadata.exit_code/metadata.success for telemetry only, and a Python traceback counts only as the final stderr block with a non-zero exit (or empty stdout on exit 0). bash completes on process exit again with a short drain grace, so backgrounded children do not hang the tool. Aborted tools complete before the boundary check; warehouse_test declares its guidance a successful diagnostic; batch no longer counts inner error envelopes as success; the truncation hint no longer claims success on a failed call.
  2. Valid read-only syntax refused. The native parser rejected //, negated GLOB, DuckDB prior.* aliases and standalone/CTE PIVOT/UNPIVOT. The classifier now adapts only the parser input per dialect; executed SQL is unchanged, and parser/lexical statement-count disagreement still fails closed. SQLite // stays refused (the engine does not support it).
  3. Mongo shapes. {collection, find|aggregate|count|distinct} documents are normalised in the driver; the materialize refusal lists accepted shapes; embedded $out/$merge are still refused.
  4. Spill rows. Absent values are written as null so every row has one value per column.
  5. Long text. read gains char_offset/max_chars (clip banner gives the next window); sql_execute gains full_text_columns: all selected columns are still returned, the named columns take the output budget first (in row order) and a cell that does not fit is clipped with a continued-in-spill marker; full-text mode always writes one complete lossless JSONL spill and sets result_path, so clipped text is recoverable with read; structured and binary cells are serialised losslessly; the schema line keeps its ordinary caps; notes are counted inside the budget; the formatter enforces the 20-row bound even when a connector returns more; an empty or null list (what strict-schema clients send for every unused property) or an omitted field turns the mode off; with a non-empty list an omitted limit means 20 and a larger one is clamped to 20 with a note in the result; a named column missing from the result adds a one-line note listing the available columns and the rows are still returned. Byte limits and permissions are intact; the truncation banner points to the mode. The analyst prompt gains one sentence: read a deciding rule in full and evaluate each condition against the row's own fields and dates before choosing.

Base. Stacked on feat/analyst-harness-dab-2026-10 (#1441); the diff shown here is only this change.

Behaviour deltas (was → now).

  • Tools returning backend metadata.error without success:true: status: ok → status: error + SDK tool-error event (all agents).
  • bash: completion waited on stream close → on process exit with bounded drain; non-zero exit: unchanged for the SDK, now recorded in metadata.exit_code/metadata.success.
  • Read-only classifier: the four syntax families above: refused → read (dialect-gated).
  • read/sql_execute: two new optional parameters; defaults unchanged.
  • Analyst prompt: one sentence added. Other agents: no prompt change.

How did you verify your code works?

Unit/integration, run outside any sandbox after an independent review (approve-with-changes; the three P1s it raised are fixed in the second commit):

  • Focused suites for every touched area (test/tool/bash.test.ts, batch, tool-define, truncation, read, test/altimate/tool-result-errors, test/session/tools-result-errors, tools-permissions, test/altimate/tools/*, test/altimate/sql-read-syntax.test.ts): 711 pass / 0 fail.
  • Regression fixtures are the real error payloads from recorded sessions (test/altimate/fixtures/read-syntax-errors.json, the long-line SQL fixture for read).
  • packages/drivers Mongo unit suite: 27 pass.
  • bun run typecheck clean; every edit in upstream-shared files (src/tool/*, src/session/*) is inside altimate_change markers (audited per file); the read tool description carries no marker text.

Not run: live warehouse E2E for the Mongo shapes beyond the driver unit suite; HTTP/browser journeys do not apply.

Screenshots / recordings

Not a 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 read/write SQL classification, Mongo command normalization, and SDK tool-error semantics used by all agents; behavior is fail-closed with broad test coverage but misclassification could still affect permissions or execution paths.

Overview
Fixes analyst harness friction by making tool failures visible, read-only SQL/Mongo accepted when valid, and long cell text recoverable without changing executed query text for classification adapters.

Tool outcomes. Backend failures and explicit error envelopes now surface as status: "error" and are rejected at the SDK via Tool.assertSuccess (session/prompt, batch inner calls). bash keeps ordinary non-zero exits as successful tool results but adds telemetry (exit_code, failure_kind); stderr tracebacks and timeouts are distinguished. warehouse_test infrastructure guidance is marked success: true. Truncation hints respect failed vs succeeded calls.

Read gates. sql-classify adapts parser input only (DuckDB //, prior.*, standalone/CTE PIVOT/UNPIVOT; SQLite NOT GLOB) so valid reads are not blocked; executed SQL is unchanged. Mongo normalizeMqlCommand accepts {collection, find|aggregate|count|distinct} shorthand, maps count → countDocuments, and throws on conflicting shorthand vs explicit args.

Long text. sql_execute adds optional full_text_columns (20-row preview cap, always spills full rows); read adds char_offset/max_chars for line windows. Result formatting streams JSONL spill serialization and pads missing column values with null.

Prompt. Analyst instructions add reading full policy/rule text and evaluating each condition against row fields before choosing an answer.

Reviewed by Cursor Bugbot for commit 2525fa7bc2265f06714cc3c72fcb564a81f2a998. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • New Features
    • SQL result previews can prioritize selected columns and show up to 20 rows, with notes for missing columns and clipped content. Full results remain available through saved output.
    • Long lines can be read in bounded character windows, making it easier to retrieve large SQL or text lines in sections.
    • MongoDB read commands support additional collection-based command formats.
  • Bug Fixes
    • SQL reads now recognize more SQLite and DuckDB syntax while continuing to detect writes and multiple statements.
    • Tool failures are more consistently reported across commands, including timeouts and explicit error results.