tools: dispatcher errors are returned as data, and most tools never check for them

Open
#1,210 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
68/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Active
Tech stack
typescript
Domain
tooling

Research direction

Start by reading the cited handlers in native/connections/register.ts and comparing their result shapes with the ten listed consumers: dbt-lineage.ts, impact-analysis.ts, project-scan.ts, schema-cache-status.ts, schema-detect-pii.ts, schema-index.ts, schema-search.ts, sql-autocomplete.ts, sql-diff.ts, and warehouse-list.ts. Use tools/response-normalization.ts as the existing reference. Done means each handler's failure convention is handled without rendering failures as valid empty data, with coverage for the agreed behavior.

Written by the indexing model from the issue text.

Description

What happens

The native dispatcher handlers do not throw. They catch every error and return it as a field on a result-shaped object:

// native/connections/register.ts:437 — sql.execute
return { columns: [], rows: [], row_count: 0, truncated: false, error: errorMsg } as SqlExecuteResult & { error: string }

schema.inspect (:562), sql.explain (:605) and dbt.profiles (:624) do the same. That shape is fine on its own — but it means every consuming tool is responsible for checking error, and most do not.

sql_execute was one of them, and it is fixed in #1204: an unresolvable warehouse rendered as the literal string (0 rows) — followed by a cheerful tip about query optimization — because formatResult() only looked at row_count. An agent asking what tables exist was told the warehouse was healthy and empty.

The rest of the class

These tools call Dispatcher.call(...) and never read error in any form:

  • dbt-lineage.ts
  • impact-analysis.ts
  • project-scan.ts
  • schema-cache-status.ts
  • schema-detect-pii.ts
  • schema-index.ts
  • schema-search.ts
  • sql-autocomplete.ts
  • sql-diff.ts
  • warehouse-list.ts

Not all of them are necessarily wrong — some call handlers that use a success: false convention instead — but each needs checking, because the failure mode is the worst kind: a failure that renders as valid, empty data. There is no fault string for a caller or a harness to key on, and an agent handed an empty result answers confidently from nothing.

schema_inspect, sql_analyze, lineage_check and altimate_core_column_lineage already do this correctly via normalizeError (tools/response-normalization.ts), so the house pattern exists and is cheap to apply.

Suggested fix
  1. Audit each tool above against the handler it calls; apply normalizeError(result.error) where the handler can return one.
  2. Consider a shared helper so a tool cannot forget — e.g. a wrapper around Dispatcher.call that throws when the result carries an error, with an opt-out for the handful of callers that genuinely want to inspect it.
  3. Add a lint or a test that fails when a tool calls the dispatcher and never consults error.
Why not fixed in #1204

#1204 fixes the specific path that produced the reported defect (sql_execute) plus the driver-level causes behind it. Sweeping ten more tools is a separate change with per-tool semantics to establish, and it should be reviewable on its own rather than buried in a warehouse-path fix.

Dominant language
TypeScript
Stars
813
Forks
134
Avg merge
2d 5h
Merged PRs (30d)
62

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from AltimateAI/altimate-code

All issues in AltimateAI/altimate-code

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.