fix(skill-mcp): use correct sessionID when registering skill MCP connections
Fixes #3021
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({
|
const skillMcpTool = createSkillMcpTool({
|
||||||
manager: managers.skillMcpManager,
|
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 type { ToolContext } from "@opencode-ai/plugin/tool"
|
||||||
import { createSkillMcpTool, applyGrepFilter } from "./tools"
|
import { createSkillMcpTool, applyGrepFilter } from "./tools"
|
||||||
import { SkillMcpManager } from "../../features/skill-mcp-manager"
|
import { SkillMcpManager } from "../../features/skill-mcp-manager"
|
||||||
@@ -165,6 +165,34 @@ describe("skill_mcp tool", () => {
|
|||||||
expect(tool.description).toBeDefined()
|
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", () => {
|
describe("applyGrepFilter", () => {
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
import { tool, type ToolDefinition } from "@opencode-ai/plugin"
|
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 { BUILTIN_MCP_TOOL_HINTS, SKILL_MCP_DESCRIPTION } from "./constants"
|
||||||
import type { SkillMcpArgs } from "./types"
|
import type { SkillMcpArgs } from "./types"
|
||||||
import type { SkillMcpManager, SkillMcpClientInfo, SkillMcpServerContext } from "../../features/skill-mcp-manager"
|
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 {
|
interface SkillMcpToolOptions {
|
||||||
manager: SkillMcpManager
|
manager: SkillMcpManager
|
||||||
getLoadedSkills: () => LoadedSkill[]
|
getLoadedSkills: () => LoadedSkill[]
|
||||||
getSessionID: () => string
|
getSessionID?: () => string | undefined
|
||||||
}
|
}
|
||||||
|
|
||||||
type OperationType = { type: "tool" | "resource" | "prompt"; name: string }
|
type OperationType = { type: "tool" | "resource" | "prompt"; name: string }
|
||||||
@@ -136,7 +137,7 @@ export function createSkillMcpTool(options: SkillMcpToolOptions): ToolDefinition
|
|||||||
.optional()
|
.optional()
|
||||||
.describe("Regex pattern to filter output lines (only matching lines returned)"),
|
.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 operation = validateOperationParams(args)
|
||||||
const skills = getLoadedSkills()
|
const skills = getLoadedSkills()
|
||||||
const found = findMcpServer(args.mcp_name, skills)
|
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 = {
|
const info: SkillMcpClientInfo = {
|
||||||
serverName: args.mcp_name,
|
serverName: args.mcp_name,
|
||||||
skillName: found.skill.name,
|
skillName: found.skill.name,
|
||||||
sessionID: getSessionID(),
|
sessionID,
|
||||||
}
|
}
|
||||||
|
|
||||||
const context: SkillMcpServerContext = {
|
const context: SkillMcpServerContext = {
|
||||||
|
|||||||
@@ -172,6 +172,34 @@ describe("skill tool - MCP schema display", () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
describe("formatMcpCapabilities with inputSchema", () => {
|
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 () => {
|
it("displays tool inputSchema when available", async () => {
|
||||||
// given
|
// given
|
||||||
const mockToolsWithSchema: McpTool[] = [
|
const mockToolsWithSchema: McpTool[] = [
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
import { dirname } from "node:path"
|
import { dirname } from "node:path"
|
||||||
import { tool, type ToolDefinition } from "@opencode-ai/plugin"
|
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 { TOOL_DESCRIPTION_NO_SKILLS, TOOL_DESCRIPTION_PREFIX } from "./constants"
|
||||||
import type { SkillArgs, SkillInfo, SkillLoadOptions } from "./types"
|
import type { SkillArgs, SkillInfo, SkillLoadOptions } from "./types"
|
||||||
import type { LoadedSkill } from "../../features/opencode-skill-loader"
|
import type { LoadedSkill } from "../../features/opencode-skill-loader"
|
||||||
@@ -316,7 +317,7 @@ export function createSkillTool(options: SkillLoadOptions = {}): ToolDefinition
|
|||||||
.optional()
|
.optional()
|
||||||
.describe("Optional arguments or context for command invocation. Example: name='publish', user_message='patch'"),
|
.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 skills = await getSkills()
|
||||||
const commands = getCommands()
|
const commands = getCommands()
|
||||||
cachedDescription = formatCombinedDescription(skills.map(loadedSkillToInfo), commands)
|
cachedDescription = formatCombinedDescription(skills.map(loadedSkillToInfo), commands)
|
||||||
@@ -359,11 +360,17 @@ export function createSkillTool(options: SkillLoadOptions = {}): ToolDefinition
|
|||||||
body,
|
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(
|
const mcpInfo = await formatMcpCapabilities(
|
||||||
matchedSkill,
|
matchedSkill,
|
||||||
options.mcpManager,
|
options.mcpManager,
|
||||||
options.getSessionID()
|
sessionID
|
||||||
)
|
)
|
||||||
if (mcpInfo) {
|
if (mcpInfo) {
|
||||||
output.push(mcpInfo)
|
output.push(mcpInfo)
|
||||||
|
|||||||
@@ -29,7 +29,7 @@ export interface SkillLoadOptions {
|
|||||||
/** MCP manager for querying skill-embedded MCP servers */
|
/** MCP manager for querying skill-embedded MCP servers */
|
||||||
mcpManager?: SkillMcpManager
|
mcpManager?: SkillMcpManager
|
||||||
/** Session ID getter for MCP client identification */
|
/** Session ID getter for MCP client identification */
|
||||||
getSessionID?: () => string
|
getSessionID?: () => string | undefined
|
||||||
/** Git master configuration for watermark/co-author settings */
|
/** Git master configuration for watermark/co-author settings */
|
||||||
gitMasterConfig?: GitMasterConfig
|
gitMasterConfig?: GitMasterConfig
|
||||||
disabledSkills?: Set<string>
|
disabledSkills?: Set<string>
|
||||||
|
|||||||
Reference in New Issue
Block a user