Merge tag 'v0.1.21' of github.com:google-gemini/gemini-cli into chore/sync-gemini-cli-v0.1.21

packages/core/src/tools/memoryTool.test.ts

@@ -202,9 +202,11 @@ describe('MemoryTool', () => {
       expect(memoryTool.schema.parametersJsonSchema).toBeDefined();
     });
 
-    it('should call performAddMemoryEntry with correct parameters and return success', async () => {
+    it('should call performAddMemoryEntry with correct parameters and return success for global scope', async () => {
       const params = { fact: 'The sky is blue', scope: 'global' as const };
-      const result = await memoryTool.execute(params, mockAbortSignal);
+      const invocation = memoryTool.build(params);
+      const result = await invocation.execute(mockAbortSignal);
+
       // Use getCurrentGeminiMdFilename for the default expectation before any setGeminiMdFilename calls in a test
       const expectedFilePath = path.join(
         os.homedir(),
@@ -231,16 +233,44 @@ describe('MemoryTool', () => {
       expect(result.returnDisplay).toBe(successMessage);
     });
 
+    it('should call performAddMemoryEntry with correct parameters and return success for project scope', async () => {
+      const params = { fact: 'The sky is blue', scope: 'project' as const };
+      const invocation = memoryTool.build(params);
+      const result = await invocation.execute(mockAbortSignal);
+
+      // For project scope, expect the file to be in current working directory
+      const expectedFilePath = path.join(
+        process.cwd(),
+        getCurrentGeminiMdFilename(),
+      );
+
+      // For this test, we expect the actual fs methods to be passed
+      const expectedFsArgument = {
+        readFile: fs.readFile,
+        writeFile: fs.writeFile,
+        mkdir: fs.mkdir,
+      };
+
+      expect(performAddMemoryEntrySpy).toHaveBeenCalledWith(
+        params.fact,
+        expectedFilePath,
+        expectedFsArgument,
+      );
+      const successMessage = `Okay, I've remembered that in project memory: "${params.fact}"`;
+      expect(result.llmContent).toBe(
+        JSON.stringify({ success: true, message: successMessage }),
+      );
+      expect(result.returnDisplay).toBe(successMessage);
+    });
+
     it('should return an error if fact is empty', async () => {
       const params = { fact: ' ' }; // Empty fact
-      const result = await memoryTool.execute(params, mockAbortSignal);
-      const errorMessage = 'Parameter "fact" must be a non-empty string.';
-
-      expect(performAddMemoryEntrySpy).not.toHaveBeenCalled();
-      expect(result.llmContent).toBe(
-        JSON.stringify({ success: false, error: errorMessage }),
+      expect(memoryTool.validateToolParams(params)).toBe(
+        'Parameter "fact" must be a non-empty string.',
+      );
+      expect(() => memoryTool.build(params)).toThrow(
+        'Parameter "fact" must be a non-empty string.',
       );
-      expect(result.returnDisplay).toBe(`Error: ${errorMessage}`);
     });
 
     it('should handle errors from performAddMemoryEntry', async () => {
@@ -250,7 +280,8 @@ describe('MemoryTool', () => {
       );
       performAddMemoryEntrySpy.mockRejectedValue(underlyingError);
 
-      const result = await memoryTool.execute(params, mockAbortSignal);
+      const invocation = memoryTool.build(params);
+      const result = await invocation.execute(mockAbortSignal);
 
       expect(result.llmContent).toBe(
         JSON.stringify({
@@ -262,6 +293,18 @@ describe('MemoryTool', () => {
         `Error saving memory: ${underlyingError.message}`,
       );
     });
+
+    it('should return error when executing without scope parameter', async () => {
+      const params = { fact: 'Test fact' };
+      const invocation = memoryTool.build(params);
+      const result = await invocation.execute(mockAbortSignal);
+
+      expect(result.llmContent).toContain(
+        'Please specify where to save this memory',
+      );
+      expect(result.returnDisplay).toContain('Global:');
+      expect(result.returnDisplay).toContain('Project:');
+    });
   });
 
   describe('shouldConfirmExecute', () => {
@@ -269,18 +312,14 @@ describe('MemoryTool', () => {
 
     beforeEach(() => {
       memoryTool = new MemoryTool();
-      // Clear the allowlist before each test
-      (MemoryTool as unknown as { allowlist: Set<string> }).allowlist.clear();
       // Mock fs.readFile to return empty string (file doesn't exist)
       vi.mocked(fs.readFile).mockResolvedValue('');
     });
 
-    it('should return confirmation details when memory file is not allowlisted', async () => {
+    it('should return confirmation details when memory file is not allowlisted for global scope', async () => {
       const params = { fact: 'Test fact', scope: 'global' as const };
-      const result = await memoryTool.shouldConfirmExecute(
-        params,
-        mockAbortSignal,
-      );
+      const invocation = memoryTool.build(params);
+      const result = await invocation.shouldConfirmExecute(mockAbortSignal);
 
       expect(result).toBeDefined();
       expect(result).not.toBe(false);
@@ -301,7 +340,30 @@ describe('MemoryTool', () => {
       }
     });
 
-    it('should return false when memory file is already allowlisted', async () => {
+    it('should return confirmation details when memory file is not allowlisted for project scope', async () => {
+      const params = { fact: 'Test fact', scope: 'project' as const };
+      const invocation = memoryTool.build(params);
+      const result = await invocation.shouldConfirmExecute(mockAbortSignal);
+
+      expect(result).toBeDefined();
+      expect(result).not.toBe(false);
+
+      if (result && result.type === 'edit') {
+        const expectedPath = path.join(process.cwd(), 'QWEN.md');
+        expect(result.title).toBe(
+          `Confirm Memory Save: ${expectedPath} (project)`,
+        );
+        expect(result.fileName).toBe(expectedPath);
+        expect(result.fileDiff).toContain('Index: QWEN.md');
+        expect(result.fileDiff).toContain('+## Qwen Added Memories');
+        expect(result.fileDiff).toContain('+- Test fact');
+        expect(result.originalContent).toBe('');
+        expect(result.newContent).toContain('## Qwen Added Memories');
+        expect(result.newContent).toContain('- Test fact');
+      }
+    });
+
+    it('should return false when memory file is already allowlisted for global scope', async () => {
       const params = { fact: 'Test fact', scope: 'global' as const };
       const memoryFilePath = path.join(
         os.homedir(),
@@ -309,20 +371,36 @@ describe('MemoryTool', () => {
         getCurrentGeminiMdFilename(),
       );
 
-      // Add the memory file to the allowlist with the new key format
-      (MemoryTool as unknown as { allowlist: Set<string> }).allowlist.add(
-        `${memoryFilePath}_global`,
-      );
+      const invocation = memoryTool.build(params);
+      // Add the memory file to the allowlist with the scope-specific key format
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      (invocation.constructor as any).allowlist.add(`${memoryFilePath}_global`);
 
-      const result = await memoryTool.shouldConfirmExecute(
-        params,
-        mockAbortSignal,
-      );
+      const result = await invocation.shouldConfirmExecute(mockAbortSignal);
 
       expect(result).toBe(false);
     });
 
-    it('should add memory file to allowlist when ProceedAlways is confirmed', async () => {
+    it('should return false when memory file is already allowlisted for project scope', async () => {
+      const params = { fact: 'Test fact', scope: 'project' as const };
+      const memoryFilePath = path.join(
+        process.cwd(),
+        getCurrentGeminiMdFilename(),
+      );
+
+      const invocation = memoryTool.build(params);
+      // Add the memory file to the allowlist with the scope-specific key format
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      (invocation.constructor as any).allowlist.add(
+        `${memoryFilePath}_project`,
+      );
+
+      const result = await invocation.shouldConfirmExecute(mockAbortSignal);
+
+      expect(result).toBe(false);
+    });
+
+    it('should add memory file to allowlist when ProceedAlways is confirmed for global scope', async () => {
       const params = { fact: 'Test fact', scope: 'global' as const };
       const memoryFilePath = path.join(
         os.homedir(),
@@ -330,10 +408,8 @@ describe('MemoryTool', () => {
         getCurrentGeminiMdFilename(),
       );
 
-      const result = await memoryTool.shouldConfirmExecute(
-        params,
-        mockAbortSignal,
-      );
+      const invocation = memoryTool.build(params);
+      const result = await invocation.shouldConfirmExecute(mockAbortSignal);
 
       expect(result).toBeDefined();
       expect(result).not.toBe(false);
@@ -342,27 +418,53 @@ describe('MemoryTool', () => {
         // Simulate the onConfirm callback
         await result.onConfirm(ToolConfirmationOutcome.ProceedAlways);
 
-        // Check that the memory file was added to the allowlist with the new key format
+        // Check that the memory file was added to the allowlist with the scope-specific key format
         expect(
-          (MemoryTool as unknown as { allowlist: Set<string> }).allowlist.has(
+          // eslint-disable-next-line @typescript-eslint/no-explicit-any
+          (invocation.constructor as any).allowlist.has(
             `${memoryFilePath}_global`,
           ),
         ).toBe(true);
       }
     });
 
+    it('should add memory file to allowlist when ProceedAlways is confirmed for project scope', async () => {
+      const params = { fact: 'Test fact', scope: 'project' as const };
+      const memoryFilePath = path.join(
+        process.cwd(),
+        getCurrentGeminiMdFilename(),
+      );
+
+      const invocation = memoryTool.build(params);
+      const result = await invocation.shouldConfirmExecute(mockAbortSignal);
+
+      expect(result).toBeDefined();
+      expect(result).not.toBe(false);
+
+      if (result && result.type === 'edit') {
+        // Simulate the onConfirm callback
+        await result.onConfirm(ToolConfirmationOutcome.ProceedAlways);
+
+        // Check that the memory file was added to the allowlist with the scope-specific key format
+        expect(
+          // eslint-disable-next-line @typescript-eslint/no-explicit-any
+          (invocation.constructor as any).allowlist.has(
+            `${memoryFilePath}_project`,
+          ),
+        ).toBe(true);
+      }
+    });
+
     it('should not add memory file to allowlist when other outcomes are confirmed', async () => {
-      const params = { fact: 'Test fact' };
+      const params = { fact: 'Test fact', scope: 'global' as const };
       const memoryFilePath = path.join(
         os.homedir(),
         '.qwen',
         getCurrentGeminiMdFilename(),
       );
 
-      const result = await memoryTool.shouldConfirmExecute(
-        params,
-        mockAbortSignal,
-      );
+      const invocation = memoryTool.build(params);
+      const result = await invocation.shouldConfirmExecute(mockAbortSignal);
 
       expect(result).toBeDefined();
       expect(result).not.toBe(false);
@@ -370,22 +472,16 @@ describe('MemoryTool', () => {
       if (result && result.type === 'edit') {
         // Simulate the onConfirm callback with different outcomes
         await result.onConfirm(ToolConfirmationOutcome.ProceedOnce);
-        expect(
-          (MemoryTool as unknown as { allowlist: Set<string> }).allowlist.has(
-            memoryFilePath,
-          ),
-        ).toBe(false);
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        const allowlist = (invocation.constructor as any).allowlist;
+        expect(allowlist.has(`${memoryFilePath}_global`)).toBe(false);
 
         await result.onConfirm(ToolConfirmationOutcome.Cancel);
-        expect(
-          (MemoryTool as unknown as { allowlist: Set<string> }).allowlist.has(
-            memoryFilePath,
-          ),
-        ).toBe(false);
+        expect(allowlist.has(`${memoryFilePath}_global`)).toBe(false);
       }
     });
 
-    it('should handle existing memory file with content', async () => {
+    it('should handle existing memory file with content for global scope', async () => {
       const params = { fact: 'New fact', scope: 'global' as const };
       const existingContent =
         'Some existing content.\n\n## Qwen Added Memories\n- Old fact\n';
@@ -393,10 +489,8 @@ describe('MemoryTool', () => {
       // Mock fs.readFile to return existing content
       vi.mocked(fs.readFile).mockResolvedValue(existingContent);
 
-      const result = await memoryTool.shouldConfirmExecute(
-        params,
-        mockAbortSignal,
-      );
+      const invocation = memoryTool.build(params);
+      const result = await invocation.shouldConfirmExecute(mockAbortSignal);
 
       expect(result).toBeDefined();
       expect(result).not.toBe(false);
@@ -416,10 +510,8 @@ describe('MemoryTool', () => {
 
     it('should prompt for scope selection when scope is not specified', async () => {
       const params = { fact: 'Test fact' };
-      const result = await memoryTool.shouldConfirmExecute(
-        params,
-        mockAbortSignal,
-      );
+      const invocation = memoryTool.build(params);
+      const result = await invocation.shouldConfirmExecute(mockAbortSignal);
 
       expect(result).toBeDefined();
       expect(result).not.toBe(false);
@@ -435,15 +527,58 @@ describe('MemoryTool', () => {
       }
     });
 
-    it('should return error when executing without scope parameter', async () => {
+    it('should show correct file paths in scope selection prompt', async () => {
       const params = { fact: 'Test fact' };
-      const result = await memoryTool.execute(params, mockAbortSignal);
+      const invocation = memoryTool.build(params);
+      const result = await invocation.shouldConfirmExecute(mockAbortSignal);
 
-      expect(result.llmContent).toContain(
-        'Please specify where to save this memory',
-      );
-      expect(result.returnDisplay).toContain('Global:');
-      expect(result.returnDisplay).toContain('Project:');
+      expect(result).toBeDefined();
+      expect(result).not.toBe(false);
+
+      if (result && result.type === 'edit') {
+        const globalPath = path.join('~', '.qwen', 'QWEN.md');
+        const projectPath = path.join(process.cwd(), 'QWEN.md');
+
+        expect(result.fileDiff).toContain(`Global: ${globalPath}`);
+        expect(result.fileDiff).toContain(`Project: ${projectPath}`);
+        expect(result.fileDiff).toContain('(shared across all projects)');
+        expect(result.fileDiff).toContain('(current project only)');
+      }
+    });
+  });
+
+  describe('getDescription', () => {
+    let memoryTool: MemoryTool;
+
+    beforeEach(() => {
+      memoryTool = new MemoryTool();
+    });
+
+    it('should return correct description for global scope', () => {
+      const params = { fact: 'Test fact', scope: 'global' as const };
+      const invocation = memoryTool.build(params);
+      const description = invocation.getDescription();
+
+      const expectedPath = path.join('~', '.qwen', 'QWEN.md');
+      expect(description).toBe(`${expectedPath} (global)`);
+    });
+
+    it('should return correct description for project scope', () => {
+      const params = { fact: 'Test fact', scope: 'project' as const };
+      const invocation = memoryTool.build(params);
+      const description = invocation.getDescription();
+
+      const expectedPath = path.join(process.cwd(), 'QWEN.md');
+      expect(description).toBe(`${expectedPath} (project)`);
+    });
+
+    it('should default to global scope when scope is not specified', () => {
+      const params = { fact: 'Test fact' };
+      const invocation = memoryTool.build(params);
+      const description = invocation.getDescription();
+
+      const expectedPath = path.join('~', '.qwen', 'QWEN.md');
+      expect(description).toBe(`${expectedPath} (global)`);
     });
   });
 });