fix(refinery): Send MERGE_FAILED to Witness when merge is rejected
When the Refinery detects a build error or test failure and refuses to merge, the polecat was never notified. This fixes the notification pipeline by: 1. Adding MERGE_FAILED protocol support to Witness: - PatternMergeFailed regex pattern - ProtoMergeFailed protocol type constant - MergeFailedPayload struct with all failure details - ParseMergeFailed parser function - ClassifyMessage case for MERGE_FAILED 2. Adding HandleMergeFailed handler to Witness: - Parses the failure notification - Sends HIGH priority mail to polecat with fix instructions - Includes branch, issue, failure type, and error details 3. Adding mail notification in Refinery's handleFailureFromQueue: - Creates mail.Router for sending protocol messages - Sends MERGE_FAILED to Witness when merge fails - Includes failure type (build/tests/conflict) and error 4. Adding comprehensive unit tests: - TestParseMergeFailed for full body parsing - TestParseMergeFailed_MinimalBody for minimal body - TestParseMergeFailed_InvalidSubject for error handling - ClassifyMessage test cases for MERGE_FAILED Fixes #114 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
committed by
Steve Yegge
parent
9cb14cc41a
commit
6fe25c757c
@@ -16,7 +16,9 @@ import (
|
|||||||
|
|
||||||
"github.com/steveyegge/gastown/internal/beads"
|
"github.com/steveyegge/gastown/internal/beads"
|
||||||
"github.com/steveyegge/gastown/internal/git"
|
"github.com/steveyegge/gastown/internal/git"
|
||||||
|
"github.com/steveyegge/gastown/internal/mail"
|
||||||
"github.com/steveyegge/gastown/internal/mrqueue"
|
"github.com/steveyegge/gastown/internal/mrqueue"
|
||||||
|
"github.com/steveyegge/gastown/internal/protocol"
|
||||||
"github.com/steveyegge/gastown/internal/rig"
|
"github.com/steveyegge/gastown/internal/rig"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -80,6 +82,7 @@ type Engineer struct {
|
|||||||
workDir string
|
workDir string
|
||||||
output io.Writer // Output destination for user-facing messages
|
output io.Writer // Output destination for user-facing messages
|
||||||
eventLogger *mrqueue.EventLogger
|
eventLogger *mrqueue.EventLogger
|
||||||
|
router *mail.Router // Mail router for sending protocol messages
|
||||||
|
|
||||||
// stopCh is used for graceful shutdown
|
// stopCh is used for graceful shutdown
|
||||||
stopCh chan struct{}
|
stopCh chan struct{}
|
||||||
@@ -100,6 +103,7 @@ func NewEngineer(r *rig.Rig) *Engineer {
|
|||||||
workDir: r.Path,
|
workDir: r.Path,
|
||||||
output: os.Stdout,
|
output: os.Stdout,
|
||||||
eventLogger: mrqueue.NewEventLoggerFromRig(r.Path),
|
eventLogger: mrqueue.NewEventLoggerFromRig(r.Path),
|
||||||
|
router: mail.NewRouter(r.Path),
|
||||||
stopCh: make(chan struct{}),
|
stopCh: make(chan struct{}),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -562,6 +566,21 @@ func (e *Engineer) handleFailureFromQueue(mr *mrqueue.MR, result ProcessResult)
|
|||||||
_, _ = fmt.Fprintf(e.output, "[Engineer] Warning: failed to log merge_failed event: %v\n", err)
|
_, _ = fmt.Fprintf(e.output, "[Engineer] Warning: failed to log merge_failed event: %v\n", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Notify Witness of the failure so polecat can be alerted
|
||||||
|
// Determine failure type from result
|
||||||
|
failureType := "build"
|
||||||
|
if result.Conflict {
|
||||||
|
failureType = "conflict"
|
||||||
|
} else if result.TestsFailed {
|
||||||
|
failureType = "tests"
|
||||||
|
}
|
||||||
|
msg := protocol.NewMergeFailedMessage(e.rig.Name, mr.Worker, mr.Branch, mr.SourceIssue, mr.Target, failureType, result.Error)
|
||||||
|
if err := e.router.Send(msg); err != nil {
|
||||||
|
fmt.Fprintf(e.output, "[Engineer] Warning: failed to send MERGE_FAILED to witness: %v\n", err)
|
||||||
|
} else {
|
||||||
|
fmt.Fprintf(e.output, "[Engineer] Notified witness of merge failure for %s\n", mr.Worker)
|
||||||
|
}
|
||||||
|
|
||||||
// If this was a conflict, create a conflict-resolution task for dispatch
|
// If this was a conflict, create a conflict-resolution task for dispatch
|
||||||
// and block the MR until the task is resolved (non-blocking delegation)
|
// and block the MR until the task is resolved (non-blocking delegation)
|
||||||
if result.Conflict {
|
if result.Conflict {
|
||||||
|
|||||||
@@ -294,6 +294,56 @@ func HandleMerged(workDir, rigName string, msg *mail.Message) *HandlerResult {
|
|||||||
return result
|
return result
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// HandleMergeFailed processes a MERGE_FAILED message from the Refinery.
|
||||||
|
// Notifies the polecat that their merge was rejected and rework is needed.
|
||||||
|
func HandleMergeFailed(workDir, rigName string, msg *mail.Message, router *mail.Router) *HandlerResult {
|
||||||
|
result := &HandlerResult{
|
||||||
|
MessageID: msg.ID,
|
||||||
|
ProtocolType: ProtoMergeFailed,
|
||||||
|
}
|
||||||
|
|
||||||
|
// Parse the message
|
||||||
|
payload, err := ParseMergeFailed(msg.Subject, msg.Body)
|
||||||
|
if err != nil {
|
||||||
|
result.Error = fmt.Errorf("parsing MERGE_FAILED: %w", err)
|
||||||
|
return result
|
||||||
|
}
|
||||||
|
|
||||||
|
// Notify the polecat about the failure
|
||||||
|
polecatAddr := fmt.Sprintf("%s/polecats/%s", rigName, payload.PolecatName)
|
||||||
|
notification := &mail.Message{
|
||||||
|
From: fmt.Sprintf("%s/witness", rigName),
|
||||||
|
To: polecatAddr,
|
||||||
|
Subject: fmt.Sprintf("Merge failed: %s", payload.FailureType),
|
||||||
|
Priority: mail.PriorityHigh,
|
||||||
|
Type: mail.TypeTask,
|
||||||
|
Body: fmt.Sprintf(`Your merge request was rejected.
|
||||||
|
|
||||||
|
Branch: %s
|
||||||
|
Issue: %s
|
||||||
|
Failure: %s
|
||||||
|
Error: %s
|
||||||
|
|
||||||
|
Please fix the issue and resubmit with 'gt done'.`,
|
||||||
|
payload.Branch,
|
||||||
|
payload.IssueID,
|
||||||
|
payload.FailureType,
|
||||||
|
payload.Error,
|
||||||
|
),
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := router.Send(notification); err != nil {
|
||||||
|
result.Error = fmt.Errorf("sending failure notification: %w", err)
|
||||||
|
return result
|
||||||
|
}
|
||||||
|
|
||||||
|
result.Handled = true
|
||||||
|
result.MailSent = notification.ID
|
||||||
|
result.Action = fmt.Sprintf("notified %s of merge failure: %s - %s", payload.PolecatName, payload.FailureType, payload.Error)
|
||||||
|
|
||||||
|
return result
|
||||||
|
}
|
||||||
|
|
||||||
// HandleSwarmStart processes a SWARM_START message from the Mayor.
|
// HandleSwarmStart processes a SWARM_START message from the Mayor.
|
||||||
// Creates a swarm tracking wisp to monitor batch polecat work.
|
// Creates a swarm tracking wisp to monitor batch polecat work.
|
||||||
func HandleSwarmStart(workDir string, msg *mail.Message) *HandlerResult {
|
func HandleSwarmStart(workDir string, msg *mail.Message) *HandlerResult {
|
||||||
|
|||||||
@@ -22,6 +22,9 @@ var (
|
|||||||
// MERGED <name> - refinery confirms branch merged
|
// MERGED <name> - refinery confirms branch merged
|
||||||
PatternMerged = regexp.MustCompile(`^MERGED\s+(\S+)`)
|
PatternMerged = regexp.MustCompile(`^MERGED\s+(\S+)`)
|
||||||
|
|
||||||
|
// MERGE_FAILED <name> - refinery reporting merge failure
|
||||||
|
PatternMergeFailed = regexp.MustCompile(`^MERGE_FAILED\s+(\S+)`)
|
||||||
|
|
||||||
// HANDOFF - session continuity message
|
// HANDOFF - session continuity message
|
||||||
PatternHandoff = regexp.MustCompile(`^🤝\s*HANDOFF`)
|
PatternHandoff = regexp.MustCompile(`^🤝\s*HANDOFF`)
|
||||||
|
|
||||||
@@ -37,6 +40,7 @@ const (
|
|||||||
ProtoLifecycleShutdown ProtocolType = "lifecycle_shutdown"
|
ProtoLifecycleShutdown ProtocolType = "lifecycle_shutdown"
|
||||||
ProtoHelp ProtocolType = "help"
|
ProtoHelp ProtocolType = "help"
|
||||||
ProtoMerged ProtocolType = "merged"
|
ProtoMerged ProtocolType = "merged"
|
||||||
|
ProtoMergeFailed ProtocolType = "merge_failed"
|
||||||
ProtoHandoff ProtocolType = "handoff"
|
ProtoHandoff ProtocolType = "handoff"
|
||||||
ProtoSwarmStart ProtocolType = "swarm_start"
|
ProtoSwarmStart ProtocolType = "swarm_start"
|
||||||
ProtoUnknown ProtocolType = "unknown"
|
ProtoUnknown ProtocolType = "unknown"
|
||||||
@@ -70,6 +74,16 @@ type MergedPayload struct {
|
|||||||
MergedAt time.Time
|
MergedAt time.Time
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// MergeFailedPayload contains parsed data from a MERGE_FAILED message.
|
||||||
|
type MergeFailedPayload struct {
|
||||||
|
PolecatName string
|
||||||
|
Branch string
|
||||||
|
IssueID string
|
||||||
|
FailureType string // "build", "test", "lint", etc.
|
||||||
|
Error string
|
||||||
|
FailedAt time.Time
|
||||||
|
}
|
||||||
|
|
||||||
// SwarmStartPayload contains parsed data from a SWARM_START message.
|
// SwarmStartPayload contains parsed data from a SWARM_START message.
|
||||||
type SwarmStartPayload struct {
|
type SwarmStartPayload struct {
|
||||||
SwarmID string
|
SwarmID string
|
||||||
@@ -89,6 +103,8 @@ func ClassifyMessage(subject string) ProtocolType {
|
|||||||
return ProtoHelp
|
return ProtoHelp
|
||||||
case PatternMerged.MatchString(subject):
|
case PatternMerged.MatchString(subject):
|
||||||
return ProtoMerged
|
return ProtoMerged
|
||||||
|
case PatternMergeFailed.MatchString(subject):
|
||||||
|
return ProtoMergeFailed
|
||||||
case PatternHandoff.MatchString(subject):
|
case PatternHandoff.MatchString(subject):
|
||||||
return ProtoHandoff
|
return ProtoHandoff
|
||||||
case PatternSwarmStart.MatchString(subject):
|
case PatternSwarmStart.MatchString(subject):
|
||||||
@@ -207,6 +223,43 @@ func ParseMerged(subject, body string) (*MergedPayload, error) {
|
|||||||
return payload, nil
|
return payload, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ParseMergeFailed extracts payload from a MERGE_FAILED message.
|
||||||
|
// Subject format: MERGE_FAILED <polecat-name>
|
||||||
|
// Body format:
|
||||||
|
//
|
||||||
|
// Branch: <branch>
|
||||||
|
// Issue: <issue-id>
|
||||||
|
// FailureType: <type>
|
||||||
|
// Error: <error-message>
|
||||||
|
func ParseMergeFailed(subject, body string) (*MergeFailedPayload, error) {
|
||||||
|
matches := PatternMergeFailed.FindStringSubmatch(subject)
|
||||||
|
if len(matches) < 2 {
|
||||||
|
return nil, fmt.Errorf("invalid MERGE_FAILED subject: %s", subject)
|
||||||
|
}
|
||||||
|
|
||||||
|
payload := &MergeFailedPayload{
|
||||||
|
PolecatName: matches[1],
|
||||||
|
FailedAt: time.Now(),
|
||||||
|
}
|
||||||
|
|
||||||
|
// Parse body for structured fields
|
||||||
|
for _, line := range strings.Split(body, "\n") {
|
||||||
|
line = strings.TrimSpace(line)
|
||||||
|
switch {
|
||||||
|
case strings.HasPrefix(line, "Branch:"):
|
||||||
|
payload.Branch = strings.TrimSpace(strings.TrimPrefix(line, "Branch:"))
|
||||||
|
case strings.HasPrefix(line, "Issue:"):
|
||||||
|
payload.IssueID = strings.TrimSpace(strings.TrimPrefix(line, "Issue:"))
|
||||||
|
case strings.HasPrefix(line, "FailureType:"):
|
||||||
|
payload.FailureType = strings.TrimSpace(strings.TrimPrefix(line, "FailureType:"))
|
||||||
|
case strings.HasPrefix(line, "Error:"):
|
||||||
|
payload.Error = strings.TrimSpace(strings.TrimPrefix(line, "Error:"))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return payload, nil
|
||||||
|
}
|
||||||
|
|
||||||
// ParseSwarmStart extracts payload from a SWARM_START message.
|
// ParseSwarmStart extracts payload from a SWARM_START message.
|
||||||
// Body format is JSON: {"swarm_id": "batch-123", "beads": ["bd-a", "bd-b"]}
|
// Body format is JSON: {"swarm_id": "batch-123", "beads": ["bd-a", "bd-b"]}
|
||||||
func ParseSwarmStart(body string) (*SwarmStartPayload, error) {
|
func ParseSwarmStart(body string) (*SwarmStartPayload, error) {
|
||||||
|
|||||||
@@ -16,6 +16,8 @@ func TestClassifyMessage(t *testing.T) {
|
|||||||
{"HELP: Git conflict", ProtoHelp},
|
{"HELP: Git conflict", ProtoHelp},
|
||||||
{"MERGED nux", ProtoMerged},
|
{"MERGED nux", ProtoMerged},
|
||||||
{"MERGED valkyrie", ProtoMerged},
|
{"MERGED valkyrie", ProtoMerged},
|
||||||
|
{"MERGE_FAILED nux", ProtoMergeFailed},
|
||||||
|
{"MERGE_FAILED ace", ProtoMergeFailed},
|
||||||
{"🤝 HANDOFF: Patrol context", ProtoHandoff},
|
{"🤝 HANDOFF: Patrol context", ProtoHandoff},
|
||||||
{"🤝HANDOFF: No space", ProtoHandoff},
|
{"🤝HANDOFF: No space", ProtoHandoff},
|
||||||
{"SWARM_START", ProtoSwarmStart},
|
{"SWARM_START", ProtoSwarmStart},
|
||||||
@@ -157,6 +159,65 @@ func TestParseMerged_InvalidSubject(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestParseMergeFailed(t *testing.T) {
|
||||||
|
subject := "MERGE_FAILED nux"
|
||||||
|
body := `Branch: feature-nux
|
||||||
|
Issue: gt-abc123
|
||||||
|
FailureType: tests
|
||||||
|
Error: unit tests failed with 3 errors`
|
||||||
|
|
||||||
|
payload, err := ParseMergeFailed(subject, body)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ParseMergeFailed() error = %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if payload.PolecatName != "nux" {
|
||||||
|
t.Errorf("PolecatName = %q, want %q", payload.PolecatName, "nux")
|
||||||
|
}
|
||||||
|
if payload.Branch != "feature-nux" {
|
||||||
|
t.Errorf("Branch = %q, want %q", payload.Branch, "feature-nux")
|
||||||
|
}
|
||||||
|
if payload.IssueID != "gt-abc123" {
|
||||||
|
t.Errorf("IssueID = %q, want %q", payload.IssueID, "gt-abc123")
|
||||||
|
}
|
||||||
|
if payload.FailureType != "tests" {
|
||||||
|
t.Errorf("FailureType = %q, want %q", payload.FailureType, "tests")
|
||||||
|
}
|
||||||
|
if payload.Error != "unit tests failed with 3 errors" {
|
||||||
|
t.Errorf("Error = %q, want %q", payload.Error, "unit tests failed with 3 errors")
|
||||||
|
}
|
||||||
|
if payload.FailedAt.IsZero() {
|
||||||
|
t.Error("FailedAt should not be zero")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestParseMergeFailed_MinimalBody(t *testing.T) {
|
||||||
|
subject := "MERGE_FAILED ace"
|
||||||
|
body := "FailureType: build"
|
||||||
|
|
||||||
|
payload, err := ParseMergeFailed(subject, body)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ParseMergeFailed() error = %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if payload.PolecatName != "ace" {
|
||||||
|
t.Errorf("PolecatName = %q, want %q", payload.PolecatName, "ace")
|
||||||
|
}
|
||||||
|
if payload.FailureType != "build" {
|
||||||
|
t.Errorf("FailureType = %q, want %q", payload.FailureType, "build")
|
||||||
|
}
|
||||||
|
if payload.Branch != "" {
|
||||||
|
t.Errorf("Branch = %q, want empty", payload.Branch)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestParseMergeFailed_InvalidSubject(t *testing.T) {
|
||||||
|
_, err := ParseMergeFailed("Not a merge failed", "body")
|
||||||
|
if err == nil {
|
||||||
|
t.Error("ParseMergeFailed() expected error for invalid subject")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestCleanupWispLabels(t *testing.T) {
|
func TestCleanupWispLabels(t *testing.T) {
|
||||||
labels := CleanupWispLabels("nux", "pending")
|
labels := CleanupWispLabels("nux", "pending")
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user