fix(vfs): resolve two critical VFS bugs causing directory listing corruption
Bug 1: verbCountsByType optimization skipping requested verb types - After restart, stale statistics could cause VerbType.Contains to be skipped - readdir() would return empty/incomplete results - Fixed by never skipping verb types explicitly requested in filter - Added fast path for sourceId + verbType combo (common VFS pattern) - Save statistics on first entity of each type (not just every 100th) Bug 2: UnifiedCache not invalidated on path deletion - rmdir() cleared local caches but NOT the global UnifiedCache - When folder recreated, resolve() returned stale entity ID - Caused "Source entity not found" errors - Fixed by adding deleteByPrefix() to UnifiedCache - Fixed invalidatePath() to also clear UnifiedCache entries Files modified: - src/storage/baseStorage.ts (verbCountsByType fix + fast path) - src/utils/unifiedCache.ts (deleteByPrefix method) - src/vfs/PathResolver.ts (cache invalidation fix) Tests added: - tests/unit/storage/vfs-mkdir-bug.test.ts (7 tests) Reported by: Soulcraft Workshop team 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
1da6048245
commit
2ba69eccdc
4 changed files with 375 additions and 13 deletions
|
|
@ -1582,6 +1582,52 @@ export abstract class BaseStorage extends BaseStorageAdapter {
|
|||
nextCursor
|
||||
}
|
||||
}
|
||||
|
||||
// v6.2.9: Fast path for SINGLE sourceId + verbType combo (common VFS pattern)
|
||||
// This avoids the slow type-iteration fallback for VFS operations
|
||||
// NOTE: Only use fast path for single sourceId to avoid incomplete results
|
||||
const isSingleSourceId = options.filter.sourceId &&
|
||||
!Array.isArray(options.filter.sourceId)
|
||||
if (
|
||||
isSingleSourceId &&
|
||||
options.filter.verbType &&
|
||||
!options.filter.targetId &&
|
||||
!options.filter.service &&
|
||||
!options.filter.metadata
|
||||
) {
|
||||
const sourceId = options.filter.sourceId as string
|
||||
const verbTypes = Array.isArray(options.filter.verbType)
|
||||
? options.filter.verbType
|
||||
: [options.filter.verbType]
|
||||
|
||||
prodLog.debug(`[BaseStorage] getVerbs: Using fast path for sourceId=${sourceId}, verbTypes=${verbTypes.join(',')}`)
|
||||
|
||||
// Get verbs by source (uses GraphAdjacencyIndex if available)
|
||||
const verbsBySource = await this.getVerbsBySource_internal(sourceId)
|
||||
|
||||
// Filter by verbType in memory (fast - usually small number of verbs per source)
|
||||
const filtered = verbsBySource.filter(v => verbTypes.includes(v.verb))
|
||||
|
||||
// Apply pagination
|
||||
const paginatedVerbs = filtered.slice(offset, offset + limit)
|
||||
const hasMore = offset + limit < filtered.length
|
||||
|
||||
// Set next cursor if there are more items
|
||||
let nextCursor: string | undefined = undefined
|
||||
if (hasMore && paginatedVerbs.length > 0) {
|
||||
const lastItem = paginatedVerbs[paginatedVerbs.length - 1]
|
||||
nextCursor = lastItem.id
|
||||
}
|
||||
|
||||
prodLog.debug(`[BaseStorage] getVerbs: Fast path returned ${filtered.length} verbs (${paginatedVerbs.length} after pagination)`)
|
||||
|
||||
return {
|
||||
items: paginatedVerbs,
|
||||
totalCount: filtered.length,
|
||||
hasMore,
|
||||
nextCursor
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// For more complex filtering or no filtering, use a paginated approach
|
||||
|
|
@ -1656,15 +1702,39 @@ export abstract class BaseStorage extends BaseStorageAdapter {
|
|||
const totalVerbCountFromArray = this.verbCountsByType.reduce((sum, c) => sum + c, 0)
|
||||
const useOptimization = totalVerbCountFromArray > 0
|
||||
|
||||
// v6.2.9 BUG FIX: Pre-compute requested verb types to avoid skipping them
|
||||
// When a specific verbType filter is provided, we MUST check that type
|
||||
// even if verbCountsByType shows 0 (counts can be stale after restart)
|
||||
const requestedVerbTypes = options?.filter?.verbType
|
||||
const requestedVerbTypesSet = requestedVerbTypes
|
||||
? new Set(Array.isArray(requestedVerbTypes) ? requestedVerbTypes : [requestedVerbTypes])
|
||||
: null
|
||||
|
||||
// Iterate through all 127 verb types (Stage 3 CANONICAL) with early termination
|
||||
// OPTIMIZATION: Skip types with zero count (only if counts are reliable)
|
||||
for (let i = 0; i < VERB_TYPE_COUNT && collectedVerbs.length < targetCount; i++) {
|
||||
// Skip empty types for performance (but only if optimization is enabled)
|
||||
if (useOptimization && this.verbCountsByType[i] === 0) {
|
||||
const type = TypeUtils.getVerbFromIndex(i)
|
||||
|
||||
// v6.2.9 FIX: Never skip a type that's explicitly requested in the filter
|
||||
// This fixes VFS bug where Contains relationships were skipped after restart
|
||||
// when verbCountsByType[Contains] was 0 due to stale statistics
|
||||
const isRequestedType = requestedVerbTypesSet?.has(type) ?? false
|
||||
const countIsZero = this.verbCountsByType[i] === 0
|
||||
|
||||
// Skip empty types for performance (but only if optimization is enabled AND not requested)
|
||||
if (useOptimization && countIsZero && !isRequestedType) {
|
||||
continue
|
||||
}
|
||||
|
||||
const type = TypeUtils.getVerbFromIndex(i)
|
||||
// v6.2.9: Log when we DON'T skip a requested type that would have been skipped
|
||||
// This helps diagnose stale statistics issues in production
|
||||
if (useOptimization && countIsZero && isRequestedType) {
|
||||
prodLog.debug(
|
||||
`[BaseStorage] getVerbs: NOT skipping type=${type} despite count=0 (type was explicitly requested). ` +
|
||||
`Statistics may be stale - consider running rebuildTypeCounts().`
|
||||
)
|
||||
}
|
||||
|
||||
try {
|
||||
const verbsOfType = await this.getVerbsByType_internal(type)
|
||||
|
||||
|
|
@ -2650,8 +2720,11 @@ export abstract class BaseStorage extends BaseStorageAdapter {
|
|||
// COW-aware write (v5.0.1): Use COW helper for branch isolation
|
||||
await this.writeObjectToBranch(path, noun)
|
||||
|
||||
// Periodically save statistics (every 100 saves)
|
||||
if (this.nounCountsByType[typeIndex] % 100 === 0) {
|
||||
// Periodically save statistics
|
||||
// v6.2.9: Also save on first noun of each type to ensure low-count types are tracked
|
||||
const shouldSave = this.nounCountsByType[typeIndex] === 1 || // First noun of type
|
||||
this.nounCountsByType[typeIndex] % 100 === 0 // Every 100th
|
||||
if (shouldSave) {
|
||||
await this.saveTypeStatistics()
|
||||
}
|
||||
}
|
||||
|
|
@ -2773,7 +2846,11 @@ export abstract class BaseStorage extends BaseStorageAdapter {
|
|||
}
|
||||
|
||||
// Periodically save statistics
|
||||
if (this.verbCountsByType[typeIndex] % 100 === 0) {
|
||||
// v6.2.9: Also save on first verb of each type to ensure low-count types are tracked
|
||||
// This prevents stale statistics after restart for types with < 100 verbs (common for VFS)
|
||||
const shouldSave = this.verbCountsByType[typeIndex] === 1 || // First verb of type
|
||||
this.verbCountsByType[typeIndex] % 100 === 0 // Every 100th
|
||||
if (shouldSave) {
|
||||
await this.saveTypeStatistics()
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -415,6 +415,24 @@ export class UnifiedCache {
|
|||
return false
|
||||
}
|
||||
|
||||
/**
|
||||
* Delete all items with keys starting with the given prefix
|
||||
* v6.2.9: Added for VFS cache invalidation (fixes stale parent ID bug)
|
||||
* @param prefix - The key prefix to match
|
||||
* @returns Number of items deleted
|
||||
*/
|
||||
deleteByPrefix(prefix: string): number {
|
||||
let deleted = 0
|
||||
for (const [key, item] of this.cache) {
|
||||
if (key.startsWith(prefix)) {
|
||||
this.currentSize -= item.size
|
||||
this.cache.delete(key)
|
||||
deleted++
|
||||
}
|
||||
}
|
||||
return deleted
|
||||
}
|
||||
|
||||
/**
|
||||
* Clear cache or specific type
|
||||
*/
|
||||
|
|
|
|||
|
|
@ -345,30 +345,44 @@ export class PathResolver {
|
|||
|
||||
/**
|
||||
* Invalidate cache entries for a path and its children
|
||||
* v6.2.9 FIX: Also invalidates UnifiedCache to prevent stale entity IDs
|
||||
* This fixes the "Source entity not found" bug after delete+recreate operations
|
||||
*/
|
||||
invalidatePath(path: string, recursive = false): void {
|
||||
const normalizedPath = this.normalizePath(path)
|
||||
|
||||
// Remove from all caches
|
||||
// v6.2.9 FIX: Clear parent cache BEFORE deleting from pathCache
|
||||
// (we need the entityId from the cache entry)
|
||||
const cached = this.pathCache.get(normalizedPath)
|
||||
if (cached) {
|
||||
this.parentCache.delete(cached.entityId)
|
||||
}
|
||||
|
||||
// Remove from local caches
|
||||
this.pathCache.delete(normalizedPath)
|
||||
this.hotPaths.delete(normalizedPath)
|
||||
|
||||
// v6.2.9 CRITICAL FIX: Also invalidate UnifiedCache (global LRU cache)
|
||||
// This was missing before, causing stale entity IDs to be returned after delete
|
||||
const cacheKey = `vfs:path:${normalizedPath}`
|
||||
getGlobalCache().delete(cacheKey)
|
||||
|
||||
if (recursive) {
|
||||
// Remove all paths that start with this path
|
||||
const prefix = normalizedPath.endsWith('/') ? normalizedPath : normalizedPath + '/'
|
||||
|
||||
for (const [cachedPath] of this.pathCache) {
|
||||
for (const [cachedPath, entry] of this.pathCache) {
|
||||
if (cachedPath.startsWith(prefix)) {
|
||||
this.pathCache.delete(cachedPath)
|
||||
this.hotPaths.delete(cachedPath)
|
||||
}
|
||||
// v6.2.9: Also clear parent cache for this entry
|
||||
this.parentCache.delete(entry.entityId)
|
||||
}
|
||||
}
|
||||
|
||||
// Clear parent cache for the entity
|
||||
const cached = this.pathCache.get(normalizedPath)
|
||||
if (cached) {
|
||||
this.parentCache.delete(cached.entityId)
|
||||
// v6.2.9 CRITICAL FIX: Also invalidate UnifiedCache entries with this prefix
|
||||
const globalCachePrefix = `vfs:path:${prefix}`
|
||||
getGlobalCache().deleteByPrefix(globalCachePrefix)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
253
tests/unit/storage/vfs-mkdir-bug.test.ts
Normal file
253
tests/unit/storage/vfs-mkdir-bug.test.ts
Normal file
|
|
@ -0,0 +1,253 @@
|
|||
/**
|
||||
* VFS mkdir() Bug Fix Test (v6.2.9)
|
||||
*
|
||||
* Tests for the bug where vfs.mkdir() caused previously created directories
|
||||
* to disappear from vfs.readdir('/').
|
||||
*
|
||||
* Root cause: The verbCountsByType optimization skipped verb types with count 0,
|
||||
* which could happen when statistics were stale after restart.
|
||||
*
|
||||
* Fix:
|
||||
* 1. Option A: Never skip a verb type that's explicitly requested in the filter
|
||||
* 2. Option B: Added fast path for sourceId + verbType combo (common VFS pattern)
|
||||
*
|
||||
* Bug Report: /home/dpsifr/Projects/workshop/docs/BRAINY_VFS_BUG_REPORT.md
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest'
|
||||
import { Brainy, VerbType } from '../../../src/index.js'
|
||||
|
||||
describe('VFS mkdir() Bug Fix (v6.2.9)', () => {
|
||||
let brain: Brainy
|
||||
|
||||
beforeEach(async () => {
|
||||
brain = new Brainy({
|
||||
storage: { type: 'memory' }
|
||||
})
|
||||
await brain.init()
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
await brain.close()
|
||||
})
|
||||
|
||||
describe('mkdir() should not corrupt VFS index', () => {
|
||||
it('should show all directories after mkdir() is called', async () => {
|
||||
const vfs = brain.vfs
|
||||
await vfs.init()
|
||||
|
||||
// Step 1: Create directory via writeFile (auto-creates parent directories)
|
||||
await vfs.writeFile('/Personas/user.md', 'content')
|
||||
console.log('Created /Personas/user.md')
|
||||
|
||||
// Verify /Personas is visible
|
||||
const entries1 = await vfs.readdir('/', { withFileTypes: true })
|
||||
const names1 = entries1.map((e: any) => e.name)
|
||||
console.log('After writeFile - root entries:', names1)
|
||||
expect(names1).toContain('Personas')
|
||||
|
||||
// Step 2: Create another directory via mkdir
|
||||
await vfs.mkdir('/apps/my-app', { recursive: true })
|
||||
console.log('Created /apps/my-app')
|
||||
|
||||
// Step 3: Verify BOTH directories are visible (THIS IS THE BUG)
|
||||
const entries2 = await vfs.readdir('/', { withFileTypes: true })
|
||||
const names2 = entries2.map((e: any) => e.name)
|
||||
console.log('After mkdir - root entries:', names2)
|
||||
|
||||
// This assertion failed before the fix
|
||||
expect(names2).toContain('Personas')
|
||||
expect(names2).toContain('apps')
|
||||
expect(names2.length).toBe(2)
|
||||
})
|
||||
|
||||
it('should handle multiple nested directories', async () => {
|
||||
const vfs = brain.vfs
|
||||
await vfs.init()
|
||||
|
||||
// Create multiple directories via different methods
|
||||
await vfs.writeFile('/docs/readme.md', 'readme')
|
||||
await vfs.mkdir('/config', { recursive: true })
|
||||
await vfs.writeFile('/data/users/user1.json', '{}')
|
||||
await vfs.mkdir('/logs/archive', { recursive: true })
|
||||
|
||||
// Verify all root directories are visible
|
||||
const entries = await vfs.readdir('/', { withFileTypes: true })
|
||||
const names = entries.map((e: any) => e.name)
|
||||
console.log('All root entries:', names)
|
||||
|
||||
expect(names).toContain('docs')
|
||||
expect(names).toContain('config')
|
||||
expect(names).toContain('data')
|
||||
expect(names).toContain('logs')
|
||||
expect(names.length).toBe(4)
|
||||
})
|
||||
})
|
||||
|
||||
describe('getRelations should query Contains relationships correctly', () => {
|
||||
it('should find Contains relationships via sourceId + verbType filter', async () => {
|
||||
// Create some entities and relationships
|
||||
const parent = await brain.add({
|
||||
data: 'Parent entity',
|
||||
type: 'collection',
|
||||
metadata: { name: 'parent' }
|
||||
})
|
||||
|
||||
const child1 = await brain.add({
|
||||
data: 'Child 1',
|
||||
type: 'document',
|
||||
metadata: { name: 'child1' }
|
||||
})
|
||||
|
||||
const child2 = await brain.add({
|
||||
data: 'Child 2',
|
||||
type: 'document',
|
||||
metadata: { name: 'child2' }
|
||||
})
|
||||
|
||||
// Create Contains relationships
|
||||
await brain.relate({ from: parent, to: child1, type: VerbType.Contains })
|
||||
await brain.relate({ from: parent, to: child2, type: VerbType.Contains })
|
||||
|
||||
// Query relationships (this is what getChildren() does)
|
||||
const relations = await brain.getRelations({
|
||||
from: parent,
|
||||
type: VerbType.Contains
|
||||
})
|
||||
|
||||
console.log('Relations found:', relations.length)
|
||||
expect(relations.length).toBe(2)
|
||||
expect(relations.map(r => r.to)).toContain(child1)
|
||||
expect(relations.map(r => r.to)).toContain(child2)
|
||||
})
|
||||
|
||||
it('should return relationships even with mixed verb types', async () => {
|
||||
// Create entities
|
||||
const entityA = await brain.add({ data: 'Entity A', type: 'concept' })
|
||||
const entityB = await brain.add({ data: 'Entity B', type: 'concept' })
|
||||
const entityC = await brain.add({ data: 'Entity C', type: 'concept' })
|
||||
|
||||
// Create different types of relationships
|
||||
await brain.relate({ from: entityA, to: entityB, type: VerbType.Contains })
|
||||
await brain.relate({ from: entityA, to: entityC, type: VerbType.RelatedTo })
|
||||
|
||||
// Query only Contains relationships
|
||||
const containsRelations = await brain.getRelations({
|
||||
from: entityA,
|
||||
type: VerbType.Contains
|
||||
})
|
||||
|
||||
expect(containsRelations.length).toBe(1)
|
||||
expect(containsRelations[0].to).toBe(entityB)
|
||||
|
||||
// Query only RelatedTo relationships
|
||||
const relatedRelations = await brain.getRelations({
|
||||
from: entityA,
|
||||
type: VerbType.RelatedTo
|
||||
})
|
||||
|
||||
expect(relatedRelations.length).toBe(1)
|
||||
expect(relatedRelations[0].to).toBe(entityC)
|
||||
})
|
||||
})
|
||||
|
||||
describe('Fast path for sourceId + verbType filter', () => {
|
||||
it('should use fast path for VFS-style queries', async () => {
|
||||
// Create parent and children
|
||||
const parent = await brain.add({
|
||||
data: 'Parent',
|
||||
type: 'collection',
|
||||
metadata: { name: 'parent' }
|
||||
})
|
||||
|
||||
// Create many children to verify performance
|
||||
const children: string[] = []
|
||||
for (let i = 0; i < 10; i++) {
|
||||
const child = await brain.add({
|
||||
data: `Child ${i}`,
|
||||
type: 'document',
|
||||
metadata: { name: `child${i}` }
|
||||
})
|
||||
children.push(child)
|
||||
await brain.relate({ from: parent, to: child, type: VerbType.Contains })
|
||||
}
|
||||
|
||||
// Query should use the new fast path
|
||||
const startTime = performance.now()
|
||||
const relations = await brain.getRelations({
|
||||
from: parent,
|
||||
type: VerbType.Contains
|
||||
})
|
||||
const elapsed = performance.now() - startTime
|
||||
|
||||
console.log(`Query returned ${relations.length} relations in ${elapsed.toFixed(2)}ms`)
|
||||
expect(relations.length).toBe(10)
|
||||
|
||||
// Verify all children are found
|
||||
for (const child of children) {
|
||||
expect(relations.map(r => r.to)).toContain(child)
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
describe('Delete and recreate folder (v6.2.9 cache invalidation fix)', () => {
|
||||
it('should handle delete folder → recreate folder without corruption', async () => {
|
||||
const vfs = brain.vfs
|
||||
await vfs.init()
|
||||
|
||||
// Step 1: Create a folder with content
|
||||
await vfs.mkdir('/test-folder', { recursive: true })
|
||||
await vfs.writeFile('/test-folder/file1.txt', 'content1')
|
||||
console.log('Created /test-folder with file1.txt')
|
||||
|
||||
// Verify folder exists
|
||||
const entries1 = await vfs.readdir('/', { withFileTypes: true })
|
||||
expect(entries1.map((e: any) => e.name)).toContain('test-folder')
|
||||
|
||||
// Step 2: Delete the folder
|
||||
await vfs.rmdir('/test-folder', { recursive: true })
|
||||
console.log('Deleted /test-folder')
|
||||
|
||||
// Verify folder is gone
|
||||
const entries2 = await vfs.readdir('/', { withFileTypes: true })
|
||||
expect(entries2.map((e: any) => e.name)).not.toContain('test-folder')
|
||||
|
||||
// Step 3: Recreate the folder (THIS IS WHERE THE BUG WAS)
|
||||
await vfs.mkdir('/test-folder', { recursive: true })
|
||||
console.log('Recreated /test-folder')
|
||||
|
||||
// Step 4: Create new content
|
||||
await vfs.writeFile('/test-folder/file2.txt', 'content2')
|
||||
console.log('Created /test-folder/file2.txt')
|
||||
|
||||
// Step 5: Verify everything works
|
||||
const entries3 = await vfs.readdir('/', { withFileTypes: true })
|
||||
expect(entries3.map((e: any) => e.name)).toContain('test-folder')
|
||||
|
||||
const folderContents = await vfs.readdir('/test-folder', { withFileTypes: true })
|
||||
expect(folderContents.map((e: any) => e.name)).toContain('file2.txt')
|
||||
console.log('Folder recreate test passed!')
|
||||
})
|
||||
|
||||
it('should handle multiple delete/recreate cycles', async () => {
|
||||
const vfs = brain.vfs
|
||||
await vfs.init()
|
||||
|
||||
for (let i = 0; i < 3; i++) {
|
||||
// Create
|
||||
await vfs.mkdir('/cycle-test', { recursive: true })
|
||||
await vfs.writeFile(`/cycle-test/file${i}.txt`, `content${i}`)
|
||||
|
||||
// Verify
|
||||
const contents = await vfs.readdir('/cycle-test', { withFileTypes: true })
|
||||
expect(contents.length).toBe(1)
|
||||
expect(contents[0].name).toBe(`file${i}.txt`)
|
||||
|
||||
// Delete
|
||||
await vfs.rmdir('/cycle-test', { recursive: true })
|
||||
|
||||
console.log(`Cycle ${i + 1} completed`)
|
||||
}
|
||||
})
|
||||
})
|
||||
})
|
||||
Loading…
Add table
Add a link
Reference in a new issue