Skip to content

fix(desktop): open active bot sessions from Active now - #90772

Closed
okazakijiso-creator wants to merge 1 commit into
NousResearch:mainfrom
okazakijiso-creator:fix/bots-active-now-session
Closed

okazakijiso-creator wants to merge 1 commit into
NousResearch:mainfrom
okazakijiso-creator:fix/bots-active-now-session

Conversation

@okazakijiso-creator

Copy link
Copy Markdown

What does this PR do?

The Desktop Bots pane marks a bot as Active now from recent human-session, busy-turn, or worker activity, but the chip's click handler still routes through openBotCanonicalChat. When recent human activity belongs to a different session, clicking the chip can open or create the canonical Bot Chat/Home draft instead of the session that caused the activity indicator.

This change opens the exact recent human-facing activity session first, using its resolved lineage tip when available. Worker-only and busy-only activity do not reuse stale human history; they keep the canonical-chat path. The canonical path is also retained when the stored activity ID is definitively gone. Hydration/time-out failures still surface instead of silently navigating to the wrong chat. Ordinary roster-row clicks remain unchanged and continue to open the canonical Bot Chat.

Related Issue

No linked issue. Related prior PR: #90167. That PR was withdrawn after assuming the canonical opener was equivalent, but current main at 645f85c2fdaeacf0bcd6c91648320a8040e2a1d7 still sends the Active now chip through the canonical path.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • apps/desktop/src/plugins/hermes-bots/plugin.js
    • Open the recent human-facing session returned by the same activity-window logic before the canonical Bot Chat path.
    • Avoid redirecting worker-only or busy-only activity to stale human history.
    • Prefer resolved_id over the durable root ID after lineage rotation.
    • Fall back only when no activity session exists or resume reports Session not found.
    • Preserve transient hydration/time-out errors and the existing click-generation guard.
    • Update the chip title to describe the active-chat target.
  • apps/desktop/src/plugins/hermes-bots/tests/active-now-session.test.mjs
    • Cover direct activity-session navigation, lineage tips, missing-session fallback, and transient-error behavior.
  • apps/desktop/src/plugins/hermes-bots/tests/active-now-strip.test.mjs
    • Lock the Active now handler ordering before the canonical fallback.

How to Test

  1. In Desktop → Bots, make a bot active in a human-facing session that is not its canonical Bot Chat.
  2. Click that bot in the Active now strip.
  3. Confirm the active session opens. Confirm ordinary roster-row clicks still open the canonical Bot Chat.
  4. Run from apps/desktop:
npm run check:test:plugins
npm run typecheck
npm run lint -- --quiet

Verified results:

  • Hermes plugin tests: 333 passed
  • TypeScript checks: passed
  • ESLint: passed
  • New test file Prettier check: passed
  • git diff --check: passed

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • Python pytest is N/A for this Desktop JavaScript plugin-only change; applicable Desktop checks are listed above
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.5.1 (arm64)

Documentation & Housekeeping

  • Documentation update: N/A — no user-facing API or workflow changed
  • cli-config.yaml.example: N/A — no configuration keys changed
  • CONTRIBUTING.md / AGENTS.md: N/A — no architecture or contributor workflow changed
  • Cross-platform impact considered — this is platform-neutral Desktop state/navigation logic
  • Tool descriptions/schemas: N/A — no tool behavior changed

Screenshots / Logs

Not applicable; this changes navigation behavior rather than visual styling, and the behavior is covered by focused plugin tests.

@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 20, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

Review of "fix(desktop): open active bot sessions from Active now". Right fix for a real routing bug: chips now open the human-facing session that produced the activity signal (resolved lineage tip honored), fall back to the canonical chat when there is no live human session or it is definitively gone, and deliberately surface transient hydration failures instead of dumping the user in Home. Error triage in openActiveBotSession and the pure activeHumanSession window helper are both cleanly tested. Suggestions:

  1. apps/desktop/src/plugins/hermes-bots/plugin.js:3397 (error-string coupling) — the fallback decision hinges on /session not found/i matching a human-written message from host.openSession; if upstream ever rewords it ("unknown session", "no such stored session"), stale-chip clicks start THROWING at users instead of falling back to the canonical chat — prefer matching a structured error code/cause if the SDK offers one, or broaden the matcher and add a shared constant so plugin.js and tests can't drift.

  2. apps/desktop/src/plugins/hermes-bots/tests/active-now-session.test.mjs:8 (slice-extraction fragility) — loading the function by indexOf('async function openActiveBotSession(') plus the next-section comment anchor means renaming the section header silently empties the extracted slice (the notEqual assertions catch it, good), but three test files now each carry near-identical vm-slice loaders — consider a tiny shared test helper that loads named functions once, so boundary renames get fixed in one place.

  3. apps/desktop/src/plugins/hermes-bots/plugin.js:9940 (generation race nuance) — the staleness check runs only after the FIRST attempt resolves: if the roster regenerates while openActiveBotSession is awaiting hydration, a successful open still returns through the stale path (fine), but a null result correctly falls through to canonical — worth one comment noting the intent, since future readers often "fix" this ordering and accidentally double-open.

No blocking issues found.

@teknium1

Copy link
Copy Markdown
Collaborator

Closing as wrong-direction after the Bot Mode identity change in #92129 — with credit: the inconsistency you observed (chip advertises one session, click opens another) was real on the code you built against.

Two things changed underneath this PR:

  1. Canonical identity is now (profile, session titled "Bot Chat"), resolved by exact-title registry lookup on every open. The pin machinery this diff extends (pinnedChat, the 3-arg openBotCanonicalChat, the Home-draft fallback) no longer exists.

  2. Activity signals now key off canonical_session, and most bot activity (DMs, bot-to-bot messages, cron deliveries) lands IN the Bot Chat — so the chip's activity session usually IS the canonical chat, and the current path already opens the session that lit the chip.

The remaining divergence — activity in a visible scratch session steering the chip away from the forever-chat — is the exact preference the project explicitly rejected (#91791#92042, now codified in AGENTS.md: recency and "where the user left off" are never the bot target's authority). Scratch sessions stay reachable in the Sessions sidebar by design; every bot entry point lands in the forever-chat.

Appreciate the careful test coverage and the honest writeup of #90167's history — the observation was right, the architecture just moved out from under the fix.

@teknium1 teknium1 closed this Aug 22, 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/*) P3 Low — cosmetic, nice to have sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants