diff --git a/cli/src/components/chat-history-screen.tsx b/cli/src/components/chat-history-screen.tsx index 5c9f256e1..b9de476e3 100644 --- a/cli/src/components/chat-history-screen.tsx +++ b/cli/src/components/chat-history-screen.tsx @@ -7,7 +7,11 @@ import { SelectableList } from './selectable-list' import { useSearchableList } from '../hooks/use-searchable-list' import { useTerminalLayout } from '../hooks/use-terminal-layout' import { useTheme } from '../hooks/use-theme' -import { getAllChats, formatRelativeTime } from '../utils/chat-history' +import { + deleteChatSession, + formatRelativeTime, + getAllChats, +} from '../utils/chat-history' import type { SelectableListItem } from './selectable-list' @@ -21,6 +25,7 @@ const LAYOUT = { MAX_RENDERED_CHATS: 100, // Only render this many in the list TIME_COL_WIDTH: 12, // e.g., "2 hours ago" MSGS_COL_WIDTH: 8, // e.g., "99 msgs" + DELETE_COL_WIDTH: 8, // e.g., " Delete " GAP_WIDTH: 3, // gap between columns } as const @@ -42,34 +47,37 @@ export const ChatHistoryScreen: React.FC = ({ const contentWidth = terminalWidth - LAYOUT.CONTENT_PADDING // Two-phase loading: load initial chats immediately, then more in background - const initialChats = useMemo(() => getAllChats(LAYOUT.INITIAL_CHATS), []) - const [backgroundChats, setBackgroundChats] = useState( - [], - ) + const [chats, setChats] = useState(() => getAllChats(LAYOUT.INITIAL_CHATS)) + const [statusMessage, setStatusMessage] = useState(null) // Load more chats in the background after initial render useEffect(() => { // Use setTimeout to defer the expensive loading to after first paint const timer = setTimeout(() => { - const moreChats = getAllChats( - LAYOUT.INITIAL_CHATS + LAYOUT.BACKGROUND_CHATS, - ) - // Only keep the chats beyond the initial set - setBackgroundChats(moreChats.slice(LAYOUT.INITIAL_CHATS)) + setChats(getAllChats(LAYOUT.INITIAL_CHATS + LAYOUT.BACKGROUND_CHATS)) }, 0) return () => clearTimeout(timer) }, []) - // Combine initial and background chats - const chats = useMemo( - () => [...initialChats, ...backgroundChats], - [initialChats, backgroundChats], - ) + const handleDeleteChat = useCallback((chatId: string) => { + const deleted = deleteChatSession(chatId) + if (deleted) { + setChats((prev) => prev.filter((chat) => chat.chatId !== chatId)) + setStatusMessage('Chat deleted') + return + } + + setStatusMessage('Could not delete chat') + }, []) // Calculate available width for the prompt text (last column, variable width) - // Format: "[time] [msgs] [prompt...]" + // Format: "[time] [msgs] [prompt...] [Delete]" const reservedWidth = - LAYOUT.TIME_COL_WIDTH + LAYOUT.MSGS_COL_WIDTH + LAYOUT.GAP_WIDTH * 2 + 2 // +2 for padding + LAYOUT.TIME_COL_WIDTH + + LAYOUT.MSGS_COL_WIDTH + + LAYOUT.DELETE_COL_WIDTH + + LAYOUT.GAP_WIDTH * 2 + + 2 // +2 for padding const maxPromptWidth = Math.max(20, contentWidth - reservedWidth) // Truncate text to fit single line @@ -146,6 +154,13 @@ export const ChatHistoryScreen: React.FC = ({ [onSelectChat], ) + const handleChatDelete = useCallback( + (item: SelectableListItem) => { + handleDeleteChat(item.id) + }, + [handleDeleteChat], + ) + // Handle keyboard input const handleKeyIntercept = useCallback( (key: { name?: string; shift?: boolean; ctrl?: boolean }) => { @@ -275,9 +290,11 @@ export const ChatHistoryScreen: React.FC = ({ items={filteredItems.slice(0, LAYOUT.MAX_RENDERED_CHATS)} focusedIndex={focusedIndex} onSelect={handleChatSelect} + actionLabel="Delete" + onAction={handleChatDelete} onFocusChange={handleFocusChange} emptyMessage={ - initialChats.length === 0 + chats.length === 0 ? 'No chat history yet' : searchQuery ? 'No matching chats' @@ -314,8 +331,14 @@ export const ChatHistoryScreen: React.FC = ({ {/* Help text */} - ↑↓ navigate · Enter select · Esc cancel + ↑↓ navigate · Enter select · Click Delete to remove · Esc cancel + {statusMessage && ( + + {' · '} + {statusMessage} + + )} {/* Buttons - hidden on narrow screens */} diff --git a/cli/src/components/selectable-list.tsx b/cli/src/components/selectable-list.tsx index 99291097f..4c5c3ad0a 100644 --- a/cli/src/components/selectable-list.tsx +++ b/cli/src/components/selectable-list.tsx @@ -40,6 +40,8 @@ export interface SelectableListProps { /** Optional max height - if not provided, list fills available space */ maxHeight?: number onSelect: (item: SelectableListItem, index: number) => void + actionLabel?: string + onAction?: (item: SelectableListItem, index: number) => void onFocusChange?: (index: number) => void emptyMessage?: string } @@ -53,7 +55,16 @@ export const SelectableList = forwardRef< SelectableListProps >( ( - { items, focusedIndex, maxHeight, onSelect, onFocusChange, emptyMessage = 'No items' }, + { + items, + focusedIndex, + maxHeight, + onSelect, + actionLabel, + onAction, + onFocusChange, + emptyMessage = 'No items', + }, ref, ) => { const theme = useTheme() @@ -141,13 +152,21 @@ export const SelectableList = forwardRef< const isHighlighted = isFocused || isHovered // Use subtle highlight that works in both light and dark themes - const backgroundColor = isHighlighted ? theme.surfaceHover : 'transparent' + const backgroundColor = isHighlighted + ? theme.surfaceHover + : 'transparent' const textColor = isHighlighted ? theme.foreground : theme.muted return ( - + {actionLabel && onAction && ( + )} - + ) })} diff --git a/cli/src/utils/__tests__/chat-history.test.ts b/cli/src/utils/__tests__/chat-history.test.ts new file mode 100644 index 000000000..31acf47f3 --- /dev/null +++ b/cli/src/utils/__tests__/chat-history.test.ts @@ -0,0 +1,74 @@ +import { describe, test, expect, beforeEach, afterEach, mock } from 'bun:test' +import * as fs from 'fs' +import * as os from 'os' +import * as path from 'path' + +let tempDataDir = '' + +mock.module('../../project-files', () => ({ + getProjectDataDir: () => tempDataDir, +})) + +mock.module('../logger', () => ({ + logger: { + debug: () => {}, + info: () => {}, + warn: () => {}, + error: () => {}, + fatal: () => {}, + }, +})) + +import { deleteChatSession, getAllChats } from '../chat-history' + +function writeChat(chatId: string, prompt: string) { + const chatDir = path.join(tempDataDir, 'chats', chatId) + fs.mkdirSync(chatDir, { recursive: true }) + fs.writeFileSync( + path.join(chatDir, 'chat-messages.json'), + JSON.stringify([ + { + id: `${chatId}-message`, + variant: 'user', + content: prompt, + timestamp: new Date().toISOString(), + blocks: [], + }, + ]), + ) +} + +describe('chat-history', () => { + beforeEach(() => { + tempDataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codebuff-history-')) + }) + + afterEach(() => { + fs.rmSync(tempDataDir, { recursive: true, force: true }) + }) + + test('deleteChatSession removes a saved chat directory', () => { + writeChat('chat-a', 'hello from chat a') + writeChat('chat-b', 'hello from chat b') + + expect(deleteChatSession('chat-a')).toBe(true) + + expect(fs.existsSync(path.join(tempDataDir, 'chats', 'chat-a'))).toBe(false) + expect(fs.existsSync(path.join(tempDataDir, 'chats', 'chat-b'))).toBe(true) + expect(getAllChats().map((chat) => chat.chatId)).toEqual(['chat-b']) + }) + + test('deleteChatSession rejects invalid chat ids', () => { + const outsideDir = path.join(tempDataDir, 'outside') + fs.mkdirSync(outsideDir, { recursive: true }) + + expect(deleteChatSession('../outside')).toBe(false) + expect(deleteChatSession('..')).toBe(false) + + expect(fs.existsSync(outsideDir)).toBe(true) + }) + + test('deleteChatSession returns false when the chat does not exist', () => { + expect(deleteChatSession('missing-chat')).toBe(false) + }) +}) diff --git a/cli/src/utils/chat-history.ts b/cli/src/utils/chat-history.ts index 1a97101a8..2a4a51612 100644 --- a/cli/src/utils/chat-history.ts +++ b/cli/src/utils/chat-history.ts @@ -13,6 +13,10 @@ export interface ChatHistoryEntry { messageCount: number } +function getChatsDir(): string { + return path.join(getProjectDataDir(), 'chats') +} + /** * Get the first user message from a list of chat messages */ @@ -43,14 +47,14 @@ interface ChatDirInfo { */ export function getAllChats(maxChats: number = 500): ChatHistoryEntry[] { try { - const chatsDir = path.join(getProjectDataDir(), 'chats') - + const chatsDir = getChatsDir() + if (!fs.existsSync(chatsDir)) { return [] } const chatDirs = fs.readdirSync(chatsDir) - + // First pass: get mtime for all chat directories (fast, no file reading) const chatDirInfos: ChatDirInfo[] = [] for (const chatId of chatDirs) { @@ -58,7 +62,7 @@ export function getAllChats(maxChats: number = 500): ChatHistoryEntry[] { try { const stat = fs.statSync(chatPath) if (!stat.isDirectory()) continue - + chatDirInfos.push({ chatId, chatPath, @@ -69,14 +73,14 @@ export function getAllChats(maxChats: number = 500): ChatHistoryEntry[] { // Skip directories we can't stat } } - + // Sort by mtime first (most recent first) chatDirInfos.sort((a, b) => b.mtime.getTime() - a.mtime.getTime()) - + // Second pass: only read message content for the top N chats const chats: ChatHistoryEntry[] = [] const chatsToLoad = chatDirInfos.slice(0, maxChats) - + for (const info of chatsToLoad) { try { let messageCount = 0 @@ -100,8 +104,11 @@ export function getAllChats(maxChats: number = 500): ChatHistoryEntry[] { } } catch (error) { logger.debug( - { chatId: info.chatId, error: error instanceof Error ? error.message : String(error) }, - 'Failed to read chat messages' + { + chatId: info.chatId, + error: error instanceof Error ? error.message : String(error), + }, + 'Failed to read chat messages', ) } } @@ -110,12 +117,55 @@ export function getAllChats(maxChats: number = 500): ChatHistoryEntry[] { } catch (error) { logger.error( { error: error instanceof Error ? error.message : String(error) }, - 'Failed to list chats' + 'Failed to list chats', ) return [] } } +/** + * Delete a saved chat session from local history. + */ +export function deleteChatSession(chatId: string): boolean { + try { + const safeChatId = chatId.trim() + if ( + !safeChatId || + safeChatId === '.' || + safeChatId === '..' || + path.basename(safeChatId) !== safeChatId + ) { + logger.warn({ chatId }, 'Refusing to delete invalid chat id') + return false + } + + const chatsDir = getChatsDir() + const chatPath = path.join(chatsDir, safeChatId) + + if (!fs.existsSync(chatPath)) { + return false + } + + const stat = fs.statSync(chatPath) + if (!stat.isDirectory()) { + logger.warn( + { chatId, chatPath }, + 'Refusing to delete non-directory chat path', + ) + return false + } + + fs.rmSync(chatPath, { recursive: true, force: false }) + return true + } catch (error) { + logger.error( + { chatId, error: error instanceof Error ? error.message : String(error) }, + 'Failed to delete chat session', + ) + return false + } +} + /** * Format a timestamp relative to now (e.g., "2 hours ago", "yesterday") */