From a7010ec51f8f3eff35442d1b68c04127ffe7d172 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Sun, 4 Oct 2026 03:33:41 -0700 Subject: [PATCH] fix(orcad): the terminal gate asks the relays before treating a detached terminal as running (#25200) * fix(orcad): the terminal gate asks the relays before treating a detached terminal as running, and retiring a host drops its relay recovery record * fix(ssh): earlier-relay census gaps and an unanswered relay stay unverifiable; asking the terminal gate changes nothing - the gate is read-only; the conversion and delta move retire proven detached leases themselves - an expired lease also needs every earlier-build relay to answer before it reads as exited - a failed, input-less or truncated census marks its older relays unverifiable, so reattach holds - a legacy relay route closes only when no attach or listing still awaits it - a disposed session forgets the census it started --------- Co-authored-by: m4air --- .../ssh/orcad-migration-delta-move.test.ts | 3 +- src/main/ssh/orcad-migration-delta-move.ts | 2 + .../orcad-migration-relay-pty-lister.test.ts | 10 +++ .../ssh/orcad-migration-relay-pty-lister.ts | 10 ++- .../orcad-migration-source-retirement.test.ts | 18 ++++ .../ssh/orcad-migration-source-retirement.ts | 3 + .../ssh/orcad-migration-terminal-gate.test.ts | 67 +++++++++++++-- src/main/ssh/orcad-migration-terminal-gate.ts | 78 +++++++++++++---- src/main/ssh/orcad-runtime-conversion.ts | 2 + src/main/ssh/ssh-legacy-relay-route.ts | 5 ++ src/main/ssh/ssh-legacy-relay-router.test.ts | 31 +++++++ src/main/ssh/ssh-legacy-relay-router.ts | 86 +++++++++++++++---- src/main/ssh/ssh-legacy-relay-routing.ts | 26 +++++- .../ssh/ssh-previous-relay-terminals.test.ts | 59 ++++++++++++- src/main/ssh/ssh-previous-relay-terminals.ts | 79 ++++++++++++----- src/main/ssh/ssh-relay-session.ts | 5 +- 16 files changed, 417 insertions(+), 67 deletions(-) diff --git a/src/main/ssh/orcad-migration-delta-move.test.ts b/src/main/ssh/orcad-migration-delta-move.test.ts index 1c1fb710967..0cf101b1861 100644 --- a/src/main/ssh/orcad-migration-delta-move.test.ts +++ b/src/main/ssh/orcad-migration-delta-move.test.ts @@ -130,7 +130,8 @@ function deltaMove() { target, environment: listEnvironments(userDataPath)[0]!, destination, - listRelayPtyIds: async () => [], + // This relay and every earlier-build relay answer that nothing runs. + listRelayPtyIds: Object.assign(async () => [], { previous: async () => [] }), releaseDirectSession: async () => {}, ensureTunnel: async () => {}, now diff --git a/src/main/ssh/orcad-migration-delta-move.ts b/src/main/ssh/orcad-migration-delta-move.ts index f5376d1b257..c2e9b3f457c 100644 --- a/src/main/ssh/orcad-migration-delta-move.ts +++ b/src/main/ssh/orcad-migration-delta-move.ts @@ -30,6 +30,7 @@ import { import { retainOrcadMigrationSource } from './orcad-migration-source-retention' import { assessOrcadMigrationTerminals, + retireProvenDetachedLeases, type ListRelayPtyIds } from './orcad-migration-terminal-gate' import { currentOrcadSourceFingerprint } from './orcad-retained-source' @@ -67,6 +68,7 @@ export async function runOrcadDeltaMove(args: OrcadDeltaMoveArgs): Promise { it('has no lister without a connected relay session', () => { expect(orcadMigrationRelayPtyLister(TARGET)).toBeNull() }) + + it("asks earlier-build relays in the leases' spelling, keeping an unknown answer null", async () => { + const provider = { listProcesses: async () => [] } + const held = orcadMigrationRelayPtyLister(TARGET, provider, Date.now, async () => [ + toAppSshPtyId(TARGET, 'pty-old') + ]) + expect(await held?.previous?.()).toEqual(['pty-old']) + const unknown = orcadMigrationRelayPtyLister(TARGET, provider, Date.now, async () => null) + expect(await unknown?.previous?.()).toBeNull() + }) }) diff --git a/src/main/ssh/orcad-migration-relay-pty-lister.ts b/src/main/ssh/orcad-migration-relay-pty-lister.ts index f4bbd147846..a8f5e9c19eb 100644 --- a/src/main/ssh/orcad-migration-relay-pty-lister.ts +++ b/src/main/ssh/orcad-migration-relay-pty-lister.ts @@ -10,6 +10,7 @@ import { getSshPtyProvider } from '../ipc/pty/provider/registry' import type { IPtyProvider } from '../providers/types' import { toRelaySshPtyId } from '../providers/ssh-pty-id' import type { ListRelayPtyIds } from './orcad-migration-terminal-gate' +import { listPreviousRelayPtyIds } from './ssh-legacy-relay-routing' /** Long enough for a Windows relay's first process-table read, short enough to block a click. */ export const ORCAD_MIGRATION_RELAY_LIST_BUDGET_MS = 10_000 @@ -18,15 +19,20 @@ export const ORCAD_MIGRATION_RELAY_LIST_BUDGET_MS = 10_000 export function orcadMigrationRelayPtyLister( targetId: string, provider: Pick | undefined = getSshPtyProvider(targetId), - now: () => number = Date.now + now: () => number = Date.now, + listPrevious: (targetId: string) => Promise = listPreviousRelayPtyIds ): ListRelayPtyIds | null { if (!provider) { return null } - return async () => { + const list: ListRelayPtyIds = async () => { const processes = await provider.listProcesses({ deadlineMs: now() + ORCAD_MIGRATION_RELAY_LIST_BUDGET_MS }) return processes.map((process) => toRelaySshPtyId(targetId, process.id)) } + // Earlier-build relays answer through their own bridge, POSIX only; null keeps the gate blocked. + list.previous = async () => + (await listPrevious(targetId))?.map((id) => toRelaySshPtyId(targetId, id)) ?? null + return list } diff --git a/src/main/ssh/orcad-migration-source-retirement.test.ts b/src/main/ssh/orcad-migration-source-retirement.test.ts index 5ae154906bd..65c31a660b5 100644 --- a/src/main/ssh/orcad-migration-source-retirement.test.ts +++ b/src/main/ssh/orcad-migration-source-retirement.test.ts @@ -114,6 +114,24 @@ describe('retiring a migrated source', () => { expect(h.store.getWorkspaceSession().activeConnectionIdsAtShutdown).toEqual(['ssh-other']) }) + it("drops the moved host's relay recovery record and keeps other hosts'", async () => { + const h = await setup() + const record = (targetId: string) => ({ + targetId, + clientInstanceId: 'client-1', + serverBuildId: '0.1.0', + clientGeneration: 1, + ownerGeneration: 1, + ownerLease: 'lease' + }) + await h.store.upsertSshPtyConsumerRecovery(record(TARGET.id)) + await h.store.upsertSshPtyConsumerRecovery(record('ssh-other')) + + await expect(h.retire()).resolves.toMatchObject({ phase: 'source-retired' }) + expect(h.store.getSshPtyConsumerRecovery(TARGET.id)).toBeNull() + expect(h.store.getSshPtyConsumerRecovery('ssh-other')).toMatchObject({ targetId: 'ssh-other' }) + }) + it('refuses to retire anything before the destination proved its commit', async () => { const h = await setup({ commit: false }) await expect(h.retire()).rejects.toThrow('orcad_migration_retire_before_commit') diff --git a/src/main/ssh/orcad-migration-source-retirement.ts b/src/main/ssh/orcad-migration-source-retirement.ts index 827651e6900..cb190c9f75a 100644 --- a/src/main/ssh/orcad-migration-source-retirement.ts +++ b/src/main/ssh/orcad-migration-source-retirement.ts @@ -23,6 +23,7 @@ export type OrcadMigrationRetirementStore = Pick< | 'flushPendingOrThrowAsync' | 'getSshRemotePtyLeases' | 'getSshTarget' + | 'removeSshPtyConsumerRecovery' | 'removeSshRemotePtyLease' | 'retireOrcadMigrationSourceCatalog' > @@ -64,6 +65,8 @@ export async function retireOrcadMigrationSource( } context.store.retireOrcadMigrationSourceCatalog(cutover.manifest) retireProvenLeases(context.store, cutover) + // The relay consumer's recovery record would only re-dial a relay this host no longer runs. + await context.store.removeSshPtyConsumerRecovery(cutover.sshTargetId) await context.store.flushPendingOrThrowAsync({ signal: context.signal, drainToStableGeneration: false diff --git a/src/main/ssh/orcad-migration-terminal-gate.test.ts b/src/main/ssh/orcad-migration-terminal-gate.test.ts index 6be8455dcf9..20158ba2321 100644 --- a/src/main/ssh/orcad-migration-terminal-gate.test.ts +++ b/src/main/ssh/orcad-migration-terminal-gate.test.ts @@ -1,8 +1,10 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import type { SshRemotePtyLease } from '../../shared/ssh-types' import { assessOrcadMigrationTerminals, - confirmOrcadMigrationTerminalsUnderFence + confirmOrcadMigrationTerminalsUnderFence, + retireProvenDetachedLeases, + type ListRelayPtyIds } from './orcad-migration-terminal-gate' function store(leases: Pick[]) { @@ -10,6 +12,12 @@ function store(leases: Pick[]) { return { getSshRemotePtyLeases: () => full } } +function relay(current: string[] | null, previous: string[] | null): ListRelayPtyIds { + const list: ListRelayPtyIds = async () => current + list.previous = async () => previous + return list +} + describe('migration terminal gate', () => { it('proves exit from terminated leases and an empty relay', async () => { await expect( @@ -21,12 +29,59 @@ describe('migration terminal gate', () => { ).resolves.toEqual({ verdict: 'exited', provenPtyIds: ['a'] }) }) - it.each(['attached', 'detached'] as const)('blocks a %s lease as live', async (state) => { + it('blocks an attached lease as live', async () => { await expect( - assessOrcadMigrationTerminals(store([{ ptyId: 'a', state }]), 'ssh-1', async () => []) + assessOrcadMigrationTerminals( + store([{ ptyId: 'a', state: 'attached' }]), + 'ssh-1', + async () => [] + ) ).resolves.toMatchObject({ verdict: 'live', ptyIds: ['a'] }) }) + it('proves a detached terminal exited once this relay and earlier relays both answer without it', async () => { + const leases = store([{ ptyId: 'a', state: 'detached' }]) + const proof = await assessOrcadMigrationTerminals(leases, 'ssh-1', relay([], [])) + expect(proof).toEqual({ verdict: 'exited', provenPtyIds: ['a'] }) + + // Only the move acting on the proof retires the lease; asking alone changes nothing. + const markSshRemotePtyLease = vi.fn() + retireProvenDetachedLeases({ ...leases, markSshRemotePtyLease }, 'ssh-1', proof) + expect(markSshRemotePtyLease).toHaveBeenCalledWith('ssh-1', 'a', 'terminated') + }) + + it('blocks a detached terminal an earlier relay still runs as live', async () => { + await expect( + assessOrcadMigrationTerminals( + store([{ ptyId: 'a', state: 'detached' }]), + 'ssh-1', + relay([], ['a']) + ) + ).resolves.toMatchObject({ verdict: 'live', ptyIds: ['a'] }) + }) + + it.each([ + ['this relay did not answer', relay(null, [])], + ['earlier relays could not be asked (Windows, no census, unreachable)', relay([], null)], + ['no earlier-relay lister', async () => []] + ])('keeps a detached or expired lease unverifiable when %s', async (_label, list) => { + for (const state of ['detached', 'expired'] as const) { + await expect( + assessOrcadMigrationTerminals(store([{ ptyId: 'a', state }]), 'ssh-1', list) + ).resolves.toMatchObject({ verdict: 'unverifiable', ptyIds: ['a'] }) + } + }) + + it('blocks an expired terminal an earlier relay still runs as live', async () => { + await expect( + assessOrcadMigrationTerminals( + store([{ ptyId: 'old', state: 'expired' }]), + 'ssh-1', + relay([], ['old']) + ) + ).resolves.toMatchObject({ verdict: 'live', ptyIds: ['old'] }) + }) + it('blocks terminals the relay still runs even with no lease for them', async () => { await expect( assessOrcadMigrationTerminals(store([]), 'ssh-1', async () => ['x']) @@ -48,12 +103,12 @@ describe('migration terminal gate', () => { ).resolves.toMatchObject({ verdict: 'unverifiable', ptyIds: ['old'] }) }) - it('accepts an expired lease once the relay answers that nothing runs', async () => { + it('accepts an expired lease once every relay answers that nothing runs', async () => { await expect( assessOrcadMigrationTerminals( store([{ ptyId: 'old', state: 'expired' }]), 'ssh-1', - async () => [] + relay([], []) ) ).resolves.toEqual({ verdict: 'exited', provenPtyIds: ['old'] }) }) diff --git a/src/main/ssh/orcad-migration-terminal-gate.ts b/src/main/ssh/orcad-migration-terminal-gate.ts index d31c90ffe6b..b3a72f7121c 100644 --- a/src/main/ssh/orcad-migration-terminal-gate.ts +++ b/src/main/ssh/orcad-migration-terminal-gate.ts @@ -10,28 +10,28 @@ export type OrcadMigrationTerminalVerdict = | { verdict: 'exited'; provenPtyIds: string[] } | { verdict: 'live' | 'unverifiable'; ptyIds: string[]; reason: string } -/** The relay's own process list for the target; `null` when it did not answer. */ -export type ListRelayPtyIds = () => Promise +/** + * The relay's own process list for the target; `null` when it did not answer. `previous` asks the + * relays an earlier Orca build left running the same way, `null` when they cannot be asked. + */ +export type ListRelayPtyIds = (() => Promise) & { + previous?: () => Promise +} type LeaseStore = Pick -/** Taken before the fence, while the relay can still be asked. */ +/** Taken before the fence, while the relay can still be asked. Read-only, so previews may ask. */ export async function assessOrcadMigrationTerminals( store: LeaseStore, targetId: string, listRelayPtyIds: ListRelayPtyIds | null ): Promise { const leases = store.getSshRemotePtyLeases(targetId) - const live = leases.filter((lease) => lease.state === 'attached' || lease.state === 'detached') - if (live.length > 0) { - return refuse('live', live, 'terminals on this host are still running') - } - let relayPtyIds: string[] | null - try { - relayPtyIds = listRelayPtyIds ? await listRelayPtyIds() : null - } catch { - relayPtyIds = null + const attached = leases.filter((lease) => lease.state === 'attached') + if (attached.length > 0) { + return refuse('live', attached, 'terminals on this host are still running') } + const relayPtyIds = await ask(listRelayPtyIds) if (relayPtyIds && relayPtyIds.length > 0) { return { verdict: 'live', @@ -39,14 +39,60 @@ export async function assessOrcadMigrationTerminals( reason: 'the SSH relay still runs terminals on this host' } } - // An expired lease lost its owner without an exit record; only the relay can rule it out. - const expired = leases.filter((lease) => lease.state === 'expired') - if (expired.length > 0 && relayPtyIds === null) { - return refuse('unverifiable', expired, 'the SSH relay could not confirm expired terminals') + // A detached or expired lease may run on this relay or one an earlier build left; both answer. + const unresolved = leases.filter( + (lease) => lease.state === 'detached' || lease.state === 'expired' + ) + if (unresolved.length === 0) { + return { verdict: 'exited', provenPtyIds: leases.map((lease) => lease.ptyId) } + } + if (relayPtyIds === null) { + return refuse( + 'unverifiable', + unresolved, + 'the SSH relay could not confirm these terminals exited' + ) + } + const previousPtyIds = await ask(listRelayPtyIds?.previous) + if (previousPtyIds === null) { + return refuse('unverifiable', unresolved, 'Orca could not confirm its terminals here exited') + } + const held = new Set(previousPtyIds) + const running = unresolved.filter((lease) => held.has(lease.ptyId)) + if (running.length > 0) { + return refuse('live', running, 'an earlier Orca relay still runs terminals on this host') } return { verdict: 'exited', provenPtyIds: leases.map((lease) => lease.ptyId) } } +/** + * A move acting on an exited proof marks the detached leases it covers terminated, so the checks + * after it (the fenced re-check, preflight's lease blocker) stop reading them as running. + */ +export function retireProvenDetachedLeases( + store: Pick, + targetId: string, + proof: OrcadMigrationTerminalVerdict +): void { + if (proof.verdict !== 'exited') { + return + } + const proven = new Set(proof.provenPtyIds) + for (const lease of store.getSshRemotePtyLeases(targetId)) { + if (lease.state === 'detached' && proven.has(lease.ptyId)) { + store.markSshRemotePtyLease(targetId, lease.ptyId, 'terminated') + } + } +} + +async function ask(list: (() => Promise) | null | undefined) { + try { + return list ? await list() : null + } catch { + return null + } +} + /** * Re-checked under the fence, after the relay was let go: the fence stops new leases, so any * lease the earlier proof did not cover means a terminal started in between. diff --git a/src/main/ssh/orcad-runtime-conversion.ts b/src/main/ssh/orcad-runtime-conversion.ts index ebae584928c..db8ec92c0d9 100644 --- a/src/main/ssh/orcad-runtime-conversion.ts +++ b/src/main/ssh/orcad-runtime-conversion.ts @@ -30,6 +30,7 @@ import { } from './orcad-migration-source-fence' import { assessOrcadMigrationTerminals, + retireProvenDetachedLeases, type ListRelayPtyIds } from './orcad-migration-terminal-gate' import { createManagedOrcadEnvironment } from './orcad-runtime-deployment' @@ -158,6 +159,7 @@ async function fenceOrResume( if (terminalProof.verdict !== 'exited') { return refuse(terminalProof.verdict, 'orcad_migration_terminals', terminalProof.reason) } + retireProvenDetachedLeases(store, target.id, terminalProof) await args.releaseDirectSession(target.id) const result = await runTargetLifecycle(target.id, () => fenceOrcadMigrationSource({ diff --git a/src/main/ssh/ssh-legacy-relay-route.ts b/src/main/ssh/ssh-legacy-relay-route.ts index 86929a48739..27c453b1ded 100644 --- a/src/main/ssh/ssh-legacy-relay-route.ts +++ b/src/main/ssh/ssh-legacy-relay-route.ts @@ -121,6 +121,11 @@ export class SshLegacyRelayRoute { } } + /** The app PTY ids the old relay listed when the route opened, minus those that exited since. */ + heldPtyIds(): string[] { + return this.closed ? [] : [...this.listed] + } + holds(appPtyId: string): boolean { return !this.closed && this.listed.has(appPtyId) } diff --git a/src/main/ssh/ssh-legacy-relay-router.test.ts b/src/main/ssh/ssh-legacy-relay-router.test.ts index 0585865052f..afbeadf5867 100644 --- a/src/main/ssh/ssh-legacy-relay-router.test.ts +++ b/src/main/ssh/ssh-legacy-relay-router.test.ts @@ -13,6 +13,7 @@ function fakeRoute(listed: string[]) { const provider: SshPtyProvider = Object.create(null) const route = { provider, + heldPtyIds: () => [...listed], holds: (id: string) => listed.includes(id), serves: (id: string) => served.has(id), get servesAny() { @@ -107,4 +108,34 @@ describe('SshLegacyRelayRouter', () => { expect(route.close).toHaveBeenCalledWith('legacy-relay-router-disposed') expect(router.providerFor(HELD)).toBeUndefined() }) + + it('lists what older relays hold for the terminal gate, then hangs up unserved routes', async () => { + const route = fakeRoute([HELD]) + const { router } = routerFor(route) + + await expect(router.listHeld()).resolves.toEqual([HELD]) + expect(route.close).toHaveBeenCalledWith('legacy-relay-listed-for-terminal-gate') + }) + + it('answers null, never empty, when an older relay cannot be asked', async () => { + const { router } = routerFor(null) + + await expect(router.listHeld()).resolves.toBeNull() + await expect(routerFor(null, []).router.listHeld()).resolves.toEqual([]) + }) + + it('never hangs up a route another attach is still awaiting', async () => { + const route = fakeRoute([HELD]) + const { router } = routerFor(route) + + const [missing, served] = await Promise.all([ + router.attach('ssh:target-1@@pty2:new:9'), + router.attach(HELD) + ]) + + expect(missing).toBeNull() + expect(served?.provider).toBe(route.provider) + expect(route.close).not.toHaveBeenCalled() + expect(router.providerFor(HELD)).toBe(route.provider) + }) }) diff --git a/src/main/ssh/ssh-legacy-relay-router.ts b/src/main/ssh/ssh-legacy-relay-router.ts index 6ea70a4bcbe..cef5b0fd518 100644 --- a/src/main/ssh/ssh-legacy-relay-router.ts +++ b/src/main/ssh/ssh-legacy-relay-router.ts @@ -19,10 +19,13 @@ type RouteEntry = { pending: Promise /** Set once opened; synchronous lookups read only routes that finished opening. */ route?: SshLegacyRelayRoute | null + /** Callers awaiting or reading the route; it closes only once none remain and it serves nothing. */ + users: number } export class SshLegacyRelayRouter implements SshPtyLegacyRelayRouting { private readonly routes = new Map() + private readonly disposeListeners = new Set<() => void>() private disposed = false constructor(private readonly options: SshLegacyRelayRouterOptions) {} @@ -32,26 +35,70 @@ export class SshLegacyRelayRouter implements SshPtyLegacyRelayRouting { appPtyId: string ): Promise<{ provider: SshPtyProvider; release: () => void } | null> { for (const sockPath of await this.options.endpoints()) { - const route = await this.route(sockPath) - if (this.disposed) { - return null - } - if (route?.holds(appPtyId)) { - route.beginServing(appPtyId) - return { provider: route.provider, release: () => this.release(route, appPtyId) } - } - if (route && !route.servesAny) { - route.close('legacy-relay-holds-no-requested-terminal') + const served = await this.use( + sockPath, + 'legacy-relay-holds-no-requested-terminal', + (route) => { + if (!route?.holds(appPtyId) || this.disposed) { + return null + } + route.beginServing(appPtyId) + return { + provider: route.provider, + release: () => this.release(sockPath, route, appPtyId) + } + } + ) + if (served || this.disposed) { + return served } } return null } + /** + * Every PTY the older relays still run, for the migration terminal gate. Null when one of them + * could not be asked: an unreachable or non-bridgeable relay is unverifiable, never empty. + */ + async listHeld(): Promise { + const held: string[] = [] + for (const sockPath of await this.options.endpoints()) { + const listed = await this.use(sockPath, 'legacy-relay-listed-for-terminal-gate', (route) => + route && !this.disposed ? route.heldPtyIds() : null + ) + if (!listed) { + return null + } + held.push(...listed) + } + return held + } + /** Releases a pane whose attach through the route did not complete. */ - private release(route: SshLegacyRelayRoute, appPtyId: string): void { + private release(sockPath: string, route: SshLegacyRelayRoute, appPtyId: string): void { route.stopServing(appPtyId) - if (!route.servesAny) { - route.close('legacy-relay-attach-abandoned') + this.closeIfIdle(this.routes.get(sockPath), 'legacy-relay-attach-abandoned') + } + + /** Never closes a route another caller is still awaiting: only the last user may hang it up. */ + private async use( + sockPath: string, + idleReason: string, + read: (route: SshLegacyRelayRoute | null) => T + ): Promise { + const entry = this.entry(sockPath) + entry.users += 1 + try { + return read(await entry.pending) + } finally { + entry.users -= 1 + this.closeIfIdle(entry, idleReason) + } + } + + private closeIfIdle(entry: RouteEntry | undefined, reason: string): void { + if (entry?.route && entry.users === 0 && !entry.route.servesAny) { + entry.route.close(reason) } } @@ -75,20 +122,25 @@ export class SshLegacyRelayRouter implements SshPtyLegacyRelayRouting { return providers } + onDispose(listener: () => void): void { + this.disposeListeners.add(listener) + } + dispose(): void { this.disposed = true + this.disposeListeners.forEach((listener) => listener()) for (const { route } of this.routes.values()) { route?.close('legacy-relay-router-disposed') } this.routes.clear() } - private route(sockPath: string): Promise { + private entry(sockPath: string): RouteEntry { const existing = this.routes.get(sockPath) if (existing) { - return existing.pending + return existing } - const entry: RouteEntry = { pending: Promise.resolve(null) } + const entry: RouteEntry = { pending: Promise.resolve(null), users: 0 } entry.pending = this.options.openRoute(sockPath).then( (route) => { entry.route = route @@ -115,6 +167,6 @@ export class SshLegacyRelayRouter implements SshPtyLegacyRelayRouting { } ) this.routes.set(sockPath, entry) - return entry.pending + return entry } } diff --git a/src/main/ssh/ssh-legacy-relay-routing.ts b/src/main/ssh/ssh-legacy-relay-routing.ts index e91ac223135..614ad72e73d 100644 --- a/src/main/ssh/ssh-legacy-relay-routing.ts +++ b/src/main/ssh/ssh-legacy-relay-routing.ts @@ -4,6 +4,23 @@ import { SshLegacyRelayRoute, type LegacyRelayRouteSink } from './ssh-legacy-rel import { SshLegacyRelayRouter } from './ssh-legacy-relay-router' import { previousRelayCensus } from './ssh-previous-relay-terminals' +const routersByTarget = new Map() + +/** + * The PTYs this target's earlier-build relays still run. Null when that cannot be known: no census + * for an enumerable host (Windows never has one), no router, or an older relay that did not answer. + */ +export async function listPreviousRelayPtyIds(targetId: string): Promise { + const census = await previousRelayCensus(targetId) + if (!census.complete) { + return null + } + if (census.endpoints.length === 0) { + return [] + } + return (await routersByTarget.get(targetId)?.listHeld()) ?? null +} + /** The router a target's provider consults for PTYs only an earlier build's relay still runs. */ export function createSshLegacyRelayRouter(args: { targetId: string @@ -12,7 +29,7 @@ export function createSshLegacyRelayRouter(args: { sink: LegacyRelayRouteSink }): SshLegacyRelayRouter { const { targetId } = args - return new SshLegacyRelayRouter({ + const router = new SshLegacyRelayRouter({ targetId, endpoints: async () => (await previousRelayCensus(targetId)).endpoints, openRoute: async (sockPath) => { @@ -32,4 +49,11 @@ export function createSshLegacyRelayRouter(args: { }) } }) + routersByTarget.set(targetId, router) + router.onDispose(() => { + if (routersByTarget.get(targetId) === router) { + routersByTarget.delete(targetId) + } + }) + return router } diff --git a/src/main/ssh/ssh-previous-relay-terminals.test.ts b/src/main/ssh/ssh-previous-relay-terminals.test.ts index f6bb55a0f3e..78937dcafc6 100644 --- a/src/main/ssh/ssh-previous-relay-terminals.test.ts +++ b/src/main/ssh/ssh-previous-relay-terminals.test.ts @@ -19,6 +19,7 @@ import { clearPreviousRelayCensus, isReattachHeldByPreviousRelay, mayHoldTerminals, + previousRelayCensus, startPreviousRelayCensus } from './ssh-previous-relay-terminals' @@ -120,11 +121,63 @@ describe('previous relay terminals', () => { ).resolves.toBe(false) }) - it('keeps the existing path when the census could not run or never started', async () => { + it('keeps the existing path when no census started, and on Windows hosts', async () => { await expect(isReattachHeldByPreviousRelay('target-1', notFound)).resolves.toBe(false) - execCommand.mockRejectedValue(new Error('channel closed')) - startPreviousRelayCensus(conn, 'target-1', deployed) + startPreviousRelayCensus(conn, 'target-1', { + ...deployed, + hostPlatform: getRemoteHostPlatform('win32-x64') + }) await expect(isReattachHeldByPreviousRelay('target-1', notFound)).resolves.toBe(false) }) + + it.each([ + ['could not run', () => execCommand.mockRejectedValue(new Error('channel closed')), deployed], + ['had no node to probe with', () => {}, { ...deployed, nodePath: undefined }], + [ + 'listed more endpoints than it probes', + () => { + execCommand.mockResolvedValue( + Array.from({ length: 33 }, (_, i) => `/home/dev/.orca-remote/relay-${i}/r.sock`).join( + '\n' + ) + ) + probeRelayEndpointIncumbent.mockResolvedValue(incumbent({ verdict: 'exited' })) + }, + deployed + ] + ])('holds a not-found reattach when the census %s', async (_label, arrange, input) => { + arrange() + startPreviousRelayCensus(conn, 'target-1', input) + + await expect(previousRelayCensus('target-1')).resolves.toMatchObject({ + complete: false, + unverifiable: true + }) + await expect(isReattachHeldByPreviousRelay('target-1', notFound)).resolves.toBe(true) + }) + + it('marks a census complete only when every endpoint of an enumerable host was censused', async () => { + await expect(previousRelayCensus('target-1')).resolves.toMatchObject({ complete: false }) + + execCommand.mockResolvedValue('') + startPreviousRelayCensus(conn, 'target-1', deployed) + await expect(previousRelayCensus('target-1')).resolves.toEqual({ + endpoints: [], + nodePath: deployed.nodePath, + complete: true, + unverifiable: false + }) + }) + + it("forgets a session's census on teardown without dropping a newer deploy's", async () => { + execCommand.mockResolvedValue('') + const older = startPreviousRelayCensus(conn, 'target-1', deployed) + const newer = startPreviousRelayCensus(conn, 'target-1', deployed) + + clearPreviousRelayCensus('target-1', older) + await expect(previousRelayCensus('target-1')).resolves.toMatchObject({ complete: true }) + clearPreviousRelayCensus('target-1', newer) + await expect(previousRelayCensus('target-1')).resolves.toMatchObject({ complete: false }) + }) }) diff --git a/src/main/ssh/ssh-previous-relay-terminals.ts b/src/main/ssh/ssh-previous-relay-terminals.ts index 432635f0378..f696b92b02f 100644 --- a/src/main/ssh/ssh-previous-relay-terminals.ts +++ b/src/main/ssh/ssh-previous-relay-terminals.ts @@ -30,8 +30,23 @@ export type PreviousRelayCensusInput = { } const MAX_CENSUS_ENDPOINTS = 32 -type PreviousRelayCensus = { endpoints: string[]; nodePath?: string } +/** + * `complete` only when every older endpoint on an enumerable host was censused. `unverifiable` when + * such a host could not be fully censused: a failed run, missing inputs, or too many endpoints. + */ +type PreviousRelayCensus = { + endpoints: string[] + nodePath?: string + complete: boolean + unverifiable: boolean +} const censusByTarget = new Map>() +const NO_CENSUS: PreviousRelayCensus = { endpoints: [], complete: false, unverifiable: false } + +/** Windows pipes are not enumerable (see the superseded sweep), so those hosts keep today's path. */ +function isEnumerableRelayHost(input: PreviousRelayCensusInput): boolean { + return Boolean(input.hostPlatform && !isWindowsRemoteHost(input.hostPlatform)) +} /** An older relay that holds nothing, or is gone, cannot be running this target's terminals. */ export function mayHoldTerminals(incumbent: RelayEndpointIncumbent): boolean { @@ -44,13 +59,21 @@ export async function censusPreviousRelays( targetId: string, input: PreviousRelayCensusInput ): Promise { + return (await runPreviousRelayCensus(conn, targetId, input))?.endpoints ?? [] +} + +/** Null when the host is enumerable but an input the census needs is missing. */ +async function runPreviousRelayCensus( + conn: SshConnection, + targetId: string, + input: PreviousRelayCensusInput +): Promise<{ endpoints: string[]; truncated: boolean } | null> { const { hostPlatform, remoteHome, remoteRelayDir, nodePath, sockPath } = input - // Windows pipes are not enumerable (see the superseded sweep), so those hosts keep today's path. - if (!hostPlatform || isWindowsRemoteHost(hostPlatform) || !remoteHome || !remoteRelayDir) { - return [] + if (!hostPlatform || !isEnumerableRelayHost(input)) { + return { endpoints: [], truncated: false } } - if (!nodePath) { - return [] + if (!remoteHome || !remoteRelayDir || !nodePath) { + return null } const listing = await execCommand( conn, @@ -64,19 +87,18 @@ export async function censusPreviousRelays( }), { wrapCommand: true } ) - const sockPaths = listing + const listed = listing .split('\n') .map((line) => line.trim()) .filter((line) => line.startsWith('/')) - .slice(0, MAX_CENSUS_ENDPOINTS) const holding: string[] = [] - for (const endpoint of sockPaths) { + for (const endpoint of listed.slice(0, MAX_CENSUS_ENDPOINTS)) { const incumbent = await probeRelayEndpointIncumbent(conn, hostPlatform, nodePath, endpoint) if (mayHoldTerminals(incumbent)) { holding.push(endpoint) } } - return holding + return { endpoints: holding, truncated: listed.length > MAX_CENSUS_ENDPOINTS } } /** Starts this deploy's census; a newer deploy for the target replaces it. */ @@ -84,33 +106,50 @@ export function startPreviousRelayCensus( conn: SshConnection, targetId: string, input: PreviousRelayCensusInput -): void { - const census = censusPreviousRelays(conn, targetId, input).then( - (endpoints) => ({ endpoints, nodePath: input.nodePath }), - (error: unknown) => { - // A census that could not run leaves the reattach on today's path, which never kills anything. +): Promise { + const census = runPreviousRelayCensus(conn, targetId, input).then( + (ran): PreviousRelayCensus => { + const enumerable = isEnumerableRelayHost(input) + const unverifiable = enumerable && (!ran || ran.truncated) + return { + endpoints: ran?.endpoints ?? [], + nodePath: input.nodePath, + complete: enumerable && !unverifiable, + unverifiable + } + }, + (error: unknown): PreviousRelayCensus => { + // Not "no older relay": a census that could not run leaves its terminals unverifiable. console.warn( `[ssh-relay] Previous relay census did not run for ${targetId}: ${ error instanceof Error ? error.message : String(error) }` ) - return { endpoints: [] } + return { endpoints: [], complete: false, unverifiable: true } } ) censusByTarget.set(targetId, census) + return census } /** This deploy's census, with the node it ran under, which can also run an older bridge. */ export function previousRelayCensus(targetId: string): Promise { - return censusByTarget.get(targetId) ?? Promise.resolve({ endpoints: [] }) + return censusByTarget.get(targetId) ?? Promise.resolve(NO_CENSUS) } export async function previousRelayMayHoldTerminals(targetId: string): Promise { - return (await previousRelayCensus(targetId)).endpoints.length > 0 + const census = await previousRelayCensus(targetId) + return census.endpoints.length > 0 || census.unverifiable } -export function clearPreviousRelayCensus(targetId: string): void { - censusByTarget.delete(targetId) +/** A session's teardown passes the census it started, so a newer deploy's census survives it. */ +export function clearPreviousRelayCensus( + targetId: string, + census?: Promise +): void { + if (!census || censusByTarget.get(targetId) === census) { + censusByTarget.delete(targetId) + } } /** A not-found reattach this target must keep, because an older build's relay may run the PTY. */ diff --git a/src/main/ssh/ssh-relay-session.ts b/src/main/ssh/ssh-relay-session.ts index ddd50c17a96..7ec591b4941 100644 --- a/src/main/ssh/ssh-relay-session.ts +++ b/src/main/ssh/ssh-relay-session.ts @@ -16,6 +16,7 @@ import type { TerminalUnavailableCause } from '../../shared/terminal-unavailable import { replayPendingSshPtyKills } from './ssh-pending-pty-kill-replay' import { sweepOrphanedRelayPtys } from './ssh-orphan-relay-pty-sweep' import { + clearPreviousRelayCensus, isReattachHeldByPreviousRelay, startPreviousRelayCensus } from './ssh-previous-relay-terminals' @@ -340,6 +341,7 @@ export class SshRelaySession { private _onReady: ((targetId: string) => void) | null = null private portScanner: PortScanner | null = null private currentConnection: SshConnection | null = null + private previousRelayCensus: ReturnType | undefined // Why: a self-driven repair reconnect must not silently re-negotiate the target's grace window. private lastGraceTimeSeconds: number | undefined = undefined private hostPlatform: RemoteHostPlatform | null = null @@ -936,6 +938,7 @@ export class SshRelaySession { // Why here and not on reconnect: an explicit disconnect is user action, so the host earns a // fresh node-pty repair attempt. A reconnect must not, or the repair becomes a loop. forgetRelayNodePtyRepairs(this.targetId) + clearPreviousRelayCensus(this.targetId, this.previousRelayCensus) const recoveryRemoval = forgetSshPtyConsumerRecovery( this.targetId, this.ptyConsumerClientInstanceId, @@ -1056,7 +1059,7 @@ export class SshRelaySession { ): Promise> | null> { try { const deployed = await deployAndLaunchRelay(conn, undefined, graceTimeSeconds, this.targetId) - startPreviousRelayCensus(conn, this.targetId, deployed) + this.previousRelayCensus = startPreviousRelayCensus(conn, this.targetId, deployed) return deployed } catch (err) { // Why system SSH is excluded: it has no ssh2 shell or SFTP channel to degrade onto.