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.

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

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