fix(query): don't close stdin on a result frame while tasks are in flight - #1103
Conversation
…ight 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. wait_for_result_and_end_input() closed stdin on the first result frame, so a still-running subagent's SDK-MCP tool calls failed with "Stream closed" and its PreToolUse hooks were silently bypassed (built-in tools kept executing without the hook callback). Track in-flight tasks from the task_started / task_notification / terminal task_updated lifecycle frames (via the existing TERMINAL_TASK_STATUSES) and only treat a result frame as run-ending when no tasks are in flight. Each task completion wakes the parent for a follow-up turn that ends in another result frame, so stdin still closes promptly — including for chained background tasks — and the reader's finally block still guarantees the waiter is unblocked if the process exits early. Fixes anthropics#1088. Used AI assistance; reviewed and tested by me (repro'd the "Stream closed" failure end-to-end against the live CLI before the fix and verified the same scenario passes after; regression tests fail on current main; ruff, mypy, and the full unit suite green). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes a streaming-mode stdin teardown bug where Query.wait_for_result_and_end_input() closed stdin on the first result frame even when background Tasks were still running, breaking subsequent hook / SDK-MCP control responses (issue #1088).
Changes:
- Track in-flight background task IDs from
task_started/ terminaltask_notification/ terminaltask_updatedframes. - Treat a
resultframe as run-ending (and thus eligible to trigger stdin closure) only when no tasks are in flight. - Add regression + unit tests covering the intermediate-result + background-task lifecycle.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/claude_agent_sdk/_internal/query.py |
Track task lifecycle and defer stdin closure until a result arrives with no in-flight tasks. |
tests/test_query.py |
Add tests asserting stdin remains open across intermediate results while tasks are in flight, plus lifecycle unit coverage. |
Comments suppressed due to low confidence (1)
src/claude_agent_sdk/_internal/query.py:877
- This debug log still says "Waiting for first result", but the code now waits for a run-ending result (a result with no tasks in flight). Adjusting the message will make logs match actual behavior during debugging.
logger.debug(
"Waiting for first result before closing stdin "
f"(sdk_mcp_servers={len(self.sdk_mcp_servers)}, "
f"has_hooks={bool(self.hooks)})"
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Track first result for proper stream closure with SDK MCP servers | ||
| self._first_result_event = anyio.Event() | ||
| # Task IDs of started-but-not-finished tasks. A result frame only ends |
Address Copilot review on anthropics#1103: the _first_result_event comment and the debug log still said "first result", but the waiter now wakes on a run-ending result (a result frame with no tasks in flight). Wording only; no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Confirmed this fix's semantics end-to-end against CLI 2.1.215 from a production incident (details in #1088 (comment)): with in-flight-task tracking and close-only-on-quiescent-result, the previously-denied post-first-result SDK-MCP calls and hook callbacks succeed and the run closes cleanly. One robustness suggestion: also consume the CLI's |
…hanged
Consume the CLI's authoritative `background_tasks_changed` snapshot frame
({"type":"system","subtype":"background_tasks_changed","tasks":[{"task_id":...}]})
as a set-replacement of `_inflight_tasks`, in addition to the incremental
task_started / task_notification / terminal task_updated tracking.
This self-heals a missed start or terminal frame (dropped lines, older CLIs
with different terminal statuses): tracking drift can then only *delay* the
stdin close (bounded by the reader's finally), never wedge it open or close it
early. Handled before the task_id guard since the snapshot has no top-level
task_id; a malformed frame leaves live tracking untouched.
Suggested by @b-randongee, who observed the frame emitted reliably on CLI
2.1.215 in a production incident (anthropics#1088).
Used AI assistance; reviewed and tested by me (ruff/mypy clean, full unit
suite green on asyncio and trio).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks for taking the time to write this up — the production timeline (and especially the "presents as a phantom permissions problem" bit) is really valuable context. Glad the fix held up end-to-end on 2.1.215 🙏 The |
The in-flight ledger tracked every task type reported by task_started, including background shells. A shell such as a dev server or `tail -f` never reaches a terminal status, so the set never emptied and stdin was never closed. Because the CLI only exits on stdin EOF, the process then never exited either, so the reader's finally block never ran and query() hung instead of returning. Track only local_agent and local_workflow: the types whose completion wakes the parent for the follow-up turn this relies on. Shells, monitors, teammates and remote agents can all run indefinitely by design, and are bounded by the CLI's own cleanup once stdin closes. Also stop consuming background_tasks_changed. It reports the live background set, but a subagent is registered in the foreground and only flips to backgrounded later without emitting a second task_started, so a tracked agent that is still running can be absent from it. Narrowing against the snapshot therefore drops work that goes on to outlive its turn, and widening from it can admit an observer agent whose start and terminal frames are both suppressed and which no later frame clears. The lifecycle frames are the only self-consistent source. Adds a regression test whose transport stays open until end_input() is called; every other stream in the suite self-terminates, so none of them could detect a missing close.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1103 +/- ##
=======================================
Coverage ? 90.03%
=======================================
Files ? 23
Lines ? 4154
Branches ? 0
=======================================
Hits ? 3740
Misses ? 414
Partials ? 0 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Pushed 3e74937, quick summary of what changed. The ledger was tracking every background task, including shells. A dev server or I also removed the Added a test where the stream stays open until stdin actually closes, since none of the existing tests could catch a missing close. Heads up this is a mitigation for #1088 rather than a complete fix. If a subagent happens to finish before the turn's result arrives, stdin still closes at that result. Fixing that fully needs a separate change. But this covers the common case. |
Summary
Fixes #1088.
A
resultframe 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:"Stream closed"(the error reported in SDK closes stdin on first result frame, breaking bidirectional control from nested Task/Agent subagents #1088), andReproduction
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 arun_in_background: trueTask. Observed timeline onmain: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/ terminaltask_updatedlifecycle frames (using the existingTERMINAL_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-shotquery()forever. The narrower rule preserves prompt closure:finallystill 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_notificationand terminaltask_updatedpatch) — 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.Used AI assistance; reviewed and tested by me.
🤖 Generated with Claude Code