Skip to content

fix(native-chat): a message waiting on an unconfirmed stop goes out as soon as the old process exits - #24362

Closed
brennanb2025 wants to merge 26 commits into
mainfrom
brennanb2025/late-exit-wakes-held-send
Closed

brennanb2025 wants to merge 26 commits into
mainfrom
brennanb2025/late-exit-wakes-held-send

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 7 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​489 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​488
Prod 23 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​234 $\color{#cf222e}{\Huge{\mathbf{−}}}$​48 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​186

What this fixes, and where

The problem. After #24333, when you press Stop on a native chat and Orca can't prove the old agent process exited, your next message no longer fails: it waits in the chat under the note "Orca couldn't confirm Claude's previous process ended. Messages wait to be sent until Orca confirms it has ended." It goes out once a later retry proves the exit. But if the old process exits on its own after Orca's retry gave up, nothing told the host. The message kept waiting for the next retry: an action of yours (a model change, Steer on a queued card), or Orca's background check for idle chats, which runs every 5 minutes. So a process that exited a moment after the retry still left your message stuck for up to 5 minutes.

Why it happened. A retry can give up two ways, and neither left anyone listening:

  1. The proof failed (the process hadn't exited when the proof ran). When it exits later, Claude's and Codex's connections do see the exit, but while a close is under way they report nothing: they treat every exit then as the close's own business. With the close already finished, nobody hears it.
  2. The proof ran past the stop's 10 s step deadline. Orca stops waiting, but the close keeps running. When it later proves the exit, it updates the host's record that the process ended, and nothing more: the stop it belonged to had already given up, so nothing finishes the stop or sends the message.

This was a known gap in #24333, listed there as a Temporary difference.

Scope. Native (structured) chats, Claude and Codex, on a local or SSH host. Everything here runs on the host that owns the process, so over SSH it runs on the remote host. Terminal agent tabs are unaffected. A stop that proves the exit the first time behaves exactly as before.

ELI5

When Orca stops an agent, it waits to be sure the old process is gone before starting a new one. If it couldn't be sure, your message waits. Before, if the old process then quit on its own, Orca didn't notice until its 5-minute check. Now Orca notices the moment that process quits, double-checks, and sends your message right away.

What Changed

Before (for you):

  • After a Stop or tab close whose exit Orca couldn't confirm, a message you sent waited under the note even after the old process had exited, until your next action or the 5-minute check.

After (for you):

  • When the old process exits, Orca retries ending it right then. If that proves it gone, the waiting message goes to a fresh agent within moments, with nothing else from you.
  • If nothing was waiting, Orca still finishes the stop then, so the chat is ready for the next message without the 5-minute wait.
  • Nothing changes when the process never exits: the message keeps waiting under the same note, Stop still withdraws it, and the 5-minute check still retries.

Mechanism.

  • The connection reports an exit nobody is waiting for. Claude's and Codex's connections get a new callback, onExitAfterClose, called when the process is seen to exit after a close of it gave up, and only then. An exit seen while a close is still running stays the close's own business, as before, so a normal stop reports nothing new. Both connections decide this from the close's own result: Claude reports when it is closing and the last close returned unproven; Codex reports when its exit proof no longer holds an attempt, which happens only after an attempt failed to prove the exit (a proven attempt stays held). So the second "process ended" notice Node sends once the process's output pipes close, which can land after a close already proved the exit, reports nothing.
  • The adapter tells the host which process exited. The Claude and Codex adapters turn that callback into a new host lifecycle event, exitAfterClose, carrying the exact process it is about (the lease fence and the per-process generation). It travels the path the existing started and ended events already use, from the adapter to the host's lifecycle handler. Like started, it stays off the queue that handles crashed-process recovery one at a time across all chats, so a retry waiting on one chat never delays another chat's recovery; teardown still waits for it. The event is declared beside started and ended in structured-agent-session-adapter.ts.
  • A stop that proves the exit late says so. The host's stop now notes when its pass gave up. If the abandoned close later proves the exit, the host's record of the process ends as before, and the host now also asks for the owed stop to be finished. Several passes can be waiting on the same hung close (the Stop, a waiting message's retry, each idle check), and they all see it prove the exit at once; only the pass whose proof actually ends the host's record of the process asks, so one late exit runs one retry.
  • One way to finish. Both signals call the same host entry point, which runs on the chat's own one-at-a-time operation queue. It retries the owed stop (the same finishOwedStructuredAgentSessionWindDownUnderSerialize pass fix(native-chat): after a Stop whose exit can't be confirmed, the next message retries the stop instead of failing #24333 uses, with the stop's original cause) only if the owed stop is for exactly the process named; a report about any other process does nothing. When the retry lands, the stop's own last step wakes message delivery, as in fix(native-chat): after a Stop whose exit can't be confirmed, the next message retries the stop instead of failing #24333, so the waiting message goes out. Being a retry, it writes no Stop event of its own, so the person's Stop stays the one the cut turn's end reads (fix(native-chat): a Stop still reads as yours after Orca restarts, because the turn's end reads the Stop's event #24311): that turn still reads as their cancellation.
  • The report is evidence, not a verdict. Seeing the process exit does not end anything by itself: the retry runs the stop's own exit proof again (for Claude, the root process and its descendants; a root exit seen first-hand is final, as before). Losing contact with a process is still never treated as an exit, so the verdict stays live, unverifiable or exited, and only an observed exit triggers this.
  • The 5-minute check stays as the backstop for anything not reported (see Notes).

What ends each piece of state. No new stored state. The host keeps nothing new, and neither connection adds a flag: each reads "a close gave up" from the close's own result. Each report is used once: it either finds the owed stop for its process and retries it, or it finds nothing and is dropped. A retry that still can't prove the exit leaves the stop owed exactly as before, and the existing retries (your next action, the 5-minute check, quit) keep going.

Why

  • Listen where the exit is already seen. Both connections already watch their process exit; they just stayed silent during a close. Reporting from there needs no polling and no new process watcher.
  • Finish through the existing stop, not a new path. The owed stop already knows how to end the process's record at the time the stop was asked for, settle what the process left, release Orca's hold on the chat and send what waited. Re-running it keeps one place that does all of that.
  • Fence by the exact process. A chat outlives many processes. A late report about an old process must never retry, stop or settle a newer one, so the event names its process and the host acts only when the owed stop is for that same process.
  • Alternatives considered.
    • Report through the existing crashed-process event (ended with unexpected-exit). Rejected: the host settles that as a crash (it says the agent exited and records the turn as cut off by the exit), but this process ended because the user stopped it. The Stop's cause and its place in the chat must be kept.
    • Report every exit seen during a close. Rejected: every normal stop would then queue an extra no-op step on the chat's operation queue.
    • Retry on a short timer while a stop is owed. Rejected: a timer guesses; the exit is an event Orca already observes.

Differences from the common pattern

  • Intended: a late exit report doesn't end the process record by itself; the host re-runs the stop's own exit proof. Orca keeps one rule for "gone": a process is gone only once its exit is proven, so a new agent never starts beside one that may still run. An exit seen while a close is still running belongs to that close. If the close finishes within the stop's 10 s step deadline, the stop settles it as before. If the stop gave up first, the close keeps running and, once it proves the exit, asks the host to finish the stop (the late-proof signal above). The one case in between, a close that saw the exit and then failed for another reason, is left to the 5-minute check (see Notes).
  • Intended: a stop that couldn't finish is retried every 5 minutes, a rule inherited from fix(native-chat): after a Stop whose exit can't be confirmed, the next message retries the stop instead of failing #24333. The common pattern also re-stops a session whose stop failed, on a periodic sweep, but only after a long idle. Orca's check is more frequent and re-derives what is owed from the chat's state on each run, so it never acts on a stale record.
  • Intended: a stop's completion means the exit was proven. In the common pattern, a stop can complete without proving the process exited. Orca waits for the proof, which is what makes a message wait here at all.

Linked Issue

Follow-up to #24333 (its Temporary difference: a late exit isn't noticed). Builds on #24235.

Visual Proof

N/A: the state can't be forced on a real host. It needs a stop whose exit proof fails or runs past 10 s, and on macOS Orca always sees its own child's exit within the proof, so this can't be shown without admin rights (the same limit as #24333). The host-level tests below drive it on the shipping adapters and host.

Testing

New host-level tests run the shipping adapter and host against a fake connection that, like the real ones, refuses every write once a close begins.

Claude, in structured-agent-session-claude-unproven-stop-send.test.ts (#24333's file):

  1. The old process exits after the retry gave up: with a message waiting, the connection reports the exit; the message goes to a resumed Claude with no idle-check run, after exactly one more close of the old process.
  2. A stop that ran past its deadline proves the exit late: every close of the old process waits on an exit that comes only after both the Stop and the message's retry gave up (the test shortens the step deadline to 200 ms). Once the exit comes, the message goes to a resumed Claude with no idle-check run.
  3. A report naming another process: with a message waiting, an exitAfterClose for a different generation retries nothing and the message keeps waiting.
  4. A late report from an old process after a newer one started: the newer process's tab close is unproven; the old connection reporting its exit again does not retry the newer process's stop.
  5. Several passes gave up on one hung close: the Stop, the waiting message's retry and two idle checks all give up on the same close; when the exit comes and the one retry then fails to release Orca's hold on the chat, nothing retries it again right away (exactly one release attempt; before, every pass that gave up asked for its own retry).
  6. Nothing waiting: after an unproven Stop with no message waiting, the exit report finishes the owed stop: one more close, the hold on the chat released, no new agent started.

Also in that file, for #24311's Stop events: when only the late exit's retry ends the turn the Stop cut, the latest Stop event is still the person's, and that turn reads as their cancellation.

Codex, in the new structured-agent-session-codex-late-exit.test.ts (adapter events routed to the host as the runtime routes them):
7. The old app-server exits after its close gave up: a tab close unproven twice (the close and the message's retry), then the exit report; the message is sent on a new app-server as a turn/start.
8. A report from an app-server that is no longer the chat's: after that, the old connection reporting again publishes nothing and does not close the new app-server.

Connections and delivery:
9. Claude connection (claude-stream-json-connection-close.test.ts) and 10. Codex connection (codex-app-server-connection.test.ts): an exit after an unproven close is reported once through onExitAfterClose and not as an unexpected exit; an exit seen while a close is running reports nothing, and for Codex neither does the late second notice after a close proved the exit.
11. Lifecycle delivery (new structured-agent-session-lifecycle-delivery.test.ts): a report whose handling waits on its own chat does not hold up another chat's crash recovery, and teardown's drain still waits for it.

Ablations. Each reverts one piece and runs that piece's test file(s). Counts are the file sizes when each was run (the first set before tests 5 and 6 were added):

  • Claude connection report removed: test 9 fails (1 of 6). Reporting during a running close too: test 9 fails (1 of 6).
  • Claude adapter report removed: tests 1 and 4 fail (2 of 17). Adapter not checking the report's connection is the session's: test 4 fails (1 of 17).
  • Claude adapter event not passed to the host: tests 1 and 4 fail (2 of 17).
  • Host not routing exitAfterClose: tests 1, 4, 7 and 8 fail (4 of 19, Claude and Codex files together).
  • Host retrying whatever stop is owed, not only the named process's: test 3 fails (1 of 17).
  • Host not noting a pass that gave up (no late-proof signal): test 2 fails (1 of 17).
  • Every pass that gave up asking for a retry, not only the one that ended the process: test 5 fails (1 of 18; two release attempts instead of one).
  • Host entry point retrying nothing: test 6 fails (1 of 19).
  • Report put on the cross-chat recovery queue: test 11 fails (1 of 1).
  • Codex connection report removed: test 10 fails (1 of 33). Reporting during a running close too: test 10 fails (1 of 33). Codex connection back on the earlier "no close is running" rule: test 10 fails (1 of 33), on the late second notice after a proven close.
  • Codex adapter report removed: tests 7 and 8 fail (2 of 2). Adapter not checking the connection: test 8 fails (1 of 2). Codex event not passed to the host: tests 7 and 8 fail (2 of 2).

Suites. Locally, every test file that imports a module this PR touched, the new files, and the tests main's merge brought into these areas: 243 files, 2206 passed, 1 skipped. One real-process Claude connection test (proves a natural SDK exit and cleans up its descendant before recovery) failed its exit proof once under that full parallel run and passes alone (3 of 3) and with its whole file. CI runs the suites at 9d91204b70d.

Lint. oxlint on the changed files: clean. check:code-quality:changed: 0 new findings.

Typecheck. pnpm tc:node: passes.

Platforms: logic tests on macOS. Not exercised on a live Claude or Codex, Windows, Linux or a real SSH host.

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

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

  • Security: no new inputs, paths or processes. The retry is the existing stop.
  • Cross-platform: the exit is the one each connection already observes on every platform; the retry runs the same per-platform exit proof as Stop.
  • SSH: the connection, adapter and host all run on the host that owns the process; nothing crosses the wire.
  • Mixed versions / wire: none. The new event is in-process on the execution host; no RPC, stream frame or published content changes.
  • Mobile: no change.
  • Performance: nothing on a normal stop. One queued step per late exit.
  • Still left to the 5-minute check: a close that ran past the deadline, saw the exit, and then failed for another reason (for Claude, writing the resume point). Its exit was seen while that close was running, so there is no later exit to report, and the close did not prove the exit, so there is no late proof either.
  • Line budget: structured-agent-session-host.ts was at its 300-line limit. The restart lease reconciler, which only restart restore, attach and start use, now lives in the restore factory that already wrapped it (its own refactor commit, no behavior change: still one instance with the same in-flight sharing). That leaves room for the new wiring with every explicit return type kept. The new lifecycle event sits beside the others in structured-agent-session-adapter.ts, which stays under its limit.

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)

…ld not verify

Adds the `previousExitUnverifiable` failure fact, a status row only, never a
reason a message was not sent: Orca couldn't confirm the agent's previous
process ended, and the message will send once it has. Copy in every catalog.
…ies the stop, then waits

When a Claude Stop could not prove its child gone, the child stayed in place
with a connection that refuses writes, so the next message failed with
"Orca couldn't hand this message to the agent" and every later one did too,
until the idle sweep retried the stop after 30 minutes of quiet.

The delivery loop now retries an owed stop before it starts or writes to a
child, once per new message. Still unproven, the message stays queued under one
warning row saying why, and the idle sweep's next tick retries the stop, child
or not, and hands the message over once the exit is proven.
…owed stop first

Option changes, card answers, background-task stops, goal changes and rewinds
went to a child a Stop could not prove gone, whose connection takes no input.
The retry now lives in one place, `finishOwedStructuredAgentSessionStop`:
`ensureStructuredAgentSessionAgent` calls it before reporting or starting a
child, and operations on the running child prepare through it. Still unproven,
it refuses with `previousExitUnverifiable` and the exit `unverifiable`; the
delivery loop turns that refusal into the visible wait, so a send still waits
under its one row instead of being refused.
…e that waited on it

- The stop itself wakes delivery where its owed wind-down clears, so a message held on an
  unproven exit goes out whichever retry lands: an option change, a background-task stop, the
  sweep or the next message. Only the sweep woke it before, so an option change that proved the
  exit left the message queued with nothing to send it.
- The owed stop keeps where it was asked for, and the child's end is ordered there. A message
  accepted while retries ran is no longer rejected as "chat closed" (a tab close) or failed as a
  host stop when the retry that proves the exit lands after it.
- A wind-down owed by an earlier child never stops a different live child in front of it.
…nd its note stays true

- A waiting message holds against the newest pass that failed to prove the exit, kept on the
  owed-stop record (`failedAt`), not against the note's position. The note is written once per
  process, so after a second message every later journal commit re-ran a retry of up to 10 s.
- The note no longer promises this message will send: "Messages wait to be sent until Orca
  confirms it has ended." stays true after a Stop withdraws the message, a close, or a restart
  rejects it. Every catalog follows; es and zh lose the mismatched informal possessive, and the
  French reads naturally.
- Tests: commits after a second waiting message retry nothing; with follow-up queueing on (the
  default) a follow-up becomes a draft, and Steer retries once and waits under the same note.
… a new ask

A second tab close of a chat whose exit stayed unproven kept the first close's position, so a
message sent between the two closes was delivered once a retry proved the exit, even though the
second close closed it (its own rejection of what was queued had failed). Continuation is now
explicit: only the retry of an owed stop passes `retry`; every other stop stamps a new position.

Tests: a second close whose rejection fails still closes the message it closed; the default-
queueing test waits for a draft's commit to settle before asserting it retried nothing, and bounds
Steer's retries; the one-retry-per-message test has the budget to show each commit's retry.
When another operation's retry of the unproven stop failed between a message's accept and its
delivery step, the step held the message (it had already waited through a failed retry) and
stopped before writing the note, so the message sat Working with no reason shown. The hold now
makes sure of the note too; it is written once per unproven process, so its own commit still
retries nothing.
…t on bookkeeping

When a stop proved the old process gone but a later step of its wind-down kept failing, an option
change retried that bookkeeping, and refused the change when it failed again. On main the change
was kept at rest. Operations that start no child (option change, card answer, background-task stop)
now hold back only while the exit itself is unproven, its child still on record; with the exit
proven, the bookkeeping retry is reported and the operation goes on as with nothing owed. Sends
still wait: a start needs the released lease that bookkeeping gives back.
…hat stop at once

A stop that could not prove the child's exit (its proof failed, or ran past
the 10 s step deadline) left a waiting message until the idle sweep's next
5-minute tick, even when the old process exited moments later: the connection
saw that exit but told no one, and a stop that proved it late had no caller.

Both are now reported. The Claude and Codex connections report a root exit
seen after a close gave up; the adapters publish it as an exitAfterClose
lifecycle event for that exact child; and a stop step that proves the exit
after its pass gave up says so too. Either way the host retries the owed stop
on the session's own lane, only for the child it names, and the stop that
lands hands over what waited on it. The sweep stays the backstop.
…fter its stop gave up

Claude and Codex, on the shipping adapters and host: a late exit report, and a
stop that proves the exit after its deadline, each send the held message with
no sweep tick. A report naming another child, or from a connection that is no
longer the chat's, touches nothing. Connection-level checks pin when an exit is
reported (after a close gave up, never during a running one), and the delivery
keeps the report off the cross-session exit chain.
… as a late exit

Node reports a child's end twice: 'exit', then 'close' once its stdio drains.
When 'close' landed after a close's proof had settled true, the connection
reported an exit after a close gave up, because it gated on "no close is
running" rather than "the last close gave up". The adapter usually dropped it
(the session was already gone); when settlement admission had refused, the
host ran one needless retry. The gate now reads the exit proof itself, which
drops an attempt only when it was unproven, the same rule the Claude
connection uses.
… passes gave up on it

Every stop pass started while a Claude close hangs joins that one close, so a
Stop, a held message's retry and each sweep tick could all give up on it. When
the exit finally proved, each of them asked for a retry of the owed stop. If
the first retry then failed a bookkeeping step, every other ask ran another
full pass back to back, each with its own 10 s step deadline, holding up that
chat. Now only the pass whose proof actually ended the child asks; a failed
retry waits for the sweep or the next action, like any other.
…it already wraps

The host built the restart lease reconciler only to hand it to the restore
factory and to attach and start. The factory now builds that one instance from
the host's probe and clock and exposes it, so attach and start share the same
in-flight reconcile as before. The host file regains the room to keep its
explicit return types on flushStreamedEvents and subscribe.
@OrcaWin

OrcaWin commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

QA screenshots (M4Air)

Head SHA tested: 58351a1b74207fcb7a1d2eea020e7299a64f0f44. Dev build on macOS (M4 Air) with an isolated HOME, CODEX_HOME and profile. Screenshots are CDP captures of a hidden window.

No credentials were copied into the isolated HOME, so both chats ran against stub agents placed first on the rig's PATH. The claude stub speaks the stream-json protocol, and the codex stub speaks the app-server JSON-RPC protocol. Each reply prints the stub's process ID.

Claude regression (normal Stop, then send): PASS

  1. First message is mid-response, with a partial reply showing and Stop available.
    02a
  2. After Stop: "Interrupted after 17s". The stub (pid 6436) got the interrupt, then stdin close, then SIGTERM, and exited.
    02b
  3. The second message went to a fresh agent process (pid 14971) and got a reply. There was no "couldn't hand this message" error and no warning note (both checked in the page text).
    02c

Codex regression (normal Stop, then send): PASS
4. First Codex message is mid-response, with Stop available.
04a
5. After Stop: "Interrupted after 10s". The app-server got turn/interrupt. A Codex Stop only interrupts the turn; it does not close the app-server.
04b
6. The second message started a new turn on the same app-server (pid 45453), got a reply, and the turn finished ("Worked for 0s"). There was no error and no warning note.
04c

Late-exit path (message held after an unproven stop, then delivered when the old process exits): BLOCKED on this host, so there are no screenshots for it.

  • Claude: Orca stops its direct child (the provider supervisor) by SIGKILL through its own process handle, so the root's exit is always seen and the stop is never left unproven. This matches the earlier QA of the base PR.
  • Codex: a Stop never closes the app-server. An unproven close needs a chat close where the supervisor fails to exit within its 5.5 s bound and the descendant check then fails, for example because a child born in the same second as the snapshot ignores SIGTERM. Only then does Orca resume the root without killing it. Building that from the UI would mean freezing Orca's supervisor and racing birth seconds, which would not be a fair test of the product.
  • That path is covered only by the PR's unit tests here.

@brennanb2025

brennanb2025 commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor Author

Review status (head 9d91204): review finished; draft until a final verification pass

The problem. After #24333, if you press Stop on a native Claude or Codex chat and Orca can't confirm the old agent process ended, your next message waits under the note "Orca couldn't confirm Claude's previous process ended. Messages wait to be sent until Orca confirms it has ended." If that process then exited after Orca's retry had given up, nothing told Orca. The message sat there until your next action or Orca's background check, which runs every 5 minutes.

What changes for you. When that old process exits, Orca notices right away, re-checks that it's gone, and sends the waiting message to a fresh agent within moments. You don't need to do anything else. If nothing was waiting, the chat is simply ready again. A normal Stop, where the exit is confirmed at once, behaves exactly as before.

Fixed during review (each with a test that fails without the fix):

  • A Codex stop that had confirmed the exit could still report a "late exit" when Codex's output pipe closed a moment after the check. Orca now reports a late exit only when the last stop attempt actually gave up, the same rule the Claude connection already used. (2efc967)
  • One late exit could set off several back-to-back retries, one for every stop attempt that had given up while waiting on the same stuck stop. Now only the attempt that actually records the process as ended asks for a retry. (5d040c8)
  • To stay under the file-size limit, structured-agent-session-host.ts had dropped two explicit return types. The restart lease check now lives with the restart-restore code that already wrapped it, and both return types are back. No behavior change. (6d23c5f)
  • Added a test for a claim the PR makes: an exit with no message waiting still finishes the stop and starts no agent. (58351a1)
  • PR body: corrected the claim that "a close that is still running settles its own exit", which isn't true past the 10 s step deadline. Also fixed one test description.

Deferred (small, none user-visible):

  • Test-only: one Codex test checks a count without waiting for queued work, one test comment is inaccurate, and no single test runs the connection, adapter and host together.
  • Code tidiness: an unreachable guard in the Codex close code, a no-op branch in the Codex journal translator, a 15-line Codex event mapper that could live inline, and a now-stale "no line budget left" comment in structured-agent-session-host.ts.
  • Two existing gaps that this PR doesn't touch: a Codex forced close after a journal failure that can't confirm the exit, and a Claude close after a fault that gives up.

Update: merged #24333's latest branch (cef967c, which brings in current main). The one code change this needed: the error catch on the late-exit retry now reports through the host's new required logger, under the scope stop-after-exit, because #24312 removed the old callback. The late-exit report also gets its own log scope (lifecycle-exit-after-close), and a test checks it. A separate check of the conflict resolutions found nothing lost from either side. tc:node passed, 1,981 tests in 224 files pass, oxlint and the code-quality gate are clean, and CI is green at cef967c (18 passed, 15 skipped). QA was not re-run after this merge: it changed only error reporting, and the earlier QA below ran on 58351a1.

Update: #24333 landed on main; this PR now targets main. Merged current main (de6cffe). The four conflicts all came from #24333's squashed copy meeting the same lines here; after the merge, this PR's diff against main is only its own 29 files, and the lockfile is identical to main's. Then moved the late-exit event back into structured-agent-session-adapter.ts beside the other lifecycle events and deleted the extra file (72279e3; the adapter file is at 258 of 300 lines). tc:node passed locally. The local test run couldn't finish because the machine's disk was full (21 of 229 files failed on "no space left on device" errors only), so CI was the test gate: green at 72279e3 (18 passed, 15 skipped). QA was not re-run.

Update: merged main again (05e26ce, including #24311, which changed how a Stop is recorded so that a turn's end reads the person's Stop). The conflicts were three import lines. One extra change was needed to stay under the file-size limit: claude-structured-session-acquisition.ts reached 301 lines, so the code that applies a turn's first status message to the session moved, unchanged, into observeClaudeInitFacts in claude-structured-init-proof.ts. A separate check confirmed it matches line for line. #24311 and this PR work together: the late-exit retry is a retry of the original Stop, so it writes no Stop record of its own. The person's Stop stays the one the cut turn reads, so that turn still shows as their cancellation. A new test pins this (9d91204). tc:node passed; 2,206 tests in 243 files passed locally, apart from one real-process connection test that failed once under full parallel load, passes alone, and can't reach this PR's change. oxlint and the code-quality gate are clean. CI is green at 9d91204 (18 passed, 15 skipped).

Verified:

  • Three independent reviews (connection layer, host, architecture) and a follow-up review of the fixes. The design was kept: two signals are needed, one for "the process exited after the stop gave up" and one for "the stop's own check finished after its deadline".
  • The readiness checklist passed: no wire or saved-data change, no new timers or polling, and it runs on whichever machine owns the process, including over SSH.
  • tc:node typecheck passed. 1,498 tests in 169 related files passed. oxlint is clean. CI was green at 58351a1.
  • On the M4Air, a normal Stop followed by another message works for Claude and Codex, with no error and no note: screenshots.

Not verified:

  • The late-exit path itself on a real machine. On macOS, Orca always sees its own Claude child exit, and forcing an unconfirmed Codex close would mean freezing Orca's own supervisor. That path is covered only by the tests.
  • The order in which competing stop attempts see the exit was checked with a model of the promise chains, not by running the shipped code.
  • Whether a retry triggered just before quit delays quit's cleanup of that chat (at most one 10 s step) was not measured.

# Conflicts:
#	src/main/native-chat/agent-session-wire/structured-agent-session-idle-sweep.ts
…en retries that stop, then goes to a fresh Codex
…-stop' into brennanb2025/late-exit-wakes-held-send

# Conflicts:
#	src/main/native-chat/agent-session-wire/structured-agent-session-event-recovery.ts
#	src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.ts
#	src/main/native-chat/agent-session-wire/structured-agent-session-host.ts
#	src/main/native-chat/agent-session-wire/structured-agent-session-reveal.ts
#	src/main/runtime/structured-agent-session-lifecycle-delivery.ts
#	src/main/runtime/structured-agent-session-runtime.ts
@brennanb2025
brennanb2025 changed the base branch from brennanb2025/send-after-unproven-stop to main October 1, 2026 20:34
…t-wakes-held-send

# Conflicts:
#	src/main/native-chat/agent-session-wire/structured-agent-session-claude-unproven-stop-send.test.ts
#	src/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.ts
#	src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.ts
#	src/main/native-chat/agent-session-wire/structured-agent-session-host.ts
…fecycle events

The exit-after-close event and the lifecycle union had moved to their own file
only because the adapter contract file was over its line limit on the stacked
base. Main's split of that file left room, so both go back beside the started
and ended events, and the extra file is gone.
@brennanb2025
brennanb2025 marked this pull request as ready for review October 1, 2026 21:02
@greptile-apps

greptile-apps Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Fixes message delivery timing after a failed process stop.

The PR appears safe to merge; no outstanding or new actionable finding was identified.

Summary

The PR reports a native-chat process exit after an unproven close and retries the stop owed for that process, allowing a waiting message to proceed without waiting for the idle sweep.

  • Adds Claude and Codex exit notifications, host lifecycle routing, and regression tests.
  • Keeps the stop’s original cause and checks the process identity before retrying.
Diagram
sequenceDiagram
  participant Process as Old process
  participant Adapter
  participant Host
  participant Queue as Chat operation queue
  Process->>Adapter: Exits after close gave up
  Adapter->>Host: exitAfterClose(process identity)
  Host->>Queue: Queue fenced stop retry
  Queue->>Queue: Prove exit and finish owed stop
  Queue->>Host: Wake waiting message delivery
Loading

Reviews (2) · Last reviewed commit: "test(native-chat): a late exit's retry k..."

@coderabbitai

coderabbitai Bot commented Oct 1, 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: 23e7fca6-a18b-4960-917e-622c9a52d9bb

📥 Commits

Reviewing files that changed from the base of the PR and between 72279e3 and 9d91204.

📒 Files selected for processing (11)
  • src/main/claude/claude-structured-init-proof.ts
  • src/main/claude/claude-structured-session-acquisition.ts
  • src/main/claude/claude-structured-session-adapter.ts
  • src/main/claude/claude-structured-session-state.ts
  • src/main/codex/codex-structured-journal-translation.ts
  • src/main/codex/codex-structured-session-close.ts
  • src/main/codex/codex-structured-session-state.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-claude-unproven-stop-send.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-conversation-lifetime.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-lifetime.ts

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


📝 Walkthrough

Walkthrough

Claude and Codex connections now report child exits observed after close gives up proving exit. Session lifecycle handling routes these events to serialized recovery for matching owed wind-downs. Runtime delivery tracks non-ended event settlements, and tests cover late exits, stale child reports, retries, and held messages. The host restore factory now constructs and exposes lease reconciliation.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 9d912

This change delivers a waiting message promptly once an old Claude or Codex process exits after a stop that could not be confirmed. Recovery is limited to the matching process, so a stale report cannot affect a newer one. No merge-blocking issue was found in the supplied changes.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 9d912

The change resumes waiting messages after a matching stopped process exits. Identity checks and ordered cleanup constrain that handoff, and no concrete security regression was found in the reviewed path. Broader exposure and rollback coverage remain incomplete.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The reviewed handoff affects a session's child, journal settlement and stored ownership lease. A report must match the owed generation and fence, and a different active child prevents the old obligation from being selected.

Trust Boundaries and Controls

  • observed — The examined producers derive late-exit evidence from child-process observations, not provider protocol payloads. They associate reports with the current session connection. The consumer passes child identity into serialized recovery rather than granting release authority to the report itself.

Resilience and Maintainability Implications

  • observed — Failed or timed-out eviction retains the owed wind-down. Duplicate stop passes schedule late-proof recovery only when they actually end the matching child. Once conversation lifetime is disposed, late reports no longer start this recovery path.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 30 functions across 30 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 is concise, specific, and accurately describes the primary user-facing fix: releasing a message when the old process exits after an unconfirmed stop.
Description check ✅ Passed The description is detailed and on topic. It covers the user impact, mechanism, rationale, alternatives, testing, visual proof, platform scope, risks, and checklist. It includes an issue reference and…
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

Autopilot is currently an internal CodeRabbit preview.


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.

✅ No new issues found.

Reviewed changes

  • Connections report a late exit. claude-stream-json-connection.ts and codex-app-server-connection.ts gain onExitAfterClose, fired only when the root is seen exiting after a close that already gave up (Claude: closing && !closePromise; Codex: exitObserved && closing && !exitProof.holdsAttempt). The new RetryableProcessExitProof.holdsAttempt getter is what distinguishes "a proven attempt is held" from "the attempt failed".
  • New exitAfterClose host lifecycle event. Both adapters turn the callback into it (reportClaudeExitAfterClose / reportCodexExitAfterClose, each filtered on connection identity; Codex also on session.ended), carrying fence + acquisition generation. It rides the started/ended path, stays off the cross-chat crash-recovery chain, and is still drained at teardown.
  • Late-proof retry in the host. stopStructuredAgentSessionAgentUnderSerialize now flags gaveUp when its deadline aborts, and onProviderChildStopped schedules finishStopAfterExit only for the pass whose proof actually ended the child. Both signals converge on finishOwedStructuredAgentSessionWindDownAfterExitUnderSerialize, which re-checks pendingProviderChildWindDown + sameProviderChild before re-running the existing owed-stop retry with the stop's original cause.
  • reconcileLeases refactor. Moved into createStructuredAgentSessionHostRestore; verified behavior-equivalent (one instance, same probe/now/probeMany, returned to attach/start).
  • Tests. New host-level Claude/Codex late-exit files, connection-level report tests, and a lifecycle-delivery drain test; each new behavior has a stated ablation.

I traced the concurrency and fence guards independently (two specialist passes): the per-session serialize lane, endProviderChild's one-winner match, execution-time identity re-check, and the mutually-exclusive A/B triggers hold for the interleavings I could construct, including several passes giving up on one hung close and a newer child starting.

ℹ️ Non-blocking: Claude only clears closePromise on the return-false path

Worth noting, not a request for changes — this mirrors an existing contract and I could not demonstrate a reachable trigger.

Technical details
# Claude `close()` reset is asymmetric with Codex

## Affected sites
- `src/main/claude/claude-stream-json-connection.ts` — `closePromise = null` happens only inside the
  non-throwing `if (!proven)` branch; a close IIFE that *rejects* leaves `closePromise` non-null and
  `closing` true forever.
- `src/main/codex/codex-app-server-connection.ts` + `src/shared/child-process/retryable-process-exit-proof.ts` —
  `RetryableProcessExitProof.run` clears `inFlight` on both `false` and rejection, so Codex still reports.

## Required outcome
None required for this PR. Recorded so a future change that makes a Claude close able to reject knows the new
`onExitAfterClose` report silently depends on `closePromise === null`.

## Notes
- No test exercises a throwing close; all new Claude tests go through `proveClaudeChildExit` resolving `false`.
- A fix would be one line (clear `closePromise` on rejection, as Codex does), but it guards a case that is not
  currently reachable, so it is out of scope here.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

… end of the turn it cut

The stop that lands after the old child exits late is the owed stop's retry,
so it writes no Stop event of its own: the person's Stop stays the latest, and
the turn it cut, ended only by that retry, reads as their cancellation.
@brennanb2025
brennanb2025 marked this pull request as draft October 2, 2026 04:03
@brennanb2025
brennanb2025 marked this pull request as ready for review October 2, 2026 04:26

@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 new issues found.

Reviewed changes

This run reviewed the delta since the prior pullfrog review at 72279e34f5: a single test-only commit. No production code changed, and the underlying mechanism still matches the previously approved shape.

  • Late-exit Stop-event test. structured-agent-session-claude-unproven-stop-send.test.ts gains "keeps the person's Stop as the cut turn's end when the old child exits late, writing no Stop of its own". It drives an unproven Stop deliberately without framing an interrupted result, holds a message, then fires onExitAfterClose and asserts the late retry writes no Stop event — the person's user-stop mark stays latest — and the cut turn still reads interrupted / cancellation. This pins the #24311 contract that the late retry reuses the owed stop rather than issuing a new Stop. The test is non-vacuous: it first asserts the captured mark matches { event: { reason: 'user-stop' } }, so a missing mark fails before the equality check.

The connection predicates (closing && !closePromise for Claude; exitObserved && closing && !exitProof.holdsAttempt for Codex), the host's gaveUp/finishStopAfterExit retry fence, and the off-chain lifecycle delivery are unchanged from the reviewed-and-approved head.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Superseded by #24862, which removes the root cause this PR worked around: #24333's stored unfinished-stop record that a waiting message sat behind. In #24862 the process's own exit ends that record, and the next start joins the one shared close, so there is no late-exit signal to add. Branch kept for reference.

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.

2 participants