diff --git a/lat.md/sidebar-navigation.md b/lat.md/sidebar-navigation.md index 518091eff..591d8a791 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. + +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. ## Full-list modal diff --git a/src/main/session-cache.ts b/src/main/session-cache.ts index b592ec342..39a7ed1ed 100644 --- a/src/main/session-cache.ts +++ b/src/main/session-cache.ts @@ -1,17 +1,21 @@ 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 { + 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 @@ -258,31 +262,76 @@ export function listCachedSessions(limit = 50, offset = 0): CachedSession[] { return cache.sessions.slice(offset, offset + limit); } -// Update title for a specific session +/** + * 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 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); + 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(normalized, sessionId) as { id: string } | undefined; + if (conflict) { + throw new Error( + t("sessions.renameDuplicate", locale, { title: normalized }), + ); + } + + let changes = 0; + try { + changes = db + .prepare("UPDATE sessions SET title = ? WHERE id = ?") + .run(normalized, sessionId).changes; + } catch (err) { + if (isSessionTitleUniqueViolation(err)) { + throw new Error( + t("sessions.renameDuplicate", locale, { title: normalized }), + ); + } + throw err instanceof Error ? err : new Error(String(err)); + } + + 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 = normalized; 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..57567d87b 100644 --- a/src/renderer/src/screens/Layout/SidebarRecentSessions.tsx +++ b/src/renderer/src/screens/Layout/SidebarRecentSessions.tsx @@ -19,6 +19,7 @@ import { Pin, X, } from "../../assets/icons"; +import { confirmSessionRename } from "../Sessions/confirmSessionRename"; import SidebarSessionMenu, { type SidebarMenuProject, type SidebarMenuTarget, @@ -509,28 +510,29 @@ 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 write fails. - setSessions((prev) => - prev.map((s) => (s.id === id ? { ...s, title: trimmed } : s)), - ); - if (editingIdRef.current === id) cancelRename(); - try { - await window.hermesAPI.updateSessionTitle(id, trimmed); - } catch (err) { - console.error("Failed to rename session", id, err); - setSessions((prev) => - prev.map((s) => (s.id === id ? { ...s, title: previous } : s)), - ); - } + 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], + [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..6c68a5929 100644 --- a/src/renderer/src/screens/Sessions/Sessions.tsx +++ b/src/renderer/src/screens/Sessions/Sessions.tsx @@ -1,6 +1,7 @@ import { useEffect, useState, useRef, useCallback, useMemo, memo } from "react"; import { Plus, Search, X, ChatBubble, Trash, Pencil } from "../../assets/icons"; import { useI18n } from "../../components/useI18n"; +import { confirmSessionRename } from "./confirmSessionRename"; interface CachedSession { id: string; @@ -393,54 +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); - } 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, - ), - ); - } - // 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(""); - } }, - [cancelRename], + [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/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/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); +} 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"); + }); +});