Skip to content

fix(desktop): reuse registry primary for owned session RPCs - #94824

Closed
aryaxo wants to merge 4 commits into
NousResearch:mainfrom
aryaxo:fix/desktop-reuse-registry-primary-session-route
Closed

aryaxo wants to merge 4 commits into
NousResearch:mainfrom
aryaxo:fix/desktop-reuse-registry-primary-session-route

Conversation

@aryaxo

@aryaxo aryaxo commented Aug 25, 2026 •

Copy link
Copy Markdown

Summary

  • publish the registry connection identity owned by the Desktop window's primary gateway across boot, reconnect, soft-switch, and HMR adoption
  • reuse that already-open primary socket when requestGatewayForAgent targets the exact same (connectionId, profile) owner
  • keep different profiles and different registry sources isolated on their existing secondary routes

Problem

requestGatewayForAgent currently treats every explicit connectionId as requiring a registry secondary. That is wrong when the selected remote registry connection is already the window primary: the durable session's owner metadata points back to the socket that is already open.

The duplicate helper dial can resolve the public endpoint through a different backend/profile route. The result is a split symptom: REST history from the primary renders correctly, while session.resume on the helper returns session not found and the live transcript remains static.

This was reproduced against clean current main with a focused regression test: the owner-matched resume went to a newly-created secondary and never called the primary. The observed macOS trace independently showed the same shape: correct durable stored ID, session.resume returning 4007 session not found, and a short-lived helper WebSocket; the same stored ID resumed successfully through the existing primary connection.

Fix

The gateway registry now remembers the primary descriptor's connectionId. requestGatewayForAgent collapses to requestGatewayForProfile only when both owner dimensions match:

  • requested connectionId equals the primary's published registry source
  • requested profile equals the primary profile

Unknown identity, a different source, or a different profile keeps the existing isolated-secondary behavior. Changing the primary gateway clears the remembered identity before a new descriptor is published.

Related work / duplicate check

This is complementary rather than duplicative:

Related symptom family: #93888. This patch addresses the durable-ID/helper-route variant, not that issue's reported cross-gateway runtime-ID leak.

Verification

  • regression proved red on clean main: primary request count 0, helper created
  • focused Desktop UI suites: 118 passed
  • existing REST-fallback coverage included (use-session-actions.test.tsx)
  • npm run typecheck
  • ESLint on all changed files: 0 issues
  • Prettier check on all changed files
  • git diff --check
  • full UI suite on Windows: 5,704 passed / 9 failed in five untouched files, plus four worker-start errors under parallel load
  • serial rerun of those five files: 66 passed / 3 failed; two failures are Windows locale/NBSP assertions, and the remaining gateway-settings.test.tsx collection STACK_TRACE_ERROR reproduces unchanged on pristine origin/main

Risk

Low and fail-closed. Primary reuse requires exact source and profile equality; all ambiguous or cross-profile routes preserve their prior secondary isolation. No backend protocol, persistence schema, credentials, or network configuration changes.

External macOS validation (sanitized)

A source-built Apple Silicon client at final head ed4f3c73e9bd9fcd7661237a8a38dd2ca2e29261 was validated against a generic registry-backed remote connection using profile default:

  • the existing durable session history loaded successfully
  • live activity appeared in place while the session was running, without switching or refreshing
  • a message sent from the resumed session completed and returned in the same view
  • after navigating to another stored session and returning, history reloaded and a second resumed-session send also completed in place
  • no new session not found response or RPC error was observed during either send
  • the source checkout and embedded application stamp matched the final PR head
  • macOS-focused validation passed 3/3 files and 102/102 tests; Desktop typecheck passed
  • the official installed application, remote gateway, backend, credentials, and network configuration were left unchanged

This is supplementary end-to-end evidence for the registry-owner route; it does not introduce a gateway-specific assumption into the implementation or tests.

@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 area/sessions Session lifecycle, resume, persistence, history P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 25, 2026
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference, author can ignore or act on any point.

This PR fixes a real session-routing bug in the desktop gateway layer: when the primary gateway socket is already owned by a registry connection, requesting a gateway for the same (connectionId, profile) pair would dial a secondary socket that may resolve to a different backend, causing session.resume to fail with "session not found" even though the primary has the session. The fix adds a primaryConnectionId field to the registry state, published via setPrimaryGatewayConnection at every point the primary socket is established, and collapses requestGatewayForAgent to reuse the primary when both the profile and connection ID match. The test coverage is thorough — verifying both the reuse path and the isolation of different profiles on the same connection. The comment at gateway.ts:683 explaining the collapse condition is well-written.

