Skip to content

fix(desktop): route profile-scoped settings to the right profile in global remote mode - #46458

Closed
rod-nxtlevel wants to merge 1 commit into
NousResearch:mainfrom
nxtlevelsaas:fix/desktop-remote-profile-scoped-settings
Closed

fix(desktop): route profile-scoped settings to the right profile in global remote mode#46458
rod-nxtlevel wants to merge 1 commit into
NousResearch:mainfrom
nxtlevelsaas:fix/desktop-remote-profile-scoped-settings

Conversation

@rod-nxtlevel

Copy link
Copy Markdown
Contributor

Problem

In global remote mode (Settings → connect Desktop to a remote gateway URL), the Desktop talks to a single shared backend that serves every profile, disambiguated only by a ?profile= query param.

The renderer sends profile-scoped settings calls — /api/skills, /api/tools/toolsets, /api/config, /api/env, /api/model/*, … — with a bare path, carrying the profile only in request.profile. The hermes:api handler resolves a backend with ensureBackend(request.profile), but in global remote mode there is exactly one backend, so request.profile can't disambiguate anything and the bare path hits the backend's default profile.

Result: with a non-default profile selected in the rail, Skills & Tools (and config/env/model) read and write the remote's default profile:

  • Switching to a non-default profile on Skills & Tools shows the default profile's skills/tools (e.g. count stays on default's value and never updates).
  • Toggling a skill/toolset while a non-default profile is selected mutates the default profile instead.

Sessions already avoid this via interceptSessionRequestForRemote(), which rewrites session endpoints to carry ?profile= in global remote mode. The settings endpoints never got the equivalent.

Repro

  1. Remote Desktop (global remote mode) against a gateway that hosts two profiles with different skill states (e.g. default = 90 enabled, pm-james = 95 enabled).
  2. Open Skills & Tools under default → 90.
  3. Switch the rail to pm-james, click refresh.
  4. Expected: 95. Actual: stays 90 (reads default). Toggling a skill writes default, not pm-james.

(A refresh ruling out stale React state is what isolates this to the wire request, not the renderer.)

Fix

Add interceptProfileScopedSettingForRemote() in apps/desktop/electron/main.cjs, called from the hermes:api handler right after the existing session interceptor. In global remote mode it appends ?profile=<profile> and routes the call through the shared backend (fetchJsonForProfile/requestJsonForProfile) — mirroring interceptSessionRequestForRemote()'s global-remote branch.

The endpoint allowlist (PROFILE_SCOPED_REMOTE_PREFIXES) is kept identical to the web dashboard's PROFILE_SCOPED_PREFIXES in web/src/lib/api.ts (withManagementProfile), which already does exactly this rewrite for the same backend.

Why no backend change

Every targeted endpoint already accepts a profile query param — reads use _profile_scope(profile), writes use _profile_scope(body.profile or profile) (see hermes_cli/web_server.py). The web dashboard relies on this today; web_server.py even documents it ("the query param injected by the global dashboard profile switcher"). This change just makes Desktop do in global remote mode what the dashboard already does.

Safety / scope

  • Local mode: gated on globalRemoteActive() → returns undefined, so the existing per-profile-backend path is unchanged.
  • Default profile: early-returns (the bare path already resolves to default on the shared backend) → no behavior change.
  • Per-profile remote overrides: early-returns via profileHasRemoteOverride(), leaving those to ensureBackend(profile), whose bare path already targets the profile's own backend (matches the session interceptor).
  • Bodies untouched: the rewrite only touches the query string and forwards request.body verbatim — handlers read the query param (a typed body.profile still wins where present), so typed bodies like /api/config's {config} are never mutated.
  • Explicit ?profile= already present on the path → left as-is.

Test plan

  • Standalone logic check covering 12 cases — rewrite only on global-remote, non-default settings calls; no-op for local mode / default profile / non-allowlisted paths / per-profile overrides / an explicit ?profile= already present; existing query preserved with the correct ?/& separator; request body forwarded untouched. All pass.
  • node --check and the Electron --build-only package build both pass.
  • Mechanism traced end-to-end: Desktop appends ?profile=; the backend honors it via _profile_scope(body.profile or profile) — the same path the web dashboard already exercises against this backend in production.
  • Recommended manual check (needs a remote Desktop with two profiles of differing skill state): open Skills & Tools, switch the rail to the non-default profile — its counts should now show, and toggling a skill should persist to that profile's config.yaml (skills.disabled), leaving default untouched.

Prior art

Desktop analogue of the web-dashboard fix that introduced profile-scoped settings (web/src/lib/api.ts withManagementProfile / PROFILE_SCOPED_PREFIXES).

…al remote mode

In global remote mode the Desktop talks to a single shared backend that serves
every profile, disambiguated only by a ?profile= query param. The renderer sends
profile-scoped settings calls (skills, tools/toolsets, config, env, model, ...)
with a bare path and the profile only in request.profile. ensureBackend() can't
use request.profile to pick a backend when there's just one, so these calls
silently read and write the remote's DEFAULT profile regardless of which profile
the rail shows -- switching to a non-default profile on Skills & Tools shows the
default profile's skills, and toggling a skill mutates the default profile.

Sessions already handle this via interceptSessionRequestForRemote(); settings
never got the equivalent. Add interceptProfileScopedSettingForRemote(), which in
global remote mode appends ?profile= and routes through the shared backend for
the same profile-scoped endpoint families the web dashboard already rewrites
(web/src/lib/api.ts withManagementProfile / PROFILE_SCOPED_PREFIXES). Backends
already honor the query param (_profile_scope(body.profile or profile)), so no
backend change is needed.

- No-op in local mode and for the default profile (the bare path already
  resolves to default on the shared backend).
- Leaves per-profile-remote-override profiles to ensureBackend(profile), whose
  bare path already targets their own backend.
- Forwards request.body untouched (handlers read the query param), so typed
  bodies like /api/config's {config} are never mutated.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/tui Terminal UI (ui-tui/ + tui_gateway/) comp/gateway Gateway runner, session dispatch, delivery labels Jun 15, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

Fix for desktop profile-scoped settings routing in global remote mode: adds interceptProfileScopedSettingForRemote() to append the ?profile= query parameter for settings endpoints, mirroring what interceptSessionRequestForRemote does for session requests.

Looks Good

  • Well-documented comment explaining the security/routing rationale
  • Clean analogy to the dashboard withManagementProfile pattern
  • PROFILE_SCOPED_REMOTE_PREFIXES aligns with PROFILE_SCOPED_PREFIXES
  • No security concerns

Reviewed by Hermes Agent

@rod-nxtlevel

Copy link
Copy Markdown
Contributor Author

I searched for related profile-scoping issues/PRs and don’t see a direct duplicate for this exact Desktop settings path.

This looks like the settings-side gap left after the global-remote session fixes in #39921/#39993 and the later Desktop session propagation work in #44529/#45057. It also mirrors the dashboard-side profile-scoping pattern from #44792, where /api/messaging/platforms was added to PROFILE_SCOPED_PREFIXES.

So I’d frame this PR as: not a replacement for those earlier fixes, but the Desktop/Electron counterpart for profile-scoped management/settings endpoints (skills, toolsets, config, env, model, mcp, messaging/platforms) in app-global remote mode.

@alt-glitch alt-glitch added comp/desktop Electron desktop app (apps/desktop/*) area/config Config system, migrations, profiles and removed comp/gateway Gateway runner, session dispatch, delivery comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels Jun 26, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the careful analysis and the focused Desktop fix. This is an automated hermes-sweeper review; the requested behavior is already on current main.

  • Merged PR #47011, commit 0441b7f19feb9f1fdd9aac2af8cf022f1b50b174 (fix(desktop): route global remote profile REST calls), implemented global-remote ?profile= forwarding.
  • Current apps/desktop/electron/main.ts:7925 applies pathWithGlobalRemoteProfile() to every Electron API request before it is sent to the shared backend.
  • apps/desktop/electron/connection-config.ts:205 preserves explicit profile queries and skips local/per-profile override routes; apps/desktop/electron/connection-config.test.ts:127 covers those cases.
  • The current implementation is broader than this PR's settings-only allowlist, covering renderer profile-scoped REST calls through one routing seam.

@teknium1 teknium1 closed this Jul 14, 2026
@teknium1 teknium1 added sweeper:implemented-on-main Sweeper: behavior already present on current main sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles 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:implemented-on-main Sweeper: behavior already present on current main sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants