Skip to content

Address async review feedback: Emit indirection, FlushSafe logging, CancellationToken propagation in McpClientPlugin#403

Merged
rido-min merged 2 commits intofix/async-issues-396-401from
copilot/sub-pr-402
Mar 26, 2026
Merged

Address async review feedback: Emit indirection, FlushSafe logging, CancellationToken propagation in McpClientPlugin#403
rido-min merged 2 commits intofix/async-issues-396-401from
copilot/sub-pr-402

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 26, 2026

Three issues flagged in review of the async anti-pattern fixes PR were left unaddressed.

Changes

  • Stream.Emit – Remove unnecessary EmitAsync indirection; call onChunk(text).GetAwaiter().GetResult() directly. ConfigureAwait(false) on a synchronously-blocked call has no effect.

  • AspNetCorePlugin.Stream / FlushSafe – Add optional ILogger? Logger property to the nested Stream class. CreateStream wires it to Logger.Child("stream"). FlushSafe now logs at Warn with the full exception before scheduling the retry timer, making send failures diagnosable in production.

  • McpClientPlugin – Thread CancellationToken from OnBuildFunctions through the full discovery call chain:

    OnBuildFunctions(cancellationToken)
      → FetchToolsIfNeeded(cancellationToken)
        → FetchToolsFromServer(..., cancellationToken)
          → McpClientFactory.CreateAsync(..., cancellationToken: cancellationToken)
          → client.ListToolsAsync(cancellationToken: cancellationToken)
    

    Previously the token was accepted but never forwarded, so callers could not cancel in-flight tool discovery.


⌨️ Start Copilot coding agent tasks without leaving your editor — available in VS Code, Visual Studio, JetBrains IDEs and Eclipse.

…ns, thread CancellationToken through McpClientPlugin

Agent-Logs-Url: https://github.com/microsoft/teams.net/sessions/7c6b11c0-cbde-47f8-aaec-5a6aad475a5e

Co-authored-by: rido-min <14916339+rido-min@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 26, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • token.botframework.com
    • Triggering command: /usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/teams.net/teams.net/Tests/Microsoft.Teams.Apps.Tests/bin/Debug/net10.0/Microsoft.Teams.Apps.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/teams.net/teams.net/Tests/Microsoft.Teams.Apps.Tests/bin/Debug/net10.0/Microsoft.Teams.Apps.Tests.deps.json /home/REDACTED/work/teams.net/teams.net/Tests/Microsoft.Teams.Apps.Tests/bin/Debug/net10.0/testhost.dll --port 38797 --endpoint 127.0.0.1:038797 --role client --parentprocessid 5781 --telemetryoptedin false (dns block)
    • Triggering command: /usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/teams.net/teams.net/Tests/Microsoft.Teams.Apps.Tests/bin/Debug/net8.0/Microsoft.Teams.Apps.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/teams.net/teams.net/Tests/Microsoft.Teams.Apps.Tests/bin/Debug/net8.0/Microsoft.Teams.Apps.Tests.deps.json /home/REDACTED/work/teams.net/teams.net/Tests/Microsoft.Teams.Apps.Tests/bin/Debug/net8.0/testhost.dll --port 44581 --endpoint 127.0.0.1:044581 --role client --parentprocessid 5782 --telemetryoptedin false (dns block)
    • Triggering command: /usr/share/dotnet/dotnet /usr/share/dotnet/dotnet exec --runtimeconfig /home/REDACTED/work/teams.net/teams.net/Tests/Microsoft.Teams.Apps.Tests/bin/Debug/net8.0/Microsoft.Teams.Apps.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/teams.net/teams.net/Tests/Microsoft.Teams.Apps.Tests/bin/Debug/net8.0/Microsoft.Teams.Apps.Tests.deps.json /home/REDACTED/work/teams.net/teams.net/Tests/Microsoft.Teams.Apps.Tests/bin/Debug/net8.0/testhost.dll --port 33971 --endpoint 127.0.0.1:033971 --role client --parentprocessid 6665 --telemetryoptedin false n (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [WIP] Fix async anti-patterns across all library projects Address async review feedback: Emit indirection, FlushSafe logging, CancellationToken propagation in McpClientPlugin Mar 26, 2026
Copilot AI requested a review from rido-min March 26, 2026 17:57
@rido-min rido-min marked this pull request as ready for review March 26, 2026 19:34
@rido-min rido-min merged commit 9ba530e into fix/async-issues-396-401 Mar 26, 2026
1 check passed
@rido-min rido-min deleted the copilot/sub-pr-402 branch March 26, 2026 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants