Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion apps/desktop/electron/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9988,7 +9988,8 @@ async function interceptSessionRequestForRemote(request) {

const cronSp = sliceParams('cron_limit', '50', { profile: 'all', source: 'cron' })

const messagingSp = sliceParams('messaging_limit', '100', { profile: 'all' })
const messagingProfile = (searchParams.get('messaging_profile') || 'all').trim() || 'all'
const messagingSp = sliceParams('messaging_limit', '100', { profile: messagingProfile })
const messagingExclude = searchParams.get('messaging_exclude')

if (messagingExclude) {
Expand Down
33 changes: 27 additions & 6 deletions apps/desktop/src/app/chat/sidebar/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,15 @@ import {
toggleSidebarMessagingOpen,
unpinSession
} from '@/store/layout'
import { $newChatProfile, $profiles, $profileScope, ALL_PROFILES, normalizeProfileKey } from '@/store/profile'
import {
$newChatProfile,
$profiles,
$profileScope,
ALL_PROFILES,
messagingProfileFor,
messagingTotalsKey,
normalizeProfileKey
} from '@/store/profile'
import {
$activeProjectId,
$projects,
Expand Down Expand Up @@ -884,20 +892,33 @@ export function ChatSidebar({

// Each messaging platform is its own self-managed section: split the
// separately-fetched messaging slice by source, newest platform first, rows
// within a platform by recency. Per-platform totals (when a "load more" has
// resolved them) drive the count + whether more remain on disk.
// within a platform by recency. Per-platform totals for the active profile
// (when a "load more" has resolved them) drive the count + whether more
// remain on disk.
const messagingGroups = useMemo<MessagingSection[]>(() => {
if (!messagingSessions.length) {
return []
}

// The fetch is already profile-scoped, but a profile switch doesn't wipe
// $messagingSessions (only a gateway-mode switch does), so the previous
// profile's rows would linger until the next refresh lands. Filtering here
// makes the switch instant; it's a no-op once the scoped rows arrive.
const visibleMessaging = showAllProfiles
? messagingSessions
: messagingSessions.filter(s => normalizeProfileKey(s.profile) === profileScope)

// Totals are cached per (profile, source), so read the slot belonging to the
// scope this render is showing rather than whichever profile resolved it last.
const messagingProfile = messagingProfileFor(profileScope)

const bySource = new Map<string, SessionInfo[]>()
// Rows this platform owns that the Pinned section is showing instead. The
// backend's per-platform total counts them, so discount it or "load more"
// promises rows that will never appear.
const pinnedBySource = new Map<string, number>()

for (const session of messagingSessions) {
for (const session of visibleMessaging) {
const sourceId = normalizeSessionSource(session.source)

if (!sourceId) {
Expand All @@ -918,7 +939,7 @@ export function ChatSidebar({
return [...bySource.entries()]
.map(([sourceId, list]) => {
const ordered = [...list].sort((a, b) => sessionTime(b) - sessionTime(a))
const known = messagingPlatformTotals[sourceId]
const known = messagingPlatformTotals[messagingTotalsKey(messagingProfile, sourceId)]
const unpinnedKnown = known == null ? null : Math.max(0, known - (pinnedBySource.get(sourceId) ?? 0))
const total = Math.max(ordered.length, unpinnedKnown ?? 0)

Expand All @@ -934,7 +955,7 @@ export function ChatSidebar({
}
})
.sort((a, b) => sessionTime(b.sessions[0]) - sessionTime(a.sessions[0]))
}, [messagingSessions, messagingPlatformTotals, messagingTruncated, isPinnedSession])
}, [messagingSessions, messagingPlatformTotals, messagingTruncated, isPinnedSession, profileScope, showAllProfiles])

// ALL-profiles view: one collapsible group per profile, color on the header
// (not on every row). Default profile floats to the top, the rest alpha.
Expand Down
111 changes: 102 additions & 9 deletions apps/desktop/src/app/session/hooks/use-session-list-actions.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import type { SessionInfo, SidebarSessionsResponse } from '@/hermes'
import {
$cronSessions,
$messagingPlatformTotals,
$messagingSessions,
$sessions,
$sessionsLoading,
setCronSessions,
setMessagingPlatformTotals,
setMessagingSessions,
setMessagingTruncated,
setSessions,
setSessionsLoading
} from '@/store/session'
Expand Down Expand Up @@ -70,21 +73,27 @@ vi.mock('@/store/projects', () => ({
$removedSessionIds: { get: () => removed.ids }
}))

beforeEach(() => {
listSidebarSessions.mockReset()
listAllProfileSessions.mockReset()
removed.ids = new Set()
// $messagingPlatformTotals / $messagingTruncated are module-level atoms that
// nothing in the app clears on a profile switch, so leaving them dirty between
// tests makes the profile-scope assertions below order-dependent.
const resetSessionStores = () => {
setSessions([])
setCronSessions([])
setMessagingSessions([])
setMessagingPlatformTotals({})
setMessagingTruncated(false)
setSessionsLoading(false)
}

beforeEach(() => {
listSidebarSessions.mockReset()
listAllProfileSessions.mockReset()
removed.ids = new Set()
resetSessionStores()
})

afterEach(() => {
setSessions([])
setCronSessions([])
setMessagingSessions([])
setSessionsLoading(false)
resetSessionStores()
})

describe('refreshSessions identity + loading hygiene', () => {
Expand Down Expand Up @@ -223,7 +232,8 @@ describe('refreshSessions batches slices into one request', () => {
expect.objectContaining({
recentsProfile: 'work',
recentsExclude: expect.arrayContaining(['cron']),
messagingExclude: expect.arrayContaining(['cron'])
messagingExclude: expect.arrayContaining(['cron']),
messagingProfile: 'work'
})
)
})
Expand All @@ -248,4 +258,87 @@ describe('refreshSessions batches slices into one request', () => {

expect(getCronJobs).toHaveBeenLastCalledWith('all')
})

// Messaging conversations live in the owning profile's state.db and every
// messaging read windows a shared row budget, so an unscoped fetch let a busy
// profile crowd the quieter ones out of the window — the sidebar's WeChat /
// Telegram sections showed a truncated union no matter which profile was
// selected.
it('scopes the messaging slices to the active profile (all → unified view)', async () => {
listSidebarSessions.mockResolvedValue(sidebar({ sessions: [] }))
listAllProfileSessions.mockResolvedValue({ sessions: [], total: 0 })

const scoped = renderHook(() => useSessionListActions({ profileScope: 'work' }))

await act(async () => {
await scoped.result.current.refreshMessagingSessions()
})

// 5th positional arg of listAllProfileSessions is the profile.
expect(listAllProfileSessions.mock.calls.at(-1)?.[4]).toBe('work')

await act(async () => {
await scoped.result.current.loadMoreMessagingForPlatform('weixin')
})

expect(listAllProfileSessions.mock.calls.at(-1)?.[4]).toBe('work')

const unified = renderHook(() => useSessionListActions({ profileScope: '__all__' }))

await act(async () => {
await unified.result.current.refreshMessagingSessions()
})

expect(listAllProfileSessions.mock.calls.at(-1)?.[4]).toBe('all')

await act(async () => {
await unified.result.current.refreshSessions()
})

expect(listSidebarSessions).toHaveBeenLastCalledWith(expect.objectContaining({ messagingProfile: 'all' }))
})

// Regression: per-platform totals are what drive a section's count and its
// "load more" affordance, and scoping the fetch made each total profile-
// specific. Keyed by source alone they leaked across a profile switch — too
// high showed a phantom "load more", too low SUPPRESSED a real one (a known
// total overrides the coarse truncation flag, and nothing re-fetches to
// correct it). Nothing clears these on a profile switch, so the key has to
// carry the profile.
it('keeps per-platform totals separate across a profile switch', async () => {
listSidebarSessions.mockResolvedValue(sidebar({ sessions: [] }))
listAllProfileSessions.mockResolvedValue({
sessions: [row('wx-1', { profile: 'work', source: 'weixin' })],
total: 42
})

const { rerender, result } = renderHook(({ profileScope }) => useSessionListActions({ profileScope }), {
initialProps: { profileScope: 'work' }
})

await act(async () => {
await result.current.loadMoreMessagingForPlatform('weixin')
})

expect($messagingPlatformTotals.get()['work:weixin']).toBe(42)

// Switch profiles: the quieter profile must not inherit work's count, and
// work's resolved total must survive so switching back doesn't re-fetch.
rerender({ profileScope: 'other' })

expect($messagingPlatformTotals.get()['other:weixin']).toBeUndefined()
expect($messagingPlatformTotals.get()['work:weixin']).toBe(42)

// The next profile resolves its own slot, side by side with work's.
listAllProfileSessions.mockResolvedValue({
sessions: [row('wx-2', { profile: 'other', source: 'weixin' })],
total: 3
})

await act(async () => {
await result.current.loadMoreMessagingForPlatform('weixin')
})

expect($messagingPlatformTotals.get()).toMatchObject({ 'other:weixin': 3, 'work:weixin': 42 })
})
})
72 changes: 47 additions & 25 deletions apps/desktop/src/app/session/hooks/use-session-list-actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
} from '@/lib/session-source'
import { setCronJobs } from '@/store/cron'
import { $pinnedSessionIds, $sessionsLimit, bumpSessionsLimit, SIDEBAR_SESSIONS_PAGE_SIZE } from '@/store/layout'
import { ALL_PROFILES, normalizeProfileKey } from '@/store/profile'
import { ALL_PROFILES, messagingProfileFor, messagingTotalsKey, normalizeProfileKey } from '@/store/profile'
import { $removedSessionIds } from '@/store/projects'
import {
$messagingSessions,
Expand Down Expand Up @@ -75,13 +75,20 @@ interface UseSessionListActionsArgs {
export function useSessionListActions({ profileScope }: UseSessionListActionsArgs) {
const refreshSessionsRequestRef = useRef(0)

// Messaging conversations are stored in the owning profile's state.db, and
// every messaging read windows a shared row budget — so an unscoped fetch
// lets a busy profile crowd the others out of the window. Scope them like
// recents/cron: a concrete profile sees only its own platform conversations,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

messagingPlatformTotals remains global even though this makes fetched rows profile-scoped. A per-platform load-more in profile A stores A's exact total, and after switching to B the sidebar still uses that value for its count and hasMore. Reset or key these totals by messagingProfile, and add a profile-switch regression test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed by keying the map rather than resetting it — pushed as a fresh commit on current main (the old base was ~425 commits behind and had gone dirty).

$messagingPlatformTotals is now keyed profile:source, built by a shared messagingTotalsKey / messagingProfileFor pair in store/profile.ts so the writer (loadMoreMessagingForPlatform) and the reader (the sidebar's messagingGroups memo) can't disagree about which profile a cached value belongs to. Two notes on the details:

  • Keying beats resetting here, because a reset throws away A's resolved total on every switch and drops hasMore back to the coarse $messagingTruncated flag until something re-pages. Keyed, A→B→A restores A's real count. It also needs no new effect or subscription: a scope change reads a different slot, so there's nothing to invalidate. (A reset hung off $activeGatewayProfile would also have missed the $showAllProfiles toggle, which changes the effective messaging profile without touching the gateway.)
  • The sidebar derives the scope from $profileScope, not showAllProfiles — the latter is multiProfile && profileScope === ALL_PROFILES, so on a single-profile install it disagrees with what the hook actually sent as messaging_profile.

Worth recording that the low-reading direction was the worse one: a stale total that's too high shows a phantom "load more" and self-heals after one wasted round-trip, but too low makes known > ordered.length false, which overrides $messagingTruncated and suppresses a legitimate "load more" — and nothing triggers a per-platform fetch, so that one never self-heals.

Backend salvaged as you suggested. The edit is now in hermes_cli/web_routers/profiles.py per 27b1377b4c; the web_server.py re-export means TestClient(app) still serves the route, so no mount change was needed.

Tests — the profile-switch regression you asked for is in use-session-list-actions.test.tsx, using a real rerender-driven switch after a per-platform load-more (the previous messaging test rendered two independent hook instances, so it proved nothing about state carried across a switch). It asserts the new profile doesn't inherit the count and the old profile's survives. I verified it fails without the keying. That file also now resets $messagingPlatformTotals / $messagingTruncated between tests — neither was reset before, which made these assertions order-dependent.

On the backend side there was no test to extend: the sidebar-handler tests were removed in the two pruning waves (6b81590c55, 39975613b1) and there's no tests/hermes_cli/web_routers/. So tests/hermes_cli/test_web_server_sidebar_sessions.py is new, following the isolated_profiles + client fixture idiom from test_web_server_messaging_profiles.py. It covers scoping, the all/omitted unified view, that messaging.total narrows with the scope, and that the cron/recents windows are unaffected. Two of the four fail without the gate.

One semantic change to flag: messaging.total now means "rows for the requested profile". That's what the desktop resolves a section's exact count from, and no consumer reads it as a cross-profile figure (SidebarSessionSlice only declares sessions + profiles_truncated), so I left it as-is rather than adding a second field.

Green locally: full ui vitest project (3125 tests), electron project (867), all three tsc projects, eslint, prettier, and scripts/run_tests.sh on the new file plus the adjacent profile/web-server suites.

Left out on purpose

Two adjacent leaks in the same family that I kept out to hold this diff to the review ask — happy to fold either in if you'd rather:

  • messagingVisible / messagingLoadMorePending (sidebar/index.tsx) are React-local and also survive a profile switch, so a busy profile's expanded reveal cap carries into a quieter one.
  • store/gateway-switch.test.ts never asserts the three messaging stores that wipeSessionListsForGatewaySwitch clears — pre-existing coverage gap.
  • Minor: loadMoreMessagingForPlatform's loaded count filters $messagingSessions by platform only, so mid-switch it can briefly count the previous profile's rows. Self-corrects on the next refresh.

// ALL_PROFILES keeps the unified view.
const messagingProfile = messagingProfileFor(profileScope)

// Messaging-platform sessions as their own slice, fetched separately from
// local recents so each platform renders a self-managed section and never
// competes with local chats for the recents page budget. One combined fetch
// 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', messagingProfile, {
excludeSources: MESSAGING_EXCLUDED_SOURCES
})

Expand All @@ -96,29 +103,42 @@ export function useSessionListActions({ profileScope }: UseSessionListActionsArg
} catch {
// Non-fatal: the messaging sections just stay empty/stale.
}
}, [])
}, [messagingProfile])

// Page a single platform's section independently (mirrors the per-profile
// pager): fetch that source's next window and merge it back in place, leaving
// every other platform's rows untouched. Resolves the platform's exact total.
const loadMoreMessagingForPlatform = useCallback(async (platform: string) => {
const inPlatform = (s: SessionInfo) => normalizeSessionSource(s.source) === platform
const loaded = $messagingSessions.get().filter(inPlatform).length

const result = await listAllProfileSessions(loaded + SIDEBAR_SESSIONS_PAGE_SIZE, 1, 'exclude', 'recent', 'all', {
source: platform
})

const incoming = result.sessions.filter(s => normalizeSessionSource(s.source) === platform)

setMessagingSessions(prev => [
...prev.filter(s => !inPlatform(s)),
...mergeSessionPage(prev.filter(inPlatform), incoming, sessionsToKeep())
])

const total = result.total ?? incoming.length
setMessagingPlatformTotals(prev => ({ ...prev, [platform]: Math.max(total, incoming.length) }))
}, [])
const loadMoreMessagingForPlatform = useCallback(
async (platform: string) => {
const inPlatform = (s: SessionInfo) => normalizeSessionSource(s.source) === platform
const loaded = $messagingSessions.get().filter(inPlatform).length

const result = await listAllProfileSessions(
loaded + SIDEBAR_SESSIONS_PAGE_SIZE,
1,
'exclude',
'recent',
messagingProfile,
{ source: platform }
)

const incoming = result.sessions.filter(s => normalizeSessionSource(s.source) === platform)

setMessagingSessions(prev => [
...prev.filter(s => !inPlatform(s)),
...mergeSessionPage(prev.filter(inPlatform), incoming, sessionsToKeep())
])

const total = result.total ?? incoming.length
// Stamp the total against the profile it was counted in — the same platform
// has a different count per profile, and switching profiles must not inherit
// this one.
const totalsKey = messagingTotalsKey(messagingProfile, platform)

setMessagingPlatformTotals(prev => ({ ...prev, [totalsKey]: Math.max(total, incoming.length) }))
},
[messagingProfile]
)

// Cron *jobs* drive the sidebar "Cron jobs" section. Jobs are created
// synchronously (agent tool call or the cron UI), so refreshing here right
Expand Down Expand Up @@ -161,8 +181,9 @@ export function useSessionListActions({ profileScope }: UseSessionListActionsArg
// the same rows tagged profile="default".
// Scope recents to the active profile (not always 'all') so a profile
// with few recent sessions isn't windowed out of the cross-profile
// recency page — the empty-history-on-profile-switch bug. Cron + messaging
// stay cross-profile.
// recency page — the empty-history-on-profile-switch bug. Messaging is
// scoped the same way (see messagingProfile); only cron stays
// cross-profile.
const sessionProfile = profileScope === ALL_PROFILES ? 'all' : profileScope

// Batched: one request opens each profile DB once and returns all three
Expand All @@ -174,7 +195,8 @@ export function useSessionListActions({ profileScope }: UseSessionListActionsArg
recentsExclude: SIDEBAR_EXCLUDED_SOURCES,
cronLimit: CRON_SECTION_LIMIT,
messagingLimit: MESSAGING_SECTION_LIMIT,
messagingExclude: MESSAGING_EXCLUDED_SOURCES
messagingExclude: MESSAGING_EXCLUDED_SOURCES,
messagingProfile
})

if (refreshSessionsRequestRef.current === requestId) {
Expand Down Expand Up @@ -236,7 +258,7 @@ export function useSessionListActions({ profileScope }: UseSessionListActionsArg

// Cron *jobs* are a distinct API (getCronJobs), not a session slice.
void refreshCronJobs()
}, [profileScope, refreshCronJobs])
}, [messagingProfile, profileScope, refreshCronJobs])

const loadMoreSessions = useCallback(async () => {
bumpSessionsLimit()
Expand Down
24 changes: 22 additions & 2 deletions apps/desktop/src/hermes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,19 +81,39 @@ describe('Hermes REST helpers', () => {
recentsExclude: ['cron', 'tool'],
cronLimit: 50,
messagingLimit: 100,
messagingExclude: ['cron', 'desktop']
messagingExclude: ['cron', 'desktop'],
messagingProfile: 'work'
})

expect(api).toHaveBeenCalledWith(
expect.objectContaining({
path:
'/api/profiles/sessions/sidebar?recents_profile=work&recents_limit=30&cron_limit=50' +
'&messaging_limit=100&recents_exclude=cron%2Ctool&messaging_exclude=cron%2Cdesktop',
'&messaging_limit=100&messaging_profile=work&recents_exclude=cron%2Ctool' +
'&messaging_exclude=cron%2Cdesktop',
timeoutMs: 60_000
})
)
})

// messagingProfile is optional so existing callers keep the cross-profile
// default; omitting it must still send an explicit messaging_profile=all
// rather than dropping the param.
it('defaults the messaging slice to all profiles when no messagingProfile is given', async () => {
api.mockResolvedValue({ recents: { sessions: [] }, cron: { sessions: [] }, messaging: { sessions: [] } })

await listSidebarSessions({
recentsProfile: 'work',
recentsLimit: 30,
recentsExclude: [],
cronLimit: 50,
messagingLimit: 100,
messagingExclude: []
})

expect(api.mock.calls.at(-1)?.[0].path).toContain('messaging_profile=all')
})

it('defaults missing sidebar slices to empty session arrays', async () => {
api.mockResolvedValue({})

Expand Down
Loading