.NET: Preserve harness middleware for dynamic tools - #8402
Roger Barreto (rogerbarreto) merged 7 commits into
Conversation
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (1 commit(s)): ef0182a95c90
Model: gpt-5.6-sol-fast
Overview
The change keeps function middleware request-scoped by cloning run and chat options, wraps collection mutations, and reapplies wrapping after whole-collection replacement; the added tests cover ordering, approval metadata, streaming parity, and shared-client isolation. One composition gap remains: middleware-chain discovery depends on every custom chat-client decorator forwarding GetService, so a supported opaque decorator can cause later replacement tools to lose callbacks.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
1 verified finding remained after source verification (1 medium) across 1 file. Details are attached to the affected lines below.
Affected areas: dotnet/src/Microsoft.Agents.AI/FunctionInvocationDelegatingAgent.cs
Keep harness callbacks consistent as tool collections change. Preserve existing function wrappers and request-local composition without coupling reusable options to agent instances.
Keep callback composition independent of client service discovery. Use request-local execution scopes without changing reusable options. Cover scope restoration and tool repair after collection updates.
ef0182a to
d9eb7fd
Compare
Keep harness callback composition consistent across function-loop iterations by retaining the full chain on function wrappers. Cover real client-driven clones, repeated calls and approval flows.
Eduard van Valkenburg (eavanvalkenburg)
left a comment
There was a problem hiding this comment.
Small nit on the code.
Cover Python harness callback consistency across progressive tool loading, continuation changes, failures, approvals and reused options.
There was a problem hiding this comment.
MAF Automated Review — Iteration 2
Result: Findings reported
Scope: full PR (4 commit(s)): af615706a689, d9eb7fd9fded, 20087cc93201, 33dd41ef4641
Model: gpt-5.6-sol-fast
Overview
The PR uses per-request client decoration, a middleware-aware tool collection, and wrapper-carried metadata to preserve callbacks across dynamic tool mutations and option clones. Its regression suite covers callback ordering, streaming, nested runs, approvals, failures, and caller-owned option isolation. Three composition boundaries remain unsafe: replacing client factories can discard the preserving layer, ambient run identity can merge unrelated clients, and opaque function decorators can cause duplicate callback execution.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
3 verified findings remained after source verification (3 medium) across 1 file. Details are attached to the affected lines below.
Affected areas: dotnet/src/Microsoft.Agents.AI/FunctionInvocationDelegatingAgent.cs
Build callback chains around factory results instead of sharing them between active client calls. Track active invocation callbacks across opaque function decorators while preserving explicit continuations. Cover replacement factories, captured clients and failure cleanup.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Motivation & Context
Keep harness function middleware consistent when an agent's tool collection evolves during a run. This proposal preserves middleware composition for dynamic tools while retaining the existing function-calling pipeline and reusable run-options configuration.
Description & Review Guide
FunctionMiddlewarePreservingChatClientand a middleware-aware tool collection. Preserve callback composition across tool additions, replacements, and option cloning without adding agent state to the options types. Extend .NET regression coverage and add Python regression-only coverage for its existing behavior.FunctionInvokingChatClientremain responsible for their current roles; caller-owned options and shared clients are not modified.Related Issue
No public issue linked.
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.