diff --git a/web/server/index.ts b/web/server/index.ts index 492ea7232..e1fedb1c0 100644 --- a/web/server/index.ts +++ b/web/server/index.ts @@ -64,10 +64,12 @@ import { recreateWorktreeIfMissing } from "./migration.js"; import { access } from "node:fs/promises"; import { RelaunchQueue } from "./relaunch-queue.js"; import { + NAMER_TRIGGER_SOURCES, shouldAllowUserMessageOverrideOnNameMismatch, type NamerMutationRecord, type NamerTriggerSource, } from "./session-namer-arbitration.js"; +import { formatAutoNamerSkipReason, getAutoNamerSkipReason } from "./session-namer-guard.js"; import type { SocketData } from "./ws-bridge.js"; import type { ServerWebSocket } from "bun"; @@ -401,7 +403,7 @@ function endNamerCall(sessionId: string, source: NamerTriggerSource, controller: /** Cancel ALL in-flight namer calls for a session (all trigger sources). */ function cancelAllNamersForSession(sessionId: string): void { - for (const source of ["user_message", "turn_completed", "agent_paused"] as const) { + for (const source of NAMER_TRIGGER_SOURCES) { const key = getNamerKey(sessionId, source); const ctrl = inFlightNamer.get(key); if (ctrl) { @@ -471,6 +473,25 @@ async function isQuestOwningSessionName(sessionId: string): Promise { ); } +async function shouldSkipAutoNamer( + sessionId: string, + source: NamerTriggerSource, + stage: "start" | "apply", +): Promise { + const reason = await getAutoNamerSkipReason({ + isAutoNamerEnabled: () => getSettings().autoNamerEnabled, + isNoAutoNameSession: () => !!launcher.getSession(sessionId)?.noAutoName, + isUserNamed: () => sessionNames.isUserNamed(sessionId), + isQuestOwningName: () => isQuestOwningSessionName(sessionId), + }); + if (!reason) return false; + + const verb = stage === "apply" ? "Discarding" : "Skipping"; + const noun = stage === "apply" ? `${source} namer result` : `${source} namer`; + console.log(`[session-namer] ${verb} ${noun} for ${sessionId} (${formatAutoNamerSkipReason(reason)})`); + return true; +} + /** Apply a naming result: set name, broadcast, add task entry. Shared by all triggers. */ async function applyNamingResult( sessionId: string, @@ -479,11 +500,7 @@ async function applyNamingResult( history: import("./session-types.js").BrowserIncomingMessage[], source: NamerTriggerSource, ): Promise { - // Re-check: quest may have been claimed while the namer was in-flight - if (await isQuestOwningSessionName(sessionId)) { - console.log(`[session-namer] Discarding namer result for ${sessionId} (quest owns session name)`); - return; - } + if (await shouldSkipAutoNamer(sessionId, source, "apply")) return; // Merge keywords regardless of naming action if (result.keywords?.length) { mergeSessionKeywords(sessionId, result.keywords); @@ -593,15 +610,7 @@ wsBridge.onSessionNamedByQuest = (sessionId, title) => { // Continuous session auto-naming via Claude Haiku (triggered on each user message) wsBridge.onUserMessage = async (sessionId, history, cwd, wasGenerating) => { - // Suppress auto-namer when disabled in settings - if (!getSettings().autoNamerEnabled) return; - // Suppress auto-namer for sessions with noAutoName flag (e.g. temporary reviewer sessions) - if (launcher.getSession(sessionId)?.noAutoName) return; - // Suppress auto-namer while a quest owns the session name - if (await isQuestOwningSessionName(sessionId)) { - console.log(`[session-namer] Skipping user-message namer for ${sessionId} (quest owns session name)`); - return; - } + if (await shouldSkipAutoNamer(sessionId, "user_message", "start")) return; const currentName = sessionNames.getName(sessionId); const isRandomName = isRandomSessionName(currentName); const isFirstEvaluation = !autoNamingEvaluated.has(sessionId); @@ -629,11 +638,7 @@ wsBridge.onUserMessage = async (sessionId, history, cwd, wasGenerating) => { const result = await generateFirstName(sessionId, history, cwd, { signal, isGenerating, claimedQuest }); if (signal.aborted) return; if (!result || result.action !== "name") return; - // Re-check: quest may have been claimed while we were generating - if (await isQuestOwningSessionName(sessionId)) { - console.log(`[session-namer] Discarding first-name result for ${sessionId} (quest owns session name)`); - return; - } + if (await shouldSkipAutoNamer(sessionId, "user_message", "apply")) return; // Don't overwrite if user renamed while we were generating const freshName = sessionNames.getName(sessionId); if (freshName && !isRandomSessionName(freshName)) return; @@ -667,12 +672,7 @@ wsBridge.onUserMessage = async (sessionId, history, cwd, wasGenerating) => { // The agent has done meaningful research/work to produce the plan, providing // rich context for naming — and it's a natural breakpoint before execution. wsBridge.onAgentPaused = async (sessionId, history, cwd) => { - if (!getSettings().autoNamerEnabled) return; - if (launcher.getSession(sessionId)?.noAutoName) return; - if (await isQuestOwningSessionName(sessionId)) { - console.log(`[session-namer] Skipping agent-paused namer for ${sessionId} (quest owns session name)`); - return; - } + if (await shouldSkipAutoNamer(sessionId, "agent_paused", "start")) return; const currentName = sessionNames.getName(sessionId); if (!currentName) return; @@ -693,12 +693,7 @@ wsBridge.onAgentPaused = async (sessionId, history, cwd) => { // The turn-completed namer runs independently from user-message naming. // User-message outcomes are preferred when both produce competing revisions. wsBridge.onTurnCompleted = async (sessionId, history, cwd) => { - if (!getSettings().autoNamerEnabled) return; - if (launcher.getSession(sessionId)?.noAutoName) return; - if (await isQuestOwningSessionName(sessionId)) { - console.log(`[session-namer] Skipping turn-completed namer for ${sessionId} (quest owns session name)`); - return; - } + if (await shouldSkipAutoNamer(sessionId, "turn_completed", "start")) return; const currentName = sessionNames.getName(sessionId); if (!currentName) return; diff --git a/web/server/routes.get-api-images-sessionid-imageid-thumb.test.ts b/web/server/routes.get-api-images-sessionid-imageid-thumb.test.ts index 0c8323cae..c10ca063a 100644 --- a/web/server/routes.get-api-images-sessionid-imageid-thumb.test.ts +++ b/web/server/routes.get-api-images-sessionid-imageid-thumb.test.ts @@ -552,7 +552,10 @@ async function parseSSE(res: Response): Promise<{ event: string; data: string }[ return events; } -describe("GET /api/images/:sessionId/:imageId/thumb", () => { +// The image thumb route uses Bun.file() internally, so these tests only work on Bun. +const describeBun = typeof globalThis.Bun !== "undefined" ? describe : describe.skip; + +describeBun("GET /api/images/:sessionId/:imageId/thumb", () => { it("serves the real thumbnail with immutable caching when it exists", async () => { const tempRoot = await mkdtemp(join(tmpdir(), "routes-thumb-")); const thumbPath = join(tempRoot, "thumb.jpeg"); diff --git a/web/server/routes.patch-api-sessions-id-name.test.ts b/web/server/routes.patch-api-sessions-id-name.test.ts index 3eafa1fe5..72c4c5e34 100644 --- a/web/server/routes.patch-api-sessions-id-name.test.ts +++ b/web/server/routes.patch-api-sessions-id-name.test.ts @@ -87,6 +87,9 @@ vi.mock("./git-utils.js", () => ({ vi.mock("./session-names.js", () => ({ getName: vi.fn(() => undefined), setName: vi.fn(), + setUserNamed: vi.fn(), + isUserNamed: vi.fn(() => false), + clearUserNamed: vi.fn(), getAllNames: vi.fn(() => ({})), removeName: vi.fn(), getNextLeaderNumber: vi.fn(() => 1), @@ -548,6 +551,8 @@ describe("PATCH /api/sessions/:id/name", () => { const json = await res.json(); expect(json).toEqual({ ok: true, name: "Fix auth bug" }); expect(sessionNames.setName).toHaveBeenCalledWith("s1", "Fix auth bug"); + // Verify the session is marked as user-named to prevent auto-namer overwriting + expect(sessionNames.setUserNamed).toHaveBeenCalledWith("s1"); }); it("trims whitespace from name", async () => { diff --git a/web/server/routes/sessions.ts b/web/server/routes/sessions.ts index fbbae06e5..1ebff2369 100644 --- a/web/server/routes/sessions.ts +++ b/web/server/routes/sessions.ts @@ -1203,6 +1203,7 @@ export function createSessionsRoutes(ctx: RouteContext) { const session = launcher.getSession(id); if (!session) return c.json({ error: "Session not found" }, 404); sessionNames.setName(id, body.name.trim()); + sessionNames.setUserNamed(id); wsBridge.broadcastToSession(id, { type: "session_update", session: { name: body.name.trim() } } as any); return c.json({ ok: true, name: body.name.trim() }); }); diff --git a/web/server/session-namer-arbitration.ts b/web/server/session-namer-arbitration.ts index 91f4d77bc..dd3ac08bc 100644 --- a/web/server/session-namer-arbitration.ts +++ b/web/server/session-namer-arbitration.ts @@ -1,4 +1,6 @@ -export type NamerTriggerSource = "user_message" | "turn_completed" | "agent_paused"; +export const NAMER_TRIGGER_SOURCES = ["user_message", "turn_completed", "agent_paused"] as const; + +export type NamerTriggerSource = (typeof NAMER_TRIGGER_SOURCES)[number]; export interface NamerMutationRecord { source: NamerTriggerSource; diff --git a/web/server/session-namer-guard.test.ts b/web/server/session-namer-guard.test.ts new file mode 100644 index 000000000..6633ee54b --- /dev/null +++ b/web/server/session-namer-guard.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it, vi } from "vitest"; +import { NAMER_TRIGGER_SOURCES } from "./session-namer-arbitration.js"; +import { getAutoNamerSkipReason, type AutoNamerGuardChecks } from "./session-namer-guard.js"; + +function checks(overrides: Partial = {}): AutoNamerGuardChecks { + return { + isAutoNamerEnabled: vi.fn(() => true), + isNoAutoNameSession: vi.fn(() => false), + isUserNamed: vi.fn(() => false), + isQuestOwningName: vi.fn(async () => false), + ...overrides, + } satisfies AutoNamerGuardChecks; +} + +describe("getAutoNamerSkipReason", () => { + it("uses user-named protection for every automatic namer trigger", async () => { + for (const _source of NAMER_TRIGGER_SOURCES) { + const guardChecks = checks({ isUserNamed: vi.fn(() => true) }); + + await expect(getAutoNamerSkipReason(guardChecks)).resolves.toBe("user_named"); + expect(guardChecks.isQuestOwningName).not.toHaveBeenCalled(); + } + }); + + it("re-checks current manual-name state when called again for result application", async () => { + let userNamed = false; + const guardChecks = checks({ isUserNamed: vi.fn(() => userNamed) }); + + await expect(getAutoNamerSkipReason(guardChecks)).resolves.toBeNull(); + + userNamed = true; + + await expect(getAutoNamerSkipReason(guardChecks)).resolves.toBe("user_named"); + }); + + it("skips before manual-name checks when auto-namer is disabled", async () => { + const guardChecks = checks({ isAutoNamerEnabled: vi.fn(() => false) }); + + await expect(getAutoNamerSkipReason(guardChecks)).resolves.toBe("disabled"); + expect(guardChecks.isNoAutoNameSession).not.toHaveBeenCalled(); + expect(guardChecks.isUserNamed).not.toHaveBeenCalled(); + expect(guardChecks.isQuestOwningName).not.toHaveBeenCalled(); + }); + + it("skips before manual-name checks for noAutoName sessions", async () => { + const guardChecks = checks({ isNoAutoNameSession: vi.fn(() => true) }); + + await expect(getAutoNamerSkipReason(guardChecks)).resolves.toBe("no_auto_name"); + expect(guardChecks.isUserNamed).not.toHaveBeenCalled(); + expect(guardChecks.isQuestOwningName).not.toHaveBeenCalled(); + }); + + it("falls back to quest-owned protection when no earlier guard applies", async () => { + const guardChecks = checks({ isQuestOwningName: vi.fn(async () => true) }); + + await expect(getAutoNamerSkipReason(guardChecks)).resolves.toBe("quest_owned"); + }); +}); diff --git a/web/server/session-namer-guard.ts b/web/server/session-namer-guard.ts new file mode 100644 index 000000000..53272c2a6 --- /dev/null +++ b/web/server/session-namer-guard.ts @@ -0,0 +1,29 @@ +export type AutoNamerSkipReason = "disabled" | "no_auto_name" | "user_named" | "quest_owned"; + +export interface AutoNamerGuardChecks { + isAutoNamerEnabled: () => boolean; + isNoAutoNameSession: () => boolean; + isUserNamed: () => boolean; + isQuestOwningName: () => Promise; +} + +export async function getAutoNamerSkipReason(checks: AutoNamerGuardChecks): Promise { + if (!checks.isAutoNamerEnabled()) return "disabled"; + if (checks.isNoAutoNameSession()) return "no_auto_name"; + if (checks.isUserNamed()) return "user_named"; + if (await checks.isQuestOwningName()) return "quest_owned"; + return null; +} + +export function formatAutoNamerSkipReason(reason: AutoNamerSkipReason): string { + switch (reason) { + case "disabled": + return "auto-namer disabled"; + case "no_auto_name": + return "session is marked noAutoName"; + case "user_named": + return "manually renamed by user"; + case "quest_owned": + return "quest owns session name"; + } +} diff --git a/web/server/session-names.test.ts b/web/server/session-names.test.ts index 942f40b3f..d49d58051 100644 --- a/web/server/session-names.test.ts +++ b/web/server/session-names.test.ts @@ -8,6 +8,9 @@ import { getAllNames, removeName, getNextLeaderNumber, + setUserNamed, + isUserNamed, + clearUserNamed, _resetForTest, _flushForTest, } from "./session-names.js"; @@ -39,7 +42,7 @@ describe("session-names", () => { await _flushForTest(); const raw = readFileSync(join(tempDir, "session-names.json"), "utf-8"); const data = JSON.parse(raw); - expect(data).toEqual({ names: { s1: "My Session" }, leaderCounter: 0 }); + expect(data).toEqual({ names: { s1: "My Session" }, leaderCounter: 0, userNamed: [] }); }); it("getAllNames returns a copy of all names", () => { @@ -52,13 +55,16 @@ describe("session-names", () => { expect(getName("s3")).toBeUndefined(); }); - it("removeName deletes a name", async () => { + it("removeName deletes a name and clears userNamed flag", async () => { setName("s1", "Session One"); + setUserNamed("s1"); + expect(isUserNamed("s1")).toBe(true); removeName("s1"); expect(getName("s1")).toBeUndefined(); + expect(isUserNamed("s1")).toBe(false); await _flushForTest(); const raw = readFileSync(join(tempDir, "session-names.json"), "utf-8"); - expect(JSON.parse(raw)).toEqual({ names: {}, leaderCounter: 0 }); + expect(JSON.parse(raw)).toEqual({ names: {}, leaderCounter: 0, userNamed: [] }); }); it("overwrites existing name", () => { @@ -113,7 +119,7 @@ describe("leader counter", () => { const raw = readFileSync(join(tempDir, "session-names.json"), "utf-8"); const data = JSON.parse(raw); - expect(data).toEqual({ names: { s1: "Worker 1" }, leaderCounter: 1 }); + expect(data).toEqual({ names: { s1: "Worker 1" }, leaderCounter: 1, userNamed: [] }); }); it("loads counter from old-format file as 0 (backwards compat)", () => { @@ -125,3 +131,44 @@ describe("leader counter", () => { expect(getNextLeaderNumber()).toBe(1); }); }); + +describe("userNamed flag", () => { + it("isUserNamed returns false by default", () => { + expect(isUserNamed("s1")).toBe(false); + }); + + it("setUserNamed + isUserNamed round-trip", () => { + setUserNamed("s1"); + expect(isUserNamed("s1")).toBe(true); + expect(isUserNamed("s2")).toBe(false); + }); + + it("clearUserNamed removes the flag", () => { + setUserNamed("s1"); + clearUserNamed("s1"); + expect(isUserNamed("s1")).toBe(false); + }); + + it("persists userNamed to disk and survives restart", async () => { + setUserNamed("s1"); + setUserNamed("s2"); + await _flushForTest(); + + const raw = readFileSync(join(tempDir, "session-names.json"), "utf-8"); + const data = JSON.parse(raw); + expect(data.userNamed).toEqual(expect.arrayContaining(["s1", "s2"])); + + // Simulate restart + _resetForTest(join(tempDir, "session-names.json")); + expect(isUserNamed("s1")).toBe(true); + expect(isUserNamed("s2")).toBe(true); + }); + + it("backwards-compatible: loads file without userNamed field", () => { + // Old format without userNamed + writeFileSync(join(tempDir, "session-names.json"), JSON.stringify({ names: { s1: "Test" }, leaderCounter: 1 })); + _resetForTest(join(tempDir, "session-names.json")); + expect(isUserNamed("s1")).toBe(false); + expect(getName("s1")).toBe("Test"); + }); +}); diff --git a/web/server/session-names.ts b/web/server/session-names.ts index 77037feb7..22f5bab27 100644 --- a/web/server/session-names.ts +++ b/web/server/session-names.ts @@ -18,10 +18,12 @@ const DEFAULT_PATH = join(homedir(), ".companion", "session-names.json"); interface PersistedData { names: Record; leaderCounter: number; + userNamed?: string[]; } let names: Record = {}; let leaderCounter = 0; +let userNamed: Set = new Set(); let loaded = false; let filePath = DEFAULT_PATH; let _pendingWrite: Promise = Promise.resolve(); @@ -37,21 +39,24 @@ function ensureLoaded(): void { // New format names = parsed.names as Record; leaderCounter = typeof parsed.leaderCounter === "number" ? parsed.leaderCounter : 0; + userNamed = new Set(Array.isArray(parsed.userNamed) ? parsed.userNamed : []); } else { // Old format: flat Record names = parsed as Record; leaderCounter = 0; + userNamed = new Set(); } } } catch { names = {}; leaderCounter = 0; + userNamed = new Set(); } loaded = true; } function persist(): void { - const data: PersistedData = { names, leaderCounter }; + const data: PersistedData = { names, leaderCounter, userNamed: [...userNamed] }; const json = JSON.stringify(data, null, 2); const path = filePath; mkdirSync(dirname(path), { recursive: true }); // sync-ok: cold path, ensure dir exists @@ -80,6 +85,7 @@ export function getAllNames(): Record { export function removeName(sessionId: string): void { ensureLoaded(); delete names[sessionId]; + userNamed.delete(sessionId); persist(); } @@ -91,6 +97,26 @@ export function getNextLeaderNumber(): number { return leaderCounter; } +/** Mark a session as manually named by the user (prevents auto-namer from overwriting). */ +export function setUserNamed(sessionId: string): void { + ensureLoaded(); + userNamed.add(sessionId); + persist(); +} + +/** Check if a session was manually named by the user. */ +export function isUserNamed(sessionId: string): boolean { + ensureLoaded(); + return userNamed.has(sessionId); +} + +/** Clear the user-named flag (e.g. when a session is deleted). */ +export function clearUserNamed(sessionId: string): void { + ensureLoaded(); + userNamed.delete(sessionId); + persist(); +} + /** Wait for any pending async writes to complete. Test-only. */ export function _flushForTest(): Promise { return _pendingWrite; @@ -100,6 +126,7 @@ export function _flushForTest(): Promise { export function _resetForTest(customPath?: string): void { names = {}; leaderCounter = 0; + userNamed = new Set(); loaded = false; filePath = customPath || DEFAULT_PATH; _pendingWrite = Promise.resolve(); diff --git a/web/src/components/Sidebar.session-row.test.tsx b/web/src/components/Sidebar.session-row.test.tsx new file mode 100644 index 000000000..0ea9b4458 --- /dev/null +++ b/web/src/components/Sidebar.session-row.test.tsx @@ -0,0 +1,432 @@ +// @vitest-environment jsdom +import { render, screen, fireEvent } from "@testing-library/react"; +import "@testing-library/jest-dom"; +import type { SessionNotification, SessionState, SdkSessionInfo } from "../types.js"; + +const mockConnectSession = vi.fn(); +const mockConnectAllSessions = vi.fn(); +const mockDisconnectSession = vi.fn(); +const scrollTargetSessionIds: string[] = []; +const mockScrollIntoView = vi.fn(function (this: Element) { + scrollTargetSessionIds.push((this as HTMLElement).dataset.sessionId ?? ""); +}); + +vi.mock("../ws.js", () => ({ + connectSession: (...args: unknown[]) => mockConnectSession(...args), + connectAllSessions: (...args: unknown[]) => mockConnectAllSessions(...args), + disconnectSession: (...args: unknown[]) => mockDisconnectSession(...args), +})); + +vi.mock("../utils/pending-creation.js", () => ({ + cancelPendingCreation: vi.fn(), +})); + +const mockApi = { + listSessions: vi.fn().mockResolvedValue([]), + searchSessions: vi.fn().mockResolvedValue({ query: "", tookMs: 0, totalMatches: 0, results: [] }), + deleteSession: vi.fn().mockResolvedValue({}), + archiveSession: vi.fn().mockResolvedValue({}), + archiveGroup: vi.fn().mockResolvedValue({ ok: true, archived: 1, failed: 0 }), + unarchiveSession: vi.fn().mockResolvedValue({}), + createTreeGroup: vi.fn().mockResolvedValue({ ok: true, group: { id: "group-2", name: "Group 2" } }), + assignSessionToTreeGroup: vi.fn().mockResolvedValue({ ok: true }), + assignSessionsToTreeGroup: vi.fn().mockResolvedValue({ ok: true }), + herdWorkerToLeader: vi + .fn() + .mockResolvedValue({ herded: ["worker-1"], notFound: [], conflicts: [], reassigned: [], leaders: [] }), + getSettings: vi.fn().mockResolvedValue({ serverName: "" }), + updateSettings: vi.fn().mockResolvedValue({}), + getTreeGroups: vi + .fn() + .mockResolvedValue({ groups: [{ id: "default", name: "Default" }], assignments: {}, nodeOrder: {} }), +}; + +vi.mock("../api.js", () => ({ + api: { + listSessions: (...args: unknown[]) => mockApi.listSessions(...args), + searchSessions: (...args: unknown[]) => mockApi.searchSessions(...args), + deleteSession: (...args: unknown[]) => mockApi.deleteSession(...args), + archiveSession: (...args: unknown[]) => mockApi.archiveSession(...args), + archiveGroup: (...args: unknown[]) => mockApi.archiveGroup(...args), + unarchiveSession: (...args: unknown[]) => mockApi.unarchiveSession(...args), + createTreeGroup: (...args: unknown[]) => mockApi.createTreeGroup(...args), + assignSessionToTreeGroup: (...args: unknown[]) => mockApi.assignSessionToTreeGroup(...args), + assignSessionsToTreeGroup: (...args: unknown[]) => mockApi.assignSessionsToTreeGroup(...args), + herdWorkerToLeader: (...args: unknown[]) => mockApi.herdWorkerToLeader(...args), + getSettings: (...args: unknown[]) => mockApi.getSettings(...args), + updateSettings: (...args: unknown[]) => mockApi.updateSettings(...args), + getTreeGroups: (...args: unknown[]) => mockApi.getTreeGroups(...args), + }, +})); + +vi.mock("../utils/copy-utils.js", () => ({ + writeClipboardText: vi.fn().mockResolvedValue(undefined), +})); + +interface MockStoreState { + sessions: Map; + sdkSessions: SdkSessionInfo[]; + currentSessionId: string | null; + cliConnected: Map; + cliDisconnectReason: Map; + sessionStatus: Map; + sessionNames: Map; + sessionPreviews: Map; + sessionPreviewUpdatedAt: Map; + sessionTaskPreview: Map; + sessionTaskHistory: Map>; + sessionKeywords: Map; + sessionNotifications: Map; + recentlyRenamed: Set; + questNamedSessions: Set; + pendingPermissions: Map>; + sessionAttention: Map; + diffFileStats: Map>; + shortcutSettings: { + enabled: boolean; + preset: "standard" | "vscode-light" | "vim-light"; + overrides: Record; + }; + searchPreviewSessionId: string | null; + sessionInfoOpenSessionId: string | null; + reorderMode: boolean; + setReorderMode: ReturnType; + sessionSortMode: "created" | "activity"; + setSessionSortMode: ReturnType; + pendingSessions: Map; + serverName: string; + treeGroups: Array<{ id: string; name: string }>; + treeAssignments: Map; + treeNodeOrder: Map; + collapsedTreeGroups: Set; + collapsedTreeNodes: Set; + expandedHerdNodes: Set; + toggleTreeGroupCollapse: ReturnType; + toggleTreeNodeCollapse: ReturnType; + toggleHerdNodeExpand: ReturnType; + setServerName: ReturnType; + setSearchPreviewSessionId: ReturnType; + setCurrentSession: ReturnType; + removeSession: ReturnType; + newSession: ReturnType; + setSidebarOpen: ReturnType; + setSessionName: ReturnType; + setSessionPreview: ReturnType; + setSessionTaskHistory: ReturnType; + setSessionKeywords: ReturnType; + markRecentlyRenamed: ReturnType; + clearRecentlyRenamed: ReturnType; + setSdkSessions: ReturnType; + closeTerminal: ReturnType; + openNewSessionModal: ReturnType; + closeNewSessionModal: ReturnType; + markSessionViewed: ReturnType; + markAllSessionsViewed: ReturnType; + markSessionUnread: ReturnType; + clearSessionAttention: ReturnType; + setTreeGroups: ReturnType; + focusComposer: ReturnType; +} + +function makeSession(id: string, overrides: Partial = {}): SessionState { + return { + session_id: id, + model: "claude-sonnet-4-5-20250929", + cwd: "/home/user/projects/myapp", + tools: [], + permissionMode: "default", + claude_code_version: "1.0", + mcp_servers: [], + agents: [], + slash_commands: [], + skills: [], + total_cost_usd: 0, + num_turns: 0, + context_used_percent: 0, + is_compacting: false, + git_branch: "", + is_worktree: false, + is_containerized: false, + repo_root: "", + git_ahead: 0, + git_behind: 0, + total_lines_added: 0, + total_lines_removed: 0, + ...overrides, + }; +} + +function makeSdkSession(id: string, overrides: Partial = {}): SdkSessionInfo { + return { + sessionId: id, + state: "connected", + cwd: "/home/user/projects/myapp", + createdAt: Date.now(), + archived: false, + ...overrides, + }; +} + +let mockState: MockStoreState; + +function createMockState(overrides: Partial = {}): MockStoreState { + return { + sessions: new Map(), + sdkSessions: [], + currentSessionId: null, + cliConnected: new Map(), + cliDisconnectReason: new Map(), + sessionStatus: new Map(), + sessionNames: new Map(), + sessionPreviews: new Map(), + sessionPreviewUpdatedAt: new Map(), + sessionTaskPreview: new Map(), + sessionTaskHistory: new Map(), + sessionKeywords: new Map(), + sessionNotifications: new Map(), + recentlyRenamed: new Set(), + questNamedSessions: new Set(), + pendingPermissions: new Map(), + sessionAttention: new Map(), + diffFileStats: new Map(), + shortcutSettings: { enabled: false, preset: "standard", overrides: {} }, + searchPreviewSessionId: null, + sessionInfoOpenSessionId: null, + reorderMode: false, + setReorderMode: vi.fn(), + sessionSortMode: "created", + setSessionSortMode: vi.fn(), + pendingSessions: new Map(), + serverName: "", + treeGroups: [], + treeAssignments: new Map(), + treeNodeOrder: new Map(), + collapsedTreeGroups: new Set(), + collapsedTreeNodes: new Set(), + expandedHerdNodes: new Set(), + toggleTreeGroupCollapse: vi.fn(), + toggleTreeNodeCollapse: vi.fn(), + toggleHerdNodeExpand: vi.fn(), + setServerName: vi.fn(), + setSearchPreviewSessionId: vi.fn(), + setCurrentSession: vi.fn(), + removeSession: vi.fn(), + newSession: vi.fn(), + setSidebarOpen: vi.fn(), + setSessionName: vi.fn(), + setSessionPreview: vi.fn(), + setSessionTaskHistory: vi.fn(), + setSessionKeywords: vi.fn(), + markRecentlyRenamed: vi.fn(), + clearRecentlyRenamed: vi.fn(), + setSdkSessions: vi.fn(), + closeTerminal: vi.fn(), + openNewSessionModal: vi.fn(), + closeNewSessionModal: vi.fn(), + markSessionViewed: vi.fn(), + markAllSessionsViewed: vi.fn(), + markSessionUnread: vi.fn(), + clearSessionAttention: vi.fn(), + setTreeGroups: vi.fn(), + focusComposer: vi.fn(), + ...overrides, + }; +} + +vi.mock("../store.js", () => { + const useStoreFn = (selector: (state: MockStoreState) => unknown) => selector(mockState); + useStoreFn.getState = () => mockState; + const countUserPermissions = (perms: Map | undefined): number => { + if (!perms) return 0; + let count = 0; + for (const p of perms.values()) { + const perm = p as { evaluating?: boolean; autoApproved?: string }; + if (!perm?.evaluating && !perm?.autoApproved) count++; + } + return count; + }; + return { useStore: useStoreFn, countUserPermissions }; +}); + +import { Sidebar } from "./Sidebar.js"; + +beforeEach(() => { + vi.clearAllMocks(); + scrollTargetSessionIds.length = 0; + Element.prototype.scrollIntoView = mockScrollIntoView; + mockState = createMockState(); + window.location.hash = ""; + Object.defineProperty(window, "matchMedia", { + writable: true, + value: vi.fn().mockImplementation((query: string) => ({ + matches: false, + media: query, + onchange: null, + addListener: vi.fn(), + removeListener: vi.fn(), + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + dispatchEvent: vi.fn(), + })), + }); +}); + +describe("Sidebar session rows", { timeout: 10000 }, () => { + it("active session has highlighted styling", () => { + const session = makeSession("s1"); + const sdk = makeSdkSession("s1"); + mockState = createMockState({ + sessions: new Map([["s1", session]]), + sdkSessions: [sdk], + currentSessionId: "s1", + }); + + render(); + + expect(screen.getByText("claude-sonnet-4-5-20250929").closest("button")).toHaveClass("bg-cc-active"); + }); + + it("auto-scrolls the sidebar to keep the active session row visible", () => { + const session1 = makeSession("s1", { model: "Session One" }); + const session2 = makeSession("s2", { model: "Session Two" }); + const sdk1 = makeSdkSession("s1", { model: "Session One", createdAt: 2 }); + const sdk2 = makeSdkSession("s2", { model: "Session Two", createdAt: 1 }); + mockState = createMockState({ + sessions: new Map([ + ["s1", session1], + ["s2", session2], + ]), + sdkSessions: [sdk1, sdk2], + currentSessionId: "s2", + treeGroups: [{ id: "default", name: "Default" }], + treeNodeOrder: new Map([["default", ["s1", "s2"]]]), + }); + + render(); + + expect(screen.getByText("Session Two").closest("button")).toHaveAttribute("data-active-session", "true"); + expect(mockScrollIntoView).toHaveBeenCalledWith({ block: "nearest" }); + expect(scrollTargetSessionIds).toContain("s2"); + }); + + it("clicking a session navigates to the session hash", () => { + const session = makeSession("s1"); + const sdk = makeSdkSession("s1"); + mockState = createMockState({ + sessions: new Map([["s1", session]]), + sdkSessions: [sdk], + currentSessionId: null, + }); + + render(); + fireEvent.click(screen.getByText("claude-sonnet-4-5-20250929").closest("button")!); + + expect(window.location.hash).toBe("#/session/s1"); + }); + + it("default tree group plus button opens new session modal with tree defaults", () => { + const session = makeSession("s1", { + cwd: "/home/user/projects/myapp", + repo_root: "/home/user/projects/myapp", + }); + const sdk = makeSdkSession("s1"); + mockState = createMockState({ + sessions: new Map([["s1", session]]), + sdkSessions: [sdk], + }); + + render(); + fireEvent.click(screen.getByLabelText("Create session in Default Session Space")); + + expect(mockState.openNewSessionModal).toHaveBeenCalledWith({ + treeGroupId: "default", + newSessionDefaultsKey: "tree-group:default", + }); + }); + + it("tree group plus button opens new session modal with a tree-scoped defaults key", () => { + const session = makeSession("s1", { + cwd: "/home/user/projects/myapp", + repo_root: "/home/user/projects/myapp", + }); + const sdk = makeSdkSession("s1"); + mockState = createMockState({ + sessions: new Map([["s1", session]]), + sdkSessions: [sdk], + treeGroups: [{ id: "team-alpha", name: "Takode" }], + treeAssignments: new Map([["s1", "team-alpha"]]), + }); + + render(); + fireEvent.click(screen.getByLabelText("Create session in Takode Session Space")); + + expect(mockState.openNewSessionModal).toHaveBeenCalledWith({ + treeGroupId: "team-alpha", + newSessionDefaultsKey: "tree-group:team-alpha", + }); + }); + + it("does not render the old sidebar view mode toggle", () => { + const session = makeSession("s1"); + const sdk = makeSdkSession("s1"); + mockState = createMockState({ + sessions: new Map([["s1", session]]), + sdkSessions: [sdk], + }); + + render(); + + expect(screen.queryByTitle("Tree view (herd groups)")).not.toBeInTheDocument(); + expect(screen.queryByTitle("Linear view (project groups)")).not.toBeInTheDocument(); + }); + + it("double-clicking a session enters edit mode", async () => { + const session = makeSession("s1"); + const sdk = makeSdkSession("s1"); + mockState = createMockState({ + sessions: new Map([["s1", session]]), + sdkSessions: [sdk], + }); + + render(); + fireEvent.doubleClick(screen.getByText("claude-sonnet-4-5-20250929").closest("button")!); + + const input = await screen.findByDisplayValue("claude-sonnet-4-5-20250929"); + expect(input).toBeInTheDocument(); + expect(input.tagName).toBe("INPUT"); + }); + + it("does not steal focus back to the composer after double-click rename starts", async () => { + const session = makeSession("s1"); + const sdk = makeSdkSession("s1"); + const animationFrameCallbacks: FrameRequestCallback[] = []; + const requestAnimationFrameMock = vi.fn((callback: FrameRequestCallback) => { + animationFrameCallbacks.push(callback); + return animationFrameCallbacks.length; + }); + const originalRequestAnimationFrame = globalThis.requestAnimationFrame; + vi.stubGlobal("requestAnimationFrame", requestAnimationFrameMock); + mockState = createMockState({ + sessions: new Map([["s1", session]]), + sdkSessions: [sdk], + }); + + try { + render(); + const sessionButton = screen.getByText("claude-sonnet-4-5-20250929").closest("button")!; + fireEvent.click(sessionButton); + fireEvent.doubleClick(sessionButton); + + await screen.findByDisplayValue("claude-sonnet-4-5-20250929"); + expect(animationFrameCallbacks.length).toBeGreaterThan(0); + mockState.focusComposer.mockClear(); + while (animationFrameCallbacks.length > 0) { + animationFrameCallbacks.shift()?.(performance.now()); + } + + expect(mockState.focusComposer).not.toHaveBeenCalled(); + } finally { + vi.stubGlobal("requestAnimationFrame", originalRequestAnimationFrame); + } + }); +}); diff --git a/web/src/components/Sidebar.test.tsx b/web/src/components/Sidebar.test.tsx index d05133189..bee74e69c 100644 --- a/web/src/components/Sidebar.test.tsx +++ b/web/src/components/Sidebar.test.tsx @@ -890,137 +890,6 @@ describe("Sidebar", { timeout: 10000 }, () => { expect(sessionButton.textContent).toContain("-625"); }); - it("active session has highlighted styling (bg-cc-active class)", () => { - const session = makeSession("s1"); - const sdk = makeSdkSession("s1"); - mockState = createMockState({ - sessions: new Map([["s1", session]]), - sdkSessions: [sdk], - currentSessionId: "s1", - }); - - render(); - // Find the session button element - const sessionButton = screen.getByText("claude-sonnet-4-5-20250929").closest("button"); - expect(sessionButton).toHaveClass("bg-cc-active"); - }); - - it("auto-scrolls the sidebar to keep the active session row visible", () => { - const session1 = makeSession("s1", { model: "Session One" }); - const session2 = makeSession("s2", { model: "Session Two" }); - const sdk1 = makeSdkSession("s1", { model: "Session One", createdAt: 2 }); - const sdk2 = makeSdkSession("s2", { model: "Session Two", createdAt: 1 }); - mockState = createMockState({ - sessions: new Map([ - ["s1", session1], - ["s2", session2], - ]), - sdkSessions: [sdk1, sdk2], - currentSessionId: "s2", - treeGroups: [{ id: "default", name: "Default" }], - treeNodeOrder: new Map([["default", ["s1", "s2"]]]), - }); - - render(); - - const activeButton = screen.getByText("Session Two").closest("button"); - expect(activeButton).toHaveAttribute("data-active-session", "true"); - expect(mockScrollIntoView).toHaveBeenCalledWith({ block: "nearest" }); - expect(scrollTargetSessionIds).toContain("s2"); - }); - - it("clicking a session navigates to the session hash", () => { - // Sidebar now delegates to URL-based routing: it sets the hash to #/session/{id} - // and App.tsx's hash effect handles setCurrentSession + connectSession - const session = makeSession("s1"); - const sdk = makeSdkSession("s1"); - mockState = createMockState({ - sessions: new Map([["s1", session]]), - sdkSessions: [sdk], - currentSessionId: null, - }); - - render(); - const sessionButton = screen.getByText("claude-sonnet-4-5-20250929").closest("button")!; - fireEvent.click(sessionButton); - - expect(window.location.hash).toBe("#/session/s1"); - }); - - it("default tree group plus button opens new session modal with tree defaults", () => { - const session = makeSession("s1", { - cwd: "/home/user/projects/myapp", - repo_root: "/home/user/projects/myapp", - }); - const sdk = makeSdkSession("s1"); - mockState = createMockState({ - sessions: new Map([["s1", session]]), - sdkSessions: [sdk], - }); - - render(); - fireEvent.click(screen.getByLabelText("Create session in Default Session Space")); - - expect(mockState.openNewSessionModal).toHaveBeenCalledWith({ - treeGroupId: "default", - newSessionDefaultsKey: "tree-group:default", - }); - }); - - it("tree group plus button opens new session modal with a tree-scoped defaults key", () => { - const session = makeSession("s1", { - cwd: "/home/user/projects/myapp", - repo_root: "/home/user/projects/myapp", - }); - const sdk = makeSdkSession("s1"); - mockState = createMockState({ - sessions: new Map([["s1", session]]), - sdkSessions: [sdk], - treeGroups: [{ id: "team-alpha", name: "Takode" }], - treeAssignments: new Map([["s1", "team-alpha"]]), - }); - - render(); - fireEvent.click(screen.getByLabelText("Create session in Takode Session Space")); - - expect(mockState.openNewSessionModal).toHaveBeenCalledWith({ - treeGroupId: "team-alpha", - newSessionDefaultsKey: "tree-group:team-alpha", - }); - }); - - it("does not render the old sidebar view mode toggle", () => { - const session = makeSession("s1"); - const sdk = makeSdkSession("s1"); - mockState = createMockState({ - sessions: new Map([["s1", session]]), - sdkSessions: [sdk], - }); - - render(); - - expect(screen.queryByTitle("Tree view (herd groups)")).not.toBeInTheDocument(); - expect(screen.queryByTitle("Linear view (project groups)")).not.toBeInTheDocument(); - }); - - it("double-clicking a session enters edit mode", async () => { - const session = makeSession("s1"); - const sdk = makeSdkSession("s1"); - mockState = createMockState({ - sessions: new Map([["s1", session]]), - sdkSessions: [sdk], - }); - - render(); - const sessionButton = screen.getByText("claude-sonnet-4-5-20250929").closest("button")!; - fireEvent.doubleClick(sessionButton); - - // After double-click, an input should appear for renaming - const input = await screen.findByDisplayValue("claude-sonnet-4-5-20250929"); - expect(input).toBeInTheDocument(); - expect(input.tagName).toBe("INPUT"); - }); - it("archive button exists in the DOM for session items", () => { const session = makeSession("s1"); const sdk = makeSdkSession("s1"); diff --git a/web/src/components/Sidebar.tsx b/web/src/components/Sidebar.tsx index e6b04b190..8a99fe68a 100644 --- a/web/src/components/Sidebar.tsx +++ b/web/src/components/Sidebar.tsx @@ -148,6 +148,7 @@ export function Sidebar() { const [hoveredSession, setHoveredSession] = useState<{ sessionId: string; rect: DOMRect } | null>(null); const [hash, setHash] = useState(() => (typeof window !== "undefined" ? window.location.hash : "")); const editInputRef = useRef(null); + const editingSessionIdRef = useRef(null); const [editingServerName, setEditingServerName] = useState(false); const [serverNameDraft, setServerNameDraft] = useState(""); const serverNameInputRef = useRef(null); @@ -452,6 +453,9 @@ export function Sidebar() { navigateToSession(sessionId); requestAnimationFrame(() => { requestAnimationFrame(() => { + // Skip focus-steal if user double-clicked to rename -- the rename input + // needs to keep focus; stealing it would blur, confirm, and exit. + if (editingSessionIdRef.current != null) return; useStore.getState().focusComposer(); }); }); @@ -576,16 +580,19 @@ export function Sidebar() { api.renameSession(editingSessionId, editingName.trim()).catch(() => {}); } setEditingSessionId(null); + editingSessionIdRef.current = null; setEditingName(""); } function cancelRename() { setEditingSessionId(null); + editingSessionIdRef.current = null; setEditingName(""); } function handleStartRename(id: string, currentName: string) { setEditingSessionId(id); + editingSessionIdRef.current = id; setEditingName(currentName); }