Merge pull request #74234 from NousResearch/bb/session-pins-server-owned

Pins are server-owned, so they survive paging and follow you between apps

apps/desktop/src/store/session-pin-sync.test.ts

@@ -93,3 +93,76 @@ describe('watchSessionPins', () => {
     expect(patch).not.toHaveBeenCalled()
   })
 })
+
+describe('watchSessionPins remote pull', () => {
+  it('adopts a pin another app made', async () => {
+    $sessions.set([row('remote', { pinned: true })])
+    await flush()
+
+    expect($pinnedSessionIds.get()).toContain('remote')
+  })
+
+  it('adopts a remote pin on the durable lineage root, not the live tip', async () => {
+    $sessions.set([row('tip', { _lineage_root_id: 'root', pinned: true })])
+    await flush()
+
+    expect($pinnedSessionIds.get()).toEqual(['root'])
+  })
+
+  it('does not echo an adopted pin back as a redundant write', async () => {
+    $sessions.set([row('adopted', { pinned: true })])
+    await flush()
+
+    expect(patch).not.toHaveBeenCalled()
+  })
+
+  it('drops a local pin the server reports as unpinned', async () => {
+    $pinnedSessionIds.set(['gone'])
+    $sessions.set([row('gone', { pinned: true })])
+    await flush()
+    patch.mockClear()
+
+    // Another app unpinned it; our next refresh carries the new truth.
+    $sessions.set([row('gone', { pinned: false })])
+    await flush()
+
+    expect($pinnedSessionIds.get()).not.toContain('gone')
+  })
+
+  it('leaves the local set alone when the backend omits the flag', async () => {
+    $pinnedSessionIds.set(['legacy'])
+    // No `pinned` key at all — a runtime predating the column.
+    $sessions.set([row('legacy')])
+    await flush()
+
+    expect($pinnedSessionIds.get()).toContain('legacy')
+  })
+
+  it('ignores a stale page that contradicts a write still in flight', async () => {
+    let settle: (v: { ok: boolean }) => void = () => {}
+
+    patch.mockImplementationOnce(() => new Promise(resolve => (settle = resolve)))
+
+    $sessions.set([row('race')])
+    $pinnedSessionIds.set(['race'])
+    await flush()
+    expect(patch).toHaveBeenCalledWith('race', true, undefined)
+
+    // A list request issued before the PATCH lands still says pinned=false.
+    // Honouring it would silently undo the pin the user just made.
+    $sessions.set([row('race', { pinned: false })])
+    await flush()
+
+    expect($pinnedSessionIds.get()).toContain('race')
+
+    // Once the write is acked, later server truth is honoured again.
+    settle({ ok: true })
+    await flush()
+    await flush()
+
+    $sessions.set([row('race', { pinned: false }), row('other')])
+    await flush()
+
+    expect($pinnedSessionIds.get()).not.toContain('race')
+  })
+})

apps/desktop/src/store/session-pin-sync.ts

@@ -1,35 +1,106 @@
 /**
- * Mirror the sidebar's localStorage pins into the backend "keep" flag.
+ * Reconcile the sidebar's pins with the backend "keep" flag, both directions.
  *
- * Pins live in `$pinnedSessionIds` (localStorage) and drive the sidebar UI.
- * The `sessions.auto_archive` sweep, however, runs backend-side and is blind to
- * localStorage — so without this bridge it could hide a pinned chat. This
- * watcher PATCHes `pinned` on the session REST endpoint whenever the pinned set
- * changes, and re-asserts the whole current set at boot, which transparently
- * migrates pre-existing pins (no flag, no user action — the sweep just starts
- * honouring them). It never touches the sidebar's own display; localStorage
- * stays the source of truth there.
+ * Pins drive the sidebar UI out of `$pinnedSessionIds` (localStorage), but the
+ * durable record is `sessions.pinned` in each profile's state.db. Two things
+ * depend on the backend copy: the `sessions.auto_archive` sweep runs
+ * server-side and would otherwise hide a pinned chat, and a second Desktop app
+ * pointed at the same gateway has its own, separate localStorage.
+ *
+ * Push: PATCH `pinned` whenever the local set changes, and re-assert the whole
+ * set at boot — which transparently migrates pre-existing pins with no user
+ * action.
+ *
+ * Pull: session rows now carry `pinned`, and the list endpoints back-fill
+ * pinned conversations past their LIMIT, so a row's absence from a page no
+ * longer says anything about its pin state. That makes the server row
+ * authoritative: adopt pins this app hasn't seen, and drop local pins the
+ * server says are gone. Only rows actually present in the payload are
+ * consulted, so a backend predating the flag (`pinned === undefined`) leaves
+ * the local set untouched.
  */
 
 import { setSessionPinnedRemote } from '@/hermes'
