feat(refinery): implement MR state transition validation (gt-h5n.3)

- Update MRStatus to use beads-style statuses (open, in_progress, closed)
- Add CloseReason enum for tracking why MRs were closed
- Implement ValidateTransition() to enforce valid state transitions:
  - open → in_progress (Engineer claims MR)
  - in_progress → closed (merge success or rejection)
  - in_progress → open (failure, reassign to worker)
  - open → closed (manual rejection)
  - closed → anything is blocked (immutable once closed)
- Add convenience methods: Claim(), Close(), Reopen(), SetStatus()
- Add status check methods: IsClosed(), IsOpen(), IsInProgress()
- Update ProcessMR and completeMR to use new state transition methods
- Update display code to handle new status values
- Add comprehensive tests for state transitions

Reference: docs/merge-queue-design.md#state-machine

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

internal/cmd/refinery.go

@@ -274,14 +274,27 @@ func runRefineryQueue(cmd *cobra.Command, args []string) error {
 			status = style.Bold.Render("[processing]")
 		} else {
 			switch item.MR.Status {
-			case refinery.MRPending:
-				status = style.Dim.Render("[pending]")
-			case refinery.MRMerged:
-				status = style.Bold.Render("[merged]")
-			case refinery.MRFailed:
-				status = style.Dim.Render("[failed]")
-			case refinery.MRSkipped:
-				status = style.Dim.Render("[skipped]")
+			case refinery.MROpen:
+				if item.MR.Error != "" {
+					status = style.Dim.Render("[needs-rework]")
+				} else {
+					status = style.Dim.Render("[pending]")
+				}
+			case refinery.MRInProgress:
+				status = style.Bold.Render("[processing]")
+			case refinery.MRClosed:
+				switch item.MR.CloseReason {
+				case refinery.CloseReasonMerged:
+					status = style.Bold.Render("[merged]")
+				case refinery.CloseReasonRejected:
+					status = style.Dim.Render("[rejected]")
+				case refinery.CloseReasonConflict:
+					status = style.Dim.Render("[conflict]")
+				case refinery.CloseReasonSuperseded:
+					status = style.Dim.Render("[superseded]")
+				default:
+					status = style.Dim.Render("[closed]")
+				}
 			}
 		}
 

internal/refinery/manager.go

@@ -244,7 +244,7 @@ func (m *Manager) branchToMR(branch string) *MergeRequest {
 		IssueID:      issueID,
 		TargetBranch: "main", // Default; swarm would use integration branch
 		CreatedAt:    time.Now(), // Would ideally get from git
-		Status:       MRPending,
+		Status:       MROpen,
 	}
 }
 
