Skip to content

Commit e6e07f1

Browse files
yayayouyoushihyayouclaudeashwin-ant
authored
fix(query): don't close stdin on a result frame while tasks are in flight (#1103)
## Summary Fixes #1088. A `result` frame marks the end of one **turn**, not the end of the **run**: a background Task keeps running past it and still needs stdin for hook and SDK-MCP control responses. `Query.wait_for_result_and_end_input()` closed stdin on the *first* result frame, which broke everything a still-running subagent needed afterwards: - its SDK-MCP tool calls failed with exactly **`"Stream closed"`** (the error reported in #1088), and - its **PreToolUse hooks were silently bypassed** — built-in tools kept executing with no hook callback delivered, so deny-gate hooks stopped gating. ## Reproduction Reproduced end-to-end against the live CLI (2.1.206) with `query()` + a PreToolUse hook + an in-process SDK-MCP server whose tool sleeps 12s, called from a `run_in_background: true` Task. Observed timeline on `main`: ``` [19.3s] parent turn ends → ResultMessage #1 → SDK closes stdin (task still running) [25s+] subagent's ToolSearch calls execute with NO PreToolUse callback (hooks silently bypassed) [~25s] subagent calls the SDK-MCP tool → tool_result is_error=true: "Stream closed" (×2 retries) [47.9s] ResultMessage #2 arrives (task completion wakes the parent for a follow-up turn) ``` The second result frame is the key observation: result frames are per-turn, so closing stdin on the first one is the bug. (On current CLI, two *sequential foreground* subagents — the issue's literal scenario — no longer emit intermediate result frames; background tasks are the deterministic trigger.) ## Fix Track in-flight tasks from the `task_started` / `task_notification` / terminal `task_updated` lifecycle frames (using the existing `TERMINAL_TASK_STATUSES`, whose docstring already prescribes exactly this clearing behavior), and only treat a result frame as run-ending when no tasks are in flight. Why not the issue's first suggestion (keep stdin open until `close()` whenever hooks/SDK-MCP are registered): the CLI in stream-json mode only exits on stdin EOF, so that would hang one-shot `query()` forever. The narrower rule preserves prompt closure: - no background tasks → behavior unchanged (first result still closes stdin; existing tests cover this), - background task in flight → stdin stays open; its completion wakes the parent for a follow-up turn that ends in another result frame, which closes stdin (this also makes *chained* background tasks work), - early process exit → the reader's `finally` still unblocks the waiter, so no new hang mode is introduced. ## Tests - `test_result_with_inflight_task_keeps_stdin_open` (parametrized over both drain frames: `task_notification` and terminal `task_updated` patch) — asserts stdin stays open across an intermediate result and closes on the first result with no tasks in flight. **Fails on current main**, passes with the fix. - `test_track_task_lifecycle_unit` — add/non-terminal/terminal/unknown-id/missing-id transitions. - Full suite: 1059 passed, 5 skipped (asyncio + trio); ruff and mypy clean. - E2E re-run of the reproduction against the live CLI with the fix: hook fires for the background subagent's calls, the SDK-MCP tool executes and returns its result, stdin closes exactly at the final result frame, run exits normally. Used AI assistance; reviewed and tested by me. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: shihyayou <shihyayouyou@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Ashwin Bhat <ashwin@anthropic.com>
1 parent 74fe445 commit e6e07f1

2 files changed

Lines changed: 426 additions & 9 deletions

File tree

src/claude_agent_sdk/_internal/query.py

