Conversation
WalkthroughChat client handling now marks session transitions as system-initiated and navigates to real sessions using actualSessionId when provided. Server Codex completions include an actualSessionId, and project/session parsing/matching logic was improved to normalize paths and adjust payload parsing/counting. Changes
Sequence DiagramsequenceDiagram
%% Styling (subtle colors via notes)
actor Server as Server
actor User as User/System
participant Chat as ChatInterface
participant Nav as NavigationHandler
participant State as SessionState
User->>Chat: send message / trigger session-created
Server->>Chat: emit codex-complete (sessionId, actualSessionId)
alt actualSessionId present
Chat->>Chat: set currentSessionId = actualSessionId\nmark isSystemSessionChange
Chat->>Nav: onNavigateToSession(actualSessionId)
Nav->>State: update selected session & URL
Note right of Chat: Messages preserved\n(no history cleared)
else fallback to pending ID
Chat->>Chat: replace temporary session id\nset isSystemSessionChange
Chat->>Nav: onNavigateToSession(pendingId)
Nav->>State: update selected session & URL
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
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 |
…ix session navigation in ChatInterface
|
@viper151 This should work |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/ChatInterface.jsx (1)
3516-3550: Block-scope thecodex-completecase and fix the console.log variableThe
codex-completecase has two issues:
- Biome
noSwitchDeclarationserror: Theconstdeclarations at lines 3535–3536 must be wrapped in a block to prevent accidental cross-case access and hoisting bugs.- Console.log bug: Line 3548 logs
codexPendingSessionIdinstead ofcodexActualSessionId—the value actually being set bysetCurrentSessionId().Wrap the case body in braces, correct the logged variable, and optionally add
&& codexActualSessionIdto the condition for defensive safety:Proposed fix: block-scope the
codex-completecase- case 'codex-complete': - // Handle Codex session completion - const codexCompletedSessionId = latestMessage.sessionId || currentSessionId || sessionStorage.getItem('pendingSessionId'); - - if (codexCompletedSessionId === currentSessionId || !currentSessionId) { - setIsLoading(false); - setCanAbortSession(false); - setClaudeStatus(null); - } - - if (codexCompletedSessionId) { - if (onSessionInactive) { - onSessionInactive(codexCompletedSessionId); - } - if (onSessionNotProcessing) { - onSessionNotProcessing(codexCompletedSessionId); - } - } - - const codexPendingSessionId = sessionStorage.getItem('pendingSessionId'); - const codexActualSessionId = latestMessage.actualSessionId || codexPendingSessionId; - if (codexPendingSessionId && !currentSessionId) { - setCurrentSessionId(codexActualSessionId); - setIsSystemSessionChange(true); - if (onNavigateToSession) { - onNavigateToSession(codexActualSessionId); - } - sessionStorage.removeItem('pendingSessionId'); - console.log('Codex session complete, ID set to:', codexPendingSessionId); - } - - if (selectedProject) { - safeLocalStorage.removeItem(`chat_messages_${selectedProject.name}`); - } - break; + case 'codex-complete': { + // Handle Codex session completion + const codexCompletedSessionId = + latestMessage.sessionId || + currentSessionId || + sessionStorage.getItem('pendingSessionId'); + + if (codexCompletedSessionId === currentSessionId || !currentSessionId) { + setIsLoading(false); + setCanAbortSession(false); + setClaudeStatus(null); + } + + if (codexCompletedSessionId) { + if (onSessionInactive) { + onSessionInactive(codexCompletedSessionId); + } + if (onSessionNotProcessing) { + onSessionNotProcessing(codexCompletedSessionId); + } + } + + const codexPendingSessionId = sessionStorage.getItem('pendingSessionId'); + const codexActualSessionId = latestMessage.actualSessionId || codexPendingSessionId; + + if (codexPendingSessionId && !currentSessionId && codexActualSessionId) { + setCurrentSessionId(codexActualSessionId); + setIsSystemSessionChange(true); + if (onNavigateToSession) { + onNavigateToSession(codexActualSessionId); + } + sessionStorage.removeItem('pendingSessionId'); + console.log('Codex session complete, ID set to:', codexActualSessionId); + } + + if (selectedProject) { + safeLocalStorage.removeItem(`chat_messages_${selectedProject.name}`); + } + break; + }Note: Similar block-scoping should also be applied to other cases with
constdeclarations (e.g.,claude-response,codex-response,claude-complete,cursor-result) for consistency and to satisfy the linter across the switch statement.
🧹 Nitpick comments (1)
server/projects.js (1)
1203-1225: Codex session matching and message parsing improvements look good; consider defensively stringifyinglastUserMessage
The updated
getCodexSessionslogic that:
- strips leading
\\?\from bothsessionData.cwdandprojectPath, and- matches on direct equality or
path.relative(cleanSessionCwd, cleanProjectPath) === ''
is a reasonable way to align Codex sessions with projects on Windows and POSIX, including long-path prefixes and minor normalization differences.In
parseCodexSessionFile, switching to:
entry.payload.messageforuser_messageevents, and- counting only
response_itementries wherepayload.type === 'message'andpayload.role === 'assistant'
better reflects the current Codex event schema and prevents over-counting non-message items.One small robustness tweak:
lastUserMessageis later used with.lengthand.substring, so if Codex ever changes shape andentry.payload.messageis not a plain string, this will throw. You could defensively coerce to string:- if (entry.type === 'event_msg' && entry.payload?.type === 'user_message') { - messageCount++; - if (entry.payload.message) { - lastUserMessage = entry.payload.message; - } - } + if (entry.type === 'event_msg' && entry.payload?.type === 'user_message') { + messageCount++; + if (entry.payload.message != null) { + lastUserMessage = String(entry.payload.message); + } + }This keeps the current behavior for string payloads but prevents surprises if the Codex SDK ever returns a non-string
message.Also applies to: 1278-1288
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
server/openai-codex.jsserver/projects.jssrc/components/ChatInterface.jsx
🧰 Additional context used
🧬 Code graph analysis (2)
server/openai-codex.js (1)
src/components/ChatInterface.jsx (1)
currentSessionId(1680-1680)
src/components/ChatInterface.jsx (1)
server/openai-codex.js (1)
currentSessionId(205-205)
🪛 Biome (2.1.2)
src/components/ChatInterface.jsx
[error] 3536-3537: 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 (2)
server/openai-codex.js (1)
281-285: AddingactualSessionIdto Codex completion payload looks correctExposing
actualSessionId: thread.idalongsidesessionIdcleanly supports the ChatInterface logic that now prefers the real thread ID but falls back to the pending ID, without changing existing consumers ofsessionId. No issues from a backend/API perspective.src/components/ChatInterface.jsx (1)
2971-2983: Markingsession-createdtransitions as system-initiated is consistent with existing session-change handlingSetting
isSystemSessionChange(true)when a new session is created ensures the subsequentselectedSessionupdate won’t clear currentchatMessagesinloadMessages, matching the way Claude/Cursor system-initiated session switches are already treated. This looks correct and keeps the UX stable during ID replacement.
2 participants
TLDR:
This pull request introduces an improvement to the session management logic in the
ChatInterfacecomponent. The main change ensures a smoother transition when a temporary session is replaced with a real session ID, preventing message loss and keeping the UI in sync.Session transition enhancements:
setIsSystemSessionChange(true)) to prevent messages from being cleared during session ID updates.onNavigateToSession, keeping the URL andselectedSessionup to date.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.