From 996cea9ea11e67985d2cc99a5aed2b3daa08c43a Mon Sep 17 00:00:00 2001 From: zerob13 Date: Sat, 11 Apr 2026 18:17:58 +0800 Subject: [PATCH 1/3] feat(skills): activate tools after skill_view --- resources/skills/deepchat-settings/SKILL.md | 15 +- .../agentRuntimePresenter/dispatch.ts | 22 ++- .../presenter/agentRuntimePresenter/index.ts | 4 + .../agentRuntimePresenter/process.ts | 13 +- .../presenter/agentRuntimePresenter/types.ts | 1 + src/main/presenter/skillPresenter/index.ts | 8 +- .../agentTools/agentToolManager.ts | 35 +++- .../components/chat-input/McpIndicator.vue | 37 ++++- .../agentRuntimePresenter/dispatch.test.ts | 52 ++++++ .../agentRuntimePresenter/process.test.ts | 157 ++++++++++++++++++ .../skillPresenter/skillPresenter.test.ts | 76 +++++++++ .../agentToolManagerSettings.test.ts | 58 +++++++ test/renderer/components/McpIndicator.test.ts | 45 ++++- 13 files changed, 508 insertions(+), 15 deletions(-) diff --git a/resources/skills/deepchat-settings/SKILL.md b/resources/skills/deepchat-settings/SKILL.md index 236eb522a..3fc5657de 100644 --- a/resources/skills/deepchat-settings/SKILL.md +++ b/resources/skills/deepchat-settings/SKILL.md @@ -17,10 +17,11 @@ Use this skill to safely change DeepChat *application* settings during a convers - Only change settings when the user is asking to change **DeepChat** settings. - Use the dedicated settings tools; never attempt arbitrary key/value writes. -- These tools are intended to be available only when this skill is active; if they are missing, activate this skill via `skill_control`. +- These tools are intended to be available only when this skill is active. +- Viewing the main `deepchat-settings` `SKILL.md` activates this skill for the current conversation and exposes the `deepchat_settings_*` tools in the next tool loop iteration. +- Viewing linked files under this skill does **not** activate the skill. - If the request is ambiguous, ask a clarifying question before applying. - For unsupported or high-risk settings (MCP, prompts, providers, API keys, paths): do **not** apply changes; instead explain where to change it and open Settings. -- After completing the settings task, deactivate this skill via `skill_control` to keep context small. ## Supported settings (initial allowlist) @@ -43,15 +44,15 @@ Settings navigation (open-only): ## Workflow 1. Confirm the user is requesting a DeepChat settings change. -2. Determine the target setting and the intended value. -3. If the setting is supported, call the matching tool: +2. If the settings tools are not yet present, inspect the main `deepchat-settings` skill document first so the skill becomes active for this conversation. +3. Determine the target setting and the intended value. +4. If the setting is supported, call the matching tool: - toggles: `deepchat_settings_toggle` - language: `deepchat_settings_set_language` - theme: `deepchat_settings_set_theme` - font size: `deepchat_settings_set_font_size` -4. Confirm back to the user what changed (include the final value). -5. If the setting is unsupported, call `deepchat_settings_open` (with `section`) and provide a short pointer to the correct Settings section. Do not call it if the requested change has already been applied. -6. Deactivate this skill via `skill_control`. +5. Confirm back to the user what changed (include the final value). +6. If the setting is unsupported, call `deepchat_settings_open` (with `section`) and provide a short pointer to the correct Settings section. Do not call it if the requested change has already been applied. ## Examples (activate this skill) diff --git a/src/main/presenter/agentRuntimePresenter/dispatch.ts b/src/main/presenter/agentRuntimePresenter/dispatch.ts index 722b2e98a..5644bf564 100644 --- a/src/main/presenter/agentRuntimePresenter/dispatch.ts +++ b/src/main/presenter/agentRuntimePresenter/dispatch.ts @@ -324,6 +324,19 @@ function extractSubagentToolState(rawData: MCPToolResponse): { } } +function shouldRefreshToolsAfterCall(toolName: string, rawData: MCPToolResponse): boolean { + if (toolName !== 'skill_view') { + return false + } + + const toolResult = + rawData.toolResult && typeof rawData.toolResult === 'object' + ? (rawData.toolResult as Record) + : null + + return toolResult?.activationApplied === true +} + function persistToolExecutionState(io: IoParams, state: StreamState): void { if (!state.dirty) { return @@ -615,6 +628,7 @@ export async function executeTools( ): Promise<{ executed: number pendingInteractions: PendingToolInteraction[] + toolsChanged: boolean terminalError?: string }> { finalizePendingNarrativeBeforeToolExecution(state) @@ -673,6 +687,7 @@ export async function executeTools( conversation.push(assistantMessage) let executed = 0 + let toolsChanged = false const pendingInteractions: PendingToolInteraction[] = [] const stagedResults: StagedToolResult[] = [] @@ -851,6 +866,10 @@ export async function executeTools( } } + if (shouldRefreshToolsAfterCall(tc.name, toolRawData)) { + toolsChanged = true + } + const searchPayload = extractSearchPayload( toolRawData.content, toolContext.name, @@ -927,13 +946,14 @@ export async function executeTools( return { executed, pendingInteractions, + toolsChanged, terminalError: fittedResults.message } } } persistToolExecutionState(io, state) - return { executed, pendingInteractions } + return { executed, pendingInteractions, toolsChanged } } export function finalizePaused(state: StreamState, io: IoParams): void { diff --git a/src/main/presenter/agentRuntimePresenter/index.ts b/src/main/presenter/agentRuntimePresenter/index.ts index 316ed0041..6445281e7 100644 --- a/src/main/presenter/agentRuntimePresenter/index.ts +++ b/src/main/presenter/agentRuntimePresenter/index.ts @@ -1509,6 +1509,7 @@ export class AgentRuntimePresenter implements IAgentImplementation { const result = await processStream({ messages, tools, + refreshTools: async () => await this.loadToolDefinitionsForSession(sessionId, projectDir), toolPresenter: this.toolPresenter, coreStream: async function* ( requestMessages, @@ -2263,6 +2264,9 @@ export class AgentRuntimePresenter implements IAgentImplementation { lines.push( 'Before replying, always scan available skills. If any skill plausibly matches the task, call `skill_view` first.' ) + lines.push( + 'Viewing a skill root `SKILL.md` pins it to the current conversation; viewing linked skill files is read-only and does not pin the skill.' + ) hasContent = true } if (capabilities.canRunSkillScripts) { diff --git a/src/main/presenter/agentRuntimePresenter/process.ts b/src/main/presenter/agentRuntimePresenter/process.ts index c075da570..06d97f9ec 100644 --- a/src/main/presenter/agentRuntimePresenter/process.ts +++ b/src/main/presenter/agentRuntimePresenter/process.ts @@ -115,6 +115,7 @@ export async function processStream(params: ProcessParams): Promise Promise toolPresenter: IToolPresenter | null coreStream: ( messages: ChatMessage[], diff --git a/src/main/presenter/skillPresenter/index.ts b/src/main/presenter/skillPresenter/index.ts index 335af9bf7..9bcb9e2fa 100644 --- a/src/main/presenter/skillPresenter/index.ts +++ b/src/main/presenter/skillPresenter/index.ts @@ -569,6 +569,12 @@ export class SkillPresenter implements ISkillPresenter { const rawContent = fs.readFileSync(metadata.path, 'utf-8') const { content } = matter(rawContent) + let nextIsPinned = isPinned + + if (options?.conversationId && !isPinned) { + await this.setActiveSkills(options.conversationId, [...pinnedSkills, metadata.name]) + nextIsPinned = true + } return { success: true, @@ -580,7 +586,7 @@ export class SkillPresenter implements ISkillPresenter { platforms: metadata.platforms, metadata: metadata.metadata, linkedFiles: this.listSkillLinkedFiles(metadata.skillRoot), - isPinned + isPinned: nextIsPinned } } catch (error) { const errorMessage = error instanceof Error ? error.message : String(error) diff --git a/src/main/presenter/toolPresenter/agentTools/agentToolManager.ts b/src/main/presenter/toolPresenter/agentTools/agentToolManager.ts index f14ef4058..a3e0fda4d 100644 --- a/src/main/presenter/toolPresenter/agentTools/agentToolManager.ts +++ b/src/main/presenter/toolPresenter/agentTools/agentToolManager.ts @@ -1768,8 +1768,41 @@ export class AgentToolManager { if (!validationResult.success) { throw new Error(`Invalid arguments for skill_view: ${validationResult.error.message}`) } + const isLinkedFileView = typeof validationResult.data.file_path === 'string' + const wasActive = + conversationId && !isLinkedFileView + ? (await this.getSkillPresenter().getActiveSkills(conversationId)).includes( + validationResult.data.name + ) + : false const result = await skillTools.handleSkillView(conversationId, validationResult.data) - return { content: JSON.stringify(result) } + const activationApplied = + Boolean(conversationId) && + result.success === true && + !isLinkedFileView && + !wasActive && + result.isPinned === true + const activationSource = + !conversationId || result.success !== true + ? 'none' + : activationApplied + ? 'skill_md' + : isLinkedFileView + ? 'file' + : 'none' + const content = JSON.stringify(result) + + return { + content, + rawData: { + content, + toolResult: { + activationApplied, + activationSource, + ...(activationApplied ? { activatedSkill: validationResult.data.name } : {}) + } + } + } } if (toolName === 'skill_manage') { diff --git a/src/renderer/src/components/chat-input/McpIndicator.vue b/src/renderer/src/components/chat-input/McpIndicator.vue index 55881a1f2..fc4e40d36 100644 --- a/src/renderer/src/components/chat-input/McpIndicator.vue +++ b/src/renderer/src/components/chat-input/McpIndicator.vue @@ -205,7 +205,7 @@ diff --git a/test/main/presenter/agentRuntimePresenter/dispatch.test.ts b/test/main/presenter/agentRuntimePresenter/dispatch.test.ts index e3ed0a2af..d4d76d1f7 100644 --- a/test/main/presenter/agentRuntimePresenter/dispatch.test.ts +++ b/test/main/presenter/agentRuntimePresenter/dispatch.test.ts @@ -483,6 +483,58 @@ describe('dispatch', () => { expect(state.blocks[0].tool_call!.server_description).toBe('Test server') }) + it('flags toolsChanged when skill_view activates a skill via main SKILL.md', async () => { + const tools = [makeTool('skill_view')] + const toolPresenter = { + ...createMockToolPresenter(), + callTool: vi.fn().mockResolvedValue({ + content: '{"success":true,"name":"deepchat-settings","isPinned":true}', + rawData: { + toolCallId: 'tc1', + content: '{"success":true,"name":"deepchat-settings","isPinned":true}', + isError: false, + toolResult: { + activationApplied: true, + activationSource: 'skill_md', + activatedSkill: 'deepchat-settings' + } + } + }) + } as unknown as IToolPresenter + + state.blocks.push({ + type: 'tool_call', + content: '', + status: 'pending', + timestamp: Date.now(), + tool_call: { + id: 'tc1', + name: 'skill_view', + params: '{"name":"deepchat-settings"}', + response: '' + } + }) + state.completedToolCalls = [ + { id: 'tc1', name: 'skill_view', arguments: '{"name":"deepchat-settings"}' } + ] + + const result = await executeTools( + state, + [], + 0, + tools, + toolPresenter, + 'gpt-4', + io, + 'full_access', + new ToolOutputGuard(), + 32000, + 1024 + ) + + expect(result.toolsChanged).toBe(true) + }) + it('includes reasoning_content when interleaved compatibility is enabled', async () => { const tools = [makeTool('search')] const toolPresenter = createMockToolPresenter({ search: 'result' }) diff --git a/test/main/presenter/agentRuntimePresenter/process.test.ts b/test/main/presenter/agentRuntimePresenter/process.test.ts index 381f8c6a9..8759b174c 100644 --- a/test/main/presenter/agentRuntimePresenter/process.test.ts +++ b/test/main/presenter/agentRuntimePresenter/process.test.ts @@ -242,6 +242,163 @@ describe('processStream', () => { expect(toolResultMsg.content).toBe('Sunny, 72F') }) + it('refreshes tools for the next loop iteration after skill_view activates a skill', async () => { + let callCount = 0 + const toolPresenter = { + ...createMockToolPresenter(), + callTool: vi + .fn() + .mockResolvedValueOnce({ + content: '{"success":true,"name":"deepchat-settings","isPinned":true}', + rawData: { + toolCallId: 'tc1', + content: '{"success":true,"name":"deepchat-settings","isPinned":true}', + isError: false, + toolResult: { + activationApplied: true, + activationSource: 'skill_md', + activatedSkill: 'deepchat-settings' + } + } + }) + .mockResolvedValueOnce({ + content: '{"ok":true}', + rawData: { + toolCallId: 'tc2', + content: '{"ok":true}', + isError: false + } + }) + } as unknown as IToolPresenter + const refreshTools = vi + .fn() + .mockResolvedValue([makeTool('skill_view'), makeTool('deepchat_settings_set_theme')]) + + const coreStream = vi.fn( + function (_messages, _modelId, _modelConfig, _temperature, _maxTokens, tools) { + callCount++ + if (callCount === 1) { + expect(tools.map((tool) => tool.function.name)).toEqual(['skill_view']) + return (async function* () { + yield { + type: 'tool_call_start', + tool_call_id: 'tc1', + tool_call_name: 'skill_view' + } as LLMCoreStreamEvent + yield { + type: 'tool_call_end', + tool_call_id: 'tc1', + tool_call_arguments_complete: '{"name":"deepchat-settings"}' + } as LLMCoreStreamEvent + yield { type: 'stop', stop_reason: 'tool_use' } as LLMCoreStreamEvent + })() + } + if (callCount === 2) { + expect(tools.map((tool) => tool.function.name)).toEqual([ + 'skill_view', + 'deepchat_settings_set_theme' + ]) + return (async function* () { + yield { + type: 'tool_call_start', + tool_call_id: 'tc2', + tool_call_name: 'deepchat_settings_set_theme' + } as LLMCoreStreamEvent + yield { + type: 'tool_call_end', + tool_call_id: 'tc2', + tool_call_arguments_complete: '{"theme":"dark"}' + } as LLMCoreStreamEvent + yield { type: 'stop', stop_reason: 'tool_use' } as LLMCoreStreamEvent + })() + } + return (async function* () { + yield { type: 'text', content: 'Done' } as LLMCoreStreamEvent + yield { type: 'stop', stop_reason: 'complete' } as LLMCoreStreamEvent + })() + } + ) as unknown as ProcessParams['coreStream'] + + const params = createParams({ + coreStream, + toolPresenter, + tools: [makeTool('skill_view')], + refreshTools + }) + + const promise = processStream(params) + await vi.runAllTimersAsync() + await promise + + expect(refreshTools).toHaveBeenCalledTimes(1) + expect(coreStream).toHaveBeenCalledTimes(3) + expect(toolPresenter.callTool).toHaveBeenCalledTimes(2) + }) + + it('does not refresh tools after linked-file skill_view reads', async () => { + let callCount = 0 + const toolPresenter = { + ...createMockToolPresenter(), + callTool: vi.fn().mockResolvedValue({ + content: + '{"success":true,"name":"deepchat-settings","filePath":"references/guide.md","isPinned":false}', + rawData: { + toolCallId: 'tc1', + content: + '{"success":true,"name":"deepchat-settings","filePath":"references/guide.md","isPinned":false}', + isError: false, + toolResult: { + activationApplied: false, + activationSource: 'file' + } + } + }) + } as unknown as IToolPresenter + const refreshTools = vi.fn().mockResolvedValue([makeTool('deepchat_settings_set_theme')]) + + const coreStream = vi.fn( + function (_messages, _modelId, _modelConfig, _temperature, _maxTokens, tools) { + callCount++ + if (callCount === 1) { + expect(tools.map((tool) => tool.function.name)).toEqual(['skill_view']) + return (async function* () { + yield { + type: 'tool_call_start', + tool_call_id: 'tc1', + tool_call_name: 'skill_view' + } as LLMCoreStreamEvent + yield { + type: 'tool_call_end', + tool_call_id: 'tc1', + tool_call_arguments_complete: + '{"name":"deepchat-settings","file_path":"references/guide.md"}' + } as LLMCoreStreamEvent + yield { type: 'stop', stop_reason: 'tool_use' } as LLMCoreStreamEvent + })() + } + expect(tools.map((tool) => tool.function.name)).toEqual(['skill_view']) + return (async function* () { + yield { type: 'text', content: 'Done' } as LLMCoreStreamEvent + yield { type: 'stop', stop_reason: 'complete' } as LLMCoreStreamEvent + })() + } + ) as unknown as ProcessParams['coreStream'] + + const params = createParams({ + coreStream, + toolPresenter, + tools: [makeTool('skill_view')], + refreshTools + }) + + const promise = processStream(params) + await vi.runAllTimersAsync() + await promise + + expect(refreshTools).not.toHaveBeenCalled() + expect(coreStream).toHaveBeenCalledTimes(2) + }) + it('offloads large tool results before the next provider call', async () => { tempHome = await fs.mkdtemp(path.join(os.tmpdir(), 'deepchat-process-offload-')) getPathSpy = vi.spyOn(app, 'getPath').mockReturnValue(tempHome) diff --git a/test/main/presenter/skillPresenter/skillPresenter.test.ts b/test/main/presenter/skillPresenter/skillPresenter.test.ts index 170d7d9fa..7c541df6c 100644 --- a/test/main/presenter/skillPresenter/skillPresenter.test.ts +++ b/test/main/presenter/skillPresenter/skillPresenter.test.ts @@ -667,6 +667,82 @@ describe('SkillPresenter', () => { ]) }) + it('activates a skill after viewing the main SKILL.md in a new-agent session', async () => { + ;(skillSessionStatePort.hasNewSession as Mock).mockResolvedValue(true) + ;(eventBus.sendToRenderer as Mock).mockClear() + + const result = await skillPresenter.viewSkill('test-skill', { + conversationId: 'conv-view-auto-activate' + }) + + expect(result).toEqual( + expect.objectContaining({ + success: true, + name: 'test-skill', + isPinned: true + }) + ) + expect(await skillPresenter.getActiveSkills('conv-view-auto-activate')).toEqual([ + 'test-skill' + ]) + expect(eventBus.sendToRenderer).toHaveBeenCalledWith(SKILL_EVENTS.ACTIVATED, 'all', { + conversationId: 'conv-view-auto-activate', + skills: ['test-skill'] + }) + }) + + it('does not activate a skill when only viewing a linked file', async () => { + ;(skillSessionStatePort.hasNewSession as Mock).mockResolvedValue(true) + ;(eventBus.sendToRenderer as Mock).mockClear() + + const result = await skillPresenter.viewSkill('test-skill', { + conversationId: 'conv-view-file-only', + filePath: 'references/guide.md' + }) + + expect(result).toEqual( + expect.objectContaining({ + success: true, + name: 'test-skill', + filePath: 'references/guide.md', + isPinned: false + }) + ) + expect(await skillPresenter.getActiveSkills('conv-view-file-only')).toEqual([]) + expect(eventBus.sendToRenderer).not.toHaveBeenCalledWith( + SKILL_EVENTS.ACTIVATED, + 'all', + expect.objectContaining({ + conversationId: 'conv-view-file-only' + }) + ) + }) + + it('does not emit a second activation event when viewing an already pinned skill', async () => { + ;(skillSessionStatePort.hasNewSession as Mock).mockResolvedValue(true) + await skillPresenter.setActiveSkills('conv-view-existing', ['test-skill']) + ;(eventBus.sendToRenderer as Mock).mockClear() + + const result = await skillPresenter.viewSkill('test-skill', { + conversationId: 'conv-view-existing' + }) + + expect(result).toEqual( + expect.objectContaining({ + success: true, + name: 'test-skill', + isPinned: true + }) + ) + expect(eventBus.sendToRenderer).not.toHaveBeenCalledWith( + SKILL_EVENTS.ACTIVATED, + 'all', + expect.objectContaining({ + conversationId: 'conv-view-existing' + }) + ) + }) + it('rejects oversized skill markdown files before loading content', async () => { ;(fs.statSync as Mock).mockReturnValue({ isFile: () => true, diff --git a/test/main/presenter/toolPresenter/agentTools/agentToolManagerSettings.test.ts b/test/main/presenter/toolPresenter/agentTools/agentToolManagerSettings.test.ts index 6a1df5669..919e327bb 100644 --- a/test/main/presenter/toolPresenter/agentTools/agentToolManagerSettings.test.ts +++ b/test/main/presenter/toolPresenter/agentTools/agentToolManagerSettings.test.ts @@ -18,6 +18,7 @@ describe('AgentToolManager DeepChat settings tool gating', () => { const skillPresenter = { getActiveSkills: vi.fn(), getActiveSkillsAllowedTools: vi.fn(), + viewSkill: vi.fn(), listSkillScripts: vi.fn().mockResolvedValue([]), manageDraftSkill: vi.fn(), getSkillExtension: vi.fn().mockResolvedValue({ @@ -63,6 +64,13 @@ describe('AgentToolManager DeepChat settings tool gating', () => { resolveConversationWorkdir.mockResolvedValue(null) resolveConversationSessionInfo.mockResolvedValue(null) skillPresenter.listSkillScripts.mockResolvedValue([]) + skillPresenter.viewSkill.mockResolvedValue({ + success: true, + name: 'code-review', + filePath: null, + content: '# Code Review', + isPinned: false + }) skillPresenter.manageDraftSkill.mockResolvedValue({ success: true, action: 'create' }) getToolDefinitions.mockReturnValue([]) }) @@ -148,6 +156,56 @@ describe('AgentToolManager DeepChat settings tool gating', () => { expect(names).not.toContain('skill_control') }) + it('returns skill_view activation metadata after viewing a main SKILL.md', async () => { + skillPresenter.getActiveSkills.mockResolvedValue([]) + skillPresenter.getActiveSkillsAllowedTools.mockResolvedValue([]) + skillPresenter.viewSkill.mockResolvedValue({ + success: true, + name: 'deepchat-settings', + filePath: null, + content: '# Skill', + isPinned: true + }) + + const manager = buildManager() + const result = (await manager.callTool( + 'skill_view', + { name: 'deepchat-settings' }, + 'conv-1' + )) as { content: string; rawData?: { toolResult?: unknown } } + + expect(result.content).toContain('"isPinned":true') + expect(result.rawData?.toolResult).toEqual({ + activationApplied: true, + activationSource: 'skill_md', + activatedSkill: 'deepchat-settings' + }) + }) + + it('does not mark linked file views as skill activations', async () => { + skillPresenter.getActiveSkills.mockResolvedValue([]) + skillPresenter.getActiveSkillsAllowedTools.mockResolvedValue([]) + skillPresenter.viewSkill.mockResolvedValue({ + success: true, + name: 'deepchat-settings', + filePath: 'references/guide.md', + content: '# Guide', + isPinned: false + }) + + const manager = buildManager() + const result = (await manager.callTool( + 'skill_view', + { name: 'deepchat-settings', file_path: 'references/guide.md' }, + 'conv-1' + )) as { rawData?: { toolResult?: unknown } } + + expect(result.rawData?.toolResult).toEqual({ + activationApplied: false, + activationSource: 'file' + }) + }) + it('rejects skill_manage create requests without content before calling the presenter', async () => { skillPresenter.getActiveSkills.mockResolvedValue([]) skillPresenter.getActiveSkillsAllowedTools.mockResolvedValue([]) diff --git a/test/renderer/components/McpIndicator.test.ts b/test/renderer/components/McpIndicator.test.ts index ecd3142e5..349f0b755 100644 --- a/test/renderer/components/McpIndicator.test.ts +++ b/test/renderer/components/McpIndicator.test.ts @@ -58,6 +58,27 @@ const setup = async (options?: { }) => { vi.resetModules() + const ipcListeners = new Map void>>() + const ipcRenderer = { + on: vi.fn((event: string, handler: (...args: unknown[]) => void) => { + const listeners = ipcListeners.get(event) ?? new Set() + listeners.add(handler) + ipcListeners.set(event, listeners) + }), + removeListener: vi.fn((event: string, handler: (...args: unknown[]) => void) => { + ipcListeners.get(event)?.delete(handler) + }), + emit: (event: string, payload?: unknown) => { + for (const listener of ipcListeners.get(event) ?? []) { + listener({}, payload) + } + } + } + + ;(window as any).electron = { + ipcRenderer + } + const mcpStore = reactive({ enabledServers: [{ name: 'demo-server', icons: 'D', enabled: true }], enabledServerCount: 1, @@ -209,7 +230,8 @@ const setup = async (options?: { wrapper, draftStore, toolPresenter, - agentSessionPresenter + agentSessionPresenter, + ipcRenderer } } @@ -322,4 +344,25 @@ describe('McpIndicator', () => { expect(wrapper.emitted('toggle-subagents')).toEqual([[false]]) }) + + it('reloads deepchat tools when the active session emits skill activation changes', async () => { + const { toolPresenter, ipcRenderer } = await setup({ + hasActiveSession: true, + activeAgentId: 'deepchat' + }) + + toolPresenter.getAllToolDefinitions.mockClear() + ipcRenderer.emit('skill:activated', { + conversationId: 's1', + skills: ['deepchat-settings'] + }) + await flushPromises() + + expect(toolPresenter.getAllToolDefinitions).toHaveBeenCalledTimes(1) + expect(toolPresenter.getAllToolDefinitions).toHaveBeenCalledWith({ + chatMode: 'agent', + conversationId: 's1', + agentWorkspacePath: '/tmp/workspace' + }) + }) }) From 7dc01f79f5f5ea79c5966cc183ebf642c568ffb6 Mon Sep 17 00:00:00 2001 From: zerob13 Date: Sat, 11 Apr 2026 19:12:58 +0800 Subject: [PATCH 2/3] fix(skills): confirm activation state --- src/main/presenter/skillPresenter/index.ts | 13 +++++++---- .../agentTools/agentToolManager.ts | 23 +++++++++++-------- src/shared/types/skill.ts | 2 +- .../agentSessionPresenter.test.ts | 2 +- .../skillPresenter/skillTools.test.ts | 2 +- 5 files changed, 26 insertions(+), 16 deletions(-) diff --git a/src/main/presenter/skillPresenter/index.ts b/src/main/presenter/skillPresenter/index.ts index 9bcb9e2fa..16163dc59 100644 --- a/src/main/presenter/skillPresenter/index.ts +++ b/src/main/presenter/skillPresenter/index.ts @@ -572,8 +572,11 @@ export class SkillPresenter implements ISkillPresenter { let nextIsPinned = isPinned if (options?.conversationId && !isPinned) { - await this.setActiveSkills(options.conversationId, [...pinnedSkills, metadata.name]) - nextIsPinned = true + const updatedSkills = await this.setActiveSkills(options.conversationId, [ + ...pinnedSkills, + metadata.name + ]) + nextIsPinned = updatedSkills.includes(metadata.name) } return { @@ -1474,14 +1477,14 @@ export class SkillPresenter implements ISkillPresenter { /** * Set active skills for a conversation */ - async setActiveSkills(conversationId: string, skills: string[]): Promise { + async setActiveSkills(conversationId: string, skills: string[]): Promise { try { const isNewSession = await this.isNewAgentSession(conversationId) // Validate skill names const validSkills = await this.validateSkillNames(skills) if (!isNewSession) { this.warnLegacySkillRetired(conversationId) - return + return await this.getActiveSkills(conversationId) } const previousSkills = await this.getActiveSkills(conversationId) @@ -1506,6 +1509,8 @@ export class SkillPresenter implements ISkillPresenter { skills: deactivated }) } + + return validSkills } catch (error) { console.error(`[SkillPresenter] Error setting active skills for ${conversationId}:`, error) throw error diff --git a/src/main/presenter/toolPresenter/agentTools/agentToolManager.ts b/src/main/presenter/toolPresenter/agentTools/agentToolManager.ts index a3e0fda4d..b9f8cd788 100644 --- a/src/main/presenter/toolPresenter/agentTools/agentToolManager.ts +++ b/src/main/presenter/toolPresenter/agentTools/agentToolManager.ts @@ -1768,20 +1768,25 @@ export class AgentToolManager { if (!validationResult.success) { throw new Error(`Invalid arguments for skill_view: ${validationResult.error.message}`) } - const isLinkedFileView = typeof validationResult.data.file_path === 'string' - const wasActive = + const normalizedFilePath = + typeof validationResult.data.file_path === 'string' + ? validationResult.data.file_path.trim() + : '' + const isLinkedFileView = normalizedFilePath.length > 0 + const previousActiveSkills = conversationId && !isLinkedFileView - ? (await this.getSkillPresenter().getActiveSkills(conversationId)).includes( - validationResult.data.name - ) - : false + ? await this.getSkillPresenter().getActiveSkills(conversationId) + : [] const result = await skillTools.handleSkillView(conversationId, validationResult.data) + const nextActiveSkills = + conversationId && !isLinkedFileView + ? await this.getSkillPresenter().getActiveSkills(conversationId) + : previousActiveSkills const activationApplied = Boolean(conversationId) && - result.success === true && !isLinkedFileView && - !wasActive && - result.isPinned === true + !previousActiveSkills.includes(validationResult.data.name) && + nextActiveSkills.includes(validationResult.data.name) const activationSource = !conversationId || result.success !== true ? 'none' diff --git a/src/shared/types/skill.ts b/src/shared/types/skill.ts index 11a84d1cd..b6002fdb2 100644 --- a/src/shared/types/skill.ts +++ b/src/shared/types/skill.ts @@ -204,7 +204,7 @@ export interface ISkillPresenter { // Session state management getActiveSkills(conversationId: string): Promise - setActiveSkills(conversationId: string, skills: string[]): Promise + setActiveSkills(conversationId: string, skills: string[]): Promise clearNewAgentSessionSkills?(conversationId: string): Promise validateSkillNames(names: string[]): Promise diff --git a/test/main/presenter/agentSessionPresenter/agentSessionPresenter.test.ts b/test/main/presenter/agentSessionPresenter/agentSessionPresenter.test.ts index 3a6d3d850..d2c20ad24 100644 --- a/test/main/presenter/agentSessionPresenter/agentSessionPresenter.test.ts +++ b/test/main/presenter/agentSessionPresenter/agentSessionPresenter.test.ts @@ -158,7 +158,7 @@ function createMockLlmProviderPresenter() { function createMockSkillPresenter() { return { - setActiveSkills: vi.fn().mockResolvedValue(undefined), + setActiveSkills: vi.fn().mockResolvedValue([]), clearNewAgentSessionSkills: vi.fn().mockResolvedValue(undefined) } as any } diff --git a/test/main/presenter/skillPresenter/skillTools.test.ts b/test/main/presenter/skillPresenter/skillTools.test.ts index 59521bb32..8a6df1d68 100644 --- a/test/main/presenter/skillPresenter/skillTools.test.ts +++ b/test/main/presenter/skillPresenter/skillTools.test.ts @@ -77,7 +77,7 @@ describe('SkillTools', () => { saveSkillExtension: vi.fn().mockResolvedValue(undefined), listSkillScripts: vi.fn().mockResolvedValue([]), getActiveSkills: vi.fn().mockResolvedValue([]), - setActiveSkills: vi.fn().mockResolvedValue(undefined), + setActiveSkills: vi.fn().mockResolvedValue([]), validateSkillNames: vi.fn().mockImplementation((names: string[]) => { const available = new Set(mockSkillMetadata.map((skill) => skill.name)) return Promise.resolve(names.filter((name) => available.has(name))) From 82aa82c7f410ce96d30989e073007211b1f25b2e Mon Sep 17 00:00:00 2001 From: zerob13 Date: Sat, 11 Apr 2026 19:13:21 +0800 Subject: [PATCH 3/3] test(skills): update activation expectations --- .../toolPresenter/agentTools/agentToolManagerSettings.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/main/presenter/toolPresenter/agentTools/agentToolManagerSettings.test.ts b/test/main/presenter/toolPresenter/agentTools/agentToolManagerSettings.test.ts index 919e327bb..81a20fa7d 100644 --- a/test/main/presenter/toolPresenter/agentTools/agentToolManagerSettings.test.ts +++ b/test/main/presenter/toolPresenter/agentTools/agentToolManagerSettings.test.ts @@ -157,7 +157,9 @@ describe('AgentToolManager DeepChat settings tool gating', () => { }) it('returns skill_view activation metadata after viewing a main SKILL.md', async () => { - skillPresenter.getActiveSkills.mockResolvedValue([]) + skillPresenter.getActiveSkills + .mockResolvedValueOnce([]) + .mockResolvedValueOnce(['deepchat-settings']) skillPresenter.getActiveSkillsAllowedTools.mockResolvedValue([]) skillPresenter.viewSkill.mockResolvedValue({ success: true,