fix(ssh): let the durable pane binding outrank recency when retiring leases

Choosing the newest lease for a pane is wrong whenever a newer lease
exists that no pane is bound to: it retires the lease the pane is
actually attached to, detaching a live terminal instead of healing it.

Two changes. Arbitration now prefers the lease matching the pane's
durable binding, across both the SSH-target and local partitions,
falling back to recency only when no binding names either candidate.

And supersession at upsert time now defers rather than expiring a bound
predecessor. When a lease arrives for a pane that is still bound to a
different PTY, the binding has not caught up yet, so both stay live and
reattach arbitrates once the binding is available.

Co-authored-by: Orca <help@stably.ai>

config/scripts/run-ssh-docker-reconnect-pane-cardinality-e2e.mjs

@@ -0,0 +1,41 @@
+import { spawnSync } from 'node:child_process'
+
+const rawExtraArgs = process.argv.slice(2)
+const extraArgs = rawExtraArgs[0] === '--' ? rawExtraArgs.slice(1) : rawExtraArgs
+const pnpm = process.platform === 'win32' ? 'pnpm.cmd' : 'pnpm'
+const env = {
+  ...process.env,
+  ORCA_E2E_SSH_DOCKER: '1'
+}
+
+// Why: Node's CVE-2024-27980 hardening rejects .cmd spawns without shell on Windows.
+const spawnOptions = {
+  stdio: 'inherit',
+  env,
+  shell: process.platform === 'win32'
+}
+
+const runtime = spawnSync(pnpm, ['run', 'ensure:electron-runtime'], spawnOptions)
+
+if (runtime.status !== 0) {
+  process.exit(runtime.status ?? 1)
+}
+
+const result = spawnSync(
+  pnpm,
+  [
+    'exec',
+    'playwright',
+    'test',
+    'tests/e2e/ssh-reconnect-pane-cardinality.spec.ts',
+    '--config',
+    'tests/playwright.config.ts',
+    '--project',
+    'electron-headless',
+    '--workers=1',
+    ...extraArgs
+  ],
+  spawnOptions
+)
+
+process.exit(result.status ?? 1)

package.json

@@ -97,6 +97,7 @@
     "test:e2e:ssh-docker-perf": "node config/scripts/run-ssh-docker-perf-e2e.mjs",
     "test:e2e:ssh-docker-watcher-isolation": "node config/scripts/run-ssh-docker-watcher-isolation-e2e.mjs",
     "test:e2e:ssh-docker-terminal-parking": "node config/scripts/run-ssh-docker-terminal-parking-e2e.mjs",
+    "test:e2e:ssh-docker-reconnect-pane-cardinality": "node config/scripts/run-ssh-docker-reconnect-pane-cardinality-e2e.mjs",
     "test:e2e:nested-runtime-ssh": "node config/scripts/run-nested-runtime-ssh-e2e.mjs",
     "test:e2e:source-control-scale": "pnpm run ensure:electron-runtime && npx playwright test tests/e2e/source-control-large-file-count.spec.ts --config tests/playwright.config.ts --project electron-headless --workers=1",
     "win-update-e2e": "node tests/tools/win-update-e2e/run.mjs",

src/main/persistence.ts

