Fix critical bugs: bd-169, bd-28, bd-393
- bd-169: Add -q/--quiet flag to bd init command - bd-28: Improve error handling in RemoveDependency - Now checks RowsAffected and returns error if dependency doesn't exist - New removeDependencyIfExists() helper for collision remapping - bd-393: CRITICAL - Fix auto-import skipping collisions - Auto-import was LOSING work from other workers - Now automatically remaps collisions to new IDs - Calls RemapCollisions() instead of skipping All tests pass. Amp-Thread-ID: https://ampcode.com/threads/T-cba86837-28db-47ce-94eb-67fade82376a Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
@@ -466,11 +466,10 @@ func updateDependencyReferences(ctx context.Context, s *SQLiteStorage, idMapping
|
||||
|
||||
// Phase 2: Apply all collected changes
|
||||
for _, update := range updates {
|
||||
// Remove old dependency
|
||||
if err := s.RemoveDependency(ctx, update.oldIssueID, update.oldDependsOnID, "import-remap"); err != nil {
|
||||
// If the dependency doesn't exist (e.g., already removed), that's okay
|
||||
// This can happen if both IssueID and DependsOnID were remapped
|
||||
continue
|
||||
// Remove old dependency - use RemoveDependencyIfExists which doesn't error on missing deps
|
||||
if err := s.removeDependencyIfExists(ctx, update.oldIssueID, update.oldDependsOnID, "import-remap"); err != nil {
|
||||
return fmt.Errorf("failed to remove old dependency %s -> %s: %w",
|
||||
update.oldIssueID, update.oldDependsOnID, err)
|
||||
}
|
||||
|
||||
// Add new dependency with updated IDs
|
||||
|
||||
@@ -271,13 +271,65 @@ func (s *SQLiteStorage) RemoveDependency(ctx context.Context, issueID, dependsOn
|
||||
}
|
||||
defer tx.Rollback()
|
||||
|
||||
_, err = tx.ExecContext(ctx, `
|
||||
result, err := tx.ExecContext(ctx, `
|
||||
DELETE FROM dependencies WHERE issue_id = ? AND depends_on_id = ?
|
||||
`, issueID, dependsOnID)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to remove dependency: %w", err)
|
||||
}
|
||||
|
||||
// Check if dependency existed
|
||||
rowsAffected, err := result.RowsAffected()
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to check rows affected: %w", err)
|
||||
}
|
||||
if rowsAffected == 0 {
|
||||
return fmt.Errorf("dependency from %s to %s does not exist", issueID, dependsOnID)
|
||||
}
|
||||
|
||||
_, err = tx.ExecContext(ctx, `
|
||||
INSERT INTO events (issue_id, event_type, actor, comment)
|
||||
VALUES (?, ?, ?, ?)
|
||||
`, issueID, types.EventDependencyRemoved, actor,
|
||||
fmt.Sprintf("Removed dependency on %s", dependsOnID))
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to record event: %w", err)
|
||||
}
|
||||
|
||||
// Mark both issues as dirty for incremental export
|
||||
if err := markIssuesDirtyTx(ctx, tx, []string{issueID, dependsOnID}); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
return tx.Commit()
|
||||
}
|
||||
|
||||
// removeDependencyIfExists removes a dependency, returning nil if it doesn't exist
|
||||
// This is useful during remapping where dependencies may have been already removed
|
||||
func (s *SQLiteStorage) removeDependencyIfExists(ctx context.Context, issueID, dependsOnID string, actor string) error {
|
||||
tx, err := s.db.BeginTx(ctx, nil)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to begin transaction: %w", err)
|
||||
}
|
||||
defer tx.Rollback()
|
||||
|
||||
result, err := tx.ExecContext(ctx, `
|
||||
DELETE FROM dependencies WHERE issue_id = ? AND depends_on_id = ?
|
||||
`, issueID, dependsOnID)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to remove dependency: %w", err)
|
||||
}
|
||||
|
||||
// Check if dependency existed - if not, that's okay, just skip the event
|
||||
rowsAffected, err := result.RowsAffected()
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to check rows affected: %w", err)
|
||||
}
|
||||
if rowsAffected == 0 {
|
||||
// Dependency didn't exist, nothing to do
|
||||
return nil
|
||||
}
|
||||
|
||||
_, err = tx.ExecContext(ctx, `
|
||||
INSERT INTO events (issue_id, event_type, actor, comment)
|
||||
VALUES (?, ?, ?, ?)
|
||||
|
||||
Reference in New Issue
Block a user