fix(llm): fail closed on malformed native tool calls - #144
Open
canblmz1 wants to merge 1 commit into
Open
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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:
{}afterJSON.parsefailedfinish_reasonnor[DONE]was treated like a confirmed completionThat allowed malformed or unconfirmed tool arguments to reach the normal execution path.
Fix
{}Legitimate zero-argument calls remain supported.
A provider that sends an explicit
finish_reasonwithout[DONE]remains accepted, explicitfinish_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:
finish_reason: "length"-> 0Also verified:
finish_reasonwithout[DONE]remains acceptedTests
npm run lint(typecheck): cleannpm run build: cleanmain(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.