diff --git a/src/cli/client_adapter.c b/src/cli/client_adapter.c index 839b3d87b..73cd9f112 100644 --- a/src/cli/client_adapter.c +++ b/src/cli/client_adapter.c @@ -49,6 +49,39 @@ static void sb_append(adapter_sb_t *sb, const char *s) { sb->buf[sb->len] = '\0'; } +/* Append a single-quoted JavaScript string literal, escaping the characters + * that could terminate or corrupt it. Avoids the fixed buffer sizing that + * cbm_client_adapter_escape_js imposes, so long registry descriptions fit. */ +static void sb_append_js_string(adapter_sb_t *sb, const char *s) { + if (!s) { + sb_append(sb, "''"); + return; + } + sb_append(sb, "'"); + for (const char *p = s; *p; p++) { + switch (*p) { + case '\\': + sb_append(sb, "\\\\"); + break; + case '\'': + sb_append(sb, "\\'"); + break; + case '\n': + sb_append(sb, "\\n"); + break; + case '\r': + sb_append(sb, "\\r"); + break; + default: { + char ch[2] = {*p, '\0'}; + sb_append(sb, ch); + break; + } + } + } + sb_append(sb, "'"); +} + bool cbm_client_adapter_escape_js(const char *in, char *out, size_t out_sz) { if (!in || !out || out_sz == 0) { return false; @@ -131,7 +164,7 @@ char *cbm_client_adapter_pi(const char *binary_path) { sb_append( &sb, "async function call(tool, args, signal) {\n" " return new Promise((resolve) => {\n" - " const child = spawn(BIN, ['cli', tool, JSON.stringify(args ?? {})], {\n" + " const child = spawn(BIN, ['cli', '--json', tool, JSON.stringify(args ?? {})], {\n" " stdio: ['ignore', 'pipe', 'pipe'],\n" " env: { ...process.env, CBM_LOG_LEVEL: 'error' },\n" " });\n" @@ -170,11 +203,35 @@ char *cbm_client_adapter_pi(const char *binary_path) { if (!name || !name[0]) { continue; } - sb_append(&sb, " pi.registerTool({ name: '"); - sb_append(&sb, name); - sb_append(&sb, "', run: (args, ctx) => call('"); - sb_append(&sb, name); - sb_append(&sb, "', args, ctx?.signal) });\n"); + const char *title = cbm_mcp_tool_title(name); + const char *description = cbm_mcp_tool_description(name); + const char *schema = cbm_mcp_tool_input_schema(name); + sb_append(&sb, " pi.registerTool({\n"); + sb_append(&sb, " name: "); + sb_append_js_string(&sb, name); + sb_append(&sb, ",\n label: "); + sb_append_js_string(&sb, title ? title : name); + sb_append(&sb, ",\n description: "); + sb_append_js_string(&sb, description ? description : ""); + sb_append(&sb, ",\n parameters: "); + /* input_schema is compact JSON, which is a valid JavaScript object + * literal; embedding it directly keeps the generated module free of a + * JSON.parse indirection and of any escaping drift. */ + sb_append(&sb, schema ? schema : "{}"); + sb_append(&sb, ",\n execute: async (args, ctx) => {\n"); + sb_append(&sb, " const result = await call("); + sb_append_js_string(&sb, name); + sb_append(&sb, + ", args, ctx?.signal);\n" + " if (result && typeof result === 'object' && result.error) {\n" + " throw new Error(String(result.error));\n" + " }\n" + " const content = result && Array.isArray(result.content)\n" + " ? result.content\n" + " : [{ type: 'text', text: JSON.stringify(result ?? null, null, 2) }];\n" + " return { content, details: result ?? {} };\n" + " },\n" + " });\n"); } sb_append(&sb, "}\n"); diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 411c15ebd..7af4fe159 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -939,6 +939,30 @@ const char *cbm_mcp_tool_name(int index) { return TOOLS[index].name; } +const char *cbm_mcp_tool_title(const char *tool_name) { + if (!tool_name) { + return NULL; + } + for (int i = 0; i < TOOL_COUNT; i++) { + if (strcmp(TOOLS[i].name, tool_name) == 0) { + return TOOLS[i].title; + } + } + return NULL; +} + +const char *cbm_mcp_tool_description(const char *tool_name) { + if (!tool_name) { + return NULL; + } + for (int i = 0; i < TOOL_COUNT; i++) { + if (strcmp(TOOLS[i].name, tool_name) == 0) { + return TOOLS[i].description; + } + } + return NULL; +} + /* Render the top-level --help "Tools:" block from the registry tools/list * serves. The list used to be hand-maintained in the help text and drifted * when check_index_coverage was added (#1361); deriving it here makes that diff --git a/src/mcp/mcp.h b/src/mcp/mcp.h index a70b6372b..d55d58f3d 100644 --- a/src/mcp/mcp.h +++ b/src/mcp/mcp.h @@ -72,6 +72,12 @@ const char *cbm_mcp_tool_input_schema(const char *tool_name); int cbm_mcp_tool_count(void); const char *cbm_mcp_tool_name(int index); +/* Tool metadata by name (static; do not free; NULL when unknown). Used by the + * generated client adapters so they can emit descriptions and parameter + * schemas without drifting from tools/list. */ +const char *cbm_mcp_tool_title(const char *tool_name); +const char *cbm_mcp_tool_description(const char *tool_name); + /* Render the top-level --help "Tools:" block from the registry so the help * text cannot drift from tools/list (#1361). Heap-allocated; caller frees. */ char *cbm_mcp_tools_help_list(void); diff --git a/tests/test_agent_clients.c b/tests/test_agent_clients.c index 15794c2b1..f15e69a66 100644 --- a/tests/test_agent_clients.c +++ b/tests/test_agent_clients.c @@ -1114,6 +1114,27 @@ TEST(client_adapter_pi_default_exports_its_factory_issue1550) { PASS(); } +/* Pi's ToolDefinition requires `parameters` and `execute`. The adapter used to + * emit `{ name, run }`, which Pi accepted but treated as a tool with no + * parameter schema — strict providers (xAI/Grok) then reject the request with + * a 422 "missing field parameters", and the tool is uncallable because Pi + * invokes `execute`, never `run`. Pin the corrected shape. */ +TEST(client_adapter_pi_emits_parameters_and_execute) { + char *js = cbm_client_adapter_pi("/usr/local/bin/codebase-memory-mcp"); + ASSERT_NOT_NULL(js); + ASSERT_NOT_NULL(strstr(js, "execute: async (args, ctx) => {")); + ASSERT_NOT_NULL(strstr(js, "result.content")); + ASSERT_NULL(strstr(js, "run: (args, ctx)")); + ASSERT_NOT_NULL(strstr(js, "parameters:")); + /* The registry input_schema is embedded as a JSON object literal. */ + ASSERT_NOT_NULL(strstr(js, "\"type\":\"object\"")); + /* Raw JSON output is required so the bridge can parse the MCP result; the + * human-readable path would leave `call` with nothing to JSON.parse. */ + ASSERT_NOT_NULL(strstr(js, "'cli', '--json'")); + free(js); + PASS(); +} + /* #616 rejected `"` in its path template but not `\`, so a Windows home like * C:\Users\urs\bin produced `\u` — an invalid unicode escape — and the whole * auto-loaded plugin failed to parse. A broken plugin in an auto-load directory @@ -1204,6 +1225,7 @@ SUITE(agent_clients) { RUN_TEST(agent_clients_continue_refuses_foreign_same_name_and_nonsequence_section); RUN_TEST(client_adapter_pi_registers_every_registry_tool); RUN_TEST(client_adapter_pi_default_exports_its_factory_issue1550); + RUN_TEST(client_adapter_pi_emits_parameters_and_execute); RUN_TEST(client_adapter_escapes_windows_paths_and_quotes); RUN_TEST(client_adapter_opencode_sends_the_required_hook_event); RUN_TEST(client_adapter_rejects_missing_binary_path);