fix(ssh): resolve a pane's binding from the target partition, not the stale local copy (#18546)

One SSH pane accumulated one extra reattachable lease per relay restart (2, 3, 4,
5, 6 across five), and every one of them costs a `pty.attach` round trip on every
later connect, forever. Nothing prunes `sshRemotePtyLeases`, so the fan-out only
grows.

`supersedeSiblingLeasesForPane` is fenced on the PTY the pane is durably bound to,
and `durablyBoundPtyIdForPane` read `state.workspaceSession` (local) before
`workspaceSessionsByHostId['ssh:<target>']`. But `persistPtyBinding(binding, hostId)`
updates ONLY the host partition:

  AFTER-PERSIST  local= ssh:t@@pty2:old:1   host= ssh:t@@pty2:new:1

So for the length of a reconnect the local copy still names the predecessor, the
fence resolved to it, supersession took an already-`expired` lease as its winner,
and returned having marked nothing. Both partitions agree again once the renderer
republishes its layout, which is why the settled store looks consistent and hid
this.

Read both partitions as an ordered list, target's own first, and test the fence by
membership rather than by equality with whichever was read first. Pick the winner
preferring a lease this client still has a route to, since the stale partition
names an expired one. Never retire a lease that is both bound and live, so a
partition disagreement can't strand a running remote process.

Superseded predecessors stay `expired` and are never `terminated`: losing a lease
is not evidence the shell died (docs/reference/ssh-execution-boundary.md). A pane
with no binding is skipped rather than pruned, so a genuine orphan stays askable.

Also re-runs supersession from the binding side after each spawn commit's binding
write, so the lease/binding order at a call site no longer decides, and reconciles
every pane for a target immediately before `reattachKnownPtys` reads the set it
feeds to `pty.attach` — that repairs stores which already accumulated these rows.

The guard suite could not catch this: every assertion bound the pane BEFORE
upserting the lease, an order no caller uses. Rewritten to the spawn commits' real
order (lease, then binding, then the binding-side trigger); it fails 8 assertions
without this change. Added a suite that drives the real `persistPtyIpcSpawnCommit`
rather than the store primitives, including the exact stale-partition state written
by production's own binding writer.

Verified on the Docker SSH lane: five `relay.js` SIGKILLs with recovery between
each, reattachable leases flat at one per pane.

