From 22d34a22dca512e0d17db5365e8176551bc67509 Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Thu, 27 Nov 2025 21:45:42 -0800 Subject: [PATCH] Fix bd migrate loop: skip prefix validation during auto-import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When auto-importing issues from JSONL, issues with different prefixes (e.g., gt-1 vs gastown-) would fail validation and cause an infinite loop of failed migrations. The fix adds SkipPrefixValidation option to CreateIssuesWithFullOptions which propagates through EnsureIDs to skip prefix validation for issues that already have IDs during import. This allows importing issues with any prefix while still validating new issues created interactively. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- internal/importer/importer.go | 6 +++++- internal/storage/sqlite/batch_ops.go | 20 +++++++++++++++++--- internal/storage/sqlite/batch_ops_test.go | 22 ++++++++++++++++++++-- internal/storage/sqlite/ids.go | 12 ++++++++---- 4 files changed, 50 insertions(+), 10 deletions(-) diff --git a/internal/importer/importer.go b/internal/importer/importer.go index 152a6b37..cd6bb500 100644 --- a/internal/importer/importer.go +++ b/internal/importer/importer.go @@ -620,7 +620,11 @@ if len(newIssues) > 0 { } } if len(batchForDepth) > 0 { - if err := sqliteStore.CreateIssuesWithOptions(ctx, batchForDepth, "import", opts.OrphanHandling); err != nil { + batchOpts := sqlite.BatchCreateOptions{ + OrphanHandling: opts.OrphanHandling, + SkipPrefixValidation: opts.SkipPrefixValidation, + } + if err := sqliteStore.CreateIssuesWithFullOptions(ctx, batchForDepth, "import", batchOpts); err != nil { return fmt.Errorf("error creating depth-%d issues: %w", depth, err) } result.Created += len(batchForDepth) diff --git a/internal/storage/sqlite/batch_ops.go b/internal/storage/sqlite/batch_ops.go index e75c342d..a9ed3c40 100644 --- a/internal/storage/sqlite/batch_ops.go +++ b/internal/storage/sqlite/batch_ops.go @@ -33,7 +33,7 @@ func validateBatchIssues(issues []*types.Issue) error { } // generateBatchIDs generates IDs for all issues that need them atomically -func (s *SQLiteStorage) generateBatchIDs(ctx context.Context, conn *sql.Conn, issues []*types.Issue, actor string, orphanHandling OrphanHandling) error { +func (s *SQLiteStorage) generateBatchIDs(ctx context.Context, conn *sql.Conn, issues []*types.Issue, actor string, orphanHandling OrphanHandling, skipPrefixValidation bool) error { // Get prefix from config (needed for both generation and validation) var prefix string err := conn.QueryRowContext(ctx, `SELECT value FROM config WHERE key = ?`, "issue_prefix").Scan(&prefix) @@ -45,7 +45,7 @@ func (s *SQLiteStorage) generateBatchIDs(ctx context.Context, conn *sql.Conn, is } // Generate or validate IDs for all issues - if err := EnsureIDs(ctx, conn, prefix, issues, actor, orphanHandling); err != nil { + if err := EnsureIDs(ctx, conn, prefix, issues, actor, orphanHandling, skipPrefixValidation); err != nil { return wrapDBError("ensure IDs", err) } @@ -126,8 +126,22 @@ func (s *SQLiteStorage) CreateIssues(ctx context.Context, issues []*types.Issue, return s.CreateIssuesWithOptions(ctx, issues, actor, OrphanResurrect) } +// BatchCreateOptions contains options for batch issue creation +type BatchCreateOptions struct { + OrphanHandling OrphanHandling // How to handle missing parent issues + SkipPrefixValidation bool // Skip prefix validation for existing IDs (used during import) +} + // CreateIssuesWithOptions creates multiple issues with configurable orphan handling func (s *SQLiteStorage) CreateIssuesWithOptions(ctx context.Context, issues []*types.Issue, actor string, orphanHandling OrphanHandling) error { + return s.CreateIssuesWithFullOptions(ctx, issues, actor, BatchCreateOptions{ + OrphanHandling: orphanHandling, + SkipPrefixValidation: false, + }) +} + +// CreateIssuesWithFullOptions creates multiple issues with full options control +func (s *SQLiteStorage) CreateIssuesWithFullOptions(ctx context.Context, issues []*types.Issue, actor string, opts BatchCreateOptions) error { if len(issues) == 0 { return nil } @@ -157,7 +171,7 @@ func (s *SQLiteStorage) CreateIssuesWithOptions(ctx context.Context, issues []*t }() // Phase 3: Generate IDs for issues that need them - if err := s.generateBatchIDs(ctx, conn, issues, actor, orphanHandling); err != nil { + if err := s.generateBatchIDs(ctx, conn, issues, actor, opts.OrphanHandling, opts.SkipPrefixValidation); err != nil { return wrapDBError("generate batch IDs", err) } diff --git a/internal/storage/sqlite/batch_ops_test.go b/internal/storage/sqlite/batch_ops_test.go index 88c12307..fba2bc8e 100644 --- a/internal/storage/sqlite/batch_ops_test.go +++ b/internal/storage/sqlite/batch_ops_test.go @@ -270,7 +270,7 @@ func TestGenerateBatchIDs(t *testing.T) { {Title: "Issue 3", Description: "Third", CreatedAt: time.Now()}, } - err = s.generateBatchIDs(ctx, conn, issues, "test-actor", OrphanAllow) + err = s.generateBatchIDs(ctx, conn, issues, "test-actor", OrphanAllow, false) if err != nil { t.Fatalf("failed to generate IDs: %v", err) } @@ -299,11 +299,29 @@ func TestGenerateBatchIDs(t *testing.T) { {ID: "wrong-prefix-123", Title: "Wrong", CreatedAt: time.Now()}, } - err = s.generateBatchIDs(ctx, conn, issues, "test-actor", OrphanAllow) + err = s.generateBatchIDs(ctx, conn, issues, "test-actor", OrphanAllow, false) if err == nil { t.Fatal("expected error for wrong prefix") } }) + + t.Run("skips prefix validation when flag is set", func(t *testing.T) { + conn, err := s.db.Conn(ctx) + if err != nil { + t.Fatalf("failed to get connection: %v", err) + } + defer conn.Close() + + issues := []*types.Issue{ + {ID: "wrong-prefix-123", Title: "Wrong", CreatedAt: time.Now()}, + } + + // With skipPrefixValidation=true, should not error + err = s.generateBatchIDs(ctx, conn, issues, "test-actor", OrphanAllow, true) + if err != nil { + t.Fatalf("should not error with skipPrefixValidation=true: %v", err) + } + }) } func TestBulkOperations(t *testing.T) { diff --git a/internal/storage/sqlite/ids.go b/internal/storage/sqlite/ids.go index 943a220c..97644aea 100644 --- a/internal/storage/sqlite/ids.go +++ b/internal/storage/sqlite/ids.go @@ -187,15 +187,19 @@ func tryResurrectParent(parentID string, issues []*types.Issue) bool { // For issues with empty IDs, generates unique hash-based IDs // For issues with existing IDs, validates they match the prefix and parent exists (if hierarchical) // For hierarchical IDs with missing parents, behavior depends on orphanHandling mode -func EnsureIDs(ctx context.Context, conn *sql.Conn, prefix string, issues []*types.Issue, actor string, orphanHandling OrphanHandling) error { +// When skipPrefixValidation is true, existing IDs are not validated against the prefix (used during import) +func EnsureIDs(ctx context.Context, conn *sql.Conn, prefix string, issues []*types.Issue, actor string, orphanHandling OrphanHandling, skipPrefixValidation bool) error { usedIDs := make(map[string]bool) - + // First pass: record explicitly provided IDs for i := range issues { if issues[i].ID != "" { // Validate that explicitly provided ID matches the configured prefix (bd-177) - if err := ValidateIssueIDPrefix(issues[i].ID, prefix); err != nil { - return wrapDBErrorf(err, "validate ID prefix for %s", issues[i].ID) + // Skip validation during import to allow issues with different prefixes (e.g., from renamed repos) + if !skipPrefixValidation { + if err := ValidateIssueIDPrefix(issues[i].ID, prefix); err != nil { + return wrapDBErrorf(err, "validate ID prefix for %s", issues[i].ID) + } } // For hierarchical IDs (bd-a3f8e9.1), ensure parent exists