A few concerns:

  1. apps/desktop/src/store/gateway.ts:192 — setPrimaryGateway clears primaryConnectionId when the gateway object changes, but not when it's set to the same object. If setPrimaryGateway is called with the same gateway reference but a different profile (e.g., during a profile switch on the same socket), primaryConnectionId is preserved even though the profile changed. The collapse check at line 689 requires key === g.primaryProfile, so a stale primaryConnectionId paired with a new primaryProfile won't cause incorrect collapsing — but the state is still semantically inconsistent. Consider clearing primaryConnectionId whenever the profile changes, not just when the gateway object changes.

  2. apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts — setPrimaryGatewayConnection is called at four sites but not at all publish(conn) sites. There are publish(conn) calls elsewhere in the boot hook (e.g., in error recovery or fallback paths) that don't update primaryConnectionId. If the primary connection changes through one of those paths, primaryConnectionId becomes stale and the collapse logic may either incorrectly reuse the primary (if the old connectionId matches) or incorrectly dial a secondary (if it doesn't). An audit of all publish call sites to ensure setPrimaryGatewayConnection is always called would close this gap.

  3. apps/desktop/src/store/gateway.ts:690 — the Boolean(g.primaryConnectionId) guard is redundant. If primaryConnectionId is null, the string comparison g.primaryConnectionId === String(connectionId).trim() would already be false (since connectionId is a non-null string from the function parameter). The Boolean check adds readability but also adds a subtle implication that primaryConnectionId could be an empty string — which setPrimaryGatewayConnection already normalizes to null. Consider removing the Boolean wrapper for clarity, or document why the explicit null check is needed.

  4. apps/desktop/src/store/gateway-profile-request.test.ts:196 — test mock for hermesDesktop is set after setPrimaryGateway but before ensureGatewayForProfile. The ordering works because ensureGatewayForProfile is async, but it relies on the global window.hermesDesktop being mutated between test setup and the actual call. If tests run in parallel or the mock isn't cleaned up, this could leak into other tests. Consider scoping the mock more tightly or adding cleanup.

@aryaxo

aryaxo commented Aug 26, 2026

Copy link
Copy Markdown
Author

@Enough1122 Thanks for the review. I audited these points against final head ed4f3c73e:

  1. primaryConnectionId intentionally tracks the registry owner of the primary socket rather than its currently adopted profile. The same primary socket can adopt another profile; clearing its connection identity during that adoption would discard valid ownership. Primary reuse still requires an exact match against primaryProfile, so a mismatched profile remains isolated.
  2. The primary identity is published on every path that establishes or adopts the primary socket: cold boot, reconnect, soft switch, and HMR adoption. The remaining publish() calls update active-display, fallback, invalidation, or window metadata and can describe a secondary route; pairing all of them with setPrimaryGatewayConnection() would allow those updates to overwrite the primary socket's owner.
  3. The explicit Boolean guards are retained to make the non-empty, fail-closed owner requirement visible. They are defensive/readability guards rather than a separate behavioral branch.
  4. The test already closes secondary gateways, clears mocks, and deletes window.hermesDesktop in afterEach, so the global mock does not leak between tests.

The follow-up commits also expanded coverage across request/open/ensure and resumed-session send routing. Current focused final-head verification passes 108/108 tests. Supplementary sanitized Apple Silicon validation against a registry-backed remote connection loaded existing history, followed live activity in place, and completed resumed-session sends both before and after navigation without another session not found error.

teknium1 pushed a commit that referenced this pull request Aug 26, 2026
…nsport-error recovery

Salvage of #94192's unique work (owner-hardening portions that overlap
the class-1 branch — #94824/#93451 seams — and out-of-cluster #94864 are
intentionally excluded):

- use-gateway-request: recognize the full transport-error family
  (ECONNRESET & friends, including error.code and error.cause.code) so a
  reset SSH/remote socket triggers the connection-owned reconnect instead
  of surfacing as a request failure; background profiles keep the
  registry reconnect path for composite remote/SSH sources.
- session-tile-actions: tile attachment uploads and session RPCs follow
  the tile's composite owner (connectionId+profile) even when the active
  gateway moved to a same-named profile on another source.
- knownSessionOwner: sessions expose their complete owner (registry
  connection + profile) instead of a bare profile name that silently
  collapsed the route back to the local path; delegate/wiring resolve
  owners through it.

Fixes the SSH-reconnect share of #91365-adjacent routing gaps.
Salvaged (partial) from #94192.
teknium1 pushed a commit that referenced this pull request Aug 26, 2026
…nsport-error recovery

