fix: emit rich mutation events for status changes (bd-313v)

- Add emitRichMutation() function for events with metadata - handleClose now emits MutationStatus with old/new status - handleUpdate detects status changes and emits MutationStatus - Add comprehensive tests for rich mutation events Also: - Add activity.go test coverage (bd-3jcw): - Tests for parseDurationString, filterEvents, formatEvent - Tests for all mutation type displays - Fix silent error handling in --follow mode (bd-csnr): - Track consecutive daemon failures - Show warning after 5 failures (rate-limited to 30s) - Show reconnection message on recovery 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2025-12-23 04:16:26 -08:00
parent 3f51ca5454
commit da809061db
5 changed files with 662 additions and 8 deletions
--- a/internal/rpc/server_core.go
+++ b/internal/rpc/server_core.go
@@ -139,10 +139,19 @@ func NewServer(socketPath string, store storage.Storage, workspacePath string, d
 // Non-blocking: drops event if channel is full (sync will happen eventually).
 // Also stores in recent mutations buffer for polling.
 func (s *Server) emitMutation(eventType, issueID string) {
-	event := MutationEvent{
-		Type:      eventType,
-		IssueID:   issueID,
-		Timestamp: time.Now(),
+	s.emitRichMutation(MutationEvent{
+		Type:    eventType,
+		IssueID: issueID,
+	})
+}
+
+// emitRichMutation sends a pre-built mutation event with optional metadata.
+// Use this for events that include additional context (status changes, bonded events, etc.)
+// Non-blocking: drops event if channel is full (sync will happen eventually).
+func (s *Server) emitRichMutation(event MutationEvent) {
+	// Always set timestamp if not provided
+	if event.Timestamp.IsZero() {
+		event.Timestamp = time.Now()
 	}

 	// Send to mutation channel for daemon
--- a/internal/rpc/server_issues_epics.go
+++ b/internal/rpc/server_issues_epics.go
@@ -465,7 +465,17 @@ func (s *Server) handleUpdate(req *Request) Response {

 	// Emit mutation event for event-driven daemon (only if any updates or label operations were performed)
 	if len(updates) > 0 || len(updateArgs.SetLabels) > 0 || len(updateArgs.AddLabels) > 0 || len(updateArgs.RemoveLabels) > 0 {
-		s.emitMutation(MutationUpdate, updateArgs.ID)
+		// Check if this was a status change - emit rich MutationStatus event
+		if updateArgs.Status != nil && *updateArgs.Status != string(issue.Status) {
+			s.emitRichMutation(MutationEvent{
+				Type:      MutationStatus,
+				IssueID:   updateArgs.ID,
+				OldStatus: string(issue.Status),
+				NewStatus: *updateArgs.Status,
+			})
+		} else {
+			s.emitMutation(MutationUpdate, updateArgs.ID)
+		}
 	}

 	updatedIssue, getErr := store.GetIssue(ctx, updateArgs.ID)
@@ -517,6 +527,12 @@ func (s *Server) handleClose(req *Request) Response {
 		}
 	}

+	// Capture old status for rich mutation event
+	oldStatus := ""
+	if issue != nil {
+		oldStatus = string(issue.Status)
+	}
+
 	if err := store.CloseIssue(ctx, closeArgs.ID, closeArgs.Reason, s.reqActor(req)); err != nil {
 		return Response{
 			Success: false,
@@ -524,8 +540,13 @@ func (s *Server) handleClose(req *Request) Response {
 		}
 	}

-	// Emit mutation event for event-driven daemon
-	s.emitMutation(MutationUpdate, closeArgs.ID)
+	// Emit rich status change event for event-driven daemon
+	s.emitRichMutation(MutationEvent{
+		Type:      MutationStatus,
+		IssueID:   closeArgs.ID,
+		OldStatus: oldStatus,
+		NewStatus: "closed",
+	})

 	closedIssue, _ := store.GetIssue(ctx, closeArgs.ID)
 	data, _ := json.Marshal(closedIssue)
--- a/internal/rpc/server_mutations_test.go
+++ b/internal/rpc/server_mutations_test.go
@@ -234,6 +234,72 @@ func TestMutationEventTypes(t *testing.T) {
 	}
 }

+// TestEmitRichMutation verifies that rich mutation events include metadata fields
+func TestEmitRichMutation(t *testing.T) {
+	store := memory.New("/tmp/test.jsonl")
+	server := NewServer("/tmp/test.sock", store, "/tmp", "/tmp/test.db")
+
+	// Emit a rich status change event
+	server.emitRichMutation(MutationEvent{
+		Type:      MutationStatus,
+		IssueID:   "bd-456",
+		OldStatus: "open",
+		NewStatus: "in_progress",
+	})
+
+	mutations := server.GetRecentMutations(0)
+	if len(mutations) != 1 {
+		t.Fatalf("expected 1 mutation, got %d", len(mutations))
+	}
+
+	m := mutations[0]
+	if m.Type != MutationStatus {
+		t.Errorf("expected type %s, got %s", MutationStatus, m.Type)
+	}
+	if m.IssueID != "bd-456" {
+		t.Errorf("expected issue ID bd-456, got %s", m.IssueID)
+	}
+	if m.OldStatus != "open" {
+		t.Errorf("expected OldStatus 'open', got %s", m.OldStatus)
+	}
+	if m.NewStatus != "in_progress" {
+		t.Errorf("expected NewStatus 'in_progress', got %s", m.NewStatus)
+	}
+	if m.Timestamp.IsZero() {
+		t.Error("expected Timestamp to be set automatically")
+	}
+}
+
+// TestEmitRichMutation_Bonded verifies bonded events include step count
+func TestEmitRichMutation_Bonded(t *testing.T) {
+	store := memory.New("/tmp/test.jsonl")
+	server := NewServer("/tmp/test.sock", store, "/tmp", "/tmp/test.db")
+
+	// Emit a bonded event with metadata
+	server.emitRichMutation(MutationEvent{
+		Type:      MutationBonded,
+		IssueID:   "bd-789",
+		ParentID:  "bd-parent",
+		StepCount: 5,
+	})
+
+	mutations := server.GetRecentMutations(0)
+	if len(mutations) != 1 {
+		t.Fatalf("expected 1 mutation, got %d", len(mutations))
+	}
+
+	m := mutations[0]
+	if m.Type != MutationBonded {
+		t.Errorf("expected type %s, got %s", MutationBonded, m.Type)
+	}
+	if m.ParentID != "bd-parent" {
+		t.Errorf("expected ParentID 'bd-parent', got %s", m.ParentID)
+	}
+	if m.StepCount != 5 {
+		t.Errorf("expected StepCount 5, got %d", m.StepCount)
+	}
+}
+
 func TestMutationTimestamps(t *testing.T) {
 	store := memory.New("/tmp/test.jsonl")
 	server := NewServer("/tmp/test.sock", store, "/tmp", "/tmp/test.db")
@@ -276,6 +342,225 @@ func TestEmitMutation_NonBlocking(t *testing.T) {
 	}
 }

+// TestHandleClose_EmitsStatusMutation verifies that close operations emit MutationStatus events
+// with old/new status metadata (bd-313v fix)
+func TestHandleClose_EmitsStatusMutation(t *testing.T) {
+	store := memory.New("/tmp/test.jsonl")
+	server := NewServer("/tmp/test.sock", store, "/tmp", "/tmp/test.db")
+
+	// Create an issue first
+	createArgs := CreateArgs{
+		Title:     "Test Issue for Close",
+		IssueType: "bug",
+		Priority:  1,
+	}
+	createJSON, _ := json.Marshal(createArgs)
+	createReq := &Request{
+		Operation: OpCreate,
+		Args:      createJSON,
+		Actor:     "test-user",
+	}
+
+	createResp := server.handleCreate(createReq)
+	if !createResp.Success {
+		t.Fatalf("failed to create test issue: %s", createResp.Error)
+	}
+
+	var createdIssue map[string]interface{}
+	if err := json.Unmarshal(createResp.Data, &createdIssue); err != nil {
+		t.Fatalf("failed to parse created issue: %v", err)
+	}
+	issueID := createdIssue["id"].(string)
+
+	// Clear mutation buffer
+	time.Sleep(10 * time.Millisecond)
+	checkpoint := time.Now().UnixMilli()
+	time.Sleep(10 * time.Millisecond)
+
+	// Close the issue
+	closeArgs := CloseArgs{
+		ID:     issueID,
+		Reason: "test complete",
+	}
+	closeJSON, _ := json.Marshal(closeArgs)
+	closeReq := &Request{
+		Operation: OpClose,
+		Args:      closeJSON,
+		Actor:     "test-user",
+	}
+
+	closeResp := server.handleClose(closeReq)
+	if !closeResp.Success {
+		t.Fatalf("close operation failed: %s", closeResp.Error)
+	}
+
+	// Verify MutationStatus event was emitted with correct metadata
+	mutations := server.GetRecentMutations(checkpoint)
+	var statusMutation *MutationEvent
+	for _, m := range mutations {
+		if m.Type == MutationStatus && m.IssueID == issueID {
+			statusMutation = &m
+			break
+		}
+	}
+
+	if statusMutation == nil {
+		t.Fatalf("expected MutationStatus event for issue %s, but none found in mutations: %+v", issueID, mutations)
+	}
+
+	if statusMutation.OldStatus != "open" {
+		t.Errorf("expected OldStatus 'open', got %s", statusMutation.OldStatus)
+	}
+	if statusMutation.NewStatus != "closed" {
+		t.Errorf("expected NewStatus 'closed', got %s", statusMutation.NewStatus)
+	}
+}
+
+// TestHandleUpdate_EmitsStatusMutationOnStatusChange verifies that status updates emit MutationStatus
+func TestHandleUpdate_EmitsStatusMutationOnStatusChange(t *testing.T) {
+	store := memory.New("/tmp/test.jsonl")
+	server := NewServer("/tmp/test.sock", store, "/tmp", "/tmp/test.db")
+
+	// Create an issue first
+	createArgs := CreateArgs{
+		Title:     "Test Issue for Status Update",
+		IssueType: "task",
+		Priority:  2,
+	}
+	createJSON, _ := json.Marshal(createArgs)
+	createReq := &Request{
+		Operation: OpCreate,
+		Args:      createJSON,
+		Actor:     "test-user",
+	}
+
+	createResp := server.handleCreate(createReq)
+	if !createResp.Success {
+		t.Fatalf("failed to create test issue: %s", createResp.Error)
+	}
+
+	var createdIssue map[string]interface{}
+	if err := json.Unmarshal(createResp.Data, &createdIssue); err != nil {
+		t.Fatalf("failed to parse created issue: %v", err)
+	}
+	issueID := createdIssue["id"].(string)
+
+	// Clear mutation buffer
+	time.Sleep(10 * time.Millisecond)
+	checkpoint := time.Now().UnixMilli()
+	time.Sleep(10 * time.Millisecond)
+
+	// Update status to in_progress
+	status := "in_progress"
+	updateArgs := UpdateArgs{
+		ID:     issueID,
+		Status: &status,
+	}
+	updateJSON, _ := json.Marshal(updateArgs)
+	updateReq := &Request{
+		Operation: OpUpdate,
+		Args:      updateJSON,
+		Actor:     "test-user",
+	}
+
+	updateResp := server.handleUpdate(updateReq)
+	if !updateResp.Success {
+		t.Fatalf("update operation failed: %s", updateResp.Error)
+	}
+
+	// Verify MutationStatus event was emitted
+	mutations := server.GetRecentMutations(checkpoint)
+	var statusMutation *MutationEvent
+	for _, m := range mutations {
+		if m.Type == MutationStatus && m.IssueID == issueID {
+			statusMutation = &m
+			break
+		}
+	}
+
+	if statusMutation == nil {
+		t.Fatalf("expected MutationStatus event, but none found in mutations: %+v", mutations)
+	}
+
+	if statusMutation.OldStatus != "open" {
+		t.Errorf("expected OldStatus 'open', got %s", statusMutation.OldStatus)
+	}
+	if statusMutation.NewStatus != "in_progress" {
+		t.Errorf("expected NewStatus 'in_progress', got %s", statusMutation.NewStatus)
+	}
+}
+
+// TestHandleUpdate_EmitsUpdateMutationForNonStatusChanges verifies non-status updates emit MutationUpdate
+func TestHandleUpdate_EmitsUpdateMutationForNonStatusChanges(t *testing.T) {
+	store := memory.New("/tmp/test.jsonl")
+	server := NewServer("/tmp/test.sock", store, "/tmp", "/tmp/test.db")
+
+	// Create an issue first
+	createArgs := CreateArgs{
+		Title:     "Test Issue for Non-Status Update",
+		IssueType: "task",
+		Priority:  2,
+	}
+	createJSON, _ := json.Marshal(createArgs)
+	createReq := &Request{
+		Operation: OpCreate,
+		Args:      createJSON,
+		Actor:     "test-user",
+	}
+
+	createResp := server.handleCreate(createReq)
+	if !createResp.Success {
+		t.Fatalf("failed to create test issue: %s", createResp.Error)
+	}
+
+	var createdIssue map[string]interface{}
+	if err := json.Unmarshal(createResp.Data, &createdIssue); err != nil {
+		t.Fatalf("failed to parse created issue: %v", err)
+	}
+	issueID := createdIssue["id"].(string)
+
+	// Clear mutation buffer
+	time.Sleep(10 * time.Millisecond)
+	checkpoint := time.Now().UnixMilli()
+	time.Sleep(10 * time.Millisecond)
+
+	// Update title (not status)
+	newTitle := "Updated Title"
+	updateArgs := UpdateArgs{
+		ID:    issueID,
+		Title: &newTitle,
+	}
+	updateJSON, _ := json.Marshal(updateArgs)
+	updateReq := &Request{
+		Operation: OpUpdate,
+		Args:      updateJSON,
+		Actor:     "test-user",
+	}
+
+	updateResp := server.handleUpdate(updateReq)
+	if !updateResp.Success {
+		t.Fatalf("update operation failed: %s", updateResp.Error)
+	}
+
+	// Verify MutationUpdate event was emitted (not MutationStatus)
+	mutations := server.GetRecentMutations(checkpoint)
+	var updateMutation *MutationEvent
+	for _, m := range mutations {
+		if m.IssueID == issueID {
+			updateMutation = &m
+			break
+		}
+	}
+
+	if updateMutation == nil {
+		t.Fatal("expected mutation event, but none found")
+	}
+
+	if updateMutation.Type != MutationUpdate {
+		t.Errorf("expected MutationUpdate type, got %s", updateMutation.Type)
+	}
+}
+
 // TestHandleDelete_EmitsMutation verifies that delete operations emit mutation events
 // This is a regression test for the issue where delete operations bypass the daemon
 // and don't trigger auto-sync. The delete RPC handler should emit MutationDelete events.