mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
fix(runtime): defer websocket heartbeat startup probe (#17810)
* fix(runtime): defer websocket heartbeat startup probe * fix(runtime): defer heartbeat probes until websocket auth
This commit is contained in:
@@ -2493,30 +2493,31 @@
|
||||
"https://github.com/stablyai/orca/issues/11298",
|
||||
"https://github.com/stablyai/orca/pull/11300"
|
||||
],
|
||||
"invariant": "Every accepted runtime socket installs message, pong, close, and error ownership before any heartbeat probe. With uninterrupted timer delivery, the first socket that arms an idle heartbeat is probed immediately and an unresponsive socket is reaped within one interval. Later sockets join the existing shared cadence without another timer or immediate sweep and are reaped within two intervals. Responsive sockets survive, pause recovery grants a fresh probe, and close or error-to-close releases connection listeners and timers.",
|
||||
"oracle": "With one fake clock and exact socket identities, accept the first socket at 0 ms and require an immediate owned probe plus reaping at 100 ms when unresponsive. Keep a responsive first socket, accept an unresponsive later socket at 50 ms, require the same shared timer, its first probe at 100 ms, no early reap, and termination at 200 ms. Inject synchronous message, pong, close, and error events, then require exact heartbeat membership and zero retained timers/listeners after final close. Production transport tests independently cover real socket round trips, pre-auth and capacity bounds, revocation, shutdown, and half-open cleanup.",
|
||||
"invariant": "Every accepted runtime socket installs message, pong, close, and error ownership before any heartbeat probe. Unauthenticated sockets receive no heartbeat control frames during E2EE and are bounded by the pre-auth timeout; authenticated sockets share one periodic cadence, tolerate missed probes and event-loop pause, and release listeners and timers on close or error.",
|
||||
"oracle": "With one fake clock and exact socket identities, accept an authenticated first socket at 0 ms and require its first probe on the 100 ms tick. Accept an unauthenticated later socket at 50 ms and require no probe at the 100 ms tick; authenticate it, then require its first probe on the next shared tick and termination only after the configured consecutive-miss budget. Inject synchronous message, pong, close, and error events, then require exact heartbeat membership and zero retained timers/listeners after final close. Production transport tests independently cover real socket round trips, pre-auth and capacity bounds, revocation, shutdown, and half-open cleanup.",
|
||||
"commands": [
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/rpc/ws-transport-accept-order.test.ts src/main/runtime/rpc/remote-runtime-server-heartbeat.test.ts src/main/runtime/rpc/ws-transport.test.ts"
|
||||
"pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/rpc/ws-transport-accept-order.test.ts src/main/runtime/rpc/remote-runtime-server-heartbeat.test.ts src/main/runtime/rpc/ws-transport.test.ts src/main/runtime/rpc/ws-transport-transient-packet-loss.test.ts"
|
||||
],
|
||||
"testFiles": [
|
||||
"src/main/runtime/rpc/ws-transport-accept-order.test.ts",
|
||||
"src/main/runtime/rpc/remote-runtime-server-heartbeat.test.ts",
|
||||
"src/main/runtime/rpc/ws-transport.test.ts"
|
||||
"src/main/runtime/rpc/ws-transport.test.ts",
|
||||
"src/main/runtime/rpc/ws-transport-transient-packet-loss.test.ts"
|
||||
],
|
||||
"assertionRefs": [
|
||||
{
|
||||
"file": "src/main/runtime/rpc/ws-transport-accept-order.test.ts",
|
||||
"assertions": [
|
||||
"the first synchronous probe observes message, pong, close, and error ownership",
|
||||
"the first unresponsive socket is reaped at one interval",
|
||||
"a later socket keeps the original shared timer, is first probed on the shared tick, and is reaped within two intervals",
|
||||
"the first periodic probe observes message, pong, close, and error ownership",
|
||||
"an authenticated unresponsive socket is reaped on the configured consecutive-miss budget",
|
||||
"an unauthenticated later socket is not probed before authentication, then joins the original shared timer",
|
||||
"final close releases heartbeat membership, listeners, and timers"
|
||||
]
|
||||
},
|
||||
{
|
||||
"file": "src/main/runtime/rpc/remote-runtime-server-heartbeat.test.ts",
|
||||
"assertions": [
|
||||
"one missed probe reaps only the unresponsive client",
|
||||
"a single missed probe does not reap the unresponsive client",
|
||||
"event-loop resume grants clients a fresh probe"
|
||||
]
|
||||
},
|
||||
@@ -2527,6 +2528,12 @@
|
||||
"pre-auth, raw TCP, and accepted WebSocket resource bounds remain enforced",
|
||||
"error and close races finalize membership once"
|
||||
]
|
||||
},
|
||||
{
|
||||
"file": "src/main/runtime/rpc/ws-transport-transient-packet-loss.test.ts",
|
||||
"assertions": [
|
||||
"an authenticated real WebSocket survives one swallowed pong and responds to later probes"
|
||||
]
|
||||
}
|
||||
],
|
||||
"evidenceRuns": [
|
||||
@@ -2534,10 +2541,10 @@
|
||||
"date": "2026-07-29",
|
||||
"runner": "local",
|
||||
"platform": "macos",
|
||||
"command": "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/rpc/ws-transport-accept-order.test.ts src/main/runtime/rpc/remote-runtime-server-heartbeat.test.ts src/main/runtime/rpc/ws-transport.test.ts",
|
||||
"command": "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/rpc/ws-transport-accept-order.test.ts src/main/runtime/rpc/remote-runtime-server-heartbeat.test.ts src/main/runtime/rpc/ws-transport.test.ts src/main/runtime/rpc/ws-transport-transient-packet-loss.test.ts",
|
||||
"result": "passed",
|
||||
"durationSeconds": 0.91,
|
||||
"summary": "Three runtime transport files and 35 tests passed with deterministic first- and later-socket cadence, exact listener/timer ownership, pause recovery, security bounds, and cleanup."
|
||||
"summary": "Four runtime transport files and 42 tests passed with deterministic authenticated heartbeat cadence, handshake grace for later sockets, exact listener/timer ownership, pause recovery, transient packet-loss tolerance, security bounds, and cleanup."
|
||||
}
|
||||
],
|
||||
"runtimeBudget": {
|
||||
@@ -2550,7 +2557,7 @@
|
||||
},
|
||||
"redGreenEvidence": {
|
||||
"status": "complete",
|
||||
"evidence": "On latest main and with the listener-order fix disabled, the first synchronous probe observed no message, pong, close, or error owner. The structural listener-order fix passed those assertions. The published delayed-first-sweep alternative missed the first-socket one-interval cleanup bound. A later-socket oracle now separately pins the intended shared cadence at a 100 ms first probe and 200 ms reap after acceptance at 50 ms."
|
||||
"evidence": "On the candidate before this review fix, an authenticated first socket kept the shared timer alive while an unauthenticated socket accepted at 50 ms was pinged at the 100 ms tick; the new oracle failed. After filtering heartbeat clients to the authenticated transport map, the same byte-identical oracle passes: no pre-auth ping, first post-auth probe on the next shared tick, and bounded cleanup."
|
||||
},
|
||||
"performanceBudget": {
|
||||
"required": true,
|
||||
|
||||
+12
-1
@@ -19,7 +19,12 @@ describe('RemoteRuntimeServerHeartbeat missed-probe tolerance', () => {
|
||||
const socket = makeSocket()
|
||||
const heartbeat = new RemoteRuntimeServerHeartbeat(INTERVAL_MS, () => now)
|
||||
heartbeat.noteAlive(socket)
|
||||
heartbeat.start(() => [socket]) // probe #1
|
||||
heartbeat.start(() => [socket])
|
||||
|
||||
// Probe #1 goes out on the first interval tick.
|
||||
now += INTERVAL_MS
|
||||
await vi.advanceTimersByTimeAsync(INTERVAL_MS)
|
||||
expect(socket.ping).toHaveBeenCalledTimes(1)
|
||||
heartbeat.noteAlive(socket) // pongs probe #1
|
||||
|
||||
// A transient blackhole: probe #2 goes out and its pong is stuck in the network.
|
||||
@@ -47,6 +52,9 @@ describe('RemoteRuntimeServerHeartbeat missed-probe tolerance', () => {
|
||||
const heartbeat = new RemoteRuntimeServerHeartbeat(INTERVAL_MS, () => now)
|
||||
heartbeat.noteAlive(socket)
|
||||
heartbeat.start(() => [socket])
|
||||
now += INTERVAL_MS
|
||||
await vi.advanceTimersByTimeAsync(INTERVAL_MS)
|
||||
expect(socket.ping).toHaveBeenCalledTimes(1)
|
||||
heartbeat.noteAlive(socket)
|
||||
|
||||
const missesBeforeReap: number[] = []
|
||||
@@ -72,6 +80,9 @@ describe('RemoteRuntimeServerHeartbeat missed-probe tolerance', () => {
|
||||
const heartbeat = new RemoteRuntimeServerHeartbeat(INTERVAL_MS, () => now)
|
||||
heartbeat.noteAlive(socket)
|
||||
heartbeat.start(() => [socket])
|
||||
now += INTERVAL_MS
|
||||
await vi.advanceTimersByTimeAsync(INTERVAL_MS)
|
||||
expect(socket.ping).toHaveBeenCalledTimes(1)
|
||||
heartbeat.noteAlive(socket)
|
||||
|
||||
// Two silent probes, then a single inbound frame, repeated well past any fixed budget.
|
||||
|
||||
@@ -7,6 +7,20 @@ afterEach(() => {
|
||||
})
|
||||
|
||||
describe('RemoteRuntimeServerHeartbeat', () => {
|
||||
it('defers the first probe until after the WebSocket handshake window', async () => {
|
||||
vi.useFakeTimers()
|
||||
const socket = { ping: vi.fn(), terminate: vi.fn() } as unknown as WebSocket
|
||||
const heartbeat = new RemoteRuntimeServerHeartbeat(100)
|
||||
heartbeat.noteAlive(socket)
|
||||
heartbeat.start(() => [socket])
|
||||
|
||||
expect(socket.ping).not.toHaveBeenCalled()
|
||||
|
||||
await vi.advanceTimersByTimeAsync(100)
|
||||
expect(socket.ping).toHaveBeenCalledTimes(1)
|
||||
heartbeat.stop()
|
||||
})
|
||||
|
||||
it('still reaps a persistently silent client while another remains alive', async () => {
|
||||
vi.useFakeTimers()
|
||||
let now = 1_000
|
||||
@@ -16,19 +30,23 @@ describe('RemoteRuntimeServerHeartbeat', () => {
|
||||
const heartbeat = new RemoteRuntimeServerHeartbeat(100, () => now, 128, 2)
|
||||
heartbeat.noteAlive(responsiveSocket)
|
||||
heartbeat.noteAlive(deadSocket)
|
||||
// start() probes immediately: both are pinged now (probe #1) and cleared to await a pong.
|
||||
heartbeat.start(() => [responsiveSocket, deadSocket])
|
||||
// Only the responsive socket pongs the immediate probe.
|
||||
expect(responsiveSocket.ping).not.toHaveBeenCalled()
|
||||
expect(deadSocket.ping).not.toHaveBeenCalled()
|
||||
|
||||
// Tick #1: probe #1 goes out to both.
|
||||
now += 100
|
||||
await vi.advanceTimersByTimeAsync(100)
|
||||
heartbeat.noteAlive(responsiveSocket)
|
||||
|
||||
// Miss #1: not yet evidence, so the silent socket is re-probed rather than reaped.
|
||||
// Tick #2: miss #1 is not yet evidence, so the silent socket is re-probed.
|
||||
now += 100
|
||||
await vi.advanceTimersByTimeAsync(100)
|
||||
heartbeat.noteAlive(responsiveSocket)
|
||||
expect(deadSocket.ping).toHaveBeenCalledTimes(2)
|
||||
expect(deadSocket.terminate).not.toHaveBeenCalled()
|
||||
|
||||
// Miss #2 reaches the limit: consecutive silence is evidence.
|
||||
// Tick #3: miss #2 reaches the limit: consecutive silence is evidence.
|
||||
now += 100
|
||||
await vi.advanceTimersByTimeAsync(100)
|
||||
|
||||
@@ -46,8 +64,9 @@ describe('RemoteRuntimeServerHeartbeat', () => {
|
||||
// Limit of 1 isolates the resume grant: without it the very next sweep would reap.
|
||||
const heartbeat = new RemoteRuntimeServerHeartbeat(100, () => now, 128, 1)
|
||||
heartbeat.noteAlive(socket)
|
||||
// start() probes immediately (ping #1); the socket pongs it.
|
||||
heartbeat.start(() => [socket])
|
||||
now += 100
|
||||
await vi.advanceTimersByTimeAsync(100) // ping #1, socket pongs
|
||||
heartbeat.noteAlive(socket)
|
||||
|
||||
now += 100
|
||||
|
||||
@@ -2,8 +2,8 @@ import type { WebSocket } from 'ws'
|
||||
|
||||
// Why: one unanswered probe is UNKNOWN, not death — a cellular/Tailscale blackhole or a stalled TCP
|
||||
// retransmit routinely swallows a single pong from a peer that is still there (STA-3320). Only a run of
|
||||
// consecutive unanswered probes is evidence. Three matches the web client's own 45s liveness budget
|
||||
// (25s idle + 20s probe grace), so both ends of a paired session give up on roughly the same evidence.
|
||||
// consecutive unanswered probes is evidence. Three matches the web client's liveness budget, so both
|
||||
// ends of a paired session give up on roughly the same evidence after the initial grace interval.
|
||||
const MISSED_PROBE_LIMIT = 3
|
||||
|
||||
export class RemoteRuntimeServerHeartbeat {
|
||||
@@ -33,11 +33,6 @@ export class RemoteRuntimeServerHeartbeat {
|
||||
this.lastTickAt = this.now()
|
||||
this.timer = setInterval(() => this.sweep(getClients()), this.intervalMs)
|
||||
this.timer.unref?.()
|
||||
// Why: the interval's first tick is a full intervalMs (~15s) out, so arming on the first accepted
|
||||
// connection would leave that socket unprobed for the whole window. Sweep once now so the first
|
||||
// liveness ping goes out immediately; seeded-alive sockets are pinged (not reaped) and have until
|
||||
// the next tick to pong. WS pong is answered at the protocol level, so a live socket always survives.
|
||||
this.sweep(getClients())
|
||||
}
|
||||
|
||||
stop(): void {
|
||||
|
||||
@@ -63,13 +63,14 @@ describe('WebSocketTransport accepted socket ordering', () => {
|
||||
socket.once('open', () => events.push('open'))
|
||||
socket.emit('open')
|
||||
lifecycle.handleConnection(socket)
|
||||
transport.setClientId(socket, 'client')
|
||||
await vi.advanceTimersByTimeAsync(100)
|
||||
|
||||
expect(firstProbeListeners).toEqual({ pong: 1, message: 1, close: 1, error: 1 })
|
||||
expect(events.slice(0, 4)).toEqual(['open', 'ping', 'ready', 'pong'])
|
||||
expect(socket.terminate).not.toHaveBeenCalled()
|
||||
expect(lifecycle.heartbeatConnections.size).toBe(1)
|
||||
expect(vi.getTimerCount()).toBe(2)
|
||||
expect(vi.getTimerCount()).toBe(1)
|
||||
|
||||
events.push('close')
|
||||
socket.emit('close')
|
||||
@@ -94,15 +95,17 @@ describe('WebSocketTransport accepted socket ordering', () => {
|
||||
socket.emit('close')
|
||||
}
|
||||
})
|
||||
const { lifecycle } = createHarness(socket)
|
||||
const { lifecycle, transport } = createHarness(socket)
|
||||
|
||||
socket.once('open', () => events.push('open'))
|
||||
socket.emit('open')
|
||||
lifecycle.handleConnection(socket)
|
||||
transport.setClientId(socket, 'client')
|
||||
expect(socket.terminate).not.toHaveBeenCalled()
|
||||
// A single silent interval is not evidence: the socket is re-probed, not reaped.
|
||||
await vi.advanceTimersByTimeAsync(100)
|
||||
expect(socket.terminate).not.toHaveBeenCalled()
|
||||
await vi.advanceTimersByTimeAsync(199)
|
||||
await vi.advanceTimersByTimeAsync(299)
|
||||
expect(socket.terminate).not.toHaveBeenCalled()
|
||||
|
||||
await vi.advanceTimersByTimeAsync(1)
|
||||
@@ -125,11 +128,12 @@ describe('WebSocketTransport accepted socket ordering', () => {
|
||||
firstSocket.emit('pong')
|
||||
}
|
||||
})
|
||||
const { lifecycle } = createHarness(firstSocket)
|
||||
const { lifecycle, transport } = createHarness(firstSocket)
|
||||
|
||||
lifecycle.handleConnection(firstSocket)
|
||||
transport.setClientId(firstSocket, 'first-client')
|
||||
const sharedTimer = lifecycle.heartbeat.timer
|
||||
expect(firstPingTimes).toEqual([0])
|
||||
expect(firstPingTimes).toEqual([])
|
||||
|
||||
await vi.advanceTimersByTimeAsync(50)
|
||||
const laterPingTimes: number[] = []
|
||||
@@ -147,21 +151,25 @@ describe('WebSocketTransport accepted socket ordering', () => {
|
||||
|
||||
expect(lifecycle.heartbeat.timer).toBe(sharedTimer)
|
||||
expect(laterPingTimes).toEqual([])
|
||||
expect(vi.getTimerCount()).toBe(3)
|
||||
expect(vi.getTimerCount()).toBe(2)
|
||||
|
||||
await vi.advanceTimersByTimeAsync(50)
|
||||
expect(laterPingTimes).toEqual([100])
|
||||
expect(laterPingTimes).toEqual([])
|
||||
expect(laterSocket.terminate).not.toHaveBeenCalled()
|
||||
|
||||
transport.setClientId(laterSocket, 'later-client')
|
||||
await vi.advanceTimersByTimeAsync(100)
|
||||
expect(laterPingTimes).toEqual([200])
|
||||
|
||||
// Re-probed on every sweep while its misses bank, so a recovered path can answer immediately.
|
||||
await vi.advanceTimersByTimeAsync(299)
|
||||
expect(laterPingTimes).toEqual([100, 200, 300])
|
||||
expect(laterPingTimes).toEqual([200, 300, 400])
|
||||
expect(laterSocket.terminate).not.toHaveBeenCalled()
|
||||
|
||||
await vi.advanceTimersByTimeAsync(1)
|
||||
expect(laterSocket.terminate).toHaveBeenCalledTimes(1)
|
||||
expect(laterReapTimes).toEqual([400])
|
||||
expect(firstPingTimes).toEqual([0, 100, 200, 300, 400])
|
||||
expect(laterReapTimes).toEqual([500])
|
||||
expect(firstPingTimes).toEqual([100, 200, 300, 400, 500])
|
||||
expect(
|
||||
['pong', 'message', 'close', 'error'].map((event) => laterSocket.listenerCount(event))
|
||||
).toEqual([0, 0, 0, 0])
|
||||
@@ -182,11 +190,12 @@ describe('WebSocketTransport accepted socket ordering', () => {
|
||||
socket.emit('error', new Error('probe failed'))
|
||||
}
|
||||
})
|
||||
const { lifecycle } = createHarness(socket)
|
||||
const { lifecycle, transport } = createHarness(socket)
|
||||
|
||||
socket.once('open', () => events.push('open'))
|
||||
socket.emit('open')
|
||||
lifecycle.handleConnection(socket)
|
||||
transport.setClientId(socket, 'client')
|
||||
await vi.advanceTimersByTimeAsync(100)
|
||||
events.push('close')
|
||||
socket.emit('close')
|
||||
|
||||
@@ -68,6 +68,12 @@ describe('WebSocketTransport under transient packet loss', () => {
|
||||
client.once('open', resolve)
|
||||
client.once('error', reject)
|
||||
})
|
||||
// Heartbeats intentionally begin after authentication; stamp this synthetic peer as authenticated
|
||||
// so the liveness oracle exercises the production heartbeat path rather than pre-auth expiry.
|
||||
const wss = (transport as unknown as { wss: { clients: Set<WebSocket> } }).wss
|
||||
for (const serverSocket of wss.clients) {
|
||||
transport.setClientId(serverSocket, 'test-client')
|
||||
}
|
||||
|
||||
// Bounded by counted probe events, never by elapsed time.
|
||||
await vi.waitFor(() => expect(closed || probesReceived >= swallowedProbe + 2).toBe(true), {
|
||||
|
||||
@@ -107,8 +107,7 @@ describe('WebSocketTransport', () => {
|
||||
await waitForHeartbeatLifecycle(transport, 1, true)
|
||||
const firstServerSocket = Array.from(lifecycle.wss.clients)[0]
|
||||
expect(firstServerSocket).toBeDefined()
|
||||
// Note: arming probes immediately, so `alive` membership is racy here (the client's protocol-level
|
||||
// pong re-adds the socket right after the arm sweep clears it). Assert the arm/disarm lifecycle only.
|
||||
// Note: periodic probes run on timer interval ticks; assert the arm/disarm lifecycle only.
|
||||
const firstTimer = lifecycle.heartbeat.timer
|
||||
|
||||
const secondClient = await connectWs(transport)
|
||||
|
||||
@@ -319,11 +319,13 @@ export class WebSocketTransport implements RpcTransport {
|
||||
ws.on('close', finalizeConnection)
|
||||
ws.on('error', onError)
|
||||
|
||||
// Why: every lifecycle event must have an owner before the first synchronous probe.
|
||||
// Why: install lifecycle ownership before periodic heartbeat ticks can observe this socket.
|
||||
this.heartbeatConnections.add(ws)
|
||||
this.heartbeat.noteAlive(ws)
|
||||
if (this.heartbeatConnections.size === 1) {
|
||||
this.heartbeat.start(() => this.wss?.clients ?? [])
|
||||
// Unauthenticated sockets are protected by the pre-auth timeout; heartbeat probes begin only
|
||||
// after E2EE binds a client id, avoiding control frames during the handshake.
|
||||
this.heartbeat.start(() => this.wsClientIds.keys())
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user