-import { $pinnedSessionIds } from '@/store/layout'
-import { $sessions, sessionMatchesStoredId } from '@/store/session'
+import { $pinnedSessionIds, pinSession, unpinSession } from '@/store/layout'
+import { $sessions, sessionMatchesStoredId, sessionPinId } from '@/store/session'
 
 // pin ids we've successfully PATCHed pinned=true this session.
 const mirrored = new Set<string>()
 // pin ids awaiting their row so we can resolve the owning profile before PATCH.
 const pending = new Set<string>()
+// Writes we've issued but not yet had acked, id -> value written. A list page
+// already in flight when we PATCH still carries the old value, so it must not
+// be read as the server disagreeing with us. Cleared when the write settles —
+// the request's own lifetime is the guard, so nothing can leave one open.
+const unconfirmed = new Map<string, boolean>()
 
 function profileFor(pinId: string): null | string | undefined {
   return $sessions.get().find(row => sessionMatchesStoredId(row, pinId))?.profile
 }
 
+/** PATCH the flag, guarding reads against pages that predate the write. */
+function writePin(id: string, pinned: boolean, profile?: null | string): Promise<void> {
+  unconfirmed.set(id, pinned)
+
+  return setSessionPinnedRemote(id, pinned, profile).then(
+    () => {
+      unconfirmed.delete(id)
+    },
+    (err: unknown) => {
+      unconfirmed.delete(id)
+      throw err
+    }
+  )
+}
+
+/**
+ * Adopt the server's pin state for every row in the current page.
+ *
+ * Runs before the push pass so a remote pin is already in the local set by the
+ * time we reconcile — it gets marked as mirrored rather than echoed straight
+ * back as a redundant PATCH.
+ */
+function pullRemotePins(): void {
+  const local = new Set($pinnedSessionIds.get())
+
+  for (const row of $sessions.get()) {
+    // A backend without the flag has no opinion; never act on `undefined`.
+    if (typeof row.pinned !== 'boolean') {
+      continue
+    }
+
+    // Pins are keyed on the durable lineage root so they survive compression
+    // tip rotation; the row may surface under either identity.
+    const pinId = sessionPinId(row)
+    const heldLocally = local.has(pinId) || local.has(row.id)
+
+    // A write of ours the page hasn't caught up to yet is newer than the page.
+    const awaited = unconfirmed.has(pinId) ? unconfirmed.get(pinId) : unconfirmed.get(row.id)
+
+    if (awaited !== undefined && awaited !== row.pinned) {
+      continue
+    }
+
+    if (row.pinned && !heldLocally) {
+      pinSession(pinId)
+      // Already true server-side; record it so the push pass doesn't re-PATCH.
+      mirrored.add(pinId)
+    } else if (!row.pinned && heldLocally) {
+      unpinSession(local.has(pinId) ? pinId : row.id)
+      mirrored.delete(pinId)
+      mirrored.delete(row.id)
+    }
+  }
+}
+
 function reconcile(): void {
   // Config/session REST is only reachable through the Electron bridge.
   if (!window.hermesDesktop) {
     return
   }
 
+  pullRemotePins()
+
   const current = new Set($pinnedSessionIds.get())
 
   // Unpinned: anything we were tracking that's no longer in the set.
@@ -37,7 +108,7 @@ function reconcile(): void {
     if (!current.has(id)) {
       mirrored.delete(id)
       pending.delete(id)
-      void setSessionPinnedRemote(id, false, profileFor(id)).catch(() => {})
+      void writePin(id, false, profileFor(id)).catch(() => {})
     }
   }
 
@@ -59,7 +130,7 @@ function reconcile(): void {
 
     pending.delete(id)
     mirrored.add(id)
-    void setSessionPinnedRemote(id, true, row.profile).catch(() => {
+    void writePin(id, true, row.profile).catch(() => {
       // Let a later reconcile retry the mirror.
       mirrored.delete(id)
       pending.add(id)

apps/desktop/src/types/hermes.ts

@@ -472,6 +472,13 @@ export interface SessionInfo {
   output_tokens: number
   /** Parent conversation when this row is a /branch fork. */
   parent_session_id?: null | string
+  /** Durable server-side pin flag (`sessions.pinned`). The list endpoints
+   *  back-fill pinned conversations past their LIMIT, so a pinned row is
+   *  always present in a page — which makes this authoritative for the
+   *  sidebar's Pinned section and lets a second app adopt pins made
+   *  elsewhere. Undefined against a backend predating the flag; treat that as
+   *  "no opinion" and leave the local pin set alone. */
+  pinned?: boolean
   preview: null | string
   source: null | string
   started_at: number

hermes_cli/web_server.py

@@ -4796,6 +4796,7 @@ def get_sessions(
                 # rows, skip the system_prompt blob inside SQLite too (pairs
                 # with the API-level _strip_session_list_rows below).
                 compact_rows=not full,
+                include_pinned=True,
             )
             total = db.session_count(
                 source=source or None,
@@ -4818,6 +4819,7 @@ def get_sessions(
                     s["is_default_profile"] = profile_name == "default"
                 # SQLite stores the flag as 0/1; expose a real JSON boolean.
                 s["archived"] = bool(s.get("archived"))
+                s["pinned"] = bool(s.get("pinned"))
             if not full:
                 _strip_session_list_rows(sessions)
             return {"sessions": sessions, "total": total, "limit": limit, "offset": offset}
@@ -4920,6 +4922,7 @@ def get_profiles_sessions(
                 order_by_last_active=order == "recent",
                 # Same SQL-level blob skip as /api/sessions (see above).
                 compact_rows=not full,
+                include_pinned=True,
             )
             profile_total = db.session_count(
                 source=source_filter,
@@ -4940,6 +4943,7 @@ def get_profiles_sessions(
                     and (now - s.get("last_active", s.get("started_at", 0))) < 300
                 )
                 s["archived"] = bool(s.get("archived"))
+                s["pinned"] = bool(s.get("pinned"))
                 merged.append(s)
         except Exception as exc:
             errors.append({"profile": name, "error": str(exc)})
@@ -4948,7 +4952,12 @@ def get_profiles_sessions(
 
     sort_key = "last_active" if order == "recent" else "started_at"
     merged.sort(key=lambda s: s.get(sort_key) or s.get("started_at") or 0, reverse=True)
+    # Pinned rows are back-filled past each profile's LIMIT on purpose; keep
+    # them in the merged window instead of re-dropping them on recency.
     window = merged[offset:offset + limit]
+    if len(merged) > offset + limit:
+        seen = {id(s) for s in window}
+        window.extend(s for s in merged[offset + limit:] if s.get("pinned") and id(s) not in seen)
     if not full:
         _strip_session_list_rows(window)
     return {
@@ -5025,6 +5034,9 @@ def get_profiles_sessions_sidebar(
                 and (now - s.get("last_active", s.get("started_at", 0))) < 300
             )
             s["archived"] = bool(s.get("archived"))
+            # SQLite stores the pin as 0/1; the sidebar needs a real boolean to
+            # render the Pinned section from server state.
+            s["pinned"] = bool(s.get("pinned"))
         return rows
 
     def _slice(db, *, source=None, exclude=None, cap):
@@ -5038,6 +5050,9 @@ def get_profiles_sessions_sidebar(
             archived_only=False,
             order_by_last_active=True,
             compact_rows=True,
+            # A pinned conversation must reach the sidebar even when it has
+            # aged past the window — otherwise its Pinned row renders empty.
+            include_pinned=True,
         )
 
     for name, home in targets:
@@ -5055,8 +5070,10 @@ def get_profiles_sessions_sidebar(
                 # A full window means more rows remain on disk. That is all the
                 # sidebar's "load more" needs, and unlike an exact COUNT(*) per
                 # profile per refresh it costs nothing beyond the rows already
-                # read.
-                recents_truncated[name] = len(profile_rows) >= recents_cap
+                # read. Discount pinned back-fills — they arrive past the LIMIT
+                # and would otherwise fake a full page on a short list.
+                unpinned_count = sum(1 for s in profile_rows if not s.get("pinned"))
+                recents_truncated[name] = unpinned_count >= recents_cap
                 recents_rows.extend(_tag(profile_rows, name))
             cron_rows.extend(_tag(_slice(db, source="cron", cap=cron_cap), name))
             messaging_rows.extend(
@@ -5069,7 +5086,13 @@ def get_profiles_sessions_sidebar(
 
     def _window(rows: List[Dict[str, Any]], cap: int) -> List[Dict[str, Any]]:
         rows.sort(key=lambda s: s.get("last_active") or s.get("started_at") or 0, reverse=True)
+        # Pinned rows survive the cap. The per-profile queries deliberately
+        # back-fill them past the LIMIT, so truncating the merged window on
+        # recency alone would throw away exactly what the back-fill fetched.
         win = rows[:cap]
+        if len(rows) > cap:
+            seen = {id(s) for s in win}
+            win.extend(s for s in rows[cap:] if s.get("pinned") and id(s) not in seen)
         _strip_session_list_rows(win)
         return win
 

hermes_state.py

@@ -4694,6 +4694,7 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin)
         id_query: str = None,
         search_query: str = None,
         compact_rows: bool = False,
+        include_pinned: bool = False,
     ) -> List[Dict[str, Any]]:
         """List sessions with preview (first user message) and last active timestamp.
 
@@ -4733,6 +4734,14 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin)
         the SELECT so SQLite never copies it out of the B-tree page — a
         significant I/O saving on large databases where the blob routinely
         runs to tens of kilobytes per row.
+
+        Pass ``include_pinned=True`` to back-fill any conversation carrying the
+        durable ``pinned`` flag that the LIMIT/OFFSET window left out. A pin is
+        a "this must always be reachable" statement, so a pinned conversation
+        aging past the requested page is a bug, not a paging outcome — the
+        desktop sidebar would render an empty Pinned section. Back-filled rows
+        obey the same filters (source, archived, min_message_count) as the
+        page: an archived or filtered-out conversation stays out.
         """
         # Rows carry token/cost totals — drain queued deltas first so
         # listings (sidebar, /resume, dashboards) show exact counters.
@@ -4780,6 +4789,9 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin)
             where_clauses.append("s.archived = 0")
 
         where_sql = f"WHERE {' AND '.join(where_clauses)}" if where_clauses else ""
+        # Snapshot the filter params before the query builders below extend
+        # them with LIMIT/OFFSET — the pinned back-fill reuses the same WHERE.
+        base_where_params = list(params)
 
         # Optional session-id filter, pushed into SQL so callers (Desktop
         # session-id search) don't have to fetch every row and filter in
@@ -4933,6 +4945,45 @@ class SessionDB(SessionSearchMixin, SessionSchemaMixin, SessionPortabilityMixin)
             s.pop("_effective_last_active", None)
             sessions.append(s)
 
+        # Back-fill pinned conversations the page missed. A pin outlives
+        # recency, so this runs BEFORE compression projection below — a
+        # back-filled root then projects to its live tip exactly like a row
+        # that had made the page on its own. One extra query, bounded by the
+        # number of pins (a handful), never N+1 per pin.
+        if include_pinned:
+            seen_ids = {s["id"] for s in sessions}
+            pinned_where = (
+                f"{where_sql} AND s.pinned = 1" if where_sql else "WHERE s.pinned = 1"
+            )
+            _sel = self._compact_session_cols() if compact_rows else "s.*"
+            pinned_query = f"""
+                SELECT {_sel},
+                    COALESCE(
+                        (SELECT {_PREVIEW_RAW_SELECT}
+                         FROM messages m
+                         WHERE m.session_id = s.id AND m.role = 'user' AND m.content IS NOT NULL
+                         ORDER BY m.timestamp, m.id LIMIT 1),
+                        ''
+                    ) AS _preview_raw,
+                    COALESCE(
+                        (SELECT MAX(m2.timestamp) FROM messages m2 WHERE m2.session_id = s.id),
+                        s.started_at
+                    ) AS last_active
+                FROM sessions s
+                {pinned_where}
+                ORDER BY s.started_at DESC
+            """
+            with self._read_ctx() as conn:
+                pinned_cursor = conn.execute(pinned_query, base_where_params)
+                pinned_rows = pinned_cursor.fetchall()
+            for row in pinned_rows:
+                s = dict(row)
+                if s["id"] in seen_ids:
+                    continue
+                s["preview"] = _shape_preview(s.pop("_preview_raw", ""))
+                seen_ids.add(s["id"])
+                sessions.append(s)
+
         # Project compression roots forward to their tips. Each row whose
         # end_reason is 'compression' has a continuation child; replace the
         # surfaced fields (id, message_count, title, last_active, ended_at,

tests/test_hermes_state.py

@@ -6960,6 +6960,99 @@ class TestSessionPinAndStaleArchive:
         assert self._pinned(db, "root") == 1
         assert self._pinned(db, "tip") == 1
 
+    # ── pinned back-fill past the page window ─────────────────────────────
+    def test_pinned_session_survives_the_limit_window(self, db):
+        """A pin outlives recency: paging must not evict a pinned row.
+
+        Without ``include_pinned`` the desktop's Pinned section renders empty
+        for any conversation that has aged off the sidebar page.
+        """
+        for i in range(6):
+            self._make_idle(db, f"s{i}", days_idle=6 - i)
+        db.set_session_pinned("s0", True)  # the oldest — off a 3-row page
+
+        def ids(**kw):
+            return [
+                s["id"]
+                for s in db.list_sessions_rich(
+                    limit=3, min_message_count=1, order_by_last_active=True, **kw
+                )
+            ]
+
+        page = ids()
+        assert "s0" not in page, "precondition: the pin is off the page"
+
+        with_pins = ids(include_pinned=True)
+        assert "s0" in with_pins
+        # The page itself is untouched; the pin is additive.
+        assert with_pins[:3] == page
+        assert len(with_pins) == len(page) + 1
+
+    def test_pinned_backfill_still_obeys_the_page_filters(self, db):
+        """A back-filled pin is not a bypass — archived/short rows stay out."""
+        for i in range(4):
+            self._make_idle(db, f"f{i}", days_idle=4 - i)
+        self._make_idle(db, "archived_pin", days_idle=9)
+        db.set_session_pinned("archived_pin", True)
+        db.set_session_archived("archived_pin", True)
+        # Pinned but with no messages at all.
+        db.create_session(session_id="empty_pin", source="cli")
+        db.set_session_pinned("empty_pin", True)
+
+        ids = [
+            s["id"]
+            for s in db.list_sessions_rich(
+                limit=2,
+                min_message_count=1,
+                order_by_last_active=True,
+                include_pinned=True,
+            )
+        ]
+
+        assert "archived_pin" not in ids
+        assert "empty_pin" not in ids
+
+    def test_pinned_backfill_does_not_duplicate_an_on_page_row(self, db):
+        for i in range(3):
+            self._make_idle(db, f"p{i}", days_idle=3 - i)
+        db.set_session_pinned("p2", True)  # newest — already on the page
+
+        ids = [
+            s["id"]
+            for s in db.list_sessions_rich(
+                limit=3,
+                min_message_count=1,
+                order_by_last_active=True,
+                include_pinned=True,
+            )
+        ]
+
+        assert ids.count("p2") == 1
+
+    def test_pinned_backfill_projects_a_compression_root_to_its_tip(self, db):
+        """A back-filled root goes through tip projection like any other row."""
+        for i in range(4):
+            self._make_idle(db, f"n{i}", days_idle=4 - i)
+
+        self._make_idle(db, "old_root", days_idle=30)
+        db.end_session("old_root", end_reason="compression")
+        db.create_session(
+            session_id="old_tip", source="cli", parent_session_id="old_root"
+        )
+        db.append_message(session_id="old_tip", role="user", content="continued")
+        db.set_session_pinned("old_root", True)
+
+        rows = db.list_sessions_rich(
+            limit=2,
+            min_message_count=1,
+            order_by_last_active=True,
+            include_pinned=True,
+        )
+        backfilled = next(s for s in rows if s.get("_lineage_root_id") == "old_root")
+
+        # Surfaced under the live tip's identity, keyed on the durable root.
+        assert backfilled["id"] == "old_tip"
+
     # ── stale archive ─────────────────────────────────────────────────────
     def test_archives_only_sessions_idle_past_threshold(self, db):
         self._make_idle(db, "stale", days_idle=5)