e816e91ecb
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>
69 lines
2.4 KiB
Go
69 lines
2.4 KiB
Go
package sqlite
|
|
|
|
import (
|
|
"context"
|
|
"fmt"
|
|
"strings"
|
|
)
|
|
|
|
// getNextChildNumber atomically increments and returns the next child counter for a parent issue.
|
|
// Uses INSERT...ON CONFLICT to ensure atomicity without explicit locking.
|
|
func (s *SQLiteStorage) getNextChildNumber(ctx context.Context, parentID string) (int, error) {
|
|
var nextChild int
|
|
err := s.db.QueryRowContext(ctx, `
|
|
INSERT INTO child_counters (parent_id, last_child)
|
|
VALUES (?, 1)
|
|
ON CONFLICT(parent_id) DO UPDATE SET
|
|
last_child = last_child + 1
|
|
RETURNING last_child
|
|
`, parentID).Scan(&nextChild)
|
|
if err != nil {
|
|
return 0, fmt.Errorf("failed to generate next child number for parent %s: %w", parentID, err)
|
|
}
|
|
return nextChild, nil
|
|
}
|
|
|
|
// GetNextChildID generates the next hierarchical child ID for a given parent
|
|
// Returns formatted ID as parentID.{counter} (e.g., bd-a3f8e9.1 or bd-a3f8e9.1.5)
|
|
// Works at any depth (max 3 levels)
|
|
func (s *SQLiteStorage) GetNextChildID(ctx context.Context, parentID string) (string, error) {
|
|
// Validate parent exists
|
|
var count int
|
|
err := s.db.QueryRowContext(ctx, `SELECT COUNT(*) FROM issues WHERE id = ?`, parentID).Scan(&count)
|
|
if err != nil {
|
|
return "", fmt.Errorf("failed to check parent existence: %w", err)
|
|
}
|
|
if count == 0 {
|
|
// Try to resurrect parent from JSONL history before failing (bd-dvd fix, bd-ar2.4)
|
|
// Note: Using TryResurrectParent instead of TryResurrectParentChain because we're
|
|
// already given the direct parent ID. TryResurrectParent will handle the direct parent,
|
|
// and if the parent itself has missing ancestors, those should have been resurrected
|
|
// when the parent was originally created.
|
|
resurrected, resurrectErr := s.TryResurrectParent(ctx, parentID)
|
|
if resurrectErr != nil {
|
|
return "", fmt.Errorf("failed to resurrect parent %s: %w", parentID, resurrectErr)
|
|
}
|
|
if !resurrected {
|
|
return "", fmt.Errorf("parent issue %s does not exist and could not be resurrected from JSONL history", parentID)
|
|
}
|
|
}
|
|
|
|
// Calculate current depth by counting dots
|
|
depth := strings.Count(parentID, ".")
|
|
if depth >= 3 {
|
|
return "", fmt.Errorf("maximum hierarchy depth (3) exceeded for parent %s", parentID)
|
|
}
|
|
|
|
// Get next child number atomically
|
|
nextNum, err := s.getNextChildNumber(ctx, parentID)
|
|
if err != nil {
|
|
return "", err
|
|
}
|
|
|
|
// Format as parentID.counter
|
|
childID := fmt.Sprintf("%s.%d", parentID, nextNum)
|
|
return childID, nil
|
|
}
|
|
|
|
// generateHashID moved to ids.go (bd-0702)
|