mirror of
https://github.com/CherryHQ/cherry-studio.git
synced 2026-07-06 22:55:56 +08:00
0ae8ea4758
### What this PR does Before this PR: Deleting a global MCP server via the UI removed the row from `mcp_server` and (via FK cascade) the `assistant_mcp_server` junction table, but each agent stored its MCP servers as an `agent.mcps` JSON `string[]` column with no foreign key. The deleted MCP's ID lingered in that JSON array, so agents kept trying to connect to a non-existent MCP during startup/heartbeat — generating thousands of "Failed to connect to MCP" error logs daily. After this PR: The agent↔MCP relation is **normalized**. The `agent.mcps` JSON column is dropped and replaced by an `agent_mcp_server` junction table with `ON DELETE CASCADE` on both sides (mirroring the existing `assistant_mcp_server` table). Deleting an MCP server — or an agent — now cascades structurally instead of leaving dangling JSON references. `McpServerService.delete()` removes the server inside a single `withWriteTx` transaction; `AgentService.removeMcpFromAllAgentsTx()` first captures the affected agent IDs and explicitly deletes their junction rows (the FK cascade is a safety net), so that after the transaction commits `AgentService.emitAgentUpdatedForIds()` fires `onAgentUpdated` for each affected agent and lets warm sessions refresh their tool policy live. The v1→v2 migration backfills the new table: `migrateAgentMcps()` reads the legacy `agents.mcps` JSON arrays, remaps the old MCP IDs to their new UUIDs (via `McpServerMigrator`'s `mcpServerIdMapping`), drops dangling references, and writes `agent_mcp_server` rows. Fixes #15986 ### Why we need it and why it was done in this way `agent.mcps` stored MCP server IDs as an unconstrained JSON `string[]`, so SQLite could not cascade-delete them and the data model allowed agents to reference MCP servers that no longer existed. Normalizing the relation into a junction table with FK `ON DELETE CASCADE` is the root-cause fix: referential integrity is now enforced by the database, deletes cascade structurally, and no application-side JSON rewriting is needed on every MCP delete. All agent↔MCP writes (create / update / delete) go through `AgentService` inside `withWriteTx` and roll back atomically; `onAgentUpdated` events fire only after commit so sessions never refresh against an uncommitted state. Reads project the junction rows back into the agent DTO's `mcps` field, so the public API shape is unchanged for consumers. The following alternatives were considered: - **Keep the JSON column and filter it in JS / `json_remove()` on delete**: leaves the unconstrained column (and the dangling-reference class of bugs) in place, and requires rewriting every agent row on each MCP delete. Rejected in favor of normalizing the relation, which fixes the invariant at the schema level. ### Breaking changes None for users. Internally, the `agent.mcps` column is dropped in favor of the `agent_mcp_server` junction table; the v1→v2 migrator (`migrateAgentMcps`) carries existing associations forward, remapping legacy MCP IDs to their new UUIDs. ### Special notes for your reviewer - `agent_mcp_server` mirrors `assistant_mcp_server` exactly (schema, naming, FK cascade); the `assistant` side was already covered by FK cascade and is unchanged. - `AgentService.emitAgentUpdatedForIds()` uses a batch `inArray` query (not N+1 `getAgent()` calls); events fire only after the write transaction commits, and the emission is wrapped so a post-commit refresh failure cannot misreport a successful delete. - `removeMcpFromAllAgentsTx()` captures affected agent IDs before deleting the junction rows so events can be emitted post-commit; the MCP-server delete's FK cascade is a redundant safety net. - v1→v2: `migrateAgentMcps()` runs while `agents_legacy` is attached and before `remapAgentPrefixIds()`, following the same `mcpServerIdMapping` remap + dangling-drop pattern as `AssistantMigrator`. ### Checklist - [x] Branch: This PR targets the correct branch — `main` for active development - [x] PR: The PR description is expressive enough and will help future contributors - [x] Code: Write code that humans can understand and Keep it simple - [x] Refactor: You have left the code cleaner than you found it (Boy Scout Rule) - [x] Upgrade: Impact of this change on upgrade flows was considered and addressed if required - [ ] Documentation: A user-guide update was considered and is present (link) or not required. Check this only when the PR introduces or changes a user-facing feature or behavior. - [x] Self-review: I have reviewed my own code (e.g., via gh-pr-review, gh pr diff, or GitHub UI) before requesting review from others ### Release note ```release-note Bug fix: Deleting a global MCP server now cascade-removes its references from all agents, preventing stale "Failed to connect to MCP" errors. ``` --------- Signed-off-by: suyao <sy20010504@gmail.com> Co-authored-by: SuYao <sy20010504@gmail.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: fullex <106392080+0xfullex@users.noreply.github.com>
11 lines
456 B
SQL
11 lines
456 B
SQL
CREATE TABLE `agent_mcp_server` (
|
|
`agent_id` text NOT NULL,
|
|
`mcp_server_id` text NOT NULL,
|
|
`created_at` integer NOT NULL,
|
|
`updated_at` integer NOT NULL,
|
|
PRIMARY KEY(`agent_id`, `mcp_server_id`),
|
|
FOREIGN KEY (`agent_id`) REFERENCES `agent`(`id`) ON UPDATE no action ON DELETE cascade,
|
|
FOREIGN KEY (`mcp_server_id`) REFERENCES `mcp_server`(`id`) ON UPDATE no action ON DELETE cascade
|
|
);
|
|
--> statement-breakpoint
|
|
ALTER TABLE `agent` DROP COLUMN `mcps`; |