From c461cee790c7933283612e4ca65cb6ca3098f893 Mon Sep 17 00:00:00 2001 From: Bobby Lansing Date: Tue, 21 Jul 2026 23:23:17 -0700 Subject: [PATCH 1/2] fix: persist session renames to state.db and surface failures Renames previously updated the local sessions.json cache first and swallowed state.db errors, so the UI looked successful until the next syncSessionCache restored the old Hermes title. Write the durable DB row first (honoring unique titles and the 100-char cap), only then mirror into the cache, and toast/rollback when the write fails. Co-authored-by: Cursor --- lat.md/sidebar-navigation.md | 6 + src/main/session-cache.ts | 85 +++++++--- .../screens/Layout/SidebarRecentSessions.tsx | 18 ++- .../src/screens/Sessions/Sessions.tsx | 24 ++- src/shared/i18n/locales/en/sessions.ts | 6 + tests/session-cache-sync.test.ts | 149 +++++++++++++++++- 6 files changed, 250 insertions(+), 38 deletions(-) diff --git a/lat.md/sidebar-navigation.md b/lat.md/sidebar-navigation.md index 518091eff..3dafd21f7 100644 --- a/lat.md/sidebar-navigation.md +++ b/lat.md/sidebar-navigation.md @@ -42,6 +42,12 @@ The menu is styled light-based and stroke-free (`.sidebar-session-menu`): no 1px Each action calls an existing desktop API with an optimistic local update and rollback on failure: Rename → `updateSessionTitle` (inline `.sidebar-recent-session-rename` input), Move → [[src/main/session-context-folder-store.ts#setSessionContextFolder]] then a `hermes-session-context-folder-changed` event so other surfaces re-group, Delete → a confirmation dialog (portal overlay) then [[src/main/sessions.ts#deleteSessionRows|deleteSession]]. Deleting the open chat calls `onSessionDeleted`, which [[src/renderer/src/screens/Layout/Layout.tsx#Layout]] uses to drop to a fresh New Chat. +### Rename persistence + +Session renames must survive `syncSessionCache`, so the durable `state.db` write happens before the JSON cache is updated. + +[[src/main/session-cache.ts#updateSessionTitle]] trims/collapses whitespace, enforces Hermes' 100-character cap ([[src/main/session-cache.ts#MAX_SESSION_TITLE_LENGTH]]), rejects empty titles, and checks uniqueness against Hermes' `idx_sessions_title_unique` before `UPDATE`ing. Failures throw (duplicate / not found / unavailable) instead of being swallowed — the sidebar and Sessions modal roll back their optimistic title, toast the error, and keep the inline editor open. A successful rename updates `sessions.json` only after `state.db` commits, so the next sync cannot resurrect the old name. + Pinned rows are a desktop-only affordance: their ids live in `localStorage` (`hermes.sidebar.pinnedSessions`), and pinned sessions are pulled out of the normal grouping into a collapsible **Pinned** section at the top of the list. ## Full-list modal diff --git a/src/main/session-cache.ts b/src/main/session-cache.ts index b592ec342..df64923e1 100644 --- a/src/main/session-cache.ts +++ b/src/main/session-cache.ts @@ -1,11 +1,6 @@ import { existsSync, readFileSync } from "fs"; import { join } from "path"; -import { - activeStateDbPath, - profileHome, - getActiveProfileNameSync, - safeWriteFile, -} from "./utils"; +import { profileHome, getActiveProfileNameSync, safeWriteFile } from "./utils"; import Database from "better-sqlite3"; import { t } from "../shared/i18n"; import { getAppLocale } from "./locale"; @@ -258,31 +253,71 @@ export function listCachedSessions(limit = 50, offset = 0): CachedSession[] { return cache.sessions.slice(offset, offset + limit); } -// Update title for a specific session +/** Hermes Agent caps session titles at 100 characters (schema + CLI). */ +export const MAX_SESSION_TITLE_LENGTH = 100; + +/** + * Persist a user-chosen session title to state.db, then mirror it into the + * desktop sessions.json cache. + * + * Order matters: the durable DB write must succeed before the cache is + * updated. The previous cache-first + swallow-errors approach left the UI + * looking renamed while the next syncSessionCache restored the old DB title + * (Hermes enforces UNIQUE non-NULL titles via idx_sessions_title_unique). + */ export function updateSessionTitle(sessionId: string, title: string): void { + const locale = getAppLocale(); + const trimmed = title.trim().replace(/\s+/g, " "); + if (!trimmed) { + throw new Error(t("sessions.renameInvalid", locale)); + } + if (trimmed.length > MAX_SESSION_TITLE_LENGTH) { + throw new Error( + t("sessions.renameTooLong", locale, { + max: String(MAX_SESSION_TITLE_LENGTH), + }), + ); + } + + const db = getDbConnection(false); + if (!db) { + throw new Error(t("sessions.renameUnavailable", locale)); + } + + // Match Hermes SessionDB.set_session_title: reject conflicts before write so + // we never partially update the JSON cache on a UNIQUE constraint failure. + const conflict = db + .prepare("SELECT id FROM sessions WHERE title = ? AND id != ?") + .get(trimmed, sessionId) as { id: string } | undefined; + if (conflict) { + throw new Error(t("sessions.renameDuplicate", locale, { title: trimmed })); + } + + let changes = 0; + try { + changes = db + .prepare("UPDATE sessions SET title = ? WHERE id = ?") + .run(trimmed, sessionId).changes; + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + if (/UNIQUE|constraint/i.test(message)) { + throw new Error( + t("sessions.renameDuplicate", locale, { title: trimmed }), + ); + } + throw err instanceof Error ? err : new Error(message); + } + + if (changes === 0) { + throw new Error(t("sessions.renameNotFound", locale)); + } + const cache = readCache(); const idx = cache.sessions.findIndex((s) => s.id === sessionId); if (idx >= 0) { - cache.sessions[idx].title = title; + cache.sessions[idx].title = trimmed; writeCache(cache); } - // Also persist in state.db so the rename survives cache rebuilds - try { - const dbPath = activeStateDbPath(); - if (existsSync(dbPath)) { - const db = new Database(dbPath); - try { - db.prepare("UPDATE sessions SET title = ? WHERE id = ?").run( - title, - sessionId, - ); - } finally { - db.close(); - } - } - } catch { - // ignore DB errors — cache update above is the fast path - } } // Remove a session entry from the local cache. Called after the underlying diff --git a/src/renderer/src/screens/Layout/SidebarRecentSessions.tsx b/src/renderer/src/screens/Layout/SidebarRecentSessions.tsx index ee94105db..4e858ad87 100644 --- a/src/renderer/src/screens/Layout/SidebarRecentSessions.tsx +++ b/src/renderer/src/screens/Layout/SidebarRecentSessions.tsx @@ -8,6 +8,7 @@ import { type RefObject, } from "react"; import { createPortal } from "react-dom"; +import toast from "react-hot-toast"; import { useI18n } from "../../components/useI18n"; import { ChevronDown, @@ -516,21 +517,32 @@ const SidebarRecentSessions = memo(function SidebarRecentSessions({ return; } const previous = current?.title ?? ""; - // Optimistic local update; roll back if the write fails. + // Optimistic local update; roll back if the durable write fails so a + // subsequent sync cannot resurrect a cache-only title. setSessions((prev) => prev.map((s) => (s.id === id ? { ...s, title: trimmed } : s)), ); - if (editingIdRef.current === id) cancelRename(); try { await window.hermesAPI.updateSessionTitle(id, trimmed); + if (editingIdRef.current === id) cancelRename(); } catch (err) { console.error("Failed to rename session", id, err); setSessions((prev) => prev.map((s) => (s.id === id ? { ...s, title: previous } : s)), ); + const message = + err instanceof Error && err.message + ? err.message + : t("sessions.renameFailed"); + toast.error(message); + // Keep the inline editor open so the user can pick a unique name. + setTimeout(() => { + renameInputRef.current?.focus(); + renameInputRef.current?.select(); + }, 0); } }, - [cancelRename], + [cancelRename, t], ); const handleMoveToProject = useCallback( diff --git a/src/renderer/src/screens/Sessions/Sessions.tsx b/src/renderer/src/screens/Sessions/Sessions.tsx index f5a3c85cd..2575d16d9 100644 --- a/src/renderer/src/screens/Sessions/Sessions.tsx +++ b/src/renderer/src/screens/Sessions/Sessions.tsx @@ -1,4 +1,5 @@ import { useEffect, useState, useRef, useCallback, useMemo, memo } from "react"; +import toast from "react-hot-toast"; import { Plus, Search, X, ChatBubble, Trash, Pencil } from "../../assets/icons"; import { useI18n } from "../../components/useI18n"; @@ -417,6 +418,12 @@ function Sessions({ }); try { await window.hermesAPI.updateSessionTitle(sessionId, trimmed); + // Guard: only clear editing state if the user hasn't started editing + // a different session while this request was in flight. + if (editingSessionIdRef.current === sessionId) { + setEditingSessionId(null); + setEditingTitle(""); + } } catch (err) { console.error("Failed to rename session", sessionId, err); // Rollback optimistic update @@ -432,15 +439,18 @@ function Sessions({ : r, ), ); - } - // Guard: only clear editing state if the user hasn't started editing - // a different session while this request was in flight. - if (editingSessionIdRef.current === sessionId) { - setEditingSessionId(null); - setEditingTitle(""); + const message = + err instanceof Error && err.message + ? err.message + : t("sessions.renameFailed"); + toast.error(message); + setTimeout(() => { + renameInputRef.current?.focus(); + renameInputRef.current?.select(); + }, 0); } }, - [cancelRename], + [cancelRename, t], ); const cancelDelete = useCallback((): void => { diff --git a/src/shared/i18n/locales/en/sessions.ts b/src/shared/i18n/locales/en/sessions.ts index 1e336819e..65216babc 100644 --- a/src/shared/i18n/locales/en/sessions.ts +++ b/src/shared/i18n/locales/en/sessions.ts @@ -16,6 +16,12 @@ export default { messageSingular: "msg", delete: "Delete conversation", rename: "Rename conversation", + renameFailed: "Couldn't rename conversation", + renameInvalid: "Enter a valid name (max 100 characters)", + renameTooLong: "Title too long (max {{max}} characters)", + renameUnavailable: "Session database unavailable", + renameDuplicate: 'Title "{{title}}" is already in use', + renameNotFound: "Session not found", deleteConfirmTitle: "Delete conversation", deleteConfirm: "Delete this conversation? This cannot be undone — both the messages and the session record will be permanently removed.", diff --git a/tests/session-cache-sync.test.ts b/tests/session-cache-sync.test.ts index b05880a7c..7c02876dd 100644 --- a/tests/session-cache-sync.test.ts +++ b/tests/session-cache-sync.test.ts @@ -46,7 +46,17 @@ vi.mock("../src/main/utils", () => ({ // Stub the i18n + locale modules so the cache code doesn't need the // renderer-side translation files at test time. vi.mock("../src/shared/i18n", () => ({ - t: (key: string) => key, + t: (key: string, _lang?: string, options?: Record) => { + const messages: Record = { + "sessions.newConversation": "sessions.newConversation", + "sessions.renameInvalid": "Enter a valid name (max 100 characters)", + "sessions.renameTooLong": `Title too long (max ${options?.max ?? 100} characters)`, + "sessions.renameUnavailable": "Session database unavailable", + "sessions.renameDuplicate": `Title "${options?.title ?? ""}" is already in use`, + "sessions.renameNotFound": "Session not found", + }; + return messages[key] ?? key; + }, })); vi.mock("../src/main/locale", () => ({ getAppLocale: () => "en", @@ -143,6 +153,32 @@ vi.mock("better-sqlite3", () => { return { changes: 1 }; } + // Desktop rename path — mirrors Hermes `idx_sessions_title_unique` + // (NULLs allowed; non-NULL titles must be unique across sessions). + if ( + this.sql.includes("UPDATE sessions SET title") && + this.sql.includes("WHERE id = ?") + ) { + const [title, id] = args; + const session = this.store.sessions.get(String(id)); + if (!session) return { changes: 0 }; + const nextTitle = + title === null || title === undefined ? null : String(title); + if (nextTitle !== null) { + for (const other of this.store.sessions.values()) { + if (other.id !== session.id && other.title === nextTitle) { + const err = new Error( + "UNIQUE constraint failed: sessions.title", + ) as Error & { code: string }; + err.code = "SQLITE_CONSTRAINT_UNIQUE"; + throw err; + } + } + } + session.title = nextTitle; + return { changes: 1 }; + } + throw new Error(`Unhandled fake run SQL: ${this.sql}`); } @@ -179,7 +215,7 @@ vi.mock("better-sqlite3", () => { throw new Error(`Unhandled fake all SQL: ${this.sql}`); } - get(...args: unknown[]): { content: string } | undefined { + get(...args: unknown[]): { content: string } | { id: string } | undefined { // `tableExists` probes sqlite_master before reading context folders // (issue #27). The desktop context-folder table is never created in // these tests, so report it absent — sessions then resolve to a null @@ -201,6 +237,20 @@ vi.mock("better-sqlite3", () => { return match ? { content: match.content } : undefined; } + // Uniqueness probe used by updateSessionTitle before writing. + if ( + this.sql.includes("SELECT id FROM sessions") && + this.sql.includes("WHERE title = ?") && + this.sql.includes("AND id != ?") + ) { + const [title, sessionId] = args; + const match = Array.from(this.store.sessions.values()).find( + (session) => + session.title === String(title) && session.id !== String(sessionId), + ); + return match ? { id: match.id } : undefined; + } + throw new Error(`Unhandled fake get SQL: ${this.sql}`); } } @@ -229,7 +279,11 @@ vi.mock("better-sqlite3", () => { }); import Database from "better-sqlite3"; -import { syncSessionCache } from "../src/main/session-cache"; +import { + listCachedSessions, + syncSessionCache, + updateSessionTitle, +} from "../src/main/session-cache"; import { closeDbConnection } from "../src/main/db"; const CACHE_FILE = join(TEST_HOME, "desktop", "sessions.json"); @@ -628,3 +682,92 @@ describe("syncSessionCache", () => { expect(elapsed).toBeLessThan(500); }, 30000); }); + +describe("updateSessionTitle", () => { + // @lat: [[sidebar-navigation#Sidebar recent sessions#Row context menu#Rename persistence]] + it("persists a rename to state.db so the next sync keeps the new title", () => { + const future = Math.floor(Date.now() / 1000) + 600; + seedDb([ + { + id: "s1", + started_at: future, + message_count: 2, + title: "Old title", + firstUserMessage: "hi", + }, + ]); + syncSessionCache(); + + updateSessionTitle("s1", "Renamed chat"); + + expect(listCachedSessions(10)[0]?.title).toBe("Renamed chat"); + const afterSync = syncSessionCache(); + expect(afterSync[0]?.title).toBe("Renamed chat"); + }); + + it("rejects duplicate titles without leaving a dirty cache entry", () => { + const future = Math.floor(Date.now() / 1000) + 600; + seedDb([ + { + id: "s1", + started_at: future, + message_count: 1, + title: "Taken name", + firstUserMessage: "one", + }, + { + id: "s2", + started_at: future + 1, + message_count: 1, + title: "Other chat", + firstUserMessage: "two", + }, + ]); + syncSessionCache(); + + expect(() => updateSessionTitle("s2", "Taken name")).toThrow( + /already in use/i, + ); + + const cached = listCachedSessions(10); + expect(cached.find((s) => s.id === "s2")?.title).toBe("Other chat"); + + // Sync must not have been poisoned by a cache-only write either. + const afterSync = syncSessionCache(); + expect(afterSync.find((s) => s.id === "s2")?.title).toBe("Other chat"); + }); + + it("throws when the session id is unknown", () => { + const future = Math.floor(Date.now() / 1000) + 600; + seedDb([ + { + id: "s1", + started_at: future, + message_count: 1, + title: "Exists", + firstUserMessage: "hi", + }, + ]); + syncSessionCache(); + + expect(() => updateSessionTitle("missing", "Nope")).toThrow(/not found/i); + }); + + it("rejects empty or oversized titles before touching the DB", () => { + const future = Math.floor(Date.now() / 1000) + 600; + seedDb([ + { + id: "s1", + started_at: future, + message_count: 1, + title: "Exists", + firstUserMessage: "hi", + }, + ]); + syncSessionCache(); + + expect(() => updateSessionTitle("s1", " ")).toThrow(/valid/i); + expect(() => updateSessionTitle("s1", "x".repeat(101))).toThrow(/100/); + expect(listCachedSessions(10)[0]?.title).toBe("Exists"); + }); +}); From b4f347195eb4d93fbb16647c55ac6cb5a3c979ba Mon Sep 17 00:00:00 2001 From: Bobby Lansing Date: Tue, 21 Jul 2026 23:27:18 -0700 Subject: [PATCH 2/2] refactor: share session title normalize and rename confirm flow Pull title policy into src/shared/session-title and the optimistic rename UX into confirmSessionRename so the sidebar and Sessions modal stop duplicating toast/rollback/refocus logic. Keeps both screens under the 1k-line ceiling and prevents UI/DB whitespace drift on rename. Co-authored-by: Cursor --- lat.md/sidebar-navigation.md | 2 +- src/main/session-cache.ts | 56 ++++++----- .../screens/Layout/SidebarRecentSessions.tsx | 54 +++++------ .../src/screens/Sessions/Sessions.tsx | 93 ++++++++----------- .../screens/Sessions/confirmSessionRename.ts | 56 +++++++++++ src/shared/session-title.test.ts | 45 +++++++++ src/shared/session-title.ts | 39 ++++++++ 7 files changed, 235 insertions(+), 110 deletions(-) create mode 100644 src/renderer/src/screens/Sessions/confirmSessionRename.ts create mode 100644 src/shared/session-title.test.ts create mode 100644 src/shared/session-title.ts diff --git a/lat.md/sidebar-navigation.md b/lat.md/sidebar-navigation.md index 3dafd21f7..591d8a791 100644 --- a/lat.md/sidebar-navigation.md +++ b/lat.md/sidebar-navigation.md @@ -46,7 +46,7 @@ Each action calls an existing desktop API with an optimistic local update and ro Session renames must survive `syncSessionCache`, so the durable `state.db` write happens before the JSON cache is updated. -[[src/main/session-cache.ts#updateSessionTitle]] trims/collapses whitespace, enforces Hermes' 100-character cap ([[src/main/session-cache.ts#MAX_SESSION_TITLE_LENGTH]]), rejects empty titles, and checks uniqueness against Hermes' `idx_sessions_title_unique` before `UPDATE`ing. Failures throw (duplicate / not found / unavailable) instead of being swallowed — the sidebar and Sessions modal roll back their optimistic title, toast the error, and keep the inline editor open. A successful rename updates `sessions.json` only after `state.db` commits, so the next sync cannot resurrect the old name. +Title policy lives in [[src/shared/session-title.ts]] (`normalizeSessionTitle`, `MAX_SESSION_TITLE_LENGTH`) so the renderer optimistic path and [[src/main/session-cache.ts#updateSessionTitle]] cannot diverge. Main writes `state.db` first (Hermes `idx_sessions_title_unique`), then mirrors into `sessions.json`; failures throw. Both the sidebar and Sessions modal call [[src/renderer/src/screens/Sessions/confirmSessionRename.ts#confirmSessionRename]] for optimistic update, toast/rollback, and keep-editor-on-failure. Pinned rows are a desktop-only affordance: their ids live in `localStorage` (`hermes.sidebar.pinnedSessions`), and pinned sessions are pulled out of the normal grouping into a collapsible **Pinned** section at the top of the list. diff --git a/src/main/session-cache.ts b/src/main/session-cache.ts index df64923e1..39a7ed1ed 100644 --- a/src/main/session-cache.ts +++ b/src/main/session-cache.ts @@ -3,10 +3,19 @@ import { join } from "path"; import { profileHome, getActiveProfileNameSync, safeWriteFile } from "./utils"; import Database from "better-sqlite3"; import { t } from "../shared/i18n"; +import { + isSessionTitleUniqueViolation, + MAX_SESSION_TITLE_LENGTH, + normalizeSessionTitle, + validateNormalizedSessionTitle, +} from "../shared/session-title"; import { getAppLocale } from "./locale"; import { getDbConnection } from "./db"; import { getSessionContextFolders } from "./session-context-folder-store"; +// Re-export for callers/docs that historically imported the cap from here. +export { MAX_SESSION_TITLE_LENGTH } from "../shared/session-title"; + /** * The session cache lives alongside its own profile's data so profiles * don't share a single cache file. The default profile keeps @@ -253,9 +262,6 @@ export function listCachedSessions(limit = 50, offset = 0): CachedSession[] { return cache.sessions.slice(offset, offset + limit); } -/** Hermes Agent caps session titles at 100 characters (schema + CLI). */ -export const MAX_SESSION_TITLE_LENGTH = 100; - /** * Persist a user-chosen session title to state.db, then mirror it into the * desktop sessions.json cache. @@ -267,16 +273,23 @@ export const MAX_SESSION_TITLE_LENGTH = 100; */ export function updateSessionTitle(sessionId: string, title: string): void { const locale = getAppLocale(); - const trimmed = title.trim().replace(/\s+/g, " "); - if (!trimmed) { - throw new Error(t("sessions.renameInvalid", locale)); - } - if (trimmed.length > MAX_SESSION_TITLE_LENGTH) { - throw new Error( - t("sessions.renameTooLong", locale, { - max: String(MAX_SESSION_TITLE_LENGTH), - }), - ); + const normalized = normalizeSessionTitle(title); + const validation = validateNormalizedSessionTitle(normalized); + switch (validation) { + case "empty": + throw new Error(t("sessions.renameInvalid", locale)); + case "too_long": + throw new Error( + t("sessions.renameTooLong", locale, { + max: String(MAX_SESSION_TITLE_LENGTH), + }), + ); + case null: + break; + default: { + const _exhaustive: never = validation; + throw new Error(String(_exhaustive)); + } } const db = getDbConnection(false); @@ -288,24 +301,25 @@ export function updateSessionTitle(sessionId: string, title: string): void { // we never partially update the JSON cache on a UNIQUE constraint failure. const conflict = db .prepare("SELECT id FROM sessions WHERE title = ? AND id != ?") - .get(trimmed, sessionId) as { id: string } | undefined; + .get(normalized, sessionId) as { id: string } | undefined; if (conflict) { - throw new Error(t("sessions.renameDuplicate", locale, { title: trimmed })); + throw new Error( + t("sessions.renameDuplicate", locale, { title: normalized }), + ); } let changes = 0; try { changes = db .prepare("UPDATE sessions SET title = ? WHERE id = ?") - .run(trimmed, sessionId).changes; + .run(normalized, sessionId).changes; } catch (err) { - const message = err instanceof Error ? err.message : String(err); - if (/UNIQUE|constraint/i.test(message)) { + if (isSessionTitleUniqueViolation(err)) { throw new Error( - t("sessions.renameDuplicate", locale, { title: trimmed }), + t("sessions.renameDuplicate", locale, { title: normalized }), ); } - throw err instanceof Error ? err : new Error(message); + throw err instanceof Error ? err : new Error(String(err)); } if (changes === 0) { @@ -315,7 +329,7 @@ export function updateSessionTitle(sessionId: string, title: string): void { const cache = readCache(); const idx = cache.sessions.findIndex((s) => s.id === sessionId); if (idx >= 0) { - cache.sessions[idx].title = trimmed; + cache.sessions[idx].title = normalized; writeCache(cache); } } diff --git a/src/renderer/src/screens/Layout/SidebarRecentSessions.tsx b/src/renderer/src/screens/Layout/SidebarRecentSessions.tsx index 4e858ad87..57567d87b 100644 --- a/src/renderer/src/screens/Layout/SidebarRecentSessions.tsx +++ b/src/renderer/src/screens/Layout/SidebarRecentSessions.tsx @@ -8,7 +8,6 @@ import { type RefObject, } from "react"; import { createPortal } from "react-dom"; -import toast from "react-hot-toast"; import { useI18n } from "../../components/useI18n"; import { ChevronDown, @@ -20,6 +19,7 @@ import { Pin, X, } from "../../assets/icons"; +import { confirmSessionRename } from "../Sessions/confirmSessionRename"; import SidebarSessionMenu, { type SidebarMenuProject, type SidebarMenuTarget, @@ -510,37 +510,27 @@ const SidebarRecentSessions = memo(function SidebarRecentSessions({ const confirmRename = useCallback( async (id: string, value: string): Promise => { - const trimmed = value.trim(); - const current = sessionsRef.current.find((s) => s.id === id); - if (!trimmed || trimmed === (current?.title ?? "")) { - cancelRename(); - return; - } - const previous = current?.title ?? ""; - // Optimistic local update; roll back if the durable write fails so a - // subsequent sync cannot resurrect a cache-only title. - setSessions((prev) => - prev.map((s) => (s.id === id ? { ...s, title: trimmed } : s)), - ); - try { - await window.hermesAPI.updateSessionTitle(id, trimmed); - if (editingIdRef.current === id) cancelRename(); - } catch (err) { - console.error("Failed to rename session", id, err); - setSessions((prev) => - prev.map((s) => (s.id === id ? { ...s, title: previous } : s)), - ); - const message = - err instanceof Error && err.message - ? err.message - : t("sessions.renameFailed"); - toast.error(message); - // Keep the inline editor open so the user can pick a unique name. - setTimeout(() => { - renameInputRef.current?.focus(); - renameInputRef.current?.select(); - }, 0); - } + const previous = + sessionsRef.current.find((s) => s.id === id)?.title ?? ""; + await confirmSessionRename({ + sessionId: id, + value, + currentTitle: previous, + isStillEditing: () => editingIdRef.current === id, + applyOptimistic: (title) => + setSessions((prev) => + prev.map((s) => (s.id === id ? { ...s, title } : s)), + ), + rollback: () => + setSessions((prev) => + prev.map((s) => (s.id === id ? { ...s, title: previous } : s)), + ), + clearEditing: cancelRename, + inputRef: renameInputRef, + fallbackErrorMessage: t("sessions.renameFailed"), + persist: (sessionId, title) => + window.hermesAPI.updateSessionTitle(sessionId, title), + }); }, [cancelRename, t], ); diff --git a/src/renderer/src/screens/Sessions/Sessions.tsx b/src/renderer/src/screens/Sessions/Sessions.tsx index 2575d16d9..6c68a5929 100644 --- a/src/renderer/src/screens/Sessions/Sessions.tsx +++ b/src/renderer/src/screens/Sessions/Sessions.tsx @@ -1,7 +1,7 @@ import { useEffect, useState, useRef, useCallback, useMemo, memo } from "react"; -import toast from "react-hot-toast"; import { Plus, Search, X, ChatBubble, Trash, Pencil } from "../../assets/icons"; import { useI18n } from "../../components/useI18n"; +import { confirmSessionRename } from "./confirmSessionRename"; interface CachedSession { id: string; @@ -394,63 +394,44 @@ function Sessions({ const confirmRename = useCallback( async (sessionId: string, newTitle: string): Promise => { - const trimmed = newTitle.trim(); - if (!trimmed) { - cancelRename(); - return; - } - // Capture old titles so we can roll back on failure. - let oldSessionTitle = ""; - let oldSearchResultTitle = ""; - // Optimistic update - setSessions((prev) => { - oldSessionTitle = prev.find((s) => s.id === sessionId)?.title ?? ""; - return prev.map((s) => - s.id === sessionId ? { ...s, title: trimmed } : s, - ); + const oldSessionTitle = + sessions.find((s) => s.id === sessionId)?.title ?? ""; + const oldSearchResultTitle = + searchResults.find((r) => r.sessionId === sessionId)?.title ?? ""; + await confirmSessionRename({ + sessionId, + value: newTitle, + currentTitle: oldSessionTitle, + isStillEditing: () => editingSessionIdRef.current === sessionId, + applyOptimistic: (title) => { + setSessions((prev) => + prev.map((s) => (s.id === sessionId ? { ...s, title } : s)), + ); + setSearchResults((prev) => + prev.map((r) => (r.sessionId === sessionId ? { ...r, title } : r)), + ); + }, + rollback: () => { + setSessions((prev) => + prev.map((s) => + s.id === sessionId ? { ...s, title: oldSessionTitle } : s, + ), + ); + setSearchResults((prev) => + prev.map((r) => + r.sessionId === sessionId + ? { ...r, title: oldSearchResultTitle } + : r, + ), + ); + }, + clearEditing: cancelRename, + inputRef: renameInputRef, + fallbackErrorMessage: t("sessions.renameFailed"), + persist: (id, title) => window.hermesAPI.updateSessionTitle(id, title), }); - setSearchResults((prev) => { - oldSearchResultTitle = - prev.find((r) => r.sessionId === sessionId)?.title ?? ""; - return prev.map((r) => - r.sessionId === sessionId ? { ...r, title: trimmed } : r, - ); - }); - try { - await window.hermesAPI.updateSessionTitle(sessionId, trimmed); - // Guard: only clear editing state if the user hasn't started editing - // a different session while this request was in flight. - if (editingSessionIdRef.current === sessionId) { - setEditingSessionId(null); - setEditingTitle(""); - } - } catch (err) { - console.error("Failed to rename session", sessionId, err); - // Rollback optimistic update - setSessions((prev) => - prev.map((s) => - s.id === sessionId ? { ...s, title: oldSessionTitle } : s, - ), - ); - setSearchResults((prev) => - prev.map((r) => - r.sessionId === sessionId - ? { ...r, title: oldSearchResultTitle } - : r, - ), - ); - const message = - err instanceof Error && err.message - ? err.message - : t("sessions.renameFailed"); - toast.error(message); - setTimeout(() => { - renameInputRef.current?.focus(); - renameInputRef.current?.select(); - }, 0); - } }, - [cancelRename, t], + [cancelRename, searchResults, sessions, t], ); const cancelDelete = useCallback((): void => { diff --git a/src/renderer/src/screens/Sessions/confirmSessionRename.ts b/src/renderer/src/screens/Sessions/confirmSessionRename.ts new file mode 100644 index 000000000..eacee2396 --- /dev/null +++ b/src/renderer/src/screens/Sessions/confirmSessionRename.ts @@ -0,0 +1,56 @@ +import type { RefObject } from "react"; +import toast from "react-hot-toast"; +import { normalizeSessionTitle } from "../../../../shared/session-title"; + +export interface ConfirmSessionRenameOptions { + sessionId: string; + value: string; + currentTitle: string; + /** True when this session is still the one being edited. */ + isStillEditing: () => boolean; + applyOptimistic: (title: string) => void; + rollback: () => void; + clearEditing: () => void; + inputRef: RefObject; + fallbackErrorMessage: string; + persist: (sessionId: string, title: string) => Promise; +} + +export type ConfirmSessionRenameResult = "cancelled" | "saved" | "failed"; + +/** + * Shared inline-rename flow for the sidebar and Sessions modal: normalize, + * no-op cancel, optimistic update, durable persist, then clear editing on + * success or toast + rollback + refocus on failure. + */ +export async function confirmSessionRename( + opts: ConfirmSessionRenameOptions, +): Promise { + const normalized = normalizeSessionTitle(opts.value); + if (!normalized || normalized === opts.currentTitle) { + opts.clearEditing(); + return "cancelled"; + } + + opts.applyOptimistic(normalized); + try { + await opts.persist(opts.sessionId, normalized); + if (opts.isStillEditing()) opts.clearEditing(); + return "saved"; + } catch (err) { + console.error("Failed to rename session", opts.sessionId, err); + opts.rollback(); + const message = + err instanceof Error && err.message + ? err.message + : opts.fallbackErrorMessage; + toast.error(message); + if (opts.isStillEditing()) { + setTimeout(() => { + opts.inputRef.current?.focus(); + opts.inputRef.current?.select(); + }, 0); + } + return "failed"; + } +} diff --git a/src/shared/session-title.test.ts b/src/shared/session-title.test.ts new file mode 100644 index 000000000..9c9ea1306 --- /dev/null +++ b/src/shared/session-title.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it } from "vitest"; +import { + isSessionTitleUniqueViolation, + MAX_SESSION_TITLE_LENGTH, + normalizeSessionTitle, + validateNormalizedSessionTitle, +} from "./session-title"; + +describe("normalizeSessionTitle", () => { + it("trims and collapses internal whitespace", () => { + expect(normalizeSessionTitle(" Hello World ")).toBe("Hello World"); + }); +}); + +describe("validateNormalizedSessionTitle", () => { + it("rejects empty and oversized titles", () => { + expect(validateNormalizedSessionTitle("")).toBe("empty"); + expect( + validateNormalizedSessionTitle("x".repeat(MAX_SESSION_TITLE_LENGTH + 1)), + ).toBe("too_long"); + expect(validateNormalizedSessionTitle("ok")).toBeNull(); + }); +}); + +describe("isSessionTitleUniqueViolation", () => { + it("matches sessions.title UNIQUE failures only", () => { + expect( + isSessionTitleUniqueViolation( + new Error("UNIQUE constraint failed: sessions.title"), + ), + ).toBe(true); + + const coded = new Error( + "UNIQUE constraint failed: sessions.title", + ) as Error & { code: string }; + coded.code = "SQLITE_CONSTRAINT_UNIQUE"; + expect(isSessionTitleUniqueViolation(coded)).toBe(true); + + expect( + isSessionTitleUniqueViolation( + new Error("CHECK constraint failed: message_count"), + ), + ).toBe(false); + }); +}); diff --git a/src/shared/session-title.ts b/src/shared/session-title.ts new file mode 100644 index 000000000..d2b2d6e2c --- /dev/null +++ b/src/shared/session-title.ts @@ -0,0 +1,39 @@ +/** + * Session title policy shared by main (state.db writes) and the renderer + * (optimistic UI). Mirrors Hermes Agent: unique non-NULL titles, 100-char + * max, whitespace collapsed. + */ + +/** Hermes Agent caps session titles at 100 characters (schema + CLI). */ +export const MAX_SESSION_TITLE_LENGTH = 100; + +/** Trim and collapse internal whitespace so UI and DB agree on the title. */ +export function normalizeSessionTitle(title: string): string { + return title.trim().replace(/\s+/g, " "); +} + +export type SessionTitleValidationError = "empty" | "too_long"; + +/** + * Validate an already-normalized title. Returns an error code, or null when + * the title is acceptable for a durable write. + */ +export function validateNormalizedSessionTitle( + title: string, +): SessionTitleValidationError | null { + if (!title) return "empty"; + if (title.length > MAX_SESSION_TITLE_LENGTH) return "too_long"; + return null; +} + +/** True when a SQLite error is the sessions.title UNIQUE constraint. */ +export function isSessionTitleUniqueViolation(err: unknown): boolean { + const message = err instanceof Error ? err.message : String(err ?? ""); + if (/UNIQUE constraint failed:\s*sessions\.title/i.test(message)) { + return true; + } + if (!err || typeof err !== "object") return false; + const code = "code" in err ? String((err as { code: unknown }).code) : ""; + // better-sqlite3: SQLITE_CONSTRAINT_UNIQUE; message usually names the column. + return code === "SQLITE_CONSTRAINT_UNIQUE" && /title/i.test(message); +}