From 16d4701822e26862fa57865f590688afe89e7648 Mon Sep 17 00:00:00 2001 From: Adam Weidman <65992621+adamfweidman@users.noreply.github.com> Date: Fri, 3 Oct 2025 00:52:16 +0200 Subject: [PATCH] Fix /chat list not write terminal escape codes directly (#10415) --- .../cli/src/ui/commands/chatCommand.test.ts | 90 ++++++------------- packages/cli/src/ui/commands/chatCommand.ts | 50 ++++------- .../src/ui/components/HistoryItemDisplay.tsx | 4 + .../src/ui/components/views/ChatList.test.tsx | 46 ++++++++++ .../cli/src/ui/components/views/ChatList.tsx | 46 ++++++++++ .../__snapshots__/ChatList.test.tsx.snap | 20 +++++ packages/cli/src/ui/types.ts | 14 ++- 7 files changed, 171 insertions(+), 99 deletions(-) create mode 100644 packages/cli/src/ui/components/views/ChatList.test.tsx create mode 100644 packages/cli/src/ui/components/views/ChatList.tsx create mode 100644 packages/cli/src/ui/components/views/__snapshots__/ChatList.test.tsx.snap diff --git a/packages/cli/src/ui/commands/chatCommand.test.ts b/packages/cli/src/ui/commands/chatCommand.test.ts index e85ae2c5f6..7a98b51415 100644 --- a/packages/cli/src/ui/commands/chatCommand.test.ts +++ b/packages/cli/src/ui/commands/chatCommand.test.ts @@ -7,11 +7,7 @@ import type { Mocked } from 'vitest'; import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest'; -import type { - MessageActionReturn, - SlashCommand, - type CommandContext, -} from './types.js'; +import type { SlashCommand, CommandContext } from './types.js'; import { createMockCommandContext } from '../../test-utils/mockCommandContext.js'; import type { Content } from '@google/genai'; import type { GeminiClient } from '@google/gemini-cli-core'; @@ -98,69 +94,37 @@ describe('chatCommand', () => { listCommand = getSubCommand('list'); }); - it('should inform when no checkpoints are found', async () => { - mockFs.readdir.mockImplementation( - (async (_: string): Promise => - [] as string[]) as unknown as typeof fsPromises.readdir, - ); - const result = await listCommand?.action?.(mockContext, ''); - expect(result).toEqual({ - type: 'message', - messageType: 'info', - content: 'No saved conversation checkpoints found.', - }); - }); - - it('should list found checkpoints', async () => { + it('should add a chat_list item to the UI', async () => { const fakeFiles = ['checkpoint-test1.json', 'checkpoint-test2.json']; - const date = new Date(); - - mockFs.readdir.mockImplementation( - (async (_: string): Promise => - fakeFiles as string[]) as unknown as typeof fsPromises.readdir, - ); - mockFs.stat.mockImplementation((async (path: string): Promise => { - if (path.endsWith('test1.json')) { - return { mtime: date } as Stats; - } - return { mtime: new Date(date.getTime() + 1000) } as Stats; - }) as unknown as typeof fsPromises.stat); - - const result = (await listCommand?.action?.( - mockContext, - '', - )) as MessageActionReturn; - - const content = result?.content ?? ''; - expect(result?.type).toBe('message'); - expect(content).toContain('List of saved conversations:'); - const isoDate = date - .toISOString() - .match(/(\d{4}-\d{2}-\d{2})T(\d{2}:\d{2}:\d{2})/); - const formattedDate = isoDate ? `${isoDate[1]} ${isoDate[2]}` : ''; - expect(content).toContain(formattedDate); - const index1 = content.indexOf('- \u001b[36mtest1\u001b[0m'); - const index2 = content.indexOf('- \u001b[36mtest2\u001b[0m'); - expect(index1).toBeGreaterThanOrEqual(0); - expect(index2).toBeGreaterThan(index1); - }); - - it('should handle invalid date formats gracefully', async () => { - const fakeFiles = ['checkpoint-baddate.json']; - const badDate = { - toISOString: () => 'an-invalid-date-string', - } as Date; + const date1 = new Date(); + const date2 = new Date(date1.getTime() + 1000); mockFs.readdir.mockResolvedValue(fakeFiles); - mockFs.stat.mockResolvedValue({ mtime: badDate } as Stats); + mockFs.stat.mockImplementation(async (path: string): Promise => { + if (path.endsWith('test1.json')) { + return { mtime: date1 } as Stats; + } + return { mtime: date2 } as Stats; + }); - const result = (await listCommand?.action?.( - mockContext, - '', - )) as MessageActionReturn; + await listCommand?.action?.(mockContext, ''); - const content = result?.content ?? ''; - expect(content).toContain('(saved on Invalid Date)'); + expect(mockContext.ui.addItem).toHaveBeenCalledWith( + { + type: 'chat_list', + chats: [ + { + name: 'test1', + mtime: date1.toISOString(), + }, + { + name: 'test2', + mtime: date2.toISOString(), + }, + ], + }, + expect.any(Number), + ); }); }); describe('save subcommand', () => { diff --git a/packages/cli/src/ui/commands/chatCommand.ts b/packages/cli/src/ui/commands/chatCommand.ts index 8a8fa23478..b0f6eac29c 100644 --- a/packages/cli/src/ui/commands/chatCommand.ts +++ b/packages/cli/src/ui/commands/chatCommand.ts @@ -17,15 +17,14 @@ import type { import { CommandKind } from './types.js'; import { decodeTagName } from '@google/gemini-cli-core'; import path from 'node:path'; -import type { HistoryItemWithoutId } from '../types.js'; +import type { + HistoryItemWithoutId, + HistoryItemChatList, + ChatDetail, +} from '../types.js'; import { MessageType } from '../types.js'; import type { Content } from '@google/genai'; -interface ChatDetail { - name: string; - mtime: Date; -} - const getSavedChatTags = async ( context: CommandContext, mtSortDesc: boolean, @@ -39,7 +38,7 @@ const getSavedChatTags = async ( const file_head = 'checkpoint-'; const file_tail = '.json'; const files = await fsPromises.readdir(geminiDir); - const chatDetails: Array<{ name: string; mtime: Date }> = []; + const chatDetails: ChatDetail[] = []; for (const file of files) { if (file.startsWith(file_head) && file.endsWith(file_tail)) { @@ -48,15 +47,15 @@ const getSavedChatTags = async ( const tagName = file.slice(file_head.length, -file_tail.length); chatDetails.push({ name: decodeTagName(tagName), - mtime: stats.mtime, + mtime: stats.mtime.toISOString(), }); } } chatDetails.sort((a, b) => mtSortDesc - ? b.mtime.getTime() - a.mtime.getTime() - : a.mtime.getTime() - b.mtime.getTime(), + ? b.mtime.localeCompare(a.mtime) + : a.mtime.localeCompare(b.mtime), ); return chatDetails; @@ -69,34 +68,15 @@ const listCommand: SlashCommand = { name: 'list', description: 'List saved conversation checkpoints', kind: CommandKind.BUILT_IN, - action: async (context): Promise => { + action: async (context): Promise => { const chatDetails = await getSavedChatTags(context, false); - if (chatDetails.length === 0) { - return { - type: 'message', - messageType: 'info', - content: 'No saved conversation checkpoints found.', - }; - } - const maxNameLength = Math.max( - ...chatDetails.map((chat) => chat.name.length), - ); - - let message = 'List of saved conversations:\n\n'; - for (const chat of chatDetails) { - const paddedName = chat.name.padEnd(maxNameLength, ' '); - const isoString = chat.mtime.toISOString(); - const match = isoString.match(/(\d{4}-\d{2}-\d{2})T(\d{2}:\d{2}:\d{2})/); - const formattedDate = match ? `${match[1]} ${match[2]}` : 'Invalid Date'; - message += ` - \u001b[36m${paddedName}\u001b[0m \u001b[90m(saved on ${formattedDate})\u001b[0m\n`; - } - message += `\n\u001b[90mNote: Newest last, oldest first\u001b[0m`; - return { - type: 'message', - messageType: 'info', - content: message, + const item: HistoryItemChatList = { + type: MessageType.CHAT_LIST, + chats: chatDetails, }; + + context.ui.addItem(item, Date.now()); }, }; diff --git a/packages/cli/src/ui/components/HistoryItemDisplay.tsx b/packages/cli/src/ui/components/HistoryItemDisplay.tsx index c265a72e47..19c42d7e7a 100644 --- a/packages/cli/src/ui/components/HistoryItemDisplay.tsx +++ b/packages/cli/src/ui/components/HistoryItemDisplay.tsx @@ -29,6 +29,7 @@ import { ExtensionsList } from './views/ExtensionsList.js'; import { getMCPServerStatus } from '@google/gemini-cli-core'; import { ToolsList } from './views/ToolsList.js'; import { McpStatus } from './views/McpStatus.js'; +import { ChatList } from './views/ChatList.js'; interface HistoryItemDisplayProps { item: HistoryItem; @@ -140,6 +141,9 @@ export const HistoryItemDisplay: React.FC = ({ {itemForDisplay.type === 'mcp_status' && ( )} + {itemForDisplay.type === 'chat_list' && ( + + )} ); }; diff --git a/packages/cli/src/ui/components/views/ChatList.test.tsx b/packages/cli/src/ui/components/views/ChatList.test.tsx new file mode 100644 index 0000000000..f55aa8b611 --- /dev/null +++ b/packages/cli/src/ui/components/views/ChatList.test.tsx @@ -0,0 +1,46 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { render } from 'ink-testing-library'; +import { describe, it, expect } from 'vitest'; +import { ChatList } from './ChatList.js'; +import type { ChatDetail } from '../../types.js'; + +const mockChats: ChatDetail[] = [ + { + name: 'chat-1', + mtime: '2025-10-02T10:00:00.000Z', + }, + { + name: 'another-chat', + mtime: '2025-10-01T12:30:00.000Z', + }, +]; + +describe('', () => { + it('renders correctly with a list of chats', () => { + const { lastFrame } = render(); + expect(lastFrame()).toMatchSnapshot(); + }); + + it('renders correctly with no chats', () => { + const { lastFrame } = render(); + expect(lastFrame()).toContain('No saved conversation checkpoints found.'); + expect(lastFrame()).toMatchSnapshot(); + }); + + it('handles invalid date formats gracefully', () => { + const mockChatsWithInvalidDate: ChatDetail[] = [ + { + name: 'bad-date-chat', + mtime: 'an-invalid-date-string', + }, + ]; + const { lastFrame } = render(); + expect(lastFrame()).toContain('(Invalid Date)'); + expect(lastFrame()).toMatchSnapshot(); + }); +}); diff --git a/packages/cli/src/ui/components/views/ChatList.tsx b/packages/cli/src/ui/components/views/ChatList.tsx new file mode 100644 index 0000000000..9e6e36b3a1 --- /dev/null +++ b/packages/cli/src/ui/components/views/ChatList.tsx @@ -0,0 +1,46 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import type React from 'react'; +import { Box, Text } from 'ink'; +import { theme } from '../../semantic-colors.js'; +import type { ChatDetail } from '../../types.js'; + +interface ChatListProps { + chats: readonly ChatDetail[]; +} + +export const ChatList: React.FC = ({ chats }) => { + if (chats.length === 0) { + return No saved conversation checkpoints found.; + } + + return ( + + List of saved conversations: + + {chats.map((chat) => { + const isoString = chat.mtime; + const match = isoString.match( + /(\d{4}-\d{2}-\d{2})T(\d{2}:\d{2}:\d{2})/, + ); + const formattedDate = match + ? `${match[1]} ${match[2]}` + : 'Invalid Date'; + return ( + + + {' '}- {chat.name}{' '} + ({formattedDate}) + + + ); + })} + + Note: Newest last, oldest first + + ); +}; diff --git a/packages/cli/src/ui/components/views/__snapshots__/ChatList.test.tsx.snap b/packages/cli/src/ui/components/views/__snapshots__/ChatList.test.tsx.snap new file mode 100644 index 0000000000..eddfa2ca2b --- /dev/null +++ b/packages/cli/src/ui/components/views/__snapshots__/ChatList.test.tsx.snap @@ -0,0 +1,20 @@ +// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html + +exports[` > handles invalid date formats gracefully 1`] = ` +"List of saved conversations: + + - bad-date-chat (Invalid Date) + +Note: Newest last, oldest first" +`; + +exports[` > renders correctly with a list of chats 1`] = ` +"List of saved conversations: + + - chat-1 (2025-10-02 10:00:00) + - another-chat (2025-10-01 12:30:00) + +Note: Newest last, oldest first" +`; + +exports[` > renders correctly with no chats 1`] = `"No saved conversation checkpoints found."`; diff --git a/packages/cli/src/ui/types.ts b/packages/cli/src/ui/types.ts index e4f2abee60..91ecbe1ac8 100644 --- a/packages/cli/src/ui/types.ts +++ b/packages/cli/src/ui/types.ts @@ -165,6 +165,16 @@ export type HistoryItemExtensionsList = HistoryItemBase & { type: 'extensions_list'; }; +export interface ChatDetail { + name: string; + mtime: string; +} + +export type HistoryItemChatList = HistoryItemBase & { + type: 'chat_list'; + chats: ChatDetail[]; +}; + export interface ToolDefinition { name: string; displayName: string; @@ -234,7 +244,8 @@ export type HistoryItemWithoutId = | HistoryItemCompression | HistoryItemExtensionsList | HistoryItemToolsList - | HistoryItemMcpStatus; + | HistoryItemMcpStatus + | HistoryItemChatList; export type HistoryItem = HistoryItemWithoutId & { id: number }; @@ -255,6 +266,7 @@ export enum MessageType { EXTENSIONS_LIST = 'extensions_list', TOOLS_LIST = 'tools_list', MCP_STATUS = 'mcp_status', + CHAT_LIST = 'chat_list', } // Simplified message structure for internal feedback