diff --git a/server/cmd/server/comment_trigger_integration_test.go b/server/cmd/server/comment_trigger_integration_test.go new file mode 100644 index 00000000..a353ea70 --- /dev/null +++ b/server/cmd/server/comment_trigger_integration_test.go @@ -0,0 +1,332 @@ +package main + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "testing" +) + +// authRequestWithAgent makes an authenticated request with X-Agent-ID header, +// causing the server to resolve the actor as an agent instead of a member. +func authRequestWithAgent(t *testing.T, method, path string, body any, agentID string) *http.Response { + t.Helper() + resp := authRequest(t, method, path, body) + // We can't add headers after authRequest, so we build it manually: + resp.Body.Close() + + var bodyReader io.Reader + if body != nil { + b, _ := json.Marshal(body) + bodyReader = &readCloserWrapper{data: b} + } + req, err := http.NewRequest(method, testServer.URL+path, bodyReader) + if err != nil { + t.Fatalf("failed to create request: %v", err) + } + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", "Bearer "+testToken) + req.Header.Set("X-Workspace-ID", testWorkspaceID) + req.Header.Set("X-Agent-ID", agentID) + + r, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("request failed: %v", err) + } + return r +} + +type readCloserWrapper struct { + data []byte + pos int +} + +func (r *readCloserWrapper) Read(p []byte) (int, error) { + if r.pos >= len(r.data) { + return 0, io.EOF + } + n := copy(p, r.data[r.pos:]) + r.pos += n + return n, nil +} + +// countPendingTasks returns the number of queued/dispatched tasks for an issue. +func countPendingTasks(t *testing.T, issueID string) int { + t.Helper() + var count int + err := testPool.QueryRow(context.Background(), + `SELECT count(*) FROM agent_task_queue WHERE issue_id = $1 AND status IN ('queued', 'dispatched')`, + issueID).Scan(&count) + if err != nil { + t.Fatalf("failed to count pending tasks: %v", err) + } + return count +} + +// clearTasks deletes all tasks for an issue (cleanup between subtests). +func clearTasks(t *testing.T, issueID string) { + t.Helper() + _, err := testPool.Exec(context.Background(), + `DELETE FROM agent_task_queue WHERE issue_id = $1`, issueID) + if err != nil { + t.Fatalf("failed to clear tasks: %v", err) + } +} + +// getAgentID returns the ID of the first agent in the test workspace. +func getAgentID(t *testing.T) string { + t.Helper() + resp := authRequest(t, "GET", "/api/agents?workspace_id="+testWorkspaceID, nil) + var agents []map[string]any + readJSON(t, resp, &agents) + if len(agents) == 0 { + t.Fatal("no agents in test workspace") + } + return agents[0]["id"].(string) +} + +// createIssueAssignedToAgent creates a todo issue assigned to the given agent. +func createIssueAssignedToAgent(t *testing.T, title, agentID string) string { + t.Helper() + resp := authRequest(t, "PUT", fmt.Sprintf("/api/issues/%s", createIssue(t, title)), map[string]any{ + "assignee_type": "agent", + "assignee_id": agentID, + }) + var issue map[string]any + readJSON(t, resp, &issue) + return issue["id"].(string) +} + +// createIssue creates a basic todo issue and returns its ID. +func createIssue(t *testing.T, title string) string { + t.Helper() + resp := authRequest(t, "POST", "/api/issues?workspace_id="+testWorkspaceID, map[string]any{ + "title": title, + "status": "todo", + }) + if resp.StatusCode != 201 { + body, _ := io.ReadAll(resp.Body) + resp.Body.Close() + t.Fatalf("CreateIssue: expected 201, got %d: %s", resp.StatusCode, body) + } + var issue map[string]any + readJSON(t, resp, &issue) + return issue["id"].(string) +} + +// postComment posts a comment as the test member. +func postComment(t *testing.T, issueID, content string, parentID *string) string { + t.Helper() + body := map[string]any{ + "content": content, + "type": "comment", + } + if parentID != nil { + body["parent_id"] = *parentID + } + resp := authRequest(t, "POST", "/api/issues/"+issueID+"/comments", body) + if resp.StatusCode != 201 { + b, _ := io.ReadAll(resp.Body) + resp.Body.Close() + t.Fatalf("postComment: expected 201, got %d: %s", resp.StatusCode, b) + } + var comment map[string]any + readJSON(t, resp, &comment) + return comment["id"].(string) +} + +// postCommentAsAgent posts a comment with the X-Agent-ID header. +func postCommentAsAgent(t *testing.T, issueID, content, agentID string, parentID *string) string { + t.Helper() + body := map[string]any{ + "content": content, + "type": "comment", + } + if parentID != nil { + body["parent_id"] = *parentID + } + resp := authRequestWithAgent(t, "POST", "/api/issues/"+issueID+"/comments", body, agentID) + if resp.StatusCode != 201 { + b, _ := io.ReadAll(resp.Body) + resp.Body.Close() + t.Fatalf("postCommentAsAgent: expected 201, got %d: %s", resp.StatusCode, b) + } + var comment map[string]any + readJSON(t, resp, &comment) + return comment["id"].(string) +} + +// strPtr returns a pointer to a string. +func strPtr(s string) *string { return &s } + +// TestCommentTriggerOnComment tests on_comment trigger scenarios end-to-end. +// Verifies that the agent task queue is populated correctly based on: +// - top-level vs threaded comments +// - member vs agent thread starters +// - presence/absence of @mentions +func TestCommentTriggerOnComment(t *testing.T) { + agentID := getAgentID(t) + issueID := createIssueAssignedToAgent(t, "Comment trigger integration test", agentID) + t.Cleanup(func() { + clearTasks(t, issueID) + resp := authRequest(t, "DELETE", "/api/issues/"+issueID, nil) + resp.Body.Close() + }) + + t.Run("top-level comment without mentions triggers agent", func(t *testing.T) { + clearTasks(t, issueID) + postComment(t, issueID, "Please fix this bug", nil) + if n := countPendingTasks(t, issueID); n != 1 { + t.Errorf("expected 1 pending task, got %d", n) + } + }) + + t.Run("top-level comment mentioning only others suppresses trigger", func(t *testing.T) { + clearTasks(t, issueID) + // Mention a fake agent UUID that is not the assignee. + content := "[@SomeoneElse](mention://agent/00000000-0000-0000-0000-000000000001) what do you think?" + postComment(t, issueID, content, nil) + if n := countPendingTasks(t, issueID); n != 0 { + t.Errorf("expected 0 pending tasks, got %d", n) + } + }) + + t.Run("top-level comment mentioning assignee triggers agent", func(t *testing.T) { + clearTasks(t, issueID) + content := fmt.Sprintf("[@Agent](mention://agent/%s) fix this", agentID) + postComment(t, issueID, content, nil) + if n := countPendingTasks(t, issueID); n != 1 { + t.Errorf("expected 1 pending task, got %d", n) + } + }) + + t.Run("reply to agent thread without mentions triggers agent", func(t *testing.T) { + clearTasks(t, issueID) + // Agent starts a thread. + threadID := postCommentAsAgent(t, issueID, "I analyzed the issue.", agentID, nil) + // Member replies in the agent's thread. + postComment(t, issueID, "Looks good, please proceed", strPtr(threadID)) + if n := countPendingTasks(t, issueID); n != 1 { + t.Errorf("expected 1 pending task, got %d", n) + } + }) + + t.Run("reply to member thread without mentions suppresses trigger", func(t *testing.T) { + clearTasks(t, issueID) + // Member starts a thread. + threadID := postComment(t, issueID, "Hey team, what do you think?", nil) + // Clear the task that was created by the top-level comment. + clearTasks(t, issueID) + // Another member reply (same user in this test, but the key is parent is by member). + postComment(t, issueID, "I agree with you", strPtr(threadID)) + if n := countPendingTasks(t, issueID); n != 0 { + t.Errorf("expected 0 pending tasks (member-to-member reply), got %d", n) + } + }) + + t.Run("reply to member thread mentioning assignee triggers agent", func(t *testing.T) { + clearTasks(t, issueID) + // Member starts a thread. + threadID := postComment(t, issueID, "Question about this", nil) + clearTasks(t, issueID) + // Reply mentioning the assignee agent. + content := fmt.Sprintf("[@Agent](mention://agent/%s) can you help with this?", agentID) + postComment(t, issueID, content, strPtr(threadID)) + if n := countPendingTasks(t, issueID); n != 0 { + // The mention of the assignee agent unblocks on_comment but + // the assignee-mention path in on_mention skips the assignee. + // Either 0 or 1 is acceptable depending on the on_comment logic. + // With our implementation: isReplyToMemberThread returns false + // (assignee mentioned), and commentMentionsOthersButNotAssignee + // returns false (assignee is mentioned). So on_comment triggers. + // Let's re-check. + } + if n := countPendingTasks(t, issueID); n != 1 { + t.Errorf("expected 1 pending task (assignee mentioned in member thread), got %d", n) + } + }) +} + +// TestCommentTriggerOnAssignNoStatusGate verifies that assigning an agent to +// a non-todo issue still triggers the agent (status gate was removed). +func TestCommentTriggerOnAssignNoStatusGate(t *testing.T) { + agentID := getAgentID(t) + + // Create an in_progress issue. + issueID := createIssue(t, "On-assign status gate test") + resp := authRequest(t, "PUT", "/api/issues/"+issueID, map[string]any{ + "status": "in_progress", + }) + resp.Body.Close() + + t.Cleanup(func() { + clearTasks(t, issueID) + resp := authRequest(t, "DELETE", "/api/issues/"+issueID, nil) + resp.Body.Close() + }) + + // Assign the agent — should trigger despite non-todo status. + resp = authRequest(t, "PUT", "/api/issues/"+issueID, map[string]any{ + "assignee_type": "agent", + "assignee_id": agentID, + }) + if resp.StatusCode != 200 { + body, _ := io.ReadAll(resp.Body) + resp.Body.Close() + t.Fatalf("assign agent: expected 200, got %d: %s", resp.StatusCode, body) + } + resp.Body.Close() + + if n := countPendingTasks(t, issueID); n != 1 { + t.Errorf("expected 1 pending task after assigning to in_progress issue, got %d", n) + } +} + +// TestCommentTriggerOnMentionNoStatusGate verifies that @mentioning an agent +// on a done issue still triggers the agent (no status gate on on_mention). +func TestCommentTriggerOnMentionNoStatusGate(t *testing.T) { + agentID := getAgentID(t) + + // Create a done issue (not assigned to agent). + issueID := createIssue(t, "On-mention done issue test") + resp := authRequest(t, "PUT", "/api/issues/"+issueID, map[string]any{ + "status": "done", + }) + resp.Body.Close() + + t.Cleanup(func() { + clearTasks(t, issueID) + resp := authRequest(t, "DELETE", "/api/issues/"+issueID, nil) + resp.Body.Close() + }) + + // @mention the agent on a done issue — should still trigger. + content := fmt.Sprintf("[@Agent](mention://agent/%s) found a problem here", agentID) + postComment(t, issueID, content, nil) + + if n := countPendingTasks(t, issueID); n != 1 { + t.Errorf("expected 1 pending task after @mention on done issue, got %d", n) + } +} + +// TestCommentTriggerCoalescing verifies that rapid-fire comments don't create +// duplicate tasks (coalescing dedup). +func TestCommentTriggerCoalescing(t *testing.T) { + agentID := getAgentID(t) + issueID := createIssueAssignedToAgent(t, "Coalescing test", agentID) + t.Cleanup(func() { + clearTasks(t, issueID) + resp := authRequest(t, "DELETE", "/api/issues/"+issueID, nil) + resp.Body.Close() + }) + + // Post two comments rapidly — only 1 task should be created (coalescing). + postComment(t, issueID, "First comment", nil) + postComment(t, issueID, "Second comment", nil) + + if n := countPendingTasks(t, issueID); n != 1 { + t.Errorf("expected 1 pending task (coalescing), got %d", n) + } +} diff --git a/server/internal/handler/agent.go b/server/internal/handler/agent.go index d2bab8eb..0818e8e2 100644 --- a/server/internal/handler/agent.go +++ b/server/internal/handler/agent.go @@ -275,7 +275,7 @@ func (h *Handler) CreateAgent(w http.ResponseWriter, r *http.Request) { triggers, _ := json.Marshal(req.Triggers) if req.Triggers == nil { - triggers = []byte("[]") + triggers = defaultAgentTriggers() } agent, err := h.Queries.CreateAgent(r.Context(), db.CreateAgentParams{ diff --git a/server/internal/handler/comment.go b/server/internal/handler/comment.go index c4f7690a..9f10aed8 100644 --- a/server/internal/handler/comment.go +++ b/server/internal/handler/comment.go @@ -116,6 +116,7 @@ func (h *Handler) CreateComment(w http.ResponseWriter, r *http.Request) { } var parentID pgtype.UUID + var parentComment *db.Comment if req.ParentID != nil { parentID = parseUUID(*req.ParentID) parent, err := h.Queries.GetComment(r.Context(), parentID) @@ -123,6 +124,7 @@ func (h *Handler) CreateComment(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "invalid parent comment") return } + parentComment = &parent } // Determine author identity: agent (via X-Agent-ID header) or member. @@ -164,8 +166,11 @@ func (h *Handler) CreateComment(w http.ResponseWriter, r *http.Request) { // Skip when the comment comes from the assigned agent itself to avoid loops. // Also skip when the comment @mentions others but not the assignee agent — // the user is talking to someone else, not requesting work from the assignee. + // Also skip when replying in a member-started thread without mentioning the + // assignee — the user is continuing a member-to-member conversation. if authorType == "member" && h.shouldEnqueueOnComment(r.Context(), issue) && - !h.commentMentionsOthersButNotAssignee(comment.Content, issue) { + !h.commentMentionsOthersButNotAssignee(comment.Content, issue) && + !h.isReplyToMemberThread(parentComment, comment.Content, issue) { // Resolve thread root: if the comment is a reply, agent should reply // to the thread root (matching frontend behavior where all replies // in a thread share the same top-level parent). @@ -208,10 +213,39 @@ func (h *Handler) commentMentionsOthersButNotAssignee(content string, issue db.I return true // Others mentioned but not assignee — suppress trigger } +// isReplyToMemberThread returns true if the comment is a reply in a thread +// started by a member and does NOT @mention the issue's assignee agent. +// When a member replies in a member-started thread, they are most likely +// continuing a human conversation — not requesting work from the assigned agent. +// Replying to an agent-started thread, or explicitly @mentioning the assignee +// in the reply, still triggers on_comment as expected. +func (h *Handler) isReplyToMemberThread(parent *db.Comment, content string, issue db.Issue) bool { + if parent == nil { + return false // Not a reply — normal top-level comment + } + if parent.AuthorType != "member" { + return false // Thread started by an agent — allow trigger + } + // Thread was started by a member. Suppress on_comment unless the reply + // explicitly @mentions the assignee agent. + if !issue.AssigneeID.Valid { + return true // No assignee to mention + } + assigneeID := uuidToString(issue.AssigneeID) + for _, m := range util.ParseMentions(content) { + if m.ID == assigneeID { + return false // Assignee explicitly mentioned — allow trigger + } + } + return true // Reply to member thread without mentioning agent — suppress +} + // enqueueMentionedAgentTasks parses @agent mentions from comment content and // enqueues a task for each mentioned agent. Skips self-mentions, agents that // are already the issue's assignee (handled by on_comment), and agents with // on_mention trigger disabled. +// Note: no status gate here — @mention is an explicit action and should work +// even on done/cancelled issues (the agent can reopen the issue if needed). func (h *Handler) enqueueMentionedAgentTasks(ctx context.Context, issue db.Issue, comment db.Comment, authorType, authorID string) { mentions := util.ParseMentions(comment.Content) for _, m := range mentions { diff --git a/server/internal/handler/issue.go b/server/internal/handler/issue.go index 0c5a0d6a..3a74d830 100644 --- a/server/internal/handler/issue.go +++ b/server/internal/handler/issue.go @@ -45,6 +45,17 @@ type agentTriggerSnapshot struct { Config map[string]any `json:"config"` } +// defaultAgentTriggers returns the default trigger config for new agents: +// all three triggers explicitly enabled. +func defaultAgentTriggers() []byte { + b, _ := json.Marshal([]agentTriggerSnapshot{ + {Type: "on_assign", Enabled: true}, + {Type: "on_comment", Enabled: true}, + {Type: "on_mention", Enabled: true}, + }) + return b +} + func issueToResponse(i db.Issue, issuePrefix string) IssueResponse { identifier := issuePrefix + "-" + strconv.Itoa(int(i.Number)) return IssueResponse{ @@ -472,18 +483,19 @@ func (h *Handler) canAssignAgent(ctx context.Context, r *http.Request, agentID, return false, "cannot assign to private agent" } +// shouldEnqueueAgentTask returns true when an issue assignment should trigger +// the assigned agent. No status gate — assignment is an explicit human action, +// so it should trigger regardless of issue status (e.g. assigning an agent to +// a done issue to fix a discovered problem). func (h *Handler) shouldEnqueueAgentTask(ctx context.Context, issue db.Issue) bool { - if issue.Status != "todo" { - return false - } return h.isAgentTriggerEnabled(ctx, issue, "on_assign") } // shouldEnqueueOnComment returns true if a member comment on this issue should -// trigger the assigned agent. Conditions: issue is assigned to an agent, the -// agent has on_comment trigger enabled, and no task is already active. +// trigger the assigned agent. Fires for any non-terminal status — comments are +// conversational and can happen at any stage of active work. func (h *Handler) shouldEnqueueOnComment(ctx context.Context, issue db.Issue) bool { - // Don't trigger on terminal statuses. + // Don't trigger on terminal statuses (done, cancelled). if issue.Status == "done" || issue.Status == "cancelled" { return false } @@ -502,7 +514,7 @@ func (h *Handler) shouldEnqueueOnComment(ctx context.Context, issue db.Issue) bo // isAgentTriggerEnabled checks if an issue is assigned to an agent with a // specific trigger type enabled. Returns true if the agent has no triggers -// configured (default-enabled behavior). +// configured (default-enabled behavior for backwards compatibility). func (h *Handler) isAgentTriggerEnabled(ctx context.Context, issue db.Issue, triggerType string) bool { if !issue.AssigneeType.Valid || issue.AssigneeType.String != "agent" || !issue.AssigneeID.Valid { return false @@ -512,20 +524,8 @@ func (h *Handler) isAgentTriggerEnabled(ctx context.Context, issue db.Issue, tri if err != nil || !agent.RuntimeID.Valid { return false } - if agent.Triggers == nil || len(agent.Triggers) == 0 { - return true - } - var triggers []agentTriggerSnapshot - if err := json.Unmarshal(agent.Triggers, &triggers); err != nil { - return false - } - for _, trigger := range triggers { - if trigger.Type == triggerType && trigger.Enabled { - return true - } - } - return false + return agentHasTriggerEnabled(agent.Triggers, triggerType) } // isAgentMentionTriggerEnabled checks if a specific agent has the on_mention @@ -536,20 +536,32 @@ func (h *Handler) isAgentMentionTriggerEnabled(ctx context.Context, agentID pgty if err != nil || !agent.RuntimeID.Valid { return false } - if agent.Triggers == nil || len(agent.Triggers) == 0 { - return true // No config = all triggers enabled by default + + return agentHasTriggerEnabled(agent.Triggers, "on_mention") +} + +// agentHasTriggerEnabled checks if a trigger type is enabled in the agent's +// trigger config. Returns true (default-enabled) when the triggers list is +// empty or does not contain the requested type — for backwards compatibility +// with agents created before explicit trigger config was introduced. +func agentHasTriggerEnabled(raw []byte, triggerType string) bool { + if raw == nil || len(raw) == 0 { + return true } var triggers []agentTriggerSnapshot - if err := json.Unmarshal(agent.Triggers, &triggers); err != nil { + if err := json.Unmarshal(raw, &triggers); err != nil { return false } + if len(triggers) == 0 { + return true // Empty array = default-enabled (backwards compat) + } for _, trigger := range triggers { - if trigger.Type == "on_mention" { + if trigger.Type == triggerType { return trigger.Enabled } } - return true // on_mention not configured = enabled by default + return true // Trigger type not configured = enabled by default } func (h *Handler) DeleteIssue(w http.ResponseWriter, r *http.Request) { diff --git a/server/internal/handler/trigger_test.go b/server/internal/handler/trigger_test.go new file mode 100644 index 00000000..82db7d57 --- /dev/null +++ b/server/internal/handler/trigger_test.go @@ -0,0 +1,333 @@ +package handler + +import ( + "encoding/json" + "fmt" + "testing" + + "github.com/jackc/pgx/v5/pgtype" + db "github.com/multica-ai/multica/server/pkg/db/generated" +) + +// Helper to build a pgtype.UUID from a string. +func testUUID(s string) pgtype.UUID { + return parseUUID(s) +} + +// Helper to build a pgtype.Text. +func testText(s string) pgtype.Text { + return pgtype.Text{String: s, Valid: true} +} + +const ( + agentAssigneeID = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" + otherAgentID = "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb" + memberID = "cccccccc-cccc-cccc-cccc-cccccccccccc" + otherMemberID = "dddddddd-dddd-dddd-dddd-dddddddddddd" +) + +func issueWithAgentAssignee() db.Issue { + return db.Issue{ + AssigneeType: testText("agent"), + AssigneeID: testUUID(agentAssigneeID), + } +} + +func issueNoAssignee() db.Issue { + return db.Issue{} +} + +// ------------------------------------------------------------------- +// commentMentionsOthersButNotAssignee +// ------------------------------------------------------------------- + +func TestCommentMentionsOthersButNotAssignee(t *testing.T) { + h := &Handler{} // nil handler — method doesn't use h + + issue := issueWithAgentAssignee() + + tests := []struct { + name string + content string + want bool + }{ + { + name: "no mentions → allow trigger", + content: "just a plain comment", + want: false, + }, + { + name: "mentions assignee → allow trigger", + content: fmt.Sprintf("[@Agent](mention://agent/%s) please fix", agentAssigneeID), + want: false, + }, + { + name: "mentions other agent only → suppress", + content: fmt.Sprintf("[@Other](mention://agent/%s) what do you think?", otherAgentID), + want: true, + }, + { + name: "mentions other member only → suppress", + content: fmt.Sprintf("[@Bob](mention://member/%s) take a look", memberID), + want: true, + }, + { + name: "mentions both assignee and other → allow trigger", + content: fmt.Sprintf("[@Agent](mention://agent/%s) and [@Other](mention://agent/%s)", agentAssigneeID, otherAgentID), + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := h.commentMentionsOthersButNotAssignee(tt.content, issue) + if got != tt.want { + t.Errorf("commentMentionsOthersButNotAssignee() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestCommentMentionsOthersButNotAssignee_NoAssignee(t *testing.T) { + h := &Handler{} + issue := issueNoAssignee() + + // Any mention on an unassigned issue → suppress + content := fmt.Sprintf("[@Agent](mention://agent/%s) help", otherAgentID) + if got := h.commentMentionsOthersButNotAssignee(content, issue); !got { + t.Errorf("expected true for mentions on unassigned issue, got false") + } +} + +// ------------------------------------------------------------------- +// isReplyToMemberThread +// ------------------------------------------------------------------- + +func TestIsReplyToMemberThread(t *testing.T) { + h := &Handler{} + issue := issueWithAgentAssignee() + + memberParent := &db.Comment{AuthorType: "member", AuthorID: testUUID(memberID)} + agentParent := &db.Comment{AuthorType: "agent", AuthorID: testUUID(agentAssigneeID)} + + tests := []struct { + name string + parent *db.Comment + content string + want bool + }{ + { + name: "top-level comment (nil parent) → allow", + parent: nil, + content: "a comment", + want: false, + }, + { + name: "reply to agent thread, no mentions → allow", + parent: agentParent, + content: "sounds good", + want: false, + }, + { + name: "reply to agent thread, mention other member → allow (handled by other check)", + parent: agentParent, + content: fmt.Sprintf("[@Bob](mention://member/%s) thoughts?", memberID), + want: false, // isReplyToMemberThread only checks member threads + }, + { + name: "reply to member thread, no mentions → suppress", + parent: memberParent, + content: "I agree with you", + want: true, + }, + { + name: "reply to member thread, mention other member → suppress", + parent: memberParent, + content: fmt.Sprintf("[@Alice](mention://member/%s) what about this?", otherMemberID), + want: true, + }, + { + name: "reply to member thread, mention assignee agent → allow", + parent: memberParent, + content: fmt.Sprintf("[@Agent](mention://agent/%s) can you help?", agentAssigneeID), + want: false, + }, + { + name: "reply to member thread, mention other agent (not assignee) → suppress", + parent: memberParent, + content: fmt.Sprintf("[@Other](mention://agent/%s) take a look", otherAgentID), + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := h.isReplyToMemberThread(tt.parent, tt.content, issue) + if got != tt.want { + t.Errorf("isReplyToMemberThread() = %v, want %v", got, tt.want) + } + }) + } +} + +// ------------------------------------------------------------------- +// Combined trigger decision (simulates the full on_comment check) +// ------------------------------------------------------------------- + +func TestOnCommentTriggerDecision(t *testing.T) { + h := &Handler{} + issue := issueWithAgentAssignee() + + memberParent := &db.Comment{AuthorType: "member", AuthorID: testUUID(memberID)} + agentParent := &db.Comment{AuthorType: "agent", AuthorID: testUUID(agentAssigneeID)} + + // Simulates the combined check from CreateComment: + // !commentMentionsOthersButNotAssignee && !isReplyToMemberThread + shouldTrigger := func(parent *db.Comment, content string) bool { + return !h.commentMentionsOthersButNotAssignee(content, issue) && + !h.isReplyToMemberThread(parent, content, issue) + } + + tests := []struct { + name string + parent *db.Comment + content string + want bool + }{ + {"top-level, no mention", nil, "hello agent", true}, + {"top-level, mention assignee", nil, fmt.Sprintf("[@Agent](mention://agent/%s) fix this", agentAssigneeID), true}, + {"top-level, mention other only", nil, fmt.Sprintf("[@Other](mention://agent/%s) look", otherAgentID), false}, + {"reply agent thread, no mention", agentParent, "got it", true}, + {"reply agent thread, mention other member", agentParent, fmt.Sprintf("[@Bob](mention://member/%s) ?", memberID), false}, + {"reply agent thread, mention assignee", agentParent, fmt.Sprintf("[@Agent](mention://agent/%s) yes", agentAssigneeID), true}, + {"reply member thread, no mention", memberParent, "agreed", false}, + {"reply member thread, mention other member", memberParent, fmt.Sprintf("[@Bob](mention://member/%s) ok", memberID), false}, + {"reply member thread, mention assignee", memberParent, fmt.Sprintf("[@Agent](mention://agent/%s) help", agentAssigneeID), true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := shouldTrigger(tt.parent, tt.content) + if got != tt.want { + t.Errorf("shouldTrigger() = %v, want %v", got, tt.want) + } + }) + } +} + +// ------------------------------------------------------------------- +// agentHasTriggerEnabled +// ------------------------------------------------------------------- + +func TestAgentHasTriggerEnabled(t *testing.T) { + tests := []struct { + name string + raw []byte + triggerType string + want bool + }{ + { + name: "nil triggers → enabled (backwards compat)", + raw: nil, + triggerType: "on_comment", + want: true, + }, + { + name: "empty byte slice → enabled", + raw: []byte{}, + triggerType: "on_comment", + want: true, + }, + { + name: "empty JSON array → enabled (backwards compat)", + raw: []byte("[]"), + triggerType: "on_comment", + want: true, + }, + { + name: "on_comment explicitly enabled", + raw: mustJSON([]agentTriggerSnapshot{{Type: "on_comment", Enabled: true}}), + triggerType: "on_comment", + want: true, + }, + { + name: "on_comment explicitly disabled", + raw: mustJSON([]agentTriggerSnapshot{{Type: "on_comment", Enabled: false}}), + triggerType: "on_comment", + want: false, + }, + { + name: "on_mention not configured but others are → enabled by default", + raw: mustJSON([]agentTriggerSnapshot{{Type: "on_comment", Enabled: true}}), + triggerType: "on_mention", + want: true, + }, + { + name: "invalid JSON → disabled (fail safe)", + raw: []byte("{bad json"), + triggerType: "on_comment", + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := agentHasTriggerEnabled(tt.raw, tt.triggerType) + if got != tt.want { + t.Errorf("agentHasTriggerEnabled() = %v, want %v", got, tt.want) + } + }) + } +} + +// ------------------------------------------------------------------- +// defaultAgentTriggers +// ------------------------------------------------------------------- + +func TestDefaultAgentTriggers(t *testing.T) { + raw := defaultAgentTriggers() + + var triggers []agentTriggerSnapshot + if err := json.Unmarshal(raw, &triggers); err != nil { + t.Fatalf("failed to unmarshal default triggers: %v", err) + } + + if len(triggers) != 3 { + t.Fatalf("expected 3 default triggers, got %d", len(triggers)) + } + + expected := map[string]bool{ + "on_assign": true, + "on_comment": true, + "on_mention": true, + } + for _, tr := range triggers { + want, ok := expected[tr.Type] + if !ok { + t.Errorf("unexpected trigger type: %s", tr.Type) + continue + } + if tr.Enabled != want { + t.Errorf("trigger %s: enabled = %v, want %v", tr.Type, tr.Enabled, want) + } + delete(expected, tr.Type) + } + for typ := range expected { + t.Errorf("missing trigger type: %s", typ) + } + + // Verify all triggers are enabled via agentHasTriggerEnabled + for _, typ := range []string{"on_assign", "on_comment", "on_mention"} { + if !agentHasTriggerEnabled(raw, typ) { + t.Errorf("agentHasTriggerEnabled(default, %q) = false, want true", typ) + } + } +} + +func mustJSON(v any) []byte { + b, err := json.Marshal(v) + if err != nil { + panic(err) + } + return b +}