From 614ea3f6a99ee736957791d67669fbb4a69de7a1 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 16:25:10 -0700 Subject: [PATCH] fix(session): an emptied workspace still survives the reconnect merge localActiveWorkspaceSurvives decided whether the workspace the user is standing in still exists by counting its tabs. A workspace they had just closed the last terminal in reads as zero, so it "did not survive", the host's null activeWorktreeId was taken literally, and the reconnect dropped them onto the home screen -- for closing a tab. An explicit empty tabsByWorktree row is the record that the user closed the last terminal, not the absence of a workspace (initial-terminal.ts). hasLocalTabsRow two hundred lines up in this same function already draws that distinction with Object.hasOwn; this line was simply missed. The counterweight is pinned too: presence must not turn into "never follow the host", so a host that does name an active worktree still wins over the emptied local one. --- ...space-session-merge-local-survival.test.ts | 33 +++++++++++++++++++ .../hooks/remote-workspace-session-merge.ts | 6 +++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/renderer/src/hooks/remote-workspace-session-merge-local-survival.test.ts b/src/renderer/src/hooks/remote-workspace-session-merge-local-survival.test.ts index 361493b34ef..95b4004fa72 100644 --- a/src/renderer/src/hooks/remote-workspace-session-merge-local-survival.test.ts +++ b/src/renderer/src/hooks/remote-workspace-session-merge-local-survival.test.ts @@ -353,6 +353,39 @@ describe('local rows the snapshot carries no answer for', () => { expect(merged.tabsByWorktree[WORKTREE]).toEqual([]) }) + it('keeps the user standing in the workspace they emptied', () => { + // Same row, one level up. `localActiveWorkspaceSurvives` counted tabs, so the workspace the + // user had just closed the last terminal in read as "did not survive the merge" and the host's + // null active worktree was taken literally — the home screen, for closing a tab. + const current = sessionState({ tabsByWorktree: { [WORKTREE]: [] } }) + const remote = sessionState({ + activeWorktreeId: null, + activeWorkspaceKey: null, + activeRepoId: null, + tabsByWorktree: { [WORKTREE]: [] } + }) + + const merged = merge(current, remote, { [WORKTREE]: [] }) + + expect(merged.activeWorktreeId).toBe(WORKTREE) + expect(merged.activeWorkspaceKey).toBe(worktreeWorkspaceKey(WORKTREE)) + expect(merged.activeRepoId).toBe('repo-1') + }) + + it('still lets the host move the user off a workspace it does name one for', () => { + // The counterweight: presence must not turn into "never follow the host". A host that names an + // active worktree still wins over the emptied local one. + const current = sessionState({ tabsByWorktree: { [WORKTREE]: [] } }) + const remote = sessionState({ + activeWorktreeId: OTHER_WORKTREE, + tabsByWorktree: { [WORKTREE]: [] } + }) + + const merged = merge(current, remote, { [WORKTREE]: [] }) + + expect(merged.activeWorktreeId).toBe(OTHER_WORKTREE) + }) + it('invents no row for a worktree neither side has one for', () => { // The counterweight: presence has to come from a real local row, not from membership in the // replace set, or a never-initialized workspace gets a tombstone it never earned. diff --git a/src/renderer/src/hooks/remote-workspace-session-merge.ts b/src/renderer/src/hooks/remote-workspace-session-merge.ts index fa247d8bbbd..71b4e7d1884 100644 --- a/src/renderer/src/hooks/remote-workspace-session-merge.ts +++ b/src/renderer/src/hooks/remote-workspace-session-merge.ts @@ -258,10 +258,14 @@ export function mergeDirectSshRemoteWorkspaceSession( // the workspace or its path did not resolve to a local id, and taking that literally drops the // user onto the home screen while their terminals keep running. So it is only overridden when the // workspace they are standing in demonstrably still exists in the merged result. + // Why presence and not length, the same reading `hasLocalTabsRow` above already gives: an + // explicit empty row is the record that the user closed the last terminal, and a workspace they + // emptied still exists in the merged result. Counting rows sent them to the home screen for + // having closed their last tab. const localActiveWorkspaceSurvives = current.activeWorktreeId != null && replaceWorktreeIds.has(current.activeWorktreeId) && - (tabsByWorktree[current.activeWorktreeId]?.length ?? 0) > 0 + Object.hasOwn(tabsByWorktree, current.activeWorktreeId) const preservedActiveWorktreeId = localActiveWorkspaceSurvives ? current.activeWorktreeId : null // The three active-* fields have to describe ONE workspace, so they are all derived from whichever // worktree wins rather than each choosing a source. Taking the repo from the host while the