Repository navigation
feat: edit queued messages inline with the full chat composer - #31180
Conversation
…n its card write fails Every reader of a queued card goes through one derived view of the journal's dispatch facts, so a card whose message was refused reads as returned even when its card-table update and heal both failed. One decision and one writer store the derived state for every action.
…, not a second table A queued message lived in its own SQL table, and a hook that may be skipped copied each refusal from the journal into it. When that write failed, a refused card vanished: the cards behind it sent, and its Send and Delete did nothing. Each queued message is now a `queue` row in the chat's journal beside the submission that hands it off and the dispatch row that answers it. The reducer folds the card from those rows, so a refusal returns the card and holds the cards behind it with nothing written, and replay reads the same. Every action writes its own rows in one transaction. Cards an earlier build stored are read through the same classification until they move: each action moves its own card, and a bulk import runs once per open. A new epoch restates every card still owed an answer in the transaction that deletes the old rows. The settlement hook and its savepoint, the open-time heal, the drain's heal and the derived-card view are gone. The published card list is unchanged.
Cards are written through the journal, so tests queue them with their body alone and hand them off under their real fingerprint. Tests of the removed settlement hook, heal and SQL store are deleted; the text-update spec now covers the edit decision alone, and a test of an earlier build's stored holds seeds the legacy table and reads the imported cards. Rolling over an epoch now keeps a card whose hand-off was never handed over, so that test checks the retried id never sends rather than that nothing sends.
…ew epoch and a restart One whole-flow test through the host: the refusal writes no card row, a rewind that retires the refused hand-off and a rolled-over epoch keep the card returned, a restart and Resume send nothing behind it, its own Send refused again still holds the queue, and its Delete releases the next card in order.
…nal once, each read the same One mixed-import test through the host. The first open's import fails and moves nothing, yet every card reads as it will; Delete and Send each move only their own card. The next open moves the rest in one transaction, writing a refusal only where its hand-off is gone, nothing for answered cards, and leaving an unreadable row on disk. A later command turn spends a moved command card, a third open writes nothing, an old Delete replays from its receipt, and a chat with only answered legacy rows clears them in a delete-only transaction.
The row-kind table lists a queue row, the clear and legacy-import tests match the current signatures, and the downgrade test keeps only the leftover-send case: a queued card is now a newer-version journal row an older build keeps read-only, and users don't downgrade.
…on hook No caller passes one since the queue's savepoint bookkeeping left.
…uilds read The highest version this build reads is now 4, the queue row's. A newer build's row in these probes is written at version 3 so the older build meets its kind, not a newer version, and the Stop-event probe hands the older build the rows as stored, since a read raises each row's version.
The legacy-import test opens real SQLite, so it runs in the Node runtime project. The newest version this build reads is now 4, and a read raises a row's version to it, so the turn-lifecycle test no longer pins the in-memory version (the written one stays pinned by journalRowSchemaVersion).
…s both The per-row delivery check returned only the first waiting card an echo proved delivered, so when one settlement carried echoes for two cards the second stayed waiting and was sent again. It now returns every card the row proves delivered, and the fold withdraws each; replay folds the same.
…urnal first While the open's import had failed, an earlier build's card that read as refused through its hand-off lost that reading when an in-place rewind retired the hand-off: it disappeared and the card behind it sent. The rewind now adopts every legacy card ahead of its tombstones and deletes their rows in its own transaction, as an epoch replacement already does, so the fold latches each card's reading before its hand-off is gone.
…l unconfirmed The import dropped a legacy card handed off as a send whose delivery was unknown, though the fold keeps such a card linked and a later refusal returns it. The import now keeps every card whose hand-off a later row can still answer, through the same check the reducer folds dispatch rows by.
…ecycle-queue-derived-cards
One provider echo proves one delivery, yet it withdrew every waiting card with the same text whose hand-off was refused, so a second identical card was lost. The fold now pairs each echo with one card: on its own row the earliest-positioned match no other echo of that row took, and for a card that waits again the earliest applied echo no card has taken. Paired echoes live in the fold's state, so live and replay pair the same; readers of cards not yet imported work on a copy.
…only on proof it never got it Pressing Stop after Orca handed a queued or steered message to the agent, before the agent confirmed it, put the card back in the queue, and Resume or the next send sent it again. A check matching the agent's echo of the text was the only guard against a double send, and it kept trading a resend for a lost card. Whether the card waited again was decided by the rejection's category, not by proof the agent never got the message. A refused hand-off now waits again only where Orca proves the agent never received it: the host still held it, or whoever settled it recorded that. A failure still returns the card and holds the cards behind it. Anything else may have reached the agent, so the card is spent and its message reads as sent, derived on every fold from the stored rejection: with the turn it was answered into, as that turn's unconfirmed message where it was handed over; withdrawn before any turn opened, as a person's own send a Stop took back. The reopen mark ignores a recovered doubt, and the restart reconciler no longer judges a queued hand-off unsent from what history lacks. The echo text matching and its pairing are deleted. A close or quit while the agent starts settles sends with the unrun hold. The host hands nothing to a child before its start answers (structured-agent-session-delivery-loop.ts: a handover needs phase ready), so such a send went to an earlier child and may have run: it now reads as sent instead of waiting again.
…run lets the card wait again Claude says when it removed a send without running it: the interrupt receipt's cancelled list, or a command_lifecycle cancelled, discarded or refused frame for a command that never started. Every one settles through rejectClaudeDispatchWaiters, which now marks the settlement unrun. For a queued card's hand-off the host records that as proof the agent never got it, so the card waits again under the queue's pause. A direct send keeps the plain refusal: there the proof would owe it as stopped history.
…lows the same rule on upgrade Earlier builds put a card back to waiting after any refusal without a verdict and cleared its link, so a card the agent may already have would be sent again after the upgrade. The card's own hand-offs in the journal decide now: unchanged and last sent as one that may have reached the agent, it is spent on that hand-off and its message reads as sent; edited or sent again since, it is held for its own Send under a host-only marker that publishes as a plain hold; with no hand-off in doubt, including one that never left Orca, it waits as before. The import writes a card spent on a hand-off in doubt, so its message shows.
… uses it The lookup's callback lost the narrowing of the row's change, which the mobile test typecheck rejects.
…in the Stop tests Three tests pinned the old rule, that a card handed into a running turn comes back to waiting when a Stop ends it. Under the new rule nothing proves the agent never got it, so the card is spent: with the turn named, its message reads as that turn's unconfirmed one; without, as a send the Stop took back. Neither is ever sent again.
…ecycle-queue-derived-cards Conflicts, resolved so main's change stands and queued messages stay in the chat's journal, sent again only on proof the agent never got them: - #28046 (choices fixed per accepted message): its options type and its one reader reach the journal's queue row and the card fold too; the queued_messages writers it retyped stay deleted. - #30403 (Codex late receipt): the late settlement keeps both main's accept-at-its-turn path and this branch's unrun proof for a card. - A drain logging test main deleted stays deleted.
…he agent waits again A Stop that aborts a hand-off after its row is written but before the agent call now records the withdrawal as never reaching the agent, for queued hand-offs only, so plan D's rule returns the card to waiting under the Stop's pause instead of spending it. Direct sends keep the plain withdrawal (the flag would owe them as stopped history). The agent's own refusal while aborted is kept rather than overwritten, and an abort thrown during the call stays in doubt.
…t waits again The abort signal reaches only an adapter's preparation (its options, the previous turn opening), never its write, so the signal's own reason thrown back is the adapter saying nothing was written. The handover now reads that, for queued hand-offs only, as never reaching the agent, so the card waits again under the Stop's pause. Claude no longer wraps that abort as an option refusal, which had returned a stopped card as a refused one blocking the queue. A real refusal keeps its cause and its barrier; any other throw stays in doubt and is never resent.
…ecycle-queue-derived-cards
…ard through the journal queue Main's #30396 wrote its backlog card with the stored fingerprint and host-instance stamp the journal queue derives instead; the card the Continue must yield to is the same waiting card.
…s cancelled-unrun receipt carries
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough
Priority: Merge Risk:
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟠 Major · Prevent fallback Edit from deleting image cards. · use-mobile-structured-queued-message-controls.ts:220-224
mobile/src/session/use-mobile-structured-queued-message-controls.ts:220-224
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPrevent fallback Edit from deleting image cards.
When
editCapableis false, this path copies onlycard.textand then deletes the card. The fallback does not preservecard.images. Image-card projection depends on the client image-card capability, not onqueuedMessageEdit, so an image card can reach this fallback when queued-message editing is unavailable.🐛 Suggested fix
- if (!card || card.command || !appendComposerText?.(card.text)) { + // Copying cannot carry the card's images; deleting it would lose them. + if (!card || card.command || card.images || !appendComposerText?.(card.text)) { return false }
🧹 Nitpick comments: 1
mobile/src/session/MobileNativeChatOverlay.tsx: 1
85-94: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winMemoize the
onEditwrapper.The controller can emit streamed-part updates at most every 50 ms. Each update can re-render the overlay while the queued edit behavior remains unchanged. The inline wrapper then changes
editQueuedMessage, invalidates the composer tray memo, and creates a newMobileNativeChatQueuedMessageselement.
MobileNativeChatViewrenders the tray directly, andMobileNativeChatQueuedMessagesis not memoized. This adds avoidable queued-message reconciliation during streaming. Use a stable callback and preserve the current image-edit handler.Suggested fix
-import { useMemo } from 'react' +import { useCallback, useMemo } from 'react' ... const queued = controller.nativeChatQueued const editQueued = queued.edit + const forQueuedEdit = images.forQueuedEdit + const onEditQueued = useCallback( + (messageId: string, onCopied?: () => void) => + editQueued ? editQueued(messageId, onCopied, forQueuedEdit) : Promise.resolve(false), + [editQueued, forQueuedEdit] + ) const composerTray = useMobileNativeChatComposerTray({ ... - onEdit: editQueued - ? (messageId, onCopied) => editQueued(messageId, onCopied, images.forQueuedEdit) - : undefined, + onEdit: editQueued ? onEditQueued : undefined,
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
aa17e469-b31e-4e4c-b614-3388310bfd1e
📒 Files selected for processing: 17
mobile/src/session/MobileNativeChatComposerAttachments.tsxmobile/src/session/MobileNativeChatOverlay.tsxmobile/src/session/MobileNativeChatQueuedMessageEditor.tsxmobile/src/session/MobileNativeChatQueuedMessages.test.tsxmobile/src/session/MobileNativeChatQueuedMessages.tsxmobile/src/session/MobileQueuedMessageCardRow.tsxmobile/src/session/mobile-native-chat-queued-message-styles.tsmobile/src/session/mobile-queued-edit-images.tsmobile/src/session/mobile-structured-queued-message-cards.tsmobile/src/session/use-mobile-native-chat-image-attachments.tsmobile/src/session/use-mobile-native-chat-image-upload.tsmobile/src/session/use-mobile-session-image-attachments.tsmobile/src/session/use-mobile-structured-agent-session.tsmobile/src/session/use-mobile-structured-queued-edit.test.tsxmobile/src/session/use-mobile-structured-queued-edit.tsmobile/src/session/use-mobile-structured-queued-message-controls.tsmobile/src/transport/mobile-runtime-client-capabilities.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.
The inline editor is the chat composer, so it keeps the composer's own identity model and no longer refuses Save for text that looks like a command. Command rows picked from the menu are still refused, Enter still means Save, and Edit never sends or runs anything. Regression tests cover Undo after deleting an accepted command or skill pill, a scanned skill named like a command, two saved files behind one token, and typed command text saving literally. Also fixes two merge leftovers in tests: a missing import and a fixture that stored the whole menu row as a skill.
… under max-lines (#34929) * fix(i18n): drop queued-message keys #31180 removed from en #31180 removed components.native-chat.queuedMessages.cancelEditing, .pasteOrDropImages and .saveNeedsContent from en.json; tr.json, added since, still carried them, so the locale catalog check failed on main. They are the only stray keys in any locale. * fix(lint): split runtime-file-read-client under the max-lines cap Move the remote chunked-download helpers into runtime-remote-file-download.ts so runtime-file-read-client.ts is back under the 300-line lint cap. No behaviour change.
Queued-message Edit used a smaller input than the normal chat box. This change opens the real chat composer inside the queue row on desktop and phone, with images, skills, @files, paste, dictation and Save/Cancel. The bottom draft stays separate and the message keeps its position. Editing an agent message sends the changed message as the user.
The inline editor reuses the normal input component and its existing content model. Enter saves. Picking a command/control menu row is refused in Edit; text typed with the menu closed saves literally, including command-looking text. Edit cannot send a message, execute a command, set a goal, resume a session or write to a terminal. Ordinary bottom-composer sends retain main's behavior.
Validation keeps the editor open. If the target disappears, changes, or the editor closes involuntarily, unsaved input returns once after the original conversation's existing draft, including captured late uploads and pasted-text mappings, with one toast. Cancel discards local input and revokes its late completions; it cannot undo a host update already dispatched.
Compatibility and intended outcomes
The optional skills field is sent only to hosts advertising
agent-session.queued-message-skills-edit.v1. Present values replace the set, including[]; omission preserves legacy behavior. Old hosts receive no new key. The host changes the sender only as part of a durable changed-body decision. Unchanged saves close without an update. No draft-key migration, new persisted format or minimum paired-server version is added. Reordering landed separately in #34026 and is integrated here. Existing phone image-delivery limitations remain tracked in STA-12378.Typed command-looking text saving literally is intended and matches the common pattern. Skill identity uses the composer's existing content: desktop pills each hold one file path; phone uses its picked-file list. For a saved token referring to two same-name files, unchanged Save retains both; changed Save keeps the desktop pill's file or the phone pick list. This platform-specific result is intended. Stock Undo restores exactly the normal composer's content, with no additional identity/history layer. OpenCode and OpenCode2 scanned-skill picks that carry no file path save their token without a skill-file ref, exactly as the normal composer represents them. This is intended and matches the common pattern.
Unsaved inline edits are held in memory: a hard client crash loses the unsaved edit while the durable queue card survives. An unknown update followed by definite target loss can conservatively return input the host applied without confirmation; nothing automatically resends.
Verification
The review verifies the shared input/action boundary, ordinary Send parity, full handback, host CAS/sender decisions, durable journal reopen and mixed-version omission. A reproduced rich-handback acceptance defect was fixed and checked through the actual next Send, remount, persistence and IME paths. Latest targeted validation: 67 unique tests across five named files for that repair; the preceding ordinary-Send/fixture/performance repair has 115 unique passing tests across 11 named files. These are assembled final affected-file runs, not one combined execution. Normal hooks passed.
The performance review accepts zero catalog reads for ordinary prose at 10/100 reported commands and mounted-consumer isolation with separate upstream counts. No timing, heap or provider-result claim is made. All changed flows are the review table's tests rows; existing branch-only desktop and shipping phone screenshots carry for unchanged appearance.
Current main, including upstream correction #34721, is merged at
a0703891952. Headd262af58607additionally removes six obsolete strings from the two newly added locale catalogs; all other parsed translations are unchanged. Both named localization suites pass (16 tests), and the normal hook passed. The final independent source/reference comparison accepts this exact head, with all differences labelled and no new finding.All actual CI analysis, typecheck, test, build, mobile, persistence, WSL and packaging jobs passed. The separate CodeQL summary check remains failed on alert #177, independently traced as a false positive in the isolated synthetic happy-dom replay test. The fixture is byte-identical to the previously reviewed version. No scanner-driven source rewrite, suppression or alert dismissal was made. CI is therefore not fully green; the completion marker and READY claim remain withheld.
Visual Proof
Desktop inline composer and separate bottom draft (branch
c44799c6):Desktop image-only editor with its Edit-message placeholder (branch
81f22e85):Shipping phone editor and bounded skill picker (branch host/page
b81ed9a1):Independent source comparison retains these images only for unchanged appearance. Phone proof used the shipping WebView route and matching branch page, with unchanged native shell inputs from
8fd1749b. Branch-only screenshots used no agents; no provider/lifecycle run is claimed.Agent skill upstream boundary