mirror of
https://github.com/stablyai/orca.git
synced 2026-10-21 16:03:26 +00:00
fix(ssh): release the relay-loss watcher before teardown mux writes
Cancelling the in-flight port scan emits rpc.cancel on the control lane.
If that lane is saturated, enqueue admission fails and the writer calls
fail() -> mux.dispose('connection_lost'), which fires the relay-loss
watcher during our own teardown and schedules a redundant relay redeploy.
teardownProviders already released the watcher first, but stopPortScanning
runs ahead of it and is what emits the frame. Hoist the release into a
named method and call it before stopPortScanning on all four teardown
paths.
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
@@ -232,6 +232,53 @@ describe('SshRelaySession relay loss during setup', () => {
|
||||
expect(unregisterSshFilesystemProvider).toHaveBeenCalledWith('target-1')
|
||||
})
|
||||
|
||||
// #13548 follow-up: cancelling the in-flight port scan emits rpc.cancel on the control
|
||||
// lane, which a saturated writer turns into mux.dispose('connection_lost'). That fires
|
||||
// during our own teardown, so it must not be reported as a lost relay.
|
||||
it.each([
|
||||
['detach', (session: SshRelaySession) => session.beginShutdownDetach()],
|
||||
['dispose', (session: SshRelaySession) => session.dispose()]
|
||||
])(
|
||||
'does not report relay loss when the port-scan cancel kills the mux on %s',
|
||||
async (_path, teardown) => {
|
||||
const { session, onRelayLost } = createSession()
|
||||
// Mirrors SshChannelMultiplexer.request: an in-flight request whose signal aborts
|
||||
// emits rpc.cancel, and the mock mux dies on that notify like a saturated writer.
|
||||
muxRequestMock.mockImplementation(
|
||||
async (method: string, _params?: unknown, options?: { signal?: AbortSignal }) => {
|
||||
if (method !== 'ports.detect') {
|
||||
return []
|
||||
}
|
||||
const mux = session.getMux() as unknown as {
|
||||
failNotifyMethod: string | null
|
||||
notify: (method: string, params?: unknown) => void
|
||||
}
|
||||
mux.failNotifyMethod = 'rpc.cancel'
|
||||
return new Promise((_resolve, reject) => {
|
||||
options?.signal?.addEventListener(
|
||||
'abort',
|
||||
() => {
|
||||
mux.notify('rpc.cancel', { id: 1 })
|
||||
reject(new Error('cancelled'))
|
||||
},
|
||||
{ once: true }
|
||||
)
|
||||
})
|
||||
}
|
||||
)
|
||||
|
||||
await session.establish({} as SshConnection)
|
||||
expect(session.getState()).toBe('ready')
|
||||
expect(muxRequestMock).toHaveBeenCalledWith('ports.detect', undefined, expect.anything())
|
||||
const mux = session.getMux() as unknown as { notify: ReturnType<typeof vi.fn> }
|
||||
|
||||
teardown(session)
|
||||
|
||||
expect(mux.notify).toHaveBeenCalledWith('rpc.cancel', { id: 1 })
|
||||
expect(onRelayLost).not.toHaveBeenCalled()
|
||||
}
|
||||
)
|
||||
|
||||
// #11953: reattachKnownPtys swallows every per-PTY failure, so a mux killed by the
|
||||
// reattach burst itself never reaches the catch — the post-reattach gate has to notice.
|
||||
it('routes a mux that dies during PTY reattach into relay-loss recovery', async () => {
|
||||
|
||||
@@ -651,6 +651,7 @@ export class SshRelaySession {
|
||||
this.currentConnection = conn
|
||||
|
||||
// Why: stop scanning before teardownProviders so the poll timer can't fire against a disposed multiplexer.
|
||||
this.releaseRelayLossWatcher()
|
||||
this.stopPortScanning()
|
||||
await this.portForwardManager.removeAllForwards(this.targetId)
|
||||
this.broadcastEmptyLists()
|
||||
@@ -836,6 +837,7 @@ export class SshRelaySession {
|
||||
// Why: the whole in-memory half runs before any await so a concurrent connect can never observe
|
||||
// a half-torn session; only the durability barriers below are deferred onto the returned promise.
|
||||
this.abortController?.abort()
|
||||
this.releaseRelayLossWatcher()
|
||||
this.stopPortScanning()
|
||||
this.broadcastEmptyLists()
|
||||
this.teardownProviders('shutdown')
|
||||
@@ -909,6 +911,7 @@ export class SshRelaySession {
|
||||
}
|
||||
detachSshPtyConsumerRecovery(this.targetId, this.ptyConsumerClientInstanceId)
|
||||
this.abortController?.abort()
|
||||
this.releaseRelayLossWatcher()
|
||||
this.stopPortScanning()
|
||||
this.broadcastEmptyLists()
|
||||
this.teardownProviders('connection_lost')
|
||||
@@ -928,6 +931,7 @@ export class SshRelaySession {
|
||||
// teardown call below throws unexpectedly.
|
||||
detachSshPtyConsumerRecovery(this.targetId, this.ptyConsumerClientInstanceId)
|
||||
this.abortController?.abort()
|
||||
this.releaseRelayLossWatcher()
|
||||
this.stopPortScanning()
|
||||
this.broadcastEmptyLists()
|
||||
// Why: disconnect keeps PTY ownership so a later manual connect can reattach.
|
||||
@@ -947,9 +951,19 @@ export class SshRelaySession {
|
||||
|
||||
// ── Private ───────────────────────────────────────────────────────
|
||||
|
||||
// Why: teardown itself can kill the mux — an aborted request emits rpc.cancel, and a saturated
|
||||
// control lane turns that admission failure into mux.dispose('connection_lost'). Every teardown
|
||||
// path must release the watcher before its first mux write, or our own shutdown re-enters
|
||||
// recovery as a spurious relay loss. teardownProviders is not early enough: stopPortScanning
|
||||
// runs ahead of it and is what emits that frame.
|
||||
private releaseRelayLossWatcher(): void {
|
||||
this.muxDisposeCleanup?.()
|
||||
this.muxDisposeCleanup = null
|
||||
}
|
||||
|
||||
// Why: onStateChange only fires on SSH-level reconnects, so watch for relay-channel loss while SSH stays up and fire onRelayLost.
|
||||
private watchMuxForRelayLoss(mux: SshChannelMultiplexer): void {
|
||||
this.muxDisposeCleanup?.()
|
||||
this.releaseRelayLossWatcher()
|
||||
this.muxDisposeCleanup = mux.onDispose((reason) => {
|
||||
if (reason === 'connection_lost' && this.mux === mux && !this.isDisposed()) {
|
||||
console.warn(
|
||||
@@ -1560,8 +1574,7 @@ export class SshRelaySession {
|
||||
reason: 'shutdown' | 'connection_lost',
|
||||
outputGenerationReason: string = reason
|
||||
): void {
|
||||
this.muxDisposeCleanup?.()
|
||||
this.muxDisposeCleanup = null
|
||||
this.releaseRelayLossWatcher()
|
||||
this.muxNotificationCleanup?.()
|
||||
this.muxNotificationCleanup = null
|
||||
for (const cleanup of this.ptyRecoveryNotificationCleanups) {
|
||||
|
||||
Reference in New Issue
Block a user