Skip to content

fix(desktop): restore pinned sessions after restart - #39360

Closed
Tamaz-sujashvili wants to merge 1 commit into
NousResearch:mainfrom
Tamaz-sujashvili:fix/38858-hydrate-pinned-sessions
Closed

fix(desktop): restore pinned sessions after restart#39360
Tamaz-sujashvili wants to merge 1 commit into
NousResearch:mainfrom
Tamaz-sujashvili:fix/38858-hydrate-pinned-sessions

Conversation

@Tamaz-sujashvili

Copy link
Copy Markdown
Contributor

Summary

  • After cold boot, mergeSessionPage drops pin ids not in the first page; fetch missing pins via searchSessions and merge.
  • Share searchResultToSession helper for FTS hits.

Test plan

  • Pin a session, restart Hermes Desktop, pinned row still appears and resumes

Fixes #38858

Made with Cursor

Cold boot merges an empty previous page, so pin ids dropped out of the
sidebar. Fetch missing pin rows via session search and merge them in.

Fixes NousResearch#38858

Co-authored-by: Cursor <cursoragent@cursor.com>

@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 the cold-boot gap. The premise remains valid on current main: useSessionListActions.ts:176-182 loads only the bounded recent page, while sidebar/index.tsx:344-377 can render pins only from loaded rows.

Problems

  • apps/desktop/src/store/session.ts:112 falls back to res.results[0] when no returned result matches the pin. The search endpoint combines ID and FTS matches (hermes_cli/web_server.py:4193-4347), so a stale pin can add an unrelated session.
  • apps/desktop/src/app/desktop-controller.tsx:250-251 awaits hydration after the request-id check, then writes without rechecking it; a newer refresh can be overwritten by this stale response.
  • The diff adds no regression test for a cold-start, off-page pin.

Suggested changes

  • Port the behavior to current main's apps/desktop/src/app/session/hooks/use-session-list-actions.ts:155-194, preserving request-generation safety after awaits.
  • Hydrate exact ID/lineage matches only, and cover cold boot, failed lookup, compression lineage, and overlapping refreshes.
  • Use a profile-aware hydration path for the current unified profile list; related open PR #60019 already documents that design direction.

Automated hermes-sweeper review.

try {
const res = await searchSessions(pinId)
const hit =
res.results.find(r => r.session_id === pinId || r.lineage_root === pinId) ?? res.results[0]

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.

searchSessions() also returns FTS content matches. If this stale pin has no exact session_id/lineage_root match, falling back to results[0] can inject an unrelated session into the sidebar. Hydrate only the exact match and leave a missing pin unresolved.

setSessions(prev => mergeSessionPage(prev, result.sessions, keepIds))
const merged = mergeSessionPage($sessions.get(), result.sessions, keepIds)
const withPins = await hydratePinnedSessions(merged, $pinnedSessionIds.get())
setSessions(withPins)

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.

This write occurs after an await, but the request-id guard was checked before hydration. A newer refresh can complete during that await and then be overwritten by this older snapshot. Recheck refreshSessionsRequestRef.current === requestId immediately before publishing.

@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 labels Jul 14, 2026
@OutThisLife

Copy link
Copy Markdown
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

[Bug]: Hermes Desktop pinned sessions are unpinned after app restart

4 participants