mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-13 05:16:29 +02:00
* feat(composio): server-side connect flow + connections REST (Notion MVP) (MUL-3720) (#4608)
* feat(composio): server-side connect flow + connections REST (Notion MVP) (MUL-3720)
Compose the merged server/pkg/composio SDK into a user-facing connection
manager: signed-state connect handshake, local user_composio_connection
mirror, idempotent disconnect, and a per-user MCP session helper (not yet
wired into task dispatch).
- migration 127_user_composio_connection (no FK/cascade, per DB rules)
- sqlc queries: upsert (idempotent on user_id+connected_account_id), list
active, owner-scoped get, mark revoked
- internal/integrations/composio: signed HMAC-SHA256 state, BeginConnect,
CompleteCallback (idempotent upsert), ListConnections, Disconnect
(upstream 404 = idempotent success), CreateMCPSession (no-op when empty,
pins connected_accounts per toolkit), CallbackRedirect
- REST handlers under /api/integrations/composio (user-scoped, 503 when
COMPOSIO_API_KEY unset): connect/init, callback (302), connections list,
delete
- router wiring gated by COMPOSIO_API_KEY; COMPOSIO_AUTH_CONFIGS_JSON maps
toolkit->auth_config (MVP: notion); state secret from COMPOSIO_STATE_SECRET
or derived from JWT_SECRET; callback base from COMPOSIO_CALLBACK_BASE_URL
or MULTICA_PUBLIC_URL
- tests: state (expire/tamper/wrong-secret), service (mapping, callback
idempotency, non-success, disconnect owner/404 idempotency, MCP pin),
handlers (httptest), redact regression for Bearer mcp_ tokens
MVP scope: Notion only; no task-dispatch overlay, sharing, or webhook
event handling (later stages).
Co-authored-by: multica-agent <github@multica.ai>
* fix(composio): bind callback account to user + idempotent revoked disconnect (MUL-3720)
Address PR 4608 review (CHANGES_REQUESTED):
- callback: verify connected_account_id with Composio before mirroring it.
The signed state only proved user/toolkit/exp, so a valid state paired with
a tampered connected_account_id would be written verbatim. CompleteCallback
now calls ListConnectedAccounts and fails closed (ErrAccountVerification)
unless the account belongs to the state's user (composio_user_id == multica
user id) and was created under the toolkit's auth config. No row is written
on mismatch / unknown account / upstream error.
- disconnect: short-circuit to a no-op when the local row is already revoked,
before touching upstream. Previously a second DELETE re-hit Composio and a
non-404 upstream error surfaced as a 502, breaking the 204-idempotent
contract.
- CreateMCPSession: document the v1 single-active-connection-per-(user,toolkit)
constraint and make duplicate selection deterministic (newest-wins, rows are
connected_at DESC) instead of order-dependent map overwrite. Stage 3 owns the
real single-account-enforcement vs multi-account-shape decision.
Tests: tampered/wrong-auth-config/unknown-account callback rejection, revoked-row
disconnect no-op (asserts upstream not re-hit). composio pkg 85% coverage; all
green.
Co-authored-by: multica-agent <github@multica.ai>
* feat(composio): list all toolkits + dynamic auth-config resolution (MUL-3720)
Yushen's follow-up to the Notion MVP: surface the full Composio toolkit
catalog, render it in Settings, and drop the static env mapping in favor of
dynamic auth-config discovery.
Config correctness (per Composio docs):
- Remove COMPOSIO_AUTH_CONFIGS_JSON entirely. The toolkit→auth_config mapping
is now resolved at request time from the project's /auth_configs (cached,
5-min TTL), so enabling a toolkit is a dashboard action, not a redeploy.
- Do NOT add COMPOSIO_PROJECT_ID. The project API key (x-api-key) authenticates
to exactly one project; the project is resolved from the key. Only org-level
endpoints use x-org-api-key, which this integration never calls.
Backend:
- SDK: server/pkg/composio/auth_configs.go — ListAuthConfigs (toolkit_slug,
is_composio_managed, show_disabled, limit, cursor).
- service: dynamic resolver (authConfigMap cache; betterAuthConfig prefers a
custom/white-label config over Composio-managed, newest wins); BeginConnect
and CompleteCallback resolve via it; ListToolkits fetches the full catalog
(paginated, capped) annotated with connectable = has an enabled auth config,
connectable-first ordering.
- handler + route: GET /api/integrations/composio/toolkits (user-scoped, 503
when COMPOSIO_API_KEY unset) returning slug/name/logo/category/connectable.
Frontend:
- core: ComposioToolkit/ComposioConnection types, api client methods, and
composio query options (@multica/core/composio).
- views: Settings → Integrations now has a Composio section rendering every
toolkit as a card with search. Connect is gated on `connectable`;
non-connectable toolkits show a muted "not configured" hint instead of a
dead button. Connected toolkits show a badge + Disconnect (with confirm).
- i18n: composio block added to en/zh-Hans/ja/ko settings.
Tests: SDK + service (dynamic resolution, custom-over-managed preference,
connectable flag, resolver-error soft-degrade) and handler toolkits endpoint;
composio pkg 85.7% coverage. go build/vet/gofmt clean; core+views typecheck,
core+views lint, and core tests (691) all green.
Co-authored-by: multica-agent <github@multica.ai>
* fix(composio): close cross-toolkit callback fail-open by signing auth_config_id into state (MUL-3720)
Re-review blocker: CompleteCallback resolved the toolkit's auth config at
callback time and ignored a resolve error/empty result, while
verifyAccountOwnership skipped the auth-config comparison when the expected
value was empty. A user could then pass another toolkit's connected_account_id
into this toolkit's callback — the owner check passed and it was written under
the wrong toolkit_slug/account binding.
Fix: the auth_config_id is already resolved in BeginConnect (before the state
is signed), so sign it into the state and compare it exactly at callback. No
re-resolve, no fail-open. verifyAccountOwnership now fails closed when the
expected auth config is empty (rejects instead of skipping) and requires an
exact match — closing the cross-toolkit binding gap.
Tests: state round-trips auth_config_id; BeginConnect signs it; callback
rejects wrong/cross-toolkit auth config and an empty (no-mapping) auth config
fails closed. composio pkg 85.2% coverage, all green.
Frontend (non-blocking): the Composio settings tab now surfaces an error when
the connections query fails instead of silently rendering everything as
unconnected.
Co-authored-by: multica-agent <github@multica.ai>
* fix(composio): hide Settings section entirely when integration unconfigured (MUL-3720)
Decision (option 2, hide-then-merge): don't show a card that leaks the internal
COMPOSIO_API_KEY env-var name to every end user. IntegrationsTab now gates the
whole Composio section (heading + body) on the toolkits query — a 503 means the
key is unset, so the section is withheld instead of rendering the not-configured
card. Admin-only setup guidance is a later, role-gated affordance.
Removed the notConfigured card (and now-unused ApiError import) from
ComposioTab; it only mounts when configured. views typecheck + lint clean.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: multica-agent <github@multica.ai>
* feat(composio): Stage 2 frontend polish — callback toast, last_used & expired UI, e2e (MUL-3718) (#4688)
* feat(composio): callback toast + refresh, last_used & expired UI, e2e (MUL-3718)
Co-authored-by: multica-agent <github@multica.ai>
* fix(composio): real callback redirect route + StrictMode-safe toast dedup (MUL-3718 review)
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: multica-agent <github@multica.ai>
* fix(composio): callback endpoint should not require Multica auth (MUL-3843) (#4709)
* fix(composio): move OAuth callback out of the Auth group (MUL-3843)
Composio 302-redirects the browser to /api/integrations/composio/callback
at the end of the OAuth flow, but PR #4608 mounted it inside the cookie-auth
middleware group. When the session cookie is absent (expired session,
SameSite=Strict / Safari ITP, private window, self-hosted callback subdomain)
the Auth middleware returned a hard 401 and a JSON blob instead of the
settings redirect, breaking the flow.
Identity never came from the cookie anyway: it is carried by the HMAC-signed
state param that CompleteCallback verifies (signature, expiry, replay) and
cross-checked by verifyAccountOwnership; h.Composio == nil still 503s. So the
callback is registered alongside the other public OAuth/webhook routes; the
other four composio endpoints stay session-gated.
Refs MUL-3843, MUL-3715.
Co-authored-by: multica-agent <github@multica.ai>
* fix(composio): correct stale callback routing comments (MUL-3843)
The package header and ComposioCallback doc comments still described the
callback as sitting under the Auth middleware group. After the route was
moved out (this PR), update both to state it is a public route whose identity
comes from the signed state — addressing review nit from 张大彪.
Refs MUL-3843.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: multica-agent <github@multica.ai>
* feat(composio): inject MCP overlay into agent runtime at task dispatch (MUL-3721) (#4704)
Stage 3 of the Composio epic. Wires the per-user Composio MCP session into
every agent task so the agent process sees the initiator's connected tools
without any prompt-time plumbing.
Server side
- Migration 128 adds agent_task_queue.runtime_mcp_overlay JSONB plus a
BEFORE-UPDATE trigger that wipes the column on any transition into a
terminal status (completed / failed / cancelled). A trigger is the single
source of truth — future queries that flip status cannot bypass it.
- composio.Service.BuildTaskOverlay(userID) reuses CreateMCPSession and
emits the Claude-style { mcpServers: { composio: { type: http, url,
headers } } } shape the daemon's existing sidecar generators consume.
Returns (nil, nil) on zero active connections so we never burn a
Composio session for a user with nothing to call.
- TaskService grows a Composio ComposioOverlayBuilder seam, wired in
router.go after composiointeg.NewService succeeds. Five enqueue paths
(issue / mention / quick-create / chat / auto-retry) attach the overlay
after CreateAgentTask returns and before the daemon is notified — so
every claim reads a settled row, with no second daemon hop. Best-effort:
a builder failure logs and proceeds with no overlay.
- resolveInitiatorFromTriggerComment derives the initiator user from the
trigger comment when it was authored by a member. Agent-authored
triggers are not treated as initiators (their connected-apps view is
empty by construction).
Daemon side
- handler/daemon.go claim path merges task.runtime_mcp_overlay onto
agent.mcp_config via mergeMCPOverlay before populating
TaskAgentData.McpConfig. Overlay wins on server-name collisions
because it carries the live user-scoped session URL. Errors fall back
to the agent config unchanged — a bad overlay must not surprise-disable
saved MCP tools. The existing execenv sidecar generators (cursor /
codex / openclaw / opencode / hermes-kiro) need no changes: they keep
consuming the merged result through TaskAgentData.McpConfig.
Tests
- 9 merge cases (mcp_overlay_test): both-nil short-circuit, agent-only
pass-through, overlay-only canonicalization, two-side merge, name
collision (overlay wins), top-level key preservation, malformed agent
fallback, malformed overlay fallback, non-object server rejection.
- 4 dispatch cases (composio): zero-connections returns nil without
CreateSession, happy-path emits the right shape with the right user
id, empty-URL defensive branch, SDK error surfacing.
- 4 TaskService helper cases: nil Composio is a no-op (Queries-safe),
invalid initiator does not call the builder, nil overlay skips the
UPDATE, builder error swallowed without panic.
- Migration 128 verified to roll up + down + up cleanly against the test
database.
Out of scope (deferred): assignment-triggered enqueue paths with no
trigger comment get no overlay attached today (no initiator UUID flows
through enqueueIssueTask in that case). Retry paths recompute the overlay
fresh from the parent's initiator_user_id instead of inheriting the bearer
from the parent row, so a stale token can never resurface on a retry.
Co-authored-by: Eve <eve@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
* feat(composio): per-agent allowlist + originator-scoped MCP overlay (MUL-3869) (#4736)
* feat(composio): per-agent allowlist + originator-scoped MCP overlay (MUL-3869)
Stage 3.1 of the Composio epic (MUL-3721 parent). PR #4704 wired in the
runtime_mcp_overlay column and a per-task dispatch hook; this change
inverts the default from "all-on" to opt-in and locks the overlay to the
agent owner's own connected apps:
- Agents carry composio_toolkit_allowlist TEXT[]. NULL or [] => no MCP.
Owner-only read/write; non-owner GET/PUT silently redacts/drops the
field (same shape as mcp_config).
- agent_task_queue carries originator_user_id UUID. Set from the
top-of-chain HUMAN at every enqueue path:
* issue/mention comment by member -> author_id
* issue/mention comment by agent -> inherit via comment.source_task_id
-> parent task originator_user_id
* quick-create -> requester_id
* chat -> initiator_user_id
* retry -> SQL-inherited from parent row
* autopilot -> NULL (system-driven)
- BuildTaskOverlay (composio dispatch) now takes (ctx, originatorUserID,
agent) and short-circuits on five gates: invalid originator,
originator != agent.owner_id, empty allowlist, empty intersection of
allowlist ∩ active connections, defensive empty session URL. Composio
CreateSession is called with BOTH `toolkits.slugs` (the intersection)
AND `connected_accounts` (the pinned account ids), narrowing the
tool-router twice.
- The originator-vs-owner gate closes the agent-fanout privacy hole: any
workspace member who can @-mention a public agent used to project the
owner's connected apps into their run. Now the overlay only mounts
when the human at the top of the chain IS the agent owner.
Tests:
- dispatch_test.go covers all 5 gates plus uppercase/whitespace slug
normalisation.
- task_runtime_mcp_overlay_test.go covers the no-op gates of the new
applyRuntimeMCPOverlay signature.
- agent_composio_allowlist_test.go (handler): owner roundtrip
(list/empty/null), workspace-admin silent-drop, owner-only GET
visibility, pure normaliseComposioToolkitAllowlist.
- resolve_originator_test.go (service, DB-backed): member-authored,
agent-authored inherits via comment.source_task_id, invalid id.
Migration 129 up/down/up verified against docker postgres.
Co-authored-by: multica-agent <github@multica.ai>
* chore(composio): gofmt + regenerate sqlc with v1.31.1 (MUL-3869 review nits)
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: Eve <eve@multica-ai.local>
Co-authored-by: multica-agent <github@multica.ai>
* fix(composio): accept nested connected account auth config
* feat(views): creator-only MCP tab for per-agent Composio allowlist (MUL-3870) (#4743)
Stage 3.2 frontend on top of the Stage 3.1 backend (MUL-3869, 4708dba97).
Adds an agent-detail tab that lets the agent owner pick which of their own
active Composio connections this agent may mount as MCP servers, writing the
selection to agent.composio_toolkit_allowlist via the existing PUT /api/agents.
- core/types: composio_toolkit_allowlist (+ _redacted) on Agent; tri-state
composio_toolkit_allowlist on UpdateAgentRequest (omit/no-change, null/clear,
array/replace), matching the backend contract.
- core/agents: useUpdateAgentAllowlist - optimistic mutation hook (patches the
cached workspace agent list, rolls back on error, invalidates on settle).
- views: AgentMcpTab renders the owner's active connections as checkboxes;
empty state links to Settings -> Integrations; defensive redacted state.
- views: wired into AgentOverviewPane as tab "composio_mcp", labeled "MCP Apps"
to disambiguate from the existing raw-JSON "MCP" (mcp_config) tab. The entry
is gated to the creator (currentUserId === agent.owner_id), matching the
backend's owner-only read/write of the allowlist.
- i18n: tabs.composio_mcp + tab_body.composio_mcp.* in en/ja/ko/zh-Hans.
- tests: agent-mcp-tab.test.tsx (gating, toggle->allowlist body, active-only,
empty, redacted); e2e/agent-mcp.spec.ts (creator sees tab + PUT body,
non-creator hidden) with Composio + agent endpoints mocked at the boundary.
Note: the product spec says "creator"; the schema has no creator_id - the
backend gate and redaction are keyed on owner_id, so the tab uses owner_id.
Co-authored-by: multica-agent <github@multica.ai>
* fix(composio): mount remote MCP for codex
* feat(agents): agent invocation permission system (MUL-3963) (#4844)
* 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>
* fix(agents): agent access owner-only editable, read-only for others (MUL-3963) (#4853)
* fix(agents): make agent access owner-only editable, read-only for others (MUL-3963)
Interaction bug: a non-owner (incl. workspace admin) could open the AccessPicker
and set an agent public — the backend silently ignored it and the UI bounced
back to private. Access is owner-only, so non-owners must see a read-only state
and the backend must reject real changes explicitly.
Frontend:
- AccessPicker renders a static, non-interactive read-only state when the
viewer is not the owner: the current access value + a lock affordance + a
tooltip "Only the agent owner can change who can run this agent." No clickable
trigger is rendered, so a non-owner can never open a control the backend would
reject (the GitHub/Notion pattern for permission settings you can see but not
edit). The editable multi-select picker is unchanged for the owner.
- agent-detail-inspector gates the picker on ownership specifically
(currentUserId === agent.owner_id), NOT the general canEdit (which also admits
admins, who may edit other fields but not access).
- New locale key access.owner_only_readonly (en/zh-Hans/ja/ko).
Backend:
- UpdateAgent now returns an explicit 403 when a non-owner submits a REAL
permission change (permissionInputChangesAgent compares requested mode +
target set against the persisted state); a no-op resubmit (admin PATCH-as-PUT
echoing unchanged permission) is still tolerated so admin edits of other
fields keep working. Replaces the previous silent-drop that caused the bounce.
Tests:
- access-picker.test.tsx: non-owner gets a non-interactive read-only display
with the owner-only tooltip; owner gets an interactive picker; owner can pick
a member and stack workspace + member.
- TestUpdateAgent_AccessChangeIsOwnerOnly: admin real change → 403; admin no-op
resubmit → 200; admin editing other fields → 200; owner change → 200.
Incidental: fixed a pre-existing base typecheck break in
slash-command-suggestion.test.tsx (stray `signal` arg not in the suggestion
items type) that otherwise fails the whole @multica/views typecheck.
Refs MUL-3963.
Co-authored-by: multica-agent <github@multica.ai>
* fix(agents): compare legacy visibility, not expanded permission, for no-op detection (MUL-3963)
PR #4853 review: permissionInputChangesAgent expanded a legacy-only
visibility:"private" into a real private permission and compared it against the
agent's actual permission. A member-only public_to agent derives legacy
visibility "private", so an admin PATCH-as-PUT echoing visibility:"private"
while editing another field was misread as a public_to→private downgrade and
rejected with 403 — contradicting the "unchanged permission no-op is allowed"
contract.
Fix (per review): when a request carries ONLY legacy `visibility` (no
permission_mode / invocation_targets), derive the agent's CURRENT legacy
visibility from its real targets and compare the legacy string values. Equal =
no-op (allowed); a real legacy change (e.g. "workspace") still returns 403.
Requests that carry permission_mode / invocation_targets keep the precise
mode+target comparison.
Regression test TestUpdateAgent_LegacyVisibilityNoOpForMemberOnlyPublicTo:
member-only public_to agent — admin submitting visibility:"private" + a
non-permission field → 200 with targets unchanged; admin submitting
visibility:"workspace" → 403.
Go handler/composio suites green; migration 130 applied; go vet clean.
Refs MUL-3963.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: multica-agent <github@multica.ai>
* feat(composio): brief agents on connected apps
* feat(composio): gate MCP apps behind feature flag
* fix(mobile): parse agent invocation permissions
* fix(tests): update agent fixtures for access fields
---------
Co-authored-by: multica-agent <github@multica.ai>
Co-authored-by: Multica Eve <eve@devv.ai>
Co-authored-by: Eve <eve@multica.ai>
Co-authored-by: Eve <eve@multica-ai.local>
598 lines
22 KiB
Go
598 lines
22 KiB
Go
package handler
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"testing"
|
|
|
|
"github.com/multica-ai/multica/server/internal/middleware"
|
|
"github.com/multica-ai/multica/server/internal/util"
|
|
db "github.com/multica-ai/multica/server/pkg/db/generated"
|
|
)
|
|
|
|
// TestMemberAllowedToViewAgent_Pure exercises the pure predicate that drives
|
|
// the private-agent VIEW gate. For a private agent it must allow:
|
|
// - workspace owner / admin (regardless of agent ownership)
|
|
// - the agent owner (regardless of role)
|
|
//
|
|
// And deny everyone else. This test runs without a database.
|
|
func TestMemberAllowedToViewAgent_Pure(t *testing.T) {
|
|
ownerUserID := "11111111-1111-1111-1111-111111111111"
|
|
otherUserID := "22222222-2222-2222-2222-222222222222"
|
|
|
|
agent := db.Agent{
|
|
OwnerID: util.MustParseUUID(ownerUserID),
|
|
PermissionMode: "private",
|
|
}
|
|
|
|
cases := []struct {
|
|
name string
|
|
userID string
|
|
role string
|
|
want bool
|
|
}{
|
|
{"workspace owner, not agent owner", otherUserID, "owner", true},
|
|
{"workspace admin, not agent owner", otherUserID, "admin", true},
|
|
{"agent owner with member role", ownerUserID, "member", true},
|
|
{"agent owner with admin role", ownerUserID, "admin", true},
|
|
{"plain member, not agent owner", otherUserID, "member", false},
|
|
{"plain member with no role string", otherUserID, "", false},
|
|
}
|
|
for _, tc := range cases {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
got := memberAllowedToViewAgent(agent, nil, tc.userID, tc.role)
|
|
if got != tc.want {
|
|
t.Fatalf("memberAllowedToViewAgent(userID=%s, role=%s) = %v; want %v",
|
|
tc.userID, tc.role, got, tc.want)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// privateAgentTestFixture sets up a private agent owned by a freshly created
|
|
// user, plus a second non-admin member in the workspace. Returns the agent
|
|
// id, the owner's user id, and the unrelated member's user id. The caller's
|
|
// own testUserID stays workspace owner so it can act as the privileged
|
|
// admin path.
|
|
func privateAgentTestFixture(t *testing.T) (agentID, ownerID, memberID string) {
|
|
t.Helper()
|
|
|
|
ctx := context.Background()
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO "user" (name, email)
|
|
VALUES ('Private Agent Owner', 'private-agent-owner@multica.test')
|
|
RETURNING id
|
|
`).Scan(&ownerID); err != nil {
|
|
t.Fatalf("create owner user: %v", err)
|
|
}
|
|
t.Cleanup(func() {
|
|
testPool.Exec(context.Background(),
|
|
`DELETE FROM "user" WHERE email = 'private-agent-owner@multica.test'`)
|
|
})
|
|
|
|
if _, err := testPool.Exec(ctx, `
|
|
INSERT INTO member (workspace_id, user_id, role)
|
|
VALUES ($1, $2, 'member')
|
|
`, testWorkspaceID, ownerID); err != nil {
|
|
t.Fatalf("add owner as member: %v", err)
|
|
}
|
|
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO "user" (name, email)
|
|
VALUES ('Plain Member', 'plain-member@multica.test')
|
|
RETURNING id
|
|
`).Scan(&memberID); err != nil {
|
|
t.Fatalf("create plain member user: %v", err)
|
|
}
|
|
t.Cleanup(func() {
|
|
testPool.Exec(context.Background(),
|
|
`DELETE FROM "user" WHERE email = 'plain-member@multica.test'`)
|
|
})
|
|
|
|
if _, err := testPool.Exec(ctx, `
|
|
INSERT INTO member (workspace_id, user_id, role)
|
|
VALUES ($1, $2, 'member')
|
|
`, testWorkspaceID, memberID); err != nil {
|
|
t.Fatalf("add plain member: %v", err)
|
|
}
|
|
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO agent (
|
|
workspace_id, name, description, runtime_mode, runtime_config,
|
|
runtime_id, visibility, max_concurrent_tasks, owner_id,
|
|
instructions, custom_env, custom_args
|
|
)
|
|
VALUES ($1, 'private-access-test-agent', '', 'cloud', '{}'::jsonb,
|
|
$2, 'private', 1, $3, '', '{}'::jsonb, '[]'::jsonb)
|
|
RETURNING id
|
|
`, testWorkspaceID, handlerTestRuntimeID(t), ownerID).Scan(&agentID); err != nil {
|
|
t.Fatalf("create private agent: %v", err)
|
|
}
|
|
t.Cleanup(func() {
|
|
testPool.Exec(context.Background(),
|
|
`DELETE FROM agent WHERE id = $1`, agentID)
|
|
})
|
|
|
|
return agentID, ownerID, memberID
|
|
}
|
|
|
|
func newRequestAs(userID, method, path string, body any) *http.Request {
|
|
req := newRequest(method, path, body)
|
|
req.Header.Set("X-User-ID", userID)
|
|
return req
|
|
}
|
|
|
|
// TestGetAgent_PrivateAgentForbidsPlainMember verifies the private-agent
|
|
// visibility gate at the read-detail endpoint: a workspace member who is
|
|
// neither the agent owner nor a workspace owner/admin gets 403, while the
|
|
// agent owner and workspace owner both succeed. Mirrors the four-entry-point
|
|
// gate (chat, history, edit, delete) on its read surface.
|
|
func TestGetAgent_PrivateAgentForbidsPlainMember(t *testing.T) {
|
|
if testHandler == nil {
|
|
t.Skip("database not available")
|
|
}
|
|
|
|
agentID, ownerID, memberID := privateAgentTestFixture(t)
|
|
|
|
// Workspace owner (testUserID): allowed via role.
|
|
w := httptest.NewRecorder()
|
|
testHandler.GetAgent(w, withURLParam(newRequest("GET", "/api/agents/"+agentID, nil), "id", agentID))
|
|
if w.Code != http.StatusOK {
|
|
t.Fatalf("GetAgent as workspace owner: expected 200, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
|
|
// Agent owner (plain member who happens to own the agent): allowed.
|
|
w = httptest.NewRecorder()
|
|
testHandler.GetAgent(w, withURLParam(newRequestAs(ownerID, "GET", "/api/agents/"+agentID, nil), "id", agentID))
|
|
if w.Code != http.StatusOK {
|
|
t.Fatalf("GetAgent as agent owner: expected 200, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
|
|
// Plain member (not in allowed_principals): denied with 403.
|
|
w = httptest.NewRecorder()
|
|
testHandler.GetAgent(w, withURLParam(newRequestAs(memberID, "GET", "/api/agents/"+agentID, nil), "id", agentID))
|
|
if w.Code != http.StatusForbidden {
|
|
t.Fatalf("GetAgent as plain member: expected 403, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
}
|
|
|
|
// TestListAgents_FiltersPrivateForPlainMember verifies that the workspace
|
|
// agents listing hides private agents from members who lack access. This is
|
|
// what makes the @-mention autocomplete picker (which feeds off this list)
|
|
// drop unreachable private agents without any client-side logic.
|
|
func TestListAgents_FiltersPrivateForPlainMember(t *testing.T) {
|
|
if testHandler == nil {
|
|
t.Skip("database not available")
|
|
}
|
|
|
|
agentID, _, memberID := privateAgentTestFixture(t)
|
|
|
|
// Workspace owner sees the agent.
|
|
w := httptest.NewRecorder()
|
|
testHandler.ListAgents(w, newRequest("GET", "/api/agents", nil))
|
|
if w.Code != http.StatusOK {
|
|
t.Fatalf("ListAgents as owner: expected 200, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
if !listContainsAgent(t, w.Body.Bytes(), agentID) {
|
|
t.Fatalf("ListAgents as owner did not include private agent %s", agentID)
|
|
}
|
|
|
|
// Plain member does NOT see the agent.
|
|
w = httptest.NewRecorder()
|
|
testHandler.ListAgents(w, newRequestAs(memberID, "GET", "/api/agents", nil))
|
|
if w.Code != http.StatusOK {
|
|
t.Fatalf("ListAgents as plain member: expected 200, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
if listContainsAgent(t, w.Body.Bytes(), agentID) {
|
|
t.Fatalf("ListAgents as plain member leaked private agent %s", agentID)
|
|
}
|
|
}
|
|
|
|
func listContainsAgent(t *testing.T, body []byte, agentID string) bool {
|
|
t.Helper()
|
|
var resp []AgentResponse
|
|
if err := json.Unmarshal(body, &resp); err != nil {
|
|
t.Fatalf("decode ListAgents response: %v", err)
|
|
}
|
|
for _, a := range resp {
|
|
if a.ID == agentID {
|
|
return true
|
|
}
|
|
}
|
|
return false
|
|
}
|
|
|
|
// TestListAgentTasks_PrivateAgentForbidsPlainMember verifies that the agent
|
|
// task history endpoint (the "查看历史会话" surface) is also gated.
|
|
func TestListAgentTasks_PrivateAgentForbidsPlainMember(t *testing.T) {
|
|
if testHandler == nil {
|
|
t.Skip("database not available")
|
|
}
|
|
|
|
agentID, ownerID, memberID := privateAgentTestFixture(t)
|
|
|
|
w := httptest.NewRecorder()
|
|
testHandler.ListAgentTasks(w, withURLParam(newRequestAs(ownerID, "GET", "/api/agents/"+agentID+"/tasks", nil), "id", agentID))
|
|
if w.Code != http.StatusOK {
|
|
t.Fatalf("ListAgentTasks as owner: expected 200, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
|
|
w = httptest.NewRecorder()
|
|
testHandler.ListAgentTasks(w, withURLParam(newRequestAs(memberID, "GET", "/api/agents/"+agentID+"/tasks", nil), "id", agentID))
|
|
if w.Code != http.StatusForbidden {
|
|
t.Fatalf("ListAgentTasks as plain member: expected 403, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
}
|
|
|
|
// TestCreateIssue_AssignToPrivateAgentForbidsPlainMember verifies that the
|
|
// issue-assignment surface is gated by the same predicate. Without this gate
|
|
// a plain workspace member could side-step chat/@-mention by assigning a
|
|
// private agent to an issue and letting normal task dispatch run it.
|
|
func TestCreateIssue_AssignToPrivateAgentForbidsPlainMember(t *testing.T) {
|
|
if testHandler == nil {
|
|
t.Skip("database not available")
|
|
}
|
|
|
|
agentID, ownerID, memberID := privateAgentTestFixture(t)
|
|
|
|
body := func(actorID string) map[string]any {
|
|
return map[string]any{
|
|
"title": "assign-to-private-agent test " + actorID,
|
|
"status": "todo",
|
|
"priority": "medium",
|
|
"assignee_type": "agent",
|
|
"assignee_id": agentID,
|
|
}
|
|
}
|
|
|
|
// Workspace owner (testUserID) who is NOT the agent owner: DENIED under
|
|
// the invocation-permission model (MUL-3963) — admin/owner status no
|
|
// longer grants the ability to invoke someone else's private agent.
|
|
w := httptest.NewRecorder()
|
|
testHandler.CreateIssue(w, newRequest("POST", "/api/issues?workspace_id="+testWorkspaceID, body(testUserID)))
|
|
if w.Code != http.StatusForbidden {
|
|
t.Fatalf("CreateIssue as workspace owner (not agent owner): expected 403, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
|
|
// Agent owner (plain member who happens to own the agent): allowed.
|
|
w = httptest.NewRecorder()
|
|
testHandler.CreateIssue(w, newRequestAs(ownerID, "POST", "/api/issues?workspace_id="+testWorkspaceID, body(ownerID)))
|
|
if w.Code != http.StatusCreated {
|
|
t.Fatalf("CreateIssue as agent owner: expected 201, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
|
|
// Plain member: denied with 403 — closes the back door where issue
|
|
// assignment would otherwise hand the agent a task without going
|
|
// through chat / @-mention.
|
|
w = httptest.NewRecorder()
|
|
testHandler.CreateIssue(w, newRequestAs(memberID, "POST", "/api/issues?workspace_id="+testWorkspaceID, body(memberID)))
|
|
if w.Code != http.StatusForbidden {
|
|
t.Fatalf("CreateIssue as plain member: expected 403, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
}
|
|
|
|
// TestCreateChatSession_PrivateAgentForbidsPlainMember verifies that members
|
|
// who can't access the private agent cannot start a chat session against it.
|
|
// The chat handler reads workspace context from middleware, so we set it
|
|
// explicitly via middleware.SetMemberContext before invoking the handler
|
|
// (the test harness doesn't run the real middleware chain).
|
|
func TestCreateChatSession_PrivateAgentForbidsPlainMember(t *testing.T) {
|
|
if testHandler == nil {
|
|
t.Skip("database not available")
|
|
}
|
|
|
|
agentID, _, memberID := privateAgentTestFixture(t)
|
|
|
|
// Load the plain member's row so we can build a realistic context.
|
|
memberRow, err := testHandler.Queries.GetMemberByUserAndWorkspace(context.Background(), db.GetMemberByUserAndWorkspaceParams{
|
|
UserID: util.MustParseUUID(memberID),
|
|
WorkspaceID: util.MustParseUUID(testWorkspaceID),
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("load plain member row: %v", err)
|
|
}
|
|
|
|
body := map[string]any{
|
|
"agent_id": agentID,
|
|
"title": "should be denied",
|
|
}
|
|
w := httptest.NewRecorder()
|
|
req := newRequestAs(memberID, "POST", "/api/chat/sessions", body)
|
|
req = req.WithContext(middleware.SetMemberContext(req.Context(), testWorkspaceID, memberRow))
|
|
testHandler.CreateChatSession(w, req)
|
|
if w.Code != http.StatusForbidden {
|
|
t.Fatalf("CreateChatSession as plain member: expected 403, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
}
|
|
|
|
// TestGetAgent_RejectsForgedAgentIDHeader is the regression test for the
|
|
// #2359 review finding "X-Agent-ID can be forged by a plain member to bypass
|
|
// the private gate". A workspace member sets X-Agent-ID to any visible
|
|
// agent's UUID without supplying a valid X-Task-ID — resolveActor must now
|
|
// fall back to the member identity, so the private-agent gate stays effective.
|
|
func TestGetAgent_RejectsForgedAgentIDHeader(t *testing.T) {
|
|
if testHandler == nil {
|
|
t.Skip("database not available")
|
|
}
|
|
|
|
agentID, _, memberID := privateAgentTestFixture(t)
|
|
|
|
w := httptest.NewRecorder()
|
|
req := newRequestAs(memberID, "GET", "/api/agents/"+agentID, nil)
|
|
// Forge X-Agent-ID without X-Task-ID. Pre-fix this would have made
|
|
// resolveActor return ("agent", agentID) and canAccessPrivateAgent
|
|
// would have unconditionally allowed the read.
|
|
req.Header.Set("X-Agent-ID", agentID)
|
|
req = withURLParam(req, "id", agentID)
|
|
testHandler.GetAgent(w, req)
|
|
if w.Code != http.StatusForbidden {
|
|
t.Fatalf("GetAgent with forged X-Agent-ID: expected 403, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
}
|
|
|
|
// TestListChatMessages_PrivateAgentForbidsAfterAccessRevoked is the regression
|
|
// test for the #2359 review finding "chat history read path doesn't re-gate".
|
|
// A member who created a chat session is later denied access to the agent
|
|
// (here simulated by the member never being on the allowlist for a private
|
|
// agent owned by someone else; the equivalent of an after-the-fact ownership
|
|
// transfer). The session row still names them as creator, but the read
|
|
// endpoints must refuse to surface the transcript.
|
|
func TestListChatMessages_PrivateAgentForbidsAfterAccessRevoked(t *testing.T) {
|
|
if testHandler == nil {
|
|
t.Skip("database not available")
|
|
}
|
|
|
|
ctx := context.Background()
|
|
agentID, _, memberID := privateAgentTestFixture(t)
|
|
|
|
// Insert a chat session row directly with the plain member as creator,
|
|
// bypassing CreateChatSession's own gate. This represents a session
|
|
// that existed before the member lost access (or before the gate
|
|
// landed).
|
|
var sessionID string
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO chat_session (workspace_id, agent_id, creator_id, title, status)
|
|
VALUES ($1, $2, $3, 'pre-revocation session', 'active')
|
|
RETURNING id
|
|
`, testWorkspaceID, agentID, memberID).Scan(&sessionID); err != nil {
|
|
t.Fatalf("seed chat session: %v", err)
|
|
}
|
|
t.Cleanup(func() {
|
|
testPool.Exec(context.Background(), `DELETE FROM chat_session WHERE id = $1`, sessionID)
|
|
})
|
|
|
|
memberRow, err := testHandler.Queries.GetMemberByUserAndWorkspace(ctx, db.GetMemberByUserAndWorkspaceParams{
|
|
UserID: util.MustParseUUID(memberID),
|
|
WorkspaceID: util.MustParseUUID(testWorkspaceID),
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("load plain member row: %v", err)
|
|
}
|
|
|
|
w := httptest.NewRecorder()
|
|
req := newRequestAs(memberID, "GET", "/api/chat/sessions/"+sessionID+"/messages", nil)
|
|
req = req.WithContext(middleware.SetMemberContext(req.Context(), testWorkspaceID, memberRow))
|
|
req = withURLParam(req, "sessionId", sessionID)
|
|
testHandler.ListChatMessages(w, req)
|
|
if w.Code != http.StatusForbidden {
|
|
t.Fatalf("ListChatMessages on stale session: expected 403, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
}
|
|
|
|
// TestMentionAgent_RejectsCrossWorkspaceAgentUUID is the regression test for
|
|
// the #2359 review finding "@mention path doesn't constrain the mentioned
|
|
// agent to the current workspace". A plain member in workspace A who happens
|
|
// to be owner of workspace B should NOT be able to @mention a private agent
|
|
// in workspace B from a comment on a workspace-A issue and have it pass the
|
|
// gate (the gate was being applied against the wrong workspace's roles).
|
|
func TestMentionAgent_RejectsCrossWorkspaceAgentUUID(t *testing.T) {
|
|
if testHandler == nil {
|
|
t.Skip("database not available")
|
|
}
|
|
|
|
ctx := context.Background()
|
|
|
|
// Create a separate workspace + agent runtime + private agent.
|
|
var foreignWorkspaceID, foreignUserID, foreignRuntimeID, foreignAgentID string
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO "user" (name, email)
|
|
VALUES ('Foreign Owner', 'cross-ws-foreign@multica.test')
|
|
RETURNING id
|
|
`).Scan(&foreignUserID); err != nil {
|
|
t.Fatalf("create foreign user: %v", err)
|
|
}
|
|
t.Cleanup(func() {
|
|
testPool.Exec(context.Background(),
|
|
`DELETE FROM "user" WHERE email = 'cross-ws-foreign@multica.test'`)
|
|
})
|
|
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO workspace (name, slug, description, issue_prefix)
|
|
VALUES ('Cross-WS Foreign', 'cross-ws-foreign', '', 'XWF')
|
|
RETURNING id
|
|
`).Scan(&foreignWorkspaceID); err != nil {
|
|
t.Fatalf("create foreign workspace: %v", err)
|
|
}
|
|
t.Cleanup(func() {
|
|
testPool.Exec(context.Background(),
|
|
`DELETE FROM workspace WHERE slug = 'cross-ws-foreign'`)
|
|
})
|
|
if _, err := testPool.Exec(ctx, `
|
|
INSERT INTO member (workspace_id, user_id, role)
|
|
VALUES ($1, $2, 'owner')
|
|
`, foreignWorkspaceID, foreignUserID); err != nil {
|
|
t.Fatalf("add foreign member: %v", err)
|
|
}
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO agent_runtime (workspace_id, daemon_id, name, runtime_mode, provider, status, device_info, metadata, last_seen_at)
|
|
VALUES ($1, NULL, 'Foreign Runtime', 'cloud', 'foreign_test', 'online', 'Foreign', '{}'::jsonb, now())
|
|
RETURNING id
|
|
`, foreignWorkspaceID).Scan(&foreignRuntimeID); err != nil {
|
|
t.Fatalf("create foreign runtime: %v", err)
|
|
}
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO agent (workspace_id, name, description, runtime_mode, runtime_config, runtime_id, visibility, max_concurrent_tasks, owner_id, instructions, custom_env, custom_args)
|
|
VALUES ($1, 'foreign-private-agent', '', 'cloud', '{}'::jsonb, $2, 'private', 1, $3, '', '{}'::jsonb, '[]'::jsonb)
|
|
RETURNING id
|
|
`, foreignWorkspaceID, foreignRuntimeID, foreignUserID).Scan(&foreignAgentID); err != nil {
|
|
t.Fatalf("create foreign agent: %v", err)
|
|
}
|
|
|
|
// Create an issue in OUR workspace and a comment that @mentions the
|
|
// foreign agent's UUID. testUserID is owner of our workspace; pre-fix
|
|
// the gate would have applied our-workspace-owner status to the foreign
|
|
// agent and enqueued a task.
|
|
var issueID, commentID string
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO issue (workspace_id, title, status, priority, creator_type, creator_id, number)
|
|
VALUES ($1, 'cross-ws mention test', 'todo', 'medium', 'member', $2,
|
|
COALESCE((SELECT MAX(number) FROM issue WHERE workspace_id = $1), 0) + 1)
|
|
RETURNING id
|
|
`, testWorkspaceID, testUserID).Scan(&issueID); err != nil {
|
|
t.Fatalf("create test issue: %v", err)
|
|
}
|
|
t.Cleanup(func() {
|
|
testPool.Exec(context.Background(), `DELETE FROM issue WHERE id = $1`, issueID)
|
|
})
|
|
|
|
// Multica's mention format is markdown-linked: [@Name](mention://agent/<uuid>).
|
|
mention := "[@Foreign](mention://agent/" + foreignAgentID + ")"
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO comment (workspace_id, issue_id, author_type, author_id, content)
|
|
VALUES ($1, $2, 'member', $3, $4)
|
|
RETURNING id
|
|
`, testWorkspaceID, issueID, testUserID, mention).Scan(&commentID); err != nil {
|
|
t.Fatalf("create test comment: %v", err)
|
|
}
|
|
t.Cleanup(func() {
|
|
testPool.Exec(context.Background(), `DELETE FROM comment WHERE id = $1`, commentID)
|
|
})
|
|
|
|
issue, err := testHandler.Queries.GetIssue(ctx, util.MustParseUUID(issueID))
|
|
if err != nil {
|
|
t.Fatalf("load test issue: %v", err)
|
|
}
|
|
comment, err := testHandler.Queries.GetComment(ctx, util.MustParseUUID(commentID))
|
|
if err != nil {
|
|
t.Fatalf("load test comment: %v", err)
|
|
}
|
|
|
|
// Count tasks for the foreign agent before. Calling the dispatcher
|
|
// directly bypasses HTTP-layer concerns and exercises only the
|
|
// workspace-scoping check.
|
|
var beforeCount int
|
|
if err := testPool.QueryRow(ctx,
|
|
`SELECT COUNT(*) FROM agent_task_queue WHERE agent_id = $1`,
|
|
foreignAgentID,
|
|
).Scan(&beforeCount); err != nil {
|
|
t.Fatalf("count tasks before: %v", err)
|
|
}
|
|
|
|
enqueueMentionedAgentTasksForTest(t, ctx, issue, comment, nil, "member", testUserID)
|
|
|
|
var afterCount int
|
|
if err := testPool.QueryRow(ctx,
|
|
`SELECT COUNT(*) FROM agent_task_queue WHERE agent_id = $1`,
|
|
foreignAgentID,
|
|
).Scan(&afterCount); err != nil {
|
|
t.Fatalf("count tasks after: %v", err)
|
|
}
|
|
if afterCount != beforeCount {
|
|
t.Fatalf("foreign agent task count changed: before=%d after=%d — cross-workspace mention was not rejected",
|
|
beforeCount, afterCount)
|
|
}
|
|
}
|
|
|
|
// TestShouldEnqueueOnComment_PrivateAgentGate is the regression test for
|
|
// GH #3300: after an owner/admin assigns a private agent to an issue, the
|
|
// agent's UUID is "welded" onto that issue and any member with comment
|
|
// access could previously dispatch a new task to the private agent simply by
|
|
// posting a plain (non-@mention) comment, bypassing the visibility gate that
|
|
// #2359 added to chat / @mention / assignment.
|
|
//
|
|
// The gate must:
|
|
// - reject plain workspace members (not owner, not admin, not agent owner)
|
|
// - allow the agent owner
|
|
// - allow workspace owners/admins
|
|
// - allow agent-to-agent traffic regardless of agent visibility
|
|
func TestShouldEnqueueOnComment_PrivateAgentGate(t *testing.T) {
|
|
if testHandler == nil || testPool == nil {
|
|
t.Skip("database not available")
|
|
}
|
|
|
|
ctx := context.Background()
|
|
agentID, ownerID, memberID := privateAgentTestFixture(t)
|
|
|
|
// Assign the private agent to a fresh issue. Owner/admin would normally
|
|
// be the one performing this step; we insert directly so the test
|
|
// focuses on the on_comment trigger path.
|
|
var issueID string
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO issue (workspace_id, title, status, priority, creator_type, creator_id,
|
|
assignee_type, assignee_id, number)
|
|
VALUES ($1, 'on_comment private-agent gate test', 'todo', 'medium', 'member', $2,
|
|
'agent', $3,
|
|
COALESCE((SELECT MAX(number) FROM issue WHERE workspace_id = $1), 0) + 1)
|
|
RETURNING id
|
|
`, testWorkspaceID, testUserID, agentID).Scan(&issueID); err != nil {
|
|
t.Fatalf("create issue assigned to private agent: %v", err)
|
|
}
|
|
t.Cleanup(func() {
|
|
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)
|
|
}
|
|
|
|
cases := []struct {
|
|
name string
|
|
actorType string
|
|
actorID string
|
|
want bool
|
|
reason string
|
|
}{
|
|
{
|
|
name: "plain member — denied",
|
|
actorType: "member",
|
|
actorID: memberID,
|
|
want: false,
|
|
reason: "GH #3300: plain members must not be able to dispatch a task to a private agent via on_comment",
|
|
},
|
|
{
|
|
name: "agent owner — allowed",
|
|
actorType: "member",
|
|
actorID: ownerID,
|
|
want: true,
|
|
reason: "agent owner is always in the allowed_principals set",
|
|
},
|
|
{
|
|
name: "workspace owner — denied (not agent owner)",
|
|
actorType: "member",
|
|
actorID: testUserID,
|
|
want: false,
|
|
reason: "MUL-3963: workspace owners/admins no longer bypass a private agent's invocation gate",
|
|
},
|
|
{
|
|
name: "agent-to-agent — denied without allowed originator",
|
|
actorType: "agent",
|
|
actorID: agentID,
|
|
want: false,
|
|
reason: "MUL-3963: A2A is judged by the top-of-chain originator; a private agent denies an agent actor with no owner/allow-listed originator",
|
|
},
|
|
}
|
|
|
|
for _, tc := range cases {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
got := testHandler.shouldEnqueueAssigneeFallback(ctx, issue, tc.actorType, tc.actorID, commentTriggerComputeOptions{})
|
|
if got != tc.want {
|
|
t.Fatalf("%s\n actor=%s/%s got=%v want=%v",
|
|
tc.reason, tc.actorType, tc.actorID, got, tc.want)
|
|
}
|
|
})
|
|
}
|
|
}
|