fix(ssh): restore cleared bindings on rollback, bound the hoist ledger

F3 (P2): the supersession undo restored lease state but not the pane bindings
the forward path cleared, so a failed flush left a predecessor holding a live
lease with no binding — and the clear queues its own save, writing that hybrid
to disk. Now snapshots via the same helpers the async twin already uses.

F5 (P3): sshHoistedTabIds grew for the life of the profile and, because the CLI
can reuse a tab id after its tab closes, a reused id was stranded forever.
Bounded to the newest 512 — far above any realistic open-tab count, and the
worst case of eviction is re-hoisting a long-closed tab once.

F4 (P3): the 'counterfactual' test no longer isolates mayCreate now that the
hoist runs before it, so it is relabelled as the end-to-end assertion it
actually is rather than left claiming a property it does not test.
This commit is contained in:
Neil
2026-08-13 20:17:07 -07:00
parent 38f660f65c
commit 4716cc2fea
2 changed files with 18 additions and 5 deletions
+13 -1
View File
@@ -2850,6 +2850,9 @@ export type PtyBindingSourceExpectation = {
}
/** Caps the hoist ledger. Far above any realistic open-tab count, so a live tab is never evicted. */
const MAX_HOISTED_TAB_IDS = 512
/** Expired leases are unreachable from reattach; kept only long enough to outlive any in-flight
* recovery window by orders of magnitude. */
const EXPIRED_SSH_LEASE_RETENTION_MS = 7 * 24 * 60 * 60 * 1000
@@ -2941,7 +2944,12 @@ export function hoistSshPartitionsIntoLocalSession(
}
}
if (changed) {
state.settings = { ...state.settings, sshHoistedTabIds: [...alreadyHoisted] }
// Bounded, newest-last. A tab id can be reused by the CLI after its tab closes, and an
// unbounded list would both grow for the life of the profile and strand such a pane forever.
// Dropping the oldest entries only risks re-hoisting a long-closed tab once, which is the
// milder failure of the two.
const bounded = [...alreadyHoisted].slice(-MAX_HOISTED_TAB_IDS)
state.settings = { ...state.settings, sshHoistedTabIds: bounded }
}
return changed
}
@@ -7631,11 +7639,15 @@ 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.
// Snapshot before clearing: the clear mutates live pane state (tab.ptyId, ptyIdsByLeafId), so
// restoring only the lease rows would leave a predecessor holding a live lease with no binding.
const bindingsBefore = this.snapshotSshLeaseBindings(winner.targetId, superseded)
this.clearSshRemotePtyBindingsForLeases(winner.targetId, superseded, 'local')
return () => {
for (const undo of restore) {
undo()
}
this.restoreSshLeaseBindings(winner.targetId, superseded, bindingsBefore)
}
}
@@ -137,10 +137,11 @@ describe('an SSH pane whose membership was persisted into the ssh partition', ()
).toBe(true)
})
// The counterfactual that makes this a REGRESSION rather than a standing limitation: before
// `mayCreate` existed, the same state re-created the tab instead of discarding it. That is what
// "before, many of the tabs were retained" described — they were being recreated.
it('was recreated rather than discarded before the refusal was introduced', async () => {
// NOTE: with the hoist in place this no longer isolates `mayCreate` — the reopen has already
// folded the tab into local, so the bind would succeed either way. Kept as a plain end-to-end
// assertion that the upgraded profile can bind, not as a counterfactual. The counterfactual it
// used to encode is now carried by the hoist's own mutation test.
it('binds without needing to create, once the pane has been hoisted', async () => {
const before = await createStore()
before.setWorkspaceSession(paneSession() as never, SSH_PARTITION)
before.flushOrThrow()