mirror of
https://github.com/onyx-dot-app/onyx.git
synced 2026-10-23 03:42:59 -05:00
Evan LohnandClaude Opus 5.5
c7f9a0e859
## 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.
- [x] [Optional] Override Linear Check
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- This is an auto-generated description by cubic. -->
---
## 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`.
<sup>Written for commit 70699450d1.
Summary will update on new commits.</sup>
<a
href="https://cubic.dev/pr/onyx-dot-app/onyx/pull/16336?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="View guided diff"
src="https://www.cubic.dev/buttons/review-in-cubic-light.svg"></picture></a>
<a
href="https://www.cubic.dev/action/auto-fix/pr/onyx-dot-app/onyx/16336?returnTo=https%3A%2F%2Fgithub.com%2Fonyx-dot-app%2Fonyx%2Fpull%2F16336&source=description"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/turn-on-auto-fix-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/turn-on-auto-fix-light.svg"><img
alt="Turn on auto-fix"
src="https://www.cubic.dev/buttons/turn-on-auto-fix-light.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->
## 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.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
