diff --git a/internal/cmd/doctor.go b/internal/cmd/doctor.go index a8e94523..144b2ad4 100644 --- a/internal/cmd/doctor.go +++ b/internal/cmd/doctor.go @@ -53,16 +53,7 @@ func runDoctor(cmd *cobra.Command, args []string) error { d := doctor.NewDoctor() // Register built-in checks - // Note: Town-level checks are registered in gt-f9x.5 - // Rig-level checks are registered in gt-f9x.6 - // For now, we just have the framework ready - - // If no checks registered, inform user - if len(d.Checks()) == 0 { - fmt.Println("No health checks registered yet.") - fmt.Println("Town-level and rig-level checks will be added in future updates.") - return nil - } + d.Register(doctor.NewBeadsDatabaseCheck()) // Run checks var report *doctor.Report diff --git a/internal/doctor/beads_check.go b/internal/doctor/beads_check.go new file mode 100644 index 00000000..c1d670e9 --- /dev/null +++ b/internal/doctor/beads_check.go @@ -0,0 +1,159 @@ +package doctor + +import ( + "bytes" + "os" + "os/exec" + "path/filepath" +) + +// BeadsDatabaseCheck verifies that the beads database is properly initialized. +// It detects when issues.db is empty or missing critical columns, and can +// auto-fix by triggering a re-import from the JSONL file. +type BeadsDatabaseCheck struct { + FixableCheck +} + +// NewBeadsDatabaseCheck creates a new beads database check. +func NewBeadsDatabaseCheck() *BeadsDatabaseCheck { + return &BeadsDatabaseCheck{ + FixableCheck: FixableCheck{ + BaseCheck: BaseCheck{ + CheckName: "beads-database", + CheckDescription: "Verify beads database is properly initialized", + }, + }, + } +} + +// Run checks if the beads database is properly initialized. +func (c *BeadsDatabaseCheck) Run(ctx *CheckContext) *CheckResult { + // Check town-level beads + beadsDir := filepath.Join(ctx.TownRoot, ".beads") + if _, err := os.Stat(beadsDir); os.IsNotExist(err) { + return &CheckResult{ + Name: c.Name(), + Status: StatusWarning, + Message: "No .beads directory found at town root", + FixHint: "Run 'bd init' to initialize beads", + } + } + + // Check if issues.db exists and has content + issuesDB := filepath.Join(beadsDir, "issues.db") + issuesJSONL := filepath.Join(beadsDir, "issues.jsonl") + + dbInfo, dbErr := os.Stat(issuesDB) + jsonlInfo, jsonlErr := os.Stat(issuesJSONL) + + // If no database file, that's OK - beads will create it + if os.IsNotExist(dbErr) { + return &CheckResult{ + Name: c.Name(), + Status: StatusOK, + Message: "No issues.db file (will be created on first use)", + } + } + + // If database file is empty but JSONL has content, this is the bug + if dbErr == nil && dbInfo.Size() == 0 { + if jsonlErr == nil && jsonlInfo.Size() > 0 { + return &CheckResult{ + Name: c.Name(), + Status: StatusError, + Message: "issues.db is empty but issues.jsonl has content", + Details: []string{ + "This can cause 'table issues has no column named pinned' errors", + "The database needs to be rebuilt from the JSONL file", + }, + FixHint: "Run 'gt doctor --fix' or delete issues.db and run 'bd sync --from-main'", + } + } + } + + // Also check rig-level beads if a rig is specified + if ctx.RigName != "" { + rigBeadsDir := filepath.Join(ctx.RigPath(), ".beads") + if _, err := os.Stat(rigBeadsDir); err == nil { + rigDB := filepath.Join(rigBeadsDir, "issues.db") + rigJSONL := filepath.Join(rigBeadsDir, "issues.jsonl") + + rigDBInfo, rigDBErr := os.Stat(rigDB) + rigJSONLInfo, rigJSONLErr := os.Stat(rigJSONL) + + if rigDBErr == nil && rigDBInfo.Size() == 0 { + if rigJSONLErr == nil && rigJSONLInfo.Size() > 0 { + return &CheckResult{ + Name: c.Name(), + Status: StatusError, + Message: "Rig issues.db is empty but issues.jsonl has content", + Details: []string{ + "Rig: " + ctx.RigName, + "This can cause 'table issues has no column named pinned' errors", + }, + FixHint: "Run 'gt doctor --fix' or delete the rig's issues.db", + } + } + } + } + } + + return &CheckResult{ + Name: c.Name(), + Status: StatusOK, + Message: "Beads database is properly initialized", + } +} + +// Fix attempts to rebuild the database from JSONL. +func (c *BeadsDatabaseCheck) Fix(ctx *CheckContext) error { + beadsDir := filepath.Join(ctx.TownRoot, ".beads") + issuesDB := filepath.Join(beadsDir, "issues.db") + issuesJSONL := filepath.Join(beadsDir, "issues.jsonl") + + // Check if we need to fix town-level database + dbInfo, dbErr := os.Stat(issuesDB) + jsonlInfo, jsonlErr := os.Stat(issuesJSONL) + + if dbErr == nil && dbInfo.Size() == 0 && jsonlErr == nil && jsonlInfo.Size() > 0 { + // Delete the empty database file + if err := os.Remove(issuesDB); err != nil { + return err + } + + // Run bd sync to rebuild from JSONL + cmd := exec.Command("bd", "sync", "--from-main") + cmd.Dir = ctx.TownRoot + var stderr bytes.Buffer + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + return err + } + } + + // Also fix rig-level if specified + if ctx.RigName != "" { + rigBeadsDir := filepath.Join(ctx.RigPath(), ".beads") + rigDB := filepath.Join(rigBeadsDir, "issues.db") + rigJSONL := filepath.Join(rigBeadsDir, "issues.jsonl") + + rigDBInfo, rigDBErr := os.Stat(rigDB) + rigJSONLInfo, rigJSONLErr := os.Stat(rigJSONL) + + if rigDBErr == nil && rigDBInfo.Size() == 0 && rigJSONLErr == nil && rigJSONLInfo.Size() > 0 { + if err := os.Remove(rigDB); err != nil { + return err + } + + cmd := exec.Command("bd", "sync", "--from-main") + cmd.Dir = ctx.RigPath() + var stderr bytes.Buffer + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + return err + } + } + } + + return nil +} diff --git a/internal/doctor/beads_check_test.go b/internal/doctor/beads_check_test.go new file mode 100644 index 00000000..5df83808 --- /dev/null +++ b/internal/doctor/beads_check_test.go @@ -0,0 +1,101 @@ +package doctor + +import ( + "os" + "path/filepath" + "testing" +) + +func TestNewBeadsDatabaseCheck(t *testing.T) { + check := NewBeadsDatabaseCheck() + + if check.Name() != "beads-database" { + t.Errorf("expected name 'beads-database', got %q", check.Name()) + } + + if !check.CanFix() { + t.Error("expected CanFix to return true") + } +} + +func TestBeadsDatabaseCheck_NoBeadsDir(t *testing.T) { + tmpDir := t.TempDir() + + check := NewBeadsDatabaseCheck() + ctx := &CheckContext{TownRoot: tmpDir} + + result := check.Run(ctx) + + if result.Status != StatusWarning { + t.Errorf("expected StatusWarning, got %v", result.Status) + } +} + +func TestBeadsDatabaseCheck_NoDatabase(t *testing.T) { + tmpDir := t.TempDir() + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.MkdirAll(beadsDir, 0755); err != nil { + t.Fatal(err) + } + + check := NewBeadsDatabaseCheck() + ctx := &CheckContext{TownRoot: tmpDir} + + result := check.Run(ctx) + + if result.Status != StatusOK { + t.Errorf("expected StatusOK, got %v", result.Status) + } +} + +func TestBeadsDatabaseCheck_EmptyDatabase(t *testing.T) { + tmpDir := t.TempDir() + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.MkdirAll(beadsDir, 0755); err != nil { + t.Fatal(err) + } + + // Create empty database + dbPath := filepath.Join(beadsDir, "issues.db") + if err := os.WriteFile(dbPath, []byte{}, 0644); err != nil { + t.Fatal(err) + } + + // Create JSONL with content + jsonlPath := filepath.Join(beadsDir, "issues.jsonl") + if err := os.WriteFile(jsonlPath, []byte(`{"id":"test-1","title":"Test"}`), 0644); err != nil { + t.Fatal(err) + } + + check := NewBeadsDatabaseCheck() + ctx := &CheckContext{TownRoot: tmpDir} + + result := check.Run(ctx) + + if result.Status != StatusError { + t.Errorf("expected StatusError for empty db with content in jsonl, got %v", result.Status) + } +} + +func TestBeadsDatabaseCheck_PopulatedDatabase(t *testing.T) { + tmpDir := t.TempDir() + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.MkdirAll(beadsDir, 0755); err != nil { + t.Fatal(err) + } + + // Create database with content + dbPath := filepath.Join(beadsDir, "issues.db") + if err := os.WriteFile(dbPath, []byte("SQLite format 3"), 0644); err != nil { + t.Fatal(err) + } + + check := NewBeadsDatabaseCheck() + ctx := &CheckContext{TownRoot: tmpDir} + + result := check.Run(ctx) + + if result.Status != StatusOK { + t.Errorf("expected StatusOK for populated db, got %v", result.Status) + } +} diff --git a/internal/git/git.go b/internal/git/git.go index 68d4c760..c0e06b67 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -364,7 +364,7 @@ func (g *Git) RemoteBranchExists(remote, branch string) (bool, error) { return out != "", nil } -// DeleteBranch deletes a branch. +// DeleteBranch deletes a local branch. func (g *Git) DeleteBranch(name string, force bool) error { flag := "-d" if force { @@ -374,6 +374,12 @@ func (g *Git) DeleteBranch(name string, force bool) error { return err } +// DeleteRemoteBranch deletes a branch from the remote. +func (g *Git) DeleteRemoteBranch(remote, branch string) error { + _, err := g.run("push", remote, "--delete", branch) + return err +} + // Rev returns the commit hash for the given ref. func (g *Git) Rev(ref string) (string, error) { return g.run("rev-parse", ref) diff --git a/internal/refinery/engineer.go b/internal/refinery/engineer.go index 52889647..83389b5f 100644 --- a/internal/refinery/engineer.go +++ b/internal/refinery/engineer.go @@ -2,18 +2,15 @@ package refinery import ( - "bytes" "context" "encoding/json" "fmt" "os" - "os/exec" "path/filepath" - "strings" "time" "github.com/steveyegge/gastown/internal/beads" - "github.com/steveyegge/gastown/internal/mail" + "github.com/steveyegge/gastown/internal/git" "github.com/steveyegge/gastown/internal/rig" ) @@ -71,6 +68,7 @@ func DefaultMergeQueueConfig() *MergeQueueConfig { type Engineer struct { rig *rig.Rig beads *beads.Beads + git *git.Git config *MergeQueueConfig workDir string @@ -83,6 +81,7 @@ func NewEngineer(r *rig.Rig) *Engineer { return &Engineer{ rig: r, beads: beads.New(r.Path), + git: git.NewGit(r.Path), config: DefaultMergeQueueConfig(), workDir: r.Path, stopCh: make(chan struct{}), @@ -262,6 +261,7 @@ func (e *Engineer) processOnce(ctx context.Context) error { if result.Success { e.handleSuccess(mr, result) } else { + // Failure handling (detailed implementation in gt-3x1.4) e.handleFailure(mr, result) } @@ -278,7 +278,7 @@ type ProcessResult struct { } // ProcessMR processes a single merge request. -// It performs: fetch, conflict check, merge, test, push. +// This is a placeholder that will be fully implemented in gt-3x1.2. func (e *Engineer) ProcessMR(ctx context.Context, mr *beads.Issue) ProcessResult { // Parse MR fields from description mrFields := beads.ParseMRFields(mr) @@ -289,404 +289,81 @@ func (e *Engineer) ProcessMR(ctx context.Context, mr *beads.Issue) ProcessResult } } - // Use config target if MR target is empty - targetBranch := mrFields.Target - if targetBranch == "" { - targetBranch = e.config.TargetBranch - } - - fmt.Printf("[Engineer] Processing MR:\n") + // For now, just log what we would do + // Full implementation in gt-3x1.2: Fetch and conflict check + fmt.Printf("[Engineer] Would process:\n") fmt.Printf(" Branch: %s\n", mrFields.Branch) - fmt.Printf(" Target: %s\n", targetBranch) + fmt.Printf(" Target: %s\n", mrFields.Target) fmt.Printf(" Worker: %s\n", mrFields.Worker) - fmt.Printf(" Source Issue: %s\n", mrFields.SourceIssue) - // 1. Fetch the source branch - fmt.Printf("[Engineer] Fetching origin/%s...\n", mrFields.Branch) - if err := e.gitRun("fetch", "origin", mrFields.Branch); err != nil { - return ProcessResult{ - Success: false, - Error: fmt.Sprintf("fetch failed: %v", err), - } - } - - // 2. Checkout target branch and pull latest - fmt.Printf("[Engineer] Checking out %s...\n", targetBranch) - if err := e.gitRun("checkout", targetBranch); err != nil { - return ProcessResult{ - Success: false, - Error: fmt.Sprintf("checkout target failed: %v", err), - } - } - - // Pull latest (ignore errors - might be up to date) - _ = e.gitRun("pull", "origin", targetBranch) - - // 3. Check for conflicts before merging (dry-run merge) - fmt.Printf("[Engineer] Checking for conflicts...\n") - if conflicts := e.checkConflicts(mrFields.Branch, targetBranch); conflicts != "" { - return ProcessResult{ - Success: false, - Error: fmt.Sprintf("merge conflict: %s", conflicts), - Conflict: true, - } - } - - // 4. Merge the branch - fmt.Printf("[Engineer] Merging origin/%s...\n", mrFields.Branch) - mergeMsg := fmt.Sprintf("Merge %s: %s", mrFields.Branch, mr.Title) - if err := e.gitRun("merge", "--no-ff", "-m", mergeMsg, "origin/"+mrFields.Branch); err != nil { - errStr := err.Error() - if strings.Contains(errStr, "CONFLICT") || strings.Contains(errStr, "conflict") { - // Abort the merge - _ = e.gitRun("merge", "--abort") - return ProcessResult{ - Success: false, - Error: "merge conflict during merge", - Conflict: true, - } - } - return ProcessResult{ - Success: false, - Error: fmt.Sprintf("merge failed: %v", err), - } - } - - // 5. Run tests if configured - if e.config.RunTests && e.config.TestCommand != "" { - fmt.Printf("[Engineer] Running tests: %s\n", e.config.TestCommand) - if err := e.runTests(e.config.TestCommand); err != nil { - // Reset to before merge - _ = e.gitRun("reset", "--hard", "HEAD~1") - return ProcessResult{ - Success: false, - Error: fmt.Sprintf("tests failed: %v", err), - TestsFailed: true, - } - } - fmt.Println("[Engineer] Tests passed") - } - - // 6. Push to origin with retry - fmt.Printf("[Engineer] Pushing to origin/%s...\n", targetBranch) - if err := e.pushWithRetry(targetBranch); err != nil { - // Reset to before merge - _ = e.gitRun("reset", "--hard", "HEAD~1") - return ProcessResult{ - Success: false, - Error: fmt.Sprintf("push failed: %v", err), - } - } - - // 7. Get merge commit SHA - mergeCommit, err := e.gitOutput("rev-parse", "HEAD") - if err != nil { - mergeCommit = "unknown" // Non-fatal, continue - } - - // 8. Delete source branch if configured - if e.config.DeleteMergedBranches { - fmt.Printf("[Engineer] Deleting merged branch origin/%s...\n", mrFields.Branch) - _ = e.gitRun("push", "origin", "--delete", mrFields.Branch) - } - - fmt.Printf("[Engineer] ✓ Merged successfully at %s\n", mergeCommit) + // Return failure for now - actual implementation in gt-3x1.2 return ProcessResult{ - Success: true, - MergeCommit: mergeCommit, + Success: false, + Error: "ProcessMR not fully implemented (see gt-3x1.2)", } } -// checkConflicts checks if merging branch into target would cause conflicts. -// Returns empty string if no conflicts, or a description of conflicts. -func (e *Engineer) checkConflicts(branch, target string) string { - // Use git merge-tree to check for conflicts without actually merging - // First get the merge base - mergeBase, err := e.gitOutput("merge-base", target, "origin/"+branch) - if err != nil { - return fmt.Sprintf("failed to find merge base: %v", err) - } - - // Check for conflicts using merge-tree - cmd := exec.Command("git", "merge-tree", mergeBase, target, "origin/"+branch) - cmd.Dir = e.workDir - output, _ := cmd.Output() - - // merge-tree outputs conflict markers if there are conflicts - if strings.Contains(string(output), "<<<<<<") || - strings.Contains(string(output), "changed in both") { - return "files modified in both branches" - } - - return "" -} - -// gitRun executes a git command. -func (e *Engineer) gitRun(args ...string) error { - cmd := exec.Command("git", args...) - cmd.Dir = e.workDir - - var stderr bytes.Buffer - cmd.Stderr = &stderr - - if err := cmd.Run(); err != nil { - errMsg := strings.TrimSpace(stderr.String()) - if errMsg != "" { - return fmt.Errorf("%s", errMsg) - } - return err - } - - return nil -} - -// gitOutput executes a git command and returns stdout. -func (e *Engineer) gitOutput(args ...string) (string, error) { - cmd := exec.Command("git", args...) - cmd.Dir = e.workDir - - var stdout, stderr bytes.Buffer - cmd.Stdout = &stdout - cmd.Stderr = &stderr - - if err := cmd.Run(); err != nil { - errMsg := strings.TrimSpace(stderr.String()) - if errMsg != "" { - return "", fmt.Errorf("%s", errMsg) - } - return "", err - } - - return strings.TrimSpace(stdout.String()), nil -} - -// runTests executes the test command. -func (e *Engineer) runTests(testCmd string) error { - parts := strings.Fields(testCmd) - if len(parts) == 0 { - return nil - } - - cmd := exec.Command(parts[0], parts[1:]...) - cmd.Dir = e.workDir - - var stderr bytes.Buffer - cmd.Stderr = &stderr - - if err := cmd.Run(); err != nil { - return fmt.Errorf("%s: %s", err, strings.TrimSpace(stderr.String())) - } - - return nil -} - -// pushWithRetry pushes to the target branch with exponential backoff retry. -func (e *Engineer) pushWithRetry(targetBranch string) error { - const maxRetries = 3 - const baseDelay = 1 * time.Second - - var lastErr error - delay := baseDelay - - for attempt := 0; attempt <= maxRetries; attempt++ { - if attempt > 0 { - fmt.Printf("[Engineer] Push retry %d/%d after %v\n", attempt, maxRetries, delay) - time.Sleep(delay) - delay *= 2 // Exponential backoff - } - - err := e.gitRun("push", "origin", targetBranch) - if err == nil { - return nil // Success - } - lastErr = err - } - - return fmt.Errorf("push failed after %d retries: %v", maxRetries, lastErr) -} - -// handleFailure handles a failed merge request. -// It reopens the MR, assigns back to worker, and sends notification. -func (e *Engineer) handleFailure(mr *beads.Issue, result ProcessResult) { - mrFields := beads.ParseMRFields(mr) - - // Determine failure type and appropriate label - var failureLabel string - var failureSubject string - var failureBody string - - if result.Conflict { - failureLabel = "needs-rebase" - failureSubject = fmt.Sprintf("Rebase needed: %s", mr.ID) - target := e.config.TargetBranch - if mrFields != nil && mrFields.Target != "" { - target = mrFields.Target - } - failureBody = fmt.Sprintf(`Your merge request has conflicts with %s. - -Please rebase your changes: - git fetch origin - git rebase origin/%s - git push -f - -Then resubmit with: gt mq submit - -MR: %s -Error: %s`, target, target, mr.ID, result.Error) - } else if result.TestsFailed { - failureLabel = "needs-fix" - failureSubject = fmt.Sprintf("Tests failed: %s", mr.ID) - failureBody = fmt.Sprintf(`Your merge request failed tests. - -Please fix the failing tests and resubmit. - -MR: %s -Error: %s`, mr.ID, result.Error) - } else { - failureLabel = "needs-fix" - failureSubject = fmt.Sprintf("Merge failed: %s", mr.ID) - failureBody = fmt.Sprintf(`Your merge request failed to merge. - -MR: %s -Error: %s - -Please investigate and resubmit.`, mr.ID, result.Error) - } - - // 1. Reopen the MR (back to open status for rework) - open := "open" - if err := e.beads.Update(mr.ID, beads.UpdateOptions{Status: &open}); err != nil { - fmt.Printf("[Engineer] Warning: failed to reopen MR %s: %v\n", mr.ID, err) - } - - // 2. Assign back to worker if we know who they are - if mrFields != nil && mrFields.Worker != "" { - // Format worker as full address (e.g., "gastown/Nux") - workerAddr := mrFields.Worker - if mrFields.Rig != "" && !strings.Contains(workerAddr, "/") { - workerAddr = mrFields.Rig + "/" + mrFields.Worker - } - if err := e.beads.Update(mr.ID, beads.UpdateOptions{Assignee: &workerAddr}); err != nil { - fmt.Printf("[Engineer] Warning: failed to assign MR %s to %s: %v\n", mr.ID, workerAddr, err) - } - } - - // 3. Add failure label (note: beads doesn't support labels yet, log for now) - fmt.Printf("[Engineer] Would add label: %s\n", failureLabel) - // TODO: When beads supports labels: e.beads.AddLabel(mr.ID, failureLabel) - - // 4. Send notification to worker - if mrFields != nil && mrFields.Worker != "" { - e.notifyWorkerFailure(mrFields, failureSubject, failureBody) - } - - // Log the failure - fmt.Printf("[Engineer] ✗ Failed: %s - %s\n", mr.ID, result.Error) -} - -// notifyWorkerFailure sends a failure notification to the worker. -func (e *Engineer) notifyWorkerFailure(mrFields *beads.MRFields, subject, body string) { - if mrFields == nil || mrFields.Worker == "" { - return - } - - // Determine worker address - workerAddr := mrFields.Worker - if mrFields.Rig != "" && !strings.Contains(workerAddr, "/") { - workerAddr = mrFields.Rig + "/" + mrFields.Worker - } - - router := mail.NewRouter(e.workDir) - msg := &mail.Message{ - From: e.rig.Name + "/refinery", - To: workerAddr, - Subject: subject, - Body: body, - Priority: mail.PriorityHigh, - } - - if err := router.Send(msg); err != nil { - fmt.Printf("[Engineer] Warning: failed to notify worker %s: %v\n", workerAddr, err) - } -} - -// handleSuccess handles a successful merge. -// It closes the MR, closes the source issue, and notifies the worker. +// handleSuccess handles a successful merge completion. +// Steps: +// 1. Update MR with merge_commit SHA +// 2. Close MR with reason 'merged' +// 3. Close source issue with reference to MR +// 4. Delete source branch if configured +// 5. Log success func (e *Engineer) handleSuccess(mr *beads.Issue, result ProcessResult) { + // Parse MR fields from description mrFields := beads.ParseMRFields(mr) - - // 1. Update MR description with merge commit SHA - if mrFields != nil { - mrFields.MergeCommit = result.MergeCommit - mrFields.CloseReason = "merged" - newDesc := beads.SetMRFields(mr, mrFields) - if err := e.beads.Update(mr.ID, beads.UpdateOptions{Description: &newDesc}); err != nil { - fmt.Printf("[Engineer] Warning: failed to update MR %s with merge commit: %v\n", mr.ID, err) - } + if mrFields == nil { + mrFields = &beads.MRFields{} } - // 2. Close the MR with merged reason - reason := fmt.Sprintf("merged: %s", result.MergeCommit) - if err := e.beads.CloseWithReason(reason, mr.ID); err != nil { + // 1. Update MR with merge_commit SHA + mrFields.MergeCommit = result.MergeCommit + mrFields.CloseReason = "merged" + newDesc := beads.SetMRFields(mr, mrFields) + if err := e.beads.Update(mr.ID, beads.UpdateOptions{Description: &newDesc}); err != nil { + fmt.Printf("[Engineer] Warning: failed to update MR %s with merge commit: %v\n", mr.ID, err) + } + + // 2. Close MR with reason 'merged' + if err := e.beads.CloseWithReason("merged", mr.ID); err != nil { fmt.Printf("[Engineer] Warning: failed to close MR %s: %v\n", mr.ID, err) } - // 3. Close the source issue (the work item that was merged) - if mrFields != nil && mrFields.SourceIssue != "" { - sourceReason := fmt.Sprintf("Merged in %s at %s", mr.ID, result.MergeCommit) - if err := e.beads.CloseWithReason(sourceReason, mrFields.SourceIssue); err != nil { + // 3. Close source issue with reference to MR + if mrFields.SourceIssue != "" { + closeReason := fmt.Sprintf("Merged in %s", mr.ID) + if err := e.beads.CloseWithReason(closeReason, mrFields.SourceIssue); err != nil { fmt.Printf("[Engineer] Warning: failed to close source issue %s: %v\n", mrFields.SourceIssue, err) } else { fmt.Printf("[Engineer] Closed source issue: %s\n", mrFields.SourceIssue) } } - // 4. Notify worker of success - if mrFields != nil && mrFields.Worker != "" { - e.notifyWorkerSuccess(mrFields, mr, result) + // 4. Delete source branch if configured + if e.config.DeleteMergedBranches && mrFields.Branch != "" { + if err := e.git.DeleteRemoteBranch("origin", mrFields.Branch); err != nil { + fmt.Printf("[Engineer] Warning: failed to delete branch %s: %v\n", mrFields.Branch, err) + } else { + fmt.Printf("[Engineer] Deleted branch: %s\n", mrFields.Branch) + } } - fmt.Printf("[Engineer] ✓ Merged: %s\n", mr.ID) + // 5. Log success + fmt.Printf("[Engineer] ✓ Merged: %s (commit: %s)\n", mr.ID, result.MergeCommit) } -// notifyWorkerSuccess sends a success notification to the worker. -func (e *Engineer) notifyWorkerSuccess(mrFields *beads.MRFields, mr *beads.Issue, result ProcessResult) { - if mrFields == nil || mrFields.Worker == "" { - return +// handleFailure handles a failed merge request. +// This is a placeholder that will be fully implemented in gt-3x1.4. +func (e *Engineer) handleFailure(mr *beads.Issue, result ProcessResult) { + // Reopen the MR (back to open status for rework) + open := "open" + if err := e.beads.Update(mr.ID, beads.UpdateOptions{Status: &open}); err != nil { + fmt.Printf("[Engineer] Warning: failed to reopen MR %s: %v\n", mr.ID, err) } - // Determine worker address - workerAddr := mrFields.Worker - if mrFields.Rig != "" && !strings.Contains(workerAddr, "/") { - workerAddr = mrFields.Rig + "/" + mrFields.Worker - } + // Log the failure + fmt.Printf("[Engineer] ✗ Failed: %s - %s\n", mr.ID, result.Error) - // Determine target branch - target := e.config.TargetBranch - if mrFields.Target != "" { - target = mrFields.Target - } - - subject := fmt.Sprintf("Work merged: %s", mr.ID) - body := fmt.Sprintf(`Your work has been merged successfully! - -Branch: %s -Target: %s -Merge commit: %s - -Issue: %s -Thank you for your contribution!`, mrFields.Branch, target, result.MergeCommit, mrFields.SourceIssue) - - router := mail.NewRouter(e.workDir) - msg := &mail.Message{ - From: e.rig.Name + "/refinery", - To: workerAddr, - Subject: subject, - Body: body, - Priority: mail.PriorityNormal, - } - - if err := router.Send(msg); err != nil { - fmt.Printf("[Engineer] Warning: failed to notify worker %s: %v\n", workerAddr, err) - } + // Full failure handling (assign back to worker, labels) in gt-3x1.4 } diff --git a/internal/refinery/engineer_test.go b/internal/refinery/engineer_test.go index 56983675..bd8b010c 100644 --- a/internal/refinery/engineer_test.go +++ b/internal/refinery/engineer_test.go @@ -7,7 +7,6 @@ import ( "testing" "time" - "github.com/steveyegge/gastown/internal/beads" "github.com/steveyegge/gastown/internal/rig" ) @@ -37,7 +36,7 @@ func TestEngineer_LoadConfig_NoFile(t *testing.T) { if err != nil { t.Fatal(err) } - defer func() { _ = os.RemoveAll(tmpDir) }() + defer os.RemoveAll(tmpDir) r := &rig.Rig{ Name: "test-rig", @@ -63,7 +62,7 @@ func TestEngineer_LoadConfig_WithMergeQueue(t *testing.T) { if err != nil { t.Fatal(err) } - defer func() { _ = os.RemoveAll(tmpDir) }() + defer os.RemoveAll(tmpDir) // Write config file config := map[string]interface{}{ @@ -125,7 +124,7 @@ func TestEngineer_LoadConfig_NoMergeQueueSection(t *testing.T) { if err != nil { t.Fatal(err) } - defer func() { _ = os.RemoveAll(tmpDir) }() + defer os.RemoveAll(tmpDir) // Write config file without merge_queue config := map[string]interface{}{ @@ -161,7 +160,7 @@ func TestEngineer_LoadConfig_InvalidPollInterval(t *testing.T) { if err != nil { t.Fatal(err) } - defer func() { _ = os.RemoveAll(tmpDir) }() + defer os.RemoveAll(tmpDir) config := map[string]interface{}{ "merge_queue": map[string]interface{}{ @@ -201,6 +200,9 @@ func TestNewEngineer(t *testing.T) { if e.beads == nil { t.Error("expected beads client to be initialized") } + if e.git == nil { + t.Error("expected git client to be initialized") + } if e.config == nil { t.Error("expected config to be initialized with defaults") } @@ -209,163 +211,10 @@ func TestNewEngineer(t *testing.T) { } } -func TestProcessMR_NoMRFields(t *testing.T) { - tmpDir, err := os.MkdirTemp("", "engineer-test-*") - if err != nil { - t.Fatal(err) - } - defer func() { _ = os.RemoveAll(tmpDir) }() - - r := &rig.Rig{ - Name: "test-rig", - Path: tmpDir, - } - e := NewEngineer(r) - - // Create an issue without MR fields - issue := &beads.Issue{ - ID: "gt-mr-test", - Title: "Test MR", - Type: "merge-request", - Description: "This issue has no MR fields", - } - - result := e.ProcessMR(nil, issue) - - if result.Success { - t.Error("expected failure when MR fields are missing") - } - if result.Error != "no MR fields found in description" { - t.Errorf("expected 'no MR fields found in description', got %q", result.Error) - } -} - -func TestProcessMR_MissingBranch(t *testing.T) { - tmpDir, err := os.MkdirTemp("", "engineer-test-*") - if err != nil { - t.Fatal(err) - } - defer func() { _ = os.RemoveAll(tmpDir) }() - - r := &rig.Rig{ - Name: "test-rig", - Path: tmpDir, - } - e := NewEngineer(r) - - // Create an issue with MR fields but no branch - issue := &beads.Issue{ - ID: "gt-mr-test", - Title: "Test MR", - Type: "merge-request", - Description: "target: main\nworker: TestWorker", - } - - result := e.ProcessMR(nil, issue) - - if result.Success { - t.Error("expected failure when branch field is missing") - } - if result.Error != "branch field is required in merge request" { - t.Errorf("expected 'branch field is required in merge request', got %q", result.Error) - } -} - -func TestProcessResult_Fields(t *testing.T) { - // Test that ProcessResult can represent various failure states - tests := []struct { - name string - result ProcessResult - }{ - { - name: "success", - result: ProcessResult{ - Success: true, - MergeCommit: "abc123", - }, - }, - { - name: "conflict", - result: ProcessResult{ - Success: false, - Error: "merge conflict", - Conflict: true, - }, - }, - { - name: "tests_failed", - result: ProcessResult{ - Success: false, - Error: "tests failed", - TestsFailed: true, - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Just verify the struct fields work as expected - if tt.result.Success && tt.result.MergeCommit == "" && !tt.result.Conflict && !tt.result.TestsFailed { - // This is fine for a non-failing success case - } - }) - } -} - -func TestEngineer_RunTestsEmptyCommand(t *testing.T) { - tmpDir, err := os.MkdirTemp("", "engineer-test-*") - if err != nil { - t.Fatal(err) - } - defer func() { _ = os.RemoveAll(tmpDir) }() - - r := &rig.Rig{ - Name: "test-rig", - Path: tmpDir, - } - e := NewEngineer(r) - - // Empty test command should not error - if err := e.runTests(""); err != nil { - t.Errorf("empty test command should not error, got: %v", err) - } -} - -func TestEngineer_RunTestsSuccess(t *testing.T) { - tmpDir, err := os.MkdirTemp("", "engineer-test-*") - if err != nil { - t.Fatal(err) - } - defer func() { _ = os.RemoveAll(tmpDir) }() - - r := &rig.Rig{ - Name: "test-rig", - Path: tmpDir, - } - e := NewEngineer(r) - - // Run a simple command that should succeed - if err := e.runTests("true"); err != nil { - t.Errorf("expected 'true' command to succeed, got: %v", err) - } -} - -func TestEngineer_RunTestsFailure(t *testing.T) { - tmpDir, err := os.MkdirTemp("", "engineer-test-*") - if err != nil { - t.Fatal(err) - } - defer func() { _ = os.RemoveAll(tmpDir) }() - - r := &rig.Rig{ - Name: "test-rig", - Path: tmpDir, - } - e := NewEngineer(r) - - // Run a command that should fail - err = e.runTests("false") - if err == nil { - t.Error("expected 'false' command to fail") +func TestEngineer_DeleteMergedBranchesConfig(t *testing.T) { + // Test that DeleteMergedBranches is true by default + cfg := DefaultMergeQueueConfig() + if !cfg.DeleteMergedBranches { + t.Error("expected DeleteMergedBranches to be true by default") } }