Skip to content

fix(desktop): surface profile-switch failures instead of swallowing them - #89621

Closed
darkrhodes7 wants to merge 1 commit into
NousResearch:mainfrom
darkrhodes7:fix/desktop-surfaced-profile-switch-failures
Closed

darkrhodes7 wants to merge 1 commit into
NousResearch:mainfrom
darkrhodes7:fix/desktop-surfaced-profile-switch-failures

Conversation

@darkrhodes7

Copy link
Copy Markdown

Summary

The desktop profile rail's gateway switch used to fail silently. Every rejection inside ensureGatewayProfile (descriptor lookup, gateway preparation, socket dial) was caught by an empty .catch(), so a failed switch to another profile published nothing — no log, no UI state, no error. A dead click read as a broken rail, and the real reason (e.g. a failing descriptor IPC) was invisible in both the console and the swap overlay. This became visible in the field as "clicking Kuro/Vesper does nothing; only the All view still opens sessions."

Changes

  • Publish failures to a new $gatewaySwitchError atom + log to console — the ChatSwapOverlay now renders "Couldn't switch to <profile><message>" in place of the spinner when a switch fails, instead of a silent no-op. i18n keys added for en/ar/ja/zh/zh-hant + the shared types.ts contract.
  • Bound the gatewaySwitch mutex wait (30s) — a wedged in-flight switch (hung descriptor IPC or stuck dial) previously blocked every later click forever: each new switch awaited the previous promise with no timeout, so one stuck switch bricked the whole rail for the session. The activation epoch guard makes a stale switch's late publish a no-op, so failing open is safe. Applied to both the profile and agent paths.
  • Guard the mutex-release finally with identity — when the bounded wait lets a newer switch start while the old one is still wedged, the stale finally no longer nulls out the newer switch's promise or swap target.

