fix(core): bound the MCP request timeout before arming setTimeout - #83
Open
zkasuran wants to merge 1 commit into
Open
fix(core): bound the MCP request timeout before arming setTimeout#83zkasuran wants to merge 1 commit into
zkasuran wants to merge 1 commit into
Conversation
The public callMcpTool timeoutSeconds override and a caller-built config.timeoutSeconds reached setTimeout without an upper bound, so a value above 2147483 seconds or a non-finite one collapsed the delay to 1ms and aborted the request at once. Clamp both timer sites through one helper: cap oversized values at the setTimeout ceiling and fall back to the session timeout for unusable ones. This is the cap the CLI already applies to --timeout-seconds, now applied in the transport too.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@call-e/core0.3.0 gavecallMcpToola publictimeoutSecondsoverride that the CLI passes as 150 forplan_call. That value, together with aconfig.timeoutSecondsbuilt by any direct@call-e/coreconsumer, reaches the timer arithmetic inmcp-client.jswithout an upper bound:setTimeoutcollapses any delay above 2147483647ms (about 24.8 days) to 1ms, so atimeoutSecondsabove 2147483 aborts the request almost immediately instead of waiting. A non-finite or non-positive value has the same effect, becauseMath.max(NaN, 1000)staysNaNandsetTimeouttreats that as 1ms.This is the immediate-abort failure PR #73 fixed for
--timeout-secondsat the flag boundary. The CLI validator caps that flag atMAX_TIMER_SECONDSand rejects non-finite durations, but the core transport does its own arithmetic, so the new public override and a caller-built config skip that guard.Fix
One helper bounds both timer sites in
packages/core/lib/mcp-client.js:setTimeoutcan hold rather than firing at 1ms.openMcpSessionandcallMcpToolboth call it, soconfig.timeoutSecondsand the per-call override are bounded the same way. No public surface changes:timeoutSecondsis still an optional number.Tests
packages/core/test/core.test.jsgains four cases that read the delay handed tosetTimeoutfor each request:config.timeoutSecondsis capped for the handshake and the list callThe first three fail without the change. Reverting only
mcp-client.jswhile keeping the tests:Verification
From the repository root on Node 22.22.2 with pnpm 10.18.3.
The single skip is the pre-existing Windows-only case. No CALL-E account or live call is involved; every case drives an injected
fetchImpl.AI assistance
AI (Claude, Anthropic) was used in developing this change. The design, review and verification were done by the author. Verified locally before submitting:
pnpm test,pnpm checkandpnpm pack:dry-runall pass on Node 22.22.2 with pnpm 10.18.3.