MM-65671: Agents CRUD, legacy bot migration, and bot account sync

[nickmisasi]

Summary

Implements user agent CRUD improvements, migration of legacy config-defined bots into user agents, and synchronization of Mattermost bot accounts when agents are updated or deleted (display name patch, bot deactivation). Updates the agents UI, system console wiring, i18n strings, and adds e2e coverage plus a small permissions helper.

Tools list on RHS is based on allowed tools for selected agent

Vs an agent with no tools enabled

Ticket Link

Jira https://mattermost.atlassian.net/browse/MM-65671

Release Note

NONE

Made with Cursor

Summary by CodeRabbit

• New Features

  • Full Agents product: REST APIs and UI to create/edit/delete agents (avatar upload), access controls, service & model selection, per-agent MCP tool controls, delegated admins, license gating, Agents product page and nav integration, and RHS/tool-picker respect per-agent allowlists.

• Migrations

  • One-time migration of legacy bots into Agents and new Agents DB table + persistence APIs.

• Tests

  • Expanded unit, integration, and Playwright E2E coverage for agents, access control, MCP tools, provider/config flows, and migrations.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds DB-backed self-service AI agents with REST endpoints, migrations, bots registry integration and cluster events, per-agent MCP tool allowlists, frontend Agents UI and client APIs, E2E test suites and helpers, and assorted plumbing and dependency updates.

Changes

