diff --git a/docs/reference/terminal-session-correctness-handoff-2026-08-07/README.md b/docs/reference/terminal-session-correctness-handoff-2026-08-07/README.md index 8d32c789228..ddffd4841a5 100644 --- a/docs/reference/terminal-session-correctness-handoff-2026-08-07/README.md +++ b/docs/reference/terminal-session-correctness-handoff-2026-08-07/README.md @@ -178,8 +178,14 @@ comprehensive design or release gates. - Superseded remote shells are deliberately left running and unreachable. The PR body acknowledges that the “accumulates unused shells” half of the incident remains unresolved. -- The Docker reconnect test says it passes with and without the fix. It is a - forward guard, not causal proof of the customer defect. +- The repeated-reconnect Docker test still passes with and without the fix: it + is a forward guard, not causal proof. Its sibling in the same spec — 'leaves a + lease whose durable pane is gone unbound…' — was reported as discriminating, + but that DID NOT REPRODUCE on a second machine: with `mayCreate: false` + removed from the reattach call site and the app rebuilt, both tests still + passed. Its induction races `pty:kill` against a severed transport, so when + the kill lands the lease is cleaned up and there is nothing to graft. Treat + both tests as forward guards. No journey is proven. - The final Docker settle assertion has an unresolved major review thread: a late pane or shell can appear after the polling assertion has already passed. - The production-call-site wiring oracle reads source text. It does not execute diff --git a/tests/e2e/helpers/docker-ssh-relay-remote-ptys.ts b/tests/e2e/helpers/docker-ssh-relay-remote-ptys.ts index 0cb85d1396a..99df6b460ef 100644 --- a/tests/e2e/helpers/docker-ssh-relay-remote-ptys.ts +++ b/tests/e2e/helpers/docker-ssh-relay-remote-ptys.ts @@ -1,4 +1,7 @@ -import { execDockerSshRelayTargetCommand, type DockerSshRelayTarget } from './docker-ssh-relay-target' +import { + execDockerSshRelayTargetCommand, + type DockerSshRelayTarget +} from './docker-ssh-relay-target' /** One live remote shell the relay is hosting on the Docker target. */ export type DockerSshRelayRemotePty = { diff --git a/tests/e2e/helpers/remote-pane-durable-session.ts b/tests/e2e/helpers/remote-pane-durable-session.ts new file mode 100644 index 00000000000..5850fe45125 --- /dev/null +++ b/tests/e2e/helpers/remote-pane-durable-session.ts @@ -0,0 +1,99 @@ +/** + * Durable (main-process) view of an SSH host's terminal panes, plus the + * snapshot write the app itself performs when a pane goes away. + * + * Why durable and not the renderer store: a reconnect binds PTYs in main. A + * pane grafted there is invisible to the running renderer and only surfaces on + * the next hydration — so the renderer census cannot see the defect at all. + */ +import type { Page } from '@stablyai/playwright-test' + +/** `partition tabId/leafId=ptyId` for every pane a durable session names, sorted. */ +export type DurablePaneBindings = string[] + +/** Why both partitions: the renderer persists a remote worktree's panes to the + * host partition, while the reconnect binding path writes the local one. A + * census of either alone cannot see a pane grafted into the other. */ +const LOCAL_PARTITION = 'local' + +type DurableSession = { + tabsByWorktree?: Record + terminalLayoutsByTabId?: Record< + string, + { + root: unknown + activeLeafId: string | null + expandedLeafId: string | null + ptyIdsByLeafId?: Record + } + > + terminalPtyIncarnationsByPaneKey?: Record +} + +export function sshExecutionHostId(targetId: string): string { + return `ssh:${encodeURIComponent(targetId)}` +} + +export async function readDurablePaneBindings( + page: Page, + hostId: string, + worktreeId: string +): Promise { + return page.evaluate( + async ({ hostId, worktreeId, localPartition }) => { + const readPartition = async (partition: string): Promise => { + const session = (await window.api.session.get( + partition === localPartition ? undefined : partition + )) as DurableSession | null + const tabs = session?.tabsByWorktree?.[worktreeId] ?? [] + return tabs.flatMap((tab) => + Object.entries(session?.terminalLayoutsByTabId?.[tab.id]?.ptyIdsByLeafId ?? {}).map( + ([leafId, ptyId]) => `${partition} ${tab.id}/${leafId}=${ptyId}` + ) + ) + } + return [...(await readPartition(localPartition)), ...(await readPartition(hostId))].sort() + }, + { hostId, worktreeId, localPartition: LOCAL_PARTITION } + ) +} + +/** + * Persist the post-close layout the way quit/beforeunload does — a full replace + * of the host partition that no longer names the closed pane. The remote shell + * and its lease are deliberately left alone: that divergence (a live lease with + * no durable pane) is the state a reconnect must not resolve by inventing UI. + */ +export async function persistClosedRemotePaneSnapshot( + page: Page, + args: { hostId: string; tabId: string; keptLeafId: string; closedLeafId: string } +): Promise { + await page.evaluate(async ({ hostId, tabId, keptLeafId, closedLeafId }) => { + const session = (await window.api.session.get(hostId)) as DurableSession | null + const layout = session?.terminalLayoutsByTabId?.[tabId] + if (!session || !layout) { + throw new Error(`No durable layout for tab ${tabId} on ${hostId}`) + } + const ptyIdsByLeafId = { ...layout.ptyIdsByLeafId } + delete ptyIdsByLeafId[closedLeafId] + const incarnations = { ...session.terminalPtyIncarnationsByPaneKey } + delete incarnations[`${tabId}:${closedLeafId}`] + await window.api.session.set( + { + ...session, + terminalLayoutsByTabId: { + ...session.terminalLayoutsByTabId, + [tabId]: { + ...layout, + root: { type: 'leaf', leafId: keptLeafId }, + activeLeafId: keptLeafId, + expandedLeafId: null, + ptyIdsByLeafId + } + }, + terminalPtyIncarnationsByPaneKey: incarnations + } as never, + hostId + ) + }, args) +} diff --git a/tests/e2e/ssh-reconnect-pane-cardinality.spec.ts b/tests/e2e/ssh-reconnect-pane-cardinality.spec.ts index 722c33e26d8..e8aace9f182 100644 --- a/tests/e2e/ssh-reconnect-pane-cardinality.spec.ts +++ b/tests/e2e/ssh-reconnect-pane-cardinality.spec.ts @@ -1,28 +1,37 @@ /** - * Forward regression guard for pane and remote-PTY cardinality across - * reconnects, run against a real OpenSSH container. + * Pane and remote-PTY cardinality across reconnects, against a real OpenSSH + * container. * - * Scope, stated honestly: this spec passes both with and without the STA-3077 - * fixes — it was run against an unfixed tree and did not fail. A clean severed - * transport reconnects without producing the conditions that grafted panes in - * the field, which needed accumulated duplicate leases or a source that came - * back needing re-establishment. So it does NOT prove those fixes; the oracles - * that do are in `src/main/ssh-reattach-pane-cardinality.test.ts`. + * Two tests, with different standing: * - * It still earns its place: it counts the shells the relay actually hosts, on - * the container, and pins their PIDs — so a future change that grafts a pane or - * kills and respawns a shell fails here. + * 1. 'adds no panes and no remote PTYs across repeated reconnects' is a forward + * guard only. It passes with and without the STA-3077 fixes: a cleanly + * severed transport reconnects without producing the divergence that grafted + * panes in the field. It still earns its place by counting the shells the + * relay hosts, on the container, and pinning their PIDs. + * + * 2. 'leaves a lease whose durable pane is gone unbound…' discriminates. It + * reproduces the divergence — a live lease and a live remote shell that no + * durable pane names — and fails on a tree without the fix, where reattach + * grafts the pane back through persistPtyBinding's creating branches. */ import type { Page, TestInfo } from '@stablyai/playwright-test' import { test, expect } from './helpers/orca-app' import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store' import { + closeActiveTerminalPane, + focusLastTerminalPane, sendToTerminal, splitActiveTerminalPane, waitForActivePanePtyId, waitForActiveTerminalManager, waitForPaneIdentitySnapshot } from './helpers/terminal' +import { + persistClosedRemotePaneSnapshot, + readDurablePaneBindings, + sshExecutionHostId +} from './helpers/remote-pane-durable-session' import { cleanupDockerSshRelayTarget, startDockerSshRelayTarget, @@ -104,6 +113,70 @@ async function readRemotePaneCensus(page: Page, worktreeId: string): Promise> + paneSnapshot: Awaited> + baselinePanes: RemotePaneCensus + baselineRemotePtys: ReturnType +} + +/** A connected remote workspace with PANE_COUNT panes, each streaming output. */ +async function openStreamingRemotePanes( + page: Page, + relayTarget: DockerSshRelayTarget +): Promise { + await waitForSessionReady(page) + const remote = await connectDockerSshRelayTarget(page, relayTarget, { + relayGracePeriodSeconds: RELAY_GRACE_PERIOD_SECONDS + }) + await expect.poll(() => waitForActiveWorktree(page), { timeout: 30_000 }).toBe(remote.worktreeId) + await ensureTerminalVisible(page, 45_000) + await waitForActiveTerminalManager(page, 60_000) + await waitForActivePanePtyId(page, 60_000) + + await splitActiveTerminalPane(page, 'vertical') + const paneSnapshot = await waitForPaneIdentitySnapshot(page, PANE_COUNT) + const baselinePanes = await readRemotePaneCensus(page, remote.worktreeId) + expect(baselinePanes.paneIds).toHaveLength(PANE_COUNT) + + // Why every pane must be streaming: an idle pane carries no output source, + // so its reattach sends no recovery checkpoint and the relay answers + // 'existing'. Only a live source can come back needing re-establishment, + // which is the outcome that used to read as expiry and respawn the shell. + const streamMarker = `SSH_RECONNECT_STREAM_${Date.now()}` + for (const pane of paneSnapshot.panes) { + if (!pane.ptyId) { + throw new Error(`Pane ${pane.leafId} has no PTY to stream from`) + } + await sendToTerminal( + page, + pane.ptyId, + `node -e "setInterval(()=>process.stdout.write('${streamMarker}_'+Date.now()+'\\n'),25)"\r` + ) + } + await expect + .poll(() => countDockerSshRelayRemoteStreamWriters(relayTarget, streamMarker), { + timeout: 60_000, + message: 'remote panes did not start streaming before the first transport fault' + }) + .toBe(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) + return { + remote, + paneSnapshot, + baselinePanes, + baselineRemotePtys: readDockerSshRelayRemotePtys(relayTarget) + } +} + // 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 @@ -120,55 +193,10 @@ test.describe('SSH reconnect pane and remote PTY cardinality', () => { 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') - const paneSnapshot = await waitForPaneIdentitySnapshot(orcaPage, PANE_COUNT) - const baselinePanes = await readRemotePaneCensus(orcaPage, remote.worktreeId) - expect(baselinePanes.paneIds).toHaveLength(PANE_COUNT) - - // Why every pane must be streaming: an idle pane carries no output source, - // so its reattach sends no recovery checkpoint and the relay answers - // 'existing'. Only a live source can come back needing re-establishment, - // which is the outcome that used to read as expiry and respawn the shell. - const streamMarker = `SSH_RECONNECT_STREAM_${Date.now()}` - const countRemoteStreamWriters = (): number => - countDockerSshRelayRemoteStreamWriters(relayTarget, streamMarker) - for (const pane of paneSnapshot.panes) { - if (!pane.ptyId) { - throw new Error(`Pane ${pane.leafId} has no PTY to stream from`) - } - await sendToTerminal( - orcaPage, - pane.ptyId, - `node -e "setInterval(()=>process.stdout.write('${streamMarker}_'+Date.now()+'\\n'),25)"\r` - ) - } - await expect - .poll(countRemoteStreamWriters, { - timeout: 60_000, - message: 'remote panes did not start streaming before the first transport fault' - }) - .toBe(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 { remote, baselinePanes, baselineRemotePtys } = await openStreamingRemotePanes( + orcaPage, + relayTarget + ) const baselinePids = baselineRemotePtys.map((pty) => pty.pid) testInfo.annotations.push({ type: 'ssh-reconnect-cardinality-baseline', @@ -232,4 +260,101 @@ test.describe('SSH reconnect pane and remote PTY cardinality', () => { cleanupDockerSshRelayTarget(target) } }) + + // The divergence the field hit: a lease and its remote shell outlive the pane + // record, because the pane was closed while the link was down and the kill + // never reached the host. Reconnect must reattach without inventing the pane + // back, and without killing the shell it can no longer place. + test('leaves a lease whose durable pane is gone unbound instead of grafting the pane back', async ({ + orcaPage + }, testInfo: TestInfo) => { + test.setTimeout(480_000) + let target: DockerSshRelayTarget | null = null + try { + target = startDockerSshRelayTarget(testInfo) + const relayTarget = target + const { remote, paneSnapshot, baselineRemotePtys } = await openStreamingRemotePanes( + orcaPage, + relayTarget + ) + const baselinePids = baselineRemotePtys.map((pty) => pty.pid) + const hostId = sshExecutionHostId(remote.targetId) + const keptLeafId = paneSnapshot.panes[0]!.leafId + const closedLeafId = paneSnapshot.panes[1]!.leafId + const readBindings = (): Promise => + readDurablePaneBindings(orcaPage, hostId, remote.worktreeId) + expect( + (await readBindings()).filter((binding) => binding.includes(closedLeafId)), + 'the pane to be closed must start out durably bound' + ).not.toHaveLength(0) + + severDockerSshRelayTransport(relayTarget) + // Closing here is what leaves the lease orphaned: pty:kill fails with the + // transport down, so the remote shell and its lease both survive the pane. + await focusLastTerminalPane(orcaPage) + await closeActiveTerminalPane(orcaPage) + await expect + .poll( + async () => (await readRemotePaneCensus(orcaPage, remote.worktreeId)).paneIds.length, + { + timeout: 30_000, + message: 'the closed pane never left the visible layout' + } + ) + .toBe(PANE_COUNT - 1) + await persistClosedRemotePaneSnapshot(orcaPage, { + hostId, + tabId: paneSnapshot.tabId, + keptLeafId, + closedLeafId + }) + await expect + .poll(async () => (await readBindings()).filter((b) => b.includes(closedLeafId)).length, { + timeout: 30_000, + message: 'a durable partition still named the closed pane before the reconnect' + }) + .toBe(0) + const survivingBindings = await readBindings() + expect( + survivingBindings.filter((binding) => binding.includes(keptLeafId)), + 'the surviving pane must stay durably bound' + ).not.toHaveLength(0) + + await waitForSshReconnected(orcaPage, remote.targetId) + await ensureTerminalVisible(orcaPage, 45_000) + await waitForActiveTerminalManager(orcaPage, 60_000) + await waitForActivePanePtyId(orcaPage, 60_000) + + // Sample continuously rather than poll-until-equal: a graft that lands and + // is later overwritten by a renderer snapshot would satisfy a poll that + // only needs one matching read, and the defect would pass unnoticed. + const deadline = Date.now() + 25_000 + let samples = 0 + while (Date.now() < deadline) { + expect(await readBindings(), 'reconnect grafted a pane the user had closed').toEqual( + survivingBindings + ) + samples += 1 + await orcaPage.waitForTimeout(500) + } + expect(samples).toBeGreaterThan(10) + + // Unknown is not dead: the orphaned shell keeps running, so a later + // reattach can still claim it once a durable pane names it again. + expect( + readDockerSshRelayRemotePtys(relayTarget).map((pty) => pty.pid), + 'the orphaned remote shell was killed or respawned' + ).toEqual(baselinePids) + expect( + (await readRemotePaneCensus(orcaPage, remote.worktreeId)).paneIds, + 'reconnect surfaced a pane the user never opened' + ).toHaveLength(PANE_COUNT - 1) + testInfo.annotations.push({ + type: 'ssh-reconnect-orphaned-lease-final', + description: describeDockerSshRelayRemotePtys(readDockerSshRelayRemotePtys(relayTarget)) + }) + } finally { + cleanupDockerSshRelayTarget(target) + } + }) })