fix(relay): bound a client that clears the handshake and then never frames anything

This commit is contained in:
Neil
2026-09-02 14:13:23 -07:00
parent d82ffdf79d
commit 063291071d
3 changed files with 57 additions and 13 deletions
+27 -12
View File
@@ -12,6 +12,12 @@ import type {
} from './dispatcher-contract'
import { RelayDispatcherClientState } from './dispatcher-client-state'
// Why: a client that clears the endpoint handshake and then never frames anything is invisible to
// the silence window -- it has no lastReceivedAt to go stale -- so its socket, writer and client
// entry are held for the life of the relay. The bound is deliberately several windows wide: a real
// client frames immediately after the handshake, so only a peer that is already gone reaches it.
const SILENT_CONNECT_TIMEOUT_MS = TIMEOUT_MS * 6
export abstract class RelayDispatcherClientLifecycle extends RelayDispatcherClientState {
// Why: redirect outgoing frames to the reconnected socket without rebuilding the dispatcher + handler tree.
// Why: a new multiplexer restarts at seq=1; reset state to avoid stalled acknowledgements.
@@ -122,6 +128,7 @@ export abstract class RelayDispatcherClientLifecycle extends RelayDispatcherClie
bulkChain: Promise.resolve(),
nextOutgoingSeq: 1,
highestReceivedSeq: 0,
attachedAt: Date.now(),
lastReceivedAt: null,
keepaliveObserved: false,
generation: 0,
@@ -146,6 +153,7 @@ export abstract class RelayDispatcherClientLifecycle extends RelayDispatcherClie
protected resetClient(client: RelayClient): void {
client.nextOutgoingSeq = 1
client.highestReceivedSeq = 0
client.attachedAt = Date.now()
client.lastReceivedAt = null
client.keepaliveObserved = false
client.decoder.reset()
@@ -185,8 +193,11 @@ export abstract class RelayDispatcherClientLifecycle extends RelayDispatcherClie
if (client.closed) {
continue
}
if (resumedAfterPause && client.lastReceivedAt !== null) {
client.lastReceivedAt = now
if (resumedAfterPause) {
client.attachedAt = now
if (client.lastReceivedAt !== null) {
client.lastReceivedAt = now
}
}
client.writer.enqueue(
'liveness',
@@ -220,20 +231,24 @@ export abstract class RelayDispatcherClientLifecycle extends RelayDispatcherClie
if (client === this.primaryClient) {
continue
}
// Why the null check is not just defensive: a relay is launched before its client finishes
// handshaking, and on a slow link that can exceed the window. Reaping a client that has never
// spoken would break the connect it is still completing, so silence only counts against a
// client that has already proven it can talk.
if (client.closed) {
continue
}
// Why a client that has never spoken gets its own, much wider bound: a relay is launched
// before its client finishes handshaking, and on a slow link that can exceed the silence
// window, so judging it there would break the connect it is still completing. Leaving it
// unbounded instead held its socket and client entry forever.
if (client.lastReceivedAt === null) {
if (now - client.attachedAt > SILENT_CONNECT_TIMEOUT_MS) {
this.closeClient(client, new Error('Relay client never spoke'), true)
}
continue
}
// Why keepaliveObserved gates this: not every client speaks the keepalive protocol. The
// remote `orca` CLI sends one `orca.cli` request and waits for a result budgeted in minutes
// (src/relay/remote-cli-timeout.ts), so judging it on inbound silence would kill
// `terminal wait`, `--wait` and `orchestration ask` after 20s.
if (
client.closed ||
!client.keepaliveObserved ||
client.lastReceivedAt === null ||
now - client.lastReceivedAt <= TIMEOUT_MS
) {
if (!client.keepaliveObserved || now - client.lastReceivedAt <= TIMEOUT_MS) {
continue
}
this.closeClient(client, new Error('Relay client stopped answering'), true)
+3
View File
@@ -51,6 +51,9 @@ export type RelayClient = {
bulkChain: Promise<void>
nextOutgoingSeq: number
highestReceivedSeq: number
// Why: a client that never frames anything has no staleness to measure, so the silence window
// cannot see it. Its attach time is the only clock it has.
attachedAt: number
// Why: the relay had no inbound-liveness signal at all, so a half-open client was never reaped
// and kept its owner lease and paused PTYs indefinitely.
lastReceivedAt: number | null
@@ -61,7 +61,7 @@ describe('RelayDispatcher silent-client reaper', () => {
expect(detachListener).not.toHaveBeenCalled()
})
it('never reaps a client that has not spoken yet', () => {
it('does not judge a client that has not spoken yet by the silence window', () => {
// A relay is launched before its client finishes handshaking, and on a slow link that can
// outlast the window. Reaping there would break the connect the client is still completing.
const detachListener = vi.fn()
@@ -74,6 +74,32 @@ describe('RelayDispatcher silent-client reaper', () => {
expect(detachListener).not.toHaveBeenCalledWith(clientId, expect.anything())
})
it('still bounds a client that never speaks at all', () => {
// Otherwise it is invisible to the silence window forever -- no lastReceivedAt to go stale --
// and its socket, writer and client entry are held for the life of the relay.
const detachListener = vi.fn()
dispatcher = new RelayDispatcher(() => true)
dispatcher.onClientDetached(detachListener)
const clientId = dispatcher.attachClient(() => true)
vi.advanceTimersByTime(TIMEOUT_MS * 7)
expect(detachListener).toHaveBeenCalledWith(clientId, 'local')
})
it("does not spend a mute client's connect budget while the host was suspended", () => {
// Same rebase the silence window gets: a paused process is not a peer that went away.
const detachListener = vi.fn()
dispatcher = new RelayDispatcher(() => true)
dispatcher.onClientDetached(detachListener)
const clientId = dispatcher.attachClient(() => true)
vi.setSystemTime(60 * 60_000)
vi.advanceTimersByTime(KEEPALIVE_SEND_MS)
expect(detachListener).not.toHaveBeenCalledWith(clientId, expect.anything())
})
it('never reaps a client that does not send keepalives at all', () => {
// The remote `orca` CLI opens the socket, sends one `orca.cli` request and then waits for a
// result budgeted in minutes (remote-cli-timeout.ts: 5min default, 10min for wait, 11min for