Skip to content

.NET: Fix declarative workflow regressions for hosted agents#5905

Open
alliscode wants to merge 2 commits into
microsoft:mainfrom
alliscode:triage-fix
Open

.NET: Fix declarative workflow regressions for hosted agents#5905
alliscode wants to merge 2 commits into
microsoft:mainfrom
alliscode:triage-fix

Conversation

@alliscode
Copy link
Copy Markdown
Member

Three regressions surfaced when running a declarative workflow as a Foundry hosted agent. Together they caused every condition group to fall through to else Actions and the raw agent JSON to leak to the caller.

  1. AgentProviderExtensions.InvokeAgentAsync forced autoSend to true whenever the agent ran on the workflow conversation, which overrode the explicit autoSend: false declared in workflow.yaml and streamed the raw structured-output JSON straight to the user. Honor the caller-supplied autoSend instead.

  2. IWorkflowContextExtensions.ReadState / QueueStateUpdateAsync / QueueStateResetAsync took the variable name and namespace alias directly from PropertyPath.VariableName / NamespaceAlias. Against Microsoft.Agents.ObjectModel 2026.2.4.1 those properties return null for a dotted reference such as Local.Triage even when SegmentCount == 2 and IsValid == true, so every assignment threw ArgumentNullException via Throw.IfNull. Fall back to Segments() to reconstruct the name and alias when the parser returns null.

  3. The same ObjectModel version no longer recognizes the user-facing Local scope alias: VariableScopeNames.IsValidName(Local) returns false and GetNamespaceFromName(Local) returns Unknown, so the declarative interpreter's IsManagedScope check fails and the State.Set call is silently skipped. Translate the Local alias to its canonical Topic form before forwarding to QueueStateUpdateAsync; WorkflowFormulaState.Bind continues to expose it as Local to PowerFx.

Verified end-to-end against a deployed Foundry hosted agent: the declarative triage workflow now routes Technical / Billing / General inputs correctly and only the autoSend-eligible messages reach the caller.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • The PR follows the Contribution Guidelines
  • All unit tests pass, and I have added new tests where possible
  • Is this a breaking change? If yes, add "[BREAKING]" prefix to the title of the PR.

Three regressions surfaced when running a declarative workflow as a
Foundry hosted agent. Together they caused every condition group to fall
through to elseActions and the raw agent JSON to leak to the caller.

1. AgentProviderExtensions.InvokeAgentAsync forced autoSend to true
   whenever the agent ran on the workflow conversation, which overrode
   the explicit autoSend: false declared in workflow.yaml and streamed
   the raw structured-output JSON straight to the user. Honor the
   caller-supplied autoSend instead.

2. IWorkflowContextExtensions.ReadState / QueueStateUpdateAsync /
   QueueStateResetAsync took the variable name and namespace alias
   directly from PropertyPath.VariableName / NamespaceAlias. Against
   Microsoft.Agents.ObjectModel 2026.2.4.1 those properties return null
   for a dotted reference such as `Local.Triage` even when
   SegmentCount == 2 and IsValid == true, so every assignment threw
   ArgumentNullException via Throw.IfNull. Fall back to Segments() to
   reconstruct the name and alias when the parser returns null.

3. The same ObjectModel version no longer recognizes the user-facing
   `Local` scope alias: VariableScopeNames.IsValidName(`Local`)
   returns false and GetNamespaceFromName(`Local`) returns Unknown, so
   the declarative interpreter's IsManagedScope check fails and the
   State.Set call is silently skipped. Translate the `Local` alias to
   its canonical `Topic` form before forwarding to
   QueueStateUpdateAsync; WorkflowFormulaState.Bind continues to expose
   it as `Local` to PowerFx.

Verified end-to-end against a deployed Foundry hosted agent: the
declarative triage workflow now routes Technical / Billing / General
inputs correctly and only the autoSend-eligible messages reach the
caller.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 15, 2026 21:23
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Fixes three regressions that prevented declarative workflows from running correctly as Foundry hosted agents: an autoSend override leaking raw agent JSON, a PropertyPath parsing regression in ObjectModel 2026.2.4.1, and a Local scope alias no longer being recognized.

