From 6ec4aa8c3a9bfa7bae3db3d94eeb8ead98202ccd Mon Sep 17 00:00:00 2001 From: chelsealong Date: Wed, 19 Aug 2026 02:00:46 +0000 Subject: [PATCH] fix(desktop): move the hydration-timeout retry into host.openSession A review of the previous commit found that retrying at the plugin layer (openStoredBotChat catching and re-calling host.openSession) didn't fix the reported bug: host.openSession's own catch block unconditionally calls setResumeExhaustedSessionId on a hydration timeout before rethrowing, and only an explicit resumeSession() (the manual Retry button) clears that latch for the currently-routed session. A plugin-side retry is a different code path that can hydrate the transcript fine while the full-screen "Couldn't load this session" overlay stays latched over it. host.openSession now takes a retryHydrationTimeoutOnce option and retries the open+hydration-wait internally, before the latch is ever set, so a successful retry never arms the overlay. openStoredBotChat just opts in via that option. --- .../desktop/src/plugins/hermes-bots/plugin.js | 9 +- .../tests/canonical-chat-identity.test.mjs | 60 ++++++++- apps/desktop/src/sdk/index.ts | 115 ++++++++++++------ apps/desktop/src/sdk/profile-routing.test.ts | 51 ++++++++ 4 files changed, 196 insertions(+), 39 deletions(-) diff --git a/apps/desktop/src/plugins/hermes-bots/plugin.js b/apps/desktop/src/plugins/hermes-bots/plugin.js index df073db64b..1641da4e06 100644 --- a/apps/desktop/src/plugins/hermes-bots/plugin.js +++ b/apps/desktop/src/plugins/hermes-bots/plugin.js @@ -3276,12 +3276,19 @@ async function openStoredBotChat(name, storedId, summary) { typeof summary?.message_count === 'number' && Number.isFinite(summary.message_count) const expectHistory = hasAuthoritativeCount ? summary.message_count > 0 : true + // A profile backend that just woke up can lose the hydration-timeout race + // even though the session is fine (hermes-agent#89617) — clicking Retry + // succeeds because the backend is warm by then. retryHydrationTimeoutOnce + // asks the SDK layer to retry that same wait internally, BEFORE it arms the + // core stranded-session overlay: a plugin-side retry can't do this because + // only host.openSession sees the resume-exhausted latch that overlay reads. await host.openSession(storedId, { profile: name, intent: 'main', awaitHydration: true, expectHistory, - keepAllProfilesScope: false + keepAllProfilesScope: false, + retryHydrationTimeoutOnce: true }) return storedId diff --git a/apps/desktop/src/plugins/hermes-bots/tests/canonical-chat-identity.test.mjs b/apps/desktop/src/plugins/hermes-bots/tests/canonical-chat-identity.test.mjs index 7fd8e34bb6..b59e11f20b 100644 --- a/apps/desktop/src/plugins/hermes-bots/tests/canonical-chat-identity.test.mjs +++ b/apps/desktop/src/plugins/hermes-bots/tests/canonical-chat-identity.test.mjs @@ -112,7 +112,8 @@ test('pin: preferred_session present opens the resolved session and keeps the pi intent: 'main', awaitHydration: true, expectHistory: true, - keepAllProfilesScope: false + keepAllProfilesScope: false, + retryHydrationTimeoutOnce: true } }]) assert.equal(runtime.saved.length, 0, 'a live pin must not be rewritten') @@ -214,6 +215,63 @@ test('pin: precise hit but failed hydration keeps the pin and surfaces the failu 'must not fork the forever-chat on a hiccup') }) +test('pin: a waking-backend hydration timeout asks the SDK to retry internally', async () => { + // The internal retry-and-succeed behavior lives in host.openSession itself + // (apps/desktop/src/sdk/index.ts) now, because only that layer sees the + // $resumeExhaustedSessionId latch that the core stranded-session overlay + // reads — a plugin-side retry can silently resolve while that overlay stays + // latched (hermes-agent#89617). This harness stubs host.openSession with a + // bare mock, so it can only prove the plugin ASKS for the retry, not that + // the overlay never appears; see profile-routing.test.ts for that. + const opts = [] + const runtime = loadOpenPath({ + openSession: async (id, options) => { opts.push(options) }, + request: async method => { + if (method === 'profiles.list') { + return { + profiles: [{ + name: 'ops', + preferred_session: { + id: 'pin-1', resolved_id: 'pin-1', title: 'Bot Chat', + preview: 'latest', started_at: 1, last_active: 2, message_count: 3 + } + }] + } + } + return {} + } + }) + + const result = await runtime.openBotCanonicalChat('ops', 'pin-1', HISTORY) + + assert.equal(result, 'pin-1') + assert.equal(opts.length, 1) + assert.equal(opts[0].retryHydrationTimeoutOnce, true, 'the SDK must own the hydration-timeout retry') +}) + +test('pin: a persistent hydration timeout still surfaces the failure', async () => { + const runtime = loadOpenPath({ + openSession: async () => { throw new Error("Timed out loading ops's session history.") }, + request: async method => { + if (method === 'profiles.list') { + return { + profiles: [{ + name: 'ops', + preferred_session: { + id: 'pin-1', resolved_id: 'pin-1', title: 'Bot Chat', + preview: 'latest', started_at: 1, last_active: 2, message_count: 3 + } + }] + } + } + return {} + } + }) + + await assert.rejects(runtime.openBotCanonicalChat('ops', 'pin-1', HISTORY), /Timed out loading/) + assert.equal(runtime.saved.length, 0, 'a confirmed-live pin must survive a persistent hydration timeout') +}) + // ── transient failures must never destroy the pin ────────────────────────── test('transient: profiles.list failure keeps the pin when the direct open works', async () => { diff --git a/apps/desktop/src/sdk/index.ts b/apps/desktop/src/sdk/index.ts index 18bedda009..e514259764 100644 --- a/apps/desktop/src/sdk/index.ts +++ b/apps/desktop/src/sdk/index.ts @@ -226,6 +226,12 @@ interface PluginOpenSessionOptions { intent?: OpenSessionIntent keepAllProfilesScope?: boolean profile?: null | string + /** A cold profile backend can lose the hydration-timeout race once and still + * be fine on a second try. When set, a hydration timeout is retried + * internally before it reaches the caller or arms the core stranded-session + * overlay ($resumeExhaustedSessionId) — a caller-side retry can't do this + * itself because only this SDK layer sees $resumeExhaustedSessionId. */ + retryHydrationTimeoutOnce?: boolean } function waitForFocusedSessionHydration({ @@ -567,6 +573,11 @@ export const host = { // is a gateway problem, a slow transcript is a backend-warmup one. let wakePhase: 'activation' | 'hydration' = 'activation' + // Bounded to 2 attempts (never more): a cold profile backend can lose the + // hydration-timeout race once and still be fine moments later, but this is + // a caller-opt-in retry of the SAME wait, not a backoff loop. + const maxAttempts = options.awaitHydration && options.retryHydrationTimeoutOnce ? 2 : 1 + try { if (profile && profile !== $activeGatewayProfile.get()) { // Bounded only on the hydration contract, which is where a budget and a @@ -595,47 +606,76 @@ export const host = { $gatewaySwapTarget.set(targetProfile) } - openSession( - storedSessionId, - (to: string, opts?: { replace?: boolean }) => { - const target = to.startsWith('#') ? to : `#${to}` + // Only the HYDRATION half retries. Activation already failed its own + // bounded wait above, and a wedged dial does not get better by dialling + // again inside the same wake — that is the Retry surface's job. + for (let attempt = 1; attempt <= maxAttempts; attempt++) { + try { + openSession( + storedSessionId, + (to: string, opts?: { replace?: boolean }) => { + const target = to.startsWith('#') ? to : `#${to}` - if (opts?.replace) { - window.location.replace(target) - } else { - window.location.hash = target - } - }, - options.intent ?? 'in-place' - ) + if (opts?.replace) { + window.location.replace(target) + } else { + window.location.hash = target + } + }, + options.intent ?? 'in-place' + ) - // Judge the main surface AFTER the open: on a cold start the persisted - // route can already point at this session while selection has not - // settled, so a pre-open "already selected" precondition skips the - // resume exactly when it is needed (#89206 — blank Bot Chat with the - // roster preview intact). The surface is healthy only when this stored - // session is selected, a runtime is bound, and the expected transcript - // is present; anything less gets an explicit sequenced resume request. - // The route-resume effect only honors the request while the route - // points at this session, and consumes it alongside any resume the - // navigation itself triggers, so a redundant request is a no-op. - const surfaceHealthy = - $selectedStoredSessionId.get() === storedSessionId && - Boolean($activeSessionId.get()) && - (!expectHistory || $messages.get().length > 0) + // Judge the main surface AFTER the open: on a cold start the persisted + // route can already point at this session while selection has not + // settled, so a pre-open "already selected" precondition skips the + // resume exactly when it is needed (#89206 — blank Bot Chat with the + // roster preview intact). The surface is healthy only when this stored + // session is selected, a runtime is bound, and the expected transcript + // is present; anything less gets an explicit sequenced resume request. + // The route-resume effect only honors the request while the route + // points at this session, and consumes it alongside any resume the + // navigation itself triggers, so a redundant request is a no-op. + const surfaceHealthy = + $selectedStoredSessionId.get() === storedSessionId && + Boolean($activeSessionId.get()) && + (!expectHistory || $messages.get().length > 0) - if (options.awaitHydration && !surfaceHealthy) { - requestSessionResume(storedSessionId) - } + if (options.awaitHydration && !surfaceHealthy) { + requestSessionResume(storedSessionId) + } - if (options.awaitHydration) { - await waitForFocusedSessionHydration({ - expectHistory, - generation, - profile: targetProfile, - storedSessionId, - timeoutMs: hydrationTimeoutMs - }) + if (options.awaitHydration) { + await waitForFocusedSessionHydration({ + expectHistory, + generation, + profile: targetProfile, + storedSessionId, + timeoutMs: hydrationTimeoutMs + }) + } + + break + } catch (error) { + const retryable = + options.awaitHydration && + generation === openSessionGeneration && + attempt < maxAttempts && + error instanceof Error && + error.message.startsWith('Timed out loading ') + + if (!retryable) { + throw error + } + + // Logged per attempt so a support bundle shows the retry happened at + // all; the terminal failure is reported once by the catch below. + console.warn('[bot-wake] hydration timed out, retrying', { + attempt, + hydrationWaitMs: Date.now() - profileActiveAt, + profile: targetProfile, + storedSessionId + }) + } } } catch (error) { if ( @@ -647,6 +687,7 @@ export const host = { const timedOutAt = Date.now() console.warn('[bot-wake] hydration timed out', { + attempts: wakePhase === 'hydration' ? maxAttempts : 1, hydrationWaitMs: wakePhase === 'hydration' ? timedOutAt - profileActiveAt : 0, phase: wakePhase, profile: targetProfile, diff --git a/apps/desktop/src/sdk/profile-routing.test.ts b/apps/desktop/src/sdk/profile-routing.test.ts index c96d47479c..770cc7f8cc 100644 --- a/apps/desktop/src/sdk/profile-routing.test.ts +++ b/apps/desktop/src/sdk/profile-routing.test.ts @@ -471,6 +471,57 @@ describe('profile-aware plugin session opens', () => { expect($gatewaySwapTarget.get()).toBeNull() }) + it('retries a Bot Chat hydration timeout once without ever arming the Retry surface (#89617)', async () => { + $activeGatewayProfile.set('hyoseob') + + // First attempt: leave the surface unhealthy so the hydration wait below + // times out, exactly like a profile backend still waking up. Second + // attempt: the backend is warm now — hydrate immediately. Queued as two + // one-shots (not a standing mockImplementation) so this doesn't leak into + // later tests via the shared afterEach's vi.clearAllMocks(), which clears + // call history but not a standing implementation. + vi.mocked(openSessionCore).mockImplementationOnce(() => undefined).mockImplementationOnce(() => { + setMockAtom($selectedStoredSessionId, 'waking-bot-chat') + setMockAtom($activeSessionId, 'runtime-waking') + setMockAtom($messages, [{ id: 'history-waking', parts: [], role: 'assistant' }] as never) + }) + + await host.openSession('waking-bot-chat', { + profile: 'hyoseob', + awaitHydration: true, + expectHistory: true, + hydrationTimeoutMs: 1, + retryHydrationTimeoutOnce: true + }) + + expect(openSessionCore).toHaveBeenCalledTimes(2) + // The overlay in apps/desktop/src/app/chat/index.tsx is gated purely on + // this atom equalling the routed session id — if it were ever set here, + // the user would land on "Couldn't load this session" even though the + // retry above succeeded underneath, since only a manual resumeSession() + // call clears it for the currently-routed session. + expect(setResumeExhaustedSessionId).not.toHaveBeenCalled() + expect($gatewaySwapTarget.get()).toBeNull() + }) + + it('still arms the Retry surface when a retried Bot Chat hydration times out twice', async () => { + $activeGatewayProfile.set('hyoseob') + + await expect( + host.openSession('stranded-bot-chat', { + profile: 'hyoseob', + awaitHydration: true, + expectHistory: true, + hydrationTimeoutMs: 1, + retryHydrationTimeoutOnce: true + }) + ).rejects.toThrow(/timed out loading/i) + + expect(openSessionCore).toHaveBeenCalledTimes(2) + expect(setResumeExhaustedSessionId).toHaveBeenCalledWith('stranded-bot-chat') + expect($gatewaySwapTarget.get()).toBeNull() + }) + it('lets the latest rapid bot selection win and cancels the older hydration wait', async () => { vi.mocked(ensureGatewayProfile).mockImplementation(async (target: null | string | undefined) => { $activeGatewayProfile.set(target || 'default')