Conversation
…g long executions Root cause: every MCP request shared one finite wall-clock cap sized for cheap metadata calls. The engine-side stdio proxy answered tools/call under the generic 120s request budget, and the TUI pool defaulted its execute timeout to 60s; a legitimate tool run that takes minutes — a build, a test suite, a long script driven through an MCP server — was killed mid-flight and the model got "timed out" for healthy work, then retried and compounded the cost. Worse, raising the documented `execute_timeout` knob did not actually govern: the connection's inner response read wait stayed at `read_timeout`, fired first, marked the connection Disconnected, and silently cut the call off at the read knob. Mechanism: - crates/mcp stdio proxy: tools/call now uses a dedicated CALL_TOOL_TIMEOUT; every other request keeps the generic 120s REQUEST_TIMEOUT. - TUI pool: default execute timeout 60s -> 1800s. The per-server and global `execute_timeout` config fields remain the override path (`effective_execute_timeout`), so the constants are documented defaults only. - TUI connection: the per-request inner read wait is widened to max(read_timeout, that request's own budget), so a server that is silent for the whole execution of a long tools/call is not declared dead mid-call. Requests whose budget does not exceed the read knob (resources/read, discovery) still fail at the configured read budget. - TUI HTTP transports: the client's total request ceiling is now max(read, execute) so a raised execute_timeout governs HTTP servers too; the read knob itself stays intact for the connection-level waits. Numbers: 1800s (30 minutes) covers long-but-bounded tool workloads (builds, test suites, remote jobs) while staying a real bound — a wedged server still cannot hang a consumer forever. The stdio constant matches the TUI pool default so both surfaces behave the same for one server. Coordination: PR #6711 touches stream-open budgets in the engine; this change deliberately does not touch stream-open or retry logic — it only re-budgets per-request MCP calls. Signed-off-by: asto18089 <asto18089@126.com>
…liation Signed-off-by: CodeWhale Bot <bot@codewhale.net>
Preserve the current 30s connection/discovery default and unconditional initialize error classification with reviewed-plugin suppression. Retain the dedicated 1800s execution default and per-request receive budget. No unfinished integration-wave commits included. Affected source/control/native and lint qualification pending before original-branch push. Signed-off-by: CodeWhale Bot <bot@codewhale.net>
Original PR #6741 history is preserved with ordinary green-main reconciliation at f3446e3. Restore the retired legacy stdio client to current main bytes so the active TUI deadline work does not extend a client being removed by the integration wave. Keep the English and Chinese timeout examples consistent with the current 30s connection default. Add verified contributor @asto18089 to the existing unreleased web credit surface. Only four preparation paths changed. Contributor credit check and git diff --check passed. No fresh Rust, Node/web, native MCP cancellation, or negative-control qualification is claimed for this WIP. Active TUI deadline source is inherited from the original PR/main merge; real stdio fixtures and complete affected qualification remain before any original-branch push. Signed-off-by: CodeWhale Bot <bot@codewhale.net>
PR #6741 widened the per-request inner read wait to max(read_timeout, request budget) so the read knob could no longer cut a long tools/call short. That left the inner wait equal to the outer request budget in the default configuration (tools/call 1800s/1800s, resources/read at the read knob), so two identical deadlines raced: when the inner one won, the connection was marked Disconnected; when the outer one won, it was kept. Which timer fired decided the connection's fate. The PR's hanging-resource test asserted disconnection under that race. call_method now receives with no per-frame deadline, so the request's own budget (execute_timeout for tools/call and prompts/get, read_timeout for resources/read) is the only one. On expiry the request is abandoned and the connection kept. This matches main's pinned partial-frame contract (execute_timeout_after_partial_stdio_response_does_not_corrupt_next_call), and a late reply is skipped by id. Handshake and discovery keep main's per-frame read-knob receive; recv() and its direct disconnect fixture are restored to main's shape. per_request_read_budget and its unit test are removed because the helper no longer has a caller. Tests: - a_wedged_request_fails_at_its_own_budget_and_keeps_the_connection replaces the racing test. It uses distinct budgets (knob 1s, request 2s) and asserts the real outcome: a method-named timeout after 2s, with the connection still ready. - recv_times_out_waiting_for_mcp_response_and_disconnects is unchanged from main (per-frame read-knob disconnect). Known limitations are written beside the code. No notifications/cancelled is sent for abandoned requests, so a sequential server answers the next call only after it finishes the abandoned one. Streamable HTTP reads the reply inside the POST, so it is bounded by the HTTP ceiling max(read, execute) rather than by the request budget. Evidence: rustfmt --edition 2024 --check is clean; check-blocking-calls-budget.py is within budget. No cargo build or test was run in this lane: the lead compiles and qualifies. Refs #6741 Signed-off-by: CodeWhale Bot <bot@codewhale.net> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
These tests use a real `sh` child that speaks MCP over stdio through the pool (McpPool::call_tool -> StdioTransport), following the existing COUNTING_STDIO_SERVER pattern. DEADLINE_STDIO_SERVER logs `init <pid>` for each initialize and `<tool> <id>` for each tools/call. Each reply's text is that same logged line, so a test can prove which request a reply answered and whether the connection or child was reused. - stdio_tool_reply_after_the_read_knob_completes_within_the_execute_budget: read_timeout 1s, execute_timeout 30s, and the tool replies after 3s. The call succeeds. The next call on the same child gets its own reply. Exactly two tools/call are logged (no replay) and there is one init. Fix-off control: in McpConnection::call_method, change `self.recv_reply(call_id, None)` to `self.recv_reply(call_id, Some(self.read_timeout_secs))`, which is the old per-request read-knob receive. The slow call then fails at 1s with "Timed out waiting for MCP JSON-RPC response ... after 1s". - explicit_shorter_execute_budget_ends_a_stdio_tool_call_and_skips_its_late_reply: execute_timeout 2s (read knob at the 120s default) and a 3s tool. The call fails at 2s with the method-named timeout. The connection is kept (one init). The child's late reply reaches the pipe ahead of the next call's reply and is skipped, so the next call gets its own reply. - cancelling_an_inflight_stdio_tool_call_does_not_wait_for_the_execute_budget: default 1800s budget and a tool that never replies. Cancelling the connection's own cancel_token (the authority that plugin revocation and connection drop use) returns the call within 5s with "was cancelled", and the server stops reporting as connected. The next call rebuilds on a fresh child (two inits), the cancelled child's pid is gone within STDIO_SHUTDOWN_GRACE + 1s, and the hang call is not replayed. The cross-process plugin revocation and idle-child cancellation tests are unchanged. Evidence: I ran the fixture shell script by hand with sh, outside cargo. Its replies and logs matched what the tests expect: slow/fast ids echoed, the init pid equal to the sh pid, and the hang loop ending within 200ms of SIGTERM. rustfmt --check is clean. No cargo build or test was run in this lane: the lead compiles and qualifies. Refs #6741 Signed-off-by: CodeWhale Bot <bot@codewhale.net> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The English and Chinese server-field sections now state the budgets the TUI MCP pool enforces: - connect_timeout: 30s default, covering spawn, initialize, and the first tools/list. - execute_timeout: 1800s default, used for tools/call and prompts/get. An explicit shorter value is respected, and read_timeout never cuts a running tool short. - read_timeout: 120s default. It bounds each reply wait during the handshake and discovery, and is the budget for resources/read. The sections also describe what happens when a budget expires: the request is abandoned, the connection is kept, and a late reply is discarded. They name two known limitations: no notifications/cancelled is sent for abandoned requests, and Streamable HTTP is bounded by max(read, execute). Refs #6741 Signed-off-by: CodeWhale Bot <bot@codewhale.net> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The execute-budget change raised the Streamable HTTP client ceiling to the larger of the read and execute knobs, and HTTP reads the reply inside the POST. With only the receive bounded, a wedged `resources/read` over HTTP ran up to 1800s instead of its 120s budget, and an explicit shorter `execute_timeout` stopped at the read knob. One deadline now covers send and receive for every transport. A send abandoned at the deadline marks the connection Disconnected: on a stream transport the frame boundary is unknown after a partial write, so the pool rebuilds it instead of reusing it. Test (added after the fix): a_request_blocked_inside_send_ends_at_its_own_budget. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: CodeWhale Bot <bot@codewhale.net>
Follows the send-and-receive deadline fix: the reply read inside the POST is no longer left to the larger client ceiling. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: CodeWhale Bot <bot@codewhale.net>
…udget A watchdog around the blocked-send fixture turns a missing send deadline into a test failure instead of a wedged test binary. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: CodeWhale Bot <bot@codewhale.net>
This branch has not been deployed
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
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.
Lands #6741 by @asto18089 as itself: give MCP
tools/callits own request budget and stop undercutting long executions.cw-land, the finishing work lives on an integration branch here.3ffbb9017248is an ancestor, unmodified, so the PR closes as merged when this lands.What the original change needed to be correct
e778ee98). The PR set the per-frame receive wait tomax(read_timeout, request budget).call_methodhas exactly one deadline, the request's own. On expiry the request is abandoned, the connection is kept, and a late reply is skipped by id.0ad2600d).resources/readto the PR's raised client ceiling (1800s instead of 120s).ee63b68f), using a realshMCP child:a_wedged_request_fails_at_its_own_budget_and_keeps_the_connection.docs/MCP.mdstate the real connect (30s), execute (1800s; explicit shorter budgets respected) and read budgets, what happens on expiry, and the remaining limitation. Turn cancellation drops the call without sendingnotifications/cancelledto the server.Evidence (local, exact source
077f7c3f4, macOS arm64, governed slots)All TUI MCP tests:
329 passed; 0 failed.Fix-off control. Restoring the per-request read-knob receive and removing the send bound failed exactly the four predicted tests:
stdio_tool_reply_after_the_read_knob…a_tool_response_after_the_read_knob…a_wedged_request…a_request_blocked_inside_send…The explicit-budget and cancellation fixtures kept passing; they cover other contracts. Restored byte-exact, then
6 passed; 0 failed.Workspace CI-policy Clippy: pass. rustfmt: clean. Contributor-credit check: pass.
The legacy
crates/mcpclient is untouched here; its removal is separate work on the integration wave.No-Issue: contributor PR #6741 finished and landed as itself; the PR carries its own motivation.
🤖 Generated with Claude Code