Fix terminal state loss when switching workspaces #2717

stevenvo · 2025-12-26T10:09:35Z

Summary

Fixes issue where TUI applications (vim, htop, opencode, etc.) would lose terminal state when switching between workspaces, causing inability to scroll and display corruption.

Problem

When switching workspaces:

All tab views were destroyed via removeAllChildViews()
Terminal views (xterm.js instances) were recreated from cache
Terminal state was lost, including:
- Alternate screen buffer mode (used by TUI apps)
- Scrollback buffer and scrolling capability
- Terminal display state

This made it impossible to scroll in terminals after switching back to a workspace with running TUI applications.

Root Cause

The switchworkspace operation called removeAllChildViews() which destroyed all tab views including terminals. When recreated, the xterm.js terminal instances lost their state even though the backend shell processes continued running.

Tab switching vs Workspace switching:

Tab switching: Repositions views (preserves state) ✅
Workspace switching: Destroys and recreates views (loses state) ❌

Solution

Cache tab views across workspace switches instead of destroying them:

On workspace switch: Move tab views to allTabViewsCache and position off-screen
On switch back: Reuse cached view if available
Cleanup: Destroy cached views only when tab explicitly closed or window closes

This preserves:

Terminal buffer state (normal and alternate screen modes)
Scrollback history and scrolling capability
Running processes and their output
Cursor position and all terminal modes

Changes

// emain/emain-window.ts
+ allTabViewsCache: Map<string, WaveTabView>  // Cache field

// Workspace switch: cache instead of destroy
- this.removeAllChildViews()
+ for (const [tabId, tabView] of this.allLoadedTabViews.entries()) {
+     tabView.positionTabOffScreen(bounds)
+     this.allTabViewsCache.set(tabId, tabView)
+ }

// Tab load: check cache first
+ let tabView = this.allTabViewsCache.get(tabId)
+ if (tabView) {
+     this.allTabViewsCache.delete(tabId)  // Reuse
+ } else {
+     [tabView, tabInitialized] = await getOrCreateWebViewForTab(...)
+ }

Memory Management

Cached views kept alive indefinitely (like traditional terminal apps)
Destroyed only when: tab explicitly closed OR window closed
Typical memory: 1-5MB per cached terminal
No timeout - can switch back after hours

Testing

Tested with opencode (TUI app using @OpenTui):

✅ Run opencode in workspace A
✅ Switch to workspace B
✅ Switch back to workspace A (even after hours)
✅ Terminal state preserved, scrolling works correctly
✅ No display corruption

Additional Changes

Includes StreamCancelFn type fix from #2716 to ensure this branch builds correctly. This fix corrects the type definition in wshrpctypes.go from func() to func(context.Context) error.

Behavior Match

Now matches macOS Terminal, iTerm2, and other terminal emulators where terminal state is preserved across workspace/tab switches.

chatgpt-codex-connector · 2025-12-26T10:09:44Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

coderabbitai · 2025-12-26T10:09:46Z

Walkthrough

Adds a per-window tab-view cache (allTabViewsCache: Map<string, WaveTabView>) to WaveBrowserWindow. During workspace switches, loaded tab views are moved into the cache and positioned off-screen instead of being destroyed. When activating a tab after a workspace switch, the cache is consulted first to reuse a cached view; otherwise a new WebView is created. Tab removal and window teardown now handle cached views (destroying and clearing them). Comments document terminal/state preservation and cached view lifecycle across workspace switches.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The pull request title accurately and concisely summarizes the main change: fixing terminal state loss when switching workspaces, which is the core objective of the changeset.
Description check	✅ Passed	The pull request description is comprehensive and directly related to the changeset, providing problem statement, root cause analysis, solution details, testing evidence, and implementation specifics.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 90011a7 and 562732e.

⛔ Files ignored due to path filters (1)

package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (1)

emain/emain-window.ts

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-10-15T03:18:52.647Z

Learnt from: sawka
Repo: wavetermdev/waveterm PR: 2433
File: emain/emain-window.ts:811-828
Timestamp: 2025-10-15T03:18:52.647Z
Learning: In emain/emain-window.ts, within the relaunchBrowserWindows function, ClientService.GetClientData() is guaranteed to return a valid client object and never null/undefined. The backend ensures a client is initialized before startup, so no null-guard is needed when accessing clientData.windowids.

Applied to files:

emain/emain-window.ts

🔇 Additional comments (5)

emain/emain-window.ts (5)

142-142: LGTM!

The cache field is well-documented and properly initialized alongside the existing allLoadedTabViews map. Using a Map<string, WaveTabView> provides efficient O(1) lookup by tab ID.

Also applies to: 222-222

357-361: LGTM!

Proper cleanup of cached views on window teardown. Destroying each cached view before clearing the map prevents memory leaks.

586-610: Good defensive handling for rapid workspace switching.

The edge case handling at lines 594-598 correctly destroys any existing cached view before caching the new one, preventing memory leaks during rapid workspace switches. The decision to keep views as children of contentView (positioned off-screen) rather than removing them is sound for preserving WebView/xterm.js state.

615-623: LGTM!

