fix(gateway): recover persisted turns after WebSocket replacement - #83166
fix(gateway): recover persisted turns after WebSocket replacement#83166100yenadmin wants to merge 8 commits into
Conversation
Fixes NousResearch#32997 The Ink TUI WebSocket client had no heartbeat and no auto-reconnect, so a silent connection drop (macOS sleep, proxy idle timeout, VPN reconnect) left the UI stranded at a dead socket with only Ctrl+C as an escape. Add a periodic ping keepalive plus dead-connection detection that forces a reconnect with exponential backoff; never reconnect after an intentional kill(). Supersedes NousResearch#30114 (which only restarted on the exit event and could not heal silent drops) by moving reconnect into the transport client and adding the heartbeat.
Co-authored-by: Tamaz Sujashvili <tamazsujashvili@labarum.onmicrosoft.com> Co-authored-by: Indigo Karasu <mx.indigo.karasu@gmail.com>
|
Superseded by the current-head review request below; this request referred to an earlier PR head. |
|
@codex review current head 8b145ca. The final delta aligns backend stream-owner expiry with the negotiated 45-second heartbeat deadline, so a replacement socket can activate immediately instead of waiting behind a transport the client has declared dead. Please focus on message delivery and transport ownership. |
|
@OutThisLife could you approve the fork workflows and provide (or route) one current-head semantic review for message delivery and session transport ownership? The exact head is 8b145ca; both workflows are currently blocked at GitHub’s action_required approval gate, and the local disconnect-mid-turn/silent-blackhole harnesses are green without duplicate prompt or final rows. |
|
@alt-glitch @Teknium this isn't P2, remote gateways break every 1-4 hrs and become P0 without these fixes so at best this is a P1 given there are other "options" but remote gateway usage is P0-P1 |
|
This PR bundles five separable concerns and is stale against current main, so rather than push a 1,682-line rebase I'm splitting it into single-concern PRs on current main ( Slice 1 is up: #89958 — Planned follow-ons: I'll file them in dependency order as each is rebased and green. Once slice 1 lands (or is cherry-picked), I'll close this omnibus and track the rest against the slices. Holding this PR open only as the umbrella until then. |
|
Closing this omnibus — it's been split into single-concern PRs on current main, each with its own tests. That's a much fairer ask than this 28-file, +1682 diff, and it lets the independent pieces land without waiting on the rest:
Two notes so nothing looks quietly dropped:
Nothing is lost by closing this; the PRs above are the same work, current and green. |
Stacks on NousResearch#89958 (the gateway.ping heartbeat wire contract), which provides the transport liveness signals this change reads (WSTransport.closed / last_inbound_at / mark_inbound). Part of NousResearch#83166. A reconnecting renderer could not reclaim its session stream while a dropped or silent socket still "owned" it. Introduce a bounded ownership window keyed to the negotiated heartbeat deadline: - _TRANSPORT_OWNERSHIP_LIVE_S (45s), _transport_is_recently_live(): a socket that is closed, unregistered, detached, or past the heartbeat window no longer blocks a reclaim. Stdio/test transports keep their historical behavior; the conflict contract is only for competing WS clients. - _bind_session_transport(): atomically rebinds a session unless another recently-live WS still owns it, returning False so callers can reject. - Route the session-stream ingress paths through the guard: session.activate, session.resume (incl. the fast-path and adopt-other-session reuse via the _reuse_live_payload -> _reuse_live_response rename), and prompt.submit. Each returns JSON-RPC 4091 "session stream is owned by another active client" when a live owner is present. - Move _live_session_payload's transport binding to the same guard. Rework note (verdict-flagged): the queued-prompt drain (server.py) is guarded inline rather than via _bind_session_transport because it already holds history_lock, which the helper re-acquires (non-reentrant) — a queued prompt from an old renderer no longer steals a recently-live owner. The compute-host reattach sites (compute_host.py) are intentionally out of this claim: they are the single-owner compute-host channel rebinding its own transport, not a competing-renderer steal vector, and refusing the bind there would break the attach path. The PR body claim is narrowed accordingly to the WS ingress paths plus the queued-prompt drain. Tests: a recently-live owner is protected from steal, an expired/released owner can be rebound (unit + WS loopback), and a queued prompt cannot steal a recently-live owner. The stale-busy-state reconciliation is a separate slice and is intentionally excluded here.
the client half of the gateway.ping heartbeat contract (NousResearch#89958); detects a silently-dropped socket via missed ping-acks and reconnects with bounded backoff; part of the NousResearch#83166 recovery series. (cherry picked from commit 57bd8fd)
…GatewayClient The shared-client half of the gateway.ping heartbeat contract (NousResearch#89958); tracks lastInboundAt, sends pings, invalidates a silently-dead socket. Part of NousResearch#83166. (cherry picked from commit 4c4a410)
Stacks on NousResearch#89958 (the gateway.ping heartbeat wire contract), which provides the transport liveness signals this change reads (WSTransport.closed / last_inbound_at / mark_inbound). Part of NousResearch#83166. A reconnecting renderer could not reclaim its session stream while a dropped or silent socket still "owned" it. Introduce a bounded ownership window keyed to the negotiated heartbeat deadline: - _TRANSPORT_OWNERSHIP_LIVE_S (45s), _transport_is_recently_live(): a socket that is closed, unregistered, detached, or past the heartbeat window no longer blocks a reclaim. Stdio/test transports keep their historical behavior; the conflict contract is only for competing WS clients. - _bind_session_transport(): atomically rebinds a session unless another recently-live WS still owns it, returning False so callers can reject. - Route the session-stream ingress paths through the guard: session.activate, session.resume (incl. the fast-path and adopt-other-session reuse via the _reuse_live_payload -> _reuse_live_response rename), and prompt.submit. Each returns JSON-RPC 4091 "session stream is owned by another active client" when a live owner is present. - Move _live_session_payload's transport binding to the same guard. Rework note (verdict-flagged): the queued-prompt drain (server.py) is guarded inline rather than via _bind_session_transport because it already holds history_lock, which the helper re-acquires (non-reentrant) — a queued prompt from an old renderer no longer steals a recently-live owner. The compute-host reattach sites (compute_host.py) are intentionally out of this claim: they are the single-owner compute-host channel rebinding its own transport, not a competing-renderer steal vector, and refusing the bind there would break the attach path. The PR body claim is narrowed accordingly to the WS ingress paths plus the queued-prompt drain. Tests: a recently-live owner is protected from steal, an expired/released owner can be rebound (unit + WS loopback), and a queued prompt cannot steal a recently-live owner. The stale-busy-state reconciliation is a separate slice and is intentionally excluded here. (cherry picked from commit b05a60b)
A finished or dead prompt-run worker thread could leave session state wedged: `running` and `inflight_turn` stayed pinned forever after the worker had already exited (e.g. a dropped transport). Reconnect callers read the live payload as their authority, so a stale busy projection stranded the client on a spinner. Reconcile that impossible state in `_live_session_payload` before it is returned: if the session is marked running but its `_run_thread` has exited, clear `running` and the inflight turn. A missing thread is left untouched (other paths briefly set `running` before registering a worker). This is a standalone fix, independent of the WebSocket heartbeat wire contract (NousResearch#89958); part of the NousResearch#83166 recovery work. Receipts: scripts/run_tests.sh tests/test_tui_gateway_server.py \ -k activate_clears_stale_busy fail-before (reconcile call removed): 1 failed pass-after: 1 passed, 0 failed (cherry picked from commit 8146527)
the client half of the gateway.ping heartbeat contract (NousResearch#89958); detects a silently-dropped socket via missed ping-acks and reconnects with bounded backoff; part of the NousResearch#83166 recovery series. (cherry picked from commit 57bd8fd)
…GatewayClient The shared-client half of the gateway.ping heartbeat contract (NousResearch#89958); tracks lastInboundAt, sends pings, invalidates a silently-dead socket. Part of NousResearch#83166. (cherry picked from commit 4c4a410)
Stacks on NousResearch#89958 (the gateway.ping heartbeat wire contract), which provides the transport liveness signals this change reads (WSTransport.closed / last_inbound_at / mark_inbound). Part of NousResearch#83166. A reconnecting renderer could not reclaim its session stream while a dropped or silent socket still "owned" it. Introduce a bounded ownership window keyed to the negotiated heartbeat deadline: - _TRANSPORT_OWNERSHIP_LIVE_S (45s), _transport_is_recently_live(): a socket that is closed, unregistered, detached, or past the heartbeat window no longer blocks a reclaim. Stdio/test transports keep their historical behavior; the conflict contract is only for competing WS clients. - _bind_session_transport(): atomically rebinds a session unless another recently-live WS still owns it, returning False so callers can reject. - Route the session-stream ingress paths through the guard: session.activate, session.resume (incl. the fast-path and adopt-other-session reuse via the _reuse_live_payload -> _reuse_live_response rename), and prompt.submit. Each returns JSON-RPC 4091 "session stream is owned by another active client" when a live owner is present. - Move _live_session_payload's transport binding to the same guard. Rework note (verdict-flagged): the queued-prompt drain (server.py) is guarded inline rather than via _bind_session_transport because it already holds history_lock, which the helper re-acquires (non-reentrant) — a queued prompt from an old renderer no longer steals a recently-live owner. The compute-host reattach sites (compute_host.py) are intentionally out of this claim: they are the single-owner compute-host channel rebinding its own transport, not a competing-renderer steal vector, and refusing the bind there would break the attach path. The PR body claim is narrowed accordingly to the WS ingress paths plus the queued-prompt drain. Tests: a recently-live owner is protected from steal, an expired/released owner can be rebound (unit + WS loopback), and a queued prompt cannot steal a recently-live owner. The stale-busy-state reconciliation is a separate slice and is intentionally excluded here. (cherry picked from commit b05a60b)
A finished or dead prompt-run worker thread could leave session state wedged: `running` and `inflight_turn` stayed pinned forever after the worker had already exited (e.g. a dropped transport). Reconnect callers read the live payload as their authority, so a stale busy projection stranded the client on a spinner. Reconcile that impossible state in `_live_session_payload` before it is returned: if the session is marked running but its `_run_thread` has exited, clear `running` and the inflight turn. A missing thread is left untouched (other paths briefly set `running` before registering a worker). This is a standalone fix, independent of the WebSocket heartbeat wire contract (NousResearch#89958); part of the NousResearch#83166 recovery work. Receipts: scripts/run_tests.sh tests/test_tui_gateway_server.py \ -k activate_clears_stale_busy fail-before (reconcile call removed): 1 failed pass-after: 1 passed, 0 failed (cherry picked from commit 8146527)
Stacks on NousResearch#89958 (the gateway.ping heartbeat wire contract), which provides the transport liveness signals this change reads (WSTransport.closed / last_inbound_at / mark_inbound). Part of NousResearch#83166. A reconnecting renderer could not reclaim its session stream while a dropped or silent socket still "owned" it. Introduce a bounded ownership window keyed to the negotiated heartbeat deadline: - _TRANSPORT_OWNERSHIP_LIVE_S (45s), _transport_is_recently_live(): a socket that is closed, unregistered, detached, or past the heartbeat window no longer blocks a reclaim. Stdio/test transports keep their historical behavior; the conflict contract is only for competing WS clients. - _bind_session_transport(): atomically rebinds a session unless another recently-live WS still owns it, returning False so callers can reject. - Route the session-stream ingress paths through the guard: session.activate, session.resume (incl. the fast-path and adopt-other-session reuse via the _reuse_live_payload -> _reuse_live_response rename), and prompt.submit. Each returns JSON-RPC 4091 "session stream is owned by another active client" when a live owner is present. - Move _live_session_payload's transport binding to the same guard. Rework note (verdict-flagged): the queued-prompt drain (server.py) is guarded inline rather than via _bind_session_transport because it already holds history_lock, which the helper re-acquires (non-reentrant) — a queued prompt from an old renderer no longer steals a recently-live owner. The compute-host reattach sites (compute_host.py) are intentionally out of this claim: they are the single-owner compute-host channel rebinding its own transport, not a competing-renderer steal vector, and refusing the bind there would break the attach path. The PR body claim is narrowed accordingly to the WS ingress paths plus the queued-prompt drain. Tests: a recently-live owner is protected from steal, an expired/released owner can be rebound (unit + WS loopback), and a queued prompt cannot steal a recently-live owner. The stale-busy-state reconciliation is a separate slice and is intentionally excluded here. (cherry picked from commit b05a60b) (cherry picked from commit d67bef8)
Closes #83147.
What changed
gateway.readyprompt.submitnon-replayable after ambiguous transport outcomes; reconcile authoritative session state or restore the draft/attachments with a retryable delivery-not-confirmed errorrunning/inflight state only when a registered prompt worker is confirmed deadkill()suppressionOlder backends remain compatible because heartbeat timers start only when the backend advertises the capability.
Fault-injection proof
hermes-ws-recovery-v1passes on head8b145ca89:message.completeidlecloseprompt.submitremains at exactly one callFocused local checks on the exact head:
git diff --check: passedSource PR credit
This combines the still-relevant behavior and tests from:
The source PRs were used as behavioral specifications and test sources; obsolete code was not blindly cherry-picked.
Scope boundary
This PR does not add a durable event outbox or delivery acknowledgement, change Relay concurrency/readiness authentication, alter proxy lifetime policy, or perform any deployment/backport. Passing proof establishes PR-head behavior only, not released or deployed customer behavior.