@@ -7223,6 +7223,14 @@ export class Store {
     if (winner.state === 'terminated' || winner.state === 'expired') {
       return
     }
+    // Why consult the binding here: at upsert time the caller's 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 = this.durablyBoundPtyIdForPane(winner.targetId, winner.tabId, winner.leafId)
+    if (boundPtyId && boundPtyId !== winner.ptyId) {
+      return
+    }
     const superseded: SshRemotePtyLease[] = []
     for (const lease of this.state.sshRemotePtyLeases ?? []) {
       if (
@@ -7252,6 +7260,50 @@ export class Store {
    *
    * Returns the number of leases retired, for logging.
    */
+  /** The PTY a pane is durably bound to, across the target and local partitions. */
+  private durablyBoundPtyIdForPane(
+    targetId: string,
+    tabId: string,
+    leafId: string
+  ): string | undefined {
+    for (const session of [
+      this.state.workspaceSessionsByHostId?.[toSshExecutionHostId(targetId)],
+      this.state.workspaceSession
+    ]) {
+      const boundPtyId = session?.terminalLayoutsByTabId?.[tabId]?.ptyIdsByLeafId?.[leafId]
+      if (boundPtyId) {
+        return this.getRelayPtyIdForSshLeaseComparison(targetId, boundPtyId)
+      }
+    }
+    return undefined
+  }
+
+  /**
+   * The durable pane binding outranks recency. Picking the newest lease alone
+   * would retire the one the pane is actually bound to whenever a newer unbound
+   * lease exists, which detaches a live pane instead of healing it.
+   */
+  private outranksForPane(
+    candidate: SshRemotePtyLease,
+    incumbent: SshRemotePtyLease,
+    targetId: string
+  ): boolean {
+    const boundPtyId = this.durablyBoundPtyIdForPane(
+      targetId,
+      candidate.tabId ?? '',
+      candidate.leafId ?? ''
+    )
+    if (boundPtyId) {
+      if (incumbent.ptyId === boundPtyId) {
+        return false
+      }
+      if (candidate.ptyId === boundPtyId) {
+        return true
+      }
+    }
+    return isNewerSshRemotePtyLease(candidate, incumbent)
+  }
+
   supersedeDuplicatePaneLeases(targetId: string): number {
     const live = (this.state.sshRemotePtyLeases ?? []).filter(
       (lease) =>
@@ -7264,7 +7316,7 @@ export class Store {
       }
       const paneKey = [lease.worktreeId, lease.tabId, lease.leafId].join('\0')
       const incumbent = winnerByPane.get(paneKey)
-      if (!incumbent || isNewerSshRemotePtyLease(lease, incumbent)) {
+      if (!incumbent || this.outranksForPane(lease, incumbent, targetId)) {
         winnerByPane.set(paneKey, lease)
       }
     }

src/main/ssh-reattach-pane-cardinality.test.ts

@@ -103,24 +103,50 @@ describe('STA-3077: one pane owns at most one live remote PTY lease', () => {
   })
 })
 
-describe('STA-3077: superseding a lease scrubs only the predecessor binding', () => {
-  it('drops the stale pane binding without disturbing the successor', async () => {
-    const store = await createStore()
-
-    // Spawn-shaped setup: the creating branches are intentionally allowed here.
+describe('STA-3077: superseding respects the durable pane binding', () => {
+  // Spawn-shaped setup: the creating branches are intentionally allowed here.
+  function bindPaneTo(store: Awaited<ReturnType<typeof createStore>>, ptyId: string) {
     store.persistPtyBinding({
       worktreeId: WORKTREE,
       tabId: TAB,
       leafId: LEAF,
-      ptyId: 'relay-pty-a',
-      incarnationId: 'inc-a'
+      ptyId,
+      incarnationId: `inc-${ptyId}`
     })
+  }
+
+  it('scrubs the predecessor binding when the arriving lease is the bound one', async () => {
+    const store = await createStore()
+    bindPaneTo(store, 'relay-pty-a')
+    store.upsertSshRemotePtyLease(leaseFor('relay-pty-a', 1))
+    bindPaneTo(store, 'relay-pty-b')
+    store.upsertSshRemotePtyLease(leaseFor('relay-pty-b', 2))
+
+    expect(liveLeasesForPane(store).map((lease) => lease.ptyId)).toEqual(['relay-pty-b'])
+    const layout = store.getWorkspaceSession().terminalLayoutsByTabId?.[TAB]
+    expect(layout?.ptyIdsByLeafId?.[LEAF]).toBe('relay-pty-b')
+  })
+
+  // Expiring the bound predecessor here would detach a live pane, so both stay
+  // live and reattach arbitrates with the binding in hand.
+  it('defers instead of expiring the lease the pane is bound to', async () => {
+    const store = await createStore()
+    bindPaneTo(store, 'relay-pty-a')
     store.upsertSshRemotePtyLease(leaseFor('relay-pty-a', 1))
     store.upsertSshRemotePtyLease(leaseFor('relay-pty-b', 2))
 
-    const layout = store.getWorkspaceSession().terminalLayoutsByTabId?.[TAB]
-    expect(layout?.ptyIdsByLeafId?.[LEAF]).not.toBe('relay-pty-a')
-    expect(liveLeasesForPane(store).map((lease) => lease.ptyId)).toEqual(['relay-pty-b'])
+    expect(
+      liveLeasesForPane(store)
+        .map((lease) => lease.ptyId)
+        .sort()
+    ).toEqual(['relay-pty-a', 'relay-pty-b'])
+    expect(store.getWorkspaceSession().terminalLayoutsByTabId?.[TAB]?.ptyIdsByLeafId?.[LEAF]).toBe(
+      'relay-pty-a'
+    )
+
+    // Arbitration keeps the bound lease, not the newer one.
+    expect(store.supersedeDuplicatePaneLeases(TARGET)).toBe(1)
+    expect(liveLeasesForPane(store).map((lease) => lease.ptyId)).toEqual(['relay-pty-a'])
   })
 })
 
@@ -141,6 +167,26 @@ describe('STA-3077: existing duplicate leases are healed, not revived', () => {
     expect(liveLeasesForPane(store).map((lease) => lease.ptyId)).toEqual(['relay-pty-19'])
   })
 
+  // Recency alone would retire the lease the pane is actually bound to whenever a
+  // newer unbound lease exists, detaching a live pane instead of healing it.
+  it('keeps the durably bound lease even when an unbound one is newer', async () => {
+    const store = await createStore()
+    store.persistPtyBinding({
+      worktreeId: WORKTREE,
+      tabId: TAB,
+      leafId: LEAF,
+      ptyId: 'relay-pty-bound',
+      incarnationId: 'inc-bound'
+    })
+    store.upsertSshRemotePtyLease({ ...leaseFor('relay-pty-bound', 1), createdAt: 1 })
+    // Arrives later but no pane is bound to it.
+    store.upsertSshRemotePtyLease({ ...leaseFor('relay-pty-newer', 99), createdAt: 99 })
+
+    store.supersedeDuplicatePaneLeases(TARGET)
+
+    expect(liveLeasesForPane(store).map((lease) => lease.ptyId)).toEqual(['relay-pty-bound'])
+  })
+
   it('leaves distinct panes alone', async () => {
     const otherLeaf = '8a2b4c6d-1e3f-4a5b-8c7d-9e0f1a2b3c4d'
     const store = await createStore({

tests/e2e/helpers/docker-ssh-relay-remote-ptys.ts

@@ -0,0 +1,87 @@
+import { execDockerSshRelayTargetCommand, type DockerSshRelayTarget } from './docker-ssh-relay-target'
+
+/** One live remote shell the relay is hosting on the Docker target. */
+export type DockerSshRelayRemotePty = {
+  pid: number
+  relayPid: number
+  pts: string
+  paneKey: string | null
+  tabId: string | null
+  worktreeId: string | null
+}
+
+// A remote PTY is a direct child of a detached relay whose stdin is a pts —
+// node-pty's forkpty child. That excludes relay-watcher.js and every sshd
+// session, so the count is exactly "shells the relay is hosting right now".
+const LIST_RELAY_REMOTE_PTYS_COMMAND = `
+relay_pids=()
+for proc in /proc/[0-9]*; do
+  [ -r "$proc/cmdline" ] || continue
+  argv=()
+  mapfile -d '' -t argv < "$proc/cmdline" 2>/dev/null || continue
+  [ "\${argv[1]##*/}" = relay.js ] || continue
+  case " \${argv[*]:2} " in *" --detached "*) relay_pids+=("\${proc##*/}") ;; esac
+done
+[ \${#relay_pids[@]} -gt 0 ] || exit 0
+for proc in /proc/[0-9]*; do
+  [ -r "$proc/status" ] || continue
+  ppid="$(awk '/^PPid:/{print $2}' "$proc/status" 2>/dev/null)"
+  [ -n "$ppid" ] || continue
+  matched=
+  for relay in "\${relay_pids[@]}"; do
+    if [ "$relay" = "$ppid" ]; then matched=1; break; fi
+  done
+  [ -n "$matched" ] || continue
+  pts="$(readlink "$proc/fd/0" 2>/dev/null)"
+  case "$pts" in /dev/pts/*) ;; *) continue ;; esac
+  pane=; tab=; worktree=
+  while IFS= read -r -d '' entry; do
+    case "$entry" in
+      ORCA_PANE_KEY=*) pane="\${entry#ORCA_PANE_KEY=}" ;;
+      ORCA_TAB_ID=*) tab="\${entry#ORCA_TAB_ID=}" ;;
+      ORCA_WORKTREE_ID=*) worktree="\${entry#ORCA_WORKTREE_ID=}" ;;
+    esac
+  done < "$proc/environ"
+  printf '%s\\t%s\\t%s\\t%s\\t%s\\t%s\\n' "\${proc##*/}" "$ppid" "$pts" "\${pane:--}" "\${tab:--}" "\${worktree:--}"
+done
+`
+
+function optional(value: string | undefined): string | null {
+  return value === undefined || value === '' || value === '-' ? null : value
+}
+
+export function readDockerSshRelayRemotePtys(
+  target: DockerSshRelayTarget
+): DockerSshRelayRemotePty[] {
+  const output = execDockerSshRelayTargetCommand(target, LIST_RELAY_REMOTE_PTYS_COMMAND)
+  if (!output) {
+    return []
+  }
+  return output
+    .split('\n')
+    .map((line) => {
+      const [rawPid, rawRelayPid, pts, paneKey, tabId, worktreeId] = line.split('\t')
+      const pid = Number(rawPid)
+      const relayPid = Number(rawRelayPid)
+      // Why: a vanished /proc entry must throw so expect.poll retries rather
+      // than folding a half-read row into the census as pid 0.
+      if (!Number.isInteger(pid) || !Number.isInteger(relayPid) || !pts?.startsWith('/dev/pts/')) {
+        throw new Error(`Unexpected Docker SSH relay remote PTY row: ${line}`)
+      }
+      return {
+        pid,
+        relayPid,
+        pts,
+        paneKey: optional(paneKey),
+        tabId: optional(tabId),
+        worktreeId: optional(worktreeId)
+      }
+    })
+    .sort((left, right) => left.pid - right.pid)
+}
+
+export function describeDockerSshRelayRemotePtys(ptys: DockerSshRelayRemotePty[]): string {
+  return ptys
+    .map((pty) => `${pty.pid}@${pty.relayPid} ${pty.pts} pane=${pty.paneKey ?? '-'}`)
+    .join(', ')
+}

tests/e2e/ssh-reconnect-pane-cardinality.spec.ts

@@ -0,0 +1,166 @@
+import type { Page, TestInfo } from '@stablyai/playwright-test'
+import { test, expect } from './helpers/orca-app'
+import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store'
+import {
+  splitActiveTerminalPane,
+  waitForActivePanePtyId,
+  waitForActiveTerminalManager,
+  waitForPaneIdentitySnapshot
+} from './helpers/terminal'
+import {
+  cleanupDockerSshRelayTarget,
+  startDockerSshRelayTarget,
+  type DockerSshRelayTarget
+} from './helpers/docker-ssh-relay-target'
+import {
+  connectDockerSshRelayTarget,
+  reconnectDockerSshRelayTarget
+} from './helpers/docker-ssh-relay-connection'
+import {
+  describeDockerSshRelayRemotePtys,
+  readDockerSshRelayRemotePtys
+} from './helpers/docker-ssh-relay-remote-ptys'
+
+const RUN_DOCKER_SSH = process.env.ORCA_E2E_SSH_DOCKER === '1'
+const PANE_COUNT = 2
+const RECONNECT_CYCLES = 3
+// Why: the relay must outlive every disconnect. If it exits with the client the
+// remote shells die too, reconnect degrades to a cold spawn, and the reattach
+// path this spec exists to bound is never entered.
+const RELAY_GRACE_PERIOD_SECONDS = 900
+
+test.use({ seedTestRepo: false })
+
+/** Every terminal pane the user can see in a workspace, keyed tab/leaf. */
+type RemotePaneCensus = {
+  tabIds: string[]
+  paneIds: string[]
+}
+
+async function readRemotePaneCensus(page: Page, worktreeId: string): Promise<RemotePaneCensus> {
+  return page.evaluate((worktreeId) => {
+    const state = window.__store?.getState()
+    if (!state) {
+      throw new Error('Store unavailable')
+    }
+    type LayoutNode =
+      | { type: 'leaf'; leafId: string }
+      | { type: 'split'; first: LayoutNode; second: LayoutNode }
+      | null
+    const collectLeafIds = (node: LayoutNode): string[] => {
+      if (!node) {
+        return []
+      }
+      return node.type === 'leaf'
+        ? [node.leafId]
+        : [...collectLeafIds(node.first), ...collectLeafIds(node.second)]
+    }
+    const tabs = state.tabsByWorktree[worktreeId] ?? []
+    const paneIds = tabs.flatMap((tab) => {
+      const leafIds = collectLeafIds(
+        (state.terminalLayoutsByTabId[tab.id]?.root ?? null) as LayoutNode
+      )
+      // Why: `root: null` is the implicit single-pane layout a fresh tab carries
+      // until it is first split, so it still counts as one visible pane.
+      return leafIds.length > 0
+        ? leafIds.map((leafId) => `${tab.id}/${leafId}`)
+        : [`${tab.id}/<root>`]
+    })
+    return { tabIds: tabs.map((tab) => tab.id), paneIds: paneIds.sort() }
+  }, worktreeId)
+}
+
+// STA-3077: reconnecting an SSH-backed workspace must be cardinality-neutral.
+// The report had relay PTYs go 2 -> 19 -> 20 over three reconnects while panes
+// the user never opened appeared alongside them, so both counts are asserted
+// against the same fixed workspace after every cycle.
+test.describe('SSH reconnect pane and remote PTY cardinality', () => {
+  test.skip(!RUN_DOCKER_SSH, 'Set ORCA_E2E_SSH_DOCKER=1 to run Docker-backed SSH tests.')
+  test.skip(process.platform === 'win32', 'Docker SSH reconnect uses POSIX SSH tooling.')
+
+  test('adds no panes and no remote PTYs across repeated reconnects', async ({
+    orcaPage
+  }, testInfo: TestInfo) => {
+    test.setTimeout(480_000)
+    let target: DockerSshRelayTarget | null = null
+    try {
+      target = startDockerSshRelayTarget(testInfo)
+      const relayTarget = target
+      await waitForSessionReady(orcaPage)
+      const remote = await connectDockerSshRelayTarget(orcaPage, relayTarget, {
+        relayGracePeriodSeconds: RELAY_GRACE_PERIOD_SECONDS
+      })
+      await expect
+        .poll(() => waitForActiveWorktree(orcaPage), { timeout: 30_000 })
+        .toBe(remote.worktreeId)
+      await ensureTerminalVisible(orcaPage, 45_000)
+      await waitForActiveTerminalManager(orcaPage, 60_000)
+      await waitForActivePanePtyId(orcaPage, 60_000)
+
+      await splitActiveTerminalPane(orcaPage, 'vertical')
+      await waitForPaneIdentitySnapshot(orcaPage, PANE_COUNT)
+      const baselinePanes = await readRemotePaneCensus(orcaPage, remote.worktreeId)
+      expect(baselinePanes.paneIds).toHaveLength(PANE_COUNT)
+
+      // The remote census is the reporter's oracle: shells the relay hosts right
+      // now, counted on the container rather than inferred from app state.
+      await expect
+        .poll(() => readDockerSshRelayRemotePtys(relayTarget).length, {
+          timeout: 60_000,
+          message: 'remote shells did not settle at one per pane before the first reconnect'
+        })
+        .toBe(PANE_COUNT)
+      const baselineRemotePtys = readDockerSshRelayRemotePtys(relayTarget)
+      const baselinePids = baselineRemotePtys.map((pty) => pty.pid)
+      testInfo.annotations.push({
+        type: 'ssh-reconnect-cardinality-baseline',
+        description: describeDockerSshRelayRemotePtys(baselineRemotePtys)
+      })
+
+      for (let cycle = 1; cycle <= RECONNECT_CYCLES; cycle += 1) {
+        await reconnectDockerSshRelayTarget(orcaPage, remote.targetId)
+        await ensureTerminalVisible(orcaPage, 45_000)
+        await waitForActiveTerminalManager(orcaPage, 60_000)
+        await waitForActivePanePtyId(orcaPage, 60_000)
+
+        // Poll rather than sample once: a leaked pane or a duplicate shell can
+        // land after reattach reports ready, and a single read would miss it.
+        await expect
+          .poll(() => readDockerSshRelayRemotePtys(relayTarget).map((pty) => pty.pid), {
+            timeout: 90_000,
+            message: `reconnect ${cycle} changed the live remote shells`
+          })
+          .toEqual(baselinePids)
+        await expect
+          .poll(async () => (await readRemotePaneCensus(orcaPage, remote.worktreeId)).paneIds, {
+            timeout: 30_000,
+            message: `reconnect ${cycle} changed the visible terminal panes`
+          })
+          .toEqual(baselinePanes.paneIds)
+        expect(
+          (await readRemotePaneCensus(orcaPage, remote.worktreeId)).tabIds,
+          `reconnect ${cycle} changed the workspace tabs`
+        ).toEqual(baselinePanes.tabIds)
+
+        // Settle before the next cycle so a late graft is attributed to the
+        // reconnect that caused it instead of leaking into the next assertion.
+        await expect
+          .poll(() => readDockerSshRelayRemotePtys(relayTarget).map((pty) => pty.pid), {
+            intervals: [2_000, 2_000, 2_000],
+            timeout: 8_000,
+            message: `reconnect ${cycle} grew the remote shells after settling`
+          })
+          .toEqual(baselinePids)
+      }
+
+      testInfo.annotations.push({
+        type: 'ssh-reconnect-cardinality-final',
+        description: `${RECONNECT_CYCLES} reconnects: ${describeDockerSshRelayRemotePtys(
+          readDockerSshRelayRemotePtys(relayTarget)
+        )}`
+      })
+    } finally {
+      cleanupDockerSshRelayTarget(target)
+    }
+  })
+})