Release v1.1.0: Phase 1.1 - Path Normalization & Error Handling

- Add PathUtils for cross-platform path normalization and validation - Add ErrorMessages with context-aware, actionable error messages - Update all tool implementations with enhanced path handling - Improve tool descriptions for AI agents with detailed guidance - Add Jest testing infrastructure with 43 passing tests - Add comprehensive documentation (Tool Selection Guide, error improvements) - Fix cross-platform path issues (Windows backslashes, case sensitivity) - Fix delete folder error message (clear 'cannot delete folders' message) - Fix parent folder detection with specific error messages - All changes backward compatible with v1.0.0 New files: - src/utils/path-utils.ts - Path normalization utilities - src/utils/error-messages.ts - Enhanced error messages - tests/__mocks__/obsidian.ts - Mock Obsidian API - tests/path-utils.test.ts - 43 unit tests - tests/README.md - Testing guide - jest.config.js - Jest configuration - docs/TOOL_SELECTION_GUIDE.md - Comprehensive tool guide - docs/ERROR_MESSAGE_IMPROVEMENTS.md - Error message documentation - docs/TOOL_DESCRIPTION_IMPROVEMENTS.md - AI agent improvements - PHASE_1.1_IMPLEMENTATION.md - Implementation summary - RELEASE_NOTES_v1.1.0.md - Release notes Updated: - CHANGELOG.md - Add v1.1.0 entry - ROADMAP.md - Mark Phase 1.1 complete, add Phase 1.5 proposal - manifest.json - Bump to v1.1.0 - package.json - Bump to v1.1.0, add test scripts - src/tools/index.ts - Enhanced tool descriptions - src/tools/note-tools.ts - Use PathUtils and ErrorMessages - src/tools/vault-tools.ts - Use PathUtils and ErrorMessages
2025-10-16 21:27:23 -04:00
parent 08cc6e9ea6
commit 7524271eaa
15 changed files with 5638 additions and 119 deletions
--- a/tests/README.md
+++ b/tests/README.md
@@ -0,0 +1,132 @@
+# Tests
+
+This directory contains unit and integration tests for the Obsidian MCP Server plugin.
+
+## Current Status
+
+The test files are currently documentation of expected behavior. To actually run these tests, you need to set up a testing framework.
+
+## Setting Up Jest (Recommended)
+
+1. Install Jest and related dependencies:
+```bash
+npm install --save-dev jest @types/jest ts-jest
+```
+
+2. Create a `jest.config.js` file in the project root:
+```javascript
+module.exports = {
+  preset: 'ts-jest',
+  testEnvironment: 'node',
+  roots: ['<rootDir>/tests'],
+  testMatch: ['**/*.test.ts'],
+  moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json', 'node'],
+  collectCoverageFrom: [
+    'src/**/*.ts',
+    '!src/**/*.d.ts',
+  ],
+};
+```
+
+3. Add test script to `package.json`:
+```json
+{
+  "scripts": {
+    "test": "jest",
+    "test:watch": "jest --watch",
+    "test:coverage": "jest --coverage"
+  }
+}
+```
+
+4. Run tests:
+```bash
+npm test
+```
+
+## Test Files
+
+### `path-utils.test.ts`
+
+Tests for the `PathUtils` class, covering:
+- Path normalization (cross-platform)
+- Path validation
+- File/folder resolution
+- Path manipulation utilities
+
+**Key Test Categories:**
+- **normalizePath**: Tests for handling leading/trailing slashes, backslashes, drive letters
+- **isValidVaultPath**: Tests for path validation rules
+- **Cross-platform**: Tests for Windows, macOS, and Linux path handling
+
+## Mocking Obsidian API
+
+Since these tests run outside of Obsidian, you'll need to mock the Obsidian API:
+
+```typescript
+// Example mock setup
+jest.mock('obsidian', () => ({
+  App: jest.fn(),
+  TFile: jest.fn(),
+  TFolder: jest.fn(),
+  TAbstractFile: jest.fn(),
+  // ... other Obsidian types
+}));
+```
+
+## Running Tests Without Jest
+
+If you prefer not to set up Jest, you can:
+
+1. Use the test files as documentation of expected behavior
+2. Manually test the functionality through the MCP server
+3. Use TypeScript's type checking to catch errors: `npm run build`
+
+## Future Improvements
+
+- [ ] Set up Jest testing framework
+- [ ] Add integration tests with mock Obsidian vault
+- [ ] Add tests for error-messages.ts
+- [ ] Add tests for tool implementations
+- [ ] Add tests for MCP server endpoints
+- [ ] Set up CI/CD with automated testing
+- [ ] Add code coverage reporting
+
+## Test Coverage Goals
+
+- **PathUtils**: 100% coverage (critical for cross-platform support)
+- **ErrorMessages**: 100% coverage (important for user experience)
+- **Tool implementations**: 80%+ coverage
+- **Server/middleware**: 70%+ coverage
+
+## Writing New Tests
+
+When adding new features, please:
+
+1. Write tests first (TDD approach recommended)
+2. Test both success and error cases
+3. Test edge cases and boundary conditions
+4. Test cross-platform compatibility where relevant
+5. Add descriptive test names that explain the expected behavior
+
+Example test structure:
+```typescript
+describe('FeatureName', () => {
+  describe('methodName', () => {
+    test('should handle normal case', () => {
+      // Arrange
+      const input = 'test';
+      
+      // Act
+      const result = method(input);
+      
+      // Assert
+      expect(result).toBe('expected');
+    });
+
+    test('should handle error case', () => {
+      expect(() => method(null)).toThrow();
+    });
+  });
+});
+```
--- a/tests/mocks/obsidian.ts
+++ b/tests/mocks/obsidian.ts
@@ -0,0 +1,73 @@
+/**
+ * Mock Obsidian API for testing
+ * This provides minimal mocks for the Obsidian types used in tests
+ */
+
+export class TFile {
+	path: string;
+	basename: string;
+	extension: string;
+
+	constructor(path: string) {
+		this.path = path;
+		const parts = path.split('/');
+		const filename = parts[parts.length - 1];
+		const dotIndex = filename.lastIndexOf('.');
+		this.basename = dotIndex > 0 ? filename.substring(0, dotIndex) : filename;
+		this.extension = dotIndex > 0 ? filename.substring(dotIndex + 1) : '';
+	}
+}
+
+export class TFolder {
+	path: string;
+	name: string;
+
+	constructor(path: string) {
+		this.path = path;
+		const parts = path.split('/');
+		this.name = parts[parts.length - 1];
+	}
+}
+
+export class TAbstractFile {
+	path: string;
+	name: string;
+
+	constructor(path: string) {
+		this.path = path;
+		const parts = path.split('/');
+		this.name = parts[parts.length - 1];
+	}
+}
+
+export class Vault {
+	private files: Map<string, TFile | TFolder> = new Map();
+
+	getAbstractFileByPath(path: string): TFile | TFolder | null {
+		return this.files.get(path) || null;
+	}
+
+	// Helper method for tests to add mock files
+	_addMockFile(path: string, isFolder = false) {
+		this.files.set(path, isFolder ? new TFolder(path) : new TFile(path));
+	}
+
+	// Helper method for tests to clear mock files
+	_clearMockFiles() {
+		this.files.clear();
+	}
+}
+
+export class App {
+	vault: Vault;
+
+	constructor() {
+		this.vault = new Vault();
+	}
+}
+
+// Export other commonly used types as empty classes/interfaces
+export class Plugin {}
+export class Notice {}
+export class PluginSettingTab {}
+export class Setting {}
--- a/tests/path-utils.test.ts
+++ b/tests/path-utils.test.ts
@@ -0,0 +1,261 @@
+/**
+ * Unit tests for PathUtils
+ */
+
+import { PathUtils } from '../src/utils/path-utils';
+import { App, TFile, TFolder } from 'obsidian';
+
+describe('PathUtils', () => {
+	describe('normalizePath', () => {
+		test('should strip leading slashes', () => {
+			expect(PathUtils.normalizePath('/folder/note.md')).toBe('folder/note.md');
+			expect(PathUtils.normalizePath('//folder/note.md')).toBe('folder/note.md');
+		});
+
+		test('should strip trailing slashes', () => {
+			expect(PathUtils.normalizePath('folder/note.md/')).toBe('folder/note.md');
+			expect(PathUtils.normalizePath('folder/note.md//')).toBe('folder/note.md');
+		});
+
+		test('should convert backslashes to forward slashes', () => {
+			expect(PathUtils.normalizePath('folder\\note.md')).toBe('folder/note.md');
+			expect(PathUtils.normalizePath('folder\\subfolder\\note.md')).toBe('folder/subfolder/note.md');
+		});
+
+		test('should handle multiple consecutive slashes', () => {
+			expect(PathUtils.normalizePath('folder//subfolder///note.md')).toBe('folder/subfolder/note.md');
+		});
+
+		test('should handle Windows drive letters', () => {
+			expect(PathUtils.normalizePath('C:/folder/note.md')).toBe('C:/folder/note.md');
+		});
+
+		test('should handle empty strings', () => {
+			expect(PathUtils.normalizePath('')).toBe('');
+		});
+
+		test('should handle simple filenames', () => {
+			expect(PathUtils.normalizePath('note.md')).toBe('note.md');
+		});
+
+		test('should handle complex paths', () => {
+			expect(PathUtils.normalizePath('/folder/subfolder/note.md/')).toBe('folder/subfolder/note.md');
+			expect(PathUtils.normalizePath('\\folder\\subfolder\\note.md\\')).toBe('folder/subfolder/note.md');
+		});
+	});
+
+	describe('isValidVaultPath', () => {
+		test('should accept valid paths', () => {
+			expect(PathUtils.isValidVaultPath('note.md')).toBe(true);
+			expect(PathUtils.isValidVaultPath('folder/note.md')).toBe(true);
+			expect(PathUtils.isValidVaultPath('folder/subfolder/note.md')).toBe(true);
+		});
+
+		test('should reject empty paths', () => {
+			expect(PathUtils.isValidVaultPath('')).toBe(false);
+			expect(PathUtils.isValidVaultPath('   ')).toBe(false);
+		});
+
+		test('should reject paths with invalid characters', () => {
+			expect(PathUtils.isValidVaultPath('note<test>.md')).toBe(false);
+			expect(PathUtils.isValidVaultPath('note:test.md')).toBe(false);
+			expect(PathUtils.isValidVaultPath('note|test.md')).toBe(false);
+			expect(PathUtils.isValidVaultPath('note?test.md')).toBe(false);
+			expect(PathUtils.isValidVaultPath('note*test.md')).toBe(false);
+		});
+
+		test('should reject parent directory traversal', () => {
+			expect(PathUtils.isValidVaultPath('../note.md')).toBe(false);
+			expect(PathUtils.isValidVaultPath('folder/../note.md')).toBe(false);
+		});
+
+		test('should accept paths after normalization', () => {
+			// These should be valid after normalization
+			expect(PathUtils.isValidVaultPath('/folder/note.md')).toBe(true);
+			expect(PathUtils.isValidVaultPath('folder/note.md/')).toBe(true);
+		});
+	});
+
+	describe('ensureMarkdownExtension', () => {
+		test('should add .md extension if missing', () => {
+			expect(PathUtils.ensureMarkdownExtension('note')).toBe('note.md');
+			expect(PathUtils.ensureMarkdownExtension('folder/note')).toBe('folder/note.md');
+		});
+
+		test('should not add .md if already present', () => {
+			expect(PathUtils.ensureMarkdownExtension('note.md')).toBe('note.md');
+			expect(PathUtils.ensureMarkdownExtension('folder/note.md')).toBe('folder/note.md');
+		});
+
+		test('should normalize path before adding extension', () => {
+			expect(PathUtils.ensureMarkdownExtension('/folder/note/')).toBe('folder/note.md');
+		});
+	});
+
+	describe('getParentPath', () => {
+		test('should return parent folder path', () => {
+			expect(PathUtils.getParentPath('folder/note.md')).toBe('folder');
+			expect(PathUtils.getParentPath('folder/subfolder/note.md')).toBe('folder/subfolder');
+		});
+
+		test('should return empty string for root-level files', () => {
+			expect(PathUtils.getParentPath('note.md')).toBe('');
+		});
+
+		test('should normalize path first', () => {
+			expect(PathUtils.getParentPath('/folder/note.md/')).toBe('folder');
+		});
+	});
+
+	describe('getBasename', () => {
+		test('should return filename without path', () => {
+			expect(PathUtils.getBasename('folder/note.md')).toBe('note.md');
+			expect(PathUtils.getBasename('folder/subfolder/note.md')).toBe('note.md');
+		});
+
+		test('should return the filename for root-level files', () => {
+			expect(PathUtils.getBasename('note.md')).toBe('note.md');
+		});
+
+		test('should normalize path first', () => {
+			expect(PathUtils.getBasename('/folder/note.md/')).toBe('note.md');
+		});
+	});
+
+	describe('joinPath', () => {
+		test('should join path segments', () => {
+			expect(PathUtils.joinPath('folder', 'note.md')).toBe('folder/note.md');
+			expect(PathUtils.joinPath('folder', 'subfolder', 'note.md')).toBe('folder/subfolder/note.md');
+		});
+
+		test('should handle empty segments', () => {
+			expect(PathUtils.joinPath('folder', '', 'note.md')).toBe('folder/note.md');
+			expect(PathUtils.joinPath('', 'folder', 'note.md')).toBe('folder/note.md');
+		});
+
+		test('should normalize each segment', () => {
+			expect(PathUtils.joinPath('/folder/', '/subfolder/', '/note.md/')).toBe('folder/subfolder/note.md');
+		});
+
+		test('should handle single segment', () => {
+			expect(PathUtils.joinPath('note.md')).toBe('note.md');
+		});
+	});
+});
+
+/**
+ * Integration tests for PathUtils with Obsidian App
+ */
+describe('PathUtils - Integration with Obsidian', () => {
+	let mockApp: App;
+
+	beforeEach(() => {
+		mockApp = new App();
+		// Clear any previous mock files
+		(mockApp.vault as any)._clearMockFiles();
+	});
+
+	describe('resolveVaultPath', () => {
+		test('should return null for invalid paths', () => {
+			expect(PathUtils.resolveVaultPath(mockApp, '../note.md')).toBe(null);
+			expect(PathUtils.resolveVaultPath(mockApp, '')).toBe(null);
+		});
+
+		test('should normalize path before resolving', () => {
+			(mockApp.vault as any)._addMockFile('folder/note.md', false);
+			
+			const result = PathUtils.resolveVaultPath(mockApp, '/folder/note.md/');
+			expect(result).not.toBe(null);
+			expect(result?.path).toBe('folder/note.md');
+		});
+
+		test('should return file when it exists', () => {
+			(mockApp.vault as any)._addMockFile('note.md', false);
+			
+			const result = PathUtils.resolveVaultPath(mockApp, 'note.md');
+			expect(result).toBeInstanceOf(TFile);
+			expect(result?.path).toBe('note.md');
+		});
+
+		test('should return folder when it exists', () => {
+			(mockApp.vault as any)._addMockFile('folder', true);
+			
+			const result = PathUtils.resolveVaultPath(mockApp, 'folder');
+			expect(result).toBeInstanceOf(TFolder);
+			expect(result?.path).toBe('folder');
+		});
+	});
+
+	describe('fileExists', () => {
+		test('should return true if file exists', () => {
+			(mockApp.vault as any)._addMockFile('note.md', false);
+			expect(PathUtils.fileExists(mockApp, 'note.md')).toBe(true);
+		});
+
+		test('should return false if file does not exist', () => {
+			expect(PathUtils.fileExists(mockApp, 'note.md')).toBe(false);
+		});
+
+		test('should return false if path is a folder', () => {
+			(mockApp.vault as any)._addMockFile('folder', true);
+			expect(PathUtils.fileExists(mockApp, 'folder')).toBe(false);
+		});
+	});
+
+	describe('folderExists', () => {
+		test('should return true if folder exists', () => {
+			(mockApp.vault as any)._addMockFile('folder', true);
+			expect(PathUtils.folderExists(mockApp, 'folder')).toBe(true);
+		});
+
+		test('should return false if folder does not exist', () => {
+			expect(PathUtils.folderExists(mockApp, 'folder')).toBe(false);
+		});
+
+		test('should return false if path is a file', () => {
+			(mockApp.vault as any)._addMockFile('note.md', false);
+			expect(PathUtils.folderExists(mockApp, 'note.md')).toBe(false);
+		});
+	});
+
+	describe('getPathType', () => {
+		test('should return "file" for files', () => {
+			(mockApp.vault as any)._addMockFile('note.md', false);
+			expect(PathUtils.getPathType(mockApp, 'note.md')).toBe('file');
+		});
+
+		test('should return "folder" for folders', () => {
+			(mockApp.vault as any)._addMockFile('folder', true);
+			expect(PathUtils.getPathType(mockApp, 'folder')).toBe('folder');
+		});
+
+		test('should return null for non-existent paths', () => {
+			expect(PathUtils.getPathType(mockApp, 'nonexistent')).toBe(null);
+		});
+	});
+});
+
+/**
+ * Test cases for cross-platform path handling
+ */
+describe('PathUtils - Cross-platform', () => {
+	describe('Windows paths', () => {
+		test('should handle backslashes', () => {
+			expect(PathUtils.normalizePath('folder\\subfolder\\note.md')).toBe('folder/subfolder/note.md');
+		});
+
+		test('should handle mixed slashes', () => {
+			expect(PathUtils.normalizePath('folder\\subfolder/note.md')).toBe('folder/subfolder/note.md');
+		});
+	});
+
+	describe('macOS/Linux paths', () => {
+		test('should preserve forward slashes', () => {
+			expect(PathUtils.normalizePath('folder/subfolder/note.md')).toBe('folder/subfolder/note.md');
+		});
+
+		test('should handle leading slashes', () => {
+			expect(PathUtils.normalizePath('/folder/note.md')).toBe('folder/note.md');
+		});
+	});
+});