Refactor: Streamline memoryUtils and update slash commands (#478)

This commit is contained in:
Allen Hutchison 2025-05-22 10:57:06 -07:00 committed by GitHub
parent 0c192555bb
commit 581709df80
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 101 additions and 135 deletions

View File

@ -0,0 +1,92 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import {
describe,
it,
expect,
vi,
beforeEach,
// afterEach, // Removed as it's not used
type Mocked,
type Mock,
} from 'vitest';
import * as path from 'path';
import { homedir } from 'os';
import * as fs from 'fs/promises';
import { getGlobalMemoryFilePath, addMemoryEntry } from './memoryUtils.js';
import { SETTINGS_DIRECTORY_NAME } from './settings.js';
import {
MemoryTool,
GEMINI_MD_FILENAME,
// MEMORY_SECTION_HEADER, // Removed as it's not used
// getErrorMessage, // Removed as it's not used
} from '@gemini-code/server';
// Mock the entire fs/promises module
vi.mock('fs/promises');
// Mock MemoryTool static method
vi.mock('@gemini-code/server', async (importOriginal) => {
const actual = await importOriginal<typeof import('@gemini-code/server')>();
return {
...actual,
MemoryTool: {
...actual.MemoryTool,
performAddMemoryEntry: vi.fn(),
},
};
});
describe('memoryUtils', () => {
beforeEach(() => {
// Reset mocks before each test
vi.resetAllMocks();
});
describe('getGlobalMemoryFilePath', () => {
it('should return the correct global memory file path', () => {
const expectedPath = path.join(
homedir(),
SETTINGS_DIRECTORY_NAME,
GEMINI_MD_FILENAME,
);
expect(getGlobalMemoryFilePath()).toBe(expectedPath);
});
});
describe('addMemoryEntry', () => {
const mockFs = fs as Mocked<typeof fs>; // Type cast for mocked fs
const mockPerformAddMemoryEntry = MemoryTool.performAddMemoryEntry as Mock;
it('should call MemoryTool.performAddMemoryEntry with correct parameters', async () => {
const testText = 'Remember this important fact.';
const expectedFilePath = getGlobalMemoryFilePath();
await addMemoryEntry(testText);
expect(mockPerformAddMemoryEntry).toHaveBeenCalledOnce();
expect(mockPerformAddMemoryEntry).toHaveBeenCalledWith(
testText,
expectedFilePath,
{
readFile: mockFs.readFile,
writeFile: mockFs.writeFile,
mkdir: mockFs.mkdir,
},
);
});
it('should propagate errors from MemoryTool.performAddMemoryEntry', async () => {
const testText = 'This will fail.';
const expectedError = new Error('Failed to add memory entry');
mockPerformAddMemoryEntry.mockRejectedValueOnce(expectedError);
await expect(addMemoryEntry(testText)).rejects.toThrow(expectedError);
});
});
// More tests will be added here
});

View File

@ -9,10 +9,10 @@ import * as path from 'path';
import { homedir } from 'os';
import { SETTINGS_DIRECTORY_NAME } from './settings.js';
import {
getErrorMessage,
// getErrorMessage, // Removed as it's not used
MemoryTool,
GEMINI_MD_FILENAME,
MEMORY_SECTION_HEADER,
// MEMORY_SECTION_HEADER, // Removed as it's not used
} from '@gemini-code/server';
/**
@ -35,129 +35,3 @@ export async function addMemoryEntry(text: string): Promise<void> {
mkdir: fs.mkdir,
});
}
/**
* Deletes the last added memory entry from the "Gemini Added Memories" section.
*/
export async function deleteLastMemoryEntry(): Promise<boolean> {
const filePath = getGlobalMemoryFilePath();
try {
let content = await fs.readFile(filePath, 'utf-8');
const headerIndex = content.indexOf(MEMORY_SECTION_HEADER);
if (headerIndex === -1) return false; // Section not found
const startOfSectionContent = headerIndex + MEMORY_SECTION_HEADER.length;
let endOfSectionIndex = content.indexOf('\n## ', startOfSectionContent);
if (endOfSectionIndex === -1) {
endOfSectionIndex = content.length;
}
const sectionPart = content.substring(
startOfSectionContent,
endOfSectionIndex,
);
const lines = sectionPart.split(/\r?\n/).map((line) => line.trimEnd());
let lastBulletLineIndex = -1;
for (let i = lines.length - 1; i >= 0; i--) {
if (lines[i].trim().startsWith('- ')) {
lastBulletLineIndex = i;
break;
}
}
if (lastBulletLineIndex === -1) return false; // No bullets found in section
lines.splice(lastBulletLineIndex, 1);
const newSectionPart = lines
.filter((line) => line.trim().length > 0)
.join('\n');
const beforeHeader = content.substring(0, headerIndex);
const afterSection = content.substring(endOfSectionIndex);
if (newSectionPart.trim().length === 0) {
// If section is now empty (no bullets), remove header too or leave it clean
// For simplicity, let's leave the header but ensure it has a newline after if content follows
content = `${beforeHeader}${MEMORY_SECTION_HEADER}\n${afterSection}`
.replace(/\n{3,}/g, '\n\n')
.trimEnd();
if (content.length > 0) content += '\n';
} else {
content =
`${beforeHeader}${MEMORY_SECTION_HEADER}\n${newSectionPart}\n${afterSection}`
.replace(/\n{3,}/g, '\n\n')
.trimEnd();
if (content.length > 0) content += '\n';
}
await fs.writeFile(filePath, content, 'utf-8');
return true;
} catch (error) {
if (error instanceof Error && 'code' in error && error.code === 'ENOENT') {
return false;
}
console.error(`Error deleting last memory entry from ${filePath}:`, error);
throw new Error(
`Failed to delete last memory entry: ${getErrorMessage(error)}`,
);
}
}
/**
* Deletes all added memory entries (the entire "Gemini Added Memories" section).
*/
export async function deleteAllAddedMemoryEntries(): Promise<number> {
const filePath = getGlobalMemoryFilePath();
try {
let content = await fs.readFile(filePath, 'utf-8');
const headerIndex = content.indexOf(MEMORY_SECTION_HEADER);
if (headerIndex === -1) return 0; // Section not found
let endOfSectionIndex = content.indexOf(
'\n## ',
headerIndex + MEMORY_SECTION_HEADER.length,
);
if (endOfSectionIndex === -1) {
endOfSectionIndex = content.length; // Section goes to EOF
}
const sectionContent = content.substring(headerIndex, endOfSectionIndex);
const bulletCount = (sectionContent.match(/\n- /g) || []).length;
if (bulletCount === 0 && !sectionContent.includes('- ')) {
// No bullets found
// If we only remove if bullets exist, or remove header if no bullets.
// For now, if header exists but no bullets, consider 0 deleted if we only count bullets.
// If the goal is to remove the section if it exists, this logic changes.
// Let's assume we only care about bulleted items for the count.
}
// Remove the section including the header
const beforeHeader = content.substring(0, headerIndex);
const afterSection = content.substring(endOfSectionIndex);
content = (
beforeHeader.trimEnd() +
(afterSection.length > 0 ? '\n' + afterSection.trimStart() : '')
).trim();
if (content.length > 0) content += '\n';
await fs.writeFile(filePath, content, 'utf-8');
return bulletCount; // This counts '\n- ' occurrences, might need refinement for exact bullet count
} catch (error) {
if (error instanceof Error && 'code' in error && error.code === 'ENOENT') {
return 0;
}
console.error(
`Error deleting all added memory entries from ${filePath}:`,
error,
);
throw new Error(
`Failed to delete all added memory entries: ${getErrorMessage(error)}`,
);
}
}

View File

@ -77,13 +77,13 @@ describe('useSlashCommandProcessor', () => {
performAddMemoryEntrySpy.mockReset();
(open as Mock).mockClear();
vi.spyOn(memoryUtils, 'deleteLastMemoryEntry').mockImplementation(vi.fn());
vi.spyOn(memoryUtils, 'deleteAllAddedMemoryEntries').mockImplementation(
vi.fn(),
);
// vi.spyOn(memoryUtils, 'deleteLastMemoryEntry').mockImplementation(vi.fn());
// vi.spyOn(memoryUtils, 'deleteAllAddedMemoryEntries').mockImplementation(
// vi.fn(),
// );
vi.mocked(memoryUtils.deleteLastMemoryEntry).mockClear();
vi.mocked(memoryUtils.deleteAllAddedMemoryEntries).mockClear();
// vi.mocked(memoryUtils.deleteLastMemoryEntry).mockClear();
// vi.mocked(memoryUtils.deleteAllAddedMemoryEntries).mockClear();
mockProcessExit.mockClear();
(ShowMemoryCommandModule.createShowMemoryAction as Mock).mockClear();

View File

@ -113,7 +113,7 @@ export const useSlashCommandProcessor = (
{
name: 'memory',
description:
'Manage memory. Usage: /memory <show|refresh|add|delete_last|delete_all_added> [text for add]',
'Manage memory. Usage: /memory <show|refresh|add> [text for add]',
action: (mainCommand, subCommand, args) => {
switch (subCommand) {
case 'show':