From 2ac34506efe717fd4b3fd6000ce7de99b056c3f4 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Mon, 3 Aug 2026 22:10:00 -0700 Subject: [PATCH] fix(ssh): scope connect locks to their acquisition, not target A tracked request settling after its lock was cleared (via reset or explicit end) must not unlock a newer connect on the same target. Lock IDs ensure only the owning acquisition releases, preventing stale settlements from clearing active locks. --- .../src/ssh/ssh-connect-in-flight.test.ts | 24 ++++++++++ src/renderer/src/ssh/ssh-connect-in-flight.ts | 45 ++++++++++++++----- src/renderer/src/ssh/ssh-connect-verb.ts | 8 +++- 3 files changed, 65 insertions(+), 12 deletions(-) diff --git a/src/renderer/src/ssh/ssh-connect-in-flight.test.ts b/src/renderer/src/ssh/ssh-connect-in-flight.test.ts index 0ec38f06ab6..4225c0d9f06 100644 --- a/src/renderer/src/ssh/ssh-connect-in-flight.test.ts +++ b/src/renderer/src/ssh/ssh-connect-in-flight.test.ts @@ -112,6 +112,30 @@ describe('ssh connect in-flight registry', () => { vi.useRealTimers() } }) + + // A tracked request outlives whatever dropped its lock — a reset between specs, or an + // explicit end. Its release must not unlock a later connect on the same target. + it('ignores a settle whose lock was already cleared, so a later connect keeps its lock', async () => { + let settleAbandoned: (value: string) => void = () => {} + const abandoned = trackSshConnect( + 'ssh-a', + new Promise((resolve) => { + settleAbandoned = resolve + }) + ) + resetSshConnectInFlightForTests() + + const listener = vi.fn() + subscribeSshConnectInFlight(listener) + beginSshConnect('ssh-a') + + settleAbandoned('connected') + await abandoned + await Promise.resolve() + + expect(isSshConnectInFlight('ssh-a')).toBe(true) + expect(listener).toHaveBeenCalledTimes(1) + }) }) it('stops notifying after unsubscribe, so unmounted sidebar rows do not leak', () => { diff --git a/src/renderer/src/ssh/ssh-connect-in-flight.ts b/src/renderer/src/ssh/ssh-connect-in-flight.ts index 8fbc9b99312..4a06f078c1f 100644 --- a/src/renderer/src/ssh/ssh-connect-in-flight.ts +++ b/src/renderer/src/ssh/ssh-connect-in-flight.ts @@ -4,7 +4,11 @@ import { useCallback, useSyncExternalStore } from 'react' // after ssh.connect starts), and every workspace card on a host shares one connection. // Component-local state would let two surfaces — or N cards on the same host — each fire // a connect, which on a passphrase-gated target means N credential prompts. -const inFlightTargetIds = new Set() +// Keyed by target, valued by the id of the acquisition holding the lock: a release can then +// clear only its own acquisition, so a settle that lands after the lock was dropped (a test +// reset, an explicit end) cannot take down a newer connect's lock. +const inFlightLockIds = new Map() +let lastLockId = 0 const listeners = new Set<() => void>() function emit(): void { @@ -20,23 +24,38 @@ export function subscribeSshConnectInFlight(listener: () => void): () => void { } } -export function beginSshConnect(targetId: string): void { - if (inFlightTargetIds.has(targetId)) { +function acquire(targetId: string): number { + const held = inFlightLockIds.get(targetId) + if (held !== undefined) { + return held + } + lastLockId += 1 + inFlightLockIds.set(targetId, lastLockId) + emit() + return lastLockId +} + +function releaseOwned(targetId: string, lockId: number): void { + if (inFlightLockIds.get(targetId) !== lockId) { return } - inFlightTargetIds.add(targetId) + inFlightLockIds.delete(targetId) emit() } +export function beginSshConnect(targetId: string): void { + acquire(targetId) +} + export function endSshConnect(targetId: string): void { - if (!inFlightTargetIds.delete(targetId)) { + if (!inFlightLockIds.delete(targetId)) { return } emit() } export function isSshConnectInFlight(targetId: string): boolean { - return inFlightTargetIds.has(targetId) + return inFlightLockIds.has(targetId) } /** @@ -46,9 +65,9 @@ export function isSshConnectInFlight(targetId: string): boolean { * credential prompt. Also survives unmount, unlike a `finally` in a component handler. */ export function trackSshConnect(targetId: string, request: Promise): Promise { - beginSshConnect(targetId) + const lockId = acquire(targetId) const release = (): void => { - endSshConnect(targetId) + releaseOwned(targetId, lockId) } // Two-arg then, not finally: a derived rejected promise here would go unhandled. void request.then(release, release) @@ -56,12 +75,16 @@ export function trackSshConnect(targetId: string, request: Promise): Promi } export function useSshConnectInFlight(targetId: string): boolean { - const getSnapshot = useCallback(() => inFlightTargetIds.has(targetId), [targetId]) + const getSnapshot = useCallback(() => inFlightLockIds.has(targetId), [targetId]) return useSyncExternalStore(subscribeSshConnectInFlight, getSnapshot, getSnapshot) } -/** Test-only: the registry is module state, so specs must reset it between cases. */ +/** + * Test-only: the registry is module state, so specs must reset it between cases. + * A request tracked before the reset stays pending; its release is scoped to the cleared + * lock id, so it settles into a no-op rather than unlocking the next case's target. + */ export function resetSshConnectInFlightForTests(): void { - inFlightTargetIds.clear() + inFlightLockIds.clear() emit() } diff --git a/src/renderer/src/ssh/ssh-connect-verb.ts b/src/renderer/src/ssh/ssh-connect-verb.ts index ec0b37ecd86..d5dd98a06e8 100644 --- a/src/renderer/src/ssh/ssh-connect-verb.ts +++ b/src/renderer/src/ssh/ssh-connect-verb.ts @@ -11,7 +11,13 @@ export function sshConnectVerb(status: SshConnectionStatus | null | undefined): case 'error': case 'reconnection-failed': return translate('auto.ssh.sshConnectVerb.retry', 'Retry') - default: + case null: + case undefined: + case 'connected': + case 'connecting': + case 'deploying-relay': + case 'disconnected': + case 'reconnecting': return translate('auto.ssh.sshConnectVerb.connect', 'Connect') } }