Skip to content

fix(desktop): scope pluginSocket's connection to the active profile - #73044

Closed
pierrenode wants to merge 1 commit into
NousResearch:mainfrom
pierrenode:fix/desktop-plugin-socket-profile-routing
Closed

fix(desktop): scope pluginSocket's connection to the active profile#73044
pierrenode wants to merge 1 commit into
NousResearch:mainfrom
pierrenode:fix/desktop-plugin-socket-profile-routing

Conversation

@pierrenode

Copy link
Copy Markdown
Contributor

Summary

pluginSocket (apps/desktop/src/hermes.ts) is documented in its own docstring as "the live twin of pluginRest, scoped the same way". It isn't:

  • pluginRest sends ...profileScoped()window.hermesDesktop.api({..., profile})hermes:api IPC → ensureBackend(profile) → the profile-routing table.
  • pluginSocket calls window.hermesDesktop.getConnection() with no argumenthermes:connection IPC → ensureBackend(undefined).

ensureBackend's own comment says "an empty / unknown profile resolves to the primary" — confirmed in electron/main.ts:

async function ensureBackend(profile) {
  const key = profile && String(profile).trim() ? String(profile).trim() : primaryProfileKey()
  ...
}

So an unscoped getConnection() call always resolves to the primary profile's backend, regardless of which profile is actually active.

apps/desktop/src/lib/voice-playback.ts's resolveSpeakStreamUrl has the identical gap (desktop.getConnection(), no profile).

Impact

For a plugin used from a non-primary profile (e.g. kanban), REST calls (pluginRest) go to the correct pooled backend while the plugin's live WebSocket (pluginSocket) silently connects to the primary profile's backend instead. A multi-profile user opening the kanban board on a secondary profile sees that profile's board data (from pluginRest) but live events sourced from the wrong (primary) profile's backend — updates never arrive, and the primary profile's unrelated events may leak in.

TTS streaming (startSpeechStreamresolveSpeakStreamUrl) has the same misrouting: voice playback always uses the primary profile's TTS configuration/backend even when speaking from a secondary profile's session.

Fix

Both call sites now pass the active profile through to getConnection, mirroring the sibling call sites that already do this correctly (use-gateway-request.ts, use-gateway-boot.ts, store/gateway.ts, store/profile.ts):

  • hermes.ts::pluginSocket — passes the module-local _apiProfile (the same state profileScoped() reads for pluginRest, kept module-private specifically to avoid a store import cycle per its own comment).
  • voice-playback.ts::resolveSpeakStreamUrl — imports $activeGatewayProfile from @/store/profile and passes .get(), matching the pattern already used in use-gateway-request.ts.

No behavior change for single-profile users (both resolve to 'default', matching primaryProfileKey()'s own fallback).

Test plan

  • New tests in hermes.test.ts (pluginSocket describe block): asserts getConnection is called with the scoped profile when one is set via setApiRequestProfile, and with null when none is scoped.

  • New voice-playback.test.ts: asserts getConnection is called with $activeGatewayProfile's current value.

  • Mutation-verify: stashed both production fixes and reran the new tests against pre-fix code — all 4 new assertions fail as expected (getConnection called with no arguments instead of the profile).

  • Broader regression sweep: npx vitest run src/store src/lib src/hermes.test.ts — 1020 tests pass, no regressions.

  • npx tsc --noEmit clean (no circular-import issue from the new @/store/profile import in voice-playback.ts).

  • npx eslint and npx prettier --check clean on all changed files.

  • New regression tests pass against the fix

  • New regression tests fail against pre-fix code (mutation-verify)

  • Broader src/store + src/lib suite passes (1020 passed)

  • tsc --noEmit, eslint, prettier --check all clean

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) tool/tts Text-to-speech and transcription area/profiles Multi-profile isolation, HERMES_HOME scoping labels Jul 28, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for identifying the profile-routing gap. The pluginSocket premise remains valid on current main: apps/desktop/src/hermes.ts:324 still calls getConnection() without a profile, while apps/desktop/electron/main.ts:7917-7919 resolves an empty profile to the primary backend.

