feat(core): allow mcp.json to override connect/discover timeouts#136
Merged
Conversation
github-actions
Bot
added
area: cli
bug-ops
enabled auto-merge (squash)
July 9, 2026 15:38
ServerConfig's connect_timeout/discover_timeout (added in #120) were only reachable through the builder, so every server configured via mcp.json or the mcp-server MCP tool was locked to the 30s defaults. Add optional connectTimeoutSecs/discoverTimeoutSecs to McpServerEntry and IntrospectServerParams, wired into the existing builder setters only when present so omitting them preserves the current defaults. Bounds (0 < timeout <= 600s) are enforced once in validate_server_config, the single gate both the CLI and mcp-server paths already funnel through via discover_server, so the check can't be bypassed. Map validation failures on the MCP introspect_server path to invalid_params instead of internal_error, matching the sibling validate_server_id error handling. Closes #128
bug-ops
force-pushed
the
serverconfig-connect-discover
branch
from
July 9, 2026 15:39
259ddd4 to
bffd0e9
Compare
This was referenced Jul 9, 2026
bug-ops
added a commit
that referenced
this pull request
Jul 9, 2026
6 tasks
bug-ops
added a commit
that referenced
this pull request
Jul 9, 2026
…rospect Expose --connect-timeout-secs and --discover-timeout-secs on the manual (non-from-config) introspect and generate commands, routed through the same validate_server_config chokepoint used by mcp.json's connectTimeoutSecs/discoverTimeoutSecs, so both paths accept and reject identical values. Also documents the decision to permanently reject a zero timeout rather than treat it as "no timeout": an unbounded wait would let a hung or malicious server block this tool's non-interactive CLI/MCP-server invocations forever, reopening the DoS exposure these timeouts were introduced to close. Resolves the two TODO(critic) markers left by #136. Closes #144, #145
bug-ops
added a commit
that referenced
this pull request
Jul 9, 2026
…rospect (#150) Expose --connect-timeout-secs and --discover-timeout-secs on the manual (non-from-config) introspect and generate commands, routed through the same validate_server_config chokepoint used by mcp.json's connectTimeoutSecs/discoverTimeoutSecs, so both paths accept and reject identical values. Also documents the decision to permanently reject a zero timeout rather than treat it as "no timeout": an unbounded wait would let a hung or malicious server block this tool's non-interactive CLI/MCP-server invocations forever, reopening the DoS exposure these timeouts were introduced to close. Resolves the two TODO(critic) markers left by #136. Closes #144, #145
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
1 participant
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
connectTimeoutSecs/discoverTimeoutSecsfields toMcpServerEntry(mcp.json, camelCase) andIntrospectServerParams(MCP tool params), wired into the existingServerConfigBuildersetters only when present so omitting them preserves the current 30s defaults.0 < timeout <= 600s) are enforced once insidevalidate_server_config, the single security gate both the CLI (--from-config) andmcp-execution-serverpaths already funnel through viadiscover_server— the check cannot be bypassed.introspect_serverpath toinvalid_paramsinstead ofinternal_error, matching the existingvalidate_server_iderror handling.mcp.jsonfiles without the new keys are unaffected.Context
ServerConfiggainedconnect_timeout/discover_timeout(30s defaults) in #120 to boundIntrospector::discover_server's awaits, but there was no path from~/.claude/mcp.jsonto those fields. This closes that gap per #128.Developed via architect -> critic -> developer -> (tester, perf, security, impl-critic) -> reviewer pipeline; see
.local/handoff/for the full design rationale, adversarial critique, and validation reports.Test plan
cargo +nightly fmt --checkcargo +stable clippy --all-targets --all-features --workspace -- -D warningscargo nextest run --all-features --workspace --no-fail-fast(707 passed, 2 pre-existing unrelated leaky tests)cargo test --doc --all-features --workspaceRUSTDOCFLAGS="--deny rustdoc::broken_intra_doc_links" cargo doc --no-depsServerConfig, zero/above-MAX_TIMEOUTrejection, in-bounds acceptance, S1 error-mapping end-to-endCloses #128