From 38445f63f00c42a9abf42b3e436d47eb39a01745 Mon Sep 17 00:00:00 2001 From: Seth Troisi Date: Tue, 1 Jul 2025 17:17:08 -0700 Subject: [PATCH] make tag required for /chat (#2904) --- .../cli/src/ui/hooks/slashCommandProcessor.ts | 24 +++++++++++++++--- packages/core/src/core/logger.test.ts | 23 +++++------------ packages/core/src/core/logger.ts | 25 ++++++++----------- 3 files changed, 36 insertions(+), 36 deletions(-) diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.ts index ffc3d7d1..01378d89 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.ts @@ -699,7 +699,7 @@ export const useSlashCommandProcessor = ( { name: 'chat', description: - 'Manage conversation history. Usage: /chat [tag]', + 'Manage conversation history. Usage: /chat ', action: async (_mainCommand, subCommand, args) => { const tag = (args || '').trim(); const logger = new Logger(config?.getSessionId() || ''); @@ -716,19 +716,27 @@ export const useSlashCommandProcessor = ( if (!subCommand) { addMessage({ type: MessageType.ERROR, - content: 'Missing command\nUsage: /chat [tag]', + content: 'Missing command\nUsage: /chat ', timestamp: new Date(), }); return; } switch (subCommand) { case 'save': { + if (!tag) { + addMessage({ + type: MessageType.ERROR, + content: 'Missing tag. Usage: /chat save ', + timestamp: new Date(), + }); + return; + } const history = chat.getHistory(); if (history.length > 0) { await logger.saveCheckpoint(chat?.getHistory() || [], tag); addMessage({ type: MessageType.INFO, - content: `Conversation checkpoint saved${tag ? ' with tag: ' + tag : ''}.`, + content: `Conversation checkpoint saved with tag: ${tag}.`, timestamp: new Date(), }); } else { @@ -743,11 +751,19 @@ export const useSlashCommandProcessor = ( case 'resume': case 'restore': case 'load': { + if (!tag) { + addMessage({ + type: MessageType.ERROR, + content: 'Missing tag. Usage: /chat resume ', + timestamp: new Date(), + }); + return; + } const conversation = await logger.loadCheckpoint(tag); if (conversation.length === 0) { addMessage({ type: MessageType.INFO, - content: `No saved checkpoint found${tag ? ' with tag: ' + tag : ''}.`, + content: `No saved checkpoint found with tag: ${tag}.`, timestamp: new Date(), }); return; diff --git a/packages/core/src/core/logger.test.ts b/packages/core/src/core/logger.test.ts index 7a9e930b..848a2ce1 100644 --- a/packages/core/src/core/logger.test.ts +++ b/packages/core/src/core/logger.test.ts @@ -393,12 +393,6 @@ describe('Logger', () => { { role: 'model', parts: [{ text: 'Hi there' }] }, ]; - it('should save a checkpoint to the default file when no tag is provided', async () => { - await logger.saveCheckpoint(conversation); - const fileContent = await fs.readFile(TEST_CHECKPOINT_FILE_PATH, 'utf-8'); - expect(JSON.parse(fileContent)).toEqual(conversation); - }); - it('should save a checkpoint to a tagged file when a tag is provided', async () => { const tag = 'my-test-tag'; await logger.saveCheckpoint(conversation, tag); @@ -418,7 +412,7 @@ describe('Logger', () => { .mockImplementation(() => {}); await expect( - uninitializedLogger.saveCheckpoint(conversation), + uninitializedLogger.saveCheckpoint(conversation, 'tag'), ).resolves.not.toThrow(); expect(consoleErrorSpy).toHaveBeenCalledWith( 'Logger not initialized or checkpoint file path not set. Cannot save a checkpoint.', @@ -439,11 +433,6 @@ describe('Logger', () => { ); }); - it('should load from the default checkpoint file when no tag is provided', async () => { - const loaded = await logger.loadCheckpoint(); - expect(loaded).toEqual(conversation); - }); - it('should load from a tagged checkpoint file when a tag is provided', async () => { const tag = 'my-load-tag'; const taggedConversation = [ @@ -468,9 +457,9 @@ describe('Logger', () => { expect(loaded).toEqual([]); }); - it('should return an empty array if the default checkpoint file does not exist', async () => { + it('should return an empty array if the checkpoint file does not exist', async () => { await fs.unlink(TEST_CHECKPOINT_FILE_PATH); // Ensure it's gone - const loaded = await logger.loadCheckpoint(); + const loaded = await logger.loadCheckpoint('missing'); expect(loaded).toEqual([]); }); @@ -479,11 +468,11 @@ describe('Logger', () => { const consoleErrorSpy = vi .spyOn(console, 'error') .mockImplementation(() => {}); - const loadedCheckpoint = await logger.loadCheckpoint(); + const loadedCheckpoint = await logger.loadCheckpoint('missing'); expect(loadedCheckpoint).toEqual([]); expect(consoleErrorSpy).toHaveBeenCalledWith( expect.stringContaining('Failed to read or parse checkpoint file'), - expect.any(SyntaxError), + expect.any(Error), ); }); @@ -493,7 +482,7 @@ describe('Logger', () => { const consoleErrorSpy = vi .spyOn(console, 'error') .mockImplementation(() => {}); - const loadedCheckpoint = await uninitializedLogger.loadCheckpoint(); + const loadedCheckpoint = await uninitializedLogger.loadCheckpoint('tag'); expect(loadedCheckpoint).toEqual([]); expect(consoleErrorSpy).toHaveBeenCalledWith( 'Logger not initialized or checkpoint file path not set. Cannot load checkpoint.', diff --git a/packages/core/src/core/logger.ts b/packages/core/src/core/logger.ts index e4ca659d..e857bd46 100644 --- a/packages/core/src/core/logger.ts +++ b/packages/core/src/core/logger.ts @@ -10,7 +10,6 @@ import { Content } from '@google/genai'; import { getProjectTempDir } from '../utils/paths.js'; const LOG_FILE_NAME = 'logs.json'; -const CHECKPOINT_FILE_NAME = 'checkpoint.json'; export enum MessageSenderType { USER = 'user', @@ -27,7 +26,6 @@ export interface LogEntry { export class Logger { private geminiDir: string | undefined; private logFilePath: string | undefined; - private checkpointFilePath: string | undefined; private sessionId: string | undefined; private messageId = 0; // Instance-specific counter for the next messageId private initialized = false; @@ -98,7 +96,6 @@ export class Logger { this.geminiDir = getProjectTempDir(process.cwd()); this.logFilePath = path.join(this.geminiDir, LOG_FILE_NAME); - this.checkpointFilePath = path.join(this.geminiDir, CHECKPOINT_FILE_NAME); try { await fs.mkdir(this.geminiDir, { recursive: true }); @@ -234,18 +231,18 @@ export class Logger { } } - _checkpointPath(tag: string | undefined): string { - if (!this.checkpointFilePath || !this.geminiDir) { - throw new Error('Checkpoint file path not set.'); + _checkpointPath(tag: string): string { + if (!tag.length) { + throw new Error('No checkpoint tag specified.'); } - if (!tag) { - return this.checkpointFilePath; + if (!this.geminiDir) { + throw new Error('Checkpoint file path not set.'); } return path.join(this.geminiDir, `checkpoint-${tag}.json`); } - async saveCheckpoint(conversation: Content[], tag?: string): Promise { - if (!this.initialized || !this.checkpointFilePath) { + async saveCheckpoint(conversation: Content[], tag: string): Promise { + if (!this.initialized) { console.error( 'Logger not initialized or checkpoint file path not set. Cannot save a checkpoint.', ); @@ -259,8 +256,8 @@ export class Logger { } } - async loadCheckpoint(tag?: string): Promise { - if (!this.initialized || !this.checkpointFilePath) { + async loadCheckpoint(tag: string): Promise { + if (!this.initialized) { console.error( 'Logger not initialized or checkpoint file path not set. Cannot load checkpoint.', ); @@ -268,7 +265,6 @@ export class Logger { } const path = this._checkpointPath(tag); - try { const fileContent = await fs.readFile(path, 'utf-8'); const parsedContent = JSON.parse(fileContent); @@ -280,12 +276,12 @@ export class Logger { } return parsedContent as Content[]; } catch (error) { + console.error(`Failed to read or parse checkpoint file ${path}:`, error); const nodeError = error as NodeJS.ErrnoException; if (nodeError.code === 'ENOENT') { // File doesn't exist, which is fine. Return empty array. return []; } - console.error(`Failed to read or parse checkpoint file ${path}:`, error); return []; } } @@ -293,7 +289,6 @@ export class Logger { close(): void { this.initialized = false; this.logFilePath = undefined; - this.checkpointFilePath = undefined; this.logs = []; this.sessionId = undefined; this.messageId = 0;