db_query returns undefined content text for multi-statement SQL, breaking the session #59

Closed
opened 2026-08-20 11:52:59 +00:00 by david · 0 comments
Owner

Summary

db_query (and secondarily db_query_one) in the postgres extension mishandles node-postgres' multi-statement results. When the model runs SQL containing multiple statements, pg resolves with an array of Result objects; the tool then returns { content: [{ type: "text", text: undefined }] }. That malformed block is persisted into session history and makes every subsequent LLM call throw Error: Cannot read properties of undefined (reading 'length'), so the session appears hung until it is restarted.

Background

  • Extension: extensions/postgres/index.ts — registers db_query, db_query_one, db_list_tables, db_table_schema.
  • node-postgres (pg@8.x) executes multi-statement strings via the simple query protocol and resolves with an array of Result objects instead of a single result. See _checkForMultirow() in node_modules/pg/lib/query.js (converts this._results into an array) and callback(null, this._results) on end.
  • Reproduced locally against a local postgres:
    • pool.query("SELECT 1 AS a") → single Result, rows = [{"a":1}]
    • pool.query("SELECT 1 AS a; SELECT 2 AS b")array of two Results; .rows is undefined; JSON.stringify(result.rows) → JS undefined (not a string)

Failure chain:

  1. db_query.execute does JSON.stringify(result.rows, null, 2) — for an array result this yields JS undefined.
  2. Tool returns { content: [{ type: "text", text: undefined }], details: { rowCount: undefined } }.
  3. pi persists the toolResult message into session history as-is (convertToLlm passes toolResult messages through untouched).
  4. On every subsequent LLM call, pi-ai's buildBaseOptionsclampMaxTokensToContextestimateMessageTokens reads block.text.length unguarded (pi-ai utils/estimate.js) and throws Cannot read properties of undefined (reading 'length').
  5. The throw happens before the request is sent, so every subsequent prompt fails immediately with that error — the session looks hung/broken until a new one is started.

Secondary: db_query_one does result.rows.length === 0, which throws the same error message for array results; pi's tool runner catches it and returns it as a recoverable tool error, so it doesn't poison the session — but it still fails on legitimate multi-statement input.

Implementation Details

File: extensions/postgres/index.ts

  • Add a small helper that normalizes pg query results to an array of rows:
    function collectRows(result: unknown): Record<string, unknown>[] {
      const results = Array.isArray(result) ? result : [result];
      return results.flatMap((r) => (Array.isArray(r?.rows) ? r.rows : []));
    }
    
  • db_query: use collectRows(result) for the JSON payload; derive details.rowCount safely (e.g. sum of per-result rowCounts, omitting undefineds). Ensure text is always a string — JSON.stringify(array) is always a string once normalized.
  • db_query_one: flatten rows across all results and return the first row (or "(no rows returned)" if none); never read .length on an unnormalized result.
  • Add regression tests that exercise the normalization logic with pg's array-of-results shape — no live database required: extract the helper so it can be unit-tested directly, and/or mock pg.Pool.prototype.query.

Acceptance Criteria

  • db_query with multi-statement SQL returns all rows (flattened across statements) as a JSON string; content text is never undefined.
  • db_query_one with multi-statement SQL returns the first row or "(no rows returned)" instead of throwing.
  • Single-statement behavior unchanged for SELECT, INSERT/UPDATE/DELETE (empty rows array), and DDL.
  • Regression test(s) covering pg's array-of-results shape pass under bun test.
  • Existing tests continue to pass (bun test in extensions/postgres).

Test Plan

  1. Unit: cd extensions/postgres && bun test — new regression tests simulate pool.query resolving with [Result, Result] and assert the tool returns a string text payload.
  2. Manual (optional, requires local postgres): load the extension against a scratch DB and run via pi:
    • db_query with CREATE TEMP TABLE t(x int); INSERT INTO t VALUES (1) → no error, session stays usable.
    • A follow-up prompt in the same session still works (no "Cannot read properties of undefined" on subsequent LLM calls).

Notes / Out of scope

  • pi-ai's unguarded block.text.length in its token estimation is an upstream robustness gap (any extension returning text: undefined breaks a session this way); consider filing separately against the pi project. This issue only fixes the postgres extension side.
