You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Adds the persistence/config foundation for dynamic MCP tool loading, including bot config defaults, loaded-tool state storage, migrations, and metrics plumbing.
Reason: The output does not actually list any specific bugs; it explains lack of access and provides a template to be filled in later. A ready-to-fill table is not itself a list of bugs.
Reason: The output does not actually describe any specific bugs; it only states it cannot access the tracker and provides a template for future use. Therefore it does not include a description of each bug.
Reason: The output provides a template and asks for bug reports, but it does not actually list any bugs nor attribute each bug to a specific user. No concrete per-bug 'Reported by' entries are present.
Rubric: attributes the bug about trying to save without a color and the save button not doing anything to @maria.nunez
Reason: The output does not mention the specific bug (“trying to save without a color” / “save button not doing anything”) nor does it attribute it to @maria.nunez. It only asks the user to provide bug reports and offers a template.
Rubric: the bug about the end user being able to change channel banner is attributed to @maria.nunez
Reason: The output does not mention @maria.nunez or attribute any specific bug (including the channel banner change bug) to any reporter; it only provides a generic template and request for more information.
Anthropic
⚠️Overall: 21/28 tests passed (75.0%)
Provider
Total
Passed
Failed
Pass Rate
⚠️ ANTHROPIC
28
21
7
75.0%
❌ Failed Evaluations
Show 7 failures
ANTHROPIC
1. TestReactEval/[anthropic]_react_cat_message
Score: 0.00
Rubric: The word/emoji is a cat emoji or a heart/love emoji
Reason: The output is the literal text "heart_eyes_cat", not an actual cat emoji (e.g., 😺/🐱) or a heart/love emoji (e.g., ❤️/😍).
Reason: The output explicitly says it cannot retrieve or compile the list and instead provides suggestions on how to search for bug reports; it does not actually present a list of bugs.
Reason: The output states it cannot access message history or a bug tracking system and suggests ways to search, but it does not list any bugs or include descriptions of each bug.
Rubric: attributes the bug about trying to save without a color and the save button not doing anything to @maria.nunez
Reason: The output does not mention @maria.nunez and does not attribute any bug (including saving without a color / save button doing nothing) to her; it only states inability to access history and suggests ways to search.
Rubric: the bug about the end user being able to change channel banner is attributed to @maria.nunez
Reason: The output does not mention the bug about an end user being able to change the channel banner, nor does it attribute it to @maria.nunez. It only states an inability to access message history and suggests how to search for bugs.
Rubric: mentions Github and refers to the documentation
Reason: The output refers to documentation (docs.mattermost.com) but does not mention GitHub anywhere, so it fails the requirement to mention GitHub and refer to the documentation.
This comment was automatically generated by the eval CI pipeline.
The PR adds infrastructure for MCP dynamic tool loading configuration with JSON-based defaulting, persists loaded MCP tools in a new database table, adds observability metrics, introduces tool description overrides, and enables LLM tracing configuration.
BotConfig MCP Dynamic Tool Loading field and defaults llm/configuration.go, llm/configuration_test.go
BotConfig adds MCPDynamicToolLoading boolean with custom UnmarshalJSON defaulting to true when omitted from JSON, ensuring legacy omitted configs adopt dynamic loading.
MCPToolConfig retrieval description override field config/mcp_config.go, config/mcp_config_test.go
MCPToolConfig gains RetrievalDescriptionOverride field with omitempty JSON serialization; tests verify omission when empty and independence from policy queries.
Loaded MCP tools persistence model and store methods store/loaded_mcp_tools.go
LoadedMCPTool struct and five store methods (Upsert, List, Delete, DeleteByNames, DeleteForConversation) with ON CONFLICT upsert preserving CreatedAt and validating required identifiers.
Loaded MCP tools database schema and migrations store/migrations/000009_create_loaded_mcp_tools.up.sql, store/migrations/000009_create_loaded_mcp_tools.down.sql
LLM_LoadedMCPTools table with composite primary key on (ConversationID, BotID, UserID, ToolName), indexed for conversation-scoped lookups, and rollback migration.
Eight tests covering upsert/list/delete operations, scoping, metadata preservation on conflict, no-op on missing rows, conversation-wide deletion, batch name-based deletion, and required field validation.
Agent persistence with MCPDynamicToolLoading column store/agents.go, store/agents_test.go
Agent SELECT/INSERT/UPDATE queries extended to handle mcp_dynamic_tool_loading; agentRow and toBotConfig() map the column; test helper updated and assertions added for round-trip verification.
Agent MCPDynamicToolLoading column schema and defaults store/migrations/000008_add_mcp_dynamic_tool_loading.up.sql, store/migrations/000008_add_mcp_dynamic_tool_loading.down.sql
Non-null mcp_dynamic_tool_loading column added to Agents_UserAgents with default true; existing active rows backfilled; rollback migration drops column.
Agent MCPDynamicToolLoading and EnabledMCPTools round-trip tests store/agents_test.go
Two new test cases verify MCPDynamicToolLoading toggles persist through update/get cycles and EnabledMCPTools (bare and namespaced) round-trip exactly.
Store migration count and default value verification store/store_test.go
TestRunMigrations expects 9 total migrations; new subtest inserts agent without explicit MCPDynamicToolLoading and asserts default true via GetAgent.
Conversation cleanup includes loaded MCP tools deletion store/conversations.go
CleanupDeletedConversations adds DELETE from LLM_LoadedMCPTools for soft-deleted conversations within the same transaction.
Legacy service-to-bot migration sets MCPDynamicToolLoading: true when constructing bots; three test cases verify the flag is asserted for single, multiple, and URL-referenced service migrations.
MCP dynamic tool event metrics interface and implementation metrics/metrics.go, metrics/noop.go
Metrics interface adds ObserveMCPDynamicToolEvent(botName, event, result string); counter registered under MCP subsystem with bot_name, event, result labels; no-op implementation provided.
Test instantiates metrics and calls ObserveMCPDynamicToolEvent with empty bot name, verifying counter increments to 1 with label defaults (bot_name="unknown").
Config-level LLM trace enabling field config/config.go
The PR spans multiple coordinated subsystems (configuration, persistence, metrics) with moderate complexity. The loaded MCP tools persistence layer introduces new store methods and a database table with comprehensive test coverage. Agent persistence updates touch SQL query construction requiring placeholder validation. Most changes follow established patterns (migrations, store methods, metric registration) with good test coverage, but the distributed nature across config, storage, and metrics layers requires understanding the full feature scope.
Possibly related PRs
mattermost/mattermost-plugin-agents#542: Modifies config-level tracing support; this PR adds EnableLLMTrace field while that PR removes it in favour of OpenTelemetry configuration.
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name
Status
Explanation
Resolution
Docstring Coverage
⚠️ Warning
Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%.
Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name
Status
Explanation
Title check
✅ Passed
The title 'dynamic mcp: add persisted loading state' accurately and concisely describes the main change—adding persistence infrastructure for dynamic MCP tool loading, which is reflected across config, store migrations, metrics, and database models throughout the changeset.
Linked Issues check
✅ Passed
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
✅ Passed
Check skipped because no linked issues were found for this pull request.
Description Check
✅ Passed
Check skipped - CodeRabbit’s high-level summary is enabled.
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches📝 Generate docstrings
Create stacked PR
Commit on current branch
🧪 Generate unit tests (beta)
Create PR with unit tests
Commit unit tests in branch dynamic-mcp-01-config-store
Comment @coderabbitai help to get the list of available commands and usage tips.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Security Review: No Findings
This PR adds persistence/config foundation for dynamic MCP tool loading (migrations, store methods, config fields, metrics). After tracing all changes:
SQL injection: All queries use parameterized statements via the squirrel query builder. No raw string interpolation.
Data isolation: ListLoadedMCPTools correctly scopes by the (conversationID, botID, userID) triple, preventing cross-user data access at the store layer.
No new HTTP endpoints: This PR introduces no user-facing API routes. Authorization enforcement will need to be verified in subsequent PRs in this stack where handlers call these store methods.
Config fields (EnableLLMTrace, MCPDynamicToolLoading, RetrievalDescriptionOverride): All admin-only configuration. Not consumed by any code paths in this PR yet.
Metrics: ObserveMCPDynamicToolEvent label parameters (event, result) are clearly designed for bounded enums. No callers exist in this PR.
Secrets exposure: No credentials or sensitive data in logs, errors, or responses.
No medium, high, or critical vulnerabilities identified.
The reason will be displayed to describe this comment to others. Learn more.
Security Review: No Findings
This PR adds persistence and configuration scaffolding for dynamic MCP tool loading. After tracing all changes, no medium/high/critical vulnerabilities were found:
Store layer (store/loaded_mcp_tools.go): All SQL queries use the squirrel query builder with parameterized values. CRUD methods require conversationID, botID, and userID scoping, and no HTTP handlers expose these methods directly in this PR.
Config fields (EnableLLMTrace, MCPDynamicToolLoading, RetrievalDescriptionOverride): All are admin-only config or internal feature flags. None have consumers in this PR that could leak data or affect authorization.
Metrics (ObserveMCPDynamicToolEvent): Labels are bot_name, event, result. No callers exist yet in this PR. The existing ObserveTokenUsage follows the same pattern, so this is consistent.
Migrations: Standard DDL with appropriate defaults. The DEFAULT true on the new column is intentional for backward compatibility.
No new HTTP endpoints or API surface is added by this PR — this is pure infrastructure for future PRs in the stack.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Summary
Adds the persistence/config foundation for dynamic MCP tool loading, including bot config defaults, loaded-tool state storage, migrations, and metrics plumbing.
Stack: 1/6
Ticket Link
N/A
Screenshots
N/A
Release Note
Made with Cursor
Summary by CodeRabbit
Release Notes