Skip to content

fix(mcp): bound every read tool call by an explicit deadline - #43

Merged
kyle-elliott merged 2 commits into
fix/typed-tool-errorsfrom
fix/tool-call-deadlines
Oct 10, 2026
Merged

kyle-elliott merged 2 commits into
fix/typed-tool-errorsfrom
fix/tool-call-deadlines

Conversation

@kyle-elliott

@kyle-elliott kyle-elliott commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Part 2 of 3 for #37, plus follow-up #46. Stack: #42 → this PR → #44 → #46.

Problem

Client::send retried 409 GraphLoading for up to 60 s, and the HTTP client had no deadline. OMP stops an MCP request after 30 s, so agents saw a generic timeout. Gateway errors were never retried. Retrieval calls waited for a first index with no limit.

Change

  • CallBudget (src/mcp/budget.rs): our own call_tool builds one budget per call and puts it in the request extensions. Each tool receives it through Extension<CallBudget> and passes it explicitly to client_for* and to the first-index waits. No deadline is read from ambient state. A timeout_at backstop returns an isError result when the deadline passes.
  • The default deadline is 25 s. SEMCTX_TOOL_DEADLINE_SECS sets it (5–600; unset, 0, or unreadable gives 25). It travels through the IPC handshake like SEMCTX_MCP_RESYNC_SECS.
  • The edit tools (rename_symbol, safe_delete_symbol, replace_symbol_body, insert_before_symbol, insert_after_symbol, undo_edit) get an unbounded budget. Their commit runs in spawn_blocking, so a deadline could only misreport an applied edit and lose its edit_id.
  • src/client/retry.rs: a client with a deadline keeps loading retries, token fetches, permit waits, and each attempt inside the deadline minus a 500 ms margin. A loading answer that no longer fits becomes Retryable { after }. A client without a deadline (CLI, daemon) behaves as before.
  • One retry after 1 s for 502/503/504/530 and refused connections. It applies only to requests that are safe to repeat: GETs, /v1/search, and the graph batch lookup. Sync uploads, registration, and edit planning are never retried.
  • The transport gets a 10 s connect timeout and no request timeout, so large uploads on slow links are not cut off.
  • Retrieval waits at most 5 s for a first index, then returns IndexPending. A partial first index is still never served.
  • index_codebase registers its codebase in its own task, so a deadline cannot leave a claimed first-index gate unfinished. The body moves to src/mcp/indexing.rs (size limit).

Verification

  • Full gate passes.
  • 20 stub-server tests for retry, deadline, and idempotency; end-to-end tests through the rmcp handler; paused-clock readiness tests; session and handshake tests for the new variable.
  • Smoke test: a stub server answers 409 GraphLoading + Retry-After: 1 forever, with SEMCTX_TOOL_DEADLINE_SECS=5. find_definition and grep returned isError: true with next: retry this call once after 1 s… after 4.0 s. Before this change the client kept retrying for 60 s, so OMP would have stopped the call at 30 s.

Every read-only MCP tool call now ends within one deadline. The default is
25 seconds, below the 30 seconds after which Oh My Pi ends an MCP request.
SEMCTX_TOOL_DEADLINE_SECS overrides it. The session reads it once, clamps it
to 5..=600 seconds, and carries it through the attach handshake.

- call_tool builds a CallBudget for each call, passes it to the tool through
  an Extension, and drops the call at the deadline as a backstop. Edit tools
  get an unbounded budget, so a call cut off after a commit cannot misreport
  an applied edit.
- Each tool passes the budget on explicitly. client_for and bound return a
  Client that carries the deadline. The client the server keeps for watchers
  never carries one.
- The client keeps each attempt, loading retry, and retry wait inside the
  deadline. A loading answer that no longer fits returns a retryable failure.
  A call with no time left fails without sending.
- One retry after 1 second for 502, 503, 504, 530, and a refused
  connection. Only requests that are safe to repeat use it: every GET, the
  search POST, and the batch lookup POST. Sync uploads, registration, and
  edit planning are never retried. The transport gets a 10 second connect
  timeout and no request timeout.
- Retrieval waits at most 5 seconds for a first index, then reports
  IndexPending. A partial first index is still never served.
- index_codebase registers its codebase in a task of its own, so the call
  deadline cannot leave a claimed first-index gate unfinished. Its body moves
  to a child module to keep tools.rs under the size limit.

Refs #37
The refused-connect write test measured elapsed time. macOS holds a
connect to a bound socket that does not listen until the 10 s connect
timeout, and Windows retries a refused connect for about 2 s, so the
test failed on both. Check the retry decision that the connect branch
uses instead.

Refs #37
@kyle-elliott
kyle-elliott force-pushed the fix/tool-call-deadlines branch from 1faf245 to 50300cc Compare October 10, 2026 00:05
@kyle-elliott
kyle-elliott merged commit 08e21ab into main Oct 10, 2026
6 of 9 checks passed
@kyle-elliott
kyle-elliott deleted the fix/tool-call-deadlines branch October 10, 2026 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant