feat: per-service timeout in services.json

[rodaddy]

Summary

• Adds optional timeout field to service config schema (shared across stdio/http/websocket backends)
• Daemon reads per-service timeout before falling back to MCP2CLI_TOOL_TIMEOUT env var, then 30s default
• Minimal change: 2 files, 7 lines added

Context

Discovered while integrating Apple services -- mcp-apple-calendar (AppleScript) took >60s to list events. Swapped to mcp-ical (EventKit, ~3s) but the per-service timeout is still useful for other slow backends.

Test plan

• Verify services.json with timeout field passes validation
• Verify tool calls respect per-service timeout over global default

🤖 Generated with Claude Code

[github-actions]

Claude finished @rodaddy's task —— View job

---

PR Review: feat/per-service-timeout

• Gather context and read changed files
• Analyze for bugs, security issues, resource leaks, error handling, TypeScript strictness
• Post review feedback

---

Overall this is a clean, minimal change with good schema validation. A few issues worth addressing:

---

Bug: parseInt on env var can produce NaN, causing immediate timeouts

src/daemon/server.ts:114

const timeout = perServiceTimeout ?? parseInt(process.env.MCP2CLI_TOOL_TIMEOUT ?? "30000", 10);

If MCP2CLI_TOOL_TIMEOUT is set to a non-numeric string (e.g. "disabled", "1m"), parseInt returns NaN. Passing NaN to setTimeout causes it to fire immediately (treated as 0), so every tool call will instantly time out until the daemon restarts. This is a silent footgun.

const envTimeout = parseInt(process.env.MCP2CLI_TOOL_TIMEOUT ?? "", 10);
const timeout = perServiceTimeout ?? (Number.isFinite(envTimeout) && envTimeout > 0 ? envTimeout : 30000);

---

Minor memory leak: abort listener never removed

src/daemon/server.ts:126

controller.signal.addEventListener("abort", () =>
  reject(new ToolError(...))
);

The listener is added without { once: true }. When the tool call succeeds before the timeout fires, controller.abort() is never called so the listener stays on the signal object for its lifetime. Add { once: true }:

controller.signal.addEventListener("abort", () =>
  reject(new ToolError(...)),
  { once: true }
);

---

Redundant "timeout" in serviceConfig check

src/daemon/server.ts:113

const perServiceTimeout = serviceConfig && "timeout" in serviceConfig ? serviceConfig.timeout : undefined;

Since all three service schema variants include timeout as an optional field (via accessControlFields), TypeScript already knows serviceConfig.timeout exists (typed as number | undefined). The "timeout" in serviceConfig guard is always true for valid config objects and doesn't add type narrowing value. Simpler:

const perServiceTimeout = serviceConfig?.timeout;

---

TOCTOU: getConfig() called twice

src/daemon/server.ts:108,112

const conn = await pool.getConnection(body.service, getConfig());  // line 108
const serviceConfig = getConfig().services[body.service];           // line 112

getConfig() is called twice across an await. With live config reloading (configManager), the config could change between the two calls — e.g., the service's timeout could be updated between connection acquisition and timeout lookup. Low probability but worth snapshotting:

const currentConfig = getConfig();
const conn = await pool.getConnection(body.service, currentConfig);
const serviceConfig = currentConfig.services[body.service];

---

Schema: no upper bound on timeout

src/config/schema.ts:13

timeout: z.number().int().positive().optional(),

A user could set timeout: 9007199254740991. Consider .max(3_600_000) (1 hour) or similar to prevent accidental misconfiguration tying up connections indefinitely.

---

Summary

Severity	Issue
Bug	NaN from bad MCP2CLI_TOOL_TIMEOUT causes instant timeouts
Minor	Abort listener not marked { once: true }
Cleanup	Redundant "timeout" in serviceConfig guard
Minor	TOCTOU double getConfig() across await
Nit	No upper bound on schema timeout

The schema addition in schema.ts is clean and correct. The core timeout logic is sound — just the env var fallback path and the listener leak need addressing.