fix(tools): normalize optional fillers before validation

Merged
AN
2d ago
+625-63

Issue for this PR

Closes #1495

Type of change

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

What does this PR do?

Stacked on feat/analyst-harness-dab-2026-10 (#1441).

Why. Strict-schema model clients send every optional tool property, filling unused ones with "", [], 0 or null. These were validated as real values, so valid calls were rejected (zero row limit, empty arrays against minimum-length schemas, null) or behaved unsafely (a zero query timeout disables the deadline in the drivers).

Root cause. The shared tool wrapper, the legacy zod adapter and plugin tools all validated raw arguments with no optional-filler contract; the SQL tool schemas also accepted zero timeouts and rejected oversized caps.

Before -> after.

  • Top-level optional [], "", 0, null that make validation fail are treated as omitted and validation is retried (new fillerSafeParameters), for native, legacy and plugin tools. Required fields, nested payloads, explicit false and nonempty/positive values are unchanged.
  • sql_execute, sql_materialize, sql_check: filler timeouts keep the configured deadline instead of disabling it; positive overrides still work.
  • sql_execute.limit: 0 uses the default; oversized values clamp to 10,000 instead of failing.
  • sql_materialize byte budget: oversized values reach the existing hard cap instead of failing schema validation.
  • sql_check: coverage still requires an explicit pattern (empty pattern stays meaningful); published schema stays flat for strict providers.

Fallback, not unconditional stripping. Arguments are validated as sent first, so schema-valid intentional values (max_entries: 0, description: "", integration_ids: []) are preserved. Only a call the schema rejects is retried, dropping just the top-level optional "", [], 0, null values that are not valid on their own and keeping valid siblings. SQL tool query_timeout_ms now requires a positive value, so a filler 0 falls back to the configured deadline (an explicit 0 no longer disables the deadline through these tools).

Known limits / follow-ups (P2).

  • Plugin tools: the handler receives normalized arguments only when they validate (unlisted arguments are kept; otherwise raw arguments are passed, as before). In batch, plugin arguments are still decoded twice, which breaks input-changing transforms.
  • The tool.definition plugin hook output may not reach the published JSON schema because it is captured before the hook runs.
  • When several fillers are individually valid but invalid together, the retry drops all of them; the retry also runs the decoder once per filler candidate (cost only on the failure path).
  • Nested payload fillers are not rewritten (could alter intentional data).

How did you verify your code works?

  • New and updated tests: test/tool/filler-parameters.test.ts (registry-wide fillers, native/legacy/plugin execution, union discriminants), sql-execute, sql-materialize, sql-check. Each touched file passes in isolation (5, 47, 58, 63 tests; 0 fail); new test covers valid empty/zero values preserved and rejected fillers still accepted; test/tool/registry.test.ts 14 pass.
  • bun test test/tool test/altimate/tools plus the sql-materialize suites: 1399 pass, 4 fail, 2 errors in one combined run. Failures are 5s hook timeouts under load (all pass when rerun alone) and two pre-existing failures that also fail at the base commit (datamate-list-integrations, project-scan).
  • bun run typecheck passes; git diff --check clean.
  • Upstream markers: changed upstream-shared files src/tool/tool.ts and src/tool/registry.ts (present at the v1 baseline tag) have every added line inside altimate_change blocks, none nested.
  • Not run: live provider session.

Screenshots / recordings

N/A

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

๐Ÿค– Generated with Claude Code


[!NOTE]
Medium Risk
Changes shared tool argument decoding for all tools and alters SQL timeout/limit/cap semantics, which can affect agent tool calls and query execution boundaries.

Overview
Adds fillerSafeParameters so strict-schema clients can send every optional field as "", [], 0, or null without failing validation: arguments are decoded as sent first, and only on failure are top-level optional fillers stripped and decode retried. This is wired into native tools (Tool.wrap), legacy zod tools (tool-zod-compat), and plugin tools (fillerSafePlugin in the registry).

SQL tool behavior is tightened alongside that: filler query_timeout_ms no longer disables deadlines (positive values only at schema level; omit/0 โ†’ connection default). sql_execute treats limit: 0 like omitted/default, and clamps oversized limits to 10k instead of rejecting. sql_check validates with a discriminated union (coverage still requires pattern) but publishes a flat wire schema for strict providers. sql_materialize drops the JSON-schema max on max_bytes so huge values clamp at execution (tests updated).

New test/tool/filler-parameters.test.ts plus updates to sql-execute, sql-check, and sql-materialize tests cover registry-wide fillers and the new normalization paths.

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


Summary by cubic

Fixes valid tool calls being rejected by strict-schema model clients, which send every optional property filled with "", [], 0, or null. Arguments are validated as sent first; only when that fails are top-level optional fillers treated as omitted in a retry via the new fillerSafeParameters, so defaults and constraints behave as if the field were not sent. Intentional 0/""/[] values that validate are preserved, and valid fillers stay when an unrelated one is rejected. Closes #1495.

Behavior changes

  • Filler query timeouts keep the configured deadline; positive overrides still apply.
  • sql_execute limit 0 uses the default; values over 10,000 clamp instead of failing.
  • sql_materialize byte budgets over the 2 GiB cap clamp instead of failing validation.
  • sql_check coverage still requires an explicit pattern; the published JSON schema stays flat for strict providers.
  • Plugin tools pass normalized arguments only when they validate, keep unlisted arguments, and otherwise hand the handler the arguments as sent.