Merge pull request #1678 from musichen/fix/pi-extension-tool-schemas

fix(pi): emit parameters and execute in the generated pi adapter

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");
 

src/mcp/mcp.c

@@ -946,6 +946,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

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);

tests/test_agent_clients.c

@@ -1114,6 +1114,49 @@ 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();
+}
+
+/* Result wrap + abort: Pi's TUI reads result.content; a spawn/parse failure
+ * must throw rather than return a result without content; AbortSignal must
+ * kill the cli child. String-shape only; a live Pi process is not gated. */
+TEST(client_adapter_pi_wraps_result_and_honors_abort) {
+    char *js = cbm_client_adapter_pi("/usr/local/bin/codebase-memory-mcp");
+    ASSERT_NOT_NULL(js);
+    ASSERT_NOT_NULL(strstr(js, "return { content, details: result ?? {} };"));
+    ASSERT_NOT_NULL(strstr(js, "if (result && typeof result === 'object' && result.error)"));
+    ASSERT_NOT_NULL(strstr(js, "throw new Error(String(result.error));"));
+    ASSERT_NOT_NULL(strstr(js, "Array.isArray(result.content)"));
+    ASSERT_NOT_NULL(
+        strstr(js, "[{ type: 'text', text: JSON.stringify(result ?? null, null, 2) }]"));
+    ASSERT_NOT_NULL(strstr(js, "signal?.addEventListener('abort', onAbort, { once: true })"));
+    ASSERT_NOT_NULL(strstr(js, "if (!child.killed) child.kill();"));
+    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 +1247,8 @@ 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_pi_wraps_result_and_honors_abort);
     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);