mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-13 05:16:29 +02:00
* feat(agents): agent invocation permission system (permission_mode + invocation targets) MUL-3963: split who may INVOKE an agent out of the overloaded visibility column into an explicit, extensible model on feature/composio-integration. - DB: agent.permission_mode (private|public_to) + agent_invocation_target table (workspace/member/team targets) + lossless backfill from visibility (migration 130). - canInvokeAgent: owner-only for private (NO admin bypass, NO A2A bypass); public_to honours the allow-list; A2A judged by the top-of-chain originator. - All trigger paths rewired: issue assign, comment @agent/@squad, chat, quick-create, autopilot, squad leader, child-done. - Agent API: permission_mode + invocation_targets on responses and create/update (owner-only writes); legacy visibility kept as a derived field so old clients never see a permission widening. - Composio: BuildTaskOverlay now FOLLOWS invocation permission and uses the agent OWNER connection (removed the originator==owner gate); front-end warns when a shared agent enables Composio apps. - CLI: --permission-mode / --public-to-workspace / --public-to-member (legacy --visibility still mapped). - Frontend: AccessPicker (Private / workspace / specific people / team soon), permission rules mirror canInvokeAgent, Composio warning banner. - Tests: migration backfill, admin cannot invoke others private, public_to workspace/member whitelist, A2A by originator, Composio overlay uses owner connection. Co-authored-by: multica-agent <github@multica.ai> * feat(agents): stackable, mixed public_to invocation targets (MUL-3963) Follow-up on PR #4844: public_to now supports selecting MULTIPLE, MIXED targets on one agent (e.g. Public to workspace + specific people + team), with canInvokeAgent admitting on ANY matching target (OR). - Frontend AccessPicker: reworked from a single exclusive kind into a stackable multi-select — an "Everyone in workspace" toggle, a member multi-select checklist, and a (disabled, v1) team placeholder can be combined freely. Emits the full union of selected targets; empty union collapses to Private. Existing team targets are preserved across saves. Added the access.public_group locale string (en/zh-Hans/ja/ko). - Backend already supported this (agent_invocation_target is multi-row per agent; create/update take a target ARRAY and batch-replace the whole allow-list; canInvokeAgent OR-matches). Added tests to lock it in: mixed member+team targets, overlapping-member batch replace, and workspace+member stacking then narrowing. Refs MUL-3963. Co-authored-by: multica-agent <github@multica.ai> * fix(agents): address review on invocation permission (MUL-3963) 张大彪 review on PR #4844 — three blockers + product ruling + nits: 1. Migration 130: drop the FK/cascade on agent_invocation_target (agent_id, created_by) per the Multica no-FK rule; relationships are now maintained in the app layer (matching MUL-3515 §4). Added DeleteAgentInvocationTargetsByArchivedRuntimeAgents and call it before DeleteArchivedAgentsByRuntime in all three runtime-delete paths (runtime.go x2, runtime_profile.go) so hard-deleting agents can't orphan target rows. 2. revokeAndRemoveMember: prune the leaving member's member-target grants (DeleteAgentInvocationTargetsByMember) in the same tx as the member-row delete, so a re-invited user can't reclaim a stale invocation grant. 3. Empty public_to is a phantom — parsePermissionInput now normalises a public_to with no resolvable targets to a single workspace target, so `--permission-mode public_to` alone (and any empty target array) means "public to workspace" instead of "shared but nobody can run it". Product ruling: the system/no-human-originator → workspace-target path in canInvokeAgent is a deliberate, documented exception (webhook/system/ workspace-wide automation); member/team targets still fail closed without a resolved originator. Documented in code + locked with a test. Nits: refreshed the stale "originator must be owner" comments — models.go (via migration 130 COMMENT ON COLUMN + sqlc regen for composio_toolkit_allowlist and originator_user_id) and agent-mcp-tab.tsx — to the owner-connection + invocation-permission rules. Tests: member remove/re-add regression, system workspace exception + member fail-closed, empty public_to → workspace (plus the earlier mixed/overlap/ batch-replace suite). Migration 130 applied to the test DB; Go handler/service/ composio suites green; views typecheck clean. Refs MUL-3963. Co-authored-by: multica-agent <github@multica.ai> * fix(agents): scope member invocation-target cleanup to one workspace (MUL-3963) 张大彪 3rd review — cross-workspace permission bug + comment nits: - DeleteAgentInvocationTargetsByMember was a GLOBAL delete by user id, so removing a user from workspace A also wiped their member-target grants on agents in workspace B. Scoped it to a single workspace by joining through agent.workspace_id; revokeAndRemoveMember now passes (workspaceID, userID). - Regression test TestRevokeMember_InvocationTargetCleanupIsWorkspaceScoped: same user allow-listed by agents in two workspaces; removal from one leaves the other workspace's target intact. - Nits: refreshed the remaining stale "originator == agent.owner_id" / "owner-vs-originator" comments — CreateRetryTask (agent.sql, regenerated), and the AgentResponse allowlist doc + ListAgents/UpdateAgent redaction rationale in agent.go — to the owner-connection + invocation-permission rule. Migration 130 applied to the test DB; Go handler/service/composio suites green; go vet clean. Refs MUL-3963. Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: multica-agent <github@multica.ai>
118 lines
3.8 KiB
Go
118 lines
3.8 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{}
|
|
}
|
|
|
|
func TestHasAgentOrSquadMention(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
content string
|
|
want bool
|
|
}{
|
|
{"plain", "just a plain comment", false},
|
|
{"agent", fmt.Sprintf("[@Agent](mention://agent/%s) please fix", agentAssigneeID), true},
|
|
{"squad", "[@Squad](mention://squad/aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa) please coordinate", true},
|
|
{"member only", fmt.Sprintf("[@Bob](mention://member/%s) take a look", memberID), false},
|
|
{"issue only", "[PAN-1](mention://issue/44c266e7-f6dd-4be3-9140-5ac40233f79c) is related", false},
|
|
{"all only", "[@all](mention://all/all) heads up", false},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
got := hasAgentOrSquadMention(parseMentionsForTest(tt.content))
|
|
if got != tt.want {
|
|
t.Errorf("hasAgentOrSquadMention() = %v, want %v", got, tt.want)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func parseMentionsForTest(content string) []util.Mention {
|
|
return util.ParseMentions(content)
|
|
}
|
|
|
|
// -------------------------------------------------------------------
|
|
// 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 the cascade tries to resolve mentions, parent ownership, or assignee
|
|
// fallback. A nil-Queries Handler would panic if the /note guard were missing or
|
|
// moved below those branches.
|
|
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, memberID, nil)
|
|
}
|