fix(dotnet): forward Context and ForwardedProperties from RawRepresentationFactory input (#2151)#2177
Conversation
…pplied RunAgentInput (#2151) `AGUIChatClientHandler.BuildRunAgentInput` copied Messages/Tools/State/ParentRunId from a `ChatOptions.RawRepresentationFactory`-supplied `RunAgentInput` onto the request actually sent, but silently dropped `Context` and `ForwardedProperties` — so caller-supplied run context/props never reached the AG-UI server. Forward both, mirroring the existing field-copy pattern (`Context` when non-empty, `ForwardedProperties` when its JsonElement is not Undefined). Adds a unit test (CapturingTransport) asserting both survive onto the sent input; verified it fails on the pre-fix code (Context null) and passes after. Full AGUI.Client.UnitTests suite green (111/111, net10.0). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Python Preview PackagesVersion
Install with uvAdd the TestPyPI index to your [[tool.uv.index]]
name = "testpypi"
url = "https://test.pypi.org/simple/"
explicit = trueThen install the packages you need: # Core SDK
uv add 'ag-ui-protocol==0.0.0.dev1784125344' --index testpypi
# Integrations (each already depends on the matching ag-ui-protocol preview)
uv add 'ag-ui-langgraph==0.0.0.dev1784125344' --index testpypi
uv add 'ag-ui-crewai==0.0.0.dev1784125344' --index testpypi
# NOTE: ag-ui-agent-spec depends on pyagentspec (git-only, not on PyPI).
# You will need to install pyagentspec separately from its git repo.
uv add 'ag-ui-agent-spec==0.0.0.dev1784125344' --index testpypi
uv add 'ag_ui_adk==0.0.0.dev1784125344' --index testpypi
uv add 'ag_ui_strands==0.0.0.dev1784125344' --index testpypiInstall with pippip install \
--index-url https://test.pypi.org/simple/ \
--extra-index-url https://pypi.org/simple/ \
ag-ui-protocol==0.0.0.dev1784125344
Commit: 84fc630 |
@ag-ui/a2a-middleware
@ag-ui/a2ui-middleware
@ag-ui/event-throttle-middleware
@ag-ui/mcp-apps-middleware
@ag-ui/mcp-middleware
@ag-ui/a2a
@ag-ui/adk
@ag-ui/ag2
@ag-ui/agno
@ag-ui/aws-strands
@ag-ui/claude-agent-sdk
@ag-ui/crewai
@ag-ui/langchain
@ag-ui/langgraph
@ag-ui/llamaindex
@ag-ui/mastra
@ag-ui/pydantic-ai
@ag-ui/vercel-ai-sdk
@ag-ui/watsonx
@ag-ui/a2ui-toolkit
create-ag-ui-app
@ag-ui/client
@ag-ui/core
@ag-ui/encoder
@ag-ui/proto
commit: |
There was a problem hiding this comment.
@BenTaylorDev Did you mean to leave this untouched? Seems like it would have the same issue, but only if RawRepresentationFactory is used rather than resume.
My explanation is wrong, but I poked at the critique and it appears to be valid. |
|
@contextablemark that link resolves to On resume: it's the same |
contextablemark
left a comment
contextablemark
left a comment
There was a problem hiding this comment.
See review... the agent still thinks there's an issue with Resume not being copied from providedInput. If dropping Resume is intentional, then the critique falls apart.
| if (providedInput.ForwardedProperties.ValueKind != JsonValueKind.Undefined) | ||
| { | ||
| input.ForwardedProperties = providedInput.ForwardedProperties; | ||
| } |
There was a problem hiding this comment.
Does it make sense to add something here like :
| } | |
| } | |
| if (providedInput.Resume is { Count: > 0 }) | |
| { | |
| input.Resume = providedInput.Resume; | |
| } |
or is the Resume not relevant here?
There was a problem hiding this comment.
The analysis indicates that, until the change above is made, input.Resume will always be null, so this check will be dead, but if the change above is made then it could then be skipped. It all boils down to whether it is intentional not to copy providedInput.Resume into input.Resume. If not, you would need something like this.
// Convert ToolApprovalResponseContent list (passed from AGUIChatClient) to resume payload
if (options?.AdditionalProperties?.TryGetValue(AGUIClientInternalKeys.ApprovalResponses, out List<ToolApprovalResponseContent>? approvalResponses) is true
&& approvalResponses is { Count: > 0 })
{
var resumeList = input.Resume is { Count: > 0 } existing
? new List<AGUIResume>(existing)
: new List<AGUIResume>(approvalResponses.Count);
// ... unchanged foreach ...
input.Resume = resumeList;
}
And then adding a new test :
public async Task GetStreamingResponse_RawRepresentationFactory_ForwardsResume()
{
var transport = new CapturingTransport();
using var client = new AGUIChatClient(new() { Transport = transport });
var options = new ChatOptions
{
RawRepresentationFactory = _ => new RunAgentInput
{
Resume = new List<AGUIResume>
{
new() { InterruptId = "int-1", Status = ResumeStatus.Resolved },
},
},
};
await DrainAsync(client.GetStreamingResponseAsync(
new List<ChatMessage> { new(ChatRole.User, "Hello") }, options));
var sent = transport.LastInput!;
Assert.NotNull(sent.Resume);
Assert.Equal("int-1", Assert.Single(sent.Resume!).InterruptId);
}
…#2151 review) Follow-up to the Context/ForwardedProperties fix: BuildRunAgentInput also dropped providedInput.Resume. The approval-response translation's `if (input.Resume is null && ...)` guard was effectively dead (nothing set input.Resume first), confirming it was meant to be copied. Forward it in the same block; a caller-supplied Resume now takes precedence over the approval translation, and interrupt responses still append. Tests for both (forwarded + precedence); full client suite green (113). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Confirmed — Pushed the forwarding in the same block (permalink) so a caller-supplied Resume is forwarded and takes precedence over the approval-response translation; interrupt responses still append. Added tests for both (forwarded + precedence) — confirmed each fails on the pre-fix code and passes after — and the full client suite is green (113). |
|
@NathanTarbert apologies — I missed your outstanding #2164 when I opened this, so #2177 duplicates the same Context/ForwardedProperties fix (identical guarded pattern to yours). The one difference: this PR additionally forwards Since they overlap, would you be OK closing #2164 in favor of this one as the superset? Equally happy to do it the other way — port the |
|
@BenTaylorDev
So if this asymmetry is intended... fine - otherwise something else to look at |
|
It's not. It was seemingly always present but no code hit it until I added the additional cases for the resume handling. Adding a fix for it. |
…oval and interrupt responses Previously a caller-supplied Resume (via RawRepresentationFactory) dropped approval responses (the approval block was gated on `input.Resume is null`) but interrupt responses were still appended unconditionally. That asymmetry was dormant until Resume forwarding was added in this PR, which is the only path that populates input.Resume before the translation blocks run. Gate both translations on a single callerSuppliedResume flag so a caller who hand-builds Resume owns it entirely, keeping the two resume paths symmetric. Since the dropped approval/interrupt responses were already stripped from the outgoing messages, emit an ActivityEvent so the drop is observable instead of silent. Adds a mirror test for the interrupt precedence path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Good catch @contextablemark — the asymmetry was real, and not intended. Fixed in a2b545d. Root cause: the two blocks used different merge strategies. Approval replaced Fix (caller-wins, symmetric):
All 114 AGUI.Client unit tests pass locally (net10.0). |
2 participants
Fixes #2151.
Problem
When a caller seeds the outgoing request via
ChatOptions.RawRepresentationFactory(aRunAgentInput),AGUIChatClientHandler.BuildRunAgentInputcopiesMessages,Tools,StateandParentRunIdonto the request it actually sends — but never readsContextorForwardedProperties. Both are silently dropped, so caller-supplied run context/props never reach the AG-UI server.Fix
Forward the two missing fields inside the existing
providedInput is not nullblock, mirroring the surrounding pattern:ContextisIList<AGUIContext>?(guarded on non-empty) andForwardedPropertiesis a non-nullableJsonElement(defaultValueKindisUndefined, so the guard skips an unset value).Testing
AGUIChatClientTestusing the existingCapturingTransportto assert bothContextandForwardedPropertiessurvive onto theRunAgentInputsent to the transport.Assert.NotNullonContext→ null) and passes after the fix.AGUI.Client.UnitTestssuite green: 111/111 (net10.0). Ran locally with.NET 10.0.100(dotnet test … /p:SignAssembly=false, since the test-onlyInternalsVisibleTois gated on unsigned builds).🤖 Generated with Claude Code