fix(cli): reject non-numeric --timeout-seconds instead of passing NaN downstream

.changeset/brave-moons-repeat.md

@@ -0,0 +1,9 @@
+---
+"@call-e/cli": patch
+---
+
+Reject duration flags that are not plain numbers, and bound the ones backed by a timer.
+
+`--timeout-seconds 30s` used to resolve to `NaN`, which reached the MCP timeout arithmetic where `Math.max(NaN, 1000)` stays `NaN` and `setTimeout` substitutes 1ms, so every call aborted before it left. Durations are now validated where they are read, with the offending flag named in the error.
+
+The constraints are per option rather than shared: `--min-ttl-seconds 0` keeps working, since zero disables the minimum remaining-lifetime window, while the timeout flags stay strictly positive. Timer-backed values are also capped at 2147483 seconds, because `setTimeout` collapses any longer delay to 1ms and would reproduce the same immediate abort.

.changeset/shaggy-taxis-shout.md

@@ -0,0 +1,7 @@
+---
+"@call-e/core": patch
+---
+
+Guard the MCP request timeout against a non-numeric duration.
+
+`Math.max(NaN, 1000)` is `NaN`, and `setTimeout(fn, NaN)` fires after 1ms, so an unreadable timeout aborted every request immediately instead of falling back to a usable one.

packages/cli/lib/config.js

@@ -45,6 +45,39 @@ function firstOptionValue(value) {
   return Array.isArray(value) ? value[0] : value;
 }
 
+// Node turns any setTimeout delay above this into 1ms, which brings back the very
+// immediate-abort failure this validator exists to prevent: --timeout-seconds 2147484
+// is 2,147,484,000ms, so the request would abort at once instead of waiting.
+// https://nodejs.org/api/timers.html#settimeoutcallback-delay-args
+const MAX_TIMER_DELAY_MS = 2_147_483_647;
+const MAX_TIMER_SECONDS = Math.floor(MAX_TIMER_DELAY_MS / 1000);
+
+// Zero is meaningful for --min-ttl-seconds (it disables the minimum remaining-lifetime
+// window) and meaningless for a timeout, so the bounds are per option rather than shared.
+function secondsOption(value, fallback, flag, { allowZero = false, timerBacked = true } = {}) {
+  const provided = firstOptionValue(value);
+  // Not `value || fallback`: a numeric 0 is falsy, and for --min-ttl-seconds it is a
+  // value the user asked for, not an absent one.
+  const raw = provided === undefined || provided === null || provided === "" ? fallback : provided;
+  const parsed = Number(raw);
+  const wanted = allowZero ? "a non-negative" : "a positive";
+
+  if (!Number.isFinite(parsed) || parsed < 0 || (!allowZero && parsed === 0)) {
+    throw new Error(
+      `${flag} expects ${wanted} number of seconds, got "${raw}". Use "30", not "30s".`,
+    );
+  }
+
+  if (timerBacked && parsed > MAX_TIMER_SECONDS) {
+    throw new Error(
+      `${flag} expects at most ${MAX_TIMER_SECONDS} seconds, got "${raw}". ` +
+        `Node collapses a longer timer to 1ms, which would abort immediately.`,
+    );
+  }
+
+  return parsed;
+}
+
 function isDisabledFlag(value) {
   return ["0", "false", "no", "off", "disabled"].includes(String(value || "").trim().toLowerCase());
 }
