From fe576ba48e595451329bf17c5a6d0239f6735305 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Fri, 28 Aug 2026 06:03:35 -0700 Subject: [PATCH] fix(desktop): a 'This device' Capabilities pick reaches the local machine again under a remote registry primary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since 1e9a12a71 (v0.20.6), a remote/cloud/ssh registry PRIMARY makes globalRemoteActive() true, so the ambient v1 route — and with it every unpinned Capabilities read — resolves to the remote gateway. The only road back to this machine is an explicit connectionId:'local' pin, but capabilityScoped() deliberately DROPPED that pin (pre-#91564, absent id always meant the local pool), and profileScopeKey() collapsed 'local::' to the bare profile key. Consequences on any desktop whose registry primary is remote: - Capabilities -> MCP showed the REMOTE host's mcp_servers under every scope; locally configured (and connected) MCP servers vanished from the UI entirely. - Picking 'default - This device' in the scope selector was a silent no-op: the collapsed cache key equaled the current scope key, so changeScope() early-returned and the selector snapped back. Fix: capabilityScoped() forwards EVERY non-empty connection id, 'local' included — Electron's apiRequestRegistryConnectionId/ensureRegistryBackend already own 'local' pins (forced-local pooled child) and this is the documented contract there. profileScopeKey() namespaces every explicit pin ('local::') so a This-device pick and the ambient path never share a cache row. profiles.ts profileOwnerScoped, which hand-patched this exact hole for profile mutations, reduces to a named alias. Live A/B (Electron + CDP, remote registry primary, local-only servers in local config): v0.20.6 = local servers invisible, local pick no-op; fixed = 'This device' lists local-server-alpha/beta, remote scope unchanged. --- apps/desktop/src/api/client.ts | 36 +++++++++++-------- apps/desktop/src/api/profiles.ts | 9 +++-- .../src/hermes-capability-scope.test.ts | 14 +++++--- 3 files changed, 35 insertions(+), 24 deletions(-) diff --git a/apps/desktop/src/api/client.ts b/apps/desktop/src/api/client.ts index 8a925be2dd..fc98817b35 100644 --- a/apps/desktop/src/api/client.ts +++ b/apps/desktop/src/api/client.ts @@ -90,12 +90,11 @@ export function connectionScoped(): { connectionId?: string } { * routing may override the active source for an explicitly-owned resource. * * Helpers under `api/` go through here rather than calling the preload bridge - * directly, so the connection tag cannot be forgotten on a new one — with one - * exception. A capabilityScoped() helper must NOT: that scope says "the local - * pool" by omitting `connectionId` entirely, and an absent key cannot override - * the ambient tag spread underneath it, so a 'local' pin would silently route - * to whatever remote gateway happened to be active. Those helpers call the - * bridge directly and own their routing end to end. */ + * directly, so the connection tag cannot be forgotten on a new one. + * capabilityScoped() now emits an explicit `connectionId` for EVERY object + * pin — `'local'` included — so a pin always overrides the ambient tag spread + * underneath it. (It used to omit the key for 'local', which made the pin + * unable to beat the ambient tag; helpers then had to bypass this wrapper.) */ export function hermesApi(request: HermesApiRequest): Promise { return window.hermesDesktop.api({ ...connectionScoped(), ...request }) } @@ -113,10 +112,14 @@ export function hermesApi(request: HermesApiRequest): Promise { // connection tag (connectionScoped, same contract the cron helpers adopted // in #87882). Without the tag, a window activated onto a registered remote // gateway read the LOCAL pool's skills/tools/MCP — the wrong machine. -// - `{ connectionId, profile }` → explicit pin. `''`/`'local'` connection -// ids mean the local pool and deliberately DROP the ambient connection -// tag, so a local-profile pick made while a remote gateway is active still -// routes to the local machine. +// - `{ connectionId, profile }` → explicit pin. A non-empty connection id — +// `'local'` INCLUDED — is sent through so Electron's registry resolver +// owns the routing. Dropping the `'local'` pin (the pre-#91564 behavior, +// when an absent id always meant the local pool) silently re-routes a +// "This device" pick to the registry PRIMARY once that primary is a +// remote/cloud/ssh gateway: the v1 fallback route treats a remote registry +// primary as global-remote, so the explicit pin is the ONLY way back to +// this machine (see apiRequestRegistryConnectionId in Electron main). export type ProfileScope = undefined | null | string | { connectionId?: null | string; profile?: null | string } export function capabilityScoped(scope?: ProfileScope): { connectionId?: string; profile?: string } { @@ -126,22 +129,25 @@ export function capabilityScoped(scope?: ProfileScope): { connectionId?: string; return { ...(profile ? { profile } : {}), - ...(connectionId && connectionId !== 'local' ? { connectionId } : {}) + ...(connectionId ? { connectionId } : {}) } } return { ...profileScoped(scope), ...connectionScoped() } } -/** Stable cache-key for a capability scope: `profile` for the local/legacy - * path, `connectionId::profile` for an explicit remote pin. Mirrors - * normalizeProfileKey for plain strings so existing keys stay byte-identical. */ +/** Stable cache-key for a capability scope: `profile` for the ambient/legacy + * path, `connectionId::profile` for ANY explicit pin — `local` included. An + * explicit "This device" pick and the ambient path are no longer guaranteed + * to hit the same backend (a remote registry PRIMARY makes the ambient path + * remote), so sharing the bare-profile cache row between them painted one + * machine's config under the other's scope (AGENTS.md scope-in-key rule). */ export function profileScopeKey(scope?: ProfileScope): string { if (scope && typeof scope === 'object') { const profile = (scope.profile ?? '').trim() || 'default' const connectionId = (scope.connectionId ?? '').trim() - return connectionId && connectionId !== 'local' ? `${connectionId}::${profile}` : profile + return connectionId ? `${connectionId}::${profile}` : profile } return (scope ?? '').trim() || 'default' diff --git a/apps/desktop/src/api/profiles.ts b/apps/desktop/src/api/profiles.ts index 12b2f6cd20..cdade9099c 100644 --- a/apps/desktop/src/api/profiles.ts +++ b/apps/desktop/src/api/profiles.ts @@ -25,12 +25,11 @@ export function createProfile(body: ProfileCreatePayload): Promise<{ name: strin // Explicit (connection, profile) pin for a profile that lives on a gateway // other than the foreground one — the fleet profile rail edits a remote -// square's SOUL/name in place. Same contract as deleteProfile's scope. +// square's SOUL/name in place. capabilityScoped now forwards a `'local'` pin +// itself (it must, or a remote registry PRIMARY absorbs "This device" reads), +// so this is a plain alias kept for the call sites' self-documenting name. function profileOwnerScoped(scope?: ProfileScope): { connectionId?: string; profile?: string } { - return { - ...capabilityScoped(scope), - ...(scope && typeof scope === 'object' && scope.connectionId?.trim() === 'local' ? { connectionId: 'local' } : {}) - } + return capabilityScoped(scope) } export function renameProfile( diff --git a/apps/desktop/src/hermes-capability-scope.test.ts b/apps/desktop/src/hermes-capability-scope.test.ts index fadd349f2d..b50e8236a8 100644 --- a/apps/desktop/src/hermes-capability-scope.test.ts +++ b/apps/desktop/src/hermes-capability-scope.test.ts @@ -89,21 +89,27 @@ describe('capability helpers are connection-scoped', () => { } }) - it("a 'local' pin routes to the local pool even while a remote gateway is active", () => { + it("a 'local' pin carries an explicit connectionId even while a remote gateway is active", () => { setApiRequestProfile('research') setApiRequestConnection('gw-tailscale') void getSkills({ connectionId: 'local', profile: 'coder' }) + // The explicit pin must survive to Electron main: its registry resolver + // owns 'local' (forced-local pooled child). Omitting the key here let the + // ambient tag — or, worse, a remote registry PRIMARY on the v1 fallback + // route — absorb a "This device" pick (v0.20.6 regression, #91564 rung). expect(last().profile).toBe('coder') - expect(last()).not.toHaveProperty('connectionId') + expect(last().connectionId).toBe('local') }) - it('profileScopeKey keeps legacy keys byte-identical and namespaces remote pins', () => { + it('profileScopeKey keeps legacy keys byte-identical and namespaces every explicit pin', () => { expect(profileScopeKey()).toBe('default') expect(profileScopeKey(null)).toBe('default') expect(profileScopeKey('coder')).toBe('coder') - expect(profileScopeKey({ connectionId: 'local', profile: 'coder' })).toBe('coder') + // A 'local' pin and the ambient path can resolve to DIFFERENT backends + // when the registry primary is remote — they must not share a cache row. + expect(profileScopeKey({ connectionId: 'local', profile: 'coder' })).toBe('local::coder') expect(profileScopeKey({ connectionId: 'homelab', profile: 'coder' })).toBe('homelab::coder') expect(profileScopeKey({ connectionId: 'homelab' })).toBe('homelab::default') })