Enforce MCP tool name uniqueness per capset - #513
Conversation
|
PR Title: Enforce MCP tool name uniqueness per capset Commit: 本变更为 capset_methods 表增加 MCP 工具名在 capset 级（capset_id + \x1f + mcp_tool_name）的唯一性约束：新增迁移（mcp_tool_key 列、回填 UPDATE、部分唯一索引）、AddCapsetMethod 事务化并返回 ErrMCPToolNameConflict、admin 层映射 HTTP 409、以及新增单元测试。 总体设计与既有应用层预检查 MCPToolNameExists 语义一致，且把约束下沉到数据库层，能消除并发竞态，方向正确。核查确认 Open() 启动即执行 Migrate，且 PRAGMA foreign_keys=ON 可排除孤儿行场景。 提交了三条发现：
|
![monkeyscan[bot]](https://avatars.githubusercontent.com/u/231811742?s=60&v=4){kind=small}
|
PR Title: Enforce MCP tool name uniqueness per capset Commit: 本变更仅修改 internal/store/store.go：将 capset_methods.mcp_tool_key 的生成从“capset 级”（capset_instances.capset_id + 分隔符 + mcp_tool_name）改为“capset 实例级”（capset_instance_id + 分隔符 + mcp_tool_name）。改动涉及两处——Migrate 中的历史数据回填 SQL（新 119 行）和 AddCapsetMethod 中的 toolKey 构造（新 807 行），两者同步修改、方向一致。 评估：该修改把 DB 唯一索引 uq_capset_methods_mcp_tool_key 对 mcp_tool_name 的唯一性约束作用域从“同一 capset”放宽到“同一 capset 实例”。但 store.go 内 FindTool/MCPToolNameExists 仍以 capset 作用域解析工具名，且 findToolByName 对同 capset 内重复工具名返回 "ambiguous MCP tool name"。因此放宽后，同一 capset 的多个实例可各自写入同名 MCP 工具而不触发唯一冲突（旧代码会返回 ErrMCPToolNameConflict/409），导致运行期工具解析歧义。此外，对已用旧代码迁移的库，存量行 key 仍为 capset_id 前缀而新写入为 capset_instance_id 前缀，两套格式并存，放大不一致。主要发现聚焦于这一唯一性语义与下游查找逻辑的回归。 |
|
PR Title: Enforce MCP tool name uniqueness per capset Commit: 本变更将 MCP 工具名的 DB 级唯一性作用域从「capset 实例级」改回「capset 级」，修复上一版本实例级 key 导致的 FindTool 歧义问题，并用 SELECT 排查取代脆弱的 SQLite 错误文本匹配。Migrate 的回填 UPDATE 改为 join capset_instances 以 capset_id 生成 mcp_tool_key，并在建唯一索引前新增重复检测、命中时返回含指引的错误阻止启动；AddCapsetMethod 写前查询 capset_id 生成 capset 级 key，插入失败后通过二次 SELECT 区分主键冲突与工具名冲突。测试同步改为断言同 capset 跨实例同名工具被拒、跨 capset 允许。整体方向正确；主要残留问题：从上一版本（已带实例级唯一索引且可能存在跨实例同名数据）升级时，回填 UPDATE 会先撞上既有唯一索引而以原始 UNIQUE 错误失败，新增的友好冲突检测在该真实升级路径上不可达（新测试用 DROP INDEX 模拟恰好绕开），建议先 DROP INDEX 再回填检测重建，并补相应升级测试。 |
| if err := addColumnIfMissing(ctx, s.db, "capset_methods", "mcp_tool_key", "TEXT NOT NULL DEFAULT ''"); err != nil { | ||
| return err | ||
| } | ||
| if _, err := s.db.ExecContext(ctx, `UPDATE capset_methods SET mcp_tool_key = ( |
There was a problem hiding this comment.
迁移在“上一版本已建实例级唯一索引且存在跨实例同名工具”的库上会先在回填 UPDATE 处失败，新增友好冲突检测不可达
本变更的目的之一是在迁移阶段对同 capset 内跨实例重复 mcp_tool_name 的存量数据给出可操作的“remove or rename duplicates”指引（新增 127-133 行重复检测 + 新测试）。但上一版本（base）的迁移已创建实例级唯一索引 uq_capset_methods_mcp_tool_key，且其 key 前缀为 capset_instance_id，因此 base 代码允许同 capset 的不同实例写入同名 MCP 工具，即这类问题库必然同时带有该索引。升级时，新 Migrate 的回填 UPDATE（119-124 行）把两条记录都改写为 capset 级 key（capset_id + \x1f + name），在既有唯一索引仍在的情况下，第二条记录被更新成相同 key 时 UPDATE 语句即报 UNIQUE constraint failed 并整体回滚，返回原始 sqlite 错误，执行不到 127-133 行的友好重复检测。证据：新增测试 TestMigrateReportsExistingMCPToolConflicts 必须先执行 DROP INDEX uq_capset_methods_mcp_tool_key 才能让 Migrate 走到 "conflicting methods" 分支，恰好证明在真实升级路径（索引存在）下该分支不可达。因此历史问题“升级遇重复数据时服务无法启动且仅报原始 UNIQUE 错误、无清理指引”在主要升级场景下仍然残留，且新增的错误消息本身展示的是含 \x1f 控制符的内部 mcp_tool_key（如 %q 输出 capset_id\x1ftool_name），可操作性有限。
Problem code:
Changed code at internal/store/store.go:119
Recommendation:
在回填 UPDATE 之前先删除既有索引，使重复 key 能被 UPDATE 生成、再由新增的重复检测返回友好错误，随后再重建唯一索引，例如在 addColumnIfMissing 之后插入 DROP INDEX IF EXISTS uq_capset_methods_mcp_tool_key，并把 CREATE UNIQUE INDEX IF NOT EXISTS 保留在重复检测之后。同时补充一个“保留旧实例级索引且已存在跨实例重复工具数据”的升级迁移测试，验证得到的是含指引的 "conflicting methods" 错误而非原始 UNIQUE 错误；并考虑让错误消息直接展示 capset 与 mcp_tool_name 而非含 \x1f 的内部 key。
Suggested diff:
if err := addColumnIfMissing(ctx, s.db, "capset_methods", "mcp_tool_key", "TEXT NOT NULL DEFAULT ''"); err != nil {
return err
}
+ if _, err := s.db.ExecContext(ctx, `DROP INDEX IF EXISTS uq_capset_methods_mcp_tool_key`); err != nil {
+ return err
+ }
if _, err := s.db.ExecContext(ctx, `UPDATE capset_methods SET mcp_tool_key = (
SELECT ci.capset_id || char(31) || capset_methods.mcp_tool_name
FROM capset_instances ci
WHERE ci.id = capset_methods.capset_instance_id
)
WHERE mcp_tool_name <> ''`); err != nil {
return err
}
|
Follow-up fixes after CI/code review:
Verification: |
1 participant
问题
同一 Capset Instance 内，MCP Tool 名称先查询再插入，检查和写入不是原子操作；数据库没有最终唯一约束。并发请求可能为同一实例写入相同名称。
影响
同一实例内出现重复名称会导致工具列表重复，调用路由不确定。不同实例在同一 Capset 中使用相同名称仍保留为明确的歧义状态，由现有路由逻辑返回歧义错误，不静默选择实例。
修复内容
capset_methods增加按capset_instance_id + mcp_tool_name计算的唯一 key。AddCapsetMethod在插入时生成 key，并将唯一冲突转换为明确错误。409 Conflict。验证
go test ./internal/store通过。