.NET: Bind always approval responses to surfaced requests - #8432
westey (westey-m) merged 2 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Stale and colliding request IDs can still create unintended standing approval rules.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Binds persistent approval responses to requests actually surfaced by the harness.
Changes:
- Records surfaced approval requests across queue and capped paths.
- Rebinds always-approve responses to recorded tool calls.
- Adds security, replay, streaming, and persistence tests.
File summaries
| File | Description |
|---|---|
ToolApprovalAgent.cs |
Implements request-bound standing approvals. |
ToolApprovalAgentTests.cs |
Expands approval-binding coverage. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (1 commit(s)): bc91c761efcc
Model: gpt-5.6-sol-fast
Overview
The PR records model-originated approval requests in session state and binds always-approve wrappers to snapshots of requests actually surfaced to the caller, with strong tests for forgery, substitution, replay, and streaming abandonment. One-use removal and the downstream approval-binding client protect the normal wrapper path. However, stale entries after plain responses and ambiguous request IDs on capped turns can still create standing rules without valid consent.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
2 verified findings remained after source verification (2 high) across 1 file. Details are attached to the affected lines below.
Affected areas: dotnet/src/Microsoft.Agents.AI/Harness/ToolApproval/ToolApprovalAgent.cs
Motivation & Context
Description & Review Guide
Bind always approval responses to surfaced requests
Related Issue
Fixes #
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.