From 7c7a077d764bb8f75aa88ed101de0e1c307810c4 Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Sun, 21 Dec 2025 22:18:10 -0800 Subject: [PATCH] refactor(refinery): use io.Writer instead of fmt.Print for output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add output field (io.Writer) to Manager and Engineer structs with SetOutput() methods to enable testability and output redirection. Replace all 30+ fmt.Printf/Println calls with fmt.Fprintf/Fprintln using the configurable output writer, defaulting to os.Stdout. This enables: - Testing output without capturing stdout - Redirecting output in different contexts - Following cobra best practices Closes: gt-cvfg 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- internal/refinery/engineer.go | 47 +++++++++++++++++++++-------------- internal/refinery/manager.go | 29 +++++++++++++-------- 2 files changed, 47 insertions(+), 29 deletions(-) diff --git a/internal/refinery/engineer.go b/internal/refinery/engineer.go index 83389b5f..5fccf87a 100644 --- a/internal/refinery/engineer.go +++ b/internal/refinery/engineer.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "fmt" + "io" "os" "path/filepath" "time" @@ -71,6 +72,7 @@ type Engineer struct { git *git.Git config *MergeQueueConfig workDir string + output io.Writer // Output destination for user-facing messages // stopCh is used for graceful shutdown stopCh chan struct{} @@ -84,10 +86,17 @@ func NewEngineer(r *rig.Rig) *Engineer { git: git.NewGit(r.Path), config: DefaultMergeQueueConfig(), workDir: r.Path, + output: os.Stdout, stopCh: make(chan struct{}), } } +// SetOutput sets the output writer for user-facing messages. +// This is useful for testing or redirecting output. +func (e *Engineer) SetOutput(w io.Writer) { + e.output = w +} + // LoadConfig loads merge queue configuration from the rig's config.json. func (e *Engineer) LoadConfig() error { configPath := filepath.Join(e.rig.Path, "config.json") @@ -187,7 +196,7 @@ func (e *Engineer) Run(ctx context.Context) error { return fmt.Errorf("merge queue is disabled in configuration") } - fmt.Printf("[Engineer] Starting for rig %s (poll_interval=%s)\n", + fmt.Fprintf(e.output, "[Engineer] Starting for rig %s (poll_interval=%s)\n", e.rig.Name, e.config.PollInterval) ticker := time.NewTicker(e.config.PollInterval) @@ -195,20 +204,20 @@ func (e *Engineer) Run(ctx context.Context) error { // Run one iteration immediately, then on ticker if err := e.processOnce(ctx); err != nil { - fmt.Printf("[Engineer] Error: %v\n", err) + fmt.Fprintf(e.output, "[Engineer] Error: %v\n", err) } for { select { case <-ctx.Done(): - fmt.Println("[Engineer] Shutting down (context cancelled)") + fmt.Fprintln(e.output, "[Engineer] Shutting down (context cancelled)") return nil case <-e.stopCh: - fmt.Println("[Engineer] Shutting down (stop signal)") + fmt.Fprintln(e.output, "[Engineer] Shutting down (stop signal)") return nil case <-ticker.C: if err := e.processOnce(ctx); err != nil { - fmt.Printf("[Engineer] Error: %v\n", err) + fmt.Fprintf(e.output, "[Engineer] Error: %v\n", err) } } } @@ -246,7 +255,7 @@ func (e *Engineer) processOnce(ctx context.Context) error { // bd ready already returns sorted by priority then age, so first is best mr := readyMRs[0] - fmt.Printf("[Engineer] Processing: %s (%s)\n", mr.ID, mr.Title) + fmt.Fprintf(e.output, "[Engineer] Processing: %s (%s)\n", mr.ID, mr.Title) // 4. Claim: bd update --status=in_progress inProgress := "in_progress" @@ -291,10 +300,10 @@ func (e *Engineer) ProcessMR(ctx context.Context, mr *beads.Issue) ProcessResult // 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", mrFields.Target) - fmt.Printf(" Worker: %s\n", mrFields.Worker) + fmt.Fprintln(e.output, "[Engineer] Would process:") + fmt.Fprintf(e.output, " Branch: %s\n", mrFields.Branch) + fmt.Fprintf(e.output, " Target: %s\n", mrFields.Target) + fmt.Fprintf(e.output, " Worker: %s\n", mrFields.Worker) // Return failure for now - actual implementation in gt-3x1.2 return ProcessResult{ @@ -322,35 +331,35 @@ func (e *Engineer) handleSuccess(mr *beads.Issue, result ProcessResult) { 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) + fmt.Fprintf(e.output, "[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) + fmt.Fprintf(e.output, "[Engineer] Warning: failed to close MR %s: %v\n", mr.ID, err) } // 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) + fmt.Fprintf(e.output, "[Engineer] Warning: failed to close source issue %s: %v\n", mrFields.SourceIssue, err) } else { - fmt.Printf("[Engineer] Closed source issue: %s\n", mrFields.SourceIssue) + fmt.Fprintf(e.output, "[Engineer] Closed source issue: %s\n", mrFields.SourceIssue) } } // 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) + fmt.Fprintf(e.output, "[Engineer] Warning: failed to delete branch %s: %v\n", mrFields.Branch, err) } else { - fmt.Printf("[Engineer] Deleted branch: %s\n", mrFields.Branch) + fmt.Fprintf(e.output, "[Engineer] Deleted branch: %s\n", mrFields.Branch) } } // 5. Log success - fmt.Printf("[Engineer] ✓ Merged: %s (commit: %s)\n", mr.ID, result.MergeCommit) + fmt.Fprintf(e.output, "[Engineer] ✓ Merged: %s (commit: %s)\n", mr.ID, result.MergeCommit) } // handleFailure handles a failed merge request. @@ -359,11 +368,11 @@ 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) + fmt.Fprintf(e.output, "[Engineer] Warning: failed to reopen MR %s: %v\n", mr.ID, err) } // Log the failure - fmt.Printf("[Engineer] ✗ Failed: %s - %s\n", mr.ID, result.Error) + fmt.Fprintf(e.output, "[Engineer] ✗ Failed: %s - %s\n", mr.ID, result.Error) // Full failure handling (assign back to worker, labels) in gt-3x1.4 } diff --git a/internal/refinery/manager.go b/internal/refinery/manager.go index 5d6c287b..ee4a4aa1 100644 --- a/internal/refinery/manager.go +++ b/internal/refinery/manager.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "os" "os/exec" "path/filepath" @@ -28,6 +29,7 @@ var ( type Manager struct { rig *rig.Rig workDir string + output io.Writer // Output destination for user-facing messages } // NewManager creates a new refinery manager for a rig. @@ -35,9 +37,16 @@ func NewManager(r *rig.Rig) *Manager { return &Manager{ rig: r, workDir: r.Path, + output: os.Stdout, } } +// SetOutput sets the output writer for user-facing messages. +// This is useful for testing or redirecting output. +func (m *Manager) SetOutput(w io.Writer) { + m.output = w +} + // stateFile returns the path to the refinery state file. func (m *Manager) stateFile() string { return filepath.Join(m.rig.Path, ".gastown", "refinery.json") @@ -219,7 +228,7 @@ func (m *Manager) Start(foreground bool) error { // Prime the agent after Claude starts to load refinery context if err := t.SendKeysDelayed(sessionID, "gt prime", 2000); err != nil { // Warning only - don't fail startup - fmt.Printf("Warning: could not send prime command: %v\n", err) + fmt.Fprintf(m.output, "Warning: could not send prime command: %v\n", err) } return nil @@ -357,8 +366,8 @@ func (m *Manager) branchToMR(branch string) *MergeRequest { // run is the main processing loop (for foreground mode). func (m *Manager) run(ref *Refinery) error { - fmt.Println("Refinery running...") - fmt.Println("Press Ctrl+C to stop") + fmt.Fprintln(m.output, "Refinery running...") + fmt.Fprintln(m.output, "Press Ctrl+C to stop") ticker := time.NewTicker(10 * time.Second) defer ticker.Stop() @@ -366,7 +375,7 @@ func (m *Manager) run(ref *Refinery) error { for range ticker.C { // Process queue if err := m.ProcessQueue(); err != nil { - fmt.Printf("Queue processing error: %v\n", err) + fmt.Fprintf(m.output, "Queue processing error: %v\n", err) } } return nil @@ -384,13 +393,13 @@ func (m *Manager) ProcessQueue() error { continue } - fmt.Printf("Processing: %s (%s)\n", item.MR.Branch, item.MR.Worker) + fmt.Fprintf(m.output, "Processing: %s (%s)\n", item.MR.Branch, item.MR.Worker) result := m.ProcessMR(item.MR) if result.Success { - fmt.Printf(" ✓ Merged successfully\n") + fmt.Fprintln(m.output, " ✓ Merged successfully") } else { - fmt.Printf(" ✗ Failed: %s\n", result.Error) + fmt.Fprintf(m.output, " ✗ Failed: %s\n", result.Error) } } @@ -516,7 +525,7 @@ func (m *Manager) completeMR(mr *MergeRequest, closeReason CloseReason, errMsg s // Close the MR (in_progress → closed) if err := mr.Close(closeReason); err != nil { // Log error but continue - this shouldn't happen - fmt.Printf("Warning: failed to close MR: %v\n", err) + fmt.Fprintf(m.output, "Warning: failed to close MR: %v\n", err) } switch closeReason { case CloseReasonMerged: @@ -534,7 +543,7 @@ func (m *Manager) completeMR(mr *MergeRequest, closeReason CloseReason, errMsg s // Reopen the MR for rework (in_progress → open) if err := mr.Reopen(); err != nil { // Log error but continue - fmt.Printf("Warning: failed to reopen MR: %v\n", err) + fmt.Fprintf(m.output, "Warning: failed to reopen MR: %v\n", err) } ref.Stats.TotalFailed++ ref.Stats.TodayFailed++ @@ -669,7 +678,7 @@ func (m *Manager) pushWithRetry(targetBranch string, config MergeConfig) error { for attempt := 0; attempt <= config.PushRetryCount; attempt++ { if attempt > 0 { - fmt.Printf("Push retry %d/%d after %v\n", attempt, config.PushRetryCount, delay) + fmt.Fprintf(m.output, "Push retry %d/%d after %v\n", attempt, config.PushRetryCount, delay) time.Sleep(delay) delay *= 2 // Exponential backoff }