Problems

  • The TTS half has already been fixed on current main by e27997d63f771ce06319a2354924040189359ff6. Its implementation at apps/desktop/src/lib/voice-playback.ts:110-124 uses the existing getApiRequestProfile() seam and appends ?profile=. The latter is required because hermes_cli/web_server.py:4516-4520 reads the WebSocket query parameter to resolve the profile's TTS provider chain.
  • The proposed TTS test checks only getConnection; it cannot catch omission of that required URL parameter.

Suggested changes

  • Salvage the focused pluginSocket fix and its regression coverage.
  • Retain main's existing TTS implementation rather than the PR's stale voice-playback hunk.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/streaming Streaming responses: gateway delivery, provider wire labels Jul 30, 2026
pluginSocket (hermes.ts) is documented as "the live twin of pluginRest,
scoped the same way", but it calls window.hermesDesktop.getConnection()
with no profile argument, while pluginRest passes the active profile via
profileScoped(). getConnection's IPC handler (ensureBackend in
electron/main.ts) falls back to the primary profile whenever the profile
argument is empty, so an unscoped call always resolves to the primary
profile's backend regardless of which profile is actually active.

For a plugin used from a non-primary profile (e.g. kanban), this means REST
calls go to the correct pooled backend while the plugin's WebSocket silently
connects to the wrong one — a multi-profile user sees one profile's data
with another profile's live events.

Fix: pass the active profile through, mirroring the sibling call sites that
already do this correctly (use-gateway-request.ts, use-gateway-boot.ts,
store/gateway.ts, store/profile.ts).

voice-playback.ts's resolveSpeakStreamUrl had the same gap originally, but
main has since fixed it independently (via the getApiRequestProfile()
getter rather than direct store access) — dropped from this PR as
redundant, keeping only the still-open pluginSocket gap.
@pierrenode
pierrenode force-pushed the fix/desktop-plugin-socket-profile-routing branch from 0100cb9 to aab2cd9 Compare August 11, 2026 15:01
@pierrenode pierrenode changed the title fix(desktop): scope plugin socket and TTS streaming to the active profile fix(desktop): scope pluginSocket's connection to the active profile Aug 11, 2026
@pierrenode

Copy link
Copy Markdown
Contributor Author

Rebased onto current `upstream/main` and dropped the `voice-playback.ts`/TTS half — `main` fixed that gap independently in the meantime (via the `getApiRequestProfile()` getter rather than direct `$activeGatewayProfile` store access), so this PR's version of that hunk was fully redundant (zero net diff against current `main`). Removed both the stale fix and its now-superfluous test file, kept only the still-open `pluginSocket` gap in `hermes.ts`.

Retitled the PR to match the narrowed scope.

Mutation-verified: temporarily reverted `pluginSocket`'s `getConnection(_apiProfile)` back to the unscoped `getConnection()`, confirmed both new tests fail (asserting the mock was called with `[]` instead of `['work']`/`[null]`), restored, confirmed green.

Full desktop `src/` vitest suite (413 files, 3682 tests) passes, `tsc --noEmit` clean. Fresh competitor search: no overlap.

Squashed to a single commit on top of current `upstream/main`.

@teknium1

Copy link
Copy Markdown
Contributor

Merged via #87874 with your commit cherry-picked onto current main — authorship preserved. The voice-playback hunk was dropped (already fixed on main independently); the pluginSocket fix was adapted to resolve through the post-#87600 connection source of truth so it also follows registry-agent activations, with mutation-verified regression tests. Thank you!

@teknium1 teknium1 closed this Aug 16, 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 area/streaming Streaming responses: gateway delivery, provider wire comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data tool/tts Text-to-speech and transcription type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants