You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(providers): pass through a client tool named tool_call on kiro - #18823
The Kiro executor treated every toolUseEvent named tool_call as its internal MCP wrapper and required input.name / input.arguments. Agent runtimes that declare their own deferred-tool bridge under that name (OpenClaw tool_call {id, args}, Hermes Agent tool_call {calls: [...]}) had the model's call rejected with Invalid Kiro tool_call payload: missing nested MCP tool name at input.name, or kiro_tool_call_repair_retry_failed with the repair flag on.
execute() now checks the translated request for a client tool named tool_call (kiroRequestDeclaresToolCallWrapperName, which reads the current message's userInputMessageContext.tools, where openai-to-kiro always puts the tool list). When it is declared, transformEventStreamToSSE sends those events down the normal client-tool path, so the call reaches the client with its arguments unchanged. This skips both validation sites the second reporter pointed out: the buffering branch and flushPendingWrapperToolCalls.
Requests that do not declare tool_call are unchanged: wrapper validation and the one-shot repair still apply. The exported validateKiroToolUse gets the same optional clientDeclaresWrapperName switch so it gives the same answer as the stream path.
Base is the active release/v3.8.x branch (release/v3.8.53), not main
New feature? Not applicable: this is a bug fix
Validation
Change type: provider executor
Focused tests: the new test file plus every tests/unit/kiro-*.test.ts, executor-kiro, translator-openai-to-kiro and kiroThinking suite
eslint (with the repo suppressions file) and prettier --check on the changed files, plus npm run check:open-sse-typecheck
Reconciled with the current release/v3.8.53 tip
Production-code change includes a new automated test
Without the fix, the new test fails everywhere except the case that confirms the wrapper path still works:
✖ #18675 a client-declared tool_call streams through with the client's arguments
✖ #18675 string-fragment input for a client tool_call is concatenated unchanged
✔ #18675 without a client tool_call the wrapper validation still applies
✖ #18675 execute() detects a declared tool_call in the translated request
✖ #18675 kiroRequestDeclaresToolCallWrapperName reads the current message tool list
✖ #18675 validateKiroToolUse skips wrapper checks only when the client declared tool_call
ℹ tests 6
ℹ pass 1
ℹ fail 5
With the fix:
$ node --import tsx/esm --import ./open-sse/utils/setupPolyfill.ts --import ./tests/_setup/isolateDataDir.ts --test --test-force-exit tests/unit/kiro-client-tool-call-name-18675.test.ts
ℹ tests 6
ℹ pass 6
ℹ fail 0
# 53 files: tests/unit/kiro-*.test.ts, executor-kiro, translator-openai-to-kiro, kiroThinking
ℹ tests 391
ℹ pass 391
ℹ fail 0
$ npm run check:open-sse-typecheck
openSseTypecheckErrors=0
$ npm run check:file-size && npm run check:complexity && npm run check:test-discovery
OK / OK / OK
a client-declared tool_call with object input (the OpenClaw {id, args} shape) is emitted as a single tool_call tool call with the client's arguments and finish_reason: "tool_calls";
string-fragment input (the Hermes {calls: [...]} shape) is concatenated unchanged;
without the declaration, the same payload still fails with invalid_kiro_tool_call;
execute() with the repair flag on picks up the declaration from the translated body, sends no repair retry, and streams the call through;
unit cases for kiroRequestDeclaresToolCallWrapperName (declared, not declared, no tools, malformed) and validateKiroToolUse with and without the option.
Coverage Notes
Every changed line in open-sse/executors/kiro.ts and open-sse/executors/kiroToolCallValidation.ts is exercised by the new test file. The existing wrapper and repair suites (kiro-tool-call-validation, kiro-tool-call-repair) cover the unchanged default path.
Reviewer Notes
The fix decides once per request, from the declared tools, and does not guess from each event's input shape. The workaround in the issue comment (skip validation when the input has neither name nor arguments) would still break a client schema that happens to contain a name key, and it would also let genuinely malformed wrappers through on requests that never declared tool_call.
When a client declares tool_call, Kiro wrapper validation is off for that request. The name is ambiguous at that point anyway, and the client is the one that declared the tool, so its schema wins.
Edge case: when a request carries tool-call history but no tools, openai-to-kiro synthesizes a minimal schema from the tool names in history. If one of those names is tool_call, the request counts as declaring it and wrapper validation is skipped for that turn. A wrapper call that arrives malformed would then reach the client as-is instead of ending the stream with invalid_kiro_tool_call. I left this alone because the synthesized schema can't be told apart from a real client declaration, and passing the call through does no harm. Happy to narrow it if you'd prefer.
The Kiro executor treated every toolUseEvent named "tool_call" as its MCP
wrapper and required input.name / input.arguments. Agent runtimes that
declare their own deferred-tool bridge under that name (OpenClaw, Hermes
Agent) had every such call rejected with "missing nested MCP tool name".
execute() now checks whether the translated request declares a client tool
named tool_call and, if so, the stream transform handles those events as
ordinary tool calls with their arguments unchanged. Requests that do not
declare it keep the wrapper validation and the one-shot repair.
Closescheaper-inference#18675
Auto incremental reviews are disabled on this repository.
Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.
You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.
Use the checkbox below for a quick retry:
Trigger review
Summary
Summary by CodeRabbit
Bug Fixes
Kiro now correctly handles client-declared tool_call tools, passing their arguments through unchanged instead of rejecting them as malformed.
Wrapper validation continues to apply when the client has not declared a tool with that name.
Kiro now checks whether the request declares a client tool named tool_call. When it does, Kiro passes matching tool-use events through as client tool calls rather than treating them as MCP wrapper calls.
A request helper checks the current message’s tool list for a trimmed tool_call name. Wrapper input validation skips validation only when the client declares that name. Tests cover declared, absent, and malformed tool lists.
KiroExecutor.execute passes the declaration flag to both SSE transforms. The transform uses the direct tool-call path for a declared tool_call. Tests cover streamed arguments, repair retries, and undeclared wrapper validation. The changelog records the change.
Requests with no declared tools but a historical tool_call can pass a malformed wrapper to the client. This bounded edge case warrants a fix or explicit acceptance before merge.
Security architecture risk: Low · up to 26fcc
The change is narrowly scoped to tool-call interpretation. However, a tool name recovered from conversation history can disable wrapper checks even without an explicit current tool declaration. No new server-side tool execution authority was identified; consequences for client-side execution remain unverified.
Retained concerns
Low · architecture · observed: History-derived tool specifications also disable wrapper validation. When tools are omitted or empty, existing translation synthesizes specifications from historical assistant tool names. A historical tool_call therefore satisfies the new declaration predicate, allowing subsequent tool_call events to bypass wrapper validation and repair without an explicit current tool declaration. This changes the client handoff contract beyond the intended name-collision exception. Privileged downstream execution is not established.
Security review details
Security Blast Radius
inferred — The demonstrated effect is request-local interpretation and forwarding of Kiro tool events. A requester able to supply assistant history can satisfy the new predicate through a historical tool_call name. Broader asset access depends on downstream tool execution and authorization, which were not established.
Security Findings and Attack Paths
inferred — The supported control-change path is historical assistant tool name to synthesized current specification to wrapper-validation exemption to client-visible provider arguments. This establishes a provenance and contract gap, not a verified privileged-execution attack.
Trust Boundaries and Controls
observed — History synthesis predates this PR, but its use to disable wrapper validation is new. The detector checks only the resulting tool name and retains no distinction between an explicit client declaration and a specification reconstructed from assistant history.
Resilience and Maintainability Implications
observed — Repair does not replay a response after meaningful output. A second malformed pre-output attempt terminates with a repair error rather than entering another retry, limiting replay-related failure propagation.
Hardening Proposals
proposed — Preserve explicit-versus-synthesized declaration provenance and use explicit client declarations to select the wrapper exception, while retaining history synthesis for upstream compatibility.
Pre-merge checks: 5 · 1 · 1
Failed checks: 1 warning · 1 inconclusive
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…
Write docstrings for the functions missing them to satisfy the coverage threshold.
No Ai Attribution
The reviewed commits contain no AI or bot attribution. The only Co-authored-by trailer names the human contributor diegosouzapw. The supplied PR body is explicitly truncated, and the full body is no…
Provide the complete PR body or event payload, then check it for AI/bot Co-Authored-By trailers and Generated with <AI tool> footers.
Passed checks: 5 passed
Check name
Status
Explanation
Title check
The title uses the required Conventional Commits format: fix(providers): subject. It clearly describes the Kiro client tool_call pass-through change.
Description check
The description includes the required summary, related issues, target branch, validation results, tests, coverage notes, and reviewer notes. It provides specific implementation details and documents t…
Linked Issues check
The PR satisfies the coding requirement in #18675. kiroRequestDeclaresToolCallWrapperName detects tool_call in the translated request's current-message tool list. The stream path skips MCP-wrapper…
Out of Scope Changes check
The reported changes stay within #18675. They modify Kiro request detection, tool-call validation, stream transformation, repair-path behavior, and focused automated tests. The changelog entry documen…
Tests For Production Code
The PR changes production code under open-sse/ in kiro.ts and kiroToolCallValidation.ts. It adds tests/unit/kiro-client-tool-call-name-18675.test.ts with six focused tests covering the new beh…
Full details: Description check
Explanation
The description includes the required summary, related issues, target branch, validation results, tests, coverage notes, and reviewer notes. It provides specific implementation details and documents the known edge case.
Full details: Linked Issues check
Explanation
The PR satisfies the coding requirement in #18675. kiroRequestDeclaresToolCallWrapperName detects tool_call in the translated request's current-message tool list. The stream path skips MCP-wrapper validation when that client tool is declared. It preserves client arguments for object input and string fragments. Requests without the declaration retain wrapper validation. execute() passes the declaration state into the stream and repair paths. validateKiroToolUse exposes the matching option. The added tests cover both paths and request detection. #18303 is a related issue, not a directly linked target, and the PR does not need to implement its separate behavior.
Full details: Out of Scope Changes check
Explanation
The reported changes stay within #18675. They modify Kiro request detection, tool-call validation, stream transformation, repair-path behavior, and focused automated tests. The changelog entry documents the fix. No unrelated product behavior is identified. The noted synthesized-tool-list edge case remains part of the same declaration-detection behavior.
Full details: Tests For Production Code
Explanation
The PR changes production code under open-sse/ in kiro.ts and kiroToolCallValidation.ts. It adds tests/unit/kiro-client-tool-call-name-18675.test.ts with six focused tests covering the new behavior and retained validation paths.
Full details: Docstring Coverage
Explanation
Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 3 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Full details: No Ai Attribution
Explanation
The reviewed commits contain no AI or bot attribution. The only Co-authored-by trailer names the human contributor diegosouzapw. The supplied PR body is explicitly truncated, and the full body is not available in the checkout, so a prohibited footer in the omitted text cannot be ruled out.
Resolution
Provide the complete PR body or event payload, then check it for AI/bot Co-Authored-By trailers and Generated with <AI tool> footers.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.
… declaration
When a request omits `tools`, buildKiroPayload synthesizes tool specs from
history. A synthesized `tool_call` entry was read as a client declaration,
which skipped the MCP wrapper validation and the one-shot repair stream.
buildKiroPayload now records synthesized names in an in-memory
`_kiroSynthesizedToolNames` Map (dropped by the executor allowlist), and
kiroRequestDeclaresToolCallWrapperName ignores them. The executor reads the
pre-allowlist body so the marker is visible. Adds regression tests.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Summary
toolUseEventnamedtool_callas its internal MCP wrapper and requiredinput.name/input.arguments. Agent runtimes that declare their own deferred-tool bridge under that name (OpenClawtool_call {id, args}, Hermes Agenttool_call {calls: [...]}) had the model's call rejected withInvalid Kiro tool_call payload: missing nested MCP tool name at input.name, orkiro_tool_call_repair_retry_failedwith the repair flag on.execute()now checks the translated request for a client tool namedtool_call(kiroRequestDeclaresToolCallWrapperName, which reads the current message'suserInputMessageContext.tools, whereopenai-to-kiroalways puts the tool list). When it is declared,transformEventStreamToSSEsends those events down the normal client-tool path, so the call reaches the client with its arguments unchanged. This skips both validation sites the second reporter pointed out: the buffering branch andflushPendingWrapperToolCalls.tool_callare unchanged: wrapper validation and the one-shot repair still apply. The exportedvalidateKiroToolUsegets the same optionalclientDeclaresWrapperNameswitch so it gives the same answer as the stream path.Related Issues
Target Branch
release/v3.8.xbranch (release/v3.8.53), notmainValidation
tests/unit/kiro-*.test.ts,executor-kiro,translator-openai-to-kiroandkiroThinkingsuiteeslint(with the repo suppressions file) andprettier --checkon the changed files, plusnpm run check:open-sse-typecheckrelease/v3.8.53tipWithout the fix, the new test fails everywhere except the case that confirms the wrapper path still works:
With the fix:
Tests Added Or Updated
Changelog fragment:
changelog.d/fixes/18823-kiro-client-tool-call-name.md.tests/unit/kiro-client-tool-call-name-18675.test.ts(new):tool_callwith object input (the OpenClaw{id, args}shape) is emitted as a singletool_calltool call with the client's arguments andfinish_reason: "tool_calls";{calls: [...]}shape) is concatenated unchanged;invalid_kiro_tool_call;execute()with the repair flag on picks up the declaration from the translated body, sends no repair retry, and streams the call through;kiroRequestDeclaresToolCallWrapperName(declared, not declared, no tools, malformed) andvalidateKiroToolUsewith and without the option.Coverage Notes
open-sse/executors/kiro.tsandopen-sse/executors/kiroToolCallValidation.tsis exercised by the new test file. The existing wrapper and repair suites (kiro-tool-call-validation,kiro-tool-call-repair) cover the unchanged default path.Reviewer Notes
The fix decides once per request, from the declared tools, and does not guess from each event's input shape. The workaround in the issue comment (skip validation when the input has neither
namenorarguments) would still break a client schema that happens to contain anamekey, and it would also let genuinely malformed wrappers through on requests that never declaredtool_call.When a client declares
tool_call, Kiro wrapper validation is off for that request. The name is ambiguous at that point anyway, and the client is the one that declared the tool, so its schema wins.Edge case: when a request carries tool-call history but no
tools,openai-to-kirosynthesizes a minimal schema from the tool names in history. If one of those names istool_call, the request counts as declaring it and wrapper validation is skipped for that turn. A wrapper call that arrives malformed would then reach the client as-is instead of ending the stream withinvalid_kiro_tool_call. I left this alone because the synthesized schema can't be told apart from a real client declaration, and passing the call through does no harm. Happy to narrow it if you'd prefer.