fix(desktop): render pinned search sessions - #37590
Conversation
|
One thing I would change before merge: this only caches the search result in component state. It fixes the immediate pin action, but the pin can disappear again after a full app reload if the session is still outside the normal loaded page. The API already has GET /api/sessions/{id}; hydrating missing pinned ids from that endpoint would make the fix durable without widening the sidebar list. |
- Keep both pin-rendering fixes (getMissingPinnedSessionIds, resolvePinnedSessions) and upstream's mergeWorkingSessions - Both sets of tests preserved - session.ts: indexSessionsByPinnedId + mergeWorkingSessions coexist
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the persisted-search-pin failure. The underlying issue is still present on current main: apps/desktop/src/app/chat/sidebar/index.tsx:363-377 resolves persisted IDs only from loaded sidebar rows, while search pinning at :1147 persists only an ID.
Problems
- The new lookup at
apps/desktop/src/app/chat/sidebar/index.tsx:409callsgetSession(pinId)without a profile. Current main supports profile-scoped rows (apps/desktop/src/types/hermes.ts:358-361) and its existing uncached-ID resolver probes the active and other profiles (apps/desktop/src/app/session/hooks/use-session-actions/utils.ts:215-248). A pin from another profile would remain unresolved after reload. - The replacement resolver at
apps/desktop/src/app/chat/sidebar/index.tsx:442receives onlysessions. Current main intentionally includescronSessionsin the pinned index (apps/desktop/src/app/chat/sidebar/index.tsx:349-361), so this would remove pinned cron rows.
Suggested changes
- Rework the hydration around the current profile-aware resolution path and preserve the resolved row's profile.
- Keep cron pin resolution and cover it with a regression test in addition to the search-only reload case.
This is an automated hermes-sweeper review.
|
|
||
| let cancelled = false | ||
|
|
||
| void Promise.allSettled(missingPinnedSessionIds.map(pinId => getSession(pinId))).then(results => { |
There was a problem hiding this comment.
Current main supports profile-scoped sessions, but this only queries the active backend. Please use the current cross-profile stored-session resolution/probing behavior so persisted pins from another profile can hydrate after reload.
| }, [missingPinnedSessionIds]) | ||
|
|
||
| const pinnedSessions = useMemo( | ||
| () => resolvePinnedSessions(pinnedSessionIds, sessions, pinnedSearchSessions), |
There was a problem hiding this comment.
This replaces the current resolver, which deliberately indexes cronSessions as well as recents for pinned rows. Passing only sessions would regress pinned cron-session rendering; preserve that source in the salvaged resolver.
|
Superseded by #74234, which makes pins server-owned so they survive paging and sync between apps. Your work is carried in it and you're credited as a co-author — thank you. |
Summary
GET /api/sessions/{id}after app reload._lineage_root_idbehavior.Root Cause
Pinned session IDs were persisted correctly, but the Desktop sidebar rendered pinned rows by resolving those IDs only against the current loaded
$sessionslist. Search can return sessions that are not in that loaded list, so pinning a search-only result stored the pin but left the pinned section with no session record to render. The previous component-local search-result cache fixed the immediate render, but did not survive a full app reload.Fix
GET /api/sessions/{id}.SessionInfoobject.Test Plan
npm run buildnpm run test:ui -- src/store/session.test.tsnpm run type-checknpx eslint src/store/session.ts src/store/session.test.ts src/hermes.ts src/app/chat/sidebar/index.tsxgit diff --checkVerification Notes
The regression tests now cover:
_lineage_root_idfallback resolution;Verified against the actual affected local session from [Bug]: Desktop pinned search result does not appear when session is hidden from normal list #37582:
20260602_133018_d1c043/desktop ui bugexists in the local session DB;list_sessions_rich(..., include_children=False)sidebar list;bug*FTS query pattern used by/api/sessions/search?q=bugreturns that session;GET /api/sessions/20260602_133018_d1c043-equivalent DB lookup returns the raw session record;/api/sessions/20260602_133018_d1c043, and then resolves it into the pinned session list.Headless Desktop UI repro was run for real on this PR branch/build:
npm run build, producing install stamp8d134f7502a0 (fix/desktop-pinned-search-session).bugand shift-click pinned the search result for the affected session (20260602_133018_d1c043).localStorage['hermes.desktop.pinnedSessions']as["20260602_133018_d1c043"].PINNEDdesktop ui bugSESSIONS 4/12Negative control on current
origin/mainwithout this PR fix was also run with the same real local state DB:origin/mainatb34ee80741db2fdf188dcdc5c5caa78ee72642ffwith a clean install stamp (dirty: false).["20260602_133018_d1c043"]) and reloaded the Desktop app.Shift click to pin a chat) and did not renderdesktop ui bug.desktop ui bugafter reload.Portable fresh-fixture negative control was also run on unpatched
origin/mainso the repro does not depend on the reporter's private local DB:repro_parent_20260602_newpinand child sessionrepro_child_20260602_newpintitlednew pinned child reprointo a backed-up localstate.dbtest run.GET /api/sessions/repro_child_20260602_newpinreturned the raw child session record withparent_session_id = repro_parent_20260602_newpinandmessage_count = 1./api/sessions?limit=50&offset=0&min_messages=1&archived=exclude&order=recent) did not include the child session.origin/mainatb34ee8074headlessly with CDP, persistedlocalStorage['hermes.desktop.pinnedSessions']as["repro_child_20260602_newpin"], and reloaded.PINNEDwithShift click to pin a chatand did not rendernew pinned child repro.Security / Privacy Impact
Scope Boundaries
_lineage_root_iddurable pin behavior.parent_session_idbehavior described in the issue context.Checklist
Code
fix(scope):,feat(scope):, etc.).npm run build,npm run test:ui -- src/store/session.test.ts,npm run type-check, targetednpx eslint, andgit diff --check.Documentation & Housekeeping
cli-config.yaml.exampleupdate: N/A — no config keys changed.CONTRIBUTING.md/AGENTS.mdupdate: N/A — no architecture or workflow changed.For New Skills
Screenshots / Logs
Fixes #37582