.NET: Create session for tool approval agent when none provided - #7310
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a sessionless tool-approval auto-approval flow in the .NET harness where the inner agent could be re-invoked with only an injected approval response and no conversation history, resulting in an empty input sent to the underlying model provider. The change ensures a session is created (via the inner agent) when the caller doesn’t provide one, and that the same session is threaded through all inner invocations (including streaming), preserving history across the auto-approval loop.
Changes:
- Create an inner-agent session when
ToolApprovalAgentis invoked without a session, and reuse it across all inner calls in the auto-approval loop (non-streaming and streaming). - Add unit tests verifying session creation and that the same session instance is threaded across inner calls for both
RunAsyncandRunStreamingAsync.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| dotnet/src/Microsoft.Agents.AI/Harness/ToolApproval/ToolApprovalAgent.cs | Creates an inner-agent session when none is provided and threads it through all inner invocations (streaming + non-streaming). |
| dotnet/tests/Microsoft.Agents.AI.UnitTests/Harness/ToolApproval/ToolApprovalAgentTests.cs | Adds tests covering the no-session auto-approval re-invocation behavior for both run modes and validates single session creation. |
|
Flagged issue Creating a hidden session at ToolApprovalAgent.cs:123 lets the agent save queued approval state into that internal session (PrepareInboundMessagesAsync reads it at lines 437-462; ProcessAndQueueOutboundAprovalRequestsAsync persists it at lines 573-579 and 628), but AgentResponse does not expose any session handle back to the caller—it only carries a continuation token for background responses (AgentResponse.cs:165-180). Existing tests show qued approvals only work when the same caller-owned session is passed across turns (ToolApprovalAgentTests.cs:1547-1561). A no-session run that surfaces multiple non-auto-approved tool requests will appear to work on the first call, then lose its queue/rule state on the next call. Source: automated DevFlow PR review |
There was a problem hiding this comment.
Automated Code Review
Reviewers: 5 | Confidence: 92%
✓ Correctness
The change correctly creates a session when none is supplied, threading it through all inner agent calls in the auto-approval loop to preserve conversation history. The pattern mirrors the existing LoopAgent approach (lines 144-146). State initialization with a null session in PrepareInboundMessagesAsync is safe because (a) on first call the queue is empty so no SaveState-with-null occurs, and (b) after session creation all subsequent SaveState calls use the real session. The
??=operator is a no-op when a session is already provided, so existing behavior is unchanged. Tests cover both streaming and non-streaming paths as well as the single-call (no re-invocation) case.
✓ Security Reliability
The change creates an inner agent session when none is supplied, threading it through auto-approval re-invocations to preserve conversation history. The pattern mirrors LoopAgent (lines 144-146) and is well-tested. State management is correct: PrepareInboundMessagesAsync with a null session creates a fresh in-memory state (no queued items), and after session creation the same state reference is saved to the new session on subsequent SaveState calls. No security, resource leak, or reliability issues found.
✓ Test Coverage
The PR adds three well-structured tests covering the core fix (session creation when none is supplied) for both non-streaming and streaming auto-approval re-invocation paths, plus a non-streaming single-call path. Test coverage is good overall. One minor gap: the streaming path lacks a counterpart for the 'no approval request, no session' test that exists for the non-streaming path. No blocking issues found.
✓ Failure Modes
The change correctly creates an internal session when none is provided, ensuring conversation history is preserved across auto-approval re-invocations. The pattern is consistent with how LoopAgent (line 144-146) handles the same scenario. State initialization via GetOrInitializeState(null) in PrepareInboundMessagesAsync is safe because the in-memory state object is the same reference used throughout, and subsequent SaveState calls with the now-non-null session correctly persist it. Session creation (ChatClientAgentSession) is lightweight (just a constructor call, line 418 of ChatClientAgent.cs) so the unconditional creation before the loop has negligible cost. No concrete silent failures, lost errors, or resource leaks were identified.
✗ Design Approach
The implicit-session fix addresses the reported auto-approval retry, but it also introduces a broader no-session mode where ToolApprovalAgent can persist queued approval state into an internal session that the caller can never reuse. That leaves the ordinary multi-step approval path silently unrecoverable when no explicit session is supplied.
Flagged Issues
- Creating a hidden session at ToolApprovalAgent.cs:123 lets the agent save queued approval state into that internal session (PrepareInboundMessagesAsync reads it at lines 437-462; ProcessAndQueueOutboundAprovalRequestsAsync persists it at lines 573-579 and 628), but AgentResponse does not expose any session handle back to the caller—it only carries a continuation token for background responses (AgentResponse.cs:165-180). Existing tests show qued approvals only work when the same caller-owned session is passed across turns (ToolApprovalAgentTests.cs:1547-1561). A no-session run that surfaces multiple non-auto-approved tool requests will appear to work on the first call, then lose its queue/rule state on the next call.
Automated review by westey-m's agents
rogerbarreto
left a comment
There was a problem hiding this comment.
Should we really be permissive allowing the agent to work without sessions? We might need just to enforce the caller to always provide the session. Otherwise if the caller forgot by mistake it won't be able to recover the session as he will not have a pointer for it
Allowing a call without a session is existing behavior which is by design to allow easier usage of AF if multi-turn usage is not required. Changing this would be a breaking change. Let's discuss further offline. |
Motivation & Context
When an agent is wrapped with the tool-approval middleware (
UseToolApproval/ToolApprovalAgent) and configured with auto-approval rules, invoking it without an explicitAgentSessioncould fail after the first tool call with an HTTP 400 from the model provider:The tool-approval agent auto-approves the surfaced approval requests and then re-invokes the inner agent with only the injected approval-response message. When no session was supplied, the inner
ChatClientAgenthad no conversation history to reconstruct the request, so it sent an emptyinputto the Responses API, which the service rejected.Because the history is otherwise persisted in the session, the same flow works when a session is passed — matching the "works with a session, fails without one" symptom reported in #7210. I was unable to reproduce the reporter's exact scenario, but this is a strong candidate for the underlying cause.
Description & Review Guide
What are the major changes?
ToolApprovalAgent.RunCoreAsyncandRunCoreStreamingAsync, when the caller does not supply a session, one is now created viathis.InnerAgent.CreateSessionAsync(cancellationToken)before the auto-approval loop and threaded through every inner run. This preserves conversation history across auto-approval re-invocations.What is the impact of these changes?
What do you want reviewers to focus on?
Related Issue
Related to #7210. Linking for tracking; not using a closing keyword because the reporter's exact repro is unconfirmed, so this should not auto-close the issue on merge.
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.