Repository navigation
fix(goal-loop): ask the provider whether it can take input before delivering - #1050
Merged
Merged
Conversation
…ivering Closes #1033, the last finding from #1028's review. Our Stop hook allowing a turn to end says nothing about the user's OWN hooks: Claude runs every configured Stop hook in parallel and Codex aggregates them, and both keep the turn going when any one blocks. A project or plugin hook that blocks ("run the tests before stopping") therefore left the turn running after we were told it ended, and the continuation landed mid-turn as a queued command — exactly #1024's symptom, from a different cause. The turn boundary is no longer trusted alone. Immediately before typing, the loop asks the PROVIDER whether input is ready — the same signal the composer gate uses — and skips the delivery when it is not. The turn's real end re-triggers the loop, so nothing is lost. A session main has no snapshot for answers yes, deliberately: "I cannot tell" must not silently stop a loop forever, and deliverPromptToAgent still refuses a genuinely unready target retry-safely. The system test drives the REAL MCP hook host: a stop arrives while the provider reports input unavailable, and nothing is delivered; when it becomes ready, the next stop delivers exactly once. It fails without the gate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review of the first #1033 fix found three defects in it, all real. 1. Input readiness never proved the turn ended. It answers "can something be typed", which is a composer question: Claude derives it from blocking conditions, composer occupancy and transcript replay, and Codex latches it true for the whole session once a composer has ever painted. Both are TRUE mid-turn — that is exactly why a mid-turn prompt becomes a QUEUED command instead of being refused. The gate therefore did not catch the case it was written for. The gate is now the provider's own activity: `process-state`, which every runtime derives from its own detector (the TUI spinner for Claude, Codex and Grok; the session event stream for OpenCode). It stays up while hooks run and while the model continues after one of them blocks. 2. Skipping the delivery permanently lost the continuation. For a hook session the Stop we were answering IS the turn's end, and no further Stop fires while the model works through another hook's block, so "the turn's real end re-triggers us" was false: the loop sat active with zero deliveries and nothing left to observe. A blocked continuation is now HELD and released by the provider's own active->idle edge, and a hold that never clears (a latched detector) pauses the loop visibly after fifteen minutes instead of dying quiet. 3. Terminal-runtime UI readiness must not gate programmatic delivery. OpenCode Terminal's readiness waits on PTY paint while its HTTP prompt channel is already usable. Activity is runtime-correct, and the hook suite now reports input that is NOT ready throughout, so a reinstated readiness gate fails every test in the file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… the loop
Round-2 review found the activity gate wrong in BOTH directions, and three
ways a held continuation could be lost. The rewrite drops the idea that any
one signal can be trusted.
The rule now: hold only while something POSITIVELY says the agent is working,
bound every hold, and when the evidence runs out, deliver.
1. Our own Stop boundary (a hook session's turn is open until its Stop).
2. The tracked phase, but only once an event has arrived SINCE that Stop —
which is exactly the case where another hook blocked ours and the model
kept going. Claude hides its spinner while streaming visible text, so
this is what covers a turn the screen calls idle.
3. The screen, for at most 45 s. A live spinner is the only signal during
the gap between our Stop and a blocked turn's next request, but it
latches on a completed `⏺ … (ctrl+o to expand)` row, so it may postpone
a delivery and never prevent one.
A held continuation re-evaluates itself every second while it exists. Every
edge-driven version of this lost the continuation when the edge never came —
a turn that reopens and then ends through Esc, a provider that stops
emitting, a latched detector. A turn that reopens restarts the stall clock
instead of pausing live work, and a second Resume no longer throws away the
first one's hold.
The residual is stated in the code: if another hook blocks while the spinner
is down, the continuation is typed into a live turn, where Claude QUEUES it.
One early continuation beats a loop that dies silently, and closing that
window would mean outlasting an arbitrary user hook.
Five new tests, each proven by mutation to fail without its fix.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round-3 review, three findings, all reproduced by the reviewer. A delivery that lands mid-turn is QUEUED rather than started, and the turn that is running belongs to someone else — so its Stop closed our turn-open guard while our continuation had not begun, and the loop sent a second one. The probe measured two deliveries where the design promises at most one early. A queue acceptance is now tracked until the prompt is seen to start (a UserPromptSubmit hook, or the phase going to work for a provider without hooks). The "has an event arrived since the Stop" flag is gone. It marked freshness for ANY event, so a trailing one about already-finished work validated the stale busy state we seed for our own delivery, and the loop held until the 30-minute pause. An allowed Stop now simply drops the tracked phase: the provider said that turn ended, so every reading describing it is history, and anything published afterwards is exactly the evidence the gate wants. Both deadlines now measure silence rather than age. A reopened turn that streamed every minute for thirty minutes paused its own loop; the clock restarts on any hook or state-changing event, so it bounds how long we wait on a stuck signal and never how long an agent may work. Three more tests, each proven by mutation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… as progress Round-4 review, five findings, all reproduced. The queue latch's phase fallback is for providers WITHOUT hooks. For a hook session it is the #1024 false positive itself — a Claude subagent flow publishes working→idle mid-turn — and the recorded sequence released a queued continuation and delivered a second one with no UserPromptSubmit in sight. A stall pause now drops the latch. Keeping it made Resume useless: it re-held on the same stale reason and paused again half an hour later, forever, without ever delivering. Progress is ANY provider event, recorded before the reducer's identity check. A streamed answer is mostly text and thinking deltas, which the reducer collapses to the same state, so a loop held through forty minutes of visible streaming paused itself for "silence" while the model was talking. The screen grace and the stall pause now measure different things, because they are about different failures: the screen latches on a stale row, so the time it may hold a delivery is measured from the hold's START and cannot be renewed — turn metadata arriving every thirty seconds renewed it indefinitely — while the stall pause measures silence, so an agent that keeps working is never paused for it. The queue regression test waits for the first continuation to be RECORDED, not merely issued: the re-entrancy guard holds until the loop is persisted, so a slow store let that test pass with the latch removed. Four more tests, each proven by mutation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Round-5 review. `flow_selected` and `flow_ignored` report which upstream /v1/messages call the proxy renders from — title generation, a retry, a subagent's stream. They are published ABOUT OTHER FLOWS and say nothing about this agent working, but they renewed the stall clock like any event, so one a minute kept a stale `turn-open` or stuck `phase-working` hold alive for two simulated hours with zero deliveries. Background chatter can no longer postpone a pause forever. Everything else still counts, deltas included: a streamed answer must never read as silence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tall Round-6 review, both reproduced. Filtering `flow_selected` and `flow_ignored` by name was not enough: the adapter publishes `flow_selected → requesting → idle → flow_ignored` when it promotes a sidecar or subagent flow on its first chunk and demotes it, and the two PHASES in the middle kept renewing the stall clock. Replaying that recorded sequence once a minute held a dead continuation for forty. A phase says what a stream is doing and the stream can be somebody else's, so no phase transition counts as progress now — everything that still counts carries content or tool activity, which the adapter publishes only for the flow it actually renders. The phase's VALUE is unaffected; the delivery gate reads it directly. And a dispatched tool that has not come back, with a live process behind it, is not a stuck signal — it is a build or a test suite. Pausing there cost the loop its own turn, because the tool's eventual result and Stop cannot continue a paused loop. Both shapes count as dispatched: a tool block in `pendingTools`, and Claude's `awaiting-tool` phase carrying the tool id. Two tests, each proven by mutation. The round-3 case is now the long-tool scenario, since its old fixture — phase flips and nothing else — is exactly the bookkeeping this round reclassifies, and round 4's streamed-answer test already covers real content. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…hases Round-7 review, and the second time this file got the same question wrong in the opposite direction. Excluding `stream_phase` from progress caught Claude's sidecar churn, and also threw away the ONLY progress signal Grok and managed OpenCode Terminal publish during a turn — their content rides a separate channel — so a working loop on either paused itself after thirty minutes. The same event type means work on one provider and bookkeeping on another, so no classification by type can separate them. Phases count again; only the flow-selection diagnostics are excluded. What separates them is time, so the hold now has two deadlines: thirty minutes of SILENCE (which a dispatched tool with a live process is exempt from, because that is a build, not a stuck signal), and an absolute two-hour limit that nothing is exempt from. The reviewer's six-hour silent tool and minute-by-minute flow chatter both end there, visibly, instead of holding a continuation forever. Two tests, each proven by mutation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Juliusolsson05
added a commit
that referenced
this pull request
Sep 20, 2026
…entory Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1033 — the last finding from #1028's review.
The gap
Since #1028 the loop continues on Agent Code's own allowed
Stophook. But "allowed" only means our hook allowed it:query.ts).should_blockacross all handlers (stop.rs).So a user, project or plugin hook that blocks — "run the tests before stopping" — leaves the turn running after we were told it ended, and the continuation lands mid-turn as a queued command. That is #1024's symptom from a second cause, and the phase trigger had the same hole.
The fix
The turn boundary is no longer trusted on its own. Immediately before typing, the loop asks the provider whether input is ready — the same readiness signal the composer gate uses — and skips the delivery when it is not. The turn's real end re-triggers the loop, so nothing is lost and no state is invented.
A session main has no snapshot for answers yes, deliberately: "I cannot tell" must never silently stop a loop forever, and
deliverPromptToAgentstill refuses a genuinely unready target retry-safely.Test
A system test through the real MCP hook host: a
stoparrives while the provider reports input unavailable → nothing is delivered; readiness returns → the nextstopdelivers exactly once. It fails with the gate removed.npx tsc -bclean; 69 tests across the goal-loop and MCP suites.🤖 Generated with Claude Code