From 4bb7e4913fe9e7ec53a07e4d8448aecdd4c65b7b Mon Sep 17 00:00:00 2001 From: Christopher Strnad Date: Mon, 17 Aug 2026 19:59:58 -0500 Subject: [PATCH] fix(desktop): preserve cross-machine group routing --- apps/desktop/electron/hardening.test.ts | 13 ++++ apps/desktop/electron/main.ts | 25 ++----- .../desktop/src/plugins/hermes-bots/plugin.js | 70 +++++++++++++------ .../tests/create-group-chat.test.mjs | 6 ++ .../tests/multi-source-roster.test.mjs | 23 ++++++ .../hermes-bots/tests/roster-groups.test.mjs | 42 ++++++++++- 6 files changed, 136 insertions(+), 43 deletions(-) diff --git a/apps/desktop/electron/hardening.test.ts b/apps/desktop/electron/hardening.test.ts index 8f24af37c1..2c7bf3cc18 100644 --- a/apps/desktop/electron/hardening.test.ts +++ b/apps/desktop/electron/hardening.test.ts @@ -922,6 +922,19 @@ function readMain() { return fs.readFileSync(path.join(__dirname, 'main.ts'), 'utf8').replace(/\r\n/g, '\n') } +test('registry JSON helpers retain native OAuth bearer authentication', () => { + const source = readMain() + const postStart = source.indexOf('async function postJsonForBackend(') + const fetchStart = source.indexOf('async function fetchJsonForBackend(', postStart) + const helpers = source.slice(postStart, fetchStart) + + assert.notEqual(postStart, -1) + assert.notEqual(fetchStart, -1) + assert.match(helpers, /return fetchJsonForBackend\(descriptor, path, \{ \.\.\.opts, body: body \?\? \{\}, method: 'POST' \}\)/) + assert.match(helpers, /return fetchJsonForBackend\(descriptor, path, opts\)/) + assert.doesNotMatch(helpers, /fetchJsonViaOauthSession/) +}) + test('coerceDesktopConnectionConfig routes token persistence through resolvePersistedRemoteToken', () => { const source = readMain() const fnStart = source.indexOf('function coerceDesktopConnectionConfig(') diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index bd7b25b80b..24fe308914 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -12825,29 +12825,16 @@ ipcMain.handle('hermes:connections:update-all', async () => { return { ok: true, results } }) -// POST helper against a resolved backend descriptor. Token-auth descriptors -// use the session-token header; OAuth descriptors have token: null and -// authenticate via the OAuth partition's cookies (same split as the rest of -// the REST surface). +// Convenience wrappers around the bearer-aware descriptor request path. +// Native OAuth sessions are cookieless, so these must not bypass +// fetchJsonForBackend and fall straight through to the cookie partition. async function postJsonForBackend(descriptor, path, body, opts: any = {}) { - const url = `${descriptor.baseUrl}${path}` - - if (descriptor.authMode === 'oauth') { - return fetchJsonViaOauthSession(url, { ...opts, body: body ?? {}, method: 'POST' }) - } - - return fetchJson(url, descriptor.token, { ...opts, body: body ?? {}, method: 'POST' }) + return fetchJsonForBackend(descriptor, path, { ...opts, body: body ?? {}, method: 'POST' }) } -// GET twin of postJsonForBackend — same token/cookie auth split. +// GET twin of postJsonForBackend. async function getJsonForBackend(descriptor, path, opts: any = {}) { - const url = `${descriptor.baseUrl}${path}` - - if (descriptor.authMode === 'oauth') { - return fetchJsonViaOauthSession(url, requestOptionsWithHeaders(opts, descriptor.headers || {})) - } - - return fetchJson(url, descriptor.token, requestOptionsWithHeaders(opts, descriptor.headers || {})) + return fetchJsonForBackend(descriptor, path, opts) } // Any-method REST call against a resolved backend descriptor — the descriptor diff --git a/apps/desktop/src/plugins/hermes-bots/plugin.js b/apps/desktop/src/plugins/hermes-bots/plugin.js index 2d330462c0..66f8c6cd83 100644 --- a/apps/desktop/src/plugins/hermes-bots/plugin.js +++ b/apps/desktop/src/plugins/hermes-bots/plugin.js @@ -2402,7 +2402,28 @@ function mergeMultiSourceRoster(local, union, activeConnectionId, previous = []) // This-device shadow of default. const liveProvided = arguments.length >= 3 const liveId = String(activeConnectionId || '').trim() - const activeId = liveId || (liveProvided ? '' : String(union?.primaryConnectionId || '').trim()) + let activeId = liveId || (liveProvided ? '' : String(union?.primaryConnectionId || '').trim()) + + // Migrated remote-primary windows can still expose a legacy remote + // descriptor without connectionId. That produces a null live id even + // though profiles.list is answering from the registry primary. Infer the + // primary only when its inventory matches the rich rows and the local + // inventory does not. A genuinely local window has a matching local row, + // so it keeps the null-is-local behavior used after clicking This device. + if (!activeId && liveProvided) { + const primaryId = String(union?.primaryConnectionId || '').trim() + const richNames = new Set(localProfiles.map(profile => String(profile?.name || '').trim()).filter(Boolean)) + const localMatches = agents.some( + agent => agent?.connectionKind === 'local' && richNames.has(String(agent?.profile || '').trim()) + ) + const primaryMatches = agents.some( + agent => String(agent?.connectionId || '').trim() === primaryId && richNames.has(String(agent?.profile || '').trim()) + ) + + if (!localMatches && primaryId && primaryMatches) { + activeId = primaryId + } + } const activeByName = new Map() // Treat the rich list as one row per active-source profile. Clone every @@ -3178,6 +3199,21 @@ function groupChatMemberBots(group, roster, metaByName) { return [...local, ...remote] } +/** Persist source-qualified identities for every selected member. The active + * source's row may become remote after a connection switch, so retaining it + * here is what keeps the same room intact across machines. */ +function durableGroupChatMembers(bots) { + return (bots || []).map(bot => ({ + name: bot.name, + handle: bot.handle || bot.name, + connectionId: bot.connectionId, + connectionKind: bot.connectionKind, + connectionLabel: bot.connectionLabel, + remoteSource: true, + sourceScoped: true + })) +} + /** Existing group names, alphabetical — feeds the Move-to-group dialog. */ function knownGroups(metaByName) { const names = new Set() @@ -3412,8 +3448,8 @@ function updateGroupChat(group, mutate) { // with the pre-turn message baseline. Survives reloads so finished // work is still harvested after a window restart. stranded: room.stranded || {}, - // Cross-connection member descriptors — remote bots can't ride - // bot-meta, so the room record carries who they are. + // Source-qualified member descriptors keep the room whole when the + // active connection changes and today's local members become remote. members: Array.isArray(room.members) ? room.members : [] } } @@ -7361,27 +7397,15 @@ function CreateGroupChatDialog({ open, roster, onClose, onCreated }) { } } - // Members from OTHER connections can't ride bot-meta (it is scoped to - // the active gateway and name-keyed). Their descriptors live on the room - // record instead, so the roster merge can seat them every refresh. - const remoteMembers = selected - .filter(bot => bot.remoteSource) - .map(bot => ({ - name: bot.name, - handle: bot.handle || bot.name, - connectionId: bot.connectionId, - connectionKind: bot.connectionKind, - connectionLabel: bot.connectionLabel, - remoteSource: true, - sourceScoped: true - })) + // Persist every machine identity, including today's active source. That + // member becomes remote after a source switch and cannot rely on the new + // gateway's name-keyed bot metadata to remain seated in this room. + const roomMembers = durableGroupChatMembers(selected) - if (remoteMembers.length) { - updateGroupChat(groupName, room => { - room.members = remoteMembers - return room - }) - } + updateGroupChat(groupName, room => { + room.members = roomMembers + return room + }) host.notify({ kind: 'info', message: `“${groupName}” created with ${selected.length} bots` }) onClose() diff --git a/apps/desktop/src/plugins/hermes-bots/tests/create-group-chat.test.mjs b/apps/desktop/src/plugins/hermes-bots/tests/create-group-chat.test.mjs index e7d9c8b47c..56473f0c1c 100644 --- a/apps/desktop/src/plugins/hermes-bots/tests/create-group-chat.test.mjs +++ b/apps/desktop/src/plugins/hermes-bots/tests/create-group-chat.test.mjs @@ -33,3 +33,9 @@ test('source contract: create-group modal has search, checkboxes, name, create', test('source contract: group name falls back to member names, Discord-style', () => { assert.match(pluginSource, /selected\.map\(bot => displayName\(bot, botRosterMeta\(bot, allMeta\)\)\)\.join\(', '\)/) }) + +test('source contract: every selected machine is persisted in the durable room record', () => { + assert.match(pluginSource, /const roomMembers = durableGroupChatMembers\(selected\)/) + assert.match(pluginSource, /room\.members = roomMembers/) + assert.doesNotMatch(pluginSource, /const remoteMembers = selected/) +}) diff --git a/apps/desktop/src/plugins/hermes-bots/tests/multi-source-roster.test.mjs b/apps/desktop/src/plugins/hermes-bots/tests/multi-source-roster.test.mjs index a8ae80fb08..f097e95326 100644 --- a/apps/desktop/src/plugins/hermes-bots/tests/multi-source-roster.test.mjs +++ b/apps/desktop/src/plugins/hermes-bots/tests/multi-source-roster.test.mjs @@ -380,6 +380,29 @@ test('merge: live-null local window does not treat registry primary as active', ) }) +test('merge: legacy remote descriptor infers a matching remote primary when local inventory differs', () => { + const { __mergeMultiSourceRoster: merge } = runtime() + const local = { profiles: [{ name: 'default', last_session: { id: 'noah-chat' } }] } + const union = { + primaryConnectionId: 'noah', + agents: [ + { connectionId: 'local', connectionKind: 'local', connectionLabel: 'This device', profile: 'archie', handle: 'archie' }, + { connectionId: 'noah', connectionKind: 'remote', connectionLabel: 'Noah', profile: 'default', handle: 'default' } + ] + } + + // Legacy remote descriptors have mode:'remote' but no connectionId, so the + // host state is null. The matching primary row must annotate the rich row, + // while Archie remains a selectable other-source agent. + const out = merge(local, union, null) + + assert.equal(out.profiles.length, 2) + assert.equal(out.profiles.find(p => p.name === 'default').connectionId, 'noah') + assert.equal(out.profiles.find(p => p.name === 'default').remoteSource, undefined) + assert.equal(out.profiles.find(p => p.name === 'archie').connectionId, 'local') + assert.equal(out.profiles.find(p => p.name === 'archie').remoteSource, true) +}) + test('merge: previously seen remotes survive a connect-on-demand empty union', () => { const { __mergeMultiSourceRoster: merge } = runtime() const previous = [ diff --git a/apps/desktop/src/plugins/hermes-bots/tests/roster-groups.test.mjs b/apps/desktop/src/plugins/hermes-bots/tests/roster-groups.test.mjs index 125100836f..fdb70652ed 100644 --- a/apps/desktop/src/plugins/hermes-bots/tests/roster-groups.test.mjs +++ b/apps/desktop/src/plugins/hermes-bots/tests/roster-groups.test.mjs @@ -30,7 +30,7 @@ function load() { .replace(/^import .* from 'react\/jsx-runtime'\r?\n/m, '') .replace('export default {', 'globalThis.plugin = {') .concat( - '\nglobalThis.__groups = { groupChatNames, groupLastActivity, groupChatMemberBots, knownGroups, stripPreviewMarkdown, $groupChats };\n' + '\nglobalThis.__groups = { groupChatNames, groupLastActivity, groupChatMemberBots, durableGroupChatMembers, knownGroups, stripPreviewMarkdown, $groupChats };\n' ) vm.runInNewContext(source, context, { filename: 'plugin.js' }) return context.__groups @@ -83,6 +83,46 @@ test('groupChatMemberBots: seats local meta members plus stored remote descripto assert.equal(members[1], roster[2]) }) +test('durableGroupChatMembers: retains active and remote source identities', () => { + const { durableGroupChatMembers } = load() + const members = durableGroupChatMembers([ + { name: 'default', handle: 'noah', connectionId: 'noah', connectionKind: 'remote', connectionLabel: 'Noah' }, + { + name: 'default', + handle: 'maya', + connectionId: 'maya', + connectionKind: 'remote', + connectionLabel: 'Maya', + remoteSource: true + } + ]) + + assert.equal(members.length, 2) + assert.deepEqual( + JSON.parse(JSON.stringify(members)), + [ + { + name: 'default', + handle: 'noah', + connectionId: 'noah', + connectionKind: 'remote', + connectionLabel: 'Noah', + remoteSource: true, + sourceScoped: true + }, + { + name: 'default', + handle: 'maya', + connectionId: 'maya', + connectionKind: 'remote', + connectionLabel: 'Maya', + remoteSource: true, + sourceScoped: true + } + ] + ) +}) + test('knownGroups: unique, trimmed, alphabetical', () => { const { knownGroups } = load()