[dotnet-port-api] Port tool approval rule context - #967
Quim Muntal (qmuntal) merged 4 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The same ToolAutoApprovalRuleContext pointer is reused across multiple rules, allowing accidental rule side effects via context mutation to influence subsequent rule evaluation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR ports the richer .NET tool auto-approval rule context into the Go toolapproval middleware by introducing an exported ToolAutoApprovalRuleContext and updating auto-approval rule evaluation to receive surrounding run metadata (agent/session/request messages/options).
Changes:
- Added
toolapproval.ToolAutoApprovalRuleContextandtoolapproval.AutoApprovalRule, and updatedConfig.AutoApprovalRulesto use the new signature. - Threaded the original request messages and run options through the middleware so rules can evaluate against run metadata.
- Updated and extended tests to validate the new context (including agent pipeline metadata capture).
File summaries
| File | Description |
|---|---|
| agent/harness/toolapproval/toolapproval.go | Introduces the exported auto-approval rule context/type and passes run metadata into rule evaluation. |
| agent/harness/toolapproval/toolapproval_test.go | Updates tests for the new rule signature and adds coverage for captured run metadata in the rule context. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…pi-toolapproval-context-cf1f1aefbf4f5a99 # Conflicts: # agent/harness/toolapproval/toolapproval.go # agent/harness/toolapproval/toolapproval_test.go
…-cf1f1aefbf4f5a99
This comment has been minimized.
This comment has been minimized.
Co-authored-by: michelle-clayton-work <262183035+michelle-clayton-work@users.noreply.github.com>
|
Scope: public API, user-visible behavior Changed Go contract: New exported Upstream evidence reviewed:
Result: aligned. The Go Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "github.com"See Network Configuration for more information.
|
Summary
Aligned Go tool approval auto-approval rules with the richer .NET rule context by adding an exported
toolapproval.ToolAutoApprovalRuleContextandtoolapproval.AutoApprovalRule, then updatingtoolapproval.Config.AutoApprovalRulesto receive the function call together with agent, session, request-message, and run-option metadata.The tests were updated to use the new rule signature and now cover that the context carries the expected run metadata when the middleware executes inside an
agent.Agentpipeline.Ported .NET PRs
b3f2e5392350d32835a40455d5069c18cac47a97Breaking Changes
Yes.
toolapproval.Config.AutoApprovalRulespreviously accepted functions with the signaturefunc(context.Context, *message.FunctionCallContent) (bool, error). It now acceptstoolapproval.AutoApprovalRule, which receives*toolapproval.ToolAutoApprovalRuleContextand exposes the original function call throughFunctionCallplus the surrounding run metadata. This is acceptable because the Go SDK is still in beta and the new shape matches the richer upstream .NET capability.Tests and Examples
go test ./agent/harness/toolapprovalAutoApprovalRulesNotes
Closes #823