fix(desktop): boot retry, post-reconnect busy reconciliation, refresh coalescing - #58109
Conversation
- resolveGatewayWsUrl maps mint failures to reauth only when auth-shaped (401/403/needsOauthLogin); transport errors stay retryable. - Failed initial boot retries on 5s/15s/30s/60s jittered backoff and on wake/online/visibility nudges; genuine auth failures latch with the sign-in overlay instead of thrashing. - After a socket reconnect (primary or secondary), reconcile cached busy sessions against session.active_list — clears composers stuck on 'Thinking…' when message.complete died with the socket. - Coalesce refreshSessions (1.5s trailing + single-flight, ancillary cron/messaging fan-out throttled to 30s) and debounce session.info config refetches (5s, gated to config-bearing events). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
tonydwb
left a comment
There was a problem hiding this comment.
Verdict: Approved (read-only token - formal approval deferred)
Small, well-scoped fix. No concerns.
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved (LGTM)
Desktop boot retry, post-reconnect busy reconciliation, and refresh coalescing. Multiple UX improvements for the desktop app. Well-scoped.
What Looks Good
- Multiple related desktop improvements
- No security concerns
- No debug artifacts
Reviewed by Hermes Agent
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Comment (large surface area)
Desktop boot retry PR (+1139/-28, 17 files): boot retry, post-reconnect busy reconciliation, refresh coalescing. Substantial but focused on the desktop restart/reconnect lifecycle.
Looks Good
- Well-scoped by concern (desktop lifecycle)
- Error handling and retry logic appear sound
Reviewed by Hermes Agent
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing the verified boot/reconnect and refresh-storm paths. The current main still has the initial-boot guard at apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts:210 and the unconditional session.info config refresh at apps/desktop/src/app/session/hooks/use-message-stream/gateway-event.ts:277.
Problems
apps/desktop/src/app/session/hooks/use-message-stream/index.ts:45makesscheduleSessionsRefreshrequired, but two current-main harnesses added after this PR's July 3 base still passrefreshSessions:compaction-event.test.tsx:27andapproval-mode-event.test.tsx:27. Update both to the new callback shape before salvaging;apps/desktop/tsconfig.jsonincludessrc, andpackage.jsontypecheck runstsc -p . --noEmit.
Suggested changes
- Pass
scheduleSessionsRefresh: vi.fn()in those two harnesses and run the desktop typecheck plus affected Vitest files after reconciling with current main.
This is an automated hermes-sweeper review.
| /** Coalesced sidebar refresh — every message.complete asks for one, so the | ||
| * callback must debounce/single-flight internally (scheduleSessionsRefresh), | ||
| * not fire a full 4-fetch fan-out per completed turn. */ | ||
| scheduleSessionsRefresh: () => void |
There was a problem hiding this comment.
This required rename also needs the current-main compaction-event.test.tsx and approval-mode-event.test.tsx harnesses updated: both still pass refreshSessions only. They were added after this PR's base, so include them in the salvage to keep tsc -p . --noEmit green.
Problem
Three renderer lifecycle gaps, reproduced on a Desktop running in global remote mode with many concurrent sessions:
reconnectNow()early-returns while!bootCompleted, and a failedboot()only latches the failure — so a backend unreachable at app start (VPN tunnel not yet up after login) leaves the boot-failure overlay until a manual retry, even after the network comes back.refreshHermesConfig()+refreshSessions()run. Nothing resets per-sessionbusy/awaitingResponsefor turns whosemessage.completedied with the socket; the 8-min watchdog clears only sidebar spinners; therunning:falseguard deliberately skips pre-first-delta turns.session.infotriggers an unthrottled/api/config+/api/config/defaultsdouble-fetch, and everymessage.completefrom any profile socket triggers a 4-request sidebar fan-out (recents + cron sessions + cron jobs + messaging). With N busy sessions this is O(N) request storms at the backend.Plus the renderer twin of the main-process misclassification fixed in #58108:
resolveGatewayWsUrlwraps any ticket-mint failure intoGatewayReauthRequiredError.Fix
resolveGatewayWsUrl(apps/shared/websocket-url.ts) wraps mint failures as reauth only when auth-shaped (newisGatewayAuthShapedError: needsOauthLogin / statusCode 401-403 / word-bounded 401|403 markers / canonical reauth phrasings — IPC-flatten-safe); transport errors rethrow unchanged so every caller keeps its backoff.boot()immediately when the previous failure was transport-shaped. Genuine auth failures latch (sign-in overlay stays; no thrash). Double-boot guarded; retry timer cleaned on unmount.useReconnectReconciliation: after a successful reconnect (primary or secondary socket), queriessession.active_liston that gateway and clearsbusy/awaitingResponse/streamIdfor cached-busy sessions that are no longer working/starting/waiting — composer included. Probe failure keeps every latch (conservative). Secondary sockets get the same treatment via a new optionalonReconnectedregistry hook.createTrailingCoalescer(trailing coalesce + single-flight + at-most-one queued follow-up):refreshSessionscoalesced at 1.5s with the cron/messaging ancillary fan-out throttled to once per 30s;session.infoconfig refetch debounced at 5s and gated to events that can actually change config-derived state.Tests
31 new/updated vitest cases across
gateway-ws-url.test.ts(20),use-gateway-boot.test.tsx(transport-retry/auth-latch/nudge/reconcile-once),use-reconnect-reconciliation.test.tsx(9),refresh-coalescer.test.ts(5),use-session-list-actions.test.tsx(3),config-refresh.test.tsx(4). Full desktop typecheck + lint clean; no regressions in the suite (verified against a stash baseline).🤖 Generated with Claude Code