fix(desktop): auto-start shell on session restore without popup#1076
Closed
fix(desktop): auto-start shell on session restore without popup#1076
Conversation
Remove the "Session restored" popup overlay that required user interaction to start a new shell. Now when a terminal session is cold-restored after an app restart, the shell starts automatically in the background while preserving the previous scrollback.
Contributor
📝 WalkthroughWalkthroughRemoves the RestoredModeOverlay and changes the cold-restore flow so restored sessions start the shell directly via an updated Changes
Sequence Diagram(s)sequenceDiagram
participant Terminal as Terminal UI
participant Hook as useTerminalColdRestore
participant SessionSvc as SessionManager
participant Focus as UI Focus/Renderer
Terminal->>Hook: notify stored restore / coldRestoreResult (cwd)
Hook->>Hook: determine cwdToUse = cwdArg ?? restoredCwdRef.current
Hook->>SessionSvc: create/attach session (cwdToUse)
SessionSvc-->>Hook: onSuccess(session)
Hook->>Terminal: report session started
Hook->>Focus: schedule focus after start
Focus-->>Terminal: focus applied
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx`:
- Around line 486-496: During cold-restore, input can be dropped because
isRestoredModeRef is never set and writes occur while the old session is gone;
fix by enabling a gating flag during the restore-and-attach window and flushing
buffered writes only after the new session is attached: set
isRestoredModeRef.current = true (or set isStreamReadyRef.current = false)
before calling xterm.write(...) in the cold-restore branch (the block that reads
coldRestoreState and calls handleStartShell/createOrAttachRef), keep the flag
set until createOrAttachRef() (or the promise returned by handleStartShell)
completes successfully, then clear the flag and flush the PtyWriteQueue so
manager.write() is only attempted when the new session is live; update
handleTerminalInput to check isRestoredModeRef (or isStreamReadyRef) and queue
inputs instead of calling manager.write when the gate is active.
Comment on lines
486
to
496
| const storedColdRestore = coldRestoreState.get(paneId); | ||
| if (storedColdRestore?.isRestored) { | ||
| setIsRestoredMode(true); | ||
| setRestoredCwd(storedColdRestore.cwd); | ||
| if (storedColdRestore.scrollback && xterm) { | ||
| xterm.write( | ||
| storedColdRestore.scrollback, | ||
| scheduleScrollToBottom, | ||
| ); | ||
| } | ||
| didFirstRenderRef.current = true; | ||
| handleStartShell(storedColdRestore.cwd); | ||
| return; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect write buffering / readiness handling around terminal writes
rg -n --type=ts -C3 'useTerminalConnection' apps/desktop/src/renderer
rg -n --type=ts -C3 'writeRef\.current|write\(' apps/desktop/src/renderer
rg -n --type=ts -C3 'isStreamReadyRef' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TerminalRepository: superset-sh/superset
Length of output: 50376
🏁 Script executed:
#!/bin/bash
# Get the exact lines in question
sed -n '480,515p' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
# Look for the writeRef usage context - check if it's subject to readiness gating
rg -n 'writeRef\.current\(' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx -A 2 -B 2
# Check how events are flushed and understand the queuing mechanism
rg -n 'flushPendingEvents|pendingEventsRef' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalStream.ts -A 2 -B 2Repository: superset-sh/superset
Length of output: 2416
🏁 Script executed:
#!/bin/bash
# Find the writeMutation definition and see if it has any buffering/gating
rg -n 'writeMutation|useMutation' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalConnection.ts -A 5 -B 2
# Check if there's any check for session readiness before accepting writes
rg -n 'isStreamReady|session.*ready|attach' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx -B 2 -A 2 | head -100
# Look for any input buffering or queueing in the component
rg -n 'buffer|queue.*input|input.*queue' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal -iRepository: superset-sh/superset
Length of output: 16215
🏁 Script executed:
#!/bin/bash
# Check attach-scheduler to see if it manages session readiness
cat apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/attach-scheduler.ts
# Check handleStartShell to see when it subscribes to the stream
rg -n 'handleStartShell' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx -A 15 | head -50
# Look for any input validation/buffering in handleWrite or before writeRef is called
rg -n 'const handleWrite|handleKeyPress|onData' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx -A 10 | head -80Repository: superset-sh/superset
Length of output: 9287
🏁 Script executed:
#!/bin/bash
# Check if there's any gating of writeRef calls based on session readiness
rg -n 'handleTerminalInput|onData' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx -B 5 -A 10 | head -80
# Look at the actual terminal input handler
rg -n 'const handleTerminalInput' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx -A 8Repository: superset-sh/superset
Length of output: 1846
🏁 Script executed:
#!/bin/bash
# Check if there's any buffering on the tRPC write mutation or backend
# First, let's see if tRPC mutation implementation has any built-in buffering
rg -n 'terminal.write' apps/desktop/src -l
# Check the backend implementation if available
fd -t f -e ts 'router' apps/main/src | head -5Repository: superset-sh/superset
Length of output: 564
🏁 Script executed:
#!/bin/bash
# Check the backend write implementation
cat apps/desktop/src/lib/trpc/routers/terminal/terminal.ts | head -200
# Look for session.write or input handling
rg -n 'write.*session|session.*write|input.*buffer|buffer.*input' apps/desktop/src/lib/trpc/routers/terminal/terminal.ts -B 2 -A 5Repository: superset-sh/superset
Length of output: 5902
🏁 Script executed:
#!/bin/bash
# Get more of the write implementation
sed -n '185,230p' apps/desktop/src/lib/trpc/routers/terminal/terminal.ts
# Check the stream subscription to understand when isStreamReadyRef gets set to true
rg -n 'stream.*subscription|stream.*mutate' apps/desktop/src/lib/trpc/routers/terminal/terminal.ts -B 3 -A 15 | head -100
# Check the underlying terminal.write implementation
rg -n 'terminal\.write' apps/desktop/src/main/lib/terminal -lRepository: superset-sh/superset
Length of output: 1084
🏁 Script executed:
#!/bin/bash
# Check handleStartShell to see if/when stream subscription starts
rg -n 'const handleStartShell' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalColdRestore.ts -A 30
# Check when stream subscription is enabled/started
rg -n 'useSubscription\|enabled.*true' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx -B 3 -A 3Repository: superset-sh/superset
Length of output: 2094
🏁 Script executed:
#!/bin/bash
# Get the complete handleStartShell function
sed -n '172,240p' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalColdRestore.ts
# Check when useSubscription actually subscribes (before or after createOrAttach)
sed -n '240,250p' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
# Look for how the terminal.write backend handles session initialization
cat apps/desktop/src/main/lib/terminal.ts | head -100Repository: superset-sh/superset
Length of output: 2414
🏁 Script executed:
#!/bin/bash
# Find the terminal implementation
find apps/desktop/src/main -name "*terminal*" -type f | head -10
# Check if there's buffering in the pty implementation
rg -n 'write.*pty|pty.*write|input.*buffer' apps/desktop/src/main/lib -iRepository: superset-sh/superset
Length of output: 2407
🏁 Script executed:
#!/bin/bash
# Examine the PtyWriteQueue implementation
cat apps/desktop/src/main/lib/terminal/pty-write-queue.ts
# Check how manager.write() handles non-existent sessions
rg -n 'write.*session|write\(' apps/desktop/src/main/lib/terminal/manager.ts -B 2 -A 8 | head -100Repository: superset-sh/superset
Length of output: 4942
🏁 Script executed:
#!/bin/bash
# Check if isRestoredModeRef is properly set during cold restore
rg -n 'isRestoredModeRef' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx -B 1 -A 1
# Check when isRestoredModeRef is set/cleared
rg -n 'isRestoredModeRef.*true|isRestoredModeRef.*false' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalColdRestore.ts -B 1 -A 1
# Check the handleTerminalInput gate more carefully
sed -n '399,407p' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxRepository: superset-sh/superset
Length of output: 901
🏁 Script executed:
#!/bin/bash
# Check when isRestoredMode is set/used during cold restore
rg -n 'isRestoredMode\|setIsRestoredMode' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal -B 2 -A 2 | head -150
# Check if there's any readiness gate between scrollback write and session creation
sed -n '486,520p' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxRepository: superset-sh/superset
Length of output: 1173
🏁 Script executed:
#!/bin/bash
# Find where isRestoredMode state is defined and initialized
rg -n 'useState.*isRestoredMode\|const.*isRestoredMode.*useState' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal
# Check the full useTerminalColdRestore return to see if it manages isRestoredMode state
rg -n 'setIsRestoredMode' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalColdRestore.ts -B 3 -A 1
# See the complete picture - check if isRestoredMode is a return value from the hook
sed -n '50,70p' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalColdRestore.tsRepository: superset-sh/superset
Length of output: 1481
🏁 Script executed:
#!/bin/bash
# Check if isRestoredMode is ever set to true before cold restore processing
rg -n 'setIsRestoredMode.*true' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal
# Check the complete flow - when/where does cold restore mode get activated?
rg -n 'coldRestoreState\|isColdRestore' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx -B 1 -A 1 | head -100
# Verify: is isRestoredMode supposed to be set to true somewhere?
rg -n 'setIsRestoredMode.*true' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks -B 3 -A 1Repository: superset-sh/superset
Length of output: 46
Input loss window during cold restore session transition.
While PtyWriteQueue does buffer outgoing writes, user keystrokes between xterm.write(scrollback) and new session creation can be lost. The gate isRestoredModeRef.current in handleTerminalInput (line 400) never evaluates to true—isRestoredMode is initialized to false and only reset to false; it's never set to true. This leaves input unguarded during the createOrAttachRef() async transition, causing writes to fail when manager.write() cannot find the old session (throwing "Terminal session not found or not alive"). The error is caught and emitted, but the keystroke is dropped. Consider actually enabling isRestoredMode during cold restore or adding explicit isStreamReadyRef gating to prevent writes until the new session is fully attached.
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx`
around lines 486 - 496, During cold-restore, input can be dropped because
isRestoredModeRef is never set and writes occur while the old session is gone;
fix by enabling a gating flag during the restore-and-attach window and flushing
buffered writes only after the new session is attached: set
isRestoredModeRef.current = true (or set isStreamReadyRef.current = false)
before calling xterm.write(...) in the cold-restore branch (the block that reads
coldRestoreState and calls handleStartShell/createOrAttachRef), keep the flag
set until createOrAttachRef() (or the promise returned by handleStartShell)
completes successfully, then clear the flag and flush the PtyWriteQueue so
manager.write() is only attempted when the new session is live; update
handleTerminalInput to check isRestoredModeRef (or isStreamReadyRef) and queue
inputs instead of calling manager.write when the gate is active.
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
Test plan
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.