@@ -139,16 +172,28 @@ export function resolveRuntimeConfig(options = {}, env = process.env) {
     scope: options.scope || DEFAULT_SCOPE,
     clientName: options.clientName || DEFAULT_CLIENT_NAME,
     cacheRoot: expandHomePath(options.cacheRoot || DEFAULT_CACHE_ROOT),
-    timeoutSeconds: Number(options.timeoutSeconds || DEFAULT_TIMEOUT_SECONDS),
-    pollTimeoutSeconds: Number(options.pollTimeoutSeconds || DEFAULT_POLL_TIMEOUT_SECONDS),
-    minTtlSeconds: Number(options.minTtlSeconds || DEFAULT_MIN_TTL_SECONDS),
+    timeoutSeconds: secondsOption(
+      options.timeoutSeconds,
+      DEFAULT_TIMEOUT_SECONDS,
+      "--timeout-seconds",
+    ),
+    pollTimeoutSeconds: secondsOption(
+      options.pollTimeoutSeconds,
+      DEFAULT_POLL_TIMEOUT_SECONDS,
+      "--poll-timeout-seconds",
+    ),
+    // Not timer-backed, and 0 disables the minimum remaining-lifetime window.
+    minTtlSeconds: secondsOption(options.minTtlSeconds, DEFAULT_MIN_TTL_SECONDS, "--min-ttl-seconds", {
+      allowZero: true,
+      timerBacked: false,
+    }),
     serverName: options.serverName || DEFAULT_SERVER_NAME,
     telemetryEnabled: resolveTelemetryEnabled(options, env),
     telemetryUrl: resolveTelemetryUrl({ telemetryUrl: options.telemetryUrl, baseUrl }, env),
-    telemetryTimeoutSeconds: Number(
-      firstOptionValue(options.telemetryTimeoutSeconds) ||
-        env.CALLE_TELEMETRY_TIMEOUT_SECONDS ||
-        DEFAULT_TELEMETRY_TIMEOUT_SECONDS,
+    telemetryTimeoutSeconds: secondsOption(
+      firstOptionValue(options.telemetryTimeoutSeconds) || env.CALLE_TELEMETRY_TIMEOUT_SECONDS,
+      DEFAULT_TELEMETRY_TIMEOUT_SECONDS,
+      "--telemetry-timeout-seconds",
     ),
   };
 }

packages/cli/test/cli.test.js

@@ -6,7 +6,7 @@ import assert from "node:assert/strict";
 
 import { POST_AUTH_HELP_MESSAGE, preAuthHelpMessage, runCli } from "../lib/cli.js";
 import { pendingCachePath, tokenCachePath, writePrivateJson } from "../lib/cache.js";
-import { CLI_VERSION } from "../lib/config.js";
+import { CLI_VERSION, resolveRuntimeConfig } from "../lib/config.js";
 
 const defaultIntegrationHeader = `cli/cli/${CLI_VERSION}`;
 
@@ -1876,3 +1876,96 @@ test("telemetry opt-out flags and failures do not affect command output", async
   assert.equal(result.code, 0);
   assert.equal(JSON.parse(result.stdout).usable, false);
 });
