Skip to content

fix: scope desktop messaging sessions to active profile - #58983

Closed
Thankyouadd wants to merge 1 commit into
NousResearch:mainfrom
Thankyouadd:fix/desktop-profile-scoped-messaging-sessions
Closed

fix: scope desktop messaging sessions to active profile#58983
Thankyouadd wants to merge 1 commit into
NousResearch:mainfrom
Thankyouadd:fix/desktop-profile-scoped-messaging-sessions

Conversation

@Thankyouadd

Copy link
Copy Markdown

What does this PR do?

Fixes Desktop sidebar profile leakage for external messaging sessions.

Previously, the Desktop sidebar's messaging slice always queried /api/profiles/sessions with profile=all. In multi-profile setups, this could surface Telegram sessions from a dedicated telegram profile while the user was viewing the default profile.

This PR scopes messaging session fetches to the active sidebar profile, while preserving the explicit all-profiles view behavior. It also keeps messaging sections renderable when the sidebar is in Projects/Worktree grouped mode, so a profile that only has Telegram sessions still shows its Telegram section instead of an empty session state.

Related Issue

N/A no issue filed.

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/app/session/hooks/use-session-list-actions.ts
    • Scope refreshMessagingSessions() to the current profileScope instead of always using profile=all.
    • Scope loadMoreMessagingForPlatform() to the current profileScope as well.
  • apps/desktop/src/app/chat/sidebar/index.tsx
    • Count messaging and cron groups when deciding whether the sidebar should show session sections instead of the blank state.
    • Allow messaging sections to render even when Projects/Worktree grouped mode is active.
  • apps/desktop/src/app/session/hooks/use-session-list-actions.test.tsx
    • Add regression tests for profile-scoped messaging fetches and the all-profiles exception.
  • tests/hermes_cli/test_web_server.py
    • Add backend regression coverage for /api/profiles/sessions?profile=<name> filtering so sibling profile rows do not leak.

How to Test

  1. Run the focused Desktop UI tests:
    cd apps/desktop
    npm run test:ui -- src/app/session/hooks/use-session-list-actions.test.tsx src/hermes.test.ts
  2. Run Desktop typecheck and build:
    cd apps/desktop
    npm run typecheck
    npm run build
  3. Run the focused backend regression test:
    python3 -m pytest tests/hermes_cli/test_web_server.py::TestWebServerEndpoints::test_profiles_sessions_filters_to_requested_profile -q
  4. Manual verification in Desktop with separate default and telegram profiles:
    • default profile: Telegram section/sessions are not shown.
    • telegram profile: Telegram section is shown with Telegram sessions.

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)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: WSL2 Ubuntu on Windows

Documentation & Housekeeping

  • N/A
  • N/A
  • N/A
  • N/A
  • N/A

Screenshots / Logs

Focused verification completed locally:

npm run test:ui -- src/app/session/hooks/use-session-list-actions.test.tsx src/hermes.test.ts
— 2 test files passed, 10 tests passed

npm run typecheck
— tsc -p . --noEmit

python3 -m pytest tests/hermes_cli/test_web_server.py::TestWebServerEndpoints::test_profiles_sessions_filters_to_requested_profile -q
— 1 passed

npm run build
— built
— assert-dist-built

Manual Desktop check:

  • default profile: no Telegram section/sessions shown.
  • telegram profile: Telegram section shown with Telegram sessions.

@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have duplicate This issue or pull request already exists labels Jul 5, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #56635 — both make the identical core change in use-session-list-actions.ts (sessionProfile = profileScope === ALL_PROFILES ? 'all' : profileScope in refreshMessagingSessions + loadMoreMessagingForPlatform, plus profileScope in the deps). #56635 is the earlier open PR (2026-07-01) and is canonical; this one adds only a minor sidebar render tweak. Related: #42934 (a competing filter-based approach to the same messaging-sidebar profile-scope gap). A maintainer should pick one.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for tracing both the initial messaging slice and the per-platform pager. The current main still passes "all" in both paths at apps/desktop/src/app/session/hooks/use-session-list-actions.ts:102 and :126, while the backend already honors a concrete profile at hermes_cli/web_server.py:4132-4134, so the reported leak is real.

Problems

  • The scoped requests can still leak through an out-of-order response. refreshSessions uses refreshSessionsRequestRef before publishing results (use-session-list-actions.ts:156-188), but messaging writes directly after await at :110-113 and :132-138. Toggling All Profiles only updates $showAllProfiles (apps/desktop/src/store/profile.ts:329-335), so an earlier all-profile request may resolve after the new scoped request and overwrite the shared messaging rows.

Suggested changes

  • Add the same request-generation/scope validation to both messaging fetch paths before publishing rows and totals.
  • Add a deferred-response regression test covering a profile-scope change while the first fetch is in flight.

Automated hermes-sweeper review.

const refreshMessagingSessions = useCallback(async () => {
try {
const result = await listAllProfileSessions(MESSAGING_SECTION_LIMIT, 1, 'exclude', 'recent', 'all', {
const sessionProfile = profileScope === ALL_PROFILES ? 'all' : profileScope

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please pair this scope change with a request-generation or captured-scope check before setMessagingSessions. Unlike refreshSessions, this callback publishes every completed response without guarding against an older scope's request resolving last; toggling All Profiles does not wipe this shared messaging store.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state 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 area/sessions Session lifecycle, resume, persistence, history area/profiles Multi-profile isolation, HERMES_HOME scoping labels Jul 15, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Resolved on main by #87566 (Desktop sidebar profile scoping) — this PR's messaging fetch-scoping approach is fully covered there. #42934 submitted the same fix first and is credited in the merged commits. Thanks for the contribution!

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

Labels

area/profiles Multi-profile isolation, HERMES_HOME scoping area/sessions Session lifecycle, resume, persistence, history comp/desktop Electron desktop app (apps/desktop/*) duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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.

3 participants