Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions server/cmd/server/notification_listeners.go
Original file line number Diff line number Diff line change
Expand Up @@ -558,8 +558,11 @@ func registerNotificationListeners(bus *events.Bus, queries *db.Queries) {
// Track who already got notified to avoid duplicates
skip := map[string]bool{e.ActorID: true}

// Direct notification to assignee
if issue.AssigneeType != nil && issue.AssigneeID != nil {
// Direct notification to assignee. Skip squad assignees —
// inbox_item.recipient_type is CHECK-constrained to
// ('member','agent'), and the squad leader is dispatched via
// its task instead (EnqueueTaskForSquadLeader).
if issue.AssigneeType != nil && issue.AssigneeID != nil && *issue.AssigneeType != "squad" {
skip[*issue.AssigneeID] = true
notifyDirect(ctx, queries, bus,
*issue.AssigneeType, *issue.AssigneeID,
Expand Down Expand Up @@ -613,8 +616,9 @@ func registerNotificationListeners(bus *events.Bus, queries *db.Queries) {
}
assigneeDetails, _ := json.Marshal(detailsMap)

// Direct: notify new assignee about assignment
if issue.AssigneeType != nil && issue.AssigneeID != nil {
// Direct: notify new assignee about assignment. Skip squad
// assignees (see issue:created listener for rationale).
if issue.AssigneeType != nil && issue.AssigneeID != nil && *issue.AssigneeType != "squad" {
notifyDirect(ctx, queries, bus,
*issue.AssigneeType, *issue.AssigneeID,
e.WorkspaceID, e, issue.ID, issue.Status,
Expand Down
95 changes: 95 additions & 0 deletions server/cmd/server/notification_listeners_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,101 @@ func TestSubscriberSystemCommentDoesNotSubscribe(t *testing.T) {
}
}

// TestNotification_SquadAssigneeSkipsInbox is the squad-assignee analog of
// TestNotification_SystemCommentSkipsInboxAndMentions: when an issue is created
// or reassigned with assignee_type='squad', the notification listener must
// NOT call notifyDirect with recipient_type='squad' — inbox_item.recipient_type
// is CHECK-constrained to ('member','agent') (SQLSTATE 23514), and the squad
// leader is dispatched via EnqueueTaskForSquadLeader instead. The fix mirrors
// the author_type='system' gate from MUL-2538.
//
// Covers both the issue:created and issue:updated (assignee_changed) paths.
// Same caveat as the system-comment test: the behavioral assertion holds
// either way (the insert fails), but the test pins the invariant and will
// fail loudly if anyone widens inbox_item.recipient_type to include 'squad'
// without revisiting this listener.
func TestNotification_SquadAssigneeSkipsInbox(t *testing.T) {
queries := db.New(testPool)
bus := newNotificationBus(t, queries)

squadID := createTestSquad(t, testWorkspaceID, testUserID, "Notification Squad")
t.Cleanup(func() { cleanupTestSquad(t, squadID) })

issueID := createTestIssue(t, testWorkspaceID, testUserID)
t.Cleanup(func() {
cleanupInboxForIssue(t, issueID)
cleanupTestIssue(t, issueID)
})

var inboxEvents []events.Event
bus.Subscribe(protocol.EventInboxNew, func(e events.Event) {
inboxEvents = append(inboxEvents, e)
})

// --- issue:created with squad assignee ---
createdType := "squad"
bus.Publish(events.Event{
Type: protocol.EventIssueCreated,
WorkspaceID: testWorkspaceID,
ActorType: "member",
ActorID: testUserID,
Payload: map[string]any{
"issue": handler.IssueResponse{
ID: issueID,
WorkspaceID: testWorkspaceID,
Title: "squad-assigned at creation",
Status: "todo",
Priority: "medium",
CreatorType: "member",
CreatorID: testUserID,
AssigneeType: &createdType,
AssigneeID: &squadID,
},
},
})

// --- issue:updated reassigning to the squad ---
updatedType := "squad"
bus.Publish(events.Event{
Type: protocol.EventIssueUpdated,
WorkspaceID: testWorkspaceID,
ActorType: "member",
ActorID: testUserID,
Payload: map[string]any{
"issue": handler.IssueResponse{
ID: issueID,
WorkspaceID: testWorkspaceID,
Title: "squad-assigned at creation",
Status: "todo",
Priority: "medium",
CreatorType: "member",
CreatorID: testUserID,
AssigneeType: &updatedType,
AssigneeID: &squadID,
},
"assignee_changed": true,
},
})

// No inbox item should exist for the squad (recipient_type='squad' would
// be rejected anyway, but the listener must short-circuit before the
// insert — the assertion pins the invariant).
squadItems, err := queries.ListInboxItems(context.Background(), db.ListInboxItemsParams{
WorkspaceID: util.MustParseUUID(testWorkspaceID),
RecipientType: "squad",
RecipientID: util.MustParseUUID(squadID),
})
if err != nil {
t.Fatalf("ListInboxItems for squad: %v", err)
}
if len(squadItems) != 0 {
t.Fatalf("expected 0 inbox items for squad assignee, got %d", len(squadItems))
}
if len(inboxEvents) != 0 {
t.Fatalf("expected 0 inbox:new events for squad-assigned issue, got %d", len(inboxEvents))
}
}

// TestNotification_AssigneeChanged verifies the full assignee change flow:
// - New assignee gets "issue_assigned" (Direct)
// - Old assignee gets "unassigned" (Direct)
Expand Down
14 changes: 11 additions & 3 deletions server/cmd/server/subscriber_listeners.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,15 @@ func registerSubscriberListeners(bus *events.Bus, queries *db.Queries) {
// Subscribe the creator
addSubscriber(bus, queries, e.WorkspaceID, issue.ID, issue.CreatorType, issue.CreatorID, "creator")

// Subscribe the assignee if exists and different from creator
// Subscribe the assignee if exists and different from creator.
// Skip squad assignees — the squad leader is dispatched via
// EnqueueTaskForSquadLeader (service/autopilot.go), and
// issue_subscriber.user_type is CHECK-constrained to
// ('member','agent') so a squad row would only log noise
// (cf. the author_type=='system' gate in the comment:created
// listener, MUL-2538).
if issue.AssigneeType != nil && issue.AssigneeID != nil &&
*issue.AssigneeType != "squad" &&
!(*issue.AssigneeType == issue.CreatorType && *issue.AssigneeID == issue.CreatorID) {
addSubscriber(bus, queries, e.WorkspaceID, issue.ID, *issue.AssigneeType, *issue.AssigneeID, "assignee")
}
Expand All @@ -55,9 +62,10 @@ func registerSubscriberListeners(bus *events.Bus, queries *db.Queries) {
return
}

// Subscribe new assignee if assignee changed
// Subscribe new assignee if assignee changed. Skip squad assignees
// (see issue:created listener above for rationale).
if assigneeChanged, _ := payload["assignee_changed"].(bool); assigneeChanged {
if issue.AssigneeType != nil && issue.AssigneeID != nil {
if issue.AssigneeType != nil && issue.AssigneeID != nil && *issue.AssigneeType != "squad" {
addSubscriber(bus, queries, e.WorkspaceID, issue.ID, *issue.AssigneeType, *issue.AssigneeID, "assignee")
}
}
Expand Down
130 changes: 130 additions & 0 deletions server/cmd/server/subscriber_listeners_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,34 @@ func cleanupTestUser(t *testing.T, email string) {
testPool.Exec(context.Background(), `DELETE FROM "user" WHERE email = $1`, email)
}

// createTestSquad inserts a squad led by a workspace agent and returns its
// UUID string. Mirrors the INSERT shape used in
// server/internal/handler/squad_assign_trigger_test.go.
func createTestSquad(t *testing.T, workspaceID, creatorID, name string) string {
t.Helper()
ctx := context.Background()
var leaderID string
if err := testPool.QueryRow(ctx, `
SELECT id::text FROM agent WHERE workspace_id = $1 ORDER BY created_at ASC LIMIT 1
`, workspaceID).Scan(&leaderID); err != nil {
t.Fatalf("load fixture agent for squad leader: %v", err)
}
var squadID string
if err := testPool.QueryRow(ctx, `
INSERT INTO squad (workspace_id, name, description, leader_id, creator_id)
VALUES ($1, $2, '', $3, $4)
RETURNING id
`, workspaceID, name, leaderID, creatorID).Scan(&squadID); err != nil {
t.Fatalf("createTestSquad: %v", err)
}
return squadID
}

func cleanupTestSquad(t *testing.T, squadID string) {
t.Helper()
testPool.Exec(context.Background(), `DELETE FROM squad WHERE id = $1`, squadID)
}

func isSubscribed(t *testing.T, queries *db.Queries, issueID, userType, userID string) bool {
t.Helper()
subscribed, err := queries.IsIssueSubscriber(context.Background(), db.IsIssueSubscriberParams{
Expand Down Expand Up @@ -415,3 +443,105 @@ func TestParseUUIDConsistency(t *testing.T) {
}()
_ = parseUUID("")
}

// TestSubscriberIssueCreated_SquadAssigneeNotSubscribed guards the fix for
// the squad-assignee CHECK-constraint noise: when an issue is created with
// assignee_type='squad', the subscriber listener must NOT call
// AddIssueSubscriber with user_type='squad' (which the DB rejects with
// SQLSTATE 23514 — cf. the author_type='system' gate for MUL-2538). The
// squad leader is dispatched via EnqueueTaskForSquadLeader instead, so a
// subscriber row is both impossible and redundant.
//
// Same caveat as TestSubscriberSystemCommentDoesNotSubscribe: the behavioral
// assertion (no squad row) holds either way because the insert fails, but
// the test pins the invariant and will fail loudly if anyone widens the
// issue_subscriber.user_type CHECK to include 'squad' without also revisiting
// this listener.
func TestSubscriberIssueCreated_SquadAssigneeNotSubscribed(t *testing.T) {
queries := db.New(testPool)
bus := events.New()
registerSubscriberListeners(bus, queries)

squadID := createTestSquad(t, testWorkspaceID, testUserID, "Subscriber Squad (created)")
t.Cleanup(func() { cleanupTestSquad(t, squadID) })

issueID := createTestIssue(t, testWorkspaceID, testUserID)
t.Cleanup(func() { cleanupTestIssue(t, issueID) })

assigneeType := "squad"
bus.Publish(events.Event{
Type: protocol.EventIssueCreated,
WorkspaceID: testWorkspaceID,
ActorType: "member",
ActorID: testUserID,
Payload: map[string]any{
"issue": handler.IssueResponse{
ID: issueID,
WorkspaceID: testWorkspaceID,
Title: "squad-assigned at creation",
Status: "todo",
Priority: "medium",
CreatorType: "member",
CreatorID: testUserID,
AssigneeType: &assigneeType,
AssigneeID: &squadID,
},
},
})

// Creator is the only subscriber — the squad assignee must not produce
// a second row (nor a failed insert logged at error level).
if isSubscribed(t, queries, issueID, "squad", squadID) {
t.Fatal("squad assignee must not be subscribed as user_type='squad'")
}
if count := subscriberCount(t, queries, issueID); count != 1 {
t.Fatalf("expected 1 subscriber (creator only) for squad-assigned issue, got %d", count)
}
if !isSubscribed(t, queries, issueID, "member", testUserID) {
t.Fatal("creator must still be subscribed on a squad-assigned issue")
}
}

// TestSubscriberIssueUpdated_SquadAssigneeNotSubscribed is the issue:updated
// analog: reassigning an issue to a squad must not try to insert a squad
// subscriber row.
func TestSubscriberIssueUpdated_SquadAssigneeNotSubscribed(t *testing.T) {
queries := db.New(testPool)
bus := events.New()
registerSubscriberListeners(bus, queries)

squadID := createTestSquad(t, testWorkspaceID, testUserID, "Subscriber Squad (updated)")
t.Cleanup(func() { cleanupTestSquad(t, squadID) })

issueID := createTestIssue(t, testWorkspaceID, testUserID)
t.Cleanup(func() { cleanupTestIssue(t, issueID) })

assigneeType := "squad"
bus.Publish(events.Event{
Type: protocol.EventIssueUpdated,
WorkspaceID: testWorkspaceID,
ActorType: "member",
ActorID: testUserID,
Payload: map[string]any{
"issue": handler.IssueResponse{
ID: issueID,
WorkspaceID: testWorkspaceID,
Title: "reassigned to squad",
Status: "todo",
Priority: "medium",
CreatorType: "member",
CreatorID: testUserID,
AssigneeType: &assigneeType,
AssigneeID: &squadID,
},
"assignee_changed": true,
},
})

if isSubscribed(t, queries, issueID, "squad", squadID) {
t.Fatal("squad assignee must not be subscribed as user_type='squad' after reassignment")
}
if count := subscriberCount(t, queries, issueID); count != 0 {
t.Fatalf("expected 0 subscribers when only a squad assignee change fired, got %d", count)
}
}
Loading