Skip to content

feat(mobile): mid-turn messages wait as cards above the phone composer - #23736

Merged
brennanb2025 merged 225 commits into
mainfrom
mid-turn-queue-mobile
Sep 30, 2026
Merged

brennanb2025 merged 225 commits into
mainfrom
mid-turn-queue-mobile

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 22 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​2640 $\color{#cf222e}{\Huge{\mathbf{−}}}$​247 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​2393
Prod 29 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1773 $\color{#cf222e}{\Huge{\mathbf{−}}}$​304 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1469

Where this fits: the plan for messages sent while the agent is working

The problem. In a structured (native) chat, a message you sent while the agent was still working went straight to the agent. What happened next depended on the agent:

  • Claude folds it into the work it is already doing, at its next step.
  • Other agents start a new turn or refuse it.

Orca then drew the conversation wrong:

  • The "Working for" bar jumped to the newest message.
  • A single piece of work showed up as two turns (for example "Worked for 17s" and "Worked for 52s"), with the time counted twice.
  • On the phone, the running turn's rows could disappear.

The plan. A message sent mid-turn waits as a card above the message box, with three actions:

  • Steer: send it now, into the running turn.
  • Edit.
  • Delete.

Otherwise it sends by itself when the turn ends. Only Steer puts a message into a running turn. With queueing turned off in Settings, mid-turn messages still go straight in, as they do today.

The work is split into six PRs:

# PR What it does Why the queue needs it
1 #23573 (merged) The turn's bar stays on the message that opened the turn. A steered message still lands mid-turn, and so does every mid-turn message with queueing off.
2 #23671 (merged) Chat rows are grouped by the turn that produced them, not by the nearest message above them. A steered message sits among its turn's rows. A turn the agent opens by itself gets its own bar.
3 #23621 (merged) Claude: a message folded into the running turn is recorded as joining that turn, not as a new turn. Steer on Claude is exactly this fold. Without it, every Steer splits the turn in two; desktop QA of the queue showed that split.
4 #23726 (merged) The queue itself, held by the computer running the agent. The core.
5 #23731 Desktop cards. The desktop interface.
6 #23736 Phone cards. The phone interface.

Steer on Codex also needs Codex's turn-steer support from #21062, which is still open.

The queue stays switched off, meaning the host does not advertise it and every client behaves as today, until PRs 1–6 and #21062 have landed. Turning it on is then a one-line change.

This PR is step 6: the phone cards for the queue in #23726.

ELI5

On the phone, a message you send while the agent is still working now waits as a card above the message box, and it looks like the desktop (#23731): one box above the message box, with one compact row per message. You can Steer it (send it now), delete it with the trash icon, or edit it from the "…" menu. Otherwise it sends by itself when the turn ends. The phone and the desktop show the same queue, because the queue lives on the computer running the agent (#23726).

What Changed

Before: sending from the phone mid-turn pushed the message straight into the running turn.

After, against a host that queues messages:

  • A mid-turn send becomes a row in the queue box, laid out like the desktop. The queue is one bordered box above the composer (not in the transcript), with thin dividers between rows. Each message is one compact row: a queue icon, the text (up to two lines, since the phone has no hover to show the rest), and three actions:
    • ↳ Steer sends it now into the running turn (accessibility label "Submit without interrupting the model"). A card that came back unsent, or whose own send failed, shows Send instead, with a red alert icon.
    • Trash deletes it.
    • "…" opens the app's action sheet with Edit message, which moves the text into the composer, focuses it, and removes the card.
    • A plain waiting message has no extra label. A caption line appears only when there is something to explain (a refused message's reason, "Waiting for your answer", "Waiting — a message ahead needs attention").
  • A paused queue shows its reason as the box's first row, with a ▶ Resume button ("Queue paused because you interrupted" / "…because Orca restarted" / "…after you cleared the conversation"), announced to screen readers. The cards keep Steer. The row shows only while Resume would send something.
  • Touch targets. Every action is a 44 pt touch target, kept inside its row so Android doesn't drop touches.
  • Differences from the desktop, on purpose. Two lines of text instead of one (one line cut every message to about 20 characters on a phone). The "…" menu has no "Turn off queueing", because the phone has no queueing setting.
  • The card list is the host's. It is the published queuedMessages list, which the phone only renders. The actions are the host's queued-message RPCs.
  • A card hides only when the host says it was sent. The host now names the draft on each submission it makes from one (queuedMessageId), with a fresh id on every hand-off. The phone hides a waiting card only when a submission that names it exists and was not rejected, and never compares a draft id with a submission id. A draft Stop sent back stays visible, and a card and its bubble never show together.
  • Returned cards say why. A card the host sent back is worded from its typed rejection, the same words the desktop uses; a card Stop took back reads "Stopped before it was sent". The reason wraps instead of being cut off.
  • Edit never loses text. Edit copies the text into the composer first and deletes the card only if the copy happened. If the draft had already gone out, the phone says "Already sent — your text is still in the composer."
  • Queue delivery is opt-in per host. The phone asks for it only when the host advertises the capability, and messages with images always send right away.
  • Sends whose answer was lost. Delivery-unknown sends keep today's rule: the retry replays with the same id, so a message whose answer was lost is never sent twice. A saved send record is cleared as soon as the host's answer or the live stream names its hand-off, and a failure to update that record never blocks sending the same text again.
    • One addition: if an older host's schema rejects the replay's queue field, that id is retired, because that host can never accept that request. Otherwise every later send of the same text would be refused.
    • An auth refusal does not retire the id, because it says nothing about whether the first attempt arrived.
  • The phone's saved send list keeps its format, so an older build of the app still reads it.

Why

The queue is host-owned so the phone, the desktop and a reconnecting client always agree, and so the queue survives app restarts on either side. The phone keeps no queue of its own, only presentation. Earlier versions of this branch pulled draft text back to the phone on Stop and /clear. That needed a restore journal, re-asking after a lost answer, and relaunch rules, and it kept producing bugs. With the host holding paused drafts, all of that is gone.

Linked Issue

None; follows the mid-turn send problems found during QA of #23537 and #23573.

Visual Proof

iOS Simulator paired to a dev host with queueing switched on in a local build only.

The phone's queue now matches the desktop's layout (redesigned after review: the earlier version drew each message as a large separate card).

Two messages queued while a turn runs Stop pauses the queue; Resume in the box's first row Resume drains one per turn
"…" opens Edit message After Edit: text in the composer, row gone After the trash icon

The desktop for comparison (#23731):

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

Latest live QA (the redesigned layout). iOS Simulator paired to a dev host built from this branch at 5234bfa6054, with a real Claude: 4/4 scenarios pass (two messages queued mid-turn; Stop then Resume draining one per turn; "…" then Edit message, never sent; trash, never sent). A message the host sends back is covered by tests only, since staging a refused hand-off live is not practical. Not checked live: whether the software keyboard rises after Edit (the simulator uses a hardware keyboard).

Earlier live QA (queue-level pause, old card look). Live phone QA in the iOS Simulator paired to a dev host at 7ae9f138a97, phone bundle at 38f06968472: 5/5 scenarios pass (paused row with Resume; Resume drains one per turn; Steer while paused; /clear; empty-queue Stop leaves no pause), cross-checked against the desktop.

Live phone QA, 7/7 scenarios passing (at an earlier head of this stack, before the host's draft link and the review fixes; the card flows are the same), driven with orca emulator and cross-checked against the paired desktop at every step:

  • queue and drain;
  • Delete;
  • Edit;
  • Send now into the running turn;
  • Stop pausing two cards, then a new message sending right away and the paused cards draining one per turn;
  • /clear keeping a paused card;
  • the desktop showing the same cards.

Provenance. The bundle served to the simulator contained the queued-card component, and the host showed the "Queue follow-ups" switch, which appears only when the capability is on.

Automated tests cover:

  • card rendering and actions, including the 44 pt targets and the Steer label;
  • focusing the composer after Edit;
  • send delivery rules: a lost answer keeps its id, a schema rejection retires it, an auth refusal keeps it;
  • the draft link: cards hide only through it, a Stop-requeued draft stays visible, and a replay answered by the hand-off spends its record;
  • storage failures: a resend never sends twice, even if storage recovers before the retry;
  • relaunch replay;
  • the old-host path.

All 1,915 phone session tests pass. Removing any single fix fails its own test. Mobile typecheck, the tests typecheck ratchet, and the root, mobile and React Doctor lint configs pass.

AI Disclosure

Review

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Deleted: restart-before-admission, recordFailedRestart, the fence rebase,
Claude's startup gate, the attach's forget on failure and its own crash
boundary. Clients without agent-session.accepted-send.v1 get their reply held
until the handover; the desktop and paired desktop lists advertise it.
A child that proved its start and then exited before its message was handed
over left the message queued: the exit settlement returned early when nothing
else was in flight. Delivery then started another child for it, and a child
that died the same way started another, without end and without a row.

A retried settlement for an earlier generation, run by the attach that
delivery started, did the opposite: with that generation's turn unfinished it
rejected the message queued for the child being attached.

The settlement now takes the rejection for queued messages from its caller.
The unexpected exit and the eviction pass one, and it applies even with no
other work in flight; the retry for an earlier generation passes none.
#23059 makes /compact a message the chat sends and reads turn facts from the
turn record. The queue follows it:

- /compact is now a queued message and then a turn, not a command holding the
  session's lane for its whole provider call. A capable send during it becomes
  a card held by the ordinary `working` hold, and the drain waits behind the
  compaction's turn like any turn. So the compact side lane
  (structured-conversation-command-lane.ts), draft-only admission, Send-now's
  up-front refusal and the queue gate's `command` hold are gone. Delete, Resume
  and Send-now run on the session's lane, which a compaction no longer holds,
  so they answer at once; Send-now's card is accepted and handed over after the
  compaction, the order every send keeps. A /compact from the command RPC is
  the person's own turn (origin 'client').
- The queue gate's `blocked` hold is now main's send block itself, so a clear in
  doubt, a superseded source and an uncertain rewind hold the queue by one rule.
- Submission folding moved to journal-submission-fold.ts on main; the draft
  link, origin, the latest person's turn and the shared settlement guard are
  carried there. The /clear carry is re-applied to main's clear flow.
- To stay within max-lines, the row writer's cursor-returning append replaces
  the store's appendRow, the open-time draft repair rides the restore, and the
  queue wiring reads the host's mutation context.
With turn facts read from the turn record, a draft Steer hands over while a
turn runs is scoped by the host to that turn, under a fresh submission id
that names the card. On the phone it joins that turn's group, gets no bar of
its own, and stays live with it.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes

This run reviewed the delta since the prior pullfrog review (38f0696): the base branch's native-chat turn-membership refactor (turn facts now read from the turn record, NativeChatTurnJournal replacing the two scalar controller fields) merged in, plus one test pinning the queue's Steer flow onto that model.

  • Controller contract follows the new turn journal — MobileNativeChatController now exposes nativeChatTurnJournal: NativeChatTurnJournal | null in place of nativeChatActiveTurnOpenedBy and nativeChatTurnKeysByItemId; use-mobile-native-chat-controller.ts and MobileNativeChatOverlay.tsx thread the new field through, and no stale references to the removed fields remain.
  • Steer joins the running turn — use-mobile-native-chat-turn-disclosure.test.tsx adds a case where a draft handed over mid-turn (scoped to the running turn, under a fresh submission id) gets no bar of its own and stays live with that turn.

Both prior pullfrog threads remain resolved (the withdrawn-replay silent drop fixed in 1111b0d; the shape-rejection rollback concern withdrawn as a documented trade-off).

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

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes

This run reviewed the delta since the prior pullfrog review (368f4d2): a single merge of origin/main into the branch. The merge is clean and touches no file under mobile/, so the PR's authored phone-queue surface is byte-identical to the previously reviewed revision.

  • Merge-up of origin/main — e493ea6943 merges main's #23923 ("bring constants.ts back under the max-lines limit"), which moved the onboarding, notification, and terminal-platform defaults out of src/shared/constants.ts into new modules and updated their call sites. None of the moved symbols is imported by the mobile session code, so the diff growth (48 → 79 files) is entirely main's refactor arriving through the merge, not new work on this branch.

The prior review stands. Both earlier pullfrog threads remain resolved.

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

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Status: ready to merge after #23726. Head e493ea69431, CI all green (the phone check suite), no conflicts.

# Conflicts:
#	mobile/src/session/mobile-structured-send-delivery.test.ts
#	mobile/src/session/mobile-structured-send-delivery.ts
@brennanb2025
brennanb2025 changed the base branch from mid-turn-queue-host to main September 29, 2026 22:24
@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

Next included review available in 12 seconds.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: c1c5c2a0-f682-4cf0-8446-45fbb7831db9

📥 Commits

Reviewing files that changed from the base of the PR and between 40cdcb6 and c2d7871.

📒 Files selected for processing (51)
  • mobile/src/session/MobileNativeChatComposer.test.ts
  • mobile/src/session/MobileNativeChatComposer.tsx
  • mobile/src/session/MobileNativeChatOverlay.test.ts
  • mobile/src/session/MobileNativeChatOverlay.tsx
  • mobile/src/session/MobileNativeChatQueuedMessages.test.tsx
  • mobile/src/session/MobileNativeChatQueuedMessages.tsx
  • mobile/src/session/MobileNativeChatView.test.ts
  • mobile/src/session/MobileNativeChatView.tsx
  • mobile/src/session/MobileNativeChatView.turn-status.test.ts
  • mobile/src/session/MobileNativeChatView.waiting-rows.test.ts
  • mobile/src/session/mobile-native-chat-controller-contract.ts
  • mobile/src/session/mobile-native-chat-draft-reconcile.ts
  • mobile/src/session/mobile-native-chat-pending-echo.ts
  • mobile/src/session/mobile-native-chat-send.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-agent-session-send-bypass.test.ts
  • mobile/src/session/mobile-structured-agent-session-send.ts
  • mobile/src/session/mobile-structured-composer-command.test.ts
  • mobile/src/session/mobile-structured-queued-message-cards.test.ts
  • mobile/src/session/mobile-structured-queued-message-cards.ts
  • mobile/src/session/mobile-structured-queued-message-feed.test.ts
  • mobile/src/session/mobile-structured-queued-message-feed.ts
  • mobile/src/session/mobile-structured-send-delivery.test.ts
  • mobile/src/session/mobile-structured-send-delivery.ts
  • mobile/src/session/mobile-structured-send-operation-journal.test.ts
  • mobile/src/session/mobile-structured-send-operation-journal.ts
  • mobile/src/session/use-mobile-native-chat-controller.test.ts
  • mobile/src/session/use-mobile-native-chat-controller.ts
  • mobile/src/session/use-mobile-native-chat-drafts-queued-hold.test.ts
  • mobile/src/session/use-mobile-native-chat-drafts.ts
  • mobile/src/session/use-mobile-native-chat-queued-edit-focus.test.tsx
  • mobile/src/session/use-mobile-native-chat-queued-edit-focus.ts
  • mobile/src/session/use-mobile-native-chat-queued-slot.test.tsx
  • mobile/src/session/use-mobile-native-chat-queued-slot.ts
  • mobile/src/session/use-mobile-native-chat-session-lane.ts
  • mobile/src/session/use-mobile-native-chat-turn-disclosure.test.tsx
  • mobile/src/session/use-mobile-native-chat-unconfirmed-sends.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.test.tsx
  • mobile/src/session/use-mobile-structured-agent-session.ts
  • mobile/src/session/use-mobile-structured-agent-state.ts
  • mobile/src/session/use-mobile-structured-native-chat-send-bridge.test.ts
  • mobile/src/session/use-mobile-structured-native-chat-send-bridge.ts
  • mobile/src/session/use-mobile-structured-queued-message-controls.ts
  • mobile/src/session/use-mobile-structured-send-operation-reconciliation.ts
  • mobile/src/session/use-mobile-structured-send-with-outcome.ts

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: ce40cb9e-44c6-4d1b-ae4f-d6c6b3117c44

📥 Commits

Reviewing files that changed from the base of the PR and between f161f7e and 40cdcb6.

📒 Files selected for processing (3)
  • mobile/src/session/MobileNativeChatView.test.ts
  • mobile/src/session/MobileNativeChatView.turn-status.test.ts
  • mobile/src/session/MobileNativeChatView.waiting-rows.test.ts

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


📝 Walkthrough

Walkthrough

The mobile structured chat now supports host-held queued drafts. It tracks queued messages and pause state, converts them into cards, and exposes send, delete, edit, and resume actions. Queued sends have a distinct outcome and operation-journal handling. Draft reconciliation matches sends against newly published queued cards. Editing a queued card can copy its text to the composer and focus the input.

Priority: ⬇️ Low

Merge Risk: 🔵 Low · up to 40cdc

The queued-message flow has one bounded UI issue: an open menu can unexpectedly reopen after its last card disappears and returns. Mergeable with owner awareness and follow-up for that reset.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 40cdc

Queued messages remain mediated by the existing session delivery boundary. The inspected paths preserve session identity and guard against repeated delivery and conflicting queue actions. No introduced security finding was established, but recovery and authorization coverage is incomplete.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated exposure is message execution within the targeted host session, including the agent work that follows delivery. No cross-session expansion was established. The agent's downstream tool privileges and full environment exposure were not inspected, so maximum independently attackable scope remains unquantified.

Trust Boundaries and Controls

  • observed — User-controlled message bodies and queue action IDs cross the existing host mutation boundary with explicit session identity, operation identity, and expected runtime fence. Explicit send checks blocked and pending-prompt conditions. These are observed controls, not verification of the complete authorization chain.
  • observed — Session feeds are keyed by session identity, and mutations target their captured session. Connection status intentionally gates the live hold rather than all mutations; cached queue actions therefore still rely on transport results and host fence enforcement.

Resilience and Maintainability Implications

  • observed — Journal submission installs a queued-message consume hook. That hook requires the same database handle and throws when the expected-state consume fails. Automatic draining handles lost races and holds a draft after a pre-consume failure rather than repeatedly dispatching it. The underlying transaction writer was not independently verified.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 27.66% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 94 functions across 51 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: mobile mid-turn messages wait as cards above the composer.
Description check ✅ Passed The description is detailed and covers the user impact, implementation, rationale, visual proof, testing, compatibility, and scope. The Linked Issue section says “None” despite the template requiring …
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.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR

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.

@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

This run reviewed the delta since the prior pullfrog review (e493ea6): the PR was retargeted from mid-turn-queue-host onto main (now that #23726 has landed there), and one merge of origin/main came in — with no file under mobile/ changing.

  • Base retarget — the PR now diffs only its authored phone surface (48 files, all mobile/src/session/) against main, rather than also carrying the host stack from mid-turn-queue-host.
  • Merge-up of origin/main — a1643484e3 merges #23524 (an unfinished /clear or refused Codex rewind no longer locks the chat), #23812 and #23852 (browser viewport fixes), plus the already-matched #23726 host queue. git diff e493ea6 a164348 -- mobile/ is empty, so the card surface is byte-identical to the previously reviewed revision.
  • No contract break from the merge — the only shared modules mobile imports that changed (agent-session-refusal-notice.ts, agent-session-refusal-details.ts) are comment-only, agent-session-rewind.ts drops a providerApplied field the phone never references, and mobile imports nothing from src/main.

Both prior pullfrog threads remain resolved (the withdrawn-replay silent drop fixed in 1111b0d; the shape-rejection rollback concern withdrawn as a documented trade-off).

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

Each queued message was its own tall card with a "Queued" header and a row of
Steer / Edit / Delete text buttons, and the paused-queue line floated above
them. The phone now draws the desktop's layout: one bordered box whose first
row is the pause line with Resume (only while paused), then one compact row
per message with a leading queue or alert icon, the text (up to two lines,
since a phone has no hover title), a caption only when there is something to
say, and Steer (Send for a returned or failed card), a trash button, and a
"..." menu holding Edit message.

The card model's label becomes a nullable caption with the desktop's rules
(no caption for a plain wait or the queue's pause) plus a needsAttention flag
for the alert icon. Touch targets stay 44pt inside their rows.
Comment on lines +54 to +55
if (!cards || cards.length === 0) {
return null

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Stale Edit menu reopens If the last card disappears while its More actions menu is open, this return skips the code that clears the selected card. The component keeps that selection while the list is empty. Stop can requeue the draft under the same ID, causing its old Edit menu to reopen without a tap.

Comment on lines +123 to +125
: facts.pendingPrompt
? 'Waiting for your answer'
: null

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Waiting status is inaccessible An ordinary waiting card now has no caption, and its row has no accessible status label. After the message leaves the composer, a screen-reader user hears its text and action buttons but is not told that it is queued rather than sent.

@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: 4656265f-4a25-489a-9e02-ff2ca86d04e9

📥 Commits

Reviewing files that changed from the base of the PR and between a164348 and 5234bfa.

📒 Files selected for processing (6)
  • mobile/src/session/MobileNativeChatQueuedMessages.test.tsx
  • mobile/src/session/MobileNativeChatQueuedMessages.tsx
  • mobile/src/session/mobile-structured-queued-message-cards.test.ts
  • mobile/src/session/mobile-structured-queued-message-cards.ts
  • mobile/src/session/use-mobile-native-chat-queued-slot.test.tsx
  • mobile/src/session/use-mobile-structured-agent-session-queued.test.tsx

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

Comment on lines +71 to +76
// A card that drained or was removed while its menu was open closes the menu, for good: a
// Stop-requeued draft comes back under the same id and must not reopen it.
const menuCard = cards.find((card) => card.messageId === menuFor)
if (menuFor !== null && !menuCard) {
setMenuFor(null)
}

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.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Clear stale menuFor in the reset path, not only when cards is non-empty.

The early return at Line 54 runs before the reset at Lines 74-76. Suppose the last card drains while its menu is open. The component then returns null, and menuFor keeps the old id. If a Stop later requeues a draft under the same id, menuCard resolves again and the sheet reopens. Lines 71-72 say this case must not happen. The same mismatch exists when all cards drain at once.

Move the stale-menu reset above the empty-list return, or clear menuFor whenever cards is empty.

🐛 Proposed fix
   const [menuFor, setMenuFor] = useState<string | null>(null)
+  const menuCard = cards?.find((card) => card.messageId === menuFor)
+  if (menuFor !== null && !menuCard) {
+    setMenuFor(null)
+  }
   if (!cards || cards.length === 0) {
     return null
   }
@@
-  const menuCard = cards.find((card) => card.messageId === menuFor)
-  if (menuFor !== null && !menuCard) {
-    setMenuFor(null)
-  }

@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

This run reviewed the one commit since the prior pullfrog review (a164348): the queued-draft surface was redesigned from stacked cards into one compact box of rows, matching the desktop layout.

  • One compact box — MobileNativeChatQueuedMessages.tsx now draws the pause row and every card as hairline-divided rows inside a single bordered box, with the pause row first, the text capped at two lines, and each action a 44 pt target kept inside its row (so Android cannot drop touches outside it).
  • Menu-driven Edit — Edit moved behind a per-card "…" action sheet (ActionSheetModal with closeBeforePress), so the composer Edit fill can only take focus after the sheet's native Modal is gone; Steer, Delete and Resume stay direct actions.
  • Presentation-only card model — mobile-structured-queued-message-cards.ts replaces the single label with caption (null for a plainly-waiting card, and for the paused queue's cards) plus needsAttention (returned, or a failed-send hold), letting the render layer choose the alert icon and destructive caption color from that.
  • Tests rebuilt — the component suite now asserts the box geometry (row testIDs, hairline dividers, 44 pt targets) and the menu open/close lifecycle, and the slot and queued-hook fixtures follow the new caption / needsAttention shape.

The two reviewer threads already open on this commit (the emptied-list Edit menu, and the waiting-card status announced to screen readers) are unchanged; this review adds no duplicate comments.

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

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes

This run reviewed the one commit since the prior pullfrog review (5234bfa): a merge of origin/main into the branch. No file this PR authors changed, so the phone queue surface is byte-identical to the previously reviewed revision.

  • Base merge-up — f161f7e36d merges origin/main (now a57863ba16), bringing main's agent workspace-trust refactor, mobile web-shell changes, and the host-side #23940 queued-queue change ("an unfinished /clear changed nothing the chat reads, so it refuses no send"). None of it touches the 48 files this PR authors under mobile/src/session/.
  • No contract break for the phone — the only mobile/src/session files the merge changes (mobile-native-chat-message-text.ts un-exports its font-scale clamp; its test is deleted; mobile-native-chat-answer-stepping.test.ts drops a constants assertion) sit outside the queued-card surface. The symbols mobile reads (MOBILE_NATIVE_CHAT_QUESTION_STEP_MS, FONT_SCALE_STEP, quantizeFontScale) stay exported, and nothing in the queue path imports the diminished ones.
  • Pause wording still fits the host — #23940 narrows the queue hold to an uncertain rewind or a superseded source; the completed-clear pause the phone labels cleared is unaffected, and the phone only reads the published queuePause reason.

Both prior pullfrog threads remain resolved. The two reviewer threads on 5234bfa (the emptied-list Edit menu, and the waiting-card status for screen readers) are unchanged by this merge and stay as they are.

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

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes

This run reviewed the delta since the prior pullfrog review (f161f7e): the chat view's turn-status wiring tests moved into their own type-checked file, then a merge of origin/main into the branch.

  • Type-checked test move — 40cdcb66 lifts the structured turn status wiring block out of MobileNativeChatView.test.ts (still grandfathered in mobile/tests-typecheck-baseline.txt) into a new MobileNativeChatView.turn-status.test.ts that compiles under tsconfig.test.json; all ten cases move across faithfully, the suite passes, and check:tests-typecheck stays green.
  • Merge-up reconciliation touches two PR-authored files — c2d7871 merges origin/main (7980ab99). mobile-structured-agent-session-rpc.ts gains main's thrown-refusal refactor (AgentSessionRpcResponseError.data, agentSessionThrownFailure, agentSessionReadFailureText) while keeping this PR's hostRejectedByRequestSchema union member and its invalid_argument-only spread; use-mobile-structured-agent-state.ts now reads read-failure text through agentSessionReadFailureText. The combination is coherent: the schema-rejection flag is still keyed off the raw error.code, so only a schema refusal retires an in-flight id and an auth refusal keeps it.
  • Everything else in the merge is base drift — mobile-native-chat-stream-frame.*, use-mobile-native-chat-session.ts, the session-stream-end/refusal tests, and opened-mobile-session-tab.ts (modified, not deleted, so the use-mobile-session-file-actions.ts / mobile-file-tap-open.ts imports still resolve) are main's own changes and sit outside this PR's queue surface.

The two open reviewer threads on 5234bfa (the emptied-list Edit menu, and the waiting-card status for screen readers) are unchanged by these commits and remain as they are.

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