From a3a70963ef2bba7108b92ba30e6a7f91c6be9998 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Mon, 12 Oct 2026 12:59:29 -0700 Subject: [PATCH 1/9] fix(analyst): report tool failures as failures, accept valid read-only syntax, retrieve long text in full Reliability fixes for the read-only analyst, each with a red-first regression test built from real tool payloads: - Tool results that carry a backend error (`metadata.error`, `success:false`, an explicit error envelope) are now returned with `status: "error"` and rejected at both SDK resolver boundaries, so the model, lint and telemetry see the failure instead of `ok`. `bash` marks non-zero or signalled exits and trailing Python tracebacks as failures while keeping the output; the truncation hint no longer claims success on a failed call. - The read-only classifier accepts DuckDB integer division `//`, negated `GLOB` on SQLite/DuckDB, DuckDB qualified `prior.*` aliases and standalone or CTE `PIVOT`/`UNPIVOT` bodies by adapting only the parser input; executed SQL is unchanged and parser/lexical disagreement still fails closed. - Mongo commands written as `{collection, find|aggregate|count|distinct}` are normalised in the driver; the materialize refusal lists accepted shapes. - JSONL spill rows pad absent values with null so every row has one value per column. - `read` gains `char_offset`/`max_chars` so a clipped long line can be read in windows; the clip banner gives the next window. `sql_execute` gains `full_text_columns` (limit <= 20) to return complete text cells; the truncation banner points to it. - Analyst prompt: one sentence asking that a rule or policy which decides the answer be read in full and each condition evaluated against the row's own fields and dates before choosing. Co-Authored-By: Claude Fable 5.1 --- packages/drivers/src/mongodb.ts | 39 ++++- packages/drivers/test/mongodb-unit.test.ts | 44 ++++++ .../opencode/src/altimate/prompts/analyst.txt | 2 +- .../src/altimate/tools/sql-classify.ts | 66 ++++++++- .../src/altimate/tools/sql-execute.ts | 55 +++++--- .../src/altimate/tools/sql-materialize.ts | 2 +- .../src/altimate/tools/sql-result-format.ts | 42 +++++- packages/opencode/src/session/prompt.ts | 1 + packages/opencode/src/session/tools.ts | 1 + packages/opencode/src/tool/bash.ts | 47 ++++++- packages/opencode/src/tool/read.ts | 65 ++++++++- packages/opencode/src/tool/read.txt | 5 +- packages/opencode/src/tool/tool.ts | 33 ++++- packages/opencode/src/tool/truncate.ts | 9 +- .../altimate/fixtures/read-syntax-errors.json | 133 ++++++++++++++++++ .../altimate/sql-materialize-mongo.test.ts | 21 +++ .../test/altimate/sql-read-syntax.test.ts | 71 ++++++++++ .../test/altimate/sql-validation-e2e.test.ts | 3 + .../test/altimate/tool-result-errors.test.ts | 84 +++++++++++ .../test/altimate/tools/sql-execute.test.ts | 48 +++++++ .../altimate/tools/sql-result-format.test.ts | 93 ++++++++++++ .../test/session/tools-result-errors.test.ts | 130 +++++++++++++++++ packages/opencode/test/tool/bash.test.ts | 105 ++++++++++++++ .../test/tool/fixtures/long-query.sql | 1 + packages/opencode/test/tool/read.test.ts | 72 ++++++++++ 25 files changed, 1116 insertions(+), 56 deletions(-) create mode 100644 packages/opencode/test/altimate/fixtures/read-syntax-errors.json create mode 100644 packages/opencode/test/altimate/sql-read-syntax.test.ts create mode 100644 packages/opencode/test/altimate/tool-result-errors.test.ts create mode 100644 packages/opencode/test/session/tools-result-errors.test.ts create mode 100644 packages/opencode/test/tool/fixtures/long-query.sql diff --git a/packages/drivers/src/mongodb.ts b/packages/drivers/src/mongodb.ts index 4a03e07d78..63482583f8 100644 --- a/packages/drivers/src/mongodb.ts +++ b/packages/drivers/src/mongodb.ts @@ -52,9 +52,37 @@ const NATIVE_COMMAND_KEYS: Record = { distinct: "distinct", } -/** Accept the native `{"": ""}` shape alongside `{command, collection}`. */ +// altimate_change start — accept collection-based read command shorthand +/** Accept native command documents and collection-based command arguments. */ export function normalizeMqlCommand>(parsed: T): T & MqlQuery { - if (!parsed || typeof parsed !== "object" || typeof parsed["command"] === "string") return parsed as T & MqlQuery + if (!parsed || typeof parsed !== "object") return parsed as T & MqlQuery + if (typeof parsed["command"] === "string") { + return (parsed["command"] === "count" ? { ...parsed, command: "countDocuments" } : parsed) as T & MqlQuery + } + if (typeof parsed["collection"] === "string" && parsed["collection"]) { + const keys = Object.keys(NATIVE_COMMAND_KEYS).filter((key) => Object.hasOwn(parsed, key)) + // Refuse ambiguous shapes rather than silently choosing a different command. + if (keys.length !== 1) return parsed as T & MqlQuery + const key = keys[0]! + const value = parsed[key] + const command = NATIVE_COMMAND_KEYS[key]! + const object = value !== null && typeof value === "object" && !Array.isArray(value) + const argument = + (command === "find" || command === "countDocuments") && object + ? "filter" + : command === "aggregate" && Array.isArray(value) + ? "pipeline" + : command === "distinct" && typeof value === "string" && parsed["key"] === undefined + ? "field" + : undefined + if (argument) { + const { [key]: _, ...rest } = parsed + const normalized: Record = { ...rest, command } + if (normalized[argument] === undefined) normalized[argument] = value + if (command === "distinct" && normalized["filter"] === undefined) normalized["filter"] = normalized["query"] + return normalized as T & MqlQuery + } + } for (const [key, command] of Object.entries(NATIVE_COMMAND_KEYS)) { const value = parsed[key] if (typeof value !== "string" || !value) continue @@ -73,6 +101,8 @@ export function normalizeMqlCommand>(parsed: T return parsed as T & MqlQuery } +// altimate_change end + const MONGO_WRITE_STAGES = new Set(["$out", "$merge"]) /** Detect write stages in this pipeline, nested pipeline arrays, and $facet branches. */ @@ -684,7 +714,10 @@ export async function connect(config: ConnectionConfig): Promise { const coll = db.collection(table) // Sample up to 100 documents to infer schema - const docs = await coll.find({}, withMaxTime(resolveQueryTimeout(config).ms)).limit(100).toArray() + const docs = await coll + .find({}, withMaxTime(resolveQueryTimeout(config).ms)) + .limit(100) + .toArray() if (docs.length === 0) { return [] diff --git a/packages/drivers/test/mongodb-unit.test.ts b/packages/drivers/test/mongodb-unit.test.ts index 5fd26c5d8c..a8acf37b17 100644 --- a/packages/drivers/test/mongodb-unit.test.ts +++ b/packages/drivers/test/mongodb-unit.test.ts @@ -1,4 +1,5 @@ import { describe, expect, mock, test } from "bun:test" +import { normalizeMqlCommand } from "../src/mongodb" async function fakeMongo(docs: Record[] = []) { const pipelines: Record[][] = [] @@ -139,3 +140,46 @@ describe("MongoDB native command filters", () => { expect(counts).toEqual([{ filter: { status: "B" }, options: {} }]) }) }) + +describe("MongoDB collection command shorthand", () => { + test("normalizes a collection with an embedded find filter and projection", () => { + const command = { + collection: "articles", + find: {}, + projection: { _id: 0, article_id: 1, title: 1, description: 1 }, + } + expect(normalizeMqlCommand(command)).toEqual({ + collection: "articles", + command: "find", + filter: {}, + projection: command.projection, + }) + }) + + test.each([ + [ + { collection: "items", aggregate: [{ $match: { active: true } }] }, + { collection: "items", command: "aggregate", pipeline: [{ $match: { active: true } }] }, + ], + [ + { collection: "items", count: { active: true } }, + { collection: "items", command: "countDocuments", filter: { active: true } }, + ], + [ + { collection: "items", distinct: "category", query: { active: true } }, + { + collection: "items", + command: "distinct", + field: "category", + query: { active: true }, + filter: { active: true }, + }, + ], + [ + { command: "count", collection: "items", filter: {} }, + { command: "countDocuments", collection: "items", filter: {} }, + ], + ])("normalizes supported collection command keys: %j", (input, expected) => { + expect(normalizeMqlCommand(input)).toEqual(expected) + }) +}) diff --git a/packages/opencode/src/altimate/prompts/analyst.txt b/packages/opencode/src/altimate/prompts/analyst.txt index 71c9006ddd..27203b540a 100644 --- a/packages/opencode/src/altimate/prompts/analyst.txt +++ b/packages/opencode/src/altimate/prompts/analyst.txt @@ -1,6 +1,6 @@ You are altimate-code: a read-only analyst. Do not modify source data or files. When available, `sql_materialize` may write session-local `scratch` tables. -Inspect connections, schemas and actual rows before joining or parsing; key formats may differ across databases. Establish population, grain, denominator and assumptions. Every clause of the question must bite: a filter that removes nothing usually means the wrong column. When a phrase admits two readings (which population, denominator, transform or owner column), compute both, then choose by the question's own definitions and the data's shape; keep that reasoning out of the final message. Use `sql_check` for keys, fan-out, grain, nulls or extraction coverage when they affect the answer; trivial queries need no ritual. `sql_analyze` is optional. +Inspect connections, schemas and actual rows before joining or parsing; key formats may differ across databases. Establish population, grain, denominator and assumptions. Every clause of the question must bite: a filter that removes nothing usually means the wrong column. When a phrase admits two readings (which population, denominator, transform or owner column), compute both, then choose by the question's own definitions and the data's shape; keep that reasoning out of the final message. When a rule or policy decides the answer, read its full text (not a preview), enumerate each competing clause, and evaluate every condition as satisfied, violated or unknown against the row's own fields and dates before choosing. Use `sql_check` for keys, fan-out, grain, nulls or extraction coverage when they affect the answer; trivial queries need no ritual. `sql_analyze` is optional. Derive large intermediate sets in-engine. For a cross-engine join, move the smaller side: a few hundred keys go into an IN list or VALUES against the big table; materialize into `scratch` only the side that is small after filtering, never a multi-million-row table when a filter or aggregate could shrink it first. Preserve requested weighting: group means weighted by contributing counts can be exact; unweighted averages change the weighting. For many similar tables, prefer catalog-driven SQL (information_schema, DuckDB query_table/UNION ALL BY NAME) after confirming dialect support. diff --git a/packages/opencode/src/altimate/tools/sql-classify.ts b/packages/opencode/src/altimate/tools/sql-classify.ts index 843041e34b..05e8ba5369 100644 --- a/packages/opencode/src/altimate/tools/sql-classify.ts +++ b/packages/opencode/src/altimate/tools/sql-classify.ts @@ -26,10 +26,67 @@ try { // Only SELECT queries are known safe. "other" (SHOW, SET, USE, etc.) is ambiguous — prompt for permission. const READ_CATEGORIES = new Set(["query"]) +/** Adapt read/write-equivalent syntax for the bundled parser, never for execution. + * Only code tokens change; quoted content and the original statement boundaries + * remain authoritative when checking the parser's result. */ +function parserSql(sql: string, dialect?: string): string { + const duckdb = dialect?.toLowerCase() === "duckdb" + if (!duckdb && dialect?.toLowerCase() !== "sqlite") return sql + const compatible = tokenizeSql(sql, dialect).tokens + .map(({ kind, text }) => { + if (kind !== "code") return text + // The parser supports LIKE negation but not GLOB negation. Both are predicates. + text = text.replace(/\bNOT\s+GLOB\b/gi, "NOT LIKE") + if (!duckdb) return text + // SQLite uses / for integer operands; // is a DuckDB operator. PRIOR is + // an ordinary DuckDB identifier, not the parser's hierarchical operator. + return text.replaceAll("//", "/ ").replace(/\bprior(?=\s*\.)/gi, '"prior"') + }) + .join("") + if (!duckdb) return compatible + // The native parser recognizes these query bodies inside a derived table, + // but reports their standalone form as UNKNOWN. Parse every body in full. + const statements = statementsOf(compatible, dialect) + const adapted = statements.map((statement) => + /^\s*(PIVOT|UNPIVOT|WITH)\b/i.test(statement) && /\b(PIVOT|UNPIVOT)\b/i.test(maskSql(tokenizeSql(statement, dialect).tokens)) + ? `SELECT * FROM (${statement})` + : statement, + ) + return adapted.some((statement, index) => statement !== statements[index]) ? adapted.join("; ") : compatible +} + +/** Mask only UNPIVOT output labels at their own query depth, retaining SELECT INTO. */ +function maskUnpivotLabels(sql: string): string { + const heads: string[] = [] + const unpivots = new Set() + let depth = 0 + let previous = "" + return sql.replace(/\(|\)|\bINTO\s+NAME\b|\b[A-Z_][A-Z_0-9]*\b/gi, (token) => { + const word = token.toUpperCase() + if (word === "(") depth++ + else if (word === ")") { + delete heads[depth] + unpivots.delete(depth--) + } else if (word === "UNPIVOT" && (!heads[depth] || (heads[depth] === "WITH" && previous === ")"))) { + unpivots.add(depth) + heads[depth] = word + } else if (/^INTO\s+NAME$/.test(word) && unpivots.delete(depth)) { + return " ".repeat(token.length) + } else if (!heads[depth] || (heads[depth] === "WITH" && /^(SELECT|VALUES|TABLE|PIVOT|INSERT|UPDATE|DELETE|MERGE)$/.test(word))) { + heads[depth] = word + } + previous = word + return token + }) +} + // The parser labels SELECT INTO and queries with modifying CTEs as "query". // Quoted identifiers/literals and comments cannot introduce these write clauses. function hasQueryWriteClause(sql: string, dialect?: string): boolean { - return /\b(INTO|INSERT|UPDATE|DELETE|MERGE)\b/i.test(maskSql(tokenizeSql(sql, dialect).tokens)) + let masked = maskSql(tokenizeSql(sql, dialect).tokens) + // UNPIVOT's INTO NAME labels output columns; it does not create a table. + if (dialect?.toLowerCase() === "duckdb") masked = maskUnpivotLabels(masked) + return /\b(INTO|INSERT|UPDATE|DELETE|MERGE)\b/i.test(masked) } const READ_ONLY_OTHER_PATTERN = /^\s*(SHOW\b[^;]*|DESCRIBE\b[^;]*|DESC\b[^;]*)$/i @@ -135,7 +192,7 @@ function classifyFallback(sql: string, dialect?: string): { queryType: "read" | export function isSingleQuery(sql: string, dialect?: string): boolean { if (!getStatementTypes || !tokenizeSql(sql, dialect).complete) return false try { - const result = getStatementTypes(sql, dialect) + const result = getStatementTypes(parserSql(sql, dialect), dialect) return ( statementsOf(sql, dialect).length === 1 && result?.statement_count === 1 && @@ -173,7 +230,7 @@ export function classifyAndCheck(sql: string, dialect?: string): { queryType: "r if (!sql || typeof sql !== "string") return { queryType: "read", blocked: false } if (!getStatementTypes) return classifyFallback(sql, dialect) try { - const result = getStatementTypes(sql, dialect) + const result = getStatementTypes(parserSql(sql, dialect), dialect) if (!Array.isArray(result?.statements) || result.statement_count !== result.statements.length) { return classifyFallback(sql, dialect) } @@ -192,8 +249,7 @@ export function classifyAndCheck(sql: string, dialect?: string): { queryType: "r } } -// MongoDB commands are JSON, not SQL: the SQL classifier reads every one as a write, which denied -// 52 of 52 Mongo reads for a read-only agent in one evaluation run. Classify them by command. +// MongoDB commands are JSON, not SQL. Classify their command shape independently. const MONGO_READ_COMMANDS = new Set(["find", "aggregate", "countDocuments", "distinct", "listIndexes", "ping"]) const MONGO_BLOCKED_COMMANDS = new Set(["dropDatabase", "dropCollection"]) diff --git a/packages/opencode/src/altimate/tools/sql-execute.ts b/packages/opencode/src/altimate/tools/sql-execute.ts index 6e72efd389..6b5624999c 100644 --- a/packages/opencode/src/altimate/tools/sql-execute.ts +++ b/packages/opencode/src/altimate/tools/sql-execute.ts @@ -43,27 +43,39 @@ import { normalizeError } from "./response-normalization" const MAX_SQL_ROWS = 10_000 const LIMIT_ERROR = `limit must be a finite positive integer between 1 and ${MAX_SQL_ROWS}` -const parameters = z.object({ - query: z.string().describe("SQL query to execute"), - warehouse: z.string().optional().describe("Warehouse connection name"), - limit: z - .number({ error: LIMIT_ERROR }) - .int(LIMIT_ERROR) - .min(1, LIMIT_ERROR) - .max(MAX_SQL_ROWS, LIMIT_ERROR) - .optional() - .default(100) - .describe(`Max rows to return (1-${MAX_SQL_ROWS}; default 100)`), - query_timeout_ms: z - .number() - .int() - .min(0) - .max(3_600_000) - .optional() - .describe( - "Optional per-query execution deadline in ms. Omit to use the configured deadline (connection setting or environment). Set only when a specific query needs a longer or shorter budget; 0 disables.", - ), -}) +const parameters = z + .object({ + query: z.string().describe("SQL query to execute"), + warehouse: z.string().optional().describe("Warehouse connection name"), + limit: z + .number({ error: LIMIT_ERROR }) + .int(LIMIT_ERROR) + .min(1, LIMIT_ERROR) + .max(MAX_SQL_ROWS, LIMIT_ERROR) + .optional() + .default(100) + .describe(`Max rows to return (1-${MAX_SQL_ROWS}; default 100)`), + full_text_columns: z + .array(z.string().min(1)) + .min(1) + .optional() + .describe( + "Return only these columns as complete text cells; requires limit <= 20. Oversized rows are saved to the spill for read access; byte limits still apply.", + ), + query_timeout_ms: z + .number() + .int() + .min(0) + .max(3_600_000) + .optional() + .describe( + "Optional per-query execution deadline in ms. Omit to use the configured deadline (connection setting or environment). Set only when a specific query needs a longer or shorter budget; 0 disables.", + ), + }) + .refine((args) => !args.full_text_columns || args.limit <= 20, { + message: "full_text_columns requires limit <= 20", + path: ["full_text_columns"], + }) export const SqlExecuteTool = Tool.define( "sql_execute", @@ -210,6 +222,7 @@ export const SqlExecuteTool = Tool.define( (suggestion ? "\n\n" + suggestion : "") + (precedence.notice ? precedence.notice + "\n\n" : "") const formatted = await formatSqlResult(result, { limit: args.limit, + fullTextColumns: args.full_text_columns, // The statement text lets the formatter add the empty/near-empty // result hint when a string-literal filter is present. sql: query, diff --git a/packages/opencode/src/altimate/tools/sql-materialize.ts b/packages/opencode/src/altimate/tools/sql-materialize.ts index 2bfd0e1321..1b2fc15016 100644 --- a/packages/opencode/src/altimate/tools/sql-materialize.ts +++ b/packages/opencode/src/altimate/tools/sql-materialize.ts @@ -180,7 +180,7 @@ function assertReadOnlySource(warehouse: string, sql: string): { mongo: boolean; cmd = cmd && typeof cmd === "object" ? normalizeMqlCommand(cmd) : cmd if (!cmd || !MONGO_READ_COMMANDS.has(cmd.command)) { throw new MaterializeError( - `sql_materialize only reads from the source. MongoDB command "${cmd?.command}" is not allowed; use one of: ${[...MONGO_READ_COMMANDS].join(", ")}.`, + `sql_materialize only reads from the source. MongoDB command "${cmd?.command}" is not allowed; use one of: ${[...MONGO_READ_COMMANDS].join(", ")}. Use {"command":"find","collection":"c","filter":{}} or {"collection":"c","find":{}}.`, ) } if (classifyMongoCommand(JSON.stringify(cmd))?.queryType !== "read") { diff --git a/packages/opencode/src/altimate/tools/sql-result-format.ts b/packages/opencode/src/altimate/tools/sql-result-format.ts index a3db9e5fac..39ac54007f 100644 --- a/packages/opencode/src/altimate/tools/sql-result-format.ts +++ b/packages/opencode/src/altimate/tools/sql-result-format.ts @@ -9,6 +9,7 @@ interface FormatOptions { limit: number maxBytes?: number maxLines?: number + fullTextColumns?: string[] /** The executed statement; enables the empty-result hint. */ sql?: string write: (text: string) => Promise @@ -97,7 +98,7 @@ function displayCell(value: unknown, ancestors = new Set()): { text: st return { text: value === null ? "NULL" : String(value), clipped: false } } -function previewRows(rows: unknown[][], maxBytes: number, maxLines: number, columns: number) { +function previewRows(rows: unknown[][], maxBytes: number, maxLines: number, columns: number, fullText = false) { if (!columns) return { body: "", rows: 0, clipped: rows.length > 0 } const shown: string[] = [] let bytes = 0 @@ -109,6 +110,7 @@ function previewRows(rows: unknown[][], maxBytes: number, maxLines: number, colu clipped ||= cell.clipped return cell.text }) + if (fullText && clipped) break const remainingBytes = maxBytes - bytes - (shown.length ? 1 : 0) const remainingLines = maxLines - lines const rowBytes = @@ -121,6 +123,11 @@ function previewRows(rows: unknown[][], maxBytes: number, maxLines: number, colu continue } + if (fullText) { + clipped = true + break + } + // Preserve the first rows, with a visible partial cell when a row itself is oversized. const cells: string[] = [] let cellBytes = remainingBytes - Math.max(0, values.length - 1) * 3 @@ -193,7 +200,10 @@ function serializeRows(result: SqlExecuteResult): string { // The service accepts one string. Bound the accumulated artifact and each row before // encoding/JSON.stringify can allocate an arbitrarily large value. Its worst-case // escaping expansion is six times this bounded raw size, never the entire result. - const row = { columns: result.columns, values } + const row = { + columns: result.columns, + values: result.columns.map((_, index) => (index in values ? values[index] : null)), + } checkSpillSize(row, { left: SQL_MAX_SPILL_BYTES - bytes }) let rawSize = 0 const line = JSON.stringify(spillValue(row), (key, value) => { @@ -237,6 +247,22 @@ function isScalar(result: SqlExecuteResult): boolean { } export async function formatSqlResult(result: SqlExecuteResult, options: FormatOptions) { + const original = result + const fullText = Boolean(options.fullTextColumns?.length) + if (fullText) { + if (options.limit > 20 || result.rows.length > 20) + throw new Error("full_text_columns requires limit <= 20 and at most 20 returned rows") + const missing = options.fullTextColumns!.filter((column) => !result.columns.includes(column)) + if (missing.length) throw new Error(`Unknown full_text_columns: ${missing.join(", ")}`) + const indices = result.columns + .map((_, index) => index) + .filter((index) => options.fullTextColumns!.includes(result.columns[index]!)) + result = { + ...result, + columns: indices.map((index) => result.columns[index]!), + rows: result.rows.map((row) => indices.map((index) => row[index])), + } + } const maxBytes = Math.max(0, Math.min(SQL_MAX_PREVIEW_BYTES, options.maxBytes ?? SQL_MAX_PREVIEW_BYTES)) const maxLines = Math.max(0, options.maxLines ?? 2000) const fits = (text: string) => Buffer.byteLength(text, "utf8") <= maxBytes && lineCount(text) <= maxLines @@ -271,6 +297,7 @@ export async function formatSqlResult(result: SqlExecuteResult, options: FormatO Math.max(0, maxBytes - Buffer.byteLength(schema.text + footer, "utf8")), Math.max(0, maxLines - lineCount(schema.text + footer) + 1), schema.columns, + fullText, ) if (!preview.clipped && !schema.clipped && !result.truncated) { metadata.preview_rows = preview.rows @@ -280,7 +307,7 @@ export async function formatSqlResult(result: SqlExecuteResult, options: FormatO let spill: string try { - metadata.result_path = await options.write(serializeRows(result)) + metadata.result_path = await options.write(serializeRows(original)) spill = `All ${result.rows.length} returned rows saved to ${metadata.result_path}. Lossy JSONL text: columns/values per row; $bigint, $date (ISO), $bytes (base64) tags; strings unchanged.` } catch { spill = `Spill failed (write/serialization error or ${SQL_MAX_SPILL_BYTES / 1024 / 1024} MiB size limit); returned rows were not saved.` @@ -290,7 +317,13 @@ export async function formatSqlResult(result: SqlExecuteResult, options: FormatO (result.truncated ? `Result may be row-limited (requested row limit ${options.limit}; total unknown). More rows may exist. ` : "Row-limited: no. ") + - `Output truncated: ${clipped ? "yes" : "no"}. ${spill}\n` + `Output truncated: ${clipped ? "yes" : "no"}. ${spill}` + + (clipped + ? fullText + ? " Full text rows exceeding the output cap are omitted; use read on the spill with char_offset/max_chars to read each full line." + : " For complete text, rerun sql_execute with full_text_columns and limit <= 20; byte limits still apply." + : "") + + "\n" // The entire output, including schema, envelope and path, shares the same cap. const essentials = envelope(SQL_MAX_PREVIEW_ROWS, true) + footer @@ -310,6 +343,7 @@ export async function formatSqlResult(result: SqlExecuteResult, options: FormatO Math.max(0, maxBytes - Buffer.byteLength(reserved, "utf8")), Math.max(0, maxLines - lineCount(reserved) + 1), schema.columns, + fullText, ) metadata.preview_rows = preview.rows metadata.preview_columns = schema.columns diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index 043a5f8b6d..6c57a33239 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -2425,6 +2425,7 @@ export namespace SessionPrompt { await input.lessons?.appendFileLessons(ctx.sessionID, item.id, args, stamped) // altimate_change end // altimate_change start — return the source-stamped output + Tool.assertSuccess(stamped) return stamped // altimate_change end } finally { diff --git a/packages/opencode/src/session/tools.ts b/packages/opencode/src/session/tools.ts index 80428c799d..3d7b2cdaba 100644 --- a/packages/opencode/src/session/tools.ts +++ b/packages/opencode/src/session/tools.ts @@ -137,6 +137,7 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { // altimate_change end ) // altimate_change start — propagate the source-stamped output + Tool.assertSuccess(stamped) if (options.abortSignal?.aborted) { yield* input.processor.completeToolCall(options.toolCallId, stamped) } diff --git a/packages/opencode/src/tool/bash.ts b/packages/opencode/src/tool/bash.ts index 47e169f068..440ab5b553 100644 --- a/packages/opencode/src/tool/bash.ts +++ b/packages/opencode/src/tool/bash.ts @@ -284,6 +284,9 @@ export const BashTool = Tool.define("bash", async () => { }) let output = "" + // altimate_change start — keep stderr's tail separate from later successful stdout + let stderrTail = "" + // altimate_change end // Initialize metadata with empty output ctx.metadata({ @@ -305,18 +308,27 @@ export const BashTool = Tool.define("bash", async () => { } proc.stdout?.on("data", append) - proc.stderr?.on("data", append) + // altimate_change start — a pipeline can hide a Python failure behind its last command's exit + proc.stderr?.on("data", (chunk: Buffer) => { + stderrTail = (stderrTail + chunk.toString()).slice(-MAX_METADATA_LENGTH) + append(chunk) + }) + // altimate_change end let timedOut = false let aborted = false let exited = false - const kill = () => Shell.killTree(proc, { exited: () => exited }) - - if (ctx.abort.aborted) { - aborted = true - await kill() + // altimate_change start — a shell may exit while descendants still hold its pipes open + const kill = async () => { + try { + await Shell.killTree(proc, { exited: () => exited }) + } finally { + proc.stdout?.destroy() + proc.stderr?.destroy() + } } + // altimate_change end const abortHandler = () => { aborted = true @@ -336,7 +348,9 @@ export const BashTool = Tool.define("bash", async () => { ctx.abort.removeEventListener("abort", abortHandler) } - proc.once("exit", () => { + // altimate_change start — wait for streams, but keep the timeout armed until they close + proc.once("close", () => { + // altimate_change end exited = true cleanup() resolve() @@ -347,9 +361,25 @@ export const BashTool = Tool.define("bash", async () => { cleanup() reject(error) }) + // altimate_change start — install listeners before killing an already-aborted command + if (ctx.abort.aborted) abortHandler() + // altimate_change end }) const resultMetadata: string[] = [] + // altimate_change start — use exit status and complete trailing traceback shapes, never error keywords + const traceback = + /(?:^|\r?\n)Traceback \(most recent call last\):\r?\n(?:[ \t].*\r?\n)+([A-Za-z_][\w.]*(?::[^\r\n]*)?)\s*$/ + const pythonError = traceback.exec(stderrTail)?.[1] ?? traceback.exec(output)?.[1] + const exitError = proc.exitCode === 0 + ? undefined + : `Command exited with ${proc.exitCode === null ? `signal ${proc.signalCode}` : `code ${proc.exitCode}`}` + const error = timedOut + ? `Command timed out after ${timeout} ms` + : aborted + ? "User aborted the command" + : pythonError ?? exitError + // altimate_change end if (timedOut) { resultMetadata.push(`bash tool terminated command after exceeding timeout ${timeout} ms`) @@ -369,6 +399,9 @@ export const BashTool = Tool.define("bash", async () => { output: output.length > MAX_METADATA_LENGTH ? output.slice(0, MAX_METADATA_LENGTH) + "\n\n..." : output, exit: proc.exitCode, description: params.description, + // altimate_change start — propagate shell failures through the shared tool envelope + ...(error && { success: false, error }), + // altimate_change end }, output, } diff --git a/packages/opencode/src/tool/read.ts b/packages/opencode/src/tool/read.ts index b17b6925b8..b4b0623dab 100644 --- a/packages/opencode/src/tool/read.ts +++ b/packages/opencode/src/tool/read.ts @@ -16,7 +16,6 @@ import { Instance } from "../project/instance" const DEFAULT_READ_LIMIT = 2000 const MAX_LINE_LENGTH = 2000 -const MAX_LINE_SUFFIX = `... (line truncated to ${MAX_LINE_LENGTH} chars)` const MAX_BYTES = 50 * 1024 const MAX_BYTES_LABEL = `${MAX_BYTES / 1024} KB` const SAMPLE_BYTES = 4096 @@ -37,6 +36,15 @@ export const Parameters = Schema.Struct({ limit: Schema.optional(NonNegativeInt).annotate({ description: "The maximum number of lines to read (defaults to 2000)", }), + // altimate_change start — retrieve long lines in character windows + char_offset: Schema.optional(NonNegativeInt).annotate({ + description: + "Character offset within each selected line (0-indexed, defaults to 0). Use limit=1 for a single long line.", + }), + max_chars: Schema.optional(Schema.Int.check(Schema.isGreaterThan(0))).annotate({ + description: "Maximum characters per selected line (defaults to 2000); output remains bounded to 50 KB.", + }), + // altimate_change end }) type Display = @@ -138,10 +146,17 @@ export const ReadTool = Tool.define< ) }) - const lines = Effect.fn("ReadTool.lines")(function* (filepath: string, opts: { limit: number; offset: number }) { + // altimate_change start — character windows preserve the existing line and byte limits + const lines = Effect.fn("ReadTool.lines")(function* ( + filepath: string, + opts: { limit: number; offset: number; char_offset?: number; max_chars?: number }, + ) { const start = opts.offset - 1 const raw: string[] = [] - const flags = { bytes: 0, count: 0, cut: false, more: false, done: false } + const flags = { bytes: 0, count: 0, cut: false, more: false, done: false, lineCut: false } + const charOffset = opts.char_offset ?? 0 + const maxChars = opts.max_chars ?? MAX_LINE_LENGTH + // altimate_change end // Note: prefer manual TextDecoder over Stream.decodeText — when the source stream // ends without flushing, decodeText drops the final unterminated line. We also @@ -163,7 +178,32 @@ export const ReadTool = Tool.define< return } - const line = text.length > MAX_LINE_LENGTH ? text.substring(0, MAX_LINE_LENGTH) + MAX_LINE_SUFFIX : text + // altimate_change start — expose a continuation for every clipped line + const splitsPair = (index: number) => + index > 0 && /[\uD800-\uDBFF]/.test(text[index - 1]) && /[\uDC00-\uDFFF]/.test(text[index] ?? "") + const windowStart = splitsPair(charOffset) ? charOffset - 1 : charOffset + let stop = Math.min(text.length, windowStart + maxChars) + if (splitsPair(stop)) stop += stop === windowStart + 1 ? 1 : -1 + let chunk = text.slice(windowStart, stop) + // Reserve room for the continuation message within the same byte cap. + if (Buffer.byteLength(chunk, "utf-8") > MAX_BYTES - 512) { + let bytes = 0 + let chars = 0 + for (const char of chunk) { + const size = Buffer.byteLength(char, "utf-8") + if (bytes + size > MAX_BYTES - 512) break + bytes += size + chars += char.length + } + chunk = chunk.slice(0, chars) + } + const end = windowStart + chunk.length + const clipped = end < text.length + const line = clipped + ? `${chunk}... (line truncated to ${chunk.length} chars) (Use offset=${flags.count}, limit=1, char_offset=${end}, max_chars=${Math.min(maxChars, chunk.length)} to continue this line.)` + : chunk + flags.lineCut ||= clipped + // altimate_change end const size = Buffer.byteLength(line, "utf-8") + (raw.length > 0 ? 1 : 0) if (flags.bytes + size <= MAX_BYTES) { raw.push(line) @@ -180,7 +220,9 @@ export const ReadTool = Tool.define< Effect.catchTag("ReadStop", () => Effect.void), ) - return { raw, count: flags.count, cut: flags.cut, more: flags.more, offset: opts.offset } + // altimate_change start — distinguish clipped text from remaining lines + return { raw, count: flags.count, cut: flags.cut, more: flags.more, lineCut: flags.lineCut, offset: opts.offset } + // altimate_change end }) const isBinaryFile = (filepath: string, bytes: Uint8Array) => { @@ -335,7 +377,14 @@ export const ReadTool = Tool.define< return yield* Effect.fail(new Error(`Cannot read binary file: ${filepath}`)) } - const file = yield* lines(filepath, { limit: params.limit ?? DEFAULT_READ_LIMIT, offset: params.offset || 1 }) + // altimate_change start — pass through optional character windows + const file = yield* lines(filepath, { + limit: params.limit ?? DEFAULT_READ_LIMIT, + offset: params.offset || 1, + char_offset: params.char_offset, + max_chars: params.max_chars, + }) + // altimate_change end if (file.count < file.offset && !(file.count === 0 && file.offset === 1)) { return yield* Effect.fail( new Error(`Offset ${file.offset} is out of range for this file (${file.count} lines)`), @@ -347,7 +396,9 @@ export const ReadTool = Tool.define< const last = file.offset + file.raw.length - 1 const next = last + 1 - const truncated = file.more || file.cut + // altimate_change start — include clipped lines in truncation metadata + const truncated = file.more || file.cut || file.lineCut + // altimate_change end if (file.cut) { output += `\n\n(Output capped at ${MAX_BYTES_LABEL}. Showing lines ${file.offset}-${last}. Use offset=${next} to continue.)` } else if (file.more) { diff --git a/packages/opencode/src/tool/read.txt b/packages/opencode/src/tool/read.txt index 368174cc8d..77c8b7ec7b 100644 --- a/packages/opencode/src/tool/read.txt +++ b/packages/opencode/src/tool/read.txt @@ -8,7 +8,10 @@ Usage: - Use the grep tool to find specific content in large files or files with long lines. - If you are unsure of the correct file path, use the glob tool to look up filenames by glob pattern. - Contents are returned with each line prefixed by its line number as `: `. For example, if a file has contents "foo\n", you will receive "1: foo\n". For directories, entries are returned one per line (without line numbers) with a trailing `/` for subdirectories. -- Any line longer than 2000 characters is truncated. +// altimate_change start — document retrieval of complete long lines +- By default, any line longer than 2000 characters is truncated. +- For long lines, use offset with limit=1, then char_offset (0-indexed) and max_chars to read successive character windows. The 50 KB output cap still applies; each clipped line gives the next window's parameters. +// altimate_change end - Call this tool in parallel when you know there are multiple files you want to read. - Avoid tiny repeated slices (30 line chunks). If you need more context, read a larger window. - This tool can read image files and PDFs and return them as file attachments. diff --git a/packages/opencode/src/tool/tool.ts b/packages/opencode/src/tool/tool.ts index 5190017eec..bafd037f5b 100644 --- a/packages/opencode/src/tool/tool.ts +++ b/packages/opencode/src/tool/tool.ts @@ -71,9 +71,29 @@ export interface ExecuteResult { title: string metadata: M output: string + // altimate_change start — explicit failures survive the tool result envelope + status?: "ok" | "error" + // altimate_change end attachments?: Omit[] } +// altimate_change start — distinguish execution failures from successful diagnostic findings +function isErrorResult(result: ExecuteResult) { + return ( + result.status === "error" || + result.metadata.success === false || + (result.metadata.success !== true && Boolean(result.metadata.error)) + ) +} + +/** Turn a failed envelope into the SDK's tool-error event without discarding its output. */ +export function assertSuccess(result: ExecuteResult) { + if (isErrorResult(result)) { + throw new Error(result.output || String(result.metadata.error || "Tool execution failed")) + } +} +// altimate_change end + export interface Def< Parameters extends Schema.Decoder = Schema.Decoder, M extends Metadata = Metadata, @@ -308,18 +328,25 @@ function wrap, Result extends Metadat }), ), ) + const failed = isErrorResult(rawResult) // Ended before anything else can throw, so a call that finished is never left "running". - endTrace(rawResult.metadata?.success === false ? "error" : "success") + endTrace(failed ? "error" : "success") // humanize the tool-call title at the source so any client (chat webview, // TUI, ...) can render a readable label from state.title. - const result = { ...rawResult, title: describeToolCall(id, decoded, rawResult.title) ?? rawResult.title } + const result = { + ...rawResult, + ...(failed && { status: "error" as const, metadata: { ...rawResult.metadata, success: false } }), + title: describeToolCall(id, decoded, rawResult.title) ?? rawResult.title, + } altimateTrackSuccess(id, decoded, ctx, startTime, result) // altimate_change end if (result.metadata.truncated !== undefined) { return result } const agent = yield* agents.get(ctx.agent) - const truncated = yield* truncate.output(result.output, {}, agent) + // altimate_change start — truncation cannot re-label a failed result as successful + const truncated = yield* truncate.output(result.output, { outcome: failed ? "error" : "success" }, agent) + // altimate_change end return { ...result, output: truncated.content, diff --git a/packages/opencode/src/tool/truncate.ts b/packages/opencode/src/tool/truncate.ts index 0172eb2410..d3a8a3b0b4 100644 --- a/packages/opencode/src/tool/truncate.ts +++ b/packages/opencode/src/tool/truncate.ts @@ -24,7 +24,9 @@ export const GLOB = path.join(TRUNCATION_DIR, "*") export type Result = { content: string; truncated: false } | { content: string; truncated: true; outputPath: string } -export type Options = TruncateCore.Options +// altimate_change start — keep failed results failed in their truncation hint +export type Options = TruncateCore.Options & { outcome?: "success" | "error" } +// altimate_change end // altimate_change start — upstream_fix: named task allow-lists must not suggest denied subagents. async function taskSubagent(agent?: Agent.Info) { @@ -118,9 +120,10 @@ export const layer = Layer.effect( const file = yield* write(text) const subagent = yield* Effect.promise(() => taskSubagent(agent)) + const outcome = options.outcome === "error" ? "failed" : "succeeded" const hint = subagent - ? `The tool call succeeded but the output was truncated. Full output saved to: ${file}\nUse the Task tool to have ${subagent} agent process this file with Grep and Read (with offset/limit). Do NOT read the full file yourself - delegate to save context.` - : `The tool call succeeded but the output was truncated. Full output saved to: ${file}\nUse Grep to search the full content or Read with offset/limit to view specific sections.` + ? `The tool call ${outcome} but the output was truncated. Full output saved to: ${file}\nUse the Task tool to have ${subagent} agent process this file with Grep and Read (with offset/limit). Do NOT read the full file yourself - delegate to save context.` + : `The tool call ${outcome} but the output was truncated. Full output saved to: ${file}\nUse Grep to search the full content or Read with offset/limit to view specific sections.` return { content: TruncateCore.assemble(preview, hint, direction), diff --git a/packages/opencode/test/altimate/fixtures/read-syntax-errors.json b/packages/opencode/test/altimate/fixtures/read-syntax-errors.json new file mode 100644 index 0000000000..5224e27510 --- /dev/null +++ b/packages/opencode/test/altimate/fixtures/read-syntax-errors.json @@ -0,0 +1,133 @@ +[ + { + "name": "integer division in a CTE", + "spanId": "01a13148-cb97-7000-92a3-470964226a91", + "trace": "ses_eceb7e1eeffe14hbmGK64UM2ri.json", + "input": { + "query": "with a as (select Symbol, (row_number() over (order by Symbol)-1)//50 g from arca_etf)\nselect string_agg(q, ' union all ' order by g) from (select g, 'select '||g||' g,max(\"Adj Close\") m from query_table(['||string_agg(''''||Symbol||'''', ',' order by Symbol)||'],true) where Date like ''2015%''' q from a group by g)", + "warehouse": "scratch" + }, + "output": { + "error": "Error: The user has specified a rule which prevents you from using this specific tool call. Here are some of the relevant rules [{\"permission\":\"*\",\"pattern\":\"*\",\"action\":\"allow\"},{\"permission\":\"*\",\"pattern\":\"*\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"*\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"DROP DATABASE *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"DROP SCHEMA *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"TRUNCATE *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"drop database *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"drop schema *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"truncate *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Drop Database *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Drop Schema *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Truncate *\",\"action\":\"deny\"}]" + }, + "dialect": "duckdb" + }, + { + "name": "integer division in a nested query", + "spanId": "01a13148-e9ec-7000-b389-84fc74ce2f5a", + "trace": "ses_eceb7e1eeffe14hbmGK64UM2ri.json", + "input": { + "query": "select string_agg(q, ' union all ' order by g) from (select g, 'select '||g||' g,max(\"Adj Close\") m from query_table(['||string_agg(''''||Symbol||'''', ',' order by Symbol)||'],true) where Date like ''2015%''' q from (select Symbol, (row_number() over (order by Symbol)-1)//50 g from arca_etf) a group by g) b", + "warehouse": "scratch" + }, + "output": { + "error": "Error: The user has specified a rule which prevents you from using this specific tool call. Here are some of the relevant rules [{\"permission\":\"*\",\"pattern\":\"*\",\"action\":\"allow\"},{\"permission\":\"*\",\"pattern\":\"*\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"*\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"DROP DATABASE *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"DROP SCHEMA *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"TRUNCATE *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"drop database *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"drop schema *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"truncate *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Drop Database *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Drop Schema *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Truncate *\",\"action\":\"deny\"}]" + }, + "dialect": "duckdb" + }, + { + "name": "integer division with ordered aggregation", + "spanId": "01a13148-f629-7000-b939-a1b12fc2e6d8", + "trace": "ses_eceb7e1eeffe14hbmGK64UM2ri.json", + "input": { + "query": "select g, string_agg('''' || Symbol || '''', ',' order by Symbol) l from (select Symbol, (row_number() over (order by Symbol)-1)//100 g from arca_etf) a group by g order by g", + "warehouse": "scratch" + }, + "output": { + "error": "Error: The user has specified a rule which prevents you from using this specific tool call. Here are some of the relevant rules [{\"permission\":\"*\",\"pattern\":\"*\",\"action\":\"allow\"},{\"permission\":\"*\",\"pattern\":\"*\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"*\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"DROP DATABASE *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"DROP SCHEMA *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"TRUNCATE *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"drop database *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"drop schema *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"truncate *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Drop Database *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Drop Schema *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Truncate *\",\"action\":\"deny\"}]" + }, + "dialect": "duckdb" + }, + { + "name": "integer division with generated SQL", + "spanId": "01a13144-1d86-7000-bea4-27cf772c506d", + "trace": "ses_ecebcc6e6ffembt21s7RmFYqdI.json", + "input": { + "limit": 20, + "query": "WITH t AS (SELECT Symbol, (row_number() OVER (ORDER BY Symbol)-1)//100 g FROM arca_etf)\nSELECT g, 'SELECT '||g||' g, max(\"Adj Close\") m FROM query_table(['||string_agg(''''||Symbol||'''', ',' ORDER BY Symbol)||'],true) WHERE \"Date\" LIKE ''2015%'' AND \"Adj Close\">200' q FROM t GROUP BY g ORDER BY g", + "warehouse": "scratch" + }, + "output": { + "error": "Error: The user has specified a rule which prevents you from using this specific tool call. Here are some of the relevant rules [{\"permission\":\"*\",\"pattern\":\"*\",\"action\":\"allow\"},{\"permission\":\"*\",\"pattern\":\"*\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"*\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"DROP DATABASE *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"DROP SCHEMA *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"TRUNCATE *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"drop database *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"drop schema *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"truncate *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Drop Database *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Drop Schema *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Truncate *\",\"action\":\"deny\"}]" + }, + "dialect": "duckdb" + }, + { + "name": "integer division with grouped symbols", + "spanId": "01a13144-31d9-7000-9cb0-fb6ec2777d13", + "trace": "ses_ecebcc6e6ffembt21s7RmFYqdI.json", + "input": { + "limit": 20, + "query": "WITH t AS (SELECT Symbol, (row_number() OVER (ORDER BY Symbol)-1)//100 g FROM arca_etf)\nSELECT g, string_agg(Symbol, ',' ORDER BY Symbol) syms FROM t GROUP BY g ORDER BY g", + "warehouse": "scratch" + }, + "output": { + "error": "Error: The user has specified a rule which prevents you from using this specific tool call. Here are some of the relevant rules [{\"permission\":\"*\",\"pattern\":\"*\",\"action\":\"allow\"},{\"permission\":\"*\",\"pattern\":\"*\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"*\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"DROP DATABASE *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"DROP SCHEMA *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"TRUNCATE *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"drop database *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"drop schema *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"truncate *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Drop Database *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Drop Schema *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Truncate *\",\"action\":\"deny\"}]" + }, + "dialect": "duckdb" + }, + { + "name": "integer division with a derived table", + "spanId": "01a13144-3e6d-7000-8da4-d1822918329a", + "trace": "ses_ecebcc6e6ffembt21s7RmFYqdI.json", + "input": { + "limit": 20, + "query": "SELECT g, string_agg(Symbol, ',' ORDER BY Symbol) syms FROM (SELECT Symbol, (row_number() OVER (ORDER BY Symbol)-1)//100 AS g FROM arca_etf) x GROUP BY g ORDER BY g", + "warehouse": "scratch" + }, + "output": { + "error": "Error: The user has specified a rule which prevents you from using this specific tool call. Here are some of the relevant rules [{\"permission\":\"*\",\"pattern\":\"*\",\"action\":\"allow\"},{\"permission\":\"*\",\"pattern\":\"*\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"*\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"DROP DATABASE *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"DROP SCHEMA *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"TRUNCATE *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"drop database *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"drop schema *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"truncate *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Drop Database *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Drop Schema *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Truncate *\",\"action\":\"deny\"}]" + }, + "dialect": "duckdb" + }, + { + "name": "negated glob predicate", + "spanId": "01a13142-c957-7000-84ae-12b2ecbcd2e3", + "trace": "ses_ecebd6b1dffeRDuonEl8X8feO7.json", + "input": { + "query": "select filing_date from publicationinfo where substr(trim(filing_date),-4) not glob '[12][0-9][0-9][0-9]' group by substr(filing_date,1,3) limit 30", + "warehouse": "publication_database" + }, + "output": { + "error": "Error: The user has specified a rule which prevents you from using this specific tool call. Here are some of the relevant rules [{\"permission\":\"*\",\"pattern\":\"*\",\"action\":\"allow\"},{\"permission\":\"*\",\"pattern\":\"*\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"*\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"DROP DATABASE *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"DROP SCHEMA *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"TRUNCATE *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"drop database *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"drop schema *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"truncate *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Drop Database *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Drop Schema *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Truncate *\",\"action\":\"deny\"}]" + }, + "dialect": "sqlite" + }, + { + "name": "pivot of a select", + "spanId": "01a1314c-3e88-7000-bd64-3d1a426b1b46", + "trace": "ses_eceb4d139ffegfOExGvw2GUq0e.json", + "input": { + "query": "pivot (select region, cat from cls) on cat using count(*)", + "warehouse": "scratch" + }, + "output": { + "error": "Error: The user has specified a rule which prevents you from using this specific tool call. Here are some of the relevant rules [{\"permission\":\"*\",\"pattern\":\"*\",\"action\":\"allow\"},{\"permission\":\"*\",\"pattern\":\"*\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"*\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"DROP DATABASE *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"DROP SCHEMA *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"TRUNCATE *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"drop database *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"drop schema *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"truncate *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Drop Database *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Drop Schema *\",\"action\":\"deny\"},{\"permission\":\"sql_execute_write\",\"pattern\":\"Truncate *\",\"action\":\"deny\"}]" + }, + "dialect": "duckdb" + }, + { + "name": "materialized integer division", + "spanId": "01a1314c-6b05-7000-926d-277fe6273296", + "trace": "ses_eceb4d139ffegfOExGvw2GUq0e.json", + "input": { + "source_warehouse": "scratch", + "sql": "with d as (select article_id, ' '||regexp_replace(lower(coalesce(title,'')||' '||coalesce(description,'')),'[^a-z0-9.\\-]+',' ','g')||' ' as txt from arts),\nlx as (select distinct cat, kw from lex),\ns as (select d.article_id, lx.cat, sum((len(d.txt)-len(replace(d.txt,' '||kw||' ','')))//(len(kw)+2)) as sc\n from d cross join lx group by all),\nr as (select article_id, cat, sc, row_number() over (partition by article_id order by sc desc, cat) rn, lead(sc) over (partition by article_id order by sc desc, cat) sc2 from s)\nselect article_id, cat, sc, sc2 from r where rn=1 and sc>=2 and sc>=sc2+2", + "target_table": "seed" + }, + "output": "Failed to materialize: sql_materialize only reads from the source warehouse: use a single read-only SELECT (or WITH ... SELECT), without modifying CTEs, SELECT INTO or side-effect functions. Its only write target is the scratch database.", + "dialect": "duckdb" + }, + { + "name": "materialized joins with prior aliases", + "spanId": "01a1314c-3f6e-7000-8de6-7dacf7e051ca", + "trace": "ses_eceb6072effebB5RHVoSWCUtKj.json", + "input": { + "source_warehouse": "scratch", + "sql": "with wc as (select l.c, t.w, count(*) n from toks t join lab0 l using(article_id) group by 1,2),\nvocab as (select w from wc group by w having sum(n)>=3),\nV as (select count(*) v from vocab),\ntot as (select c, sum(n) tn from wc where w in (select w from vocab) group by c),\ncls as (select distinct c from lab0),\nprior as (select c, ln(count(*)*1.0/(select count(*) from lab0)) lp from lab0 group by c),\nlp as (select cls.c, vocab.w, ln((coalesce(wc.n,0)+1.0)/(tot.tn+(select v from V))) l\n from cls cross join vocab join tot on tot.c=cls.c left join wc on wc.c=cls.c and wc.w=vocab.w),\nsc as (select t.article_id, lp.c, sum(lp.l) s from toks t join lp using(w) group by 1,2),\nsc2 as (select sc.article_id, sc.c, sc.s+prior.lp s from sc join prior using(c))\nselect article_id, arg_max(c, s) c from sc2 group by 1", + "target_table": "lab1" + }, + "output": "Failed to materialize: sql_materialize only reads from the source warehouse: use a single read-only SELECT (or WITH ... SELECT), without modifying CTEs, SELECT INTO or side-effect functions. Its only write target is the scratch database.", + "dialect": "duckdb" + } +] diff --git a/packages/opencode/test/altimate/sql-materialize-mongo.test.ts b/packages/opencode/test/altimate/sql-materialize-mongo.test.ts index 6f2cd34c51..f60d5ee1e5 100644 --- a/packages/opencode/test/altimate/sql-materialize-mongo.test.ts +++ b/packages/opencode/test/altimate/sql-materialize-mongo.test.ts @@ -263,3 +263,24 @@ describe("MongoDB materialization preflight", () => { expect(seen).toHaveLength(0) }) }) + +test("materializes a collection with an embedded find filter and projection", async () => { + const command = { collection: "articles", find: {}, projection: { _id: 0, article_id: 1, title: 1, description: 1 } } + const seen = fakeMongo((_call, index) => (index === 0 ? result([[1]], ["n"]) : result([["body"]]))) + const materialized = await materialize({ ...input, sql: JSON.stringify(command) }) + expect(materialized.rows).toBe(1) + expect(seen[2].command).toEqual({ + collection: "articles", + command: "find", + filter: {}, + projection: command.projection, + }) +}) + +test.each(["$out", "$merge"])("embedded aggregate %s is refused before source reads", async (stage) => { + const seen = fakeMongo(() => result([[1]], ["n"])) + await expect( + materialize({ ...input, sql: JSON.stringify({ collection: "articles", aggregate: [{ [stage]: "target" }] }) }), + ).rejects.toThrow(/only reads/) + expect(seen).toHaveLength(0) +}) diff --git a/packages/opencode/test/altimate/sql-read-syntax.test.ts b/packages/opencode/test/altimate/sql-read-syntax.test.ts new file mode 100644 index 0000000000..a4c96dc1d9 --- /dev/null +++ b/packages/opencode/test/altimate/sql-read-syntax.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, test } from "bun:test" +import { classifyAndCheck, isSingleQuery } from "../../src/altimate/tools/sql-classify" +import { readSql } from "../../src/altimate/tools/sql-check" +import fixtures from "./fixtures/read-syntax-errors.json" + +describe("read-only dialect syntax", () => { + for (const fixture of fixtures) { + test(fixture.name, () => { + const sql = fixture.input.query ?? fixture.input.sql! + expect(classifyAndCheck(sql, fixture.dialect)).toEqual({ queryType: "read", blocked: false }) + expect(isSingleQuery(sql, fixture.dialect)).toBe(true) + if (fixture.input.sql) expect(readSql(sql, fixture.dialect)).toBe(sql) + }) + } + + test("DuckDB unpivot accepts a table or a select source", () => { + for (const source of ["monthly_sales", "(SELECT jan, feb FROM monthly_sales)"]) { + const sql = `UNPIVOT ${source} ON jan, feb INTO NAME month VALUE sales` + expect(classifyAndCheck(sql, "duckdb")).toEqual({ queryType: "read", blocked: false }) + expect(isSingleQuery(sql, "duckdb")).toBe(true) + } + }) + + test("DuckDB pivots and unpivots remain reads inside CTEs and derived tables", () => { + for (const sql of [ + "WITH t AS (SELECT 1 region, 2 cat) PIVOT t ON cat USING count(*)", + "WITH t AS (SELECT 1 jan, 2 feb) UNPIVOT t ON jan, feb INTO NAME month VALUE sales", + "SELECT * FROM (UNPIVOT monthly_sales ON jan, feb INTO NAME month VALUE sales)", + "WITH t AS (UNPIVOT monthly_sales ON jan, feb INTO NAME month VALUE sales) SELECT * FROM t", + ]) { + expect(classifyAndCheck(sql, "duckdb")).toEqual({ queryType: "read", blocked: false }) + expect(isSingleQuery(sql, "duckdb")).toBe(true) + } + }) + + test("negated glob is read-only on SQLite and DuckDB", () => { + for (const dialect of ["sqlite", "duckdb"]) { + expect(classifyAndCheck("SELECT 'abc' NOT GLOB 'a*'", dialect)).toEqual({ queryType: "read", blocked: false }) + } + }) + + test("dialect syntax cannot hide writes, extra statements, or broken quoting", () => { + for (const sql of [ + "SELECT 10//3; DELETE FROM users", + "SELECT 10//3 INTO copy FROM users", + "SELECT * FROM users WHERE name NOT GLOB 'a*'; UPDATE users SET name = 'b'", + "PIVOT (DELETE FROM users RETURNING *) ON name USING count(*)", + "PIVOT users ON name USING count(*); COPY users TO '/tmp/export.csv'", + "UNPIVOT (DELETE FROM users RETURNING *) ON jan, feb INTO NAME month VALUE sales", + "UNPIVOT monthly_sales ON jan, feb INTO NAME month VALUE sales; SET threads = 1", + "SELECT * INTO name FROM (UNPIVOT monthly_sales ON jan, feb INTO NAME month VALUE sales)", + "SELECT unpivot INTO name FROM users", + "WITH t AS (SELECT 1 unpivot) SELECT unpivot INTO name FROM t", + "WITH t AS (UNPIVOT monthly_sales ON jan, feb INTO NAME month VALUE sales) SELECT * INTO name FROM t", + "UNPIVOT (SELECT * INTO name FROM monthly_sales) ON jan, feb INTO NAME month VALUE sales", + "SELECT 10//3, 'unterminated", + "SELECT 10//3; /*! DELETE FROM users */", + "SELECT 10//3; CHECKPOINT", + "PIVOT users ON name USING count(*); CHECKPOINT", + ]) { + expect(classifyAndCheck(sql, "duckdb").queryType).toBe("write") + } + expect(classifyAndCheck("SELECT 10//3; TRUNCATE users", "duckdb")).toEqual({ queryType: "write", blocked: true }) + }) + + test("unsupported dialects do not inherit DuckDB-only syntax", () => { + expect(classifyAndCheck("SELECT 10//3", "sqlite").queryType).toBe("write") + expect(classifyAndCheck("SELECT 10//3", "postgres").queryType).toBe("write") + expect(classifyAndCheck("PIVOT users ON name USING count(*)", "sqlite").queryType).toBe("write") + }) +}) diff --git a/packages/opencode/test/altimate/sql-validation-e2e.test.ts b/packages/opencode/test/altimate/sql-validation-e2e.test.ts index 62b1c22453..09db026ada 100644 --- a/packages/opencode/test/altimate/sql-validation-e2e.test.ts +++ b/packages/opencode/test/altimate/sql-validation-e2e.test.ts @@ -67,6 +67,9 @@ describe("Tool name consistency in prompts", () => { const analyst = await Agent.get("analyst") expect(analyst).toBeDefined() expect(analyst!.prompt).not.toContain("sql_validate") + expect(analyst!.prompt).toContain( + "keep that reasoning out of the final message. When a rule or policy decides the answer, read its full text (not a preview), enumerate each competing clause, and evaluate every condition as satisfied, violated or unknown against the row's own fields and dates before choosing.", + ) }, }) }) diff --git a/packages/opencode/test/altimate/tool-result-errors.test.ts b/packages/opencode/test/altimate/tool-result-errors.test.ts new file mode 100644 index 0000000000..91fca75772 --- /dev/null +++ b/packages/opencode/test/altimate/tool-result-errors.test.ts @@ -0,0 +1,84 @@ +import { afterEach, describe, expect, spyOn, test } from "bun:test" +import z from "zod" +import { Tool } from "../../src/tool/tool" +import { Telemetry } from "../../src/altimate/telemetry" +import { initTool } from "./tool-fixture" + +const parserOutput = `Failed to execute SQL: Error: Parser Error: syntax error at or near "nulls" + +LINE 1: ..., sum(case when normalized_count is null then 1 else 0 end) nulls from lgg l join igf2 i on i.ParticipantBarcode=l.b... + ^` +const materializeOutput = + "Failed to materialize: The source query has more than 2,000,000 rows, above the row cap of 2,000,000 rows; nothing was materialized. Aggregate or filter in the source query, select fewer columns, or raise max_rows (hard limit 20,000,000) / max_bytes (hard limit 2 GiB)." + +const ctx: any = { + sessionID: "ses_tool_errors", + messageID: "msg_tool_errors", + agent: "build", + abort: AbortSignal.any([]), + messages: [], + metadata() {}, + async ask() {}, +} +const spies: Array<{ mockRestore(): void }> = [] +afterEach(() => { + for (const spy of spies.splice(0)) spy.mockRestore() +}) + +async function execute(id: string, output: string, metadata: Record) { + const tool = await initTool( + Tool.define(id, { + description: "Tool result fixture", + parameters: z.object({}), + async execute() { + return { title: id, output, metadata } + }, + }), + ) + return tool.execute({}, ctx) +} + +describe("tool result failures", () => { + test.each([ + ["sql_execute", parserOutput, parserOutput.slice("Failed to execute SQL: ".length)], + ["sql_materialize", materializeOutput, materializeOutput.slice("Failed to materialize: ".length)], + ])("%s marks backend error metadata as a failed envelope and telemetry event", async (id, output, error) => { + const events: Telemetry.Event[] = [] + spies.push( + spyOn(Telemetry, "track").mockImplementation((event) => { + events.push(event) + }), + ) + const result = await execute(id, output, { error, truncated: false }) + expect(result.status).toBe("error") + expect(result.metadata.success).toBe(false) + expect(result.output).toBe(output) + expect(events.find((event) => event.type === "tool_call")).toMatchObject({ status: "error" }) + expect(events.some((event) => event.type === "core_failure")).toBe(true) + expect(() => Tool.assertSuccess(result)).toThrow(output) + }) + + test("explicit soft failure becomes an SDK error even without error metadata", async () => { + const result = await execute("analysis", "Could not run the requested operation", { success: false }) + expect(result.status).toBe("error") + expect(() => Tool.assertSuccess(result)).toThrow(result.output) + }) + + test("successful text containing errors is not classified as execution failure", async () => { + const result = await execute("sql_execute", `message\n${parserOutput}`, { rowCount: 1 }) + expect(result.status).not.toBe("error") + expect(() => Tool.assertSuccess(result)).not.toThrow() + }) + + test("successful validation may carry findings in error metadata", async () => { + const result = await execute("altimate_core_validate", "Table 'users' not found", { + success: true, + valid: false, + error: "Table 'users' not found", + findings: [{ category: "missing_table" }], + }) + expect(result.status).not.toBe("error") + expect(result.metadata.success).toBe(true) + expect(() => Tool.assertSuccess(result)).not.toThrow() + }) +}) diff --git a/packages/opencode/test/altimate/tools/sql-execute.test.ts b/packages/opencode/test/altimate/tools/sql-execute.test.ts index fb66cf4016..7cd06e6ebf 100644 --- a/packages/opencode/test/altimate/tools/sql-execute.test.ts +++ b/packages/opencode/test/altimate/tools/sql-execute.test.ts @@ -311,3 +311,51 @@ failedSpill.live("a spill service defect keeps SQL success, preview and clipping expect(output.metadata.output_truncated).toBe(true) }), ) + +describe("sql_execute full text selection", () => { + test("requires an explicit row bound for full_text_columns before dispatch", async () => { + const tool = await initTool(SqlExecuteTool) + for (const limit of [undefined, 21]) { + await expect(tool.execute({ query: "SELECT 42", full_text_columns: ["answer"], limit }, ctx)).rejects.toThrow( + "limit <= 20", + ) + } + expect(dispatch).not.toHaveBeenCalled() + expect(precedence).not.toHaveBeenCalled() + }) + + test("passes selected columns to the formatter without changing the executed query", async () => { + const text = "Complete policy condition. ".repeat(350) + dispatch.mockResolvedValue({ + columns: ["policy", "notes"], + rows: [[text, "x".repeat(20000)]], + row_count: 1, + truncated: false, + } as never) + const tool = await initTool(SqlExecuteTool) + const output = await tool.execute( + { query: "SELECT policy, notes FROM rules", limit: 1, full_text_columns: ["policy"] }, + ctx, + ) + expect(output.output).toContain(text) + expect(output.output).not.toContain("cell clipped") + expect(dispatch).toHaveBeenCalledWith( + "sql.execute", + expect.objectContaining({ sql: "SELECT policy, notes FROM rules", limit: 1 }), + ) + }) +}) + +test("full text selection retains write permission checks", async () => { + const tool = await initTool(SqlExecuteTool) + const ask = spyOn(ctx, "ask").mockImplementation(() => Effect.die(new Error("write denied"))) + try { + await expect( + tool.execute({ query: "DELETE FROM rules RETURNING policy", limit: 1, full_text_columns: ["policy"] }, ctx), + ).rejects.toThrow("write denied") + expect(ask).toHaveBeenCalledTimes(1) + expect(dispatch).not.toHaveBeenCalled() + } finally { + ask.mockRestore() + } +}) diff --git a/packages/opencode/test/altimate/tools/sql-result-format.test.ts b/packages/opencode/test/altimate/tools/sql-result-format.test.ts index e0bbf3bc5c..1cd6969376 100644 --- a/packages/opencode/test/altimate/tools/sql-result-format.test.ts +++ b/packages/opencode/test/altimate/tools/sql-result-format.test.ts @@ -441,3 +441,96 @@ describe("empty-result hint", () => { expect((await run([], undefined as any)).output).toBe("(0 rows)") }) }) + +describe("SQL full text retrieval", () => { + test("returns selected text columns without clipping competing wide cells", async () => { + const saved = spill() + const text = "Complete policy condition. ".repeat(350) + const formatted = await formatSqlResult( + result([[text, "x".repeat(9000), "y".repeat(9000)]], ["policy", "detail", "notes"]), + { + limit: 1, + fullTextColumns: ["policy"], + write: saved.write, + }, + ) + expect(formatted.output).toContain(text) + expect(formatted.output).not.toContain("cell clipped") + expect(formatted.metadata.preview_columns).toBe(1) + expect(Buffer.byteLength(formatted.output)).toBeLessThanOrEqual(SQL_MAX_PREVIEW_BYTES) + }) + + test("spills oversized full text without displaying a partial cell", async () => { + const saved = spill() + const text = "Full condition. ".repeat(2000) + const formatted = await formatSqlResult(result([[text]], ["policy"]), { + limit: 1, + fullTextColumns: ["policy"], + write: saved.write, + }) + expect(formatted.output).not.toContain("cell clipped") + expect(formatted.metadata.preview_rows).toBe(0) + expect(formatted.metadata.output_truncated).toBe(true) + expect(formatted.output).toContain("read") + expect(saved.rows()[0].values[0]).toBe(text) + expect(Buffer.byteLength(formatted.output)).toBeLessThanOrEqual(SQL_MAX_PREVIEW_BYTES) + }) + + test("truncation explains bounded full text retrieval", async () => { + const formatted = await formatSqlResult(result([["x".repeat(20000)]]), { limit: 100, write: spill().write }) + expect(formatted.output).toContain("full_text_columns") + expect(formatted.output).toContain("limit <= 20") + }) + + test("rejects full text above the row bound or for unknown columns", async () => { + await expect( + formatSqlResult(result([["text"]]), { limit: 21, fullTextColumns: ["value"], write: spill().write }), + ).rejects.toThrow("limit <= 20") + await expect( + formatSqlResult(result([["text"]]), { limit: 1, fullTextColumns: ["missing"], write: spill().write }), + ).rejects.toThrow("Unknown full_text_columns") + }) + + test("pads absent spill values with null while retaining explicit undefined and column order", async () => { + const saved = spill() + await formatSqlResult(result([[1], [2, undefined], [3, null]], ["id", "grant_date"], true), { + limit: 3, + write: saved.write, + }) + expect(saved.rows()).toEqual([ + { columns: ["id", "grant_date"], values: [1, null] }, + { columns: ["id", "grant_date"], values: [2, { $sql_type: "undefined" }] }, + { columns: ["id", "grant_date"], values: [3, null] }, + ]) + }) +}) + +test("full text selection respects configured bytes and spills the original columns", async () => { + const saved = spill() + const text = "Complete condition. ".repeat(100) + const formatted = await formatSqlResult(result([[17, text]], ["id", "policy"]), { + limit: 1, + fullTextColumns: ["policy"], + maxBytes: 1024, + write: saved.write, + }) + expect(Buffer.byteLength(formatted.output)).toBeLessThanOrEqual(1024) + expect(formatted.metadata.preview_rows).toBe(0) + expect(saved.rows()[0]).toEqual({ columns: ["id", "policy"], values: [17, text] }) +}) + +test.each([[new Uint8Array(SQL_MAX_PREVIEW_BYTES * 2)], [Array(20000).fill("large")]])( + "full text omits rows containing internally clipped values", + async (value) => { + const saved = spill() + const formatted = await formatSqlResult(result([[value]]), { + limit: 1, + fullTextColumns: ["value"], + write: saved.write, + }) + expect(formatted.metadata.preview_rows).toBe(0) + expect(formatted.output).not.toContain("cell clipped") + expect(formatted.output).not.toContain("array clipped") + expect(saved.rows()).toHaveLength(1) + }, +) diff --git a/packages/opencode/test/session/tools-result-errors.test.ts b/packages/opencode/test/session/tools-result-errors.test.ts new file mode 100644 index 0000000000..c3d4af717b --- /dev/null +++ b/packages/opencode/test/session/tools-result-errors.test.ts @@ -0,0 +1,130 @@ +// altimate_change start — failed backend results must reject at the SDK tool boundary +import { expect } from "bun:test" +import { Cause, Effect, Exit, Layer, Schema } from "effect" +import { ProjectV2 } from "@opencode-ai/core/project" +import { Agent } from "@/agent/agent" +import { MCP } from "@/mcp" +import { Permission } from "@/permission" +import { Plugin } from "@/plugin" +import type { Provider } from "@/provider/provider" +import { ModelID, ProviderID } from "@/provider/schema" +import { MessageID, SessionID } from "@/session/schema" +import { SessionTools } from "@/session/tools" +import { ToolRegistry } from "@/tool/registry" +import { Truncate } from "@/tool/truncate" +import { testEffect } from "../lib/effect" + +const model: Provider.Model = { + id: ModelID.make("test-model"), + providerID: ProviderID.make("test"), + api: { id: "test-model", url: "https://example.com", npm: "@ai-sdk/openai" }, + name: "Test model", + capabilities: { + temperature: true, + reasoning: false, + attachment: false, + toolcall: true, + interleaved: false, + input: { text: true, audio: false, image: false, video: false, pdf: false }, + output: { text: true, audio: false, image: false, video: false, pdf: false }, + }, + cost: { input: 0, output: 0, cache: { read: 0, write: 0 } }, + limit: { context: 128000, output: 4096 }, + status: "active", + options: {}, + headers: {}, + release_date: "2026-01-01", +} +const agent: Agent.Info = { + name: "build", + mode: "primary", + options: {}, + permission: Permission.fromConfig({ "*": "allow" }), +} +const failure = 'Failed to execute SQL: Error: Parser Error: syntax error at or near "nulls"' +const it = testEffect( + Layer.mergeAll( + Layer.mock(Plugin.Service, { trigger: (_name, _input, output) => Effect.succeed(output) }), + Layer.mock(Permission.Service, {}), + Layer.mock(MCP.Service, { tools: () => Effect.succeed({}) }), + Layer.mock(Truncate.Service, {}), + Layer.mock(ToolRegistry.Service, { + tools: () => + Effect.succeed([ + { + id: "backend_failure", + description: "Return a backend failure", + parameters: Schema.Struct({}), + registrySource: undefined, + execute: () => + Effect.succeed({ + title: "SQL: ERROR", + output: failure, + status: "error" as const, + metadata: { success: false, error: failure }, + }), + }, + ]), + }), + ), +) + +it.live("session/tools rejects backend error envelopes at the SDK boundary", () => + Effect.gen(function* () { + const sessionID = SessionID.make("ses_tools_errors") + const messageID = MessageID.make("msg_tools_errors") + const tools = yield* SessionTools.resolve({ + agent, + model, + bypassAgentCheck: false, + messages: [], + session: { + id: sessionID, + slug: "tools-errors", + projectID: ProjectV2.ID.make("project"), + directory: "/tmp", + title: "Tool errors", + version: "1", + time: { created: 0, updated: 0 }, + }, + processor: { + message: { + id: messageID, + sessionID, + role: "assistant", + time: { created: 0 }, + parentID: MessageID.make("msg_user"), + modelID: model.id, + providerID: model.providerID, + mode: "build", + agent: "build", + path: { cwd: "/tmp", root: "/tmp" }, + cost: 0, + tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, + }, + updateToolCall: () => Effect.succeed(undefined), + completeToolCall: () => Effect.void, + }, + promptOps: { + resolvePromptParts: () => Effect.succeed([]), + prompt: () => Effect.die("unexpected prompt"), + cancel: () => Effect.void, + }, + }) + const result = yield* Effect.promise(() => + Promise.resolve( + tools.backend_failure.execute!( + {}, + { + toolCallId: "call_error", + messages: [], + abortSignal: AbortSignal.any([]), + }, + ), + ), + ).pipe(Effect.exit) + expect(Exit.isFailure(result)).toBe(true) + if (Exit.isFailure(result)) expect(Cause.pretty(result.cause)).toContain(failure) + }), +) +// altimate_change end diff --git a/packages/opencode/test/tool/bash.test.ts b/packages/opencode/test/tool/bash.test.ts index 982ed0a412..7060db513b 100644 --- a/packages/opencode/test/tool/bash.test.ts +++ b/packages/opencode/test/tool/bash.test.ts @@ -500,3 +500,108 @@ describe("tool.bash PATH injection", () => { }) }) // altimate_change end + +// altimate_change start — failed shell commands and Python tracebacks are failed tool results +describe("tool.bash error envelopes", () => { + const traceback = `Traceback (most recent call last): + File "/tmp/ema.py", line 13, in + else: g,f,cls=r['grant_date'],r['filing_date'],r['cls'] + ~^^^^^^^^^^^^^^ +KeyError: 'grant_date' +` + const missingModule = `Traceback (most recent call last): + File "", line 1, in +ModuleNotFoundError: No module named 'sklearn' +` + + test("marks a nonzero exit as an error and preserves output", async () => { + await provideInstance(projectRoot, async () => { + const bash = await initTool(BashTool) + const result = await bash.execute({ command: "echo 'failed command'; exit 7", description: "Fail a command" }, ctx) + expect(result.metadata.exit).toBe(7) + expect(result.metadata.success).toBe(false) + expect(result.status).toBe("error") + expect(result.output).toBe("failed command\n") + }) + }) + + test("marks a trailing Python traceback as an error despite a zero shell exit", async () => { + await provideInstance(projectRoot, async () => { + const bash = await initTool(BashTool) + const result = await bash.execute({ command: `cat <<'TRACEBACK'\n${traceback}TRACEBACK`, description: "Show traceback" }, ctx) + expect(result.metadata.exit).toBe(0) + expect(result.status).toBe("error") + expect(result.metadata.error).toContain("KeyError: 'grant_date'") + expect(result.output).toBe(traceback) + }) + }) + + test("recognizes a trailing stderr traceback followed by successful stdout", async () => { + await provideInstance(projectRoot, async () => { + const bash = await initTool(BashTool) + const result = await bash.execute({ + command: `cat >&2 <<'TRACEBACK'\n${missingModule}TRACEBACK\necho 'next command succeeded'`, + description: "Show missing module and subsequent output", + }, ctx) + expect(result.metadata.exit).toBe(0) + expect(result.status).toBe("error") + expect(result.metadata.error).toContain("ModuleNotFoundError: No module named 'sklearn'") + expect(result.output).toContain("next command succeeded") + }) + }) + + test("successful output mentioning errors or a recovered stdout traceback stays successful", async () => { + await provideInstance(projectRoot, async () => { + const bash = await initTool(BashTool) + const result = await bash.execute({ + command: `cat <<'TRACEBACK'\n${traceback}Recovered successfully\nTRACEBACK`, + description: "Show a recovered traceback", + }, ctx) + expect(result.metadata.exit).toBe(0) + expect(result.status).not.toBe("error") + }) + }) +}) +// altimate_change end + +// altimate_change start — the truncation envelope must not relabel failed commands as successful +test("tool.bash truncation retains the failure outcome and complete output spill", async () => { + await provideInstance(projectRoot, async () => { + const bash = await initToolWithTruncate(BashTool) + const result = await bash.execute({ + command: `seq 1 ${Truncate.MAX_LINES + 10}; exit 1`, description: "Fail after generating long output", + }, ctx) + expect(result.status).toBe("error") + expect(result.metadata.truncated).toBe(true) + expect(result.output).toContain("The tool call failed but the output was truncated") + expect(result.output).not.toContain("The tool call succeeded") + expect(await Filesystem.readText(result.metadata.outputPath)).toContain(`${Truncate.MAX_LINES + 10}\n`) + }) +}) +// altimate_change end + +// altimate_change start — draining pipes must remain bounded after the shell exits +test("tool.bash timeout still bounds output held open by a background child", async () => { + await provideInstance(projectRoot, async () => { + const bash = await initTool(BashTool) + const started = Date.now() + const result = await bash.execute({ + command: "sleep 2 & exit 0", description: "Leave a child holding the output pipe", timeout: 20, + }, ctx) + expect(result.status).toBe("error") + expect(result.metadata.error).toContain("timed out") + expect(Date.now() - started).toBeLessThan(1500) + }) +}) + +test("tool.bash handles an already-aborted command without losing the close event", async () => { + await provideInstance(projectRoot, async () => { + const bash = await initTool(BashTool) + const result = await bash.execute({ command: "sleep 2", description: "Abort a command", timeout: 20 }, { + ...ctx, abort: AbortSignal.abort(), + }) + expect(result.status).toBe("error") + expect(result.metadata.error).toContain("aborted") + }) +}, 3000) +// altimate_change end diff --git a/packages/opencode/test/tool/fixtures/long-query.sql b/packages/opencode/test/tool/fixtures/long-query.sql new file mode 100644 index 0000000000..7d2dbb2b43 --- /dev/null +++ b/packages/opencode/test/tool/fixtures/long-query.sql @@ -0,0 +1 @@ +SELECT DISTINCT s FROM(SELECT'AAAU's FROM"AAAU"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AADR's FROM"AADR"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'ABEQ's FROM"ABEQ"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'ACSG's FROM"ACSG"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'ACWF's FROM"ACWF"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AFK's FROM"AFK"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AFLG's FROM"AFLG"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AFMC's FROM"AFMC"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AFSM's FROM"AFSM"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AFTY's FROM"AFTY"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AGG's FROM"AGG"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AGGP's FROM"AGGP"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AGGY's FROM"AGGY"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AGQ's FROM"AGQ"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AGZ's FROM"AGZ"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AIEQ's FROM"AIEQ"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AIIQ's FROM"AIIQ"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AMLP's FROM"AMLP"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AMOM's FROM"AMOM"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AMZA's FROM"AMZA"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AOA's FROM"AOA"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AOK's FROM"AOK"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AOM's FROM"AOM"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AOR's FROM"AOR"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'ARGT's FROM"ARGT"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'ARKF's FROM"ARKF"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'ARKK's FROM"ARKK"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'ARKW's FROM"ARKW"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'ARMR's FROM"ARMR"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'ASEA's FROM"ASEA"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'ASHR's FROM"ASHR"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'ASHS's FROM"ASHS"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'ASHX's FROM"ASHX"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AUSF's FROM"AUSF"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AVDE's FROM"AVDE"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AVDV's FROM"AVDV"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AVEM's FROM"AVEM"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AVUS's FROM"AVUS"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AVUV's FROM"AVUV"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AWAY's FROM"AWAY"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AWTM's FROM"AWTM"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'AXJL's FROM"AXJL"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BAB's FROM"BAB"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BATT's FROM"BATT"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BBC's FROM"BBC"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BBP's FROM"BBP"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BCD's FROM"BCD"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BCI's FROM"BCI"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BDCY's FROM"BDCY"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BDRY's FROM"BDRY"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BFOR's FROM"BFOR"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BIBL's FROM"BIBL"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BIL's FROM"BIL"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BIV's FROM"BIV"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BIZD's FROM"BIZD"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BKF's FROM"BKF"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BKLN's FROM"BKLN"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BLES's FROM"BLES"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BLHY's FROM"BLHY"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BLOK's FROM"BLOK"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BLV's FROM"BLV"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BNDC's FROM"BNDC"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BNO's FROM"BNO"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BOIL's FROM"BOIL"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BOND's FROM"BOND"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BOUT's FROM"BOUT"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BRF's FROM"BRF"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BRZU's FROM"BRZU"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BSV's FROM"BSV"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BTAL's FROM"BTAL"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BUL's FROM"BUL"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BUY's FROM"BUY"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BUYN's FROM"BUYN"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BVAL's FROM"BVAL"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BWX's FROM"BWX"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BWZ's FROM"BWZ"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BYLD's FROM"BYLD"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'BZQ's FROM"BZQ"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CANE's FROM"CANE"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CBON's FROM"CBON"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CCOR's FROM"CCOR"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CEF's FROM"CEF"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CEW's FROM"CEW"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CGW's FROM"CGW"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CHAD's FROM"CHAD"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CHAU's FROM"CHAU"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CHEP's FROM"CHEP"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CHGX's FROM"CHGX"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CHIC's FROM"CHIC"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CHIE's FROM"CHIE"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CHIH's FROM"CHIH"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CHII's FROM"CHII"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CHIK's FROM"CHIK"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CHIL's FROM"CHIL"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CHIM's FROM"CHIM"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CHIQ's FROM"CHIQ"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CHIR's FROM"CHIR"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CHIS's FROM"CHIS"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CHIU's FROM"CHIU"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CHIX's FROM"CHIX"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CLIX's FROM"CLIX"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CLTL's FROM"CLTL"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CMBS's FROM"CMBS"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CMDY's FROM"CMDY"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CMF's FROM"CMF"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CN's FROM"CN"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CNBS's FROM"CNBS"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CNRG's FROM"CNRG"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CNXT's FROM"CNXT"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'COM's FROM"COM"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'COMB's FROM"COMB"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'COPX's FROM"COPX"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CORN's FROM"CORN"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CORP's FROM"CORP"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CPER's FROM"CPER"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CPI's FROM"CPI"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CQQQ's FROM"CQQQ"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CRAK's FROM"CRAK"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CRBN's FROM"CRBN"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CROP's FROM"CROP"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CSD's FROM"CSD"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CURE's FROM"CURE"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CUT's FROM"CUT"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CVY's FROM"CVY"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CWB's FROM"CWB"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CWEB's FROM"CWEB"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CWI's FROM"CWI"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CWS's FROM"CWS"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CYB's FROM"CYB"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'CZA's FROM"CZA"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'DBA's FROM"DBA"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL SELECT'DBAW's FROM"DBAW"WHERE"Adj Close">200 AND Date LIKE'2015%' UNION ALL diff --git a/packages/opencode/test/tool/read.test.ts b/packages/opencode/test/tool/read.test.ts index e2be87a31e..5c1fa3f191 100644 --- a/packages/opencode/test/tool/read.test.ts +++ b/packages/opencode/test/tool/read.test.ts @@ -333,6 +333,78 @@ describe("tool.read env file permissions", () => { }) describe("tool.read truncation", () => { + // altimate_change start — character windows recover complete long lines + it.instance("retrieves the remainder of a clipped SQL line in character windows", () => + Effect.gen(function* () { + const test = yield* TestInstance + const content = (yield* load(path.join(FIXTURES_DIR, "long-query.sql"))).trimEnd() + const filepath = path.join(test.directory, "query.sql") + yield* put(filepath, content) + + const first = yield* run({ filePath: filepath, limit: 1 }) + expect(first.output).toContain(content.slice(0, 2000)) + expect(first.output).not.toContain(content.slice(0, 2001)) + expect(first.output).toContain("offset=1, limit=1, char_offset=2000, max_chars=2000") + expect(first.metadata.truncated).toBe(true) + + const parts: string[] = [] + for (let char_offset = 0; char_offset < content.length; char_offset += 2000) { + const result = yield* run({ filePath: filepath, limit: 1, char_offset, max_chars: 2000 }) + const text = result.metadata.display?.type === "file" ? result.metadata.display.text : "" + parts.push(text.split("... (line truncated")[0]) + } + expect(parts.join("")).toBe(content) + }), + ) + + it.instance("allows larger character windows on a selected line and keeps read permissions", () => + Effect.gen(function* () { + const test = yield* TestInstance + const filepath = path.join(test.directory, "policy.txt") + const content = "Clause ".repeat(500) + yield* put(filepath, `header\n${content}\nfooter`) + const { items, next } = asks() + const result = yield* run({ filePath: filepath, offset: 2, limit: 1, char_offset: 500, max_chars: 4000 }, next) + expect(result.metadata.display).toMatchObject({ text: content.slice(500), lineStart: 2, lineEnd: 2 }) + expect(items.some((item) => item.permission === "read")).toBe(true) + }), + ) + + it.instance("bounds oversized multibyte character windows without losing the continuation", () => + Effect.gen(function* () { + const test = yield* TestInstance + const filepath = path.join(test.directory, "long.txt") + const content = "\u754c".repeat(30_000) + yield* put(filepath, content) + const result = yield* run({ filePath: filepath, limit: 1, max_chars: 30_000 }) + const text = result.metadata.display?.type === "file" ? result.metadata.display.text : "" + expect(Buffer.byteLength(text)).toBeLessThanOrEqual(50 * 1024) + const shown = text.split("... (line truncated")[0] + expect(shown.length).toBeGreaterThan(2000) + expect(shown).toBe(content.slice(0, shown.length)) + expect(result.output).toContain(`char_offset=${shown.length}`) + expect(result.metadata.truncated).toBe(true) + }), + ) + it.instance("keeps complete astral characters at character window boundaries", () => + Effect.gen(function* () { + const test = yield* TestInstance + const filepath = path.join(test.directory, "unicode.txt") + const content = "a".repeat(1999) + "😀tail" + yield* put(filepath, content) + const first = yield* run({ filePath: filepath, limit: 1 }) + const firstText = first.metadata.display?.type === "file" ? first.metadata.display.text : "" + expect(firstText.split("... (line truncated")[0]).toBe("a".repeat(1999)) + expect(first.output).toContain("char_offset=1999") + const next = yield* run({ filePath: filepath, limit: 1, char_offset: 1999, max_chars: 1 }) + expect(next.output).toContain("1: 😀... (line truncated") + expect(next.output).toContain("char_offset=2001") + const aligned = yield* run({ filePath: filepath, limit: 1, char_offset: 2000 }) + expect(aligned.metadata.display).toMatchObject({ text: "😀tail" }) + }), + ) + // altimate_change end + it.instance("truncates large file by bytes and sets truncated metadata", () => Effect.gen(function* () { const test = yield* TestInstance From 0379b21ca7b78a70734bd91971fb2ede252af616 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Mon, 12 Oct 2026 13:24:49 -0700 Subject: [PATCH 2/9] fix(tool): keep bash results ordinary, finish on exit, and drop markers from the read prompt Review corrections to the tool-failure reporting change: - `bash` completes on process exit again, draining buffered output for a short bounded grace instead of waiting for stream close, so a command that leaves a background child running returns promptly and is not reported as a failure. - A non-zero `bash` exit (grep no-match, diff, test) stays an ordinary tool result the model reads from the exit code; `metadata.exit_code` and `metadata.success` record it for telemetry only. SDK error rejection is reserved for explicit error envelopes and backend `metadata.error` (`success:false` alone no longer qualifies). A Python traceback counts only when it is the final stderr block with a non-zero exit, or with empty stdout on a zero exit; tracebacks printed as data do not. - `read`'s tool description no longer contains source-marker comment lines. - Aborted tools call `completeToolCall` before the SDK-boundary check; `warehouse_test` declares its STORE LOCKED / INFRASTRUCTURE FAILURE guidance as a successful diagnostic; `batch` no longer counts an inner error envelope as a success. Co-Authored-By: Claude Fable 5.1 --- .../src/altimate/tools/warehouse-test.ts | 3 + packages/opencode/src/session/tools.ts | 2 +- packages/opencode/src/tool/bash.ts | 54 ++++--- packages/opencode/src/tool/batch.ts | 3 + packages/opencode/src/tool/read.txt | 2 - packages/opencode/src/tool/tool.ts | 9 +- .../test/altimate/tool-result-errors.test.ts | 23 ++- .../altimate/tools/warehouse-test.test.ts | 75 +++++++++ .../test/session/tools-result-errors.test.ts | 134 +++++++++------- packages/opencode/test/tool/bash.test.ts | 150 +++++++++++++++--- packages/opencode/test/tool/batch.test.ts | 40 +++++ packages/opencode/test/tool/read.test.ts | 12 ++ 12 files changed, 398 insertions(+), 109 deletions(-) create mode 100644 packages/opencode/test/altimate/tools/warehouse-test.test.ts diff --git a/packages/opencode/src/altimate/tools/warehouse-test.ts b/packages/opencode/src/altimate/tools/warehouse-test.ts index aa1d7d3ab3..6591aadbde 100644 --- a/packages/opencode/src/altimate/tools/warehouse-test.ts +++ b/packages/opencode/src/altimate/tools/warehouse-test.ts @@ -30,10 +30,12 @@ export const WarehouseTestTool = Tool.define("warehouse_test", { // the other process lets go, and the driver's own message names the PID // holding the lock. Telling the model to stop and report would be wrong // advice for the one fault it can actually act on. + // These guidance responses are successful diagnostics of an unhealthy connection. if (result.recoverable) { return { title: `Connection '${args.name}': STORE LOCKED`, metadata: { + success: true, connected: false, error: result.error, error_category: result.error_category, @@ -56,6 +58,7 @@ export const WarehouseTestTool = Tool.define("warehouse_test", { return { title: `Connection '${args.name}': INFRASTRUCTURE FAILURE`, metadata: { + success: true, connected: false, error: result.error, error_category: result.error_category, diff --git a/packages/opencode/src/session/tools.ts b/packages/opencode/src/session/tools.ts index 3d7b2cdaba..bac4dac095 100644 --- a/packages/opencode/src/session/tools.ts +++ b/packages/opencode/src/session/tools.ts @@ -137,10 +137,10 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { // altimate_change end ) // altimate_change start — propagate the source-stamped output - Tool.assertSuccess(stamped) if (options.abortSignal?.aborted) { yield* input.processor.completeToolCall(options.toolCallId, stamped) } + Tool.assertSuccess(stamped) return stamped // altimate_change end }), diff --git a/packages/opencode/src/tool/bash.ts b/packages/opencode/src/tool/bash.ts index 440ab5b553..96f501d23a 100644 --- a/packages/opencode/src/tool/bash.ts +++ b/packages/opencode/src/tool/bash.ts @@ -286,6 +286,7 @@ export const BashTool = Tool.define("bash", async () => { let output = "" // altimate_change start — keep stderr's tail separate from later successful stdout let stderrTail = "" + let hasStdout = false // altimate_change end // Initialize metadata with empty output @@ -307,8 +308,11 @@ export const BashTool = Tool.define("bash", async () => { }) } - proc.stdout?.on("data", append) - // altimate_change start — a pipeline can hide a Python failure behind its last command's exit + // altimate_change start — distinguish stderr failures from tracebacks printed as stdout data + proc.stdout?.on("data", (chunk: Buffer) => { + hasStdout ||= chunk.length > 0 + append(chunk) + }) proc.stderr?.on("data", (chunk: Buffer) => { stderrTail = (stderrTail + chunk.toString()).slice(-MAX_METADATA_LENGTH) append(chunk) @@ -343,22 +347,40 @@ export const BashTool = Tool.define("bash", async () => { }, timeout + 100) await new Promise((resolve, reject) => { + // altimate_change start — background descendants may keep pipes open after shell exit + let drainTimer: ReturnType | undefined const cleanup = () => { clearTimeout(timeoutTimer) + clearTimeout(drainTimer) ctx.abort.removeEventListener("abort", abortHandler) } - // altimate_change start — wait for streams, but keep the timeout armed until they close - proc.once("close", () => { - // altimate_change end - exited = true + const finish = () => { cleanup() + proc.removeListener("close", finish) + proc.stdout?.destroy() + proc.stderr?.destroy() resolve() + } + + proc.once("exit", () => { + exited = true + cleanup() + // Exit ends the command timeout. Give buffered output a bounded drain; + // a background child retaining a pipe is not a failed command. + drainTimer = setTimeout(finish, 250) }) + proc.once("close", finish) + // altimate_change end proc.once("error", (error) => { exited = true cleanup() + // altimate_change start — stop reading when spawning fails too + proc.removeListener("close", finish) + proc.stdout?.destroy() + proc.stderr?.destroy() + // altimate_change end reject(error) }) // altimate_change start — install listeners before killing an already-aborted command @@ -367,18 +389,13 @@ export const BashTool = Tool.define("bash", async () => { }) const resultMetadata: string[] = [] - // altimate_change start — use exit status and complete trailing traceback shapes, never error keywords + // altimate_change start — shell outcomes are telemetry, not SDK errors const traceback = /(?:^|\r?\n)Traceback \(most recent call last\):\r?\n(?:[ \t].*\r?\n)+([A-Za-z_][\w.]*(?::[^\r\n]*)?)\s*$/ - const pythonError = traceback.exec(stderrTail)?.[1] ?? traceback.exec(output)?.[1] - const exitError = proc.exitCode === 0 - ? undefined - : `Command exited with ${proc.exitCode === null ? `signal ${proc.signalCode}` : `code ${proc.exitCode}`}` - const error = timedOut - ? `Command timed out after ${timeout} ms` - : aborted - ? "User aborted the command" - : pythonError ?? exitError + // A zero-exit traceback is only a failure when it ends stderr and stdout + // is empty. Tracebacks printed on stdout are ordinary command output. + const pythonError = !hasStdout && traceback.test(stderrTail) + const failed = timedOut || aborted || proc.exitCode !== 0 || pythonError // altimate_change end if (timedOut) { @@ -399,8 +416,9 @@ export const BashTool = Tool.define("bash", async () => { output: output.length > MAX_METADATA_LENGTH ? output.slice(0, MAX_METADATA_LENGTH) + "\n\n..." : output, exit: proc.exitCode, description: params.description, - // altimate_change start — propagate shell failures through the shared tool envelope - ...(error && { success: false, error }), + // altimate_change start — retain the legacy exit field and record telemetry without metadata.error + exit_code: proc.exitCode, + ...(failed && { success: false }), // altimate_change end }, output, diff --git a/packages/opencode/src/tool/batch.ts b/packages/opencode/src/tool/batch.ts index 6b3faa9506..b6b8212221 100644 --- a/packages/opencode/src/tool/batch.ts +++ b/packages/opencode/src/tool/batch.ts @@ -109,6 +109,9 @@ export const BatchTool = Tool.define("batch", { // altimate_change start — v1.17.9: Tool.Def.execute returns an Effect; run via AppRuntime const result = await AppRuntime.runPromise(tool.execute(validatedParams, toEffectContext(ctx, partID))) // altimate_change end + // altimate_change start — inner error envelopes follow direct-call failure semantics + Tool.assertSuccess(result) + // altimate_change end // altimate_change start — append file lessons before persisting each inner result const lessons = ctx.extra?.lessons as LessonDelivery | undefined await lessons?.appendFileLessons(ctx.sessionID, call.tool, call.parameters, result) diff --git a/packages/opencode/src/tool/read.txt b/packages/opencode/src/tool/read.txt index 77c8b7ec7b..a15566355d 100644 --- a/packages/opencode/src/tool/read.txt +++ b/packages/opencode/src/tool/read.txt @@ -8,10 +8,8 @@ Usage: - Use the grep tool to find specific content in large files or files with long lines. - If you are unsure of the correct file path, use the glob tool to look up filenames by glob pattern. - Contents are returned with each line prefixed by its line number as `: `. For example, if a file has contents "foo\n", you will receive "1: foo\n". For directories, entries are returned one per line (without line numbers) with a trailing `/` for subdirectories. -// altimate_change start — document retrieval of complete long lines - By default, any line longer than 2000 characters is truncated. - For long lines, use offset with limit=1, then char_offset (0-indexed) and max_chars to read successive character windows. The 50 KB output cap still applies; each clipped line gives the next window's parameters. -// altimate_change end - Call this tool in parallel when you know there are multiple files you want to read. - Avoid tiny repeated slices (30 line chunks). If you need more context, read a larger window. - This tool can read image files and PDFs and return them as file attachments. diff --git a/packages/opencode/src/tool/tool.ts b/packages/opencode/src/tool/tool.ts index bafd037f5b..db30749539 100644 --- a/packages/opencode/src/tool/tool.ts +++ b/packages/opencode/src/tool/tool.ts @@ -79,9 +79,11 @@ export interface ExecuteResult { // altimate_change start — distinguish execution failures from successful diagnostic findings function isErrorResult(result: ExecuteResult) { + // success:false is telemetry only (for example, grep's ordinary no-match exit). + // SDK errors require an explicit error envelope or backend error metadata; + // successful diagnostics may include error details by declaring success:true. return ( result.status === "error" || - result.metadata.success === false || (result.metadata.success !== true && Boolean(result.metadata.error)) ) } @@ -329,8 +331,9 @@ function wrap, Result extends Metadat ), ) const failed = isErrorResult(rawResult) + const unsuccessful = failed || rawResult.metadata.success === false // Ended before anything else can throw, so a call that finished is never left "running". - endTrace(failed ? "error" : "success") + endTrace(unsuccessful ? "error" : "success") // humanize the tool-call title at the source so any client (chat webview, // TUI, ...) can render a readable label from state.title. const result = { @@ -345,7 +348,7 @@ function wrap, Result extends Metadat } const agent = yield* agents.get(ctx.agent) // altimate_change start — truncation cannot re-label a failed result as successful - const truncated = yield* truncate.output(result.output, { outcome: failed ? "error" : "success" }, agent) + const truncated = yield* truncate.output(result.output, { outcome: unsuccessful ? "error" : "success" }, agent) // altimate_change end return { ...result, diff --git a/packages/opencode/test/altimate/tool-result-errors.test.ts b/packages/opencode/test/altimate/tool-result-errors.test.ts index 91fca75772..fc54df63af 100644 --- a/packages/opencode/test/altimate/tool-result-errors.test.ts +++ b/packages/opencode/test/altimate/tool-result-errors.test.ts @@ -25,13 +25,13 @@ afterEach(() => { for (const spy of spies.splice(0)) spy.mockRestore() }) -async function execute(id: string, output: string, metadata: Record) { +async function execute(id: string, output: string, metadata: Record, status?: "ok" | "error") { const tool = await initTool( Tool.define(id, { description: "Tool result fixture", parameters: z.object({}), async execute() { - return { title: id, output, metadata } + return { title: id, output, metadata, status } }, }), ) @@ -58,8 +58,23 @@ describe("tool result failures", () => { expect(() => Tool.assertSuccess(result)).toThrow(output) }) - test("explicit soft failure becomes an SDK error even without error metadata", async () => { - const result = await execute("analysis", "Could not run the requested operation", { success: false }) + test("unsuccessful telemetry alone does not turn an ordinary result into an SDK error", async () => { + const events: Telemetry.Event[] = [] + spies.push( + spyOn(Telemetry, "track").mockImplementation((event) => { + events.push(event) + }), + ) + const result = await execute("bash", "No matching lines", { success: false, exit_code: 1, truncated: false }) + expect(result.status).not.toBe("error") + expect(result.metadata).toMatchObject({ success: false, exit_code: 1 }) + expect(() => Tool.assertSuccess(result)).not.toThrow() + expect(events.find((event) => event.type === "tool_call")).toMatchObject({ status: "error" }) + expect(events.some((event) => event.type === "core_failure")).toBe(true) + }) + + test("explicit error envelopes become SDK errors without error metadata", async () => { + const result = await execute("analysis", "Could not run the requested operation", {}, "error") expect(result.status).toBe("error") expect(() => Tool.assertSuccess(result)).toThrow(result.output) }) diff --git a/packages/opencode/test/altimate/tools/warehouse-test.test.ts b/packages/opencode/test/altimate/tools/warehouse-test.test.ts new file mode 100644 index 0000000000..3a407fef05 --- /dev/null +++ b/packages/opencode/test/altimate/tools/warehouse-test.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, spyOn, test } from "bun:test" +import { Dispatcher } from "../../../src/altimate/native" +import { WarehouseTestTool } from "../../../src/altimate/tools/warehouse-test" +import { Tool } from "../../../src/tool/tool" +import { initTool } from "../tool-fixture" + +describe("warehouse_test diagnostic results", () => { + test.each([ + { + title: "STORE LOCKED", + error_category: "store_locked", + recoverable: true, + guidance: "Close that connection and try again", + }, + { + title: "INFRASTRUCTURE FAILURE", + error_category: "driver_open_timeout", + recoverable: false, + guidance: "Stop and report this", + }, + ])("returns $title guidance as a successful diagnostic", async ({ title, error_category, recoverable, guidance }) => { + const dispatcher = spyOn(Dispatcher, "call").mockResolvedValue({ + connected: false, + error: "Store could not be opened", + error_category, + infrastructure: true, + recoverable, + } as any) + try { + const tool = await initTool(WarehouseTestTool) + const result = await tool.execute({ name: "local" }, { + sessionID: "ses_warehouse_diagnostics", + messageID: "msg_warehouse_diagnostics", + agent: "build", + abort: AbortSignal.any([]), + messages: [], + }) + expect(result.title).toContain(title) + expect(result.output).toContain(guidance) + expect(result.metadata).toMatchObject({ + success: true, + connected: false, + error: "Store could not be opened", + error_category, + infrastructure: true, + }) + expect(result.status).not.toBe("error") + expect(() => Tool.assertSuccess(result)).not.toThrow() + } finally { + dispatcher.mockRestore() + } + }) + + test.each(["failed connectivity", "dispatcher exception"])("preserves %s as an SDK error", async (kind) => { + const dispatcher = spyOn(Dispatcher, "call").mockImplementation(async () => { + if (kind === "dispatcher exception") throw new Error("Connection rejected") + return { connected: false, error: "Connection rejected", error_category: "config_error" } as any + }) + try { + const tool = await initTool(WarehouseTestTool) + const result = await tool.execute({ name: "local" }, { + sessionID: "ses_warehouse_diagnostics", + messageID: "msg_warehouse_diagnostics", + agent: "build", + abort: AbortSignal.any([]), + messages: [], + }) + expect(result.metadata.success).toBe(false) + expect(result.status).toBe("error") + expect(() => Tool.assertSuccess(result)).toThrow("Connection rejected") + } finally { + dispatcher.mockRestore() + } + }) +}) diff --git a/packages/opencode/test/session/tools-result-errors.test.ts b/packages/opencode/test/session/tools-result-errors.test.ts index c3d4af717b..6578d29369 100644 --- a/packages/opencode/test/session/tools-result-errors.test.ts +++ b/packages/opencode/test/session/tools-result-errors.test.ts @@ -69,62 +69,82 @@ const it = testEffect( ), ) -it.live("session/tools rejects backend error envelopes at the SDK boundary", () => - Effect.gen(function* () { - const sessionID = SessionID.make("ses_tools_errors") - const messageID = MessageID.make("msg_tools_errors") - const tools = yield* SessionTools.resolve({ - agent, - model, - bypassAgentCheck: false, - messages: [], - session: { - id: sessionID, - slug: "tools-errors", - projectID: ProjectV2.ID.make("project"), - directory: "/tmp", - title: "Tool errors", - version: "1", - time: { created: 0, updated: 0 }, - }, - processor: { - message: { - id: messageID, - sessionID, - role: "assistant", - time: { created: 0 }, - parentID: MessageID.make("msg_user"), - modelID: model.id, - providerID: model.providerID, - mode: "build", - agent: "build", - path: { cwd: "/tmp", root: "/tmp" }, - cost: 0, - tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, - }, - updateToolCall: () => Effect.succeed(undefined), - completeToolCall: () => Effect.void, - }, - promptOps: { - resolvePromptParts: () => Effect.succeed([]), - prompt: () => Effect.die("unexpected prompt"), - cancel: () => Effect.void, - }, - }) - const result = yield* Effect.promise(() => - Promise.resolve( - tools.backend_failure.execute!( - {}, - { - toolCallId: "call_error", - messages: [], - abortSignal: AbortSignal.any([]), +for (const aborted of [false, true]) { + it.live( + aborted + ? "session/tools completes aborted error envelopes before rejecting at the SDK boundary" + : "session/tools rejects backend error envelopes at the SDK boundary", + () => + Effect.gen(function* () { + const sessionID = SessionID.make("ses_tools_errors") + const messageID = MessageID.make("msg_tools_errors") + const completed: Array<{ callID: string; result: unknown }> = [] + const tools = yield* SessionTools.resolve({ + agent, + model, + bypassAgentCheck: false, + messages: [], + session: { + id: sessionID, + slug: "tools-errors", + projectID: ProjectV2.ID.make("project"), + directory: "/tmp", + title: "Tool errors", + version: "1", + time: { created: 0, updated: 0 }, }, - ), - ), - ).pipe(Effect.exit) - expect(Exit.isFailure(result)).toBe(true) - if (Exit.isFailure(result)) expect(Cause.pretty(result.cause)).toContain(failure) - }), -) + processor: { + message: { + id: messageID, + sessionID, + role: "assistant", + time: { created: 0 }, + parentID: MessageID.make("msg_user"), + modelID: model.id, + providerID: model.providerID, + mode: "build", + agent: "build", + path: { cwd: "/tmp", root: "/tmp" }, + cost: 0, + tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, + }, + updateToolCall: () => Effect.succeed(undefined), + completeToolCall: (callID, result) => + Effect.sync(() => { + completed.push({ callID, result }) + }), + }, + promptOps: { + resolvePromptParts: () => Effect.succeed([]), + prompt: () => Effect.die("unexpected prompt"), + cancel: () => Effect.void, + }, + }) + const result = yield* Effect.promise(() => + Promise.resolve( + tools.backend_failure.execute!( + {}, + { + toolCallId: "call_error", + messages: [], + abortSignal: aborted ? AbortSignal.abort() : AbortSignal.any([]), + }, + ), + ), + ).pipe(Effect.exit) + expect(Exit.isFailure(result)).toBe(true) + if (Exit.isFailure(result)) expect(Cause.pretty(result.cause)).toContain(failure) + expect(completed).toEqual( + aborted + ? [ + { + callID: "call_error", + result: expect.objectContaining({ output: failure, status: "error" }), + }, + ] + : [], + ) + }), + ) +} // altimate_change end diff --git a/packages/opencode/test/tool/bash.test.ts b/packages/opencode/test/tool/bash.test.ts index 7060db513b..fb286bed39 100644 --- a/packages/opencode/test/tool/bash.test.ts +++ b/packages/opencode/test/tool/bash.test.ts @@ -14,6 +14,7 @@ import { Effect, Layer, ManagedRuntime } from "effect" import { Agent } from "../../src/agent/agent" import { Truncate as TruncateService } from "../../src/tool/truncate" import { InstanceRef } from "../../src/effect/instance-ref" +import { Tool } from "../../src/tool/tool" // The shared tool fixture stubs Truncate with a no-op so most tool tests don't // truncate. Truncation tests need the REAL Truncate service, so build the tool @@ -501,8 +502,8 @@ describe("tool.bash PATH injection", () => { }) // altimate_change end -// altimate_change start — failed shell commands and Python tracebacks are failed tool results -describe("tool.bash error envelopes", () => { +// altimate_change start — shell failures are telemetry, not SDK tool errors +describe("tool.bash failure telemetry", () => { const traceback = `Traceback (most recent call last): File "/tmp/ema.py", line 13, in else: g,f,cls=r['grant_date'],r['filing_date'],r['cls'] @@ -514,29 +515,48 @@ KeyError: 'grant_date' ModuleNotFoundError: No module named 'sklearn' ` - test("marks a nonzero exit as an error and preserves output", async () => { + test("records a nonzero exit without an SDK error and preserves output", async () => { await provideInstance(projectRoot, async () => { const bash = await initTool(BashTool) const result = await bash.execute({ command: "echo 'failed command'; exit 7", description: "Fail a command" }, ctx) expect(result.metadata.exit).toBe(7) + expect(result.metadata.exit_code).toBe(7) expect(result.metadata.success).toBe(false) - expect(result.status).toBe("error") + expect(result.status).not.toBe("error") + expect(result.metadata.error).toBeUndefined() + expect(() => Tool.assertSuccess(result)).not.toThrow() expect(result.output).toBe("failed command\n") }) }) - test("marks a trailing Python traceback as an error despite a zero shell exit", async () => { + test.each([ + ["grep no match", "printf 'haystack\\n' | grep needle"], + ["diff differences", "diff <(printf 'left\\n') <(printf 'right\\n')"], + ])("records %s exit one without an SDK error", async (_, command) => { + await provideInstance(projectRoot, async () => { + const bash = await initTool(BashTool) + const result = await bash.execute({ command, description: "Compare ordinary command results" }, ctx) + expect(result.metadata.exit_code).toBe(1) + expect(result.metadata.success).toBe(false) + expect(result.status).not.toBe("error") + expect(result.metadata.error).toBeUndefined() + expect(() => Tool.assertSuccess(result)).not.toThrow() + }) + }) + + test("treats a trailing Python traceback on stdout with exit zero as data", async () => { await provideInstance(projectRoot, async () => { const bash = await initTool(BashTool) const result = await bash.execute({ command: `cat <<'TRACEBACK'\n${traceback}TRACEBACK`, description: "Show traceback" }, ctx) expect(result.metadata.exit).toBe(0) - expect(result.status).toBe("error") - expect(result.metadata.error).toContain("KeyError: 'grant_date'") + expect(result.metadata.success).not.toBe(false) + expect(result.status).not.toBe("error") + expect(result.metadata.error).toBeUndefined() expect(result.output).toBe(traceback) }) }) - test("recognizes a trailing stderr traceback followed by successful stdout", async () => { + test("keeps a stderr traceback followed by successful stdout successful at exit zero", async () => { await provideInstance(projectRoot, async () => { const bash = await initTool(BashTool) const result = await bash.execute({ @@ -544,12 +564,56 @@ ModuleNotFoundError: No module named 'sklearn' description: "Show missing module and subsequent output", }, ctx) expect(result.metadata.exit).toBe(0) - expect(result.status).toBe("error") - expect(result.metadata.error).toContain("ModuleNotFoundError: No module named 'sklearn'") + expect(result.metadata.success).not.toBe(false) + expect(result.status).not.toBe("error") + expect(result.metadata.error).toBeUndefined() expect(result.output).toContain("next command succeeded") }) }) + test("records a final stderr traceback with empty stdout as failure telemetry at exit zero", async () => { + await provideInstance(projectRoot, async () => { + const bash = await initTool(BashTool) + const result = await bash.execute({ + command: `cat >&2 <<'TRACEBACK'\n${missingModule}TRACEBACK`, + description: "Show a final stderr traceback", + }, ctx) + expect(result.metadata.exit_code).toBe(0) + expect(result.metadata.success).toBe(false) + expect(result.status).not.toBe("error") + expect(result.metadata.error).toBeUndefined() + expect(() => Tool.assertSuccess(result)).not.toThrow() + }) + }) + + test("records a stderr traceback and nonzero exit despite stdout output", async () => { + await provideInstance(projectRoot, async () => { + const bash = await initTool(BashTool) + const result = await bash.execute({ + command: `echo 'command started'; cat >&2 <<'TRACEBACK'\n${traceback}TRACEBACK\nexit 1`, + description: "Fail after stdout and stderr output", + }, ctx) + expect(result.metadata.exit_code).toBe(1) + expect(result.metadata.success).toBe(false) + expect(result.status).not.toBe("error") + expect(result.metadata.error).toBeUndefined() + expect(() => Tool.assertSuccess(result)).not.toThrow() + }) + }) + + test("keeps a recovered stderr traceback successful at exit zero", async () => { + await provideInstance(projectRoot, async () => { + const bash = await initTool(BashTool) + const result = await bash.execute({ + command: `cat >&2 <<'TRACEBACK'\n${traceback}Recovered successfully\nTRACEBACK`, + description: "Show a recovered stderr traceback", + }, ctx) + expect(result.metadata.exit).toBe(0) + expect(result.metadata.success).not.toBe(false) + expect(result.status).not.toBe("error") + }) + }) + test("successful output mentioning errors or a recovered stdout traceback stays successful", async () => { await provideInstance(projectRoot, async () => { const bash = await initTool(BashTool) @@ -564,14 +628,17 @@ ModuleNotFoundError: No module named 'sklearn' }) // altimate_change end -// altimate_change start — the truncation envelope must not relabel failed commands as successful -test("tool.bash truncation retains the failure outcome and complete output spill", async () => { +// altimate_change start — truncation preserves shell telemetry and complete output +test("tool.bash truncation retains failure telemetry without an SDK error and complete output spill", async () => { await provideInstance(projectRoot, async () => { const bash = await initToolWithTruncate(BashTool) const result = await bash.execute({ command: `seq 1 ${Truncate.MAX_LINES + 10}; exit 1`, description: "Fail after generating long output", }, ctx) - expect(result.status).toBe("error") + expect(result.metadata.exit_code).toBe(1) + expect(result.metadata.success).toBe(false) + expect(result.status).not.toBe("error") + expect(() => Tool.assertSuccess(result)).not.toThrow() expect(result.metadata.truncated).toBe(true) expect(result.output).toContain("The tool call failed but the output was truncated") expect(result.output).not.toContain("The tool call succeeded") @@ -581,27 +648,62 @@ test("tool.bash truncation retains the failure outcome and complete output spill // altimate_change end // altimate_change start — draining pipes must remain bounded after the shell exits -test("tool.bash timeout still bounds output held open by a background child", async () => { - await provideInstance(projectRoot, async () => { +test("tool.bash returns promptly after shell exit while a background child holds output open", async () => { + await using tmp = await tmpdir() + await provideInstance(tmp.path, async () => { const bash = await initTool(BashTool) const started = Date.now() - const result = await bash.execute({ - command: "sleep 2 & exit 0", description: "Leave a child holding the output pipe", timeout: 20, - }, ctx) - expect(result.status).toBe("error") - expect(result.metadata.error).toContain("timed out") - expect(Date.now() - started).toBeLessThan(1500) + try { + const result = await bash.execute({ + command: "sh -c 'sleep 30 & echo done; echo $! > background.pid'", + description: "Leave a child holding the output pipe", + timeout: 1000, + }, ctx) + expect(result.output).toBe("done\n") + expect(result.metadata.exit).toBe(0) + expect(result.metadata.exit_code).toBe(0) + expect(result.metadata.success).not.toBe(false) + expect(result.metadata.error).toBeUndefined() + expect(result.status).not.toBe("error") + expect(Date.now() - started).toBeLessThan(1500) + } finally { + const pid = Number(await fs.readFile(path.join(tmp.path, "background.pid"), "utf8").catch(() => "")) + if (pid) { + try { + process.kill(pid, "SIGTERM") + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ESRCH") throw error + } + } + } }) }) -test("tool.bash handles an already-aborted command without losing the close event", async () => { +test("tool.bash still terminates a foreground command after its timeout", async () => { + await provideInstance(projectRoot, async () => { + const bash = await initTool(BashTool) + const result = await bash.execute({ command: "sleep 30", description: "Time out a command", timeout: 20 }, ctx) + expect(result.metadata.success).toBe(false) + expect(result.metadata.exit_code).toBeNull() + expect(result.metadata.error).toBeUndefined() + expect(result.status).not.toBe("error") + expect(result.output).toContain("exceeding timeout 20 ms") + expect(() => Tool.assertSuccess(result)).not.toThrow() + }) +}, 3000) + +test("tool.bash handles an already-aborted command without losing the exit event", async () => { await provideInstance(projectRoot, async () => { const bash = await initTool(BashTool) const result = await bash.execute({ command: "sleep 2", description: "Abort a command", timeout: 20 }, { ...ctx, abort: AbortSignal.abort(), }) - expect(result.status).toBe("error") - expect(result.metadata.error).toContain("aborted") + expect(result.metadata.success).toBe(false) + expect(result.metadata.exit_code).toBeNull() + expect(result.metadata.error).toBeUndefined() + expect(result.status).not.toBe("error") + expect(result.output).toContain("User aborted the command") + expect(() => Tool.assertSuccess(result)).not.toThrow() }) }, 3000) // altimate_change end diff --git a/packages/opencode/test/tool/batch.test.ts b/packages/opencode/test/tool/batch.test.ts index 556a9e9c33..74730e6e7b 100644 --- a/packages/opencode/test/tool/batch.test.ts +++ b/packages/opencode/test/tool/batch.test.ts @@ -21,6 +21,46 @@ function safeParse>(schema: S, input: unknown) : { success: false as const, error: result.failure } } +describe("BatchTool: inner result errors", () => { + test.each([ + { name: "explicit error envelope", status: "error" as const, metadata: {} }, + { name: "backend error metadata", metadata: { error: "Query failed" } }, + ])("does not count an inner $name as successful", async ({ status, metadata }) => { + const tool = await getToolInfo() + const registry = spyOn(ToolRegistry, "tools").mockResolvedValue([{ + id: "sql_execute", + registrySource: undefined, + description: "test query tool", + parameters: Schema.Unknown, + execute: () => Effect.succeed({ title: "Query", output: "Query failed", status, metadata }), + }]) + const updatePart = Object.assign(async (part: Parameters[0]) => part, Session.updatePart) + const updates = spyOn(Session, "updatePart").mockImplementation(updatePart) + try { + const result = await tool.execute({ tool_calls: [{ tool: "sql_execute", parameters: {} }] }, { + sessionID: SessionID.make("ses_batch_errors"), + messageID: MessageID.make("msg_batch_errors"), + agent: "build", + abort: AbortSignal.any([]), + messages: [], + }) + expect(result.metadata.successful).toBe(0) + expect(result.metadata.failed).toBe(1) + expect(result.metadata.details).toEqual([{ tool: "sql_execute", success: false }]) + expect(result.output).toBe("Executed 0/1 tools successfully. 1 failed.") + const parts = updates.mock.calls.map(([part]) => part) + expect(parts.some((part) => part.type === "tool" && part.state.status === "completed")).toBe(false) + expect(parts.at(-1)).toMatchObject({ + type: "tool", + state: { status: "error", error: "Query failed" }, + }) + } finally { + updates.mockRestore() + registry.mockRestore() + } + }) +}) + describe("BatchTool: file lessons", () => { test.each(["read", "edit", "write", "apply_patch"])("appends approved lessons to the inner %s result", async (toolName) => { await using tmp = await tmpdir() diff --git a/packages/opencode/test/tool/read.test.ts b/packages/opencode/test/tool/read.test.ts index 5c1fa3f191..d92338d600 100644 --- a/packages/opencode/test/tool/read.test.ts +++ b/packages/opencode/test/tool/read.test.ts @@ -148,6 +148,18 @@ const asks = () => { } } +describe("tool.read description", () => { + it.live("documents character windows without leaking source markers", () => + Effect.gen(function* () { + const dir = yield* tmpdirScoped() + const tool = yield* provideInstance(dir)(init()) + expect(tool.description).toContain("char_offset") + expect(tool.description).toContain("max_chars") + expect(tool.description).not.toContain("altimate_change") + }), + ) +}) + describe("tool.read stale-file guard", () => { it.live("records read time for successfully read files", () => Effect.gen(function* () { From 2ae497f2d72f39b3975fa822ecc7b7910955901a Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Mon, 12 Oct 2026 13:48:16 -0700 Subject: [PATCH 3/9] fix(drivers,tool): Mongo count keeps its filter, shorthand conflicts refuse, bash failure signals narrowed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Independent-review follow-ups: - The Mongo `count` → `countDocuments` alias maps `query` to `filter`, so a filtered count no longer counts every document. - Collection shorthand with a conflicting explicit argument (`find` + `filter`, `count` + `query`, `distinct` + `field`) is refused as ambiguous instead of silently dropping one side. - The production tool boundary (`SessionPrompt.resolveTools`) is covered by a test that fails if the SDK-boundary check is removed. - `bash` sets `metadata.failure_kind` only for timeout, abort, signal or a qualifying stderr traceback; ordinary non-zero exits keep `exit_code` and `success:false` for telemetry but no longer produce the failed truncation hint, an error trace status or a `core_failure` event. - `batch` appends file lessons before the SDK-boundary check, matching the direct-call path. Co-Authored-By: Claude Fable 5.1 --- packages/drivers/src/mongodb.ts | 14 ++- packages/drivers/test/mongodb-unit.test.ts | 40 +++++- packages/opencode/src/tool/bash.ts | 16 ++- packages/opencode/src/tool/batch.ts | 6 +- packages/opencode/src/tool/tool.ts | 14 ++- .../test/altimate/tool-result-errors.test.ts | 6 +- .../test/session/tools-result-errors.test.ts | 115 +++++++++++++++++- packages/opencode/test/tool/bash.test.ts | 71 +++++++++-- packages/opencode/test/tool/batch.test.ts | 51 ++++++++ 9 files changed, 302 insertions(+), 31 deletions(-) diff --git a/packages/drivers/src/mongodb.ts b/packages/drivers/src/mongodb.ts index 63482583f8..94f28969b2 100644 --- a/packages/drivers/src/mongodb.ts +++ b/packages/drivers/src/mongodb.ts @@ -18,6 +18,9 @@ import type { ConnectionConfig, Connector, ConnectorResult, ExecuteOptions, Sche import { QueryTimeoutError } from "./types" import { resolveQueryTimeout } from "./query-timeout" import { loadOptionalDriver } from "./resolve" +// altimate_change start — detect conflicting shorthand arguments +import { isDeepStrictEqual } from "node:util" +// altimate_change end // altimate_change start — cap extraction before cursor results accumulate import { createResultByteBudget } from "./result-byte-limit" // altimate_change end @@ -57,7 +60,10 @@ const NATIVE_COMMAND_KEYS: Record = { export function normalizeMqlCommand>(parsed: T): T & MqlQuery { if (!parsed || typeof parsed !== "object") return parsed as T & MqlQuery if (typeof parsed["command"] === "string") { - return (parsed["command"] === "count" ? { ...parsed, command: "countDocuments" } : parsed) as T & MqlQuery + if (parsed["command"] !== "count") return parsed as T & MqlQuery + const normalized: Record = { ...parsed, command: "countDocuments" } + if (normalized["filter"] === undefined) normalized["filter"] = normalized["query"] + return normalized as T & MqlQuery } if (typeof parsed["collection"] === "string" && parsed["collection"]) { const keys = Object.keys(NATIVE_COMMAND_KEYS).filter((key) => Object.hasOwn(parsed, key)) @@ -76,6 +82,12 @@ export function normalizeMqlCommand>(parsed: T ? "field" : undefined if (argument) { + const explicitArguments = command === "countDocuments" ? [argument, "query"] : [argument] + for (const explicit of explicitArguments) { + if (parsed[explicit] !== undefined && !isDeepStrictEqual(parsed[explicit], value)) { + throw new Error(`Ambiguous MQL command: '${key}' conflicts with explicit '${explicit}' argument`) + } + } const { [key]: _, ...rest } = parsed const normalized: Record = { ...rest, command } if (normalized[argument] === undefined) normalized[argument] = value diff --git a/packages/drivers/test/mongodb-unit.test.ts b/packages/drivers/test/mongodb-unit.test.ts index a8acf37b17..7c642d2b8c 100644 --- a/packages/drivers/test/mongodb-unit.test.ts +++ b/packages/drivers/test/mongodb-unit.test.ts @@ -1,7 +1,7 @@ import { describe, expect, mock, test } from "bun:test" import { normalizeMqlCommand } from "../src/mongodb" -async function fakeMongo(docs: Record[] = []) { +async function fakeMongo(docs?: Record[]) { const pipelines: Record[][] = [] const counts: { filter: unknown; options: unknown }[] = [] const distincts: { field: string; filter: unknown; options: unknown }[] = [] @@ -15,11 +15,11 @@ async function fakeMongo(docs: Record[] = []) { collection: () => ({ aggregate: (pipeline: Record[]) => { pipelines.push(pipeline) - return { toArray: async () => docs } + return { toArray: async () => docs ?? [] } }, - countDocuments: async (filter: unknown, options: unknown) => { + countDocuments: async (filter: Record, options: unknown) => { counts.push({ filter, options }) - return 11 + return docs?.filter((doc) => Object.entries(filter).every(([key, value]) => doc[key] === value)).length ?? 11 }, distinct: async (field: string, filter: unknown, options: unknown) => { distincts.push({ field, filter, options }) @@ -116,6 +116,18 @@ describe("MongoDB aggregate write protection", () => { }) describe("MongoDB native command filters", () => { + test("count command alias counts only documents matching the query filter", async () => { + const { connector, counts } = await fakeMongo([{ status: "A" }, { status: "B" }, { status: "A" }]) + const result = await connector.execute( + JSON.stringify({ command: "count", collection: "orders", query: { status: "A" } }), + 1, + undefined, + { queryTimeoutMs: 0 }, + ) + expect(result.rows).toEqual([[2]]) + expect(counts).toEqual([{ filter: { status: "A" }, options: {} }]) + }) + test("count forwards the native query filter", async () => { const { connector, counts } = await fakeMongo() await connector.execute(JSON.stringify({ count: "orders", query: { status: "A" } }), 1, undefined, { queryTimeoutMs: 0 }) @@ -142,6 +154,26 @@ describe("MongoDB native command filters", () => { }) describe("MongoDB collection command shorthand", () => { + test.each([ + { name: "find and filter", input: { collection: "items", find: { status: "x" }, filter: { status: "y" } } }, + { name: "count and query", input: { collection: "items", count: { status: "x" }, query: { status: "y" } } }, + { name: "distinct and field", input: { collection: "items", distinct: "status", field: "category" } }, + ])("refuses conflicting $name arguments", ({ input }) => { + expect(() => normalizeMqlCommand(input)).toThrow(/ambiguous.*conflict/i) + }) + + test("accepts equivalent embedded and explicit filters regardless of key order", () => { + expect(normalizeMqlCommand({ + collection: "items", + find: { status: "x", active: true }, + filter: { active: true, status: "x" }, + })).toEqual({ + collection: "items", + command: "find", + filter: { active: true, status: "x" }, + }) + }) + test("normalizes a collection with an embedded find filter and projection", () => { const command = { collection: "articles", diff --git a/packages/opencode/src/tool/bash.ts b/packages/opencode/src/tool/bash.ts index 96f501d23a..01b664d90f 100644 --- a/packages/opencode/src/tool/bash.ts +++ b/packages/opencode/src/tool/bash.ts @@ -394,8 +394,17 @@ export const BashTool = Tool.define("bash", async () => { /(?:^|\r?\n)Traceback \(most recent call last\):\r?\n(?:[ \t].*\r?\n)+([A-Za-z_][\w.]*(?::[^\r\n]*)?)\s*$/ // A zero-exit traceback is only a failure when it ends stderr and stdout // is empty. Tracebacks printed on stdout are ordinary command output. - const pythonError = !hasStdout && traceback.test(stderrTail) - const failed = timedOut || aborted || proc.exitCode !== 0 || pythonError + const pythonError = (proc.exitCode !== 0 || !hasStdout) && traceback.test(stderrTail) + const failureKind = timedOut + ? "timeout" + : aborted + ? "abort" + : proc.signalCode + ? "signal" + : pythonError + ? "stderr_traceback" + : undefined + const failed = failureKind !== undefined || proc.exitCode !== 0 // altimate_change end if (timedOut) { @@ -419,6 +428,9 @@ export const BashTool = Tool.define("bash", async () => { // altimate_change start — retain the legacy exit field and record telemetry without metadata.error exit_code: proc.exitCode, ...(failed && { success: false }), + // Ordinary non-zero exits (for example, grep no-match) retain their + // exit status without declaring an execution failure to the wrapper. + ...(failureKind && { failure_kind: failureKind }), // altimate_change end }, output, diff --git a/packages/opencode/src/tool/batch.ts b/packages/opencode/src/tool/batch.ts index b6b8212221..2ee8e2cdbd 100644 --- a/packages/opencode/src/tool/batch.ts +++ b/packages/opencode/src/tool/batch.ts @@ -109,13 +109,13 @@ export const BatchTool = Tool.define("batch", { // altimate_change start — v1.17.9: Tool.Def.execute returns an Effect; run via AppRuntime const result = await AppRuntime.runPromise(tool.execute(validatedParams, toEffectContext(ctx, partID))) // altimate_change end - // altimate_change start — inner error envelopes follow direct-call failure semantics - Tool.assertSuccess(result) - // altimate_change end // altimate_change start — append file lessons before persisting each inner result const lessons = ctx.extra?.lessons as LessonDelivery | undefined await lessons?.appendFileLessons(ctx.sessionID, call.tool, call.parameters, result) // altimate_change end + // altimate_change start — match direct-call ordering: append lessons before checking the result + Tool.assertSuccess(result) + // altimate_change end const attachments = result.attachments?.map((attachment) => ({ ...attachment, id: PartID.ascending(), diff --git a/packages/opencode/src/tool/tool.ts b/packages/opencode/src/tool/tool.ts index db30749539..ad278b0bb9 100644 --- a/packages/opencode/src/tool/tool.ts +++ b/packages/opencode/src/tool/tool.ts @@ -79,7 +79,7 @@ export interface ExecuteResult { // altimate_change start — distinguish execution failures from successful diagnostic findings function isErrorResult(result: ExecuteResult) { - // success:false is telemetry only (for example, grep's ordinary no-match exit). + // success:false alone is not an SDK error (for example, grep's no-match exit). // SDK errors require an explicit error envelope or backend error metadata; // successful diagnostics may include error details by declaring success:true. return ( @@ -200,7 +200,7 @@ function altimateTrackSuccess( result: ExecuteResult, ) { try { - const isSoftFailure = result.metadata?.success === false + const isSoftFailure = hasFailureSignals(id, result) const durationMs = Date.now() - startTime const toolCategory = Telemetry.categorizeToolName(id, "standard") // Skip success tool_call for high-volume/low-signal file tools; failures still tracked. @@ -239,7 +239,7 @@ function altimateTrackSuccess( errorMsg = String(rawError) } } else { - errorMsg = `soft_failure:${id}:no_error_metadata` + errorMsg = `soft_failure:${id}:${result.metadata.failure_kind ?? "no_error_metadata"}` } const maskedErrorMsg = Telemetry.maskString(errorMsg).slice(0, 500) Telemetry.track({ @@ -276,6 +276,12 @@ function altimateTrackSuccess( // Telemetry must never break tool execution } } + +function hasFailureSignals(id: string, result: ExecuteResult) { + // Bash non-zero exit codes can describe ordinary command outcomes. Only its + // explicit execution failure kinds should affect traces, hints, and telemetry. + return isErrorResult(result) || (id === "bash" ? Boolean(result.metadata.failure_kind) : result.metadata.success === false) +} // altimate_change end function wrap, Result extends Metadata>( @@ -331,7 +337,7 @@ function wrap, Result extends Metadat ), ) const failed = isErrorResult(rawResult) - const unsuccessful = failed || rawResult.metadata.success === false + const unsuccessful = hasFailureSignals(id, rawResult) // Ended before anything else can throw, so a call that finished is never left "running". endTrace(unsuccessful ? "error" : "success") // humanize the tool-call title at the source so any client (chat webview, diff --git a/packages/opencode/test/altimate/tool-result-errors.test.ts b/packages/opencode/test/altimate/tool-result-errors.test.ts index fc54df63af..b73ffdea81 100644 --- a/packages/opencode/test/altimate/tool-result-errors.test.ts +++ b/packages/opencode/test/altimate/tool-result-errors.test.ts @@ -58,7 +58,7 @@ describe("tool result failures", () => { expect(() => Tool.assertSuccess(result)).toThrow(output) }) - test("unsuccessful telemetry alone does not turn an ordinary result into an SDK error", async () => { + test("ordinary bash nonzero exits do not produce failure signals or an SDK error", async () => { const events: Telemetry.Event[] = [] spies.push( spyOn(Telemetry, "track").mockImplementation((event) => { @@ -69,8 +69,8 @@ describe("tool result failures", () => { expect(result.status).not.toBe("error") expect(result.metadata).toMatchObject({ success: false, exit_code: 1 }) expect(() => Tool.assertSuccess(result)).not.toThrow() - expect(events.find((event) => event.type === "tool_call")).toMatchObject({ status: "error" }) - expect(events.some((event) => event.type === "core_failure")).toBe(true) + expect(events.some((event) => event.type === "tool_call" && event.status === "error")).toBe(false) + expect(events.some((event) => event.type === "core_failure")).toBe(false) }) test("explicit error envelopes become SDK errors without error metadata", async () => { diff --git a/packages/opencode/test/session/tools-result-errors.test.ts b/packages/opencode/test/session/tools-result-errors.test.ts index 6578d29369..61d6291ff4 100644 --- a/packages/opencode/test/session/tools-result-errors.test.ts +++ b/packages/opencode/test/session/tools-result-errors.test.ts @@ -1,15 +1,20 @@ // altimate_change start — failed backend results must reject at the SDK tool boundary -import { expect } from "bun:test" +import { expect, spyOn } from "bun:test" +import { streamText } from "ai" +import { convertArrayToReadableStream, MockLanguageModelV3 } from "ai/test" import { Cause, Effect, Exit, Layer, Schema } from "effect" import { ProjectV2 } from "@opencode-ai/core/project" import { Agent } from "@/agent/agent" import { MCP } from "@/mcp" import { Permission } from "@/permission" import { Plugin } from "@/plugin" +import { ProjectID } from "@/project/schema" import type { Provider } from "@/provider/provider" import { ModelID, ProviderID } from "@/provider/schema" import { MessageID, SessionID } from "@/session/schema" import { SessionTools } from "@/session/tools" +import { SessionPrompt } from "@/session/prompt" +import * as Precedence from "@/altimate/workspace/precedence" import { ToolRegistry } from "@/tool/registry" import { Truncate } from "@/tool/truncate" import { testEffect } from "../lib/effect" @@ -147,4 +152,112 @@ for (const aborted of [false, true]) { }), ) } + +it.live("session/prompt resolveTools emits an SDK tool-error for a backend error envelope", () => + Effect.gen(function* () { + const sessionID = SessionID.make("ses_prompt_tools_errors") + const finished: unknown[] = [] + const registry = spyOn(ToolRegistry, "tools").mockResolvedValue([ + { + id: "backend_failure", + description: "Return a backend failure", + parameters: Schema.Struct({}), + registrySource: undefined, + execute: () => + Effect.succeed({ + title: "SQL: ERROR", + output: failure, + status: "error" as const, + metadata: { success: false, error: failure }, + }), + }, + ]) + const mcp = spyOn(MCP, "tools").mockResolvedValue({}) + const plugins = spyOn(Plugin, "trigger").mockImplementation(async (_name, _input, output) => output) + const precedence = spyOn(Precedence, "refresh").mockResolvedValue({ + enabled: false, + workspaceName: "", + shadowed: new Map(), + }) + try { + const tools = yield* Effect.promise(() => + SessionPrompt.resolveTools({ + agent, + model, + bypassAgentCheck: false, + messages: [], + session: { + id: sessionID, + slug: "prompt-tools-errors", + projectID: ProjectID.make("project"), + directory: "/tmp", + title: "Prompt tool errors", + version: "1", + time: { created: 0, updated: 0 }, + }, + processor: { + message: { + id: MessageID.make("msg_prompt_tools_errors"), + sessionID, + role: "assistant", + time: { created: 0 }, + parentID: MessageID.make("msg_user"), + modelID: model.id, + providerID: model.providerID, + mode: "build", + agent: "build", + path: { cwd: "/tmp", root: "/tmp" }, + cost: 0, + tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, + }, + beginToolExecution: (raw) => ({ raw, occurrence: 0 }), + finishToolExecution: (execution) => { + finished.push(execution) + }, + partFromToolExecution: () => undefined, + partFromToolCall: () => undefined, + process: async () => { + throw new Error("unexpected processor invocation") + }, + }, + }), + ) + const stream = streamText({ + model: new MockLanguageModelV3({ + doStream: async () => ({ + stream: convertArrayToReadableStream([ + { type: "stream-start", warnings: [] }, + { type: "tool-call", toolCallId: "call_error", toolName: "backend_failure", input: "{}" }, + { + type: "finish", + finishReason: { unified: "tool-calls", raw: "tool-calls" }, + usage: { + inputTokens: { total: 1, noCache: 1, cacheRead: 0, cacheWrite: 0 }, + outputTokens: { total: 1, text: 1, reasoning: 0 }, + }, + }, + ]), + }), + }), + prompt: "Run the query", + tools, + }) + const events = yield* Effect.promise(() => Array.fromAsync(stream.fullStream)) + expect(events.filter((event) => event.type === "tool-error")).toEqual([ + expect.objectContaining({ + toolCallId: "call_error", + toolName: "backend_failure", + error: expect.objectContaining({ message: failure }), + }), + ]) + expect(events.some((event) => event.type === "tool-result")).toBe(false) + expect(finished).toEqual([{ raw: "call_error", occurrence: 0 }]) + } finally { + precedence.mockRestore() + plugins.mockRestore() + mcp.mockRestore() + registry.mockRestore() + } + }), +) // altimate_change end diff --git a/packages/opencode/test/tool/bash.test.ts b/packages/opencode/test/tool/bash.test.ts index fb286bed39..7c43b78030 100644 --- a/packages/opencode/test/tool/bash.test.ts +++ b/packages/opencode/test/tool/bash.test.ts @@ -15,6 +15,11 @@ import { Agent } from "../../src/agent/agent" import { Truncate as TruncateService } from "../../src/tool/truncate" import { InstanceRef } from "../../src/effect/instance-ref" import { Tool } from "../../src/tool/tool" +// altimate_change start — shell outcome signals at the shared tool wrapper +import { spyOn } from "bun:test" +import { Telemetry } from "../../src/altimate/telemetry" +import * as DebugMode from "../../src/altimate/debug/mode" +// altimate_change end // The shared tool fixture stubs Truncate with a no-op so most tool tests don't // truncate. Truncation tests need the REAL Truncate service, so build the tool @@ -522,6 +527,7 @@ ModuleNotFoundError: No module named 'sklearn' expect(result.metadata.exit).toBe(7) expect(result.metadata.exit_code).toBe(7) expect(result.metadata.success).toBe(false) + expect(result.metadata.failure_kind).toBeUndefined() expect(result.status).not.toBe("error") expect(result.metadata.error).toBeUndefined() expect(() => Tool.assertSuccess(result)).not.toThrow() @@ -580,6 +586,7 @@ ModuleNotFoundError: No module named 'sklearn' }, ctx) expect(result.metadata.exit_code).toBe(0) expect(result.metadata.success).toBe(false) + expect(result.metadata.failure_kind).toBe("stderr_traceback") expect(result.status).not.toBe("error") expect(result.metadata.error).toBeUndefined() expect(() => Tool.assertSuccess(result)).not.toThrow() @@ -595,6 +602,7 @@ ModuleNotFoundError: No module named 'sklearn' }, ctx) expect(result.metadata.exit_code).toBe(1) expect(result.metadata.success).toBe(false) + expect(result.metadata.failure_kind).toBe("stderr_traceback") expect(result.status).not.toBe("error") expect(result.metadata.error).toBeUndefined() expect(() => Tool.assertSuccess(result)).not.toThrow() @@ -628,21 +636,56 @@ ModuleNotFoundError: No module named 'sklearn' }) // altimate_change end -// altimate_change start — truncation preserves shell telemetry and complete output -test("tool.bash truncation retains failure telemetry without an SDK error and complete output spill", async () => { +// altimate_change start — ordinary shell exits and execution failures have distinct signals +test.each([ + { + name: "grep no-match has a success hint and trace without core_failure", + command: `seq 1 ${Truncate.MAX_LINES + 10}; printf 'haystack\\n' | grep needle`, + timeout: undefined, + failureKind: undefined, + outcome: "success", + exitCode: 1, + }, + { + name: "timeout retains a failed hint, error trace, and core_failure", + command: `seq 1 ${Truncate.MAX_LINES + 10}; sleep 30`, + timeout: 1000, + failureKind: "timeout", + outcome: "error", + exitCode: null, + }, +])("tool.bash $name", async ({ command, timeout, failureKind, outcome, exitCode }) => { await provideInstance(projectRoot, async () => { const bash = await initToolWithTruncate(BashTool) - const result = await bash.execute({ - command: `seq 1 ${Truncate.MAX_LINES + 10}; exit 1`, description: "Fail after generating long output", - }, ctx) - expect(result.metadata.exit_code).toBe(1) - expect(result.metadata.success).toBe(false) - expect(result.status).not.toBe("error") - expect(() => Tool.assertSuccess(result)).not.toThrow() - expect(result.metadata.truncated).toBe(true) - expect(result.output).toContain("The tool call failed but the output was truncated") - expect(result.output).not.toContain("The tool call succeeded") - expect(await Filesystem.readText(result.metadata.outputPath)).toContain(`${Truncate.MAX_LINES + 10}\n`) + const events: Telemetry.Event[] = [] + const traces: string[] = [] + const track = spyOn(Telemetry, "track").mockImplementation((event) => { + events.push(event) + }) + const trace = spyOn(DebugMode, "traceToolCall").mockReturnValue((status) => { + traces.push(status) + }) + try { + const result = await bash.execute({ command, timeout, description: "Generate output then finish the command" }, ctx) + expect(result.metadata.exit_code).toBe(exitCode) + expect(result.metadata.success).toBe(false) + expect(result.metadata.failure_kind).toBe(failureKind) + expect(result.status).not.toBe("error") + expect(() => Tool.assertSuccess(result)).not.toThrow() + expect(result.metadata.truncated).toBe(true) + expect(result.output).toContain(outcome === "error" + ? "The tool call failed but the output was truncated" + : "The tool call succeeded but the output was truncated") + expect(result.output).not.toContain(outcome === "error" ? "The tool call succeeded" : "The tool call failed") + expect(traces).toEqual([outcome]) + expect(events.some((event) => event.type === "tool_call" && event.tool_name === "bash" && event.status === "error")) + .toBe(outcome === "error") + expect(events.some((event) => event.type === "core_failure" && event.tool_name === "bash")).toBe(outcome === "error") + expect(await Filesystem.readText(result.metadata.outputPath)).toContain(`${Truncate.MAX_LINES + 10}\n`) + } finally { + trace.mockRestore() + track.mockRestore() + } }) }) // altimate_change end @@ -684,6 +727,7 @@ test("tool.bash still terminates a foreground command after its timeout", async const bash = await initTool(BashTool) const result = await bash.execute({ command: "sleep 30", description: "Time out a command", timeout: 20 }, ctx) expect(result.metadata.success).toBe(false) + expect(result.metadata.failure_kind).toBe("timeout") expect(result.metadata.exit_code).toBeNull() expect(result.metadata.error).toBeUndefined() expect(result.status).not.toBe("error") @@ -699,6 +743,7 @@ test("tool.bash handles an already-aborted command without losing the exit event ...ctx, abort: AbortSignal.abort(), }) expect(result.metadata.success).toBe(false) + expect(result.metadata.failure_kind).toBe("abort") expect(result.metadata.exit_code).toBeNull() expect(result.metadata.error).toBeUndefined() expect(result.status).not.toBe("error") diff --git a/packages/opencode/test/tool/batch.test.ts b/packages/opencode/test/tool/batch.test.ts index 74730e6e7b..f59fc9e3c3 100644 --- a/packages/opencode/test/tool/batch.test.ts +++ b/packages/opencode/test/tool/batch.test.ts @@ -2,6 +2,7 @@ import { describe, test, expect, spyOn } from "bun:test" import { Effect, Result, Schema } from "effect" import path from "node:path" import { BatchTool } from "../../src/tool/batch" +import { Tool } from "../../src/tool/tool" import { ToolRegistry } from "../../src/tool/registry" import { Session } from "../../src/session" import { MessageID, SessionID } from "../../src/session/schema" @@ -62,6 +63,56 @@ describe("BatchTool: inner result errors", () => { }) describe("BatchTool: file lessons", () => { + test.each(["ok", "error"] as const)("appends lessons before checking an inner %s result", async (status) => { + const tool = await getToolInfo() + const order: string[] = [] + const innerResult = { title: "File", output: "Original output", status, metadata: {} } + const registry = spyOn(ToolRegistry, "tools").mockResolvedValue([{ + id: "read", + registrySource: undefined, + description: "test file tool", + parameters: Schema.Unknown, + execute: () => Effect.succeed(innerResult), + }]) + const updatePart = Object.assign(async (part: Parameters[0]) => part, Session.updatePart) + const updates = spyOn(Session, "updatePart").mockImplementation(updatePart) + const assertSuccess = Tool.assertSuccess + const check = spyOn(Tool, "assertSuccess").mockImplementation((result) => { + order.push("assertSuccess") + assertSuccess(result) + }) + const lessons = { + async appendFileLessons(_session: string, _tool: string, _args: unknown, result: Tool.ExecuteResult) { + // An asynchronous lesson hook must finish before the SDK boundary checks the result. + await Promise.resolve() + result.output += "\nFile lesson" + order.push("appendFileLessons") + }, + } + try { + const result = await tool.execute({ tool_calls: [{ tool: "read", parameters: { filePath: "report.sql" } }] }, { + sessionID: SessionID.make("ses_batch_order"), + messageID: MessageID.make("msg_batch_order"), + agent: "build", + abort: AbortSignal.any([]), + messages: [], + extra: { lessons }, + }) + expect(order).toEqual(["appendFileLessons", "assertSuccess"]) + expect(check).toHaveBeenCalledWith(innerResult) + expect(result.metadata.failed).toBe(status === "error" ? 1 : 0) + expect(updates.mock.calls.at(-1)?.[0]).toMatchObject({ + state: status === "error" + ? { status: "error", error: "Original output\nFile lesson" } + : { status: "completed", output: "Original output\nFile lesson" }, + }) + } finally { + check.mockRestore() + updates.mockRestore() + registry.mockRestore() + } + }) + test.each(["read", "edit", "write", "apply_patch"])("appends approved lessons to the inner %s result", async (toolName) => { await using tmp = await tmpdir() const sessionID = SessionID.make("ses_batch_lessons") From 2bfd6bb26f6ce2740de1334233c6512ac4e97cfa Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Mon, 12 Oct 2026 19:26:33 -0700 Subject: [PATCH 4/9] fix(analyst): make `sql_execute full_text_columns` tolerant of strict-schema clients and keep the projection Live sessions on a strict-schema model sent `full_text_columns` on every call (the model fills every property). Under the old contract that cost hundreds of rejected calls for an empty list, more for `limit > 20` while the mode was nominally active, and the successful calls returned only the named columns, so the model re-issued the query for the identity and metric columns it had selected. - `[]`, `null` or an omitted field turn the mode off; the call runs as a plain query. - With a non-empty list, an omitted `limit` means 20 and a larger `limit` is clamped to 20 with a note in the result instead of a validation error. Ordinary-mode limits are validated as before. - All selected columns are returned; the named columns are returned complete, without preview or spill cuts, while the other cells keep the usual clipping. A named column that is not in the result adds a one-line note listing the available columns and the rows are still returned. - Descriptions say "omit to use the default" instead of stating a literal default the model would copy. Tests cover `[]`/`null`/absent as plain queries, the 20-row default, clamping of 50 and 10001 with the note, `id, title, body` returned with `body` untruncated, the missing-column note, and the formatter's handling of oversized named cells, competing clipped cells, arrays and binary values. Co-Authored-By: Claude Fable 5.1 --- .../src/altimate/tools/sql-execute.ts | 36 +++-- .../src/altimate/tools/sql-result-format.ts | 110 +++++++++------ .../test/altimate/tools/sql-execute.test.ts | 76 ++++++++-- .../altimate/tools/sql-result-format.test.ts | 132 +++++++++++++----- 4 files changed, 242 insertions(+), 112 deletions(-) diff --git a/packages/opencode/src/altimate/tools/sql-execute.ts b/packages/opencode/src/altimate/tools/sql-execute.ts index 6b5624999c..f6d64276e8 100644 --- a/packages/opencode/src/altimate/tools/sql-execute.ts +++ b/packages/opencode/src/altimate/tools/sql-execute.ts @@ -51,16 +51,15 @@ const parameters = z .number({ error: LIMIT_ERROR }) .int(LIMIT_ERROR) .min(1, LIMIT_ERROR) - .max(MAX_SQL_ROWS, LIMIT_ERROR) .optional() - .default(100) - .describe(`Max rows to return (1-${MAX_SQL_ROWS}; default 100)`), + .describe( + `Max rows to return; omit to use the default. Ordinary mode allows 1-${MAX_SQL_ROWS}; active full_text_columns clamps larger limits to 20 with a result note.`, + ), full_text_columns: z .array(z.string().min(1)) - .min(1) - .optional() + .nullish() .describe( - "Return only these columns as complete text cells; requires limit <= 20. Oversized rows are saved to the spill for read access; byte limits still apply.", + "Keep all selected columns and return the named columns as complete text without preview or spill cuts. [] or null, or omitting this field, turns the mode off. A non-empty list caps rows at 20; omit limit to use the default, or larger limits are clamped with a result note. Missing names produce a note listing available columns; rows are still returned.", ), query_timeout_ms: z .number() @@ -72,9 +71,9 @@ const parameters = z "Optional per-query execution deadline in ms. Omit to use the configured deadline (connection setting or environment). Set only when a specific query needs a longer or shorter budget; 0 disables.", ), }) - .refine((args) => !args.full_text_columns || args.limit <= 20, { - message: "full_text_columns requires limit <= 20", - path: ["full_text_columns"], + .refine((args) => args.limit === undefined || args.limit <= MAX_SQL_ROWS || Boolean(args.full_text_columns?.length), { + message: LIMIT_ERROR, + path: ["limit"], }) export const SqlExecuteTool = Tool.define( @@ -85,10 +84,17 @@ export const SqlExecuteTool = Tool.define( Effect.gen(function* () { const bridge = yield* EffectBridge.make() return yield* legacyToInit({ - description: "Execute SQL against a connected data warehouse. Returns results as a formatted table.", + description: + "Execute SQL against a connected data warehouse. Returns all selected columns as a formatted table. full_text_columns keeps named cells complete without preview or spill cuts; [] or null, or omitting the field, turns the mode off. A non-empty list caps rows at 20: omit limit to use the default; larger limits clamp to 20 with a result note. Missing column names produce a note listing available columns while still returning rows.", parameters, formatValidationError: (error) => error.issues?.map((issue) => issue.message).join("; ") || LIMIT_ERROR, async execute(args, ctx) { + const fullText = Boolean(args.full_text_columns?.length) + const limit = fullText ? Math.min(args.limit ?? 20, 20) : (args.limit ?? 100) + const limitNote = + fullText && args.limit !== undefined && args.limit > 20 + ? `\n\nNote: limit ${args.limit} clamped to 20 for full_text_columns.` + : "" // altimate_change start - SQL write access control // Permission checks OUTSIDE try/catch so denial errors propagate to the framework // MongoDB warehouses take JSON command documents; the SQL classifier would read @@ -158,7 +164,7 @@ export const SqlExecuteTool = Tool.define( const result = await Dispatcher.call("sql.execute", { sql: query, warehouse: args.warehouse, - limit: args.limit, + limit, query_timeout_ms: args.query_timeout_ms, }) @@ -219,9 +225,11 @@ export const SqlExecuteTool = Tool.define( const limits = await bridge.promise(truncate.limits()) const suggestion = PostConnectSuggestions.getProgressiveSuggestion("sql_execute") const extra = - (suggestion ? "\n\n" + suggestion : "") + (precedence.notice ? precedence.notice + "\n\n" : "") + limitNote + + (suggestion ? "\n\n" + suggestion : "") + + (precedence.notice ? precedence.notice + "\n\n" : "") const formatted = await formatSqlResult(result, { - limit: args.limit, + limit, fullTextColumns: args.full_text_columns, // The statement text lets the formatter add the empty/near-empty // result hint when a string-literal filter is present. @@ -234,7 +242,7 @@ export const SqlExecuteTool = Tool.define( write: (text) => bridge.promise(truncate.write(text)), }) const lint = await lintSqlResult(args.query, args.warehouse, ctx.sessionID) - let output = formatted.output + (lint ? "\n\n" + lint.output : "") + let output = formatted.output + limitNote + (lint ? "\n\n" + lint.output : "") // altimate_change start — progressive disclosure suggestions if (suggestion) { output += "\n\n" + suggestion diff --git a/packages/opencode/src/altimate/tools/sql-result-format.ts b/packages/opencode/src/altimate/tools/sql-result-format.ts index 39ac54007f..a7151fa731 100644 --- a/packages/opencode/src/altimate/tools/sql-result-format.ts +++ b/packages/opencode/src/altimate/tools/sql-result-format.ts @@ -9,7 +9,7 @@ interface FormatOptions { limit: number maxBytes?: number maxLines?: number - fullTextColumns?: string[] + fullTextColumns?: string[] | null /** The executed statement; enables the empty-result hint. */ sql?: string write: (text: string) => Promise @@ -98,7 +98,7 @@ function displayCell(value: unknown, ancestors = new Set()): { text: st return { text: value === null ? "NULL" : String(value), clipped: false } } -function previewRows(rows: unknown[][], maxBytes: number, maxLines: number, columns: number, fullText = false) { +function previewRows(rows: unknown[][], maxBytes: number, maxLines: number, columns: number) { if (!columns) return { body: "", rows: 0, clipped: rows.length > 0 } const shown: string[] = [] let bytes = 0 @@ -110,7 +110,6 @@ function previewRows(rows: unknown[][], maxBytes: number, maxLines: number, colu clipped ||= cell.clipped return cell.text }) - if (fullText && clipped) break const remainingBytes = maxBytes - bytes - (shown.length ? 1 : 0) const remainingLines = maxLines - lines const rowBytes = @@ -123,11 +122,6 @@ function previewRows(rows: unknown[][], maxBytes: number, maxLines: number, colu continue } - if (fullText) { - clipped = true - break - } - // Preserve the first rows, with a visible partial cell when a row itself is oversized. const cells: string[] = [] let cellBytes = remainingBytes - Math.max(0, values.length - 1) * 3 @@ -146,6 +140,32 @@ function previewRows(rows: unknown[][], maxBytes: number, maxLines: number, colu return { body: shown.join("\n"), rows: shown.length, clipped: clipped || shown.length < rows.length } } +function fullTextRows(result: SqlExecuteResult, columns: Set, maxBytes: number, maxLines: number) { + let clipped = false + const ordinaryColumns = result.columns.filter((column) => !columns.has(column)).length + const rows = result.rows.map((row) => { + // Full text cells do not consume the ordinary preview budget. Share that budget + // across all rows so clipping another column cannot hide later full text cells. + let bytes = Math.max(0, Math.floor(maxBytes / result.rows.length) - Math.max(0, result.columns.length - 1) * 3) + let lines = Math.max(1, Math.floor(maxLines / result.rows.length)) + let remaining = ordinaryColumns + return result.columns + .map((column, index) => { + const value = row[index] + if (columns.has(column)) return value === null ? "NULL" : String(value) + const cell = displayCell(value) + const text = clipCell(cell.text, Math.floor(bytes / remaining), lines) ?? "…[cell clipped]" + clipped ||= cell.clipped || text !== cell.text + bytes -= Buffer.byteLength(text, "utf8") + lines -= lineCount(text) - 1 + remaining-- + return text + }) + .join(" | ") + }) + return { body: rows.join("\n"), rows: rows.length, clipped } +} + // Preflight before encoding/stringifying, bounding nested values and binary expansion // without materializing them. The binary estimate is deliberately conservative. function checkSpillSize(value: unknown, budget: { left: number }, ancestors = new Set()) { @@ -247,26 +267,22 @@ function isScalar(result: SqlExecuteResult): boolean { } export async function formatSqlResult(result: SqlExecuteResult, options: FormatOptions) { - const original = result - const fullText = Boolean(options.fullTextColumns?.length) - if (fullText) { - if (options.limit > 20 || result.rows.length > 20) - throw new Error("full_text_columns requires limit <= 20 and at most 20 returned rows") - const missing = options.fullTextColumns!.filter((column) => !result.columns.includes(column)) - if (missing.length) throw new Error(`Unknown full_text_columns: ${missing.join(", ")}`) - const indices = result.columns - .map((_, index) => index) - .filter((index) => options.fullTextColumns!.includes(result.columns[index]!)) - result = { - ...result, - columns: indices.map((index) => result.columns[index]!), - rows: result.rows.map((row) => indices.map((index) => row[index])), - } - } + const fullTextColumns = new Set(options.fullTextColumns ?? []) + const fullText = result.columns.some((column) => fullTextColumns.has(column)) + const missing = [...fullTextColumns].filter((column) => !result.columns.includes(column)) + const note = missing.length + ? "\n\n" + + `Note: Unknown full_text_columns: ${missing.join(", ")}. Available columns: ${result.columns.join(", ") || "(none)"}.`.replace( + /[\r\n]/g, + " ", + ) + : "" const maxBytes = Math.max(0, Math.min(SQL_MAX_PREVIEW_BYTES, options.maxBytes ?? SQL_MAX_PREVIEW_BYTES)) const maxLines = Math.max(0, options.maxLines ?? 2000) const fits = (text: string) => Buffer.byteLength(text, "utf8") <= maxBytes && lineCount(text) <= maxLines - const bounded = (text: string) => clipText(text, maxBytes, maxLines, "…[output clipped]") ?? "" + // Requested text bypasses the preview cap; a missing-name note must not displace rows. + const bounded = (text: string) => + (fullText ? text : (clipText(text, maxBytes, maxLines, "…[output clipped]") ?? "")) + note const footer = `\n\n(${result.row_count} rows)` const metadata = { rowCount: result.row_count, @@ -287,17 +303,25 @@ export async function formatSqlResult(result: SqlExecuteResult, options: FormatO return { output: bounded(withHint("(0 rows)")), metadata } } - let schema = previewSchema( - result.columns, - maxBytes - Buffer.byteLength(footer, "utf8"), - maxLines - lineCount(footer) + 1, - ) - let preview = previewRows( - result.rows, + const schemaFor = (bytes: number, lines: number) => + fullText + ? { + text: + result.columns.join(" | ") + + "\n" + + result.columns.map((column) => "-".repeat(Math.max(column.length, 4))).join("-+-") + + "\n", + columns: result.columns.length, + clipped: false, + } + : previewSchema(result.columns, bytes, lines) + const rowsFor = (bytes: number, lines: number, columns: number) => + fullText ? fullTextRows(result, fullTextColumns, bytes, lines) : previewRows(result.rows, bytes, lines, columns) + let schema = schemaFor(maxBytes - Buffer.byteLength(footer, "utf8"), maxLines - lineCount(footer) + 1) + let preview = rowsFor( Math.max(0, maxBytes - Buffer.byteLength(schema.text + footer, "utf8")), Math.max(0, maxLines - lineCount(schema.text + footer) + 1), schema.columns, - fullText, ) if (!preview.clipped && !schema.clipped && !result.truncated) { metadata.preview_rows = preview.rows @@ -307,7 +331,7 @@ export async function formatSqlResult(result: SqlExecuteResult, options: FormatO let spill: string try { - metadata.result_path = await options.write(serializeRows(original)) + metadata.result_path = await options.write(serializeRows(result)) spill = `All ${result.rows.length} returned rows saved to ${metadata.result_path}. Lossy JSONL text: columns/values per row; $bigint, $date (ISO), $bytes (base64) tags; strings unchanged.` } catch { spill = `Spill failed (write/serialization error or ${SQL_MAX_SPILL_BYTES / 1024 / 1024} MiB size limit); returned rows were not saved.` @@ -320,30 +344,24 @@ export async function formatSqlResult(result: SqlExecuteResult, options: FormatO `Output truncated: ${clipped ? "yes" : "no"}. ${spill}` + (clipped ? fullText - ? " Full text rows exceeding the output cap are omitted; use read on the spill with char_offset/max_chars to read each full line." - : " For complete text, rerun sql_execute with full_text_columns and limit <= 20; byte limits still apply." + ? " Named full_text_columns cells are complete; other cells may be clipped." + : " For complete text, rerun sql_execute with full_text_columns; omit limit to use the default." : "") + "\n" - // The entire output, including schema, envelope and path, shares the same cap. + // Ordinary previews include the schema, envelope and path in the same cap. const essentials = envelope(SQL_MAX_PREVIEW_ROWS, true) + footer - if (!fits(essentials)) { + if (!fullText && !fits(essentials)) { metadata.schema_truncated = result.columns.length > 0 metadata.output_truncated = metadata.truncated = true return { output: bounded(envelope(0, true)), metadata } } - schema = previewSchema( - result.columns, - maxBytes - Buffer.byteLength(essentials, "utf8"), - maxLines - lineCount(essentials) + 1, - ) + schema = schemaFor(maxBytes - Buffer.byteLength(essentials, "utf8"), maxLines - lineCount(essentials) + 1) const reserved = envelope(SQL_MAX_PREVIEW_ROWS, true) + schema.text + footer - preview = previewRows( - result.rows, + preview = rowsFor( Math.max(0, maxBytes - Buffer.byteLength(reserved, "utf8")), Math.max(0, maxLines - lineCount(reserved) + 1), schema.columns, - fullText, ) metadata.preview_rows = preview.rows metadata.preview_columns = schema.columns diff --git a/packages/opencode/test/altimate/tools/sql-execute.test.ts b/packages/opencode/test/altimate/tools/sql-execute.test.ts index 7cd06e6ebf..7188283baf 100644 --- a/packages/opencode/test/altimate/tools/sql-execute.test.ts +++ b/packages/opencode/test/altimate/tools/sql-execute.test.ts @@ -313,37 +313,85 @@ failedSpill.live("a spill service defect keeps SQL success, preview and clipping ) describe("sql_execute full text selection", () => { - test("requires an explicit row bound for full_text_columns before dispatch", async () => { + test.each([{ full_text_columns: [] }, { full_text_columns: null }, { full_text_columns: undefined }])( + "treats full_text_columns %j like a plain query", + async ({ full_text_columns }) => { + const tool = await initTool(SqlExecuteTool) + for (const limit of [undefined, 50]) { + const plain = await tool.execute({ query: "SELECT 42", limit }, ctx) + const output = await tool.execute({ query: "SELECT 42", full_text_columns, limit }, ctx) + expect(output).toEqual(plain) + expect(dispatch).toHaveBeenLastCalledWith( + "sql.execute", + expect.objectContaining({ sql: "SELECT 42", limit: limit ?? 100 }), + ) + } + }, + ) + + test("defaults an active full text query to 20 rows", async () => { const tool = await initTool(SqlExecuteTool) - for (const limit of [undefined, 21]) { - await expect(tool.execute({ query: "SELECT 42", full_text_columns: ["answer"], limit }, ctx)).rejects.toThrow( - "limit <= 20", - ) - } - expect(dispatch).not.toHaveBeenCalled() - expect(precedence).not.toHaveBeenCalled() + const output = await tool.execute({ query: "SELECT 42", full_text_columns: ["answer"] }, ctx) + expect(dispatch).toHaveBeenCalledWith("sql.execute", expect.objectContaining({ limit: 20 })) + expect(output.output).toContain("42") + expect(output.output).not.toContain("clamped") + expect(output.metadata.error).toBeUndefined() + }) + + test.each([50, 10_001])("clamps an active full text limit of %i to 20 with a result note", async (limit) => { + const tool = await initTool(SqlExecuteTool) + const output = await tool.execute({ query: "SELECT 42", full_text_columns: ["answer"], limit }, ctx) + expect(dispatch).toHaveBeenCalledWith("sql.execute", expect.objectContaining({ limit: 20 })) + expect(output.output).toContain("42") + expect(output.output).toMatch(new RegExp(`Note:.*limit.*${limit}.*clamped.*20`, "i")) + expect(output.metadata.error).toBeUndefined() }) - test("passes selected columns to the formatter without changing the executed query", async () => { - const text = "Complete policy condition. ".repeat(350) + test("returns id, title and untruncated body without changing the executed query", async () => { + const text = "Complete body condition.\n".repeat(3000) dispatch.mockResolvedValue({ - columns: ["policy", "notes"], - rows: [[text, "x".repeat(20000)]], + columns: ["id", "title", "body"], + rows: [[17, "Policy title", text]], row_count: 1, truncated: false, } as never) const tool = await initTool(SqlExecuteTool) const output = await tool.execute( - { query: "SELECT policy, notes FROM rules", limit: 1, full_text_columns: ["policy"] }, + { query: "SELECT id, title, body FROM rules", limit: 1, full_text_columns: ["body"] }, ctx, ) + expect(output.output).toContain("id | title | body") + expect(output.output).toContain("17 | Policy title | ") expect(output.output).toContain(text) expect(output.output).not.toContain("cell clipped") + expect(output.metadata).toMatchObject({ preview_columns: 3, output_truncated: false, truncated: false }) + expect(output.metadata.result_path).toBeUndefined() expect(dispatch).toHaveBeenCalledWith( "sql.execute", - expect.objectContaining({ sql: "SELECT policy, notes FROM rules", limit: 1 }), + expect.objectContaining({ sql: "SELECT id, title, body FROM rules", limit: 1 }), ) }) + + test("notes missing full text columns and available columns while returning rows", async () => { + const tool = await initTool(SqlExecuteTool) + const output = await tool.execute({ query: "SELECT 42", full_text_columns: ["missing"] }, ctx) + expect(output.output).toContain("42") + expect(output.output).toMatch(/Note:.*missing.*available columns: answer/i) + expect(output.metadata.error).toBeUndefined() + expect(output.metadata.rows_returned).toBe(1) + }) + + test("describes full text behavior without advertising a literal default limit", async () => { + const tool = await initTool(SqlExecuteTool) + const schema = tool.jsonSchema as { properties: Record } + expect(schema.properties.limit.description).toContain("omit to use the default") + expect(schema.properties.limit.description).not.toMatch(/default\s+\d+/i) + expect(schema.properties.limit.default).toBeUndefined() + expect(tool.description).toContain("all selected columns") + expect(tool.description).toContain("[]") + expect(tool.description).toContain("null") + expect(tool.description).toContain("clamp") + }) }) test("full text selection retains write permission checks", async () => { diff --git a/packages/opencode/test/altimate/tools/sql-result-format.test.ts b/packages/opencode/test/altimate/tools/sql-result-format.test.ts index 1cd6969376..3c994f8536 100644 --- a/packages/opencode/test/altimate/tools/sql-result-format.test.ts +++ b/packages/opencode/test/altimate/tools/sql-result-format.test.ts @@ -443,24 +443,25 @@ describe("empty-result hint", () => { }) describe("SQL full text retrieval", () => { - test("returns selected text columns without clipping competing wide cells", async () => { + test("retains every selected column and complete named text while clipping competing wide cells", async () => { const saved = spill() - const text = "Complete policy condition. ".repeat(350) - const formatted = await formatSqlResult( - result([[text, "x".repeat(9000), "y".repeat(9000)]], ["policy", "detail", "notes"]), - { - limit: 1, - fullTextColumns: ["policy"], - write: saved.write, - }, - ) + const text = "Complete policy condition. ".repeat(2000) + const rows = [[text, "x".repeat(20000), "y".repeat(20000)]] + const formatted = await formatSqlResult(result(rows, ["policy", "detail", "notes"]), { + limit: 1, + fullTextColumns: ["policy"], + write: saved.write, + }) expect(formatted.output).toContain(text) - expect(formatted.output).not.toContain("cell clipped") - expect(formatted.metadata.preview_columns).toBe(1) - expect(Buffer.byteLength(formatted.output)).toBeLessThanOrEqual(SQL_MAX_PREVIEW_BYTES) + expect(formatted.output).toContain("policy | detail | notes") + expect(formatted.output).toContain("cell clipped") + expect(formatted.metadata.preview_columns).toBe(3) + expect(formatted.metadata.preview_rows).toBe(1) + expect(formatted.metadata.output_truncated).toBe(true) + expect(saved.rows()).toEqual([{ columns: ["policy", "detail", "notes"], values: rows[0] }]) }) - test("spills oversized full text without displaying a partial cell", async () => { + test("returns oversized named text inline without a preview or spill cut", async () => { const saved = spill() const text = "Full condition. ".repeat(2000) const formatted = await formatSqlResult(result([[text]], ["policy"]), { @@ -468,27 +469,74 @@ describe("SQL full text retrieval", () => { fullTextColumns: ["policy"], write: saved.write, }) + expect(formatted.output).toContain(text) expect(formatted.output).not.toContain("cell clipped") - expect(formatted.metadata.preview_rows).toBe(0) - expect(formatted.metadata.output_truncated).toBe(true) - expect(formatted.output).toContain("read") - expect(saved.rows()[0].values[0]).toBe(text) - expect(Buffer.byteLength(formatted.output)).toBeLessThanOrEqual(SQL_MAX_PREVIEW_BYTES) + expect(formatted.metadata.preview_rows).toBe(1) + expect(formatted.metadata.output_truncated).toBe(false) + expect(formatted.metadata.result_path).toBeUndefined() + expect(saved.writes).toHaveLength(0) }) - test("truncation explains bounded full text retrieval", async () => { + test("truncation explains full text retrieval without requiring an explicit limit", async () => { const formatted = await formatSqlResult(result([["x".repeat(20000)]]), { limit: 100, write: spill().write }) expect(formatted.output).toContain("full_text_columns") - expect(formatted.output).toContain("limit <= 20") + expect(formatted.output).not.toContain("limit <= 20") }) - test("rejects full text above the row bound or for unknown columns", async () => { - await expect( - formatSqlResult(result([["text"]]), { limit: 21, fullTextColumns: ["value"], write: spill().write }), - ).rejects.toThrow("limit <= 20") - await expect( - formatSqlResult(result([["text"]]), { limit: 1, fullTextColumns: ["missing"], write: spill().write }), - ).rejects.toThrow("Unknown full_text_columns") + test.each([{ fullTextColumns: undefined }, { fullTextColumns: null }, { fullTextColumns: [] }])( + "treats %p full text columns identically to ordinary formatting", + async ({ fullTextColumns }) => { + const plainSpill = spill() + const saved = spill() + const input = result([["x".repeat(SQL_MAX_PREVIEW_BYTES * 2)]]) + const plain = await formatSqlResult(input, { limit: 100, write: plainSpill.write }) + const formatted = await formatSqlResult(input, { + limit: 100, + fullTextColumns: fullTextColumns ? [...fullTextColumns] : fullTextColumns, + write: saved.write, + }) + expect(formatted).toEqual(plain) + expect(saved.writes).toEqual(plainSpill.writes) + }, + ) + + test.each([ + { rows: [[17, "A title", "Complete body"]], maxBytes: undefined }, + { rows: [], maxBytes: undefined }, + { rows: [[42]], maxBytes: 64 }, + ])("notes unknown names and available columns while returning the result %p", async ({ rows, maxBytes }) => { + const formatted = await formatSqlResult(result(rows.map((row) => [...row]), ["id", "title", "body"]), { + limit: 1, + fullTextColumns: ["missing"], + maxBytes, + write: spill().write, + }) + const note = formatted.output.split("\n").find((line) => line.includes("missing")) + expect(note).toMatch(/available columns.*id.*title.*body/i) + expect(formatted.output).toContain(rows.length ? (maxBytes ? "42" : "17 | A title | Complete body") : "(0 rows)") + expect(formatted.metadata.rows_returned).toBe(rows.length) + expect(formatted.metadata.output_truncated).toBe(false) + }) + + test("retains named text in later rows when an ordinary cell is clipped", async () => { + const saved = spill() + const first = "First complete body. ".repeat(1000) + const second = "Second complete body. ".repeat(1000) + const formatted = await formatSqlResult( + result( + [ + [1, "x".repeat(20000), first], + [2, "A title", second], + ], + ["id", "title", "body"], + ), + { limit: 2, fullTextColumns: ["body"], write: saved.write }, + ) + expect(formatted.output).toContain("id | title | body") + expect(formatted.output).toContain(first) + expect(formatted.output).toContain(second) + expect(formatted.metadata.preview_rows).toBe(2) + expect(saved.rows()).toHaveLength(2) }) test("pads absent spill values with null while retaining explicit undefined and column order", async () => { @@ -505,22 +553,28 @@ describe("SQL full text retrieval", () => { }) }) -test("full text selection respects configured bytes and spills the original columns", async () => { +test("full text preserves all selected columns beyond configured byte and line limits", async () => { const saved = spill() - const text = "Complete condition. ".repeat(100) - const formatted = await formatSqlResult(result([[17, text]], ["id", "policy"]), { + const text = "Complete condition.\n".repeat(2200) + const formatted = await formatSqlResult(result([[17, "A title", text]], ["id", "title", "body"]), { limit: 1, - fullTextColumns: ["policy"], + fullTextColumns: ["body"], maxBytes: 1024, + maxLines: 10, write: saved.write, }) - expect(Buffer.byteLength(formatted.output)).toBeLessThanOrEqual(1024) - expect(formatted.metadata.preview_rows).toBe(0) - expect(saved.rows()[0]).toEqual({ columns: ["id", "policy"], values: [17, text] }) + expect(formatted.output).toContain("id | title | body") + expect(formatted.output).toContain(`17 | A title | ${text}`) + expect(formatted.output).not.toContain("clipped") + expect(formatted.metadata.preview_rows).toBe(1) + expect(formatted.metadata.preview_columns).toBe(3) + expect(formatted.metadata.output_truncated).toBe(false) + expect(formatted.metadata.truncated).toBe(false) + expect(saved.writes).toHaveLength(0) }) test.each([[new Uint8Array(SQL_MAX_PREVIEW_BYTES * 2)], [Array(20000).fill("large")]])( - "full text omits rows containing internally clipped values", + "full text returns complete arrays and binary values without internal clipping", async (value) => { const saved = spill() const formatted = await formatSqlResult(result([[value]]), { @@ -528,9 +582,11 @@ test.each([[new Uint8Array(SQL_MAX_PREVIEW_BYTES * 2)], [Array(20000).fill("larg fullTextColumns: ["value"], write: saved.write, }) - expect(formatted.metadata.preview_rows).toBe(0) + expect(formatted.metadata.preview_rows).toBe(1) + expect(formatted.output).toContain(String(value)) expect(formatted.output).not.toContain("cell clipped") expect(formatted.output).not.toContain("array clipped") - expect(saved.rows()).toHaveLength(1) + expect(formatted.metadata.output_truncated).toBe(false) + expect(saved.writes).toHaveLength(0) }, ) From 425e6e2c340cfd723ed44a056783059faeb0a574 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Mon, 12 Oct 2026 19:39:29 -0700 Subject: [PATCH 5/9] fix(drivers): refuse the ambiguous Mongo `distinct` shorthand instead of querying the wrong collection A collection shorthand that also carried the native `key` field, such as `{ collection: "items", distinct: "category", key: "status" }`, left the shorthand argument undefined and fell through to the native-command parser, which read `"category"` as the collection name. Shorthand `distinct` is now validated against both `key` and `field`: agreeing values are accepted, conflicting values are refused with an error naming both, and any document carrying `collection` never reaches the native parser. Tests: conflicting `key`/`field` refused naming `category` and `status`; agreeing aliases accepted; distinct-as-field keeps the collection; unsupported, empty and null `collection` documents do not fall through. Co-Authored-By: Claude Fable 5.1 --- packages/drivers/src/mongodb.ts | 12 +++++-- packages/drivers/test/mongodb-unit.test.ts | 37 ++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/packages/drivers/src/mongodb.ts b/packages/drivers/src/mongodb.ts index d1dde5b2ca..ffd9653cb3 100644 --- a/packages/drivers/src/mongodb.ts +++ b/packages/drivers/src/mongodb.ts @@ -78,13 +78,19 @@ export function normalizeMqlCommand>(parsed: T ? "filter" : command === "aggregate" && Array.isArray(value) ? "pipeline" - : command === "distinct" && typeof value === "string" && parsed["key"] === undefined + : command === "distinct" && typeof value === "string" ? "field" : undefined if (argument) { - const explicitArguments = command === "countDocuments" ? [argument, "query"] : [argument] + const explicitArguments = + command === "countDocuments" ? [argument, "query"] : command === "distinct" ? [argument, "key"] : [argument] for (const explicit of explicitArguments) { if (parsed[explicit] !== undefined && !isDeepStrictEqual(parsed[explicit], value)) { + if (command === "distinct") { + throw new Error( + `Ambiguous distinct form: 'distinct' value ${JSON.stringify(value)} conflicts with explicit '${explicit}' value ${JSON.stringify(parsed[explicit])}`, + ) + } throw new Error(`Ambiguous MQL command: '${key}' conflicts with explicit '${explicit}' argument`) } } @@ -95,6 +101,8 @@ export function normalizeMqlCommand>(parsed: T return normalized as T & MqlQuery } } + // A collection-bearing document is shorthand, even when its arguments are invalid. + if (Object.hasOwn(parsed, "collection")) return parsed as T & MqlQuery for (const [key, command] of Object.entries(NATIVE_COMMAND_KEYS)) { const value = parsed[key] if (typeof value !== "string" || !value) continue diff --git a/packages/drivers/test/mongodb-unit.test.ts b/packages/drivers/test/mongodb-unit.test.ts index 7c642d2b8c..165af6f55c 100644 --- a/packages/drivers/test/mongodb-unit.test.ts +++ b/packages/drivers/test/mongodb-unit.test.ts @@ -154,6 +154,43 @@ describe("MongoDB native command filters", () => { }) describe("MongoDB collection command shorthand", () => { + test.each([ + { name: "key", aliases: { key: "status" } }, + { name: "field", aliases: { field: "status" } }, + { name: "field with an agreeing key", aliases: { key: "category", field: "status" } }, + { name: "key with an agreeing field", aliases: { key: "status", field: "category" } }, + ])("refuses ambiguous distinct form with conflicting $name and names both values", ({ aliases }) => { + expect(() => normalizeMqlCommand({ collection: "items", distinct: "category", ...aliases })).toThrow( + /ambiguous distinct form.*category.*status/i, + ) + }) + + test("normalizes distinct-as-field shorthand without changing the collection", () => { + expect(normalizeMqlCommand({ collection: "items", distinct: "category" })).toEqual({ + collection: "items", + command: "distinct", + field: "category", + }) + }) + + test.each([ + { name: "key", aliases: { key: "category" } }, + { name: "field", aliases: { field: "category" } }, + { name: "key and field", aliases: { key: "category", field: "category" } }, + ])("accepts distinct shorthand with agreeing $name", ({ aliases }) => { + const normalized = normalizeMqlCommand({ collection: "items", distinct: "category", ...aliases }) + expect(normalized).toMatchObject({ collection: "items", command: "distinct", field: "category" }) + expect(normalized).not.toHaveProperty("distinct") + }) + + test.each([ + { collection: "items", find: "other" }, + { collection: "", distinct: "category", key: "category" }, + { collection: null, distinct: "category", key: "category" }, + ])("does not fall through to native parsing when collection is present: %j", (input) => { + expect(normalizeMqlCommand(input)).toEqual(input) + }) + test.each([ { name: "find and filter", input: { collection: "items", find: { status: "x" }, filter: { status: "y" } } }, { name: "count and query", input: { collection: "items", count: { status: "x" }, query: { status: "y" } } }, From 965f97a81098dd4d342a8cfd961547ae4796088c Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Mon, 12 Oct 2026 20:13:01 -0700 Subject: [PATCH 6/9] fix(analyst): keep `full_text_columns` output bounded and always recoverable Letting named cells bypass the formatter cap did not deliver complete text: the session applies a per-result token cap before persisting any tool output, so an oversized cell was middle-truncated anyway, and because the clean path wrote no spill and reported `truncated: false`, the missing text could not be recovered. The mode is now "priority within the same bounded output, always recoverable": - Full-text mode always writes one complete lossless JSONL spill of the returned rows (including empty results and unknown names) and sets `result_path`; the banner says `read` with `char_offset`/`max_chars` recovers clipped text. - Named cells take the shared byte/line budget first, in row order; a cell that does not fit is clipped with a continued-in-spill marker and the metadata reports the clip. Once the budget is exhausted, remaining cells are omitted behind one note instead of a marker per cell. - Structured and binary cells are serialised with the same tagged serialiser as the spill, inline and on disk, instead of `String(value)`. - The schema line keeps its ordinary alias and column caps, with named columns eligible beyond the ordinary prefix; the missing-column, limit, lint and suggestion notes are counted inside the output budget. - The formatter enforces the effective row limit (20 in full-text mode), notes when a connector returned more, and spills all returned rows. Tests: 50 KiB named cell clipped within 16 KiB with its complete spill; twenty 1 MiB cells bounded with one spill; structured and binary cells lossless inline and in the spill; empty result with a missing name held to a 64-byte budget with the clip reported; 300 long aliases bounded beside one named column; 40 connector rows limited to 20 with all rows saved; notes counted in the budget. Co-Authored-By: Claude Fable 5.1 --- .../src/altimate/tools/sql-execute.ts | 56 +-- .../src/altimate/tools/sql-result-format.ts | 370 ++++++++++++------ .../test/altimate/tools/sql-execute.test.ts | 114 +++++- .../altimate/tools/sql-result-format.test.ts | 321 ++++++++++----- 4 files changed, 603 insertions(+), 258 deletions(-) diff --git a/packages/opencode/src/altimate/tools/sql-execute.ts b/packages/opencode/src/altimate/tools/sql-execute.ts index f6d64276e8..b7114e65ae 100644 --- a/packages/opencode/src/altimate/tools/sql-execute.ts +++ b/packages/opencode/src/altimate/tools/sql-execute.ts @@ -3,18 +3,13 @@ import { Effect } from "effect" import { EffectBridge } from "../../effect/bridge" import { Truncate } from "../../tool/truncate" import { legacyToInit, type LegacyToolDef } from "../tool-zod-compat" -import { formatSqlResult, SQL_MAX_PREVIEW_BYTES } from "./sql-result-format" +import { formatSqlResult } from "./sql-result-format" import { lintSqlResult } from "./sql-result-lint" import { Tool } from "../../tool/tool" import { Dispatcher } from "../native" import type { SqlExecuteResult } from "../native/types" // altimate_change start - SQL write access control + fingerprinting -import { - classifyAndCheck, - classifyMongoCommand, - computeSqlFingerprint, - isScratchSafeWrite, -} from "./sql-classify" +import { classifyAndCheck, classifyMongoCommand, computeSqlFingerprint, isScratchSafeWrite } from "./sql-classify" import { statementsOf } from "./sql-tokenize" const MONGO_WAREHOUSE_TYPES = new Set(["mongodb", "mongo"]) @@ -59,7 +54,7 @@ const parameters = z .array(z.string().min(1)) .nullish() .describe( - "Keep all selected columns and return the named columns as complete text without preview or spill cuts. [] or null, or omitting this field, turns the mode off. A non-empty list caps rows at 20; omit limit to use the default, or larger limits are clamped with a result note. Missing names produce a note listing available columns; rows are still returned.", + "Keep all selected columns and prioritize named cell text within the ordinary output budget. Always save all returned rows to result_path; use read with char_offset/max_chars to recover clipped text. [] or null, or omitting this field, turns the mode off. A non-empty list caps preview rows at 20; omit limit to use the default, or larger limits are clamped with a result note. Missing names produce a bounded note listing available columns; rows are still returned.", ), query_timeout_ms: z .number() @@ -85,7 +80,7 @@ export const SqlExecuteTool = Tool.define( const bridge = yield* EffectBridge.make() return yield* legacyToInit({ description: - "Execute SQL against a connected data warehouse. Returns all selected columns as a formatted table. full_text_columns keeps named cells complete without preview or spill cuts; [] or null, or omitting the field, turns the mode off. A non-empty list caps rows at 20: omit limit to use the default; larger limits clamp to 20 with a result note. Missing column names produce a note listing available columns while still returning rows.", + "Execute SQL against a connected data warehouse. Returns all selected columns as a formatted table. full_text_columns prioritizes named cells within the ordinary output budget and always saves returned rows to result_path; read with char_offset/max_chars recovers clipped text. [] or null, or omitting the field, turns the mode off. A non-empty list caps preview rows at 20: omit limit to use the default; larger limits clamp to 20 with a result note. Missing column names produce a bounded note listing available columns while still returning rows.", parameters, formatValidationError: (error) => error.issues?.map((issue) => issue.message).join("; ") || LIMIT_ERROR, async execute(args, ctx) { @@ -93,7 +88,7 @@ export const SqlExecuteTool = Tool.define( const limit = fullText ? Math.min(args.limit ?? 20, 20) : (args.limit ?? 100) const limitNote = fullText && args.limit !== undefined && args.limit > 20 - ? `\n\nNote: limit ${args.limit} clamped to 20 for full_text_columns.` + ? `Note: limit ${args.limit} clamped to 20 for full_text_columns.` : "" // altimate_change start - SQL write access control // Permission checks OUTSIDE try/catch so denial errors propagate to the framework @@ -122,7 +117,9 @@ export const SqlExecuteTool = Tool.define( // Connectors expose one result set; several prepare only one statement // or cannot normalize a batch response. Do not silently drop reads. if (count > 1) { - throw new Error("Multi-statement reads are not supported by this execution path. Run each read separately.") + throw new Error( + "Multi-statement reads are not supported by this execution path. Run each read separately.", + ) } } // The ephemeral scratch database is the session's own workspace (what sql_materialize @@ -224,28 +221,20 @@ export const SqlExecuteTool = Tool.define( const limits = await bridge.promise(truncate.limits()) const suggestion = PostConnectSuggestions.getProgressiveSuggestion("sql_execute") - const extra = - limitNote + - (suggestion ? "\n\n" + suggestion : "") + - (precedence.notice ? precedence.notice + "\n\n" : "") + const lint = await lintSqlResult(args.query, args.warehouse, ctx.sessionID) const formatted = await formatSqlResult(result, { limit, fullTextColumns: args.full_text_columns, // The statement text lets the formatter add the empty/near-empty // result hint when a string-literal filter is present. sql: query, - maxBytes: Math.max( - 0, - Math.min(SQL_MAX_PREVIEW_BYTES, limits.maxBytes) - Buffer.byteLength(extra, "utf8"), - ), - maxLines: Math.max(0, limits.maxLines - extra.split("\n").length + 1), + notes: [precedence.notice ?? "", limitNote, lint?.output ?? "", suggestion ?? ""].filter(Boolean), + maxBytes: limits.maxBytes, + maxLines: limits.maxLines, write: (text) => bridge.promise(truncate.write(text)), }) - const lint = await lintSqlResult(args.query, args.warehouse, ctx.sessionID) - let output = formatted.output + limitNote + (lint ? "\n\n" + lint.output : "") // altimate_change start — progressive disclosure suggestions - if (suggestion) { - output += "\n\n" + suggestion + if (suggestion && formatted.output.includes(suggestion)) { PostConnectSuggestions.trackSuggestions({ suggestionType: "progressive_disclosure", suggestionsShown: ["sql_analyze"], @@ -253,19 +242,14 @@ export const SqlExecuteTool = Tool.define( }) } // altimate_change end - // altimate_change — carries the fail-open notice when the target could not be - // attributed to the workspace; a no-op otherwise. - const response = Precedence.annotate(precedence, { - title: `SQL: ${args.query.slice(0, 60)}${args.query.length > 60 ? "..." : ""}`, - metadata: { ...formatted.metadata, ...(lint && { lint_rules: lint.rules }) }, - output, - }) - if (precedence.notice && formatted.metadata.truncated) { - // Keep the result contract first while retaining the routing notice. - const newline = output.indexOf("\n") - response.output = output.slice(0, newline + 1) + precedence.notice + "\n\n" + output.slice(newline + 1) + // Preserve routing metadata while the formatter owns the bounded notice text. + return { + ...Precedence.annotate(precedence, { + title: `SQL: ${args.query.slice(0, 60)}${args.query.length > 60 ? "..." : ""}`, + metadata: { ...formatted.metadata, ...(lint && { lint_rules: lint.rules }) }, + }), + output: formatted.output, } - return response } catch (e) { const msg = e instanceof Error ? e.message : String(e) // altimate_change: deliberately NOT fingerprinted, same reasoning as the diff --git a/packages/opencode/src/altimate/tools/sql-result-format.ts b/packages/opencode/src/altimate/tools/sql-result-format.ts index a7151fa731..775ed6e5a8 100644 --- a/packages/opencode/src/altimate/tools/sql-result-format.ts +++ b/packages/opencode/src/altimate/tools/sql-result-format.ts @@ -10,6 +10,7 @@ interface FormatOptions { maxBytes?: number maxLines?: number fullTextColumns?: string[] | null + notes?: string[] /** The executed statement; enables the empty-result hint. */ sql?: string write: (text: string) => Promise @@ -45,22 +46,32 @@ function clipCell(text: string, maxBytes: number, maxLines: number): string | un return clipText(text, maxBytes, maxLines, `…[cell clipped, ${chars} chars]`) } -function previewSchema(columns: string[], maxBytes: number, maxLines: number) { +function previewSchema(columns: string[], maxBytes: number, maxLines: number, priority?: Set) { + const order = columns.map((_, index) => index) + if (priority) order.sort((a, b) => Number(priority.has(columns[b]!)) - Number(priority.has(columns[a]!))) for (let count = Math.min(columns.length, SQL_MAX_PREVIEW_COLUMNS); count > 0; count--) { const omitted = count < columns.length ? `(+${columns.length - count} more columns, see spill)\n` : "" // Both the aliases and their separator consume space, along with delimiters/newlines. const available = maxBytes - Buffer.byteLength(omitted, "utf8") - 2 - (count - 1) * 6 const aliasBytes = Math.min(128, Math.floor(available / (2 * count))) if (aliasBytes < 4) continue - const aliases = columns.slice(0, count).map((name) => clipText(name, aliasBytes, 1, "…[alias clipped]")) + // Retain named columns even beyond the ordinary prefix, without relaxing the + // column or alias caps. Display the chosen columns in their original order. + const indices = order.slice(0, count).sort((a, b) => a - b) + const aliases = indices.map((index) => clipText(columns[index]!, aliasBytes, 1, "…[alias clipped]")) if (aliases.some((name) => name === undefined)) continue const names = aliases as string[] const text = names.join(" | ") + "\n" + names.map((name) => "-".repeat(Math.max(name.length, 4))).join("-+-") + "\n" + omitted if (Buffer.byteLength(text, "utf8") > maxBytes || lineCount(text) > maxLines) continue - return { text, columns: count, clipped: count < columns.length || names.some((name, i) => name !== columns[i]) } + return { + text, + indices, + columns: count, + clipped: count < columns.length || names.some((name, i) => name !== columns[indices[i]!]), + } } - return { text: "", columns: 0, clipped: columns.length > 0 } + return { text: "", indices: [], columns: 0, clipped: columns.length > 0 } } function displayCell(value: unknown, ancestors = new Set()): { text: string; clipped: boolean } { @@ -140,30 +151,104 @@ function previewRows(rows: unknown[][], maxBytes: number, maxLines: number, colu return { body: shown.join("\n"), rows: shown.length, clipped: clipped || shown.length < rows.length } } -function fullTextRows(result: SqlExecuteResult, columns: Set, maxBytes: number, maxLines: number) { - let clipped = false - const ordinaryColumns = result.columns.filter((column) => !columns.has(column)).length - const rows = result.rows.map((row) => { - // Full text cells do not consume the ordinary preview budget. Share that budget - // across all rows so clipping another column cannot hide later full text cells. - let bytes = Math.max(0, Math.floor(maxBytes / result.rows.length) - Math.max(0, result.columns.length - 1) * 3) - let lines = Math.max(1, Math.floor(maxLines / result.rows.length)) - let remaining = ordinaryColumns - return result.columns - .map((column, index) => { - const value = row[index] - if (columns.has(column)) return value === null ? "NULL" : String(value) - const cell = displayCell(value) - const text = clipCell(cell.text, Math.floor(bytes / remaining), lines) ?? "…[cell clipped]" +const CONTINUED_CELL = "…[continued in spill]" +const OMITTED_CELLS = "…[remaining cells omitted; see spill]" + +function namedCell(value: unknown, maxBytes: number, maxLines: number): { text?: string; clipped: boolean } { + // Strings stay readable; structured/binary values use the spill's tagged JSON. + if (typeof value === "string") { + const text = clipText(value, maxBytes, maxLines, CONTINUED_CELL) + return { text, clipped: text !== value } + } + let text = "" + let bytes = 0 + let lines = 1 + try { + for (const chunk of spillValue(value)) { + const prefix = clipText(chunk, maxBytes - bytes, maxLines - lines + 1, "") ?? "" + text += prefix + bytes += Buffer.byteLength(prefix, "utf8") + lines += lineCount(prefix) - 1 + if (prefix !== chunk) { + const head = clipText(text, maxBytes - Buffer.byteLength(CONTINUED_CELL), maxLines, "") + return { text: head === undefined ? undefined : head + CONTINUED_CELL, clipped: true } + } + } + return { text, clipped: false } + } catch { + // Cyclic/deep values also fail the spill preflight; the banner reports that failure. + return { text: clipText("…[cell unavailable; see spill status]", maxBytes, maxLines, ""), clipped: true } + } +} + +function fullTextRows( + rows: unknown[][], + names: string[], + selected: Set, + maxBytes: number, + maxLines: number, + reserveNote = false, +): { body: string; rows: number; clipped: boolean } { + if (!names.length || !rows.length) return { body: "", rows: 0, clipped: rows.length > 0 } + // Named cells spend the shared allowance first, in row order. If cells must be + // omitted, retry with space reserved for one trailing note. Each pass is bounded. + const noteBytes = reserveNote ? Buffer.byteLength(OMITTED_CELLS) + 1 : 0 + const noteLines = reserveNote ? 1 : 0 + let count = rows.length + while (count && ((names.length - 1) * 3 * count + count - 1 + noteBytes > maxBytes || count + noteLines > maxLines)) + count-- + if (!count) return { body: clipText(OMITTED_CELLS, maxBytes, maxLines, "") ?? "", rows: 0, clipped: true } + let bytes = maxBytes - (names.length - 1) * 3 * count - (count - 1) - noteBytes + let lines = maxLines - count + 1 - noteLines + const cells: (string | undefined)[][] = Array.from({ length: count }, () => Array(names.length)) + const named = names.map((name, index) => (selected.has(name) ? index : -1)).filter((index) => index >= 0) + const ordinary = names.map((name, index) => (!selected.has(name) ? index : -1)).filter((index) => index >= 0) + let clipped = count < rows.length + let omitted = count < rows.length + for (const indices of [named, ordinary]) { + let remaining = count * indices.length + for (let row = 0; row < count; row++) { + for (const column of indices) { + if (bytes <= 0 || lines <= 0) { + omitted = clipped = true + continue + } + const priority = selected.has(names[column]!) + const value = rows[row]![column] + const cell = priority ? namedCell(value, bytes, lines) : displayCell(value) + const text = priority ? cell.text : clipCell(cell.text!, Math.floor(bytes / remaining), lines) + remaining-- clipped ||= cell.clipped || text !== cell.text + if (text === undefined) { + omitted = clipped = true + continue + } + cells[row]![column] = text bytes -= Buffer.byteLength(text, "utf8") lines -= lineCount(text) - 1 - remaining-- - return text - }) - .join(" | ") + // A clipped named cell consumes the remaining allowance; do not emit a + // continuation marker for every later cell when only a few bytes remain. + if (priority && cell.clipped) bytes = 0 + } + } + } + if (omitted && !reserveNote) return fullTextRows(rows, names, selected, maxBytes, maxLines, true) + const shown = cells.flatMap((row) => { + const last = row.findLastIndex((cell) => cell !== undefined) + return last < 0 + ? [] + : [ + row + .slice(0, last + 1) + .map((cell) => cell ?? "") + .join(" | "), + ] }) - return { body: rows.join("\n"), rows: rows.length, clipped } + return { + body: shown.join("\n") + (omitted ? (shown.length ? "\n" : "") + OMITTED_CELLS : ""), + rows: shown.length, + clipped, + } } // Preflight before encoding/stringifying, bounding nested values and binary expansion @@ -193,49 +278,85 @@ function checkSpillSize(value: unknown, budget: { left: number }, ancestors = ne if (budget.left < 0) throw new Error("result exceeds the SQL spill size limit") } -function spillValue(value: unknown): unknown { - if (typeof value === "bigint") return { $bigint: value.toString() } - if (value instanceof Date) return { $date: value.toISOString() } - if (value instanceof Uint8Array) return { $bytes: Buffer.from(value).toString("base64") } +/** Tagged JSON chunks are shared by the spill and bounded named-cell previews. */ +function* spillValue(value: unknown, ancestors = new Set()): Generator { + if (typeof value === "bigint") return yield* spillValue({ $bigint: value.toString() }, ancestors) + if (value instanceof Date) return yield* spillValue({ $date: value.toISOString() }, ancestors) + if (value instanceof Uint8Array) { + yield '{"$bytes":"' + // Multiples of three preserve base64 grouping without copying the entire BLOB. + for (let start = 0; start < value.byteLength; start += 768) { + yield Buffer.from(value.buffer, value.byteOffset + start, Math.min(768, value.byteLength - start)).toString( + "base64", + ) + } + yield '"}' + return + } if (typeof value === "number" && (!Number.isFinite(value) || Object.is(value, -0))) { - return { $sql_type: "number", value: Object.is(value, -0) ? "-0" : String(value) } + return yield* spillValue({ $sql_type: "number", value: Object.is(value, -0) ? "-0" : String(value) }, ancestors) + } + if (value === undefined) return yield* spillValue({ $sql_type: "undefined" }, ancestors) + if (typeof value === "string") { + yield '"' + for (let start = 0; start < value.length; ) { + let end = Math.min(start + 1024, value.length) + const last = value.charCodeAt(end - 1) + if (end < value.length && last >= 0xd800 && last <= 0xdbff) end-- + yield JSON.stringify(value.slice(start, end)).slice(1, -1) + start = end + } + yield '"' + return } - if (value === undefined) return { $sql_type: "undefined" } - if (Array.isArray(value)) return value.map(spillValue) if (value && typeof value === "object") { - return Object.fromEntries(Object.entries(value).map(([key, item]) => [key, spillValue(item)])) + if (ancestors.has(value) || ancestors.size >= 100) throw new Error("SQL result is cyclic or too deeply nested") + ancestors.add(value) + if (Array.isArray(value)) { + yield "[" + for (let i = 0; i < value.length; i++) { + if (i) yield "," + yield* spillValue(value[i], ancestors) + } + yield "]" + } else { + yield "{" + let first = true + for (const key in value) { + if (!Object.hasOwn(value, key)) continue + if (!first) yield "," + first = false + yield* spillValue(key, ancestors) + yield ":" + yield* spillValue((value as Record)[key], ancestors) + } + yield "}" + } + ancestors.delete(value) + return } - return value + yield JSON.stringify(value) ?? "null" } -/** - * Lossy JSONL text artifact, not a typed round-trip format: ordered columns/values - * preserve duplicate aliases, and special values use readable tags. Literal objects - * may resemble those tags; plain strings remain strings. No decoder is implied. - */ +/** Ordered columns/values retain duplicate aliases; tags preserve special values. */ function serializeRows(result: SqlExecuteResult): string { - const lines: string[] = [] + const chunks: string[] = [] let bytes = 0 for (const values of result.rows) { - // The service accepts one string. Bound the accumulated artifact and each row before - // encoding/JSON.stringify can allocate an arbitrarily large value. Its worst-case - // escaping expansion is six times this bounded raw size, never the entire result. const row = { columns: result.columns, values: result.columns.map((_, index) => (index in values ? values[index] : null)), } checkSpillSize(row, { left: SQL_MAX_SPILL_BYTES - bytes }) - let rawSize = 0 - const line = JSON.stringify(spillValue(row), (key, value) => { - rawSize += key.length + (typeof value === "string" ? value.length : 8) - if (rawSize > SQL_MAX_SPILL_BYTES - bytes) throw new Error("result exceeds the SQL spill size limit") - return value - }) - bytes += Buffer.byteLength(line, "utf8") + 1 - if (bytes > SQL_MAX_SPILL_BYTES) throw new Error("result exceeds the SQL spill size limit") - lines.push(line + "\n") + for (const chunk of spillValue(row)) { + bytes += Buffer.byteLength(chunk, "utf8") + if (bytes + 1 > SQL_MAX_SPILL_BYTES) throw new Error("result exceeds the SQL spill size limit") + chunks.push(chunk) + } + bytes++ + chunks.push("\n") } - return lines.join("") + return chunks.join("") } const EMPTY_RESULT_HINT = @@ -268,108 +389,135 @@ function isScalar(result: SqlExecuteResult): boolean { export async function formatSqlResult(result: SqlExecuteResult, options: FormatOptions) { const fullTextColumns = new Set(options.fullTextColumns ?? []) - const fullText = result.columns.some((column) => fullTextColumns.has(column)) + const fullText = fullTextColumns.size > 0 + const limit = fullText ? Math.min(options.limit, 20) : options.limit + const rows = result.rows.slice(0, limit) + const excess = result.rows.length > limit const missing = [...fullTextColumns].filter((column) => !result.columns.includes(column)) - const note = missing.length - ? "\n\n" + + const notes = [...(options.notes ?? [])] + if (missing.length) { + notes.push( `Note: Unknown full_text_columns: ${missing.join(", ")}. Available columns: ${result.columns.join(", ") || "(none)"}.`.replace( /[\r\n]/g, " ", - ) - : "" + ), + ) + } const maxBytes = Math.max(0, Math.min(SQL_MAX_PREVIEW_BYTES, options.maxBytes ?? SQL_MAX_PREVIEW_BYTES)) const maxLines = Math.max(0, options.maxLines ?? 2000) const fits = (text: string) => Buffer.byteLength(text, "utf8") <= maxBytes && lineCount(text) <= maxLines - // Requested text bypasses the preview cap; a missing-name note must not displace rows. - const bounded = (text: string) => - (fullText ? text : (clipText(text, maxBytes, maxLines, "…[output clipped]") ?? "")) + note - const footer = `\n\n(${result.row_count} rows)` + const note = notes.length ? "\n\n" + notes.join("\n\n") : "" + const footer = result.rows.length ? `\n\n(${result.row_count} rows)` : "(0 rows)" const metadata = { rowCount: result.row_count, rows_returned: result.rows.length, preview_rows: 0, preview_columns: 0, schema_truncated: false, - row_limited: result.truncated, + row_limited: result.truncated || excess, output_truncated: false, result_path: undefined as string | undefined, // This tool owns clipping; the generic truncator must not clip away the envelope. truncated: false, } + const bounded = (text: string) => { + if (fits(text)) return text + metadata.output_truncated = metadata.truncated = true + return clipText(text, maxBytes, maxLines, "…[output clipped]") ?? "" + } const hint = emptyResultHint(result, options.sql) const withHint = (text: string) => (hint && fits(text + "\n\n" + hint) ? text + "\n\n" + hint : text) - if (result.rows.length === 0 && !result.truncated) { - metadata.output_truncated = metadata.truncated = !fits("(0 rows)") - return { output: bounded(withHint("(0 rows)")), metadata } + if (!fullText && result.rows.length === 0 && !result.truncated) { + return { output: bounded(withHint(footer + note)), metadata } } - const schemaFor = (bytes: number, lines: number) => + const rowsFor = (bytes: number, lines: number, indices: number[]) => fullText - ? { - text: - result.columns.join(" | ") + - "\n" + - result.columns.map((column) => "-".repeat(Math.max(column.length, 4))).join("-+-") + - "\n", - columns: result.columns.length, - clipped: false, - } - : previewSchema(result.columns, bytes, lines) - const rowsFor = (bytes: number, lines: number, columns: number) => - fullText ? fullTextRows(result, fullTextColumns, bytes, lines) : previewRows(result.rows, bytes, lines, columns) - let schema = schemaFor(maxBytes - Buffer.byteLength(footer, "utf8"), maxLines - lineCount(footer) + 1) - let preview = rowsFor( - Math.max(0, maxBytes - Buffer.byteLength(schema.text + footer, "utf8")), - Math.max(0, maxLines - lineCount(schema.text + footer) + 1), - schema.columns, - ) - if (!preview.clipped && !schema.clipped && !result.truncated) { - metadata.preview_rows = preview.rows - metadata.preview_columns = schema.columns - return { output: bounded(withHint(schema.text + preview.body + footer)), metadata } + ? fullTextRows( + rows.map((row) => indices.map((index) => (index in row ? row[index] : null))), + indices.map((index) => result.columns[index]!), + fullTextColumns, + bytes, + lines, + ) + : previewRows(rows, bytes, lines, indices.length) + // Ordinary, complete results retain their compact presentation. Full-text mode + // always takes the spill path, even for empty results or unknown selected names. + if (!fullText) { + const schema = previewSchema( + result.columns, + maxBytes - Buffer.byteLength(footer + note), + maxLines - lineCount(footer + note) + 1, + ) + const preview = rowsFor( + Math.max(0, maxBytes - Buffer.byteLength(schema.text + footer + note)), + Math.max(0, maxLines - lineCount(schema.text + footer + note) + 1), + schema.indices, + ) + if ( + !preview.clipped && + !schema.clipped && + !metadata.row_limited && + fits(schema.text + preview.body + footer + note) + ) { + metadata.preview_rows = preview.rows + metadata.preview_columns = schema.columns + return { output: withHint(schema.text + preview.body + footer + note), metadata } + } } let spill: string try { metadata.result_path = await options.write(serializeRows(result)) - spill = `All ${result.rows.length} returned rows saved to ${metadata.result_path}. Lossy JSONL text: columns/values per row; $bigint, $date (ISO), $bytes (base64) tags; strings unchanged.` + spill = `All ${result.rows.length} returned rows saved to ${metadata.result_path}. JSONL text: columns/values per row; $bigint, $date (ISO), $bytes (base64) tags; strings unchanged. Use read with char_offset/max_chars to recover any clipped text.` } catch { spill = `Spill failed (write/serialization error or ${SQL_MAX_SPILL_BYTES / 1024 / 1024} MiB size limit); returned rows were not saved.` } - const envelope = (rows: number, clipped: boolean) => - `Preview: ${rows} of ${result.rows.length} returned rows shown. ` + - (result.truncated - ? `Result may be row-limited (requested row limit ${options.limit}; total unknown). More rows may exist. ` - : "Row-limited: no. ") + + const envelope = (shown: number, clipped: boolean) => + `Preview: ${shown} of ${result.rows.length} returned rows shown. ` + + (excess + ? `Note: connector returned ${result.rows.length} rows; showing at most ${limit} (requested row limit). ` + : result.truncated + ? `Result may be row-limited (requested row limit ${limit}; total unknown). More rows may exist. ` + : "Row-limited: no. ") + `Output truncated: ${clipped ? "yes" : "no"}. ${spill}` + - (clipped - ? fullText - ? " Named full_text_columns cells are complete; other cells may be clipped." - : " For complete text, rerun sql_execute with full_text_columns; omit limit to use the default." - : "") + + (clipped && !fullText ? " Prioritize specific cells with full_text_columns; omit limit to use the default." : "") + "\n" - // Ordinary previews include the schema, envelope and path in the same cap. - const essentials = envelope(SQL_MAX_PREVIEW_ROWS, true) + footer - if (!fullText && !fits(essentials)) { + // Notes share the output budget, including missing names and tool annotations. + // Keep room for the result contract even when a note is much larger than the cap. + const shownNote = + clipText(note, Math.floor(maxBytes / 4), Math.max(1, Math.floor(maxLines / 4)), "…[note clipped]") ?? "" + const notesClipped = shownNote !== note + const essentials = envelope(Math.min(rows.length, SQL_MAX_PREVIEW_ROWS), true) + footer + shownNote + if (!fits(essentials)) { metadata.schema_truncated = result.columns.length > 0 metadata.output_truncated = metadata.truncated = true - return { output: bounded(envelope(0, true)), metadata } + return { output: bounded(envelope(0, true) + footer + shownNote), metadata } } - schema = schemaFor(maxBytes - Buffer.byteLength(essentials, "utf8"), maxLines - lineCount(essentials) + 1) - const reserved = envelope(SQL_MAX_PREVIEW_ROWS, true) + schema.text + footer - preview = rowsFor( - Math.max(0, maxBytes - Buffer.byteLength(reserved, "utf8")), + const schema = rows.length + ? previewSchema( + result.columns, + maxBytes - Buffer.byteLength(essentials), + maxLines - lineCount(essentials) + 1, + fullText ? fullTextColumns : undefined, + ) + : { text: "", indices: [], columns: 0, clipped: false } + const reserved = envelope(Math.min(rows.length, SQL_MAX_PREVIEW_ROWS), true) + schema.text + footer + shownNote + const preview = rowsFor( + Math.max(0, maxBytes - Buffer.byteLength(reserved)), Math.max(0, maxLines - lineCount(reserved) + 1), - schema.columns, + schema.indices, ) metadata.preview_rows = preview.rows metadata.preview_columns = schema.columns metadata.schema_truncated = schema.clipped - metadata.output_truncated = preview.clipped || schema.clipped - metadata.truncated = result.truncated || metadata.output_truncated + metadata.output_truncated = preview.clipped || schema.clipped || notesClipped || excess + metadata.truncated = metadata.row_limited || metadata.output_truncated return { - output: bounded(envelope(preview.rows, metadata.output_truncated) + schema.text + preview.body + footer), + output: bounded( + withHint(envelope(preview.rows, metadata.output_truncated) + schema.text + preview.body + footer + shownNote), + ), metadata, } } diff --git a/packages/opencode/test/altimate/tools/sql-execute.test.ts b/packages/opencode/test/altimate/tools/sql-execute.test.ts index 7188283baf..92cd7252fc 100644 --- a/packages/opencode/test/altimate/tools/sql-execute.test.ts +++ b/packages/opencode/test/altimate/tools/sql-execute.test.ts @@ -10,6 +10,7 @@ import { SessionID, MessageID } from "@/session/schema" import * as Dispatcher from "@/altimate/native/dispatcher" import * as Registry from "@/altimate/native/connections/registry" import { SqlExecuteTool } from "@/altimate/tools/sql-execute" +import * as SqlResultLint from "@/altimate/tools/sql-result-lint" import { PostConnectSuggestions } from "@/altimate/tools/post-connect-suggestions" import * as Precedence from "@/altimate/workspace/precedence" import { initTool } from "../tool-fixture" @@ -103,7 +104,7 @@ describe("sql_execute parser read boundary", () => { const ask = spyOn(ctx, "ask") try { await tool.execute({ query: "VALUES (1)" }, ctx) - await tool.execute({ query: 'SELECT \'DELETE INTO\' AS "UPDATE"' }, ctx) + await tool.execute({ query: "SELECT 'DELETE INTO' AS \"UPDATE\"" }, ctx) expect(ask).not.toHaveBeenCalled() expect(dispatch).toHaveBeenCalledTimes(2) } finally { @@ -162,7 +163,9 @@ describe("sql_execute parser read boundary", () => { test("refuses multiple reads with a clear driver support message", async () => { const tool = await initTool(SqlExecuteTool) - await expect(tool.execute({ query: "SELECT 1; SELECT 2" }, ctx)).rejects.toThrow(/multi-statement reads.*not supported/i) + await expect(tool.execute({ query: "SELECT 1; SELECT 2" }, ctx)).rejects.toThrow( + /multi-statement reads.*not supported/i, + ) expect(dispatch).not.toHaveBeenCalled() expect(precedence).not.toHaveBeenCalled() }) @@ -347,7 +350,7 @@ describe("sql_execute full text selection", () => { expect(output.metadata.error).toBeUndefined() }) - test("returns id, title and untruncated body without changing the executed query", async () => { + test("prioritizes named text within the preview cap without changing the executed query", async () => { const text = "Complete body condition.\n".repeat(3000) dispatch.mockResolvedValue({ columns: ["id", "title", "body"], @@ -361,11 +364,11 @@ describe("sql_execute full text selection", () => { ctx, ) expect(output.output).toContain("id | title | body") - expect(output.output).toContain("17 | Policy title | ") - expect(output.output).toContain(text) - expect(output.output).not.toContain("cell clipped") - expect(output.metadata).toMatchObject({ preview_columns: 3, output_truncated: false, truncated: false }) - expect(output.metadata.result_path).toBeUndefined() + expect(output.output).toContain("Complete body condition.") + expect(output.output).toMatch(/continued in (?:the )?spill/i) + expect(Buffer.byteLength(output.output, "utf8")).toBeLessThanOrEqual(16 * 1024) + expect(output.metadata).toMatchObject({ preview_columns: 3, output_truncated: true, truncated: true }) + expect(output.metadata.result_path).toBeDefined() expect(dispatch).toHaveBeenCalledWith( "sql.execute", expect.objectContaining({ sql: "SELECT id, title, body FROM rules", limit: 1 }), @@ -391,9 +394,104 @@ describe("sql_execute full text selection", () => { expect(tool.description).toContain("[]") expect(tool.description).toContain("null") expect(tool.description).toContain("clamp") + expect(tool.description).toContain("priorit") + expect(tool.description).toContain("char_offset") + expect(schema.properties.full_text_columns.description).toContain("max_chars") + expect(tool.description).not.toContain("without preview or spill cuts") }) }) +const fullText = testEffect( + Layer.mergeAll( + Truncate.defaultLayer, + FSUtil.defaultLayer, + agents, + TestConfig.layer({ get: () => Effect.succeed({ tool_output: { max_bytes: 16 * 1024, max_lines: 2000 } }) }), + ), +) + +fullText.live("full text clips a 50 KiB named cell and retains its complete spill", () => + Effect.gen(function* () { + const body = "x".repeat(50 * 1024) + dispatch.mockResolvedValue({ columns: ["body"], rows: [[body]], row_count: 1, truncated: false } as never) + const tool = yield* SqlExecuteTool + const initialized = yield* tool.init() + const output = yield* initialized.execute({ query: "SELECT body FROM rules", full_text_columns: ["body"] }, ctx) + + expect(Buffer.byteLength(output.output, "utf8")).toBeLessThanOrEqual(16 * 1024) + expect(output.output).toMatch(/continued in (?:the )?spill/i) + expect(output.output).toContain("char_offset") + expect(output.output).toContain("max_chars") + expect(output.metadata).toMatchObject({ output_truncated: true, truncated: true }) + const resultPath = output.metadata.result_path as string + expect(path.isAbsolute(resultPath)).toBe(true) + const fs = yield* FSUtil.Service + const saved = yield* fs.readFileString(resultPath) + expect(JSON.parse(saved.trimEnd())).toEqual({ columns: ["body"], values: [body] }) + yield* fs.remove(resultPath) + }), +) + +fullText.live("full text saves unclipped named cells for later recovery", () => + Effect.gen(function* () { + const tool = yield* SqlExecuteTool + const initialized = yield* tool.init() + const output = yield* initialized.execute({ query: "SELECT 42", full_text_columns: ["answer"] }, ctx) + + expect(output.metadata).toMatchObject({ output_truncated: false, truncated: false }) + const resultPath = output.metadata.result_path as string + expect(path.isAbsolute(resultPath)).toBe(true) + expect(output.output).toContain(resultPath) + expect(output.output).toContain("char_offset") + const fs = yield* FSUtil.Service + expect(JSON.parse((yield* fs.readFileString(resultPath)).trimEnd())).toEqual({ columns: ["answer"], values: [42] }) + yield* fs.remove(resultPath) + }), +) + +fullText.live("full text limits a connector's 40 returned rows to 20 preview rows and saves all rows", () => + Effect.gen(function* () { + const rows = Array.from({ length: 40 }, (_, index) => [`entry-${index + 1}`]) + dispatch.mockResolvedValue({ columns: ["body"], rows, row_count: 40, truncated: false } as never) + const tool = yield* SqlExecuteTool + const initialized = yield* tool.init() + const output = yield* initialized.execute({ query: "SELECT body FROM rules", full_text_columns: ["body"] }, ctx) + + expect(dispatch).toHaveBeenCalledWith("sql.execute", expect.objectContaining({ limit: 20 })) + expect(output.metadata).toMatchObject({ rows_returned: 40, preview_rows: 20, row_limited: true, truncated: true }) + expect(output.output).toContain("entry-20") + expect(output.output).not.toContain("entry-21") + expect(output.output).toMatch(/(?:connector.*40.*20|40.*returned.*20|20.*40.*returned)/i) + const resultPath = output.metadata.result_path as string + const fs = yield* FSUtil.Service + expect((yield* fs.readFileString(resultPath)).trimEnd().split("\n")).toHaveLength(40) + yield* fs.remove(resultPath) + }), +) + +it.live("full text includes lint and suggestion notes in the configured output budget", () => + Effect.gen(function* () { + const lint = spyOn(SqlResultLint, "lintSqlResult").mockResolvedValue({ + output: "Checks:\n" + "Review this query. ".repeat(1000), + rules: ["limit_without_order_by"], + }) + suggestion.mockReturnValue("Tip: " + "Inspect the saved result. ".repeat(1000)) + try { + const tool = yield* SqlExecuteTool + const initialized = yield* tool.init() + const output = yield* initialized.execute({ query: "SELECT 42", full_text_columns: ["answer"], limit: 50 }, ctx) + + expect(Buffer.byteLength(output.output, "utf8")).toBeLessThanOrEqual(1024) + expect(output.metadata).toMatchObject({ output_truncated: true, truncated: true }) + expect(output.metadata.error).toBeUndefined() + const fs = yield* FSUtil.Service + yield* fs.remove(output.metadata.result_path as string) + } finally { + lint.mockRestore() + } + }), +) + test("full text selection retains write permission checks", async () => { const tool = await initTool(SqlExecuteTool) const ask = spyOn(ctx, "ask").mockImplementation(() => Effect.die(new Error("write denied"))) diff --git a/packages/opencode/test/altimate/tools/sql-result-format.test.ts b/packages/opencode/test/altimate/tools/sql-result-format.test.ts index 3c994f8536..9947daa6b6 100644 --- a/packages/opencode/test/altimate/tools/sql-result-format.test.ts +++ b/packages/opencode/test/altimate/tools/sql-result-format.test.ts @@ -293,7 +293,7 @@ describe("SQL result previews", () => { const formatted = await formatSqlResult(result(rows, columns, true), { limit: 1, write: saved.write }) expect(formatted.metadata.result_path).toBe(resultPath) - expect(formatted.output).toContain("Lossy JSONL text") + expect(formatted.output).toContain("JSONL text") expect(formatted.output).toContain("$bigint, $date (ISO), $bytes (base64) tags; strings unchanged") expect(saved.rows()).toEqual([ { @@ -443,44 +443,236 @@ describe("empty-result hint", () => { }) describe("SQL full text retrieval", () => { - test("retains every selected column and complete named text while clipping competing wide cells", async () => { + test("clips a 50 KiB named cell within 16 KiB and preserves its complete spill", async () => { const saved = spill() - const text = "Complete policy condition. ".repeat(2000) - const rows = [[text, "x".repeat(20000), "y".repeat(20000)]] - const formatted = await formatSqlResult(result(rows, ["policy", "detail", "notes"]), { - limit: 1, + const text = "x".repeat(50 * 1024) + const formatted = await formatSqlResult(result([[text]], ["policy"]), { + limit: 20, + maxBytes: 16 * 1024, fullTextColumns: ["policy"], write: saved.write, }) - expect(formatted.output).toContain(text) - expect(formatted.output).toContain("policy | detail | notes") + expect(Buffer.byteLength(formatted.output)).toBeLessThanOrEqual(16 * 1024) + expect(formatted.output).toContain("continued in spill") + expect(formatted.output).toContain(resultPath) + expect(formatted.output).toContain("char_offset") + expect(formatted.output).toContain("max_chars") + expect(formatted.metadata).toMatchObject({ result_path: resultPath, truncated: true, output_truncated: true }) + expect(saved.writes).toHaveLength(1) + expect(saved.rows()).toEqual([{ columns: ["policy"], values: [text] }]) + }) + + test("bounds twenty 1 MiB named cells and writes the complete spill once", async () => { + const saved = spill() + const rows = Array.from({ length: 20 }, (_, i) => [`row-${i}:` + "x".repeat(1024 * 1024)]) + const formatted = await formatSqlResult(result(rows), { + limit: 20, + fullTextColumns: ["value"], + write: saved.write, + }) + expect(Buffer.byteLength(formatted.output)).toBeLessThanOrEqual(SQL_MAX_PREVIEW_BYTES) + expect(formatted.output).toContain("row-0:") + expect(formatted.output).not.toContain("row-1:") + expect(formatted.output.match(/continued in spill/g)).toHaveLength(1) + expect(formatted.output.match(/remaining cells omitted/g)).toHaveLength(1) + expect(formatted.metadata).toMatchObject({ result_path: resultPath, truncated: true, output_truncated: true }) + expect(saved.writes).toHaveLength(1) + expect(saved.rows().map((row) => row.values)).toEqual(rows) + }) + + test("serializes structured and binary named cells losslessly inline and in the spill", async () => { + const saved = spill() + const iso = "2026-01-02T03:04:05.000Z" + const rows = [[{ a: 1, b: [1, 2] }, Buffer.from([0, 255]), [1, 2], { n: 42n, date: new Date(iso) }]] + const columns = ["object", "bytes", "array", "tagged"] + const formatted = await formatSqlResult(result(rows, columns), { + limit: 20, + fullTextColumns: columns, + write: saved.write, + }) + expect(formatted.output).toContain('{"a":1,"b":[1,2]} | {"$bytes":"AP8="} | [1,2]') + expect(formatted.output).toContain(`{"n":{"$bigint":"42"},"date":{"$date":"${iso}"}}`) + expect(formatted.metadata).toMatchObject({ result_path: resultPath, truncated: false, output_truncated: false }) + expect(saved.writes).toHaveLength(1) + expect(saved.rows()[0].values).toEqual([ + { a: 1, b: [1, 2] }, + { $bytes: "AP8=" }, + [1, 2], + { n: { $bigint: "42" }, date: { $date: iso } }, + ]) + }) + + test("spills complete small named text even when the preview is not clipped", async () => { + const saved = spill() + const formatted = await formatSqlResult(result([["complete"]]), { + limit: 20, + fullTextColumns: ["value"], + write: saved.write, + }) + expect(formatted.output).toContain("complete") + expect(formatted.output).toContain(resultPath) + expect(formatted.metadata).toMatchObject({ result_path: resultPath, truncated: false, output_truncated: false }) + expect(saved.writes).toHaveLength(1) + }) + + test("uses the last available line for a complete named cell", async () => { + const formatted = await formatSqlResult(result([["complete"]]), { + limit: 20, + maxLines: 6, + fullTextColumns: ["value"], + write: spill().write, + }) + expect(formatted.output).toContain("complete") + expect(formatted.output.split("\n").length).toBe(6) + expect(formatted.metadata.output_truncated).toBe(false) + }) + + test("bounds an empty missing-column note to 64 bytes and reports clipping", async () => { + const saved = spill() + const formatted = await formatSqlResult(result([], ["id", "title", "body"]), { + limit: 20, + maxBytes: 64, + fullTextColumns: ["missing"], + write: saved.write, + }) + expect(Buffer.byteLength(formatted.output)).toBeLessThanOrEqual(64) + expect(formatted.metadata).toMatchObject({ result_path: resultPath, truncated: true, output_truncated: true }) + expect(saved.writes).toEqual([""]) + }) + + test("keeps 300 long ordinary aliases bounded with one named column", async () => { + const saved = spill() + const columns = ["value", ...Array.from({ length: 300 }, (_, i) => `${i}_${"alias".repeat(100)}`)] + const formatted = await formatSqlResult(result([columns.map(() => "cell")], columns), { + limit: 20, + fullTextColumns: ["value"], + write: saved.write, + }) + expect(Buffer.byteLength(formatted.output)).toBeLessThanOrEqual(SQL_MAX_PREVIEW_BYTES) + expect(formatted.output).toContain("alias clipped") + expect(formatted.metadata.preview_columns).toBeLessThanOrEqual(SQL_MAX_PREVIEW_COLUMNS) + expect(formatted.metadata.schema_truncated).toBe(true) + expect(formatted.metadata.output_truncated).toBe(true) + expect(saved.rows()[0].columns).toEqual(columns) + }) + + test("prioritizes a named column beyond the ordinary schema column cap", async () => { + const saved = spill() + const columns = [...Array.from({ length: 300 }, (_, i) => `ordinary_${i}`), "named"] + const formatted = await formatSqlResult( + result([[...columns.slice(0, -1).map(() => "ordinary"), "priority text"]], columns), + { + limit: 20, + fullTextColumns: ["named"], + write: saved.write, + }, + ) + expect(formatted.output).toContain("priority text") + expect(formatted.output).toContain("named") + expect(formatted.metadata.preview_columns).toBeLessThanOrEqual(SQL_MAX_PREVIEW_COLUMNS) + expect(Buffer.byteLength(formatted.output)).toBeLessThanOrEqual(SQL_MAX_PREVIEW_BYTES) + }) + + test("renders absent named values as null while retaining explicit undefined", async () => { + const saved = spill() + const formatted = await formatSqlResult(result([[1], [2, undefined]], ["id", "named"]), { + limit: 20, + fullTextColumns: ["named"], + write: saved.write, + }) + expect(formatted.output).toContain('1 | null\n2 | {"$sql_type":"undefined"}') + expect(saved.rows().map((row) => row.values)).toEqual([ + [1, null], + [2, { $sql_type: "undefined" }], + ]) + }) + + test.each([20, 3])("enforces the effective %i-row bound when a connector returns 40 rows", async (limit) => { + const saved = spill() + const rows = Array.from({ length: 40 }, (_, i) => [`row-${String(i).padStart(2, "0")}`]) + const formatted = await formatSqlResult(result(rows), { + limit, + fullTextColumns: ["value"], + write: saved.write, + }) + expect(formatted.metadata).toMatchObject({ + preview_rows: limit, + rows_returned: 40, + row_limited: true, + truncated: true, + output_truncated: true, + }) + expect(formatted.output).toContain(`connector returned 40 rows`) + expect(formatted.output).toContain(`showing at most ${limit}`) + expect(formatted.output).not.toContain(`row-${String(limit).padStart(2, "0")}`) + expect(saved.rows().map((row) => row.values)).toEqual(rows) + }) + + test("gives named cells priority in row order before ordinary cells", async () => { + const saved = spill() + const rows = [ + ["x".repeat(20000), "first priority"], + ["y".repeat(20000), "second priority"], + ] + const formatted = await formatSqlResult(result(rows, ["ordinary", "named"]), { + limit: 20, + maxBytes: 1024, + fullTextColumns: ["named"], + write: saved.write, + }) + expect(formatted.output).toContain("first priority") + expect(formatted.output).toContain("second priority") expect(formatted.output).toContain("cell clipped") - expect(formatted.metadata.preview_columns).toBe(3) - expect(formatted.metadata.preview_rows).toBe(1) + expect(Buffer.byteLength(formatted.output)).toBeLessThanOrEqual(1024) expect(formatted.metadata.output_truncated).toBe(true) - expect(saved.rows()).toEqual([{ columns: ["policy", "detail", "notes"], values: rows[0] }]) }) - test("returns oversized named text inline without a preview or spill cut", async () => { + test("respects both byte and line limits for multiline named text", async () => { const saved = spill() - const text = "Full condition. ".repeat(2000) - const formatted = await formatSqlResult(result([[text]], ["policy"]), { - limit: 1, - fullTextColumns: ["policy"], + const text = "😀 complete condition.\n".repeat(2200) + const formatted = await formatSqlResult(result([[text]]), { + limit: 20, + maxBytes: 1024, + maxLines: 10, + fullTextColumns: ["value"], write: saved.write, }) - expect(formatted.output).toContain(text) - expect(formatted.output).not.toContain("cell clipped") - expect(formatted.metadata.preview_rows).toBe(1) - expect(formatted.metadata.output_truncated).toBe(false) - expect(formatted.metadata.result_path).toBeUndefined() - expect(saved.writes).toHaveLength(0) + expect(Buffer.byteLength(formatted.output)).toBeLessThanOrEqual(1024) + expect(formatted.output.split("\n").length).toBeLessThanOrEqual(10) + expect(formatted.output).toContain("continued in spill") + expect(formatted.output).not.toContain("\uFFFD") + expect(formatted.metadata.output_truncated).toBe(true) + expect(saved.rows()[0].values).toEqual([text]) + }) + + test("omits exhausted ordinary cells with a single bounded trailing note", async () => { + const saved = spill() + const columns = ["named", ...Array.from({ length: 50 }, (_, i) => `col${i}`)] + const formatted = await formatSqlResult( + result([["x".repeat(50000), ...columns.slice(1).map(() => "ordinary")]], columns), + { + limit: 20, + maxBytes: 4096, + fullTextColumns: ["named"], + write: saved.write, + }, + ) + expect(Buffer.byteLength(formatted.output)).toBeLessThanOrEqual(4096) + expect(formatted.output.match(/remaining cells omitted/g)).toHaveLength(1) + expect(formatted.output).not.toContain("…[cell clipped]") }) - test("truncation explains full text retrieval without requiring an explicit limit", async () => { - const formatted = await formatSqlResult(result([["x".repeat(20000)]]), { limit: 100, write: spill().write }) - expect(formatted.output).toContain("full_text_columns") - expect(formatted.output).not.toContain("limit <= 20") + test("notes unknown names and available columns within the bounded result", async () => { + const saved = spill() + const formatted = await formatSqlResult(result([[17, "A title", "Complete body"]], ["id", "title", "body"]), { + limit: 20, + fullTextColumns: ["missing"], + write: saved.write, + }) + expect(formatted.output).toContain("17 | A title | Complete body") + expect(formatted.output).toMatch(/Unknown full_text_columns: missing. Available columns: id, title, body/) + expect(formatted.metadata).toMatchObject({ result_path: resultPath, output_truncated: false }) + expect(saved.writes).toHaveLength(1) }) test.each([{ fullTextColumns: undefined }, { fullTextColumns: null }, { fullTextColumns: [] }])( @@ -500,45 +692,6 @@ describe("SQL full text retrieval", () => { }, ) - test.each([ - { rows: [[17, "A title", "Complete body"]], maxBytes: undefined }, - { rows: [], maxBytes: undefined }, - { rows: [[42]], maxBytes: 64 }, - ])("notes unknown names and available columns while returning the result %p", async ({ rows, maxBytes }) => { - const formatted = await formatSqlResult(result(rows.map((row) => [...row]), ["id", "title", "body"]), { - limit: 1, - fullTextColumns: ["missing"], - maxBytes, - write: spill().write, - }) - const note = formatted.output.split("\n").find((line) => line.includes("missing")) - expect(note).toMatch(/available columns.*id.*title.*body/i) - expect(formatted.output).toContain(rows.length ? (maxBytes ? "42" : "17 | A title | Complete body") : "(0 rows)") - expect(formatted.metadata.rows_returned).toBe(rows.length) - expect(formatted.metadata.output_truncated).toBe(false) - }) - - test("retains named text in later rows when an ordinary cell is clipped", async () => { - const saved = spill() - const first = "First complete body. ".repeat(1000) - const second = "Second complete body. ".repeat(1000) - const formatted = await formatSqlResult( - result( - [ - [1, "x".repeat(20000), first], - [2, "A title", second], - ], - ["id", "title", "body"], - ), - { limit: 2, fullTextColumns: ["body"], write: saved.write }, - ) - expect(formatted.output).toContain("id | title | body") - expect(formatted.output).toContain(first) - expect(formatted.output).toContain(second) - expect(formatted.metadata.preview_rows).toBe(2) - expect(saved.rows()).toHaveLength(2) - }) - test("pads absent spill values with null while retaining explicit undefined and column order", async () => { const saved = spill() await formatSqlResult(result([[1], [2, undefined], [3, null]], ["id", "grant_date"], true), { @@ -552,41 +705,3 @@ describe("SQL full text retrieval", () => { ]) }) }) - -test("full text preserves all selected columns beyond configured byte and line limits", async () => { - const saved = spill() - const text = "Complete condition.\n".repeat(2200) - const formatted = await formatSqlResult(result([[17, "A title", text]], ["id", "title", "body"]), { - limit: 1, - fullTextColumns: ["body"], - maxBytes: 1024, - maxLines: 10, - write: saved.write, - }) - expect(formatted.output).toContain("id | title | body") - expect(formatted.output).toContain(`17 | A title | ${text}`) - expect(formatted.output).not.toContain("clipped") - expect(formatted.metadata.preview_rows).toBe(1) - expect(formatted.metadata.preview_columns).toBe(3) - expect(formatted.metadata.output_truncated).toBe(false) - expect(formatted.metadata.truncated).toBe(false) - expect(saved.writes).toHaveLength(0) -}) - -test.each([[new Uint8Array(SQL_MAX_PREVIEW_BYTES * 2)], [Array(20000).fill("large")]])( - "full text returns complete arrays and binary values without internal clipping", - async (value) => { - const saved = spill() - const formatted = await formatSqlResult(result([[value]]), { - limit: 1, - fullTextColumns: ["value"], - write: saved.write, - }) - expect(formatted.metadata.preview_rows).toBe(1) - expect(formatted.output).toContain(String(value)) - expect(formatted.output).not.toContain("cell clipped") - expect(formatted.output).not.toContain("array clipped") - expect(formatted.metadata.output_truncated).toBe(false) - expect(saved.writes).toHaveLength(0) - }, -) From 1453824b2db5da952da1d0e369bd1c186572c8dc Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Mon, 12 Oct 2026 20:45:48 -0700 Subject: [PATCH 7/9] fix(analyst): bound spill memory per row and keep the routing notice under tight output limits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The full-text spill serialiser emitted one string fragment per JSON token and held them all until the join, so a permitted wide numeric result retained millions of fragments. It now emits one bounded string per row; the tagged serialisation and the 32 MiB preflight are unchanged. - Under a tiny `tool_output.max_lines` the fail-open routing notice was clipped before its text and then overwritten. The notice is reserved first and the result and remaining notes are fitted into what is left. Tests: 2,000 × 150 numeric rows spill with identical bytes and one retained chunk per row; a full-text query under a ten-line limit keeps the routing notice. Co-Authored-By: Claude Fable 5.1 --- .../src/altimate/tools/sql-execute.ts | 42 +++++++++++++++---- .../src/altimate/tools/sql-result-format.ts | 14 ++++--- .../test/altimate/tools/sql-execute.test.ts | 37 ++++++++++++++++ .../altimate/tools/sql-result-format.test.ts | 18 ++++++++ 4 files changed, 98 insertions(+), 13 deletions(-) diff --git a/packages/opencode/src/altimate/tools/sql-execute.ts b/packages/opencode/src/altimate/tools/sql-execute.ts index b7114e65ae..d67161e048 100644 --- a/packages/opencode/src/altimate/tools/sql-execute.ts +++ b/packages/opencode/src/altimate/tools/sql-execute.ts @@ -3,7 +3,7 @@ import { Effect } from "effect" import { EffectBridge } from "../../effect/bridge" import { Truncate } from "../../tool/truncate" import { legacyToInit, type LegacyToolDef } from "../tool-zod-compat" -import { formatSqlResult } from "./sql-result-format" +import { formatSqlResult, SQL_MAX_PREVIEW_BYTES } from "./sql-result-format" import { lintSqlResult } from "./sql-result-lint" import { Tool } from "../../tool/tool" import { Dispatcher } from "../native" @@ -220,6 +220,25 @@ export const SqlExecuteTool = Tool.define( // altimate_change end const limits = await bridge.promise(truncate.limits()) + const maxBytes = Math.min(limits.maxBytes, SQL_MAX_PREVIEW_BYTES) + // Reserve the routing explanation before the formatter budgets optional notes. + const routingNotice = fullText ? Precedence.annotate(precedence, { output: "" }).output : "" + let routingPrefix = "" + let routingBytes = 0 + let routingLines = 0 + for (const char of routingNotice) { + const size = Buffer.byteLength(char, "utf8") + if ( + routingBytes + size > maxBytes || + limits.maxLines < 1 || + (char === "\n" && routingLines + 1 >= limits.maxLines) + ) + break + routingPrefix += char + routingBytes += size + if (char === "\n") routingLines++ + } + const routingClipped = routingPrefix !== routingNotice const suggestion = PostConnectSuggestions.getProgressiveSuggestion("sql_execute") const lint = await lintSqlResult(args.query, args.warehouse, ctx.sessionID) const formatted = await formatSqlResult(result, { @@ -228,9 +247,14 @@ export const SqlExecuteTool = Tool.define( // The statement text lets the formatter add the empty/near-empty // result hint when a string-literal filter is present. sql: query, - notes: [precedence.notice ?? "", limitNote, lint?.output ?? "", suggestion ?? ""].filter(Boolean), - maxBytes: limits.maxBytes, - maxLines: limits.maxLines, + notes: [ + fullText ? "" : (precedence.notice ?? ""), + limitNote, + lint?.output ?? "", + suggestion ?? "", + ].filter(Boolean), + maxBytes: routingClipped ? 0 : maxBytes - routingBytes, + maxLines: limits.maxLines - routingLines, write: (text) => bridge.promise(truncate.write(text)), }) // altimate_change start — progressive disclosure suggestions @@ -242,13 +266,17 @@ export const SqlExecuteTool = Tool.define( }) } // altimate_change end - // Preserve routing metadata while the formatter owns the bounded notice text. + // The full-text routing prefix already has its own reserved output allowance. return { ...Precedence.annotate(precedence, { title: `SQL: ${args.query.slice(0, 60)}${args.query.length > 60 ? "..." : ""}`, - metadata: { ...formatted.metadata, ...(lint && { lint_rules: lint.rules }) }, + metadata: { + ...formatted.metadata, + ...(routingClipped && { output_truncated: true, truncated: true }), + ...(lint && { lint_rules: lint.rules }), + }, }), - output: formatted.output, + output: routingPrefix + formatted.output, } } catch (e) { const msg = e instanceof Error ? e.message : String(e) diff --git a/packages/opencode/src/altimate/tools/sql-result-format.ts b/packages/opencode/src/altimate/tools/sql-result-format.ts index 775ed6e5a8..bc5f33e403 100644 --- a/packages/opencode/src/altimate/tools/sql-result-format.ts +++ b/packages/opencode/src/altimate/tools/sql-result-format.ts @@ -339,8 +339,7 @@ function* spillValue(value: unknown, ancestors = new Set()): Generator void): void { let bytes = 0 for (const values of result.rows) { const row = { @@ -348,15 +347,16 @@ function serializeRows(result: SqlExecuteResult): string { values: result.columns.map((_, index) => (index in values ? values[index] : null)), } checkSpillSize(row, { left: SQL_MAX_SPILL_BYTES - bytes }) + let text = "" for (const chunk of spillValue(row)) { bytes += Buffer.byteLength(chunk, "utf8") if (bytes + 1 > SQL_MAX_SPILL_BYTES) throw new Error("result exceeds the SQL spill size limit") - chunks.push(chunk) + text += chunk } bytes++ - chunks.push("\n") + // Retain one bounded string per row instead of a reference per JSON token. + emit(text + "\n") } - return chunks.join("") } const EMPTY_RESULT_HINT = @@ -468,7 +468,9 @@ export async function formatSqlResult(result: SqlExecuteResult, options: FormatO let spill: string try { - metadata.result_path = await options.write(serializeRows(result)) + const chunks: string[] = [] + serializeRows(result, (chunk) => chunks.push(chunk)) + metadata.result_path = await options.write(chunks.join("")) spill = `All ${result.rows.length} returned rows saved to ${metadata.result_path}. JSONL text: columns/values per row; $bigint, $date (ISO), $bytes (base64) tags; strings unchanged. Use read with char_offset/max_chars to recover any clipped text.` } catch { spill = `Spill failed (write/serialization error or ${SQL_MAX_SPILL_BYTES / 1024 / 1024} MiB size limit); returned rows were not saved.` diff --git a/packages/opencode/test/altimate/tools/sql-execute.test.ts b/packages/opencode/test/altimate/tools/sql-execute.test.ts index 92cd7252fc..70791053fb 100644 --- a/packages/opencode/test/altimate/tools/sql-execute.test.ts +++ b/packages/opencode/test/altimate/tools/sql-execute.test.ts @@ -492,6 +492,43 @@ it.live("full text includes lint and suggestion notes in the configured output b }), ) +const shortFullText = testEffect( + Layer.mergeAll( + Truncate.defaultLayer, + FSUtil.defaultLayer, + agents, + TestConfig.layer({ get: () => Effect.succeed({ tool_output: { max_bytes: 1024, max_lines: 10 } }) }), + ), +) + +shortFullText.live("full text preserves the routing notice with a ten-line output limit", () => + Effect.gen(function* () { + const notice = "Not routed through the selected workspace: routing was unavailable, so the query ran locally." + precedence.mockResolvedValue({ notice, precedence: "undetermined" }) + suggestion.mockReturnValue("Tip: " + "Inspect the saved result. ".repeat(1000)) + const lint = spyOn(SqlResultLint, "lintSqlResult").mockResolvedValue({ + output: "Checks:\n" + "Review this query. ".repeat(1000), + rules: ["limit_without_order_by"], + }) + try { + const tool = yield* SqlExecuteTool + const initialized = yield* tool.init() + const output = yield* initialized.execute({ query: "SELECT 42", full_text_columns: ["answer"], limit: 50 }, ctx) + + expect(output.output).toStartWith(notice) + expect(output.output.split("\n").length).toBeLessThanOrEqual(10) + expect(Buffer.byteLength(output.output, "utf8")).toBeLessThanOrEqual(1024) + expect(output.metadata.precedence).toBe("undetermined") + expect(output.metadata.error).toBeUndefined() + expect(dispatch).toHaveBeenCalledTimes(1) + const fs = yield* FSUtil.Service + yield* fs.remove(output.metadata.result_path as string) + } finally { + lint.mockRestore() + } + }), +) + test("full text selection retains write permission checks", async () => { const tool = await initTool(SqlExecuteTool) const ask = spyOn(ctx, "ask").mockImplementation(() => Effect.die(new Error("write denied"))) diff --git a/packages/opencode/test/altimate/tools/sql-result-format.test.ts b/packages/opencode/test/altimate/tools/sql-result-format.test.ts index 9947daa6b6..4cc87222cd 100644 --- a/packages/opencode/test/altimate/tools/sql-result-format.test.ts +++ b/packages/opencode/test/altimate/tools/sql-result-format.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test } from "bun:test" import type { SqlExecuteResult } from "../../../src/altimate/native/types" import { formatSqlResult, + serializeRows, SQL_MAX_PREVIEW_BYTES, SQL_MAX_PREVIEW_COLUMNS, SQL_MAX_PREVIEW_ROWS, @@ -217,6 +218,23 @@ describe("SQL result previews", () => { expect(saved.rows()).toEqual([{ columns, values }]) }) + test("spills 2,000 numeric rows by 150 columns with identical bytes and one retained chunk per row", async () => { + const columns = Array.from({ length: 150 }, (_, i) => `column_${i}`) + const rows = Array.from({ length: 2_000 }, (_, row) => columns.map((_, column) => row * columns.length + column)) + const input = result(rows, columns) + const chunks: string[] = [] + serializeRows(input, (chunk) => chunks.push(chunk)) + + expect(chunks).toHaveLength(rows.length) + const expected = rows.map((values) => JSON.stringify({ columns, values }) + "\n").join("") + expect(chunks.join("")).toBe(expected) + + const saved = spill() + const formatted = await formatSqlResult(input, { limit: 2_000, write: saved.write }) + expect(formatted.metadata.result_path).toBe(resultPath) + expect(saved.writes).toEqual([expected]) + }) + test.each([0, 8, 64, 512, 1024])( "keeps wide schemas and a long spill envelope within a %i-byte cap", async (maxBytes) => { From b2973c2edf1251648fd444789f0db2fe5f3463ff Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Mon, 12 Oct 2026 20:58:19 -0700 Subject: [PATCH 8/9] fix(read): bound the character window to the byte budget before slicing A large `max_chars` on a very long single-line file sliced the whole requested window before the 50 KB byte limit applied, so a minified or generated file could allocate hundreds of megabytes although only about 50 KB can be returned. The slice end is now clamped to the byte budget (one character is at least one byte) before slicing; the UTF-8 refinement, surrogate-pair handling, clipping and continuation output are unchanged. Test: a 20 MB single-line file read with `max_chars: 10_000_000` returns the same clipped output and the slice requested from the string is bounded by the budget. Co-Authored-By: Claude Fable 5.1 --- packages/opencode/src/tool/read.ts | 7 +++-- packages/opencode/test/tool/read.test.ts | 39 ++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/packages/opencode/src/tool/read.ts b/packages/opencode/src/tool/read.ts index b4b0623dab..0a01fe5e1b 100644 --- a/packages/opencode/src/tool/read.ts +++ b/packages/opencode/src/tool/read.ts @@ -182,16 +182,17 @@ export const ReadTool = Tool.define< const splitsPair = (index: number) => index > 0 && /[\uD800-\uDBFF]/.test(text[index - 1]) && /[\uDC00-\uDFFF]/.test(text[index] ?? "") const windowStart = splitsPair(charOffset) ? charOffset - 1 : charOffset - let stop = Math.min(text.length, windowStart + maxChars) + const byteLimit = MAX_BYTES - 512 + let stop = Math.min(text.length, windowStart + maxChars, windowStart + byteLimit) if (splitsPair(stop)) stop += stop === windowStart + 1 ? 1 : -1 let chunk = text.slice(windowStart, stop) // Reserve room for the continuation message within the same byte cap. - if (Buffer.byteLength(chunk, "utf-8") > MAX_BYTES - 512) { + if (Buffer.byteLength(chunk, "utf-8") > byteLimit) { let bytes = 0 let chars = 0 for (const char of chunk) { const size = Buffer.byteLength(char, "utf-8") - if (bytes + size > MAX_BYTES - 512) break + if (bytes + size > byteLimit) break bytes += size chars += char.length } diff --git a/packages/opencode/test/tool/read.test.ts b/packages/opencode/test/tool/read.test.ts index d92338d600..dcd8ad76c6 100644 --- a/packages/opencode/test/tool/read.test.ts +++ b/packages/opencode/test/tool/read.test.ts @@ -1,5 +1,8 @@ import { PermissionV1 } from "@opencode-ai/core/v1/permission" import { afterEach, describe, expect } from "bun:test" +// altimate_change start — observe character-window slice bounds +import { spyOn } from "bun:test" +// altimate_change end import { Cause, Effect, Exit, Layer, Stream } from "effect" import path from "path" import { Agent } from "../../src/agent/agent" @@ -382,6 +385,42 @@ describe("tool.read truncation", () => { }), ) + it.instance("bounds slice length for a 20 MB single-line character window", () => + Effect.gen(function* () { + const test = yield* TestInstance + const filepath = path.join(test.directory, "long.txt") + const content = "0123456789".repeat(2 * 1024 * 1024) + const charOffset = 123 + const byteLimit = 50 * 1024 - 512 + yield* put(filepath, content) + + const slice = String.prototype.slice + const windows: number[] = [] + const spy = spyOn(String.prototype, "slice").mockImplementation(function (this: string, start, end) { + if (this.length === content.length && start === charOffset && end !== undefined) { + windows.push(end - start) + } + return slice.call(this, start, end) + }) + const result = yield* run({ + filePath: filepath, + limit: 1, + char_offset: charOffset, + max_chars: 10_000_000, + }).pipe(Effect.ensuring(Effect.sync(() => spy.mockRestore()))) + + const line = + content.slice(charOffset, charOffset + byteLimit) + + `... (line truncated to ${byteLimit} chars) (Use offset=1, limit=1, char_offset=${charOffset + byteLimit}, max_chars=${byteLimit} to continue this line.)` + expect(result.output).toBe( + `${full(filepath)}\nfile\n\n1: ${line}\n\n(End of file - total 1 lines)\n`, + ) + expect(result.metadata.truncated).toBe(true) + expect(windows.length).toBeGreaterThan(0) + for (const length of windows) expect(length).toBeLessThanOrEqual(byteLimit) + }), + ) + it.instance("bounds oversized multibyte character windows without losing the continuation", () => Effect.gen(function* () { const test = yield* TestInstance From 1bd447f1ffb4592f37889b2720e5f284af3f480a Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Tue, 13 Oct 2026 20:21:21 -0700 Subject: [PATCH 9/9] fix(analyst): prioritized full-text cells use the shared cell rendering - A `full_text_columns` value went into the preview verbatim, bypassing the cell rule the rest of the table uses, so a prioritized string containing a newline or ` | ` still split a row and the string `NULL` matched a real null. Prioritized string and null cells now go through `displayCell` before the existing clip, keeping the larger priority budget, continuation marker and spill unchanged; ordinary strings stay bare. - tests: delimiter-bearing named text on one row; exact `NULL` string vs real null in a named column; existing budget, priority-order, structured-cell and spill assertions retained. Co-Authored-By: Claude Fable 5.1 --- .../src/altimate/tools/sql-result-format.ts | 9 +++--- .../altimate/tools/sql-result-format.test.ts | 30 ++++++++++++++++++- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/packages/opencode/src/altimate/tools/sql-result-format.ts b/packages/opencode/src/altimate/tools/sql-result-format.ts index 9479e592dc..5a81109de2 100644 --- a/packages/opencode/src/altimate/tools/sql-result-format.ts +++ b/packages/opencode/src/altimate/tools/sql-result-format.ts @@ -193,10 +193,11 @@ const CONTINUED_CELL = "…[continued in spill]" const OMITTED_CELLS = "…[remaining cells omitted; see spill]" function namedCell(value: unknown, maxBytes: number, maxLines: number): { text?: string; clipped: boolean } { - // Strings stay readable; structured/binary values use the spill's tagged JSON. - if (typeof value === "string") { - const text = clipText(value, maxBytes, maxLines, CONTINUED_CELL) - return { text, clipped: text !== value } + // Strings and NULL use table rendering; structured/binary values use the spill's tagged JSON. + if (typeof value === "string" || value === null) { + const rendered = displayCell(value).text + const text = clipText(rendered, maxBytes, maxLines, CONTINUED_CELL) + return { text, clipped: text !== rendered } } let text = "" let bytes = 0 diff --git a/packages/opencode/test/altimate/tools/sql-result-format.test.ts b/packages/opencode/test/altimate/tools/sql-result-format.test.ts index 1edb8bcd19..ffb28a9ab5 100644 --- a/packages/opencode/test/altimate/tools/sql-result-format.test.ts +++ b/packages/opencode/test/altimate/tools/sql-result-format.test.ts @@ -651,6 +651,34 @@ describe("empty-result hint", () => { }) describe("SQL full text retrieval", () => { + test("quotes newline and column delimiters in named text on one row", async () => { + const saved = spill() + const text = "alpha\nbeta | gamma" + const formatted = await formatSqlResult(result([[text]], ["body"]), { + limit: 20, + fullTextColumns: ["body"], + write: saved.write, + }) + + expect(body(formatted.output, ["body"])).toBe('"alpha\\nbeta | gamma"') + expect(formatted.metadata).toMatchObject({ preview_rows: 1, output_truncated: false }) + expect(saved.rows()).toEqual([{ columns: ["body"], values: [text] }]) + }) + + test("distinguishes the exact NULL string from named null while leaving ordinary strings bare", async () => { + const saved = spill() + const rows = [["NULL"], [null], ["null"], ["true"], ["42"], ["plain"]] + const formatted = await formatSqlResult(result(rows, ["body"]), { + limit: 20, + fullTextColumns: ["body"], + write: saved.write, + }) + + expect(body(formatted.output, ["body"])).toBe('"NULL"\nNULL\nnull\ntrue\n42\nplain') + expect(formatted.metadata).toMatchObject({ preview_rows: rows.length, output_truncated: false }) + expect(saved.rows().map((row) => row.values)).toEqual(rows) + }) + test("clips a 50 KiB named cell within 16 KiB and preserves its complete spill", async () => { const saved = spill() const text = "x".repeat(50 * 1024) @@ -788,7 +816,7 @@ describe("SQL full text retrieval", () => { fullTextColumns: ["named"], write: saved.write, }) - expect(formatted.output).toContain('1 | null\n2 | {"$sql_type":"undefined"}') + expect(formatted.output).toContain('1 | NULL\n2 | {"$sql_type":"undefined"}') expect(saved.rows().map((row) => row.values)).toEqual([ [1, null], [2, { $sql_type: "undefined" }],