From ee04dbdfb9eb81016c7d0797a41cd1cb4f6ac16a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E5=BC=BA?= Date: Thu, 21 May 2026 06:37:53 -0400 Subject: [PATCH] fix(squad): gate cross-squad agent @mentions on squad-assigned issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agent-authored comments on squad-assigned issues could @mention any workspace agent the leader discovered via the A2A-bypassed `multica agent list` and silently dispatch to an outside-squad agent — the visible label canonicalized to whichever agent the leader picked, but the dispatch was never the leader's own squad members. Three layers, all server-side: - enqueueMentionedAgentTasks drops agent @mentions whose UUID is neither in squad_member nor the squad's LeaderID, with a slog.Warn. Member-authored comments and @squad mentions bypass the gate. - ListAgents honors a new ?scope=task_squad hint, narrowing the response to the issue's squad iff the request comes from an agent actor on a real leader task. No-op everywhere else. - multica agent list passes the hint by default inside a daemon-managed agent task; --all opts out. squadOperatingProtocol calls out both the silent drop and the agent-list ban so leaders stop reaching outside the roster in the first place. Tests cover gate negative / positive / leader fallback / member-author bypass / mixed-mention per-mention behavior; server scope filter on/off; CLI flag wiring with and without agent context. Co-Authored-By: Claude Opus 4.7 --- server/cmd/multica/cmd_agent.go | 15 + .../cmd/multica/cmd_agent_list_scope_test.go | 106 +++++++ server/internal/handler/agent.go | 79 +++++ server/internal/handler/comment.go | 74 +++++ .../list_agents_task_squad_scope_test.go | 153 ++++++++++ server/internal/handler/squad_briefing.go | 12 +- .../internal/handler/squad_briefing_test.go | 26 ++ .../handler/squad_cross_squad_mention_test.go | 281 ++++++++++++++++++ 8 files changed, 745 insertions(+), 1 deletion(-) create mode 100644 server/cmd/multica/cmd_agent_list_scope_test.go create mode 100644 server/internal/handler/list_agents_task_squad_scope_test.go create mode 100644 server/internal/handler/squad_cross_squad_mention_test.go diff --git a/server/cmd/multica/cmd_agent.go b/server/cmd/multica/cmd_agent.go index 56a1371fd8..ac6ed968f6 100644 --- a/server/cmd/multica/cmd_agent.go +++ b/server/cmd/multica/cmd_agent.go @@ -114,6 +114,14 @@ func init() { // agent list agentListCmd.Flags().String("output", "table", "Output format: table or json") agentListCmd.Flags().Bool("include-archived", false, "Include archived agents") + // --all opts out of the squad-leader scoping. When this CLI is invoked + // from inside a daemon-managed agent task that is a squad-leader run + // on a squad-assigned issue, `agent list` defaults to listing only that + // squad's members + the leader — `--all` returns the workspace-wide list + // instead (matching the pre-fix behavior). Has no effect outside a + // leader task. See server/internal/handler/agent.go ListAgents + + // taskSquadMemberSet for the matching server-side scope. + agentListCmd.Flags().Bool("all", false, "Return the full workspace agent list even inside a squad-leader task (default scopes to the squad's roster)") // agent get agentGetCmd.Flags().String("output", "json", "Output format: table or json") @@ -289,6 +297,13 @@ func runAgentList(cmd *cobra.Command, _ []string) error { if v, _ := cmd.Flags().GetBool("include-archived"); v { params.Set("include_archived", "true") } + // Squad-leader scoping: inside a daemon-managed agent task, the server + // narrows the list to the issue's squad iff the task is a leader task. + // Outside such a task the param is a no-op, so always passing it is + // safe. `--all` opts out. + if allFlag, _ := cmd.Flags().GetBool("all"); !allFlag && inAgentExecutionContext() { + params.Set("scope", "task_squad") + } path := "/api/agents" if len(params) > 0 { path += "?" + params.Encode() diff --git a/server/cmd/multica/cmd_agent_list_scope_test.go b/server/cmd/multica/cmd_agent_list_scope_test.go new file mode 100644 index 0000000000..670fc78db4 --- /dev/null +++ b/server/cmd/multica/cmd_agent_list_scope_test.go @@ -0,0 +1,106 @@ +package main + +import ( + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/spf13/cobra" +) + +// freshAgentListCmd mirrors agentListCmd's flag wiring for use in tests so +// each subtest gets a clean flag state. Keeps the production command's +// cobra.Command immutable across runs. +func freshAgentListCmd() *cobra.Command { + c := &cobra.Command{Use: "list"} + c.Flags().String("output", "json", "Output format") + c.Flags().Bool("include-archived", false, "Include archived agents") + c.Flags().Bool("all", false, "Return the full workspace agent list even inside a squad-leader task") + c.PersistentFlags().String("profile", "", "") + return c +} + +// captureListAgentsQuery starts a stub server that records the +// /api/agents request's RawQuery and returns an empty agent list. Caller +// gets back the captured query and the server's URL. +func captureListAgentsQuery(t *testing.T) (*httptest.Server, *string) { + t.Helper() + var captured string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/agents" { + http.NotFound(w, r) + return + } + captured = r.URL.RawQuery + _, _ = io.WriteString(w, "[]") + })) + return srv, &captured +} + +func TestRunAgentList_AddsTaskSquadScopeInAgentContext(t *testing.T) { + srv, captured := captureListAgentsQuery(t) + defer srv.Close() + + t.Setenv("HOME", t.TempDir()) + t.Setenv("MULTICA_SERVER_URL", srv.URL) + t.Setenv("MULTICA_WORKSPACE_ID", "ws-1") + t.Setenv("MULTICA_TOKEN", "test-token") + t.Setenv("MULTICA_AGENT_ID", "agent-123") + t.Setenv("MULTICA_TASK_ID", "task-456") + + cmd := freshAgentListCmd() + if err := runAgentList(cmd, nil); err != nil { + t.Fatalf("runAgentList: %v", err) + } + + if !strings.Contains(*captured, "scope=task_squad") { + t.Fatalf("expected query to include scope=task_squad inside agent context, got %q", *captured) + } +} + +func TestRunAgentList_OmitsTaskSquadScopeWithAllFlag(t *testing.T) { + srv, captured := captureListAgentsQuery(t) + defer srv.Close() + + t.Setenv("HOME", t.TempDir()) + t.Setenv("MULTICA_SERVER_URL", srv.URL) + t.Setenv("MULTICA_WORKSPACE_ID", "ws-1") + t.Setenv("MULTICA_TOKEN", "test-token") + t.Setenv("MULTICA_AGENT_ID", "agent-123") + t.Setenv("MULTICA_TASK_ID", "task-456") + + cmd := freshAgentListCmd() + if err := cmd.Flags().Set("all", "true"); err != nil { + t.Fatalf("set --all: %v", err) + } + if err := runAgentList(cmd, nil); err != nil { + t.Fatalf("runAgentList: %v", err) + } + + if strings.Contains(*captured, "scope=task_squad") { + t.Fatalf("expected --all to suppress scope param, got %q", *captured) + } +} + +func TestRunAgentList_OmitsTaskSquadScopeOutsideAgentContext(t *testing.T) { + srv, captured := captureListAgentsQuery(t) + defer srv.Close() + + t.Setenv("HOME", t.TempDir()) + t.Setenv("MULTICA_SERVER_URL", srv.URL) + t.Setenv("MULTICA_WORKSPACE_ID", "ws-1") + t.Setenv("MULTICA_TOKEN", "test-token") + t.Setenv("MULTICA_AGENT_ID", "") + t.Setenv("MULTICA_TASK_ID", "") + + cmd := freshAgentListCmd() + if err := runAgentList(cmd, nil); err != nil { + t.Fatalf("runAgentList: %v", err) + } + + if strings.Contains(*captured, "scope=task_squad") { + t.Fatalf("expected no scope param outside agent context, got %q", *captured) + } +} diff --git a/server/internal/handler/agent.go b/server/internal/handler/agent.go index 42c6c355e6..6ae35c512c 100644 --- a/server/internal/handler/agent.go +++ b/server/internal/handler/agent.go @@ -16,6 +16,7 @@ import ( "github.com/multica-ai/multica/server/internal/analytics" "github.com/multica-ai/multica/server/internal/logger" "github.com/multica-ai/multica/server/internal/service" + "github.com/multica-ai/multica/server/internal/util" "github.com/multica-ai/multica/server/pkg/agent" db "github.com/multica-ai/multica/server/pkg/db/generated" "github.com/multica-ai/multica/server/pkg/protocol" @@ -323,6 +324,22 @@ func (h *Handler) ListAgents(w http.ResponseWriter, r *http.Request) { // to preserve A2A collaboration; members must be in allowed_principals // (agent owner or workspace owner/admin) to see private agents. actorType, actorID := h.resolveActor(r, userID, workspaceID) + + // Squad-leader scope. CLI passes ?scope=task_squad by default when + // running inside a daemon-managed agent task; --all opts out. The + // hint is only honored for agent actors that are actually running a + // leader task on a squad-assigned issue — otherwise the param is a + // no-op so worker tasks and one-off CLI calls keep their full A2A + // view. Pairs with the squad cross-squad mention gate in + // enqueueMentionedAgentTasks: list narrows what the leader sees, + // the gate hard-rejects what slips through anyway. + var squadScopeSet map[string]struct{} + if actorType == "agent" && r.URL.Query().Get("scope") == "task_squad" { + if set, ok := h.taskSquadMemberSet(r); ok { + squadScopeSet = set + } + } + visible := make([]AgentResponse, 0, len(agents)) for _, a := range agents { if a.Visibility == "private" && actorType == "member" { @@ -330,6 +347,11 @@ func (h *Handler) ListAgents(w http.ResponseWriter, r *http.Request) { continue } } + if squadScopeSet != nil { + if _, inSquad := squadScopeSet[uuidToString(a.ID)]; !inSquad { + continue + } + } resp := agentToResponse(a) if skills, ok := skillMap[resp.ID]; ok { resp.Skills = skills @@ -345,6 +367,63 @@ func (h *Handler) ListAgents(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, visible) } +// taskSquadMemberSet returns the set of agent UUIDs (as strings) that belong +// to the squad of the task identified by the X-Task-ID header — the squad's +// LeaderID plus every squad_member of type "agent". The second return is +// false when the task is not a leader task on a squad-assigned issue, when +// the task isn't loadable, or when any of the squad lookups fail; callers +// treat the false return as "no scoping" and fall through to the unscoped +// list. The leader is always included even when the squad_member row was +// not auto-inserted (legacy squads predating CreateSquad's auto-add), +// matching the leader fallback in the enqueueMentionedAgentTasks gate. +func (h *Handler) taskSquadMemberSet(r *http.Request) (map[string]struct{}, bool) { + taskHeader := r.Header.Get("X-Task-ID") + if taskHeader == "" { + return nil, false + } + taskUUID, err := util.ParseUUID(taskHeader) + if err != nil { + return nil, false + } + task, err := h.Queries.GetAgentTask(r.Context(), taskUUID) + if err != nil { + return nil, false + } + if !task.IsLeaderTask { + return nil, false + } + if !task.IssueID.Valid { + return nil, false + } + issue, err := h.Queries.GetIssue(r.Context(), task.IssueID) + if err != nil { + return nil, false + } + if !issue.AssigneeType.Valid || issue.AssigneeType.String != "squad" || !issue.AssigneeID.Valid { + return nil, false + } + squad, err := h.Queries.GetSquadInWorkspace(r.Context(), db.GetSquadInWorkspaceParams{ + ID: issue.AssigneeID, + WorkspaceID: issue.WorkspaceID, + }) + if err != nil { + return nil, false + } + members, err := h.Queries.ListSquadMembers(r.Context(), squad.ID) + if err != nil { + return nil, false + } + set := make(map[string]struct{}, len(members)+1) + set[uuidToString(squad.LeaderID)] = struct{}{} + for _, m := range members { + if m.MemberType != "agent" { + continue + } + set[uuidToString(m.MemberID)] = struct{}{} + } + return set, true +} + func (h *Handler) GetAgent(w http.ResponseWriter, r *http.Request) { id := chi.URLParam(r, "id") agent, ok := h.loadAgentForUser(w, r, id) diff --git a/server/internal/handler/comment.go b/server/internal/handler/comment.go index 178ffa8299..9f7aef86dc 100644 --- a/server/internal/handler/comment.go +++ b/server/internal/handler/comment.go @@ -751,6 +751,39 @@ func (h *Handler) enqueueMentionedAgentTasks(ctx context.Context, issue db.Issue if shouldInheritParentMentions(parentComment, mentions, authorType) { mentions = util.ParseMentions(parentComment.Content) } + + // Cross-squad @-mention gate: on a squad-assigned issue, an agent-authored + // comment may only enqueue tasks for agents that belong to the issue's + // squad. Catches the failure mode where a squad leader, using the A2A + // bypass on `multica agent list`, picks a same-role agent from a + // DIFFERENT squad / no squad at all instead of its own roster (see + // squadOperatingProtocol's "Squad Roster" rule). Member (human) authors + // keep full agency — they may deliberately reach outside the squad. + // Squad mentions (`m.Type == "squad"`) are not gated either; they route + // to the target squad's leader, who decides whether to accept. + var ( + squadGate bool + squadGateSquad db.Squad + ) + if authorType == "agent" && issue.AssigneeType.Valid && issue.AssigneeType.String == "squad" && issue.AssigneeID.Valid { + if s, err := h.Queries.GetSquadInWorkspace(ctx, db.GetSquadInWorkspaceParams{ + ID: issue.AssigneeID, + WorkspaceID: issue.WorkspaceID, + }); err == nil { + squadGate = true + squadGateSquad = s + } else { + // Squad assignee FK gone (delete race / inconsistent state): + // log and skip the gate. Failing open here matches the + // posture of the surrounding error paths in this file — + // observability is the recovery hook. + slog.Warn("cross-squad mention gate disabled: failed to load squad assignee", + "issue_id", uuidToString(issue.ID), + "squad_id", uuidToString(issue.AssigneeID), + "error", err) + } + } + for _, m := range mentions { if m.Type == "squad" { // @squad mention → trigger the squad's leader agent. @@ -820,6 +853,47 @@ func (h *Handler) enqueueMentionedAgentTasks(ctx context.Context, issue db.Issue if !h.canAccessPrivateAgent(ctx, agent, authorType, authorID, wsID) { continue } + // Cross-squad gate: on a squad-assigned issue with an agent author, + // drop the @mention when the target agent is neither a squad_member + // row nor the squad's LeaderID. The leader fallback covers legacy + // squads whose creation path bypassed CreateSquad's auto squad_member + // insert — those rows still have a valid LeaderID and should be + // reachable. + if squadGate { + isLeader := uuidToString(squadGateSquad.LeaderID) == uuidToString(agentUUID) + isMember := false + gateActive := true + if !isLeader { + ok, memberErr := h.Queries.IsSquadMember(ctx, db.IsSquadMemberParams{ + SquadID: squadGateSquad.ID, + MemberType: "agent", + MemberID: agentUUID, + }) + if memberErr != nil { + // Same posture as the squad-load failure above: log and + // fail open for this mention so a transient DB error + // cannot wedge legitimate dispatch. The slog warn is + // the recovery hook. + slog.Warn("cross-squad mention gate: IsSquadMember query failed — allowing mention", + "issue_id", uuidToString(issue.ID), + "squad_id", uuidToString(squadGateSquad.ID), + "mentioned_agent_id", uuidToString(agentUUID), + "error", memberErr) + gateActive = false + } + isMember = ok + } + if gateActive && !isLeader && !isMember { + slog.Warn("cross-squad @mention dropped: target agent is not a member of the issue's squad", + "issue_id", uuidToString(issue.ID), + "squad_id", uuidToString(squadGateSquad.ID), + "squad_name", squadGateSquad.Name, + "mentioned_agent_id", uuidToString(agentUUID), + "mentioned_agent_name", agent.Name, + "author_agent_id", authorID) + continue + } + } // Dedup: skip if this agent already has a pending task for this issue. hasPending, err := h.Queries.HasPendingTaskForIssueAndAgent(ctx, db.HasPendingTaskForIssueAndAgentParams{ IssueID: issue.ID, diff --git a/server/internal/handler/list_agents_task_squad_scope_test.go b/server/internal/handler/list_agents_task_squad_scope_test.go new file mode 100644 index 0000000000..3b452605ae --- /dev/null +++ b/server/internal/handler/list_agents_task_squad_scope_test.go @@ -0,0 +1,153 @@ +package handler + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" +) + +// leaderTaskScopeFixture seeds the world for the `scope=task_squad` filter: +// +// - a squad with leader, worker (squad_member), and an outsider agent +// that is intentionally NOT in the squad +// - a squad-assigned issue +// - a running leader-task whose UUID we can send back as X-Task-ID +// +// Reuses crossSquadMentionFixture for the agent layout and adds the running +// is_leader_task row on top. Lives next to the cross-squad mention tests +// (squad_cross_squad_mention_test.go) because the two tests share the same +// invariant — leader stays inside its roster — at different layers. +type leaderTaskScopeFixture struct { + crossSquadMentionFixture + TaskID string +} + +func newLeaderTaskScopeFixture(t *testing.T, isLeaderTask bool) leaderTaskScopeFixture { + t.Helper() + ctx := context.Background() + base := newCrossSquadMentionFixture(t) + + var runtimeID string + if err := testPool.QueryRow(ctx, `SELECT runtime_id FROM agent WHERE id = $1`, base.LeaderID).Scan(&runtimeID); err != nil { + t.Fatalf("load leader runtime: %v", err) + } + + var taskID string + if err := testPool.QueryRow(ctx, ` + INSERT INTO agent_task_queue ( + agent_id, runtime_id, issue_id, + status, priority, started_at, is_leader_task + ) + VALUES ($1, $2, $3, 'running', 0, now(), $4) + RETURNING id + `, base.LeaderID, runtimeID, base.IssueID, isLeaderTask).Scan(&taskID); err != nil { + t.Fatalf("create leader task (is_leader_task=%v): %v", isLeaderTask, err) + } + t.Cleanup(func() { + testPool.Exec(context.Background(), `DELETE FROM agent_task_queue WHERE id = $1`, taskID) + }) + + return leaderTaskScopeFixture{ + crossSquadMentionFixture: base, + TaskID: taskID, + } +} + +// agentIDSetFromListAgentsResponse extracts the set of agent IDs present in a +// ListAgents JSON response body. +func agentIDSetFromListAgentsResponse(t *testing.T, body []byte) map[string]struct{} { + t.Helper() + var resp []AgentResponse + if err := json.Unmarshal(body, &resp); err != nil { + t.Fatalf("decode ListAgents response: %v", err) + } + set := make(map[string]struct{}, len(resp)) + for _, a := range resp { + set[a.ID] = struct{}{} + } + return set +} + +func newAgentTaskListAgentsRequest(agentID, taskID string, scope string) *http.Request { + q := "" + if scope != "" { + q = "?scope=" + scope + } + req := newRequest("GET", "/api/agents"+q, nil) + req.Header.Set("X-Agent-ID", agentID) + req.Header.Set("X-Task-ID", taskID) + return req +} + +// TestListAgents_TaskSquadScope_FiltersToSquadMembers locks in the squad-leader +// scoping path: when an agent actor on a leader task asks for `?scope=task_squad`, +// the response includes the squad leader and squad_member agents, but NOT +// workspace-peers that are outside the squad. +func TestListAgents_TaskSquadScope_FiltersToSquadMembers(t *testing.T) { + if testHandler == nil || testPool == nil { + t.Skip("database not available") + } + fx := newLeaderTaskScopeFixture(t, true) + + w := httptest.NewRecorder() + testHandler.ListAgents(w, newAgentTaskListAgentsRequest(fx.LeaderID, fx.TaskID, "task_squad")) + if w.Code != http.StatusOK { + t.Fatalf("ListAgents: expected 200, got %d: %s", w.Code, w.Body.String()) + } + + got := agentIDSetFromListAgentsResponse(t, w.Body.Bytes()) + if _, ok := got[fx.LeaderID]; !ok { + t.Errorf("scoped list missing leader %s — leader fallback expected to keep them visible", fx.LeaderID) + } + if _, ok := got[fx.WorkerID]; !ok { + t.Errorf("scoped list missing squad worker %s", fx.WorkerID) + } + if _, ok := got[fx.OutsiderID]; ok { + t.Errorf("scoped list unexpectedly contains outsider %s — out-of-roster agent leaked through", fx.OutsiderID) + } +} + +// TestListAgents_TaskSquadScope_NoOpForNonLeaderTask proves the scope param is +// safely ignored when the requesting task is not a leader task — workers +// keep their full A2A view of the workspace. +func TestListAgents_TaskSquadScope_NoOpForNonLeaderTask(t *testing.T) { + if testHandler == nil || testPool == nil { + t.Skip("database not available") + } + fx := newLeaderTaskScopeFixture(t, false) + + w := httptest.NewRecorder() + testHandler.ListAgents(w, newAgentTaskListAgentsRequest(fx.LeaderID, fx.TaskID, "task_squad")) + if w.Code != http.StatusOK { + t.Fatalf("ListAgents: expected 200, got %d: %s", w.Code, w.Body.String()) + } + + got := agentIDSetFromListAgentsResponse(t, w.Body.Bytes()) + if _, ok := got[fx.OutsiderID]; !ok { + t.Errorf("non-leader task with scope=task_squad should keep outsider %s visible (no-op)", fx.OutsiderID) + } +} + +// TestListAgents_TaskSquadScope_FullListWithoutScopeParam confirms the legacy +// behavior — no scope param means the existing A2A-visible list is returned +// unchanged, even on a leader task. This is the path `--all` on the CLI +// takes. +func TestListAgents_TaskSquadScope_FullListWithoutScopeParam(t *testing.T) { + if testHandler == nil || testPool == nil { + t.Skip("database not available") + } + fx := newLeaderTaskScopeFixture(t, true) + + w := httptest.NewRecorder() + testHandler.ListAgents(w, newAgentTaskListAgentsRequest(fx.LeaderID, fx.TaskID, "")) + if w.Code != http.StatusOK { + t.Fatalf("ListAgents: expected 200, got %d: %s", w.Code, w.Body.String()) + } + + got := agentIDSetFromListAgentsResponse(t, w.Body.Bytes()) + if _, ok := got[fx.OutsiderID]; !ok { + t.Errorf("no scope param: outsider %s must remain visible (existing behavior)", fx.OutsiderID) + } +} diff --git a/server/internal/handler/squad_briefing.go b/server/internal/handler/squad_briefing.go index e77ec9c8df..ad94954816 100644 --- a/server/internal/handler/squad_briefing.go +++ b/server/internal/handler/squad_briefing.go @@ -81,7 +81,17 @@ Hard rules: other suitable members. The squad exists so work is split — bypassing it defeats the point. - Do NOT @mention members who don't appear in the Squad Roster below; - they are not part of this squad. + they are not part of this squad. The server enforces this for + agent-authored comments on a squad-assigned issue: an out-of-roster + agent mention is silently dropped (no task is ever enqueued for it), + so picking a same-role agent from outside the roster will look like + you delegated but result in nothing happening at all. +- Do NOT call ` + "`" + `multica agent list` + "`" + ` (or any other + workspace-wide listing command) to find collaborators on this issue. + The Squad Roster above is authoritative — every UUID you need to + paste into a mention link is already there. Reaching outside the + roster via the global agent list is how cross-squad mis-dispatch + happens. - One delegation comment per turn is enough. Avoid spamming multiple near-identical comments. - If the squad has no member capable of the task, post a comment diff --git a/server/internal/handler/squad_briefing_test.go b/server/internal/handler/squad_briefing_test.go index 6a7214cf8d..2dbcf64607 100644 --- a/server/internal/handler/squad_briefing_test.go +++ b/server/internal/handler/squad_briefing_test.go @@ -170,6 +170,32 @@ func TestBuildSquadLeaderBriefing_SkipsArchivedAgent(t *testing.T) { } } +// TestBuildSquadLeaderBriefing_OutOfRosterGuardClauses pins the wording that +// tells the leader (a) the server silently drops out-of-roster agent mentions +// on a squad-assigned issue, and (b) `multica agent list` is off-limits for +// finding collaborators. Both lines exist because LLM leaders kept reaching +// outside the roster via the global agent list — see +// squad_cross_squad_mention_test.go for the matching server-side gate. +func TestBuildSquadLeaderBriefing_OutOfRosterGuardClauses(t *testing.T) { + ctx := context.Background() + leaderID, _ := seededLeaderAgent(t) + squad := seedSquadForBriefing(t, leaderID, "Guard Clauses", "") + + out := buildSquadLeaderBriefing(ctx, testHandler.Queries, squad) + + for _, want := range []string{ + // The server-enforcement disclosure — leader needs to know the + // drop is silent so it doesn't waste turns asserting delegation. + "silently dropped", + // The hard ban on the discovery path that leaks cross-squad agents. + "Do NOT call `multica agent list`", + } { + if !strings.Contains(out, want) { + t.Errorf("expected briefing to contain %q\n--- briefing ---\n%s", want, out) + } + } +} + // TestBuildSquadLeaderBriefing_MentionsRoundTrip is the contract test // guaranteeing every emitted mention markdown string parses back through // util.ParseMentions to its (type, id). If this ever breaks, the leader's diff --git a/server/internal/handler/squad_cross_squad_mention_test.go b/server/internal/handler/squad_cross_squad_mention_test.go new file mode 100644 index 0000000000..3f76ec52b1 --- /dev/null +++ b/server/internal/handler/squad_cross_squad_mention_test.go @@ -0,0 +1,281 @@ +package handler + +import ( + "context" + "testing" + + "github.com/multica-ai/multica/server/internal/util" + db "github.com/multica-ai/multica/server/pkg/db/generated" +) + +// crossSquadMentionFixture seeds a squad-assigned issue with three agents: +// +// - leader : the squad's leader (seeded test agent, has runtime). The +// fixture INSERTs the squad via raw SQL without an auto squad_member row +// for the leader, exercising the "leader not in squad_member rows but +// IS the squad's LeaderID" defensive branch of the cross-squad gate. +// - worker : a freshly created agent added to the squad via squad_member. +// Represents the squad's intended dispatch target. +// - outsider: a freshly created agent in the SAME workspace but NOT in the +// squad. Represents the failure mode where the leader, via +// `multica agent list` (A2A-bypassed), picks a same-role same-workspace +// agent that lives in a different squad / no squad at all. +// +// Mirrors the structure of squadCommentTriggerFixture but adds the explicit +// worker / outsider split so we can assert who got triggered without relying +// on global side-effects. +type crossSquadMentionFixture struct { + LeaderID string + WorkerID string + OutsiderID string + SquadID string + IssueID string + Issue db.Issue +} + +func newCrossSquadMentionFixture(t *testing.T) crossSquadMentionFixture { + t.Helper() + ctx := context.Background() + + // Seeded leader agent (has runtime via the fixture pool). + var leaderID string + if err := testPool.QueryRow(ctx, ` + SELECT id FROM agent WHERE workspace_id = $1 ORDER BY created_at ASC LIMIT 1 + `, testWorkspaceID).Scan(&leaderID); err != nil { + t.Fatalf("load leader agent: %v", err) + } + + workerID := createHandlerTestAgent(t, "Cross-Squad Worker", nil) + outsiderID := createHandlerTestAgent(t, "Cross-Squad Outsider", nil) + + // Raw SQL squad insert — deliberately skips the auto squad_member row for + // the leader. The cross-squad gate must still treat the leader as + // in-squad via squad.LeaderID fallback. + var squadID string + if err := testPool.QueryRow(ctx, ` + INSERT INTO squad (workspace_id, name, description, leader_id, creator_id) + VALUES ($1, $2, '', $3, $4) + RETURNING id + `, testWorkspaceID, "Cross-Squad Mention Squad", leaderID, testUserID).Scan(&squadID); err != nil { + t.Fatalf("create squad: %v", err) + } + t.Cleanup(func() { + testPool.Exec(context.Background(), `DELETE FROM squad WHERE id = $1`, squadID) + }) + + // Add worker as an explicit squad_member row. Outsider is intentionally + // NOT added. + if _, err := testPool.Exec(ctx, ` + INSERT INTO squad_member (squad_id, member_type, member_id, role) + VALUES ($1, 'agent', $2, 'worker') + `, squadID, workerID); err != nil { + t.Fatalf("add worker as squad_member: %v", err) + } + + // Per-workspace unique issue number — see selfMentionFixture for the same + // counter-bump pattern. + var number int + if err := testPool.QueryRow(ctx, ` + UPDATE workspace + SET issue_counter = GREATEST(issue_counter, (SELECT COALESCE(MAX(number), 0) FROM issue WHERE workspace_id = $1)) + 1 + WHERE id = $1 RETURNING issue_counter + `, testWorkspaceID).Scan(&number); err != nil { + t.Fatalf("next issue number: %v", err) + } + + var issueID string + if err := testPool.QueryRow(ctx, ` + INSERT INTO issue (workspace_id, creator_type, creator_id, title, assignee_type, assignee_id, number) + VALUES ($1, 'member', $2, $3, 'squad', $4, $5) + RETURNING id + `, testWorkspaceID, testUserID, "cross-squad mention gate", squadID, number).Scan(&issueID); err != nil { + t.Fatalf("create issue: %v", err) + } + t.Cleanup(func() { + testPool.Exec(context.Background(), `DELETE FROM agent_task_queue WHERE issue_id = $1`, issueID) + testPool.Exec(context.Background(), `DELETE FROM comment WHERE issue_id = $1`, issueID) + testPool.Exec(context.Background(), `DELETE FROM issue WHERE id = $1`, issueID) + }) + + issue, err := testHandler.Queries.GetIssue(ctx, util.MustParseUUID(issueID)) + if err != nil { + t.Fatalf("load issue: %v", err) + } + + return crossSquadMentionFixture{ + LeaderID: leaderID, + WorkerID: workerID, + OutsiderID: outsiderID, + SquadID: squadID, + IssueID: issueID, + Issue: issue, + } +} + +// insertComment inserts a comment on the given issue and returns the loaded row. +func insertCrossSquadComment(t *testing.T, issueID, authorType, authorID, content string) db.Comment { + t.Helper() + ctx := context.Background() + var commentID string + if err := testPool.QueryRow(ctx, ` + INSERT INTO comment (workspace_id, issue_id, author_type, author_id, content) + VALUES ($1, $2, $3, $4, $5) + RETURNING id + `, testWorkspaceID, issueID, authorType, authorID, content).Scan(&commentID); err != nil { + t.Fatalf("insert comment: %v", err) + } + t.Cleanup(func() { + testPool.Exec(context.Background(), `DELETE FROM comment WHERE id = $1`, commentID) + }) + got, err := testHandler.Queries.GetComment(ctx, util.MustParseUUID(commentID)) + if err != nil { + t.Fatalf("load comment: %v", err) + } + return got +} + +// TestEnqueueMentionedAgentTasks_SquadAssignedAgentAuthor_DropsOutsider covers +// the squad-leader cross-squad failure mode: when an issue is assigned to a +// squad and an agent author @-mentions an agent that is NOT a member of that +// squad, the @mention must NOT enqueue a task. Member-only constraint: agent +// authors are gated; member (human) authors are exempt (see the matching +// test below). +func TestEnqueueMentionedAgentTasks_SquadAssignedAgentAuthor_DropsOutsider(t *testing.T) { + if testHandler == nil || testPool == nil { + t.Skip("database not available") + } + ctx := context.Background() + fx := newCrossSquadMentionFixture(t) + + content := "[@Outsider](mention://agent/" + fx.OutsiderID + ") please pick this up" + comment := insertCrossSquadComment(t, fx.IssueID, "agent", fx.LeaderID, content) + + if got := countQueuedOrDispatched(t, fx.OutsiderID, fx.IssueID); got != 0 { + t.Fatalf("before: expected 0 tasks for outsider, got %d", got) + } + + testHandler.enqueueMentionedAgentTasks(ctx, fx.Issue, comment, nil, "agent", fx.LeaderID) + + if got := countQueuedOrDispatched(t, fx.OutsiderID, fx.IssueID); got != 0 { + t.Fatalf("cross-squad @mention from squad-leader agent author MUST be dropped, got %d task(s) for outsider", got) + } +} + +// TestEnqueueMentionedAgentTasks_SquadAssignedAgentAuthor_EnqueuesMember +// is the positive counterpart: when an agent author @mentions an agent that +// IS a member of the issue's squad, the task must still be enqueued. Pins +// down that the gate doesn't over-block. +func TestEnqueueMentionedAgentTasks_SquadAssignedAgentAuthor_EnqueuesMember(t *testing.T) { + if testHandler == nil || testPool == nil { + t.Skip("database not available") + } + ctx := context.Background() + fx := newCrossSquadMentionFixture(t) + + content := "[@Worker](mention://agent/" + fx.WorkerID + ") please pick this up" + comment := insertCrossSquadComment(t, fx.IssueID, "agent", fx.LeaderID, content) + + if got := countQueuedOrDispatched(t, fx.WorkerID, fx.IssueID); got != 0 { + t.Fatalf("before: expected 0 tasks for worker, got %d", got) + } + + testHandler.enqueueMentionedAgentTasks(ctx, fx.Issue, comment, nil, "agent", fx.LeaderID) + + if got := countQueuedOrDispatched(t, fx.WorkerID, fx.IssueID); got != 1 { + t.Fatalf("in-squad @mention from squad-leader agent author MUST be enqueued, got %d task(s) for worker", got) + } +} + +// TestEnqueueMentionedAgentTasks_SquadAssignedAgentAuthor_AllowsLeaderFallback +// proves the defensive branch: when the squad's leader is NOT present in the +// squad_member rows (legacy squad, or a raw-SQL insert that bypassed the +// CreateSquad auto-add), the cross-squad gate still treats squad.LeaderID +// as in-squad. This keeps a leader's own subsequent task chain working on +// legacy squads. +func TestEnqueueMentionedAgentTasks_SquadAssignedAgentAuthor_AllowsLeaderFallback(t *testing.T) { + if testHandler == nil || testPool == nil { + t.Skip("database not available") + } + ctx := context.Background() + fx := newCrossSquadMentionFixture(t) + + // Sanity: the fixture's leader is NOT in squad_member rows. + var memberCount int + if err := testPool.QueryRow(ctx, ` + SELECT count(*) FROM squad_member WHERE squad_id = $1 AND member_type = 'agent' AND member_id = $2 + `, fx.SquadID, fx.LeaderID).Scan(&memberCount); err != nil { + t.Fatalf("count leader squad_member rows: %v", err) + } + if memberCount != 0 { + t.Fatalf("fixture invariant broken: leader unexpectedly present in squad_member rows (count=%d)", memberCount) + } + + // A different agent author @mentions the leader (e.g. the worker pinging + // the leader back). We expect the gate to allow it because the @target + // is the squad's leader. + content := "[@Leader](mention://agent/" + fx.LeaderID + ") update — done" + comment := insertCrossSquadComment(t, fx.IssueID, "agent", fx.WorkerID, content) + + if got := countQueuedOrDispatched(t, fx.LeaderID, fx.IssueID); got != 0 { + t.Fatalf("before: expected 0 tasks for leader, got %d", got) + } + + testHandler.enqueueMentionedAgentTasks(ctx, fx.Issue, comment, nil, "agent", fx.WorkerID) + + if got := countQueuedOrDispatched(t, fx.LeaderID, fx.IssueID); got != 1 { + t.Fatalf("agent → leader @mention MUST be enqueued via leader fallback, got %d task(s) for leader", got) + } +} + +// TestEnqueueMentionedAgentTasks_SquadAssignedAgentAuthor_MixedMentions +// proves the gate is per-mention, not per-comment: when a single comment +// @mentions both an in-squad member and an outsider, the member's task is +// enqueued and the outsider's mention is dropped. Without this guarantee +// a buggy leader could "smuggle" cross-squad dispatch by burying the +// outside agent next to a legitimate squad member in the same comment. +func TestEnqueueMentionedAgentTasks_SquadAssignedAgentAuthor_MixedMentions(t *testing.T) { + if testHandler == nil || testPool == nil { + t.Skip("database not available") + } + ctx := context.Background() + fx := newCrossSquadMentionFixture(t) + + content := "[@Worker](mention://agent/" + fx.WorkerID + ") please own the backend; " + + "[@Outsider](mention://agent/" + fx.OutsiderID + ") please own the frontend" + comment := insertCrossSquadComment(t, fx.IssueID, "agent", fx.LeaderID, content) + + testHandler.enqueueMentionedAgentTasks(ctx, fx.Issue, comment, nil, "agent", fx.LeaderID) + + if got := countQueuedOrDispatched(t, fx.WorkerID, fx.IssueID); got != 1 { + t.Errorf("squad worker mention should enqueue, got %d task(s) for worker", got) + } + if got := countQueuedOrDispatched(t, fx.OutsiderID, fx.IssueID); got != 0 { + t.Errorf("outsider mention should be dropped, got %d task(s) for outsider", got) + } +} + +// TestEnqueueMentionedAgentTasks_SquadAssignedMemberAuthor_BypassesGate locks +// in that the cross-squad gate only constrains agent-authored comments. +// Member (human) authors keep their full @mention agency — they may +// deliberately reach outside the squad (e.g. a workspace owner pinging a +// specialist on a squad-assigned issue). +func TestEnqueueMentionedAgentTasks_SquadAssignedMemberAuthor_BypassesGate(t *testing.T) { + if testHandler == nil || testPool == nil { + t.Skip("database not available") + } + ctx := context.Background() + fx := newCrossSquadMentionFixture(t) + + content := "[@Outsider](mention://agent/" + fx.OutsiderID + ") please pick this up" + comment := insertCrossSquadComment(t, fx.IssueID, "member", testUserID, content) + + if got := countQueuedOrDispatched(t, fx.OutsiderID, fx.IssueID); got != 0 { + t.Fatalf("before: expected 0 tasks for outsider, got %d", got) + } + + testHandler.enqueueMentionedAgentTasks(ctx, fx.Issue, comment, nil, "member", testUserID) + + if got := countQueuedOrDispatched(t, fx.OutsiderID, fx.IssueID); got != 1 { + t.Fatalf("member-authored @mention MUST bypass the cross-squad gate, got %d task(s) for outsider", got) + } +}