Skip to content

Commit 017ac18

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 78a1a1f commit 017ac18

4 files changed

Lines changed: 32 additions & 6 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
@@ -596,8 +596,9 @@ async def confirm_delete(filename: str, ctx: Context) -> str | InputRequiredResu
596596
responses = ctx.input_responses
597597
if responses and "confirm" in responses:
598598
answer = responses["confirm"]
599-
accepted = isinstance(answer, ElicitResult) and answer.action == "accept"
600-
return f"Deleted {filename}" if accepted else f"Kept {filename}"
599+
if isinstance(answer, ElicitResult) and answer.action == "accept" and (answer.content or {}).get("confirm"):
600+
return f"Deleted {filename}"
601+
return f"Kept {filename}"
601602
return InputRequiredResult(
602603
input_requests={
603604
"confirm": ElicitRequest(

examples/stories/tasks/README.md

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

5758
## Spec
5859

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=[TasksExtension()]) 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)