.NET: Hosting OpenAI Responses protocol helpers and optional execution state#7000
.NET: Hosting OpenAI Responses protocol helpers and optional execution state#7000rogerbarreto wants to merge 18 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements the ADR-0027 “helper-first” hosting direction for .NET by exposing a public OpenAI Responses protocol conversion facade (OpenAIResponses) and adding optional, protocol-neutral execution state helpers for agent session continuity and workflow checkpoint resume, plus new samples and design docs.
Changes:
- Adds
Microsoft.Agents.AI.Hosting.OpenAI.OpenAIResponsespublic helper facade andOpenAIResponsesRunRequestfor JsonElement-bound request/response conversions (JSON + SSE). - Adds protocol-neutral execution state helpers in
Microsoft.Agents.AI.Hosting(HostedAgentState,HostedWorkflowState,HostedWorkflowRunResult) and introducesAgentSessionStore.DeleteSessionAsyncwith in-box store overrides. - Adds new hosting samples (agent + workflow) plus unit tests and ADR/spec documentation for the helper-first hosting shape.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| dotnet/tests/Microsoft.Agents.AI.Hosting.UnitTests/InMemoryAgentSessionStoreTests.cs | New tests covering AgentSessionStore.DeleteSessionAsync behavior across in-box stores. |
| dotnet/tests/Microsoft.Agents.AI.Hosting.UnitTests/HostedWorkflowStateTests.cs | New tests for HostedWorkflowState argument validation and checkpoint lookup behavior. |
| dotnet/tests/Microsoft.Agents.AI.Hosting.UnitTests/HostedAgentStateTests.cs | New tests for HostedAgentState store delegation and optional per-session locking. |
| dotnet/tests/Microsoft.Agents.AI.Hosting.OpenAI.UnitTests/OpenAIResponsesTests.cs | New tests for OpenAIResponses request mapping, session id extraction, id creation, and response rendering. |
| dotnet/src/Microsoft.Agents.AI.Hosting/NoopAgentSessionStore.cs | Implements no-op DeleteSessionAsync override. |
| dotnet/src/Microsoft.Agents.AI.Hosting/Local/InMemoryAgentSessionStore.cs | Implements DeleteSessionAsync by removing stored session content. |
| dotnet/src/Microsoft.Agents.AI.Hosting/IsolationKeyScopedAgentSessionStore.cs | Pass-through implementation for DeleteSessionAsync with scoped conversation ids. |
| dotnet/src/Microsoft.Agents.AI.Hosting/HostedWorkflowState.cs | Adds in-memory per-session checkpoint “head cursor” and RunOrResumeAsync. |
| dotnet/src/Microsoft.Agents.AI.Hosting/HostedWorkflowRunResult.cs | Adds run result wrapper carrying session id, emitted events, and head checkpoint. |
| dotnet/src/Microsoft.Agents.AI.Hosting/HostedAgentState.cs | Adds helper that pairs an agent with a session store plus optional per-session locking. |
| dotnet/src/Microsoft.Agents.AI.Hosting/DelegatingAgentSessionStore.cs | Pass-through implementation for new DeleteSessionAsync API. |
| dotnet/src/Microsoft.Agents.AI.Hosting/AgentSessionStore.cs | Adds new virtual DeleteSessionAsync with throwing default. |
| dotnet/src/Microsoft.Agents.AI.Hosting.OpenAI/OpenAIResponsesRunRequest.cs | Adds public run request carrier (messages + optional run options). |
| dotnet/src/Microsoft.Agents.AI.Hosting.OpenAI/OpenAIResponses.cs | Adds public conversion facade for Responses JSON and SSE rendering. |
| dotnet/samples/04-hosting/HostingResponsesWorkflow/README.md | New sample README for app-owned workflow route + checkpoint resume. |
| dotnet/samples/04-hosting/HostingResponsesWorkflow/Program.cs | New minimal API workflow sample using OpenAIResponses + HostedWorkflowState. |
| dotnet/samples/04-hosting/HostingResponsesWorkflow/HostingResponsesWorkflow.csproj | New workflow sample project. |
| dotnet/samples/04-hosting/HostingResponsesAgent/README.md | New sample README for app-owned agent route (sync + SSE). |
| dotnet/samples/04-hosting/HostingResponsesAgent/Program.cs | New minimal API agent sample using OpenAIResponses + HostedAgentState. |
| dotnet/samples/04-hosting/HostingResponsesAgent/HostingResponsesAgent.csproj | New agent sample project. |
| dotnet/agent-framework-dotnet.slnx | Includes the new hosting samples in the solution. |
| docs/specs/003-dotnet-hosting-protocol-helpers.md | Adds accepted spec describing the new helper + state surface and samples. |
| docs/decisions/0032-dotnet-hosting-protocol-helpers.md | Adds accepted ADR capturing the decision and rationale for the new surface. |
There was a problem hiding this comment.
Automated Code Review
Reviewers: 5 | Confidence: 88%
✓ Correctness
The framework code (OpenAIResponses facade, HostedAgentState, HostedWorkflowState, DeleteSessionAsync additions) is well-designed and correct. The one correctness issue is in the HostingResponsesAgent sample, which passes
responseIdas thesessionIdparameter toWriteResponseandWriteResponseStreamAsync, whereas the spec's own E2E sample (docs/specs/003-dotnet-hosting-protocol-helpers.md lines 183, 194) correctly passessessionId. This causes the rendered response'sconversation.idto equal the responseidrather than the actual conversation/session id, breaking the OpenAI Responses conversation-grouping semantic.
✓ Security Reliability
The PR is well-designed with clear trust boundary documentation and proper input validation. Two issues found: (1) the session locking dictionary in HostedAgentState grows without bound, creating a resource leak for long-lived services; (2) the HostingResponsesAgent sample passes
responseIdwhere the spec (in the same PR) passessessionIdas the conversation identifier, causingconversation.idin the response to be per-turn rather than stable.
✓ Test Coverage
The PR adds good unit tests for guard clauses, delegation, and basic conversion paths. However, three significant behaviors lack test coverage: (1) the
WriteResponseStreamAsyncstreaming facade has zero tests — the SSE frame formatting logic is entirely untested; (2)HostedWorkflowState.RunOrResumeAsynconly tests argument validation, not the actual run-or-resume logic, event collection, or cursor recording; (3)IsolationKeyScopedAgentSessionStore.DeleteSessionAsync(new code) has no test verifying it applies isolation-key scoping before delegating to the inner store.
✓ Failure Modes
The PR is well-structured from a failure-mode perspective. Disposal patterns for Run are correct (await using), the semaphore releaser is safe against double-release via Interlocked.Exchange, argument validation gates all state operations, and the NotSupportedException default for DeleteSessionAsync is documented, tested, and maintains backward compatibility. No silent failure paths, swallowed exceptions, or partial-write scenarios were found in the new code. The in-memory cursor limitation in HostedWorkflowState is explicitly documented as a v1 trade-off.
✓ Design Approach
I found one production-surface design issue and one sample-level design issue. The new
OpenAIResponseshelper path no longer enforces the existing OpenAI Responses invariant thatconversationandprevious_response_idare mutually exclusive, so invalid requests are silently accepted and resolved differently fromMapOpenAIResponses. Separately, the new agent sample collapsesconversation.idandprevious_response_idinto a single storage key, which breaks stableconversation.idcontinuity for callers following the documented protocol shape.
Automated review by rogerbarreto's agents
…nt key per review
…workflow resume Migrate HostingResponsesAgent and HostingResponsesWorkflow samples from Azure.AI.OpenAI to Azure.AI.Projects (AIProjectClient.AsAIAgent), using the FOUNDRY_PROJECT_ENDPOINT/FOUNDRY_MODEL convention. Fix HostedWorkflowState.RunOrResumeAsync: on subsequent turns, restore the session's latest checkpoint and run the workflow forward with the new turn's input (mirroring the Python hosting host's restore-then-run semantics) instead of resuming a halted run with no input, which waited on input indefinitely. Add round-trip resume tests and update ADR-0032/spec-003 wording.
…ests On resume, HostedWorkflowState.RunOrResumeAsync drained the workflow with the blocking WatchStreamAsync overload, so a workflow that halts at an unserviced RequestInfoEvent (human-in-the-loop / approval) blocked forever — asymmetric with the first-turn RunAsync path, which returns at the same halt. Break the drain when a superstep completes with HasPendingRequests, restoring symmetry with turn 1. Add a HITL approval-gate workflow and a resume-does-not-block test.
Add an optional ILoggerFactory to HostedWorkflowState and log a warning when a resumed turn produces no events, mirroring the Python host's zero-event restore warning (a stale checkpoint or an input that does not match the workflow's expected type leaves session state unprogressed). Add a non-chat string workflow helper, a capturing logger, and a red/green test.
Add CheckpointManager.GetLatestCheckpointAsync(sessionId) and have HostedWorkflowState fall back to it when its in-memory head cursor misses, so a durable CheckpointManager resumes a session across a process restart or a new holder instead of restarting from the workflow's start executor. Mirrors the Python host's per-turn get_latest read-through. Add a counting workflow that proves resume-vs-fresh via accumulated state, plus a red/green test, and update ADR-0032/spec-003 and the XML remarks.
A single workflow instance backs the holder and workflow instances do not support concurrent runs (the runner throws "already owned by another runner"), so concurrent turns could fault or race the head cursor. Serialize all turns through one SemaphoreSlim (mirroring the Python host's workflow lock) and make HostedWorkflowState IDisposable to own it. Add a gated workflow and a deterministic concurrency red/green test.
Add tests for HostedWorkflowState resuming a non-chat-protocol workflow (no TurnToken) and for a third turn continuing to advance the head checkpoint, closing the coverage gaps the parity review flagged.
Add HostedWorkflowState.RunOrResumeStreamingAsync, which yields the turn's WorkflowEvents as they occur (fresh run or checkpoint resume) under the same serialization lock and records the head checkpoint after the stream drains, keeping the blocking and streaming workflow paths in lockstep with the Python host. Honor stream:true in the HostingResponsesWorkflow sample by projecting AgentResponseUpdateEvent updates over the Responses SSE wire. Add a streaming resume test and update the README/spec.
…utor Demonstrate that HostedWorkflowState's generic RunOrResumeAsync<TInput> is the input-adaptation seam (parity with Python's ResponsesChannel run hook): the app adapts the Responses input into the workflow start executor's own type at the call site. Add a typed-brief workflow and a test, and note the seam in spec-003.
The resume drain used a SuperStepCompletedEvent{HasPendingRequests} proxy over
the blocking public WatchStreamAsync. That proxy (a) truncated a resumed turn
when a superstep both emitted a request and queued downstream work, and (b)
could fail to fire at all — re-introducing the indefinite hang — when a resume
input drove no superstep (e.g. a rejected non-chat input).
Make StreamingRun.WatchStreamAsync(bool blockOnPendingRequest, CancellationToken)
public and drain both the blocking and streaming resume paths with
blockOnPendingRequest:false, exactly matching the first-turn RunAsync semantics
(Run.RunToNextHaltAsync). Add guard tests: resume with a rejected input does not
hang, and a resume superstep with a request plus downstream work is not
truncated (verified red against the old proxy).
CheckpointManager.GetLatestCheckpointAsync takes the last entry of a store's index as the head checkpoint. FileSystemJsonCheckpointStore backed its index with a HashSet, whose enumeration order is not contractual: after a rollback frees and reuses a slot, enumeration can diverge from commit order, so the durable read-through could resume a stale checkpoint. Mirror the HashSet with an insertion-ordered list and enumerate it from RetrieveIndexAsync so 'latest' is reliable. Add a CheckpointManager.GetLatestCheckpointAsync contract test over the file store. Note: the HashSet disorder is only reachable via the internal rollback path, so the test locks the ordering contract rather than reproducing the rare disorder.
RunOrResumeStreamingAsync recorded the head checkpoint only after the stream was fully enumerated. If an SSE consumer disconnected mid-turn after supersteps had committed, the in-memory cursor kept the previous turn's head; because the next turn is then a cursor hit, durable read-through could not self-heal, so it resumed pre-disconnect state. Record the run's last committed checkpoint in a finally so an abandoned stream still advances the cursor. Add a red/green test.
ExtractUpdates streamed every agent's updates, so the sequential Writer->Reviewer sample streamed the intermediate draft and the final answer over SSE, differing from the non-streaming response (final message only). Filter the streamed updates to the final agent so streaming and non-streaming produce the same response. Live-verified against Foundry: one output item streamed instead of two.
The concurrency test asserted the second same-session turn did not enter the workflow, which also passes via the engine's concurrent-run ownership guard (which faults) rather than the holder lock (which waits). Assert instead that the second turn is not completed while the first holds the lock: a fault would complete the task, so a pending task isolates the holder lock from the engine guard. Verified red with the lock removed.
2fb52b3 to
06ff24b
Compare
There was a problem hiding this comment.
Automated Code Review
Reviewers: 5 | Confidence: 87%
✓ Correctness
The PR is well-structured and the core production code (helpers, state holders, store changes) is correct. One issue found in the agent hosting sample:
responseIdis passed as thesessionId(conversation id) argument toWriteResponse/WriteResponseStreamAsync, causing the renderedconversation.idto change every turn instead of being stable — contradicting both the OpenAI protocol semantics and the spec's own code sample which correctly passessessionId.
✓ Security Reliability
The PR is well-designed with clear trust boundary documentation and correct backward-compatible virtual method additions. The primary reliability concern is that
HostedAgentState._sessionLocksgrows unboundedly when session locking is enabled — every unique session ID allocates aSemaphoreSlimthat is never evicted or disposed, which is a resource leak for long-lived services with many sessions.
✓ Test Coverage
The PR adds substantial new public API surface with generally good test coverage for HostedAgentState and HostedWorkflowState. However, the OpenAIResponses facade has a notable coverage gap: WriteResponseStreamAsync (one of the two primary public methods) has zero unit tests. A secondary gap is the documented GetSessionId precedence behavior (previous_response_id preferred over conversation when both are present) which also lacks a test. The HostedWorkflowState and HostedAgentState tests are thorough, covering first-turn, resume, concurrency, streaming, abandoned streams, cursor-miss fallback, and edge cases. The InMemoryAgentSessionStore tests cover delete, unknown-id, noop, and base-default-throws. The main test coverage gap is that OpenAIResponses.WriteResponseStreamAsync — a new public SSE-framing API — has zero direct test coverage anywhere in the PR, despite being a wire-format method where framing correctness matters for interoperability.
✓ Failure Modes
The PR is well-structured with thorough documentation, tests, and separation of concerns. The main failure-mode concern is in
HostedWorkflowStatewhere theTrySendMessageAsyncreturn value is discarded during workflow resume: if the resumed workflow cannot accept the input message (type mismatch, incompatible state), the user's input is silently lost. Events from the restore itself may still be produced (cursor advances, events > 0), so the 'no progress' warning does not fire and there is no signal that the input was dropped.
✗ Design Approach
The new agent-hosting sample mixes stable
conversationids withprevious_response_idchaining on top of a single-key session store, so a client that keeps sending the sameconversationid can silently lose continuity after the first turn. Separately,CheckpointManager.GetLatestCheckpointAsyncinfers "latest" from enumeration order even though the checkpoint-store interface does not currently guarantee any ordering, which makes the new resume path incorrect for custom store implementations. I found one design-level issue: the new latest-checkpoint behavior is defined in terms of enumeration order fromRetrieveIndexAsync, but the public checkpoint-store contract does not guarantee that order. That means external stores can silently resume from a stale checkpoint even though this test now encodes the opposite assumption.
Flagged Issues
-
CheckpointManager.GetLatestCheckpointAsyncderives “latest” by taking the last element returned fromRetrieveIndexAsync(dotnet/src/Microsoft.Agents.AI.Workflows/CheckpointManager.cs:73-83), but the public store contract only promises “a collection” with no ordering guarantee (dotnet/src/Microsoft.Agents.AI.Workflows/Checkpointing/ICheckpointStore.cs:15-24). Any external checkpoint store that returns checkpoints in a different order will silently resume the wrong checkpoint on theHostedWorkflowStatecursor-miss path (dotnet/src/Microsoft.Agents.AI.Hosting/HostedWorkflowState.cs:117-120).
Suggestions
- Add at least one unit test for OpenAIResponses.WriteResponseStreamAsync that verifies the SSE frame format (event: {type}\ndata: {json}\n\n) for a single-message stream. The underlying ToStreamingResponseAsync is tested via conformance tests, but the public helper's SSE string framing in OpenAIResponses.cs:115 is new code with no direct coverage.
Automated review by rogerbarreto's agents
| /// </returns> | ||
| public async ValueTask<CheckpointInfo?> GetLatestCheckpointAsync(string sessionId, CancellationToken cancellationToken = default) | ||
| { | ||
| IEnumerable<CheckpointInfo> index = await this._impl.RetrieveIndexAsync(sessionId, withParent: null).ConfigureAwait(false); |
There was a problem hiding this comment.
GetLatestCheckpointAsync treats the last item returned by RetrieveIndexAsync as the "most recently committed" checkpoint, but the public store contract only promises "a collection" of CheckpointInfo values and does not define any ordering (dotnet/src/Microsoft.Agents.AI.Workflows/Checkpointing/ICheckpointStore.cs:15-24). That means any custom JsonCheckpointStore implementation that returns checkpoints newest-first, parent-grouped, or otherwise unsorted will silently make this method return the wrong checkpoint, and HostedWorkflowState will resume the wrong session state. Either make commit ordering an explicit invariant of RetrieveIndexAsync across the interface, or add a dedicated latest-checkpoint operation instead of inferring it from enumeration order.
There was a problem hiding this comment.
good catch. before, ICheckpointStore say nothing about order, so a custom store that give back newest-first or unsorted quietly break latest lookup. now RetrieveIndexAsync doc make commit order a hard contract: oldest first, newest last. both in-box store already do this (SessionCheckpointCache list, FileSystem OrderedCheckpointIndex list), so this just write down the invariant they follow. pick this over adding new store method to keep interface small and not break existing custom stores. GetLatestCheckpointAsync now lean on the documented order. fixed in bca52a2.
|
Flagged issue
Source: automated DevFlow PR review |
… matching Python parity
Motivation & Context
ADR-0027 refocused the hosting design toward protocol conversion helpers plus optional execution state: Agent Framework owns protocol-native <-> run conversion, while the application owns HTTP routing, authentication, middleware, and storage. The Python slice landed in #6891.
This PR realizes that direction for .NET. A first-principles gap analysis showed .NET already covers most of the capability, and more richly, but bundled behind the route-owning
MapOpenAIResponsesserver. The only genuine gap is the ownership model: an application cannot own its own route and call just the conversion primitives. So this change un-bundles the existing conversion rather than reinventing it. No new package, no OpenAI-SDK-typed reimplementation, andMapOpenAIResponsespublic behavior is unchanged.Description & Review Guide
OpenAIResponsesinMicrosoft.Agents.AI.Hosting.OpenAI(JsonElement/SSE boundary; wire DTOs stay internal), delegating to the existing internal converters:ToAgentRunRequest(JsonElement)-> messages +AgentRunOptions?WriteResponse(AgentResponse, responseId, sessionId?)-> Responses JSONWriteResponseStreamAsync(IAsyncEnumerable<AgentResponseUpdate>, responseId, ...)-> Responses SSE framesGetSessionId(JsonElement)->previous_response_id/conversationid (kept separate so the trust boundary stays visible)CreateResponseId()->resp_*Microsoft.Agents.AI.Hosting:AgentSessionStore.DeleteSessionAsync(added as virtual with a throwing default so existing external stores keep compiling; overridden in the in-box stores)HostedAgentState(agent + session store: get-or-create, save-under-new-id, delete, optional per-session locking)HostedWorkflowState+HostedWorkflowRunResult(workflow +CheckpointManager+ an internalsessionId -> CheckpointInfohead cursor;RunOrResumeAsync)dotnet/samples/04-hosting/:HostingResponsesAgent(app-owned route, sync + SSE) andHostingResponsesWorkflow(checkpoint resume).0032-dotnet-hosting-protocol-helpers.mdand spec003-dotnet-hosting-protocol-helpers.md.MapOpenAIResponses,IResponsesService, ChatCompletions, and Conversations are untouched.AgentSessionStoreinstead of cloning Python's in-memorySessionStore, and the virtual-not-abstractDeleteSessionAsync.HostedWorkflowStatehead-cursor approach for per-session checkpoint resume.Related Issue
N/A. This is the .NET realization of the accepted ADR-0027; it adds ADR-0032 and its spec. It supersedes the earlier channel-model draft in #6151 (closed). Opening as a draft for design feedback.
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.