Skip to content

fix(native-chat): a message is accepted, then delivered - #22821

Merged
brennanb2025 merged 73 commits into
mainfrom
brennanb2025/chat-accept-then-deliver
Sep 28, 2026
Merged

brennanb2025 merged 73 commits into
mainfrom
brennanb2025/chat-accept-then-deliver

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 74 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​3718 $\color{#cf222e}{\Huge{\mathbf{−}}}$​874 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​2844
Prod 89 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​2048 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1094 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​954

Scope: which chat this affects

  • Structured chat only. This PR changes the experimental structured native chat, which runs the agent through its SDK. It is behind the experimentalStructuredNativeChat setting, which is off by default (src/shared/default-global-settings.ts:137).
  • Default chat view: no change. The default chat view, backed by a terminal and reading the agent's transcript, does not use this code, and nothing about it changes.
  • Terminals: no change. Plain terminals and terminal-backed chats behave exactly as before.
  • Mobile structured chat: it shares two rules: a message waiting for its agent shows as working, and the two new internal rejection reasons read "Your message was not sent. Send it again." Mobile's send reply is still held until handover, because it does not advertise accepted sends yet.

Stacking

This PR was stacked on #22812 (writes are named by their target), #22811 (every journal append reaches the chats that are open) and #22808 (a chat's owner is reported from its record). All three are now merged into main, and the base is main. Main was merged into this branch three times, as merge commits e6dadd1838, 6374f35a27 and 724b961264, so the branch carries main up to 484598b18e and the diff against main is this PR's own change only. The commits:

ELI5

Before, sending a message to a structured chat whose agent was not running made the app start the agent first, and the send only answered once the start was done. If the start failed, the message was refused and went back into the composer with an error, and a Claude prompt sent during startup could come back "unconfirmed". Now the app takes the message straight away and shows it in the chat. A small per-chat delivery loop then starts the agent and hands the message over. If the agent can't start, the chat shows one red row saying why, and the message is marked "not sent" with a Retry button. Opening or coming back to that chat does not try the start again; only sending does. Stop takes back messages that are still waiting and stops only the agent; the chat stays open.

What Changed

Before

  • A send to a chat with no running agent restarted the agent inside the send call (restartOwnerForSend), before the message was recorded. The client waited for the whole start.
  • A failed restart refused the send with agent_session_owner_restart_failed. Nothing was recorded, and a second send restarted again.
  • Opening or returning to a chat whose last start had failed started the agent again from the view, and each of those failed starts added another failure row.
  • Claude held prompts in the adapter while it started (the "startup gate"). A prompt held that way could settle as "unconfirmed".
  • The restart continuation waited at most 30 seconds for its message to settle. A slow cold start was filed as "unconfirmed" even when the message was delivered.
  • The host kept one entry per conversation that also carried the agent process's fields (hasProviderChild, providerChildPhase, acquisitionGeneration, fence). Only closing the chat ended an agent as a whole, and an attach that failed after acquiring an agent forgot the conversation: it closed the journal and dropped the entry. An eviction marked every pending message "unknown", including ones no agent had ever received.

After

  • Accept. A send is accepted inside the conversation's serialized queue. The host records it with handoverRecorded: true, publishes it, and answers pending. It no longer needs the writer lease: a send is a write to the conversation, not to the agent.

  • The provider child is its own record on the conversation, child: {generation, fence, phase} | null (structured-agent-session-provider-child.ts). It is entered only by indexProviderChild, after an attach has fully succeeded, and ended only by endProviderChild, which an exit, a failed re-attach, Stop and eviction all share. The end is matched on generation and fence, so a late ending for an older agent cannot end a newer one. A failed attach writes no child, so nothing is unwound. endProviderChild also records how the agent ended (lastEndedChild: a user's Stop, a host stop, an exit, a failed attach or an eviction, and where the journal stood), in memory only.

  • Delivery. structured-agent-session-delivery-loop.ts runs while any message is queued, meaning accepted but not yet handed over. Each step is its own serialized task and re-reads the journal and the child record:

    1. Reject anything an earlier host process left queued.
    2. If a start already died while the oldest queued message waited on it, settle it as that message's failed start (below).
    3. Start the agent, through the same serialized attach a view's hold uses.
    4. For Claude, wait outside the queue until the agent has proven its start (adapter.awaitStarted, which resolves with the reason when the start did not land).
    5. Hand over the oldest queued message, as its own serialized step. A dispatch{pending} row is written before the adapter call.
  • One settler. The delivery loop is the only thing that settles a queued message because of a start, an agent or a leftover. An agent's exit only ends the child record; the loop reads lastEndedChild to decide. The only other writers of a queued message are Stop (withdraw) and closing the conversation (abandon).

  • One red row per failed start. A start the loop needed and didn't get writes one status row with tone: 'error', keyed by the start: the agent's generation, or the oldest queued message when no agent was published. Every queued message is rejected with the same text. That includes a Claude CLI that spawns and then dies while starting. A second report of the same failure revises the row instead of adding one. A start that Stop cancelled writes nothing.

  • A start someone else began. A start that dies while a message waits on it, such as the one a chat's view began when its tab opened, is that message's failed start. The delivery loop settles the message with that start's error row and starts nothing more. A message sent after the loop has settled the failure gets a fresh start. One accepted in the same instant, before the loop's next step, is settled with that failure, as it already is when the loop's own start fails.

  • Only a send retries a failed start. A view does not start a chat whose last start died during startup; its hold still registers and resolves, and only a send starts the agent again. This is one rule, failedProviderChildStart in structured-agent-session-provider-child.ts: there is no agent now, and the last one ended during startup and not by the user's Stop. It is derived on every read from how the last agent ended (lastEndedChild, in memory, never persisted), and the next agent to start replaces it. Three places read it: the view's hold (lastStartFailed in structured-agent-session-host-lifetime.ts, used by structured-agent-session-holds.ts), exit recovery (structured-agent-session-unexpected-exit.ts), and the delivery loop (startThatFailedWhileQueued in structured-agent-session-delivery-loop.ts). After an Orca restart that memory is empty, so the first view starts the agent once again. A user's Stop is not a failed start, so the next view starts the agent as before.

  • An orchestration worker whose agent cannot start fails its worker-start with dispatch_preamble_undelivered and the agent's startup error, the same way a chat's message reads "not sent". It is never reported ready.

  • Stop. With no writer lease and no fence check, Stop withdraws every queued message first (provider_cancelled_before_start). It then interrupts a running turn, or stops a Claude agent that is still starting. Stopping ends only the agent (stopStructuredAgentSessionAgentUnderSerialize): its lease is handed back and the chat is told it is idle, while the journal, the view's holds and its subscribers stay. With nothing to stop, it succeeds and does nothing. A withdrawn message is removed from the chat with no error, and its text is not put back in the composer.

  • Quit and tab close. Quit stops each delivery loop before its next step, waits for a start already in flight, and stops the agent that start produced. Quit and closing the chat's tab both reject whatever is still queued as provider_closed_before_delivery, with or without an agent.

  • Restart offer. "Resume interrupted chats?" counts only work an agent had. A chat whose only work is a message still queued at quit is not offered.

  • Settlement is derived from what the journal recorded:

    Message Start failure, Stop, tab close, quit Host process ended Agent exited
    Queued (never handed over) rejected with the cause rejected host_restarted_before_delivery by the delivery loop that the next open wakes rejected by the delivery loop with the exit reason (the exit only ends the agent)
    Handed over, no answer stays unknown, as today unknown; provider history decides it under a won lease unknown, or rejected if the agent never proved its start
    Legacy pending (no marker) as today unknown as today
  • A queued message keeps the chat "working", and counts as owed work so an idle release waits, whatever its fence.

  • An unknown message blocks /clear and /compact only if it is live on the current fence. Doubt left by an earlier agent no longer blocks them.

  • A compaction or rewind found prepared when a conversation opens is settled at that open, with no view needed. The open runs only when no agent is indexed, so such a command was started under an agent this process no longer has. A compaction settles as unknown with a status row, a Claude rewind settles as unsupported, and a Codex rewind the provider already applied and verified is completed. A Codex rewind only its provider can prove still waits for an attach (see Known limits).

  • The open cursor is scoped to its epoch (journal.wroteBeforeOpen(sequence)), because sequences restart when an epoch is replaced.

  • Fences on writes. Conversation writes carry the record's fence, and an agent's own writes carry that agent's fence. The journal refuses any row below the highest fence already written (assertJournalFence, called at src/main/native-chat/agent-session-journal/journal-row-writer.ts:24), and the record's fence only goes up, so a conversation write at the record's fence is always accepted.

  • Host capability. The host advertises agent-session.accepted-send.v1 in RUNTIME_CAPABILITIES (src/shared/protocol-version.ts).

  • Renderer. On a host that advertises accepted sends, the outbox no longer treats a fence move as a new owner: it does not resend, unblock or reset its probe (useStructuredAgentSessionOutboxOwnerChange in src/renderer/src/runtime/structured-agent-session-accepted-send-capability.ts). Against an older host, or if the capability probe fails, it behaves as before. A message the journal rejects after the host accepted it becomes a rejected outbox entry, and its Retry row states the reason once through the per-message failure record (lastFailure) that plain-words refusal copy on main introduced: the host's own reason for a start failure, "Your message was not sent." for an internal reason, and "Message was not sent." only when no reason is stored. Retry resends the same text under a new id. A rejected entry no longer blocks later messages; unconfirmed still does. A message rejected while its chat was closed reads the same way on reopen, reason included, and a rejection recorded before the send's own pending answer is kept. Write refusals use main's typed write outcome; this PR adds no second formatter.

What was deleted, added and kept

  • Deleted
    • restartOwnerForSend, structuredAgentSessionSendNeedsOwner and recordFailedRestart.
    • Claude's startup gate (claude-structured-session-startup-gate.ts: held prompts, holdClaudeStartupWrite, rejectClaudeStartupWrites, failClaudeStartupGate).
    • The attach's onAttachFailed forget, and the attach's own markPendingSubmissionsUnknown.
    • restoreReadableUnderSerialize, AGENT_SESSION_ADMISSION_BARRIER_TIMEOUT_MS, and the pre-dispatch refusal write inside performSend.
    • The agent fields on the conversation entry (hasProviderChild, providerChildPhase, acquisitionGeneration and the stored fence), replaced by the child record.
    • The read restore's own stale-state settle step, now done by the one open (below).
  • Added
    • structured-agent-session-conversation-open.ts, the one open: the recovering open plus the crash boundary, used by accept, read and attach. An attach now always adopts the conversation's open journal.
    • structured-agent-session-delivery-loop.ts, structured-agent-session-host-delivery.ts (the open and the loop together), and structured-agent-session-start-failure-row.ts, the one start-failure writer.
    • structured-agent-session-provider-child.ts, the child record's writers and failedProviderChildStart, and stopStructuredAgentSessionAgentUnderSerialize, which stops the agent and keeps the conversation.
    • AgentJournalSubmission.handoverRecorded (on the submission row) and handedOverAt (derived from the dispatch{pending} row).
    • journal.wroteBeforeOpen(sequence), backed by the cursor (epoch and sequence) a handle found when it opened.
    • adapter.awaitStarted, which is Claude's startup-settled promise, resolves with the reason a start did not land, and also resolves when the agent is closed.
    • The agent-session.accepted-send.v1 capability.
  • Kept
    • The admitted dispatch outcome. Claude and Codex both return it for every successful write whose identity arrives later with the echo, not only for held prompts. A handed-over message stays pending until that echo.
    • Claude's startup-settled state, now in claude-structured-session-startup-state.ts, which setOption still needs.

Main's changes carried onto the agent record

The structured-agent-session-* files below are in src/main/native-chat/agent-session-wire/. Each rule has one copy, in this PR's shapes.

  • A running subagent or background task keeps an idle chat alive (fix(native-chat): keep an idle chat alive while its subagents or background commands run #22794): one more kind of owed work in the one hasOwedWork check in structured-agent-session-host-lifetime.ts, which reads agentChildWorkLiveness over the adapter's background tasks; there is no second owed-work check.
  • Typed question answers (fix(native-chat): send typed question answers as structured answers, not an option id #22793): prepareClaudePromptReply in src/main/claude/claude-structured-prompt-ownership.ts, and respondToStructuredAgentSessionPrompt in structured-agent-session-host-mutations.ts, which takes AgentSessionPromptRequest through the same mutate path; main's code is unchanged.
  • Every latch has a way to die (fix(native-chat): every lease latch has a way to die #22820): fix(native-chat): every lease latch has a way to die #22820 deleted the settlement-retry file and its latch, and the merge keeps that deletion (this PR's earlier edits to that file went with it). No host code reads settlementRetryRequired or settlementRetryId; they are only stripped at load in src/main/runtime/agent-session-store-transaction-queue.ts, and stale state is derived from the record's death evidence by the one settleStaleStructuredAgentSessionState in structured-agent-session-dead-generation-settlement.ts.
  • Settle before a reader sees it (fix(native-chat): every lease latch has a way to die #22820): main's settle now runs in the one journal open, openStructuredAgentSessionConversation in structured-agent-session-conversation-open.ts, for every opener except an acquisition (a send, Stop, a reader, a reveal or a restart restore). An acquisition says so with { acquisition: true } and settles in onAttached (structured-agent-session-attach-orchestration.ts) from the death evidence it read before its reserve cleared it (priorDeathEvidence). A turn with no proof its owner exited becomes unverifiable, never "stopped"; only an observed exit marks it interrupted. This includes a send that is the first thing to open a chat after a crash and whose start then fails: main settled that case in the send's restore, which this PR deletes, and the open now does it.

Why

One place owns delivery. The send, the view's hold and exit recovery each used to be able to start an agent, each with its own failure handling and its own way of writing into the chat. Now a send only records the message, and the loop is the one thing that starts an agent for a send, hands a message over, and settles a message whose start failed. The loop runs exactly while something is queued, so there is no loop state to go stale.

The agent is its own record. This is the first change in which a chat must outlive its agent, so ending an agent can no longer mean ending the chat. With the agent as one record, entered in one place and ended through one function, every way an agent ends is the same operation, and nothing hand-unwinds a subset of fields.

Settlement is derived, not stored. Whether a message reached an agent follows from two rows, handoverRecorded and dispatch{pending}. That is why the delivery loop, Stop or a close can reject a queued message outright, as provably unwritten, while a handed-over message stays in doubt.

A failed start is derived too. Whether the last start failed is read from how the last agent ended, which the host already holds, so there is no flag to clear: the next agent to start supersedes it, and a relaunch starts with none. Keeping it across a restart would stop a fixed CLI from coming up after a relaunch, and the next PR removes view starts altogether.

Crash leftovers need no latch. A handle closes only with nothing queued, so a queued row at or below the handle's open sequence must come from an earlier process. The open wakes the delivery loop, whose first step rejects it.

Handover is its own serialized step because the queue is first in, first out. A Stop sent while a start holds the queue runs before the handover that would have written the message, so the withdrawal always wins.

Alternatives considered

  • Keep restart-before-admission and only fix the failure row. That keeps the client waiting for the whole start, and it keeps two start paths.
  • Resolve Claude's acquire at "started". That would hold the queue through an unbounded start, which fix(claude): open structured chat without a startup deadline, and make Retry start fresh #22364 removed, and it is out of scope until the start leaves the lock.
  • Keep the agent's fields on the conversation entry and unwind them by hand at each ending. Each ending needed its own partial unwind, and a missed one left a phantom agent after a failed attach.
  • Let an agent's exit settle queued messages too. That gave two writers for one fact: two different failure rows for one start, and a respawn loop when an agent died before handover.
  • Stop a view's hold from starting an agent while a delivery loop runs (a hold gate). It changes no interleaving that actually happens: in the failing case the view's start began before the send was accepted, and when the loop starts first, the hold already finds its agent and starts nothing.
  • Key the failure row by message instead of by start. That still costs a second start, and still gives two rows, because the exit that writes the first row does not know the message.
  • Compare the message's fence with the dead agent's fence. After an exit the lease is released at the dead agent's fence, so a Retry is accepted at that fence too, and fences cannot tell before from after.
  • Let the open guess from the lease whether an acquisition is opening it, and skip the settle then. A lease a crashed process left behind also reads as claimed when Orca cannot prove the old owner gone, so a send-first open skipped the settle and the dead turn stayed "running". The acquisition now says it is the opener.

Differences from the common pattern

  • Matches: acceptance is serialized per conversation and persisted before any start. The delivery owner is the only thing that settles a message its start or agent failed; a process exit writes process state only. The agent is a sub-record of the conversation, and stopping it keeps the conversation. A message accepted by a process that crashed is never re-sent; the user sends again.
  • Deviation: the agent record lives in host memory only, not as a persisted part of the conversation.
  • Deviation: messages queue inside the submission row. Queued is derived as handoverRecorded, still pending, no handedOverAt, rather than kept as a separate queue entity.
  • Deviation: one failure row per start, not per message, and no stored error on the conversation.
  • Deviation: Stop withdraws queued messages, silently. The precedent is Orca's own startup gate, deleted here, which did the same for held prompts. A Stop that fails returns an error and writes no row in the chat.
  • Deviation: old clients get a held reply instead of a new wire state. Older clients cannot show a rejection that arrives after pending.
  • Deviation: a send during a cold Codex start still waits behind that start in the queue. That is today's behaviour, and it changes once the start leaves the lock.
  • Staged: two callers of the start-failure row. Until later PRs remove the other starters (a view's hold and exit recovery), a start that dies with nothing queued writes its row from the exit path, through the same writer, with the same key and tone. Once they are gone, the delivery loop is the only caller.
  • Staged: a start's end is ordered against a message's acceptance by journal position. Until every start goes through one start in flight, the delivery loop orders them by journal position (kept in memory on the conversation), so a start another caller began still costs one attempt per message. Removing view starts keeps this rule, because attach, create and commands at rest still start agents. It goes away when every caller joins one start.
  • Deviation: ending an agent does not close its event stream. For a Stop or an eviction, the agent's end is recorded before the step that drains the rows it emits on its way out, so the eviction steps close the stream after that drain. A failed re-attach closes it itself.

Differences from the plan

  • admitted is kept. The plan said to delete the admitted dispatch outcome with the held prompts. But Claude returns it for every successful write, not only held ones: src/main/claude/claude-structured-dispatch.ts, the end of dispatchClaudeTurn ("The write is the admission signal … settleWaiter finishes the job"). Codex also answers turn/start before the echo. Only the held-prompt path is deleted.
  • The launch prompt is unchanged. src/main/runtime/rpc/methods/agent-launch-structured-prompt.ts (the doc comment on commitStructuredAgentSessionLaunchPrompt) deliberately returns the committed row without waiting for dispatch, so there is nothing to put a budget on.
  • The preamble and the mailbox pointer wait up to 60 s for delivery. A preamble still held when that runs out leaves the worker start-unknown with its conversation kept; the host delivers it when the agent starts, and the worker's report settles the dispatch. A pointer parks for the next journal edge and replays the same operation, so it is never re-sent. The budget is fixed, not the start's timeoutMs.
  • A leftover below the open cursor still counts as working until the delivery loop's first step rejects it. The plan excluded such rows from the working projection. One survives only if the journal cannot be written at all.
  • "Stop the agent" is split from "close the chat" here, not in PR 2. This is the first change in which a chat outlives its agent; without the split, Stop on a starting Claude agent closed the chat's journal and dropped its holds.
  • A view stops retrying a failed start here, ahead of PR 2. PR 2 removes view starts entirely. Without this rule, every look at a chat whose start failed started the agent again and added a row.
  • Stop sends status only. The plan also sent subscribers a fence snapshot on Stop. Each reader now keeps its own fence, and only the status moves.
  • The loop fails a still-starting agent only when the adapter says its start did not land. The host's "starting" phase trails the adapter's started event by one serialized step, so failing on the phase alone rejected healthy sends.
  • endProviderChild does not close the event stream, which the plan had it do (see the last deviation above).

Known limits, handed to later PRs

  • Two rows on a fresh chat whose create failed (a later PR): the chat shows the create's own failure row above the message and the send's row below it, one row per start. A later PR stops create from starting the agent, which leaves only the send's row.
  • Model picker and commands wait for a send after a failed start (as before this PR): they need a running agent, and a view no longer starts one after a failed start, so they wait until the next send. Before this PR the view's restart failed the same way, so they waited then too.
  • Codex rewind (PR 2 of this series): a Codex rewind left prepared, or provider-succeeded without hydrationVerified, is not settled when the conversation opens, because proving it needs a live Codex process (recoverCodexRewind in src/main/codex/codex-structured-rewind.ts), and a send is not accepted past it, because that recovery rebuilds the journal through replaceJournalEpoch, which deletes every row, including a queued message. On this PR a view still attaches and recovers it; PR 2, which stops views starting agents, settles it another way.
  • One-Retry window (a follow-up): If a Claude chat's agent dies while starting, the first Retry right after can fail with the same message, and the second works. If Orca can't confirm the dead process's cleanup, sends keep failing until restart. Both happen on main too, and a follow-up ends the agent the moment its exit is seen.
  • Retry one message at a time (a follow-up): When more than one message was not sent, the chat offers Retry for one at a time, the oldest first, above the composer. The others become retryable as the ones before them are delivered, and a message that was not sent can't be discarded yet. A follow-up adds Retry, the reason and discard on each message.
  • No desktop Stop while an agent starts (a follow-up): the desktop shows no Stop button until a turn runs, so withdrawing queued messages is reachable only once a turn runs. A follow-up shows Stop from the moment a message is sent.
  • No hung-start backstop (PR 2): apart from Stop, nothing ends a start that never finishes until PR 2's sweep. Closing the chat still gives up on it, as the "still starting" banner says.
  • No live error frame for journal write failures (a follow-up): when the failure row or a rejection cannot be written, the loop logs it and the messages stay queued: Stop withdraws them, the next open rejects them, and the next send's delivery loop tries them again. A live error frame is a wire change that needs a capability.
  • Pre-existing, unchanged here: the start-failure text includes internal wording ("claude stream-json exited (code 1)"), and the red row for an automatic restart with nothing queued folds into the previous turn's collapsed group.

Mixed versions

  • Clients without agent-session.accepted-send.v1 (every released client, and mobile until it can show a rejected message in place): their send reply is held until the message is handed over or rejected, with a 120 s budget (STRUCTURED_AGENT_SESSION_START_WAIT_MS). A longer start replays through the same wait. Clients without agent-session.pending-send-result.v1 wait, as before, for the provider's answer.
  • Local desktop and paired desktop send the capability, so they get the answer at acceptance. Local desktop IPC arrives as clientKind: 'runtime', which is why DESKTOP_RENDERER_RUNTIME_CLIENT_CAPABILITIES carries it.
  • The host side: the host advertises agent-session.accepted-send.v1 in its own runtime capabilities. A desktop or paired-web client that sees it stops treating a fence change as a reason to resend or unblock.
  • New client, old host: the old host still restarts the agent before admission, and does not advertise the capability, so the client keeps today's resend-on-fence-move behaviour; the same holds when the probe fails. The client handles the old host's refusals as before, and a settled-rejected refusal now becomes a rejected entry instead of blocking the queue.
  • handoverRecorded and handedOverAt are optional fields. Old readers drop them: the zod schema strips unknown keys, and the journal row parser ignores them.
  • Rolling the app back: the outbox now stores a rejected state. An older build drops entries in a state it does not know, so after a downgrade a message that was not sent disappears from the chat instead of offering Retry. For a message the host never recorded, its text lived only in that entry and is lost. Keeping an older state instead would make the older build send a rejected message again, which is worse.
  • When the held reply can be deleted: once MIN_COMPATIBLE_RUNTIME_CLIENT_VERSION passes the first release where every client advertises accepted sends.
  • Cross-version tests pin a released client's held reply and a current client's immediate one against the same host.

Linked Issue

STA-7716, STA-8245

Visual Proof

Round 3, after the merges with main (a second Mac, 158ca0a5ee)

Live Electron QA on a second Mac with a real Claude CLI and an isolated app profile. For the failed-start lane, a small wrapper in front of the Claude CLI made it exit with code 1 during startup ("not signed in"), and was then removed before Retry. The commits after 158ca0a5ee are one settle-on-open fix (ad04c3be46) and tests.

Scenario Result
An idle chat with a background command or a background subagent running (#22794) Pass for both, at 724b961264: the agent is kept while the work runs and released only after it ended.
Answering a structured question (#22793) Pass for an option and for a typed answer, at 724b961264: the answer reaches Claude and the turn continues.
A fresh chat whose start fails Pass at 158ca0a5ee: opening it gives 1 start and 1 row, and the view does not restart it; a send gives 1 start, 1 row, and "Message was not sent." plus Retry; returning to the chat later adds nothing; Retry delivers. At 724b961264 the same chat had 3 starts and 2 rows.
Send while "Claude is still starting" is showing Pass, no hang: the reply arrives, 5 of 5 runs at 724b961264 and 2 of 2 at 158ca0a5ee.
Cold send Pass: the reply arrives.

A fresh chat whose start fails: opening it shows the create's one red row, and nothing more.

Fresh chat after a failed start, before any send

After one send: one red row below the message, and "Message was not sent." with Retry.

After one send: one row below the message and Retry

Returning to the chat later adds no row.

Returned to the chat later, with no new row

After the wrapper is removed, Retry delivers.

After Retry, the message is delivered

Send while "Claude is still starting" is showing: the message waits, then the reply arrives.

Sent while Claude is still starting

The reply after the start finished

An idle chat with a background subagent (#22794): the chat was released only after the subagent ended.

Idle chat released only after its background subagent ended

A structured question answered (#22793):

Question answered and the turn continued

Cold send:

Cold send and its reply

Rounds 1 and 2

Live Electron QA on macOS with a real Claude CLI, an isolated app profile and a separate home directory. A small wrapper in front of the Claude CLI could make the chat's agent start slowly, or exit with code 1 right after it spawned. Round 1 ran at c753e8ab9a, round 2 at 0184a3d814, and round 2b at ccaf412f96. Codex was not run live. The desktop has no Stop button while an agent starts (see Known limits), so Stop during a start was sent through the chat's own Stop request.

Scenario Result
Cold send Pass, all three rounds: the message shows at once, one agent starts, the reply arrives, nothing reads "unconfirmed".
Two sends during a start Pass, rounds 1 and 2: one agent, both delivered in order, nothing lost or duplicated.
Open the tab and send while the CLI spawns and exits Pass in round 2b: one start, one red row, "Message was not sent." plus Retry, and Retry delivers. Round 2 showed two starts and two red rows; ccaf412f96 fixed it.
Agent killed while the chat is open, then a send Pass, rounds 2 and 2b: one restart attempt per trigger, one red row, no resend loop over 65 s.
Stop during a start Pass in round 2: the message is withdrawn with no error, the agent stops, the chat stays open, and a later send starts a fresh agent.
Idle agent killed, then a send Pass in round 2: a new agent starts and the send is delivered. Round 1 stuck on "unconfirmed", the same as its base; the fix is #22802, which this branch carries through main.
Quit with a message still queued Pass in round 2b: "not sent" plus Retry after relaunch, and no restart offer. Round 2 offered to resume it; 12577d8486 fixed it.
Restart continuation with a 40 s cold start Pass, rounds 1 and 2: accepted past the old 30 s budget, never filed as unconfirmed.

A start that fails, before this PR (12dde3710b): the failure row is a normal grey row.

Before: failed start shows a grey row

After (ccaf412f96): open the tab and send while the CLI fails. The message is shown while the agent starts:

Message sent while the agent starts

One red row, and "Message was not sent." with Retry:

One red start-failure row and Retry

After Retry, the message is delivered and the red row stays visible:

Retry delivers and the row stays

Agent killed with the chat open, then a send (ccaf412f96): one red row under the message, and Retry.

Send after a failed restart

Quit with a message queued, then relaunch (ccaf412f96): the message reads "not sent" with Retry, and there is no restart offer.

Relaunch after quitting with a queued message

Stop during a start (0184a3d814): no error strip, no "still starting" banner, the chat stays open; then a later send is delivered.

After Stop during a start

A later send after Stop

Restart continuation with a 40 s cold start (0184a3d814): the offer, then the continued turn.

Resume offer after quitting mid-turn

Continued turn after a slow start

Testing

Test names carry short tags: W plus a number is the item in the plan's test list (W29 to W35 cover the agent record and the single settler), R1 marks tests for the agent record, and R2 marks tests for the delivery loop being the only settler. Every host test runs against a real host, record store and journal, with a live subscriber opened before the action, and asserts the journal and the frames that subscriber saw.

  • Accept, then deliver (structured-agent-session-accept-then-deliver.test.ts)

    • A capable send is answered before the start, and an open chat sees the handover and then the reply (W2). A second send during a start is accepted before the first is handed over (W6).
    • One error row, with every queued message rejected with it, for eligibility, spawn and auth refusals (W3). An attach failure after acquiring names the start failure, not "host restarted" (W4′a).
    • An attach that fails after indexing its agent leaves no agent behind, and no status frame ever showed one (W32).
    • Crash leftovers are rejected; legacy and handed-over rows become unknown, and provider history is not read at open (W4′b, W4′c).
    • Stop withdraws a crash leftover, a message whose start holds the queue, and a Claude agent still starting (W17a to W17c).
    • An eviction between accept and handover rejects the message and never leaves it unknown, and an idle agent is not evicted while a message is queued (W24).
    • A compaction or Codex rewind an earlier agent left prepared is settled at open (R16).
  • The agent record and the single settler (structured-agent-session-provider-child-record.test.ts)

    • Stop on a starting agent ends only the agent: the same journal, holders and subscribers stay, and the next send is delivered on the same handle (W29).
    • An agent that dies while proving its start gives one error row keyed by the start, with every queued message rejected, whichever of the loop and the exit sees it first (W30).
    • The earlier agent's running turn is settled from its death evidence, and the queued message goes to the new agent at the new fence (W31).
    • With another agent indexed while the loop waits, nothing is handed over until that agent has proven its start (W33).
    • An agent that ends before handover costs one start, then the message is rejected and the loop stops, with no respawn (W34).
    • Quit with a message queued settles it the way a chat close does, with or without an agent, and waits for a start in flight and stops the agent it produced (W35).
    • A user's Stop lets the loop go on and deliver what was sent since; a host stop fails the start with its reason.
    • A view's start that dies while a message waits is that message's failed start: one row, no second start. A message sent after the failure gets a fresh start, a proven agent that crashes is restarted, and an agent an attach started since the failure is waited on instead of the failure.
    • A send whose start failed, resent with the same operation id, replays the rejection and starts no second agent.
  • A view after a failed start (structured-agent-session-view-start-after-failed-start.test.ts): with the real Claude adapter and a fake CLI that exits during startup, a fresh chat starts once for the open and once for a send, with one row each, whether the view binds after the create died or while it is still starting; holding the view again starts nothing and adds nothing.

  • Settle on open (structured-agent-session-send-open-stale-turn.test.ts): after a crash and relaunch, a turn the lost agent left running is settled as unverifiable when a send opens the chat and its start fails, and when a reader opens it, both when the old owner is proven gone and when nothing proves it.

  • Background work (structured-agent-session-owed-work-release.test.ts): a subagent, a background command and a monitor each keep an idle session until they settle, on this PR's accept-then-deliver timing.

  • Restart offer (structured-agent-session-restart-resume.test.ts): nothing is offered for a chat whose only work is a queued message, and a handed-over message is offered under its own identity even with a newer queued one.

  • Renderer: on a host that accepts first, fence moves during a send cause no resend and a blocked message stays blocked until Retry, while an older host still resends (use-structured-agent-session-outbox-fence.test.tsx). The capability hook reads a local yes and no, a remote yes, and a failed probe as an older host. A message rejected from the journal keeps its reason, and Retry uses a fresh id.

  • Also: the host advertises the capability; a queued row is left alone by provider-history reconciliation; the restart continuation reaches accepted after a 40 s start; the preamble and mailbox pointer wait for delivery; reply timing for the desktop and paired lists; the cross-version held reply; a failed start's rows are counted by row, so two rows with the same text fail (structured-agent-session-append-delivery.test.ts).

  • Ablation: each new test was checked by removing or reverting the lines it pins on a copy of the file, and went red on its own assertion (not a timeout) before the file was restored. The one exception is the same-operation-id resend test: its property comes from accepting before any start, so it has no single line to remove. Existing tests were moved to the new timing, and tests that only pinned deleted internals (held prompts, journal replacement on re-attach, pre-dispatch refusal persistence, the old entry fields) were rewritten or removed.

  • Validation at 05b8b9307b:

    • pnpm tc:node and pnpm tc:web pass.
    • pnpm exec oxlint is clean, and pnpm run check:code-quality:changed reports 0 findings.
    • The cross-version agent-session wire test (tests/e2e/cross-version-wire/cross-version-agent-session-wire.unit.test.ts) passes 16 of 16.
    • The suite with the ORCA_* environment unset (native-chat, claude, codex, runtime, shared and cross-version-wire) passes apart from claude-structured-real-cli, which needs a real Claude CLI, and load timeouts in files this PR does not touch, which pass when run alone.
  • Not run: SSH, Windows, Linux, and live Codex. Full pnpm lint and pnpm build are left to CI.

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

Review

Author: @BrennanKB5

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

  • SSH: delivery runs on the execution host, which owns the journal and the agent. Nothing is inferred from loss of contact.
  • Folder workspaces: unaffected.
  • Mobile: see Scope. Mobile does not advertise the capability yet, so its replies are held as today. Mobile's view goes through the same hold, so after a failed start it too waits for a send.
  • Out of scope, left for later PRs: views still start agents through holds, except after a failed start, and the start still runs inside the queue. The rest is listed under Known limits.
  • Left in place: the adapter's beforeDispatch hook no longer has a host caller. The restart continuation's "still resumable" check now runs at acceptance.

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)

brennanb2025 and others added 28 commits September 24, 2026 23:36
No client ever called agentSession.requestHandoff or mounted the handoff
chrome. Delete the handoff coordinator, the terminal-owner runtime, the
proof write path and the unmounted UI. Keep agentSession.handoffStatus,
which released desktop clients read for worktree activation, and let
records an older build left mid handoff reconcile through the ordinary
restart and recovery paths.
Eviction now drains delivered events before quit's resume-offer snapshot. An
unbounded wait there sits ahead of the provider stop, so a sink whose journal
write stalls kept the child running until the step deadline aborted the
eviction. The offer is advisory: bound the drain and stop the child regardless.

Co-Authored-By: Claude <noreply@anthropic.com>
`claudeAuthEnvCarriedForward`, `isPathWithinDirectory` and
`queryWindowsProcessRowsFresh` lost their last caller with the handoff. The
fresh-scan tests now go through `queryWindowsProcessDescendants({ fresh: true })`,
the teardown path that still depends on that contract.

Co-Authored-By: Claude <noreply@anthropic.com>
Six comments still named the handoff coordinator, a handoff suspend, or a
terminal-owned session as live participants in the flows they describe.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…ement

The removed restart handoff test pinned this branch; nothing else did.

Co-Authored-By: Claude <noreply@anthropic.com>
The handoff removal dropped the per-session queue from `handoffStatus`, so a
read landing mid-start reported the reservation (no owner) instead of the
settled chat owner, and shipped desktop clients blocked worktree activation on
it. The read is queued again, as it was before the removal.

Co-Authored-By: Claude <noreply@anthropic.com>
The gate only refused a write when a PTY had been bound to a chat session, and the
only code that ever bound one was the terminal handoff this branch removes. With it
gone, every admit/readmit returned "admitted" unconditionally, so the checks on the
renderer write path, the runtime controller backstop, terminal.send, agent prompts,
preview input and orchestration pointers, the refusal fields on terminal.send and
worker-start receipts, the plugin and CLI refusal copy, and the adopted-pane
orchestration routing could no longer run. Ordinary writes take the same path in
the same order as before.

Co-Authored-By: Claude <noreply@anthropic.com>
…alled

appendLegacyTranscriptMessages fed the terminal transcript catch-up and
proveClaudeTranscriptBranch backed the terminal owner's exit proof. Both lost
their last caller with the handoff. Their tests now go through the live entry
points instead: the roster bounds through the legacy import, the pinned-read and
growth tests through the ancestry replay the history window uses, and the marker
rules through the string proof in their own file rather than the session-file
resolver's.

Co-Authored-By: Claude <noreply@anthropic.com>
A send refused because the chat's owner is not settled showed "The session is
mid-handoff (<stage>)." in the composer. With the handoff gone, the stages that
reach it are a chat that is still starting, or one whose previous agent process
has not yet been confirmed stopped. The message now says which of the two it is.
The refusal code is unchanged.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
With the terminal handoff gone, the module named codex-tui-rollout-proof holds
only the pinned rollout lookup that structured Codex launches use to resume a
thread, so the name described code that no longer exists. Rename the module and
its options type. Also drop a mobile allowlist assertion that pinned the
removed agentSession.requestHandoff method, which no longer exists to allow.
The handoffStatus reply type still listed the terminal handoff's fields and
states (terminal placement, host label, proof retry, queued and waiting phases,
the to-terminal direction). No host writes them any more and the only client
reader parses the reply as unknown, so they described nothing. The reply on the
wire is unchanged.
…t decode

Nothing in this build writes a terminal owner (`runtimeKind: 'tui'`) or the
handoff's `preparing` / `old-owner-stopped` stages, but the in-memory types
still admitted them, so readers across the host kept branches for values no
path produces and the compiler could not point at them.

The store now validates the on-disk shape, which still accepts those values so
an older record is not quarantined, and maps them once while parsing:

- `preparing` and `old-owner-stopped` become `recovering`
- a `tui` lease becomes `native`; when it records a process it also becomes
  `conflicted`, the claim every build probes but never stops. A plain native
  owner would be stopped by restart recovery, here and in older builds.

Revisions are taken over the normalized state on both sides of every compare,
and the mapped record reaches disk with the store's first transaction, the
same way the tab-id backfill does.

The in-memory types narrow to what this build writes, and the branches that
existed only for the removed values go. Structured-worker identity keeps its
verdict for a former terminal owner by refusing a conflicted claim rather
than a non-native kind.
…ation

A reservation only ever names a native owner now, so the request no longer
carries a kind and the reserved lease records `native` directly. The attach
params keep `runtimeKind`: agentSession.ensure and create accept it, and the
operation fingerprint stored in the ledger covers it.
…at changes nothing else

Hiding a tab also committed the visibility index, so the no-op transaction
wrote the file even when its open-time revision was wrong. Committing the index
first leaves the pending rewrite as the only reason to write.
…ration

A write carried the fence of the last frame the pane read, and the host refused it
unless that fence was still current. An idle release and the restart after it each
move the fence, and the release publishes nothing, so a send after a release was
refused "Expected runtime fence 1; the session is at 3", and a Stop queued behind a
cold start was refused as stale.

Every write already names what it acts on: a send its conversation, a cancel its
turn, a prompt answer its item revision, a rewind its epoch; an option is
last-writer-wins. So admission stops comparing the client's fence, and the rebase
that papered over one restart (admitAtResumedFence, resumedFromFence) goes with it.
The writer-lease check stays, and so does the attach's compare-and-swap.

Frames now stamp the fence read when each frame is sent instead of a copy each
subscriber kept, which went stale on the same release.
A journal write and its delivery to open readers were two calls, and some
writers made only the first. A failed start whose lease could not be handed
back, a provider revision with no frame behind it, and eviction's settlement
were all journaled without reaching an open chat.

A journal handle now reports every durable change, and the host's session map
binds that report to the session's readers when the handle is set. Writers no
longer publish what they append; the per-writer publish calls are deleted.
…ackfill cannot supply its rewrite

The seeded record had no surface tab id, so the next open backfilled one and
that rewrite alone made the no-op transaction write. The test passed with the
legacy-lease rewrite signal removed.
The handoff removal deleted it alongside the terminal-owner tests, but it
covers the surfaced-PTY block that still guards resume, including an agent
whose ownership is unknown.
Each commit now delivers itself, so the publish a provider frame still sends
afterwards found every reader caught up but still read rows and rebuilt the
timeline for each one. A caught-up reader now skips the read.
# Conflicts:
#	src/main/native-chat/agent-session-wire/structured-agent-session-subscribers.ts
@brennanb2025
brennanb2025 force-pushed the brennanb2025/chat-accept-then-deliver branch from b3ef438 to 6bfe2e7 Compare September 25, 2026 10:29
A send to a chat with no running agent restarted the agent inside the send
call, before the message was recorded, so the client waited for the whole
start and a failed restart refused the message. Claude held prompts sent
during startup, and those could settle as "unconfirmed".

A send is now accepted inside the session's serialized queue: one ledger row
and one submission row marked handoverRecorded, published, answered pending.
A per-session delivery loop exists while a message is queued. It starts the
agent through the same serialized attach a hold uses, waits outside the queue
for a Claude child to prove its start, and hands the oldest queued message
over as its own serialized step, writing dispatch{pending} before the adapter
call. A start it needed and did not get writes one error-tone row and rejects
every queued message with the same words; a start Stop cancelled writes none.

Settlement follows from the rows. A queued message is provably unwritten, so a
close, an eviction or an exit rejects it. A handed-over message stays in doubt.
A queued row at or below the sequence a handle found when it opened was left
by an earlier process and is rejected at open, with no latch. Stop withdraws
queued messages with no writer lease and no fence. An attach failure keeps the
conversation open, and the attach adopts its journal. Owed work counts the
loop and queued rows.

A compaction or rewind found prepared when a conversation opens was started
under a child this process no longer has, so the open settles it rather than
leaving it to refuse every send until a view attaches. The open cursor is
scoped to its epoch, because sequences restart when an epoch is replaced.

Deleted: restart-before-admission, recordFailedRestart, the fence rebase,
Claude's startup gate, the attach's forget on failure and its own crash
boundary. Clients without agent-session.accepted-send.v1 get their reply held
until the handover; the desktop and paired desktop lists advertise it.

@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 in this delta.

Reviewed changes

The delta since the prior pullfrog review (e6dadd1838) is two merges of main (6374f35a27, 724b961264) with no authored commits. The first resolved 23 files both sides had changed; the second was clean. The reconciliation adopts main's newer native-chat settlement model and drops this branch's own.

  • Settlement is now derived from the lease, not a durable latch. structured-agent-session-settlement-retry.ts (and its test) is deleted, and settlementRetryRequired/settlementRetryId/settlementRetry are gone from the lease, the adapter's ended event, and Claude session state. New settleStaleStructuredAgentSessionState({journal, sessionId, fence, acquisitionGeneration, deathEvidence}) settles stale items and running turns from turnVerdictFromDeathEvidence; only exit-observed earns interrupted plus an end time and the exit copy.
  • Attach no longer refuses on a pending settlement. runAttach drops the pre-reserve retry and its agent_session_ownership_unknown refusal; it reads priorDeathEvidence from the record before the reserve clears it and settles inside onAttached, before bindAndDrain, only when an owner was acquired.
  • Read/restore settles before publishing. restoreOneStructuredAgentSessionReadUnderSerialize now awaits settleStaleState(sessionId, opened) before onReadable; the retrySettlement→settleStaleState rename runs through restart-restore, reveal, and readable-restorer, and restoreOneUnderSerialize/restoreReadableUnderSerialize are removed with no dangling callers.
  • An unexpected exit files its reason on the death evidence rather than setting a retry latch; whatever the exit settlement could not write is re-derived at the next acquire or read.
  • Smaller main adoptions reconciled here: resume-eligibility uses agentSessionLeaseIsReleased; agent-session-mutation-envelope adds the terminalOwnerRefusalMessage branch for a conflicted lease; structured-agent-session-acquisition.ts:43 wires main's new onSpawned crash-mid-start hook; host.ts drops the old restore arity.

Verification: pnpm tc:node is clean, and the 10 affected suites pass (122 tests): provider-child-record 15, accept-then-deliver 19, wedged-profile-migration 18, lease-adjudication 25, unexpected-exit 11, stale-turn-verdict 9, reveal 13, legacy-handoff-record 4, restart-restore 3, crash-mid-start 5.

The Stop/handover serialize-key race flagged in the initial review is untouched by this delta and remains open.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

A Claude chat whose CLI exits during startup left one red row per start, and
every time a view bound to it (the chat opening right after its create died,
or the user switching back to it) the hold started the CLI again, so the same
launch-failure row repeated. Only a send retries a failed start now, the same
rule provider-exit recovery already applied; the rule lives in one predicate
the hold, exit recovery and the delivery loop share.
…not a second view

A view no longer starts a child whose last start failed, so the R2 case that
waits on a child started since the failure now gets that child from a client
attach, the one non-send starter left.
…tion opens

A send that opens a chat this process had not read yet (after a crash, from a
phone or the CLI) went through the delivery open, which never settled what the
dead generation left running; only the read restore and a successful acquire
did. When the send's start then failed, the turn stayed running for every
reader. The settlement now runs in the one journal open, at the crash boundary,
for every opener except an acquisition, which settles from the evidence it read
before its reserve; the read restore's separate step is gone.
…arlier child left

The R1 case lost its only settlement assertion when the latch it checked was
deleted. It now seeds the running turn the earlier child left and asserts it
ends at the exit's receipt, with the exit's row, before the message is handed
to the new child.
Comparing the set of texts passed when two different rows carried the same
words, which is the duplicate the test exists to catch.
…, however the view binds

Opening a fresh chat whose starts fail makes one start and one row, with two
views bound before or after the create's child died; one send makes one more
of each.

@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

The delta since the prior pullfrog review (724b961264) is six commits, two of which change production behavior: a chat whose last start failed is no longer restarted by a view bind, and a gone generation's running turn is now settled wherever a conversation opens.

  • A view never restarts a chat whose last start failed. New failedProviderChildStart (!session.child, last ended during startup, cause not a user Stop) is threaded through the holds as lastStartFailed; a hold on such a session returns without calling ensureProviderChild, so only a send or an explicit attach retries. The same predicate replaces the inline check in the delivery loop's startThatFailedWhileQueued and the exitedDuringStartup check in unexpected-exit recovery — both behavior-preserving, and every other lastEndedChild producer (user-stop, evict, failed re-attach, the producer-less host-stop) leaves it false or forgets the session first.
  • A gone generation's turn is settled wherever a conversation opens. settleStaleStructuredAgentSessionState moves into the one journal open, guarded to skip only a reserved/live lease (an acquisition reserves first and settles from the evidence it read before). The separate read-restore/reveal settlement step and its deps are removed, so a send that opens a chat a dead process left running now settles it before any reader sees it.
  • Tests. The R1 attach test seeds a real running turn and asserts it ends interrupted at the exit's receipt with the exit row; the R2 test starts its child with an attach instead of a second view; append-delivery counts status rows by row id rather than by text; the two new waits get a 10 s budget.

Verification: pnpm tc:node clean; 94 tests green across the twelve affected suites (view-start-after-failed-start 1, send-open-stale-turn 2, provider-child-record 15, restart-restore 3, reveal 13, readable-restorer 1, unexpected-exit 11, holds 17, hold-resume-race 12, append-delivery 5, dead-generation-settlement 10, legacy-handoff-record 4); oxlint/oxfmt clean on the changed production files.

The Stop/handover serialize-key race flagged in the initial review is untouched by this delta and remains open, so approval is withheld.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

…n acquisition's

The journal open skipped the settlement whenever the lease read reserved or
live, to leave an acquisition's own open to the acquisition. But a lease a
crashed process left in recovery also reads live, until the next acquire
resolves it. A send that opened such a chat, from a phone or the CLI after a
crash on a host that could not prove the old owner gone, skipped the
settlement; when its start then failed, the dead turn stayed running for every
reader. The acquisition now says it is the opener, and every other open
settles, whatever the lease still claims.
The "view binds while the create is still starting" case gave the create a
300 ms head start and asserted the views bound before it died. On a loaded
runner the holds took longer, the create's exit landed first, and the case
failed its own precondition. The create's initialize now waits on a gate the
test releases once the views are bound.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 69ef1da1-9e89-4e25-8de4-16551f81a0ba

📥 Commits

Reviewing files that changed from the base of the PR and between 8f66a25 and 05b8b93.

📒 Files selected for processing (7)
  • src/main/native-chat/agent-session-wire/structured-agent-session-attach-context.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-attach-test-conversation.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-conversation-open.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host-delivery.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-send-open-stale-turn.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-view-start-after-failed-start.test.ts

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

// No child in this process writes to a journal nobody had open, so whatever it shows running
// belongs to a generation that is gone, whatever the lease still claims. Settled before any
// reader or child sees it.
if (!options.acquisition) {

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.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🔎 Supported by static analysis

🏁 Script executed:

#!/bin/bash
set -euo pipefail
ast-grep outline src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts --match 'performAttach' --view expanded
rg -n -C 6 'onAcquisitionReleased|onAttached|openConversation|settleStaleStructuredAgentSessionState' \
  src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts \
  src/main/native-chat/agent-session-wire/structured-agent-session-attach-failure.ts \
  src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts

Repository: stablyai/orca

Length of output: 14487


🏁 Script executed:

#!/bin/bash
set -euo pipefail
printf '%s\n' '--- attach flow ---'
sed -n '1,280p' src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts
printf '%s\n' '--- attach failure ---'
sed -n '1,180p' src/main/native-chat/agent-session-wire/structured-agent-session-attach-failure.ts
printf '%s\n' '--- stale settlement definitions and callers ---'
rg -n -C 12 'function settleStaleStructuredAgentSessionState|settleStaleStructuredAgentSessionState|settleFailedPostAcquisitionAttachment' src/main/native-chat/agent-session-wire
printf '%s\n' '--- conversation open ---'
sed -n '1,180p' src/main/native-chat/agent-session-wire/structured-agent-session-conversation-open.ts
printf '%s\n' '--- orchestration attach callback ---'
sed -n '140,215p' src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts

Repository: stablyai/orca

Length of output: 42159


🏁 Script executed:

#!/bin/bash
set -euo pipefail
printf '%s\n' '--- released-child cleanup ---'
rg -n -C 30 'function endReleasedChild|const endReleasedChild|endReleasedChild' src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts
printf '%s\n' '--- post-acquisition settlement implementation ---'
rg -n -C 20 'settleFailedPostAcquisitionAttachment' src/main/runtime src/main/native-chat src

Repository: stablyai/orca

Length of output: 22542


Settle stale state on an acquired attach failure.

If performAttach fails after openConversation succeeds but before onAttached, the acquisition path skips settleStaleStructuredAgentSessionState. The failure cleanup does not call that function, and endReleasedChild returns when no child is indexed. Add equivalent stale-state settlement for this failure path before leaving the opened conversation retained.

@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 in this delta.

Reviewed changes

The delta since the prior pullfrog review (8f66a252) is two commits: a fix to when a conversation open settles a gone generation's turn, and a test-determinism change.

  • The conversation open's stale-turn settlement keys on the opener, not the lease's claim status. openStructuredAgentSessionConversationJournal used to skip its crash-boundary settle when claimStatus read reserved or live. A lease a crashed process leaves in recovery also reads live until the next acquire resolves it, so a send that opened such a chat (phone/CLI after a crash on a host that could not prove the old owner gone) skipped the settle, and a start that then failed left the dead turn running for every reader. A new StructuredAgentSessionConversationOpenOptions = { acquisition?: boolean } is threaded through openStructuredAgentSessionConversation, openStructuredAgentSessionConversationJournal and the delivery's open; only runAttach (and the test helper openTestAttachConversation) pass { acquisition: true }, because the acquisition's reserve already cleared the death evidence and it settles from priorDeathEvidence inside onAttached. Every other open settles unconditionally. Safe because the open only runs when the session is not indexed in this process (the send path early-returns via sessions.get; restoreOneStructuredAgentSessionReadUnderSerialize returns on hasSession), so a genuinely live in-process child is never opened over.
  • The "view binds while the create is still starting" case is now deterministic. The initDelayMs 20/300 race is replaced by an initGate that every initializationResult awaits; the test releases the create only after both holds bind, so a loaded runner can no longer let the create's exit win the race.
  • An indeterminate owner probe is pinned. send-open-stale-turn now parameterizes over pid-absent and indeterminate and asserts the turn ends unverifiable in both; the indeterminate case fails under the old guard (verified: the turn stays running).

Verification: pnpm tc:node clean; 132 tests green across the affected suites (send-open-stale-turn 4, view-start-after-failed-start 2, read-restore 4, restart-restore 3, reveal 13, acquisition-options 22, crash-mid-start 5, wedged-profile-migration 18, dead-generation-settlement 10, accept-then-deliver 19, provider-child-record 15, settled-attach-retry 5, recovery-resolution 7, append-delivery 5).

One earlier finding is untouched by these commits and remains open: the Stop/handover serialize-key race on structured-agent-session-host-mutations.ts (Stop remaps to compact-cancel:${sessionId}, so a just-withdrawn message can still be handed over), which is why this review is not an approval.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@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 in this delta.

Reviewed changes

The delta since the prior pullfrog review (05b8b9307b) is one commit: d5716f3028 Merge main into PR 1. No authored commits, no production change on this branch.

  • Forward-merged main (484598b18e → f72079bd21). The merge brings in 13 commits of main — mobile typed question answers (#23458), terminal input-mode recovery and subscription registration (#23474, #23472, #22948), usage/analytics session identities (#23478, #18759), settings search, tab-ranking and agent-launch caller selection, the Linear team-key fix, and the reopened-tab close fix (#22922).
  • The merge is fully mechanical. git diff --name-only 484598b18e..f72079bd21 and git diff --name-only 05b8b9307b..d5716f3028 are the same 87 files, and none of them is a file this branch changed. So no file was edited by both sides and there is nothing to reconcile — every adopted file equals its main version and every branch file equals its 05b8b9307b version. The PR's native-chat/agent-session-wire code is untouched.
  • The changes nearest this PR's surface are main's own and compose correctly. structured-agent-session-tab-retirement.ts / tabs-close-actions.ts drop the caller-supplied tabId in favour of a derived agent-session:${sessionId} host id (#22922); structured-agent-session-params.ts aliases MAX_RESPONSE_OPTION_ID_LENGTH to the new AGENT_SESSION_RESPONSE_OPTION_ID_MAX_LENGTH (#23458); mobile prompt-answer routing is adopted wholesale.

Verification: pnpm tc (node + web + cli projects) is clean on the merged tree.

One earlier finding is untouched by this merge and stays open: the Stop/handover serialize-key race on structured-agent-session-host-mutations.ts (Stop remaps to compact-cancel:${sessionId}, so a just-withdrawn message can still be handed over). Approval is withheld for it.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@brennanb2025

brennanb2025 commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor Author

Review status at cb5ce82ab7: ready to merge

Verdict: PR 1 now targets main, with main merged in five times as merge commits (no rebases, because other branches stack on this one). The merge resolutions went through three more review passes, the last one clean, and a fresh readiness pass found no P0 or P1. Live Electron QA ran again on a second Mac. No open defect is caused by this PR, and each remaining limit is listed in the description with the PR that owns it.

What changed since the last status comment

Live QA (a second Mac, isolated profile, real Claude CLI; screenshots in the description)

Checks

  • Typecheck (node and web), oxlint and the changed-code quality gate are clean.
  • The cross-version wire test passes 16/16.
  • The chat host suites pass. The only failures are a test that needs a real Claude CLI, and load timeouts that pass when run alone.
  • CI: green at cb5ce82ab7 (30 passed, 11 skipped).

Known limits, each owned by a later PR (details in the description):

  • a fresh chat whose create failed also shows the create's own row;
  • after a failed start the model picker waits for a send;
  • a one-Retry window when an agent dies while starting;
  • only the oldest rejected message offers Retry;
  • no hung-start backstop other than Stop;
  • no live error frame when a journal write fails;
  • a Codex rewind left half-done still waits for a view;
  • a downgrade drops messages in the new "not sent" state.

Not run: SSH (structured chat is local-only), Windows, Linux, and live Codex.

brennanb2025 added a commit that referenced this pull request Sep 28, 2026
…ves-agent branch

Brings in #22821's crash-mid-turn settlement (the attach marks its open as an acquisition and
every other open settles, whatever the lease still claims), main's plain-words refusal copy and
the rest of main.

Resolutions: the stale-turn test keeps PR 1's probe cases and reads the journal through the async
snapshot; the view-start test keeps PR 1's start gate, with each view a subscription instead of a
hold. Locale catalogs auto-merged with every key from both sides.

@coderabbitai coderabbitai 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.

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

🟠 Major · Depend on the logical target, not the target object. · use-structured-agent-session-outbox.ts:94

src/renderer/src/components/native-chat/use-structured-agent-session-outbox.ts:94
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Depend on the logical target, not the target object.

If a caller creates a new object for the same target on each render, this effect can requeue a dispatching entry after a pending response clears the in-flight guard. The next render can then send the same operation again before journal admission.

Proposed change
-  }, [owner.fenceRef, owner.ownerChange, sessionId, target])
+  }, [owner.fenceRef, owner.ownerChange, owner.targetKey, sessionId])

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: e96a8f1c-0c89-40e3-832d-2ab184f7cdc5

📥 Commits

Reviewing files that changed from the base of the PR and between d5716f3 and 740ed1f.

📒 Files selected for processing (8)
  • src/renderer/src/components/native-chat/NativeChatDeliveryRetry.tsx
  • src/renderer/src/components/native-chat/structured-agent-session-outbox-dispatch.ts
  • src/renderer/src/components/native-chat/use-structured-agent-session-outbox-fence.test.tsx
  • src/renderer/src/components/native-chat/use-structured-agent-session-outbox-rejection-cause.test.tsx
  • src/renderer/src/components/native-chat/use-structured-agent-session-outbox.test.tsx
  • src/renderer/src/components/native-chat/use-structured-agent-session-outbox.ts
  • src/shared/structured-agent-session-outbox.ts
  • src/shared/structured-agent-session-send-disposition.ts

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

After the merge with main, every chat write in the composer path reports its
failure as a typed outcome worded by the refusal-notice table, so the send's
catch sees only a local throw. The {code, message} formatter this branch added
for it has no payload left to format, and its claim to be the one way a chat
words a failure is no longer true. The composer send is main's again.
…t was closed

The reopen test checked only that the message reads as not sent; it now also
checks the Retry row carries the host's reason.

@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

The delta since the prior pullfrog review (d5716f3028) is one merge of main and two authored commits.

  • Merged main at 89cf55dfc8 (740ed1fc0e). Brings eight main commits, nearest of which is #22999: a renderer-wide rework of chat-write refusal (agent-session-refusal-notice.ts + agent-session-write-notice-text.ts, i18n keys, a 323-line test) that carries the failure on the outbox entry as lastFailure. Its files overlap this branch's outbox, send-disposition and NativeChatDeliveryRetry, and the merge layered the branch's rejected state on top of main's shapes. git diff 89cf55dfc8 HEAD over the overlapping files shows only the branch's additions, so none of main's #22999 was lost; the branch's reconciledRejectionNotice is superseded by main's entry-carried reason. The other merged main commits (#22913, #22929, #23475, #23306, #23380, #23480) are outside this subsystem.
  • Dropped the branch's second error formatter (7002b1bfb7). agent-session-error-text.ts and its test are deleted, and the composer's catch returns to main's error.message ?? String(error). Every composer-path write (threadGoal.change, runConversationCommand, setOption) goes through mutate/write, which catch and return a typed outcome, so the catch now only sees a local throw.
  • Pinned the rejected-while-closed reason (cb5ce82ab7). The reopen test now also asserts the host's reason rides on the Retry row, not just that the message reads as not sent.

Verification: pnpm tc:web clean; 64 tests green (agent-session-refusal-notice 26, outbox-rejection-cause 7, outbox-fence 3, outbox 21, composer-send 7).

The Stop/handover serialize-key race (structured-agent-session-host-mutations.ts, PRRT_kwDORpCz1s6l-MFW) is untouched by this delta and remains open, so approval is withheld.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

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