Lines changed: 115 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
from .._errors import ProcessError
1919
from ..types import (
20+
TERMINAL_TASK_STATUSES,
2021
PermissionMode,
2122
PermissionResultAllow,
2223
PermissionResultDeny,
@@ -38,6 +39,22 @@
3839

3940
logger = logging.getLogger(__name__)
4041

42+
# Task types whose completion runs a follow-up turn, and which therefore may
43+
# still need the control channel after the turn's result frame.
44+
#
45+
# This mirrors the set the CLI itself holds a result back for, which is
46+
# narrower than its notion of "delegated agent work". The types left out are
47+
# left out on purpose, and none of them is merely an oversight:
48+
# - background shells and monitors run indefinitely by design, so deferring
49+
# the close on one withholds it forever rather than briefly;
50+
# - teammates are long-lived too — their status stays running for their whole
51+
# lifetime, so they never settle the ledger;
52+
# - remote agents can be long-running monitors the CLI likewise refuses to
53+
# wait on.
54+
# Anything added here must be a type that reliably reaches a terminal status,
55+
# or it will hang the query (see Query._track_task_lifecycle).
56+
DEFERRING_TASK_TYPES = frozenset({"local_agent", "local_workflow"})
57+
4158

4259
def _convert_hook_output_for_cli(hook_output: dict[str, Any]) -> dict[str, Any]:
4360
"""Convert Python-safe field names to CLI-expected field names.
@@ -128,8 +145,17 @@ def __init__(
128145
self._closed = False
129146
self._initialization_result: dict[str, Any] | None = None
130147

131-
# Track first result for proper stream closure with SDK MCP servers
148+
# Set when a run-ending result arrives (a result frame with no tasks
149+
# in flight) so the stdin-closing waiter can wake; see #1088 and
150+
# _inflight_tasks below. Named for history — it once tracked the
151+
# literal first result.
132152
self._first_result_event = anyio.Event()
153+
# Task IDs of started-but-not-finished tasks. A result frame only ends
154+
# one turn, not the run: background tasks keep running past it and
155+
# still need stdin for hook/SDK-MCP control responses (see #1088), so
156+
# a result that arrives while this set is non-empty must not close
157+
# stdin.
158+
self._inflight_tasks: set[str] = set()
133159
# Set to the result's error text when the most recent message is a
134160
# result with is_error=True. Used to replace the generic "exit code 1"
135161
# ProcessError with the structured error the CLI already reported.
@@ -293,14 +319,33 @@ async def _read_messages(self) -> None:
293319
)
294320
continue
295321

322+
# Track task lifecycle frames so results can tell "one turn
323+
# ended" apart from "the run is done" (see #1088).
324+
if msg_type == "system":
325+
self._track_task_lifecycle(message)
326+
296327
# Track results for proper stream closure
297328
if msg_type == "result":
298329
# Flush pending transcript mirror entries before yielding
299330
# result so consumers observing the result can rely on the
300331
# SessionStore being up to date for this turn.
301332
if self._transcript_mirror_batcher is not None:
302333
await self._transcript_mirror_batcher.flush()
303-
self._first_result_event.set()
334+
if self._inflight_tasks:
335+
# One turn ended, but background tasks are still
336+
# running and may need hook/SDK-MCP control responses
337+
# over stdin. Closing it now silently disables hooks
338+
# and fails SDK-MCP calls with "Stream closed"
339+
# (#1088). Each task completion wakes the parent for
340+
# a follow-up turn, so a later result frame arrives
341+
# with no tasks in flight and closes stdin then.
342+
logger.debug(
343+
"Result received with %d task(s) in flight; "
344+
"keeping stdin open",
345+
len(self._inflight_tasks),
346+
)
347+
else:
348+
self._first_result_event.set()
304349
if message.get("is_error"):
305350
errors = message.get("errors") or []
306351
self._last_error_result_text = "; ".join(errors) or str(
@@ -806,19 +851,80 @@ async def stop_task(self, task_id: str) -> None:
806851
}
807852
)
808853

854+
def _track_task_lifecycle(self, message: dict[str, Any]) -> None:
855+
"""Track in-flight tasks from ``system`` task lifecycle frames.
856+
857+
``task_started`` marks a task in flight; ``task_notification`` or a
858+
``task_updated`` patch with a terminal status clears it. Terminal
859+
completion can arrive as either frame (not every terminal task emits
860+
a notification), so both are handled; ``discard`` keeps the pair
861+
idempotent.
862+
863+
This is a mitigation, not a complete answer to #1088. An empty set
864+
means "nothing we know of is running", which is not the same as "the
865+
run is over": a task that settles *before* the turn's result frame
866+
leaves the set empty at that result, so stdin closes even though the
867+
completion may still wake the parent for a continuation turn. No
868+
ledger can close that gap, because the ledger cannot distinguish a
869+
settled task whose continuation is pending from no work at all — that
870+
needs a run-boundary signal from the CLI rather than an inference from
871+
task bookkeeping. What this does fix is the common ordering, where the
872+
task outlives the turn that spawned it.
873+
874+
Only delegated agent work is tracked (``DEFERRING_TASK_TYPES``). A
875+
background *shell* — ``Bash(run_in_background=True)`` on a dev server or
876+
``tail -f`` — is also reported through these frames, but it may never
877+
reach a terminal status, and the CLI in stream-json mode only exits on
878+
stdin EOF. Tracking one would therefore withhold the close forever
879+
rather than briefly: no terminal frame, no process exit, so not even the
880+
reader's ``finally`` runs. Agent tasks are the ones whose completion
881+
wakes the parent for the follow-up turn this relies on; shells and
882+
monitors are bounded by the CLI's own post-close cleanup instead.
883+
884+
``background_tasks_changed`` is deliberately *not* consumed, in either
885+
direction. Its payload is the live *background* set, while a subagent is
886+
registered in the foreground and only flips to backgrounded later,
887+
without a second ``task_started``. So the snapshot omits tracked work
888+
that is still running: narrowing against it would drop an agent that
889+
goes on to outlive its turn, which is the very close-too-early bug this
890+
method exists to prevent. Widening from it is no better — the snapshot
891+
spans every background task type and carries nothing marking an
892+
observer agent, whose start and terminal frames are both suppressed, so
893+
it could admit an id no later frame ever clears. The lifecycle frames
894+
are the only self-consistent source here (see #1088).
895+
"""
896+
subtype = message.get("subtype")
897+
task_id = message.get("task_id")
898+
if not task_id:
899+
return
900+
if subtype == "task_started":
901+
if message.get("task_type") in DEFERRING_TASK_TYPES:
902+
self._inflight_tasks.add(task_id)
903+
elif subtype == "task_notification":
904+
self._inflight_tasks.discard(task_id)
905+
elif subtype == "task_updated":
906+
patch = message.get("patch")
907+
status = patch.get("status") if isinstance(patch, dict) else None
908+
if status in TERMINAL_TASK_STATUSES:
909+
self._inflight_tasks.discard(task_id)
910+
809911
async def wait_for_result_and_end_input(self) -> None:
810-
"""Wait for the first result (if needed) then close stdin.
912+
"""Wait for a run-ending result (if needed) then close stdin.
811913
812914
If SDK MCP servers or hooks require bidirectional communication,
813-
keeps stdin open until the first result arrives. The control protocol
814-
requires stdin to remain open for the entire conversation, so no
815-
timeout is applied. The event is guaranteed to fire: either when the
816-
result message arrives, or in _read_messages' finally block if the
817-
process exits early.
915+
keeps stdin open until a result arrives with no tasks in flight. A
916+
result frame ends one turn, not necessarily the run: background tasks
917+
keep running past it and still need stdin for hook/SDK-MCP control
918+
responses (see #1088). The control protocol requires stdin to remain
919+
open for the entire conversation, so no timeout is applied. The event
920+
is guaranteed to fire: either when a result message arrives with no
921+
in-flight tasks (every task completion wakes the parent for a
922+
follow-up turn, which ends in such a result), or in _read_messages'
923+
finally block if the process exits early.
818924
"""
819925
if self.sdk_mcp_servers or self.hooks:
820926
logger.debug(
821-
"Waiting for first result before closing stdin "
927+
"Waiting for a run-ending result before closing stdin "
822928
f"(sdk_mcp_servers={len(self.sdk_mcp_servers)}, "
823929
f"has_hooks={bool(self.hooks)})"
824930
)

0 commit comments

Comments
 (0)