diff --git a/src/vfs/VirtualFileSystem.ts b/src/vfs/VirtualFileSystem.ts index a7b14fe6..42c3cbf5 100644 --- a/src/vfs/VirtualFileSystem.ts +++ b/src/vfs/VirtualFileSystem.ts @@ -77,6 +77,9 @@ export class VirtualFileSystem implements IVirtualFileSystem { // Background task timer private backgroundTimer: NodeJS.Timeout | null = null + // Mutex for preventing race conditions in directory creation + private mkdirLocks: Map> = new Map() + constructor(brain?: Brainy) { this.brain = brain || new Brainy() this.contentCache = new Map() @@ -236,15 +239,19 @@ export class VirtualFileSystem implements IVirtualFileSystem { // Get content based on storage type let content: Buffer + let isCompressed = false if (!entity.metadata.storage || entity.metadata.storage.type === 'inline') { // Content stored in metadata for new files, or try entity data for compatibility if (entity.metadata.rawData) { + // rawData is ALWAYS stored uncompressed as base64 content = Buffer.from(entity.metadata.rawData, 'base64') + isCompressed = false // rawData is never compressed } else if (!entity.data) { content = Buffer.alloc(0) } else if (Buffer.isBuffer(entity.data)) { content = entity.data + isCompressed = entity.metadata.storage?.compressed || false } else if (typeof entity.data === 'string') { content = Buffer.from(entity.data) } else { @@ -253,15 +260,17 @@ export class VirtualFileSystem implements IVirtualFileSystem { } else if (entity.metadata.storage.type === 'reference') { // Content stored in external storage content = await this.readExternalContent(entity.metadata.storage.key!) + isCompressed = entity.metadata.storage.compressed || false } else if (entity.metadata.storage.type === 'chunked') { // Content stored in chunks content = await this.readChunkedContent(entity.metadata.storage.chunks!) + isCompressed = entity.metadata.storage.compressed || false } else { throw new VFSError(VFSErrorCode.EIO, `Unknown storage type: ${entity.metadata.storage.type}`, path, 'readFile') } - // Decompress if needed - if (entity.metadata.storage?.compressed && options?.decompress !== false) { + // Decompress if needed (but NOT for rawData which is never compressed) + if (isCompressed && options?.decompress !== false) { content = await this.decompress(content) } @@ -406,7 +415,7 @@ export class VirtualFileSystem implements IVirtualFileSystem { metadata }) - // Create parent-child relationship + // Create parent-child relationship (no need to check for duplicates on new entities) await this.brain.relate({ from: parentId, to: entity, @@ -643,80 +652,108 @@ export class VirtualFileSystem implements IVirtualFileSystem { async mkdir(path: string, options?: MkdirOptions): Promise { await this.ensureInitialized() - // Check if already exists - try { - const existing = await this.pathResolver.resolve(path) - const entity = await this.getEntityById(existing) - if (entity.metadata.vfsType === 'directory') { - if (!options?.recursive) { - throw new VFSError(VFSErrorCode.EEXIST, `Directory exists: ${path}`, path, 'mkdir') - } - return // Already exists and recursive is true - } else { - // Path exists but it's not a directory - throw new VFSError(VFSErrorCode.EEXIST, `File exists: ${path}`, path, 'mkdir') - } - } catch (err) { - // Only proceed if it's a ENOENT error (path doesn't exist) - if (err instanceof VFSError && err.code !== VFSErrorCode.ENOENT) { - throw err // Re-throw non-ENOENT errors - } - // Doesn't exist, proceed to create - } - - // Parse path - const parentPath = this.getParentPath(path) - const name = this.getBasename(path) - - // Ensure parent exists (recursive mkdir if needed) - let parentId: string - if (parentPath === '/' || parentPath === null) { - parentId = this.rootEntityId! - } else if (options?.recursive) { - parentId = await this.ensureDirectory(parentPath) - } else { + // Use mutex to prevent race conditions when creating the same directory concurrently + // If another call is already creating this directory, wait for it to complete + const existingLock = this.mkdirLocks.get(path) + if (existingLock) { + await existingLock + // After waiting, check if directory now exists try { - parentId = await this.pathResolver.resolve(parentPath) + const existing = await this.pathResolver.resolve(path) + const entity = await this.getEntityById(existing) + if (entity.metadata.vfsType === 'directory') { + return // Directory was created by the other call + } } catch (err) { - throw new VFSError(VFSErrorCode.ENOENT, `Parent directory not found: ${parentPath}`, path, 'mkdir') + // Still doesn't exist, proceed to create } } - // Create directory entity - const metadata: VFSMetadata = { - path, - name, - parent: parentId, - vfsType: 'directory', - size: 0, - permissions: options?.mode || this.config.permissions?.defaultDirectory || 0o755, - owner: 'user', - group: 'users', - accessed: Date.now(), - modified: Date.now(), - ...options?.metadata - } + // Create a lock promise for this path + let resolveLock: () => void + const lockPromise = new Promise(resolve => { resolveLock = resolve }) + this.mkdirLocks.set(path, lockPromise) - const entity = await this.brain.add({ - data: path, // Directory path as string content - type: NounType.Collection, - metadata - }) + try { + // Check if already exists + try { + const existing = await this.pathResolver.resolve(path) + const entity = await this.getEntityById(existing) + if (entity.metadata.vfsType === 'directory') { + if (!options?.recursive) { + throw new VFSError(VFSErrorCode.EEXIST, `Directory exists: ${path}`, path, 'mkdir') + } + return // Already exists and recursive is true + } else { + // Path exists but it's not a directory + throw new VFSError(VFSErrorCode.EEXIST, `File exists: ${path}`, path, 'mkdir') + } + } catch (err) { + // Only proceed if it's a ENOENT error (path doesn't exist) + if (err instanceof VFSError && err.code !== VFSErrorCode.ENOENT) { + throw err // Re-throw non-ENOENT errors + } + // Doesn't exist, proceed to create + } - // Create parent-child relationship - if (parentId !== entity) { // Don't relate to self (root) - await this.brain.relate({ - from: parentId, - to: entity, - type: VerbType.Contains + // Parse path + const parentPath = this.getParentPath(path) + const name = this.getBasename(path) + + // Ensure parent exists (recursive mkdir if needed) + let parentId: string + if (parentPath === '/' || parentPath === null) { + parentId = this.rootEntityId! + } else if (options?.recursive) { + parentId = await this.ensureDirectory(parentPath) + } else { + try { + parentId = await this.pathResolver.resolve(parentPath) + } catch (err) { + throw new VFSError(VFSErrorCode.ENOENT, `Parent directory not found: ${parentPath}`, path, 'mkdir') + } + } + + // Create directory entity + const metadata: VFSMetadata = { + path, + name, + parent: parentId, + vfsType: 'directory', + size: 0, + permissions: options?.mode || this.config.permissions?.defaultDirectory || 0o755, + owner: 'user', + group: 'users', + accessed: Date.now(), + modified: Date.now(), + ...options?.metadata + } + + const entity = await this.brain.add({ + data: path, // Directory path as string content + type: NounType.Collection, + metadata }) + + // Create parent-child relationship (no need to check for duplicates on new entities) + if (parentId !== entity) { // Don't relate to self (root) + await this.brain.relate({ + from: parentId, + to: entity, + type: VerbType.Contains + }) + } + + // Update path resolver cache + await this.pathResolver.createPath(path, entity) + + // Trigger watchers + this.triggerWatchers(path, 'rename') + } finally { + // Release the lock + resolveLock!() + this.mkdirLocks.delete(path) } - - // Update path resolver cache - await this.pathResolver.createPath(path, entity) - - // Trigger watchers - this.triggerWatchers(path, 'rename') } /** @@ -2358,19 +2395,22 @@ export class VirtualFileSystem implements IVirtualFileSystem { let earliestModified: number | null = null let latestModified: number | null = null - const traverse = async (currentPath: string) => { + const traverse = async (currentPath: string, isRoot = false) => { try { const entityId = await this.pathResolver.resolve(currentPath) const entity = await this.getEntityById(entityId) if (entity.metadata.vfsType === 'directory') { - stats.directoryCount++ + // Don't count the root/starting directory itself + if (!isRoot) { + stats.directoryCount++ + } // Traverse children const children = await this.readdir(currentPath) for (const child of children) { const childPath = currentPath === '/' ? `/${child}` : `${currentPath}/${child}` - await traverse(childPath) + await traverse(childPath, false) } } else if (entity.metadata.vfsType === 'file') { stats.fileCount++ @@ -2403,7 +2443,7 @@ export class VirtualFileSystem implements IVirtualFileSystem { } } - await traverse(path) + await traverse(path, true) // Calculate averages if (stats.fileCount > 0) { diff --git a/tests/vfs/vfs-bug-fixes.unit.test.ts b/tests/vfs/vfs-bug-fixes.unit.test.ts new file mode 100644 index 00000000..82d13468 --- /dev/null +++ b/tests/vfs/vfs-bug-fixes.unit.test.ts @@ -0,0 +1,254 @@ +/** + * VFS Bug Fix Tests + * + * Tests for issues reported by Brain Studio team: + * - Issue #1: Duplicate directory nodes + * - Issue #2: File read decompression error + */ + +import { describe, it, expect, beforeEach } from 'vitest' +import { Brainy } from '../../src/brainy.js' +import { VirtualFileSystem } from '../../src/vfs/VirtualFileSystem.js' + +describe('VFS Bug Fixes', () => { + let brain: Brainy + let vfs: VirtualFileSystem + + beforeEach(async () => { + // Create fresh instance for each test + brain = new Brainy({ + storage: { type: 'memory' }, + embeddingModel: 'Q8' + }) + await brain.init() + vfs = brain.vfs() + await vfs.init() + }) + + describe('Issue #1: Duplicate Directory Nodes', () => { + it('should not create duplicate directory entries when writing multiple files to same directory', async () => { + // Write multiple files to the same directory (reproduce the bug scenario) + await vfs.writeFile('/src/index.ts', 'export const foo = 1') + await vfs.writeFile('/src/types.ts', 'export type Foo = string') + await vfs.writeFile('/src/utils.ts', 'export function bar() {}') + await vfs.writeFile('/README.md', '# Project') + + // Verify files exist + expect(await vfs.exists('/src/index.ts')).toBe(true) + expect(await vfs.exists('/README.md')).toBe(true) + + // Get direct children of root + const children = await vfs.getDirectChildren('/') + + // Debug: print what we got + console.log('Root children:', children.map(c => ({name: c.metadata.name, type: c.metadata.vfsType}))) + + // Count how many times 'src' appears + const srcDirs = children.filter(child => + child.metadata.name === 'src' && child.metadata.vfsType === 'directory' + ) + + // Should only have ONE src directory + expect(srcDirs.length).toBe(1) + + // Total children should be at least 2: src directory + README.md + expect(children.length).toBeGreaterThanOrEqual(2) + + // Verify children have correct types + const srcDir = children.find(c => c.metadata.name === 'src') + const readme = children.find(c => c.metadata.name === 'README.md') + + expect(srcDir?.metadata.vfsType).toBe('directory') + expect(readme?.metadata.vfsType).toBe('file') + }) + + it('should not create duplicate Contains relationships', async () => { + // Write multiple files to same directory + await vfs.writeFile('/src/a.ts', 'a') + await vfs.writeFile('/src/b.ts', 'b') + await vfs.writeFile('/src/c.ts', 'c') + + // Get the root entity and src directory entity + const rootEntity = await vfs.getEntity('/') + const srcEntity = await vfs.getEntity('/src') + + // Get all Contains relationships from root + const relations = await brain.getRelations({ + from: rootEntity.id, + type: 'contains' as any + }) + + // Filter for relationships pointing to src directory + const srcRelations = relations.filter(r => r.to === srcEntity.id) + + // Should only have ONE relationship from root to src + expect(srcRelations.length).toBe(1) + }) + + it('should handle concurrent file writes without creating duplicates', async () => { + // Write multiple files concurrently (more likely to trigger race conditions) + await Promise.all([ + vfs.writeFile('/data/file1.json', '{"a": 1}'), + vfs.writeFile('/data/file2.json', '{"b": 2}'), + vfs.writeFile('/data/file3.json', '{"c": 3}'), + vfs.writeFile('/data/file4.json', '{"d": 4}') + ]) + + const children = await vfs.getDirectChildren('/') + const dataDirs = children.filter(c => c.metadata.name === 'data') + + // Should only have ONE data directory + expect(dataDirs.length).toBe(1) + + // Check the data directory has exactly 4 files + const dataChildren = await vfs.getDirectChildren('/data') + expect(dataChildren.length).toBe(4) + }) + }) + + describe('Issue #2: File Read Decompression Error', () => { + it('should read file content without decompression errors', async () => { + const content = 'Hello World! This is test content.' + + // Write file + await vfs.writeFile('/test.txt', content) + + // Read file - should NOT throw decompression error + const readContent = await vfs.readFile('/test.txt') + + // Content should match + expect(readContent.toString('utf8')).toBe(content) + }) + + it('should handle reading files with various sizes', async () => { + const testFiles = [ + { path: '/small.txt', content: 'Small' }, + { path: '/medium.txt', content: 'x'.repeat(1000) }, + { path: '/large.txt', content: 'y'.repeat(50000) } + ] + + // Write all files + for (const file of testFiles) { + await vfs.writeFile(file.path, file.content) + } + + // Read all files - none should error + for (const file of testFiles) { + const readContent = await vfs.readFile(file.path) + expect(readContent.toString('utf8')).toBe(file.content) + } + }) + + it('should correctly handle rawData storage without compression', async () => { + const content = 'Test content for rawData' + + // Write file + await vfs.writeFile('/test.md', content) + + // Get the entity to inspect storage + const entity = await vfs.getEntity('/test.md') + + // Should have rawData in metadata + expect(entity.metadata.rawData).toBeDefined() + + // rawData should be base64 encoded + const decodedRawData = Buffer.from(entity.metadata.rawData!, 'base64').toString('utf8') + expect(decodedRawData).toBe(content) + + // Read should work without decompression issues + const readContent = await vfs.readFile('/test.md') + expect(readContent.toString('utf8')).toBe(content) + }) + + it('should handle binary files correctly', async () => { + // Create a binary buffer + const binaryContent = Buffer.from([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A]) + + // Write binary file + await vfs.writeFile('/image.png', binaryContent) + + // Read binary file + const readContent = await vfs.readFile('/image.png') + + // Should match exactly + expect(Buffer.compare(readContent, binaryContent)).toBe(0) + }) + + it('should handle reading after multiple writes', async () => { + // Write file + await vfs.writeFile('/counter.txt', '1') + + // Update file multiple times + await vfs.writeFile('/counter.txt', '2') + await vfs.writeFile('/counter.txt', '3') + + // Read should work + const content = await vfs.readFile('/counter.txt') + expect(content.toString('utf8')).toBe('3') + }) + }) + + describe('Integration: Both Fixes Together', () => { + it('should handle the exact Brain Studio scenario', async () => { + // Reproduce exact scenario from bug report + const files = [ + { path: '/STRICT_TAXONOMY.md', content: 'Taxonomy content' }, + { path: '/README.md', content: 'README content' }, + { path: '/package.json', content: '{"name": "test"}' }, + { path: '/tsconfig.json', content: '{"compilerOptions": {}}' }, + { path: '/src/types.ts', content: 'export type Foo = string' }, + { path: '/src/webhook-handler.ts', content: 'export function handler() {}' }, + { path: '/src/index.ts', content: 'export * from "./types"' }, + { path: '/src/sync-engine.ts', content: 'export class Engine {}' }, + { path: '/src/asana-client.ts', content: 'export class Client {}' } + ] + + // Import all files + for (const file of files) { + await vfs.writeFile(file.path, file.content) + } + + // Check root directory - should NOT have duplicate 'src' entries + const rootChildren = await vfs.getDirectChildren('/') + const srcDirs = rootChildren.filter(c => + c.metadata.name === 'src' && c.metadata.vfsType === 'directory' + ) + expect(srcDirs.length).toBe(1) + + // Should have exactly 4 root items: src (dir) + 3 files + const rootFiles = rootChildren.filter(c => c.metadata.vfsType === 'file') + expect(rootFiles.length).toBe(4) // README, package.json, tsconfig.json, STRICT_TAXONOMY + + // Check src directory has exactly 5 files + const srcChildren = await vfs.getDirectChildren('/src') + expect(srcChildren.length).toBe(5) + + // Read all files - none should throw decompression errors + for (const file of files) { + const content = await vfs.readFile(file.path) + expect(content.toString('utf8')).toBe(file.content) + } + }) + + it('should maintain correct file count statistics', async () => { + // Write files + await vfs.writeFile('/src/a.ts', 'a') + await vfs.writeFile('/src/b.ts', 'b') + await vfs.writeFile('/docs/README.md', 'readme') + + // Get statistics + const stats = await vfs.getProjectStats('/') + + // Debug: Check what directories exist + const rootChildren = await vfs.getDirectChildren('/') + console.log('Root children (stats test):', rootChildren.map(c => ({name: c.metadata.name, type: c.metadata.vfsType}))) + + // Should have exactly 3 files + expect(stats.fileCount).toBe(3) + + // Should have exactly 2 directories (src, docs) + // Note: If there's a duplicate, this will fail + expect(stats.directoryCount).toBe(2) + }) + }) +})