Skip to content

fix(native-chat): every chat action press is its own action (re-land #23916 on main) - #24301

Merged
brennanb2025 merged 294 commits into
mainfrom
brennanb2025/chat-op-id-per-action
Oct 1, 2026
Merged

brennanb2025 merged 294 commits into
mainfrom
brennanb2025/chat-op-id-per-action

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Why this PR exists. #23916 was merged into its base branch (#23935's), 2.5 minutes after #23935 itself had been squash-merged into main, so none of #23916's change reached main. This PR lands that change on main; the description below is #23916's.

What differs from #23916's merged commit (6468680). At first, nothing: all 51 files matched it. Then #24235 ("a Stop ends Claude's process") landed on main and changed the same Stop code, so main was merged in again. Git merged the text without conflicts, but the merge broke two of this PR's rules. One commit (d64c14e) restores them:

Files Added Deleted Net
Test 28 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1334 $\color{#cf222e}{\Huge{\mathbf{−}}}$​238 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1096
Prod 24 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​362 $\color{#cf222e}{\Huge{\mathbf{−}}}$​245 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​117

ELI5

When a chat action failed to get an answer (for example the connection dropped), Orca kept that action's id and quietly reused it the next time you pressed the same thing. The host then treated the new press as a repeat of the old one and did nothing. So picking an option again could leave the chat on the other one, a second Stop could do nothing, and setting a goal again could leave it cleared. Now every press is its own action. A harmless repeat, such as a second Stop of a turn you already stopped, or a second tap on a queued message's Send, is answered from what the chat already records, so it changes nothing and says nothing false.

The plan this belongs to

Goal: one operation id per user action for every structured chat write. Each press is new, only an explicit retry of that one action may reuse an id, and the host answers a harmless repeat from what the chat already records. It ships as:

What Changed

The problem. The desktop app kept operation ids in memory keyed by chat, action and exact payload, and kept one after a dropped connection, an "unknown" answer or a refusal. The next identical press, however much later, reused it, and the host replayed the old outcome instead of running. The phone's Stop and its /compact and /clear did the same. Seen as:

  • pick option A (connection drops), pick B, pick A again: the chat stayed on B;
  • a Stop naming a turn or a background task, pressed again after a failure: nothing happened;
  • set a goal (answer lost), clear it, set it again: it stayed cleared;
  • a /compact retyped later, even after newer messages, replayed the old "completed" and did not compact, and on the phone that lasted up to 24 hours.

Scope. Structured Claude and Codex chats, on desktop, paired desktop and phone, local and SSH hosts. That includes the Send, Delete and Resume buttons on messages queued while the agent works (#23731, #23736). Sends on the desktop already used a fresh id per message and are unchanged. Phone sends are step 2.

Before / After

You do Before After
Pick A (lost), B, then A again stays on B switches to A
Set a goal (lost), clear, set it again stays cleared set
Stop again after a failed Stop (desktop named Stop, background task, phone) nothing happens stops
Type /clear again after its answer was lost worked only until the app reloaded; after that "This conversation has been cleared" the chat moves to the current conversation, nothing new runs, also after a reload
Stop a turn twice by its id (a phone double-tap, or Stop on two devices), at once or a moment apart a second row, "The provider had already finished this turn.", which is false one "Cancellation requested." row
Stop a turn that already ended by itself "The provider had already finished this turn." no row; nothing was stopped
Press /compact again while the first waits or runs answered from the kept id refused: "Wait for the current turn to finish before using this command."; it runs once
Press /compact again after the first ended answered from the kept id, even after newer messages runs again
Press /clear again while the first is still running answered from the kept id refused: "Wait for the conversation operation to finish."; one new conversation
Tap a queued message's Send, Delete or Resume twice answered from the id shared by the two taps each tap has its own id; the second is answered from the card's state: already sent, already gone, or nothing left to resume

Mechanism

  • Renderer use-structured-agent-session-mutate.ts: every call mints its own id. The per-payload id map, the kept /compact and /clear id, and the shared "does this write name its target" check (structured-agent-session-operation-identity.ts) are deleted.
  • Mobile: Stop, the composer command and the queued-message buttons mint a fresh id per press (use-mobile-structured-agent-mutation.ts, use-mobile-structured-send-with-outcome.ts); the id-retention helpers are deleted.
  • Host, where a repeat is harmless, answers from what the chat records:
    • the same answer to a question already answered that way returns the stored answer (structured-agent-session-turns-prompt.ts);
    • Cancel of a question already cancelled answers ok (structured-agent-session-turns-cancel.ts);
    • a /clear from the same client after its /clear committed, with no tab left on the old conversation, returns that clear's result, replacement included, before anything starts (structured-conversation-command.ts). Only a successful clear counts; another client, or a cleared conversation reopened from history, still reads "cleared".
    • a Stop that stopped nothing writes no row (structured-agent-session-turns-cancel.ts). "The provider had already finished this turn." is gone. The one "Cancellation requested." note is keyed by the turn it stopped, found by the turn's id whether it still runs or has ended (structured-agent-session-turn-stop-notes.ts), so another Stop of that turn rewrites that row instead of adding one. A Stop that didn't do what was asked still says so: the refusal row while the turn keeps running, and "Cancellation was not confirmed." when the adapter failed.
    • a second /compact or /clear while the first waits or runs is refused by the existing gates, with the existing lines; one pressed after it ended runs again. A retry of the same operation id is answered from what it already wrote (the ledger replay, unchanged).
    • a second Send, Delete or Resume of a queued message: an already-sent card answers with the submission it made, an already-deleted one with "already withdrawn", and a Resume with nothing paused with "resumed nothing" (structured-agent-session-queued-mutations.ts, unchanged; now pinned by tests).
  • A new host capability, agent-session.repeated-stop.v1, says the host writes no row for a Stop that stopped nothing, so a repeated Stop is quiet. The desktop and phone check it, and against a host without it, each still joins a second Stop of a turn to one already on its way, as before this PR, so an older host never writes the false row.
  • The three capabilities that gate a Stop (conversation-stop, prompt-cancel and the new repeated-stop) moved together into src/shared/agent-session-stop-capabilities.ts. protocol-version.ts re-exports them, so no importer changes. Main had filled that file to its line limit, and this keeps one Stop's wire rules in one place instead of trimming unrelated lines.
  • Nothing new is saved. The only change on the wire is that one capability string.

Why

A press is a user action, and an action's identity cannot be inferred from its payload: the same pick an hour later is a new choice. Retries of one action stay possible where a stored record of that action exists (a desktop message's outbox entry; #23524's host-side /clear retry), so the client never has to keep an id it can't re-derive. Repeats that are harmless are answered from state the host already holds, not from a remembered id.

Alternatives considered:

Differences from the common pattern

  • A common approach mints a random command id per call and replays only an identical id, and records that id on every event it produces. This PR matches the per-call id and the exact-id replay. It does not record the id on every chat row, because nothing here needs to answer "which press made this row", and it would add a stored field to every row.
  • Matches: a Stop is recorded at the time it takes effect and only when it takes effect. One common approach writes its interrupted row in the same transaction that moves the thread to stopping, and a Stop that applies to nothing (already stopping, or idle) writes nothing. The other upserts the turn, keyed by turn id, to interrupted. Here the one note is written on the provider's answer to the interrupt, keyed by turn, and a Stop that stopped nothing writes nothing.
  • Matches: a failed Stop still says so. A common approach appends an "interrupt failed" activity when the interrupt fails or no session is bound. Here the refusal row and "Cancellation was not confirmed." stay.
  • Matches: /compact and /clear: each press has its own id, only a replay of the same id is answered from its record, a press while one runs or a turn is active is refused by state, and a later press runs again. That is how both common approaches handle them.
  • Intended: a common approach also joins concurrent Stops per thread in memory. Here two Stops at once reach the host one after the other on the session's lane, and the second either stops nothing, writing nothing, or rewrites the same turn-keyed row, so no join is needed on a current host.
  • Temporary: the desktop and the phone still join a second Stop of a turn to one on its way when the host lacks agent-session.repeated-stop.v1. It exists only so a new app talking to an older host doesn't write the false row. The follow-up deletes both client joins once no supported host lacks that capability.

Linked Issue

No issue. Found while fixing Stop and send delivery in #23026 and #23553.

Visual Proof

A Stop of a turn, sent twice under separate ids, and a Stop of a turn that already ended. A double Stop under separate ids is what a phone double-tap sends, or Stop pressed on two devices. Live on a second Mac with the real Claude CLI 2.1.285, in a production-renderer build driven over CDP. Two arms, each proven from its built bundle:

  • before: this PR's base, fix(native-chat): /clear starts nothing; the new chat's first message starts its agent #23935 at d4e7d53c7bd, where "The provider had already finished this turn." appears twice in the main bundle;
  • after: this PR at 8867990f21d, where it appears 0 times and the capability string 4 times.
    Each Stop goes through the host's own agentSession.cancel from the page, naming the running turn. The rows below were also read back from each arm's journal. After this PR, the note's key is the turn id, and it is written once per stopped turn.
Case Before After
Two Stops at once "Cancellation requested." then "The provider had already finished this turn." one "Cancellation requested."
The second Stop 1.5 s after the first one answered (the slow-link case) the same two rows one "Cancellation requested."
A Stop naming a turn that had already ended by itself "The provider had already finished this turn." no row
The desktop's own Stop button double-clicked (it names no turn) one row one row

Before, two Stops at once: the false line under the stopped reply:

Before: two Stops of the same turn at once leave "The provider had already finished this turn." under the stopped reply

Before, the second Stop 1.5 s after the first had answered:

Before: a second Stop 1.5 s after the first leaves "The provider had already finished this turn."

After, both at once and 1.5 s apart: no false line. The two runs stopped at the same point and rendered identically, so one image shows both:

After: two Stops of the same turn, at once or 1.5 s apart, leave no extra line

Before, a Stop naming a turn that had already finished:

Before: a Stop of a turn that had already finished writes "The provider had already finished this turn."

After, the same Stop writes nothing:

After: a Stop of a turn that had already finished writes no row

Phone QA: BLOCKED. The emulator QA skill this needs can only be started by a person, not by this session, and no other way of driving the phone is used in its place. So these phone checks were not run live: overlapping taps on a queued message's Send, Stop tapped twice against this PR's host, and the phone's Stop against a host without agent-session.repeated-stop.v1. The 68 mobile tests above cover them, including the phone's Stop join against a host without the capability, and both presses going out against one with it.

Earlier heads: nothing regressed. Live QA with the real Claude CLI on a second Mac, in a production-renderer build, driven over CDP. Three arms: this PR before its last rebase (09fa9e908eb), its base (f26e8039693), and the current head (89564af9862) for the /clear cases, each proven from its built bundle. Everything the app can produce behaves the same on every arm, so these runs show nothing regressed. The cases this PR fixes all need a lost reply, which the app cannot produce on demand; the tests below cover them.

Scenario Result, every arm
Stop pressed twice the reply stops, one Stop row, no error (at 110 ms the button is already gone, so the second tap misses)
Model Haiku → Sonnet → Haiku ends on Haiku, and the next reply ran on Haiku
/clear pressed twice ~27 ms apart one new conversation, no error, also on a conversation with a finished turn
/clear typed in the old conversation reopened from history "This conversation has been cleared. The command didn't run. Open the current conversation to continue.", as described above
Question card answered, pressed again resolves once, no error
"yes" sent twice both sent and answered

A Stop pressed twice: the reply stops and no error shows:

Stop pressed twice: the reply stops at 52 with one Stop row and no error

An option picked A, then B, then A again ends on A:

Model picked Haiku, Sonnet, then Haiku: the chat ends on Haiku and the next reply ran on it

/clear pressed twice on a conversation with a turn lands on one new, empty conversation (current head):

/clear pressed twice: one new empty conversation, no error

/clear on a cleared conversation reopened from history still says it was cleared (current head):

/clear on a reopened cleared conversation: "This conversation has been cleared"

The same text sent twice is sent twice, and a question card resolves once:

Both "yes" sends answered and the question resolved once

Testing

  • I manually tested these changes locally

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

  • New use-structured-agent-session-mutate.test.tsx (a fake host ledger) and structured-agent-session-repeat-press.test.ts (the real host): an option picked again, a goal set again, a named and a background Stop pressed again, the joined Stop reporting once, and the same answer again.

  • structured-conversation-command.test.ts and structured-chat-tab-table.test.ts: a /clear double press and a retype after a lost answer land on the current conversation with nothing started; another client and a reopened conversation still read "cleared"; a failed clear runs fresh.

  • structured-agent-session-host.test.ts and -prompt-cancel.test.ts: a second prompt Cancel, sent while the first is in flight or after it, answers quietly and asks the agent once.

  • Mobile mobile-structured-composer-command.test.ts and mobile-structured-operation-id-retirement.test.ts: fresh ids per press. Against a host without the capability, Stop joins the one on its way. Against a host with it, both presses go out.

  • New structured-agent-session-repeated-stop.test.ts (the real host and journal): a second Stop after the first settled writes nothing; two Stops at once leave one row (keyed by turn; keying by operation id again fails it); a Stop of a turn that ended by itself, or two Stops that both found nothing, write nothing (bringing back "already finished" fails 7 tests); a relaunched host writes nothing more.

  • New structured-agent-session-queued-repeat-press.test.ts (the real host): Send, Delete and Resume of a queued message pressed twice under separate ids: sent once, deleted once, resumed once, and the second press answered, not refused. Removing the host's "already sent" answer fails the Send test. Mobile use-mobile-structured-agent-session-queued.test.tsx: overlapping card presses carry their own ids; it fails with main's id sharing.

  • structured-conversation-compaction.test.ts and structured-conversation-command.test.ts: a /compact pressed again under a new id while one runs is refused and runs once, and one pressed after it ended runs again; a second /clear pressed while the first runs is refused and makes one new conversation. Joining the same client's /clear instead fails the /clear test.

  • Desktop use-structured-agent-session-mutate.test.tsx: the Stop join is used only against a host without the capability; with it, every press goes out under its own id.

  • Each new test fails on the base; removing each rule fails its test.

  • tc:node, tc:web, the mobile typecheck and its test ratchet, oxlint, check:code-quality:changed, check:react-doctor:changed, the reliability gates and the anti-slop audit pass on the current head. 21 related host and desktop test files (227 tests) and 6 mobile files (68 tests) pass.

AI Disclosure

Review

Four review loops and an architecture challenge.

  • Loop 1 found that fresh ids made a quick double Stop or double Cancel write a misleading line; fixed by answering those repeats from what the chat already records, and by joining a Stop still in flight.
  • Loops 1 and 2 both found holes in keeping a /compact or /clear id on the client for a retry (it could replay an old "done", and it was lost on reload). Because the same mechanism failed twice, an architecture challenge replaced it: clients keep no retry id at all, and a /clear retry is found by the host (fix(native-chat): an unfinished /clear or refused Codex rewind no longer locks the chat #23524 and fix(native-chat): /clear starts nothing; the new chat's first message starts its agent #23935).
  • Loop 3 found only an accidental lockfile change and a doubled error toast, both fixed.
  • Final loop after the rebases found no code issues; description wording was corrected.
  • After merging main (the queued-message buttons) and adding a host-side Stop join, one more loop found:
    • two P1s, both test-only: five renderer test mocks lacked the new capability export, and one test from main still expected /clear to reuse its id. Both fixed.
    • a P2 in the design: the in-memory join covered only the time the host spent on the first Stop. It was replaced by the journal read described above, and the capability's meaning changed with it.
    • a /compact pressed again after a lost reply could compact twice once the first had finished, and a second /compact or /clear while the first was running showed a refusal line where main was quiet. Both are now answered from the command they repeat, as described above.
  • Checked against both reference projects first-hand, after that loop: the in-memory join and the journal lookup were replaced by the shapes above. A Stop records only what it did, keyed by turn; /compact and /clear repeats are refused while one runs and run again after, with only same-id replays answered. The repeat-answer code for /compact and the same-client /clear join were removed.
    • a P3 not changed: the desktop reads the capability from the runtime's status cache before a Stop naming a turn, which can cost one status round trip against a remote host. The desktop names a turn only for background-task Stops and for hosts without the conversation Stop, so this is rare.

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

Merge order: #23524, then #23935, then #23916, then #23917.

Mixed versions (clients and hosts update separately):

Follow-ups:

  • Delete the desktop and phone Stop joins once no supported host lacks agent-session.repeated-stop.v1 (see Differences, temporary).

Known, not changed here:

  • After a /clear whose answer was lost, a pane still showing the old conversation catches up only on its next tab sync; that sync is not covered by a test.

Same on macOS, Linux, Windows and SSH hosts.

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)

Author: @BrennanKB5

… stop

The eviction step reads a stop's result through `stopAgentSessionProviderRoot` and hands that
verdict to the child's ending, so the host never forms a second view of whether the root is gone.
Every ending carries it: a stop's comes from that reading, an exit's root is gone by definition,
and a failed re-attach passes what its release saw. The end-of-child record can therefore also
carry a stop whose root was not seen to go, which nothing ends on yet.
…as it

The host now lists agent-session.accepted-send.v1 among its own runtime capabilities, the same
string capable clients already send. A client can then tell a host that answers a send at
acceptance, and admits a Stop with no writer before a turn starts, from an older one that still
restarts the agent inside the send. Additive: an older client ignores a capability it does not
know.
The attach adopts the conversation's open journal, which outlives it, so it no longer opens one
for a direct caller either. That leaves nothing for a failed adopted import to close, and the flag
that told the two cases apart is gone. Tests that attach without a host open the conversation the
way a host does.
…s first

The outbox treated any fence change as a new owner: it dropped the answer of a send in flight,
queued that send to go out again under the same id, and unblocked a refused head. On an older
host that is how a send the restart refused, unrecorded, gets another try. On a host that records
every send before it starts an agent, a fence moves because that start ran, so the same rule
resent into every failed start. With a fence stamped on every frame, that became a loop.

The outbox now reacts to a fence change only when the host has not advertised that it accepts a
send before any agent has it. On such a host, only a Retry or a new send goes out, and a failed
start reaches the client as a rejected message it keeps with its Retry. Against an older host, or
before one has answered, the outbox behaves as it did. Desktop and paired web share this hook.
…t stopped it

The end-of-child record's cause now tells a user's Stop from the host stopping the child for a
cause of its own: `user-stop` and `host-stop` replace `stop`. The delivery loop goes on after a
user's Stop, as before, and fails the start it was waiting on after a host stop, with the one
error row and every queued message rejected, in the stop's reason when it gave one. The reason
stays description only. Stop passes `user-stop`; nothing passes `host-stop` yet.
…ffered for resume

A message accepted while the agent was starting counts as working in the chat, and quit rejects it
as never sent. The teardown snapshot read the same working rule, so a relaunch offered to resume a
chat whose agent never had the message. The snapshot now reads only what was handed over.
Opening a chat no longer starts its agent. A conversation is reached through one host
accessor that opens its journal at rest, and a send is what starts the agent, through
the delivery loop. One idle sweep, every five minutes, stops an agent that has been
quiet for thirty minutes and owes no work, then drops an open journal handle that is
only a cache. Its record, tab, status row and readers stay.

- hold and release are no-ops; hold still builds the host for shipped mobile builds.
- The holders, the holds, the release clock and the exit respawn are deleted.
- Options, the model list, the goal and the context meter answer at rest; a model pick
  at rest is recorded as intent for the next start.
- Compact, rewind, clear and goal changes start the agent first. A send does too when
  a rewind is still in doubt after the conversation opens.
- Orchestration routes mail and group addresses on ownership (the record plus the chat
  tab), not on whether the process runs. An open dispatch keeps its worker running.
- The restart continuation is a send; Resume all holds each slot until the message is
  handed over or rejected.
- A read error never replaces a loaded transcript, and shows the host's own words.
…at message's failed start

Opening a chat's tab starts an agent for the view, and a send accepted meanwhile waits on it. When
that start died, its exit wrote the start's error row and left the message queued, so the delivery
loop started a second agent into the same failure and wrote a second row. A child's end now records
where the conversation's journal stood, and the loop settles a message accepted before a failed
start ended with that start: one row, under its key, and no second start. A message sent after the
failure still gets a fresh start.
A structured chat whose only message the agent's start refused read as a
green finish, and a cancelled structured turn did too: the host published a
verdict only for turn records, and structured rows carried no `interrupted`.

The host projection now reads the session's latest request: its turn's
outcome, or `failure` for a send the agent or its start refused. A send
that was withdrawn, or left undelivered by a restart or a close, fails
nobody and makes nothing listable. The ingest publishes `interrupted` as the
hook lanes do, and every reader decodes the verdict through one accessor, so
a failure reads Failed on the dot, the rollups, history and `worktree ps`,
behaves like a cancellation in every clean-finish policy, and notifies as
"failed".
…ompare

A verdict change always moves the completion clock the same check already
reads, so a second verdict compare there caught nothing new.
…liver' into brennanb2025/chat-failed-reads-failed
… offer names

A view's child that dies while a sent message waits settles that message only when it died starting
and no child has taken its place: a proven child's crash, or a second start since, gets the message
delivered. The resume offer names the handed-over message, never a newer one still queued.
…gain

The offer used to end only when the chat's newest user message changed,
because opening a chat started its agent and that start could not be told
apart from real activity. Opening a chat starts nothing now, so the host
reads the fact it already publishes: a chat's status row goes from not
host-owned to host-owned exactly when its agent is started. At that edge the
offer and any failure record for the chat are withdrawn, unless the start is
a resume action's own (its continuation is the oldest undelivered message).

A continuation and a message racing to be first are decided at acceptance:
the continuation is refused, quietly and with nothing filed, when any other
message was accepted since the restart. A failed continuation start leaves
the offer retryable, and each resume action sends its own message id.

Deleted: the newest-user-message comparison, its journal reader, the
continuation filter, and the failure ledger's own "answered by the chat"
check. The marker still carries its message id for one release, so the
previous build can read it.
Desktop: the IPC subscription controller was dropped as soon as the streaming
handler returned, which for most streams is right after it binds. A later
runtime:unsubscribe then found nothing to abort, so the host kept the subscriber
and derived and sent every publish to a channel no one listened to. The controller
now lives until the renderer unsubscribes, resubscribes the same id, or goes away.

Mobile: disposing an agentSession.subscribe stream now sends agentSession.unsubscribe
with the stream's frame id, so the host ends that subscriber and leaves a sibling
stream on the same socket running. The direct path now passes the frame id the relay
path already passed.
…ery record failed

A provider-session heartbeat that rewrites a completed recovery record kept
its interrupted flag but dropped the outcome it was copied with, so a live
failed checkpoint read as a clean finish until the next status write.
The preamble send now takes only what it reads of the host, the send, the settlement wait and the
record's fence, so its test builds that host with real types instead of `as never`.
…t field

The bell notification test still checked for agentInterrupted, which no
longer exists, so it could not catch a verdict leaking into a bell dispatch.
…liver' into brennanb2025/chat-failed-reads-failed
…liver' into brennanb2025/chat-conversation-outlives-agent

# Conflicts:
#	src/main/native-chat/agent-session-wire/structured-agent-session-unexpected-exit.test.ts
#	src/main/runtime/rpc/methods/orchestration-structured-worker-session.test.ts
Attention readers (completion time, Smart Sort, sticky retention, Cmd+J
Recent) now demote only a turn the user stopped. A failure is news the
user has not seen, so it keeps its completion time, ranks in the Done
class, stays retained after its pane goes away, and a retained failure
reads failed in the worktree rollup instead of done. Clean-finish
policy (hibernation, pane ownership, the value moment) still treats a
failure like a stop.

The retention trigger compares verdicts again: success -> failure no
longer moves the completion clock.
…ailed' into brennanb2025/chat-turn-facts-from-record
…nce the restart

The offer is live while no other message has been accepted in the chat since the
restart and its agent has not proved a start since. The offer list, the resume's
reservation check and the continuation's acceptance check all read that one fact,
so a message whose start then failed withdraws the offer too, and a stale click
finds nothing to act on.

The fact is read off the conversation's open handle, which the restart closed, so
it is retired durably whenever it may have changed: a message accepted, a start
proven. A close and reopen within the same run therefore cannot bring the offer
back. A continuation rejected before it reached the agent does not count, so a
retry after a failed start still runs.

Deleted: the quit-time gate on withdrawal, which changed nothing because the
withdrawal and the quit's own offer write share one queue; the per-action
"withdrawn" flag and the separate acceptance check it paired with.
…records

The offer lives in a file the previous release reads after a downgrade. Pin that
against the pinned release's own capsule, and run the lane when the marker or the
capsule changes.
…d when it was taken

"Since the restart" was read off the conversation's open handle, which the idle
sweep closes: after a reopen, a message the user had already sent looked older
than the handle and the withdrawn offer came back.

The offer now records the journal position (epoch and sequence) at the moment
it is taken, and a message accepted after that position, or a journal on another
epoch, means the chat moved on. That is derived from the journal, so it holds
across any number of closes and reopens. An older build's offer has no position;
only a start withdraws it. Because the message half is now durable, the offer is
no longer rewritten in the recovery file on every accepted message; a proven
start still writes it, since only the host that saw the start knows of it.
…, where main's tests still used the old calls

#24006 moved the store into the journal database and #23684 added a test on the
old open call; main's close now takes a cause. Four tests catch up.
…member

The at-rest `/` surface's read and change listener travel together as
`atRestCommands`, which the Claude catalog already is, so the adapter types
stay within their line limit after main's growth.
…st-send' into brennanb2025/chat-op-id-per-action
…client checks the host

The desktop's capability check before a named Stop could let the runtime move
underneath, so the Stop went out against the new one; it now carries the fence
read at the press.
# Conflicts:
#	src/main/native-chat/agent-session-wire/structured-conversation-command-admission.ts
@greptile-apps

greptile-apps Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[Medium risk] Refactors chat action deduplication and retry logic.

The PR does not yet appear safe to merge because a later failed Stop can still erase a successful Stop’s confirmation.

Findings

  1. P1 Later Stop erases confirmation ▶

Summary

The PR gives structured chat actions fresh operation IDs and makes harmless repeated actions answer from recorded host state. The latest integration preserves quiet repeated-Stop behavior alongside session-ending Stops.

Reviews (2) · Last reviewed commit: "fix(native-chat): keep the repeated-Stop..."

// Keyed by the turn it stopped, so another Stop of that turn rewrites this row, never adds one.
await ctx.journal.appendItem(
structuredAgentSessionStopNoteIdentity(input.clientOperationId),
structuredAgentSessionStopNoteIdentity(input.turnId ?? liveTurnId ?? input.clientOperationId),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Later Stop erases confirmation When a second Stop of the same turn times out or throws after the first succeeded, this turn-keyed write replaces the earlier “Cancellation requested.” row with “Cancellation was not confirmed.” The transcript then loses confirmation that the first Stop succeeded.

Knowledge Base Used: Agent workspaces and sessions

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

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: 2c0acd10-2ab2-44aa-88e6-12afa09cb5d6

📥 Commits

Reviewing files that changed from the base of the PR and between 2423599 and d64c14e.

📒 Files selected for processing (5)
  • src/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-prompt-state.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts

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


📝 Walkthrough

Walkthrough

Mobile and renderer clients now use fresh operation IDs for mutation presses. Repeated Stop handling uses a runtime capability and joins matching in-flight requests on hosts without quiet repeated-Stop support. Host changes handle repeated prompt answers and cancellations. The conversation-command path can return a committed /clear result before mutation admission. Tests cover these paths and update the reliability gate for a removed mobile test.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to d64c1

The repeated-action changes have no established merge-blocking issue. Concurrent matching Stops remain joined on older hosts, and capability lookup failures safely fall back to that behavior.

Security Architecture Review

Security architecture risk: 🔵 Low · up to d64c1

This changes how repeated chat actions are identified and recovered. The examined checks still protect action ownership and stale state, but older-version compatibility and interrupted recovery are not fully established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The directly evidenced control surface is session mutation state: turns, prompts, queued-message journals, and conversation replacements. Cancellation reaches provider adapters with session, turn, and runtime-fence context. This evidence does not establish tenant-wide containment or downstream credential authority.

Trust Boundaries and Controls

  • observed — Client-supplied mutation fields are fingerprint-validated at ordinary admission, operations are evaluated under callerKey, and execution uses attached-session and runtime-fence state. Fresh IDs change action identity but do not replace these admission controls.
  • observed — The committed-clear exception checks the command and session-specific clear fingerprint, then uses committedClearOfCaller to select the caller-owned committed record only after its source tab has moved away. It returns that prior outcome rather than authorizing another clear.

Resilience and Maintainability Implications

  • observed — Queued-draft carry is not atomic across source and replacement journals: the clear commits first, replacement rows are inserted individually, and carry errors are reported without failing the clear. Partial carry can therefore leave drafts on the superseded source. Base/head comparison shows this behavior predates the PR, and the prior committed-outcome replay also did not re-run carry. Carried rows start paused, while source supersession blocks draining; a separate reconciliation path was not established.

Hardening Proposals

  • proposed — As separate recovery hardening, consider making incomplete queued-draft carry explicitly reconcilable while preserving the paused replacement queue and superseded-source execution block. This addresses a preexisting recovery limitation, not an established security regression from this PR.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 32.69% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 52 functions across 46 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title clearly identifies the main change: each native-chat action press receives its own action identity. It is concise and relevant.
Description check ✅ Passed The description is detailed and covers the user impact, implementation, rationale, testing, visual proof, compatibility, review findings, and known limitations. It does not provide an actual linked is…
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • 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.

@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: d9b0257b-68b3-4305-b602-5019665056ff

📥 Commits

Reviewing files that changed from the base of the PR and between 2a83c95 and 2423599.

📒 Files selected for processing (51)
  • config/reliability-gates.jsonc
  • mobile/src/session/mobile-structured-agent-session-cancel.ts
  • mobile/src/session/mobile-structured-agent-session-host-support.test.ts
  • mobile/src/session/mobile-structured-agent-session-host-support.ts
  • mobile/src/session/mobile-structured-agent-session-rpc.ts
  • mobile/src/session/mobile-structured-composer-command.test.ts
  • mobile/src/session/mobile-structured-composer-command.ts
  • mobile/src/session/mobile-structured-operation-id-retirement.test.ts
  • mobile/src/session/mobile-structured-session-operation-retention.test.ts
  • mobile/src/session/use-mobile-structured-agent-mutation.ts
  • mobile/src/session/use-mobile-structured-agent-session-prompt-cancel.test.tsx
  • mobile/src/session/use-mobile-structured-agent-session-queued.test-fixture.ts
  • mobile/src/session/use-mobile-structured-agent-session-queued.test.tsx
  • mobile/src/session/use-mobile-structured-agent-session.ts
  • mobile/src/session/use-mobile-structured-send-with-outcome.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-claude-queued-stop.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-command-turn.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-conversation-stop.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-grouped-prompt.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-host.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel-repeat.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-prompt-cancel.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-prompt-state.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-queued-repeat-press.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-repeat-press.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-repeated-stop.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-turn-stop-notes.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-turns-cancel.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-turns-prompt.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-turns.test.ts
  • src/main/native-chat/agent-session-wire/structured-conversation-command-admission.ts
  • src/main/native-chat/agent-session-wire/structured-conversation-command.test.ts
  • src/main/native-chat/agent-session-wire/structured-conversation-command.ts
  • src/main/native-chat/agent-session-wire/structured-conversation-compaction.test.ts
  • src/main/runtime/rpc/methods/structured-chat-tab-table.test.ts
  • src/renderer/src/components/native-chat/structured-conversation-command-send.ts
  • src/renderer/src/components/native-chat/use-structured-agent-session-command-start-failure.test.tsx
  • src/renderer/src/components/native-chat/use-structured-agent-session-mutate.test.tsx
  • src/renderer/src/components/native-chat/use-structured-agent-session-mutate.ts
  • src/renderer/src/components/native-chat/use-structured-agent-session-prompt-cancel.test.tsx
  • src/renderer/src/components/native-chat/use-structured-agent-session-provisional.test.tsx
  • src/renderer/src/components/native-chat/use-structured-agent-session-stop.test.tsx
  • src/renderer/src/components/native-chat/use-structured-agent-session-write-refusal.test.tsx
  • src/renderer/src/components/native-chat/use-structured-agent-session.queued-gating.test.tsx
  • src/renderer/src/components/native-chat/use-structured-agent-session.test.tsx
  • src/renderer/src/components/native-chat/use-structured-agent-session.ts
  • src/renderer/src/runtime/structured-agent-session-client.ts
  • src/shared/agent-session-stop-capabilities.ts
  • src/shared/protocol-version.ts
  • src/shared/structured-agent-session-operation-identity.test.ts
  • src/shared/structured-agent-session-operation-identity.ts
💤 Files with no reviewable changes (5)
  • mobile/src/session/mobile-structured-session-operation-retention.test.ts
  • src/shared/structured-agent-session-operation-identity.test.ts
  • src/shared/structured-agent-session-operation-identity.ts
  • src/renderer/src/components/native-chat/structured-conversation-command-send.ts
  • mobile/src/session/use-mobile-structured-send-with-outcome.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.

Comment on lines +163 to +168
if (
!namesWhatItStops(fingerprintMethod, fields) ||
(await supportsStructuredAgentSessionQuietRepeatedStop(target))
) {
return send<T>(method, fingerprintMethod, fields, pressedFence)
}

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.

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

🔎 Supported by static analysis

🏁 Script executed:

#!/bin/bash
rg -nP -C8 'function structuredAgentSessionHostSupports' src/renderer

Repository: stablyai/orca

Length of output: 1944


Prevent overlapping Stop requests during capability detection.

structuredAgentSessionHostSupports awaits ensureLocalRuntimeCapabilities() when local capabilities are unknown. The Stop path performs this await before it records inFlightStops. Two quick Stop presses can therefore both bypass deduplication and send overlapping Stops to an older host.

Reserve a pending entry before the await, or resolve and cache the capability result so both presses use the same decision.

@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 — re-land of #23916 onto main, reviewed as the net 51-file diff against main. The change makes every structured chat write mint a fresh client operation id per press, moves the harmless-repeat answers to the host, and adds the agent-session.repeated-stop.v1 capability.

  • Desktop client (use-structured-agent-session-mutate.ts): fresh id per send; the per-payload id map and operationIdOverride are gone; pressedFence is read before the first await; a Stop naming a turn/task joins an in-flight one only against a host lacking repeated-stop, with the joined press mapping not-done→dropped so only the first press toasts.
  • Mobile client: fresh id per press for the mutation seam, composer command, and Stop; the operationIds thread and the id-retention helpers are deleted; Cancel joins in-flight only when host support is false.
  • Host Stop/prompt idempotency: a repeated prompt Cancel answers quietly (settledPrompt); a re-click of an already-resolved prompt with the same choice answers from the stored resolution (resolvedWithSameChoice), while a different choice still refuses; a Stop that stopped nothing writes no row, and the single "Cancellation requested." note is keyed by the turn it stopped.
  • Host /clear: a same-caller re-press on a source whose tab already moved is answered from the committed clear before admission (which would otherwise refuse conversationCleared), starting nothing.
  • Capability plumbing: the three Stop capabilities move to agent-session-stop-capabilities.ts, re-exported and spread into RUNTIME_CAPABILITIES; the new capability is advertised by the host and consumed by desktop and mobile.

I read all 51 files in the diff and ran the affected suites at head, all green: the host repeated-stop, turns, prompt-cancel, conversation-command, repeat-press, and queued-repeat-press tests, plus the desktop mutate/stop/command-start-failure renderer tests. Two independent passes on the host repeat/stop idempotency and the desktop/mobile fresh-id + in-flight-join + fence semantics found no reachable defect. I could not run the mobile suite here (no mobile/node_modules in this environment); the PR reports those 68 tests pass.

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

…on-ending Stop

A Stop that ends the provider's session stopped its turn, so it writes its note even when the
provider declined the interrupt; a repeated Cancel of a cancelled prompt is answered at the prompt
Cancel's new entry point too.

@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 — incremental re-review of the commits since the prior pullfrog review. main was merged in again after #24235 ("a Stop ends Claude's process") landed and rewrote the same Stop code; the merge silently broke two of this PR's rules, and commit d64c14e restores both.

  • Restored the session-ending Stop's note. The "a Stop naming a turn that stopped nothing writes no row" decision moved from an early else if (!cancelled && input.turnId !== undefined) to an after-the-fact guard, so a Stop that ends the provider's session still writes its one "Cancellation requested." row even when the provider declined the interrupt (structured-agent-session-turns-cancel.ts:195).
  • Answered a repeated prompt Cancel at the new entry point. answerCancelOfSettledPrompt is extracted into structured-agent-session-prompt-state.ts:37 and both the prompt Cancel's own file and performCancel's prompt branch route through it, so a second Cancel of an already-cancelled prompt answers quietly instead of being refused as "moved on".
  • Tests updated to pin the rules. New test structured-agent-session-conversation-stop.test.ts:401 (declined named-turn Stop on a session-ending provider); the old ALREADY_FINISHED expectations are replaced with no-row assertions.

I verified both new rules are load-bearing: reverting structured-agent-session-turns-cancel.ts and structured-agent-session-prompt-cancel.ts to the pre-fix merge commit 317f968 makes the new tests fail, and restoring them passes. The focused host suites are green at head (conversation-stop, prompt-cancel, prompt-cancel-repeat, turns, repeated-stop, repeat-press, queued-repeat-press, conversation-command, conversation-compaction, claude-queued-stop — 106 tests).

Pullfrog  | 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