fix(agent): unwrap nested tool call arguments - #10076
NayukiChiba wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Hey - I've reviewed your changes and they look great!
Sourcery assessment
Needs a human reviewer. If the unwrapping condition is wrong, a previously rejected or malformed tool call could invoke a handler with unintended arguments, including executing an unintended local shell command. Reverting prevents future calls from taking that path, but any side effects from commands or other tools that already ran would remain.
kilisamemarisaaa
left a comment
There was a problem hiding this comment.
Reviewed head 6e44463. The recursive unwrap is constrained to a single arguments key whose value is an object, and it is disabled when the tool schema explicitly declares an arguments property; non-wrapper inputs remain unchanged. Filtering and permission checks still run after normalization, while the provider's original argument object is not mutated. The added cases cover nested streaming/non-streaming calls, local shell permissions, handler tools, declared arguments, mixed keys, scalar/null/list wrappers, and nested parameter values. Linux/Windows/macOS pytest, CodeQL, build, format, and smoke checks are green. I found no correctness blocker.
ready for review |
Also fix the session-plugin event guard and adapt the wrapped-shell test to the role-based Local permission model (admins_id + nested context config).
Also fix the session-plugin event guard and adapt the wrapped-shell test to the role-based Local permission model (admins_id + nested context config).
Fixes #10074
当工具调用参数被额外包装为
{"arguments": {"command": "..."}},甚至出现多层嵌套时,核心会直接将外层arguments作为关键字参数传给工具,导致参数不匹配,工具无法执行。Modifications / 改动点
arguments包装层。arguments一个键且值为对象的包装结构。arguments参数及现有权限检查。Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Ensure tool calls with redundant nested argument wrappers are normalized without changing valid argument handling.
Bug Fixes:
argumentswrappers before parameter filtering and hook execution, allowing affected tools to execute correctly.Enhancements:
argumentsparameters and non-wrapper argument structures while retaining existing permission checks.Tests: