Skip to content

fix(desktop): filter messaging sessions by active profile - #42934

Closed
BlackishGreen33 wants to merge 6 commits into
NousResearch:mainfrom
BlackishGreen33:bg/desktop-messaging-profile-filter
Closed

fix(desktop): filter messaging sessions by active profile#42934
BlackishGreen33 wants to merge 6 commits into
NousResearch:mainfrom
BlackishGreen33:bg/desktop-messaging-profile-filter

Conversation

@BlackishGreen33

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes the Desktop sidebar messaging sections so they respect the active profile filter.

Before this change, normal Sessions followed the profile switcher, but Feishu/messaging sections could still show sessions from other profiles. This made the selected profile view noisy and confusing.

Now messaging sessions use the same profile scope as the rest of the sidebar:

  • a selected profile only shows that profile's messaging sessions
  • All profiles keeps the current cross-profile view

Related Issue

Fixes #42915

Type of Change

  • Bug fix / small behavior fix
  • Tests

Changes Made

  • Filter sidebar messaging sessions by the active profile scope before grouping by platform.
  • Fetch messaging sidebar rows with the active profile scope so profile-specific views are not windowed out by other profiles.
  • Preserve All profiles behavior.
  • Add regression coverage for profile-scoped messaging rows.

How to Test

cd apps/desktop
npm run type-check
npm run test:ui -- --run src/app/chat/sidebar/profile-scope.test.ts

Also run:

npx eslint src/app/chat/sidebar/profile-scope.ts src/app/chat/sidebar/profile-scope.test.ts src/app/chat/sidebar/index.tsx src/app/desktop-controller.tsx
git diff --check

Checklist

  • I searched existing PRs to avoid a duplicate.
  • The PR is limited to the Desktop sidebar messaging profile filter.
  • No API, config, or schema changes.

Copilot AI review requested due to automatic review settings June 9, 2026 16:08

Copilot AI 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.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR updates the desktop app’s messaging sidebar behavior to respect the currently selected profile scope (instead of always treating messaging sessions as “all profiles”), and introduces a small shared helper + tests for filtering sessions by profile.

Changes:

  • Fetch messaging sessions and per-platform “load more” results using the current profileScope (or all in All Profiles mode).
  • Filter messaging sessions shown in the sidebar by profile scope via a new filterSessionsByProfileScope helper.
  • Add Vitest coverage for the profile-scope filtering helper.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
apps/desktop/src/app/desktop-controller.tsx Fetch + paginate messaging sessions using the active profile scope and reset platform totals on refresh.
apps/desktop/src/app/chat/sidebar/profile-scope.ts Adds a reusable helper to filter sessions by profile scope.
apps/desktop/src/app/chat/sidebar/profile-scope.test.ts Adds unit tests for the new filtering helper.
apps/desktop/src/app/chat/sidebar/index.tsx Applies profile-scope filtering before grouping messaging sessions in the sidebar.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apps/desktop/src/app/desktop-controller.tsx Outdated
Comment thread apps/desktop/src/app/desktop-controller.tsx Outdated
Comment thread apps/desktop/src/app/desktop-controller.tsx Outdated
Comment thread apps/desktop/src/app/chat/sidebar/profile-scope.test.ts
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have labels Jun 9, 2026
…profile-filter

# Conflicts:
#	apps/desktop/src/app/chat/sidebar/index.tsx
#	apps/desktop/src/app/desktop-controller.tsx

@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 query and the per-platform paging path. The premise is still present on current main: apps/desktop/src/app/session/hooks/use-session-list-actions.ts:102 and :126 hard-code profile="all", unlike normal recents at :174-177.

Problems

  • apps/desktop/src/app/chat/sidebar/profile-scope.ts:8 uses showAllProfiles as the all-profiles predicate. Current main defines that as multiProfile && profileScope === ALL_PROFILES (apps/desktop/src/app/chat/sidebar/index.tsx:257-261), while $profileScope itself is ALL_PROFILES whenever the persisted setting is enabled (apps/desktop/src/store/profile.ts:294-296). With one profile and persisted all-profiles mode, the new helper filters rows against "__all__" and hides every messaging session.

Suggested changes

  • Preserve all rows based on profileScope === ALL_PROFILES (or a shared canonical scope predicate), and add coverage for that one-profile persisted-mode case.

Automated hermes-sweeper review.

Comment thread apps/desktop/src/app/chat/sidebar/profile-scope.ts Outdated
@teknium1 teknium1 added 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
@BlackishGreen33

Copy link
Copy Markdown
Contributor Author

Follow-up: the persisted single-profile / All profiles edge case is handled in c35a935. Profile filtering now uses the canonical ALL_PROFILES sentinel instead of the UI-only showAllProfiles flag, and profile-scope.test.ts covers the regression. Revalidated today: focused Desktop Vitest (3 tests) and Desktop typecheck pass.

@BlackishGreen33

Copy link
Copy Markdown
Contributor Author

Additional race fix pushed in c3efe9b. A stale profile-scope callback now exits before advancing the messaging request sequence or clearing platform totals, so it cannot invalidate the current profile request. Added a deferred-promise hook regression covering an A-to-B switch with the stale A callback returning after B. Focused Desktop Vitest (4 tests), typecheck, ESLint, diff check, and npm audit all pass.

@teknium1

Copy link
Copy Markdown
Contributor

Resolved on main by #87566, which consolidated this fix with your Co-authored-by credit preserved in the merged commits. You were the first to submit this fix — thank you!

@teknium1 teknium1 closed this Aug 16, 2026
@BlackishGreen33
BlackishGreen33 deleted the bg/desktop-messaging-profile-filter branch August 16, 2026 17:28
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 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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: Filter messaging sessions by profile in Desktop sidebar

4 participants