Skip to content

fix(desktop): refresh active sessions from external updates - #37766

Closed
joelbrilliant wants to merge 1 commit into
NousResearch:mainfrom
joelbrilliant:fix/desktop-inbound-refresh
Closed

fix(desktop): refresh active sessions from external updates#37766
joelbrilliant wants to merge 1 commit into
NousResearch:mainfrom
joelbrilliant:fix/desktop-inbound-refresh

Conversation

@joelbrilliant

Copy link
Copy Markdown
Contributor

Summary

  • Adds a guarded Hermes Desktop read-repair loop for the selected stored session.
  • Refreshes active session messages when another client/gateway, such as Telegram on phone, writes to the same session while Desktop is open.
  • Skips refresh while the active run is busy or awaiting a response so polling cannot overwrite local optimistic/in-flight messages.
  • Refreshes the sidebar session list only when the persisted session revision actually changes.

Why

Hermes Desktop remote mode can show gateway-origin messages after a manual renderer refresh, but the active session view does not currently notice those persisted changes while the app stays open. This removes the need for Cmd+R in the common “chat on phone, watch on Desktop” workflow.

Test plan

  • CI=1 npx vitest run --environment jsdom src/app/session/hooks/use-external-session-refresh.test.ts
  • npx eslint src/app/session/hooks/use-external-session-refresh.ts src/app/session/hooks/use-external-session-refresh.test.ts src/app/desktop-controller.tsx
  • npm run type-check
  • npm run build

Review notes

A first review found a race where a poll could start while idle, then overwrite an optimistic/in-flight prompt if the session became busy before the persisted read returned. The final implementation rechecks runtime session state after the awaited read and includes a regression test for that race.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have labels Jun 3, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

Overview

Refreshes active sessions in the Desktop UI when external updates (e.g. from gateway) modify session state. Fixes stale session list in the sidebar.

✅ Looks Good

  • 415 additions, 0 deletions
  • Addresses real UX issue where session list goes stale
  • Reactively updates the UI when sessions change externally
  • No security concerns
  • Clean implementation with proper event handling

Reviewed by Hermes Agent

@manualzuru

Copy link
Copy Markdown

This active-session refresh approach looks like the right place to consolidate the external-update fix, so I would avoid opening a duplicate PR for the same behavior.

Two edge cases that may be worth carrying here:

  • Profile-aware message reads: when the selected stored session came from a non-active/all-profile row, the refresh should pass that stored session's profile to getSessionMessages. Otherwise the poll can query the wrong profile/backend and miss the external update.
  • Focus/visibility refresh: in addition to interval polling, triggering a check when the Desktop window regains focus or visibility makes the active transcript catch up immediately when the user returns to the app.

Those seem like small additions that would make this PR cover the main external-update cases without needing a separate overlapping PR.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused Desktop refresh work. This is now covered on current main, so this PR is redundant.

  • Automated hermes-sweeper review verified the merged equivalent in PR fix(desktop): poll messaging sessions so platform traffic appears live #57636 (52d0d671e): apps/desktop/src/app/desktop-controller.tsx:547 refreshes the selected messaging transcript only while it is not busy, reads with the stored session's profile, and replaces state only when the persisted signature changed.
  • apps/desktop/src/app/desktop-controller.tsx:896 polls the messaging list, while :926 performs the active-transcript refresh immediately, every five seconds, and on visibilitychange.
  • This also incorporates the profile-aware and focus/visibility cases raised in the prior discussion.

Closing as implemented on main.

@teknium1 teknium1 closed this Jul 14, 2026
@teknium1 teknium1 added sweeper:implemented-on-main Sweeper: behavior already present on current main sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:implemented-on-main Sweeper: behavior already present on current main sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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.

5 participants