diff --git a/apps/desktop/src/store/session-pin-sync.test.ts b/apps/desktop/src/store/session-pin-sync.test.ts index e6613d8658c..63878c1373c 100644 --- a/apps/desktop/src/store/session-pin-sync.test.ts +++ b/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') + }) +}) diff --git a/apps/desktop/src/store/session-pin-sync.ts b/apps/desktop/src/store/session-pin-sync.ts index 35235e6dc64..5af6d3fdca5 100644 --- a/apps/desktop/src/store/session-pin-sync.ts +++ b/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() // pin ids awaiting their row so we can resolve the owning profile before PATCH. const pending = new Set() +// 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() 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 { + 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) diff --git a/apps/desktop/src/types/hermes.ts b/apps/desktop/src/types/hermes.ts index 65979f5e20e..bfc75cf6b3f 100644 --- a/apps/desktop/src/types/hermes.ts +++ b/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 diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 820f01045f0..534eba86594 100644 --- a/hermes_cli/web_server.py +++ b/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 diff --git a/hermes_state.py b/hermes_state.py index d8da7719dfe..97c14ea214f 100644 --- a/hermes_state.py +++ b/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, diff --git a/tests/test_hermes_state.py b/tests/test_hermes_state.py index 89017c6d4ce..1aad825feed 100644 --- a/tests/test_hermes_state.py +++ b/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)