fix(desktop): scope messaging sidebar by profile - #63618
Conversation
|
+1 on this approach. We hit the same bug on a Telegram-only secondary profile, independently traced it to the same two causes (profile=all seed fetch + sections hidden behind the workspace-grouping gate), and verified that fixing exactly those two things resolves it on a production two-profile setup (macOS, remote gateway). Scoping the fetch server-side is better than the client-side filter we used locally — it also keeps the per-platform totals honest. |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing both the server fetch scope and the empty-section gate. The underlying bug is present on current main: use-session-list-actions.ts:102 and :126 fetch messaging with profile=all, and sidebar/index.tsx:1007 omits messaging/cron from its visibility condition.
Problems
- The new sidebar filter at
apps/desktop/src/app/chat/sidebar/index.tsx:824will hide all messaging rows in the existing persisted-All fallback.ALL_PROFILESis"__all__"(store/profile.ts:279), whileshowAllProfilesis deliberately false when only one profile remains (sidebar/index.tsx:261; commit76b98f43). The comparison therefore rejects normaldefaultrows. - The newly scoped async messaging fetch at
use-session-list-actions.ts:103has no request-generation guard. A prior-profile response can arrive after the active profile's response and replace its messaging cache. The Desktop guide requires stale responses not to overwrite newer intent (apps/desktop/AGENTS.md:68-70).
Suggested changes
- Filter defensively only when
profileScope !== ALL_PROFILES, and cover the persisted-All/single-profile fallback. - Add a messaging refresh generation guard and an out-of-order profile-switch test.
Automated hermes-sweeper review.
| @@ -816,6 +821,10 @@ export function ChatSidebar({ | |||
| const bySource = new Map<string, SessionInfo[]>() | |||
|
|
|||
| for (const session of messagingSessions) { | |||
| if (!showAllProfiles && normalizeProfileKey(session.profile) !== profileScope) { | |||
There was a problem hiding this comment.
showAllProfiles is false when an All Profiles preference persists after the user returns to one profile (the intentional fallback from 76b98f4), but profileScope remains __all__. This condition then filters every normal default messaging row. Check profileScope !== ALL_PROFILES here instead, and add that fallback regression test.
| @@ -99,7 +100,7 @@ export function useSessionListActions({ profileScope }: UseSessionListActionsArg | |||
| // seeds every platform; the sidebar splits the rows per source. | |||
| const refreshMessagingSessions = useCallback(async () => { | |||
| try { | |||
| const result = await listAllProfileSessions(MESSAGING_SECTION_LIMIT, 1, 'exclude', 'recent', 'all', { | |||
| const result = await listAllProfileSessions(MESSAGING_SECTION_LIMIT, 1, 'exclude', 'recent', sessionProfile, { | |||
There was a problem hiding this comment.
Profile changes can leave an earlier fetch in flight; without a request-generation guard, its later response can overwrite the newly selected profile's messaging cache. Mirror refreshSessionsRequestRef and test A/B requests resolving out of order.
4eba497 to
83392d6
Compare
Signed-off-by: Ho Lim <subhoya@gmail.com>
Signed-off-by: Ho Lim <subhoya@gmail.com>
Signed-off-by: Ho Lim <subhoya@gmail.com>
83392d6 to
5eb7945
Compare
SummaryEighteen PRs address or reference this issue complex: #42934, #56635, #58983, #60688, #63618, #70630, and #71526 change messaging/profile scoping; #42654, #47429, #51531, #52910, and #67615 cover cron/profile scoping; the remaining diffs address owner resolution, deletion races, bulk deletion, fetch failures, pin hydration, or messaging-page retention. For #63593 specifically, #63618 matches both reported causes by scoping messaging fetches and keeping messaging/cron sections visible without normal sessions. Related pull requests
Duplicates#56635 and the core messaging-scoping portion of #58983 duplicate the same fetch changes covered more completely by #42934/#63618; the messaging portions of #44157, #52910, and #60688 overlap those call sites but retain distinct owner-resolution or cron-session work. #42654, #47429, and #51531 are superseded cron approaches whose relevant behavior is represented by merged #67615, while #70630 and #71526 are broader backend/routing implementations rather than exact duplicates. Suggested consolidationKeep #63618 open with a salvage path focused on its independently valuable empty-sidebar visibility logic plus its tested stale-response and ALL_PROFILES handling; coordinate the server-window and remote-routing contract with #71526 rather than retaining parallel messaging-scoping implementations. Close #56635 as a duplicate of #42934/#63618, narrow #58983 to any visibility work not already retained, narrow #44157 to current-path owner resolution and #60688 to cron-session scoping, and keep #70630 open only for a current-main salvage of the broader boundary that explicitly follows its contributor review. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I42915(["issue #42915 (open)"])
I52401(["issue #52401 (open)"])
I56631(["issue #56631 (open)"])
I60678(["issue #60678 (open)"])
I63593(["issue #63593 (open)"])
I65710(["issue #65710 (open)"])
I70629(["issue #70629 (open)"])
P63618["PR #63618 (open)"]
P63618 -->|fixes| I42915
P63618 -.->|partial| I52401
P63618 -->|fixes| I56631
P63618 -.->|partial| I60678
P63618 -->|best fix| I63593
P63618 -->|fixes| I65710
P63618 -.->|partial| I70629
class I42915 open
class I52401 open
class I56631 open
class I60678 open
class I63593 open
class I65710 open
class I70629 open
class P63618 open
class P63618 best
class P63618 target
click I42915 "https://github.com/NousResearch/hermes-agent/issues/42915"
click I52401 "https://github.com/NousResearch/hermes-agent/issues/52401"
click I56631 "https://github.com/NousResearch/hermes-agent/issues/56631"
click I60678 "https://github.com/NousResearch/hermes-agent/issues/60678"
click I63593 "https://github.com/NousResearch/hermes-agent/issues/63593"
click I65710 "https://github.com/NousResearch/hermes-agent/issues/65710"
click I70629 "https://github.com/NousResearch/hermes-agent/issues/70629"
click P63618 "https://github.com/NousResearch/hermes-agent/pull/63618"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 18 pull requests and 11 issues in this complex. Each diff was read against this issue; Assessment working set: 281 kB of PR diffs, 79 kB of issue/PR text, 63 kB of discussion (67 comments), 86 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
|
The profile-scoping half of this PR landed on main via #87566. What survives here: the section-visibility gate ( |
Summary
Why
The Desktop sidebar fetched messaging sessions with
profile=allregardless of the selected profile, then hid every session section when the normal-session/project lists were empty. This could expose another profile’s Slack rows and make the messaging section appear only after creating a local chat.Fixes #63593
Testing
npm run test:ui --workspace apps/desktop -- src/app/session/hooks/use-session-list-actions.test.ts src/app/chat/sidebar/section-states.test.ts(6 passed)npm run typecheck --workspace apps/desktopnpm exec --workspace apps/desktop -- eslint src/app/session/hooks/use-session-list-actions.ts src/app/session/hooks/use-session-list-actions.test.ts src/app/chat/sidebar/index.tsx src/app/chat/sidebar/section-states.tsx src/app/chat/sidebar/section-states.test.tsnpm exec --workspace apps/desktop -- prettier --check src/app/session/hooks/use-session-list-actions.ts src/app/session/hooks/use-session-list-actions.test.ts src/app/chat/sidebar/index.tsx src/app/chat/sidebar/section-states.tsx src/app/chat/sidebar/section-states.test.ts