Salvage of #94192's unique work (owner-hardening portions that overlap
the class-1 branch — #94824/#93451 seams — and out-of-cluster #94864 are
intentionally excluded):

- use-gateway-request: recognize the full transport-error family
  (ECONNRESET & friends, including error.code and error.cause.code) so a
  reset SSH/remote socket triggers the connection-owned reconnect instead
  of surfacing as a request failure; background profiles keep the
  registry reconnect path for composite remote/SSH sources.
- session-tile-actions: tile attachment uploads and session RPCs follow
  the tile's composite owner (connectionId+profile) even when the active
  gateway moved to a same-named profile on another source.
- knownSessionOwner: sessions expose their complete owner (registry
  connection + profile) instead of a bare profile name that silently
  collapsed the route back to the local path; delegate/wiring resolve
  owners through it.

Fixes the SSH-reconnect share of #91365-adjacent routing gaps.
Salvaged (partial) from #94192.
teknium1 pushed a commit that referenced this pull request Aug 26, 2026
…nsport-error recovery

Salvage of #94192's unique work (owner-hardening portions that overlap
the class-1 branch — #94824/#93451 seams — and out-of-cluster #94864 are
intentionally excluded):

- use-gateway-request: recognize the full transport-error family
  (ECONNRESET & friends, including error.code and error.cause.code) so a
  reset SSH/remote socket triggers the connection-owned reconnect instead
  of surfacing as a request failure; background profiles keep the
  registry reconnect path for composite remote/SSH sources.
- session-tile-actions: tile attachment uploads and session RPCs follow
  the tile's composite owner (connectionId+profile) even when the active
  gateway moved to a same-named profile on another source.
- knownSessionOwner: sessions expose their complete owner (registry
  connection + profile) instead of a bare profile name that silently
  collapsed the route back to the local path; delegate/wiring resolve
  owners through it.

Fixes the SSH-reconnect share of #91365-adjacent routing gaps.
Salvaged (partial) from #94192.
@teknium1

Copy link
Copy Markdown
Collaborator

Merged via #95080 at 682a34a — your commit was cherry-picked into the consolidated salvage PR with authorship preserved, so this work carries your name in git history.

Thanks for the registry-primary reuse for owned session RPCs — clean scoping of which connection answers for an owned session.

Closing this original PR now that the consolidated branch has landed on main. Thanks for contributing to the desktop multi-gateway campaign (tracker: #94724).

@teknium1 teknium1 closed this Aug 26, 2026
and7777 pushed a commit to and7777/hermes-agent that referenced this pull request Aug 27, 2026
…nsport-error recovery

Salvage of NousResearch#94192's unique work (owner-hardening portions that overlap
the class-1 branch — NousResearch#94824/NousResearch#93451 seams — and out-of-cluster NousResearch#94864 are
intentionally excluded):

- use-gateway-request: recognize the full transport-error family
  (ECONNRESET & friends, including error.code and error.cause.code) so a
  reset SSH/remote socket triggers the connection-owned reconnect instead
  of surfacing as a request failure; background profiles keep the
  registry reconnect path for composite remote/SSH sources.
- session-tile-actions: tile attachment uploads and session RPCs follow
  the tile's composite owner (connectionId+profile) even when the active
  gateway moved to a same-named profile on another source.
- knownSessionOwner: sessions expose their complete owner (registry
  connection + profile) instead of a bare profile name that silently
  collapsed the route back to the local path; delegate/wiring resolve
  owners through it.

Fixes the SSH-reconnect share of NousResearch#91365-adjacent routing gaps.
Salvaged (partial) from NousResearch#94192.
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
…nsport-error recovery

Salvage of NousResearch#94192's unique work (owner-hardening portions that overlap
the class-1 branch — NousResearch#94824/NousResearch#93451 seams — and out-of-cluster NousResearch#94864 are
intentionally excluded):

- use-gateway-request: recognize the full transport-error family
  (ECONNRESET & friends, including error.code and error.cause.code) so a
  reset SSH/remote socket triggers the connection-owned reconnect instead
  of surfacing as a request failure; background profiles keep the
  registry reconnect path for composite remote/SSH sources.
- session-tile-actions: tile attachment uploads and session RPCs follow
  the tile's composite owner (connectionId+profile) even when the active
  gateway moved to a same-named profile on another source.
- knownSessionOwner: sessions expose their complete owner (registry
  connection + profile) instead of a bare profile name that silently
  collapsed the route back to the local path; delegate/wiring resolve
  owners through it.

Fixes the SSH-reconnect share of NousResearch#91365-adjacent routing gaps.
Salvaged (partial) from NousResearch#94192.
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 area/sessions Session lifecycle, resume, persistence, history comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists 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