diff --git a/apps/cli/src/ai/agent-stream-runner.test.ts b/apps/cli/src/ai/agent-stream-runner.test.ts index 3403b367..6958191e 100644 --- a/apps/cli/src/ai/agent-stream-runner.test.ts +++ b/apps/cli/src/ai/agent-stream-runner.test.ts @@ -1,7 +1,7 @@ import { describe, it } from 'node:test' import assert from 'node:assert/strict' import { getChartCapabilityAllowedBuiltinTools } from '@openchatlab/node-runtime' -import { getAvailableToolDefs } from './agent-stream-runner' +import { getAllowedToolSet, getAvailableToolDefs } from './agent-stream-runner' describe('CLI chart capability tool filtering', () => { it('does not expose uncategorized raw SQL in chart-only turns', () => { @@ -39,4 +39,17 @@ describe('CLI chart capability tool filtering', () => { assert.ok(toolNames.includes('keyword_frequency')) assert.ok(!toolNames.includes('execute_sql')) }) + + it('preserves an empty assistant allowlist instead of treating it as unrestricted', () => { + const allowedToolSet = getAllowedToolSet(false, []) + + assert.ok(allowedToolSet instanceof Set) + assert.equal(allowedToolSet.size, 0) + + const toolNames = getAvailableToolDefs(false, allowedToolSet).map((tool) => tool.name) + + assert.ok(toolNames.includes('get_schema')) + assert.ok(!toolNames.includes('keyword_frequency')) + assert.ok(!toolNames.includes('execute_sql')) + }) }) diff --git a/apps/cli/src/ai/agent-stream-runner.ts b/apps/cli/src/ai/agent-stream-runner.ts index 9fc77842..431ac7d8 100644 --- a/apps/cli/src/ai/agent-stream-runner.ts +++ b/apps/cli/src/ai/agent-stream-runner.ts @@ -48,6 +48,17 @@ export function getAvailableToolDefs(isChartCapability: boolean, allowedToolSet: : AGENT_TOOL_REGISTRY } +// 区分“未配置白名单”和“配置为空白名单”:前者无限制,后者禁用 analysis 工具。 +export function getAllowedToolSet( + isChartCapability: boolean, + allowedBuiltinTools?: readonly string[] +): Set | null { + if (isChartCapability) { + return new Set(allowedBuiltinTools) + } + return allowedBuiltinTools === undefined ? null : new Set(allowedBuiltinTools) +} + export function createCliRunAgentStream( dbManager: DatabaseManager, convManager: AIConversationManager @@ -98,11 +109,10 @@ export function createCliRunAgentStream( !skillId && enableAutoSkill ? getAllowedBuiltinToolsForChartAutoSkill(assistantAllowedTools) : assistantAllowedTools - const allowedToolSet = isChartCapability - ? new Set(chartRuntime.allowedBuiltinTools) - : autoSkillAllowedTools && autoSkillAllowedTools.length > 0 - ? new Set(autoSkillAllowedTools) - : null + const allowedToolSet = getAllowedToolSet( + isChartCapability, + isChartCapability ? chartRuntime.allowedBuiltinTools : autoSkillAllowedTools + ) const availableToolDefs = getAvailableToolDefs(isChartCapability, allowedToolSet) const agentTools = db @@ -122,7 +132,11 @@ export function createCliRunAgentStream( const def = skillMgr.getSkillConfig(skillId) if (def) resolvedSkillDef = { name: def.name, prompt: def.prompt } } else if (enableAutoSkill) { - const menu = buildSkillMenuWithBuiltinChart(skillMgr.getSkillMenu(chatType ?? 'group', toolNames), locale) + const menu = buildSkillMenuWithBuiltinChart( + skillMgr.getSkillMenu(chatType ?? 'group', toolNames), + locale, + toolNames + ) if (menu) resolvedSkillMenu = menu } diff --git a/apps/desktop/main/ai/agent-stream-runner.ts b/apps/desktop/main/ai/agent-stream-runner.ts index e3a70b58..d4f5d0c1 100644 --- a/apps/desktop/main/ai/agent-stream-runner.ts +++ b/apps/desktop/main/ai/agent-stream-runner.ts @@ -10,6 +10,7 @@ import type { AgentStreamChunk as SharedAgentStreamChunk } from '@openchatlab/no import type { AgentStreamRequest } from '@openchatlab/http-routes' import { buildSkillMenuWithBuiltinChart, + CHART_CAPABILITY_ANALYSIS_TOOLS, checkAndCompress, createCompressionLlmAdapter, formatAIError, @@ -181,7 +182,9 @@ export function createElectronRunAgentStream(): ( } } else if (enableAutoSkill) { const effectiveChatType = chatType ?? 'group' - const autoSkillAllowedTools = getAllowedBuiltinToolsForChartAutoSkill(assistantConfig?.allowedBuiltinTools) + const autoSkillAllowedTools = getAllowedBuiltinToolsForChartAutoSkill(assistantConfig?.allowedBuiltinTools) ?? [ + ...CHART_CAPABILITY_ANALYSIS_TOOLS, + ] assistantConfig = { ...(assistantConfig ?? { id: resolvedAssistantId, @@ -192,7 +195,11 @@ export function createElectronRunAgentStream(): ( allowedBuiltinTools: autoSkillAllowedTools, } const allowedTools = autoSkillAllowedTools - const menu = buildSkillMenuWithBuiltinChart(skillManager.getSkillMenu(effectiveChatType, allowedTools), locale) + const menu = buildSkillMenuWithBuiltinChart( + skillManager.getSkillMenu(effectiveChatType, allowedTools), + locale, + allowedTools + ) if (menu) { skillCtx = { skillMenu: menu } } diff --git a/packages/node-runtime/src/ai/__tests__/activate-skill-tool.test.ts b/packages/node-runtime/src/ai/__tests__/activate-skill-tool.test.ts new file mode 100644 index 00000000..d8007d9a --- /dev/null +++ b/packages/node-runtime/src/ai/__tests__/activate-skill-tool.test.ts @@ -0,0 +1,42 @@ +import { describe, it } from 'node:test' +import assert from 'node:assert/strict' +import { createActivateSkillTool } from '../activate-skill-tool' +import type { SkillDef } from '../types' + +const TOOL_SKILL: SkillDef = { + id: 'tool_skill', + name: 'Tool Skill', + description: 'Requires an analysis tool', + tags: ['test'], + chatScope: 'all', + tools: ['keyword_frequency'], + prompt: 'Use keyword frequency.', +} + +describe('createActivateSkillTool', () => { + it('rejects a skill when an empty allowedTools list omits required tools', async () => { + const tool = createActivateSkillTool({ + chatType: 'group', + allowedTools: [], + getSkillConfig: () => TOOL_SKILL, + }) + + const result = await tool.execute('call_1', { skill_id: 'tool_skill' }) + + assert.equal(result.details.applicable, false) + assert.deepEqual(result.details.missingTools, ['keyword_frequency']) + }) + + it('allows core tool requirements even when no analysis tools are allowed', async () => { + const tool = createActivateSkillTool({ + chatType: 'group', + allowedTools: [], + coreToolNames: new Set(['get_schema']), + getSkillConfig: () => ({ ...TOOL_SKILL, tools: ['get_schema'] }), + }) + + const result = await tool.execute('call_1', { skill_id: 'tool_skill' }) + + assert.equal(result.details.applicable, true) + }) +}) diff --git a/packages/node-runtime/src/ai/__tests__/builtin-chart-skill.test.ts b/packages/node-runtime/src/ai/__tests__/builtin-chart-skill.test.ts index a93cf610..a7656cbc 100644 --- a/packages/node-runtime/src/ai/__tests__/builtin-chart-skill.test.ts +++ b/packages/node-runtime/src/ai/__tests__/builtin-chart-skill.test.ts @@ -5,9 +5,14 @@ import { CHART_CAPABILITY_SKILL_ID } from '@openchatlab/core' import { buildSkillMenuWithBuiltinChart, getSkillConfigWithBuiltinChart } from '../builtin-chart-skill' import { buildSkillMenuText, formatSkillMenuLine } from '../skill-menu' +function requireMenu(menu: string | null): string { + assert.ok(menu) + return menu +} + describe('builtin chart skill helpers', () => { it('adds chart_runtime to an empty auto skill menu', () => { - const menu = buildSkillMenuWithBuiltinChart(null, 'zh-CN') + const menu = requireMenu(buildSkillMenuWithBuiltinChart(null, 'zh-CN')) assert.match(menu, /chart_runtime/) assert.match(menu, /绘图助手/) @@ -23,13 +28,25 @@ describe('builtin chart skill helpers', () => { }), ]) - const menu = buildSkillMenuWithBuiltinChart(baseMenu, 'zh-CN') + const menu = requireMenu(buildSkillMenuWithBuiltinChart(baseMenu, 'zh-CN')) assert.match(menu, /existing/) assert.match(menu, /chart_runtime/) assert.ok(menu.indexOf('existing') < menu.indexOf('chart_runtime')) }) + it('does not add chart_runtime when render_chart is unavailable', () => { + const menu = buildSkillMenuWithBuiltinChart(null, 'zh-CN', []) + + assert.equal(menu, null) + }) + + it('adds chart_runtime when render_chart is explicitly available', () => { + const menu = requireMenu(buildSkillMenuWithBuiltinChart(null, 'zh-CN', ['render_chart'])) + + assert.match(menu, /chart_runtime/) + }) + it('resolves chart_runtime without a user-imported skill file', () => { const skill = getSkillConfigWithBuiltinChart(CHART_CAPABILITY_SKILL_ID, 'en-US', () => null) diff --git a/packages/node-runtime/src/ai/__tests__/skill-manager-core.test.ts b/packages/node-runtime/src/ai/__tests__/skill-manager-core.test.ts index 7424da54..096cac8c 100644 --- a/packages/node-runtime/src/ai/__tests__/skill-manager-core.test.ts +++ b/packages/node-runtime/src/ai/__tests__/skill-manager-core.test.ts @@ -36,6 +36,18 @@ tools: [] --- You are a test skill.` +const TOOL_SKILL = `--- +id: tool_skill +name: Tool Skill +description: A skill that requires an analysis tool +tags: + - test +chatScope: all +tools: + - keyword_frequency +--- +Use keyword frequency.` + function createManager(builtins?: Array<{ id: string; content: string }>) { const memFs = createMemoryFs() const deps: SkillManagerCoreDeps = { @@ -134,6 +146,14 @@ describe('SkillManagerCore', () => { assert.ok(manager.getSkillMenu('group')) assert.equal(manager.getSkillMenu('private'), null) }) + + it('getSkillMenu treats an empty allowedTools list as no analysis tools allowed', () => { + manager.init() + manager.createSkill(TOOL_SKILL) + + assert.equal(manager.getSkillMenu('group', []), null) + assert.match(manager.getSkillMenu('group', ['keyword_frequency']) ?? '', /tool_skill/) + }) }) describe('SkillManagerCore with builtins', () => { diff --git a/packages/node-runtime/src/ai/activate-skill-tool.ts b/packages/node-runtime/src/ai/activate-skill-tool.ts index d029c76d..4b7aac88 100644 --- a/packages/node-runtime/src/ai/activate-skill-tool.ts +++ b/packages/node-runtime/src/ai/activate-skill-tool.ts @@ -74,7 +74,7 @@ export function createActivateSkillTool(options: ActivateSkillToolOptions): Acti } } - if (skill.tools.length > 0 && allowedTools && allowedTools.length > 0) { + if (skill.tools.length > 0 && allowedTools !== undefined) { const missing = skill.tools.filter((t) => !(coreToolNames?.has(t) ?? false) && !allowedTools.includes(t)) if (missing.length > 0) { const msg = isZh diff --git a/packages/node-runtime/src/ai/chart-runtime.ts b/packages/node-runtime/src/ai/chart-runtime.ts index fc4afc8a..ccd4ed2c 100644 --- a/packages/node-runtime/src/ai/chart-runtime.ts +++ b/packages/node-runtime/src/ai/chart-runtime.ts @@ -90,7 +90,12 @@ function getChartMenuLine(locale: string): string { }) } -export function buildSkillMenuWithBuiltinChart(baseMenu: string | null | undefined, locale: string = 'zh-CN'): string { +export function buildSkillMenuWithBuiltinChart( + baseMenu: string | null | undefined, + locale: string = 'zh-CN', + allowedTools?: readonly string[] | null +): string | null { + if (allowedTools && !allowedTools.includes(CHART_CAPABILITY_ANALYSIS_TOOLS[0])) return baseMenu ?? null if (baseMenu?.includes(CHART_CAPABILITY_SKILL_ID)) return baseMenu const chartLine = getChartMenuLine(locale) diff --git a/packages/node-runtime/src/ai/skill-manager-core.ts b/packages/node-runtime/src/ai/skill-manager-core.ts index db60e938..753f82a1 100644 --- a/packages/node-runtime/src/ai/skill-manager-core.ts +++ b/packages/node-runtime/src/ai/skill-manager-core.ts @@ -291,7 +291,7 @@ export class SkillManagerCore { const compatible = Array.from(this.cache.values()).filter((skill) => { if (skill.chatScope !== 'all' && skill.chatScope !== chatType) return false - if (skill.tools.length > 0 && allowedTools && allowedTools.length > 0) { + if (skill.tools.length > 0 && allowedTools !== undefined) { if (!skill.tools.every((t) => allowedTools.includes(t))) return false } return true diff --git a/tests/e2e/helpers/app-launcher.js b/tests/e2e/helpers/app-launcher.js index d8748757..0ad8d3c0 100644 --- a/tests/e2e/helpers/app-launcher.js +++ b/tests/e2e/helpers/app-launcher.js @@ -11,7 +11,7 @@ const path = require('node:path') const fs = require('node:fs') const os = require('node:os') const net = require('node:net') -const electron = require('electron') +const { createRequire } = require('node:module') const DEFAULT_START_PORT = 9222 const DEFAULT_MAX_PORT_RETRIES = 100 @@ -127,6 +127,20 @@ function releaseReservation(reservationServer) { safeCloseServer(reservationServer) } +function resolveElectronExecutable(deps = {}) { + if (deps.electronPath) return deps.electronPath + + const desktopPackageJson = path.resolve(__dirname, '../../../apps/desktop/package.json') + const requireElectron = deps.requireElectron || createRequire(desktopPackageJson) + + try { + const electronModule = requireElectron('electron') + return typeof electronModule === 'string' ? electronModule : '' + } catch (error) { + throw new Error(`[AppLauncher] 无法解析 Electron 可执行文件: ${error.message}`) + } +} + /** * 启动 Electron 应用 */ @@ -172,7 +186,7 @@ async function launchApp(options = {}, deps = {}) { throw new Error(`[AppLauncher] 应用目录不存在: ${appPath}`) } - const electronExe = typeof electron === 'string' ? electron : '' + const electronExe = resolveElectronExecutable(deps) if (!electronExe || !fsImpl.existsSync(electronExe)) { throw new Error(`Electron 可执行文件不存在: ${electronExe}`) @@ -261,5 +275,6 @@ module.exports = { findAvailablePortWithReservation, terminateProcess, releaseReservation, + resolveElectronExecutable, }, } diff --git a/tests/e2e/helpers/app-launcher.test.js b/tests/e2e/helpers/app-launcher.test.js index 5bd83466..c67c387e 100644 --- a/tests/e2e/helpers/app-launcher.test.js +++ b/tests/e2e/helpers/app-launcher.test.js @@ -74,6 +74,7 @@ test('findAvailablePortWithReservation 达到最大重试会抛错', async () => }) test('launchApp 支持 startPort 并正确注入 TEST_MODE 环境', async () => { + const fakeElectronPath = '/tmp/fake-electron' const captured = { startPort: null, spawnCmd: null, @@ -108,6 +109,7 @@ test('launchApp 支持 startPort 并正确注入 TEST_MODE 环境', async () => mkdirSync: () => {}, }, sleepFn: async () => {}, + electronPath: fakeElectronPath, findPortFn: async (startPort) => { captured.startPort = startPort return { @@ -130,7 +132,7 @@ test('launchApp 支持 startPort 并正确注入 TEST_MODE 环境', async () => assert.equal(captured.startPort, 9900) assert.equal(captured.reservationClosed, true) - assert.match(captured.spawnCmd, /electron(\.cmd|\.exe)?$/) + assert.equal(captured.spawnCmd, fakeElectronPath) assert.deepEqual(captured.spawnArgs, ['--remote-debugging-port=9901', captured.spawnArgs[1]]) assert.equal(captured.spawnEnv.TEST_MODE, 'true') assert.match(captured.spawnEnv.CHATLAB_E2E_USER_DATA_DIR, /chatlab-e2e-9901$/) diff --git a/tests/e2e/smoke/chart-runtime.smoke.test.js b/tests/e2e/smoke/chart-runtime.smoke.test.js index 5efd5ece..98e957ce 100644 --- a/tests/e2e/smoke/chart-runtime.smoke.test.js +++ b/tests/e2e/smoke/chart-runtime.smoke.test.js @@ -6,7 +6,7 @@ const fs = require('node:fs') const path = require('node:path') const os = require('node:os') const http = require('node:http') -const Database = require('better-sqlite3') +const { createRequire } = require('node:module') const { launchApp } = require('../helpers/app-launcher') @@ -114,6 +114,15 @@ const CONVERSATION_ID = 'conv_chart_smoke' const CONVERSATION_TITLE = 'Chart replay smoke' const CHART_TITLE = 'Selected members smoke' +let Database = null + +function getDatabaseConstructor() { + if (Database) return Database + const nodeRuntimePackageJson = path.resolve(__dirname, '../../../packages/node-runtime/package.json') + Database = createRequire(nodeRuntimePackageJson)('better-sqlite3') + return Database +} + function resolveNativeBinding() { if (process.env.CHATLAB_TEST_SQLITE_NATIVE_BINDING) { return process.env.CHATLAB_TEST_SQLITE_NATIVE_BINDING @@ -135,6 +144,7 @@ function ensureDir(dir) { } function seedChatSessionDb(chatDataDir) { + const Database = getDatabaseConstructor() const dbDir = path.join(chatDataDir, 'databases') ensureDir(dbDir) const db = new Database(path.join(dbDir, `${SESSION_ID}.db`), { @@ -183,6 +193,7 @@ function seedChatSessionDb(chatDataDir) { } function seedAiConversationDb(homeDir) { + const Database = getDatabaseConstructor() const aiDir = path.join(homeDir, '.chatlab', 'ai') ensureDir(aiDir) const db = new Database(path.join(aiDir, 'conversations.db'), {