Repository navigation
fix(native-chat): a queue action still on its way greys its controls, like the phone - #29120
Conversation
…mposer Resume or send dialog The queued cards now sit under one header row on desktop and phone: "Queued" and the count normally, and "Paused" with the count, Clear all and Resume while the host holds the queue after a Stop. A chevron folds only the cards; the choice is local to the view and resets when the queue empties. The composer's Send no longer turns into a second Resume, and a message sent into a paused queue no longer asks "Send message?": it sends, its accepted turn lifts the pause, and the cards follow it. Clear all deletes every card through the existing per-card delete. No wire or host change.
…ip, not the queue header's
… like the phone The desktop kept in-flight queue actions only in refs (one per card, one for Resume, a joined promise for Clear all), so on a slow or remote host pressing Clear all, Delete, Steer or Send changed nothing on screen until the host answered. One keyed in-flight set, held as a ref and as render state, now covers cards, Resume and Clear all; Clear marks every card it deletes.
… guard; the steer chord skips a card already on its way Resume and Clear all now hold one shared in-flight key, so each refuses the other in the guard rather than only in the drawn state, and both header buttons grey on it. Cmd/Ctrl+Enter returns false when the newest card's action (or a Clear deleting it) is still out, so the chord no longer scrolls to the latest with nothing sent. Test controllers gain the new fields.
The queued-list test file passed its line limit; the in-flight test and the held-write render it needs now live beside it, and the list test file is back to the base's helpers.
|
On a slow or remote host, clicking a desktop queued message’s Delete, Steer or Send, or the queue’s Clear all, left the controls looking usable until the host replied. Repeat presses appeared to do nothing because duplicate suppression was hidden from the user. This PR greys the controls while their request is pending and restores them when it settles. A single-card action greys that card and Clear all; independent cards remain usable. Clear all greys both header actions and every captured card. Resume greys both header actions. Failures retain unsent cards, restore controls and use the existing error notice; a partly successful Clear retains the cards whose deletion failed. There is no new progress text or saved state. Review complete on Verified: the full PR workflow passed on this exact head: static analysis/typecheck, all five test shards, relay integration, both packaging jobs and the aggregate PR checks passed. The PR is out of draft and GitHub reports CLEAN. Real rendered QA passed on a separate M4Air, using signed-in Codex (GPT-6.1-Sol High) and a Grok driver through Playwright’s Chrome DevTools Protocol connection. The six flow types covered pending Delete and independent cards, finite Delete failure/recovery, held/repeated Clear and partial failure, slow Resume, busy Steer/keyboard repeats, and ordinary send/delete plus folder/git workspace neighbors and composer focus. Duplicate presses added no requests. A fresh full app build on the final head confirmed the header dot, Delete greying/recovery, and repeated Clear producing exactly two deletes for two captured cards, then an empty queue. Evidence limits: the six-flow run used Deferred: the inline editor remains usable while Clear deletes its card; “Turn off queueing” greys with that card’s menu; held shortcut repeat behavior is unchanged. Phone Clear/Resume parity is tracked in STA-10676. Independent-card concurrency is an intended difference from the common pattern so one slow request does not lock every other card. Focus leaving a newly disabled button is intended; normal settlement refocus remains. QA also recorded these follow-ups outside this PR’s pending-control change:
For Brennan’s review Each image is a whole hidden window. Main means the disclosed three-file baseline. PR images for Delete pending, Clear pending and Clear settled are from final head
Final-head repeated Clear settled with each card deleted once:
Partial Clear failure retained the failed card and recovered controls:
Three places where the behavior is decided:
|
…skipping the card Clear all now greys while any card in the queue has its own Steer or Delete on its way, and once that settles it claims every card. Before, it stayed enabled and quietly left the busy cards out, so a failed Delete left a card behind after Clear all, and with every card busy the click did nothing.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 8 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 43.75% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 32 functions across 28 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: queue controls become disabled while an action is in progress. Description check The description is complete and focused. It explains the user-visible behavior, implementation, motivation, alternatives, testing, visual proof, compatibility considerations, and checklist status. It … Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description is complete and focused. It explains the user-visible behavior, implementation, motivation, alternatives, testing, visual proof, compatibility considerations, and checklist status. It references issue
#28654under Linked Issue, although it uses “Stacked on” instead of the template’s “Fixes #” format.
-
✨ Finishing Touches: 💡 2
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Comment @coderabbitai help to get the list of available commands.
ELI5
Messages you send while the agent works wait as cards above the chat box. Each card has Steer (or Send), Delete and a "…" menu, and a paused queue's heading has Clear all and Resume. When the computer running the chat is slow or far away (a remote host over SSH, a busy machine), the answer to one of those presses can take seconds. Until now nothing on screen changed in that time: the card and its buttons looked exactly as before, so it looked like the press did nothing, and pressing again also seemed to do nothing.
Now, the moment you press, the buttons that action is using turn grey, the way they already do on the phone. They come back when the computer answers.
What Changed
Before (desktop):
After (desktop):
The phone is unchanged; it already greys a pressed control.
Mechanism. The desktop had three separate "already running" guards, all invisible to rendering except one: a per-card set in a ref, a Resume flag (the only one drawn), and Clear all's shared in-flight promise. They are replaced by the phone's one mechanism: a keyed in-flight set, kept both as a ref (it catches a double click within the same frame, before React re-renders) and as render state (so the buttons can grey out). Its keys are each card's message id, plus one shared key for the header's two actions, Resume and Clear all, so each refuses the other while it runs (Resume during Clear would start sending cards Clear is deleting, and the reverse). Clear all also adds every card id it deletes, so those cards grey out with it. It never skips a card: while any card's own action is running, Clear all is unavailable (the same way it already is while a just-sent message has no card on the host yet), and a same-frame press that overlaps a running card is refused whole by the in-flight set rather than narrowed. The controller exposes
busy(messageId)andqueueBusy(which replacesresuming); the card takes onebusyprop that disables Steer/Send, Delete and the menu (which holds Edit). Clear all's joining of a repeated press is gone: the button is disabled while it runs, and the ref refuses a same-frame repeat.Why
This is the same decision ("one action per control at a time, and it shows") that the phone and the desktop each kept a copy of. The desktop's copy kept the guard and dropped the drawn half. Moving the desktop onto the phone's shape removes the gap for every queue action at once, rather than adding a fourth flag next to the three. It is a small net deletion in production code.
Alternatives considered: adding render state beside each of the three existing guards (three copies of the same thing, the shape that let this drift), or a "Deleting…" caption on the row. The caption would flash on every delete on a local host, where the answer comes back immediately.
Differences from the common pattern:
Reverses a call in #28654. #28654 kept Resume and card Steer usable during Clear all and joined a repeated Clear press, to keep a late review fix narrow. That let Resume start sending cards Clear was deleting, and let a card's Steer race its own delete (ending in "This message was already sent."). Here they wait for Clear to finish instead.
Not changed: a card open in the inline editor is still deleted by Clear all, and its editor isn't greyed while that happens. That is how #28654 behaves too.
Linked Issue
Stacked on #28654 (Clear all exists only there).
Visual Proof
Before/after screenshots from rendered QA (deletes and Resume held for several seconds on the host) are in the review status comment below.
Testing
AI Disclosure
Review
Agent skill upstream boundary
Notes
Renderer only: no wire, host or saved-data change, so SSH, remote and mixed-version hosts behave as before apart from the greyed controls. No platform-specific code.
Author: Brennan Benson (X: @BrennanKB5)
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)