Skip to content

fix(desktop): restore activeBotRoute dropped by merge 0404020f7b - #92811

Closed
ZhiPengH wants to merge 1 commit into
NousResearch:mainfrom
ZhiPengH:fix/bots-activebotroute-dropped
Closed

fix(desktop): restore activeBotRoute dropped by merge 0404020f7b#92811
ZhiPengH wants to merge 1 commit into
NousResearch:mainfrom
ZhiPengH:fix/bots-activebotroute-dropped

Conversation

@ZhiPengH

Copy link
Copy Markdown

Symptom

The Desktop Bots pane shows an eternal loading spinner (no agents, no error card) on current main. Every 5s poll fails silently.

Root cause

Merge 0404020 (PR #90006) kept both call sites of activeBotRoute()

  • useRoster()'s queryFn (line ~3906)
  • sweepBotProfileSessions() (line ~1636)

— but dropped the function definition introduced in 9b7ab9d. The bundled plugin therefore throws ReferenceError: activeBotRoute is not defined on the roster's first fetch. Because useRoster sets retry: true (there for slow remote gateways), React Query retries forever and the pane never leaves the isLoading && !roster.length branch.

The sweep call site fails silently inside its try/catch, so bot-profile session cleanup (hiding 'Bot Chat' / 'Agent Inbox' plumbing rows) had also stopped running — a second, quieter casualty of the same drop.

Fix

Restore the definition verbatim from 9b7ab9d. One file, +25 lines, no behavior change beyond un-breaking the two call sites.

Verification

  • node --test src/plugins/hermes-bots/tests/*.test.mjs406/406 pass
  • Rebuilt bundle carries the definition again (No route for active bot body present, 2 call sites resolve)
  • Locally packaged the app (npm run build && npm run pack) and confirmed the Bots pane loads the roster on relaunch — was the eternal spinner before, on a machine with 3 profiles (default + 2 named)

Found while debugging a live install today; symptom appeared exactly when the auto-updater landed the merge range containing 0404020.

Merge 0404020 kept both call sites of activeBotRoute() (useRoster's
queryFn and sweepBotProfileSessions) but dropped the function definition
introduced in 9b7ab9d. The bundled plugin therefore throws
ReferenceError: activeBotRoute is not defined on the roster's first fetch;
because useRoster sets retry: true, React Query retries forever and the
Bots pane paints an eternal loading spinner.

Restore the definition verbatim from 9b7ab9d. The sweep call site also
recovers: it failed silently inside its try/catch, so bot-profile session
cleanup had stopped running.

Verified: 406/406 hermes-bots plugin tests pass; built bundle carries the
definition again and the Bots pane loads on a locally packaged app.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) labels Aug 23, 2026
@TheAngryPit

Copy link
Copy Markdown
Contributor

Independent reproduction and live validation on macOS ARM64:

  • origin/main at 503d863fcd still has both activeBotRoute() call sites and no definition.

  • A packaged Desktop built from the affected merge state rendered an empty BOTS pane while profiles.list itself returned three profiles.

  • Executing the real useRoster().queryFn in a focused harness failed with the exact runtime error:

    ReferenceError: activeBotRoute is not defined
    
  • Restoring the original route resolver made the same harness return default plus two named profiles, and the rebuilt live Desktop immediately rendered all three Bot rows after restart.

Local verification after the restoration:

  • focused roster query regression: 1/1
  • Bot Mode Node suite: 407/407
  • SDK profile routing: 32/32
  • Electron connection registry: 66/66
  • full Desktop TypeScript typecheck
  • production build + separately packaged/signed app
  • live restart/reopen canary: all three rows rendered

One coverage gap is worth closing here: the existing 406-test Bot Mode suite stayed green while the symbol was missing. A regression should invoke useRoster(), capture the registered query options, and execute queryFn rather than only testing routing helpers/source shape. That test goes red before this patch with the ReferenceError above and green after it, so another merge cannot retain the call sites while dropping the resolver again.

This independently confirms the root cause and the minimal restoration in this PR. The broader state-db lock/retry work in #92793 appears adjacent rather than required to reproduce this specific empty-roster regression.

@jonpol01

Copy link
Copy Markdown
Contributor

Independently hit this on a two-machine install and excavated the same answer before spotting your PR (#92835, now closed as a duplicate of this) — so consider this a verification report: restoring the function exactly as 9b7ab9d wrote it takes the Bots pane from a permanently spinning roster (8+ minutes against a healthy gateway; the ReferenceError is swallowed by useRoster's retry: true) to populating in ~3 seconds, local and remote-connection rows both.

If useful: #92835 carries a 4-case behavior test for the restored function whose extraction step fails whenever the definition is lost while the call sites survive — the exact merge-casualty mode that shipped this, and one nothing else gates (plugin.js sits outside eslint's *.{ts,tsx} file set and tsc, and the plugin tests evaluate slices). Feel free to lift it here; happy to PR it as a follow-up otherwise.

@TheAngryPit

Copy link
Copy Markdown
Contributor

Follow-up after validating the latest fetched origin/main (bdf10471b5dff0d67d172065f7dbf9f4c47c4c1b): the original missing-helper regression is no longer present, but the resolution on main differs from this PR.

Commit 2ec229ec5adfe7372d5a858802af406f82b5beb0 (fixup: roster query keeps SDK ambient owner route; alias index refresh preserved) moved both active roster calls to the SDK ambient-owner path:

requestForBot(activeBot, 'profiles.list', {})

Current source-contract check on origin/main:

activeBotRoute references: 0
ambient-owner profiles.list calls: 2

Fresh detached-worktree canary on bdf10471b5:

node --test \
  apps/desktop/src/plugins/hermes-bots/tests/multi-source-roster.test.mjs \
  apps/desktop/src/plugins/hermes-bots/tests/mention-roster-cache-key.test.mjs \
  apps/desktop/src/plugins/hermes-bots/tests/profile-prewarm.test.mjs

32 passed, 0 failed

The suite now includes the source contract active roster queries use the SDK ambient owner route.

So the activeBotRoute restoration proposed here should not be reapplied to current main; this PR appears superseded by the ambient-owner implementation already there. The original coverage observation remains useful: the live roster query contract—not only extracted helpers—needs to stay pinned so a future merge cannot leave unresolved call sites behind.

@teknium1

Copy link
Copy Markdown
Contributor

Closing as implemented-on-main — the missing activeBotRoute was resolved by 2ec229e ("roster query keeps SDK ambient owner route"), which removed the stranded call sites entirely in favor of SDK ambient-owner routing rather than restoring the helper. Your forensics were the earliest correct diagnosis of the merge casualty (0404020 dropping the definition while keeping the call sites) — thanks @ZhiPengH. The restore approach was superseded rather than wrong; main just went the other direction on where routing lives.

@teknium1 teknium1 closed this Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants