fix(gateway): coerce streaming tool-call argument deltas to object in client tools#57061
Conversation
Greptile SummaryThis PR fixes a bug where client (hosted) tools receive empty parameters
Confidence Score: 5/5Safe to merge — targeted bug fix with no regressions on the existing code path and comprehensive unit tests. The change is minimal, well-tested, and correctly scoped. The No files require special attention.
|
| Filename | Overview |
|---|---|
| src/agents/pi-tool-definition-adapter.ts | Adds coerceParamsRecord helper and replaces the inline isPlainObject ternary with it — correct and well-documented fix. |
| src/agents/pi-tool-definition-adapter.test.ts | Adds nine-case test suite covering all coercion edge cases for toClientToolDefinitions. |
Reviews (1): Last reviewed commit: "fix(gateway): coerce streaming tool-call..." | Re-trigger Greptile
|
I've thoroughly reviewed the implementation and added a comprehensive test suite covering all edge cases for stringified parameter parsing — valid JSON strings, invalid JSON, empty strings, arrays, primitives, null, undefined, and nested objects. The new Awaiting review, thanks. |
2f21f39 to
937d61e
Compare
obviyus
left a comment
There was a problem hiding this comment.
Reviewed latest changes; landing now.
Summary
{}instead of the actual arguments. This happens insrc/agents/pi-tool-definition-adapter.tsaround line 201.toClientToolDefinitionsfunction intercepts tool execution to delegate it to the client. It usesisPlainObject(adjustedParams)to ensure the parameters are a valid object before passing them to theonClientToolCallcallback. However, when arguments are streamed,adjustedParamscan be an accumulated JSON string rather than a parsed object. SinceisPlainObject()strictly returnsfalsefor strings, the valid JSON string is silently coerced into an empty object{}.coerceParamsRecordhelper function that first checksisPlainObject. If the value is a string, it attempts to parse it viaJSON.parse(). If the parsed result is a plain object, it returns that object; otherwise, it safely falls back to{}. This ensures that stringified JSON arguments from streaming providers are correctly deserialized before being passed to the client tool handler, without breaking existing object-based flows. Added comprehensive unit tests to verify the coercion logic.src/agents/pi-tool-definition-adapter.ts: AddedcoerceParamsRecordhelper function.src/agents/pi-tool-definition-adapter.ts: ReplacedisPlainObject(adjustedParams) ? adjustedParams : {}withcoerceParamsRecord(adjustedParams)intoClientToolDefinitions.src/agents/pi-tool-definition-adapter.test.ts: Added a new test suitetoClientToolDefinitions – param coercionto cover all edge cases of stringified parameter parsing.toToolDefinitionsremains untouched, as it passes raw parameters directly to the underlying tool execution which handles its own parsing. The coreisPlainObjectutility behavior is also unchanged to prevent global side effects.Reproduction
{}.Risk / Mitigation
JSON.parseattempt might throw an error if the accumulated string is malformed or incomplete, potentially crashing the tool execution flow.JSON.parsecall is wrapped in atry/catchblock. If parsing fails, it safely falls through to returning an empty object{}, which exactly matches the previous fallback behavior. The newly added unit tests verify this safe fallback behavior across various invalid inputs (empty string, malformed JSON, arrays, primitives).Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Fixes #57009