fix(server): @all mentions should not trigger agent execution
@all is a broadcast to all workspace members — it should not trigger the assignee agent's on_comment. Previously @all was treated as "includes everyone" and allowed the trigger. Changes: - commentMentionsOthersButNotAssignee now checks HasMentionAll() early and returns true (suppress) when @all is present - Fix authRequestWithAgent test helper that was making a duplicate HTTP request (one as member, one as agent) Tests: 5 new @all unit test cases, 2 new @all integration test cases.
This commit is contained in:
parent
05fcf35ab9
commit
02c9d9f0b0
3 changed files with 50 additions and 22 deletions
|
|
@ -1,6 +1,7 @@
|
|||
package main
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
|
|
@ -13,14 +14,10 @@ import (
|
|||
// 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}
|
||||
bodyReader = bytes.NewReader(b)
|
||||
}
|
||||
req, err := http.NewRequest(method, testServer.URL+path, bodyReader)
|
||||
if err != nil {
|
||||
|
|
@ -38,20 +35,6 @@ func authRequestWithAgent(t *testing.T, method, path string, body any, agentID s
|
|||
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()
|
||||
|
|
@ -249,6 +232,35 @@ func TestCommentTriggerOnComment(t *testing.T) {
|
|||
})
|
||||
}
|
||||
|
||||
// TestCommentTriggerAtAllSuppression verifies that @all mentions do not
|
||||
// trigger agent execution — @all is a broadcast, not a direct request.
|
||||
func TestCommentTriggerAtAllSuppression(t *testing.T) {
|
||||
agentID := getAgentID(t)
|
||||
issueID := createIssueAssignedToAgent(t, "@all suppression test", agentID)
|
||||
t.Cleanup(func() {
|
||||
clearTasks(t, issueID)
|
||||
resp := authRequest(t, "DELETE", "/api/issues/"+issueID, nil)
|
||||
resp.Body.Close()
|
||||
})
|
||||
|
||||
t.Run("top-level @all comment suppresses on_comment", func(t *testing.T) {
|
||||
clearTasks(t, issueID)
|
||||
postComment(t, issueID, "[@All](mention://all/all) heads up everyone", nil)
|
||||
if n := countPendingTasks(t, issueID); n != 0 {
|
||||
t.Errorf("expected 0 pending tasks (@all should not trigger agent), got %d", n)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("@all in agent thread suppresses on_comment", func(t *testing.T) {
|
||||
clearTasks(t, issueID)
|
||||
threadID := postCommentAsAgent(t, issueID, "Here is my analysis.", agentID, nil)
|
||||
postComment(t, issueID, "[@All](mention://all/all) FYI for the team", strPtr(threadID))
|
||||
if n := countPendingTasks(t, issueID); n != 0 {
|
||||
t.Errorf("expected 0 pending tasks (@all in agent 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) {
|
||||
|
|
|
|||
|
|
@ -193,19 +193,22 @@ func (h *Handler) CreateComment(w http.ResponseWriter, r *http.Request) {
|
|||
// anyone but does NOT @mention the issue's assignee agent. This is used to
|
||||
// suppress the on_comment trigger when the user is directing their comment at
|
||||
// someone else (e.g. sharing results with a colleague, asking another agent).
|
||||
// @all is treated as a broadcast — it suppresses the trigger because the user
|
||||
// is announcing to everyone, not specifically requesting work from the agent.
|
||||
func (h *Handler) commentMentionsOthersButNotAssignee(content string, issue db.Issue) bool {
|
||||
mentions := util.ParseMentions(content)
|
||||
if len(mentions) == 0 {
|
||||
return false // No mentions — normal on_comment behavior
|
||||
}
|
||||
// @all is a broadcast to all members — suppress agent trigger.
|
||||
if util.HasMentionAll(mentions) {
|
||||
return true
|
||||
}
|
||||
if !issue.AssigneeID.Valid {
|
||||
return true // No assignee — mentions target others
|
||||
}
|
||||
assigneeID := uuidToString(issue.AssigneeID)
|
||||
for _, m := range mentions {
|
||||
if m.IsMentionAll() {
|
||||
return false // @all includes everyone — allow trigger
|
||||
}
|
||||
if m.ID == assigneeID {
|
||||
return false // Assignee is mentioned — allow trigger
|
||||
}
|
||||
|
|
|
|||
|
|
@ -76,6 +76,16 @@ func TestCommentMentionsOthersButNotAssignee(t *testing.T) {
|
|||
content: fmt.Sprintf("[@Agent](mention://agent/%s) and [@Other](mention://agent/%s)", agentAssigneeID, otherAgentID),
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "@all mention → suppress (broadcast, not directed at agent)",
|
||||
content: "[@All](mention://all/all) heads up everyone",
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "@all with assignee mention → suppress (@all takes precedence)",
|
||||
content: fmt.Sprintf("[@All](mention://all/all) [@Agent](mention://agent/%s) fyi", agentAssigneeID),
|
||||
want: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
|
|
@ -203,6 +213,9 @@ 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},
|
||||
{"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},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue