Skip to content

Commit e3d3423

Browse files
committed
Address follow-up review: thread poll timeouts, honor the confirm answer
- Client.call_tool's per-call read_timeout_seconds now bounds each tasks/get poll as well as the initial call (per-request bound, not a whole-loop deadline); pinned by a recording test. - The everything-server's confirm_delete keeps the file when an accepted elicitation answers confirm: false (conformance legs re-verified). - The routing-header parentheticals attribute the requirement to SEP-2663 and the header family to SEP-2243.
1 parent 775463e commit e3d3423

5 files changed

Lines changed: 45 additions & 13 deletions

File tree

examples/servers/everything-server/mcp_everything_server/server.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -612,8 +612,9 @@ async def confirm_delete(filename: str, ctx: Context) -> str | InputRequiredResu
612612
responses = ctx.input_responses
613613
if responses and "confirm" in responses:
614614
answer = responses["confirm"]
615-
accepted = isinstance(answer, ElicitResult) and answer.action == "accept"
616-
return f"Deleted {filename}" if accepted else f"Kept {filename}"
615+
if isinstance(answer, ElicitResult) and answer.action == "accept" and (answer.content or {}).get("confirm"):
616+
return f"Deleted {filename}"
617+
return f"Kept {filename}"
617618
return InputRequiredResult(
618619
input_requests={
619620
"confirm": ElicitRequest(

examples/stories/tasks/README.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,9 @@ finished (completed or failed) tasks live in a pluggable `TaskStore`
4848
(`Tasks(store=...)`, in-memory default) that enforces `default_ttl_ms`. Deferred
4949
to follow-ups, each needing deeper SDK plumbing: background execution (returning
5050
`working` tasks), the in-task `input_required`/`inputResponses` loop over
51-
`tasks/update`, and `notifications/tasks` (the SEP-2243 `Mcp-Name` routing
52-
header is already handled by the shared header table).
51+
`tasks/update`, and `notifications/tasks` (SEP-2663's `Mcp-Name` routing
52+
header — the SEP-2243 header family — is already handled by the shared header
53+
table).
5354

5455
## Spec
5556

src/mcp/client/client.py

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -623,7 +623,8 @@ async def call_tool(
623623
Args:
624624
name: The name of the tool to call.
625625
arguments: Arguments to pass to the tool.
626-
read_timeout_seconds: Timeout for each underlying `tools/call` round.
626+
read_timeout_seconds: Timeout for each underlying `tools/call` round
627+
and each `tasks/get` poll.
627628
progress_callback: Callback for progress updates.
628629
input_responses: Responses to seed the first call with (e.g. when
629630
resuming from a persisted `InputRequiredResult`).
@@ -664,7 +665,7 @@ async def retry(
664665

665666
result = await self._drive_input_required(await retry(input_responses, request_state), retry)
666667
if isinstance(result, CreateTaskResult):
667-
return await self._drive_task(result, name)
668+
return await self._drive_task(result, name, read_timeout_seconds=read_timeout_seconds)
668669
return result
669670

670671
async def list_prompts(
@@ -745,19 +746,24 @@ async def dispatch(key: str, req: InputRequest) -> InputResponse | ErrorData:
745746
first, dispatch=dispatch, retry=retry, max_rounds=self.input_required_max_rounds
746747
)
747748

748-
async def _drive_task(self, created: CreateTaskResult, tool_name: str) -> CallToolResult:
749+
async def _drive_task(
750+
self, created: CreateTaskResult, tool_name: str, *, read_timeout_seconds: float | None = None
751+
) -> CallToolResult:
749752
"""Poll an SEP-2663 task to its final `CallToolResult` (the transparent flow).
750753
751754
The driver's `get_task` rounds go through `session.send_request`, so each
752-
poll carries the session read timeout and the `Mcp-Name` routing header.
753-
The polled result re-enters the same output-schema validation as the
754-
direct path, so both paths surface identical guarantees.
755+
poll carries the caller's per-request read timeout (falling back to the
756+
session read timeout) and the `Mcp-Name` routing header. The polled
757+
result re-enters the same output-schema validation as the direct path,
758+
so both paths surface identical guarantees.
755759
"""
756760
session = self.session
757761

758762
async def get_task(task_id: str) -> GetTaskResult:
759763
request = GetTaskRequest(params=GetTaskRequestParams(task_id=task_id))
760-
return await session.send_request(cast("types.ClientRequest", request), GetTaskResult)
764+
return await session.send_request(
765+
cast("types.ClientRequest", request), GetTaskResult, request_read_timeout_seconds=read_timeout_seconds
766+
)
761767

762768
result = await run_task_driver(created, get_task=get_task)
763769
if not result.is_error:

src/mcp/server/tasks.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,9 @@
4848
excluded the call. Background execution (returning `working` tasks), the in-task
4949
`input_required`/`inputResponses` loop over `tasks/update`, and
5050
`notifications/tasks` over `subscriptions/listen` are deferred follow-ups, each
51-
needing deeper SDK plumbing. (The SEP-2243 `Mcp-Name: <taskId>` routing header
52-
is already handled by the shared header table in `mcp.shared.inbound`.)
51+
needing deeper SDK plumbing. (SEP-2663's `Mcp-Name: <taskId>` routing header --
52+
the SEP-2243 header family -- is already handled by the shared header table in
53+
`mcp.shared.inbound`.)
5354
5455
Task ids are unguessable bearer capabilities: any caller presenting a valid id
5556
may poll the task. That is deliberate -- the modern wire has no sessions, and a

tests/server/test_tasks.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,29 @@ async def test_declaring_client_call_tool_transparently_polls_to_the_call_tool_r
268268
assert result.meta is None
269269

270270

271+
async def test_call_tool_read_timeout_seconds_bounds_each_task_poll(monkeypatch: pytest.MonkeyPatch) -> None:
272+
"""SDK-defined: a caller's per-call `read_timeout_seconds` is forwarded to every
273+
`tasks/get` poll as that request's own per-request bound (not a whole-loop
274+
deadline), matching the bound the initial `tools/call` carries."""
275+
async with Client(_tasks_server(), extensions={EXTENSION_ID: {}}) as client:
276+
session = client.session
277+
inner_send_request = session.send_request
278+
timeouts_by_method: dict[str, list[float | None]] = {}
279+
280+
async def recording_send_request(
281+
request: Any, result_type: Any, request_read_timeout_seconds: float | None = None, **kwargs: Any
282+
) -> Any:
283+
timeouts_by_method.setdefault(request.method, []).append(request_read_timeout_seconds)
284+
return await inner_send_request(request, result_type, request_read_timeout_seconds, **kwargs)
285+
286+
monkeypatch.setattr(session, "send_request", recording_send_request)
287+
result = await client.call_tool("echo", {"text": "hi"}, read_timeout_seconds=7.5)
288+
289+
assert result == snapshot(types.CallToolResult(content=[types.TextContent(text="hi")]))
290+
assert timeouts_by_method["tools/call"] == [7.5]
291+
assert timeouts_by_method["tasks/get"] == [7.5]
292+
293+
271294
async def test_tool_error_result_under_augmentation_surfaces_as_is_error_call_tool_result() -> None:
272295
"""SEP-2663: a tool result with `isError: true` is a `completed` task, so the
273296
transparent driver returns it as the ordinary error-shaped `CallToolResult`

0 commit comments

Comments
 (0)