Fix chat UI state that leaks across a workbench switch - #481
Merged
Merged
Conversation
A send continuation reopens the reply thread it just created using the workbench id it was sent with, with no check that workbench is still the one on screen. Covers a continuation resolving after a switch away (no navigation), after resolving on the same bench (navigates as before), and after switching away and back (still navigates, since the reader is where the send targeted).
sendPending closes over the activeWorkbenchId its own render was called with, which never changes for the life of that async call. If the reader switches workbenches while a reply's first send is still in flight, the continuation resolved and reopened that reply thread regardless — dragging the reader back into a bench they had already left, undoing use-thread-navigation's own reset-on-switch effect. A ref now tracks the live activeWorkbenchId; the navigation call only fires when it still matches the value the send was made with.
The send guard tested the sending state variable, which performSend only sets after the caller that started it has already returned. Two triggers landing in the same tick both read sending === false and both post. Proves a second click in the same tick as the first is turned away.
canSendComposerAction's sending check reads React state, which performSend only sets true after the event handler that called it has already returned to the browser. Two sends triggered in the same synchronous tick both read sending === false and both post, producing two requests with two distinct nonces that nothing downstream dedupes. The guard now lives in performSend itself (covering every caller, including the /summarize slash command) and flips synchronously before any await, so a second call in the same tick is turned away.
The coalescing refresh timer was a single ref, neither scoped to nor cleared on the active workbench. A pending timer scheduled for one workbench made a different workbench's own refreshFeed() call early-return, and the timer then invalidated the first workbench's now- inactive query key. Proves a refresh scheduled just before a switch never fires for the old bench and the new bench's own refresh fires instead, and that repeat calls for the same bench still coalesce into one.
refreshTimerRef held a bare timer handle with no record of which workbench scheduled it. Switching benches while a refresh was pending made the new bench's own refreshFeed() call early-return on the old timer, and when that timer fired it invalidated the old (now-inactive) query key instead of ever refreshing the bench actually on screen. The ref now carries the (tenantId, workbenchId) it was scheduled for; refreshFeed clears a timer scheduled for a different one instead of deferring to it, and the unmount effect's dependency array was widened so the same clear runs on every switch, not only on unmount.
Contributor
Author
Review notes (code review pass)Composer send guard, stale send-continuation navigation, and per-workbench feed-refresh timer scoping, all keyed on the ticket's three reported leaks. Composer in-flight ref release
Per-workbench refresh timerThe unmount-cleanup effect's dependency array was widened from Discarded-diff checkChecked the branch's final state directly ( Checks run (foreground)
Commit messages clean (no ticket refs, no long lines). No issues found. Not merging. |
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
1 participant
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
(tenantId, workbenchId)pair it was scheduled for, and cleared when the active workbench changes, instead of a bare timer handle shared across benchesTest plan
WORKBENCH_CHECK_SINCE=origin/main bun run typecheck(4 affected packages)bun run lintWORKBENCH_CHECK_SINCE=origin/main bun run test(chat-ui: 823 pass, 0 fail)bun run check:structuralhttps://linear.app/abklabs/issue/CL-7198/fix-chat-ui-state-that-leaks-across-a-workbench-switch