johno/gastown
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

refactor(refinery): use io.Writer instead of fmt.Print for output

Browse Source 
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 <noreply@anthropic.com>
This commit is contained in:
Steve Yegge
2025-12-21 22:18:10 -08:00
parent b31e8bc72c
commit 2a8ae43041
2 changed files with 47 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files

47
internal/refinery/engineer.go
View File

@@ -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 <id> --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
}

29
internal/refinery/manager.go
View File

@@ -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
}