diff --git a/src/cli/client_adapter.c b/src/cli/client_adapter.c index 839b3d87b..cc7454793 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; @@ -120,6 +153,10 @@ char *cbm_client_adapter_pi(const char *binary_path) { adapter_sb_t sb = {0}; emit_header(&sb, "pi"); + /* Pin the coding-agent contract so a probe against @mariozechner/pi + * (pods CLI) or an old AgentTool arity cannot be mistaken for this file. */ + sb_append(&sb, "// Target: @earendil-works/pi-coding-agent >= 0.74.0 (verified 0.84.2)\n" + "// ToolDefinition.execute(toolCallId, params, signal, onUpdate, ctx)\n"); sb_append(&sb, "import { spawn } from 'node:child_process';\n\n"); sb_append(&sb, "const BIN = '"); sb_append(&sb, bin); @@ -131,7 +168,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 +207,37 @@ 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 : "{}"); + /* 0.84.2 calls execute(toolCallId, params, signal, onUpdate, ctx). + * The previous (args, ctx) shape bound the call id as the MCP args. */ + sb_append(&sb, ",\n execute: async (toolCallId, params, signal, _onUpdate, ctx) => {\n"); + sb_append(&sb, " const result = await call("); + sb_append_js_string(&sb, name); + sb_append(&sb, + ", params, signal ?? 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 4146fdf5e..78cf1e27e 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -945,6 +945,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..89dda28ba 100644 --- a/tests/test_agent_clients.c +++ b/tests/test_agent_clients.c @@ -1114,6 +1114,31 @@ 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 (toolCallId, params, signal, _onUpdate, ctx) => {")); + ASSERT_NOT_NULL(strstr(js, ", params, signal ?? ctx?.signal)")); + ASSERT_NULL(strstr(js, "execute: async (args, ctx) => {")); + ASSERT_NULL(strstr(js, ", args, ctx?.signal)")); + ASSERT_NOT_NULL(strstr(js, "@earendil-works/pi-coding-agent")); + 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 +1229,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);