Note: this bounds the reattach SET, not the store. `sshRemotePtyLeases` still has
no cap or TTL and rows still accumulate; pruning is left alone deliberately, since
an `expired` row without `supersededBy` is a genuine orphan and must not be dropped
on age.
This commit is contained in:
Neil
2026-09-03 16:47:32 -07:00
committed by GitHub
parent e85ebb0086
commit e42c60e8a3
21 changed files with 777 additions and 137 deletions
@@ -324,6 +324,7 @@ describe('registerPtyHandlers', () => {
}
const store = {
upsertSshRemotePtyLease: vi.fn(),
supersedeSshRemotePtyLeasesForBoundPane: vi.fn(),
persistPtyBinding: vi.fn(),
removeSshRemotePtyLease: vi.fn(),
markSshRemotePtyLease: vi.fn(),
@@ -345,6 +345,7 @@ describe('registerPtyHandlers', () => {
)
const store = {
upsertSshRemotePtyLease: vi.fn(),
supersedeSshRemotePtyLeasesForBoundPane: vi.fn(),
persistPtyBinding: vi.fn()
}
registerSshPtyProvider('ssh-1', {
@@ -124,6 +124,7 @@ describe('registerPtyHandlers', () => {
flushOrThrow: vi.fn(),
persistPtyBinding: vi.fn(),
upsertSshRemotePtyLease: vi.fn(),
supersedeSshRemotePtyLeasesForBoundPane: vi.fn(),
removeSshRemotePtyLease: vi.fn(),
markSshRemotePtyLease: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn()
@@ -482,6 +483,7 @@ describe('registerPtyHandlers', () => {
} as never)
const store = {
upsertSshRemotePtyLease: vi.fn(),
supersedeSshRemotePtyLeasesForBoundPane: vi.fn(),
persistPtyBinding: vi.fn(),
removeSshRemotePtyLease: vi.fn(),
markSshRemotePtyLease: vi.fn(),
@@ -154,6 +154,7 @@ describe('registerPtyHandlers', () => {
} as never)
const store = {
upsertSshRemotePtyLease: vi.fn(),
supersedeSshRemotePtyLeasesForBoundPane: vi.fn(),
persistPtyBinding: vi.fn(),
removeSshRemotePtyLease: vi.fn(),
markSshRemotePtyLease: vi.fn(),
@@ -260,6 +261,7 @@ describe('registerPtyHandlers', () => {
} as never)
const store = {
upsertSshRemotePtyLease: vi.fn(),
supersedeSshRemotePtyLeasesForBoundPane: vi.fn(),
persistPtyBinding: vi.fn()
}
let controller: RuntimeSpawnController | null = null
@@ -370,6 +372,7 @@ describe('registerPtyHandlers', () => {
} as never)
const store = {
upsertSshRemotePtyLease: vi.fn(),
supersedeSshRemotePtyLeasesForBoundPane: vi.fn(),
persistPtyBinding: vi.fn(() => {
throw new Error('disk full')
}),
@@ -471,6 +474,7 @@ describe('registerPtyHandlers', () => {
} as never)
const store = {
upsertSshRemotePtyLease: vi.fn(),
supersedeSshRemotePtyLeasesForBoundPane: vi.fn(),
persistPtyBinding: vi.fn(),
removeSshRemotePtyLease: vi.fn(),
markSshRemotePtyLease: vi.fn(),
@@ -577,6 +581,7 @@ describe('registerPtyHandlers', () => {
} as never)
const store = {
upsertSshRemotePtyLease: vi.fn(),
supersedeSshRemotePtyLeasesForBoundPane: vi.fn(),
persistPtyBinding: vi.fn(),
removeSshRemotePtyLease: vi.fn(),
markSshRemotePtyLease: vi.fn(),
@@ -108,6 +108,7 @@ describe('registerPtyHandlers', () => {
} as never)
const store = {
upsertSshRemotePtyLease: vi.fn(),
supersedeSshRemotePtyLeasesForBoundPane: vi.fn(),
persistPtyBinding: vi.fn(),
removeSshRemotePtyLease: vi.fn(),
markSshRemotePtyLease: vi.fn(),
@@ -212,6 +213,7 @@ describe('registerPtyHandlers', () => {
} as never)
const store = {
upsertSshRemotePtyLease: vi.fn(),
supersedeSshRemotePtyLeasesForBoundPane: vi.fn(),
persistPtyBinding: vi.fn(() => {
throw new Error('disk full')
}),
@@ -386,6 +386,7 @@ describe('registerPtyHandlers', () => {
it('ignores fire-and-forget IPC for detached SSH PTYs without a provider', async () => {
const store = {
upsertSshRemotePtyLease: vi.fn(),
supersedeSshRemotePtyLeasesForBoundPane: vi.fn(),
persistPtyBinding: vi.fn(),
markSshRemotePtyLease: vi.fn(),
clearSshRemotePtyKillIntent: vi.fn()
@@ -134,6 +134,13 @@ export async function persistPtyIpcSpawnCommit(ctx: PtyIpcSpawnState): Promise<{
})
}
}
// Why here and not at the upsert: this path leases before it binds, so supersession fenced on the
// pane's binding still named the predecessor and bailed on every reconnect — one more reattachable
// lease, and one more `pty.attach`, per reconnect forever. Runs after whichever binding write this
// commit made, so the lease/binding order no longer decides.
if (ctx.deps.store && args.connectionId && ctx.validatedLeafId !== null) {
ctx.deps.store.supersedeSshRemotePtyLeasesForBoundPane(args.connectionId, ctx.validatedLeafId)
}
// Why: when the renderer has declared it will own the serializer for this paneKey, suppress the daemon-snapshot seed so its hydration path is sole authority (keyed on paneKey since the ptyId isn't known yet). See docs/mobile-prefer-renderer-scrollback.md.
const rendererPreSignaled = ctx.validatedPaneKey
? pendingByPaneKey.has(ctx.validatedPaneKey)
@@ -0,0 +1,289 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
import { rmSync, mkdtempSync } from 'node:fs'
import { join } from 'node:path'
import { tmpdir } from 'node:os'
import { testState, createStore } from '../../../persistence-test-harness'
import { TEST_LEAF_1, TEST_LEAF_2 } from '../../../persistence-session-fixtures'
import { sshRemotePtyLeaseAllowsReattach } from '../../../../shared/ssh-types'
import { toAppSshPtyId } from '../../../providers/ssh-pty-id'
import { toSshExecutionHostId } from '../../../../shared/execution-host'
import type { PtySpawnIpcArgs, PtySpawnIpcDeps } from './spawn-types'
import { createPtyIpcSpawnState } from './spawn-state'
import { persistPtyIpcSpawnCommit } from './spawn-commit-persist'
vi.mock('electron', () => ({
app: { getPath: () => testState.dir },
safeStorage: { isEncryptionAvailable: () => false }
}))
const TARGET = 'ssh-1'
const WORKTREE = 'repo1::/worktree'
const TAB = 'tab-1'
/**
* Drives the shipped IPC spawn commit rather than the store primitives it calls.
*
* The store-level suite could not catch this: it exercised bind-then-upsert, and this path does the
* opposite — it writes the lease row first so a force-quit in the renderer's debounce window cannot
* strand a running remote shell without one, then binds the pane. Supersession is fenced on the
* pane's binding, so under this real order it bailed on the predecessor every time and never re-ran,
* and each reconnect left one more reattachable lease for `reattachKnownPtys` to `pty.attach`.
*/
async function commitSshSpawn(
store: ReturnType<typeof createStore>,
args: { relayPtyId: string; leafId: string }
): Promise<void> {
const deps = { store } as unknown as PtySpawnIpcDeps
const spawnArgs = {
cols: 80,
rows: 24,
worktreeId: WORKTREE,
tabId: TAB,
leafId: args.leafId,
connectionId: TARGET
} as unknown as PtySpawnIpcArgs
const ctx = createPtyIpcSpawnState(deps, spawnArgs)
ctx.result = { id: toAppSshPtyId(TARGET, args.relayPtyId) }
ctx.validatedLeafId = args.leafId
await persistPtyIpcSpawnCommit(ctx)
}
/** One pane's layout, so the two host partitions can be given different bindings for one leaf. */
function sessionBinding(ptyId: string) {
return {
activeRepoId: 'repo1',
activeWorktreeId: WORKTREE,
activeTabId: TAB,
tabsByWorktree: {},
terminalLayoutsByTabId: {
[TAB]: {
root: { type: 'leaf' as const, leafId: TEST_LEAF_1 },
activeLeafId: TEST_LEAF_1,
expandedLeafId: null,
ptyIdsByLeafId: { [TEST_LEAF_1]: ptyId }
}
}
}
}
function bulkReattachPtyIds(store: ReturnType<typeof createStore>): string[] {
return store
.getSshRemotePtyLeases(TARGET)
.filter(sshRemotePtyLeaseAllowsReattach)
.map((lease) => lease.ptyId)
.sort()
}
describe('the IPC spawn commit keeps one reattachable lease per SSH pane', () => {
beforeEach(() => {
testState.dir = mkdtempSync(join(tmpdir(), 'orca-test-'))
})
afterEach(() => {
rmSync(testState.dir, { recursive: true, force: true })
})
// QA's measurement, driven through the real path: five relay restarts, one pane, N+1 leases.
it('holds the reattach set flat across five reconnects of one pane', async () => {
const store = await createStore()
for (let reconnect = 0; reconnect < 5; reconnect++) {
// A relay renumbers from `pty-1` on every start; a reconnect therefore re-leases the same
// pane under an id it has never used before.
await commitSshSpawn(store, { relayPtyId: `pty-${reconnect}`, leafId: TEST_LEAF_1 })
}
expect(bulkReattachPtyIds(store)).toEqual(['pty-4'])
})
it('retires each predecessor as `expired` with the winner recorded, never `terminated`', async () => {
const store = await createStore()
await commitSshSpawn(store, { relayPtyId: 'pty-0', leafId: TEST_LEAF_1 })
await commitSshSpawn(store, { relayPtyId: 'pty-1', leafId: TEST_LEAF_1 })
const predecessor = store.getSshRemotePtyLeases(TARGET).find((entry) => entry.ptyId === 'pty-0')
// `expired`, not `terminated`: losing the lease is not evidence the remote shell died, and the
// process is deliberately left running (docs/reference/ssh-execution-boundary.md).
expect(predecessor).toMatchObject({ state: 'expired', supersededBy: 'pty-1' })
})
// The failure that would be worse than the fan-out: over-superseding strands a live remote
// process behind a pane that can no longer find it.
it('leaves a genuine orphan reattachable while superseding the pane that re-leased', async () => {
const store = await createStore()
await commitSshSpawn(store, { relayPtyId: 'orphan-pty', leafId: TEST_LEAF_2 })
// The orphan's client lost its route; nothing observed the shell, so it stays askable.
store.markSshRemotePtyLease(TARGET, 'orphan-pty', 'expired')
await commitSshSpawn(store, { relayPtyId: 'pty-0', leafId: TEST_LEAF_1 })
await commitSshSpawn(store, { relayPtyId: 'pty-1', leafId: TEST_LEAF_1 })
const orphan = store.getSshRemotePtyLeases(TARGET).find((entry) => entry.ptyId === 'orphan-pty')
expect(orphan?.supersededBy).toBeUndefined()
expect(bulkReattachPtyIds(store)).toEqual(['orphan-pty', 'pty-1'])
})
/**
* The shape the Docker lane exposed, and the reason a spawn-time trigger is not enough on its
* own. When the spawn commit writes no binding, the renderer's debounced layout publish does it
* later — so at commit time the pane still names the predecessor and supersession correctly
* declines. Nothing revisited it afterwards, and the predecessor stayed reattachable forever.
*
* Measured rows agreed on target, worktree, tab and leaf and still carried no `supersededBy`.
*/
it('retires a predecessor whose successor bound the pane after the spawn commit', async () => {
const store = await createStore()
await commitSshSpawn(store, { relayPtyId: 'pty2:aaa:1', leafId: TEST_LEAF_1 })
// What `handlePtyReattachFailure` writes when a restarted relay disowns the id.
store.markSshRemotePtyLease(TARGET, 'pty2:aaa:1', 'expired')
// The successor leases without binding the pane; the binding catches up afterwards, exactly as
// the renderer's debounced publish does.
store.upsertSshRemotePtyLease({
targetId: TARGET,
ptyId: 'pty2:bbb:1',
worktreeId: WORKTREE,
tabId: TAB,
leafId: TEST_LEAF_1,
state: 'attached'
})
expect(bulkReattachPtyIds(store)).toEqual(['pty2:aaa:1', 'pty2:bbb:1'])
store.persistPtyBinding({
worktreeId: WORKTREE,
tabId: TAB,
leafId: TEST_LEAF_1,
ptyId: toAppSshPtyId(TARGET, 'pty2:bbb:1')
})
// What the connect path does before reading the set it feeds to `pty.attach`.
store.reconcileSshRemotePtyLeasesForTarget(TARGET)
expect(bulkReattachPtyIds(store)).toEqual(['pty2:bbb:1'])
})
// Reconciliation must not invent evidence: with no binding naming the pane, nothing says which
// shell owns it, so every lease stays askable.
it('leaves leases reattachable when no binding names the pane', async () => {
const store = await createStore()
store.upsertSshRemotePtyLease({
targetId: TARGET,
ptyId: 'unbound-a',
worktreeId: WORKTREE,
tabId: TAB,
leafId: TEST_LEAF_1,
state: 'expired'
})
store.upsertSshRemotePtyLease({
targetId: TARGET,
ptyId: 'unbound-b',
worktreeId: WORKTREE,
tabId: TAB,
leafId: TEST_LEAF_1,
state: 'expired'
})
store.reconcileSshRemotePtyLeasesForTarget(TARGET)
expect(bulkReattachPtyIds(store)).toEqual(['unbound-a', 'unbound-b'])
})
/**
* The measured defect, reduced to its cause.
*
* Main writes an SSH pane's binding to the `ssh:<target>` partition, but a stale copy of the same
* leaf survives in `local`. Reading `local` first named the PREDECESSOR as the pane's current
* PTY, so supersession took an already-expired lease as its winner and returned having marked
* nothing — once per relay restart, forever. Both partitions name the same PTY again once the
* renderer republishes, which is why the finished store looks consistent and hides this.
*/
it('supersedes when the local partition still names the predecessor', async () => {
const store = await createStore()
const hostId = toSshExecutionHostId(TARGET)
const predecessor = toAppSshPtyId(TARGET, 'pty2:old:1')
const successor = toAppSshPtyId(TARGET, 'pty2:new:1')
store.upsertSshRemotePtyLease({
targetId: TARGET,
ptyId: 'pty2:old:1',
worktreeId: WORKTREE,
tabId: TAB,
leafId: TEST_LEAF_1,
state: 'expired'
})
// Both partitions start on the predecessor, as they do before a relay restart.
store.setWorkspaceSession(sessionBinding(predecessor))
store.setWorkspaceSession(sessionBinding(predecessor), hostId)
store.upsertSshRemotePtyLease({
targetId: TARGET,
ptyId: 'pty2:new:1',
worktreeId: WORKTREE,
tabId: TAB,
leafId: TEST_LEAF_1,
state: 'attached'
})
// Production's writer for an SSH pane binding, and the whole point: it updates ONLY the host
// partition, so `local` is left naming the predecessor until the renderer republishes.
store.persistPtyBinding(
{ worktreeId: WORKTREE, tabId: TAB, leafId: TEST_LEAF_1, ptyId: successor },
hostId
)
expect(
store.getWorkspaceSession().terminalLayoutsByTabId?.[TAB]?.ptyIdsByLeafId?.[TEST_LEAF_1]
).toBe(predecessor)
store.supersedeSshRemotePtyLeasesForBoundPane(TARGET, TEST_LEAF_1)
const retired = store.getSshRemotePtyLeases(TARGET).find((l) => l.ptyId === 'pty2:old:1')
expect(retired).toMatchObject({ state: 'expired', supersededBy: 'pty2:new:1' })
expect(bulkReattachPtyIds(store)).toEqual(['pty2:new:1'])
})
// The mirror: a live shell the pane is still bound to must never be retired, whichever partition
// names it. Over-superseding strands a running remote process.
it('never retires a live lease the pane is still bound to', async () => {
const store = await createStore()
const live = toAppSshPtyId(TARGET, 'pty2:live:1')
store.upsertSshRemotePtyLease({
targetId: TARGET,
ptyId: 'pty2:live:1',
worktreeId: WORKTREE,
tabId: TAB,
leafId: TEST_LEAF_1,
state: 'attached'
})
// Bound BEFORE the stray lease arrives, which is the order that makes the binding meaningful:
// with no binding at all, an arriving lease is the only evidence there is and does win.
store.setWorkspaceSession(sessionBinding(live))
store.setWorkspaceSession(sessionBinding(live), toSshExecutionHostId(TARGET))
store.upsertSshRemotePtyLease({
targetId: TARGET,
ptyId: 'pty2:other:1',
worktreeId: WORKTREE,
tabId: TAB,
leafId: TEST_LEAF_1,
state: 'attached'
})
store.supersedeSshRemotePtyLeasesForBoundPane(TARGET, TEST_LEAF_1)
const stillLive = store.getSshRemotePtyLeases(TARGET).find((l) => l.ptyId === 'pty2:live:1')
expect(stillLive).toMatchObject({ state: 'attached' })
expect(stillLive?.supersededBy).toBeUndefined()
})
// Panes are independent, and supersession keys on the leaf: a second live pane on the same
// target must survive its neighbour reconnecting.
it('does not touch a sibling pane on the same target', async () => {
const store = await createStore()
await commitSshSpawn(store, { relayPtyId: 'sibling-pty', leafId: TEST_LEAF_2 })
await commitSshSpawn(store, { relayPtyId: 'pty-0', leafId: TEST_LEAF_1 })
await commitSshSpawn(store, { relayPtyId: 'pty-1', leafId: TEST_LEAF_1 })
expect(bulkReattachPtyIds(store)).toEqual(['pty-1', 'sibling-pty'])
})
})
@@ -0,0 +1,44 @@
import { isTerminalLeafId } from '../../../../shared/stable-pane-id'
import { getRelayPtyId } from '../provider/registry'
import type { Store } from '../../../persistence'
/**
* Claim a remote PTY for a pane: record the lease, then retire the pane's predecessors.
*
* The lease keeps the RELAY id, because reconnect calls `pty.attach` with target-local ids, while
* the pane binding keeps the app-facing id used for hydration.
*
* Supersession is a second step rather than something `upsertSshRemotePtyLease` finishes on its own
* because it is fenced on the pane's durable binding — it refuses to retire a predecessor the pane
* is still bound to, which would detach a live pane. A caller that leases BEFORE it binds therefore
* trips that fence on every reconnect and, with the upsert as the only trigger, never re-runs: one
* more reattachable lease, and one more `pty.attach` round trip on every later connect, forever.
* Re-running it here from the binding side is what makes the two writes commute.
*/
export function claimSshPaneLease(args: {
store: Store | undefined
connectionId: string | null | undefined
ptyId: string
worktreeId: string | undefined
tabId: string | undefined
leafId: string | undefined
}): void {
const { store, connectionId } = args
if (!store || !connectionId) {
return
}
const leafId =
typeof args.leafId === 'string' && isTerminalLeafId(args.leafId) ? args.leafId : null
store.upsertSshRemotePtyLease({
targetId: connectionId,
ptyId: getRelayPtyId(connectionId, args.ptyId),
...(typeof args.worktreeId === 'string' ? { worktreeId: args.worktreeId } : {}),
...(typeof args.tabId === 'string' ? { tabId: args.tabId } : {}),
...(leafId ? { leafId } : {}),
state: 'attached',
lastAttachedAt: Date.now()
})
if (leafId) {
store.supersedeSshRemotePtyLeasesForBoundPane(connectionId, leafId)
}
}
+9 -18
View File
@@ -1,8 +1,6 @@
import { isValidTerminalTabId } from '../../../../shared/terminal-tab-id'
import { isTerminalLeafId } from '../../../../shared/stable-pane-id'
import { ptyOwnership, ptyIncarnationById, deletePtyOwnership } from '../provider/ownership-state'
import { ptySizes } from '../delivery/visibility-state'
import { getRelayPtyId } from '../provider/registry'
import {
shouldSkipCodexHomeEnvForWindowsShell,
recordCodexPaneAccountForSpawn,
@@ -25,6 +23,7 @@ import {
requestKindSchema
} from '../../../../shared/telemetry-events'
import { persistAdmittedStablePaneBinding } from '../pane/stable-owner'
import { claimSshPaneLease } from '../pane/ssh-pane-lease-claim'
import {
isNativeWindowsLocalPtySpawn,
markNativeWindowsConptyPty
@@ -114,23 +113,15 @@ export async function commitRuntimePtySpawn(ctx: RuntimePtySpawnState) {
) {
markNativeWindowsConptyPty(ctx.result.id)
}
const persistSshLease = (): void => {
if (!ctx.deps.store || !args.connectionId) {
return
}
// Why: SSH leases keep relay ids for remote reconciliation, while session bindings keep app-facing ids for hydration.
ctx.deps.store.upsertSshRemotePtyLease({
targetId: args.connectionId,
ptyId: getRelayPtyId(args.connectionId, ctx.result.id),
...(typeof args.worktreeId === 'string' ? { worktreeId: args.worktreeId } : {}),
...(typeof args.tabId === 'string' ? { tabId: args.tabId } : {}),
...(typeof args.leafId === 'string' && isTerminalLeafId(args.leafId)
? { leafId: args.leafId }
: {}),
state: 'attached',
lastAttachedAt: Date.now()
const persistSshLease = (): void =>
claimSshPaneLease({
store: ctx.deps.store,
connectionId: args.connectionId,
ptyId: ctx.result.id,
worktreeId: args.worktreeId,
tabId: args.tabId,
leafId: args.leafId
})
}
if (!ctx.hostSessionBinding) {
persistSshLease()
}
+2
View File
@@ -25,6 +25,7 @@ export type SshLeaseStoreMock = {
upsertSshPtyConsumerRecovery: Mock
removeSshPtyConsumerRecovery: Mock
getSshRemotePtyLeases: Mock
reconcileSshRemotePtyLeasesForTarget: Mock
markSshRemotePtyLease: Mock
markSshRemotePtyLeases: Mock
markSshRemotePtyLeasesAsync: Mock
@@ -102,6 +103,7 @@ export function createSshIpcHarness(mocks: SshIpcMocks): SshIpcHarness {
upsertSshPtyConsumerRecovery: vi.fn(),
removeSshPtyConsumerRecovery: vi.fn(),
getSshRemotePtyLeases: vi.fn().mockReturnValue([]),
reconcileSshRemotePtyLeasesForTarget: vi.fn(),
markSshRemotePtyLease: vi.fn(),
markSshRemotePtyLeases: vi.fn(),
markSshRemotePtyLeasesAsync: vi.fn(),
@@ -1,9 +1,8 @@
import { toSshExecutionHostId } from '../../../shared/execution-host'
import type { PersistedState } from '../../../shared/persisted-state-types'
import type { SshRemotePtyLease } from '../../../shared/ssh-types'
import { isTerminalLeafId } from '../../../shared/stable-pane-id'
import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types'
import { invalidateLocalWorktreeMetadataPruneInputs } from '../../local-worktree-metadata-prune-gate'
import { supersedeSiblingLeasesForPane } from './ssh-pty-pane-supersession'
export type SshPtyLeaseOperations = {
state: PersistedState
@@ -15,91 +14,6 @@ export type SshPtyLeaseOperations = {
flushDurableStateOrThrowAsync: () => Promise<void>
}
/**
* The PTY a pane is durably bound to, keyed on the leaf alone — the only remint-stable half of a
* pane key, since `detachTerminalPaneToTab` moves a live pane and leaves its lease naming the tab
* it left.
*
* Reads both partitions deliberately. Main writes some SSH pane bindings to `ssh:<target>` and
* some to `local`, so a reader that consulted one would see "unbound" for a live pane and expire
* its lease. Reading both makes this fence correct whichever partition the binding landed in.
*/
function durablyBoundPtyIdForPane(
operations: SshPtyLeaseOperations,
targetId: string,
leafId: string
): string | undefined {
const findLeafBinding = (session: WorkspaceSessionState | undefined): string | undefined =>
Object.values(session?.terminalLayoutsByTabId ?? {}).find(
(layout) => layout?.ptyIdsByLeafId?.[leafId]
)?.ptyIdsByLeafId?.[leafId]
const boundPtyId =
findLeafBinding(operations.state.workspaceSession) ??
findLeafBinding(operations.state.workspaceSessionsByHostId?.[toSshExecutionHostId(targetId)])
return boundPtyId ? operations.toComparablePtyId(targetId, boundPtyId) : undefined
}
/**
* One pane owns at most one live remote PTY. Lease identity is `(targetId, ptyId)` alone, so a
* pane re-leasing under a new relay id leaves its predecessor live with nothing to retire it and
* the next reattach fans out over both — the reported 2 -> 19 -> 20 across three reconnects.
*
* Superseded leases are marked `expired`, never `terminated`: losing a lease is not evidence the
* shell died, so the remote process is deliberately left running. They also carry `supersededBy`,
* which is what keeps them out of the bulk reattach set now that plain `expired` no longer does —
* the winner's ptyId is already in hand here, so recording it needs no relay-start identity.
*/
function supersedeSiblingLeasesForPane(
operations: SshPtyLeaseOperations,
winner: SshRemotePtyLease,
now: number
): void {
if (!winner.worktreeId || !winner.leafId) {
return
}
if (winner.state === 'terminated' || winner.state === 'expired') {
return
}
// At upsert time the arriving lease may not be the one the pane is bound to yet. Expiring the
// bound predecessor would detach a live pane, so leave both live and let reattach arbitrate
// with the binding in hand.
const boundPtyId = durablyBoundPtyIdForPane(operations, winner.targetId, winner.leafId)
if (boundPtyId && boundPtyId !== winner.ptyId) {
return
}
const superseded: SshRemotePtyLease[] = []
for (const lease of operations.state.sshRemotePtyLeases ?? []) {
if (
lease.ptyId === winner.ptyId ||
lease.targetId !== winner.targetId ||
lease.worktreeId !== winner.worktreeId ||
// Leaf only: a lease freezes its tabId, so a pane broken out into a new tab would otherwise
// never compete with its own predecessor — which is the reported cardinality growth.
lease.leafId !== winner.leafId ||
lease.state === 'terminated'
) {
continue
}
if (lease.state === 'expired') {
// An already-expired predecessor is superseded by the same evidence, and marking it is what
// bounds the reattach set: without this, every past orphan for this pane stays reattachable
// forever. `updatedAt` stays put — bumping it would make a stale lease look recent to
// `getRecentExpiredSshLease`.
lease.supersededBy = winner.ptyId
continue
}
lease.state = 'expired'
lease.supersededBy = winner.ptyId
lease.updatedAt = now
superseded.push(lease)
}
if (superseded.length > 0) {
// 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.
operations.clearBindingsForLeases(winner.targetId, superseded)
}
}
/**
* Only `terminated` unbinds a pane. It is the operator-close state and the one written after a
* host-acknowledged stop; `expired` records that the CLIENT lost its route and says nothing about
@@ -0,0 +1,201 @@
import { toSshExecutionHostId } from '../../../shared/execution-host'
import type { SshRemotePtyLease } from '../../../shared/ssh-types'
import { isTerminalLeafId } from '../../../shared/stable-pane-id'
import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types'
import type { SshPtyLeaseOperations } from './ssh-pty-lease-operations'
/**
* Every PTY id any partition binds to this pane, most authoritative first.
*
* Keyed on the leaf alone — the only remint-stable half of a pane key, since
* `detachTerminalPaneToTab` moves a live pane and leaves its lease naming the tab it left.
*
* Returns a LIST, and reads the target's own partition first, because the two partitions disagree
* for the length of a reconnect and this resolved that disagreement backwards. Main writes an SSH
* pane's binding to `ssh:<target>`, while a stale copy of the same leaf survives in `local`;
* consulting `local` first therefore named the PREDECESSOR as the pane's current PTY on every relay
* restart. Supersession then took that expired predecessor as its winner and returned without
* marking anything — the per-reconnect lease growth. Both partitions are still read, because a
* reader that consulted only one would see "unbound" for a live pane and expire its lease.
*/
function durablyBoundPtyIdsForPane(
operations: SshPtyLeaseOperations,
targetId: string,
leafId: string
): string[] {
const findLeafBindings = (session: WorkspaceSessionState | undefined): string[] =>
Object.values(session?.terminalLayoutsByTabId ?? {})
.map((layout) => layout?.ptyIdsByLeafId?.[leafId])
.filter((ptyId): ptyId is string => Boolean(ptyId))
const ordered = [
...findLeafBindings(
operations.state.workspaceSessionsByHostId?.[toSshExecutionHostId(targetId)]
),
...findLeafBindings(operations.state.workspaceSession)
]
return [...new Set(ordered.map((ptyId) => operations.toComparablePtyId(targetId, ptyId)))]
}
/** A lease this client still holds a route to, as opposed to one it has already lost. */
function isLiveLeaseState(state: SshRemotePtyLease['state']): boolean {
return state === 'attached' || state === 'detached'
}
/**
* One pane owns at most one live remote PTY. Lease identity is `(targetId, ptyId)` alone, so a
* pane re-leasing under a new relay id leaves its predecessor live with nothing to retire it and
* the next reattach fans out over both — the reported 2 -> 19 -> 20 across three reconnects.
*
* Superseded leases are marked `expired`, never `terminated`: losing a lease is not evidence the
* shell died, so the remote process is deliberately left running. They also carry `supersededBy`,
* which is what keeps them out of the bulk reattach set now that plain `expired` no longer does —
* the winner's ptyId is already in hand here, so recording it needs no relay-start identity.
*/
export function supersedeSiblingLeasesForPane(
operations: SshPtyLeaseOperations,
winner: SshRemotePtyLease,
now: number
): boolean {
if (!winner.worktreeId || !winner.leafId) {
return false
}
if (winner.state === 'terminated' || winner.state === 'expired') {
return false
}
// At upsert time the arriving lease may not be the one the pane is bound to yet. Expiring the
// bound predecessor would detach a live pane, so leave both live and let reattach arbitrate
// with the binding in hand. `supersedeSshRemotePtyLeasesForBoundPane` re-runs this once the
// binding write lands, so a caller that upserts before it binds is not left bailed forever.
// Membership rather than equality: during a reconnect the two partitions name different PTYs for
// the same leaf, and requiring the winner to match the FIRST one read is what made this bail.
const boundPtyIds = durablyBoundPtyIdsForPane(operations, winner.targetId, winner.leafId)
if (boundPtyIds.length > 0 && !boundPtyIds.includes(winner.ptyId)) {
return false
}
let marked = false
const superseded: SshRemotePtyLease[] = []
for (const lease of operations.state.sshRemotePtyLeases ?? []) {
if (
lease.ptyId === winner.ptyId ||
lease.targetId !== winner.targetId ||
lease.worktreeId !== winner.worktreeId ||
// Leaf only: a lease freezes its tabId, so a pane broken out into a new tab would otherwise
// never compete with its own predecessor — which is the reported cardinality growth.
lease.leafId !== winner.leafId ||
lease.state === 'terminated' ||
// Never retire a shell the pane is BOTH still bound to and still routable to. The stale
// partition can name a predecessor, and retiring that is the point; retiring a live one
// would strand a running remote process behind a pane that can no longer reach it.
(boundPtyIds.includes(lease.ptyId) && isLiveLeaseState(lease.state))
) {
continue
}
if (lease.state === 'expired') {
// An already-expired predecessor is superseded by the same evidence, and marking it is what
// bounds the reattach set: without this, every past orphan for this pane stays reattachable
// forever. `updatedAt` stays put — bumping it would make a stale lease look recent to
// `getRecentExpiredSshLease`.
marked ||= lease.supersededBy !== winner.ptyId
lease.supersededBy = winner.ptyId
continue
}
lease.state = 'expired'
lease.supersededBy = winner.ptyId
lease.updatedAt = now
marked = true
superseded.push(lease)
}
if (superseded.length > 0) {
// 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.
operations.clearBindingsForLeases(winner.targetId, superseded)
}
return marked
}
/**
* Supersede from the lease the pane's binding names — preferring a LIVE one when the partitions
* disagree, since a reconnect leaves the stale partition naming an already-expired predecessor and
* an expired winner supersedes nothing.
*/
function supersedeFromBoundPane(
operations: SshPtyLeaseOperations,
targetId: string,
leafId: string,
now: number
): boolean {
if (!isTerminalLeafId(leafId)) {
return false
}
const boundPtyIds = durablyBoundPtyIdsForPane(operations, targetId, leafId)
if (boundPtyIds.length === 0) {
// No binding names this pane, so nothing here is evidence about which shell owns it. Leaving
// every lease reattachable is the deliberate direction: an orphan must stay askable.
return false
}
const candidates = (operations.state.sshRemotePtyLeases ?? []).filter(
(lease) =>
lease.targetId === targetId && lease.leafId === leafId && boundPtyIds.includes(lease.ptyId)
)
const winner = candidates.find((lease) => isLiveLeaseState(lease.state))
const marked = winner ? supersedeSiblingLeasesForPane(operations, winner, now) : false
return marked
}
/**
* The binding-side trigger for supersession, and the reason the two writes that together claim a
* pane are commutative.
*
* `upsertSshRemotePtyLease` is the only other trigger, and it bails whenever the pane's durable
* binding still names the predecessor. A spawn path that upserts its lease BEFORE it writes the
* binding therefore bails and never re-runs on its own. Re-resolving the winner from the binding
* is safe in the other direction too: it supersedes only from the lease the pane is actually bound
* to, so it can never strand a live orphan.
*/
export function supersedeSshRemotePtyLeasesForBoundPane(
operations: SshPtyLeaseOperations,
targetId: string,
leafId: string
): void {
if (supersedeFromBoundPane(operations, targetId, leafId, Date.now())) {
operations.flush()
}
}
/**
* Bound the reattach set to one lease per pane, re-derived from each pane's CURRENT binding.
*
* The spawn-side trigger cannot be sufficient alone, and measuring the shipped path is what showed
* it: a pane's binding has several writers — the spawn commit, the relay's reattach bind, and the
* renderer's debounced layout publish — and the last of those lands well after the spawn commit
* that leased the pty. A predecessor that was still bound when its successor was claimed therefore
* keeps its reattachability forever, because nothing revisits it once the binding catches up. The
* observed rows agreed on target, worktree, tab and leaf and still carried no mark.
*
* Running this immediately before the reattach set is read makes the answer independent of which
* writer bound the pane and when. It also repairs stores written by earlier builds, where these
* rows have already accumulated and no spawn-time trigger would ever revisit them.
*
* Panes with no binding are skipped rather than pruned: absence of a binding is not evidence about
* which shell owns the pane, and a genuine orphan has to stay askable
* (docs/reference/ssh-execution-boundary.md).
*/
export function reconcileSshRemotePtyLeasesForTarget(
operations: SshPtyLeaseOperations,
targetId: string
): void {
const leafIds = new Set<string>()
for (const lease of operations.state.sshRemotePtyLeases ?? []) {
if (lease.targetId === targetId && lease.leafId) {
leafIds.add(lease.leafId)
}
}
const now = Date.now()
let changed = false
for (const leafId of leafIds) {
changed = supersedeFromBoundPane(operations, targetId, leafId, now) || changed
}
if (changed) {
operations.flush()
}
}
@@ -23,6 +23,10 @@ import {
type SshPtyLeaseOperations,
upsertSshRemotePtyLease as upsertSshRemotePtyLeaseOperation
} from '../leasing-ssh-ptys/ssh-pty-lease-operations'
import {
reconcileSshRemotePtyLeasesForTarget as reconcileSshRemotePtyLeasesForTargetOperation,
supersedeSshRemotePtyLeasesForBoundPane as supersedeSshRemotePtyLeasesForBoundPaneOperation
} from '../leasing-ssh-ptys/ssh-pty-pane-supersession'
import {
getSshPtyConsumerRecovery as getSshPtyConsumerRecoveryOperation,
removeSshPtyConsumerRecovery as removeSshPtyConsumerRecoveryOperation,
@@ -95,6 +99,28 @@ export class SshLeaseRecoveryOperations {
upsertSshRemotePtyLeaseOperation(getSshPtyLeaseOperations(this), lease)
}
/**
* Re-run pane supersession from the binding rather than from an arriving lease. Spawn commits
* call this after their binding write so it does not matter whether the lease or the binding
* landed first; see `supersedeSshRemotePtyLeasesForBoundPane`.
*/
supersedeSshRemotePtyLeasesForBoundPane(targetId: string, leafId: string): void {
supersedeSshRemotePtyLeasesForBoundPaneOperation(
getSshPtyLeaseOperations(this),
targetId,
leafId
)
}
/**
* Re-derive one reattachable lease per pane from each pane's current binding. Called on the
* connect path immediately before the reattach set is read; see
* `reconcileSshRemotePtyLeasesForTarget`.
*/
reconcileSshRemotePtyLeasesForTarget(targetId: string): void {
reconcileSshRemotePtyLeasesForTargetOperation(getSshPtyLeaseOperations(this), targetId)
}
markSshRemotePtyLeases(targetId: string, state: SshRemotePtyLease['state']): void {
markSshRemotePtyLeasesOperation(getSshPtyLeaseOperations(this), targetId, state)
}
+31 -29
View File
@@ -82,24 +82,38 @@ function relayReattachBinds(
}
/**
* Both binding writers land before the lease upsert — spawn asserts that ordering directly, and
* the relay's reattach binds the pane before `markSshRemotePtyLeasesAttachedAsync`. Supersession
* therefore sees a session already naming the arriving shell.
* One reconnect's worth of writes, in the order the spawn commits actually issue them: the lease
* row first — so a force-quit in the renderer's debounce window cannot leave a running remote shell
* with no lease to reattach it — then the binding, then the binding-side supersession trigger.
*
* This suite used to bind BEFORE upserting, an order no caller uses. Under that order supersession
* always saw a session already naming the arriving shell and passed; under production's order it
* bailed on the predecessor's binding every time and never re-ran, so the guard could not catch the
* per-reconnect lease growth it exists to pin.
*
* Goes through `persistPtyBinding` rather than `setWorkspaceSession` because that is the writer
* production uses; a raw session write is reconciled back to the attached lease's PTY by binding
* recovery, which would make the fixture disagree with the real flow.
*/
function paneBindsTo(
function paneSpawnCommits(
store: ReturnType<typeof createStore>,
args: { tabId: string; leafId: string; ptyId: string }
args: { tabId: string; leafId: string; ptyId: string; leaseTabId?: string }
): void {
store.upsertSshRemotePtyLease({
targetId: TARGET,
ptyId: args.ptyId,
worktreeId: WORKTREE,
tabId: args.leaseTabId ?? args.tabId,
leafId: args.leafId,
state: 'attached'
})
store.persistPtyBinding({
worktreeId: WORKTREE,
tabId: args.tabId,
leafId: args.leafId,
ptyId: args.ptyId
})
store.supersedeSshRemotePtyLeasesForBoundPane(TARGET, args.leafId)
}
function liveLeasePtyIds(store: ReturnType<typeof createStore>): string[] {
@@ -300,8 +314,7 @@ describe('STA-3077: one pane keeps at most one live remote lease', () => {
const lease = { targetId: TARGET, worktreeId: WORKTREE, tabId: TAB, leafId: TEST_LEAF_1 }
store.upsertSshRemotePtyLease({ ...lease, ptyId: 'pty-1', state: 'attached' })
paneBindsTo(store, { tabId: TAB, leafId: TEST_LEAF_1, ptyId: 'pty-2' })
store.upsertSshRemotePtyLease({ ...lease, ptyId: 'pty-2', state: 'attached' })
paneSpawnCommits(store, { tabId: TAB, leafId: TEST_LEAF_1, ptyId: 'pty-2' })
expect(liveLeasePtyIds(store)).toEqual(['pty-2'])
})
@@ -314,8 +327,7 @@ describe('STA-3077: one pane keeps at most one live remote lease', () => {
const lease = { targetId: TARGET, worktreeId: WORKTREE, tabId: TAB, leafId: TEST_LEAF_1 }
store.upsertSshRemotePtyLease({ ...lease, ptyId: 'pty-1', state: 'attached' })
paneBindsTo(store, { tabId: TAB, leafId: TEST_LEAF_1, ptyId: 'pty-2' })
store.upsertSshRemotePtyLease({ ...lease, ptyId: 'pty-2', state: 'attached' })
paneSpawnCommits(store, { tabId: TAB, leafId: TEST_LEAF_1, ptyId: 'pty-2' })
const predecessor = store.getSshRemotePtyLeases(TARGET).find((entry) => entry.ptyId === 'pty-1')
expect(predecessor?.state).toBe('expired')
@@ -325,11 +337,9 @@ describe('STA-3077: one pane keeps at most one live remote lease', () => {
it('holds the live lease count flat across ten reconnects of one pane', async () => {
const store = await createStore()
store.setWorkspaceSession(sessionWithPane({ tabId: TAB, leafId: TEST_LEAF_1, ptyId: 'pty-0' }))
const lease = { targetId: TARGET, worktreeId: WORKTREE, tabId: TAB, leafId: TEST_LEAF_1 }
for (let reconnect = 0; reconnect < 10; reconnect++) {
paneBindsTo(store, { tabId: TAB, leafId: TEST_LEAF_1, ptyId: `pty-${reconnect}` })
store.upsertSshRemotePtyLease({ ...lease, ptyId: `pty-${reconnect}`, state: 'attached' })
paneSpawnCommits(store, { tabId: TAB, leafId: TEST_LEAF_1, ptyId: `pty-${reconnect}` })
}
expect(liveLeasePtyIds(store)).toEqual(['pty-9'])
@@ -349,17 +359,14 @@ describe('STA-3077: one pane keeps at most one live remote lease', () => {
state: 'attached'
})
paneBindsTo(store, { tabId: TAB, leafId: TEST_LEAF_1, ptyId: 'pty-2' })
// The successor's lease names the tab the pane sits in NOW; the predecessor's still names the
// one it was written in. Only the leaf is common, so keying on the tab would stop the two
// competing and leave both live — the cardinality growth.
store.upsertSshRemotePtyLease({
targetId: TARGET,
ptyId: 'pty-2',
worktreeId: WORKTREE,
tabId: OTHER_TAB,
paneSpawnCommits(store, {
tabId: TAB,
leafId: TEST_LEAF_1,
state: 'attached'
ptyId: 'pty-2',
leaseTabId: OTHER_TAB
})
expect(liveLeasePtyIds(store)).toEqual(['pty-2'])
@@ -394,8 +401,7 @@ describe('STA-3077: one pane keeps at most one live remote lease', () => {
const lease = { targetId: TARGET, worktreeId: WORKTREE, tabId: TAB, leafId: TEST_LEAF_1 }
store.upsertSshRemotePtyLease({ ...lease, ptyId: 'pty-1', state: 'attached' })
paneBindsTo(store, { tabId: TAB, leafId: TEST_LEAF_1, ptyId: 'pty-2' })
store.upsertSshRemotePtyLease({ ...lease, ptyId: 'pty-2', state: 'attached' })
paneSpawnCommits(store, { tabId: TAB, leafId: TEST_LEAF_1, ptyId: 'pty-2' })
expect(liveLeasePtyIds(store).sort()).toEqual(['pty-2', 'sibling-pty'])
})
@@ -455,8 +461,7 @@ describe('STA-3077: `expired` separates a superseded sibling from an orphan', ()
it('never bulk-reattaches a superseded sibling', async () => {
const store = await storeWithPane('pty-1')
paneBindsTo(store, { tabId: TAB, leafId: TEST_LEAF_1, ptyId: 'pty-2' })
store.upsertSshRemotePtyLease({ ...paneLease, ptyId: 'pty-2', state: 'attached' })
paneSpawnCommits(store, { tabId: TAB, leafId: TEST_LEAF_1, ptyId: 'pty-2' })
const predecessor = store.getSshRemotePtyLeases(TARGET).find((entry) => entry.ptyId === 'pty-1')
expect(predecessor).toMatchObject({ state: 'expired', supersededBy: 'pty-2' })
@@ -469,8 +474,7 @@ describe('STA-3077: `expired` separates a superseded sibling from an orphan', ()
store.setWorkspaceSession(sessionWithPane({ tabId: TAB, leafId: TEST_LEAF_1, ptyId: 'pty-0' }))
for (let reconnect = 0; reconnect < 10; reconnect++) {
paneBindsTo(store, { tabId: TAB, leafId: TEST_LEAF_1, ptyId: `pty-${reconnect}` })
store.upsertSshRemotePtyLease({ ...paneLease, ptyId: `pty-${reconnect}`, state: 'attached' })
paneSpawnCommits(store, { tabId: TAB, leafId: TEST_LEAF_1, ptyId: `pty-${reconnect}` })
}
expect(bulkReattachPtyIds(store)).toEqual(['pty-9'])
@@ -496,8 +500,7 @@ describe('STA-3077: `expired` separates a superseded sibling from an orphan', ()
store.markSshRemotePtyLease(TARGET, 'pty-1', 'expired')
const orphanUpdatedAt = store.getSshRemotePtyLeases(TARGET)[0].updatedAt
paneBindsTo(store, { tabId: TAB, leafId: TEST_LEAF_1, ptyId: 'pty-2' })
store.upsertSshRemotePtyLease({ ...paneLease, ptyId: 'pty-2', state: 'attached' })
paneSpawnCommits(store, { tabId: TAB, leafId: TEST_LEAF_1, ptyId: 'pty-2' })
const predecessor = store.getSshRemotePtyLeases(TARGET).find((entry) => entry.ptyId === 'pty-1')
expect(predecessor).toMatchObject({ state: 'expired', supersededBy: 'pty-2' })
@@ -510,8 +513,7 @@ describe('STA-3077: `expired` separates a superseded sibling from an orphan', ()
// belongs to the lease that lost, never to whatever claims the id next.
it('clears the supersession mark when the id is re-upserted as a live lease', async () => {
const store = await storeWithPane('pty-1')
paneBindsTo(store, { tabId: TAB, leafId: TEST_LEAF_1, ptyId: 'pty-2' })
store.upsertSshRemotePtyLease({ ...paneLease, ptyId: 'pty-2', state: 'attached' })
paneSpawnCommits(store, { tabId: TAB, leafId: TEST_LEAF_1, ptyId: 'pty-2' })
// A restarted relay hands `pty-1` to a new shell for a different pane.
store.upsertSshRemotePtyLease({
@@ -53,7 +53,8 @@ function createHarness(
shutdown
} as unknown as IPtyProvider
const store = {
getSshRemotePtyLeases: vi.fn().mockReturnValue(leases)
getSshRemotePtyLeases: vi.fn().mockReturnValue(leases),
reconcileSshRemotePtyLeasesForTarget: vi.fn()
} as unknown as Store
return { provider, store, shutdown }
}
@@ -157,6 +157,7 @@ function createSession(targetId: string): InstanceType<typeof SshRelaySession> {
upsertSshPtyConsumerRecovery: vi.fn(),
removeSshPtyConsumerRecovery: vi.fn(),
getSshRemotePtyLeases: vi.fn().mockReturnValue([]),
reconcileSshRemotePtyLeasesForTarget: vi.fn(),
markSshRemotePtyLease: vi.fn(),
markSshRemotePtyLeases: vi.fn(),
markSshRemotePtyLeasesAsync: vi.fn(),
@@ -104,6 +104,7 @@ function createMockDeps(): {
upsertSshPtyConsumerRecovery: vi.fn(),
removeSshPtyConsumerRecovery: vi.fn(),
getSshRemotePtyLeases: vi.fn().mockReturnValue([]),
reconcileSshRemotePtyLeasesForTarget: vi.fn(),
markSshRemotePtyLease: vi.fn(),
markSshRemotePtyLeases: vi.fn(),
markSshRemotePtyLeasesAsync: vi.fn(),
@@ -21,6 +21,7 @@ export function createMockDeps(): SshRelaySessionTestDeps {
upsertSshPtyConsumerRecovery: vi.fn(),
removeSshPtyConsumerRecovery: vi.fn(),
getSshRemotePtyLeases: vi.fn().mockReturnValue([]),
reconcileSshRemotePtyLeasesForTarget: vi.fn(),
getWorkspaceSession: vi.fn(),
markSshRemotePtyLease: vi.fn(),
markSshRemotePtyLeases: vi.fn(),
+6
View File
@@ -2322,6 +2322,12 @@ export class SshRelaySession {
if (!shouldContinue()) {
return
}
// Why immediately before the read: a pane's binding is written by several writers, and the
// renderer's debounced layout publish lands long after the spawn commit that leased the pty —
// so a predecessor that was still bound at spawn time never gets marked by a spawn-side
// trigger. Re-deriving from each pane's CURRENT binding here is what actually bounds this set,
// and it repairs stores that already accumulated these rows.
this.store.reconcileSshRemotePtyLeasesForTarget(this.targetId)
// Why not `state !== 'expired'`: that state covers both a superseded sibling (re-adopting it is
// the 2 -> 19 -> 20 fan-out) and an orphan whose reattach merely lost contact. Only the first
// carries a retirement mark, and only it has to be skipped.
@@ -1,5 +1,9 @@
import type { Page } from '@playwright/test'
import path from 'node:path'
import { readFileSync } from 'node:fs'
import type { ElectronApplication, Page } from '@playwright/test'
import { test, expect } from './helpers/orca-app'
import { DEFAULT_LOCAL_ORCA_PROFILE_ID } from '../../src/shared/orca-profiles'
import { sshRemotePtyLeaseAllowsReattach, type SshRemotePtyLease } from '../../src/shared/ssh-types'
import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store'
import {
execInTerminal,
@@ -49,6 +53,59 @@ async function readSshStatus(orcaPage: Page, targetId: string) {
)
}
/**
* Every lease `reattachKnownPtys` would feed to `pty.attach` on the next connect, read from the
* durable store rather than from the renderer — leases are main-owned and never published.
*
* Goes through the shipped `sshRemotePtyLeaseAllowsReattach` predicate so the measurement cannot
* drift from the fan-out it exists to bound.
*/
function readSshLeases(userDataDir: string, targetId: string): SshRemotePtyLease[] {
const dataPath = path.join(
userDataDir,
'profiles',
DEFAULT_LOCAL_ORCA_PROFILE_ID,
'orca-data.json'
)
const parsed = JSON.parse(readFileSync(dataPath, 'utf8')) as {
sshRemotePtyLeases?: SshRemotePtyLease[]
}
return (parsed.sshRemotePtyLeases ?? []).filter((lease) => lease.targetId === targetId)
}
function readReattachablePtyIds(userDataDir: string, targetId: string): string[] {
return readSshLeases(userDataDir, targetId)
.filter(sshRemotePtyLeaseAllowsReattach)
.map((lease) => lease.ptyId)
.sort()
}
/**
* Everything a cardinality failure needs to be diagnosable from the report alone.
*
* Worth keeping rather than reducing to a count: when this first failed, the count said only "2",
* and it was the per-row fields that ruled out the obvious causes — the rows agreed on worktree,
* tab and leaf, so the pane identity was never the problem.
*/
function describeSshLeases(userDataDir: string, targetId: string): string {
return JSON.stringify(
readSshLeases(userDataDir, targetId).map((lease) => ({
ptyId: lease.ptyId,
state: lease.state,
worktreeId: lease.worktreeId,
leafId: lease.leafId,
tabId: lease.tabId,
supersededBy: lease.supersededBy,
relayIdRecycled: lease.relayIdRecycled,
reattachable: sshRemotePtyLeaseAllowsReattach(lease)
}))
)
}
function readUserDataDir(electronApp: ElectronApplication): Promise<string> {
return electronApp.evaluate(({ app }) => app.getPath('userData'))
}
/**
* Not covered here on purpose: park-then-reveal after a reconnect. ssh-terminal-parking already
* covers the park/reveal round trip, and driving a park deterministically from this lane proved
@@ -117,7 +174,9 @@ test.describe('SSH transport drop recovery', () => {
// run after the flood produces no output within the poll budget. Same shape as #18018 (deaf pane
// after a stalled host resumes), and not caused by this spec. Tracked there; the three verdict
// assertions around it stay enforced.
test.fixme('stays bounded when a disconnected shell floods its pty', async ({ orcaPage }, testInfo) => {
test.fixme('stays bounded when a disconnected shell floods its pty', async ({
orcaPage
}, testInfo) => {
test.slow()
// Timeouts here are deliberately generous: this guards memory, not latency. A 48MB flood plus a
// reconnect lands near 60s wall-clock end to end, so a 60s bind timeout was marginal and made
@@ -261,6 +320,89 @@ test.describe('SSH transport drop recovery', () => {
}
})
/**
* The cardinality half of the same fault, which the verdict test above cannot see: it asserts the
* pane is re-backed, not what the pane's PREVIOUS shells left behind in the store.
*
* A pane re-leases under a new relay pty id on every relay restart, and nothing else retires the
* predecessor. When supersession fails, each generation leaves one more `expired`-but-unsuperseded
* lease that `reattachKnownPtys` still asks about — one extra `pty.attach` round trip on every
* later connect, forever, growing linearly with reconnect count. Measured as leases rather than
* as latency because latency hides the growth until it is already large.
*
* The reattachable set must stay at exactly one per pane. It must not go to zero either: a lease
* wrongly superseded is a running remote shell the pane can no longer find, which is the worse
* failure (docs/reference/ssh-execution-boundary.md).
*/
test('keeps one reattachable lease per pane across repeated relay restarts', async ({
orcaPage,
electronApp
}, testInfo) => {
test.slow()
let target: DockerSshRelayTarget | null = null
try {
target = startDockerSshRelayTarget(testInfo)
enableDockerSshRelayTargetShellTitle(target)
await waitForSessionReady(orcaPage)
await waitForActiveWorktree(orcaPage)
const remote = await connectDockerSshRelayTarget(orcaPage, target)
await ensureTerminalVisible(orcaPage, 45_000)
await waitForActiveTerminalManager(orcaPage, 60_000)
await waitForActivePanePtyId(orcaPage, 60_000)
const userDataDir = await readUserDataDir(electronApp)
const generations: string[][] = []
for (let generation = 1; generation <= 5; generation++) {
expect(
killDockerSshRelayDaemon(target),
'no relay process was found to kill'
).toBeGreaterThan(0)
await expect
.poll(() => readSshStatus(orcaPage, remote.targetId), {
timeout: 120_000,
message: `SSH target never reconnected after relay kill ${generation}`
})
.toBe('connected')
await waitForActiveTerminalManager(orcaPage, 120_000)
// The pane must be usable again before the count is meaningful: recovery is what mints the
// successor lease that retires the generation before it.
const ptyId = await waitForActivePanePtyId(orcaPage, 120_000)
const marker = `LEASE_GEN_${generation}_${Date.now()}`
await execInTerminal(orcaPage, ptyId, `printf '%s\\n' ${marker}`)
await waitForTerminalOutput(orcaPage, marker, 60_000)
try {
await expect
.poll(() => readReattachablePtyIds(userDataDir, remote.targetId).length, {
timeout: 60_000
})
.toBe(1)
} catch (error) {
// Why re-thrown with the rows: the count alone cannot say WHICH predecessor stayed
// reattachable, and the user-data dir is torn down before the report is read.
throw new Error(
`reattachable lease count never settled at 1 in generation ${generation}; leases: ${describeSshLeases(userDataDir, remote.targetId)}`,
{ cause: error }
)
}
generations.push(readReattachablePtyIds(userDataDir, remote.targetId))
}
// Stated as the whole sequence so a regression reports the growth, not just its endpoint —
// the reported shape was 2, 3, 4, 5, 6 across five restarts.
expect(
generations.map((ptyIds) => ptyIds.length),
`reattachable lease count per generation: ${JSON.stringify(generations)}`
).toEqual([1, 1, 1, 1, 1])
} finally {
if (target) {
clearDockerSshRelayFaults(target)
cleanupDockerSshRelayTarget(target)
}
}
})
/**
* The third fault shape: silence with the socket still established. `docker pause` freezes the
* container, so nothing is closed or reset — the client simply stops hearing from a host that is