Cohort / File(s)	Summary
API & Handlers api/api.go, api/api_agents.go, api/api_admin_test.go, api/api_agents_test.go	Introduces AgentStore and ClusterAgentNotifier interfaces; extends API constructor; registers authenticated, license-gated /agents and /services endpoints (CRUD, avatar, fetch models); adds extensive handler logic and tests.
Store & Migrations store/agents.go, store/agents_test.go, store/migrations/000004_create_user_agents_table.*, store/migrations/000005_user_agent_bot_fields.*, store/store_test.go	Adds Agents_UserAgents table and migrations; implements Store CRUD with JSON-backed slice fields, soft-delete semantics, mappers, and comprehensive tests; test harness updated for schema.
Domain Models & Conversion useragents/model.go, llm/configuration.go	Adds UserAgent and EnabledTool models with DB JSON helpers; adds llm.EnabledMCPTool and BotConfig.EnabledMCPTools (nil vs empty semantics) for per-agent MCP allowlists.
Bots Registry & Sync bots/bots.go, bots/bots_test.go, bots/agents_test.go, bots/permissions_test.go	MMBots accepts an AgentStore; adds ForceRefreshOnNextEnsure(); EnsureBots() loads DB-backed agents, converts to bot configs (including MCP tools) and resets force-refresh; tests include failing-agent-store case and agent-backed usage restrictions.
Cluster Events & Migration server/cluster_events.go, server/legacy_bot_migration.go, server/main.go	Adds agent_update cluster event and generic publisher; implements distributed migration of legacy config bots into UserAgents with mutex/flagging; activation runs migration → refresh → publish agent update.
LLM / MCP Tooling llm/tools.go, llm/tools_test.go, llmcontext/llm_context.go, llmcontext/llm_context_test.go	Adds ToolStore.RetainOnlyMCPTools and integrates allowlist filtering into LLM context builder; normalizes server origins and adds tests for filtering/normalization.
Server Config API config/config.go, server/cluster_events.go	Adds StorePersistedConfigWithoutNotify to update in-memory persisted config without notifying listeners; refactors cluster event publishing to support agent events.
Frontend: Agents UI & Client webapp/src/components/agents/*, webapp/src/client.tsx, webapp/src/types/agents.ts, webapp/src/index.tsx, webapp/src/utils/permissions.ts, webapp/src/bots.tsx, webapp/src/i18n/en.json	Adds Agents product route/page and components (list, modal, tabs, MCPs UI, row, delete dialog, license gate), client APIs for agents/services/models, TS types, permission helper, i18n strings, and integrates enabled-MCP-tools into RHS/tool-provider UI.
System Console & Notices webapp/src/components/system_console/*, e2e/helpers/system-console.ts	Replaces legacy inline bot editor with BotsMovedNotice; fetches runtime bots for default-bot dropdown; exports native-tools item; updates console flows to point to Agents page.
E2E Helpers & Tests e2e/helpers/*, e2e/tests/agents/*, e2e/tests/system-console/*, e2e/scripts/ci-test-groups.mjs	Adds E2E helpers (agent API, container, page object, OpenAI mock, mm client assertions), many agent-focused Playwright suites (access control, CRUD, MCP tools, provider config), retires/skips legacy System Console suites, and updates CI shard assignments.
OpenAI Mock & Test Utilities e2e/helpers/openai-mock.ts, e2e/helpers/agent-api.ts, e2e/helpers/agent-container.ts, e2e/helpers/agent-page.ts, e2e/helpers/ai-plugin.ts, e2e/helpers/mm.ts	New helpers for chat-completion mock rules, agent CRUD helpers, test container provisioning, Agents page object, RHS tool interactions, and API-based DM assertion utilities.
Test Harness & Call Sites api/api_test.go, api/api_admin_test.go, conversations/*, many tests	Updated test harness and numerous tests to match changed constructor arity and added agentStore wiring (many calls adjusted with extra nil parameters).
Misc & Build go.mod, .gitignore	Bumps Go dependencies and adds replace directives; .gitignore rule changed from excluding .planning/ to .planning.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Client
  participant API
  participant Store
  participant BotRegistry
  participant Cluster

  Client->>API: POST /agents (CreateAgentRequest)
  API->>Store: CreateAgent(agent)
  Store-->>API: created agent (ID)
  API->>BotRegistry: create/update Mattermost bot account
  BotRegistry-->>API: bot_user_id
  API->>Cluster: PublishAgentUpdate()
  Cluster->>BotRegistry: ForceRefreshOnNextEnsure() & EnsureBots()
  BotRegistry-->>Cluster: EnsureBots result
  API-->>Client: 201 Created (agent)

Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

• Speed up agent e2e CI via sharding #561: CI shard/group updates and agent-focused E2E test orchestration overlap with updated e2e/scripts/ci-test-groups.mjs.
• feat: per-tool config with vetted tool seeding and user provider toggles #520: Related to per-agent MCP allowlisting and ToolStore filtering (EnabledMCPTool/allowlist changes).
• Migrate plugin config from config.json to database #503: Overlaps API constructor/agent-store injection and cluster event publishing changes.

Suggested labels

Setup Cloud Test Server

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch MM-65671

[mm-cloud-bot]

Test server creation failed. Review the error details here.

[mm-cloud-bot]

Test server creation failed. Review the error details here.

[mm-cloud-bot]

Test server creation failed. Review the error details here.

[edgarbellot]

LGTM, thank you Nick!

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit a92efcb. Configure here.}

[mm-cloud-bot]

Test server creation failed. Review the error details here.

[mm-cloud-bot]

Test server creation failed. Review the error details here.

[mm-cloud-bot]

Test server creation failed. Review the error details here.

[mm-cloud-bot]

Test server creation failed. Review the error details here.

[mattermost-build]

Test check 0baf37d — action needed

Core agent CRUD, store, bots, permissions, bifrost, and config are well-tested across 18 unit and 4 new E2E spec files, but the 137-line legacy bot migration (server/legacy_bot_migration.go) has no unit tests despite complex idempotency, deferred-migration, and all-or-nothing logic.
Add unit tests for migrateLegacyConfigBotsToUserAgents covering: already-migrated idempotency, empty config bots no-op, deferred migration when MM bot rows are absent, happy-path agent creation + config cleanup, and username de-dup skipping.

Note: Max files limit reached: analyzed 50 of 51 source files and 42 of 42 test files.

More details (truncated)

Test Files Detected

Category	Count	Files
Unit	18	api/api_admin_test.go, api/api_agents_test.go (NEW), api/api_config_test.go, api/api_test.go, bifrost/bifrost_test.go, bifrost/config_test.go, bots/agents_test.go (NEW), bots/bots_test.go, bots/permissions_test.go, conversations/conversations_test.go, conversations/direct_message_eval_test.go, conversations/handle_messages_test.go, conversations/tool_handling_test.go, llm/configuration_responses_api_test.go, llm/tools_test.go, llmcontext/llm_context_test.go, store/agents_test.go (NEW), store/store_test.go
Integration	0	(none distinct from unit)
E2E	11	e2e/tests/agents/access-control.spec.ts (NEW), e2e/tests/agents/crud.spec.ts (NEW), e2e/tests/agents/mcp-tools.spec.ts (NEW), e2e/tests/agents/provider-config.spec.ts (NEW), e2e/tests/action-item-extraction/follow-ups.spec.ts, e2e/tests/smart-reactions/basic-reactions.spec.ts, e2e/tests/system-console/bot-management-ui.spec.ts, e2e/tests/system-console/bot-native-tools.spec.ts, e2e/tests/system-console/bot-reasoning-config.spec.ts, e2e/tests/system-console/bot-validation.spec.ts, e2e/tests/system-console/initial-state-navigation.spec.ts, e2e/tests/system-console/live-service-full-flow.spec.ts

Analysis

This is a large feature PR adding full agent CRUD with DB persistence, legacy bot migration, access controls, MCP tool allowlisting, and a rich UI. Test coverage is substantial across most layers:

Well-covered areas:

• API layer (api/api_agents_test.go): 20+ test functions covering create/list/get/update/delete handlers, all permission variants (ManageOwnAgent, ManageOthersAgent, ManageSystem, creator, agent admin, migrated-bot ManageSystem path), free-tier quota enforcement, MCP tool allowlist/auto-enable combinations, username-change prevention, service ID validation, service listing secret-stripping, models-fetch auth and error cases, JSON serialization round-trips, and canUserAccessAgent admin bypass.

• Store layer (store/agents_test.go): Full CRUD test suite using real SQLite DB — create-and-get with all field round-trips (JSON slices, booleans, timestamps), non-existent-get returns nil, list returns only active (soft-delete excluded), list-by-creator filtering, update bumps UpdateAt, soft-delete idempotency, CountActiveAgents.

• Bots layer: bots/agents_test.go verifies DB-backed bot lookup by all me

...truncated. View full analysis details

Suggestions
The primary gap is server/legacy_bot_migration.go. Add unit tests covering at minimum:

1. Already-migrated (idempotency): Set legacyConfigBotsMigratedKey = "true" — function should return (false, nil) without touching the store.
2. No config bots: dbCfg.Bots is empty — function should return (false, nil) and NOT set the migration flag.
3. MM bot not yet created (deferred): Config has a bot whose username doesn't appear in the MM bot list — function should return (false, nil) without partial writes.
4. Happy path: Config has bots with matching MM bot rows — agents are created, config.Bots is cleared, migration flag is set, returns (true, nil).
5. Already-migrated username (de-dup): Bot already exists in ListAgents result — should skip create for that bot and proceed with others.
   Additionally, consider a test for StorePersistedConfigWithoutNotify (nil config case vs valid config) and a unit test for the handleUploadAgentAvatar 10 MB cap.

To override, comment /test-analysis-override <reason> after verifying tests are adequate or not required.

[nickmisasi]

/test-analysis-override chimed in at the 11th hour after passing all day. Need to merge this for RC, so will address gaps in follow up PR.

[cursor]

Stale comment

Security Review — No Findings

Reviewed all new and modified server-side code (agent CRUD API, store layer, bot registry, permissions, legacy migration, LLM context/tool filtering, configuration, cluster events) and frontend TypeScript for this PR at commit 0baf37d.

Changes since last review (ae6bc1b → 0baf37d): License gating replaced with per-creation quota (checkAgentCreateQuota), request body size limits added (MaxAgentRequestBodyBytes), BotConfig.Validate() with MaxCustomInstructionsRunes cap, sanitizeAgentForUser strips CustomInstructions from non-admin responses, config clone on GET to prevent shared state mutation, UsageRestrictionsForUserConfig refactored to standalone function (nil-safe), EnsureBots protects active DB agents from deactivation when service is invalid.

Key security properties verified:

• Authentication: All agent CRUD endpoints sit behind MattermostAuthorizationRequired middleware, which reads the server-injected Mattermost-User-Id header. No identity from request body or query params.
• Authorization: Create requires PermissionManageOwnAgent or PermissionManageSystem plus quota check. Update/delete requires canManageAgent() (creator, admin list, PermissionManageOthersAgent, or PermissionManageSystem for ownerless migrated bots). List/get filter by canUserAccessAgent() with safe default (default: return false). Service/model endpoints require canConfigureAgentServices().
• Data sanitization: sanitizeAgentForUser() strips CustomInstructions from responses for non-manager users, preventing disclosure of sensitive organizational procedures.
• Secrets protection: ServiceInfo struct explicitly excludes API keys, AWS secrets, URLs, and credentials. handleFetchModelsForService uses admin-configured credentials server-side via serviceID lookup — no user-controlled URLs (no SSRF).
• SQL injection: All store queries use parameterized placeholders ($1–$26).
• Input validation: Username regex, BotConfig.Validate() (field presence, enum range, custom instructions length cap at 16384 runes), service ID existence check against config, username immutability on update, request body size cap (512 KiB), avatar size cap (10 MB via io.LimitReader).
• Immutable fields: UpdateAgent SQL excludes ID, CreatorID, BotUserID, CreateAt, DeleteAt.
• MCP tool filtering: RetainOnlyMCPTools preserves built-in tools (empty ServerOrigin) and filters MCP tools by per-agent allowlist. Auth errors filtered consistently. Nil allowlist = no filtering (backward compat).
• Legacy migration: Cluster mutex, idempotent flag, defers when prerequisites are missing.
• Error handling: gin.AbortWithError stores errors for ginlogger but does not write details to HTTP response bodies.

No medium, high, or critical vulnerabilities identified.

  

_{Sent by Cursor Automation: Find vulnerabilities}

[cursor]

Security Review — PR #589

Scope: Agents CRUD endpoints, permissions model, legacy bot migration, store layer, config exposure, and webapp permissions.

Summary

No medium, high, or critical vulnerabilities found. The PR implements a well-structured authorization model with proper security controls throughout.

Areas Reviewed

Authentication & Identity

• All new agent endpoints sit behind MattermostAuthorizationRequired middleware, which validates the server-injected Mattermost-User-Id header. User identity is never taken from request body or query params. ✅

Authorization

• Create: Gated by PermissionManageOwnAgent or PermissionManageSystem via canCreateAgent().
• Read (list/get): Filtered by canUserAccessAgent() which delegates to UsageRestrictionsForUserConfig for user-scope access checks.
• Update/Delete: Gated by canManageAgent() requiring agent admin status (creator or delegated), PermissionManageOthersAgent, or PermissionManageSystem for legacy bots.
• Services/Models: canConfigureAgentServices() requires agent management permissions.
• Immutable fields (ID, CreatorID, BotUserID, CreateAt, DeleteAt) are preserved from the database record and not modifiable via the update endpoint. ✅

Data Sanitization

• sanitizeAgentForUser() strips CustomInstructions from agent configs returned to non-admin users, preventing exposure of sensitive organizational procedures.
• The /services endpoint returns a ServiceInfo projection that explicitly excludes API keys, AWS credentials, and other secrets. ✅

SQL Injection

• All store queries use parameterized placeholders ($1, $2, etc.). No string concatenation of user input into SQL. ✅

Input Validation

• Request body size capped at 512 KiB via MaxBytesReader.
• Username validated against ^[a-z][a-z0-9._-]*$ regex.
• ServiceID validated against existing admin-configured services.
• BotConfig.Validate() enforces field constraints including access level ranges and CustomInstructions length limits. ✅

Secret Exposure

• handleGetConfig (returns full config with API keys) is behind admin-only middleware (PermissionManageSystem).
• handleFetchModelsForService uses stored service credentials server-side and returns only model metadata, never the credentials themselves.
• Error messages from external API calls are logged server-side via gin's error handling and do not leak into HTTP response bodies. ✅

SSRF

• handleFetchModelsForService only accepts a serviceID referencing admin-configured services; users cannot supply arbitrary URLs.
• The admin-only handleFetchModels endpoint accepts user-supplied URLs but is restricted to system admins. ✅

Migration Safety

• migrateLegacyConfigBotsToUserAgents uses a cluster mutex and idempotency flag to prevent concurrent or duplicate migrations.
• Migrated agents have CreatorID = "" and AutoEnableNewMCPTools = true, preserving pre-existing behavior. ✅

No actionable findings to report.

  

_{Sent by Cursor Automation: Find vulnerabilities}