fix: prevent FK constraint errors on concurrent issue creation (GH#956)

Root cause: CreateIssue used INSERT OR IGNORE which could silently skip
the insert (e.g., on duplicate ID from hash collision), then fail with
FOREIGN KEY constraint error when trying to record the creation event.

Fix: Add insertIssueStrict() that uses plain INSERT (fails on duplicates)
and use it for CreateIssue in both queries.go and transaction.go. The
existing insertIssue() with INSERT OR IGNORE is preserved for import
scenarios where duplicates are expected.

Added test TestCreateIssueDuplicateID to verify duplicate IDs are properly
rejected instead of silently ignored.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
wolf
2026-01-09 22:58:46 -08:00
committed by Steve Yegge
parent a851104203
commit 0777bb907c
4 changed files with 107 additions and 5 deletions

View File

@@ -20,7 +20,9 @@ func isUniqueConstraintError(err error) bool {
strings.Contains(errMsg, "constraint failed: UNIQUE")
}
// insertIssue inserts a single issue into the database
// insertIssue inserts a single issue into the database.
// Uses INSERT OR IGNORE for backward compatibility with imports.
// For fresh issue creation, use insertIssueStrict instead.
func insertIssue(ctx context.Context, conn *sql.Conn, issue *types.Issue) error {
sourceRepo := issue.SourceRepo
if sourceRepo == "" {
@@ -75,6 +77,59 @@ func insertIssue(ctx context.Context, conn *sql.Conn, issue *types.Issue) error
return nil
}
// insertIssueStrict inserts a single issue into the database, failing on duplicates.
// This is used for fresh issue creation (CreateIssue) where duplicates indicate a bug.
// For imports where duplicates are expected, use insertIssue instead.
// GH#956: Using plain INSERT prevents FK constraint errors from silent INSERT OR IGNORE failures.
func insertIssueStrict(ctx context.Context, conn *sql.Conn, issue *types.Issue) error {
sourceRepo := issue.SourceRepo
if sourceRepo == "" {
sourceRepo = "." // Default to primary repo
}
wisp := 0
if issue.Ephemeral {
wisp = 1
}
pinned := 0
if issue.Pinned {
pinned = 1
}
isTemplate := 0
if issue.IsTemplate {
isTemplate = 1
}
_, 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, created_by, updated_at, closed_at, external_ref, source_repo, close_reason,
deleted_at, deleted_by, delete_reason, original_type,
sender, ephemeral, pinned, is_template,
await_type, await_id, timeout_ns, waiters, mol_type,
event_kind, actor, target, payload,
due_at, defer_until
) 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.CreatedBy, issue.UpdatedAt,
issue.ClosedAt, issue.ExternalRef, sourceRepo, issue.CloseReason,
issue.DeletedAt, issue.DeletedBy, issue.DeleteReason, issue.OriginalType,
issue.Sender, wisp, pinned, isTemplate,
issue.AwaitType, issue.AwaitID, int64(issue.Timeout), formatJSONStringArray(issue.Waiters),
string(issue.MolType),
issue.EventKind, issue.Actor, issue.Target, issue.Payload,
issue.DueAt, issue.DeferUntil,
)
if err != nil {
return fmt.Errorf("failed to insert issue: %w", err)
}
return nil
}
// insertIssues bulk inserts multiple issues using a prepared statement
func insertIssues(ctx context.Context, conn *sql.Conn, issues []*types.Issue) error {
stmt, err := conn.PrepareContext(ctx, `