Repository navigation
fix(native-chat): a host stop decides from the agent's live state, with no wait - #24503
brennanb2025 wants to merge 3 commits into
Conversation
…th no wait A close, an eviction or the host stopping a start must decide, before it kills the child, whether it ends work and so must write its Stop event. It waited up to a second for the event sink to flush the provider's rows into the journal, and when the wait ran long it guessed from the latest accepted send. The journal trails the child: an acceptance lands straight in it while the turn row that follows queues behind the sink, and a turn's end row can queue while the journal still shows the turn running. Each adapter now reports what its child has in flight, read off the provider's own frames as they arrive (`liveWork`): Claude's open turn or unechoed send, Codex's running turn (by the journal id of a command it carries), a send on its way, or a send answered into a turn not yet open. The host stop reads that, plus the sends the journal holds unanswered, in its own serialized step. The event names the journal's running turn, or the live one when its row has not landed. A person's Stop still holds against a later host stop while every send in flight is one it stopped, now counting a send accepted whose turn row has not landed. Removes the drain, its one-second bound, and the slow-drain fallback.
…p-decides-from-live-state
Review status: held, to be superseded after #24127The problem this PR targets. When Orca stops an agent on its own (you close the chat, a worktree is torn down, a start is stopped), it decides whether the agent was working so the cut turn can read as your cancellation ("Interrupted after N") rather than news ("Failed after N"). Before this PR that decision waited up to 1 s for streamed history rows to be saved, and under load it could miss a turn the agent had just opened, so closing that chat read "Failed". What this PR does. It drops the wait and asks each agent connection what it has in flight (a new adapter member, Why it is held, not shipped. That gives the host two sources for "what is in flight", the saved history and the connection's view, and the review found they disagree in user-visible ways:
There is also a design reason. A person's own Stop still decides from the saved history alone, so with this PR the two kinds of Stop would answer from different sources. Plan. #24127 (one ordered history writer) applies each accepted row to the history in the same step it is received, so a stop can read the history directly with no wait. Once it lands, this PR will be superseded by:
This matches the common pattern by mechanism: one ordered store decides and names the cut turn, with no wait. Stacking on #24127 now was ruled out, because it has conflicts and its base predates #24311. Interplay with #24478. Whichever lands second must:
Deferred (P3, to fold into the superseding PR):
Verified:
Not verified:
The PR stays a draft. No code was changed in this review. |
ELI5
When Orca stops an agent on its own (you close the chat tab, a worktree is torn down, or a start that never came up is stopped), it first has to decide: was the agent doing anything? If so, it writes down why the work stopped, so the chat can later say "you stopped this" rather than "this failed."
Orca used to answer that by waiting up to a second for the agent's latest messages to be saved, then reading the saved history. Now it asks the agent connection directly what is running at that moment, with no wait.
The problem
Since #24311, every host-side stop (a close of the chat, an eviction, the host stopping a start) writes a lasting Stop event when it ends work. Later, the end of the cut turn reads that event: your close shows as your cancellation (muted "Interrupted after N"); any other stop shows as news ("Failed after N").
To decide "does this stop end work?", the host read the saved history, but the history lags the agent:
#24311 bridged that gap with a wait: up to 1 s for the streamed rows to be saved, then a guess when the wait ran long ("an accepted send with no turn yet counts as working").
That wait is the only one of its kind in the Stop path, and both reference designs decide from live state with no wait (see "Differences" below). It also had real gaps:
What changes for you
What changed (mechanism)
Each adapter reports what its agent has in flight (
liveWork, new optional member on the adapter contract). It reads the provider's own messages as they arrive, before they are saved:claude-request-end-wait.ts).The host stop decides in its own serialized step, with no wait (
workStopEnds):Removed: the 1 s drain, its timeout and the slow-drain fallback (
latestAcceptedSendUnopenedas a public read).SSH and paired hosts: the decision runs on the execution host, where the host and its adapters live, and reads only that host's in-memory state. No wire change, no new persisted field.
Why this approach
structured-agent-session-close-verdict.test.ts"records the user's close on a turn whose start landed only as the provider stopped" caught this; it goes red without the name.Differences from the common pattern
Linked Issue
None (maintainer). Follow-up to #24311 (Stop event part B).
Compatibility
Visual Proof
N/A: the chat looks the same. What changes is whether, under load, a turn you close reads "Interrupted" or "Failed". The automated tests below pin that.
Testing
New
structured-agent-session-host-stop-live-view.test.tscovers each host stop kind (your close, an eviction, the host stopping a start):It also checks that a person's Stop holds against a later close or eviction, both for the turn it named and for a send it stopped whose turn row has not landed, and does not hold over a send made after it.
Adapter tests:
Existing tests updated:
stop-event-entriesand the drain-failure test are replaced by the live-view file.close-verdict's adapter reports the cut turn as in flight.surface-lifetime's drain-failure helper goes back to failing the eviction's own drain, since the stop no longer drains first.Runs:
--no-file-parallelism): 4 passed. The first cold run timed out while materializing the release checkout and passed on rerun.Ablations (each rule removed alone and the named files rerun; every one goes red):
Not run:
claude-structured-session-integration.test.ts: it may spawn the real CLI.