diff --git a/src/main/persistence.ts b/src/main/persistence.ts index 5781bcd379b..6c954cfe710 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -7344,7 +7344,7 @@ export class Store { } // Why: matching on lease ptyId first means this scrubs only the predecessor's // stale binding — the winner's own binding cannot match and is left intact. - this.clearSshRemotePtyBindingsForLeases(winner.targetId, superseded) + this.clearSshRemotePtyBindingsForLeases(winner.targetId, superseded, 'local') } /** @@ -7453,7 +7453,7 @@ export class Store { return 0 } const sessionsBefore = this.cloneSshLeaseBindingSessions(targetId) - this.clearSshRemotePtyBindingsForLeases(targetId, superseded) + this.clearSshRemotePtyBindingsForLeases(targetId, superseded, 'local') try { // Why flushOrThrow: flush() swallows write errors, which would leave these // leases retired in memory but attached on disk for the rest of the session. @@ -7632,19 +7632,27 @@ export class Store { this.clearSshRemotePtyBindingsForLeases(targetId, leases ?? []) } + /** `arbitratedFrom` names the plane a supersession DECISION was made from. A decision reached by + * reading one plane may only mutate that plane: the other plane's binding never got a vote, and + * deleting it strands a shell its owner can still be using. An explicit expiry or termination is + * plane-agnostic — the pty is gone for everyone — so those callers pass nothing. */ private clearSshRemotePtyBindingsForLeases( targetId: string, - leases: SshRemotePtyLease[] + leases: SshRemotePtyLease[], + arbitratedFrom?: 'local' ): boolean { if (!leases?.length) { return false } let changed = false const sessions = new Set( - [ - this.state.workspaceSession, - this.state.workspaceSessionsByHostId?.[toSshExecutionHostId(targetId)] - ].filter((session): session is WorkspaceSessionState => Boolean(session)) + (arbitratedFrom === 'local' + ? [this.state.workspaceSession] + : [ + this.state.workspaceSession, + this.state.workspaceSessionsByHostId?.[toSshExecutionHostId(targetId)] + ] + ).filter((session): session is WorkspaceSessionState => Boolean(session)) ) for (const session of sessions) { for (const [worktreeId, tabs] of Object.entries(session.tabsByWorktree ?? {})) { diff --git a/src/main/ssh-pane-binding-partition.test.ts b/src/main/ssh-pane-binding-partition.test.ts index 9ea523e4a08..bada2d1812f 100644 --- a/src/main/ssh-pane-binding-partition.test.ts +++ b/src/main/ssh-pane-binding-partition.test.ts @@ -225,6 +225,29 @@ describe('STA-3077 step P: one pane, one live claim across partitions', () => { }) }) +describe('STA-3077: a decision made from one plane only mutates that plane', () => { + // Arbitration ranks using the desktop plane's binding. The headless plane's binding never gets a + // vote, so deleting it here would strand a shell its owner may still be using — and that owner + // would then have no durable record to reattach by. An explicit expiry is different: the pty is + // gone for everyone, and that path still scrubs both. + it('leaves the headless plane binding intact when supersession retires its lease', async () => { + const sshSession = paneSession('pty-old') as unknown as Record + const store = await createStore({ + workspaceSession: paneSession('pty-new'), + workspaceSessionsByHostId: { [SSH_PARTITION]: sshSession } + }) + sshSpawnUpsertsLease(store, 'pty-old') + + // Local names pty-new, so arbitration expires pty-old without the other plane being consulted. + sshSpawnUpsertsLease(store, 'pty-new') + + expect(liveLeaseIdsForPane(store)).toEqual(['pty-new']) + expect( + store.getWorkspaceSession(SSH_PARTITION).terminalLayoutsByTabId?.[TAB]?.ptyIdsByLeafId?.[LEAF] + ).toBe(appPtyId('pty-old')) + }) +}) + describe('STA-3077: a pane moved between tabs still supersedes its own predecessor', () => { // The reported cardinality growth, in its surviving form. A lease freezes its tabId at write // time. Break the pane out into a new tab and its next lease carries the NEW tab, so matching @@ -314,7 +337,10 @@ describe('STA-3077 step P: the desktop plane resolves from its own home', () => rendererPublishesPane(store, 'pty-1') expect(relayReattachBindsPane(store, 'pty-2')).toBe(true) - // The other plane still names the predecessor, and is deliberately left alone. + // The other plane names the predecessor. It does not get a vote here — local speaking about + // this leaf IS the desktop plane's live view, and letting the other copy outrank it is the + // STA-3077 defect. Its binding to the loser is then scrubbed with every other reference to + // that retired lease, so the two planes do not drift further apart. somethingRewritesTheSshPartition(store, 'pty-1') sshSpawnUpsertsLease(store, 'pty-2')