Files
beads/internal/storage/sqlite/comments.go
beads/crew/dave ec1a32b9a8 fix(storage): add reconnectMu RLock protection to prevent race condition (#1054)
Add missing reconnectMu.RLock() protection to storage methods that were
vulnerable to the same race condition fixed in GH#607. The FreshnessChecker
can trigger reconnect() which closes s.db while queries are in flight,
causing "database is closed" errors during daemon export operations.

Protected methods:
- labels.go: GetLabelsForIssues (GetLabels intentionally unprotected - called from GetIssue which holds lock)
- comments.go: GetIssueComments, GetCommentsForIssues
- dependencies.go: GetDependencyCounts, GetDependencyRecords, GetAllDependencyRecords, GetDependencyTree, loadDependencyGraph
- config.go: SetConfig, GetConfig, GetAllConfig, DeleteConfig, SetMetadata, GetMetadata
- dirty.go: MarkIssueDirty, GetDirtyIssues, GetDirtyIssueHash, GetDirtyIssueCount
- events.go: GetEvents, GetStatistics, GetMoleculeProgress
- hash.go: All hash methods
- hash_ids.go: GetNextChildID, ensureChildCounterUpdated (getNextChildNumber unprotected - called internally)

Internal helpers called from already-locked contexts intentionally omit
RLock to avoid deadlock (Go's RWMutex doesn't support recursive locking).

Fixes: bd-vx7fp

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Executed-By: beads/crew/dave
Rig: beads
Role: crew
2026-01-12 18:29:42 -08:00

185 lines
5.8 KiB
Go

package sqlite
import (
"context"
"fmt"
"github.com/steveyegge/beads/internal/types"
)
// AddIssueComment adds a comment to an issue
func (s *SQLiteStorage) AddIssueComment(ctx context.Context, issueID, author, text string) (*types.Comment, error) {
// Verify issue exists
var exists bool
err := s.db.QueryRowContext(ctx, `SELECT EXISTS(SELECT 1 FROM issues WHERE id = ?)`, issueID).Scan(&exists)
if err != nil {
return nil, fmt.Errorf("failed to check issue existence: %w", err)
}
if !exists {
return nil, fmt.Errorf("issue %s not found", issueID)
}
// Insert comment
result, err := s.db.ExecContext(ctx, `
INSERT INTO comments (issue_id, author, text, created_at)
VALUES (?, ?, ?, CURRENT_TIMESTAMP)
`, issueID, author, text)
if err != nil {
return nil, fmt.Errorf("failed to insert comment: %w", err)
}
// Get the inserted comment ID
commentID, err := result.LastInsertId()
if err != nil {
return nil, fmt.Errorf("failed to get comment ID: %w", err)
}
// Fetch the complete comment
comment := &types.Comment{}
err = s.db.QueryRowContext(ctx, `
SELECT id, issue_id, author, text, created_at
FROM comments WHERE id = ?
`, commentID).Scan(&comment.ID, &comment.IssueID, &comment.Author, &comment.Text, &comment.CreatedAt)
if err != nil {
return nil, fmt.Errorf("failed to fetch comment: %w", err)
}
// Mark issue as dirty for JSONL export
if err := s.MarkIssueDirty(ctx, issueID); err != nil {
return nil, fmt.Errorf("failed to mark issue dirty: %w", err)
}
return comment, nil
}
// ImportIssueComment adds a comment during import, preserving the original timestamp.
// Unlike AddIssueComment which uses CURRENT_TIMESTAMP, this method uses the provided
// createdAt time from the JSONL file. This prevents timestamp drift during sync cycles.
// GH#735: Comment created_at timestamps were being overwritten with current time during import.
func (s *SQLiteStorage) ImportIssueComment(ctx context.Context, issueID, author, text string, createdAt string) (*types.Comment, error) {
// Verify issue exists
var exists bool
err := s.db.QueryRowContext(ctx, `SELECT EXISTS(SELECT 1 FROM issues WHERE id = ?)`, issueID).Scan(&exists)
if err != nil {
return nil, fmt.Errorf("failed to check issue existence: %w", err)
}
if !exists {
return nil, fmt.Errorf("issue %s not found", issueID)
}
// Insert comment with provided timestamp
result, err := s.db.ExecContext(ctx, `
INSERT INTO comments (issue_id, author, text, created_at)
VALUES (?, ?, ?, ?)
`, issueID, author, text, createdAt)
if err != nil {
return nil, fmt.Errorf("failed to insert comment: %w", err)
}
// Get the inserted comment ID
commentID, err := result.LastInsertId()
if err != nil {
return nil, fmt.Errorf("failed to get comment ID: %w", err)
}
// Fetch the complete comment
comment := &types.Comment{}
err = s.db.QueryRowContext(ctx, `
SELECT id, issue_id, author, text, created_at
FROM comments WHERE id = ?
`, commentID).Scan(&comment.ID, &comment.IssueID, &comment.Author, &comment.Text, &comment.CreatedAt)
if err != nil {
return nil, fmt.Errorf("failed to fetch comment: %w", err)
}
// Mark issue as dirty for JSONL export
if err := s.MarkIssueDirty(ctx, issueID); err != nil {
return nil, fmt.Errorf("failed to mark issue dirty: %w", err)
}
return comment, nil
}
// GetIssueComments retrieves all comments for an issue
func (s *SQLiteStorage) GetIssueComments(ctx context.Context, issueID string) ([]*types.Comment, error) {
// Hold read lock during database operations to prevent reconnect() from
// closing the connection mid-query (GH#607 race condition fix)
s.reconnectMu.RLock()
defer s.reconnectMu.RUnlock()
rows, err := s.db.QueryContext(ctx, `
SELECT id, issue_id, author, text, created_at
FROM comments
WHERE issue_id = ?
ORDER BY created_at ASC
`, issueID)
if err != nil {
return nil, fmt.Errorf("failed to query comments: %w", err)
}
defer func() { _ = rows.Close() }()
var comments []*types.Comment
for rows.Next() {
comment := &types.Comment{}
err := rows.Scan(&comment.ID, &comment.IssueID, &comment.Author, &comment.Text, &comment.CreatedAt)
if err != nil {
return nil, fmt.Errorf("failed to scan comment: %w", err)
}
comments = append(comments, comment)
}
if err := rows.Err(); err != nil {
return nil, fmt.Errorf("error iterating comments: %w", err)
}
return comments, nil
}
// GetCommentsForIssues fetches comments for multiple issues in a single query
// Returns a map of issue_id -> []*Comment
func (s *SQLiteStorage) GetCommentsForIssues(ctx context.Context, issueIDs []string) (map[string][]*types.Comment, error) {
if len(issueIDs) == 0 {
return make(map[string][]*types.Comment), nil
}
// Hold read lock during database operations to prevent reconnect() from
// closing the connection mid-query (GH#607 race condition fix)
s.reconnectMu.RLock()
defer s.reconnectMu.RUnlock()
// Build placeholders for IN clause
placeholders := make([]interface{}, len(issueIDs))
for i, id := range issueIDs {
placeholders[i] = id
}
query := fmt.Sprintf(`
SELECT id, issue_id, author, text, created_at
FROM comments
WHERE issue_id IN (%s)
ORDER BY issue_id, created_at ASC
`, buildPlaceholders(len(issueIDs))) // #nosec G201 -- placeholders are generated internally
rows, err := s.db.QueryContext(ctx, query, placeholders...)
if err != nil {
return nil, fmt.Errorf("failed to batch get comments: %w", err)
}
defer func() { _ = rows.Close() }()
result := make(map[string][]*types.Comment)
for rows.Next() {
comment := &types.Comment{}
err := rows.Scan(&comment.ID, &comment.IssueID, &comment.Author, &comment.Text, &comment.CreatedAt)
if err != nil {
return nil, fmt.Errorf("failed to scan comment: %w", err)
}
result[comment.IssueID] = append(result[comment.IssueID], comment)
}
if err := rows.Err(); err != nil {
return nil, fmt.Errorf("error iterating comments: %w", err)
}
return result, nil
}