fix(mothership): persist queued messages, edit-in-place preserves order#4769
fix(mothership): persist queued messages, edit-in-place preserves order#4769waleedlatif1 wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Edit-in-place keeps the queued slot at its index ( UI passes Reviewed by Cursor Bugbot for commit 7482443. Configure here. |
Greptile SummaryThis PR moves the mothership message queue from component-local
Confidence Score: 4/5Safe to merge after the editing-persistence fix; the queue logic and dispatch integration are otherwise well-considered. The only material defect is in store.ts: editing: state.editing is included in the partialize snapshot written to sessionStorage. Because sessionStorage survives same-tab F5 reloads, a user who hard-reloads mid-edit gets a rehydrated store where editing[chatKey] still points to the head message. The auto-drain effect fires and the dispatch loop hits the editing === msg.id guard, silently exits, and no dep ever changes to retrigger it — leaving the queue visually frozen. The user must notice the Cancel edit X button to manually unblock. Dropping editing from partialize closes the hole entirely. apps/sim/stores/mothership-queue/store.ts — the partialize function in the persist middleware. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant useChat
participant QueueStore as useMothershipQueueStore
participant SessionStorage
participant Dispatcher as dispatchQueueHead
User->>useChat: sendMessage() while stream active
useChat->>QueueStore: enqueue(chatKey, msg)
QueueStore->>SessionStorage: persist(queues)
User->>useChat: editQueuedMessage(id)
useChat->>useChat: clearQueuedSendHandoffState(id)
useChat->>QueueStore: setEditing(chatKey, id)
useChat-->>User: returns QueuedMessage (composer pre-populated)
User->>useChat: sendMessage() [edit submit]
useChat->>QueueStore: replaceAt(chatKey, id, patch)
useChat->>QueueStore: setEditing(chatKey, null)
useChat->>Dispatcher: enqueueQueueDispatch send_head
Note over Dispatcher: Skips head if editing[chatKey] === head.id
Dispatcher->>QueueStore: read liveMsg (not closure snapshot)
Dispatcher->>useChat: startSendMessage(liveMsg.content)
useChat->>QueueStore: remove(chatKey, id)
Note over QueueStore,SessionStorage: On hard reload — editing rehydrated causes auto-drain to stall
User->>useChat: cancelQueueEdit()
useChat->>QueueStore: setEditing(chatKey, null)
useChat->>Dispatcher: enqueueQueueDispatch send_head
Reviews (2): Last reviewed commit: "fix(mothership): pause drain while head ..." | Re-trigger Greptile |
…leanup on edit, merge on migrate
|
@greptile |
|
@cursor review |
| partialize: (state) => ({ | ||
| queues: Object.fromEntries( | ||
| Object.entries(state.queues).map(([key, messages]) => [ | ||
| key, | ||
| messages.map(stripVolatile), | ||
| ]) | ||
| ), | ||
| editing: state.editing, | ||
| }), |
There was a problem hiding this comment.
editing to sessionStorage creates a queue dead-lock after a hard page reload. sessionStorage survives F5 reloads within the same browser tab — so if the user was mid-edit when they reloaded, editing[chatKey] = "msgId" is rehydrated. The auto-drain useEffect fires (queue length > 0), kicks the dispatch loop, and the loop hits if (queueState.editing[activeChatKey] === msg.id) continue — silently exiting with nothing dispatched. No dependency in the auto-drain effect changes afterward, so it never retriggers. The queue appears permanently stalled until the user notices the "Cancel edit" X button and clicks it. Because editing is purely transient UI state (the composer is not pre-populated after reload regardless), it should not be persisted.
| partialize: (state) => ({ | |
| queues: Object.fromEntries( | |
| Object.entries(state.queues).map(([key, messages]) => [ | |
| key, | |
| messages.map(stripVolatile), | |
| ]) | |
| ), | |
| editing: state.editing, | |
| }), | |
| partialize: (state) => ({ | |
| queues: Object.fromEntries( | |
| Object.entries(state.queues).map(([key, messages]) => [ | |
| key, | |
| messages.map(stripVolatile), | |
| ]) | |
| ), | |
| }), |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7482443. Configure here.
| messages.map(stripVolatile), | ||
| ]) | ||
| ), | ||
| editing: state.editing, |
There was a problem hiding this comment.
Persisted editing state blocks queue dispatch after reload
High Severity
The partialize function persists editing to sessionStorage, but editing is inherently composer-scoped and cannot survive a page reload. On browser reload, React cleanup effects don't run, so the stale editing value rehydrates. The initialChatId effect won't clear it either because chatKeyRef.current === initialChatId on same-chat reload makes the guard at line 2344 evaluate to false. The dispatch loop then sees the head message as "being edited" and continues past it, leaving the queue permanently stuck until the user manually clicks "cancel edit."
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 7482443. Configure here.
Summary
useStateinto a newuseMothershipQueueStore(Zustand + sessionStorage), keyed by chatId — queue now persists across nav and reload, and chat-delete clears its bucketreplaceAt) instead of removing + re-appending at the tailpending::<shortId>) migrates to the real chatId onadoptResolvedChatId, so first-send queues follow their resolved chatchatHistory.activeStreamIdso it doesn't race the reconnect pathqueuedSendHandoffon edit + on persist so a fresh handoff is minted at send timeType of Change
Testing
Tested manually — queue survives chat-switch, home reset, browser reload; edit on a non-head queued message preserves position; edit on the dispatching head is disabled; cancel-edit leaves the slot intact. Added 16 store unit tests covering enqueue / insertAt / replaceAt (incl. handoff strip) / remove (incl. editing cleanup) / migrate / clearChat / setEditing. Council audit caught and fixed a stale-closure send-after-edit race and a stale-handoff issue.
Checklist