fix(desktop): transport waits bounded, poll storms stopped, saved connections paint instantly (multi-gateway wave-2 phase B, tracker #94724) - #95495
Merged
Conversation
Contributor
૮ >ﻌ< ა ci reviewran on 6c607fd — fix(desktop): poll-guard reset is fire-and-forget off the re
|
alt-glitch
added
type/bug
…#93454) resolveGatewayWsUrl() in attemptReconnect() because a wedged IPC round-trip into the main process (e.g. a stuck revalidation after a liveness-probe trip) can hang these awaits forever. A later fix (e8d5660) extended the bound to resolveGatewayWsUrl() in boot() and softSwitch() too, but left the getConnection() call immediately above it in both functions unbounded. If that call wedges: during initial boot the 'Starting Hermes...' screen never resolves (bootCompleted never flips, nothing hits catch), and during a soft gateway/profile switch latches true forever since the try block's finally never runs. Wrap both with the same withTimeout()/RECONNECT_ATTEMPT_TIMEOUT_MS pattern already used for the sibling calls.
…45s constant Follow-up to the #95039 salvage: the cherry-picked bound used the 20s RECONNECT_ATTEMPT_TIMEOUT_MS on boot()/softSwitch() getConnection(), but a reviewer note (and the Phase A registry-restore work) established that boot-class awaits must ride out a full backend cold spawn — main's spawn budget is 45s (DEFAULT_BACKEND_READY_TIMEOUT_MS). A 20s renderer bound would latch boot errors on healthy-but-slow cold boots. Introduce BACKEND_BOOT_WAIT_TIMEOUT_MS (45s) in lib/with-timeout.ts as the single shared boot-class budget, point boot()/softSwitch() getConnection() and connections.ts BOOT_DESCRIPTOR_WAIT_TIMEOUT_MS at it, and keep the 20s reconnect budget only for reconnect-class awaits against an already-spawned backend. No magic-number drift: 45_000 now appears once in renderer code.
… 4001s
The composer status stack polls `process.list` every 5s while a background
process row is on screen. `process.list` is session-scoped, so against a
runtime id the gateway no longer holds it returns 4001 "session not found".
`refreshBackgroundProcesses` swallowed *every* failure with a bare `catch {}`
commented "transient socket loss". A gone session is not transient: the poll
re-sent the same dead runtime id every 5 seconds for the lifetime of the
window. On one machine this produced 31,518 gateway rejections in a day
(vs 663 the day before), 18,614 of them against a single runtime id, and it
is what users see reported as "sessions stopped with a session not found
error" after an update.
The trigger is a reconnect, not the poll itself: anything that mints a fresh
runtime (gateway restart, the #94219 reconnect/replay work, an idle-reaped
pooled backend) strands the id the status stack is still holding, and nothing
in this path ever re-checked it.
Distinguish the two failure classes:
- 4001 / "session not found" is TERMINAL for that runtime id — latch the id
and stop polling it.
- A timeout or transport error is transient — keep retrying, since the
session may well still be alive. Misclassifying that direction would
silently freeze the status stack on a healthy session.
The latch is cleared when the status stack (re)binds a session id, so a
session that comes back under a fresh runtime resumes polling normally
rather than staying dark for the life of the app.
Also name the method in the gateway's 4001 warning. That line was added in
c305839 "for diagnosability", but without the RPC name it cannot say WHICH
client call is looping — the reason this storm could not be attributed from
the logs alone. A ContextVar set in `handle_request` carries it; it is
diagnostic only and never used for authorization.
Tests:
- composer-status: 4001 stops the poll, a timeout does not, one gone session
never suppresses a healthy sibling, and a rebind resumes polling.
- tui_gateway: the rejection warning names the method.
Two review-thread deltas on the salvaged #94950 latch: - Match the gateway's structured 4001 code, not a message substring, when the rejection carries one (JsonRpcGatewayError). A coded error that merely mentions 'session not found' in wrapped text (e.g. a 5007 tool failure) must not latch the guard and freeze the status stack on a healthy session. The substring fallback survives only for codeless legacy errors. - Reset the latch on runtime re-mint, not only on status-stack rebind: wire resetBackgroundPollingGuard() at both reconnect seams that already drop stale runtime bindings (use-gateway-boot's post-reconnect resetTileRuntimeBindings and gateway.ts's reopening path), so ids the dead runtime 4001'd resume polling once a respawned backend re-mints them. Tests: code-specific match both directions; full-reset resumes every latched session.
…authoritative state settles A gateway connection that dies mid-turn leaves cached session snapshots whose busy/awaitingResponse flags can never settle: the respawned backend re-mints runtime ids, so no terminal publish ever reaches the orphaned snapshot again. #isWarmSettled treated those frozen flags as live work, so every orphan pinned its full warm transcript until app restart — roughly 5MB per reconnect cycle, which turned the restart loop in #95189 into renderer OOM. SessionStateCache now accepts an optional isAuthoritativelyActive probe. When wired, in-flight flags only block eviction while the authoritative $sessionStates record still claims work for the same runtime id; without the probe the legacy always-block behavior is preserved byte-for-byte. Eviction remains gated on needsInput, pending drafts, and active references, so a genuinely running turn (which re-asserts busy on every publish) is never a casualty. The useSessionStateCache hook wires the probe to the store it already imports. Reconnect reconciliation (reconcileBusyStatesOnReconnect) settles the authoritative record, and the next prune drains the orphaned cache entry through the normal LRU path, ownership included.
…very successful save (#95393) Live-confirmed on the Phase B build: hermesDesktop.connections.save() succeeds and the registry on disk gains the row, but the switcher menu (fed by the renderer $connectionsRegistry snapshot) keeps painting the stale list until reload. remove() already broadcasts hermes:connections:changed; save() only did so on the dial-material-edit branch, so a brand-new connection or a label rename never reached the switcher's onChanged re-pull (or any other window). Fix at the publish seam only: saveRegistryConnection now broadcasts a new 'saved' reason for every successful save that isn't a dial-material edit. 'saved' is a pure registry-refresh signal — the use-gateway-boot listener explicitly ignores it (nothing moved, so no dispose/redial/forget), while the switcher's existing onChanged listener re-pulls the snapshot. Tests: - electron/hardening.test.ts pins both broadcast branches in saveRegistryConnection (source-assertion pattern; main.ts has no exports). - connection-switcher.test.tsx mirrors the live repro scenario (/tmp/mg-ab/w2_95393.py): menu before save lacks the row, Electron's 'saved' push arrives, menu after — without reload — shows it.
The combined dynamic import meant a failed composer-status import (mocked test graphs) silently skipped resetTileRuntimeBindings too — the exact lifecycle regression CI caught. Separate best-effort trys per module.
teknium1
force-pushed
the
salvage/desktop-mg-w2b-transport
branch
from
August 26, 2026 11:30
9203616 to
7e1e7e2
Compare
composer-status imports $gateway from this module (cycle forces the dynamic import), and awaiting the module load inside openSecondary sat on the timed redial path — under CI load that pushed cold-start redials past waitFor budgets in the lifecycle suite. The reset needs no ordering guarantee relative to the dial; detach it.
This was referenced Aug 26, 2026
Finn763
added a commit
to Finn763/hermes-agent
that referenced
this pull request
Aug 26, 2026
Busy/awaiting snapshots whose runtime stays published but that no event will ever reach — reconcile leftovers skipped by scope, or turns that died before any reconnect ran — fail #isWarmSettled forever, pinning their transcripts outside both LRU bounds (NousResearch#95276). Unpublished orphans are already released deterministically by the authoritative liveness probe (NousResearch#95189, NousResearch#95495); this adds the orthogonal guard for the rest: prune() sweeps entries still busy/awaiting with no state mutation past a conservative window (30 min default; any real event resets it) as unconditional eviction candidates. Persisted sessions only; drafts, in-flight messages, needsInput waits, and live references stay protected; eviction re-validates state identity before deleting and degrades gracefully like the LRU path.
Contributor
|
Cross-reference: #73821 is open against |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Transport behavior on multi-gateway Desktop stops degrading silently: boot and soft-switch
getConnection()waits are bounded on one shared 45s constant (no more indefinite hangs, no magic-number drift), the status stack stops storming a dead session with 4001 polls (code-specific match; guard resets when the runtime re-mints), reconnect-orphaned warm transcripts release their frozen busy pins, and a newly saved connection appears in the switcher menu immediately instead of after a reload.Wave-2 Phase B (transport) of tracker #94724. Fixes #93454-shape unbounded waits, a chunk of #83134's storm, the renderer half of #95189, and #95393.
Salvaged contributor work (authorship preserved)
BACKEND_BOOT_WAIT_TIMEOUT_MS(45s appears once in renderer code)resetBackgroundPollingGuard()at both re-mint seams)#isWarmSettledunpinning for reconnect-orphaned transcriptsOur fix
connections.save/remove; regression test mirrors the live CDP repro (sabotage-proven).Validation
A/B per fix: each fires with sources at origin/main under the branch tests, clean at head. Targeted: 179/179 vitest + 611 tui-gateway pytest, tsc clean both configs. Pre-push gate caught and fixed a 5-commit stale base with leaked files before push.
Live repro: #95393 confirmed live on the rig pre-fix (scripted repro + screenshot on the issue); post-merge rig verification will use the same script.
Infographic