From 04da4bc3b752defa3d8e5ecbbcfe61855d2898e1 Mon Sep 17 00:00:00 2001 From: Jiayuan Date: Wed, 1 Apr 2026 21:37:33 +0800 Subject: [PATCH] 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) {