Skip to content

fix(chat): nil tool name in on_tool_output#3046

Merged
olimorris merged 1 commit intoolimorris:mainfrom
cairijun:fix/on_tool_output_tool_name
Apr 19, 2026
Merged

fix(chat): nil tool name in on_tool_output#3046
olimorris merged 1 commit intoolimorris:mainfrom
cairijun:fix/on_tool_output_tool_name

Conversation

@cairijun
Copy link
Copy Markdown
Contributor

Description

Fix the tool parameter in on_tool_output callback always being nil.

Root cause: In Chat:add_tool_output method, the callback args used tool_call.name, but tool_call is the raw LLM tool call object (i.e., tool.function_call) which doesn't have a top-level .name property.

Fix: Changed tool_call.name to tool.name, which is correctly set in both _handle_tool_error and _resolve_and_prepare_tool.

Files changed:

  • lua/codecompanion/interactions/chat/init.lua — single-line fix
  • tests/interactions/chat/tools/builtin/test_tool_output.lua — added test for callback parameters

AI Usage

Used CodeCompanion + GLM5 to help diagnose the issue and write the test case.

Related Issue(s)

N/A

Screenshots

N/A

Checklist

  • I've read the contributing guidelines and have adhered to them in this PR
  • I confirm that this PR has been majority created by me, and not AI (unless stated in the "AI Usage" section above)
  • I've run make all to ensure docs are generated, tests pass and StyLua has formatted the code
  • (optional) I've added test coverage for this fix/feature
  • (optional) I've updated the README and/or relevant docs pages

Copilot AI review requested due to automatic review settings April 19, 2026 05:50
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Fixes the on_tool_output callback tool argument being nil by passing the resolved tool’s name instead of the raw LLM tool-call object’s (missing) name.

Changes:

  • Update Chat:add_tool_output to pass tool.name into callback args.
  • Add a regression test asserting correct callback args and that callback mutations affect stored message content.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
lua/codecompanion/interactions/chat/init.lua Fix callback args to use tool.name so args.tool is not nil.
tests/interactions/chat/tools/builtin/test_tool_output.lua Add regression test ensuring on_tool_output receives correct args and can mutate output.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@olimorris
Copy link
Copy Markdown
Owner

Great spot and thanks for the fix. Let's move the test to test_chat as that's where the other callback tests are.

@cairijun cairijun force-pushed the fix/on_tool_output_tool_name branch from 16772d8 to d492d30 Compare April 19, 2026 09:33
@cairijun
Copy link
Copy Markdown
Contributor Author

Great spot and thanks for the fix. Let's move the test to test_chat as that's where the other callback tests are.

done

@olimorris olimorris merged commit e003fac into olimorris:main Apr 19, 2026
6 checks passed
@cairijun cairijun deleted the fix/on_tool_output_tool_name branch April 19, 2026 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants