Merge pull request #2924 from olaservo/update-tests-and-markdown-files

Add tests for memory server, fix filesystem tests, and move testing guidelines

Author: olaservo

AGENTS.md

@@ -21,6 +21,10 @@ We're more selective about:
 We don't accept:
 - **New server implementations** — We encourage you to publish them yourself, and link to them from the README.
 
+## Testing
+
+When adding or configuring tests for servers implemented in TypeScript, use **vitest** as the test framework. Vitest provides better ESM support, faster test execution, and a more modern testing experience.
+
 ## Documentation
 
 Improvements to existing documentation is welcome - although generally we'd prefer ergonomic improvements than documenting pain points if possible!

CLAUDE.md

@@ -1,4 +1,4 @@
-import { describe, it, expect } from 'vitest';
+import { describe, it, expect, afterEach } from 'vitest';
 import { normalizePath, expandHome, convertToWindowsPath } from '../path-utils.js';
 
 describe('Path Utilities', () => {
@@ -196,11 +196,8 @@ describe('Path Utilities', () => {
     });
 
     it('returns normalized non-Windows/WSL/Unix-style Windows paths as is after basic normalization', () => {
-      // Relative path
-      const relativePath = 'some/relative/path';
-      expect(normalizePath(relativePath)).toBe(relativePath.replace(/\//g, '\\'));
-
       // A path that looks somewhat absolute but isn't a drive or recognized Unix root for Windows conversion
+      // These paths should be preserved as-is (not converted to Windows C:\ format or WSL format)
       const otherAbsolutePath = '\\someserver\\share\\file';
       expect(normalizePath(otherAbsolutePath)).toBe(otherAbsolutePath);
     });
@@ -350,5 +347,19 @@ describe('Path Utilities', () => {
       expect(result).not.toContain('C:');
       expect(result).not.toContain('\\');
     });
+
+    it('should handle relative path slash conversion based on platform', () => {
+      // This test verifies platform-specific behavior naturally without mocking
+      // On Windows: forward slashes converted to backslashes
+      // On Linux/Unix: forward slashes preserved
+      const relativePath = 'some/relative/path';
+      const result = normalizePath(relativePath);
+
+      if (originalPlatform === 'win32') {
+        expect(result).toBe('some\\relative\\path');
+      } else {
+        expect(result).toBe('some/relative/path');
+      }
+    });
   });
 });

CONTRIBUTING.md

