fix(desktop): split strict connection routing from passive roster lookup

botConnectionRoute() stays the strict, throwing dispatch path for real
routing (requestForBot, session creation). botRosterMeta() is passive
display code and previously reached that throw through a bare catch,
which would have swallowed any unrelated failure the same way. It now
calls a new non-throwing resolveBotConnectionRoute() and branches on a
typed resolved | owner_removed | not_scoped status instead.

Adds witnesses for the split: the typed statuses themselves, that
strict dispatch still fails closed on an orphaned row, and that an
unrelated failure while resolving meta for a live route still
propagates instead of being swallowed.
This commit is contained in:
chelsealong
2026-08-24 03:59:14 +00:00
committed by Teknium
parent 09529afdd2
commit ec013b76db
2 changed files with 73 additions and 24 deletions
+38 -23
View File
@@ -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
@@ -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()