mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-05 13:29:44 +02:00
* feat(comments): skip agent triggering on /note-prefixed comments A comment whose first token is the reserved /note prefix (case-insensitive) is stored like any other comment but never wakes an agent. The guard sits at the top of triggerTasksForComment, the single chokepoint, so it covers all three trigger paths — assignee, squad leader, and @mentioned agents. Gating only shouldEnqueueOnComment (as originally proposed) would still let "/note @agent ..." through the mention path. Lets members leave human-only tips/notes on agent-assigned issues without burning an agent run. MUL-3115, closes #3649. Co-authored-by: multica-agent <github@multica.ai> * feat(editor): add /note built-in slash command to comment composer Enable the `/` menu in the issue comment and reply composers in a new "command" mode that lists fixed built-in commands instead of the chat skill picker. Currently one command, /note, which marks a comment as a human-only note that won't trigger the assigned agent. Selecting it inserts the plain-text "/note " prefix (not a rich node), so a menu pick and a hand-typed command are byte-identical and the backend detects either with a simple prefix match. The command menu renders nothing on a non-matching `/` (hideOnEmpty) so typing a date like 6/8 isn't noisy. The chat skill picker is unchanged. MUL-3115. Co-authored-by: multica-agent <github@multica.ai> * refactor(editor): match /note by label prefix and localize its description Address PR review feedback: - buildBuiltinCommandItems now matches the command label as a prefix only, dropping the description substring match copied from the skill picker. With one command this keeps the menu predictable (/no surfaces note; /deploy or a description word like /agent shows nothing) and avoids Enter selecting note unexpectedly. - The command description is now a localized UI string: added slash_command.commands.note to all four editor locales (en/ja/ko/zh-Hans) and the menu renders it via the typed translator. The /label itself stays literal since it's the typed token the backend matches. MUL-3115. Co-authored-by: multica-agent <github@multica.ai> * fix(editor): shorten /note command description to avoid truncation The slash menu item is single-line (truncate, w-72), so the longer copy was cut off. Shorten to "won't trigger any agents" across all four locales — also more accurate, since /note skips assignee, squad leader, and @mentioned agents, not just the assigned one. MUL-3115. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: J <j@multica.ai> Co-authored-by: multica-agent <github@multica.ai>
370 lines
14 KiB
Go
370 lines
14 KiB
Go
package handler
|
|
|
|
import (
|
|
"context"
|
|
"fmt"
|
|
"testing"
|
|
|
|
"github.com/jackc/pgx/v5/pgtype"
|
|
"github.com/multica-ai/multica/server/internal/util"
|
|
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,
|
|
},
|
|
{
|
|
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,
|
|
},
|
|
{
|
|
name: "issue mention only → allow trigger (cross-reference, not @person)",
|
|
content: "[PAN-1](mention://issue/44c266e7-f6dd-4be3-9140-5ac40233f79c) is related",
|
|
want: false,
|
|
},
|
|
{
|
|
name: "issue mention + other agent → suppress (agent mention matters)",
|
|
content: fmt.Sprintf("[PAN-1](mention://issue/44c266e7-f6dd-4be3-9140-5ac40233f79c) cc [@Other](mention://agent/%s)", otherAgentID),
|
|
want: true,
|
|
},
|
|
}
|
|
|
|
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), 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
|
|
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,
|
|
},
|
|
{
|
|
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 {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
got := h.isReplyToMemberThread(context.Background(), tt.parent, tt.content, issue)
|
|
if got != tt.want {
|
|
t.Errorf("isReplyToMemberThread() = %v, want %v", got, tt.want)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// -------------------------------------------------------------------
|
|
// shouldInheritParentMentions
|
|
// -------------------------------------------------------------------
|
|
|
|
func TestShouldInheritParentMentions(t *testing.T) {
|
|
memberParent := &db.Comment{AuthorType: "member", AuthorID: testUUID(memberID), Content: "thread starter"}
|
|
agentParent := &db.Comment{AuthorType: "agent", AuthorID: testUUID(agentAssigneeID), Content: "agent thread starter"}
|
|
someMention := []util.Mention{{Type: "agent", ID: otherAgentID}}
|
|
|
|
tests := []struct {
|
|
name string
|
|
parent *db.Comment
|
|
replyMentions []util.Mention
|
|
replyAuthorType string
|
|
want bool
|
|
}{
|
|
{"nil parent → false", nil, nil, "member", false},
|
|
{"reply has explicit mentions → false", memberParent, someMention, "member", false},
|
|
{"agent-authored reply, member parent → false (loop guard)", memberParent, nil, "agent", false},
|
|
{"member reply, agent parent → false (parent author guard)", agentParent, nil, "member", false},
|
|
{"member reply, member parent, no mentions → true (intended use)", memberParent, nil, "member", true},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
got := shouldInheritParentMentions(tt.parent, tt.replyMentions, tt.replyAuthorType)
|
|
if got != tt.want {
|
|
t.Errorf("shouldInheritParentMentions() = %v, want %v", got, tt.want)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// Regression for the case from MUL-1535: J posts a PR completion comment
|
|
// that @mentions GPT-Boy for review; later a member posts a plain follow-up
|
|
// reply asking the assignee a question. GPT-Boy must NOT be re-triggered.
|
|
func TestShouldInheritParentMentions_AgentReviewDelegationDoesNotLeak(t *testing.T) {
|
|
jPRCompletion := &db.Comment{
|
|
AuthorType: "agent",
|
|
AuthorID: testUUID(agentAssigneeID),
|
|
Content: fmt.Sprintf("PR ready. [@GPT-Boy](mention://agent/%s) please review this.", otherAgentID),
|
|
}
|
|
if got := shouldInheritParentMentions(jPRCompletion, nil, "member"); got {
|
|
t.Fatal("member follow-up to an agent's PR-review delegation must not inherit the @reviewer mention")
|
|
}
|
|
}
|
|
|
|
// -------------------------------------------------------------------
|
|
// 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), 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
|
|
shouldTrigger := func(parent *db.Comment, content string) bool {
|
|
return !h.commentMentionsOthersButNotAssignee(content, issue) &&
|
|
!h.isReplyToMemberThread(context.Background(), 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},
|
|
{"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},
|
|
}
|
|
|
|
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)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// -------------------------------------------------------------------
|
|
// isNoteComment — the /note opt-out prefix
|
|
// -------------------------------------------------------------------
|
|
|
|
func TestIsNoteComment(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
content string
|
|
want bool
|
|
}{
|
|
{"plain comment triggers", "just a plain comment", false},
|
|
{"note prefix skips", "/note check the API expiry", true},
|
|
{"bare note skips", "/note", true},
|
|
{"uppercase note skips (case-insensitive)", "/NOTE shout", true},
|
|
{"mixed case note skips", "/Note mixed", true},
|
|
{"leading whitespace tolerated", " /note leading space", true},
|
|
{"note followed by newline skips", "/note\nmultiline body", true},
|
|
{"plural notes does not match (word boundary)", "/notes are plural", false},
|
|
{"noteworthy does not match", "/noteworthy idea", false},
|
|
{"slash space note does not match", "/ note has a space", false},
|
|
{"mid-sentence note does not match", "see foo/note here", false},
|
|
{"note as second token does not match", "fyi /note", false},
|
|
{"empty content does not match", "", false},
|
|
{"whitespace-only content does not match", " ", false},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
if got := isNoteComment(tt.content); got != tt.want {
|
|
t.Errorf("isNoteComment(%q) = %v, want %v", tt.content, got, tt.want)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestTriggerTasksForComment_NoteShortCircuits proves a /note comment returns
|
|
// before any of the three enqueue paths run. shouldEnqueueOnComment,
|
|
// shouldEnqueueSquadLeaderOnComment, and enqueueMentionedAgentTasks all
|
|
// dereference h.Queries, so a nil-Queries Handler would panic if the /note
|
|
// guard were missing or moved below them. The comment also @mentions an agent
|
|
// to exercise the mention path specifically.
|
|
func TestTriggerTasksForComment_NoteShortCircuits(t *testing.T) {
|
|
h := &Handler{} // nil Queries / TaskService on purpose
|
|
issue := issueWithAgentAssignee()
|
|
comment := db.Comment{
|
|
Content: fmt.Sprintf("/note cc [@Other](mention://agent/%s) just an fyi", otherAgentID),
|
|
}
|
|
|
|
// Must not panic — the guard short-circuits before any DB access.
|
|
h.triggerTasksForComment(context.Background(), issue, comment, nil, "member", memberID)
|
|
}
|