Skip to content

fix(core): bound the MCP request timeout before arming setTimeout - #83

Open
zkasuran wants to merge 1 commit into
CALLE-AI:mainfrom
zkasuran:fix/bound-mcp-request-timeout
Open

fix(core): bound the MCP request timeout before arming setTimeout#83
zkasuran wants to merge 1 commit into
CALLE-AI:mainfrom
zkasuran:fix/bound-mcp-request-timeout

Conversation

@zkasuran

Copy link
Copy Markdown

Summary

@call-e/core 0.3.0 gave callMcpTool a public timeoutSeconds override that the CLI passes as 150 for plan_call. That value, together with a config.timeoutSeconds built by any direct @call-e/core consumer, reaches the timer arithmetic in mcp-client.js without an upper bound:

Math.max(Math.ceil(Number(timeoutSeconds) * 1000), 1000)

setTimeout collapses any delay above 2147483647ms (about 24.8 days) to 1ms, so a timeoutSeconds above 2147483 aborts the request almost immediately instead of waiting. A non-finite or non-positive value has the same effect, because Math.max(NaN, 1000) stays NaN and setTimeout treats that as 1ms.

This is the immediate-abort failure PR #73 fixed for --timeout-seconds at the flag boundary. The CLI validator caps that flag at MAX_TIMER_SECONDS and 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:

  • Values above 2147483647ms are capped at that ceiling, so a long timeout waits as long as setTimeout can hold rather than firing at 1ms.
  • Non-finite or non-positive values fall back to the session timeout the handshake already computed, rather than to a near-instant abort.
  • In-range values are unchanged. The existing lower clamp of 1000ms is kept.

openMcpSession and callMcpTool both call it, so config.timeoutSeconds and the per-call override are bounded the same way. No public surface changes: timeoutSeconds is still an optional number.

Tests

packages/core/test/core.test.js gains four cases that read the delay handed to setTimeout for each request:

  • an oversized per-call override (5000000s) is capped at 2147483647ms
  • a non-finite override falls back to the 20s session timeout
  • an oversized config.timeoutSeconds is capped for the handshake and the list call
  • an in-range 150s override is passed through unchanged

The first three fail without the change. Reverting only mcp-client.js while keeping the tests:

not ok 13 - MCP client clamps an oversized tool-call timeout so the request is not aborted immediately
not ok 14 - MCP client falls back to the session timeout when a tool-call override is unusable
not ok 15 - MCP client clamps an oversized session timeout from config
# tests 16  # pass 13  # fail 3

Verification

From the repository root on Node 22.22.2 with pnpm 10.18.3.

pnpm test           exit 0   core 24/24, cli 58 (57 pass, 1 skip) + 15/15
pnpm check          exit 0   tsc --noEmit clean, syntax check clean
pnpm pack:dry-run   exit 0

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 check and pnpm pack:dry-run all pass on Node 22.22.2 with pnpm 10.18.3.

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.
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.

1 participant