fix: add NULL hash guards to prevent v5.3.3 regression
Fixed critical bug where CommitObject.walk() didn't guard against NULL parent hash, causing "Blob metadata not found: 0000...0000" error when calling getHistory() on fresh databases. This is a SYSTEMATIC fix, not just a patch: Infrastructure: - src/storage/cow/constants.ts: NEW - NULL_HASH constant and utilities - Prevents hardcoding errors - Provides isNullHash() for semantic checks Bug Fixes: - src/storage/cow/CommitObject.ts: Guard NULL parent in walk() - src/storage/cow/BlobStorage.ts: Defensive check rejects NULL hash reads - src/storage/baseStorage.ts: Use NULL_HASH constant (not hardcoded) - src/brainy.ts: Use NULL_HASH constant (not hardcoded) Testing: - tests/integration/initial-commit-null-parent.test.ts: 5 regression tests - All 17 tests pass (5 new + 12 existing COW tests) - Zero regressions This fix addresses the root cause of 4 consecutive bugs (v5.3.0-v5.3.3): missing defensive programming for sentinel values in COW storage. Fixes: Workshop team bug report (v5.3.3 regression) Tests: 17/17 pass (5 new regression + 12 existing COW) Build: SUCCESSFUL (zero errors)
This commit is contained in:
parent
680322b7f4
commit
bb4c0bfb99
6 changed files with 228 additions and 4 deletions
|
|
@ -2357,9 +2357,12 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
|||
const entityCount = await this.getNounCount()
|
||||
const relationshipCount = await this.getVerbCount()
|
||||
|
||||
// v5.3.4: Import NULL_HASH constant
|
||||
const { NULL_HASH } = await import('./storage/cow/constants.js')
|
||||
|
||||
// Build commit object using builder pattern
|
||||
const builder = CommitBuilder.create(blobStorage)
|
||||
.tree('0000000000000000000000000000000000000000000000000000000000000000') // Empty tree hash for now
|
||||
.tree(NULL_HASH) // Empty tree hash (sentinel value)
|
||||
.message(options?.message || 'Snapshot commit')
|
||||
.author(options?.author || 'unknown')
|
||||
.timestamp(Date.now())
|
||||
|
|
|
|||
|
|
@ -303,7 +303,9 @@ export abstract class BaseStorage extends BaseStorageAdapter {
|
|||
const mainRef = await this.refManager.getRef('main')
|
||||
if (!mainRef) {
|
||||
// Create initial commit with empty tree
|
||||
const emptyTreeHash = '0000000000000000000000000000000000000000000000000000000000000000'
|
||||
// v5.3.4: Use NULL_HASH constant instead of hardcoded string
|
||||
const { NULL_HASH } = await import('./cow/constants.js')
|
||||
const emptyTreeHash = NULL_HASH
|
||||
|
||||
// Import CommitBuilder
|
||||
const { CommitBuilder } = await import('./cow/CommitObject.js')
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@
|
|||
*/
|
||||
|
||||
import { createHash } from 'crypto'
|
||||
import { NULL_HASH, isNullHash } from './constants.js'
|
||||
|
||||
/**
|
||||
* Simple key-value storage interface for COW primitives
|
||||
|
|
@ -257,6 +258,18 @@ export class BlobStorage {
|
|||
* @returns Blob data
|
||||
*/
|
||||
async read(hash: string, options: BlobReadOptions = {}): Promise<Buffer> {
|
||||
// v5.3.4 fix: Guard against NULL hash (sentinel value)
|
||||
// NULL_HASH ('0000...0000') is used as a sentinel for "no parent" or "empty tree"
|
||||
// It should NEVER be read as actual blob data
|
||||
if (isNullHash(hash)) {
|
||||
throw new Error(
|
||||
`Cannot read NULL hash (${NULL_HASH}): ` +
|
||||
`This is a sentinel value indicating "no parent commit" or "empty tree". ` +
|
||||
`If you're seeing this error from CommitObject.walk(), there's a bug in commit traversal logic. ` +
|
||||
`If you're seeing this from TreeObject operations, there's a bug in tree handling.`
|
||||
)
|
||||
}
|
||||
|
||||
// Check cache first
|
||||
if (!options.skipCache) {
|
||||
const cached = this.getFromCache(hash)
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@
|
|||
*/
|
||||
|
||||
import { BlobStorage } from './BlobStorage.js'
|
||||
import { isNullHash } from './constants.js'
|
||||
|
||||
/**
|
||||
* Commit object structure
|
||||
|
|
@ -336,7 +337,10 @@ export class CommitObject {
|
|||
let currentHash: string | null = startHash
|
||||
let depth = 0
|
||||
|
||||
while (currentHash) {
|
||||
// v5.3.4 fix: Guard against NULL hash (sentinel for "no parent")
|
||||
// The initial commit has parent = null or NULL_HASH ('0000...0000')
|
||||
// We must stop walking when we reach it, not try to read it
|
||||
while (currentHash && !isNullHash(currentHash)) {
|
||||
// Check max depth
|
||||
if (options?.maxDepth && depth >= options.maxDepth) {
|
||||
break
|
||||
|
|
@ -364,7 +368,7 @@ export class CommitObject {
|
|||
break
|
||||
}
|
||||
|
||||
// Move to parent
|
||||
// Move to parent (can be null or NULL_HASH for initial commit)
|
||||
currentHash = commit.parent
|
||||
depth++
|
||||
}
|
||||
|
|
|
|||
59
src/storage/cow/constants.ts
Normal file
59
src/storage/cow/constants.ts
Normal file
|
|
@ -0,0 +1,59 @@
|
|||
/**
|
||||
* COW Storage Constants
|
||||
*
|
||||
* Sentinel values and utilities for Copy-On-Write storage system.
|
||||
*
|
||||
* @module storage/cow/constants
|
||||
*/
|
||||
|
||||
/**
|
||||
* NULL_HASH - Sentinel value for "no parent commit" or "empty tree"
|
||||
*
|
||||
* In Git-like COW systems, we need a way to represent:
|
||||
* - Initial commit (has no parent)
|
||||
* - Empty tree (contains no files)
|
||||
*
|
||||
* We use a 64-character zero hash as a sentinel value.
|
||||
* This should NEVER be used as an actual content hash.
|
||||
*
|
||||
* @constant
|
||||
* @example
|
||||
* ```typescript
|
||||
* const builder = CommitBuilder.create(storage)
|
||||
* .tree(NULL_HASH) // Empty tree
|
||||
* .parent(null) // No parent (use null, not NULL_HASH)
|
||||
* .build()
|
||||
* ```
|
||||
*/
|
||||
export const NULL_HASH = '0000000000000000000000000000000000000000000000000000000000000000'
|
||||
|
||||
/**
|
||||
* Check if a hash is the NULL sentinel value
|
||||
*
|
||||
* @param hash - Hash to check (can be string or null)
|
||||
* @returns true if hash is null or NULL_HASH
|
||||
*
|
||||
* @example
|
||||
* ```typescript
|
||||
* if (isNullHash(commit.parent)) {
|
||||
* console.log('This is the initial commit')
|
||||
* }
|
||||
* ```
|
||||
*/
|
||||
export function isNullHash(hash: string | null | undefined): boolean {
|
||||
return hash === null || hash === undefined || hash === NULL_HASH
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a hash is valid (non-null, non-empty, proper format)
|
||||
*
|
||||
* @param hash - Hash to check
|
||||
* @returns true if hash is a valid SHA-256 hash
|
||||
*/
|
||||
export function isValidHash(hash: string | null | undefined): boolean {
|
||||
if (isNullHash(hash)) {
|
||||
return false
|
||||
}
|
||||
// SHA-256 hash must be exactly 64 hexadecimal characters
|
||||
return typeof hash === 'string' && /^[a-f0-9]{64}$/.test(hash)
|
||||
}
|
||||
143
tests/integration/initial-commit-null-parent.test.ts
Normal file
143
tests/integration/initial-commit-null-parent.test.ts
Normal file
|
|
@ -0,0 +1,143 @@
|
|||
/**
|
||||
* Regression test for v5.3.3 NULL parent bug
|
||||
*
|
||||
* BUG: CommitObject.walk() didn't guard against NULL parent hash, causing
|
||||
* "Blob metadata not found: 0000...0000" error when calling getHistory()
|
||||
* on a fresh database with only the initial commit.
|
||||
*
|
||||
* ROOT CAUSE: Initial commit has parent = null or NULL_HASH ('0000...0000'),
|
||||
* but while(currentHash) treated the string as truthy and tried to read it.
|
||||
*
|
||||
* FIXED IN: v5.3.4
|
||||
* - Added isNullHash() guard in CommitObject.walk()
|
||||
* - Added defensive check in BlobStorage.read()
|
||||
* - Created NULL_HASH constant to prevent hardcoding
|
||||
*/
|
||||
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest'
|
||||
import { Brainy } from '../../src/brainy.js'
|
||||
import { NULL_HASH } from '../../src/storage/cow/constants.js'
|
||||
import * as fs from 'fs'
|
||||
|
||||
const TEST_DATA_PATH = './test-null-parent-data'
|
||||
|
||||
describe('Initial Commit NULL Parent Bug (v5.3.3 regression)', () => {
|
||||
let brain: Brainy
|
||||
|
||||
beforeEach(async () => {
|
||||
// Clean up test data
|
||||
if (fs.existsSync(TEST_DATA_PATH)) {
|
||||
fs.rmSync(TEST_DATA_PATH, { recursive: true, force: true })
|
||||
}
|
||||
|
||||
brain = new Brainy({
|
||||
storage: {
|
||||
type: 'filesystem',
|
||||
path: TEST_DATA_PATH,
|
||||
branch: 'main',
|
||||
enableCompression: true
|
||||
},
|
||||
disableAutoRebuild: true,
|
||||
silent: true
|
||||
})
|
||||
|
||||
await brain.init()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
if (fs.existsSync(TEST_DATA_PATH)) {
|
||||
fs.rmSync(TEST_DATA_PATH, { recursive: true, force: true })
|
||||
}
|
||||
})
|
||||
|
||||
it('should handle getHistory() on fresh database with only initial commit', async () => {
|
||||
// Fresh brain has only the initial commit (created during init)
|
||||
// This was throwing "Blob metadata not found: 0000...0000" in v5.3.3
|
||||
|
||||
// Get history - should NOT throw
|
||||
const history = await brain.getHistory({ limit: 10 })
|
||||
|
||||
// Should return the initial commit
|
||||
expect(history).toBeDefined()
|
||||
expect(Array.isArray(history)).toBe(true)
|
||||
expect(history.length).toBe(1)
|
||||
|
||||
// Initial commit should have specific properties
|
||||
const initialCommit = history[0]
|
||||
expect(initialCommit.message).toBe('Initial commit')
|
||||
expect(initialCommit.author).toBe('system')
|
||||
// Parent can be null or undefined for initial commit
|
||||
expect(initialCommit.parent == null).toBe(true)
|
||||
})
|
||||
|
||||
it('should stop walk at initial commit (parent = null)', async () => {
|
||||
// Note: brain.commit() creates "Snapshot commit" messages by default
|
||||
// Create 2 commits
|
||||
await brain.add({ data: 'First entity', type: 'concept' })
|
||||
await brain.commit('First user commit')
|
||||
|
||||
await brain.add({ data: 'Second entity', type: 'concept' })
|
||||
await brain.commit('Second user commit')
|
||||
|
||||
// Get full history
|
||||
const history = await brain.getHistory({ limit: 100 })
|
||||
|
||||
// Should have at least 3 commits (2 user + 1 initial)
|
||||
expect(history.length).toBeGreaterThanOrEqual(3)
|
||||
|
||||
// Last commit should be initial commit with no parent
|
||||
const initialCommit = history[history.length - 1]
|
||||
expect(initialCommit.message).toBe('Initial commit')
|
||||
// Parent can be null or undefined for initial commit
|
||||
expect(initialCommit.parent == null).toBe(true)
|
||||
})
|
||||
|
||||
it('should not attempt to read NULL hash from BlobStorage', async () => {
|
||||
// Direct test: attempting to read NULL_HASH should throw clear error
|
||||
const blobStorage = (brain as any).storage.blobStorage
|
||||
|
||||
// Should throw clear error message
|
||||
await expect(
|
||||
blobStorage.read(NULL_HASH)
|
||||
).rejects.toThrow(/sentinel value/)
|
||||
|
||||
// Error message should mention what NULL_HASH is for
|
||||
await expect(
|
||||
blobStorage.read(NULL_HASH)
|
||||
).rejects.toThrow(/no parent/)
|
||||
})
|
||||
|
||||
it('should handle multiple calls to getHistory() consistently', async () => {
|
||||
// Ensure caching doesn't break NULL hash handling
|
||||
|
||||
// First call
|
||||
const history1 = await brain.getHistory({ limit: 10 })
|
||||
expect(history1.length).toBe(1)
|
||||
|
||||
// Add a commit
|
||||
await brain.add({ data: 'Test entity', type: 'concept' })
|
||||
await brain.commit('Test commit')
|
||||
|
||||
// Second call (should now have 2 commits)
|
||||
const history2 = await brain.getHistory({ limit: 10 })
|
||||
expect(history2.length).toBe(2)
|
||||
|
||||
// Third call (should still have 2 commits)
|
||||
const history3 = await brain.getHistory({ limit: 10 })
|
||||
expect(history3.length).toBe(2)
|
||||
|
||||
// All should end with initial commit
|
||||
expect(history1[history1.length - 1].message).toBe('Initial commit')
|
||||
expect(history2[history2.length - 1].message).toBe('Initial commit')
|
||||
expect(history3[history3.length - 1].message).toBe('Initial commit')
|
||||
})
|
||||
|
||||
it('should handle walking with maxDepth on initial commit', async () => {
|
||||
// Edge case: maxDepth = 1 on fresh database
|
||||
|
||||
const history = await brain.getHistory({ limit: 1 })
|
||||
|
||||
expect(history.length).toBe(1)
|
||||
expect(history[0].message).toBe('Initial commit')
|
||||
})
|
||||
})
|
||||
Loading…
Add table
Add a link
Reference in a new issue