fix(desktop): queue useSyncExternalStore stability, missing imports, promote mutation - #68697
Closed
re-ITRT wants to merge 3 commits into
Closed
fix(desktop): queue useSyncExternalStore stability, missing imports, promote mutation#68697re-ITRT wants to merge 3 commits into
re-ITRT wants to merge 3 commits into
Conversation
…Research#61573) Replace the nanostore-based + useEffect-driven queue system with a standalone QueueManager that survives component mount/unmount cycles. Why: - Queue auto-drain relied on React effects that died when ChatBar unmounted (e.g. navigating to Messaging tab). The drain would then inherit a stale foreground runtime id via activeSessionIdRef and deliver the queued message to the wrong session. - Resolving session identity at drain time from shared mutable refs (runtimeIdByStoredSessionIdRef, activeSessionIdRef) was fragile and produced unpredictable cross-session routing. What changed: - New file src/lib/queue-manager.ts: singleton, RAF-batched subscriber notifications, retry with backoff (3 tries x 2s), 30s polling fallback, localStorage persistence, running-guard on session.resume. - Deleted use-background-queue-drain.ts (effect) and its test. - composer-queue.ts delegates all operations to QueueManager. - use-composer-queue.ts: replace nanostore useSessionSlice with useSyncExternalStore(QueueManager.subscribe); remove auto-drain effect (QueueManager handles drain via ). - wiring.tsx: init()/destroy() QueueManager when gateway opens/closes instead of calling useBackgroundQueueDrain hook. Architecture: Gateway WS events -> -> QueueManager.subscribe | tryDrain(sid) | session.resume -> prompt.submit QueueManager lives at the wiring level, not inside any component tree, so it keeps draining regardless of sidebar navigation.
- queue-manager: stable EMPTY_QUEUE reference for getAll empty returns - queue-manager: type cast (Set→includes) at both call sites - queue-manager: promote() creates new array instead of mutating in place - composer-queue: module-level EMPTY constant for stable empty array ref All three address the same root cause: useSyncExternalStore snapshot stability. Returning a fresh '?? []' every call defeats React's Object.is bail-out and causes 'Maximum update depth exceeded' (error NousResearch#185) when the session key changes (e.g. navigating to Messaging tab).
…s, stabilize subscribe/getSnapshot - Close unclosed useCallback for queueCurrentDraft (syntax error) - Remove duplicate ref declarations that leaked after the missing brace - Add missing imports: MAX_AUTO_DRAIN_ATTEMPTS, shouldAutoDrain, notify - Wrap QueueManager.subscribe in useCallback for stable identity - Wrap getSnapshot in useCallback, use useRef([]).current for stable empty array - Restore original dep array scope.attachments
alt-glitch
added
type/bug
Contributor
Author
|
错误的操作导致了这个pr申请，抱歉 |
Contributor
Author
|
This PR was created by an automated process error. Sorry for the noise. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three fixes in the queue-manager area, all related to the QueueManager refactor (#61573).
Changes
1. useSyncExternalStore infinite re-render (error #185)
EMPTY_QUEUEreference forgetAll()empty returns (was?? []— new reference every call defeats React'sObject.isbail-out)EMPTYconstant for stable empty array refuseCallbackwrappers foruseSyncExternalStoresubscribe/getSnapshot,useRef([]).currentfor stable empty array in falsy branchQueueManager.subscribethisbinding viauseCallback2. Missing imports in use-composer-queue.ts
Added
MAX_AUTO_DRAIN_ATTEMPTS,shouldAutoDrain,notify— these symbols were used in the code body but never imported (leftover from the refactor)3. Syntax error in use-composer-queue.ts
queueCurrentDraftuseCallback was missing its closing}); removed duplicateprevQueueKeyRef/etc. declarations that leaked after the missing brace4. Pre-existing issues caught during investigation
Set<string>→ `includes()` (runtime type is `readonly string[]`)promote()now creates a new array reference instead ofsplice/unshiftmutating in place (React wouldn't detect the change)