Skip to content

fix(desktop): repair remote profile routing, sessions, and pool lifecycle - #72835

Merged
OutThisLife merged 6 commits into
mainfrom
bb/desktop-remote-routing
Jul 27, 2026
Merged

fix(desktop): repair remote profile routing, sessions, and pool lifecycle#72835
OutThisLife merged 6 commits into
mainfrom
bb/desktop-remote-routing

Conversation

@OutThisLife

Copy link
Copy Markdown
Collaborator

What does this PR do?

Consolidates the Desktop remote/profile routing cluster. #58108 reported six defects in one bundle; #68250 landed two of them and the rest have been sitting in separate PRs that each touch the same two files. Landing them one at a time would leave three overlapping predicates behind, so they come in together with the routing decision unified.

Sessions vanish from the sidebar on an OAuth gateway. fetchProfilesSessionSlice and mergeRemoteProfileSessions called fetchJson(url, primary.token, …) directly. An OAuth primary has no static token, so the call 401s and the .catch() immediately below turns that into an empty list. The rows are still in the backend and still openable by id — they just stop appearing. Both paths now go through fetchJsonForProfile, which already picks native-bearer vs cookie-partition.

Profiles on an app-global remote churned through the backend pool. One remote backend serves every profile via ?profile=, but ensureBackend() still registered non-primary profiles in backendPool. Those entries have no process, so the idle reaper collected them and the next call re-probed the same remote, forever. They now share the primary connection with their scope carried on the descriptor.

Pooled remote backends were never retired when their host died. A pooled entry backed by a remote has no child process, so the 'exit' handler that clears a dead local backend never fires, and the renderer's keepalive touch keeps the reaper off it. Revalidation only probed the primary, so a dead descriptor was served until restart. Pooled remotes now share the primary's liveness policy, keyed per base URL.

Main-process faults left no trace. Electron pre-installs its own uncaughtException handler and only warns on unhandled rejections, so the reason goes to stderr — discarded when the app launches from Finder or the Start menu. It never reached desktop.log, so it was absent from hermes debug share. Both are now recorded and flushed synchronously, along with five loadURL calls that could leave a blank window with no explanation.

One routing table instead of three predicates. profileUsesPrimaryBackend answered "which backend", pathWithGlobalRemoteProfile answered "does the path need a scope", and ensureBackend re-checked globalRemoteActive() around both. Splitting one decision across three helpers is how the global-remote case ended up registering pool entries for a backend it never owned. resolveProfileBackendRoute() states the four routes once and returns backend, descriptor scope, and path scope together.

Related Issue

Closes #41529.
Supersedes #58108, #67578, #72740, #72808.

Not included: #61033, which redirects local profile REST to the primary. Its symptom is real, but the premise doesn't hold — 121 of 224 /api routes take no profile parameter, including POST /api/memory/reset, POST /api/curator/run, and the webhook routes, and /api/config, /api/model, and /api/skills are only partially scoped. Today those calls reach a pool backend spawned with --profile, so HERMES_HOME scopes them; redirecting them to the primary would silently act on the wrong profile. That fix needs an exact-path gate, and it now has a single predicate to extend rather than a third to add. Detail in #61033 (comment).

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • apps/desktop/electron/profile-session-routing.ts (new) — fetchPrimaryProfileSessions(), shared by the legacy merge path and the batched sidebar path, preserving the empty-list fallback for a genuinely dead remote.
  • apps/desktop/electron/connection-config.tsresolveProfileBackendRoute() replaces profileUsesPrimaryBackend and drives pathWithGlobalRemoteProfile.
  • apps/desktop/electron/remote-liveness.tsrevalidatePooledRemoteBackends(), reusing the existing RemoteLivenessTracker rather than adding a second policy.
  • apps/desktop/electron/crash-forensics.ts (new) — describeCrashReason() and installCrashForensics(), dependency-injected so the handlers are tested for real.
  • apps/desktop/electron/main.ts — read the route instead of recomputing it, record entry.remoteBaseUrl for pooled remotes, probe the pool on the revalidate tick, install the fault handlers, and route the unhandled loadURL calls through a labelled helper.

One behavior change beyond the fixes: hermes:api now passes the primary profile through, so the primary stops sending itself a redundant ?profile=<self> on a global remote that already serves it.

How to Test

  1. From apps/desktop: npx vitest run --project electron
  2. From apps/desktop: npm run typecheck
  3. Point Desktop at an OAuth-gated remote gateway with several profiles. Before: the sidebar is empty while the same sessions open fine by id, and desktop.log fills with Reaping idle profile backend every 600s. After: sessions list, and non-primary profiles stay on the shared connection.
  4. Give a named profile its own remote override, open it so it pools, then stop that host. The descriptor is now dropped after the standard failure streak instead of being served until restart.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass — N/A, Desktop-only
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Credit

