Skip to content

fix(llm): fail closed on malformed native tool calls - #144

Open
canblmz1 wants to merge 1 commit into
AtomicBot-ai:mainfrom
canblmz1:fix/native-tool-call-integrity
Open

fix(llm): fail closed on malformed native tool calls#144
canblmz1 wants to merge 1 commit into
AtomicBot-ai:mainfrom
canblmz1:fix/native-tool-call-integrity

Conversation

@canblmz1

Copy link
Copy Markdown

Summary

Prevents native OpenAI-compatible tool calls from executing when their arguments are malformed or when a tool-call stream ends without a confirmed terminal signal.

Root cause

Two behaviors combined:

  • malformed non-empty tool arguments were silently parsed as {} after JSON.parse failed
  • a tool-call stream ending by bare EOF with neither finish_reason nor [DONE] was treated like a confirmed completion

That allowed malformed or unconfirmed tool arguments to reach the normal execution path.

Fix

  • malformed non-empty tool arguments now surface through the existing parse failure / repair path instead of becoming {}
  • the stream consumer records whether a terminal signal was actually observed
  • pending tool calls whose stream ends without a terminal signal route through the existing truncated/model-failure path before dispatch

Legitimate zero-argument calls remain supported.

A provider that sends an explicit finish_reason without [DONE] remains accepted, explicit finish_reason: "length" behavior is unchanged, and plain-text-only responses are unaffected.

Validation

Execution-level regression coverage using the real provider, step executor, ToolRegistry, and instrumented no-op tools:

  • healthy valid tool call -> executes once
  • malformed args + clean terminal -> 0 executions
  • malformed args + abrupt EOF -> 0
  • container-level truncation + EOF -> 0
  • syntactically complete args + ambiguous EOF -> 0
  • explicit finish_reason: "length" -> 0
  • parallel complete + truncated under ambiguous EOF -> 0 / 0
  • stream read error -> 0

Also verified:

  • explicit finish_reason without [DONE] remains accepted
  • plain-text bare-EOF behavior remains unchanged

Tests

  • npm run lint (typecheck): clean
  • npm run build: clean
  • Targeted suites (openai provider/stream-consumer/tool-call-adapter, step-executor, reliability, parallel-tool-calls integration, plus the new execution-integrity suite): 135/135 passed
  • Full suite: 3983 passed, 20 failed, 5 skipped (4008 total)
  • The same 20 failures reproduce identically on unmodified current main (same test names, same files) — confirmed by running the full suite on both in the same session. No new failure names introduced by this change.

Two behaviors combined to let a truncated or malformed native
OpenAI-compatible tool call reach real execution:

1. parseArguments() caught any JSON.parse failure on a tool call's
   arguments and silently returned {} instead of surfacing an error -
   a truncated argument string was indistinguishable from an
   intentional empty call.
2. A tool-call stream ending by bare EOF, with no provider
   finish_reason and no [DONE], was treated identically to a
   confirmed clean completion (finishReason -> null, stop -> true,
   truncated -> false). Since native tool calls with empty text
   content are intentionally allowed through when toolCalls exist,
   nothing stopped the unconfirmed call from reaching dispatch.

Fix:

- parseArguments() now throws on genuinely non-empty malformed JSON
  (or JSON that parses to something other than an object) instead of
  substituting {}. A legitimately empty/whitespace argument string
  still maps to {}. The failure surfaces through
  openAiToolCallsToBatch() as ToolCallArgumentsParseError, which
  reaches tryParseToolCalls()'s existing catch block and routes
  through the same one-shot repair path grammar-parsed batches
  already use - no new error subsystem. The raw arguments are never
  included in the error message, since they may carry sensitive data
  that reaches logs.
- The stream consumer now tracks whether a trustworthy terminal
  signal was actually observed (an explicit provider finish_reason on
  any chunk, or a parser-recognized [DONE]) before the stream ended.
  completionFromStreamFinal() folds an unconfirmed pending tool call
  into the existing truncated/stop computation, so it is caught by
  detectModelFailure's first check before parseArguments or
  validateBatch are ever reached. A provider that sends finish_reason
  without [DONE] remains accepted; explicit finish_reason: "length"
  is unchanged; plain-text-only responses are unaffected, since the
  fix only applies when a tool call is actually pending.

Verified with an execution-level regression suite driving the real
OpenAiProvider, step executor, and ToolRegistry with an instrumented
no-op tool: a malformed or unconfirmed call now executes zero times
in every case that previously executed once - malformed args under a
clean terminal, malformed args under a bare-EOF stream, container-
level truncation under a bare-EOF stream, syntactically complete args
under a bare-EOF stream, and the same case duplicated across parallel
calls. A healthy call still executes exactly once, explicit
finish_reason: "length" remains zero executions, and a stream read
error still propagates as a rejection rather than a silent no-op.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant