Skip to content

fix(mcp): retire pooled connections after consecutive request timeouts#5821

Merged
waleedlatif1 merged 2 commits into
stagingfrom
fix/mcp-timeout-circuit-breaker
Jul 21, 2026
Merged

fix(mcp): retire pooled connections after consecutive request timeouts#5821
waleedlatif1 merged 2 commits into
stagingfrom
fix/mcp-timeout-circuit-breaker

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Follow-up to fix(mcp): keep the pooled connection warm on a request timeout #5817, closing the P2 raised there ("Timed-Out Transport Remains Reusable"): a genuinely half-open transport could keep serving repeated 30s timeouts until the pool's liveness ping caught it.
  • Adds a consecutive-timeout circuit breaker to the pool: a lone request timeout still keeps the session warm (retiring on every timeout is what caused the connect/stall/reconnect churn fix(mcp): keep the pooled connection warm on a request timeout #5817 fixed), but two consecutive timeouts with no healthy request in between retire the connection. Any healthy release resets the count.
  • withServerClient now reports sawTimeout on release; dead-connection errors still poison immediately, benign errors still keep the connection warm.
  • Matches the LibreChat pattern (circuit breaker counts connection-level failures, not single request timeouts); the MCP SDK/OpenCode behavior for a lone timeout is unchanged.

Type of Change

  • Bug fix (resilience hardening)

Testing

  • 3 new pool tests: lone timeout keeps the connection; consecutive timeouts retire it (fresh connection on next acquire); healthy release resets the streak.
  • Service test updated: timeout release reports (poison=false, sawTimeout=true).
  • All 25 pool + service-pool tests green; tsc + biome clean.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@waleedlatif1
fix(mcp): retire pooled connections after consecutive request timeouts
88c64c4 
Circuit breaker for the half-open-transport gap left by #5817: a lone timeout
still keeps the session warm (retiring on every timeout caused the
connect/stall/reconnect churn), but two consecutive timeouts with no healthy
request in between retire the connection so a genuinely half-open transport
can't serve repeated 30s failures until the liveness ping catches it. A healthy
release resets the count. Matches the LibreChat pattern of counting
connection-level failures rather than reacting to one.
@vercel

vercel Bot commented Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jul 21, 2026 10:26pm

Request Review

@cursor

cursor Bot commented Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes warm-connection lifecycle for MCP tool execution and discovery; behavior is bounded and well-tested but affects production resilience under flaky transports.

Overview
Adds a consecutive-timeout circuit breaker on the MCP connection pool so a half-open transport cannot keep serving repeated slow failures until liveness ping catches it.

A single request timeout still keeps the session warm (avoiding the connect/stall/reconnect churn from retiring on every timeout). release now accepts sawTimeout: that increments a per-connection streak, and two consecutive timeouts with no successful release in between retire the pooled client; any healthy (poison=false) release resets the count. Dead-connection errors still poison immediately.

withServerClient reports timeouts via lease.release(poison, sawTimeout) instead of treating them as dead. isTimeoutError also recognizes TimeoutError (including AbortSignal.timeout / undici shapes whose message lacks "timed out"). Pool and service-pool tests cover lone timeout, double-strike retirement, streak reset, and timeout classification.

Reviewed by Cursor Bugbot for commit 67618a9. Configure here.

@greptile-apps

greptile-apps Bot commented Jul 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a timeout circuit breaker for pooled MCP connections. The main changes are:

  • Retires a pooled connection after two consecutive request timeouts.
  • Resets the timeout count after a healthy request.
  • Passes timeout status from MCP service calls into lease release.
  • Expands timeout detection to named TimeoutError values and wrapped causes.
  • Adds pool and service tests for the new behavior.

Confidence Score: 5/5

The updated timeout handling looks safe to merge.

  • The named timeout case from the earlier review is now classified correctly.
  • Timeout status reaches the pool through the single production lease-release path.
  • The counter, retirement, and reset behavior have focused test coverage.
  • No remaining blocking issue was confirmed in the changed code.

Important Files Changed

Filename Overview
apps/sim/lib/mcp/connection-pool.ts Adds per-connection timeout tracking, retirement after two consecutive timeouts, and reset after a healthy release.
apps/sim/lib/mcp/service.ts Classifies named timeout errors and reports timeout status when releasing pooled connections.
apps/sim/lib/mcp/connection-pool.test.ts Adds coverage for single timeouts, consecutive timeout retirement, and healthy-request resets.
apps/sim/lib/mcp/service-pool.test.ts Updates release expectations and tests named timeout error classification.

Reviews (2): Last reviewed commit: "fix(mcp): classify TimeoutError-named ab..." | Re-trigger Greptile

greptile-apps[bot]
greptile-apps Bot reviewed Jul 21, 2026
View reviewed changes
Comment thread apps/sim/lib/mcp/service.ts
@waleedlatif1
fix(mcp): classify TimeoutError-named aborts as timeouts for the circ…
67618a9 
…uit breaker

AbortSignal.timeout / undici surface a DOMException named TimeoutError whose
message lacks 'timed out'; without this the breaker treated it as a healthy
release and reset the streak. Adds name-based detection (incl. cause) + test.
@waleedlatif1

waleedlatif1 commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

waleedlatif1 commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator Author

@cursor review

cursor[bot]
cursor Bot reviewed Jul 21, 2026
View reviewed changes

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 67618a9. Configure here.

@waleedlatif1
waleedlatif1 merged commit 6f96d4a into staging Jul 21, 2026
20 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/mcp-timeout-circuit-breaker branch July 22, 2026 00:02
@waleedlatif1 waleedlatif1 mentioned this pull request Jul 22, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments
@cursor cursor[bot] cursor[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@waleedlatif1