Every commit is cherry-picked so authorship survives in git history:

Screenshots / Logs

Local validation on macOS, branched from e643f2e91:

  • npx vitest run --project electron: 813 passed, 2 skipped
  • npm run typecheck: passed
  • ESLint and Prettier on the changed files: passed

TheAngryPit and others added 6 commits July 27, 2026 13:41
Keep non-primary profiles that inherit the app-global remote on the primary connection descriptor instead of creating processless pool entries that the idle reaper repeatedly removes.

Preserve per-profile remote overrides and local pooled backends, and cover the routing policy with behavioral tests.

Co-authored-by: Rodrigo Fernandez <rodrigo@nxtlevelsaas.com>
A pooled backend entry pointing at a remote host has no child process, so
the 'exit' handler that clears a dead local backend never fires. The
renderer's 60s keepalive touch also spares it from the idle reaper. Nothing
was left to retire the descriptor, so once the host went away the pool kept
serving it and every profile bound to that host stayed broken until restart.

Pooled remote descriptors now share the primary's liveness policy: probed on
the same revalidate tick, keyed per base URL, and dropped only after the
same consecutive-failure limit, so the next ensureBackend() rebuilds.

Co-authored-by: Rodrigo Fernandez <rod@nxtlevel.dev>
Electron pre-installs its own uncaughtException listener and only warns on
unhandled rejections, so a main-process fault usually leaves the app running
with the reason on stderr — which nothing captures when the app is launched
from Finder or the Start menu. The fault never reaches desktop.log, so it is
absent from `hermes debug share` and the user can only describe symptoms.

Record both to desktop.log and flush synchronously, since a fault that does
prove fatal leaves no chance for the batched async flush. Five loadURL calls
were also unhandled, each able to leave a blank window with no explanation
anywhere the user can send us; they now name the surface that failed.

Co-authored-by: Rodrigo Fernandez <rod@nxtlevel.dev>
Three helpers each re-derived part of the same decision: which backend
serves profile P, and does its REST path need a `?profile=` scope.
profileUsesPrimaryBackend answered the first half, pathWithGlobalRemoteProfile
answered the second, and ensureBackend re-checked globalRemoteActive() around
both. Splitting one table across three predicates is how the global-remote
case ended up registering reapable pool entries for a backend it never owned.

resolveProfileBackendRoute() states the four routes in one place and returns
the backend, the descriptor scope, and whether the path needs a query
parameter. The call sites read the answer instead of recomputing it.

One behavior change falls out: `hermes:api` now passes the primary profile
through, so the primary no longer sends itself a redundant `?profile=<self>`
on a global remote that already serves it.
Co-authored-by: Rodrigo Fernandez <rod-nxtlevel@users.noreply.github.com>
Co-authored-by: sealca <sealca@users.noreply.github.com>
Co-authored-by: Vitor Cepeda Lopes <TheAngryPit@users.noreply.github.com>
Co-authored-by: Gille <4317663+helix4u@users.noreply.github.com>
Co-authored-by: nrmjeremy <nrmjeremy@users.noreply.github.com>
@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

૮ >ﻌ< ა ci review

ran on 5409e81

ℹ️ Info

Desktop E2E visual evidence · View test artifacts · View job

1 visual diff.

inline evidence upload failed.

Failed to upload diff-665a0833239e-onboarding-overlay-diff.png with gh image (exit code 1): Error uploading /home/runner/work/_temp/e2e-evidence/diff-665a0833239e-onboarding-overlay-diff.png: step 0 (get upload token): uploadToken not found on repo page — do you have write access to NousResearch/hermes-agent? (or, if NousResearch enforces SAML SSO, authorize at https://github.com/orgs/NousResearch/sso)

@OutThisLife
OutThisLife merged commit 5fde131 into main Jul 27, 2026
34 checks passed
@OutThisLife
OutThisLife deleted the bb/desktop-remote-routing branch July 27, 2026 18:55
@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) area/profiles Multi-profile isolation, HERMES_HOME scoping P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 27, 2026
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…mote-routing

fix(desktop): repair remote profile routing, sessions, and pool lifecycle
33hodl pushed a commit to 33hodl/hermes-agent that referenced this pull request Aug 12, 2026
…mote-routing

fix(desktop): repair remote profile routing, sessions, and pool lifecycle
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-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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.

[Bug]: Desktop hides global OAuth sessions when a token-auth remote profile is configured

4 participants