Complete bd-ar2 P2 tasks: metadata, resurrection, and testing improvements
This commit addresses the remaining P2 tasks from bd-ar2 code review follow-up: ## Completed Tasks ### bd-ar2.4: Improve parent chain resurrection - Modified `tryResurrectParentWithConn()` to recursively resurrect ancestor chain - When resurrecting bd-root.1.2, now also resurrects bd-root.1 if missing - Handles deeply nested hierarchies where intermediate parents are deleted - All resurrection tests pass including new edge cases ### bd-ar2.5: Add error handling guidance - Documented metadata update failure strategy in `updateExportMetadata()` - Explained trade-off: warnings vs errors (safe, prevents data loss) - Added user-facing message: "Next export may require running 'bd import' first" - Clarifies that worst case is requiring import before next export ### bd-ar2.6: Document transaction boundaries - Added comprehensive documentation for atomicity trade-offs - Explained crash scenarios and recovery (bd import) - Documented decision to defer defensive checks (Option 4) until needed - No code changes - current approach is acceptable for now ### bd-ar2.12: Add metadata key validation - Added keySuffix validation in `updateExportMetadata()` and `hasJSONLChanged()` - Prevents ':' separator in keySuffix to avoid malformed metadata keys - Documented metadata key format in function comments - Single-repo: "last_import_hash", multi-repo: "last_import_hash:<repo_key>" ### bd-ar2.7: Add edge case tests for GetNextChildID resurrection - TestGetNextChildID_ResurrectParent_NotInJSONL: parent not in history - TestGetNextChildID_ResurrectParent_NoJSONL: missing JSONL file - TestGetNextChildID_ResurrectParent_MalformedJSONL: invalid JSON lines - TestGetNextChildID_ResurrectParentChain: deeply nested missing parents - All tests pass, resurrection is robust against edge cases ## Files Changed - cmd/bd/daemon_sync.go: Metadata validation, error handling docs - cmd/bd/integrity.go: Added strings import, keySuffix validation - internal/storage/sqlite/hash_ids.go: Improved resurrection comments - internal/storage/sqlite/resurrection.go: Recursive ancestor resurrection - internal/storage/sqlite/child_id_test.go: Added 4 new edge case tests ## Testing All export, sync, metadata, and resurrection tests pass. Edge cases properly handled: missing JSONL, malformed JSON, deep nesting. ## Remaining Tasks - bd-ar2.8 (P3): Additional export metadata edge case tests (deferred) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -12,6 +12,7 @@ import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"sort"
|
||||
"strings"
|
||||
|
||||
"github.com/steveyegge/beads/internal/storage"
|
||||
"github.com/steveyegge/beads/internal/types"
|
||||
@@ -96,7 +97,14 @@ func computeJSONLHash(jsonlPath string) (string, error) {
|
||||
// unchanged) while still catching git operations that restore old content with new mtimes.
|
||||
//
|
||||
// In multi-repo mode, keySuffix should be the stable repo identifier (e.g., ".", "../frontend").
|
||||
// The keySuffix must not contain the ':' separator character (bd-ar2.12).
|
||||
func hasJSONLChanged(ctx context.Context, store storage.Storage, jsonlPath string, keySuffix string) bool {
|
||||
// Validate keySuffix doesn't contain the separator character (bd-ar2.12)
|
||||
if keySuffix != "" && strings.Contains(keySuffix, ":") {
|
||||
// Invalid keySuffix - treat as changed to trigger proper error handling
|
||||
return true
|
||||
}
|
||||
|
||||
// Build metadata keys with optional suffix for per-repo tracking (bd-ar2.10, bd-ar2.11)
|
||||
hashKey := "last_import_hash"
|
||||
mtimeKey := "last_import_mtime"
|
||||
|
||||
Reference in New Issue
Block a user