@@ -0,0 +1,156 @@
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
+import { promises as fs } from 'fs';
+import path from 'path';
+import { fileURLToPath } from 'url';
+import { ensureMemoryFilePath, defaultMemoryPath } from '../index.js';
+
+describe('ensureMemoryFilePath', () => {
+  const testDir = path.dirname(fileURLToPath(import.meta.url));
+  const oldMemoryPath = path.join(testDir, '..', 'memory.json');
+  const newMemoryPath = path.join(testDir, '..', 'memory.jsonl');
+
+  let originalEnv: string | undefined;
+
+  beforeEach(() => {
+    // Save original environment variable
+    originalEnv = process.env.MEMORY_FILE_PATH;
+    // Delete environment variable
+    delete process.env.MEMORY_FILE_PATH;
+  });
+
+  afterEach(async () => {
+    // Restore original environment variable
+    if (originalEnv !== undefined) {
+      process.env.MEMORY_FILE_PATH = originalEnv;
+    } else {
+      delete process.env.MEMORY_FILE_PATH;
+    }
+
+    // Clean up test files
+    try {
+      await fs.unlink(oldMemoryPath);
+    } catch {
+      // Ignore if file doesn't exist
+    }
+    try {
+      await fs.unlink(newMemoryPath);
+    } catch {
+      // Ignore if file doesn't exist
+    }
+  });
+
+  describe('with MEMORY_FILE_PATH environment variable', () => {
+    it('should return absolute path when MEMORY_FILE_PATH is absolute', async () => {
+      const absolutePath = '/tmp/custom-memory.jsonl';
+      process.env.MEMORY_FILE_PATH = absolutePath;
+
+      const result = await ensureMemoryFilePath();
+
+      expect(result).toBe(absolutePath);
+    });
+
+    it('should convert relative path to absolute when MEMORY_FILE_PATH is relative', async () => {
+      const relativePath = 'custom-memory.jsonl';
+      process.env.MEMORY_FILE_PATH = relativePath;
+
+      const result = await ensureMemoryFilePath();
+
+      expect(path.isAbsolute(result)).toBe(true);
+      expect(result).toContain('custom-memory.jsonl');
+    });
+
+    it('should handle Windows absolute paths', async () => {
+      const windowsPath = 'C:\\temp\\memory.jsonl';
+      process.env.MEMORY_FILE_PATH = windowsPath;
+
+      const result = await ensureMemoryFilePath();
+
+      // On Windows, should return as-is; on Unix, will be treated as relative
+      if (process.platform === 'win32') {
+        expect(result).toBe(windowsPath);
+      } else {
+        expect(path.isAbsolute(result)).toBe(true);
+      }
+    });
+  });
+
+  describe('without MEMORY_FILE_PATH environment variable', () => {
+    it('should return default path when no files exist', async () => {
+      const result = await ensureMemoryFilePath();
+
+      expect(result).toBe(defaultMemoryPath);
+    });
+
+    it('should migrate from memory.json to memory.jsonl when only old file exists', async () => {
+      // Create old memory.json file
+      await fs.writeFile(oldMemoryPath, '{"test":"data"}');
+
+      const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+
+      const result = await ensureMemoryFilePath();
+
+      expect(result).toBe(defaultMemoryPath);
+
+      // Verify migration happened
+      const newFileExists = await fs.access(newMemoryPath).then(() => true).catch(() => false);
+      const oldFileExists = await fs.access(oldMemoryPath).then(() => true).catch(() => false);
+
+      expect(newFileExists).toBe(true);
+      expect(oldFileExists).toBe(false);
+
+      // Verify console messages
+      expect(consoleErrorSpy).toHaveBeenCalledWith(
+        expect.stringContaining('DETECTED: Found legacy memory.json file')
+      );
+      expect(consoleErrorSpy).toHaveBeenCalledWith(
+        expect.stringContaining('COMPLETED: Successfully migrated')
+      );
+
+      consoleErrorSpy.mockRestore();
+    });
+
+    it('should use new file when both old and new files exist', async () => {
+      // Create both files
+      await fs.writeFile(oldMemoryPath, '{"old":"data"}');
+      await fs.writeFile(newMemoryPath, '{"new":"data"}');
+
+      const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+
+      const result = await ensureMemoryFilePath();
+
+      expect(result).toBe(defaultMemoryPath);
+
+      // Verify no migration happened (both files should still exist)
+      const newFileExists = await fs.access(newMemoryPath).then(() => true).catch(() => false);
+      const oldFileExists = await fs.access(oldMemoryPath).then(() => true).catch(() => false);
+
+      expect(newFileExists).toBe(true);
+      expect(oldFileExists).toBe(true);
+
+      // Verify no console messages about migration
+      expect(consoleErrorSpy).not.toHaveBeenCalled();
+
+      consoleErrorSpy.mockRestore();
+    });
+
+    it('should preserve file content during migration', async () => {
+      const testContent = '{"entities": [{"name": "test", "type": "person"}]}';
+      await fs.writeFile(oldMemoryPath, testContent);
+
+      await ensureMemoryFilePath();
+
+      const migratedContent = await fs.readFile(newMemoryPath, 'utf-8');
+      expect(migratedContent).toBe(testContent);
+    });
+  });
+
+  describe('defaultMemoryPath', () => {
+    it('should end with memory.jsonl', () => {
+      expect(defaultMemoryPath).toMatch(/memory\.jsonl$/);
+    });
+
+    it('should be an absolute path', () => {
+      expect(path.isAbsolute(defaultMemoryPath)).toBe(true);
+    });
+  });
+});