From fc6405e4be72649c485bc775ca49101e86b2f345 Mon Sep 17 00:00:00 2001 From: Bohan Jiang <52446949+Bohan-J@users.noreply.github.com> Date: Fri, 3 Apr 2026 15:07:39 +0800 Subject: [PATCH] fix(trigger): allow on_comment when thread root @mentions assignee agent (#382) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a member-started thread root @mentions the assignee agent, replies in that thread should trigger on_comment — the thread is a conversation with the agent, not a member-to-member chat. Previously isReplyToMemberThread only checked the reply content for assignee mentions. Now it also checks the parent (thread root) content. This fixes a gap where path 1 (on_comment) suppressed the trigger and path 2 (on_mention) skipped the assignee, leaving no trigger path. --- .../comment_trigger_integration_test.go | 14 +++++++ server/internal/handler/comment.go | 14 ++++++- server/internal/handler/trigger_test.go | 38 +++++++++++++++++-- 3 files changed, 60 insertions(+), 6 deletions(-) diff --git a/server/cmd/server/comment_trigger_integration_test.go b/server/cmd/server/comment_trigger_integration_test.go index cfa2028c..b9337dad 100644 --- a/server/cmd/server/comment_trigger_integration_test.go +++ b/server/cmd/server/comment_trigger_integration_test.go @@ -230,6 +230,20 @@ func TestCommentTriggerOnComment(t *testing.T) { t.Errorf("expected 1 pending task (assignee mentioned in member thread), got %d", n) } }) + + t.Run("reply to member thread that @mentioned assignee triggers without re-mention", func(t *testing.T) { + clearTasks(t, issueID) + // Member starts a thread that @mentions the assignee agent. + content := fmt.Sprintf("[@Agent](mention://agent/%s) can you review this?", agentID) + threadID := postComment(t, issueID, content, nil) + // Clear the task created by the top-level mention. + clearTasks(t, issueID) + // Reply in the thread WITHOUT re-mentioning the assignee. + postComment(t, issueID, "Here is more context for you", strPtr(threadID)) + if n := countPendingTasks(t, issueID); n != 1 { + t.Errorf("expected 1 pending task (assignee mentioned in thread root), got %d", n) + } + }) } // TestCommentTriggerAtAllSuppression verifies that @all mentions do not diff --git a/server/internal/handler/comment.go b/server/internal/handler/comment.go index d17917a8..95c00fa5 100644 --- a/server/internal/handler/comment.go +++ b/server/internal/handler/comment.go @@ -227,6 +227,8 @@ func (h *Handler) commentMentionsOthersButNotAssignee(content string, issue db.I // 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. +// If the parent (thread root) itself @mentions the assignee, the thread is +// considered a conversation with the agent, so replies are allowed to trigger. 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 @@ -235,14 +237,22 @@ func (h *Handler) isReplyToMemberThread(parent *db.Comment, content string, issu 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. + // or the parent explicitly @mentions the assignee agent. if !issue.AssigneeID.Valid { return true // No assignee to mention } assigneeID := uuidToString(issue.AssigneeID) + // Check current comment mentions. for _, m := range util.ParseMentions(content) { if m.ID == assigneeID { - return false // Assignee explicitly mentioned — allow trigger + return false // Assignee explicitly mentioned in reply — allow trigger + } + } + // Check parent (thread root) mentions — if the thread was started by + // mentioning the assignee, replies continue that conversation. + for _, m := range util.ParseMentions(parent.Content) { + if m.ID == assigneeID { + return false // Assignee mentioned in thread root — allow trigger } } return true // Reply to member thread without mentioning agent — suppress diff --git a/server/internal/handler/trigger_test.go b/server/internal/handler/trigger_test.go index e2358b88..9fe7a950 100644 --- a/server/internal/handler/trigger_test.go +++ b/server/internal/handler/trigger_test.go @@ -117,8 +117,20 @@ 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)} + memberParent := &db.Comment{AuthorType: "member", AuthorID: testUUID(memberID), Content: "plain thread starter"} + agentParent := &db.Comment{AuthorType: "agent", AuthorID: testUUID(agentAssigneeID), Content: "agent thread starter"} + // Member-started thread root that @mentions the assignee agent. + memberParentMentioningAssignee := &db.Comment{ + AuthorType: "member", + AuthorID: testUUID(memberID), + Content: fmt.Sprintf("[@Agent](mention://agent/%s) can you look at this?", agentAssigneeID), + } + // Member-started thread root that @mentions a non-assignee agent. + memberParentMentioningOther := &db.Comment{ + AuthorType: "member", + AuthorID: testUUID(memberID), + Content: fmt.Sprintf("[@Other](mention://agent/%s) what do you think?", otherAgentID), + } tests := []struct { name string @@ -168,6 +180,18 @@ func TestIsReplyToMemberThread(t *testing.T) { content: fmt.Sprintf("[@Other](mention://agent/%s) take a look", otherAgentID), want: true, }, + { + name: "reply to member thread that @mentioned assignee, no re-mention → allow", + parent: memberParentMentioningAssignee, + content: "here is more context for you", + want: false, + }, + { + name: "reply to member thread that @mentioned other agent, no re-mention → suppress", + parent: memberParentMentioningOther, + content: "here is more context", + want: true, // parent mentioned other agent, not assignee — still suppress on_comment + }, } for _, tt := range tests { @@ -188,8 +212,13 @@ 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)} + memberParent := &db.Comment{AuthorType: "member", AuthorID: testUUID(memberID), Content: "plain thread starter"} + agentParent := &db.Comment{AuthorType: "agent", AuthorID: testUUID(agentAssigneeID), Content: "agent thread starter"} + memberParentMentioningAssignee := &db.Comment{ + AuthorType: "member", + AuthorID: testUUID(memberID), + Content: fmt.Sprintf("[@Agent](mention://agent/%s) help me", agentAssigneeID), + } // Simulates the combined check from CreateComment: // !commentMentionsOthersButNotAssignee && !isReplyToMemberThread @@ -213,6 +242,7 @@ func TestOnCommentTriggerDecision(t *testing.T) { {"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}, + {"reply member thread that @mentioned assignee, no re-mention", memberParentMentioningAssignee, "here is more info", true}, {"top-level, @all broadcast", nil, "[@All](mention://all/all) heads up team", false}, {"reply agent thread, @all broadcast", agentParent, "[@All](mention://all/all) update for everyone", false}, {"reply member thread, @all broadcast", memberParent, "[@All](mention://all/all) fyi", false},