fix(cli): surface real tool name when tool call repair fails - #13446
Conversation
When the model emits a malformed tool call that cannot be repaired by case-insensitive matching, experimental_repairToolCall renamed the call to the hidden "invalid" tool. That tool is excluded from activeTools, so the AI SDK rejected it with a confusing "unavailable tool 'invalid'" error that did not tell the model what it did wrong. Return null instead so the original NoSuchToolError or InvalidToolInputError surfaces with the actual tool name and the list of available tools, letting the model self-correct. Fixes Kilo-Org#6905 Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
The experimental_repairToolCall fallback that produced tool calls named "invalid" was replaced with null in this PR, so nothing generates "invalid" tool calls anymore. Remove the now-orphaned InvalidTool definition, its registry registration, and its TUI renderer, along with the parameters test and snapshot that covered it. Agent-Signature: kilo-auto/efficient-remove-orphaned-invalid-tool on behalf of maphew
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6176d61f3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| import { WebFetchTool } from "./webfetch" | ||
| import { WriteTool } from "./write" | ||
| import { InvalidTool } from "./invalid" | ||
| import { SkillTool } from "./skill" |
There was a problem hiding this comment.
Returning null from the repair callback is sufficient to correct the reported behavior, but this hunk also begins an unnecessary cleanup that deletes the upstream invalid tool and modifies its registry, CLI renderer, and schema tests across five shared files. Retaining that now-unused definition avoids expanding Kilo's upstream delta and the resulting merge-conflict surface; the repository explicitly requires shared-file changes to be minimal and prohibits unnecessary upstream refactoring.
AGENTS.md reference: AGENTS.md:L185-L190
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed: the invalid-tool cleanup was reverted in 4c2b02e (Revert "refactor(tool): remove orphaned invalid tool"), restoring packages/opencode/src/tool/invalid.ts, the registry registration, the TUI renderer, and the schema test/snapshot to their upstream state. The branch now touches only the repair callback in session/llm.ts plus its regression test and changeset — the upstream diff is minimal again.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (3 files)
Previous Review Summary (commit f6176d6)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit f6176d6)Status: No Issues Found | Recommendation: Merge Files Reviewed (8 files)
Reviewed by grok-4.6 · Input: 132.4K · Output: 7.7K · Cached: 267.3K Review guidance: REVIEW.md from base branch |
This reverts commit f6176d6. Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
…-name Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
…tool-surfaces-real-name Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
…shard) Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
|
Merge-readiness check complete:
Ready for human review. |
Co-authored-by: kiloconnect[bot] <240665456+kiloconnect[bot]@users.noreply.github.com>
|
Thanks @maphew for the contribution. Was able to reproduce the fix and it's now merged. |
2 participants
What
When a model emits a tool call that cannot be repaired by case-insensitive matching,
experimental_repairToolCallrenamed the call to the hiddeninvalidtool. That tool is deliberately excluded fromactiveTools, so the AI SDK rejected the repaired call with a confusing, self-inflicted error:invalidis not in the available list, and the message does not say which tool the model actually tried to call, so the model cannot self-correct.Why
Users hit this repeatedly (#6905). The
invalidfallback never executes in practice:activeToolsfilters it out of the execution tool set, so the repair path produces aNoSuchToolErrorforinvalidinstead of the intended "invalid arguments" feedback. The same confusion also contributes to retry loops like #11844, where a model cannot learn from the error.Change
The repair function now returns
nullwhen it cannot map the tool call to a real tool. The AI SDK then surfaces the original error with the actual tool name and the available tool list:The model now sees exactly what it emitted and which tools exist, so it can repair the call itself (case mismatch, wrong tool, or malformed arguments all report truthfully).
Verification
Writetool and asserts the tool-error event namesWriteand does not mentioninvalid. The test fails against the old code with the exact reported message and passes with the fix.bun test ./test/session/llm.test.ts— 31 pass, 0 fail.bun run typecheckinpackages/opencode— clean.bun run script/check-opencode-annotations.ts --worktree— clean.Related