fix(hooks): apply ModifiedInput from pre-tool hooks to tool call arguments#1950
Merged
dgageot merged 1 commit intodocker:mainfrom Mar 6, 2026
Merged
Conversation
…ments The pre-tool hook system already supported returning updated_input via hook_specific_output, and the executor populated Result.ModifiedInput. However, the runtime never applied it back to toolCall.Function.Arguments before execution, making input-rewriting hooks (like RTK) ineffective. Marshal ModifiedInput to JSON and overwrite the tool call arguments when a pre-tool hook provides modified input. Assisted-By: cagent
aheritier
force-pushed
the
feat/rtk-hook-modified-input
branch
from
1a1ab28 to
cae93d4
Compare
aheritier
changed the title
Contributor
Author
Contributor
Author
|
/review |
![docker-agent[bot]](https://avatars.githubusercontent.com/in/2787757?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Contributor
Author
|
(not sure if it has some interest) |
There was a problem hiding this comment.
Review Summary
Assessment: 🟢 APPROVE
This PR correctly implements the missing piece of the pre-tool hook system: applying ModifiedInput back to tool call arguments. The code changes are well-structured and handle edge cases appropriately.
What was reviewed:
- Error handling for JSON marshaling of modified input
- Control flow with the refactored
defaultcase - Integration with existing hook blocking logic
Key observations:
- The early return when
!result.Allowedensures modified input is only processed for allowed tool calls - Failed JSON marshaling logs a warning and proceeds with original input, which is a reasonable fallback strategy
- The refactoring from
case result.SystemMessage != ""todefaultenables processing both SystemMessage and ModifiedInput together, which is an improvement
No bugs found in the changed code. The implementation looks solid and ready to merge.
Automated review - all hypotheses were verified and dismissed
dgageot
approved these changes
Mar 6, 2026
Contributor
Author
dgageot
approved these changes
Mar 6, 2026
|
❌ PR Review Failed — The review agent encountered an error and could not complete the review. View logs. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The pre-tool hook system already supported returning updated_input via
hook_specific_output, and the executor populated Result.ModifiedInput.
However, the runtime never applied it back to toolCall.Function.Arguments
before execution, making input-rewriting hooks (like RTK) ineffective.
Marshal ModifiedInput to JSON and overwrite the tool call arguments
when a pre-tool hook provides modified input.
Assisted-By: cagent