Repository navigation
fix(native-chat): every chat action press is its own action (re-land #23916 onto main) - #24298
brennanb2025 wants to merge 1 commit into
Conversation
* fix(native-chat): give every chat write press its own operation id A client kept an operation id per method and payload across presses, so a later identical press replayed an earlier action instead of running: an option picked again stayed on the other one, a goal set again stayed cleared, and a second Stop or phone Stop did nothing. Every press now mints its own id, on desktop and phone, and no client keeps a retry id. The host answers a harmless repeat from what the chat records: the same prompt answer again returns the resolution it holds, and a Cancel of a prompt already cancelled answers ok. A second Stop of the same turn while the first is on its way joins it. * fix(native-chat): answer a /clear pressed again after it committed with that clear Every press now carries its own operation id, so a /clear that reaches the host after this caller's /clear already committed (a double press, or a retype after a lost answer) no longer replays the first id. It started the cleared conversation's agent and then refused it with "This conversation has been cleared." The host now answers such a /clear from the committed record: the same caller, on a conversation whose tab moved to its replacement, gets that clear's result, replacement included, before admission starts anything. No second clear runs. Another window, or a cleared conversation reopened from history, still reads "cleared". * test(native-chat): scope the prompt-cancel tests' journal rows to the thread * test(native-chat): give the repeated prompt Cancel its own host test file * chore(reliability): drop the deleted mobile id-retention test from the gates that ran it * test(native-chat): a repeated draft press under its own id is answered by the draft's state * fix(native-chat): the host joins a second Stop of a turn it is still stopping A client joins it itself only against a host older than this, which runs both and writes a false 'already finished' row for the second. * fix(mobile): keep the Stop's fence narrowed, and type the held card answer * test(native-chat): open the repeat-press host on the shared journal database * fix(native-chat): a second Stop of a turn an earlier Stop answered for adds no row Read from the earlier Stop's note on that turn in the journal, so it holds however late the second Stop lands and from whichever client, and across a host restart. Replaces the in-memory join. The capability now says the host answers a repeated Stop quietly; clients still join a Stop on its way only for a host without it. * fix(native-chat): a /compact or /clear pressed again is answered from the one it repeats A /compact from the same caller joins its last /compact while that one waits or runs, and is answered by it once it compacted with nothing sent since, read from the journal, so a retry after a lost reply never compacts twice. The same caller's second /clear waits for the first and is answered by the committed clear. Another caller or another command is still refused. * Revert "fix(native-chat): a /compact or /clear pressed again is answered from the one it repeats" This reverts commit 25541b7. * fix(native-chat): a Stop writes its one note only when it stopped something, keyed by the turn A Stop that cancelled nothing writes no row, and the note is keyed by the turn it stopped, so another Stop of that turn rewrites it. The earlier-note lookup goes. A /compact or /clear pressed again while one runs is refused as before, and one pressed after it ended runs. * chore: the repeated-Stop capability's comments say what it now means * test(native-chat): the command-start test's client mock knows the repeated-Stop capability * fix(native-chat): a Stop keeps the fence it was pressed at while the 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. * test(native-chat): a Stop keeps its pressed fence against either kind of host
|
| } else if (!cancelled && input.turnId !== undefined) { | ||
| note = { kind: 'status', text: 'The provider had already finished this turn.' } | ||
| // Nothing was left to stop: a Stop that ends nothing writes no row. | ||
| note = null |
There was a problem hiding this comment.
Named Stop refusals go silent When the provider refuses to interrupt a named turn, this branch discards the refusal note because
cancelled is false and turnId is present. The RPC returns with cancelled: false, but the person sees no explanation while the turn keeps running. An already-finished turn can remain quiet without hiding a provider refusal.
| // 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), | ||
| note, | ||
| { fence: ctx.fence, turnScope } |
There was a problem hiding this comment.
Failed Stop overwrites success Stop notes now share a key for each named turn. If one Stop records “Cancellation requested.” and a later Stop of that turn fails without confirmation, the later note replaces the successful one. That loses the accurate record of the first Stop and makes the chat appear uncertain about an interruption it had already confirmed.
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:
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
Mechanism
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.use-mobile-structured-agent-mutation.ts,use-mobile-structured-send-with-outcome.ts); the id-retention helpers are deleted.structured-agent-session-turns-prompt.ts);structured-agent-session-turns-cancel.ts);structured-conversation-command.ts). Only a successful clear counts; another client, or a cleared conversation reopened from history, still reads "cleared".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.structured-agent-session-queued-mutations.ts, unchanged; now pinned by tests).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.conversation-stop,prompt-canceland the newrepeated-stop) moved together intosrc/shared/agent-session-stop-capabilities.ts.protocol-version.tsre-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.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
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:
d4e7d53c7bd, where "The provider had already finished this turn." appears twice in the main bundle;8867990f21d, where it appears 0 times and the capability string 4 times.Each Stop goes through the host's own
agentSession.cancelfrom 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.Before, two Stops at once: the false line under the stopped reply:
Before, the second Stop 1.5 s after the first had answered:
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:
Before, a Stop naming a turn that had already finished:
After, the same Stop writes nothing:
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.A Stop pressed twice: the reply stops and no error shows:
An option picked A, then B, then A again ends on A:
/clear pressed twice on a conversation with a turn lands on one new, empty conversation (current head):
/clear on a cleared conversation reopened from history still says it was cleared (current head):
The same text sent twice is sent twice, and a question card resolves 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) andstructured-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.tsandstructured-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.tsand-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.tsandmobile-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. Mobileuse-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.tsandstructured-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.
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand 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):
agent-session.repeated-stop.v1): the app joins a second Stop of a turn while the first is on its way, as before. A second Stop pressed after the first one's answer arrived can still write "already finished" there, as can a Stop of a turn that ended by itself. Clicking the same answer again after a lost reply shows "The question changed".Follow-ups:
agent-session.repeated-stop.v1(see Differences, temporary).Known, not changed here:
Same on macOS, Linux, Windows and SSH hosts.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)Author: @BrennanKB5