From bce6eb501462b32fc629b6febb357ab679b6bd05 Mon Sep 17 00:00:00 2001 From: Hiroaki Mitsuyoshi <131056197+flowernotfound@users.noreply.github.com> Date: Mon, 28 Jul 2025 07:18:12 +0900 Subject: [PATCH] feat(chat): Implement /chat delete command (#2401) --- .../cli/src/ui/commands/chatCommand.test.ts | 70 ++++++++++++++++++- packages/cli/src/ui/commands/chatCommand.ts | 42 ++++++++++- packages/core/src/core/logger.test.ts | 62 ++++++++++++++++ packages/core/src/core/logger.ts | 24 +++++++ 4 files changed, 194 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/ui/commands/chatCommand.test.ts b/packages/cli/src/ui/commands/chatCommand.test.ts index 0c98239a..aad0897c 100644 --- a/packages/cli/src/ui/commands/chatCommand.test.ts +++ b/packages/cli/src/ui/commands/chatCommand.test.ts @@ -40,14 +40,17 @@ describe('chatCommand', () => { let mockGetChat: ReturnType; let mockSaveCheckpoint: ReturnType; let mockLoadCheckpoint: ReturnType; + let mockDeleteCheckpoint: ReturnType; let mockGetHistory: ReturnType; - const getSubCommand = (name: 'list' | 'save' | 'resume'): SlashCommand => { + const getSubCommand = ( + name: 'list' | 'save' | 'resume' | 'delete', + ): SlashCommand => { const subCommand = chatCommand.subCommands?.find( (cmd) => cmd.name === name, ); if (!subCommand) { - throw new Error(`/memory ${name} command not found.`); + throw new Error(`/chat ${name} command not found.`); } return subCommand; }; @@ -59,6 +62,7 @@ describe('chatCommand', () => { }); mockSaveCheckpoint = vi.fn().mockResolvedValue(undefined); mockLoadCheckpoint = vi.fn().mockResolvedValue([]); + mockDeleteCheckpoint = vi.fn().mockResolvedValue(true); mockContext = createMockCommandContext({ services: { @@ -72,6 +76,7 @@ describe('chatCommand', () => { logger: { saveCheckpoint: mockSaveCheckpoint, loadCheckpoint: mockLoadCheckpoint, + deleteCheckpoint: mockDeleteCheckpoint, initialize: vi.fn().mockResolvedValue(undefined), }, }, @@ -85,7 +90,7 @@ describe('chatCommand', () => { it('should have the correct main command definition', () => { expect(chatCommand.name).toBe('chat'); expect(chatCommand.description).toBe('Manage conversation history.'); - expect(chatCommand.subCommands).toHaveLength(3); + expect(chatCommand.subCommands).toHaveLength(4); }); describe('list subcommand', () => { @@ -297,4 +302,63 @@ describe('chatCommand', () => { }); }); }); + + describe('delete subcommand', () => { + let deleteCommand: SlashCommand; + const tag = 'my-tag'; + beforeEach(() => { + deleteCommand = getSubCommand('delete'); + }); + + it('should return an error if tag is missing', async () => { + const result = await deleteCommand?.action?.(mockContext, ' '); + expect(result).toEqual({ + type: 'message', + messageType: 'error', + content: 'Missing tag. Usage: /chat delete ', + }); + }); + + it('should return an error if checkpoint is not found', async () => { + mockDeleteCheckpoint.mockResolvedValue(false); + const result = await deleteCommand?.action?.(mockContext, tag); + expect(result).toEqual({ + type: 'message', + messageType: 'error', + content: `Error: No checkpoint found with tag '${tag}'.`, + }); + }); + + it('should delete the conversation', async () => { + const result = await deleteCommand?.action?.(mockContext, tag); + + expect(mockDeleteCheckpoint).toHaveBeenCalledWith(tag); + expect(result).toEqual({ + type: 'message', + messageType: 'info', + content: `Conversation checkpoint '${tag}' has been deleted.`, + }); + }); + + describe('completion', () => { + it('should provide completion suggestions', async () => { + const fakeFiles = ['checkpoint-alpha.json', 'checkpoint-beta.json']; + mockFs.readdir.mockImplementation( + (async (_: string): Promise => + fakeFiles as string[]) as unknown as typeof fsPromises.readdir, + ); + + mockFs.stat.mockImplementation( + (async (_: string): Promise => + ({ + mtime: new Date(), + }) as Stats) as unknown as typeof fsPromises.stat, + ); + + const result = await deleteCommand?.completion?.(mockContext, 'a'); + + expect(result).toEqual(['alpha']); + }); + }); + }); }); diff --git a/packages/cli/src/ui/commands/chatCommand.ts b/packages/cli/src/ui/commands/chatCommand.ts index 739097e3..a5fa13da 100644 --- a/packages/cli/src/ui/commands/chatCommand.ts +++ b/packages/cli/src/ui/commands/chatCommand.ts @@ -206,9 +206,49 @@ const resumeCommand: SlashCommand = { }, }; +const deleteCommand: SlashCommand = { + name: 'delete', + description: 'Delete a conversation checkpoint. Usage: /chat delete ', + kind: CommandKind.BUILT_IN, + action: async (context, args): Promise => { + const tag = args.trim(); + if (!tag) { + return { + type: 'message', + messageType: 'error', + content: 'Missing tag. Usage: /chat delete ', + }; + } + + const { logger } = context.services; + await logger.initialize(); + const deleted = await logger.deleteCheckpoint(tag); + + if (deleted) { + return { + type: 'message', + messageType: 'info', + content: `Conversation checkpoint '${tag}' has been deleted.`, + }; + } else { + return { + type: 'message', + messageType: 'error', + content: `Error: No checkpoint found with tag '${tag}'.`, + }; + } + }, + completion: async (context, partialArg) => { + const chatDetails = await getSavedChatTags(context, true); + return chatDetails + .map((chat) => chat.name) + .filter((name) => name.startsWith(partialArg)); + }, +}; + export const chatCommand: SlashCommand = { name: 'chat', description: 'Manage conversation history.', kind: CommandKind.BUILT_IN, - subCommands: [listCommand, saveCommand, resumeCommand], + subCommands: [listCommand, saveCommand, resumeCommand, deleteCommand], }; diff --git a/packages/core/src/core/logger.test.ts b/packages/core/src/core/logger.test.ts index 84cc1a0f..c74b92cf 100644 --- a/packages/core/src/core/logger.test.ts +++ b/packages/core/src/core/logger.test.ts @@ -490,6 +490,68 @@ describe('Logger', () => { }); }); + describe('deleteCheckpoint', () => { + const conversation: Content[] = [ + { role: 'user', parts: [{ text: 'Content to be deleted' }] }, + ]; + const tag = 'delete-me'; + let taggedFilePath: string; + + beforeEach(async () => { + taggedFilePath = path.join( + TEST_GEMINI_DIR, + `${CHECKPOINT_FILE_NAME.replace('.json', '')}-${tag}.json`, + ); + // Create a file to be deleted + await fs.writeFile(taggedFilePath, JSON.stringify(conversation)); + }); + + it('should delete the specified checkpoint file and return true', async () => { + const result = await logger.deleteCheckpoint(tag); + expect(result).toBe(true); + + // Verify the file is actually gone + await expect(fs.access(taggedFilePath)).rejects.toThrow(/ENOENT/); + }); + + it('should return false if the checkpoint file does not exist', async () => { + const result = await logger.deleteCheckpoint('non-existent-tag'); + expect(result).toBe(false); + }); + + it('should re-throw an error if file deletion fails for reasons other than not existing', async () => { + // Simulate a different error (e.g., permission denied) + vi.spyOn(fs, 'unlink').mockRejectedValueOnce( + new Error('EACCES: permission denied'), + ); + const consoleErrorSpy = vi + .spyOn(console, 'error') + .mockImplementation(() => {}); + + await expect(logger.deleteCheckpoint(tag)).rejects.toThrow( + 'EACCES: permission denied', + ); + expect(consoleErrorSpy).toHaveBeenCalledWith( + `Failed to delete checkpoint file ${taggedFilePath}:`, + expect.any(Error), + ); + }); + + it('should return false if logger is not initialized', async () => { + const uninitializedLogger = new Logger(testSessionId); + uninitializedLogger.close(); + const consoleErrorSpy = vi + .spyOn(console, 'error') + .mockImplementation(() => {}); + + const result = await uninitializedLogger.deleteCheckpoint(tag); + expect(result).toBe(false); + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Logger not initialized or checkpoint file path not set. Cannot delete checkpoint.', + ); + }); + }); + describe('close', () => { it('should reset logger state', async () => { await logger.logMessage(MessageSenderType.USER, 'A message'); diff --git a/packages/core/src/core/logger.ts b/packages/core/src/core/logger.ts index 450a0d2f..2be9f1d4 100644 --- a/packages/core/src/core/logger.ts +++ b/packages/core/src/core/logger.ts @@ -292,6 +292,30 @@ export class Logger { } } + async deleteCheckpoint(tag: string): Promise { + if (!this.initialized || !this.geminiDir) { + console.error( + 'Logger not initialized or checkpoint file path not set. Cannot delete checkpoint.', + ); + return false; + } + + const path = this._checkpointPath(tag); + + try { + await fs.unlink(path); + return true; + } catch (error) { + const nodeError = error as NodeJS.ErrnoException; + if (nodeError.code === 'ENOENT') { + // File doesn't exist, which is fine. + return false; + } + console.error(`Failed to delete checkpoint file ${path}:`, error); + throw error; + } + } + close(): void { this.initialized = false; this.logFilePath = undefined;