diff --git a/src/transaction/operations/IndexOperations.ts b/src/transaction/operations/IndexOperations.ts index 37db6227..891a9ac3 100644 --- a/src/transaction/operations/IndexOperations.ts +++ b/src/transaction/operations/IndexOperations.ts @@ -51,13 +51,26 @@ export class AddToHNSWOperation implements Operation { } /** - * Check if item exists in index + * Check if item exists in index. + * + * `getItem` is an optional provider capability that the built-in HNSW index + * does not implement. When the capability is missing the answer must be + * `false`, not `true`: `await undefined` resolves without throwing, so the + * old probe reported every item as pre-existing and rollback of fresh adds + * never removed them — leaving phantom index entries after a failed + * transaction. The safe default is to remove what this operation added; + * update flows pair this op with a RemoveFromHNSWOperation whose own + * rollback restores the prior vector, so reverse-order rollback + * reconstructs the original state either way. */ private async itemExists(id: string): Promise { + const index = this.index as HNSWIndex & { + getItem?: (id: string) => Promise + } + if (typeof index.getItem !== 'function') return false try { - // Try to get item - if exists, no error - await (this.index as any).getItem?.(id) - return true + const item = await index.getItem(id) + return item !== undefined && item !== null } catch { return false } diff --git a/src/vfs/VirtualFileSystem.ts b/src/vfs/VirtualFileSystem.ts index 297f51e4..f47f8fbd 100644 --- a/src/vfs/VirtualFileSystem.ts +++ b/src/vfs/VirtualFileSystem.ts @@ -1664,17 +1664,6 @@ export class VirtualFileSystem implements IVirtualFileSystem { if (err.code !== VFSErrorCode.ENOENT) throw err } - // Update entity metadata - const updatedEntity = { - ...entity, - metadata: { - ...entity.metadata, - path: newPath, - name: this.getBasename(newPath), - modified: Date.now() - } - } - // Update parent relationships if needed const oldParentPath = this.getParentPath(oldPath) const newParentPath = this.getParentPath(newPath) @@ -1700,10 +1689,19 @@ export class VirtualFileSystem implements IVirtualFileSystem { } } - // Update the entity + // A rename is a path/metadata change, never a content change — issue a + // metadata-only update. Spreading the whole entity here used to forward + // its vector field into update(), which failed dimension validation when + // the entity was fetched without vectors (and would needlessly touch the + // vector index when it wasn't). await this.brain.update({ - ...updatedEntity, - id: entityId + id: entityId, + metadata: { + ...entity.metadata, + path: newPath, + name: this.getBasename(newPath), + modified: Date.now() + } }) // Update path cache diff --git a/tests/unit/vfs/vfs-rename-metadata-only.test.ts b/tests/unit/vfs/vfs-rename-metadata-only.test.ts new file mode 100644 index 00000000..e9718010 --- /dev/null +++ b/tests/unit/vfs/vfs-rename-metadata-only.test.ts @@ -0,0 +1,68 @@ +/** + * @module vfs/vfs-rename-metadata-only.test + * @description Regression test for the vfs.rename() vector-dimension crash + * (7.31.7). rename() used to spread the entire fetched entity into + * brain.update(), forwarding a vector field that failed the 384-dimension + * validation when the entity was fetched without vectors. A rename is a + * path/metadata change — the update must be metadata-only. + * + * Production repro (verbatim from the consumer report): + * await brain.vfs.writeFile('/a.txt', 'x') + * await brain.vfs.rename('/a.txt', '/b.txt') // threw pre-7.31.7 + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest' +import { Brainy } from '../../../src/index.js' + +describe('vfs.rename() issues a metadata-only update (7.31.7)', () => { + let brain: Brainy + + beforeEach(async () => { + brain = new Brainy({ storage: { type: 'memory' } }) + await brain.init() + }) + + afterEach(async () => { + await brain.close() + }) + + it('renames a file without touching vectors (the production repro)', async () => { + await brain.vfs.writeFile('/a.txt', 'x') + await brain.vfs.rename('/a.txt', '/b.txt') + + expect(await brain.vfs.exists('/b.txt')).toBe(true) + expect(await brain.vfs.exists('/a.txt')).toBe(false) + const content = await brain.vfs.readFile('/b.txt') + expect(content.toString()).toBe('x') + }) + + it('preserves file content and custom metadata across rename', async () => { + await brain.vfs.writeFile('/notes/draft.md', 'hello world') + await brain.vfs.rename('/notes/draft.md', '/notes/final.md') + + const content = await brain.vfs.readFile('/notes/final.md') + expect(content.toString()).toBe('hello world') + // The renamed entity's metadata carries the new basename + const stat = await brain.vfs.stat('/notes/final.md') + expect(stat.size).toBeGreaterThan(0) + expect(await brain.vfs.exists('/notes/draft.md')).toBe(false) + }) + + it('moves a file across directories', async () => { + await brain.vfs.mkdir('/src') + await brain.vfs.mkdir('/dest') + await brain.vfs.writeFile('/src/file.txt', 'moving') + await brain.vfs.rename('/src/file.txt', '/dest/file.txt') + + expect(await brain.vfs.exists('/dest/file.txt')).toBe(true) + expect(await brain.vfs.exists('/src/file.txt')).toBe(false) + const content = await brain.vfs.readFile('/dest/file.txt') + expect(content.toString()).toBe('moving') + }) + + it('throws EEXIST when the target already exists', async () => { + await brain.vfs.writeFile('/one.txt', '1') + await brain.vfs.writeFile('/two.txt', '2') + await expect(brain.vfs.rename('/one.txt', '/two.txt')).rejects.toThrow(/exists/i) + }) +})