@@ -275,7 +275,7 @@ func (m *Manager) ProcessQueue() error {
 	}
 
 	for _, item := range queue {
-		if item.MR.Status != MRPending {
+		if !item.MR.IsOpen() {
 			continue
 		}
 
@@ -304,9 +304,11 @@ type MergeResult struct {
 func (m *Manager) ProcessMR(mr *MergeRequest) MergeResult {
 	ref, _ := m.loadState()
 
-	// Set current MR
+	// Claim the MR (open → in_progress)
+	if err := mr.Claim(); err != nil {
+		return MergeResult{Error: fmt.Sprintf("cannot claim MR: %v", err)}
+	}
 	ref.CurrentMR = mr
-	mr.Status = MRProcessing
 	m.saveState(ref)
 
 	result := MergeResult{}
@@ -314,7 +316,7 @@ func (m *Manager) ProcessMR(mr *MergeRequest) MergeResult {
 	// 1. Fetch the branch
 	if err := m.gitRun("fetch", "origin", mr.Branch); err != nil {
 		result.Error = fmt.Sprintf("fetch failed: %v", err)
-		m.completeMR(mr, MRFailed, result.Error)
+		m.completeMR(mr, "", result.Error) // Reopen for retry
 		return result
 	}
 
@@ -322,7 +324,7 @@ func (m *Manager) ProcessMR(mr *MergeRequest) MergeResult {
 	// First, checkout target
 	if err := m.gitRun("checkout", mr.TargetBranch); err != nil {
 		result.Error = fmt.Sprintf("checkout target failed: %v", err)
-		m.completeMR(mr, MRFailed, result.Error)
+		m.completeMR(mr, "", result.Error) // Reopen for retry
 		return result
 	}
 
@@ -341,13 +343,13 @@ func (m *Manager) ProcessMR(mr *MergeRequest) MergeResult {
 			result.Error = "merge conflict"
 			// Abort the merge
 			m.gitRun("merge", "--abort")
-			m.completeMR(mr, MRFailed, "merge conflict - polecat must rebase")
+			m.completeMR(mr, "", "merge conflict - polecat must rebase") // Reopen for rebase
 			// Notify worker about conflict
 			m.notifyWorkerConflict(mr)
 			return result
 		}
 		result.Error = fmt.Sprintf("merge failed: %v", err)
-		m.completeMR(mr, MRFailed, result.Error)
+		m.completeMR(mr, "", result.Error) // Reopen for retry
 		return result
 	}
 
@@ -359,7 +361,7 @@ func (m *Manager) ProcessMR(mr *MergeRequest) MergeResult {
 			result.Error = fmt.Sprintf("tests failed: %v", err)
 			// Reset to before merge
 			m.gitRun("reset", "--hard", "HEAD~1")
-			m.completeMR(mr, MRFailed, result.Error)
+			m.completeMR(mr, "", result.Error) // Reopen for fixes
 			return result
 		}
 	}
@@ -369,13 +371,13 @@ func (m *Manager) ProcessMR(mr *MergeRequest) MergeResult {
 		result.Error = fmt.Sprintf("push failed: %v", err)
 		// Reset to before merge
 		m.gitRun("reset", "--hard", "HEAD~1")
-		m.completeMR(mr, MRFailed, result.Error)
+		m.completeMR(mr, "", result.Error) // Reopen for retry
 		return result
 	}
 
 	// Success!
 	result.Success = true
-	m.completeMR(mr, MRMerged, "")
+	m.completeMR(mr, CloseReasonMerged, "")
 
 	// Notify worker of success
 	m.notifyWorkerMerged(mr)
@@ -387,24 +389,41 @@ func (m *Manager) ProcessMR(mr *MergeRequest) MergeResult {
 }
 
 // completeMR marks an MR as complete and updates stats.
-func (m *Manager) completeMR(mr *MergeRequest, status MRStatus, errMsg string) {
+// For success, pass closeReason (e.g., CloseReasonMerged).
+// For failures that should return to open, pass empty closeReason.
+func (m *Manager) completeMR(mr *MergeRequest, closeReason CloseReason, errMsg string) {
 	ref, _ := m.loadState()
-
-	mr.Status = status
 	mr.Error = errMsg
 	ref.CurrentMR = nil
 
 	now := time.Now()
-	switch status {
-	case MRMerged:
-		ref.LastMergeAt = &now
-		ref.Stats.TotalMerged++
-		ref.Stats.TodayMerged++
-	case MRFailed:
+
+	if closeReason != "" {
+		// 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)
+		}
+		switch closeReason {
+		case CloseReasonMerged:
+			ref.LastMergeAt = &now
+			ref.Stats.TotalMerged++
+			ref.Stats.TodayMerged++
+		case CloseReasonSuperseded:
+			ref.Stats.TotalSkipped++
+		default:
+			// Other close reasons (rejected, conflict) count as failed
+			ref.Stats.TotalFailed++
+			ref.Stats.TodayFailed++
+		}
+	} else {
+		// 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)
+		}
 		ref.Stats.TotalFailed++
 		ref.Stats.TodayFailed++
-	case MRSkipped:
-		ref.Stats.TotalSkipped++
 	}
 
 	m.saveState(ref)

internal/refinery/types.go

@@ -1,7 +1,11 @@
 // Package refinery provides the merge queue processing agent.
 package refinery
 
-import "time"
+import (
+	"errors"
+	"fmt"
+	"time"
+)
 
 // State represents the refinery's running state.
 type State string
@@ -67,30 +71,46 @@ type MergeRequest struct {
 	// Status is the current status of the merge request.
 	Status MRStatus `json:"status"`
 
-	// Error contains error details if Status is MRFailed.
+	// CloseReason indicates why the MR was closed (only set when Status=closed).
+	CloseReason CloseReason `json:"close_reason,omitempty"`
+
+	// Error contains error details if the MR failed.
 	Error string `json:"error,omitempty"`
 }
 
 // MRStatus represents the status of a merge request.
+// Uses beads-style statuses for consistency with the issue tracking system.
 type MRStatus string
 
 const (
-	// MRPending means the MR is waiting to be processed.
-	MRPending MRStatus = "pending"
+	// MROpen means the MR is waiting to be processed or needs rework.
+	MROpen MRStatus = "open"
 
-	// MRProcessing means the MR is currently being merged.
-	MRProcessing MRStatus = "processing"
+	// MRInProgress means the MR is currently being merged by the Engineer.
+	MRInProgress MRStatus = "in_progress"
 
-	// MRMerged means the MR was successfully merged.
-	MRMerged MRStatus = "merged"
-
-	// MRFailed means the merge failed (conflict or error).
-	MRFailed MRStatus = "failed"
-
-	// MRSkipped means the MR was skipped (duplicate, outdated, etc).
-	MRSkipped MRStatus = "skipped"
+	// MRClosed means the MR processing is complete (merged, rejected, etc).
+	MRClosed MRStatus = "closed"
 )
 
+// CloseReason indicates why a merge request was closed.
+type CloseReason string
+
+const (
+	// CloseReasonMerged means the MR was successfully merged.
+	CloseReasonMerged CloseReason = "merged"
+
+	// CloseReasonRejected means the MR was manually rejected.
+	CloseReasonRejected CloseReason = "rejected"
+
+	// CloseReasonConflict means the MR had unresolvable conflicts.
+	CloseReasonConflict CloseReason = "conflict"
+
+	// CloseReasonSuperseded means the MR was replaced by another.
+	CloseReasonSuperseded CloseReason = "superseded"
+)
+
+
 // RefineryStats contains cumulative refinery statistics.
 type RefineryStats struct {
 	// TotalMerged is the total number of successful merges.
@@ -115,3 +135,116 @@ type QueueItem struct {
 	MR        *MergeRequest `json:"mr"`
 	Age       string    `json:"age"`
 }
+
+// State transition errors.
+var (
+	// ErrInvalidTransition is returned when a state transition is not allowed.
+	ErrInvalidTransition = errors.New("invalid state transition")
+
+	// ErrClosedImmutable is returned when attempting to change a closed MR.
+	ErrClosedImmutable = errors.New("closed merge requests are immutable")
+)
+
+// ValidateTransition checks if a state transition from -> to is valid.
+//
+// Valid transitions:
+//   - open → in_progress (Engineer claims MR)
+//   - in_progress → closed (merge success or rejection)
+//   - in_progress → open (failure, reassign to worker)
+//   - open → closed (manual rejection)
+//
+// Invalid:
+//   - closed → anything (immutable once closed)
+func ValidateTransition(from, to MRStatus) error {
+	// Same state is always valid (no-op)
+	if from == to {
+		return nil
+	}
+
+	// Closed is immutable - cannot transition to anything else
+	if from == MRClosed {
+		return fmt.Errorf("%w: cannot change status from closed", ErrClosedImmutable)
+	}
+
+	// Check valid transitions
+	switch from {
+	case MROpen:
+		// open → in_progress: Engineer claims MR
+		// open → closed: manual rejection
+		if to == MRInProgress || to == MRClosed {
+			return nil
+		}
+	case MRInProgress:
+		// in_progress → closed: merge success or rejection
+		// in_progress → open: failure, reassign to worker
+		if to == MRClosed || to == MROpen {
+			return nil
+		}
+	}
+
+	return fmt.Errorf("%w: %s → %s is not allowed", ErrInvalidTransition, from, to)
+}
+
+// SetStatus updates the MR status after validating the transition.
+// Returns an error if the transition is not allowed.
+func (mr *MergeRequest) SetStatus(newStatus MRStatus) error {
+	if err := ValidateTransition(mr.Status, newStatus); err != nil {
+		return err
+	}
+	mr.Status = newStatus
+	return nil
+}
+
+// Close closes the MR with the given reason after validating the transition.
+// Returns an error if the MR cannot be closed from its current state.
+// Once closed, an MR cannot be closed again (even with a different reason).
+func (mr *MergeRequest) Close(reason CloseReason) error {
+	// Closed MRs are immutable - cannot be closed again
+	if mr.Status == MRClosed {
+		return fmt.Errorf("%w: MR is already closed", ErrClosedImmutable)
+	}
+	if err := ValidateTransition(mr.Status, MRClosed); err != nil {
+		return err
+	}
+	mr.Status = MRClosed
+	mr.CloseReason = reason
+	return nil
+}
+
+// Reopen reopens a failed MR (transitions from in_progress back to open).
+// Returns an error if the transition is not allowed.
+func (mr *MergeRequest) Reopen() error {
+	if mr.Status != MRInProgress {
+		return fmt.Errorf("%w: can only reopen from in_progress, current status is %s",
+			ErrInvalidTransition, mr.Status)
+	}
+	mr.Status = MROpen
+	mr.CloseReason = "" // Clear any previous close reason
+	return nil
+}
+
+// Claim transitions the MR from open to in_progress (Engineer claims it).
+// Returns an error if the transition is not allowed.
+func (mr *MergeRequest) Claim() error {
+	if mr.Status != MROpen {
+		return fmt.Errorf("%w: can only claim from open, current status is %s",
+			ErrInvalidTransition, mr.Status)
+	}
+	mr.Status = MRInProgress
+	return nil
+}
+
+// IsClosed returns true if the MR is in a closed state.
+func (mr *MergeRequest) IsClosed() bool {
+	return mr.Status == MRClosed
+}
+
+// IsOpen returns true if the MR is in an open state (waiting for processing).
+func (mr *MergeRequest) IsOpen() bool {
+	return mr.Status == MROpen
+}
+
+// IsInProgress returns true if the MR is currently being processed.
+func (mr *MergeRequest) IsInProgress() bool {
+	return mr.Status == MRInProgress
+}

internal/refinery/types_test.go

@@ -0,0 +1,257 @@
+package refinery
+
+import (
+	"errors"
+	"testing"
+)
+
+func TestValidateTransition(t *testing.T) {
+	tests := []struct {
+		name    string
+		from    MRStatus
+		to      MRStatus
+		wantErr bool
+		errType error
+	}{
+		// Valid transitions
+		{
+			name:    "open to in_progress (claim)",
+			from:    MROpen,
+			to:      MRInProgress,
+			wantErr: false,
+		},
+		{
+			name:    "open to closed (manual rejection)",
+			from:    MROpen,
+			to:      MRClosed,
+			wantErr: false,
+		},
+		{
+			name:    "in_progress to closed (success/rejection)",
+			from:    MRInProgress,
+			to:      MRClosed,
+			wantErr: false,
+		},
+		{
+			name:    "in_progress to open (failure/reassign)",
+			from:    MRInProgress,
+			to:      MROpen,
+			wantErr: false,
+		},
+		{
+			name:    "same state (no-op)",
+			from:    MROpen,
+			to:      MROpen,
+			wantErr: false,
+		},
+		{
+			name:    "same state closed (no-op)",
+			from:    MRClosed,
+			to:      MRClosed,
+			wantErr: false,
+		},
+
+		// Invalid transitions
+		{
+			name:    "closed to open (immutable)",
+			from:    MRClosed,
+			to:      MROpen,
+			wantErr: true,
+			errType: ErrClosedImmutable,
+		},
+		{
+			name:    "closed to in_progress (immutable)",
+			from:    MRClosed,
+			to:      MRInProgress,
+			wantErr: true,
+			errType: ErrClosedImmutable,
+		},
+		{
+			name:    "open to open is valid (no-op)",
+			from:    MROpen,
+			to:      MROpen,
+			wantErr: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := ValidateTransition(tt.from, tt.to)
+			if tt.wantErr {
+				if err == nil {
+					t.Errorf("ValidateTransition(%s, %s) expected error, got nil", tt.from, tt.to)
+					return
+				}
+				if tt.errType != nil && !errors.Is(err, tt.errType) {
+					t.Errorf("ValidateTransition(%s, %s) error = %v, want %v", tt.from, tt.to, err, tt.errType)
+				}
+			} else {
+				if err != nil {
+					t.Errorf("ValidateTransition(%s, %s) unexpected error: %v", tt.from, tt.to, err)
+				}
+			}
+		})
+	}
+}
+
+func TestMergeRequest_Claim(t *testing.T) {
+	t.Run("claim from open succeeds", func(t *testing.T) {
+		mr := &MergeRequest{Status: MROpen}
+		err := mr.Claim()
+		if err != nil {
+			t.Errorf("Claim() unexpected error: %v", err)
+		}
+		if mr.Status != MRInProgress {
+			t.Errorf("Claim() status = %s, want %s", mr.Status, MRInProgress)
+		}
+	})
+
+	t.Run("claim from in_progress fails", func(t *testing.T) {
+		mr := &MergeRequest{Status: MRInProgress}
+		err := mr.Claim()
+		if err == nil {
+			t.Error("Claim() expected error, got nil")
+		}
+		if !errors.Is(err, ErrInvalidTransition) {
+			t.Errorf("Claim() error = %v, want %v", err, ErrInvalidTransition)
+		}
+	})
+
+	t.Run("claim from closed fails", func(t *testing.T) {
+		mr := &MergeRequest{Status: MRClosed}
+		err := mr.Claim()
+		if err == nil {
+			t.Error("Claim() expected error, got nil")
+		}
+	})
+}
+
+func TestMergeRequest_Close(t *testing.T) {
+	t.Run("close from in_progress succeeds", func(t *testing.T) {
+		mr := &MergeRequest{Status: MRInProgress}
+		err := mr.Close(CloseReasonMerged)
+		if err != nil {
+			t.Errorf("Close() unexpected error: %v", err)
+		}
+		if mr.Status != MRClosed {
+			t.Errorf("Close() status = %s, want %s", mr.Status, MRClosed)
+		}
+		if mr.CloseReason != CloseReasonMerged {
+			t.Errorf("Close() closeReason = %s, want %s", mr.CloseReason, CloseReasonMerged)
+		}
+	})
+
+	t.Run("close from open succeeds (manual rejection)", func(t *testing.T) {
+		mr := &MergeRequest{Status: MROpen}
+		err := mr.Close(CloseReasonRejected)
+		if err != nil {
+			t.Errorf("Close() unexpected error: %v", err)
+		}
+		if mr.Status != MRClosed {
+			t.Errorf("Close() status = %s, want %s", mr.Status, MRClosed)
+		}
+	})
+
+	t.Run("close from closed fails", func(t *testing.T) {
+		mr := &MergeRequest{Status: MRClosed, CloseReason: CloseReasonMerged}
+		err := mr.Close(CloseReasonRejected)
+		if err == nil {
+			t.Error("Close() expected error, got nil")
+		}
+		if !errors.Is(err, ErrClosedImmutable) {
+			t.Errorf("Close() error = %v, want %v", err, ErrClosedImmutable)
+		}
+	})
+}
+
+func TestMergeRequest_Reopen(t *testing.T) {
+	t.Run("reopen from in_progress succeeds", func(t *testing.T) {
+		mr := &MergeRequest{Status: MRInProgress}
+		err := mr.Reopen()
+		if err != nil {
+			t.Errorf("Reopen() unexpected error: %v", err)
+		}
+		if mr.Status != MROpen {
+			t.Errorf("Reopen() status = %s, want %s", mr.Status, MROpen)
+		}
+	})
+
+	t.Run("reopen from open fails", func(t *testing.T) {
+		mr := &MergeRequest{Status: MROpen}
+		err := mr.Reopen()
+		if err == nil {
+			t.Error("Reopen() expected error, got nil")
+		}
+		if !errors.Is(err, ErrInvalidTransition) {
+			t.Errorf("Reopen() error = %v, want %v", err, ErrInvalidTransition)
+		}
+	})
+
+	t.Run("reopen from closed fails", func(t *testing.T) {
+		mr := &MergeRequest{Status: MRClosed}
+		err := mr.Reopen()
+		if err == nil {
+			t.Error("Reopen() expected error, got nil")
+		}
+	})
+
+	t.Run("reopen clears close reason", func(t *testing.T) {
+		mr := &MergeRequest{Status: MRInProgress, CloseReason: CloseReasonMerged}
+		err := mr.Reopen()
+		if err != nil {
+			t.Errorf("Reopen() unexpected error: %v", err)
+		}
+		if mr.CloseReason != "" {
+			t.Errorf("Reopen() closeReason = %s, want empty", mr.CloseReason)
+		}
+	})
+}
+
+func TestMergeRequest_SetStatus(t *testing.T) {
+	t.Run("valid transition succeeds", func(t *testing.T) {
+		mr := &MergeRequest{Status: MROpen}
+		err := mr.SetStatus(MRInProgress)
+		if err != nil {
+			t.Errorf("SetStatus() unexpected error: %v", err)
+		}
+		if mr.Status != MRInProgress {
+			t.Errorf("SetStatus() status = %s, want %s", mr.Status, MRInProgress)
+		}
+	})
+
+	t.Run("invalid transition fails", func(t *testing.T) {
+		mr := &MergeRequest{Status: MRClosed}
+		err := mr.SetStatus(MROpen)
+		if err == nil {
+			t.Error("SetStatus() expected error, got nil")
+		}
+	})
+}
+
+func TestMergeRequest_StatusChecks(t *testing.T) {
+	tests := []struct {
+		status       MRStatus
+		isClosed     bool
+		isOpen       bool
+		isInProgress bool
+	}{
+		{MROpen, false, true, false},
+		{MRInProgress, false, false, true},
+		{MRClosed, true, false, false},
+	}
+
+	for _, tt := range tests {
+		t.Run(string(tt.status), func(t *testing.T) {
+			mr := &MergeRequest{Status: tt.status}
+			if mr.IsClosed() != tt.isClosed {
+				t.Errorf("IsClosed() = %v, want %v", mr.IsClosed(), tt.isClosed)
+			}
+			if mr.IsOpen() != tt.isOpen {
+				t.Errorf("IsOpen() = %v, want %v", mr.IsOpen(), tt.isOpen)
+			}
+			if mr.IsInProgress() != tt.isInProgress {
+				t.Errorf("IsInProgress() = %v, want %v", mr.IsInProgress(), tt.isInProgress)
+			}
+		})
+	}
+}