Skip to content

Fix HttpClient timeout for long-running tools without event stream store#1268

Merged
stephentoub merged 9 commits intomainfrom
copilot/add-regression-test-for-timeouts
Feb 14, 2026
Merged

Fix HttpClient timeout for long-running tools without event stream store#1268
stephentoub merged 9 commits intomainfrom
copilot/add-regression-test-for-timeouts

Conversation

Copy link
Contributor

Copilot AI commented Feb 13, 2026

  • Remove unused System.ComponentModel using from MapMcpStreamableHttpTests
  • Refactor StartMcpServerAsync to accept optional middleware configuration
  • Remove try/catch blocks from OAuth tests (overly defensive)
  • Simplify OAuth test implementations using StartMcpServerAsync
  • Fix DeleteRequest_CompletesSession_WhichCancelsLongRunningToolCalls test:
    • Use HttpCompletionOption.ResponseHeadersRead for POST requests
    • Await 200 response headers from all tool calls before DELETE
    • Verify reading response body throws HttpRequestException
  • Address code review feedback:
    • Rename variable to responseHeaders for clarity
    • Use ReadAsByteArrayAsync instead of stream read to avoid CA2022

All feedback addressed. All tests passing.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 3 commits February 13, 2026 22:47
…tool calls

Co-authored-by: halter73 <54385+halter73@users.noreply.github.com>
Co-authored-by: halter73 <54385+halter73@users.noreply.github.com>
Co-authored-by: halter73 <54385+halter73@users.noreply.github.com>
Copilot AI changed the title [WIP] Add regression test for tool call timeout issue Fix HttpClient timeout for long-running tools without event stream store Feb 13, 2026
Copilot AI requested a review from halter73 February 13, 2026 23:11
Copy link
Contributor

@halter73 halter73 left a comment

Choose a reason for hiding this comment

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

@copilot It looks like AuthorizationFlow_UsesScopeFromForbiddenHeader is now failing because it tries to change the status code in the tool call handler after we've already flushed the status and response headers leading to an InvalidOperationException. Please update this test to use middleware along with HttpReqest.EnableBuffering() to manually read the incoming JsonRpcMessages using McpJsonUtilities and then setting the 403 and short circuiting for the appropriate tool call.

Copilot AI and others added 2 commits February 13, 2026 23:37
Co-authored-by: halter73 <54385+halter73@users.noreply.github.com>
Co-authored-by: halter73 <54385+halter73@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 13, 2026

@copilot It looks like AuthorizationFlow_UsesScopeFromForbiddenHeader is now failing because it tries to change the status code in the tool call handler after we've already flushed the status and resp...

Done in d8dce51. Updated both AuthorizationFlow_UsesScopeFromForbiddenHeader and CanAuthenticate_WithTokenRefresh tests to use middleware with HttpRequest.EnableBuffering() to manually read and check JsonRpcMessages before MapMcp processes them. The middleware now returns 403/401 responses before the flush happens.

Copilot AI and others added 3 commits February 14, 2026 00:20
…eter

Co-authored-by: halter73 <54385+halter73@users.noreply.github.com>
Co-authored-by: halter73 <54385+halter73@users.noreply.github.com>
…yteArrayAsync

Co-authored-by: halter73 <54385+halter73@users.noreply.github.com>
Copilot AI requested a review from halter73 February 14, 2026 00:33
@halter73 halter73 marked this pull request as ready for review February 14, 2026 01:29
@stephentoub stephentoub merged commit fb2daf2 into main Feb 14, 2026
10 checks passed
@stephentoub stephentoub deleted the copilot/add-regression-test-for-timeouts branch February 14, 2026 15:19
@jeffhandley jeffhandley added this to the Stable public API milestone Feb 17, 2026
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.

4 participants