fix(sidebar): refresh on session-created; stabilize streaming and navigation (supersedes #159) #160
Conversation
- Live updates: incremental WS processing with immutable updates; dedupe final text; lock streaming ownership to the intended session; ignore increments when viewing another session; stable keys. (Claude unaffected) - Post-completion sync: after cursor-result, silently reload authoritative session history and replace only if richer/different; guard by current viewed session and clear timers on session change. File: src/components/ChatInterface.jsx
…n init - Only navigate to /session/:id when current URL is '/' - Avoid clearing selectedSession for Cursor when projects_updated lacks cursorSessions - Sidebar refresh now also fetches cursorSessions to update sessions list properly This addresses rapid page switches (flashing) and stale sidebar sessions.
- On receiving session-created with a sessionId, if current URL is '/', navigate to /session/:id and mark as system session change to preserve current chat content. This matches manual clicking behavior and stops the flicker loop.
- Navigate once to /session/:id when at '/' using a ref guard - Avoid await/prefetch to prevent async issues and request bursts
- Remove automatic /api/cursor/config fetch from ChatInterface - Mount only active tab (Files/Shell/Git not mounted on Chat) - Avoid auto refreshProjects calls after cursor session completes
…n-update - Trigger the exact same logic as clicking the Sidebar refresh button (`handleSidebarRefresh`) when a new real session id arrives via WebSocket. - Schedule a single delayed refresh (800ms) to avoid racing backend persistence; cancel the pending refresh if a `projects_updated` containing the session arrives first. - Loosen additive-update guard so Sidebar can update during active sessions (Cursor always additive; Claude checks existence by id only). - Prevent WS message reprocessing loop in ChatInterface by advancing processed index before early return on auto navigation.
xavierliang
changed the title
|
@xavierliang Can you check if the PR is still needed with the v1.8.1 changes in the sessions ? |
|
Yes, it's still needed |
|
@xavierliang thanks. are you able to resolve the conflicts and test it again? |
- 将 sonnet-4 更新为 sonnet-4.5 - 添加更多支持的模型选项：auto, sonnet-4.5-thinking, grok - 更新模型映射以支持从配置文件读取 - 修复 'Cannot use this model: sonnet-4' 错误
WalkthroughIntroduces incremental WebSocket processing, additive-update gating, and timer-managed Sidebar refreshes. Adds comprehensive streaming/state management for Cursor sessions: chunk accumulation, finalization, session-guarded updates, silent post-stream refresh, expanded model mapping, and navigation deduping. Selection preservation and URL/session routing are adjusted to include Cursor sessions and provider metadata. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant WS as WebSocket
participant App as App.jsx
participant Timers as window.__ensureTimers
participant Sidebar as Sidebar (refresh)
participant Store as Projects State
WS->>App: projects_updated / session_created / message events
App->>App: Check appLastProcessedIndexRef (incremental)
App->>App: isUpdateAdditive? (Cursor sessions additive)
alt Active session AND non-additive
App--xWS: Skip update (protect active session)
else Additive or no active session
App->>Store: Merge projects with cursorSessions
App->>App: Update projectsRef
end
opt session_created received
App->>Timers: schedule one-time Sidebar refresh
end
WS-->>App: projects_updated includes session
App->>Timers: cancel pending refresh (if includes session)
Timers->>Sidebar: refresh
Sidebar->>Store: fetch baseProjects
Sidebar->>Store: attach Cursor session data
Sidebar->>App: return updated list
App->>App: Preserve selected project/session if possible
sequenceDiagram
autonumber
participant WS as WebSocket
participant Chat as ChatInterface.jsx
participant Cursor as Cursor Provider
participant Router as Router
participant Store as Chat State
Note over Chat: Initialize streaming refs and guards
WS-->>Chat: claude-response / claude-output deltas
Chat->>Chat: buffer deltas (accumulate-and-flush)
Chat->>Store: appendStreamingChunk(session-scoped)
WS-->>Chat: final result / done
Chat->>Store: finalizeStreaming -> stable assistant message
Chat->>Chat: Update lastAssistantTextRef
alt Provider == Cursor
Chat->>Chat: Start silent refresh timer
Chat-->>Cursor: fetch authoritative history (deferred)
Cursor-->>Chat: history
Chat->>Store: reconcile without duplicates
end
opt session_created or session switch
Chat->>Chat: Dedup navigate (navigatedToSessionRef)
alt At root path only
Chat->>Router: navigate to new session
else
Chat--xRouter: no auto-navigation
end
end
Note over Chat: Guards prevent cross-session updates and duplicate finals
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests
Comment |
Updated PR with latest changesI've updated this PR to resolve the merge conflicts with the latest main branch (v1.8.10). Changes included:
Ready for review! 🚀 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/components/ChatInterface.jsx (3)
2039-2078: Fix Biome “noSwitchDeclarations”: wrap case bodies with blocks.Several switch cases declare
const/let(e.g.,const messageData,const cleaned,const raw,const statusData) without wrapping the clause in a block. This can leak bindings across cases and violates lints. Wrap each affectedcasebody in{ ... }.Apply a representative patch (repeat this pattern for other cases shown in the ranges):
- case 'claude-response': - const messageData = latestMessage.data.message || latestMessage.data; + case 'claude-response': { + const messageData = latestMessage.data.message || latestMessage.data; // ... existing logic ... - break; + break; + }Do the same for:
- session-created
- claude-response
- claude-output
- cursor-output
- cursor-result
- claude-status
As per static analysis hints.
Also applies to: 2079-2241, 2266-2282, 2331-2423, 2424-2445, 2502-2537
3589-3616: Eliminate double-submit race from button handlers.The submit button calls
handleSubmitononMouseDown/onTouchStartand the form also hasonSubmit. This can send twice beforeisLoadingflips.Use only the form’s
onSubmit(keeps Enter-key behavior) and remove the immediate handlers:- <button - type="submit" - disabled={!input.trim() || isLoading} - onMouseDown={(e) => { - e.preventDefault(); - handleSubmit(e); - }} - onTouchStart={(e) => { - e.preventDefault(); - handleSubmit(e); - }} + <button + type="submit" + disabled={!input.trim() || isLoading} className="absolute right-2 top-1/2 transform -translate-y-1/2 w-12 h-12 sm:w-12 sm:h-12 bg-blue-600 hover:bg-blue-700 disabled:bg-gray-400 disabled:cursor-not-allowed rounded-full flex items-center justify-center transition-colors focus:outline-none focus:ring-2 focus:ring-blue-500 focus:ring-offset-2 dark:ring-offset-gray-800" >
1800-1837: Standardize toolResult shape to match renderer expectations.
convertSessionMessagessetstoolResultas a string andtoolErrorseparately, butMessageComponentexpectsmessage.toolResultto be an object{ content, isError }. This mismatch breaks styling and content display.- converted.push({ + converted.push({ type: 'assistant', content: '', timestamp: msg.timestamp || new Date().toISOString(), isToolUse: true, toolName: part.name, toolInput: JSON.stringify(part.input), - toolResult: toolResult ? (typeof toolResult.content === 'string' ? toolResult.content : JSON.stringify(toolResult.content)) : null, - toolError: toolResult?.isError || false, - toolResultTimestamp: toolResult?.timestamp || new Date() + toolResult: toolResult + ? { + content: typeof toolResult.content === 'string' + ? toolResult.content + : JSON.stringify(toolResult.content), + isError: !!toolResult.isError + } + : null, + toolResultTimestamp: toolResult?.timestamp || new Date() });src/App.jsx (1)
233-236: Do notreturnfrom the effect mid-loop; advance the processed index and continue.Early
returninside theprojects_updatedbranch exits the effect without updatingappLastProcessedIndexRef, causing the same message to be reconsidered on future renders.Continue the loop and mark the message as processed:
- if (!isAdditiveUpdate) { - // Skip updates that would modify existing selected session/project - return; - } + if (!isAdditiveUpdate) { + // Skip updates that would modify existing selected session/project + appLastProcessedIndexRef.current = i + 1; + continue; + } @@ - appLastProcessedIndexRef.current = messages.length; + appLastProcessedIndexRef.current = messages.length;Also applies to: 169-176, 268-271
🧹 Nitpick comments (5)
src/components/ChatInterface.jsx (3)
159-167: Avoid reading provider from localStorage in MessageComponent; pass it as a prop.Reading
localStorageinside a memoized child is non-reactive and can desync icons/labels. Passproviderdown and use it.-const MessageComponent = memo(({ message, index, prevMessage, createDiff, onFileOpen, onShowSettings, autoExpandTools, showRawParameters }) => { +const MessageComponent = memo(({ message, index, prevMessage, createDiff, onFileOpen, onShowSettings, autoExpandTools, showRawParameters, provider }) => { @@ - {(localStorage.getItem('selected-provider') || 'claude') === 'cursor' ? ( + {provider === 'cursor' ? ( <CursorLogo className="w-full h-full" /> ) : ( <ClaudeLogo className="w-full h-full" /> )} @@ - {message.type === 'error' ? 'Error' : message.type === 'tool' ? 'Tool' : ((localStorage.getItem('selected-provider') || 'claude') === 'cursor' ? 'Cursor' : 'Claude')} + {message.type === 'error' ? 'Error' : message.type === 'tool' ? 'Tool' : (provider === 'cursor' ? 'Cursor' : 'Claude')} @@ - <MessageComponent + <MessageComponent key={message.id || index} message={message} index={index} prevMessage={prevMessage} createDiff={createDiff} onFileOpen={onFileOpen} onShowSettings={onShowSettings} autoExpandTools={autoExpandTools} - showRawParameters={showRawParameters} + showRawParameters={showRawParameters} + provider={provider} /> @@ - <div className="text-sm font-medium text-gray-900 dark:text-white">{(localStorage.getItem('selected-provider') || 'claude') === 'cursor' ? 'Cursor' : 'Claude'}</div> + <div className="text-sm font-medium text-gray-900 dark:text-white">{provider === 'cursor' ? 'Cursor' : 'Claude'}</div>Also applies to: 245-254, 3347-3354, 3326-3336
2113-2119: Remove redundant provider branch around finalizeStreaming.Both branches call
finalizeStreaming()identically; keep a single call for clarity.- // Only treat as finalize-with-tracking for Cursor flows - if ((localStorage.getItem('selected-provider') || 'claude') === 'cursor') { - finalizeStreaming(); - } else { - // For Claude, just mark the streaming message complete without using cursor-specific trackers - finalizeStreaming(); - } + finalizeStreaming();
1955-1963: Clear pending timers on unmount as well.You clear
cursorSilentRefreshTimerRefon session change; also clear on component unmount to prevent background updates after teardown.+ useEffect(() => { + return () => { + if (cursorSilentRefreshTimerRef.current) { + clearTimeout(cursorSilentRefreshTimerRef.current); + cursorSilentRefreshTimerRef.current = null; + } + if (streamTimerRef.current) { + clearTimeout(streamTimerRef.current); + streamTimerRef.current = null; + } + }; + }, []);src/App.jsx (2)
176-195: Timer hygiene: ensure global Sidebar refresh timers are cleaned up on unmount.
window.__ensureTimersstores timeouts keyed by session id. Add an unmount cleanup to avoid orphan timers across navigations/hot reloads.+ useEffect(() => { + return () => { + if (window.__ensureTimers) { + for (const t of window.__ensureTimers.values()) { + clearTimeout(t); + } + window.__ensureTimers.clear(); + } + }; + }, []);Also applies to: 436-495
382-396: Prefer session-derived provider when selecting a session.Rely on
session.__providerif present; falls back to localStorage only when absent. Prevents mismatches when the global selection differs from session origin.- const provider = localStorage.getItem('selected-provider') || 'claude'; - if (provider === 'cursor') { + const providerForSession = session.__provider || localStorage.getItem('selected-provider') || 'claude'; + if (providerForSession === 'cursor') { // Cursor sessions have persistent IDs sessionStorage.setItem('cursorSessionId', session.id); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/App.jsx(5 hunks)src/components/ChatInterface.jsx(22 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/App.jsx (3)
server/projects.js (4)
projects(377-377)messages(724-724)project(400-407)project(473-482)src/utils/websocket.js (1)
messages(5-5)src/utils/api.js (5)
api(23-141)api(23-141)url(53-53)authenticatedFetch(2-20)authenticatedFetch(2-20)
src/components/ChatInterface.jsx (2)
src/utils/websocket.js (1)
messages(5-5)src/App.jsx (2)
selectedSession(51-51)selectedProject(50-50)
🪛 Biome (2.1.2)
src/components/ChatInterface.jsx
[error] 2080-2082: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🔇 Additional comments (1)
src/App.jsx (1)
48-50: Projects ref for late WS handling: LGTM.Keeping
projectsRefsynced to state for timer/WS closures avoids stale reads.
| try { | ||
| const providerNow = localStorage.getItem('selected-provider') || 'claude'; | ||
| if (providerNow === 'cursor') { | ||
| const projectPath = selectedProject?.fullPath || selectedProject?.path; | ||
| const sessionToReload = currentSessionId || cursorSessionId; | ||
| if (projectPath && sessionToReload) { | ||
| // Clear any pending refresh for previous session | ||
| if (cursorSilentRefreshTimerRef.current) { | ||
| clearTimeout(cursorSilentRefreshTimerRef.current); | ||
| } | ||
| cursorSilentRefreshTimerRef.current = setTimeout(async () => { | ||
| try { | ||
| // Guard: only refresh if user is still viewing the same cursor session | ||
| const stillCursor = (localStorage.getItem('selected-provider') || 'claude') === 'cursor'; | ||
| const stillSameSession = selectedSession?.id === sessionToReload; | ||
| if (!stillCursor || !stillSameSession) return; | ||
| const fetched = await loadCursorSessionMessages(projectPath, sessionToReload); | ||
| if (Array.isArray(fetched) && fetched.length) { | ||
| setChatMessages(prev => { | ||
| const lastFetched = fetched[fetched.length - 1] || {}; | ||
| const lastPrev = prev[prev.length - 1] || {}; | ||
| const shouldReplace = fetched.length > prev.length || normalizeText(JSON.stringify(lastFetched)) !== normalizeText(JSON.stringify(lastPrev)); | ||
| return shouldReplace ? fetched : prev; | ||
| }); | ||
| } | ||
| } catch (err) { | ||
| console.warn('Silent reload of cursor session failed:', err); | ||
| } | ||
| }, 250); | ||
| } |
There was a problem hiding this comment.
Guard silent refresh timer against stale session with a ref.
setTimeout closure in cursor-result checks selectedSession?.id, but it captures a stale value. If the user switches sessions quickly, the silent refresh can replace the wrong chat.
Track the current session id in a ref and read it inside the timer:
@@
- // Cursor silent refresh timer
+ // Cursor silent refresh timer
const cursorSilentRefreshTimerRef = useRef(null);
+ // Track current selected session id for async checks
+ const selectedSessionIdRef = useRef(null);
+ useEffect(() => {
+ selectedSessionIdRef.current = selectedSession?.id || null;
+ }, [selectedSession]);
@@
- cursorSilentRefreshTimerRef.current = setTimeout(async () => {
+ cursorSilentRefreshTimerRef.current = setTimeout(async () => {
try {
// Guard: only refresh if user is still viewing the same cursor session
- const stillCursor = (localStorage.getItem('selected-provider') || 'claude') === 'cursor';
- const stillSameSession = selectedSession?.id === sessionToReload;
+ const stillCursor = (localStorage.getItem('selected-provider') || 'claude') === 'cursor';
+ const stillSameSession = selectedSessionIdRef.current === sessionToReload;
if (!stillCursor || !stillSameSession) return;Also consider clearing cursorSilentRefreshTimerRef on unmount.
Also applies to: 1169-1176
🤖 Prompt for AI Agents
In src/components/ChatInterface.jsx around lines 2388 to 2417, the setTimeout
closure captures selectedSession?.id causing stale-session refreshes; change to
track the active session id in a ref (e.g., currentSessionIdRef.current) and
update that ref whenever selectedSession changes, then read that ref inside the
silent refresh timer instead of the captured variable; ensure you clear any
existing cursorSilentRefreshTimerRef before setting a new timeout and clear the
timer on component unmount (and apply the same ref/timer-guard pattern to the
other occurrence at lines ~1169-1176).
2 participants
Summary
This PR supersedes and replaces #159. It includes all improvements from #159 (stabilizing streaming message rendering and post-completion sync), and adds a robust Sidebar refresh mechanism for newly created sessions.
Brought over from #159
activeCursorStreamSessionIdRefandpendingCursorViewingSessionIdRef.session-createdarrives, auto-navigate only from/and avoid double-handling by advancing the processed index before returning.cursor-result, silently reload and replace history if the fetched log is richer or differs.New in this PR
session-created: schedule a single delayed refresh (same behavior as clicking the Sidebar refresh button), and cancel it if an incomingprojects_updatedalready includes the new session.Related issues
Test plan
Summary by CodeRabbit
New Features
Bug Fixes
Chores