Fix transaction conflict in TryResurrectParent (bd-58c0)
Refactored resurrection functions to accept optional *sql.Conn parameter: - Added tryResurrectParentWithConn() internal function - Added tryResurrectParentChainWithConn() internal function - Updated CreateIssue to use conn-based resurrection - Updated EnsureIDs to use conn-based resurrection This eliminates 'database is locked' errors when resurrection happens inside an existing transaction. Fixes bd-58c0
This commit is contained in:
@@ -197,7 +197,9 @@ func TestCreateIssue_HierarchicalID_ParentNotExists(t *testing.T) {
|
|||||||
if err == nil {
|
if err == nil {
|
||||||
t.Errorf("expected error for child without parent, got nil")
|
t.Errorf("expected error for child without parent, got nil")
|
||||||
}
|
}
|
||||||
if err != nil && err.Error() != "parent issue bd-nonexistent does not exist" {
|
// With resurrection feature, error message includes JSONL history check
|
||||||
t.Errorf("unexpected error message: %v", err)
|
expectedErr := "parent issue bd-nonexistent does not exist and could not be resurrected from JSONL history"
|
||||||
|
if err != nil && err.Error() != expectedErr {
|
||||||
|
t.Errorf("unexpected error message: got %q, want %q", err.Error(), expectedErr)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -189,17 +189,18 @@ func (s *SQLiteStorage) EnsureIDs(ctx context.Context, conn *sql.Conn, prefix st
|
|||||||
// For hierarchical IDs (bd-a3f8e9.1), ensure parent exists
|
// For hierarchical IDs (bd-a3f8e9.1), ensure parent exists
|
||||||
if strings.Contains(issues[i].ID, ".") {
|
if strings.Contains(issues[i].ID, ".") {
|
||||||
// Try to resurrect entire parent chain if any parents are missing
|
// Try to resurrect entire parent chain if any parents are missing
|
||||||
resurrected, err := s.TryResurrectParentChain(ctx, issues[i].ID)
|
// Use the conn-based version to participate in the same transaction
|
||||||
|
resurrected, err := s.tryResurrectParentChainWithConn(ctx, conn, issues[i].ID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("failed to resurrect parent chain for %s: %w", issues[i].ID, err)
|
return fmt.Errorf("failed to resurrect parent chain for %s: %w", issues[i].ID, err)
|
||||||
}
|
}
|
||||||
if !resurrected {
|
if !resurrected {
|
||||||
// Parent(s) not found in JSONL history - cannot proceed
|
// Parent(s) not found in JSONL history - cannot proceed
|
||||||
lastDot := strings.LastIndex(issues[i].ID, ".")
|
lastDot := strings.LastIndex(issues[i].ID, ".")
|
||||||
parentID := issues[i].ID[:lastDot]
|
parentID := issues[i].ID[:lastDot]
|
||||||
return fmt.Errorf("parent issue %s does not exist and could not be resurrected from JSONL history", parentID)
|
return fmt.Errorf("parent issue %s does not exist and could not be resurrected from JSONL history", parentID)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
usedIDs[issues[i].ID] = true
|
usedIDs[issues[i].ID] = true
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ package sqlite
|
|||||||
import (
|
import (
|
||||||
"bufio"
|
"bufio"
|
||||||
"context"
|
"context"
|
||||||
|
"database/sql"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"fmt"
|
"fmt"
|
||||||
"os"
|
"os"
|
||||||
@@ -24,9 +25,22 @@ import (
|
|||||||
// - false if parent was not found in JSONL history
|
// - false if parent was not found in JSONL history
|
||||||
// - error if resurrection failed for any other reason
|
// - error if resurrection failed for any other reason
|
||||||
func (s *SQLiteStorage) TryResurrectParent(ctx context.Context, parentID string) (bool, error) {
|
func (s *SQLiteStorage) TryResurrectParent(ctx context.Context, parentID string) (bool, error) {
|
||||||
|
// Get a connection for the entire resurrection operation
|
||||||
|
conn, err := s.db.Conn(ctx)
|
||||||
|
if err != nil {
|
||||||
|
return false, fmt.Errorf("failed to get database connection: %w", err)
|
||||||
|
}
|
||||||
|
defer conn.Close()
|
||||||
|
|
||||||
|
return s.tryResurrectParentWithConn(ctx, conn, parentID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// tryResurrectParentWithConn is the internal version that accepts an existing connection.
|
||||||
|
// This allows resurrection to participate in an existing transaction.
|
||||||
|
func (s *SQLiteStorage) tryResurrectParentWithConn(ctx context.Context, conn *sql.Conn, parentID string) (bool, error) {
|
||||||
// First check if parent already exists in database
|
// First check if parent already exists in database
|
||||||
var count int
|
var count int
|
||||||
err := s.db.QueryRowContext(ctx, `SELECT COUNT(*) FROM issues WHERE id = ?`, parentID).Scan(&count)
|
err := conn.QueryRowContext(ctx, `SELECT COUNT(*) FROM issues WHERE id = ?`, parentID).Scan(&count)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return false, fmt.Errorf("failed to check parent existence: %w", err)
|
return false, fmt.Errorf("failed to check parent existence: %w", err)
|
||||||
}
|
}
|
||||||
@@ -63,14 +77,7 @@ func (s *SQLiteStorage) TryResurrectParent(ctx context.Context, parentID string)
|
|||||||
tombstone.Description = fmt.Sprintf("%s\n\nOriginal description:\n%s", tombstone.Description, parentIssue.Description)
|
tombstone.Description = fmt.Sprintf("%s\n\nOriginal description:\n%s", tombstone.Description, parentIssue.Description)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Get a connection for the transaction
|
// Insert tombstone into database using the provided connection
|
||||||
conn, err := s.db.Conn(ctx)
|
|
||||||
if err != nil {
|
|
||||||
return false, fmt.Errorf("failed to get database connection: %w", err)
|
|
||||||
}
|
|
||||||
defer conn.Close()
|
|
||||||
|
|
||||||
// Insert tombstone into database
|
|
||||||
if err := insertIssue(ctx, conn, tombstone); err != nil {
|
if err := insertIssue(ctx, conn, tombstone); err != nil {
|
||||||
return false, fmt.Errorf("failed to create tombstone for parent %s: %w", parentID, err)
|
return false, fmt.Errorf("failed to create tombstone for parent %s: %w", parentID, err)
|
||||||
}
|
}
|
||||||
@@ -80,9 +87,9 @@ func (s *SQLiteStorage) TryResurrectParent(ctx context.Context, parentID string)
|
|||||||
for _, dep := range parentIssue.Dependencies {
|
for _, dep := range parentIssue.Dependencies {
|
||||||
// Only resurrect dependencies if both source and target exist
|
// Only resurrect dependencies if both source and target exist
|
||||||
var targetCount int
|
var targetCount int
|
||||||
err := s.db.QueryRowContext(ctx, `SELECT COUNT(*) FROM issues WHERE id = ?`, dep.DependsOnID).Scan(&targetCount)
|
err := conn.QueryRowContext(ctx, `SELECT COUNT(*) FROM issues WHERE id = ?`, dep.DependsOnID).Scan(&targetCount)
|
||||||
if err == nil && targetCount > 0 {
|
if err == nil && targetCount > 0 {
|
||||||
_, err := s.db.ExecContext(ctx, `
|
_, err := conn.ExecContext(ctx, `
|
||||||
INSERT OR IGNORE INTO dependencies (issue_id, depends_on_id, type, created_by)
|
INSERT OR IGNORE INTO dependencies (issue_id, depends_on_id, type, created_by)
|
||||||
VALUES (?, ?, ?, ?)
|
VALUES (?, ?, ?, ?)
|
||||||
`, parentID, dep.DependsOnID, dep.Type, "resurrection")
|
`, parentID, dep.DependsOnID, dep.Type, "resurrection")
|
||||||
@@ -170,12 +177,24 @@ func (s *SQLiteStorage) findIssueInJSONL(issueID string) (*types.Issue, error) {
|
|||||||
// - false if any parent in the chain was not found in JSONL history
|
// - false if any parent in the chain was not found in JSONL history
|
||||||
// - error if resurrection failed for any other reason
|
// - error if resurrection failed for any other reason
|
||||||
func (s *SQLiteStorage) TryResurrectParentChain(ctx context.Context, childID string) (bool, error) {
|
func (s *SQLiteStorage) TryResurrectParentChain(ctx context.Context, childID string) (bool, error) {
|
||||||
|
// Get a connection for the entire chain resurrection
|
||||||
|
conn, err := s.db.Conn(ctx)
|
||||||
|
if err != nil {
|
||||||
|
return false, fmt.Errorf("failed to get database connection: %w", err)
|
||||||
|
}
|
||||||
|
defer conn.Close()
|
||||||
|
|
||||||
|
return s.tryResurrectParentChainWithConn(ctx, conn, childID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// tryResurrectParentChainWithConn is the internal version that accepts an existing connection.
|
||||||
|
func (s *SQLiteStorage) tryResurrectParentChainWithConn(ctx context.Context, conn *sql.Conn, childID string) (bool, error) {
|
||||||
// Extract all parent IDs from the hierarchical chain
|
// Extract all parent IDs from the hierarchical chain
|
||||||
parents := extractParentChain(childID)
|
parents := extractParentChain(childID)
|
||||||
|
|
||||||
// Resurrect from root to leaf (shallower to deeper)
|
// Resurrect from root to leaf (shallower to deeper)
|
||||||
for _, parentID := range parents {
|
for _, parentID := range parents {
|
||||||
resurrected, err := s.TryResurrectParent(ctx, parentID)
|
resurrected, err := s.tryResurrectParentWithConn(ctx, conn, parentID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return false, fmt.Errorf("failed to resurrect parent %s: %w", parentID, err)
|
return false, fmt.Errorf("failed to resurrect parent %s: %w", parentID, err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -182,17 +182,18 @@ func (s *SQLiteStorage) CreateIssue(ctx context.Context, issue *types.Issue, act
|
|||||||
// For hierarchical IDs (bd-a3f8e9.1), ensure parent exists
|
// For hierarchical IDs (bd-a3f8e9.1), ensure parent exists
|
||||||
if strings.Contains(issue.ID, ".") {
|
if strings.Contains(issue.ID, ".") {
|
||||||
// Try to resurrect entire parent chain if any parents are missing
|
// Try to resurrect entire parent chain if any parents are missing
|
||||||
resurrected, err := s.TryResurrectParentChain(ctx, issue.ID)
|
// Use the conn-based version to participate in the same transaction
|
||||||
|
resurrected, err := s.tryResurrectParentChainWithConn(ctx, conn, issue.ID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("failed to resurrect parent chain for %s: %w", issue.ID, err)
|
return fmt.Errorf("failed to resurrect parent chain for %s: %w", issue.ID, err)
|
||||||
}
|
}
|
||||||
if !resurrected {
|
if !resurrected {
|
||||||
// Parent(s) not found in JSONL history - cannot proceed
|
// Parent(s) not found in JSONL history - cannot proceed
|
||||||
lastDot := strings.LastIndex(issue.ID, ".")
|
lastDot := strings.LastIndex(issue.ID, ".")
|
||||||
parentID := issue.ID[:lastDot]
|
parentID := issue.ID[:lastDot]
|
||||||
return fmt.Errorf("parent issue %s does not exist and could not be resurrected from JSONL history", parentID)
|
return fmt.Errorf("parent issue %s does not exist and could not be resurrected from JSONL history", parentID)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Insert issue
|
// Insert issue
|
||||||
|
|||||||
Reference in New Issue
Block a user