Merge pull request #3027 from code-yeongyu/fix/issue-3021-skill-mcp-session
fix(skill-mcp): use correct sessionID when registering skill MCP connections
This commit is contained in:
@@ -153,7 +153,7 @@ export function createToolRegistry(args: {
|
||||
},
|
||||
})
|
||||
|
||||
const getSessionIDForMcp = (): string => getMainSessionID() || ""
|
||||
const getSessionIDForMcp = (): string | undefined => getMainSessionID()
|
||||
|
||||
const skillMcpTool = createSkillMcpTool({
|
||||
manager: managers.skillMcpManager,
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -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 = {
|
||||
|
||||
@@ -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[] = [
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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<string>
|
||||
|
||||
Reference in New Issue
Block a user