From 09fffa4eafcd6fdc9d952127d443e2927e10e926 Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Mon, 1 Dec 2025 21:56:41 -0800 Subject: [PATCH] fix(db): add close_reason column to issues table (bd-uyu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add migration 017_close_reason_column.go to create the column - Update all INSERT statements to include close_reason - Update all SELECT statements to include close_reason - Update doctor.go to check for close_reason in schema validation - Remove workaround code that batch-loaded close reasons from events table - Fix migrations_test.go to include close_reason in test table schema This fixes sync loops where close_reason values were silently dropped because the DB lacked the column despite the struct having the field. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- cmd/bd/doctor.go | 2 +- internal/storage/sqlite/dependencies.go | 21 ++++--------- internal/storage/sqlite/issues.go | 16 +++++----- internal/storage/sqlite/labels.go | 2 +- internal/storage/sqlite/migrations.go | 2 ++ .../migrations/017_close_reason_column.go | 31 +++++++++++++++++++ internal/storage/sqlite/migrations_test.go | 3 +- internal/storage/sqlite/multirepo.go | 6 ++-- internal/storage/sqlite/queries.go | 10 ++++-- internal/storage/sqlite/ready.go | 18 ++++++----- internal/storage/sqlite/transaction.go | 10 ++++-- 11 files changed, 79 insertions(+), 42 deletions(-) create mode 100644 internal/storage/sqlite/migrations/017_close_reason_column.go diff --git a/cmd/bd/doctor.go b/cmd/bd/doctor.go index 5b488165..f269750b 100644 --- a/cmd/bd/doctor.go +++ b/cmd/bd/doctor.go @@ -1780,7 +1780,7 @@ func checkSchemaCompatibility(path string) doctorCheck { // This is a simplified version since we can't import the internal package directly // Check all critical tables and columns criticalChecks := map[string][]string{ - "issues": {"id", "title", "content_hash", "external_ref", "compacted_at"}, + "issues": {"id", "title", "content_hash", "external_ref", "compacted_at", "close_reason"}, "dependencies": {"issue_id", "depends_on_id", "type"}, "child_counters": {"parent_id", "last_child"}, "export_hashes": {"issue_id", "content_hash"}, diff --git a/internal/storage/sqlite/dependencies.go b/internal/storage/sqlite/dependencies.go index 98598016..4c901641 100644 --- a/internal/storage/sqlite/dependencies.go +++ b/internal/storage/sqlite/dependencies.go @@ -678,7 +678,7 @@ func (s *SQLiteStorage) DetectCycles(ctx context.Context) ([][]*types.Issue, err func (s *SQLiteStorage) scanIssues(ctx context.Context, rows *sql.Rows) ([]*types.Issue, error) { var issues []*types.Issue var issueIDs []string - + // First pass: scan all issues for rows.Next() { var issue types.Issue @@ -688,12 +688,13 @@ func (s *SQLiteStorage) scanIssues(ctx context.Context, rows *sql.Rows) ([]*type var assignee sql.NullString var externalRef sql.NullString var sourceRepo sql.NullString + var closeReason sql.NullString err := rows.Scan( &issue.ID, &contentHash, &issue.Title, &issue.Description, &issue.Design, &issue.AcceptanceCriteria, &issue.Notes, &issue.Status, &issue.Priority, &issue.IssueType, &assignee, &estimatedMinutes, - &issue.CreatedAt, &issue.UpdatedAt, &closedAt, &externalRef, &sourceRepo, + &issue.CreatedAt, &issue.UpdatedAt, &closedAt, &externalRef, &sourceRepo, &closeReason, ) if err != nil { return nil, fmt.Errorf("failed to scan issue: %w", err) @@ -718,6 +719,9 @@ func (s *SQLiteStorage) scanIssues(ctx context.Context, rows *sql.Rows) ([]*type if sourceRepo.Valid { issue.SourceRepo = sourceRepo.String } + if closeReason.Valid { + issue.CloseReason = closeReason.String + } issues = append(issues, &issue) issueIDs = append(issueIDs, issue.ID) @@ -736,19 +740,6 @@ func (s *SQLiteStorage) scanIssues(ctx context.Context, rows *sql.Rows) ([]*type } } - // Third pass: batch-load close reasons for closed issues - closeReasonsMap, err := s.GetCloseReasonsForIssues(ctx, issueIDs) - if err != nil { - return nil, fmt.Errorf("failed to batch get close reasons: %w", err) - } - - // Assign close reasons to issues - for _, issue := range issues { - if reason, ok := closeReasonsMap[issue.ID]; ok { - issue.CloseReason = reason - } - } - return issues, nil } diff --git a/internal/storage/sqlite/issues.go b/internal/storage/sqlite/issues.go index 9808a0a7..73b36ca1 100644 --- a/internal/storage/sqlite/issues.go +++ b/internal/storage/sqlite/issues.go @@ -14,19 +14,19 @@ func insertIssue(ctx context.Context, conn *sql.Conn, issue *types.Issue) error if sourceRepo == "" { sourceRepo = "." // Default to primary repo } - + _, err := conn.ExecContext(ctx, ` INSERT INTO issues ( id, content_hash, title, description, design, acceptance_criteria, notes, status, priority, issue_type, assignee, estimated_minutes, - created_at, updated_at, closed_at, external_ref, source_repo - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + created_at, updated_at, closed_at, external_ref, source_repo, close_reason + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `, issue.ID, issue.ContentHash, issue.Title, issue.Description, issue.Design, issue.AcceptanceCriteria, issue.Notes, issue.Status, issue.Priority, issue.IssueType, issue.Assignee, issue.EstimatedMinutes, issue.CreatedAt, issue.UpdatedAt, - issue.ClosedAt, issue.ExternalRef, sourceRepo, + issue.ClosedAt, issue.ExternalRef, sourceRepo, issue.CloseReason, ) if err != nil { return fmt.Errorf("failed to insert issue: %w", err) @@ -40,8 +40,8 @@ func insertIssues(ctx context.Context, conn *sql.Conn, issues []*types.Issue) er INSERT INTO issues ( id, content_hash, title, description, design, acceptance_criteria, notes, status, priority, issue_type, assignee, estimated_minutes, - created_at, updated_at, closed_at, external_ref, source_repo - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + created_at, updated_at, closed_at, external_ref, source_repo, close_reason + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `) if err != nil { return fmt.Errorf("failed to prepare statement: %w", err) @@ -53,13 +53,13 @@ func insertIssues(ctx context.Context, conn *sql.Conn, issues []*types.Issue) er if sourceRepo == "" { sourceRepo = "." // Default to primary repo } - + _, err = stmt.ExecContext(ctx, issue.ID, issue.ContentHash, issue.Title, issue.Description, issue.Design, issue.AcceptanceCriteria, issue.Notes, issue.Status, issue.Priority, issue.IssueType, issue.Assignee, issue.EstimatedMinutes, issue.CreatedAt, issue.UpdatedAt, - issue.ClosedAt, issue.ExternalRef, sourceRepo, + issue.ClosedAt, issue.ExternalRef, sourceRepo, issue.CloseReason, ) if err != nil { return fmt.Errorf("failed to insert issue %s: %w", issue.ID, err) diff --git a/internal/storage/sqlite/labels.go b/internal/storage/sqlite/labels.go index 2f83350f..8018d18c 100644 --- a/internal/storage/sqlite/labels.go +++ b/internal/storage/sqlite/labels.go @@ -157,7 +157,7 @@ func (s *SQLiteStorage) GetIssuesByLabel(ctx context.Context, label string) ([]* rows, err := s.db.QueryContext(ctx, ` SELECT i.id, i.content_hash, i.title, i.description, i.design, i.acceptance_criteria, i.notes, i.status, i.priority, i.issue_type, i.assignee, i.estimated_minutes, - i.created_at, i.updated_at, i.closed_at, i.external_ref, i.source_repo + i.created_at, i.updated_at, i.closed_at, i.external_ref, i.source_repo, i.close_reason FROM issues i JOIN labels l ON i.id = l.issue_id WHERE l.label = ? diff --git a/internal/storage/sqlite/migrations.go b/internal/storage/sqlite/migrations.go index 142c0aa1..67f94fab 100644 --- a/internal/storage/sqlite/migrations.go +++ b/internal/storage/sqlite/migrations.go @@ -33,6 +33,7 @@ var migrationsList = []Migration{ {"child_counters_table", migrations.MigrateChildCountersTable}, {"blocked_issues_cache", migrations.MigrateBlockedIssuesCache}, {"orphan_detection", migrations.MigrateOrphanDetection}, + {"close_reason_column", migrations.MigrateCloseReasonColumn}, } // MigrationInfo contains metadata about a migration for inspection @@ -73,6 +74,7 @@ func getMigrationDescription(name string) string { "child_counters_table": "Adds child_counters table for hierarchical ID generation with ON DELETE CASCADE", "blocked_issues_cache": "Adds blocked_issues_cache table for GetReadyWork performance optimization (bd-5qim)", "orphan_detection": "Detects orphaned child issues and logs them for user action (bd-3852)", + "close_reason_column": "Adds close_reason column to issues table for storing closure explanations (bd-uyu)", } if desc, ok := descriptions[name]; ok { diff --git a/internal/storage/sqlite/migrations/017_close_reason_column.go b/internal/storage/sqlite/migrations/017_close_reason_column.go new file mode 100644 index 00000000..944a780f --- /dev/null +++ b/internal/storage/sqlite/migrations/017_close_reason_column.go @@ -0,0 +1,31 @@ +package migrations + +import ( + "database/sql" + "fmt" +) + +// MigrateCloseReasonColumn adds the close_reason column to the issues table. +// This column stores the reason provided when closing an issue. +func MigrateCloseReasonColumn(db *sql.DB) error { + var columnExists bool + err := db.QueryRow(` + SELECT COUNT(*) > 0 + FROM pragma_table_info('issues') + WHERE name = 'close_reason' + `).Scan(&columnExists) + if err != nil { + return fmt.Errorf("failed to check close_reason column: %w", err) + } + + if columnExists { + return nil + } + + _, err = db.Exec(`ALTER TABLE issues ADD COLUMN close_reason TEXT DEFAULT ''`) + if err != nil { + return fmt.Errorf("failed to add close_reason column: %w", err) + } + + return nil +} diff --git a/internal/storage/sqlite/migrations_test.go b/internal/storage/sqlite/migrations_test.go index 7360c001..d9384d8a 100644 --- a/internal/storage/sqlite/migrations_test.go +++ b/internal/storage/sqlite/migrations_test.go @@ -478,9 +478,10 @@ func TestMigrateContentHashColumn(t *testing.T) { original_size INTEGER, compacted_at_commit TEXT, source_repo TEXT DEFAULT '.', + close_reason TEXT DEFAULT '', CHECK ((status = 'closed') = (closed_at IS NOT NULL)) ); - INSERT INTO issues SELECT id, title, description, design, acceptance_criteria, notes, status, priority, issue_type, assignee, estimated_minutes, created_at, updated_at, closed_at, external_ref, compaction_level, compacted_at, original_size, compacted_at_commit, source_repo FROM issues_backup; + INSERT INTO issues SELECT id, title, description, design, acceptance_criteria, notes, status, priority, issue_type, assignee, estimated_minutes, created_at, updated_at, closed_at, external_ref, compaction_level, compacted_at, original_size, compacted_at_commit, source_repo, '' FROM issues_backup; DROP TABLE issues_backup; `) if err != nil { diff --git a/internal/storage/sqlite/multirepo.go b/internal/storage/sqlite/multirepo.go index adbf529d..21d16869 100644 --- a/internal/storage/sqlite/multirepo.go +++ b/internal/storage/sqlite/multirepo.go @@ -241,14 +241,14 @@ func (s *SQLiteStorage) upsertIssueInTx(ctx context.Context, tx *sql.Tx, issue * INSERT INTO issues ( id, content_hash, title, description, design, acceptance_criteria, notes, status, priority, issue_type, assignee, estimated_minutes, - created_at, updated_at, closed_at, external_ref, source_repo - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + created_at, updated_at, closed_at, external_ref, source_repo, close_reason + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `, issue.ID, issue.ContentHash, issue.Title, issue.Description, issue.Design, issue.AcceptanceCriteria, issue.Notes, issue.Status, issue.Priority, issue.IssueType, issue.Assignee, issue.EstimatedMinutes, issue.CreatedAt, issue.UpdatedAt, - issue.ClosedAt, issue.ExternalRef, issue.SourceRepo, + issue.ClosedAt, issue.ExternalRef, issue.SourceRepo, issue.CloseReason, ) if err != nil { return fmt.Errorf("failed to insert issue: %w", err) diff --git a/internal/storage/sqlite/queries.go b/internal/storage/sqlite/queries.go index dbaf04d4..cb914f58 100644 --- a/internal/storage/sqlite/queries.go +++ b/internal/storage/sqlite/queries.go @@ -157,6 +157,7 @@ func (s *SQLiteStorage) GetIssue(ctx context.Context, id string) (*types.Issue, var compactedAt sql.NullTime var originalSize sql.NullInt64 var sourceRepo sql.NullString + var closeReason sql.NullString var contentHash sql.NullString var compactedAtCommit sql.NullString @@ -164,7 +165,7 @@ func (s *SQLiteStorage) GetIssue(ctx context.Context, id string) (*types.Issue, SELECT id, content_hash, title, description, design, acceptance_criteria, notes, status, priority, issue_type, assignee, estimated_minutes, created_at, updated_at, closed_at, external_ref, - compaction_level, compacted_at, compacted_at_commit, original_size, source_repo + compaction_level, compacted_at, compacted_at_commit, original_size, source_repo, close_reason FROM issues WHERE id = ? `, id).Scan( @@ -172,7 +173,7 @@ func (s *SQLiteStorage) GetIssue(ctx context.Context, id string) (*types.Issue, &issue.AcceptanceCriteria, &issue.Notes, &issue.Status, &issue.Priority, &issue.IssueType, &assignee, &estimatedMinutes, &issue.CreatedAt, &issue.UpdatedAt, &closedAt, &externalRef, - &issue.CompactionLevel, &compactedAt, &compactedAtCommit, &originalSize, &sourceRepo, + &issue.CompactionLevel, &compactedAt, &compactedAtCommit, &originalSize, &sourceRepo, &closeReason, ) if err == sql.ErrNoRows { @@ -210,6 +211,9 @@ func (s *SQLiteStorage) GetIssue(ctx context.Context, id string) (*types.Issue, if sourceRepo.Valid { issue.SourceRepo = sourceRepo.String } + if closeReason.Valid { + issue.CloseReason = closeReason.String + } // Fetch labels for this issue labels, err := s.GetLabels(ctx, issue.ID) @@ -1263,7 +1267,7 @@ func (s *SQLiteStorage) SearchIssues(ctx context.Context, query string, filter t querySQL := fmt.Sprintf(` SELECT id, content_hash, title, description, design, acceptance_criteria, notes, status, priority, issue_type, assignee, estimated_minutes, - created_at, updated_at, closed_at, external_ref, source_repo + created_at, updated_at, closed_at, external_ref, source_repo, close_reason FROM issues %s ORDER BY priority ASC, created_at DESC diff --git a/internal/storage/sqlite/ready.go b/internal/storage/sqlite/ready.go index ddf0e5d2..4ed0e9e8 100644 --- a/internal/storage/sqlite/ready.go +++ b/internal/storage/sqlite/ready.go @@ -99,7 +99,7 @@ func (s *SQLiteStorage) GetReadyWork(ctx context.Context, filter types.WorkFilte query := fmt.Sprintf(` SELECT i.id, i.content_hash, i.title, i.description, i.design, i.acceptance_criteria, i.notes, i.status, i.priority, i.issue_type, i.assignee, i.estimated_minutes, - i.created_at, i.updated_at, i.closed_at, i.external_ref, i.source_repo + i.created_at, i.updated_at, i.closed_at, i.external_ref, i.source_repo, i.close_reason FROM issues i WHERE %s AND NOT EXISTS ( @@ -126,7 +126,7 @@ func (s *SQLiteStorage) GetStaleIssues(ctx context.Context, filter types.StaleFi id, content_hash, title, description, design, acceptance_criteria, notes, status, priority, issue_type, assignee, estimated_minutes, created_at, updated_at, closed_at, external_ref, source_repo, - compaction_level, compacted_at, compacted_at_commit, original_size + compaction_level, compacted_at, compacted_at_commit, original_size, close_reason FROM issues WHERE status != 'closed' AND datetime(updated_at) < datetime('now', '-' || ? || ' days') @@ -167,18 +167,19 @@ func (s *SQLiteStorage) GetStaleIssues(ctx context.Context, filter types.StaleFi var compactedAt sql.NullTime var compactedAtCommit sql.NullString var originalSize sql.NullInt64 - + var closeReason sql.NullString + err := rows.Scan( &issue.ID, &contentHash, &issue.Title, &issue.Description, &issue.Design, &issue.AcceptanceCriteria, &issue.Notes, &issue.Status, &issue.Priority, &issue.IssueType, &assignee, &estimatedMinutes, &issue.CreatedAt, &issue.UpdatedAt, &closedAt, &externalRef, &sourceRepo, - &compactionLevel, &compactedAt, &compactedAtCommit, &originalSize, + &compactionLevel, &compactedAt, &compactedAtCommit, &originalSize, &closeReason, ) if err != nil { return nil, fmt.Errorf("failed to scan stale issue: %w", err) } - + if contentHash.Valid { issue.ContentHash = contentHash.String } @@ -210,10 +211,13 @@ func (s *SQLiteStorage) GetStaleIssues(ctx context.Context, filter types.StaleFi if originalSize.Valid { issue.OriginalSize = int(originalSize.Int64) } - + if closeReason.Valid { + issue.CloseReason = closeReason.String + } + issues = append(issues, &issue) } - + return issues, rows.Err() } diff --git a/internal/storage/sqlite/transaction.go b/internal/storage/sqlite/transaction.go index 6782dd7b..a0251d35 100644 --- a/internal/storage/sqlite/transaction.go +++ b/internal/storage/sqlite/transaction.go @@ -253,7 +253,7 @@ func (t *sqliteTxStorage) GetIssue(ctx context.Context, id string) (*types.Issue SELECT id, content_hash, title, description, design, acceptance_criteria, notes, status, priority, issue_type, assignee, estimated_minutes, created_at, updated_at, closed_at, external_ref, - compaction_level, compacted_at, compacted_at_commit, original_size, source_repo + compaction_level, compacted_at, compacted_at_commit, original_size, source_repo, close_reason FROM issues WHERE id = ? `, id) @@ -1026,7 +1026,7 @@ func (t *sqliteTxStorage) SearchIssues(ctx context.Context, query string, filter SELECT id, content_hash, title, description, design, acceptance_criteria, notes, status, priority, issue_type, assignee, estimated_minutes, created_at, updated_at, closed_at, external_ref, - compaction_level, compacted_at, compacted_at_commit, original_size, source_repo + compaction_level, compacted_at, compacted_at_commit, original_size, source_repo, close_reason FROM issues %s ORDER BY priority ASC, created_at DESC @@ -1061,13 +1061,14 @@ func scanIssueRow(row scanner) (*types.Issue, error) { var originalSize sql.NullInt64 var sourceRepo sql.NullString var compactedAtCommit sql.NullString + var closeReason sql.NullString err := row.Scan( &issue.ID, &contentHash, &issue.Title, &issue.Description, &issue.Design, &issue.AcceptanceCriteria, &issue.Notes, &issue.Status, &issue.Priority, &issue.IssueType, &assignee, &estimatedMinutes, &issue.CreatedAt, &issue.UpdatedAt, &closedAt, &externalRef, - &issue.CompactionLevel, &compactedAt, &compactedAtCommit, &originalSize, &sourceRepo, + &issue.CompactionLevel, &compactedAt, &compactedAtCommit, &originalSize, &sourceRepo, &closeReason, ) if err != nil { return nil, fmt.Errorf("failed to scan issue: %w", err) @@ -1101,6 +1102,9 @@ func scanIssueRow(row scanner) (*types.Issue, error) { if sourceRepo.Valid { issue.SourceRepo = sourceRepo.String } + if closeReason.Valid { + issue.CloseReason = closeReason.String + } return &issue, nil }