From f5e83748ff20956bdb9cc18da50e5801599ab309 Mon Sep 17 00:00:00 2001 From: digua Date: Mon, 8 Jun 2026 19:42:41 +0800 Subject: [PATCH] fix(ai): register execute_sql tool in Electron tool registry sqlQueryTool (execute_sql) was listed in BUILTIN_TOOL_CATALOG and available in the CLI AGENT_TOOL_REGISTRY, but was never imported or registered in the Electron-specific TOOL_REGISTRY. Users who enabled the tool via the assistant config panel would trigger a missing-tool error at inference time. Add sqlQueryTool to the Electron registry following the same adaptSharedTool pattern used by all other analysis tools. Add a catalog/registry drift guard test (name + category) covering all BUILTIN_TOOL_CATALOG entries, enabled via --experimental-test-module-mocks to mock Electron runtime deps. Document the general-over-specific assertion rule in AGENTS.md. Co-Authored-By: Claude Sonnet 4.6 --- AGENTS.md | 2 +- .../main/ai/tools/definitions/index.test.ts | 35 +++++++++++++++++++ .../main/ai/tools/definitions/index.ts | 2 ++ scripts/run-tests.mjs | 2 +- scripts/run-tests.test.mjs | 1 + 5 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 apps/desktop/main/ai/tools/definitions/index.test.ts diff --git a/AGENTS.md b/AGENTS.md index 510757a5..4a823cab 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,7 +33,7 @@ - 必须加测试:修复 bug 时必须先加能失败的回归测试;修改数据库 schema/迁移、导入解析、去重逻辑、权限认证、AI 工具 allowlist、配置/API key 迁移、跨端共享 service 时必须加或更新测试 - 可以不加测试:只改文案、样式、类型声明、日志、注释、无行为变化的小重构时,可以不新增测试,但仍需运行相关类型检查、lint 和 format - 测试分层:纯函数、解析/格式化、权限判断、参数规范化、错误分支优先写单元测试;SQL 行为、数据库迁移、文件迁移、导入写库、Fastify route、跨包 service 优先写集成测试;Electron、真实浏览器、真实 LLM、真实网络仅用于少量关键 E2E/Smoke,并默认通过环境变量显式启用 -- 避免重复测试:新增测试前先搜索同类断言;如果下层单测已覆盖算法,上层只测调用链是否接通,不重复枚举算法细节;如果测试只锁定实现写法、失败后不指向用户可见风险,应合并、改写或删除 +- 避免重复测试:新增测试前先搜索同类断言;如果下层单测已覆盖算法,上层只测调用链是否接通,不重复枚举算法细节;如果测试只锁定实现写法、失败后不指向用户可见风险,应合并、改写或删除;优先写能覆盖全量场景的通用断言,而非针对单个条目的专项断言——若通用断言已完整覆盖,单个断言是冗余的 - 测试替身选择:涉及 SQL、数据库迁移、Fastify route 或跨包 service 时,优先使用轻量内存 SQLite / 临时文件 fixture 验证真实行为;只有在测试纯适配边界时才使用 mock/fake。避免用大量 `sql.includes(...)` 一类字符串匹配来模拟数据库行为 - 适配层测试边界:CLI/Electron/Web route、IPC adapter、tool adapter 只验证参数传递、权限过滤、错误映射和返回契约;core 已覆盖的算法矩阵不要在入口层重复展开 diff --git a/apps/desktop/main/ai/tools/definitions/index.test.ts b/apps/desktop/main/ai/tools/definitions/index.test.ts new file mode 100644 index 00000000..ae898446 --- /dev/null +++ b/apps/desktop/main/ai/tools/definitions/index.test.ts @@ -0,0 +1,35 @@ +import { mock, describe, it } from 'node:test' +import assert from 'node:assert/strict' +import { BUILTIN_TOOL_CATALOG } from '@openchatlab/core' + +type MinEntry = { name: string; category: string } + +describe('Electron TOOL_REGISTRY', () => { + it('covers all BUILTIN_TOOL_CATALOG entries with correct name and category', async () => { + const recorded: MinEntry[] = [] + + // Mock the Electron-specific adapter module before loading the registry. + // This prevents @openchatlab/node-runtime (pi-ai) and electron from being + // resolved in the test environment, which lacks those native bindings. + await mock.module('../shared-tool-adapter', { + namedExports: { + adaptSharedTool(tool: { name: string }, opts: { category: string }) { + const entry = { name: tool.name, category: opts.category, factory: () => ({}) } + recorded.push(entry) + return entry + }, + }, + }) + + // Dynamic import keeps the mock active before the module graph resolves. + const { TOOL_REGISTRY } = (await import('./index.js')) as { TOOL_REGISTRY: MinEntry[] } + + // Drift guard: every tool in BUILTIN_TOOL_CATALOG must appear in TOOL_REGISTRY + // with the correct name and category. + const registeredMap = new Map(TOOL_REGISTRY.map((e) => [e.name, e.category])) + const mismatches = BUILTIN_TOOL_CATALOG.filter((e) => registeredMap.get(e.name) !== e.category).map( + (e) => `${e.name} (expected category=${e.category}, got ${registeredMap.get(e.name) ?? 'missing'})` + ) + assert.deepEqual(mismatches, [], `catalog/registry drift in Electron TOOL_REGISTRY: ${mismatches.join(', ')}`) + }) +}) diff --git a/apps/desktop/main/ai/tools/definitions/index.ts b/apps/desktop/main/ai/tools/definitions/index.ts index 562faa5c..8718ac6d 100644 --- a/apps/desktop/main/ai/tools/definitions/index.ts +++ b/apps/desktop/main/ai/tools/definitions/index.ts @@ -17,6 +17,7 @@ import { getSegmentMessagesTool, getMembersTool, schemaTool, + sqlQueryTool, memberStatsTool, timeStatsTool, getMemberNameHistoryTool, @@ -61,6 +62,7 @@ export const TOOL_REGISTRY: ToolRegistryEntry[] = [ adaptSharedTool(responseTimeAnalysisTool, { category: 'analysis' }), adaptSharedTool(keywordFrequencyTool, { category: 'analysis' }), adaptSharedTool(renderChartTool, { category: 'analysis' }), + adaptSharedTool(sqlQueryTool, { category: 'analysis' }), // ==================== SQL 分析工具 ==================== ...sqlToolEntries, diff --git a/scripts/run-tests.mjs b/scripts/run-tests.mjs index b26eed13..c1cd9ad9 100644 --- a/scripts/run-tests.mjs +++ b/scripts/run-tests.mjs @@ -61,7 +61,7 @@ export function collectDefaultTestFiles(rootDir = process.cwd()) { } export function buildNodeTestArgs(testArgs) { - return ['--import', 'tsx', '--test', ...testArgs] + return ['--experimental-test-module-mocks', '--import', 'tsx', '--test', ...testArgs] } function run() { diff --git a/scripts/run-tests.test.mjs b/scripts/run-tests.test.mjs index 2847ab01..88abc72a 100644 --- a/scripts/run-tests.test.mjs +++ b/scripts/run-tests.test.mjs @@ -23,6 +23,7 @@ test('default test collection excludes e2e, smoke, and real external tests', () test('explicit test arguments are passed through without default exclusions', () => { assert.deepEqual(buildNodeTestArgs(['tests/e2e/helpers/app-launcher.test.js']), [ + '--experimental-test-module-mocks', '--import', 'tsx', '--test',