## Summary `db_query` (and secondarily `db_query_one`) in the postgres extension mishandles node-postgres' multi-statement results. When the model runs SQL containing multiple statements, pg resolves with an **array** of Result objects; the tool then returns `{ content: [{ type: "text", text: undefined }] }`. That malformed block is persisted into session history and makes every subsequent LLM call throw `Error: Cannot read properties of undefined (reading 'length')`, so the session appears hung until it is restarted. ## Background - Extension: `extensions/postgres/index.ts` — registers `db_query`, `db_query_one`, `db_list_tables`, `db_table_schema`. - node-postgres (`pg@8.x`) executes multi-statement strings via the simple query protocol and resolves with an **array of Result objects** instead of a single result. See `_checkForMultirow()` in `node_modules/pg/lib/query.js` (converts `this._results` into an array) and `callback(null, this._results)` on end. - Reproduced locally against a local postgres: - `pool.query("SELECT 1 AS a")` → single Result, `rows = [{"a":1}]` - `pool.query("SELECT 1 AS a; SELECT 2 AS b")` → **array** of two Results; `.rows` is `undefined`; `JSON.stringify(result.rows)` → JS `undefined` (not a string) Failure chain: 1. `db_query.execute` does `JSON.stringify(result.rows, null, 2)` — for an array result this yields JS `undefined`. 2. Tool returns `{ content: [{ type: "text", text: undefined }], details: { rowCount: undefined } }`. 3. pi persists the toolResult message into session history as-is (`convertToLlm` passes toolResult messages through untouched). 4. On every subsequent LLM call, pi-ai's `buildBaseOptions` → `clampMaxTokensToContext` → `estimateMessageTokens` reads `block.text.length` unguarded (pi-ai `utils/estimate.js`) and throws `Cannot read properties of undefined (reading 'length')`. 5. The throw happens before the request is sent, so every subsequent prompt fails immediately with that error — the session looks hung/broken until a new one is started. Secondary: `db_query_one` does `result.rows.length === 0`, which throws the same error message for array results; pi's tool runner catches it and returns it as a recoverable tool error, so it doesn't poison the session — but it still fails on legitimate multi-statement input. ## Implementation Details File: `extensions/postgres/index.ts` - Add a small helper that normalizes pg query results to an array of rows: ```ts function collectRows(result: unknown): Record<string, unknown>[] { const results = Array.isArray(result) ? result : [result]; return results.flatMap((r) => (Array.isArray(r?.rows) ? r.rows : [])); } ``` - `db_query`: use `collectRows(result)` for the JSON payload; derive `details.rowCount` safely (e.g. sum of per-result rowCounts, omitting undefineds). Ensure `text` is always a string — `JSON.stringify(array)` is always a string once normalized. - `db_query_one`: flatten rows across all results and return the first row (or "(no rows returned)" if none); never read `.length` on an unnormalized result. - Add regression tests that exercise the normalization logic with pg's array-of-results shape — no live database required: extract the helper so it can be unit-tested directly, and/or mock `pg.Pool.prototype.query`. ## Acceptance Criteria - [ ] `db_query` with multi-statement SQL returns all rows (flattened across statements) as a JSON string; content text is never `undefined`. - [ ] `db_query_one` with multi-statement SQL returns the first row or "(no rows returned)" instead of throwing. - [ ] Single-statement behavior unchanged for SELECT, INSERT/UPDATE/DELETE (empty rows array), and DDL. - [ ] Regression test(s) covering pg's array-of-results shape pass under `bun test`. - [ ] Existing tests continue to pass (`bun test` in `extensions/postgres`). ## Test Plan 1. Unit: `cd extensions/postgres && bun test` — new regression tests simulate `pool.query` resolving with `[Result, Result]` and assert the tool returns a string text payload. 2. Manual (optional, requires local postgres): load the extension against a scratch DB and run via pi: - `db_query` with `CREATE TEMP TABLE t(x int); INSERT INTO t VALUES (1)` → no error, session stays usable. - A follow-up prompt in the same session still works (no "Cannot read properties of undefined" on subsequent LLM calls). ## Notes / Out of scope - pi-ai's unguarded `block.text.length` in its token estimation is an upstream robustness gap (any extension returning `text: undefined` breaks a session this way); consider filing separately against the pi project. This issue only fixes the postgres extension side.
david closed this issue 2026-08-20 17:45:04 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
david/pi-extensions-and-skills#59
No description provided.