Repository navigation
fix(core): accept numeric MCP timeouts - #55851
Conversation
A bare number timeout under mcp.servers dropped the whole server. Numeric timeouts now expand to startup, catalog, and execution everywhere: per server, the global mcp.timeout, experimental.mcp_timeout, and V1 entries. Co-authored-by: dajiaohuang <[email protected]>
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Looks good. I ran the same config through ConfigNormalize.normalize on the base and on this branch. On the base, a server with "timeout": 60000 under mcp.servers is dropped, and a numeric mcp.timeout is ignored. With this change both expand to { startup, catalog, execution }, object-form timeouts are unchanged, and -1 is still rejected for that server. In packages/core, the config and v1 tests pass (267) and typecheck is clean.
Public API: config only, backwards compatible.
mcp.servers.<name>.timeoutandmcp.timeoutnow also accept a positive integer, expanded to all three phases.- Flat V1
mcp.<name>.timeoutandexperimental.mcp_timeoutnow also settimeout.startup. Existing configs keep loading. The difference is that a numeric value now replaces the 30s startup default.
This also covers #54184. PR #54174 overlaps with the V1 migration part.
One point that doesn't block the merge is in an inline comment on experimental.mcp_timeout.
| diagnostics, | ||
| ) | ||
| if (value !== undefined) { | ||
| timeout.startup = value |
There was a problem hiding this comment.
In V1, experimental.mcp_timeout was only the fallback timeout for tool calls (callTool). It never applied to connecting. The per-server timeout did cover connect and listing tools, so adding startup there matches V1. For this global key, though, it means a user who set a small value (e.g. 5000) to cap tool calls now also gets a 5s connect budget instead of the 30s default, so slow-starting servers could begin failing. Is that intended? If not, consider leaving startup out for this key.
There was a problem hiding this comment.
Good catch: V1 used experimental.mcp_timeout only as the fallback for tool calls and listing (requestTimeout), never for connect. I dropped startup for this key in 8d2ee6b, so it maps to { catalog, execution } as before. Per-server numeric timeouts still set startup, which matches V1's connectTimeout = mcp.timeout.
There was a problem hiding this comment.
I checked 8d2ee6b and it's correct. experimental.mcp_timeout now sets only catalog and execution (normalize.ts ~L260), so connecting keeps the 30s default. Per-server numeric timeouts still set startup, which matches V1's connectTimeout = mcp.timeout.
- Tests: I ran
bun test test/config test/v1inpackages/coreonpr/headand all 267 passed. I used a separate worktree at/tmp/opencode/wt55851, because the sharedrepo/checkout is still on 959d824. I didn't change anything inrepo/. - PR description: one row is now out of date. The table still says
experimental.mcp_timeout: n→ "Addsstartup". It should say it's unchanged ({ catalog, execution }). The "Public API" line in the bot section makes the same claim.
Fixes #50807
Fixes #54184
Related: #55419
A numeric
timeouton a server undermcp.servers(e.g."timeout": 60000, the V1 form) failed schema decoding, so the whole server was silently dropped. The only trace was askipped malformed recognized valuewarning in the log.A bare-number timeout on a server or on the global
mcp.timeoutnow expands to{ startup, catalog, execution }:mcp.servers.<name>.timeout: n{ startup: n, catalog: n, execution: n }mcp.timeout: n(global)mcp.<name>.timeout: n{ catalog, execution }startup, matching V1, where this timeout also bounded connectingexperimental.mcp_timeout: n{ catalog, execution }The object form is unchanged and remains the documented shape. Invalid numbers (e.g.
-1) still fail decoding for that server.This builds on @dajiaohuang's #50810, which targeted the since-removed
packages/opencodeV2 compat layer. It's credited as co-author. #54174 overlaps with the flat V1 migration part.Tests
packages/core:bun test test/config test/v1(267 pass),bun typecheck.