mirror of
https://gh-proxy.org/https://github.com/klinsmaya/multica.git
synced 2026-11-11 10:56:45 +08: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 <[email protected]>
* 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 <[email protected]>
* 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 <[email protected]>
* 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 <[email protected]>
* 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 <[email protected]>
---------
Co-authored-by: multica-agent <[email protected]>
* 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 <[email protected]>
* fix(composio): real callback redirect route + StrictMode-safe toast dedup (MUL-3718 review)
Co-authored-by: multica-agent <[email protected]>
---------
Co-authored-by: multica-agent <[email protected]>
* 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 <[email protected]>
* 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 <[email protected]>
---------
Co-authored-by: multica-agent <[email protected]>
* 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 <[email protected]>
Co-authored-by: multica-agent <[email protected]>
* 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 <[email protected]>
* chore(composio): gofmt + regenerate sqlc with v1.31.1 (MUL-3869 review nits)
Co-authored-by: multica-agent <[email protected]>
---------
Co-authored-by: Eve <[email protected]>
Co-authored-by: multica-agent <[email protected]>
* 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 <[email protected]>
* 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 <[email protected]>
* 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 <[email protected]>
* 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 <[email protected]>
* 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 <[email protected]>
---------
Co-authored-by: multica-agent <[email protected]>
* 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 <[email protected]>
* 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 <[email protected]>
---------
Co-authored-by: multica-agent <[email protected]>
* 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 <[email protected]>
Co-authored-by: Multica Eve <[email protected]>
Co-authored-by: Eve <[email protected]>
Co-authored-by: Eve <[email protected]>
651 lines
26 KiB
Go
651 lines
26 KiB
Go
package handler
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"testing"
|
|
|
|
"github.com/multica-ai/multica/server/internal/util"
|
|
db "github.com/multica-ai/multica/server/pkg/db/generated"
|
|
)
|
|
|
|
// TestCommentMentionsAnyone covers the pure helper that drives the
|
|
// "skip leader on @<anyone>" behavior. Routing-style mentions
|
|
// (agent/member/squad/all) count; issue cross-references do not. Kept as a
|
|
// unit test so it runs without a database connection.
|
|
func TestCommentMentionsAnyone(t *testing.T) {
|
|
cases := []struct {
|
|
name string
|
|
content string
|
|
want bool
|
|
}{
|
|
{name: "empty", content: "", want: false},
|
|
{name: "plain text", content: "please take a look", want: false},
|
|
{name: "literal at sign only", content: "ping @alice", want: false},
|
|
{name: "agent mention", content: "[@A](mention://agent/11111111-1111-1111-1111-111111111111) handle this", want: true},
|
|
{name: "member mention", content: "[@Bob](mention://member/22222222-2222-2222-2222-222222222222)", want: true},
|
|
{name: "squad mention", content: "[@Squad](mention://squad/44444444-4444-4444-4444-444444444444)", want: true},
|
|
{name: "mention all", content: "[@all](mention://all/all)", want: true},
|
|
{name: "issue mention only", content: "see [MUL-1](mention://issue/33333333-3333-3333-3333-333333333333)", want: false},
|
|
{name: "issue + plain text", content: "see [MUL-1](mention://issue/33333333-3333-3333-3333-333333333333) for context", want: false},
|
|
{name: "agent plus member", content: "[@A](mention://agent/11111111-1111-1111-1111-111111111111) cc [@B](mention://member/22222222-2222-2222-2222-222222222222)", want: true},
|
|
{name: "issue plus member", content: "blocks [MUL-1](mention://issue/33333333-3333-3333-3333-333333333333) — [@Bob](mention://member/22222222-2222-2222-2222-222222222222)", want: true},
|
|
}
|
|
for _, tc := range cases {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
if got := commentMentionsAnyone(tc.content); got != tc.want {
|
|
t.Fatalf("commentMentionsAnyone(%q) = %v, want %v", tc.content, got, tc.want)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// shouldEnqueueSquadLeaderOnCommentForTest reports whether the shared cascade
|
|
// would wake the issue's assigned squad leader.
|
|
func shouldEnqueueSquadLeaderOnCommentForTest(ctx context.Context, issue db.Issue, content, authorType, authorID string) bool {
|
|
return triggersContainIssueAssigneeSquadLeader(testHandler.computeCommentAgentTriggers(ctx, issue, content, nil, authorType, authorID, commentTriggerComputeOptions{}))
|
|
}
|
|
|
|
func shouldEnqueueSquadLeaderOnReplyForTest(ctx context.Context, issue db.Issue, content string, parent *db.Comment, authorType, authorID string) bool {
|
|
return triggersContainIssueAssigneeSquadLeader(testHandler.computeCommentAgentTriggers(ctx, issue, content, parent, authorType, authorID, commentTriggerComputeOptions{}))
|
|
}
|
|
|
|
func triggersContainIssueAssigneeSquadLeader(triggers []commentAgentTrigger) bool {
|
|
for _, trigger := range triggers {
|
|
if trigger.Source == commentTriggerSourceIssueAssignee && trigger.Squad != nil {
|
|
return true
|
|
}
|
|
}
|
|
return false
|
|
}
|
|
|
|
// squadCommentTriggerFixture wires a squad assigned to a fresh issue and
|
|
// returns the loaded db.Issue plus the leader agent UUID for use in
|
|
// cascade integration tests.
|
|
type squadCommentTriggerFixture struct {
|
|
Issue db.Issue
|
|
SquadID string
|
|
LeaderID string
|
|
OtherID string // second agent in workspace (with runtime), used as a non-leader @mention target
|
|
}
|
|
|
|
func newSquadCommentTriggerFixture(t *testing.T) squadCommentTriggerFixture {
|
|
t.Helper()
|
|
ctx := context.Background()
|
|
|
|
// Reuse the seeded "Handler Test Agent" as the leader — it has a runtime.
|
|
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)
|
|
}
|
|
|
|
// Spin up a second agent in the same workspace as a non-leader mention
|
|
// target. createHandlerTestAgent installs a t.Cleanup row deletion.
|
|
otherID := createHandlerTestAgent(t, "Squad Comment Other", nil)
|
|
|
|
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, "Squad Comment Trigger", 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)
|
|
})
|
|
|
|
var issueID string
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO issue (workspace_id, creator_type, creator_id, title, assignee_type, assignee_id)
|
|
VALUES ($1, 'member', $2, $3, 'squad', $4)
|
|
RETURNING id
|
|
`, testWorkspaceID, testUserID, "squad comment trigger", squadID).Scan(&issueID); err != nil {
|
|
t.Fatalf("create issue: %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)
|
|
}
|
|
|
|
return squadCommentTriggerFixture{
|
|
Issue: issue,
|
|
SquadID: squadID,
|
|
LeaderID: leaderID,
|
|
OtherID: otherID,
|
|
}
|
|
}
|
|
|
|
// TestShouldEnqueueSquadLeaderOnComment_SkipsWhenCommentRoutesElsewhere
|
|
// pins the cascade: explicit participant mentions do not also wake the assigned
|
|
// squad leader. Issue cross-references are not routing and do not suppress the
|
|
// leader.
|
|
func TestShouldEnqueueSquadLeaderOnComment_SkipsWhenMemberMentionsAnyone(t *testing.T) {
|
|
if testHandler == nil || testPool == nil {
|
|
t.Skip("database not available")
|
|
}
|
|
fx := newSquadCommentTriggerFixture(t)
|
|
ctx := context.Background()
|
|
|
|
cases := []struct {
|
|
name string
|
|
content string
|
|
authorType string
|
|
authorID string
|
|
want bool
|
|
description string
|
|
}{
|
|
{
|
|
name: "member plain comment triggers leader",
|
|
content: "what is the latest on this?",
|
|
authorType: "member",
|
|
authorID: testUserID,
|
|
want: true,
|
|
description: "no @ in body → leader must coordinate as today",
|
|
},
|
|
{
|
|
name: "member issue cross-reference only triggers leader",
|
|
content: "blocked by [MUL-1](mention://issue/" + testUserID + ")",
|
|
authorType: "member",
|
|
authorID: testUserID,
|
|
want: true,
|
|
description: "issue mentions are not routing — leader still owns dispatch",
|
|
},
|
|
{
|
|
name: "member mentions another member skips leader",
|
|
content: "[@self](mention://member/" + testUserID + ") please weigh in",
|
|
authorType: "member",
|
|
authorID: testUserID,
|
|
want: false,
|
|
description: "user routed at a human — leader stays out (extended rule)",
|
|
},
|
|
{
|
|
name: "member mentions non-leader agent skips leader",
|
|
content: "[@Other](mention://agent/" + fx.OtherID + ") please take this",
|
|
authorType: "member",
|
|
authorID: testUserID,
|
|
want: false,
|
|
description: "user routed at an agent — leader stays out",
|
|
},
|
|
{
|
|
name: "member mentions leader skips leader on comment path",
|
|
content: "[@Leader](mention://agent/" + fx.LeaderID + ") your call",
|
|
authorType: "member",
|
|
authorID: testUserID,
|
|
want: false,
|
|
description: "even @leader is dispatched via the mention path; comment path must not double-enqueue",
|
|
},
|
|
{
|
|
name: "member mention all skips leader",
|
|
content: "[@all](mention://all/all) heads up",
|
|
authorType: "member",
|
|
authorID: testUserID,
|
|
want: false,
|
|
description: "@all is a broadcast — leader does not need to wake to evaluate routing",
|
|
},
|
|
{
|
|
name: "member mentions a squad skips leader",
|
|
content: "handing to [@Other Squad](mention://squad/" + fx.SquadID + ")",
|
|
authorType: "member",
|
|
authorID: testUserID,
|
|
want: false,
|
|
description: "@squad routes the issue to that squad's leader — current leader stays out",
|
|
},
|
|
{
|
|
name: "agent comment with @agent does not also trigger leader",
|
|
content: "delegating to [@Other](mention://agent/" + fx.OtherID + ")",
|
|
authorType: "agent",
|
|
authorID: fx.OtherID,
|
|
want: false,
|
|
description: "explicit @agent routes only to the mentioned target; the assigned squad-leader fallback must not also fire (no double-enqueue)",
|
|
},
|
|
}
|
|
|
|
for _, tc := range cases {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
got := shouldEnqueueSquadLeaderOnCommentForTest(ctx, fx.Issue, tc.content, tc.authorType, tc.authorID)
|
|
if got != tc.want {
|
|
t.Fatalf("%s\n content=%q author=%s/%s\n got=%v want=%v",
|
|
tc.description, tc.content, tc.authorType, tc.authorID, got, tc.want)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestShouldEnqueueSquadLeaderOnComment_AgentAuthoredWorkerCommentsWakeLeader
|
|
// pins the MUL-3879 restored behavior in the new MUL-3794 cascade: an
|
|
// agent-authored worker-result comment on a squad-assigned issue wakes the
|
|
// assigned squad leader so the leader→worker→leader coordination loop stays
|
|
// closed, while the leader's own self-trigger loop stays suppressed via
|
|
// lastTaskWasLeader.
|
|
func TestShouldEnqueueSquadLeaderOnComment_AgentAuthoredWorkerCommentsWakeLeader(t *testing.T) {
|
|
if testHandler == nil || testPool == nil {
|
|
t.Skip("database not available")
|
|
}
|
|
fx := newSquadCommentTriggerFixture(t)
|
|
ctx := context.Background()
|
|
issueID := uuidToString(fx.Issue.ID)
|
|
|
|
t.Cleanup(func() {
|
|
testPool.Exec(context.Background(), `DELETE FROM agent_task_queue WHERE issue_id = $1`, issueID)
|
|
})
|
|
|
|
clearTasks := func() {
|
|
if _, err := testPool.Exec(ctx, `DELETE FROM agent_task_queue WHERE issue_id = $1`, issueID); err != nil {
|
|
t.Fatalf("clear tasks: %v", err)
|
|
}
|
|
}
|
|
// insertLeaderTask seeds a task for the leader agent so the
|
|
// lastTaskWasLeader guard can read the agent's most recent role on the
|
|
// issue. Separate Exec calls get distinct created_at values, so the last
|
|
// inserted row is the "latest" task.
|
|
insertLeaderTask := func(isLeader bool, status string) {
|
|
t.Helper()
|
|
var runtimeID string
|
|
if err := testPool.QueryRow(ctx, `SELECT runtime_id FROM agent WHERE id = $1`, fx.LeaderID).Scan(&runtimeID); err != nil {
|
|
t.Fatalf("load runtime: %v", err)
|
|
}
|
|
if _, err := testPool.Exec(ctx, `
|
|
INSERT INTO agent_task_queue (agent_id, runtime_id, issue_id, status, is_leader_task)
|
|
VALUES ($1, $2, $3, $4, $5)
|
|
`, fx.LeaderID, runtimeID, issueID, status, isLeader); err != nil {
|
|
t.Fatalf("insert task: %v", err)
|
|
}
|
|
}
|
|
|
|
// Case 1: a worker agent (not the leader) posts a result comment on the
|
|
// squad-assigned issue — the assigned leader must wake to coordinate.
|
|
t.Run("worker agent comment wakes squad leader", func(t *testing.T) {
|
|
clearTasks()
|
|
if got := shouldEnqueueSquadLeaderOnCommentForTest(ctx, fx.Issue, "pushed the fix, PR is up", "agent", fx.OtherID); !got {
|
|
t.Fatalf("worker agent comment: expected leader to wake, got skip")
|
|
}
|
|
})
|
|
|
|
// Case 2: a dual-role agent (leader of the squad, also runs worker tasks)
|
|
// posts while its latest task on the issue was a worker task — the leader
|
|
// role must still wake because the comment is a worker result, not a
|
|
// leader self-trigger.
|
|
t.Run("dual-role worker comment wakes leader when latest task is worker", func(t *testing.T) {
|
|
clearTasks()
|
|
insertLeaderTask(true, "completed") // older leader task
|
|
insertLeaderTask(false, "completed") // newer worker task → latest role is worker
|
|
if got := shouldEnqueueSquadLeaderOnCommentForTest(ctx, fx.Issue, "done with my worker slice", "agent", fx.LeaderID); !got {
|
|
t.Fatalf("dual-role worker comment: expected leader to wake, got skip")
|
|
}
|
|
})
|
|
|
|
// Case 3: the leader posts while its latest task was a leader task — this
|
|
// is a self-trigger loop and must stay suppressed.
|
|
t.Run("leader comment from latest leader task does not self-trigger", func(t *testing.T) {
|
|
clearTasks()
|
|
insertLeaderTask(false, "completed") // older worker task
|
|
insertLeaderTask(true, "completed") // newer leader task → latest role is leader
|
|
if got := shouldEnqueueSquadLeaderOnCommentForTest(ctx, fx.Issue, "coordinating next steps", "agent", fx.LeaderID); got {
|
|
t.Fatalf("leader self-trigger: expected skip, got wake")
|
|
}
|
|
})
|
|
|
|
// Case 4: an agent-authored comment carrying an explicit @agent mention
|
|
// routes only to the mentioned target — the assigned squad leader must NOT
|
|
// also be enqueued via the fallback path (no double-enqueue).
|
|
t.Run("explicit mention does not double-enqueue assigned leader", func(t *testing.T) {
|
|
clearTasks()
|
|
content := "handing to [@Other](mention://agent/" + fx.OtherID + ")"
|
|
if got := shouldEnqueueSquadLeaderOnCommentForTest(ctx, fx.Issue, content, "agent", fx.LeaderID); got {
|
|
t.Fatalf("explicit mention: expected no assigned-leader fallback, got wake")
|
|
}
|
|
})
|
|
}
|
|
|
|
// TestCreateComment_SquadPlainReplyToMemberParentKeepsRootMentionOwner drives the
|
|
// full CreateComment handler to lock the cascade's reply behavior:
|
|
//
|
|
// - A member top-level comment that @mentions another agent does NOT
|
|
// enqueue the squad leader (the mentioned agent owns the next step).
|
|
// - A subsequent member reply to that member-authored root with no explicit
|
|
// agent mention continues to the root owner instead of the assignee.
|
|
func TestCreateComment_SquadPlainReplyToMemberParentKeepsRootMentionOwner(t *testing.T) {
|
|
if testHandler == nil || testPool == nil {
|
|
t.Skip("database not available")
|
|
}
|
|
ctx := context.Background()
|
|
fx := newSquadCommentTriggerFixture(t)
|
|
issueID := uuidToString(fx.Issue.ID)
|
|
|
|
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)
|
|
})
|
|
|
|
countQueued := func(agentID string) int {
|
|
var n int
|
|
if err := testPool.QueryRow(ctx,
|
|
`SELECT count(*) FROM agent_task_queue WHERE issue_id = $1 AND agent_id = $2 AND status = 'queued'`,
|
|
issueID, agentID,
|
|
).Scan(&n); err != nil {
|
|
t.Fatalf("count tasks for %s: %v", agentID, err)
|
|
}
|
|
return n
|
|
}
|
|
|
|
postMemberComment := func(body map[string]any) CommentResponse {
|
|
t.Helper()
|
|
w := httptest.NewRecorder()
|
|
r := newRequest("POST", "/api/issues/"+issueID+"/comments", body)
|
|
r = withURLParam(r, "id", issueID)
|
|
testHandler.CreateComment(w, r)
|
|
if w.Code != http.StatusCreated {
|
|
t.Fatalf("CreateComment: expected 201, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
var resp CommentResponse
|
|
if err := json.NewDecoder(w.Body).Decode(&resp); err != nil {
|
|
t.Fatalf("decode comment: %v", err)
|
|
}
|
|
return resp
|
|
}
|
|
|
|
// 1. Member top-level comment mentions OtherAgent.
|
|
// Leader must be skipped; OtherAgent must be enqueued via the mention path.
|
|
parent := postMemberComment(map[string]any{
|
|
"content": "[@Other](mention://agent/" + fx.OtherID + ") please take this",
|
|
})
|
|
if got := countQueued(fx.LeaderID); got != 0 {
|
|
t.Fatalf("after parent (@OtherAgent): expected 0 leader tasks (skipped), got %d", got)
|
|
}
|
|
if got := countQueued(fx.OtherID); got != 1 {
|
|
t.Fatalf("after parent (@OtherAgent): expected 1 OtherAgent task (mention path), got %d", got)
|
|
}
|
|
|
|
// 2. Mark OtherAgent's parent task done so queued-task counts below only
|
|
// reflect what the plain reply does.
|
|
if _, err := testPool.Exec(ctx, `
|
|
UPDATE agent_task_queue SET status = 'completed'
|
|
WHERE issue_id = $1 AND agent_id = $2 AND status = 'queued'
|
|
`, issueID, fx.OtherID); err != nil {
|
|
t.Fatalf("complete OtherAgent parent task: %v", err)
|
|
}
|
|
|
|
// 3. Member posts a reply in the same thread with NO mentions.
|
|
// The root's @OtherAgent mention owns the thread, so the reply returns
|
|
// to OtherAgent instead of falling back to the assigned squad leader.
|
|
postMemberComment(map[string]any{
|
|
"content": "any update?",
|
|
"parent_id": parent.ID,
|
|
})
|
|
if got := countQueued(fx.LeaderID); got != 0 {
|
|
t.Fatalf("after plain reply: expected 0 leader tasks, got %d", got)
|
|
}
|
|
if got := countQueued(fx.OtherID); got != 1 {
|
|
t.Fatalf("after plain reply: expected 1 OtherAgent task, got %d", got)
|
|
}
|
|
}
|
|
|
|
// TestCreateComment_DualRoleAgentWorkerCommentWakesLeader pins the MUL-3879
|
|
// restored coordination loop at the full-handler level. Scenario:
|
|
//
|
|
// - Agent L is the leader of squad S and also runs worker tasks on issues
|
|
// belonging to S.
|
|
// - L is woken in its worker role (is_leader_task=false) and posts a result
|
|
// comment.
|
|
// - A leader-role task IS enqueued so the squad leader can coordinate the
|
|
// next step — the worker result must not silently strand the issue.
|
|
func TestCreateComment_DualRoleAgentWorkerCommentWakesLeader(t *testing.T) {
|
|
if testHandler == nil || testPool == nil {
|
|
t.Skip("database not available")
|
|
}
|
|
ctx := context.Background()
|
|
fx := newSquadCommentTriggerFixture(t)
|
|
issueID := uuidToString(fx.Issue.ID)
|
|
|
|
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)
|
|
})
|
|
|
|
// Seed a worker task for the leader agent on this issue so the guard
|
|
// infers "agent's last activity was a worker task" — i.e. L is running
|
|
// in its worker role when it posts the comment. We make it running (not
|
|
// completed) so we can hand its ID back through X-Task-ID for the
|
|
// resolveActor agent-identity check.
|
|
var runtimeID string
|
|
if err := testPool.QueryRow(ctx, `SELECT runtime_id FROM agent WHERE id = $1`, fx.LeaderID).Scan(&runtimeID); err != nil {
|
|
t.Fatalf("load runtime: %v", err)
|
|
}
|
|
var workerTaskID string
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO agent_task_queue (agent_id, runtime_id, issue_id, status, is_leader_task)
|
|
VALUES ($1, $2, $3, 'running', FALSE)
|
|
RETURNING id
|
|
`, fx.LeaderID, runtimeID, issueID).Scan(&workerTaskID); err != nil {
|
|
t.Fatalf("seed worker task: %v", err)
|
|
}
|
|
|
|
// L posts a comment in its agent identity (X-Agent-ID + X-Task-ID, the
|
|
// pair required by resolveActor to trust the agent header).
|
|
w := httptest.NewRecorder()
|
|
r := newRequest("POST", "/api/issues/"+issueID+"/comments", map[string]any{
|
|
"content": "done — pushed the change",
|
|
})
|
|
r.Header.Set("X-Agent-ID", fx.LeaderID)
|
|
r.Header.Set("X-Task-ID", workerTaskID)
|
|
r = withURLParam(r, "id", issueID)
|
|
testHandler.CreateComment(w, r)
|
|
if w.Code != http.StatusCreated {
|
|
t.Fatalf("CreateComment: expected 201, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
|
|
// A new leader-role task is enqueued so the leader coordinates next steps.
|
|
var leaderTasks int
|
|
if err := testPool.QueryRow(ctx, `
|
|
SELECT count(*) FROM agent_task_queue
|
|
WHERE issue_id = $1 AND agent_id = $2 AND status = 'queued' AND is_leader_task = TRUE
|
|
`, issueID, fx.LeaderID).Scan(&leaderTasks); err != nil {
|
|
t.Fatalf("count leader tasks: %v", err)
|
|
}
|
|
if leaderTasks != 1 {
|
|
t.Fatalf("after worker comment from dual-role agent: expected 1 queued leader task, got %d", leaderTasks)
|
|
}
|
|
}
|
|
|
|
// TestCreateRetryTask_InheritsIsLeaderTask locks the retry-clone contract for
|
|
// MUL-2218: auto-retry of a leader-role task must produce a child task that is
|
|
// also is_leader_task=true. Without this, MaybeRetryFailedTask silently
|
|
// demotes a retried leader task to a worker task, and role-specific claim-time
|
|
// briefing/self-mention guards lose the leader provenance.
|
|
func TestCreateRetryTask_InheritsIsLeaderTask(t *testing.T) {
|
|
if testHandler == nil || testPool == nil {
|
|
t.Skip("database not available")
|
|
}
|
|
ctx := context.Background()
|
|
fx := newSquadCommentTriggerFixture(t)
|
|
issueID := uuidToString(fx.Issue.ID)
|
|
|
|
t.Cleanup(func() {
|
|
testPool.Exec(context.Background(), `DELETE FROM agent_task_queue WHERE issue_id = $1`, issueID)
|
|
})
|
|
|
|
var runtimeID string
|
|
if err := testPool.QueryRow(ctx, `SELECT runtime_id FROM agent WHERE id = $1`, fx.LeaderID).Scan(&runtimeID); err != nil {
|
|
t.Fatalf("load runtime: %v", err)
|
|
}
|
|
|
|
cases := []struct {
|
|
name string
|
|
isLeader bool
|
|
}{
|
|
{name: "leader task retry stays leader", isLeader: true},
|
|
{name: "worker task retry stays worker", isLeader: false},
|
|
}
|
|
for _, tc := range cases {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
var parentID string
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO agent_task_queue (agent_id, runtime_id, issue_id, status, attempt, max_attempts, is_leader_task)
|
|
VALUES ($1, $2, $3, 'failed', 1, 3, $4)
|
|
RETURNING id
|
|
`, fx.LeaderID, runtimeID, issueID, tc.isLeader).Scan(&parentID); err != nil {
|
|
t.Fatalf("seed parent task: %v", err)
|
|
}
|
|
t.Cleanup(func() {
|
|
testPool.Exec(context.Background(), `DELETE FROM agent_task_queue WHERE id = $1 OR parent_task_id = $1`, parentID)
|
|
})
|
|
|
|
child, err := testHandler.Queries.CreateRetryTask(ctx, db.CreateRetryTaskParams{ID: util.MustParseUUID(parentID)})
|
|
if err != nil {
|
|
t.Fatalf("CreateRetryTask: %v", err)
|
|
}
|
|
if child.IsLeaderTask != tc.isLeader {
|
|
t.Fatalf("child.IsLeaderTask = %v, want %v (parent role must be inherited)", child.IsLeaderTask, tc.isLeader)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestCreateComment_SquadMentionPrivateLeaderBlocksPlainMember verifies that
|
|
// a plain workspace member cannot trigger a private squad leader via @squad
|
|
// mention. This is the regression test for the P1 finding: without the
|
|
// canAccessPrivateAgent gate in the squad mention branch, a member could
|
|
// bypass the private-agent restriction by mentioning the squad instead of
|
|
// the agent directly.
|
|
func TestCreateComment_SquadMentionPrivateLeaderBlocksPlainMember(t *testing.T) {
|
|
if testHandler == nil || testPool == nil {
|
|
t.Skip("database not available")
|
|
}
|
|
ctx := context.Background()
|
|
|
|
// Use privateAgentTestFixture to get a private agent + plain member.
|
|
agentID, _, memberID := privateAgentTestFixture(t)
|
|
|
|
// Create a squad with the private agent as leader.
|
|
var squadID string
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO squad (workspace_id, name, description, leader_id, creator_id)
|
|
VALUES ($1, 'Private Leader Squad', '', $2, $3)
|
|
RETURNING id
|
|
`, testWorkspaceID, agentID, 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)
|
|
})
|
|
|
|
// Create an issue.
|
|
var issueID string
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO issue (workspace_id, creator_type, creator_id, title)
|
|
VALUES ($1, 'member', $2, 'private leader squad mention test')
|
|
RETURNING id
|
|
`, testWorkspaceID, memberID).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)
|
|
})
|
|
|
|
// Plain member posts a comment mentioning the squad.
|
|
w := httptest.NewRecorder()
|
|
r := newRequestAs(memberID, "POST", "/api/issues/"+issueID+"/comments", map[string]any{
|
|
"content": "[@Squad](mention://squad/" + squadID + ") please handle",
|
|
})
|
|
r = withURLParam(r, "id", issueID)
|
|
testHandler.CreateComment(w, r)
|
|
if w.Code != http.StatusCreated {
|
|
t.Fatalf("CreateComment: expected 201, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
|
|
// The private leader must NOT have a queued task — plain member lacks access.
|
|
var count int
|
|
if err := testPool.QueryRow(ctx,
|
|
`SELECT count(*) FROM agent_task_queue WHERE issue_id = $1 AND agent_id = $2 AND status = 'queued'`,
|
|
issueID, agentID,
|
|
).Scan(&count); err != nil {
|
|
t.Fatalf("count tasks: %v", err)
|
|
}
|
|
if count != 0 {
|
|
t.Fatalf("private leader got %d queued tasks from plain member squad mention; want 0 (access denied)", count)
|
|
}
|
|
}
|
|
|
|
// TestCreateComment_SquadMentionTriggersLeader verifies that @mentioning a
|
|
// squad in a comment triggers the squad's leader agent via the mention path,
|
|
// even when the issue is NOT assigned to that squad.
|
|
func TestCreateComment_SquadMentionTriggersLeader(t *testing.T) {
|
|
if testHandler == nil || testPool == nil {
|
|
t.Skip("database not available")
|
|
}
|
|
ctx := context.Background()
|
|
|
|
// Create a squad with a leader agent.
|
|
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)
|
|
}
|
|
|
|
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, "Mention Trigger 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)
|
|
})
|
|
|
|
// Create an issue NOT assigned to the squad (assigned to nobody).
|
|
var issueID string
|
|
if err := testPool.QueryRow(ctx, `
|
|
INSERT INTO issue (workspace_id, creator_type, creator_id, title)
|
|
VALUES ($1, 'member', $2, 'squad mention trigger test')
|
|
RETURNING id
|
|
`, testWorkspaceID, testUserID).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)
|
|
})
|
|
|
|
countQueued := func(agentID string) int {
|
|
var n int
|
|
if err := testPool.QueryRow(ctx,
|
|
`SELECT count(*) FROM agent_task_queue WHERE issue_id = $1 AND agent_id = $2 AND status = 'queued'`,
|
|
issueID, agentID,
|
|
).Scan(&n); err != nil {
|
|
t.Fatalf("count tasks for %s: %v", agentID, err)
|
|
}
|
|
return n
|
|
}
|
|
|
|
// Post a comment that @mentions the squad.
|
|
w := httptest.NewRecorder()
|
|
r := newRequest("POST", "/api/issues/"+issueID+"/comments", map[string]any{
|
|
"content": "[@Squad](mention://squad/" + squadID + ") please handle this",
|
|
})
|
|
r = withURLParam(r, "id", issueID)
|
|
testHandler.CreateComment(w, r)
|
|
if w.Code != http.StatusCreated {
|
|
t.Fatalf("CreateComment: expected 201, got %d: %s", w.Code, w.Body.String())
|
|
}
|
|
|
|
// The squad's leader should have a queued task.
|
|
if got := countQueued(leaderID); got != 1 {
|
|
t.Fatalf("after @squad mention: expected 1 leader task, got %d", got)
|
|
}
|
|
}
|