fix: serialize default assistant tool updates

Merged
EV
20h ago
+514-246

Description

The default assistant is one global persona. Many callers change its tool list at the same time: the MCP setup modal, plugin forms, Chat Preferences, and parallel e2e specs. The backend did not serialize these writes.

Two failures came from this:

  • update_default_assistant_configuration set persona.tools = [] with no lock. A concurrent write changed the persona__tool rows first, so the flush failed with StaleDataError ("expected to delete N row(s); Only M were matched"). The handler caught only ValueError, so the user got a 500.
  • Callers sent a full list built from a cached copy. A stale copy dropped tools that another caller had just added, or failed with 400 "Tool with ID N not found" after another spec deleted its MCP server.

Changes:

  • All writers of the default assistant's tool list now lock the persona row (SELECT ... FOR UPDATE), the same way set_default_chat_tool__no_commit from #16281 does. The full-list update now changes persona__tool rows with core SQL, so a concurrent delete cannot cause StaleDataError.
  • New PATCH /admin/default-assistant/tools with {tool_ids, enabled}. It adds or removes only the given tools. It uses the same RBAC as the existing route (FULL_ADMIN_PANEL_ACCESS).
  • useDefaultAssistantConfiguration.updateTools uses the new route. This covers MCP enable, plugin forms and Chat Preferences toggles. The full-list PATCH /admin/default-assistant still works, and the system prompt editor still uses it.
  • The default assistant router now raises OnyxError instead of HTTPException.

This fixes the flaky e2e test web/tests/e2e/mcp/mcp_per_user_key.spec.ts:131. The test enables Chat for a new MCP server while other specs change the same tool list. The MCP enable and the e2e helper addToolsToDefaultAssistant now send only their own tool ids. The backend serializes them with the other writers, so they no longer get a 500 or lose tools. The tools_popover and default_agent specs used the same read-merge-write pattern. They now use the helper too.

How Has This Been Tested?

  • New external-dependency-unit tests in backend/tests/external_dependency_unit/db/test_default_assistant_disabled_tool.py, with real threads and Postgres:
    • Concurrent full-list updates. On main this test fails with StaleDataError: DELETE statement on table 'persona__tool' expected to delete 6 row(s); Only 0 were matched. With this change it passes (3 runs in a row).
    • Concurrent targeted adds keep every tool, and a removal drops only the given tools.
    • The new endpoint adds tools and returns 400 for an unknown tool id.
    • A default selection change leaves custom agent tools alone. A full-list resend does not enable a stale legacy OpenAPI attachment. These two replace mock-based unit tests that depended on the old ORM code path.
  • Backend unit tests: route contract manifest, OpenAPI availability, MCP default Chat availability.
  • Jest: all suites under src/sections/plugins, src/sections/actions, src/lib/agents and src/views/admin pass (47 suites, 526 tests).
  • tsc --noEmit on web/ passes. Pre-commit passes.

Additional Options

  • [Optional] Please cherry-pick this PR to the latest release version.
  • [Optional] Override Linear Check

🤖 Generated with Claude Code

Summary by cubic

Fixes races between concurrent writers of the default assistant's tool list. Parallel callers (MCP setup, plugin forms, Chat Preferences, e2e specs) could trigger a 500 from StaleDataError, silently drop tools when one caller's stale cached list overwrote another's changes, or deadlock when a selection write overlapped a custom tool update.

  • All writers now lock the default persona row; the full-list update replaces persona__tool rows with core SQL instead of an ORM collection, so concurrent deletes no longer fail the flush.
  • Custom tool create and update routes take the persona lock before saving the tool, so selection writers and tool updates share one lock order and cannot deadlock.
  • Adds PATCH /admin/default-assistant/tools ({tool_ids, enabled}) that only adds or removes the given tools, and switches MCP enable, plugin forms, Chat Preferences toggles, and the e2e helper to it.
  • Errors from the default assistant router now surface as OnyxError instead of HTTPException.

Summary by CodeRabbit

  • New Features
    • Added incremental tool management for the default assistant, allowing selected tools to be enabled or disabled without replacing other selections.
    • Added validation for tool selections and clearer error responses for invalid requests.
  • Bug Fixes
    • Improved reliability when multiple settings updates occur simultaneously.
    • Preserved tools added by other sessions during concurrent updates.
    • Prevented unrelated assistant tool settings from being changed.
  • Documentation
    • Updated default-assistant configuration and tool-management documentation.