Skip to content

.NET: Fix workflow session bug#7032

Merged
peibekwe merged 1 commit into
mainfrom
peibekwe/workflow-bugfix
Jul 10, 2026
Merged

.NET: Fix workflow session bug#7032
peibekwe merged 1 commit into
mainfrom
peibekwe/workflow-bugfix

Conversation

@peibekwe

@peibekwe peibekwe commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Motivation & Context

Fix workflow session bug.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

@peibekwe peibekwe self-assigned this Jul 9, 2026
Copilot AI review requested due to automatic review settings July 9, 2026 23:08
@giles17 giles17 added .NET Usage: [Issues, PRs], Target: .Net workflows Usage: [Issues, PRs], Target: Workflows labels Jul 9, 2026
@github-actions github-actions Bot changed the title Fix workflow session bug .NET: Fix workflow session bug Jul 9, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a .NET workflow session issue where external request “envelope” payloads could fail to be recognized/rehydrated when the recorded request type’s assembly identity (notably version) no longer matches what’s loaded at runtime. The fix shifts envelope-type resolution to the live workflow port declarations and validates type identity via TypeId.IsMatch(...) (which normalizes away assembly version/culture/token data).

Changes:

  • Updated WorkflowSession to resolve potential envelope payload types from the workflow’s live request ports (and only treat a payload as an envelope when the recorded TypeId matches the port’s declared request type).
  • Removed the previous ResolveTypeLenient + cache approach that relied on partial-name binding (Type.GetType(...) with a “simple” assembly name).
  • Consolidated and expanded unit tests to cover envelope resolution/matching across assembly-version and generic-argument version mutations, plus negative cases.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
dotnet/src/Microsoft.Agents.AI.Workflows/WorkflowSession.cs Reworks envelope detection to use live workflow port types and TypeId.IsMatch(...); removes ResolveTypeLenient and the associated cache.
dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowSessionTests.cs Adds updated unit coverage for envelope type resolution and envelope extraction based on port/type matching.
dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowSessionResolveTypeLenientTests.cs Deletes superseded tests that targeted the removed ResolveTypeLenient behavior.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Automated Code Review

Reviewers: 5 | Confidence: 90%

✓ Correctness

The PR replaces a static Type.GetType-based type resolution (ResolveTypeLenient) with a port-based resolution (ResolveEnvelopeType) that looks up the request type from the workflow's live port definitions. This is a sound architectural improvement: instead of resolving types by assembly-qualified name (which is fragile across version changes), the new code uses the port ID to find the port, then verifies the recorded TypeId matches the port's declared request type via TypeId.IsMatch (which ignores version/culture/publicKeyToken). The static-to-instance method conversions are correct and access this._workflow.Ports which is an init-only Dictionary<string, RequestPort> (read-only after construction, so thread-safe for concurrent reads). The ConcurrentDictionary cache removal is fine since the dictionary lookup + IsMatch is cheap. Tests comprehensively cover matching, version mutation, unknown port, type mismatch, and envelope detection scenarios. No correctness issues found.

✓ Security Reliability

This PR replaces Type.GetType-based dynamic type resolution (ResolveTypeLenient) with port-based type lookup (ResolveEnvelopeType), which is a net security improvement. The old approach could resolve arbitrary types from any loaded assembly given a controlled TypeId in a checkpointed payload; the new approach constrains resolution to types explicitly declared by the workflow's request ports. The TypeId.IsMatch method provides the same version-tolerant matching (comparing simple assembly names and normalized type names, ignoring version/culture/public key token). Methods that were previously static now correctly access this._workflow.Ports through instance dispatch. Test coverage is comprehensive, covering matching, version mutation, unknown port, and type mismatch scenarios. No security or reliability issues found.

✓ Test Coverage

The PR replaces the old ResolveTypeLenient approach (which used Type.GetType with partial-name binding) with a new ResolveEnvelopeType approach that resolves types from the live workflow ports. The test file was properly renamed and expanded. The new tests cover ResolveEnvelopeType with 5 cases (happy path, assembly version mutation, generic argument version mutation, unknown port ID, type mismatch) and TryGetRequestEnvelope with 3 cases (happy path, non-envelope port type, type mismatch). All key behavioral paths are tested with meaningful assertions. The only untested path is the second guard in TryGetRequestEnvelope (line 308) where TryGetDataAs succeds but the result is not an IExternalRequestEnvelope, which is a minor gap since the first guard already filters non-envelope types. Overall, test coverage is adequate for this change.

✓ Failure Modes

This PR replaces a global Type.GetType-based type resolution (ResolveTypeLenient) with port-scoped resolution (ResolveEnvelopeType) that looks up the concrete type from the workflow's registered RequestPort dictionary. The new approach is safer: it only recognizes a request as an envelope when the port the request was sent through actually declares an envelope type, avoiding silent misresolution. TypeId.IsMatch handles version-mutation tolerance. The static ConcurrentDictionary cache is removed in favor of dictionary lookups on each call, which is a minor performance trade-off but not a failure mode. No silent failure paths, swallowed exceptions, or missing cleanup were introduced.

✗ Design Approach

The new port-based envelope resolution is consistent with checkpoint matching, but the added test suite now codifies a narrower contract than IExternalRequestEnvelope currently documents. That leaves a real design mismatch for valid requests whose payload implements the envelope interface but whose port is declared as a broader assignable type.

Flagged Issues

  • The new tests lock in "only when the port declares the envelope type" semantics, but IExternalRequestEnvelope still documents envelope handling based on the runtime payload itself (dotnet/src/Microsoft.Agents.AI.Workflows/IExternalRequestEnvelope.cs:22-27). Because ExternalRequest.Create accepts any payload assignable to the port type (dotnet/src/Microsoft.Agents.AI.Workflows/ExternalRequest.cs:49-60), a valid request such as a RequestPort declared as object carrying an envelope instance would now silently lose envelope unwrapping/rewrapping behavior.

Automated review by peibekwe's agents

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Flagged issue

The new tests lock in "only when the port declares the envelope type" semantics, but IExternalRequestEnvelope still documents envelope handling based on the runtime payload itself (dotnet/src/Microsoft.Agents.AI.Workflows/IExternalRequestEnvelope.cs:22-27). Because ExternalRequest.Create accepts any payload assignable to the port type (dotnet/src/Microsoft.Agents.AI.Workflows/ExternalRequest.cs:49-60), a valid request such as a RequestPort declared as object carrying an envelope instance would now silently lose envelope unwrapping/rewrapping behavior.


Source: automated DevFlow PR review

@peibekwe peibekwe marked this pull request as ready for review July 10, 2026 00:37

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Automated Code Review

Reviewers: 5 | Confidence: 92% | Result: All clear

Reviewed: Correctness, Security Reliability, Test Coverage, Failure Modes, Design Approach


Automated review by peibekwe's agents

@peibekwe peibekwe added this pull request to the merge queue Jul 10, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 10, 2026
@peibekwe peibekwe added this pull request to the merge queue Jul 10, 2026
Merged via the queue into main with commit 737042f Jul 10, 2026
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

.NET Usage: [Issues, PRs], Target: .Net workflows Usage: [Issues, PRs], Target: Workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants