Implement merge validation logic (bd-192)
- Add merge command with --into and --dry-run flags - Validate target and source issues exist - Validate no self-merge attempts - Add comprehensive test coverage - Capture --id flag feature request as bd-9369 Amp-Thread-ID: https://ampcode.com/threads/T-22945597-9f4f-413b-afde-dcf3099eb2f0 Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
@@ -1018,7 +1018,7 @@
|
|||||||
{"id":"bd-1917","title":"Agent 5 Issue 74","description":"","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-21T16:55:03.368848-07:00","updated_at":"2025-10-21T16:55:03.368848-07:00"}
|
{"id":"bd-1917","title":"Agent 5 Issue 74","description":"","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-21T16:55:03.368848-07:00","updated_at":"2025-10-21T16:55:03.368848-07:00"}
|
||||||
{"id":"bd-1918","title":"Agent 5 Issue 75","description":"","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-21T16:55:03.370049-07:00","updated_at":"2025-10-21T16:55:03.370049-07:00"}
|
{"id":"bd-1918","title":"Agent 5 Issue 75","description":"","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-21T16:55:03.370049-07:00","updated_at":"2025-10-21T16:55:03.370049-07:00"}
|
||||||
{"id":"bd-1919","title":"Agent 5 Issue 76","description":"","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-21T16:55:03.371221-07:00","updated_at":"2025-10-21T16:55:03.371221-07:00"}
|
{"id":"bd-1919","title":"Agent 5 Issue 76","description":"","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-21T16:55:03.371221-07:00","updated_at":"2025-10-21T16:55:03.371221-07:00"}
|
||||||
{"id":"bd-192","title":"Implement merge validation logic","description":"Validate merge operations: check for circular merges, self-merge, non-existent issues, and other edge cases","notes":"Simplified: no schema field needed. Just validate issues exist and no self-merge. Close reason pattern: 'Merged into bd-X'","status":"open","priority":1,"issue_type":"task","created_at":"2025-10-21T16:26:07.513032-07:00","updated_at":"2025-10-21T17:46:47.498332-07:00"}
|
{"id":"bd-192","title":"Implement merge validation logic","description":"Validate merge operations: check for circular merges, self-merge, non-existent issues, and other edge cases","notes":"Simplified: no schema field needed. Just validate issues exist and no self-merge. Close reason pattern: 'Merged into bd-X'","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-21T16:26:07.513032-07:00","updated_at":"2025-10-21T17:51:24.046249-07:00","closed_at":"2025-10-21T17:51:24.046249-07:00"}
|
||||||
{"id":"bd-1920","title":"Agent 5 Issue 77","description":"","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-21T16:55:03.372421-07:00","updated_at":"2025-10-21T16:55:03.372421-07:00"}
|
{"id":"bd-1920","title":"Agent 5 Issue 77","description":"","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-21T16:55:03.372421-07:00","updated_at":"2025-10-21T16:55:03.372421-07:00"}
|
||||||
{"id":"bd-1921","title":"Agent 5 Issue 78","description":"","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-21T16:55:03.373638-07:00","updated_at":"2025-10-21T16:55:03.373638-07:00"}
|
{"id":"bd-1921","title":"Agent 5 Issue 78","description":"","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-21T16:55:03.373638-07:00","updated_at":"2025-10-21T16:55:03.373638-07:00"}
|
||||||
{"id":"bd-1922","title":"Agent 5 Issue 79","description":"","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-21T16:55:03.374841-07:00","updated_at":"2025-10-21T16:55:03.374841-07:00"}
|
{"id":"bd-1922","title":"Agent 5 Issue 79","description":"","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-21T16:55:03.374841-07:00","updated_at":"2025-10-21T16:55:03.374841-07:00"}
|
||||||
|
|||||||
126
cmd/bd/merge.go
Normal file
126
cmd/bd/merge.go
Normal file
@@ -0,0 +1,126 @@
|
|||||||
|
package main
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"fmt"
|
||||||
|
"os"
|
||||||
|
"strings"
|
||||||
|
|
||||||
|
"github.com/fatih/color"
|
||||||
|
"github.com/spf13/cobra"
|
||||||
|
)
|
||||||
|
|
||||||
|
var mergeCmd = &cobra.Command{
|
||||||
|
Use: "merge [source-id...] --into [target-id]",
|
||||||
|
Short: "Merge duplicate issues into a single issue",
|
||||||
|
Long: `Merge one or more source issues into a target issue.
|
||||||
|
|
||||||
|
This command:
|
||||||
|
1. Validates all issues exist and no self-merge
|
||||||
|
2. Closes source issues with reason 'Merged into bd-X'
|
||||||
|
3. Migrates all dependencies from sources to target
|
||||||
|
4. Updates text references in all issue descriptions/notes
|
||||||
|
|
||||||
|
Example:
|
||||||
|
bd merge bd-42 bd-43 --into bd-42
|
||||||
|
bd merge bd-10 bd-11 bd-12 --into bd-10 --dry-run`,
|
||||||
|
Args: cobra.MinimumNArgs(1),
|
||||||
|
Run: func(cmd *cobra.Command, args []string) {
|
||||||
|
targetID, _ := cmd.Flags().GetString("into")
|
||||||
|
if targetID == "" {
|
||||||
|
fmt.Fprintf(os.Stderr, "Error: --into flag is required\n")
|
||||||
|
os.Exit(1)
|
||||||
|
}
|
||||||
|
|
||||||
|
sourceIDs := args
|
||||||
|
dryRun, _ := cmd.Flags().GetBool("dry-run")
|
||||||
|
|
||||||
|
// Validate merge operation
|
||||||
|
if err := validateMerge(targetID, sourceIDs); err != nil {
|
||||||
|
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
|
||||||
|
os.Exit(1)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TODO: Add RPC support when daemon implements MergeIssues
|
||||||
|
if daemonClient != nil {
|
||||||
|
fmt.Fprintf(os.Stderr, "Error: merge command not yet supported in daemon mode (see bd-190)\n")
|
||||||
|
os.Exit(1)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Direct mode
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
if dryRun {
|
||||||
|
if !jsonOutput {
|
||||||
|
fmt.Println("Dry run - validation passed, no changes made")
|
||||||
|
fmt.Printf("Would merge: %s into %s\n", strings.Join(sourceIDs, ", "), targetID)
|
||||||
|
}
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
// Perform merge
|
||||||
|
if err := performMerge(ctx, targetID, sourceIDs); err != nil {
|
||||||
|
fmt.Fprintf(os.Stderr, "Error performing merge: %v\n", err)
|
||||||
|
os.Exit(1)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Schedule auto-flush
|
||||||
|
markDirtyAndScheduleFlush()
|
||||||
|
|
||||||
|
if jsonOutput {
|
||||||
|
result := map[string]interface{}{
|
||||||
|
"target_id": targetID,
|
||||||
|
"source_ids": sourceIDs,
|
||||||
|
"merged": len(sourceIDs),
|
||||||
|
}
|
||||||
|
outputJSON(result)
|
||||||
|
} else {
|
||||||
|
green := color.New(color.FgGreen).SprintFunc()
|
||||||
|
fmt.Printf("%s Merged %d issue(s) into %s\n", green("✓"), len(sourceIDs), targetID)
|
||||||
|
}
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
func init() {
|
||||||
|
mergeCmd.Flags().String("into", "", "Target issue ID to merge into (required)")
|
||||||
|
mergeCmd.Flags().Bool("dry-run", false, "Validate without making changes")
|
||||||
|
rootCmd.AddCommand(mergeCmd)
|
||||||
|
}
|
||||||
|
|
||||||
|
// validateMerge checks that merge operation is valid
|
||||||
|
func validateMerge(targetID string, sourceIDs []string) error {
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
// Check target exists
|
||||||
|
target, err := store.GetIssue(ctx, targetID)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("target issue not found: %s", targetID)
|
||||||
|
}
|
||||||
|
if target == nil {
|
||||||
|
return fmt.Errorf("target issue not found: %s", targetID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check all sources exist and validate no self-merge
|
||||||
|
for _, sourceID := range sourceIDs {
|
||||||
|
if sourceID == targetID {
|
||||||
|
return fmt.Errorf("cannot merge issue into itself: %s", sourceID)
|
||||||
|
}
|
||||||
|
|
||||||
|
source, err := store.GetIssue(ctx, sourceID)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("source issue not found: %s", sourceID)
|
||||||
|
}
|
||||||
|
if source == nil {
|
||||||
|
return fmt.Errorf("source issue not found: %s", sourceID)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// performMerge executes the merge operation
|
||||||
|
func performMerge(ctx context.Context, targetID string, sourceIDs []string) error {
|
||||||
|
// TODO: Implement actual merge logic in bd-190
|
||||||
|
// This is a placeholder for validation purposes
|
||||||
|
return fmt.Errorf("merge operation not yet implemented (see bd-190)")
|
||||||
|
}
|
||||||
182
cmd/bd/merge_test.go
Normal file
182
cmd/bd/merge_test.go
Normal file
@@ -0,0 +1,182 @@
|
|||||||
|
package main
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/steveyegge/beads/internal/storage/sqlite"
|
||||||
|
"github.com/steveyegge/beads/internal/types"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestValidateMerge(t *testing.T) {
|
||||||
|
tmpDir := t.TempDir()
|
||||||
|
dbFile := filepath.Join(tmpDir, ".beads", "issues.db")
|
||||||
|
if err := os.MkdirAll(filepath.Dir(dbFile), 0755); err != nil {
|
||||||
|
t.Fatalf("Failed to create test directory: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
testStore, err := sqlite.New(dbFile)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Failed to create test storage: %v", err)
|
||||||
|
}
|
||||||
|
defer testStore.Close()
|
||||||
|
|
||||||
|
store = testStore
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
// Create test issues
|
||||||
|
issue1 := &types.Issue{
|
||||||
|
ID: "bd-1",
|
||||||
|
Title: "Test issue 1",
|
||||||
|
Description: "Test",
|
||||||
|
Priority: 1,
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
}
|
||||||
|
issue2 := &types.Issue{
|
||||||
|
ID: "bd-2",
|
||||||
|
Title: "Test issue 2",
|
||||||
|
Description: "Test",
|
||||||
|
Priority: 1,
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
}
|
||||||
|
issue3 := &types.Issue{
|
||||||
|
ID: "bd-3",
|
||||||
|
Title: "Test issue 3",
|
||||||
|
Description: "Test",
|
||||||
|
Priority: 1,
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := testStore.CreateIssue(ctx, issue1, "test"); err != nil {
|
||||||
|
t.Fatalf("Failed to create issue1: %v", err)
|
||||||
|
}
|
||||||
|
if err := testStore.CreateIssue(ctx, issue2, "test"); err != nil {
|
||||||
|
t.Fatalf("Failed to create issue2: %v", err)
|
||||||
|
}
|
||||||
|
if err := testStore.CreateIssue(ctx, issue3, "test"); err != nil {
|
||||||
|
t.Fatalf("Failed to create issue3: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
tests := []struct {
|
||||||
|
name string
|
||||||
|
targetID string
|
||||||
|
sourceIDs []string
|
||||||
|
wantErr bool
|
||||||
|
errMsg string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "valid merge",
|
||||||
|
targetID: "bd-1",
|
||||||
|
sourceIDs: []string{"bd-2", "bd-3"},
|
||||||
|
wantErr: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "self-merge error",
|
||||||
|
targetID: "bd-1",
|
||||||
|
sourceIDs: []string{"bd-1"},
|
||||||
|
wantErr: true,
|
||||||
|
errMsg: "cannot merge issue into itself",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "self-merge in list",
|
||||||
|
targetID: "bd-1",
|
||||||
|
sourceIDs: []string{"bd-2", "bd-1"},
|
||||||
|
wantErr: true,
|
||||||
|
errMsg: "cannot merge issue into itself",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "nonexistent target",
|
||||||
|
targetID: "bd-999",
|
||||||
|
sourceIDs: []string{"bd-1"},
|
||||||
|
wantErr: true,
|
||||||
|
errMsg: "target issue not found",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "nonexistent source",
|
||||||
|
targetID: "bd-1",
|
||||||
|
sourceIDs: []string{"bd-999"},
|
||||||
|
wantErr: true,
|
||||||
|
errMsg: "source issue not found",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "multiple sources valid",
|
||||||
|
targetID: "bd-1",
|
||||||
|
sourceIDs: []string{"bd-2"},
|
||||||
|
wantErr: false,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range tests {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
err := validateMerge(tt.targetID, tt.sourceIDs)
|
||||||
|
if tt.wantErr {
|
||||||
|
if err == nil {
|
||||||
|
t.Errorf("validateMerge() expected error, got nil")
|
||||||
|
} else if tt.errMsg != "" && !contains(err.Error(), tt.errMsg) {
|
||||||
|
t.Errorf("validateMerge() error = %v, want error containing %v", err, tt.errMsg)
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
if err != nil {
|
||||||
|
t.Errorf("validateMerge() unexpected error: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestValidateMergeMultipleSelfReferences(t *testing.T) {
|
||||||
|
tmpDir := t.TempDir()
|
||||||
|
dbFile := filepath.Join(tmpDir, ".beads", "issues.db")
|
||||||
|
if err := os.MkdirAll(filepath.Dir(dbFile), 0755); err != nil {
|
||||||
|
t.Fatalf("Failed to create test directory: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
testStore, err := sqlite.New(dbFile)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Failed to create test storage: %v", err)
|
||||||
|
}
|
||||||
|
defer testStore.Close()
|
||||||
|
|
||||||
|
store = testStore
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
issue1 := &types.Issue{
|
||||||
|
ID: "bd-10",
|
||||||
|
Title: "Test issue 10",
|
||||||
|
Description: "Test",
|
||||||
|
Priority: 1,
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := testStore.CreateIssue(ctx, issue1, "test"); err != nil {
|
||||||
|
t.Fatalf("Failed to create issue: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Test merging multiple instances of same ID (should catch first one)
|
||||||
|
err = validateMerge("bd-10", []string{"bd-10", "bd-10"})
|
||||||
|
if err == nil {
|
||||||
|
t.Error("validateMerge() expected error for duplicate self-merge, got nil")
|
||||||
|
}
|
||||||
|
if !contains(err.Error(), "cannot merge issue into itself") {
|
||||||
|
t.Errorf("validateMerge() error = %v, want error containing 'cannot merge issue into itself'", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func contains(s, substr string) bool {
|
||||||
|
return len(s) >= len(substr) && (s == substr || len(s) > len(substr) && containsSubstring(s, substr))
|
||||||
|
}
|
||||||
|
|
||||||
|
func containsSubstring(s, substr string) bool {
|
||||||
|
for i := 0; i <= len(s)-len(substr); i++ {
|
||||||
|
if s[i:i+len(substr)] == substr {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return false
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user