Changes:

  • Honor caller-supplied autoSend instead of forcing it to true for the workflow conversation.
  • Fall back to Segments() to reconstruct variable name / namespace alias when PropertyPath.VariableName / NamespaceAlias return null.
  • Translate the user-facing Local scope alias to its canonical Topic form before forwarding to state-update APIs.
Show a summary per file
File Description
dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Extensions/AgentProviderExtensions.cs Removes the unconditional autoSend override for the workflow conversation.
dotnet/src/Microsoft.Agents.AI.Workflows.Declarative/Extensions/IWorkflowContextExtensions.cs Adds GetVariableName / GetNamespaceAlias helpers that compensate for ObjectModel regressions and remap LocalTopic.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 6

Comment on lines +45 to +47
private static string? GetVariableName(PropertyPath variablePath) =>
variablePath.VariableName ?? (variablePath.SegmentCount >= 2 ? variablePath.Segments().ElementAtOrDefault(1).PropertyName : variablePath.SegmentCount == 1 ? variablePath.Segments().ElementAtOrDefault(0).PropertyName : null);

Comment on lines 24 to +25
public static FormulaValue ReadState(this IWorkflowContext context, PropertyPath variablePath) =>
context.ReadState(Throw.IfNull(variablePath.VariableName), Throw.IfNull(variablePath.NamespaceAlias));
context.ReadState(Throw.IfNull(GetVariableName(variablePath)), GetNamespaceAlias(variablePath));
{
string? alias = variablePath.NamespaceAlias
?? (variablePath.SegmentCount >= 2 ? variablePath.Segments().ElementAtOrDefault(0).PropertyName : null);
return string.Equals(alias, "Local", StringComparison.Ordinal) ? VariableScopeNames.Topic : alias;
Comment on lines +41 to +47

// Workaround for ObjectModel 2026.2.4.1 regression: PropertyPath built from a dotted
// reference such as "Local.Triage" returns null for both NamespaceAlias and VariableName
// even when SegmentCount==2 and IsValid==true. Reconstruct from Segments() in that case.
private static string? GetVariableName(PropertyPath variablePath) =>
variablePath.VariableName ?? (variablePath.SegmentCount >= 2 ? variablePath.Segments().ElementAtOrDefault(1).PropertyName : variablePath.SegmentCount == 1 ? variablePath.Segments().ElementAtOrDefault(0).PropertyName : null);

// value is honored as-is — when the workflow.yaml specifies autoSend: false the
// raw agent output must not be streamed to the caller, even when the agent is
// running on the workflow conversation.
bool isWorkflowConversation = context.IsWorkflowConversation(conversationId, out string? workflowConversationId);
Comment on lines +45 to +46
private static string? GetVariableName(PropertyPath variablePath) =>
variablePath.VariableName ?? (variablePath.SegmentCount >= 2 ? variablePath.Segments().ElementAtOrDefault(1).PropertyName : variablePath.SegmentCount == 1 ? variablePath.Segments().ElementAtOrDefault(0).PropertyName : null);
@moonbox3 moonbox3 added .NET workflows Related to Workflows in agent-framework labels May 15, 2026
@github-actions github-actions Bot changed the title Fix declarative workflow regressions for hosted agents .NET: Fix declarative workflow regressions for hosted agents May 15, 2026
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated Code Review

Reviewers: 3 | Confidence: 86%

✓ Correctness

The three regression fixes are logically correct. The autoSend |= isWorkflowConversation removal properly honors caller intent, and both isWorkflowConversation and workflowConversationId remain used at line 51. The PropertyPath workaround via GetVariableName/GetNamespaceAlias correctly reconstructs the variable name and alias from Segments(), and the "Local" → VariableScopeNames.Topic translation ensures IsManagedScope (which relies on VariableScopeNames.IsValidName) succeds, allowing State.Set to execute on the managed path. The existing review thread already covers the key improvement areas (readability, null guarding, magic strings, TODO markers, test coverage). I found no new blocking correctness issues beyond those already flaged.

✓ Test Coverage

This PR fixes three declarative workflow regressions but introduces no new tests for the changed behavior. The autoSend behavioral change in AgentProviderExtensions (removing autoSend |= isWorkflowConversation) has zero test coverage — the only test exercising InvokeAgentAsync (Workflows/InvokeAgent.cs:70) hardcodes autoSend = true and never verifies the autoSend: false + workflow-conversation scenario that this fix is meant to correct. The GetVariableName/GetNamespaceAlias helper test gap was already flagged in the prior review at line 46 and is not re-raised here.

✗ Design Approach

The overall direction looks right, but the PropertyPath regression workaround is incomplete: runtime reads/writes now reconstruct dotted Local.* paths, while workflow initialization still drops those same variables when seding default state. That leaves a real class of declarative workflows partially broken even after this PR.

Flagged Issues

  • The null-PropertyPath workaround is only applied in IWorkflowContextExtensions. DeclarativeWorkflowBuilder still calls state.Initialize(...) during startup (DeclarativeWorkflowBuilder.cs:76), and WorkflowDiagnostics.InitializeDefaults silently skips any variable whose variableDiagnostic.Path.VariableName is null (WorkflowDiagnostics.cs:61-66). Since doted refs like Local.Triage now produce null VariableName, a workflow with a declared default for such a variable will still start blank. The same fallback/remapping needs to be carried into the initialization path.

Automated review by alliscode's agents

// value is honored as-is — when the workflow.yaml specifies autoSend: false the
// raw agent output must not be streamed to the caller, even when the agent is
// running on the workflow conversation.
bool isWorkflowConversation = context.IsWorkflowConversation(conversationId, out string? workflowConversationId);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test for the key behavioral change. The removed autoSend |= isWorkflowConversation line fixes regression #1 (raw JSON leaking when autoSend: false is declared in workflow.yaml), but no test verifies this scenario. The only code exercising InvokeAgentAsync hardcodes autoSend = true. Please add a test that calls InvokeAgentAsync with autoSend: false on a workflow conversation and asserts that no AgentResponseUpdateEvent/AgentResponseEvent is added to the context — otherwise this regression can be silently reintroduced.

// even when SegmentCount==2 and IsValid==true. Reconstruct from Segments() in that case.
private static string? GetVariableName(PropertyPath variablePath) =>
variablePath.VariableName ?? (variablePath.SegmentCount >= 2 ? variablePath.Segments().ElementAtOrDefault(1).PropertyName : variablePath.SegmentCount == 1 ? variablePath.Segments().ElementAtOrDefault(0).PropertyName : null);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initialization path not covered by this workaround. This fixes runtime ReadState/QueueStateUpdateAsync, but DeclarativeWorkflowBuilder seeds state via state.Initialize(...) (DeclarativeWorkflowBuilder.cs:76), and WorkflowDiagnostics.InitializeDefaults does if (variableDiagnostic?.Path?.VariableName is null) { continue; } (WorkflowDiagnostics.cs:61-66). A defaulted variable like Local.Triage will still be silently skipped at startup. Please carry the same fallback/remapping into the initialization path or the fix remains partial.

…; run approved local AIFunctions

Two regressions hit declarative workflows that use require_approval=true when
the client chains turns via previous_response_id (no conversation_id):

1. AgentFrameworkResponseHandler keyed the AgentSession store solely on
   conversation_id, so when only previous_response_id was present the
   StateBag (which holds ToolApprovalIdMap) was discarded after each turn.
   The next turn then threw 'No approval mapping recorded for wire id ...'
   in InputConverter.ConvertMcpApprovalResponse.

   Fix: fall back to previous_response_id on load and to context.ResponseId
   on save so the response-id chain becomes a valid session key. Conversation
   id remains preferred when present.

2. InvokeFunctionToolExecutor.CaptureResponseAsync only acted on
   FunctionResultContent. In the hosted Foundry path the approval response
   arrives as a ToolApprovalResponseContent with no FunctionResultContent,
   so the local AIFunction never ran and downstream PropertyPath/SendActivity
   consumers (e.g. {Local.RefundResult}) saw empty values.

   Fix: when no FunctionResultContent matches but an approved
   ToolApprovalResponseContent does, look up the registered AIFunction by
   name on agentProvider.Functions and invoke it with the evaluated
   arguments, surfacing the result through the existing assignment path.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

.NET workflows Related to Workflows in agent-framework

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants