From 04da4bc3b752defa3d8e5ecbbcfe61855d2898e1 Mon Sep 17 00:00:00 2001 From: Jiayuan Date: Wed, 1 Apr 2026 21:37:33 +0800 Subject: [PATCH 1/4] fix(server): improve comment trigger logic for agent execution - Add thread-aware on_comment suppression: when a member replies in a thread started by another member without @mentioning the assignee agent, the on_comment trigger is now suppressed. This fixes the bug where member-to-member conversations incorrectly triggered the assigned agent. - Add terminal status check to on_mention: enqueueMentionedAgentTasks now skips done/cancelled issues, consistent with on_comment behavior. - Write explicit default triggers on agent creation: new agents get [on_assign, on_comment, on_mention] all enabled, instead of relying on null/empty = all enabled. Existing agents with empty triggers still work via backwards-compat fallback in agentHasTriggerEnabled. - Consolidate trigger check logic into shared agentHasTriggerEnabled helper, fixing inconsistency where empty [] was handled differently by isAgentTriggerEnabled (returned false) vs isAgentMentionTriggerEnabled (returned true). - Add documentation comments explaining the intentional status gate difference: on_assign fires only for todo (start new work), while on_comment fires for any non-terminal status (conversational). --- server/internal/handler/agent.go | 2 +- server/internal/handler/comment.go | 34 ++++++++++++++++- server/internal/handler/issue.go | 60 +++++++++++++++++++----------- 3 files changed, 72 insertions(+), 24 deletions(-) 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 93cf8f3f..83929845 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. @@ -162,8 +164,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). @@ -203,6 +208,33 @@ 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 diff --git a/server/internal/handler/issue.go b/server/internal/handler/issue.go index 0cf7dc17..cd38e338 100644 --- a/server/internal/handler/issue.go +++ b/server/internal/handler/issue.go @@ -44,6 +44,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{ @@ -459,6 +470,10 @@ 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. Only fires for "todo" status — assignment means "start +// fresh work", so the agent shouldn't be triggered on issues already in +// progress, blocked, or done. func (h *Handler) shouldEnqueueAgentTask(ctx context.Context, issue db.Issue) bool { if issue.Status != "todo" { return false @@ -467,10 +482,11 @@ func (h *Handler) shouldEnqueueAgentTask(ctx context.Context, issue db.Issue) bo } // 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. Unlike on_assign (todo only), on_comment fires +// for any non-terminal status — comments are conversational and can happen at +// any stage of active work (todo, in_progress, in_review, blocked). 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 } @@ -489,7 +505,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 @@ -499,20 +515,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 @@ -523,20 +527,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) { From eb5aaf003c03bd9aea8514c258cb4b87272ba7a3 Mon Sep 17 00:00:00 2001 From: Jiayuan Date: Wed, 1 Apr 2026 21:46:20 +0800 Subject: [PATCH 2/4] fix(server): remove status gates from on_assign and on_mention triggers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit on_assign: Remove the todo-only restriction. Assignment is an explicit human action — if someone assigns an agent to a done/in_progress issue, they want the agent triggered (e.g. to fix a problem found after close). on_mention: Remove the done/cancelled check. @mention is an explicit action and should work on any issue status. The agent can reopen the issue if needed. --- server/internal/handler/comment.go | 7 ++----- server/internal/handler/issue.go | 14 +++++--------- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/server/internal/handler/comment.go b/server/internal/handler/comment.go index 83929845..a21bb61c 100644 --- a/server/internal/handler/comment.go +++ b/server/internal/handler/comment.go @@ -239,12 +239,9 @@ func (h *Handler) isReplyToMemberThread(parent *db.Comment, content string, issu // 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) { - // Don't trigger on terminal statuses. - if issue.Status == "done" || issue.Status == "cancelled" { - return - } - mentions := util.ParseMentions(comment.Content) for _, m := range mentions { if m.Type != "agent" { diff --git a/server/internal/handler/issue.go b/server/internal/handler/issue.go index cd38e338..71da2edd 100644 --- a/server/internal/handler/issue.go +++ b/server/internal/handler/issue.go @@ -471,20 +471,16 @@ func (h *Handler) canAssignAgent(ctx context.Context, r *http.Request, agentID, } // shouldEnqueueAgentTask returns true when an issue assignment should trigger -// the assigned agent. Only fires for "todo" status — assignment means "start -// fresh work", so the agent shouldn't be triggered on issues already in -// progress, blocked, or done. +// 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. Unlike on_assign (todo only), on_comment fires -// for any non-terminal status — comments are conversational and can happen at -// any stage of active work (todo, in_progress, in_review, blocked). +// 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 (done, cancelled). if issue.Status == "done" || issue.Status == "cancelled" { From 4f0c2afcb7738d591fb611302d18f0af9c3ac3b0 Mon Sep 17 00:00:00 2001 From: Jiayuan Date: Wed, 1 Apr 2026 22:01:33 +0800 Subject: [PATCH 3/4] test(server): add unit tests for comment trigger logic Tests cover all on_comment trigger decision scenarios: - commentMentionsOthersButNotAssignee: 6 cases - isReplyToMemberThread: 7 cases - Combined trigger decision: 9 cases matching the full decision table - agentHasTriggerEnabled: 7 cases (nil, empty, explicit, backwards compat) - defaultAgentTriggers: validates all 3 triggers present and enabled --- server/internal/handler/trigger_test.go | 333 ++++++++++++++++++++++++ 1 file changed, 333 insertions(+) create mode 100644 server/internal/handler/trigger_test.go 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 +} From b41536467d7eee30b9fe3179ca1c8a0e15aeab51 Mon Sep 17 00:00:00 2001 From: Jiayuan Date: Wed, 1 Apr 2026 22:11:46 +0800 Subject: [PATCH 4/4] test(server): add integration tests for comment trigger logic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit End-to-end tests through the full HTTP router + real database: TestCommentTriggerOnComment (6 subtests): - Top-level comment without mentions → triggers agent - Top-level comment mentioning only others → suppresses trigger - Top-level comment mentioning assignee → triggers agent - Reply to agent thread without mentions → triggers agent - Reply to member thread without mentions → suppresses trigger (Bohan's bug) - Reply to member thread mentioning assignee → triggers agent TestCommentTriggerOnAssignNoStatusGate: - Assigning agent to in_progress issue → triggers (no todo restriction) TestCommentTriggerOnMentionNoStatusGate: - @mentioning agent on done issue → triggers (no status gate) TestCommentTriggerCoalescing: - Rapid-fire comments → only 1 task created (dedup) --- .../comment_trigger_integration_test.go | 332 ++++++++++++++++++ 1 file changed, 332 insertions(+) create mode 100644 server/cmd/server/comment_trigger_integration_test.go 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) + } +}