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.
This commit is contained in:
Jinjing
2026-08-04 11:04:44 -07:00
parent 5376266b5f
commit 2ac34506ef
3 changed files with 65 additions and 12 deletions
@@ -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<string>((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', () => {
+34 -11
View File
@@ -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<string>()
// 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<string, number>()
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<T>(targetId: string, request: Promise<T>): Promise<T> {
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<T>(targetId: string, request: Promise<T>): 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()
}
+7 -1
View File
@@ -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')
}
}