+
+test("resolveRuntimeConfig rejects durations that are not plain numbers", () => {
+  // "30s" and "1m" are how people write durations, and Number() turns both into
+  // NaN. Previously that NaN reached mcp-client's timeout arithmetic, where
+  // Math.max(NaN, 1000) stays NaN and setTimeout substitutes 1ms, so every MCP
+  // call aborted before it left. Fail at the edge instead, with the flag named.
+  for (const bad of ["30s", "1m", "abc", "-5", "0"]) {
+    assert.throws(
+      () => resolveRuntimeConfig({ timeoutSeconds: bad }),
+      /--timeout-seconds expects a positive number of seconds/,
+      `expected "${bad}" to be rejected`,
+    );
+  }
+
+  assert.throws(
+    () => resolveRuntimeConfig({ pollTimeoutSeconds: "5m" }),
+    /--poll-timeout-seconds expects a positive number of seconds/,
+  );
+  assert.throws(
+    () => resolveRuntimeConfig({ minTtlSeconds: "60s" }),
+    /--min-ttl-seconds expects a non-negative number of seconds/,
+  );
+  assert.throws(
+    () => resolveRuntimeConfig({ telemetryTimeoutSeconds: "1.5s" }, {}),
+    /--telemetry-timeout-seconds expects a positive number of seconds/,
+  );
+});
+
+test("resolveRuntimeConfig keeps --min-ttl-seconds 0 working", () => {
+  // Zero disables the minimum remaining-lifetime window, which is a documented
+  // way to use the flag. The duration validator is strictly positive, so sharing
+  // it across every setting would have taken that away.
+  assert.equal(resolveRuntimeConfig({ minTtlSeconds: "0" }).minTtlSeconds, 0);
+  assert.equal(resolveRuntimeConfig({ minTtlSeconds: 0 }).minTtlSeconds, 0);
+
+  // Still not a free pass: a negative minimum is meaningless.
+  assert.throws(
+    () => resolveRuntimeConfig({ minTtlSeconds: "-1" }),
+    /--min-ttl-seconds expects a non-negative number of seconds/,
+  );
+
+  // And zero stays rejected where it would mean "abort immediately".
+  assert.throws(
+    () => resolveRuntimeConfig({ timeoutSeconds: "0" }),
+    /--timeout-seconds expects a positive number of seconds/,
+  );
+});
+
+test("resolveRuntimeConfig rejects timer values Node would collapse to 1ms", () => {
+  // setTimeout stores its delay in a signed 32-bit int. Anything over
+  // 2,147,483,647ms is silently replaced by 1ms, so a very large --timeout-seconds
+  // recreated the same immediate-abort bug the validator was added to stop.
+  const maxSeconds = Math.floor(2_147_483_647 / 1000); // 2147483
+
+  assert.equal(resolveRuntimeConfig({ timeoutSeconds: String(maxSeconds) }).timeoutSeconds, maxSeconds);
+
+  for (const flag of ["timeoutSeconds", "pollTimeoutSeconds"]) {
+    assert.throws(
+      () => resolveRuntimeConfig({ [flag]: String(maxSeconds + 1) }),
+      /expects at most 2147483 seconds/,
+      `expected ${flag} to reject ${maxSeconds + 1}`,
+    );
+  }
+
+  assert.throws(
+    () => resolveRuntimeConfig({ telemetryTimeoutSeconds: String(maxSeconds + 1) }, {}),
+    /expects at most 2147483 seconds/,
+  );
+
+  // --min-ttl-seconds never reaches setTimeout, so it is not bounded by it.
+  assert.equal(
+    resolveRuntimeConfig({ minTtlSeconds: String(maxSeconds + 1) }).minTtlSeconds,
+    maxSeconds + 1,
+  );
+});
+
+test("resolveRuntimeConfig keeps accepting valid values and defaults", () => {
+  const explicit = resolveRuntimeConfig({ timeoutSeconds: "30" });
+  assert.equal(explicit.timeoutSeconds, 30);
+
+  const defaults = resolveRuntimeConfig({}, {});
+  assert.equal(Number.isFinite(defaults.timeoutSeconds), true);
+  assert.equal(Number.isFinite(defaults.pollTimeoutSeconds), true);
+  assert.equal(Number.isFinite(defaults.minTtlSeconds), true);
+  assert.equal(Number.isFinite(defaults.telemetryTimeoutSeconds), true);
+
+  // Falsy values still fall back to the default, as before.
+  assert.equal(resolveRuntimeConfig({ timeoutSeconds: "" }).timeoutSeconds, defaults.timeoutSeconds);
+  assert.equal(
+    resolveRuntimeConfig({ timeoutSeconds: undefined }).timeoutSeconds,
+    defaults.timeoutSeconds,
+  );
+});

packages/core/lib/mcp-client.js

@@ -142,7 +142,7 @@ function accessTokenFromCache(config) {
 async function openMcpSession({ config, fetchImpl }) {
   requireFetch(fetchImpl);
   const accessToken = accessTokenFromCache(config);
-  const timeoutMs = Math.max(Math.ceil(config.timeoutSeconds * 1000), 1000);
+  const timeoutMs = Math.max(Math.ceil(Number(config.timeoutSeconds || 15) * 1000), 1000);
   const commonHeaders = {
     Accept: "application/json, text/event-stream",
     "Content-Type": "application/json",