diff --git a/src/main/ipc/pty/ipc/renderer-kill.ts b/src/main/ipc/pty/ipc/renderer-kill.ts index c8ead4d8562..68b932bc278 100644 --- a/src/main/ipc/pty/ipc/renderer-kill.ts +++ b/src/main/ipc/pty/ipc/renderer-kill.ts @@ -14,6 +14,7 @@ import { type ResolvedPtyHost } from '../provider/registry' import { finishPtyShutdown, isPtyAlreadyGoneError } from '../provider/liveness' +import { stopLegacyRelayBridgedPty } from '../runtime/kill' export type PtyKillIpcDeps = { store?: Store @@ -90,6 +91,11 @@ async function stopRendererOwnedPtyProcess( } = deps runtime?.markPtyStopRequested?.(args.id) const connectionId = getPtySshConnectionId(args.id) + const bridgedStop = stopLegacyRelayBridgedPty(args.id, connectionId, args.keepHistory === true) + if (bridgedStop) { + await bridgedStop + return + } // Why: wait for daemon startup before selecting the local provider, else a fallback shutdown falsely succeeds and orphans a restored daemon PTY (#7742). const startupPromise = getLocalPtyProviderStartupPromise(resolvePtyExecutionHost(args.id)) if (startupPromise) { diff --git a/src/main/ipc/pty/runtime/kill-legacy-relay-bridged-exit.test.ts b/src/main/ipc/pty/runtime/kill-legacy-relay-bridged-exit.test.ts new file mode 100644 index 00000000000..d14a9556f0c --- /dev/null +++ b/src/main/ipc/pty/runtime/kill-legacy-relay-bridged-exit.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it, vi } from 'vitest' +import { toAppSshPtyId } from '../../../../shared/ssh-pty-id' +import type { PtyRuntimeControllerDeps } from './controller-deps' + +const bridge = vi.hoisted(() => ({ proven: true })) + +vi.mock('../../../ssh/legacy-relay/legacy-relay-bridge-registry', () => ({ + closeLegacyRelayPane: vi.fn(), + isLegacyRelayBridgedPty: () => true, + stopLegacyRelayPaneAndWait: async () => bridge.proven +})) + +const { stopAndWaitPtyFromRuntimeController } = await import('./kill') + +describe('a proven stop-and-wait of a terminal an older relay still runs', () => { + it('tells the runtime the terminal exited, without tombstoning the lease the bridge retired', async () => { + for (const proven of [true, false]) { + bridge.proven = proven + const deps = { + runtime: { markPtyStopRequested: vi.fn(), onPtyExit: vi.fn() }, + store: null, + getLocalPtyProviderStartupPromise: () => null, + shutdownProviderAndDetectExit: vi.fn(), + finishPtyShutdown: vi.fn(() => undefined), + rememberSyntheticKillExit: vi.fn(), + sendPtyExitToRenderer: vi.fn() + } + const ptyId = toAppSshPtyId('ssh-box', 'pty2:epoch:1') + await expect( + stopAndWaitPtyFromRuntimeController( + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the bridged branch reads only these deps. + deps as unknown as PtyRuntimeControllerDeps, + ptyId, + { deadlineMs: Date.now() + 1_000 } + ) + ).resolves.toBe(proven) + if (proven) { + expect(deps.finishPtyShutdown).toHaveBeenCalledWith(ptyId, null, null) + expect(deps.runtime.onPtyExit).toHaveBeenCalledWith(ptyId, 0, undefined) + } else { + expect(deps.finishPtyShutdown).not.toHaveBeenCalled() + expect(deps.runtime.onPtyExit).not.toHaveBeenCalled() + } + } + }) +}) diff --git a/src/main/ipc/pty/runtime/kill-legacy-relay-bridged.test.ts b/src/main/ipc/pty/runtime/kill-legacy-relay-bridged.test.ts index d12b65e5395..545097d0d4d 100644 --- a/src/main/ipc/pty/runtime/kill-legacy-relay-bridged.test.ts +++ b/src/main/ipc/pty/runtime/kill-legacy-relay-bridged.test.ts @@ -1,9 +1,13 @@ import { describe, expect, it, vi } from 'vitest' import { toAppSshPtyId } from '../../../../shared/ssh-pty-id' import type { SshRemotePtyLease } from '../../../../shared/ssh-types' -import { installLegacyRelayBridge } from '../../../ssh/legacy-relay/legacy-relay-bridge-registry' +import { + closeLegacyRelayPane, + installLegacyRelayBridge +} from '../../../ssh/legacy-relay/legacy-relay-bridge-registry' +import { stopRendererOwnedPty, type PtyKillIpcDeps } from '../ipc/renderer-kill' import type { PtyRuntimeControllerDeps } from './controller-deps' -import { killPtyFromRuntimeController } from './kill' +import { killPtyFromRuntimeController, stopAndWaitPtyFromRuntimeController } from './kill' const TARGET = 'ssh-box' const lease: SshRemotePtyLease = { @@ -21,6 +25,7 @@ describe('a generic kill of a terminal an older relay still runs', () => { installLegacyRelayBridge({ leases: () => [lease], retireLease, + recordStop: vi.fn(), // Disconnected: the bridge can only record the stop for the next connect. connection: () => null, onPanesChanged: () => {}, @@ -50,3 +55,134 @@ describe('a generic kill of a terminal an older relay still runs', () => { installLegacyRelayBridge(null) }) }) + +describe('a tab close of a terminal an older relay still runs', () => { + it('hands the stop to the bridge instead of tombstoning the lease the bridge needs', async () => { + const leases: SshRemotePtyLease[] = [{ ...lease }] + const store = { + markSshRemotePtyLease: vi.fn((_target: string, relayPtyId: string) => { + for (const row of leases) { + if (row.ptyId === relayPtyId) { + row.state = 'terminated' + } + } + }) + } + installLegacyRelayBridge({ + leases: () => leases, + retireLease: vi.fn(), + recordStop: vi.fn(), + // Disconnected, so the SSH provider is unregistered: the bridge records the stop. + connection: () => null, + onPanesChanged: () => {}, + onData: () => {}, + onReplay: () => {}, + onExit: () => {}, + report: () => {} + }) + const ptyId = toAppSshPtyId(TARGET, lease.ptyId) + const deps = { + store, + getLocalPtyProviderStartupPromise: () => undefined, + shutdownProviderAndDetectExit: vi.fn(), + rememberSyntheticKillExit: vi.fn(), + sendPtyExitToRenderer: vi.fn() + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the bridged branch reads only these deps. + await stopRendererOwnedPty(deps as unknown as PtyKillIpcDeps, { id: ptyId }) + expect(store.markSshRemotePtyLease).not.toHaveBeenCalled() + expect(deps.shutdownProviderAndDetectExit).not.toHaveBeenCalled() + // The transport's own close that follows still reaches a pending stop, not `not-bridged`. + await expect(closeLegacyRelayPane(ptyId)).resolves.toEqual({ status: 'unverifiable' }) + installLegacyRelayBridge(null) + }) + + it('refuses a hibernation stop without recording a stop that would outlive it', async () => { + const recordStop = vi.fn() + const store = { markSshRemotePtyLease: vi.fn() } + installLegacyRelayBridge({ + leases: () => [lease], + retireLease: vi.fn(), + recordStop, + connection: () => null, + onPanesChanged: () => {}, + onData: () => {}, + onReplay: () => {}, + onExit: () => {}, + report: () => {} + }) + const deps = { + store, + getLocalPtyProviderStartupPromise: () => undefined, + shutdownProviderAndDetectExit: vi.fn(), + rememberSyntheticKillExit: vi.fn(), + sendPtyExitToRenderer: vi.fn() + } + await expect( + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the bridged branch reads only these deps. + stopRendererOwnedPty(deps as unknown as PtyKillIpcDeps, { + id: toAppSshPtyId(TARGET, lease.ptyId), + keepHistory: true + }) + ).rejects.toThrow('legacy_relay_bridged_pty_cannot_sleep') + expect(recordStop).not.toHaveBeenCalled() + expect(store.markSshRemotePtyLease).not.toHaveBeenCalled() + installLegacyRelayBridge(null) + }) +}) + +describe('a stop-and-wait of a terminal an older relay still runs', () => { + function install(recordStop = vi.fn()) { + installLegacyRelayBridge({ + leases: () => [lease], + retireLease: vi.fn(), + recordStop, + // Disconnected, so no SSH provider is registered for the target. + connection: () => null, + onPanesChanged: () => {}, + onData: () => {}, + onReplay: () => {}, + onExit: () => {}, + report: () => {} + }) + const deps = { + runtime: null, + store: null, + getLocalPtyProviderStartupPromise: () => null, + shutdownProviderAndDetectExit: vi.fn(), + finishPtyShutdown: vi.fn() + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the bridged branch reads only these deps. + return deps as unknown as PtyRuntimeControllerDeps & { + finishPtyShutdown: ReturnType + } + } + + it('records the stop with the bridge instead of tombstoning its lease (CLI close, worktree delete)', async () => { + const recordStop = vi.fn() + const deps = install(recordStop) + const ptyId = toAppSshPtyId(TARGET, lease.ptyId) + await expect( + stopAndWaitPtyFromRuntimeController(deps, ptyId, { deadlineMs: Date.now() + 1_000 }) + ).resolves.toBe(false) + expect(deps.finishPtyShutdown).not.toHaveBeenCalled() + expect(recordStop).toHaveBeenCalledWith( + TARGET, + lease.ptyId, + expect.objectContaining({ attempts: 0 }) + ) + installLegacyRelayBridge(null) + }) + + it('leaves a reversible stop’s pane live, with no durable stop recorded', async () => { + const recordStop = vi.fn() + const deps = install(recordStop) + const ptyId = toAppSshPtyId(TARGET, lease.ptyId) + await expect( + stopAndWaitPtyFromRuntimeController(deps, ptyId, { keepHistory: true }) + ).resolves.toBe(false) + expect(deps.finishPtyShutdown).not.toHaveBeenCalled() + expect(recordStop).not.toHaveBeenCalled() + installLegacyRelayBridge(null) + }) +}) diff --git a/src/main/ipc/pty/runtime/kill.ts b/src/main/ipc/pty/runtime/kill.ts index 5c281e35fcf..40f254739ab 100644 --- a/src/main/ipc/pty/runtime/kill.ts +++ b/src/main/ipc/pty/runtime/kill.ts @@ -11,7 +11,8 @@ import type { PtyRuntimeControllerDeps } from './controller-deps' import { errorMessage } from '../../../../shared/error-message' import { closeLegacyRelayPane, - isLegacyRelayBridgedPty + isLegacyRelayBridgedPty, + stopLegacyRelayPaneAndWait } from '../../../ssh/legacy-relay/legacy-relay-bridge-registry' // Bounds the wait for an observed exit to reach the runtime record when the caller set no deadline. @@ -28,6 +29,25 @@ function publishSyntheticExit( deps.sendPtyExitToRenderer({ id: ptyId, code, ...(incarnationId ? { incarnationId } : {}) }) } +/** + * COMPAT(legacy-relay-bridge, revisit 2027-01-15): an older relay's shell stops through the + * bridge, which retires its lease only on the relay's proof; tombstoning it here would orphan it. + * Every stop route checks this first, before reading the (absent) SSH provider. A reversible stop + * is refused: the bridge's recorded stop would outlive it and kill a shell the pane wakes back to. + */ +export function stopLegacyRelayBridgedPty( + ptyId: string, + connectionId: string | null, + reversible = false +): Promise | null { + if (!connectionId || !isLegacyRelayBridgedPty(ptyId)) { + return null + } + return reversible + ? Promise.reject(new Error('legacy_relay_bridged_pty_cannot_sleep')) + : closeLegacyRelayPane(ptyId) +} + export function killPtyFromRuntimeController( deps: PtyRuntimeControllerDeps, ptyId: string @@ -42,10 +62,7 @@ export function killPtyFromRuntimeController( } = deps runtime?.markPtyStopRequested?.(ptyId) const connectionId = getPtySshConnectionId(ptyId) - // COMPAT(legacy-relay-bridge, revisit 2027-01-15): an older relay's shell stops through the - // bridge, which retires its lease only on the relay's proof; tombstoning it here would orphan it. - if (connectionId && isLegacyRelayBridgedPty(ptyId)) { - void closeLegacyRelayPane(ptyId) + if (stopLegacyRelayBridgedPty(ptyId, connectionId)) { return true } // Why: close runtime tails without clearing provider ownership, so @@ -134,7 +151,8 @@ export function retireRejectedPtyFromRuntimeController( } /** - * Deliberately records no undelivered-stop intent, unlike `killPtyFromRuntimeController`. + * Deliberately records no undelivered-stop intent, unlike `killPtyFromRuntimeController`, except + * the legacy relay bridge's for a non-reversible stop (the tab is given up for good). * * Only a caller that gives the PTY up for good may leave a replayable kill order behind. This one * hands its failures back instead: worktree sleep marks these stops reversible and leaves the pane @@ -155,6 +173,22 @@ export async function stopAndWaitPtyFromRuntimeController( } = deps runtime?.markPtyStopRequested?.(ptyId) const connectionId = getPtySshConnectionId(ptyId) + if (connectionId && isLegacyRelayBridgedPty(ptyId)) { + // A reversible stop must not leave the bridge's durable stop behind, so the pane stays live. + if (opts?.keepHistory) { + return false + } + const deadline = opts?.deadlineMs ?? Date.now() + OBSERVED_EXIT_RECORD_WAIT_MS + const stopped = await Promise.race([ + stopLegacyRelayPaneAndWait(ptyId, deadline), + delay(Math.max(1, deadline - Date.now())).then(() => false) + ]) + if (stopped) { + // The bridge already retired the lease on the relay's proof; the runtime learns it here. + publishSyntheticExit(deps, ptyId, 0, finishPtyShutdown(ptyId, null, store)) + } + return stopped + } // Why: destructive teardown threads one absolute deadline through every await // below; each RPC leaf converts it to the remaining time when it issues, so // sequential RPCs share the budget and cannot overrun the sweep deadline. diff --git a/src/main/persistence-ssh-pending-pty-kill.test.ts b/src/main/persistence-ssh-pending-pty-kill.test.ts index 7b013b2d921..3f6a7aafb62 100644 --- a/src/main/persistence-ssh-pending-pty-kill.test.ts +++ b/src/main/persistence-ssh-pending-pty-kill.test.ts @@ -23,8 +23,8 @@ vi.mock('./telemetry/cohort-classifier', () => ({ getCohortAtEmit: vi.fn(() => ( const NOW = 1_800_000_000_000 -// Relay-era builds record undelivered stops on the lease; this build only carries them so a -// downgrade can still replay them. +// Relay-era builds record undelivered stops on the lease; this build writes them only for the +// legacy relay bridge, and otherwise carries them so a downgrade can still replay them. describe('persisted SSH pending PTY kills', () => { beforeEach(() => { testState.dir = mkdtempSync(join(tmpdir(), 'orca-test-')) @@ -71,4 +71,22 @@ describe('persisted SSH pending PTY kills', () => { { ptyId: 'pty-2', pendingKill: undefined } ]) }) + + it('keeps a stop the legacy relay bridge records across a restart, until it clears it', async () => { + const store = createStoreWithSshRemotePtyLeases([ + { targetId: 'ssh-1', ptyId: 'pty2:epoch-a:1', state: 'detached', tabId: 'tab-1' } + ]) + store.setSshRemotePtyLeasePendingKill('ssh-1', 'pty2:epoch-a:1', { + requestedAt: NOW, + attempts: 0 + }) + + const reloaded = createStore() + expect(reloaded.getSshRemotePtyLeases('ssh-1')[0]).toMatchObject({ + state: 'detached', + pendingKill: { requestedAt: NOW, attempts: 0 } + }) + reloaded.setSshRemotePtyLeasePendingKill('ssh-1', 'pty2:epoch-a:1', null) + expect(createStore().getSshRemotePtyLeases('ssh-1')[0].pendingKill).toBeUndefined() + }) }) diff --git a/src/main/persistence/leasing-ssh-ptys/ssh-pty-lease-operations.ts b/src/main/persistence/leasing-ssh-ptys/ssh-pty-lease-operations.ts index 3fee959c955..2576b708d83 100644 --- a/src/main/persistence/leasing-ssh-ptys/ssh-pty-lease-operations.ts +++ b/src/main/persistence/leasing-ssh-ptys/ssh-pty-lease-operations.ts @@ -1,9 +1,10 @@ import type { PersistedState } from '../../../shared/persisted-state-types' +import type { SshPendingPtyKill } from '../../../shared/ssh-pending-pty-kill' import type { SshRemotePtyLease } from '../../../shared/ssh-types' import { invalidateLocalWorktreeMetadataPruneInputs } from '../../local-worktree-metadata-prune-gate' import { pruneRetiredSshRemotePtyLeaseTombstones } from './ssh-pty-lease-tombstone-retention' -// Leases come only from relay-era profiles; this build retires them and never writes new ones. +// Leases come only from relay-era profiles; this build retires them and never creates new ones. export type SshPtyLeaseOperations = { state: PersistedState toStoredPtyId: (targetId: string, ptyId: string) => string @@ -49,6 +50,29 @@ export function markSshRemotePtyLease( } } +/** The legacy relay bridge's record of a closed tab's undelivered stop; null clears it. */ +export function setSshRemotePtyLeasePendingKill( + operations: SshPtyLeaseOperations, + targetId: string, + ptyId: string, + pendingKill: SshPendingPtyKill | null +): void { + const relayPtyId = operations.toStoredPtyId(targetId, ptyId) + const lease = operations.state.sshRemotePtyLeases?.find( + (entry) => entry.targetId === targetId && entry.ptyId === relayPtyId + ) + if (!lease || (!pendingKill && !lease.pendingKill)) { + return + } + if (pendingKill) { + lease.pendingKill = pendingKill + } else { + delete lease.pendingKill + } + lease.updatedAt = Date.now() + operations.flush() +} + export function removeSshRemotePtyLeases( operations: SshPtyLeaseOperations, targetId: string diff --git a/src/main/persistence/loading-store/ssh-lease-recovery-operations.ts b/src/main/persistence/loading-store/ssh-lease-recovery-operations.ts index 5787fdcf222..d1415701769 100644 --- a/src/main/persistence/loading-store/ssh-lease-recovery-operations.ts +++ b/src/main/persistence/loading-store/ssh-lease-recovery-operations.ts @@ -1,8 +1,10 @@ +import type { SshPendingPtyKill } from '../../../shared/ssh-pending-pty-kill' import type { SshRemotePtyLease } from '../../../shared/ssh-types' import { getSshRemotePtyLeases as getSshRemotePtyLeasesOperation, markSshRemotePtyLease as markSshRemotePtyLeaseOperation, removeSshRemotePtyLeases as removeSshRemotePtyLeasesOperation, + setSshRemotePtyLeasePendingKill as setSshRemotePtyLeasePendingKillOperation, type SshPtyLeaseOperations } from '../leasing-ssh-ptys/ssh-pty-lease-operations' import { @@ -58,6 +60,19 @@ export class SshLeaseRecoveryOperations { markSshRemotePtyLeaseOperation(getSshPtyLeaseOperations(this), targetId, ptyId, state) } + setSshRemotePtyLeasePendingKill( + targetId: string, + ptyId: string, + pendingKill: SshPendingPtyKill | null + ): void { + setSshRemotePtyLeasePendingKillOperation( + getSshPtyLeaseOperations(this), + targetId, + ptyId, + pendingKill + ) + } + removeSshRemotePtyLeases(targetId: string): void { removeSshRemotePtyLeasesOperation(getSshPtyLeaseOperations(this), targetId) } diff --git a/src/main/ssh/legacy-relay/README.md b/src/main/ssh/legacy-relay/README.md index 81d51d14eed..6543a51c943 100644 --- a/src/main/ssh/legacy-relay/README.md +++ b/src/main/ssh/legacy-relay/README.md @@ -19,6 +19,12 @@ nobody is prompted. - A dropped connection, or a disconnect that hangs the bridge up, is `unverifiable` and reattaches later; removing the target is what empties its panes. A pane retires (lease `terminated`) only when its relay reports the exit, or every relay for the target answered without it. +- Closing a bridged tab, by any stop route, sends the relay's own `pty.shutdown`. A stop the relay + can't take yet is kept on the lease as `pendingKill`, so it survives a restart; that lease is + listed for the stop but never served as a pane again. A stop no later run could fence to the same + process (a legacy `pty-N` id with no incarnation) gets one try, then the lease ends. +- Sleeping a bridged pane is refused (the pane stays live): its stop would be durable, so it would + outlive the sleep. A stop-and-wait caller succeeds only on the relay's exit proof. **Touches** `ipc/ssh.ts`, `ipc/ssh-connect-flow.ts`, `ipc/ssh-host-server-on-connect-wiring.ts`, `ipc/ssh-session-teardown.ts`, the conversion gate's `bridged` verdict diff --git a/src/main/ssh/legacy-relay/legacy-relay-bridge-ipc.ts b/src/main/ssh/legacy-relay/legacy-relay-bridge-ipc.ts index 6093e52b62d..8cc8aefe512 100644 --- a/src/main/ssh/legacy-relay/legacy-relay-bridge-ipc.ts +++ b/src/main/ssh/legacy-relay/legacy-relay-bridge-ipc.ts @@ -20,9 +20,9 @@ import { legacyRelayBridgedPanes, resizeLegacyRelayPane, setLegacyRelayPaneOutputPaused, - terminalsBucket, writeLegacyRelayPane } from './legacy-relay-bridge-registry' +import { terminalsBucket } from './legacy-relay-daily-activity' const PtyIdSchema = z.string().min(1).max(4096) const SizeSchema = z.number().int().positive().max(10_000) @@ -34,7 +34,13 @@ const PausedArgsSchema = z.object({ ptyId: PtyIdSchema, paused: z.boolean() }) const ResizeArgsSchema = z.object({ ptyId: PtyIdSchema, cols: SizeSchema, rows: SizeSchema }) export function registerLegacyRelayBridgeIpc(args: { - store: Pick + store: Pick< + Store, + | 'getSshRemotePtyLeases' + | 'markSshRemotePtyLease' + | 'setSshRemotePtyLeasePendingKill' + | 'getSshTarget' + > getMainWindow: () => BrowserWindow | null connection: (targetId: string) => SshConnection | null runtime?: () => OrcaRuntimeService | null @@ -47,8 +53,13 @@ export function registerLegacyRelayBridgeIpc(args: { } installLegacyRelayBridge({ leases: (targetId) => args.store.getSshRemotePtyLeases(targetId), - retireLease: (targetId, relayPtyId) => - args.store.markSshRemotePtyLease(targetId, relayPtyId, 'terminated'), + retireLease: (targetId, relayPtyId) => { + // The exit delivered any recorded stop; cleared first so the retire can prune the tombstone. + args.store.setSshRemotePtyLeasePendingKill(targetId, relayPtyId, null) + args.store.markSshRemotePtyLease(targetId, relayPtyId, 'terminated') + }, + recordStop: (targetId, relayPtyId, pendingKill) => + args.store.setSshRemotePtyLeasePendingKill(targetId, relayPtyId, pendingKill), connection: args.connection, ...(args.runtime ? { diff --git a/src/main/ssh/legacy-relay/legacy-relay-bridge-leases.ts b/src/main/ssh/legacy-relay/legacy-relay-bridge-leases.ts index 12fa0178fc8..31c7f95b607 100644 --- a/src/main/ssh/legacy-relay/legacy-relay-bridge-leases.ts +++ b/src/main/ssh/legacy-relay/legacy-relay-bridge-leases.ts @@ -15,35 +15,59 @@ export const ATTACH_WAIT_MS = 20_000 /** The one attach refusal backed by the relay's own liveness check (1.4.2xx relays). */ export const PROVEN_EXITED = /PTY ".+" not found \(process exited\)/i -/** - * One a relay-era pane still names and nobody closed: a lease with a stop still pending, or one a - * newer pane won, was meant to go away and is never brought back. - */ -export function isBridgeableLease(lease: SshRemotePtyLease): boolean { +/** A relay-era pane's lease whose shell may still run; one a newer pane won is never brought back. */ +function isRelayEraPaneLease(lease: SshRemotePtyLease): boolean { return ( lease.state !== 'terminated' && typeof lease.tabId === 'string' && lease.tabId.length > 0 && - !lease.pendingKill && !lease.supersededBy && !lease.relayIdRecycled ) } +/** One a relay-era pane still names and nobody closed. */ +export function isBridgeableLease(lease: SshRemotePtyLease): boolean { + return isRelayEraPaneLease(lease) && !lease.pendingKill +} + +/** + * One whose tab closed before its relay confirmed the stop. The bridge still lists its relay to + * send the stop, including after a restart, but never serves it as a pane again. + */ +export function isStoppingLease(lease: SshRemotePtyLease): boolean { + return isRelayEraPaneLease(lease) && Boolean(lease.pendingKill) +} + export function bridgeableLeasesByRelayId( leases: readonly SshRemotePtyLease[] ): Map { return new Map(leases.filter(isBridgeableLease).map((lease) => [lease.ptyId, lease])) } -export function leasedAppPtyIds(targetId: string, leases: SshRemotePtyLease[]): Set { - return new Set( - [...bridgeableLeasesByRelayId(leases).keys()].map((relayPtyId) => - toAppSshPtyId(targetId, relayPtyId) - ) +/** Every lease the bridge lists a relay for: the panes it serves and the stops it owes. */ +export function listedLeasesByRelayId( + leases: readonly SshRemotePtyLease[] +): Map { + return new Map( + leases + .filter((lease) => isBridgeableLease(lease) || isStoppingLease(lease)) + .map((lease) => [lease.ptyId, lease]) ) } +export function leasedAppPtyIds(targetId: string, leases: SshRemotePtyLease[]): Set { + return toAppPtyIds(targetId, bridgeableLeasesByRelayId(leases)) +} + +export function listedAppPtyIds(targetId: string, leases: SshRemotePtyLease[]): Set { + return toAppPtyIds(targetId, listedLeasesByRelayId(leases)) +} + +function toAppPtyIds(targetId: string, leases: Map): Set { + return new Set([...leases.keys()].map((relayPtyId) => toAppSshPtyId(targetId, relayPtyId))) +} + /** One leased pane the bridge tracks, and the relay endpoint that listed it. */ export type Pane = { lease: SshRemotePtyLease @@ -52,6 +76,37 @@ export type Pane = { state: 'live' | 'unverifiable' /** What the listing relay claimed about the process, for the relay-era CLI's identity checks. */ listed?: LegacyRelayPty + /** Its tab closed and its stop is still owed: listed for the stop, never served. */ + stopping?: true +} + +export function toPane( + lease: SshRemotePtyLease, + relayPtyId: string, + endpoint: string | null, + state: Pane['state'], + listed?: LegacyRelayPty +): Pane { + return { + lease, + relayPtyId, + endpoint, + state, + ...(listed ? { listed } : {}), + ...(lease.pendingKill ? { stopping: true as const } : {}) + } +} + +/** The panes a bridge serves; a closed tab's pane, its stop owed or its lease ended, is not one. */ +export function servedBridgedPanes( + targetId: string, + panes: ReadonlyMap, + leases: readonly SshRemotePtyLease[] +): LegacyRelayBridgedPane[] { + const served = bridgeableLeasesByRelayId(leases) + return [...panes] + .filter(([, pane]) => !pane.stopping && served.has(pane.relayPtyId)) + .map(([ptyId, pane]) => toBridgedPane(targetId, ptyId, pane)) } export type LegacyRelayLivePane = { ptyId: string; incarnationId?: string; terminalHandle?: string } @@ -59,15 +114,18 @@ export type LegacyRelayLivePane = { ptyId: string; incarnationId?: string; termi /** The live leased pane a pane key's stable leaf names; the lease is the desktop's own record. */ export function findLivePane( panes: ReadonlyMap, - paneKey: string + paneKey: string, + leases: readonly SshRemotePtyLease[] ): LegacyRelayLivePane | null { const parsed = parsePaneKey(paneKey) if (!parsed) { return null } + const served = bridgeableLeasesByRelayId(leases) for (const [ptyId, pane] of panes) { // Why the leaf alone: a moved pane's lease takes its new tab, but its shell keeps the old key. - if (pane.state === 'live' && pane.lease.leafId === parsed.leafId) { + const named = pane.lease.leafId === parsed.leafId && served.has(pane.relayPtyId) + if (pane.state === 'live' && !pane.stopping && named) { const { incarnationId, terminalHandle } = pane.listed ?? {} return { ptyId, @@ -79,7 +137,7 @@ export function findLivePane( return null } -export function toBridgedPane(targetId: string, ptyId: string, pane: Pane): LegacyRelayBridgedPane { +function toBridgedPane(targetId: string, ptyId: string, pane: Pane): LegacyRelayBridgedPane { return { ptyId, targetId, diff --git a/src/main/ssh/legacy-relay/legacy-relay-bridge-registry.ts b/src/main/ssh/legacy-relay/legacy-relay-bridge-registry.ts index 355cfc37197..ea05f2ee281 100644 --- a/src/main/ssh/legacy-relay/legacy-relay-bridge-registry.ts +++ b/src/main/ssh/legacy-relay/legacy-relay-bridge-registry.ts @@ -9,15 +9,14 @@ import type { LegacyRelayBridgedPane } from '../../../shared/legacy-relay-bridge-contract' import { parseAppSshPtyId } from '../../../shared/ssh-pty-id' +import type { SshPendingPtyKill } from '../../../shared/ssh-pending-pty-kill' import type { SshRemotePtyLease } from '../../../shared/ssh-types' import type { SshConnection } from '../connection/ssh-connection' import { waitForSentinel } from './ssh-relay-sentinel' -import { probeRelayEndpointIncumbent } from './ssh-relay-endpoint-incumbent' -import { isWindowsRemoteHost } from '../ssh-remote-platform' import type { OrcaRuntimeService } from '../../runtime/orca-runtime' import { LegacyRelayClient } from './legacy-relay-client' import { createLegacyRelayCliServer } from './legacy-relay-cli-server' -import { listLegacyRelayEndpoints, type LegacyRelayEndpoint } from './legacy-relay-endpoints' +import { listLegacyRelayEndpoints, posixEndpointExited } from './legacy-relay-endpoints' import { LegacyRelayTargetBridge } from './legacy-relay-target-bridge' import { LegacyRelayHiddenOutput } from './legacy-relay-hidden-output' import { LegacyRelayPaneStops } from './legacy-relay-pane-stops' @@ -25,7 +24,8 @@ import { reportDailyRelayActivity, watchDailyRelayActivity } from './legacy-rela import { bridgeableLeasesByRelayId, isBridgeableLease, - leasedAppPtyIds + listedAppPtyIds, + listedLeasesByRelayId } from './legacy-relay-bridge-leases' /** How long a connect waits for the first look at its relays before publishing the host. */ @@ -33,12 +33,14 @@ const START_BUDGET_MS = 20_000 /** How long an attach waits for a target whose connection is still coming up. */ const CONNECTION_WAIT_MS = 20_000 -type TerminalsBucket = EventProps<'ssh_host_server_legacy_relay_bridge'>['terminals'] type ReportPhase = EventProps<'ssh_host_server_legacy_relay_bridge'>['phase'] export type LegacyRelayBridgeHost = { leases: (targetId: string) => SshRemotePtyLease[] + /** The relay proved the exit; any stop still recorded for it was delivered. */ retireLease: (targetId: string, relayPtyId: string) => void + /** Persists a closed tab's stop on its lease, so a restart still sends it. */ + recordStop: (targetId: string, relayPtyId: string, pendingKill: SshPendingPtyKill) => void connection: (targetId: string) => SshConnection | null onPanesChanged: (panes: LegacyRelayBridgedPane[]) => void onData: (ptyId: string, data: string) => void @@ -84,21 +86,9 @@ export function installLegacyRelayBridge(next: LegacyRelayBridgeHost | null): vo : null } -export function terminalsBucket(count: number): TerminalsBucket { - if (count <= 0) { - return 'none' - } - if (count === 1) { - return '1' - } - if (count < 5) { - return '2_4' - } - return count < 10 ? '5_9' : '10_plus' -} - -function hasBridgeableLeases(targetId: string): boolean { - return host?.leases(targetId).some(isBridgeableLease) ?? false +/** Panes it serves, or closed tabs' stops it still owes. */ +function hasListedLeases(targetId: string): boolean { + return listedLeasesByRelayId(host?.leases(targetId) ?? []).size > 0 } function bridgeFor(targetId: string): LegacyRelayTargetBridge | null { @@ -157,6 +147,7 @@ function bridgeFor(targetId: string): LegacyRelayTargetBridge | null { onData: (ptyId, data) => hiddenOutput?.onData(ptyId, data), onExit: (ptyId, code) => { hiddenOutput?.forget(ptyId) + stopsFor(targetId).exited(ptyId) installed.onExit(ptyId, code) }, onTerminalExited: () => installed.report(targetId, 'terminal_exited', 0) @@ -170,34 +161,17 @@ function stopsFor(targetId: string): LegacyRelayPaneStops { if (existing) { return existing } - const created = new LegacyRelayPaneStops(targetId, () => host?.leases(targetId) ?? []) + const created = new LegacyRelayPaneStops(targetId, () => host) paneStops.set(targetId, created) return created } -/** A dead socket is the one failed open that proves its relay gone; Windows pipes never are. */ -async function posixEndpointExited( - conn: SshConnection, - endpoint: LegacyRelayEndpoint -): Promise { - if (isWindowsRemoteHost(endpoint.host) || !endpoint.nodePath) { - return false - } - const incumbent = await probeRelayEndpointIncumbent( - conn, - endpoint.host, - endpoint.nodePath, - endpoint.endpoint - ) - return incumbent.verdict === 'exited' -} - /** * Called once a connect reaches the host's managed server: looks at the relays this target left, * so the panes they still run are known before the renderer remounts them on the new server. */ export async function startLegacyRelayBridge(targetId: string): Promise { - if (!hasBridgeableLeases(targetId)) { + if (!hasListedLeases(targetId)) { // Leases retired while hung up publish nothing themselves; this connect is the next answer. releaseHeldPanes(targetId) return @@ -266,7 +240,9 @@ export async function attachLegacyRelayPane( replayInStream = false ): Promise { const targetId = parseAppSshPtyId(ptyId)?.connectionId - if (!targetId || !host || !hasBridgeableLeases(targetId)) { + const serves = targetId ? host?.leases(targetId).some(isBridgeableLease) : false + // A closed tab's pane whose stop is owed is never served again. + if (!targetId || !serves || stopsFor(targetId).owes(ptyId)) { return { status: 'not-bridged' } } const bridge = bridgeFor(targetId) @@ -329,22 +305,33 @@ export function setLegacyRelayPaneOutputPaused(ptyId: string, paused: boolean): hiddenOutput?.setPaused(ptyId, paused) } -/** Whether this PTY is an older relay's terminal the bridge serves, so only the bridge stops it. */ +/** Whether this PTY is an older relay's terminal the bridge serves or owes a stop, so only the + * bridge stops it. */ export function isLegacyRelayBridgedPty(ptyId: string): boolean { const targetId = parseAppSshPtyId(ptyId)?.connectionId - return Boolean(targetId && host && leasedAppPtyIds(targetId, host.leases(targetId)).has(ptyId)) + return Boolean(targetId && host && listedAppPtyIds(targetId, host.leases(targetId)).has(ptyId)) } /** Its tab closed: the relay stops the shell, or is asked once it can be reached. */ export async function closeLegacyRelayPane(ptyId: string): Promise { const targetId = parseAppSshPtyId(ptyId)?.connectionId - if (!targetId || !host || !hasBridgeableLeases(targetId)) { + if (!targetId || !host || !hasListedLeases(targetId)) { return { status: 'not-bridged' } } // Disconnected: recorded now, and the target's next connect lists the relay and sends the stop. return stopsFor(targetId).stop(ptyId, host.connection(targetId) ? bridgeFor(targetId) : null) } +/** A close that settles only on the relay's exit proof, by the deadline. */ +export async function stopLegacyRelayPaneAndWait( + ptyId: string, + deadlineMs: number +): Promise { + const targetId = parseAppSshPtyId(ptyId)?.connectionId + const bridge = targetId && host?.connection(targetId) ? bridgeFor(targetId) : null + return targetId && host ? stopsFor(targetId).stopAndWait(ptyId, bridge, deadlineMs) : false +} + export function detachLegacyRelayPane(ptyId: string, acquisitionId: number): void { bridgeForPty(ptyId)?.detach(ptyId, acquisitionId) } diff --git a/src/main/ssh/legacy-relay/legacy-relay-bridge-restart-stop.test.ts b/src/main/ssh/legacy-relay/legacy-relay-bridge-restart-stop.test.ts new file mode 100644 index 00000000000..c1ba410516d --- /dev/null +++ b/src/main/ssh/legacy-relay/legacy-relay-bridge-restart-stop.test.ts @@ -0,0 +1,153 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { toAppSshPtyId } from '../../../shared/ssh-pty-id' +import type { SshRemotePtyLease } from '../../../shared/ssh-types' +import type { SshConnection } from '../connection/ssh-connection' +import { getRemoteHostPlatform } from '../ssh-remote-platform' + +const TARGET = 'ssh-box' +const RELAY_PTY = 'pty2:epoch:1' +const PTY = toAppSshPtyId(TARGET, RELAY_PTY) + +const relay = vi.hoisted(() => { + const shutdowns: string[] = [] + return { ptys: new Set(), shutdowns } +}) + +vi.mock('./legacy-relay-endpoints', () => ({ + listLegacyRelayEndpoints: async () => ({ + kind: 'listed', + exhaustive: true, + endpoints: [ + { + endpoint: '/home/me/.orca-remote/relay-x.sock', + connectCommand: 'node relay.js --connect', + wrapCommand: true, + host: getRemoteHostPlatform('linux-x64'), + nodePath: 'node' + } + ] + }) +})) + +vi.mock('./legacy-relay-client', () => ({ + LegacyRelayClient: { + open: async () => { + const exits = new Set<(id: string, code: number) => void>() + return { + ownerClaim: null, + closed: false, + list: async () => [...relay.ptys].map((relayPtyId) => ({ relayPtyId })), + shutdown: async (id: string) => { + relay.shutdowns.push(id) + relay.ptys.delete(id) + exits.forEach((listener) => listener(id, 0)) + }, + onData: () => () => {}, + onExit: (listener: (id: string, code: number) => void) => { + exits.add(listener) + return () => exits.delete(listener) + }, + onClose: () => () => {}, + close: () => {} + } + } + } +})) + +const registry = await import('./legacy-relay-bridge-registry') + +/** The persisted leases, which are all a restart keeps. */ +let leases: SshRemotePtyLease[] +let connected: boolean + +function installHost(): void { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the mocked endpoints and client never touch the connection. + const connection = {} as SshConnection + registry.installLegacyRelayBridge({ + leases: () => leases, + retireLease: (_target, relayPtyId) => { + leases = leases.map((row) => { + if (row.ptyId !== relayPtyId) { + return row + } + const { pendingKill: _delivered, ...rest } = row + return { ...rest, state: 'terminated' } + }) + }, + recordStop: (_target, relayPtyId, pendingKill) => { + leases = leases.map((row) => { + if (row.ptyId !== relayPtyId) { + return row + } + const { pendingKill: _dropped, ...rest } = row + return pendingKill ? { ...rest, pendingKill } : rest + }) + }, + connection: () => (connected ? connection : null), + onPanesChanged: () => {}, + onData: () => {}, + onReplay: () => {}, + onExit: () => {}, + report: () => {} + }) +} + +afterEach(() => { + registry.installLegacyRelayBridge(null) +}) + +describe('a bridged tab closed while disconnected', () => { + it('still stops its shell when Orca restarts before reconnecting', async () => { + relay.ptys = new Set([RELAY_PTY]) + relay.shutdowns = [] + leases = [ + { + targetId: TARGET, + ptyId: RELAY_PTY, + tabId: 'tab-1', + leafId: 'leaf-1', + state: 'detached', + createdAt: 1, + updatedAt: 1 + } + ] + connected = false + installHost() + await expect(registry.closeLegacyRelayPane(PTY)).resolves.toEqual({ status: 'unverifiable' }) + + // Restart, then reconnect. + installHost() + await expect(registry.attachLegacyRelayPane(PTY)).resolves.toEqual({ status: 'not-bridged' }) + connected = true + await registry.startLegacyRelayBridge(TARGET) + + expect(relay.shutdowns).toEqual([RELAY_PTY]) + expect(registry.legacyRelayBridgedPanes()).toEqual([]) + expect(leases[0].state).toBe('terminated') + expect(leases[0].pendingKill).toBeUndefined() + }) +}) + +describe('a stop-and-wait of a connected bridged tab', () => { + it('settles true only on the relay’s exit report', async () => { + relay.ptys = new Set([RELAY_PTY]) + relay.shutdowns = [] + leases = [ + { + targetId: TARGET, + ptyId: RELAY_PTY, + tabId: 'tab-1', + leafId: 'leaf-1', + state: 'detached', + createdAt: 1, + updatedAt: 1 + } + ] + connected = true + installHost() + await registry.startLegacyRelayBridge(TARGET) + await expect(registry.stopLegacyRelayPaneAndWait(PTY, Date.now() + 5_000)).resolves.toBe(true) + expect(relay.shutdowns).toEqual([RELAY_PTY]) + expect(leases[0].state).toBe('terminated') + }) +}) diff --git a/src/main/ssh/legacy-relay/legacy-relay-bridge-teardown.test.ts b/src/main/ssh/legacy-relay/legacy-relay-bridge-teardown.test.ts index f4d6b5291a9..bcc86aa430e 100644 --- a/src/main/ssh/legacy-relay/legacy-relay-bridge-teardown.test.ts +++ b/src/main/ssh/legacy-relay/legacy-relay-bridge-teardown.test.ts @@ -82,6 +82,7 @@ beforeEach(async () => { registry.installLegacyRelayBridge({ leases: () => leases, retireLease: vi.fn(), + recordStop: vi.fn(), connection: () => null, // Main publishes to the window as legacy-relay-bridge-ipc.ts does. onPanesChanged: (panes) => renderer.listeners.forEach((listener) => listener(panes)), @@ -156,6 +157,7 @@ describe('attaching a bridged pane', () => { registry.installLegacyRelayBridge({ leases: () => leases, retireLease: vi.fn(), + recordStop: vi.fn(), connection: () => connection, onPanesChanged: vi.fn(), onData: vi.fn(), diff --git a/src/main/ssh/legacy-relay/legacy-relay-bridge-timers.test.ts b/src/main/ssh/legacy-relay/legacy-relay-bridge-timers.test.ts new file mode 100644 index 00000000000..ab3baa81c04 --- /dev/null +++ b/src/main/ssh/legacy-relay/legacy-relay-bridge-timers.test.ts @@ -0,0 +1,38 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { LegacyRelayExitProofs } from './legacy-relay-bridge-timers' + +beforeEach(() => { + vi.useFakeTimers() +}) + +afterEach(() => { + vi.useRealTimers() +}) + +describe('waiting on a bridged pane’s exit proof', () => { + it('settles every overlapping waiter on one exit', async () => { + const proofs = new LegacyRelayExitProofs() + const first = proofs.wait('pty', 5_000) + const second = proofs.wait('pty', 5_000) + proofs.observe('pty') + await expect(Promise.all([first, second])).resolves.toEqual([true, true]) + }) + + it('never lets an earlier waiter’s deadline drop a later one', async () => { + const proofs = new LegacyRelayExitProofs() + const stale = proofs.wait('pty', 1_000) + await vi.advanceTimersByTimeAsync(500) + const retry = proofs.wait('pty', 5_000) + await vi.advanceTimersByTimeAsync(600) + await expect(stale).resolves.toBe(false) + proofs.observe('pty') + await expect(retry).resolves.toBe(true) + }) + + it('answers false on dispose', async () => { + const proofs = new LegacyRelayExitProofs() + const waiting = proofs.wait('pty', 5_000) + proofs.dispose() + await expect(waiting).resolves.toBe(false) + }) +}) diff --git a/src/main/ssh/legacy-relay/legacy-relay-bridge-timers.ts b/src/main/ssh/legacy-relay/legacy-relay-bridge-timers.ts index 78bfcb6cd5f..c3750e57eb6 100644 --- a/src/main/ssh/legacy-relay/legacy-relay-bridge-timers.ts +++ b/src/main/ssh/legacy-relay/legacy-relay-bridge-timers.ts @@ -64,3 +64,40 @@ export class LegacyRelayWaiters { } } } + +/** Stops awaiting the relay's exit proof (its report, or a complete listing without the pane). */ +export class LegacyRelayExitProofs { + private readonly waiting = new Map void>>() + + /** Each caller settles on its own: true on the exit, false at its deadline or on dispose. */ + wait(ptyId: string, timeoutMs: number): Promise { + return new Promise((resolve) => { + const waiters = this.waiting.get(ptyId) ?? new Set() + this.waiting.set(ptyId, waiters) + const settle = (exited: boolean): void => { + clearTimeout(timer) + waiters.delete(settle) + if (waiters.size === 0 && this.waiting.get(ptyId) === waiters) { + this.waiting.delete(ptyId) + } + resolve(exited) + } + const timer = setTimeout(() => settle(false), timeoutMs) + waiters.add(settle) + }) + } + + observe(ptyId: string): void { + for (const settle of Array.from(this.waiting.get(ptyId) ?? [])) { + settle(true) + } + } + + dispose(): void { + for (const ptyId of Array.from(this.waiting.keys())) { + for (const settle of Array.from(this.waiting.get(ptyId) ?? [])) { + settle(false) + } + } + } +} diff --git a/src/main/ssh/legacy-relay/legacy-relay-daily-activity.ts b/src/main/ssh/legacy-relay/legacy-relay-daily-activity.ts index 5677d526d12..44b6ab81346 100644 --- a/src/main/ssh/legacy-relay/legacy-relay-daily-activity.ts +++ b/src/main/ssh/legacy-relay/legacy-relay-daily-activity.ts @@ -2,6 +2,9 @@ * One `active` telemetry row per target per UTC day while it bridges relay terminals, so the * relay's retirement can see who still uses it. COMPAT(legacy-relay-bridge): revisit 2027-01-15. */ +import type { EventProps } from '../../../shared/telemetry-events' + +type TerminalsBucket = EventProps<'ssh_host_server_legacy_relay_bridge'>['terminals'] type ActivityReport = (targetId: string, phase: 'active', terminals: number) => void /** Why a timer: a terminal left attached across midnight changes no pane and attaches nothing. */ @@ -43,3 +46,16 @@ export function watchDailyRelayActivity( }, CHECK_INTERVAL_MS) timer.unref?.() } + +export function terminalsBucket(count: number): TerminalsBucket { + if (count <= 0) { + return 'none' + } + if (count === 1) { + return '1' + } + if (count < 5) { + return '2_4' + } + return count < 10 ? '5_9' : '10_plus' +} diff --git a/src/main/ssh/legacy-relay/legacy-relay-endpoints.ts b/src/main/ssh/legacy-relay/legacy-relay-endpoints.ts index 96263f12bfa..2011aa775b7 100644 --- a/src/main/ssh/legacy-relay/legacy-relay-endpoints.ts +++ b/src/main/ssh/legacy-relay/legacy-relay-endpoints.ts @@ -12,6 +12,7 @@ import { SHORT_RELAY_SOCKET_DIR_PREFIX } from '../relay-socket-path-limit' import { execCommand } from '../remote-exec/remote-exec-command' import { readRelayDaemonRuntimes } from './ssh-relay-endpoint-runtime' import { relaySocketNameForInstanceId } from './ssh-relay-instance-id' +import { probeRelayEndpointIncumbent } from './ssh-relay-endpoint-incumbent' import { readRemoteHomeCommand } from '../ssh-remote-commands' import { resolveRemoteNodePath } from '../ssh-remote-node-resolution' import { detectRemoteHostPlatform } from '../ssh-remote-platform-detection' @@ -209,3 +210,20 @@ async function listWindowsEndpoints( } return { kind: 'listed', endpoints, exhaustive } } + +/** A dead socket is the one failed open that proves its relay gone; Windows pipes never are. */ +export async function posixEndpointExited( + conn: SshConnection, + endpoint: LegacyRelayEndpoint +): Promise { + if (isWindowsRemoteHost(endpoint.host) || !endpoint.nodePath) { + return false + } + const incumbent = await probeRelayEndpointIncumbent( + conn, + endpoint.host, + endpoint.nodePath, + endpoint.endpoint + ) + return incumbent.verdict === 'exited' +} diff --git a/src/main/ssh/legacy-relay/legacy-relay-pane-stops.ts b/src/main/ssh/legacy-relay/legacy-relay-pane-stops.ts index 77d15a3169b..af3233731b0 100644 --- a/src/main/ssh/legacy-relay/legacy-relay-pane-stops.ts +++ b/src/main/ssh/legacy-relay/legacy-relay-pane-stops.ts @@ -5,17 +5,24 @@ * Held outside the bridge, so a reconnect's new bridge still sends them. A stop is pending until * the relay reports the exit or a listing proves the terminal gone; while the relay can't be asked * the answer is `unverifiable` and the stop goes out once it can. Loss of contact is never exit. + * + * A stop whose process can be named after a restart is persisted as the lease's `pendingKill`, the + * relay-era record of the same intent, so closing a tab and quitting before reconnecting still + * stops the shell. The relay connection is the other proof of identity, and it does not survive. */ +import { parseAppSshPtyId, toAppSshPtyId } from '../../../shared/ssh-pty-id' +import type { SshPendingPtyKill } from '../../../shared/ssh-pending-pty-kill' import type { SshRemotePtyLease } from '../../../shared/ssh-types' import type { LegacyRelayBridgeCloseResult } from '../../../shared/legacy-relay-bridge-contract' -import { leasedAppPtyIds } from './legacy-relay-bridge-leases' -import { LegacyRelayRetry } from './legacy-relay-bridge-timers' +import { isStoppingLease, listedAppPtyIds } from './legacy-relay-bridge-leases' +import { LegacyRelayExitProofs, LegacyRelayRetry } from './legacy-relay-bridge-timers' import type { LegacyRelayClient } from './legacy-relay-client' +import type { LegacyRelayBridgeHost } from './legacy-relay-bridge-registry' import type { LegacyRelayTargetBridge } from './legacy-relay-target-bridge' type StopTarget = Pick< LegacyRelayTargetBridge, - 'snapshot' | 'liveRoute' | 'listedIncarnation' | 'refresh' + 'paneState' | 'liveRoute' | 'listedIncarnation' | 'refresh' > /** @@ -24,31 +31,55 @@ type StopTarget = Pick< */ type PendingStop = { incarnationId?: string; client?: LegacyRelayClient } +/** How long a sent unfenced stop keeps its pane tracked for the relay's exit report. */ +const UNFENCED_EXIT_WAIT_MS = 5_000 + export class LegacyRelayPaneStops { private readonly closing = new Map() private bridge: StopTarget | null = null private readonly relist = new LegacyRelayRetry(() => void this.bridge?.refresh()) + private readonly exits = new LegacyRelayExitProofs() constructor( private readonly targetId: string, - private readonly leases: () => SshRemotePtyLease[] + private readonly host: () => Pick< + LegacyRelayBridgeHost, + 'leases' | 'recordStop' | 'retireLease' + > | null ) {} + private leases(): SshRemotePtyLease[] { + return this.host()?.leases(this.targetId) ?? [] + } + + private persist(relayPtyId: string, pendingKill: SshPendingPtyKill): void { + this.host()?.recordStop(this.targetId, relayPtyId, pendingKill) + } + /** `bridge` is null while the target is disconnected; its next listing sends the stop. */ async stop(ptyId: string, bridge: StopTarget | null): Promise { - const pane = bridge?.snapshot().find((entry) => entry.ptyId === ptyId) - if (!pane && !leasedAppPtyIds(this.targetId, this.leases()).has(ptyId)) { + const state = bridge?.paneState(ptyId) + if (!state && !listedAppPtyIds(this.targetId, this.leases()).has(ptyId)) { return { status: 'exited' } } - // A repeated close while the relay can't be asked keeps the identity the first one saw. - const earlier = this.closing.get(ptyId) - const incarnationId = bridge?.listedIncarnation(ptyId) ?? earlier?.incarnationId - const client = bridge?.liveRoute(ptyId)?.client ?? earlier?.client + // A repeated close keeps the identity the first one saw; the id may now name another process. + const earlier = this.pending(ptyId) + const incarnationId = earlier ? earlier.incarnationId : bridge?.listedIncarnation(ptyId) + const client = earlier ? earlier.client : bridge?.liveRoute(ptyId)?.client this.closing.set(ptyId, { ...(incarnationId ? { incarnationId } : {}), ...(client ? { client } : {}) }) - if (!bridge || pane?.state !== 'live') { + if (!this.record(ptyId, incarnationId)) { + // Unfenced: only this connection can prove the process, and a restart would forget the close, + // so it gets one try now and the lease ends as any closed tab's does. + const attempted = Boolean(bridge) && state === 'live' + const sent = bridge && attempted ? await this.send(ptyId, bridge) : false + // A failed reply may still have stopped it, so its exit report gets the same chance. + this.abandon(ptyId, bridge, attempted) + return sent ? { status: 'requested' } : { status: 'unverifiable' } + } + if (!bridge || state !== 'live') { return { status: 'unverifiable' } } const sent = await this.send(ptyId, bridge) @@ -57,22 +88,44 @@ export class LegacyRelayPaneStops { return sent ? { status: 'requested' } : { status: 'unverifiable' } } + /** True only once the relay proves the exit (its report, or a complete listing without it). */ + async stopAndWait( + ptyId: string, + bridge: StopTarget | null, + deadlineMs: number + ): Promise { + // Armed before the stop is sent, so an exit that beats its reply, or a lost reply, still counts. + const exited = this.exits.wait(ptyId, Math.max(1, deadlineMs - Date.now())) + // A stop sent to a live pane may still prove out after a failed reply; an unsent one cannot. + const sendable = bridge?.paneState(ptyId) === 'live' + const stopped = this.stop(ptyId, bridge).then((result) => + result.status === 'exited' ? true : result.status === 'requested' || sendable ? exited : false + ) + return Promise.race([exited.then((proven) => proven || stopped), stopped]) + } + + /** The bridge saw the pane exit. */ + exited(ptyId: string): void { + this.exits.observe(ptyId) + } + /** Runs after each listing: re-sends pending stops, re-listing on backoff while one still runs. */ flush(bridge: StopTarget): void { this.bridge = bridge - const panes = new Map(bridge.snapshot().map((pane) => [pane.ptyId, pane])) - const leased = leasedAppPtyIds(this.targetId, this.leases()) + const leases = this.leases() + const listed = listedAppPtyIds(this.targetId, leases) + const owed = new Set([...this.closing.keys(), ...this.persisted(leases).keys()]) let pending = false - for (const ptyId of this.closing.keys()) { - const pane = panes.get(ptyId) - if (!pane) { - if (!leased.has(ptyId)) { + for (const ptyId of owed) { + const state = bridge.paneState(ptyId) + if (!state) { + if (!listed.has(ptyId)) { this.closing.delete(ptyId) } continue } pending = true - if (pane.state === 'live') { + if (state === 'live') { void this.send(ptyId, bridge) } } @@ -85,17 +138,73 @@ export class LegacyRelayPaneStops { dispose(): void { this.relist.cancel() + this.exits.dispose() this.closing.clear() } + /** Whether its tab closed and the stop is still owed, recorded now or by a previous run. */ + owes(ptyId: string): boolean { + return this.pending(ptyId) !== undefined + } + get size(): number { return this.closing.size } + /** This session's record of the stop, else the one a previous run persisted. */ + private pending(ptyId: string): PendingStop | undefined { + return this.closing.get(ptyId) ?? this.persisted(this.leases()).get(ptyId) + } + + private persisted(leases: SshRemotePtyLease[]): Map { + const stops = new Map() + for (const lease of leases) { + if (isStoppingLease(lease)) { + const incarnationId = lease.pendingKill?.incarnationId + stops.set(toAppSshPtyId(this.targetId, lease.ptyId), incarnationId ? { incarnationId } : {}) + } + } + return stops + } + + /** Persists a stop a later run can fence (a `pty2:` id or a listed incarnation); false if none. */ + private record(ptyId: string, incarnationId: string | undefined): boolean { + const relayPtyId = parseAppSshPtyId(ptyId)?.relayPtyId + if (!relayPtyId || (!incarnationId && !relayPtyId.startsWith('pty2:'))) { + return false + } + const earlier = this.leases().find((lease) => lease.ptyId === relayPtyId)?.pendingKill + // A repeated close keeps the first one's age, which a downgraded build's expiry reads. + if (!earlier || earlier.incarnationId !== incarnationId) { + this.persist(relayPtyId, { + requestedAt: Date.now(), + ...(incarnationId ? { incarnationId } : {}), + attempts: 0 + }) + } + return true + } + + /** + * The stop can't be proven to reach the tab's process, so it is never sent. The lease ends as any + * closed tab's does, so a process that took the id is never served under the closed tab. + */ + private abandon(ptyId: string, bridge: StopTarget | null, attempted = false): void { + this.closing.delete(ptyId) + const relayPtyId = parseAppSshPtyId(ptyId)?.relayPtyId + if (relayPtyId) { + this.host()?.retireLease(this.targetId, relayPtyId) + } + // The next listing drops the pane it still holds for that lease; after an attempted stop, only + // once the relay's exit report had its chance, since dropping the pane first would hide it. + const relist = attempted ? this.exits.wait(ptyId, UNFENCED_EXIT_WAIT_MS) : Promise.resolve() + void relist.then(() => bridge?.refresh()) + } + /** The old relay's own stop, fenced to the process the tab ran; false when it can't be asked. */ private async send(ptyId: string, bridge: StopTarget): Promise { const route = bridge.liveRoute(ptyId) - const pending = this.closing.get(ptyId) + const pending = this.pending(ptyId) if (!route || !pending) { return false } @@ -107,7 +216,7 @@ export class LegacyRelayPaneStops { ? listed === pending.incarnationId || (listed === undefined && sameProcess) : sameProcess if (!proven) { - this.closing.delete(ptyId) + this.abandon(ptyId, bridge) return false } try { @@ -116,7 +225,7 @@ export class LegacyRelayPaneStops { } catch (error) { // The relay's own fence refused it: that id now runs another process. if (/incarnation mismatch/i.test(error instanceof Error ? error.message : String(error))) { - this.closing.delete(ptyId) + this.abandon(ptyId, bridge) } return false } diff --git a/src/main/ssh/legacy-relay/legacy-relay-target-bridge.test.ts b/src/main/ssh/legacy-relay/legacy-relay-target-bridge.test.ts index 26b2eceea6d..763f749274e 100644 --- a/src/main/ssh/legacy-relay/legacy-relay-target-bridge.test.ts +++ b/src/main/ssh/legacy-relay/legacy-relay-target-bridge.test.ts @@ -123,15 +123,33 @@ let endpointExited: boolean let stops: LegacyRelayPaneStops let events: { data: string[]; exits: string[]; exitedTelemetry: number; panesChanged: number } +/** Persists stops onto the leases, as the store does, so a fresh instance models a restart. */ +function paneStops(): LegacyRelayPaneStops { + return new LegacyRelayPaneStops(TARGET, () => ({ + leases: () => leases.filter((row) => row.state !== 'terminated'), + recordStop: (_target, relayPtyId, pendingKill) => { + leases = leases.map((row) => (row.ptyId === relayPtyId ? { ...row, pendingKill } : row)) + }, + retireLease: (_target, relayPtyId) => retire(relayPtyId) + })) +} + +/** As the store's retire records: terminated, and any stop it carried delivered. */ +function retire(relayPtyId: string): void { + leases = leases.map((row) => { + if (row.ptyId !== relayPtyId) { + return row + } + const { pendingKill: _delivered, ...rest } = row + return { ...rest, state: 'terminated' } + }) +} + function bridge(overrides: Partial = {}): LegacyRelayTargetBridge { const subject: LegacyRelayTargetBridge = new LegacyRelayTargetBridge({ targetId: TARGET, leases: () => leases.filter((row) => row.state !== 'terminated'), - retireLease: (relayPtyId) => { - leases = leases.map((row) => - row.ptyId === relayPtyId ? { ...row, state: 'terminated' } : row - ) - }, + retireLease: retire, listEndpoints: async () => listing, openClient: async () => { if (relay.openError) { @@ -147,7 +165,11 @@ function bridge(overrides: Partial = {}): LegacyRel stops.flush(subject) }, onData: (_id, chunk) => events.data.push(chunk), - onExit: (id) => events.exits.push(id), + onExit: (id) => { + events.exits.push(id) + // As the registry wires it. + stops.exited(id) + }, onTerminalExited: () => { events.exitedTelemetry += 1 }, @@ -168,7 +190,7 @@ beforeEach(() => { incarnations: new Map(), exitOnShutdown: true } - stops = new LegacyRelayPaneStops(TARGET, () => leases.filter((row) => row.state !== 'terminated')) + stops = paneStops() leases = [lease()] listing = { kind: 'listed', endpoints: [ENDPOINT], exhaustive: true } endpointExited = false @@ -331,6 +353,16 @@ describe('one target’s legacy relay bridge', () => { const subject = bridge() await subject.refresh() expect(subject.snapshot()).toEqual([]) + expect(stops.owes(PTY)).toBe(false) + // Listed only for its stop, which an unfenced legacy id never proves, so it is dropped. + expect(relay.shutdowns).toEqual([]) + expect(leases[0].pendingKill).toBeUndefined() + }) + + it('never lists a relay for a lease a newer pane won', async () => { + leases = [lease({ supersededBy: 'pty-3' }), lease({ ptyId: 'pty-4', tabId: undefined })] + relay.ptys = new Set(['pty-1', 'pty-4']) + await bridge().refresh() expect(relay.clients).toEqual([]) }) @@ -347,6 +379,7 @@ describe('one target’s legacy relay bridge', () => { it('retires a stopped shell once a listing proves it gone, without an exit report', async () => { relay.exitOnShutdown = false + relay.incarnations.set('pty-1', 'inc-1') const subject = bridge() await subject.refresh() await stops.stop(PTY, subject) @@ -400,6 +433,126 @@ describe('one target’s legacy relay bridge', () => { await vi.advanceTimersByTimeAsync(10_000) expect(relay.shutdowns).toEqual([]) expect(stops.size).toBe(0) + // The closed tab's lease ends, so the stranger is never served under it. + expect(leases[0].state).toBe('terminated') + expect(subject.snapshot()).toEqual([]) + }) + + it('never re-aims a repeated close at a process that took the closed tab’s relay id', async () => { + relay.incarnations.set('pty-1', 'inc-1') + const subject = bridge() + await subject.refresh() + relay.openError = new Error('ssh down') + relay.clients[0].drop() + await stops.stop(PTY, subject) + // A restarted relay reuses the id for another shell, and the close repeats before any flush. + const client = fakeClient(relay) + const recycled = { + paneState: () => 'live' as const, + liveRoute: () => ({ client, relayPtyId: 'pty-1' }), + listedIncarnation: () => 'inc-2', + refresh: async () => {} + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: stop() reads only these members. + await stops.stop(PTY, recycled as unknown as LegacyRelayTargetBridge) + expect(relay.shutdowns).toEqual([]) + expect(leases[0].state).toBe('terminated') + }) + + it('proves a stop only by the relay’s exit report, never by the stop’s reply', async () => { + const epochPty = toAppSshPtyId(TARGET, 'pty2:epoch:1') + leases = [lease({ ptyId: 'pty2:epoch:1' })] + relay.ptys = new Set(['pty2:epoch:1']) + const subject = bridge() + await subject.refresh() + await expect(stops.stopAndWait(epochPty, subject, Date.now() + 5_000)).resolves.toBe(true) + }) + + it('settles on the relay’s exit report even when the stop’s own reply is lost', async () => { + const epochPty = toAppSshPtyId(TARGET, 'pty2:epoch:1') + leases = [lease({ ptyId: 'pty2:epoch:1' })] + const client = fakeClient(relay) + client.shutdown = async () => { + stops.exited(epochPty) + throw new Error('request timed out') + } + const lostReply = { + paneState: () => 'live' as const, + liveRoute: () => ({ client, relayPtyId: 'pty2:epoch:1' }), + listedIncarnation: () => undefined, + refresh: async () => {} + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: stop() reads only these members. + const target = lostReply as unknown as LegacyRelayTargetBridge + await expect(stops.stopAndWait(epochPty, target, Date.now() + 5_000)).resolves.toBe(true) + }) + + it('still settles on an exit report that trails a failed stop reply', async () => { + const epochPty = toAppSshPtyId(TARGET, 'pty2:epoch:1') + leases = [lease({ ptyId: 'pty2:epoch:1' })] + relay.ptys = new Set(['pty2:epoch:1']) + relay.shutdownError = new Error('request timed out') + const subject = bridge() + await subject.refresh() + const settled = stops.stopAndWait(epochPty, subject, Date.now() + 5_000) + await vi.advanceTimersByTimeAsync(100) + relay.clients[0].emitExit('pty2:epoch:1', 0) + await expect(settled).resolves.toBe(true) + }) + + it('still sees an unfenced stop’s exit report that trails its reply, never re-serving the pane', async () => { + relay.exitOnShutdown = false + const subject = bridge() + await subject.refresh() + const settled = stops.stopAndWait(PTY, subject, Date.now() + 5_000) + await vi.advanceTimersByTimeAsync(0) + expect(relay.shutdowns).toEqual(['pty-1']) + expect(subject.snapshot()).toEqual([]) + relay.clients[0].emitExit('pty-1', 0) + await expect(settled).resolves.toBe(true) + }) + + it('still sees an unfenced stop’s exit report after its reply timed out', async () => { + relay.shutdownError = new Error('request timed out') + const subject = bridge() + await subject.refresh() + const settled = stops.stopAndWait(PTY, subject, Date.now() + 5_000) + await vi.advanceTimersByTimeAsync(100) + expect(subject.snapshot()).toEqual([]) + relay.clients[0].emitExit('pty-1', 0) + await expect(settled).resolves.toBe(true) + }) + + it('leaves a taken stop unproven while no exit report or complete listing confirms it', async () => { + const epochPty = toAppSshPtyId(TARGET, 'pty2:epoch:1') + leases = [lease({ ptyId: 'pty2:epoch:1' })] + relay.ptys = new Set(['pty2:epoch:1']) + relay.exitOnShutdown = false + // A relay may run that no endpoint accounts for, so absence from the listing is not exit. + listing = { kind: 'listed', endpoints: [ENDPOINT], exhaustive: false } + const subject = bridge() + await subject.refresh() + const settled = stops.stopAndWait(epochPty, subject, Date.now() + 1_000) + await vi.advanceTimersByTimeAsync(1_000) + await expect(settled).resolves.toBe(false) + expect(relay.shutdowns).toEqual(['pty2:epoch:1']) + }) + + it('never attaches a pane whose tab closed while the attach waited for its relay', async () => { + const epochPty = toAppSshPtyId(TARGET, 'pty2:epoch:1') + leases = [lease({ ptyId: 'pty2:epoch:1' })] + relay.ptys = new Set(['pty2:epoch:1']) + relay.openError = new Error('ssh down') + const subject = bridge() + await subject.refresh() + const attach = subject.attach(epochPty) + // The tab closes while the attach waits for the unreachable relay. + await stops.stop(epochPty, null) + relay.openError = null + relay.shutdownError = new Error('timed out') + await subject.refresh() + await expect(attach).resolves.toEqual({ status: 'not-bridged' }) + expect(relay.clients.every((client) => client.writes.length === 0)).toBe(true) }) it('sends a close recorded while disconnected once the next bridge lists the pane', async () => { @@ -412,11 +565,91 @@ describe('one target’s legacy relay bridge', () => { expect(relay.shutdowns).toEqual(['pty2:epoch:1']) }) + it('sends a close recorded while disconnected after a restart before the reconnect', async () => { + const epochPty = toAppSshPtyId(TARGET, 'pty2:epoch:1') + leases = [lease({ ptyId: 'pty2:epoch:1' })] + relay.ptys = new Set(['pty2:epoch:1']) + await stops.stop(epochPty, null) + // Restart: only the persisted leases survive. + stops = paneStops() + const subject = bridge() + await subject.refresh() + expect(relay.shutdowns).toEqual(['pty2:epoch:1']) + // A closed tab's shell is stopped, never offered back as a pane to reattach. + expect(subject.snapshot()).toEqual([]) + expect(leases[0]).toMatchObject({ state: 'terminated' }) + expect(leases[0].pendingKill).toBeUndefined() + }) + + it('keeps the first close’s record when the tab’s close repeats', async () => { + const epochPty = toAppSshPtyId(TARGET, 'pty2:epoch:1') + leases = [lease({ ptyId: 'pty2:epoch:1' })] + await stops.stop(epochPty, null) + const first = leases[0].pendingKill + await vi.advanceTimersByTimeAsync(5_000) + await stops.stop(epochPty, null) + expect(leases[0].pendingKill).toBe(first) + }) + + it('restores a fenced stop across a restart and sends it only to the same process', async () => { + relay.incarnations.set('pty-1', 'inc-1') + const subject = bridge() + await subject.refresh() + relay.openError = new Error('ssh down') + relay.clients[0].drop() + await stops.stop(PTY, subject) + subject.dispose() + stops = paneStops() + relay.openError = null + await bridge().refresh() + expect(relay.shutdowns).toEqual(['pty-1#inc-1']) + }) + + it('drops a restored stop whose relay id now names another process, never serving it', async () => { + relay.incarnations.set('pty-1', 'inc-1') + const subject = bridge() + await subject.refresh() + relay.openError = new Error('ssh down') + relay.clients[0].drop() + await stops.stop(PTY, subject) + subject.dispose() + stops = paneStops() + relay.incarnations.set('pty-1', 'inc-2') + relay.openError = null + const next = bridge() + await next.refresh() + await vi.advanceTimersByTimeAsync(10_000) + expect(relay.shutdowns).toEqual([]) + expect(leases[0]).toMatchObject({ state: 'terminated' }) + expect(leases[0].pendingKill).toBeUndefined() + expect(next.snapshot()).toEqual([]) + }) + + it('stops serving an unfenced legacy pane once its one stop failed', async () => { + const subject = bridge() + await subject.refresh() + relay.shutdownError = new Error('timed out') + await stops.stop(PTY, subject) + await vi.advanceTimersByTimeAsync(0) + expect(subject.snapshot()).toEqual([]) + expect(subject.livePaneCount()).toBe(0) + }) + + it('never serves an unfenced legacy pane again after its tab closed and Orca restarted', async () => { + await expect(stops.stop(PTY, null)).resolves.toEqual({ status: 'unverifiable' }) + stops = paneStops() + const subject = bridge() + await subject.refresh() + expect(subject.snapshot()).toEqual([]) + expect(relay.shutdowns).toEqual([]) + }) + it('never replays a blind close for a legacy id that could name another shell', async () => { await stops.stop(PTY, null) await bridge().refresh() expect(relay.shutdowns).toEqual([]) - expect(leases[0].state).toBe('detached') + // The tab closed, so its lease ends as any closed tab's does. + expect(leases[0].state).toBe('terminated') }) it('never replays a fenced legacy stop to a relay that no longer reports the incarnation', async () => { @@ -441,8 +674,10 @@ describe('one target’s legacy relay bridge', () => { relay.shutdownError = null relay.clients[0].drop() await vi.advanceTimersByTimeAsync(10_000) - expect(relay.clients.length).toBeGreaterThan(1) expect(relay.shutdowns).toEqual([]) + // One try only: the lease ends as any closed tab's does, so no later relay is ever asked. + expect(leases[0].state).toBe('terminated') + expect(subject.snapshot()).toEqual([]) }) it('never republishes a pane that exited while a refresh was asking its relay', async () => { diff --git a/src/main/ssh/legacy-relay/legacy-relay-target-bridge.ts b/src/main/ssh/legacy-relay/legacy-relay-target-bridge.ts index 0835e9b82d6..aace59391c8 100644 --- a/src/main/ssh/legacy-relay/legacy-relay-target-bridge.ts +++ b/src/main/ssh/legacy-relay/legacy-relay-target-bridge.ts @@ -18,12 +18,13 @@ import type { LegacyRelayEndpoint, LegacyRelayEndpointListing } from './legacy-r import { LegacyRelayRetry, LegacyRelayWaiters } from './legacy-relay-bridge-timers' import { ATTACH_WAIT_MS, - bridgeableLeasesByRelayId, findLivePane, leasedAppPtyIds, + listedLeasesByRelayId, type LegacyRelayPty, PROVEN_EXITED, - toBridgedPane, + servedBridgedPanes, + toPane, type Pane } from './legacy-relay-bridge-leases' @@ -57,9 +58,12 @@ type EndpointState = { export class LegacyRelayTargetBridge { private readonly endpoints = new Map() private readonly panes = new Map() - readonly livePaneForKey = (paneKey: string) => findLivePane(this.panes, paneKey) + readonly livePaneForKey = (paneKey: string) => + findLivePane(this.panes, paneKey, this.deps.leases()) /** The process identity the relay listed for a pane, which fences a stop against a recycled id. */ readonly listedIncarnation = (ptyId: string) => this.panes.get(ptyId)?.listed?.incarnationId + /** Whether a relay lists a pane now, including one whose stop is owed; undefined when untracked. */ + readonly paneState = (ptyId: string) => this.panes.get(ptyId)?.state private readonly attached = new Map>() private readonly waiters = new LegacyRelayWaiters() private readonly retry = new LegacyRelayRetry(() => void this.refresh()) @@ -73,16 +77,12 @@ export class LegacyRelayTargetBridge { constructor(private readonly deps: LegacyRelayTargetBridgeDeps) {} - get targetId(): string { - return this.deps.targetId - } - livePaneCount(): number { - return [...this.panes.values()].filter((pane) => pane.state === 'live').length + return this.snapshot().filter((pane) => pane.state === 'live').length } snapshot(): LegacyRelayBridgedPane[] { - return [...this.panes].map(([ptyId, pane]) => toBridgedPane(this.deps.targetId, ptyId, pane)) + return servedBridgedPanes(this.deps.targetId, this.panes, this.deps.leases()) } /** Re-reads every relay; concurrent callers share one pass. */ @@ -106,10 +106,14 @@ export class LegacyRelayTargetBridge { ) pane = this.panes.get(ptyId) } + const leased = leasedAppPtyIds(this.deps.targetId, this.deps.leases()).has(ptyId) if (!pane) { - const leased = leasedAppPtyIds(this.deps.targetId, this.deps.leases()).has(ptyId) return leased ? { status: 'unverifiable' } : { status: 'exited' } } + // A close recorded while this attach waited: its stop is owed, so the pane is never served. + if (!leased) { + return { status: 'not-bridged' } + } const client = pane.endpoint ? this.endpoints.get(pane.endpoint)?.client : null if (pane.state !== 'live' || !client) { return { status: 'unverifiable' } @@ -173,7 +177,7 @@ export class LegacyRelayTargetBridge { return } this.looked = true - const leases = bridgeableLeasesByRelayId(this.deps.leases()) + const leases = listedLeasesByRelayId(this.deps.leases()) if (leases.size === 0) { this.closeIdleClients(new Set()) this.replacePanes(new Map()) @@ -185,7 +189,7 @@ export class LegacyRelayTargetBridge { for (const [relayPtyId, lease] of leases) { const ptyId = toAppSshPtyId(this.deps.targetId, relayPtyId) if (!seeded.has(ptyId)) { - seeded.set(ptyId, { lease, relayPtyId, endpoint: null, state: 'unverifiable' }) + seeded.set(ptyId, toPane(lease, relayPtyId, null, 'unverifiable')) } } if (seeded.size !== this.panes.size) { @@ -212,14 +216,12 @@ export class LegacyRelayTargetBridge { } const next = new Map() // Why re-read: a lease retired while the relays were asked (an exit) must not come back live. - const current = [...bridgeableLeasesByRelayId(this.deps.leases())].filter(([id]) => - leases.has(id) - ) + const current = [...listedLeasesByRelayId(this.deps.leases())].filter(([id]) => leases.has(id)) for (const [relayPtyId, lease] of current) { const ptyId = toAppSshPtyId(this.deps.targetId, relayPtyId) const listed = listedBy.get(relayPtyId) if (listed) { - next.set(ptyId, { lease, relayPtyId, endpoint: listed.endpoint, state: 'live', listed }) + next.set(ptyId, toPane(lease, relayPtyId, listed.endpoint, 'live', listed)) } else if (complete) { // Every relay this target could have left answered, and none holds it. const wasLive = this.panes.get(ptyId)?.endpoint @@ -227,7 +229,7 @@ export class LegacyRelayTargetBridge { this.emitExit(ptyId, -1, Boolean(wasLive)) } else { // Unreachable: loss of contact is never exit, so the pane waits. - next.set(ptyId, { lease, relayPtyId, endpoint: null, state: 'unverifiable' }) + next.set(ptyId, toPane(lease, relayPtyId, null, 'unverifiable')) } } this.closeIdleClients(new Set([...next.values()].map((pane) => pane.endpoint ?? ''))) diff --git a/src/shared/ssh-pending-pty-kill.ts b/src/shared/ssh-pending-pty-kill.ts index 6619dc0fc7d..b7fa477fb36 100644 --- a/src/shared/ssh-pending-pty-kill.ts +++ b/src/shared/ssh-pending-pty-kill.ts @@ -1,6 +1,6 @@ /** A `pty.shutdown` a relay-era client issued to an SSH host and could not confirm, carried on its - * `SshRemotePtyLease`. This build never records or replays one; it keeps the field through - * load and save so a downgraded build can still replay it. */ + * `SshRemotePtyLease`. This build records and replays one only through the legacy relay bridge, + * for a closed tab whose older relay still runs its shell; a downgraded build replays it too. */ export type SshPendingPtyKill = { requestedAt: number /** The host-minted PTY incarnation this kill was aimed at, and the fence. diff --git a/tests/e2e/ssh-orcad-legacy-relay-bridge.spec.ts b/tests/e2e/ssh-orcad-legacy-relay-bridge.spec.ts index abc886fa133..e0f023c2c3d 100644 --- a/tests/e2e/ssh-orcad-legacy-relay-bridge.spec.ts +++ b/tests/e2e/ssh-orcad-legacy-relay-bridge.spec.ts @@ -512,3 +512,113 @@ test('a relay terminal in a project another desktop already moved keeps working await desktopB.dispose() } }) + +test('a bridged tab closed while disconnected stops its shell even after a restart', async ({ + testRepoPath +}, testInfo: TestInfo) => { + test.setTimeout(25 * 60_000) + host = startDockerSshHostTarget(testInfo) + const targetId = `ssh-bridge-close-${Date.now()}` + const relay = await installReleasedRelayOnDockerHost(host, TEMPLATE_SOURCE!) + const ours = await startDockerRelayWithShell(host, relay, targetId, 'before-upgrade') + const theirs = await startDockerRelayWithShell(host, relay, 'another-desktop', 'theirs') + const bridgedPtyId = toAppSshPtyId(targetId, ours.relayPtyId) + const session = createRestartSession(testInfo, { ORCA_ORCAD_TEMPLATE_PATH: TEMPLATE_SOURCE! }) + const ourLease = (): { state?: unknown; pendingKill?: unknown } | undefined => { + const leases = readPersistedProfileState(session.userDataDir).sshRemotePtyLeases + return (Array.isArray(leases) ? leases : []).find( + (lease: { ptyId?: unknown }) => lease.ptyId === ours.relayPtyId + ) + } + let app: ElectronApplication | null = null + try { + const first = await session.launch() + app = first.app + await waitForSessionReady(first.page) + await first.page.evaluate((repoPath) => window.api.repos.add({ path: repoPath }), testRepoPath) + await session.close(app) + app = null + const seeded = seedRelayEraProfile( + session.userDataDir, + { + label: `legacy relay bridge close E2E ${Date.now()}`, + host: host.host, + port: host.port, + username: 'root', + identityFile: host.identityFile, + identitiesOnly: true, + relayGracePeriodSeconds: 1 + }, + { repoPath: DOCKER_SSH_HOST_REMOTE_REPO_PATH, folderPath: '/tmp' }, + { targetId, relayPtyId: ours.relayPtyId } + ) + + const upgraded = await launchUpgraded(session) + app = upgraded.app + let page = upgraded.page + await waitForSessionReady(page) + await page.evaluate( + ({ worktreeId, hostId, tabId }) => { + const state = window.__store!.getState() + state.setActiveWorktree(worktreeId, hostId) + state.setActiveTab(tabId) + state.setActiveTabType('terminal', worktreeId) + }, + { + worktreeId: seeded.worktreeId, + hostId: toSshExecutionHostId(targetId), + tabId: seeded.terminalTabId! + } + ) + await expect + .poll(() => managedServer(page, targetId), { timeout: 8 * 60_000 }) + .toMatchObject({ kind: 'managed' }) + await expect + .poll(() => bridgedPanes(page), { timeout: BRIDGE_TIMEOUT_MS }) + .toEqual([expect.objectContaining({ ptyId: bridgedPtyId, state: 'live' })]) + + // Disconnected, the tab closes through both of its routes: the renderer's PTY kill, which + // finds no SSH provider, and then the bridge transport's own close. + await page.evaluate((id) => window.api.ssh.disconnect({ targetId: id }), targetId) + await page.evaluate(async (ptyId) => { + await window.api.pty.kill(ptyId) + await window.api.legacyRelayBridge?.close({ ptyId }) + }, bridgedPtyId) + // Loss of contact is never exit: the shell still runs, and its stop is owed. + expect(await listDockerRelayPtys(host, relay, ours.sockPath)).toEqual([ours.relayPtyId]) + + // Restart before reconnecting: only the persisted stop survives. + await session.close(app) + app = null + expect(ourLease()?.state).not.toBe('terminated') + expect(ourLease()?.pendingKill).toBeTruthy() + + const restarted = await launchUpgraded(session) + app = restarted.app + page = restarted.page + await waitForSessionReady(page) + await page.evaluate((id) => window.api.ssh.connect({ targetId: id }), targetId) + // A failed listing is not absence: only the relay's own empty answer proves the shell stopped. + await expect + .poll(() => listDockerRelayPtys(host!, relay, ours.sockPath).catch(() => ['']), { + timeout: BRIDGE_TIMEOUT_MS, + intervals: [2_000] + }) + .toEqual([]) + // Never offered back as a pane to reattach. + expect(await bridgedPanes(page)).toEqual([]) + await session.close(app) + app = null + expect(ourLease()?.pendingKill).toBeUndefined() + + // Another desktop's relay keeps running its shell, untouched. + expect(await echoThroughDockerRelay(host, relay, theirs, 'still-here')).toContain( + 'GOT:still-here' + ) + } finally { + if (app) { + await session.close(app) + } + await session.dispose() + } +})