The cache-first lookup correctly reuses existing tab views, and setting tabInitialized = true ensures the view bypasses full reinitialization, preserving terminal state. Removing from cache on reuse prevents double-ownership.

679-685: The function is correctly designed for active tabs only, not cached ones.

getWaveWindowByTabId only checks allLoadedTabViews by design. It's only called via getWaveWindowByWebContentsId, which retrieves tabs from wcIdToWaveTabMap—a map containing only active tabs with running web content. Active tabs always exist in allLoadedTabViews and never in allTabViewsCache (cached tabs lack active webContents). While similar operations like removeTabView defensively check both maps, the current behavior is intentional and safe for this lookup's usage pattern.

emain/emain-window.ts

Fixes issue where TUI applications (vim, htop, opencode, etc.) would lose terminal state when switching between workspaces. This caused inability to scroll and display corruption. Root cause: Workspace switching was destroying all tab views including terminals, then recreating them from cache. This destroyed xterm.js instances and lost their state. Solution: Cache tab views across workspace switches instead of destroying them. Tab views are positioned off-screen but kept alive, preserving: - Terminal buffer state (normal and alternate screen modes) - Scrollback history and scrolling capability - Running processes and their output - Cursor position and all terminal modes Memory management: Cached views kept alive until tab closed or window closed. Note: This PR includes the StreamCancelFn type fix from wavetermdev#2716 to ensure the branch builds correctly.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

emain/emain-window.ts (1)
357-361: Missing removeChildView for cached views before destroy.

Cached views remain as children of contentView (just positioned off-screen). For consistency with the loop above (lines 351-356) and to avoid relying on implicit cleanup during window destruction, consider removing them from contentView before destroying.
🔎 Proposed fix
         // Also destroy any cached views
         for (const tabView of this.allTabViewsCache.values()) {
+            if (!this.isDestroyed()) {
+                this.contentView.removeChildView(tabView);
+            }
             tabView?.destroy();
         }
         this.allTabViewsCache.clear();

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 562732e and 978443c.

⛔ Files ignored due to path filters (1)

package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (1)

emain/emain-window.ts

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-10-15T03:18:52.647Z

Learnt from: sawka
Repo: wavetermdev/waveterm PR: 2433
File: emain/emain-window.ts:811-828
Timestamp: 2025-10-15T03:18:52.647Z
Learning: In emain/emain-window.ts, within the relaunchBrowserWindows function, ClientService.GetClientData() is guaranteed to return a valid client object and never null/undefined. The backend ensures a client is initialized before startup, so no null-guard is needed when accessing clientData.windowids.

Applied to files:

emain/emain-window.ts

🧬 Code graph analysis (1)

emain/emain-window.ts (1)

emain/emain-tabview.ts (2)

WaveTabView (106-225)

getOrCreateWebViewForTab (295-349)

🔇 Additional comments (5)

emain/emain-window.ts (5)

142-142: LGTM!

The cache declaration is well-typed and the comment clearly documents its purpose for preserving tab views across workspace switches.

222-222: LGTM!

Consistent initialization alongside allLoadedTabViews.

586-610: Good handling of workspace switch with proper edge case protection.

The implementation correctly:

Preserves terminal state by caching instead of destroying

Handles rapid workspace switching by destroying stale cached views

Documents the lifecycle clearly

The design choice to keep cached views as children of contentView (just off-screen) is valid and simpler than removing/re-adding.

615-625: LGTM!

Cache-first lookup pattern is correctly implemented. Cached views are properly removed from the cache and will be re-added to allLoadedTabViews via setTabViewIntoWindow.

655-673: Good fix - addresses the previous review comment.

The removeChildView call is now correctly placed after the if-else block (lines 668-671), ensuring cached views are removed from contentView before being destroyed. This properly handles the case where cached views remain as children of contentView (just positioned off-screen).

stevenvo · 2025-12-27T08:19:50Z

Closing temporarily for additional debugging and testing. Will reopen after issues are resolved.

coderabbitai bot reviewed Dec 26, 2025

View reviewed changes

emain/emain-window.ts Show resolved Hide resolved

stevenvo force-pushed the fix/preserve-terminal-state-workspace-switch branch from 562732e to 978443c Compare December 26, 2025 10:19

coderabbitai bot reviewed Dec 26, 2025

View reviewed changes

stevenvo closed this Dec 27, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix terminal state loss when switching workspaces #2717

Fix terminal state loss when switching workspaces #2717

Uh oh!

stevenvo commented Dec 26, 2025

Uh oh!

chatgpt-codex-connector bot commented Dec 26, 2025

Uh oh!

coderabbitai bot commented Dec 26, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

stevenvo commented Dec 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Fix terminal state loss when switching workspaces #2717

Fix terminal state loss when switching workspaces #2717

Uh oh!

Conversation

stevenvo commented Dec 26, 2025

Summary

Problem

Root Cause

Solution

Changes

Memory Management

Testing

Additional Changes

Behavior Match

Uh oh!

chatgpt-codex-connector bot commented Dec 26, 2025

Uh oh!

coderabbitai bot commented Dec 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

stevenvo commented Dec 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai bot commented Dec 26, 2025 •

edited

Loading