From 027a6b0039aaa36821fe1e78f77913b1fc5cd201 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Thu, 2 Apr 2026 15:45:39 +0900 Subject: [PATCH] fix(skill-mcp): use correct sessionID when registering skill MCP connections Fixes #3021 --- src/plugin/tool-registry.ts | 2 +- src/tools/skill-mcp/tools.test.ts | 30 +++++++++++++++++++++++++++++- src/tools/skill-mcp/tools.ts | 12 +++++++++--- src/tools/skill/tools.test.ts | 28 ++++++++++++++++++++++++++++ src/tools/skill/tools.ts | 13 ++++++++++--- src/tools/skill/types.ts | 2 +- 6 files changed, 78 insertions(+), 9 deletions(-) diff --git a/src/plugin/tool-registry.ts b/src/plugin/tool-registry.ts index 78a7fc202..a493dde51 100644 --- a/src/plugin/tool-registry.ts +++ b/src/plugin/tool-registry.ts @@ -153,7 +153,7 @@ export function createToolRegistry(args: { }, }) - const getSessionIDForMcp = (): string => getMainSessionID() || "" + const getSessionIDForMcp = (): string | undefined => getMainSessionID() const skillMcpTool = createSkillMcpTool({ manager: managers.skillMcpManager, diff --git a/src/tools/skill-mcp/tools.test.ts b/src/tools/skill-mcp/tools.test.ts index 642a0f871..825ea57af 100644 --- a/src/tools/skill-mcp/tools.test.ts +++ b/src/tools/skill-mcp/tools.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, mock } from "bun:test" +import { describe, it, expect, beforeEach, mock, spyOn } from "bun:test" import type { ToolContext } from "@opencode-ai/plugin/tool" import { createSkillMcpTool, applyGrepFilter } from "./tools" import { SkillMcpManager } from "../../features/skill-mcp-manager" @@ -165,6 +165,34 @@ describe("skill_mcp tool", () => { expect(tool.description).toBeDefined() }) }) + + describe("session resolution", () => { + it("uses the tool context sessionID when the fallback getter is empty", async () => { + // given + loadedSkills = [ + createMockSkillWithMcp("test-skill", { + "test-server": { command: "echo", args: ["test"] }, + }), + ] + const callToolSpy = spyOn(manager, "callTool").mockResolvedValue({ content: [] } as never) + const tool = createSkillMcpTool({ + manager, + getLoadedSkills: () => loadedSkills, + getSessionID: () => "", + }) + + // when + await tool.execute({ mcp_name: "test-server", tool_name: "some-tool" }, mockContext) + + // then + expect(callToolSpy).toHaveBeenCalledWith( + expect.objectContaining({ sessionID: mockContext.sessionID }), + expect.any(Object), + "some-tool", + {}, + ) + }) + }) }) describe("applyGrepFilter", () => { diff --git a/src/tools/skill-mcp/tools.ts b/src/tools/skill-mcp/tools.ts index 9791501fe..197ee62dc 100644 --- a/src/tools/skill-mcp/tools.ts +++ b/src/tools/skill-mcp/tools.ts @@ -1,4 +1,5 @@ import { tool, type ToolDefinition } from "@opencode-ai/plugin" +import type { ToolContext } from "@opencode-ai/plugin/tool" import { BUILTIN_MCP_TOOL_HINTS, SKILL_MCP_DESCRIPTION } from "./constants" import type { SkillMcpArgs } from "./types" import type { SkillMcpManager, SkillMcpClientInfo, SkillMcpServerContext } from "../../features/skill-mcp-manager" @@ -7,7 +8,7 @@ import type { LoadedSkill } from "../../features/opencode-skill-loader/types" interface SkillMcpToolOptions { manager: SkillMcpManager getLoadedSkills: () => LoadedSkill[] - getSessionID: () => string + getSessionID?: () => string | undefined } type OperationType = { type: "tool" | "resource" | "prompt"; name: string } @@ -136,7 +137,7 @@ export function createSkillMcpTool(options: SkillMcpToolOptions): ToolDefinition .optional() .describe("Regex pattern to filter output lines (only matching lines returned)"), }, - async execute(args: SkillMcpArgs) { + async execute(args: SkillMcpArgs, toolContext: ToolContext) { const operation = validateOperationParams(args) const skills = getLoadedSkills() const found = findMcpServer(args.mcp_name, skills) @@ -156,10 +157,15 @@ export function createSkillMcpTool(options: SkillMcpToolOptions): ToolDefinition ) } + const sessionID = toolContext.sessionID || getSessionID?.() + if (!sessionID) { + throw new Error("No active session available for skill MCP call.") + } + const info: SkillMcpClientInfo = { serverName: args.mcp_name, skillName: found.skill.name, - sessionID: getSessionID(), + sessionID, } const context: SkillMcpServerContext = { diff --git a/src/tools/skill/tools.test.ts b/src/tools/skill/tools.test.ts index 5c7282766..5007857ff 100644 --- a/src/tools/skill/tools.test.ts +++ b/src/tools/skill/tools.test.ts @@ -172,6 +172,34 @@ describe("skill tool - MCP schema display", () => { }) describe("formatMcpCapabilities with inputSchema", () => { + it("uses the tool context sessionID when the fallback getter is empty", async () => { + // given + loadedSkills = [ + createMockSkillWithMcp("test-skill", { + playwright: { command: "npx", args: ["-y", "@anthropic-ai/mcp-playwright"] }, + }), + ] + + const listToolsSpy = spyOn(manager, "listTools").mockResolvedValue([]) + spyOn(manager, "listResources").mockResolvedValue([]) + spyOn(manager, "listPrompts").mockResolvedValue([]) + + const tool = createSkillTool({ + skills: loadedSkills, + mcpManager: manager, + getSessionID: () => "", + }) + + // when + await tool.execute({ name: "test-skill" }, mockContext) + + // then + expect(listToolsSpy).toHaveBeenCalledWith( + expect.objectContaining({ sessionID: mockContext.sessionID }), + expect.any(Object), + ) + }) + it("displays tool inputSchema when available", async () => { // given const mockToolsWithSchema: McpTool[] = [ diff --git a/src/tools/skill/tools.ts b/src/tools/skill/tools.ts index 68ac1a827..34d31cb2e 100644 --- a/src/tools/skill/tools.ts +++ b/src/tools/skill/tools.ts @@ -1,5 +1,6 @@ import { dirname } from "node:path" import { tool, type ToolDefinition } from "@opencode-ai/plugin" +import type { ToolContext } from "@opencode-ai/plugin/tool" import { TOOL_DESCRIPTION_NO_SKILLS, TOOL_DESCRIPTION_PREFIX } from "./constants" import type { SkillArgs, SkillInfo, SkillLoadOptions } from "./types" import type { LoadedSkill } from "../../features/opencode-skill-loader" @@ -316,7 +317,7 @@ export function createSkillTool(options: SkillLoadOptions = {}): ToolDefinition .optional() .describe("Optional arguments or context for command invocation. Example: name='publish', user_message='patch'"), }, - async execute(args: SkillArgs, ctx?: { agent?: string }) { + async execute(args: SkillArgs, ctx?: ToolContext) { const skills = await getSkills() const commands = getCommands() cachedDescription = formatCombinedDescription(skills.map(loadedSkillToInfo), commands) @@ -359,11 +360,17 @@ export function createSkillTool(options: SkillLoadOptions = {}): ToolDefinition body, ] - if (options.mcpManager && options.getSessionID && matchedSkill.mcpConfig) { + if (options.mcpManager && matchedSkill.mcpConfig) { + const sessionID = ctx?.sessionID || options.getSessionID?.() + + if (!sessionID) { + return output.join("\n") + } + const mcpInfo = await formatMcpCapabilities( matchedSkill, options.mcpManager, - options.getSessionID() + sessionID ) if (mcpInfo) { output.push(mcpInfo) diff --git a/src/tools/skill/types.ts b/src/tools/skill/types.ts index 1358f88f4..c5ae02540 100644 --- a/src/tools/skill/types.ts +++ b/src/tools/skill/types.ts @@ -29,7 +29,7 @@ export interface SkillLoadOptions { /** MCP manager for querying skill-embedded MCP servers */ mcpManager?: SkillMcpManager /** Session ID getter for MCP client identification */ - getSessionID?: () => string + getSessionID?: () => string | undefined /** Git master configuration for watermark/co-author settings */ gitMasterConfig?: GitMasterConfig disabledSkills?: Set