fix: resolve VFS race conditions and decompression errors
Fixes duplicate directory nodes caused by concurrent writes and file read decompression errors caused by rawData compression state mismatch. Adds mutex-based concurrency control for mkdir operations and explicit compression tracking for file reads. Resolves issues reported by Brain Studio team: - Issue #1: Duplicate directory nodes in getDirectChildren - Issue #2: Z_DATA_ERROR when reading file content
This commit is contained in:
parent
b3426604ee
commit
1a2661fc40
2 changed files with 366 additions and 72 deletions
|
|
@ -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<string, Promise<void>> = 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<void> {
|
||||
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<void>(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) {
|
||||
|
|
|
|||
254
tests/vfs/vfs-bug-fixes.unit.test.ts
Normal file
254
tests/vfs/vfs-bug-fixes.unit.test.ts
Normal file
|
|
@ -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)
|
||||
})
|
||||
})
|
||||
})
|
||||
Loading…
Add table
Add a link
Reference in a new issue