Skip to content

fix(native-chat): a Stop binds only the turn it actually stopped - #24864

Merged
brennanb2025 merged 24 commits into
mainfrom
brennanb2025/stop-binds-at-settle
Oct 5, 2026
Merged

brennanb2025 merged 24 commits into
mainfrom
brennanb2025/stop-binds-at-settle

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 15 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1481 $\color{#cf222e}{\Huge{\mathbf{−}}}$​80 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1401
Prod 24 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​591 $\color{#cf222e}{\Huge{\mathbf{−}}}$​218 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​373

Step 1 of 2. #24369 ("Stopping…" state) is step 2 and will be restacked on this PR. The whole plan is at the end.

ELI5

When you press Stop, Orca has to decide which turn your Stop ended, so that turn reads "Interrupted after N" (you stopped it) rather than "Failed after N" (it died on its own). Before, if you pressed Stop before the turn had appeared, Orca guessed: it gave your Stop whatever turn your message opened later, even if the Stop had actually stopped nothing. Now your Stop only gets the turn it really stopped. And if Orca can't reach a Codex turn that is about to start, it ends Codex so the turn can't run on behind your back.

What Changed

The problem. Two things went wrong when you pressed Stop right after sending, before the turn showed in the chat.

  1. Wrong label. Orca decided whose a turn's end was at the moment the end was written, by re-reading the latest Stop. For a Stop that named no turn, it assumed the Stop owned the turn its stopped message opened later. That held even when the Stop found nothing to interrupt ("Codex had no turn running to stop.") and Codex then ran the turn anyway. If that turn later died on its own (Codex crashed, or Orca restarted and settled it), it read "Interrupted after N" with the muted "you cancelled this" mark, and no row said the agent had stopped.
  2. A turn could run after Stop said there was nothing to stop. A Codex Stop pressed while Codex was still starting the turn waits up to 5 seconds for it to open, then interrupts it. If the turn took longer than that, or Codex's answer to the send was lost (a timeout), the Stop gave up, said "Codex had no turn running to stop.", and Codex could still open and run the turn.

Before (for you):

  • You press Stop just after sending. Codex is slow to open the turn, the chat says "Codex had no turn running to stop.", and the turn then runs to the end anyway.
  • A turn your Stop never touched, which later crashes, reads "Interrupted after N" as if you had cancelled it.

After (for you):

  • If Codex has answered your message but hasn't opened its turn yet, Stop interrupts that turn right away, and the turn reads "Interrupted after N". If Codex says the turn isn't running yet, Stop waits for it to start, up to 5 seconds, and tries once more.
  • If that turn never starts (the wait ran out, or Codex reported the thread idle or failed without ending the turn), or Codex's answer to your message was lost, Stop ends the Codex process. A first and a second press answer the same way. The turn never runs, and nothing reads as working. The message you stopped is taken back as never sent (its text returns to your composer), never left "unconfirmed" with a Retry, so the next message you send goes out at once and starts Codex again, as after any Stop that ends the process. No "Cancellation requested." row is left behind: with your message gone there is no turn for it to describe, and it used to sit under the turn before, so that earlier finished turn read as if your Stop had cancelled it. When the message can't be taken back (Codex may already hold it), the row still reads "Cancellation requested." after it.
  • "Codex had no turn running to stop." now appears only when there really was nothing in flight to stop. If Codex couldn't be reached and its process couldn't be ended either, the row reads "Cancellation was not confirmed.", and pressing Stop again still reaches that turn.
  • A turn reads "Interrupted after N" only when your Stop actually stopped it. A turn that dies on its own after a Stop that stopped nothing reads "Failed after N", with the usual red "The agent stopped while this response was in progress." row.
  • Unchanged: a Stop pressed while a turn is showing names that turn, and that turn reads as yours, live and after a restart. Claude's Stop still ends its process; a turn that opens in the moment before that end reads as yours. The same holds when you close the chat while a message's turn is just opening.
  • Unchanged: after your Stop, if Orca later evicts or stops the chat while nothing is running and nothing was sent since, your queue stays paused and a held card waits for you, whatever your Stop did.

Mechanism.

  • The adapter names what it stopped. A Stop's answer from the provider (AgentSessionCancelOutcome) gains an optional turnId: the turn its interrupt actually took. Codex fills it in for both named and unnamed Stops (codex-structured-prompt-ownership.ts).
  • A Stop that named no turn binds at settle. When such a Stop starts, performCancel opens its settle (stopMarks.beginSettle). Until the settle closes, any turn that ends is that Stop's. When it closes, the Stop keeps only the turn its interrupt took (structured-agent-session-turns-cancel.ts, journal-stop-turn-end.ts).
    • Codex's process end after a failed interrupt runs inside performCancel, so the settle is still open around it. Claude's Stop hands the process end to its next step (endStoppedStructuredAgentSession), and the settle stays open until that step finishes, whether or not the exit was proven, so a turn the dying process opened reads as yours.
    • A person's close of the chat that named no turn opens the same settle once its event lands, and closes it when the close's child end is done.
    • A press opens a settle only when the latest Stop event is its own, or the one in force it repeats. A Stop naming a turn already over, a card's own Cancel, or a press whose event row failed to write reopens no earlier Stop.
    • The binding lives in memory on the journal's latest-Stop mark. It is never written to a row and never sent to a client.
  • The Stop ends the turn it took, once. If the provider took the interrupt but the turn still reads running when the Stop settles, the Stop writes that turn's interrupted end itself, once (settlement id stop-settled:<turnId>). A provider's own end after it still wins. Codex answers an interrupt it took before it sends the turn's interrupted end, so with Codex this write usually comes first.
  • No more guessing about later turns. Deleted from the turn-end rule (journal-stop-turn-end.ts): the inference that an unnamed Stop owns the turn opened by a send it "stopped" (isStopTarget, the opener branch of the cancellation check, and unansweredSendsAreStopTargets), plus the openedBy parameter the Claude result reader passed for it and the Claude translator plumbing that only fed it. openingSubmission stays, used only by latestAcceptedSendUnopened, which asks whether a send's turn has opened, not whose turn it is.
  • Codex: a turn Codex answered but hasn't opened is interrupted at once. Codex takes an interrupt as soon as the turn starts, which can be before Orca has read its turn/started. Before the turn starts, or once it has ended, Codex answers -32600 "no active turn to interrupt". interruptOpeningCodexTurn (codex-structured-turn-open-wait.ts) sends the interrupt at once. Only on that refusal, while the turn has neither opened nor ended, does it wait for the turn to open, up to 5 seconds, and send the interrupt once more.
    • A turn that ended meanwhile was nothing to stop: the row says "Codex had no turn running to stop." and the process keeps running.
    • A turn that never opens (the wait ran out, or the thread stopped running), and a send whose answer was lost (dispatch unknown, not recovered), answer as a refusal marked turnMayOpen. The host's existing path for a failed interrupt (fix(native-chat): a Codex Stop that Codex refuses or never answers ends the Codex process #24334) then ends the child, because the send still counts as in flight. If that end fails too, the row says "Cancellation was not confirmed." (an existing failure kind).
    • A later Stop still finds a turn an earlier wait gave up on while its send is owed. It goes through the same steps, wait included, so both presses reach the same verdict.
    • A send made before the turn opens still waits for it to open before it steers into it (before 0.148, Codex refuses a steer until then).
    • A send whose turn never opened is withdrawn, not left in doubt. Codex records a prompt only once its turn has started, so a Codex send no turn opened for never ran. When a Codex child ends with a person's Stop in force since the send, and the send started its own turn that never opened (its handover found no turn running, so its message belongs to no turn, and no turn has opened since), the settlement of the child's end withdraws that send, the verdict Codex's own interrupted-turn end already gives a send it never echoed. Before this, the end left it in doubt, and the chat's queue held every later message behind it until the person pressed Retry on the message they had stopped. It is read from the journal at whichever settlement lands, so a retry after a failed child end withdraws the same sends; nothing is stored. Any other end keeps today's verdict: a send that joined a running turn at its handover (a steer, which may already be in that turn), a send with no recorded place, a send made after the Stop, a host's close or a crash with no person's Stop in force, and Claude, whose child may already hold what was written to it. A host's own stop that lands while a person's Stop is still in force, with no turn opened for the send, withdraws it too: Codex never recorded that prompt either.
  • The host's deference to your Stop. A host stop or eviction does not write its own Stop event over a person's Stop that is still deciding the work: with a turn running, the turn that Stop named or bound; with none, while that Stop's queue pause still holds and nothing was sent since (both read from rows), or while its settle is open. A host event there would lift your queue pause.

Only native (structured) Codex and Claude chats change, on the host that runs the agent, local or SSH. Terminal agent tabs are unaffected.

Why

The old rule tried to answer "was this turn the Stop's?" at the moment the turn's end was written, long after the Stop finished. By then it no longer knew what the Stop actually did, so it guessed from which message opened the turn. Binding at the Stop's own settle uses the one moment that knows the answer: the provider just said which turn it interrupted, or that it found none.

A note on live Codex turns: Codex answers an interrupt it took first, and only then sends the turn's "interrupted" end. So the Stop usually settles, and writes the turn's end itself, before Codex's own end arrives. The binding labels that first end row; Codex's own end, which carries Codex's verdict, then agrees with it. Both orders read "Interrupted after N" from the first end row, and a test drives the real order end to end. The rule also decides the other ends written with no verdict of their own: a Claude result after its interrupt, the host's fallback end when it ends the process, and a relaunch's settle.

Alternatives considered:

Differences from the common pattern

  • Matches: a Stop binds the turn it interrupted when the Stop settles, and a Stop that settles with nothing to interrupt binds nothing. "Nothing to stop" holds only when nothing was sent.
  • Matches: when the interrupt can't be carried out, the provider's process is ended (fix(native-chat): a Codex Stop that Codex refuses or never answers ends the Codex process #24334's existing recovery); the next message starts it again.
  • Matches: a Stop of a turn Codex answered but hasn't started waits for it to start, with a bound (here 5 seconds), and interrupts it once it has; a turn that never starts fails the Stop. Every press waits again.
  • Intended: the interrupt is sent at once, and the wait happens only when Codex refuses it as finding no active turn, followed by one retry. Reason: Codex takes the interrupt as soon as the turn starts, before Orca reads turn/started, so sending first saves that round-trip when the start is already in flight. Otherwise it costs one refused request, and the Stop then waits as it would have.
  • Intended: a Codex send is withdrawn at a person's Stop only when no turn was open for it in the journal, but Codex can still start that turn while it is being ended. Orca treats the process as gone at its exit event, before its output is fully read, and drops what arrives after that. And Codex works through a request it already read, such as a turn/start whose answer was lost, before it reads the end of its input. A turn/started in that window never reaches the journal, so that send reads withdrawn though Codex may have recorded it. Reason: it needs Codex to start the turn in the last moments of a process the Stop is ending; the alternative leaves every such Stop's message in doubt, which holds the chat's queue until a Retry that re-sends the message the person stopped.
  • Intended: a Stop that took back a message whose turn never opened leaves no Stop row. In the common pattern the stopped message stays in the transcript and the stop row is tied to the run it stopped, never to an earlier one. Orca returns that message to the composer instead (see Notes), so there is no run left for a row to describe, and a conversation-level row would land under the previous finished turn. Reason: the row's meaning, "this run was stopped", has nothing to point at; the returned draft shows what happened, as Orca already does when a Stop withdraws queued messages. Follow-up: adopt the common withdrawal presentation (the stopped message stays on screen, marked as not sent, with one stop row after it): feat(native-chat): keep a message a Stop took back on screen, with one stop row after it #25051.
  • Intended: what an unnamed Stop binds is held in memory only. A host crash between the interrupt and the turn's end therefore leaves that turn reading "Failed after N". Reason: this follows the relaunch rule decided for this plan (an unsettled Stop counts only for the turn it named; a Stop that named no turn binds nothing), which removes the guess about which turn a turnless Stop owned that caused the original bug; on restart the child is ended, so the turn still settles. A Stop that named its turn (the usual case: one pressed while a turn shows) still reads as yours after a restart, because its event names the turn.

Compatibility (SSH and mixed versions)

  • SSH: the binding, the settle and the end write all run on the host that runs the agent (local, or the remote Orca over SSH). Clients only read the rows it writes.
  • No wire change. No new field, row kind or stream frame. Turn rows already carry outcome: 'cancellation', which every released client renders as "Interrupted after N" with the muted mark, and Stop rows use existing text.
  • An older client on this host sees the same rows with the corrected outcome, plus "Cancellation requested." instead of "Codex had no turn running to stop." when Codex is ended. A newer client on an older host sees that host's old labels.
  • The settle's end write is an ordinary lifecycle batch row, which every released build reads.

Linked Issue

None: maintainer change, step 1 of the #24369 rework.

Visual Proof

Live QA on a separate Mac against a background build of #24369's head (4888740, which contains this branch at 5b60057), driving a native Codex chat backed by a stand-in Codex that can open a turn normally, slowly, or never. Desktop: both cases below passed. Phone: not run.

Stop on a send Codex never started. The stand-in accepts the send but never opens its turn. Stop is refused ("no turn running"), so about 5 seconds later the Codex process is ended. The text goes back into the composer, the button reads Send, and the chat says neither "Message delivery is unconfirmed." nor "Cancellation requested.". The next message runs normally on a new process.

Right after Stop 12 s later: text back in the composer Next message
r10-06r-never-open-right-after-stop r10-06r-never-open-plus12 r10-06r-never-open-recovery

A send made while Stopping whose turn then never starts. The first turn is stopping when the second message is sent; it waits until the first turn ends "Interrupted", then Codex accepts it but never opens its turn. A second Stop puts the text back into the composer. No "unconfirmed" row and no "Cancellation requested." line anywhere.

Sent while Stopping First turn ended, second sent 12 s after the second Stop Next message
r10-06r-b-right-after-send-b r10-06r-b-b-turn-starts r10-06r-b-plus12 r10-06r-b-recovery

Testing

  • I manually tested these changes locally (live desktop QA on a separate Mac, see Visual Proof; phone not run)
  • Automated tests added/updated, or explained why not below

New tests:

  • journal-stop-turn-end.test.ts: the settle binding, settled with and without a turn, a relaunch binding nothing, a repeat press, and named and host Stops opening no settle.
  • structured-agent-session-stop-settle-binding.test.ts, at host level:
    • a Codex Stop whose wait runs out ends the child, so the turn never runs and nothing reads as working;
    • a turn opened before that end reads as yours;
    • a Claude Stop in the gap before its send echoes labels the turn that opens before the process end;
    • a person's close before the send's turn showed labels the turn its child end cuts;
    • a Codex Stop that couldn't reach the turn and whose kill failed reads "Cancellation was not confirmed.".
  • structured-agent-session-codex-interrupt-order.test.ts, end to end through the shipped host, journal and Codex adapter: Codex answers the interrupt first and ends the turn after the Stop has answered, as real Codex orders them. Every end row of the stopped turn reads interrupted by the Stop: for a Stop naming the turn, one naming none, and one pressed while turn/start was in flight. The fake Codex used by these tests now answers an interrupt before it ends the turn (it used to do the reverse); four tests that read the turn's end right after a Stop now wait for it.
  • structured-agent-session-codex-turn-end-settlement.test.ts, end to end through the shipped host, journal and Codex adapter with a fake Codex child:
    • a Stop pressed while turn/start is in flight interrupts the turn that opens, which reads interrupted, and nothing reads as working;
    • a turn that doesn't open in time ends with the child.
  • structured-agent-session-stop-event-binding.test.ts:
    • the turn the interrupt took is bound even when its end lands after the Stop answered;
    • a turn ending during the settle reads interrupted from its first end row;
    • no turn opening after a Stop that stopped nothing is bound;
    • the settle ends a still-running stopped turn;
    • after a turnless Stop that stopped nothing, took, or whose kill failed, an eviction writes no event, the queue stays paused and the held card is not handed off; mail sent after the Stop still gets the host's event;
    • a Stop naming a turn already over reopens no earlier Stop's settle.
  • structured-agent-session-stop-event-restart.test.ts: a turnless Stop Orca died while settling binds no turn after the relaunch; a Stop that named its turn still reads "Interrupted after N" after one (Claude and Codex).
  • codex-structured-turn-open-wait.test.ts:
    • an interrupt Codex takes at once once its thread runs the turn;
    • one refused before the thread runs, then sent once more when the turn opens;
    • nothing to stop, with no wait, when Codex already ended the turn, and nothing when the turn ends without opening;
    • a refusal once the wait runs out, and for a lost answer; nothing for a recovered one;
    • a second Stop after the wait gave up waits again, answers refused the same way, and stops the turn if it opens during that wait;
    • a thread reported idle or failed answers refused on the first press and on a second alike.
  • structured-agent-session-codex-turn-end-settlement.test.ts also covers, end to end: refused, then the turn opens, then the retry is taken and the send withdrawn; refused and never opened, so the child ends; a turn that ends first reads "Codex had no turn running to stop." with no process end; after a finished turn, a Stop that takes back the next send leaves no row and the finished turn stays "completed", including when Codex's answer to that send was lost; a send kept in doubt keeps "Cancellation requested.".
  • structured-agent-session-stop-note-withdrawn-send.test.ts: a Stop that ends the child writes no row when the child's end took back the send with no turn running, including when an earlier process's send is still in doubt beside it, and still writes "Cancellation requested." when the send stays in doubt or when it stopped a running turn.
  • structured-agent-session-unopened-send-withdrawal.test.ts: the child's end takes back a send still being handed over or one whose answer this process lost, but never a send an earlier process left in doubt (its child may have started it before the crash) and never a queued card's send.

Every new rule was checked by breaking it and watching its tests fail:

  • the settle window: 4 tests red;
  • the turn bound at settle: 4;
  • a settle that never closes: 7;
  • the host deferring while the pause holds: 3; with nothing sent since: 2;
  • a person's close opening its settle: 1; its settle closing once the close is done (a conversation that outlives it, as /clear does): 1;
  • a wait that ended on an idle thread answering like one that ran out: 3;
  • a press opening a settle only for its own Stop: 1;
  • the "unconfirmed" row: 1; a second Stop reaching a turn left unopened: 1; the Codex refusal being marked: 2;
  • the named-Stop rule after a relaunch: 6;
  • Codex refusing a turn still opening: 1, plus 2 end-to-end;
  • Codex refusing a lost answer: 1;
  • Codex naming the stopped turn: 8;
  • the wind-down closing Claude's settle: 1;
  • the settle's end write: 1;
  • binding at settle: 2;
  • with Codex's real order, the settle window: 1 (the Stop's own end row then has no verdict, which reads as failed until Codex's end lands); the settle's end write: 3;
  • a press whose event row failed reopening no earlier Stop: 1;
  • a send whose turn never opened withdrawn at the child's end: 2 (including a failed child end that a later retry lands); a turn that opened keeping its send in doubt: 1; a send steered into the open turn kept in doubt: 1; a send made after a Stop that started its own turn, whose stopped turn's end arrives late, withdrawn: 1; a send made after the Stop, whose child then dies, kept in doubt: 1; a host's close with no person's Stop keeping it in doubt: 1; Claude untouched: 1; the client dropping a withdrawn send so the next one goes out: 1;
  • no Stop row once the send is taken back (always writing the row): 1 end-to-end, 1 unit, and the 3 updated conversation-Stop tests; dropping the "taken back" check: 1; dropping the "no turn running" check: 1; counting only sends still being handed over, not one whose answer was lost: 1 end-to-end. The row check and the withdrawal read one shared rule, so the two cannot count different sends; that rule taking an earlier process's send in doubt: 3; taking a queued card's send: 2;
  • interrupting an answered turn at once (waiting first instead): 13; retrying once after it opens: 4; waiting on the refusal at all: 12; a turn that ended meanwhile being nothing to stop: 3; a turn that never opened failing the Stop: 5; a second press waiting again: 3.

Run locally:

  • 260 test files covering the Stop, cancel, turn-end, restart-settle, translators, Codex and Claude cancellation (including Claude's pinned "in doubt after its child ends"), the one ordered journal writer's tests (fix(native-chat): one ordered journal writer #24127), every file using the fake Codex, the dead-generation and eviction tests, plus every test importing a changed module: all pass (2,605 tests; 1 skipped as before).
  • Cross-version wire suite, run serially: 14 files, 86 tests, all pass.
  • tc:node and tc:web pass.
  • oxlint and the anti-slop config on the changed files pass, as does the changed-code quality gate.

Platforms: unit and host-level tests only, run on macOS. The change has no platform-specific code.

Review

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Ensure no issues in: Security, Cross-platoform support (Linux, Windows, Mac), Remote SSH, Mobile, general backwards compatibility, performance

  • Performance: the binding is two fields on an object the journal already holds. The settle's end write takes one journal snapshot, and only when the provider took the interrupt.
  • Mobile: reads the same turn rows; no change.
  • Follow-up: a withdrawn message's text goes back to the sender's composer, Orca's existing presentation of every Stop withdrawal; the common pattern keeps it in the transcript and marks the thread interrupted.
  • Follow-up: a message withdrawn before its turn opened has no turn row, so its header reads the client's own "Worked for Ns" rather than "Interrupted". Showing anything else would mean inventing a turn Codex never opened; what to show is to be decided against what comparable hosts render.
  • Follow-up (pre-existing on main, not changed here): when Codex refuses or never answers the interrupt of an open turn and Orca ends the process, a steered message Codex never echoed is left in doubt, and the chat's queue holds later messages behind it until Retry. Whether that send is truly in doubt (Codex may have taken it), and whether a person's Stop should leave the queue held, is a separate change.
  • Follow-up (pre-existing on main, not changed here): a Codex question or approval card's own Cancel interrupts through a route that writes no Stop event, so if Orca restarts before that turn's end is written, the turn reads "Failed after N". Live it reads correctly from Codex's own verdict. Fix: route that interrupt through the recorded Stop, so its event names the card's turn.

The whole plan (step 1 of 2)

  1. This PR: a Stop binds only the turn it actually stopped, decided when the Stop settles. A Codex turn that may still open after the Stop gave up on it ends with the process.
  2. feat(native-chat): the chat reads "Stopping…" from your Stop until the turn actually ends #24369, restacked on this: the "Stopping…" state. A chat reads Stopping while it is working and either a person's Stop is still settling on the host, or a Stop that failed is bound to the turn still running, until that turn ends. A Stop that settles with nothing to stop ends Stopping; a failed one holds it. feat(native-chat): the chat reads "Stopping…" from your Stop until the turn actually ends #24369 then reads this PR's settle binding instead of re-deriving the state from the journal. Its wire flag, its one shared display function, Stop staying enabled and the steer hold are unchanged.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

A Stop pressed before any turn showed used to claim, at end-write time,
whatever turn the stopped send later opened, even when the Stop stopped
nothing. A turn that then died on its own read as "Interrupted" (your
cancellation) instead of "Failed".

Now a person's Stop that named no turn binds, in memory only, every turn
that ends while the Stop settles, and afterwards only the turn its
interrupt took. The settle ends a still-running stopped turn once. A
relaunch finds nothing in memory, so an unsettled turnless Stop binds no
turn. A Codex Stop whose answered turn does not open within its wait, or
whose send's answer was lost, now answers refused, so the host ends the
child and the turn can never run.
brennanb2025 added a commit that referenced this pull request Oct 2, 2026
…g state

Stopping now reads #24864's settle: while working, a person's Stop is still
settling, or is bound to the turn running now (the turn it named, the turn
its settle bound, or the turn a failed Stop could not stop), until that turn
ends. A Stop that settles having stopped nothing ends Stopping, so a send
made after it is handed over and no later turn reads Stopping.

A settle edge writes no row, so it bumps a revision the status projection
keys on and tells the journal's commit listener, which republishes the
status and wakes the steer hold's delivery loop.

The rule that kept a "took" Stop row from being overwritten by a later
press is dropped: the turn's label, bound at settle, now carries what the
Stop did. A failed Claude wind-down still revises the Stop's row, and now
also binds the turn running on.
…er a Stop settles

A host stop or eviction with no turn running now defers to a person's
Stop while its queue pause still holds with nothing sent since, read from
rows, so a held card is not handed off on reopen after a Stop that did
nothing or whose kill failed.

A person's close that named no turn opens a settle around its child's
end, so a turn that end cuts reads as theirs.

A press opens its settle only when the latest Stop event is its own or
the one in force it repeats: a late Stop, a card's interrupt or a lost
event row reopens no earlier Stop.

A Codex Stop that cannot reach a turn still able to open says the Stop is
unconfirmed rather than that no turn ran, and a second Stop still reaches
a turn an earlier wait left unopened.

Also drops the unused openedBy plumbing and the unreachable "a written
cancellation stays one" rule, and pins a relaunch after a named Stop.
brennanb2025 added a commit that referenced this pull request Oct 2, 2026
…still owed, and pin the close's settle

A Codex Stop that waited for a turn Codex answered a send into now answers
"may still open" whenever that turn neither opened nor ended and its send
is still owed, however the wait ended (it ran out, or the thread went
idle). Before, a first press after an idle thread said no turn was
running and kept Codex, while an identical second press ended it.

Adds a test that a person's close the conversation outlives (as /clear
does) closes its settle, so a later turn that ends on its own reads as a
failure.
…ds-at-settle

# Conflicts:
#	src/main/native-chat/agent-session-wire/structured-agent-session-chat-stop.ts
#	src/main/native-chat/agent-session-wire/structured-agent-session-queued-stop.ts
#	src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts
…earlier Stop

With one ordered journal writer the Stop's event is in the fold when its
write returns, so the press reads whether it owns the latest Stop from the
fold instead of awaiting the write. Pins the case the read must refuse.
brennanb2025 added a commit that referenced this pull request Oct 3, 2026
…e ordered journal writer) into the Stopping state

Conflicts: chat-stop, delivery-loop, host-delivery, turns-cancel (main dropped
flushStreamedEvents; the wind-down stays in its own module), and the English
runtime catalog (both sides' keys kept). The wind-down's note revision now reads
the fold directly: every journal write lands by its call's return.
…eads interrupted throughout

Codex answers an interrupt it took before it sends turn/completed (interrupted):
on TurnAborted the app-server answers pending interrupts, then ends the turn, on
one channel. The test fake did the reverse. It now answers first and ends the
turn on a later read, and the tests that read the turn's end right after a Stop
wait for it.

New end-to-end test through the shipped host, journal and Codex adapter: with
the real order, every end row of the stopped turn reads interrupted by the Stop
(named, unnamed, and a Stop pressed while turn/start was in flight). Breaking the
settle window turns the in-flight case red: the Stop's own end row then has no
verdict, which reads as failed until Codex's end lands.
…as not opened at once

A Stop that named no turn, made after Codex answered a send but before the turn
opened, used to wait up to 5 s for the turn to open before interrupting, and
ended the Codex process when it didn't. The stated reason, that Codex refuses an
interrupt until it opens the turn, holds only part of the time: with no turn
active, Codex takes an interrupt once its thread runs (turn_interrupt_inner),
and refuses it with -32600 "no active turn to interrupt" before that or once
the turn has ended.

The Stop now sends the interrupt at once. Only on that refusal, while the turn
has neither opened nor ended, does it wait for the turn to open (bounded at
5 s) and send it once more. A turn that ended meanwhile was nothing to stop. One
that never opens, or that an earlier wait already gave up on, fails the Stop,
and the host ends the child as before. Sends still wait for the turn to open
before steering into it.

The test fake models Codex taking an interrupt once the thread runs (run()).
…t, as the first did

A Stop whose interrupt Codex refused as finding no active turn skipped the wait
when an earlier wait, a Stop's or a send's, had already given up on that turn.
Every press now waits its own bound and retries once if the turn starts, so a
turn that opens during a later press is still stopped. Both presses still reach
the same verdict when it never starts.
…ened withdraws its send

A Stop on a Codex turn that was answered but never opened ends the Codex
process. That end settled the send as in doubt (unknown, recovered), and the
client's outbox holds every later send behind a send in doubt until the person
presses Retry, which re-sends the very message they stopped. The chat looked
stuck.

Codex records a prompt only once its turn has started, so a send whose turn
never opened never ran. When the Stop's refusal says so (turnMayOpen), the child
end now settles the unanswered sends as withdrawn, the verdict Codex's own
interrupted-turn end already gives an unechoed send. The flag rides on the owed
wind-down, so a retry after a failed child end withdraws them too. Claude's
child end still leaves its unanswered send in doubt.
…ver opened

Replaces the flag the Stop carried to the child's end, and its copy on the owed
wind-down, with a reading of the journal at the settlement that lands. A Codex
child's unanswered send is withdrawn when a person's Stop is in force since it
was sent and no turn row ran, or was written, after it; any other end (a turn
that opened, a host's close, a crash, Claude) still leaves it in doubt. A
retried wind-down reads the same rows, so it withdraws the same sends.
The derived withdrawal read a turn as open for a send only if it still ran or
was written after the send. A send steered into a running Codex turn whose
interrupt failed met neither once the adapter's end settled that turn ahead of
the host's settle, so it read withdrawn, though Codex drains a steer into the
running turn and may hold it. A turn that ended after the send was handed over
was open for it too: such a send stays in doubt, as before.

Pins that case, and that a send made after the Stop, to a child that then dies
before its turn opens, stays in doubt.
… turn, not by timing

Whether a turn was open for a send was read from end times: a turn that ended
after the send's handover counted. A send made while a Stop ended the turn is
handed over once that turn reads ended, yet Codex's own end for it can arrive
later, so such a send whose own turn never opened read in doubt again, and the
chat's queue held behind it.

The handover already records where the send went: its message joins the turn
running then (a steer) or belongs to no turn (it starts its own). Only a send
that started its own turn, with none opened since, is withdrawn; one that
joined a running turn, or has no recorded place, stays in doubt.

The Codex test fake takes an answered interrupt as Codex does, dropping the
turn before its end arrives.
…at never ran

A Stop on a Codex send whose turn never opened ends the child, and the child's end
takes the send back into the composer. The Stop still wrote "Cancellation
requested." at the conversation level, so with the send gone it sat under the
previous finished turn and read as if that turn had been stopped. A Stop that found
no turn running and whose child end took back every send it found now writes no
row; a Stop of a running turn, or one that leaves a send in doubt, still does.
@brennanb2025
brennanb2025 marked this pull request as ready for review October 3, 2026 17:07
@coderabbitai

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (1)
AGENTS.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 8b8f73fd-99a1-4e00-819c-5d19fa7189f2
📥 Commits

Reviewing files that changed from the base of the PR and between 2daeeea and d8422dd.

📒 Files selected for processing (3)
  • src/main/native-chat/agent-session-wire/structured-agent-session-dead-generation-settlement.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-stop-event-restart.test.ts
  • src/shared/structured-agent-session-outbox-withdrawn-send.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

The changes revise how Stops select and bind turns, including unnamed Stops that remain active while settling. Codex cancellation can target an answered turn that has not opened, wait for it to open, and report when a turn may still open. The journal can withdraw qualifying Codex sends whose turns never opened. The changes also remove Claude’s openedBy turn attribution and add tests for Stop outcomes, settlement, restart, and send recovery.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to d8422

This change makes a Stop bind to the turn it actually interrupted and withdraws Codex sends whose turns never opened. No concrete merge-blocking problem was found in the reviewed changes.

Security Architecture Review

Security architecture risk: 🔵 Low · up to d8422

The reviewed cancellation paths retain existing access checks and distinguish confirmed cancellation from uncertain execution. No introduced security weakness was identified, but complete runtime behavior and recovery guarantees were not independently verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The reviewed effects remain scoped to the selected session's provider child, submissions, and journal. The traced entrypoint does not add a new caller class or cross-session selection authority; this does not establish coverage of all downstream provider actions.

Trust Boundaries and Controls

  • observed — The existing cancellation RPC enters host mutation admission before provider and journal effects. Admission retains caller-key and session-envelope evaluation, host-computed payload fingerprints, and operation replay handling; the base/head comparison shows no change to that admission module.

Resilience and Maintainability Implications

  • observed — Unexpected-exit handling checks child fence and acquisition generation and flushes lifecycle events before settlement. Recovered terminal dispatch states and stable settlement identities support repeated settlement without reopening withdrawn sends.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 59.46% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 74 functions across 37 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: a Stop binds only the turn it actually stopped.
Description check ✅ Passed The description covers the required explanation, changes, rationale, linked-issue context, visual proof, testing, review boundary, notes, and checklist. The maintainer exception explains why no issue …
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ Minor suggestions only.

Reviewed changes — a rework of how a native-chat Stop decides which turn it ended: the binding now happens at the Stop's own settle rather than when the turn-end row is written.

  • Settle binding — performCancel opens an in-memory settle on the latest unnamed person's Stop; while open every interrupted turn end is that Stop's, and the turn the provider's interrupt took is bound when it closes.
  • Codex opening-turn interrupt — interruptOpeningCodexTurn sends the interrupt immediately, waits and retries once on Codex's "no active turn" refusal, and answers turnMayOpen when the turn never opens so the host ends the child.
  • Unopened-send withdrawal — a Codex send whose turn never opened is marked rejected at the child's dead-generation settlement instead of left in doubt.
  • Removed inference — the old "an unnamed Stop owns the turn its stopped send opened" rule (isStopTarget, unansweredSendsAreStopTargets, and the Claude openedBy plumbing) is deleted.

ℹ️ A retried turnless user-close settles its turn with the settle already closed

A user-close that named no turn opens its settle when its event lands, and closes it in stopStructuredAgentSessionAgentUnderSerialize's finally — done, proven or not. If the eviction aborts before settleWork (a step failure or the step deadline), the wind-down is retried with retry: true, which short-circuits stopEndsWork and writes no new settle. The retry's settleWork then writes the cut turn's end with settling already false, so it reads as the provider's news rather than the person's close — the opposite of the PR's stated "a turn that opens in the moment before that end reads as yours". This is the reverse direction of a settle leak (an under-label, not a mislabel) and only on the abort-and-retry path.

Technical details
# Retried turnless user-close loses the settle binding

## Affected sites
- `src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.ts:236-239` — the `finally` closes the settle on the first (aborted) attempt.
- `src/main/native-chat/agent-session-wire/structured-agent-session-host-stop-event.ts:47` — `stopEndsWork` returns false for `ending.retry`, so `recordStopEvent` (and its `beginSettle`) never runs on the retry.
- `src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.ts:182-207` — the retry's `settleWork` writes the fallback turn end, now unbound.

## Required outcome
A turn the retried `user-close`'s child end cuts reads as the person's close (muted "Interrupted after N"), matching the non-retry path.

## Suggested approach (optional)
Reopen the settle for the retried `user-close` on the existing latest Stop (e.g. `beginSettle()` when `ending.retry && ending.cause === 'user-close'`) and close it once the retry's eviction finishes, or bind the retry's fallback end some other way. A targeted test — abort the first eviction before `settleWork`, let the turn open, then retry — would pin it.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

// is in the fold by its call's return, so a newer latest Stop is this press's event.
const ownsLatestStop =
reach === 'unrecorded'
? ctx.journal.stopMarks.latest()?.sequence !== priorStop?.sequence

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reads the fold synchronously to decide whether this press owns the latest Stop, which holds only while the journal write queue is idle. It is not idle when the journal owes an import that failed: openConversationForWrite swallows the failure and the write queue keeps owed set, so appendStopEvent joins the queue and this comparison sees the old latestStop, dropping opensSettle and the settle binding for a Stop that did take effect. (The converse — binding a stale Stop — is not reachable: there is no await between tookEffect() and this read.)

Technical details
# `ownsLatestStop` can miss this press's own Stop event

## Affected sites
- `src/main/native-chat/agent-session-wire/structured-agent-session-chat-stop.ts:115-122` — `tookEffect()` is issued, then the fold is read synchronously.
- `src/main/native-chat/agent-session-wire/structured-agent-session-send-preparation.ts:78-88` — `openConversationForWrite` logs and proceeds when the owed import fails.
- `src/main/native-chat/agent-session-journal/journal-write-queue.ts:78-84` — `payOwed` clears `owed` only on success, so a failed import keeps the queue owing and defers every later write into `join`.
- `src/main/native-chat/agent-session-journal/journal-write-queue.ts:110-119` — the deferred write commits later, after this read.

## Required outcome
`opensSettle` reflects the Stop event this press actually wrote, even when that write lands asynchronously.

## Suggested approach (optional)
Decide ownership from the write's result (e.g. compare the `AgentJournalCursor` returned by `appendStopEvent` against `priorStop`) rather than the synchronous fold, or reopen the settle once the event lands. A test that fails an owed import, presses Stop with a running turn, then lets the turn end would pin it.

@brennanb2025
brennanb2025 marked this pull request as draft October 3, 2026 18:58
@brennanb2025
brennanb2025 marked this pull request as ready for review October 3, 2026 19:05
@brennanb2025
brennanb2025 marked this pull request as draft October 3, 2026 20:18
@brennanb2025
brennanb2025 marked this pull request as ready for review October 3, 2026 20:21
@brennanb2025
brennanb2025 marked this pull request as draft October 4, 2026 06:06
…s it back

The no-row rule counted only pending sends, but the child's end also takes back a send this process left in doubt when Codex's turn/start answer was lost. That case still wrote "Cancellation requested." under the previous turn. Both now read one predicate, so they cannot drift apart.
A send an earlier process left in doubt is never withdrawn and never holds the row back, and a queued card's send is never counted.
@brennanb2025
brennanb2025 marked this pull request as ready for review October 4, 2026 09:04

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — minor suggestions inline.

Reviewed changes — the two commits since the prior pullfrog review (5b60057), plus a re-verification of the settle-binding paths they touch.

  • One take-back predicate — sendStopCanTakeBack (structured-agent-session-unopened-send-withdrawal.ts) is extracted and now drives both the withdrawal and tookBackEverySend in performCancel, so a Stop whose child end took back a send whose turn/start answer was lost (unknown, not recovered) no longer writes "Cancellation requested." under the previous turn. Before, the no-row rule counted only pending sends while the withdrawal also took unknown ones, so the two could disagree.
  • Tests — new unit coverage for sendStopCanTakeBack / withdrawCodexSendsNoTurnOpenedFor, an end-to-end lost-turn/start case, and an earlier-process send that must not be taken back. Assertions are exact and can fail.

One open minor issue remains, carried from the prior review (inline).

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

Comment on lines +236 to +238
} finally {
// A person's close binds what its child's end cut; done, proven or not, it binds no more.
void recorded.then((settle) => session.journal.stopMarks.settled(settle))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A user-close that named no turn opens its settle here, and this finally closes it unconditionally. If the first eviction pass aborts before settleWork, the retry (retry: true) short-circuits stopEndsWork, so no settle reopens; the retry's fallback turn end is then written with settling already false and reads "Failed after N" instead of the person's "Interrupted after N". Only the abort-then-retry path is affected.

Technical details
# Retried turnless user-close loses the settle binding

## Affected sites
- `src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.ts:236-238` — the `finally` closes the settle on the first (aborted) attempt.
- `src/main/native-chat/agent-session-wire/structured-agent-session-host-stop-event.ts:47` — `stopEndsWork` returns false for `ending.retry`, so `recordStopEvent` (and its `beginSettle`) never runs on the retry.
- `src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.ts:182-207` — the retry's `settleWork` writes the fallback turn end, now unbound.
- `src/main/native-chat/agent-session-journal/journal-stop-turn-end.ts:95` — `settle.settling === true || settle.turnId === turnId` is false, so no `outcome: 'cancellation'` is applied.

## Required outcome
A turn the retried `user-close`'s child end cuts reads as the person's close (muted "Interrupted after N"), matching the non-retry path.

## Suggested approach (optional)
Reopen the settle for the retried `user-close` (e.g. `beginSettle()` when `ending.retry && ending.cause === 'user-close'`) and close it once the retry's eviction finishes, or keep it open across the first abort instead of closing it in `finally`. A targeted test — abort the first eviction before `settleWork`, let the turn open, then retry — would pin it. No current test covers this path.

…ds-at-settle

# Conflicts:
#	src/main/runtime/structured-agent-session-codex-turn-end-settlement.test.ts
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review and QA status (head 2daeeeaea1a)

The problem this fixes. When you press Stop, Orca decides which turn your Stop ended, so that turn reads "Interrupted after N" (you stopped it) rather than "Failed after N" (it died on its own). Before this PR, a Stop pressed before the turn appeared was given whatever turn your message opened later, even when the Stop had stopped nothing. A Codex turn that was accepted but never started could also leave the chat reading "Working" forever.

What changes for you. A Stop only claims the turn it actually stopped. If Orca can't reach a Codex turn that is about to start, it ends Codex so that turn can't run on after your Stop. A turn you stopped still reads "Interrupted" after Orca restarts.

Review

Round Scope Result
1 Full review by a fresh reviewer, checked against the common pattern Found 2 regressions against main (fixed): a host stop after a Stop of yours that didn't take could release your paused queue and send a held message on reopen; closing a chat while a turn was starting no longer marked that turn as yours. Plus 4 smaller issues (fixed). Restart behaviour, persistence and the same-millisecond close case all held.
2 Final review of those fixes and a main merge 2 small issues (fixed): both Stop presses now agree when a Codex turn is still owed; the close's settle window is pinned by a test.
3 Readiness checklist Pass, no P0/P1/P2. It flagged that the last two commits had no review of their own.
4 Review of those last two commits One P3 (fixed): when Codex's answer to a send was lost, Stop took the message back but still wrote "Cancellation requested." under the previous turn. The withdrawal and the note now share one rule, with a real-host test. A fresh reviewer confirmed the fix.
5 Check of every difference from the common pattern Passed.

QA

Live on a separate Mac: a dev build driven by an automated checker, with a stand-in Codex in its real reply order and the real Claude CLI. Passed: Stop mid-turn (Stopping, then Interrupted, never Failed) in both reply orders; Stop before the turn opens; a second press; relaunch while stopping (Codex and Claude); and a Stop with nothing sent. This ran on #24369's head, which contains this PR, not on this PR alone.

Verified and not verified

  • Verified: CI green at 2daeeeaea1a (current main merged; its one conflict was in a test file); 14,602 tests across every module that imports a changed file; all 17 cross-version files.
  • Not verified live: the lost-answer fix (round 4), which is covered by tests only; the phone, though this PR only changes the host and the phone shows what the host decides.

Follow-ups

  • STA-9379: after a Stop whose process end failed, "Cancellation was not confirmed." stays under the previous turn even after a retry takes the message back.
  • STA-9415: a Stop can wait about 10 seconds behind a message Codex hasn't acknowledged yet.

@brennanb2025
brennanb2025 merged commit 8f846d4 into main Oct 5, 2026
33 checks passed
brennanb2025 added a commit that referenced this pull request Oct 5, 2026
main #24864 (a Stop binds only the turn it stopped): recordStopEvent now resolves to the settle a
person's close opens, and the stop closes it once the child's end is done. Re-applied on this
branch's child close: the close's `recorded` carries that settle (null when no row is written),
and the stop closes it in a finally around the restart snapshot and the close join (which awaits
the exit settlement), proven or not. A second stop joining the same close closes it again, which
is a no-op.
brennanb2025 added a commit that referenced this pull request Oct 5, 2026
- journal-store.ts imports: main #24576 dropped journalStoreLoadedFields; this branch types
  appendResolvedItem off JournalItemAppender, so neither it nor JournalResolvedItem is imported.
- event-sink-queue: keeps this branch's journalItems; journalStopDecidesTurn takes main #24864's
  two arguments (openedBy plumbing removed on main).
brennanb2025 added a commit that referenced this pull request Oct 6, 2026
…n C2's settlement

Main's withdrawn-send test drives the outbox reconcile module C2 folded into
its settlement, so it could not load. Its two cases are ported as C2 states
them: a send the host withdrew at a Stop goes back silently, and one the host
left in doubt is its record (no Retry holds the queue); either way the next
send goes out.

Two Stop tests read the draft right after the Stop, but since the hand-back
starts the drafts' load itself and waits for it (R18), they now wait for it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant