← Back to PRs

#19394: fix(agents): normalize tool call arguments dropped to {} (#19261)

by DevvGwardo open 2026-02-17 19:05 View on GitHub →
agents size: L
## Summary - Fix ALL tool calls with required parameters failing because `arguments` becomes `{}` on non-standard providers - Add a `StreamFn` wrapper that captures object-typed deltas and repairs empty arguments at stream end - Add diagnostic warning when empty params are passed to tools with required properties ## Root Cause pi-ai's `openai-completions.js` accumulates streaming tool arguments via string concatenation (`partialArgs += toolCall.function.arguments`). When a non-standard provider returns `function.arguments` as an **object** instead of a JSON string, this produces `"[object Object]"` which `parseStreamingJson()` can't parse, returning `{}`. AJV validation then throws "must have required property" for every tool with required params. **Key insight**: The original object value leaks through the `toolcall_delta` event's `delta` field — the normalizer captures it and repairs the corrupted arguments. ## Changes | File | Change | |------|--------| | `stream-tool-args-normalizer.ts` | New `StreamFn` wrapper — monitors `toolcall_delta` for object deltas, repairs `{}` at `toolcall_end`/`done` | | `extra-params.ts` | Wire normalizer as last wrapper in `applyExtraParamsToAgent` (universal safety net) | | `pi-tool-definition-adapter.ts` | `logWarn` when params are `{}` but tool has required properties | | `stream-tool-args-normalizer.test.ts` | 19 unit tests for `isEmptyObject`, `normalizeEvent`, full integration | | `pi-tool-definition-adapter.e2e.test.ts` | 4 tests for `splitToolExecuteArgs` (current + legacy formats) | | `extra-params.zai-tool-stream.test.ts` | Update mock to preserve `createAssistantMessageEventStream` export | ## Test plan - [x] `pnpm vitest run src/agents/pi-embedded-runner/stream-tool-args-normalizer.test.ts` — 19 pass - [x] `pnpm vitest run --config vitest.e2e.config.ts src/agents/pi-tool-definition-adapter.e2e.test.ts` — 6 pass - [x] `pnpm build` — clean - [x] `pnpm check` (format + typecheck + lint) — 0 errors - [x] `pnpm test` — 7246 tests pass, 0 failures - [x] Manual verification: kimi-cli and codex-cli tool calls work correctly Fixes #19261 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR fixes a streaming bug where non-standard OpenAI-compatible providers return `function.arguments` as an object instead of a JSON string, causing `pi-ai`'s string-concatenation accumulator to produce `"[object Object]"`, which then parses as `{}` and fails AJV validation. The fix adds a `StreamFn` wrapper (`createToolArgsNormalizerWrapper`) that intercepts `toolcall_delta` events, captures object-typed deltas by `contentIndex`, and repairs `{}` arguments at `toolcall_end` and `done` events. A diagnostic warning is added to `toToolDefinitions` as a last-resort safety net. The mock update in `extra-params.zai-tool-stream.test.ts` is necessary because the new normalizer imports `createAssistantMessageEventStream` from the mocked module. Key issues found: - **Wrong repository in issue URL**: Three places reference the `anthropics/claude-code` GitHub repository, but this is the `openclaw/openclaw` repo — all other issue references in the codebase use the correct repo. The user-visible warning message also embeds this incorrect URL. - **Bare `require()` in ESM fallback path** (`stream-tool-args-normalizer.ts:127`): Every other `require()` call in the project first creates a local require via `createRequire(import.meta.url)`. The fallback path would throw `ReferenceError: require is not defined` at runtime in this pure ESM module. Although the comment says this path "shouldn't happen in practice," it's still broken code. - **Event reconstruction drops unknown fields**: Both the `toolcall_end` and `done` event repairs reconstruct events with only the currently-known fields rather than spreading `{ ...event, ...repairs }`, which would silently drop any additional fields added in a future version of `@mariozechner/pi-ai`. <h3>Confidence Score: 3/5</h3> - The core fix is sound and the tests pass, but a broken fallback path (bare `require()` in ESM) and wrong issue URLs need correction before merging. - The primary fix logic is correct and well-tested with 19 unit tests. However, the fallback path in `createToolArgsNormalizerWrapper` uses a bare `require()` call that would fail at runtime in this ESM module (inconsistent with the rest of the codebase which uses `createRequire(import.meta.url)`). The issue URLs in both source comments and a user-visible warning message point to the wrong repository (`anthropics/claude-code` instead of `openclaw/openclaw`). The event reconstruction style issues are minor and forward-compatibility concerns only. - Pay close attention to `src/agents/pi-embedded-runner/stream-tool-args-normalizer.ts` (the bare `require()` fallback) and the issue URL references in `src/agents/pi-embedded-runner/extra-params.ts` and `src/agents/pi-tool-definition-adapter.ts`. <sub>Last reviewed commit: d18f50c</sub> <!-- greptile_other_comments_section --> <sub>(2/5) Greptile learns from your feedback when you react with thumbs up/down!</sub> <!-- /greptile_comment -->

Most Similar PRs