Repository navigation
fix(native-chat): edit a queued message in place, even while a question is open - #26886
Conversation
A queued card's Edit used to copy its text into the composer and delete the card, so the message lost its place, an agent-mail card lost its sender, and while a question or approval replaced the composer the text vanished. The execution host now edits the card itself: - agentSession.queuedMessageUpdate replaces only the card's text, as a compare-and-set on the body fingerprint the client last saw. It keeps the id, position, sender, kind, send mode and attachments; a command card is not editable. A resent Save answers from the card as it stands (unchanged or changed), so a lost answer never overwrites newer text. - agentSession.queuedMessageEditHold acquires, renews and releases an in-memory edit lease per caller and edit. While one is live, automatic delivery stops at the card (earlier cards still send, later ones wait); an explicit Send still overrides. Leases expire after two minutes on their own, re-derive validity from the card on every read, and die with the conversation. Release uses cleanup admission. - The published card carries an optional editHeld marker, never `paused`. - Both methods sit behind the new agent-session.queued-message-edit.v1 capability, independent of the dark mid-turn queue capability, and are on the mobile allowlist for the phone follow-up.
Edit message now opens an editor inside the queued card instead of moving the text into the composer. The card keeps its place and sender, and the editor stays usable while a question or approval holds the composer's slot (it does not lose focus to the prompt card). - Enter or Cmd/Ctrl+Enter saves, Shift+Enter is a newline, Escape cancels; IME composition keys never save or cancel. Save and Cancel replace the card's own actions while it is edited; one editor per pane. - The editor holds the host's edit lease, renewing every 30 seconds and taking it again after a lapse. Lease failures are only logged: they never block typing, Save or Cancel. - If the card is sent or removed elsewhere mid-edit, a clean edit just closes; changed text goes into the chat box after any existing draft, with one toast. A Save that lands and then sends counts as success. - Against a host without agent-session.queued-message-edit.v1 the card offers no Edit (no copy-and-delete fallback); an empty overflow menu is hidden.
…d elsewhere - Save with an unreadable answer (dropped for a moved runtime fence, or lost) now reads the published card: saved text shown -> close; card gone -> the sent-as-saved / chat-box path; otherwise keep the editor quietly. - Send and Steer stay enabled on a card edited on another device; the published editHeld marker is gone (no reader left). Automatic delivery still stops at an edited card on the host. - Save answering changed moves the typing to the chat box and reopens the editor on the newer text. Notices say when the chat box is hidden by a question or approval. - Closing the pane or switching chats with unsaved typing appends it to that chat's box quietly. - Lease renewal no longer publishes, so an open editor is not idle-sweep activity. - The consume compare-and-set also matches the body fingerprint. - Saving unchanged text keeps the original blocks.
Conflicts resolved: - Kept main's queued-message-consume-error.ts, deleted the branch's duplicate, and pointed every importer at main's file (dropping the now-unused re-export). - The queued card now builds on main's full-message expander: the editor replaces the text, the expander and the actions while it is open. - The prompt-card focus guard skips both the find bar and an open queued editor; a closing edit hands focus to an open prompt card first. - Locales take main's `fromSender` plus the branch's edit keys. - max-lines: the queue's deps type moved to journal-store-contracts.ts and the reopen-mark rule to queued-message-settlement.ts; the inline-edit list tests moved to their own file.
…ke the queue for a pruned lease, keep typing across a reload A Save whose answer was lost now closes once the card shows the saved text, and a card that went out with exactly the typed text closes quietly instead of copying it to the chat box. A renew that prunes another editor's lapsed lease wakes the queue in place of the timer it cancels. A reload, which unmounts nothing, hands unsaved typing to the chat box like closing the pane.
…echeck accepts The lease module reaches the mobile test program through journal-store, where setTimeout answers a number, so `timer.unref?.()` failed mobile's tests typecheck ratchet.
…irections The manifest must name every agentSession method; the two edit methods reach their host methods on a new build and are only called by clients on a host advertising agent-session.queued-message-edit.v1.
Review summaryThe problem. On main, "Edit message" on a queued card copied its text into the chat box and deleted the card. While the agent asks a question, the question card replaces the chat box, so the message vanished from the page, as a teammate reported. Even without a question, the card lost its place in the queue and its "From an agent" sender. The user-facing change. Edit now opens an editor inside the card. It works while a question is open, and Save keeps the card's place and sender. The agent doesn't auto-send a card while it's being edited, but Send or Steer still send it at once. Unsaved text is never dropped: it goes to the chat box if the card leaves, changes elsewhere, or the window reloads. Review round 1: approach confirmed, no P1Fixed:
Accepted deviations:
Deferred:
Review round 2: no P1 or P2Fixed:
Found and fixed while preparing QA: a window reload runs no React unmount, so the round-1 "unsaved text goes to the chat box" step never ran on reload. It now also runs when the page is being unloaded. There's a test, and the QA run checks it. Known, narrow, not fixed:
Verified
CI follow-ups after QA:
Not verified
Update ( |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (11)
💤 Files with no reviewable changes (6)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughAdds in-place editing for queued chat messages. Fingerprints guard updates and message consumption. Temporary edit leases affect automatic delivery. The host and RPC layers expose editing operations through a runtime capability. The renderer provides an inline editor with save, cancel, focus handling, and draft handoff for specified outcomes. Tests cover journal updates, lease behavior, RPC calls, and renderer interactions. Priority: ➖ Normal Merge Risk: 🔵 Low · up to A failed draft write could lose an edited message, and current tests would not catch an incomplete update response. These are bounded risks; the inspected production update path returns the required fields. Pre-merge checks |
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/e2e/cross-version-wire/structured-agent-session-host-fixture.ts (1)
61-61: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the complete queued-update success result.
AgentSessionQueuedMessageUpdateResultrequiresmessageIdandfingerprintfor anupdatedresult. The cross-version surface matcher and RPC test currently check only success envelopes, so they can pass with an invalid incomplete outcome.Suggested test fix
diff --git a/tests/e2e/cross-version-wire/structured-agent-session-host-fixture.ts b/tests/e2e/cross-version-wire/structured-agent-session-host-fixture.ts @@ - queuedMessageUpdate: vi.fn(async () => ({ ok: true, replayed: false })), + queuedMessageUpdate: vi.fn(async () => ({ + ok: true, + replayed: false, + value: { + status: 'updated', + messageId: 'queued-1', + fingerprint: 'a'.repeat(64) + } + })), diff --git a/tests/e2e/cross-version-wire/structured-agent-session-surface-calls.ts b/tests/e2e/cross-version-wire/structured-agent-session-surface-calls.ts @@ - result: { ok: true, replayed: false } + result: { + ok: true, + replayed: false, + value: { status: 'updated', messageId: 'queued-1', fingerprint: 'a'.repeat(64) } + } diff --git a/src/main/runtime/rpc/methods/structured-agent-session-queued-edit.test.ts b/src/main/runtime/rpc/methods/structured-agent-session-queued-edit.test.ts @@ - hostCalls.queuedMessageUpdate = vi.fn(async () => ({ ok: true, value: { status: 'updated' } })) + hostCalls.queuedMessageUpdate = vi.fn(async () => ({ + ok: true, + value: { status: 'updated', messageId: 'card', fingerprint: 'a'.repeat(64) } + })) @@ it.each([ @@ ])('%s reaches the host as the authenticated caller', async (method, params, name) => { - expect(await call(method, params, STRUCTURED_CLIENT)).toMatchObject({ ok: true }) + const response = await call(method, params, STRUCTURED_CLIENT) + expect(response).toMatchObject({ ok: true }) + if (name === 'queuedMessageUpdate') { + expect(response).toMatchObject({ + ok: true, + result: { + ok: true, + value: { status: 'updated', messageId: 'card', fingerprint: 'a'.repeat(64) } + } + }) + } expect(hostCalls[name]).toHaveBeenCalledWith({ callerKey: 'trusted-local:runtime' }, params) })
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
04fd4d60-791d-4df1-81b9-1a1d12396cc0
⛔ Files ignored due to path filters (1)
src/shared/rpc-contract/rpc-params-catalog.generated.tsis excluded by!**/*.generated.*
📒 Files selected for processing (59)
src/main/native-chat/agent-session-journal/journal-queued-messages.tssrc/main/native-chat/agent-session-journal/journal-store-contracts.tssrc/main/native-chat/agent-session-journal/journal-store.tssrc/main/native-chat/agent-session-journal/journal-submission-hook.tssrc/main/native-chat/agent-session-journal/queued-message-bookkeeping-failure.test.tssrc/main/native-chat/agent-session-journal/queued-message-edit-leases.test.tssrc/main/native-chat/agent-session-journal/queued-message-edit-leases.tssrc/main/native-chat/agent-session-journal/queued-message-pause.test.tssrc/main/native-chat/agent-session-journal/queued-message-pause.tssrc/main/native-chat/agent-session-journal/queued-message-settlement.tssrc/main/native-chat/agent-session-journal/queued-message-store.test.tssrc/main/native-chat/agent-session-journal/queued-message-table.tssrc/main/native-chat/agent-session-journal/queued-message-text-update.test.tssrc/main/native-chat/agent-session-journal/queued-message-text-update.tssrc/main/native-chat/agent-session-wire/structured-agent-session-host.tssrc/main/native-chat/agent-session-wire/structured-agent-session-queued-edit.test.tssrc/main/native-chat/agent-session-wire/structured-agent-session-queued-edit.tssrc/main/native-chat/agent-session-wire/structured-agent-session-queued-messages.tssrc/main/native-chat/agent-session-wire/structured-agent-session-queued-mutations.tssrc/main/native-chat/agent-session-wire/structured-agent-session-queued-published-bytes.tssrc/main/native-chat/agent-session-wire/structured-agent-session-queued-wiring.tssrc/main/runtime/mobile-rpc-allowlist.test.tssrc/main/runtime/rpc/methods/structured-agent-session-queued-edit.test.tssrc/main/runtime/rpc/methods/structured-agent-session-queued-methods.tssrc/main/runtime/rpc/methods/structured-agent-session-schemas.tssrc/main/runtime/rpc/methods/structured-agent-session.test.tssrc/main/runtime/runtime-rpc/runtime-rpc-mobile-agent-session-methods.tssrc/renderer/src/components/native-chat/NativeChatQueuedMessageCard.tsxsrc/renderer/src/components/native-chat/NativeChatQueuedMessageEditor.test.tsxsrc/renderer/src/components/native-chat/NativeChatQueuedMessageEditor.tsxsrc/renderer/src/components/native-chat/NativeChatQueuedMessageList.inline-edit.test.tsxsrc/renderer/src/components/native-chat/NativeChatQueuedMessageList.test.tsxsrc/renderer/src/components/native-chat/NativeChatQueuedMessageList.tsxsrc/renderer/src/components/native-chat/native-chat-submission-navigation.test.tsxsrc/renderer/src/components/native-chat/queued-message-edit-notices.tssrc/renderer/src/components/native-chat/structured-agent-session-queued-edit-lease.tssrc/renderer/src/components/native-chat/use-native-chat-prompt-card-focus.tssrc/renderer/src/components/native-chat/use-structured-agent-session-queued-edit.test.tsxsrc/renderer/src/components/native-chat/use-structured-agent-session-queued-edit.tssrc/renderer/src/components/native-chat/use-structured-agent-session-queued-messages.clear.test.tsxsrc/renderer/src/components/native-chat/use-structured-agent-session-queued-messages.resume.test.tsxsrc/renderer/src/components/native-chat/use-structured-agent-session-queued-messages.test.tsxsrc/renderer/src/components/native-chat/use-structured-agent-session-queued-messages.tssrc/renderer/src/components/native-chat/use-structured-agent-session.tssrc/renderer/src/components/native-chat/use-structured-native-chat-submit-reveal.test.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/i18n/locales/es.jsonsrc/renderer/src/i18n/locales/fr.jsonsrc/renderer/src/i18n/locales/ja.jsonsrc/renderer/src/i18n/locales/ko.jsonsrc/renderer/src/i18n/locales/zh.jsonsrc/renderer/src/runtime/structured-agent-session-host-capability.tssrc/shared/agent-session-queued-message-wire.tssrc/shared/protocol-version.tssrc/shared/queued-message-text-edit.tssrc/shared/rpc-contract/structured-agent-session-queued-edit-params.tstests/e2e/cross-version-wire/structured-agent-session-host-fixture.tstests/e2e/cross-version-wire/structured-agent-session-surface-calls.tstests/e2e/cross-version-wire/structured-agent-session-surface-manifest.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.
| } else if (status === 'changed') { | ||
| // Someone else's edit landed first: the typing goes to the chat box, and the editor | ||
| // opens again on the newer text. | ||
| appendNativeChatDraftCache(edit.draftKey, edit.text) | ||
| toast.error( | ||
| queuedEditNotice(latest.current.promptOpen ? 'editChangedPrompt' : 'editChanged') | ||
| ) | ||
| reopen.current = { scope, messageId: edit.messageId, stale: edit.baseFingerprint } | ||
| close() |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Return the edited text after a changed answer only when the composer append succeeds.
appendNativeChatDraftCache returns false when the composer-draft write is not durable. The changed branch ignores this return value. It then shows a toast that says "Your edit is in the chat box" and closes the editor. The gone path and handOff ignore the same return value. If storage refuses the write, for example with a quota error, the toast can be false and a reload can lose the typed text. Check the result before you close the editor. If the append fails, keep the editor open or show a different notice.
…r, field and Save footer While a card is edited, its row shows a pencil, "Editing message", the sender and an X that cancels; the field at full width; and a footer with the key hint and Save. The card's queue icon and its status line are hidden until editing ends.
…ave button The editor footer's muted key hint is gone. Save now carries the same inline key badge as the new-workspace composer's Create button, extracted into a shared ButtonKeyHint so both buttons render one badge.
ELI5
When a message is waiting in line for the agent (a "queued card" above the chat box), "Edit message" now edits that card where it is. Before, it pulled the text out of the card and dropped it into the chat box. While the agent was asking a question the chat box is hidden, so the message looked like it had disappeared.
The problem
A teammate reported: clicking Edit message on a queued card while the agent was asking a question made the message "completely disappear on the page".
Why it happened. Messages waiting for the agent show as queued cards above the chat box: mail from another agent, and messages kept across a quit. On main, Edit message was not an edit. It copied the card's text into the chat box and deleted the card. While the agent asks a question or wants an approval, that question card takes the chat box's place. So the text moved somewhere invisible, and the card was gone.
Even with no question open, the message lost its place in the queue. A card from another agent also lost its "From an agent" sender line, because it came back as the person's own draft.
PR #26004 (already merged) added the "Show full message" expander, so long cards can be read. This PR makes Edit a real edit.
What changed for the user
Visual proof
Hidden dev app, driven only through Playwright CDP. A stand-in Codex asks "Which color should I use for the chart?" partway into its turn. The queued card is real agent mail, sent to the chat through the rig's own built
orcaCLI while the agent works.BEFORE (main
40716d702e9)The card waits while the question is open:
After clicking Edit message, the card is gone and its text is nowhere on screen:
AFTER (this branch; the editing shot is from
b507748b53e, the saved shot from8617fdef4d0, the reload shot from08216725f4a)Edit message opens the editor inside the card, and the question stays open below it. The card's status line is hidden while editing. Here the text has been replaced, with Shift+Enter adding a second line, and Save carries its Enter-key badge:
After Save, the card has the new text in the same place, with the same "From an agent" line, and focus is back on the question:
After a reload in the middle of an edit, the card is unchanged and the unsaved text is in the chat box:
How it works
Host (the execution host that owns the queue):
agentSession.queuedMessageUpdate). The client sends the fingerprint of the card's body as it was when the edit began. The host replaces the text only if the card still holds that body. It changes only the text and keeps the card's id, position, sender, kind and attachments. Commands can't be edited.updatedorunchanged(a resent Save whose result is already there), orchanged(newer text landed first, never overwritten), orgone.agentSession.queuedMessageEditHold: acquire, renew, release) hold a card back from automatic delivery while someone edits it.agent-session.queued-message-edit.v1. It is separate from the still-darkqueued-messages.v1, so agent-mail cards and kept cards are editable today.Desktop: the editor lives in the card (
NativeChatQueuedMessageEditor), outside the slot that the chat box and the question card share. The focus guard leaves it alone while a question card is open. One editor per pane.Why this approach
Differences from the common pattern
Compatibility between versions
Testing
Automated, run under Node on
52b0d547bf9(the last commit,08216725f4a, only moves the notice strings into their own file and was re-checked with the editor tests andorca-ci-checks):every
*queued*test insrc/main, plus the RPC method and mobile allowlist tests: 30 files, 327 tests, all pass;every test in
src/renderer/src/components/native-chat/: 360 files, 3639 tests, all pass;tc:nodeandtc:webpass;orca-ci-checks: 23/23 on08216725f4a.Two commits came after QA, from CI:
922d21007ccchanges only how the lease timer is unref'd, so the phone's test typecheck accepts it. I re-ran the lease and host edit tests,tc:node, and the mobile typecheck: the lease file no longer errors.1b53321d4d2adds the two new methods to the cross-version wire manifest, so both skew directions call them.tests/e2e/cross-version-wire/passes locally: 34 files, 180 tests.The editor's layout (header with the X, Save footer, status line hidden while editing) changed in
8617fdef4d0, after review. On that commit: every test insrc/renderer/src/components/native-chat/andsrc/renderer/src/i18n/passes (382 files, 3878 tests),tc:webpasses, andorca-ci-checks(lint, design system, localization) passes 22/22 with the typecheck skipped becausetc:webhad just run. Its 4 new tests (the X cancels, Save is the only footer action and is disabled while saving or unchanged, the status line hides while editing and comes back, the key hint names the platform chord) fail on the old layout; the key-hint test was later replaced, see the next item. The saved shot above was taken on it with the same hidden rig.The footer's key-hint text ("Enter to save · ⇧ Enter for a new line") was replaced in
b507748b53eby a key badge on the Save button itself. The badge is the new-workspace dialog's existing one, moved into a sharedButtonKeyHintcomponent that both buttons now use; the dialog's button looks the same. The badge is hidden from screen readers, so Save is still announced as "Save". On that commit: the editor, queued-list, queued-edit and new-workspace dialog tests plussrc/renderer/src/i18n/pass (34 files, 382 tests),tc:webpasses, andorca-ci-checkspasses 22/22 (typecheck skipped aftertc:web). The editing screenshot above was re-taken on it with the same hidden rig, and the dialog's "Create worktree ⌘↵" button was checked in the same run.Two of the tests fail on the previous code:
Manual QA, with the same hidden rig on both commits, a stand-in Codex that logs every write, and an isolated home and profile:
The real
~/.claude/settings.jsonand~/.codex/config.tomlhashes were identical before and after the QA.Not verified:
Linked Issue
None. A teammate reported this in chat (maintainer PR).
Visual Proof
See "Visual proof" above.
Review
Two review rounds; a summary comment follows on this PR.
Agent skill upstream boundary
Notes
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)