Test Plan

  • apps/desktop: 44 tests pass across profile.test.ts, gateway-switch, gateway-profile-request, profile-agent-activation, chat/index, i18n/languages
  • tsc --noEmit clean
  • New tests: failed switch publishes the error and clears on retry; a wedged in-flight switch no longer blocks a later click; identity-guarded finally.
  • Pre-existing failures in electron/* (ssh, hardening, darwin staging) are Windows-host environment issues unrelated to this change — verified they reproduce on the base tree.

Notes

  • Fixes the class where a failed profile switch is indistinguishable from a broken install: the first click after this change surfaces the real error message in both console and UI.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) area/profiles Multi-profile isolation, HERMES_HOME scoping sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 19, 2026
@jackulau

Copy link
Copy Markdown
Contributor

Heads-up that this and #89609 (mine, opened ~20 minutes earlier) land in the same .catch() block in store/profile.ts, so they will conflict textually. I do not think either should be closed - they fix two different mechanisms, and #89586/#89622 need both. Laying out the split so a maintainer does not have to diff them.

They are not the same bug

A profile switch can fail to happen in two structurally different ways:

A. The switch throws. Descriptor lookup rejects, the dial rejects, the IPC is unreachable. The .catch() fires. This PR fixes that - it publishes $gatewaySwitchError, logs, and shows the overlay.

B. The switch declines. prepareGatewayForProfile resolves normally, and the thunk it returns reports false:

if (!activate()) {
  return
}

That happens when the entry was torn down mid-dial, when a newer activation superseded this one's epoch, or when an eviction re-pointed the active key at the primary. Nothing throws. The IIFE resolves, .catch() never runs, $gatewaySwitchError stays null, the overlay shows nothing, and ensureGatewayProfile resolves exactly as it does on success.

This PR does not change any return type and does not touch that guard, so path B is still silent after it. #89609 widens the three seams to Promise<boolean> and records published only past that guard, which is the only signal separating "switched" from "declined".

Concretely: your error atom cannot fire for a declined activation, and my boolean cannot explain a thrown one. Landing only one leaves half of #89586 in place.

Your mutex timeout is the piece I deliberately left out

GATEWAY_SWITCH_MUTEX_TIMEOUT_MS is exactly the thing I flagged in #89609 as a maintainer decision and declined to pick unilaterally, because 30s vs 15s vs 60s is a UX policy call on slow remote gateways. You picked one and justified it against the connect timeout plus a cold spawn, which is a better argument than "leave it unbounded". I think that half of this PR is the right answer for #89622 specifically - the "waking up" indicator that never clears is a switch that never settles, and neither the verdict boolean nor the error atom helps with that. Only bounding the wait does.

One thing worth checking there: $gatewaySwapTarget.set(null) runs in the finally of the owning switch. If the owner is genuinely wedged, the racer proceeds but the owner's finally still has not run, so a second click can leave $gatewaySwapTarget pointing at the older target. Worth a test that asserts the overlay clears after the timeout path, not just that the later switch proceeds.

If both land

They are complementary and the merge is small, but it is not automatic - both rewrite the same .catch(() => {...}). The combined form is:

})().catch((error: unknown) => {
  const message = error instanceof Error ? error.message : String(error ?? 'unknown error')
  console.error(`[profile] gateway switch to "${target}" failed: ${message}`)
  $gatewaySwitchError.set({ profile: target, message })
  // `published` stays false, so the caller is told the switch did not happen.
})

i.e. your body, with my published flag left untouched at false and the function still returning it. My console.warn should give way to your console.error + atom - it was the minimum viable version of what you built properly.

I am happy to rebase #89609 on top of this one once it lands, rather than the other way round, since yours carries the UI surface and the i18n strings and is the more disruptive diff to rebase. Just say which order you want.

One correctness note on the timeout

The previous switch failing-open here is safe - the activation epoch guard (see applyActive) means a stale switch can no longer publish once THIS one begins

Agreed for $gateway / $activeGatewayProfile, and I checked applyActive to be sure - it returns false on an epoch mismatch, so the stale thunk cannot move the pointer. But the stale switch's batch() also calls setConnection(connection) and invalidateProfileScopedQueries, and those run inside the if (!activate()) guard, so they are covered too. Where I am less sure is the finally: the stale switch will eventually run gatewaySwitch = null and $gatewaySwapTarget.set(null), and by then the new switch owns both. That could clear a live swap indicator or null out a live mutex mid-flight. Worth an epoch/identity check in that finally (if (gatewaySwitch === mine)) rather than an unconditional reset.

The rail's gateway switch used to die silently: every rejection inside
ensureGatewayProfile (descriptor lookup, gateway preparation, dial) was
caught by an empty .catch(), so a failed switch to another profile
published nothing — no log, no UI state, no error. A dead click read as
a broken rail, and the real reason (e.g. a failing descriptor IPC) was
invisible in both the console and the swap overlay.

- Publish failures to a new $gatewaySwitchError atom and log them to the
  console, so the swap overlay can render 'Couldn't switch to X' instead
  of silently doing nothing (en/ar/ja/zh/zh-hant keys added).
- Bound the gatewaySwitch mutex wait (30s): a wedged in-flight switch
  (hung descriptor IPC or stuck dial) used to block EVERY later click
  forever, since each new switch awaited the previous promise. The
  activation epoch guard makes a stale switch's late publish a no-op, so
  failing open is safe. Applied to both the profile and agent paths.
- Guard the mutex-release finally with identity: when the bounded wait
  lets a newer switch start while the old one is still wedged, the stale
  finally no longer nulls out the newer switch's promise or swap target.

Tests: extend profile.test.ts — failed switch publishes the error and
clears on retry; a wedged in-flight switch no longer blocks a later
click.
@teknium1

Copy link
Copy Markdown
Collaborator

Closing — the seams this PR hardens were rewritten during the resolution of #89622, and the specific failure it surfaces no longer exists.

Sequence on main: the atomic-publish series was reverted (#89785), profile switching was re-landed with fail-open semantics and an activation lease preventing mid-dial socket disposal (#89797), and the underlying release-build breakage turned out to be nanostores 1.4.0's @__NO_SIDE_EFFECTS__ annotation on batch() letting Rollup strip the switch publication from minified bundles — fixed by the 1.4.2 bump in #89875.

On the current code a switch cannot silently decline (the decline path is gone), and descriptor-lookup failures are logged rather than swallowed. If you see a switch-failure UX gap remaining on latest main — e.g. a user-visible toast rather than a console warning — a fresh PR against the current seams would be welcome. Thanks for the work here; the silent-failure framing was correct and informed the final design.

@teknium1 teknium1 closed this Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/profiles Multi-profile isolation, HERMES_HOME scoping comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants