Merge pull request #908 from gilbrotheraway/fix/skill-mcp-args

fix(skill-mcp): allow object args and strip quotes
This commit is contained in:
Kenny 2026-01-21 12:07:53 -05:00 committed by GitHub
commit 485bc73437
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

View File

@ -21,10 +21,10 @@ function validateOperationParams(args: SkillMcpArgs): OperationType {
if (operations.length === 0) { if (operations.length === 0) {
throw new Error( throw new Error(
`Missing operation. Exactly one of tool_name, resource_name, or prompt_name must be specified.\n\n` + `Missing operation. Exactly one of tool_name, resource_name, or prompt_name must be specified.\n\n` +
`Examples:\n` + `Examples:\n` +
` skill_mcp(mcp_name="sqlite", tool_name="query", arguments='{"sql": "SELECT * FROM users"}')\n` + ` skill_mcp(mcp_name="sqlite", tool_name="query", arguments='{"sql": "SELECT * FROM users"}')\n` +
` skill_mcp(mcp_name="memory", resource_name="memory://notes")\n` + ` skill_mcp(mcp_name="memory", resource_name="memory://notes")\n` +
` skill_mcp(mcp_name="helper", prompt_name="summarize", arguments='{"text": "..."}')` ` skill_mcp(mcp_name="helper", prompt_name="summarize", arguments='{"text": "..."}')`,
) )
} }
@ -33,12 +33,14 @@ function validateOperationParams(args: SkillMcpArgs): OperationType {
args.tool_name && `tool_name="${args.tool_name}"`, args.tool_name && `tool_name="${args.tool_name}"`,
args.resource_name && `resource_name="${args.resource_name}"`, args.resource_name && `resource_name="${args.resource_name}"`,
args.prompt_name && `prompt_name="${args.prompt_name}"`, args.prompt_name && `prompt_name="${args.prompt_name}"`,
].filter(Boolean).join(", ") ]
.filter(Boolean)
.join(", ")
throw new Error( throw new Error(
`Multiple operations specified. Exactly one of tool_name, resource_name, or prompt_name must be provided.\n\n` + `Multiple operations specified. Exactly one of tool_name, resource_name, or prompt_name must be provided.\n\n` +
`Received: ${provided}\n\n` + `Received: ${provided}\n\n` +
`Use separate calls for each operation.` `Use separate calls for each operation.`,
) )
} }
@ -47,7 +49,7 @@ function validateOperationParams(args: SkillMcpArgs): OperationType {
function findMcpServer( function findMcpServer(
mcpName: string, mcpName: string,
skills: LoadedSkill[] skills: LoadedSkill[],
): { skill: LoadedSkill; config: NonNullable<LoadedSkill["mcpConfig"]>[string] } | null { ): { skill: LoadedSkill; config: NonNullable<LoadedSkill["mcpConfig"]>[string] } | null {
for (const skill of skills) { for (const skill of skills) {
if (skill.mcpConfig && mcpName in skill.mcpConfig) { if (skill.mcpConfig && mcpName in skill.mcpConfig) {
@ -75,7 +77,10 @@ function parseArguments(argsJson: string | Record<string, unknown> | undefined):
return argsJson return argsJson
} }
try { try {
const parsed = JSON.parse(argsJson) // Strip outer single quotes if present (common in LLM output)
const jsonStr = argsJson.startsWith("'") && argsJson.endsWith("'") ? argsJson.slice(1, -1) : argsJson
const parsed = JSON.parse(jsonStr)
if (typeof parsed !== "object" || parsed === null) { if (typeof parsed !== "object" || parsed === null) {
throw new Error("Arguments must be a JSON object") throw new Error("Arguments must be a JSON object")
} }
@ -84,8 +89,8 @@ function parseArguments(argsJson: string | Record<string, unknown> | undefined):
const errorMessage = error instanceof Error ? error.message : String(error) const errorMessage = error instanceof Error ? error.message : String(error)
throw new Error( throw new Error(
`Invalid arguments JSON: ${errorMessage}\n\n` + `Invalid arguments JSON: ${errorMessage}\n\n` +
`Expected a valid JSON object, e.g.: '{"key": "value"}'\n` + `Expected a valid JSON object, e.g.: '{"key": "value"}'\n` +
`Received: ${argsJson}` `Received: ${argsJson}`,
) )
} }
} }
@ -95,10 +100,8 @@ export function applyGrepFilter(output: string, pattern: string | undefined): st
try { try {
const regex = new RegExp(pattern, "i") const regex = new RegExp(pattern, "i")
const lines = output.split("\n") const lines = output.split("\n")
const filtered = lines.filter(line => regex.test(line)) const filtered = lines.filter((line) => regex.test(line))
return filtered.length > 0 return filtered.length > 0 ? filtered.join("\n") : `[grep] No lines matched pattern: ${pattern}`
? filtered.join("\n")
: `[grep] No lines matched pattern: ${pattern}`
} catch { } catch {
return output return output
} }
@ -114,8 +117,14 @@ export function createSkillMcpTool(options: SkillMcpToolOptions): ToolDefinition
tool_name: tool.schema.string().optional().describe("MCP tool to call"), tool_name: tool.schema.string().optional().describe("MCP tool to call"),
resource_name: tool.schema.string().optional().describe("MCP resource URI to read"), resource_name: tool.schema.string().optional().describe("MCP resource URI to read"),
prompt_name: tool.schema.string().optional().describe("MCP prompt to get"), prompt_name: tool.schema.string().optional().describe("MCP prompt to get"),
arguments: tool.schema.string().optional().describe("JSON string of arguments"), arguments: tool.schema
grep: tool.schema.string().optional().describe("Regex pattern to filter output lines (only matching lines returned)"), .union([tool.schema.string(), tool.schema.record(tool.schema.string(), tool.schema.unknown())])
.optional()
.describe("JSON string or object of arguments"),
grep: tool.schema
.string()
.optional()
.describe("Regex pattern to filter output lines (only matching lines returned)"),
}, },
async execute(args: SkillMcpArgs) { async execute(args: SkillMcpArgs) {
const operation = validateOperationParams(args) const operation = validateOperationParams(args)
@ -125,9 +134,10 @@ export function createSkillMcpTool(options: SkillMcpToolOptions): ToolDefinition
if (!found) { if (!found) {
throw new Error( throw new Error(
`MCP server "${args.mcp_name}" not found.\n\n` + `MCP server "${args.mcp_name}" not found.\n\n` +
`Available MCP servers in loaded skills:\n` + `Available MCP servers in loaded skills:\n` +
formatAvailableMcps(skills) + `\n\n` + formatAvailableMcps(skills) +
`Hint: Load the skill first using the 'skill' tool, then call skill_mcp.` `\n\n` +
`Hint: Load the skill first using the 'skill' tool, then call skill_mcp.`,
) )
} }