fix(bot-mode): plugin listener leaks, jsx key prop, OAuth poll overwrite

Three bugs in apps/desktop/src/plugins/hermes-bots/plugin.js:

1. register() subscribed host.state.profile / host.state.gateway listeners
   without capturing the unbind functions, so a plugin disable -> re-enable
   cycle stacked a duplicate listener per cycle that kept firing until app
   quit (same survives-disable class as the face clock before its
   onDispose hook). The unbinds are now captured and released via
   ctx.onDispose.

2. CreateRoutineDialog passed `key: createTarget` INSIDE the jsx() props
   object. The react/jsx-runtime silently ignores a `key` prop there (key
   is the third jsx() argument), so switching the routine owner never
   remounted the dialog and it kept stale per-bot form state. Moved the
   key to the third argument; the source-shape test now pins the correct
   shape and rejects the prop form.

3. beginOAuth() overwrote pollRef.current without clearing an existing
   interval, so a retry / double-click while a poll was live orphaned a
   2s poller that ran until unmount and could flip the phase from a stale
   OAuth session. It now clears any live poll before starting a new one.
This commit is contained in:
Teknium
2026-08-17 15:58:13 -07:00
parent aa2622128f
commit 399fdeae8b
2 changed files with 30 additions and 5 deletions
+26 -4
View File
@@ -1516,6 +1516,13 @@ function McpSetupButton({ profile, entry, onDone, ensureProfile }) {
}
const beginOAuth = async () => {
// A second click (retry, impatient double-click) must not orphan the
// previous poll interval — an overwritten pollRef leaks a 2s poller that
// runs until unmount and can flip phase from a stale OAuth session.
if (pollRef.current) {
clearInterval(pollRef.current)
pollRef.current = null
}
setPhase('busy')
setMessage('')
const profile = await resolveProfile()
@@ -6647,14 +6654,15 @@ function RoutinesPane() {
})
}),
jsx(CreateRoutineDialog, {
key: createTarget,
bot: createTarget,
open: createOpen,
onClose: () => {
setCreateOpen(false)
setCreateOwner(null)
}
})
// key is the jsx() THIRD argument — as a prop it is silently ignored
// and the dialog kept stale per-bot form state when the target changed.
}, createTarget)
]
})
}
@@ -7910,12 +7918,26 @@ export default {
}
// Routines follow the chat you're in: track the live gateway profile.
host.state.profile.listen(profile => {
// Capture the unbinds: without them a disable → re-enable cycle stacks a
// duplicate listener per cycle (same survives-disable class as the face
// clock before its onDispose hook — these kept firing until app restart).
const unbindProfileListener = host.state.profile.listen(profile => {
if (profile && typeof profile === 'string') {
$selectedBot.set(profile)
}
})
host.state.gateway.listen(handleSessionsGatewayTransition)
const unbindGatewayListener = host.state.gateway.listen(handleSessionsGatewayTransition)
if (typeof ctx.onDispose === 'function') {
ctx.onDispose(() => {
if (typeof unbindProfileListener === 'function') {
unbindProfileListener()
}
if (typeof unbindGatewayListener === 'function') {
unbindGatewayListener()
}
})
}
// Reconciliation sweep: hide every Bot Mode session we know about, on
// load and again on each reconnect (a swap can land on a gateway whose
@@ -79,7 +79,10 @@ test('source contract: create mutations and dialog state retain one owner', () =
assert.match(pluginSource, /const \[createOwner, setCreateOwner\] = useState\(null\)/)
assert.match(pluginSource, /const openCreate = \(\) => \{[\s\S]*setCreateOwner\(bot\)[\s\S]*setCreateOpen\(true\)/)
assert.match(pluginSource, /const createTarget = routineCreateTarget\(createOwner, bot\)/)
assert.match(pluginSource, /key: createTarget/)
// key must be the jsx() THIRD argument (a `key:` prop is silently ignored
// by the react/jsx-runtime and the dialog would keep stale per-bot state).
assert.match(pluginSource, /jsx\(CreateRoutineDialog, \{[\s\S]*?\}, createTarget\)/)
assert.doesNotMatch(pluginSource, /key: createTarget/)
assert.match(pluginSource, /bot: createTarget/)
assert.doesNotMatch(pluginSource, /setCreateOwner\(owner =>/)
assert.doesNotMatch(pluginSource, /onChanged: \(\) => void refetch\(\)/)