Skip to content

fix: update confirmation wrapper to propagate ProcessRequest calls to underlying tools and intercept packing registration#1130

Merged
hanorik merged 4 commits into
google:mainfrom
hanorik:process_request_shadowing
Jul 8, 2026
Merged

fix: update confirmation wrapper to propagate ProcessRequest calls to underlying tools and intercept packing registration#1130
hanorik merged 4 commits into
google:mainfrom
hanorik:process_request_shadowing

Conversation

@hanorik

@hanorik hanorik commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Fixes an issue where tool.WithConfirmation (confirmationTool) shadowed custom ProcessRequest methods on wrapped tools.

Changes

  • Forward ProcessRequest: Updated confirmationTool.ProcessRequest in tool/tool.go to delegate ProcessRequest calls to the underlying tool if it implements the method.
  • Preserve Confirmation Wrapper: If the wrapped tool registers itself in req.Tools during ProcessRequest, the wrapper replaces that entry with itself so confirmation checks still execute on tool runs.
  • Tests: Added unit tests (TestWithConfirmation_ProcessRequest and TestWithConfirmation_ProcessRequest_Packing) in tool/tool_test.go verifying delegation and registration wrapping.

Testing

  • go test -race ./tool/...

@hanorik hanorik requested a review from wolo-lab July 7, 2026 11:03
… underlying tools and intercept packing registration
@hanorik hanorik force-pushed the process_request_shadowing branch from 14e0536 to f8389c3 Compare July 7, 2026 11:08
wolo-lab
wolo-lab previously approved these changes Jul 7, 2026
Comment thread tool/tool.go Outdated
Comment thread tool/tool_test.go

@wolo-lab wolo-lab left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice!

@hanorik hanorik merged commit 1a5a649 into google:main Jul 8, 2026
9 checks passed
@hanorik hanorik deleted the process_request_shadowing branch July 8, 2026 11:22
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.

2 participants