diff --git a/apps/desktop/src/plugins/hermes-bots/plugin.js b/apps/desktop/src/plugins/hermes-bots/plugin.js index 28ceef638b..e1b1536a93 100644 --- a/apps/desktop/src/plugins/hermes-bots/plugin.js +++ b/apps/desktop/src/plugins/hermes-bots/plugin.js @@ -4983,11 +4983,13 @@ function botSourceStatus(bot) { // active-gateway door. Feature-detected: older desktops without // requestProfile simply have no remote routes (callers fall back / disable). -/** Immutable owner descriptor for every source-scoped row. The active - * gateway is presentation state and is never consulted here. */ -function botConnectionRoute(bot) { +/** Non-throwing resolver behind botConnectionRoute(). Returns a typed status + * instead of throwing, so passive callers (display/meta lookups) can branch + * on `resolved | owner_removed | not_scoped` rather than catching whatever + * exception the strict wrapper below happens to throw. */ +function resolveBotConnectionRoute(bot) { if (!bot?.sourceScoped && !bot?.remoteSource) { - return null + return { status: 'not_scoped', route: null } } const candidate = bot.route || { @@ -5001,15 +5003,34 @@ function botConnectionRoute(bot) { const targetProfile = String(candidate?.targetProfile || profile).trim() || profile if (!connectionId) { - throw new Error(`Bot ${profile} has no connection owner`) + return { status: 'owner_removed', route: null, profile } } - return Object.freeze({ - connectionId, - mode: candidate.mode === 'local' || connectionId === 'local' ? 'local' : 'remote', - profile, - targetProfile - }) + return { + status: 'resolved', + route: Object.freeze({ + connectionId, + mode: candidate.mode === 'local' || connectionId === 'local' ? 'local' : 'remote', + profile, + targetProfile + }) + } +} + +/** Immutable owner descriptor for every source-scoped row. The active + * gateway is presentation state and is never consulted here. Strict: throws + * when the owning connection is gone -- correct for real dispatch + * (requestForBot, session creation, etc.), covered by + * remote-routing-races.test.mjs. Passive lookups (rendering, meta) must call + * resolveBotConnectionRoute() directly instead of catching this throw. */ +function botConnectionRoute(bot) { + const resolved = resolveBotConnectionRoute(bot) + + if (resolved.status === 'owner_removed') { + throw new Error(`Bot ${resolved.profile} has no connection owner`) + } + + return resolved.route } const BOTS_HOME_OWNER_KEY = 'bots:home' @@ -5215,18 +5236,12 @@ function aliasIdentityFor(bot) { // aliasRouteIndex above — which is connection-exact, never name-based. function botRosterMeta(bot, metaByName) { if (bot?.sourceScoped || bot?.remoteSource) { - // A row orphaned by a deleted connection (connectionId gone, e.g. from a - // stale persisted group roster) has no route to resolve. botConnectionRoute - // fails closed there for routing dispatch — correct for a network call, - // but this is a passive meta lookup, and the whole component tree above - // display-only code must not crash rendering an unroutable member. - let route = null - - try { - route = botConnectionRoute(bot) - } catch { - route = null - } + // Passive meta lookup: branch on the typed status instead of catching + // botConnectionRoute's throw, so an owner_removed row (e.g. a stale + // persisted group roster after its connection was deleted) reads as "no + // route" without masking an unrelated failure under the same catch. + const resolved = resolveBotConnectionRoute(bot) + const route = resolved.status === 'resolved' ? resolved.route : null const direct = route ? metaByName?.[botRouteKey(route)] : null 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 ead2641cd3..3065aff6ea 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 @@ -32,7 +32,7 @@ function runtime() { .replace(/^import .* from 'react\/jsx-runtime'\r?\n/m, '') .replace('export default {', 'globalThis.plugin = {') .concat( - '\nglobalThis.__mergeMultiSourceRoster = mergeMultiSourceRoster;\nglobalThis.__botHandle = botHandle;\nglobalThis.__botRosterKey = botRosterKey;\nglobalThis.__botRosterMeta = botRosterMeta;\nglobalThis.__displayName = displayName;\nglobalThis.__filterBots = filterBots;\nglobalThis.__resolveRosterMentions = resolveRosterMentions;' + '\nglobalThis.__mergeMultiSourceRoster = mergeMultiSourceRoster;\nglobalThis.__botHandle = botHandle;\nglobalThis.__botRosterKey = botRosterKey;\nglobalThis.__botRosterMeta = botRosterMeta;\nglobalThis.__displayName = displayName;\nglobalThis.__filterBots = filterBots;\nglobalThis.__resolveRosterMentions = resolveRosterMentions;\nglobalThis.__botConnectionRoute = botConnectionRoute;\nglobalThis.__resolveBotConnectionRoute = resolveBotConnectionRoute;' ) vm.runInNewContext(code, context) return context @@ -266,6 +266,40 @@ test('botRosterMeta: a group roster row orphaned by a deleted connection does no assert.equal(metaFor(orphaned, {}), null) }) +test('resolveBotConnectionRoute: typed status for resolved / owner_removed / not_scoped, and strict botConnectionRoute still fails closed', () => { + const { __resolveBotConnectionRoute: resolve, __botConnectionRoute: strictRoute } = runtime() + const orphaned = { name: 'halakukhan', connectionId: null, remoteSource: true } + const owned = { name: 'halakukhan', connectionId: 'conn-1', remoteSource: true } + const local = { name: 'default' } + + // Passive resolver: typed status, never throws. + assert.equal(resolve(orphaned).status, 'owner_removed') + assert.equal(resolve(owned).status, 'resolved') + assert.equal(resolve(owned).route.connectionId, 'conn-1') + assert.equal(resolve(local).status, 'not_scoped') + + // Strict wrapper used by real dispatch (requestForBot, session creation) + // must still fail closed on the same orphaned row -- the split only moves + // the *passive* lookup off this throw, it does not remove it. + assert.throws(() => strictRoute(orphaned), /has no connection owner/) + assert.equal(strictRoute(owned).connectionId, 'conn-1') +}) + +test('botRosterMeta: an unrelated failure while resolving meta for a live route still propagates', () => { + const { __botRosterMeta: metaFor } = runtime() + const owned = { name: 'halakukhan', connectionId: 'conn-1', remoteSource: true } + // A metaByName lookup that throws for reasons that have nothing to do with + // connection ownership must not be caught by botRosterMeta -- only the + // owner_removed status is treated as "no meta for this row". + const explodingMetaByName = new Proxy({}, { + get() { + throw new Error('unrelated invariant failure') + } + }) + + assert.throws(() => metaFor(owned, explodingMetaByName), /unrelated invariant failure/) +}) + test('botHandle: precomputed multi-source handle wins; default stays hermes', () => { const { __botHandle: botHandle } = runtime()