Skip to content

Commit ee32816

Browse files
committed
fix(pi): send tool arguments over stdin
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
1 parent 909051c commit ee32816

4 files changed

Lines changed: 141 additions & 13 deletions

File tree

scripts/smoke-test.sh

Lines changed: 91 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -838,15 +838,16 @@ else
838838
echo; echo "-- B3 first bytes (od) --"; echo "$IM_ARR" | od -c | head -6; exit 1
839839
fi
840840

841-
# B4: STDIN — piped JSON resolves; this path must NOT emit a deprecation warning.
842-
IM_STDIN=$(echo "{\"project\":\"$PROJECT\"}" | "$BINARY" cli get_graph_schema 2>"$CLI_STDERR")
843-
if ! echo "$IM_STDIN" | python3 -c "import json,sys; d=json.loads(sys.stdin.read()); sys.exit(0 if 'node_labels' in d else 1)" 2>/dev/null; then
844-
echo "FAIL B4: stdin get_graph_schema did not resolve"; echo "$IM_STDIN" | head -c 300; cat "$CLI_STDERR"; exit 1
841+
# B4: STDIN + --json is the generated-client transport. It must return the
842+
# complete MCP result envelope and must NOT emit a deprecation warning.
843+
IM_STDIN=$(printf '%s' "{\"project\":\"$PROJECT\"}" | "$BINARY" cli --json get_graph_schema 2>"$CLI_STDERR")
844+
if ! printf '%s' "$IM_STDIN" | python3 -c "import json,sys; d=json.loads(sys.stdin.read()); c=d.get('content'); p=json.loads(c[0].get('text','')) if isinstance(c,list) and c and c[0].get('type') == 'text' else None; sys.exit(0 if d.get('isError') is not True and isinstance(p,dict) and isinstance(p.get('node_labels'),list) else 1)" 2>/dev/null; then
845+
echo "FAIL B4: compact stdin + --json did not return a successful get_graph_schema MCP payload"; echo "$IM_STDIN" | head -c 300; cat "$CLI_STDERR"; exit 1
845846
fi
846847
if grep -qi 'deprecated' "$CLI_STDERR"; then
847848
echo "FAIL B4: stdin path wrongly emitted a deprecation warning"; cat "$CLI_STDERR"; exit 1
848849
fi
849-
echo "OK B4: STDIN input resolves, no deprecation warning"
850+
echo "OK B4: compact STDIN + --json returns a successful schema MCP envelope, no deprecation warning"
850851

851852
# B5: --args-file — JSON read from a file resolves; must NOT warn deprecated.
852853
IM_ARGS_FILE=$(smoke_mktemp_file)
@@ -890,7 +891,8 @@ else
890891
echo "FAIL B6c: 'notatool --help' did not report 'unknown tool'"; cat "$CLI_STDERR"; exit 1
891892
fi
892893

893-
# B7: DEPRECATION guard — one raw-JSON call MUST warn on stderr; flag form must NOT.
894+
# B7: DEPRECATION control — positional raw JSON MUST warn on stderr; this is
895+
# retained only to prove B4 is exercising the non-deprecated stdin transport.
894896
cli search_graph "{\"project\":\"$PROJECT\",\"name_pattern\":\"compute\"}" >/dev/null || true
895897
if grep -qi 'deprecated' "$CLI_STDERR"; then
896898
echo "OK B7a: raw-JSON cli emits deprecation warning on stderr"
@@ -2302,16 +2304,96 @@ if ! path_match "$CMD" "$SELF_PATH" ||
23022304
fi
23032305
echo "OK 8ak: custom KIMI_CODE_HOME MCP + durable context + UserPromptSubmit hook"
23042306

2305-
# 8al: Pi has documented instructions and skill, but no invented MCP config.
2307+
# 8al: Pi has documented instructions and skill, no invented MCP config, and
2308+
# an installed generated extension that uses the non-deprecated stdin bridge.
23062309
PI_INSTRUCTIONS="$FAKE_HOME/.pi/agent/AGENTS.md"
23072310
PI_SKILL="$FAKE_HOME/.pi/agent/skills/codebase-memory/SKILL.md"
2311+
PI_EXTENSION="$FAKE_HOME/.pi/agent/extensions/cbmem.ts"
23082312
if ! grep -q 'search_graph' "$PI_INSTRUCTIONS" 2>/dev/null ||
23092313
! grep -q 'Sessions and Subagents' "$PI_SKILL" 2>/dev/null ||
2314+
! grep -Fq "spawn(BIN, ['cli', '--json', tool], {" "$PI_EXTENSION" 2>/dev/null ||
2315+
! grep -Fq "stdio: ['pipe', 'pipe', 'pipe']" "$PI_EXTENSION" 2>/dev/null ||
2316+
! grep -Fq "child.stdin.on('error'" "$PI_EXTENSION" 2>/dev/null ||
2317+
! grep -Fq 'child.stdin.end(JSON.stringify(args ?? {}));' "$PI_EXTENSION" 2>/dev/null ||
2318+
grep -Fq 'tool, JSON.stringify(args ?? {})]' "$PI_EXTENSION" 2>/dev/null ||
2319+
grep -Fq "stdio: ['ignore', 'pipe', 'pipe']" "$PI_EXTENSION" 2>/dev/null ||
23102320
[ -e "$FAKE_HOME/.pi/agent/mcp.json" ]; then
2311-
echo "FAIL 8al: Pi durable context missing or unsupported MCP config created"
2321+
echo "FAIL 8al: Pi durable context or stdin client bridge missing, or unsupported MCP config created"
23122322
exit 1
23132323
fi
2314-
echo "OK 8al: Pi durable context only (no MCP config)"
2324+
echo "OK 8al: Pi durable context + stdin client bridge (no MCP config)"
2325+
2326+
# 8al-node: execute the generated extension against a child that exits without
2327+
# consuming a deliberately over-pipe-capacity payload. This is the lifecycle
2328+
# Node implements: without an stdin error listener, the late EPIPE is an
2329+
# unhandled EventEmitter error and crashes the Pi host. A second call emits a
2330+
# valid MCP envelope before the same early exit, proving parsed JSON remains
2331+
# authoritative over a retained stdin transport error.
2332+
#
2333+
# SKIP_WHITELIST: the minimal C-only Linux image intentionally has no Node.
2334+
# What was tried: making Node a universal core-suite prerequisite would widen
2335+
# the product's C build dependencies. The exact generated-source assertions
2336+
# above still run there; this live probe gates every Node-equipped smoke venue.
2337+
if command -v node >/dev/null 2>&1; then
2338+
PI_NODE=$(command -v node)
2339+
PI_PROBE_DIR="$TMPDIR/pi-node-probe"
2340+
mkdir -p "$PI_PROBE_DIR"
2341+
python3 - "$PI_EXTENSION" "$PI_PROBE_DIR/cbmem.mjs" <<'PYPIADAPTER'
2342+
import pathlib
2343+
import sys
2344+
2345+
source, destination = map(pathlib.Path, sys.argv[1:])
2346+
text = source.read_text(encoding="utf-8")
2347+
lines = text.splitlines()
2348+
matches = [i for i, line in enumerate(lines) if line.startswith("const BIN = ")]
2349+
if len(matches) != 1:
2350+
raise SystemExit("generated Pi extension has no unique BIN declaration")
2351+
lines[matches[0]] = "const BIN = process.execPath;"
2352+
destination.write_text("\n".join(lines) + "\n", encoding="utf-8")
2353+
PYPIADAPTER
2354+
cat >"$PI_PROBE_DIR/cli" <<'PICHILD'
2355+
const fs = require('node:fs');
2356+
if (process.argv[3] === 'get_graph_schema') {
2357+
fs.writeSync(1, JSON.stringify({ content: [{ type: 'text', text: 'schema-ok' }] }) + '\n');
2358+
}
2359+
process.exit(0);
2360+
PICHILD
2361+
if ! grep -Fxq 'const BIN = process.execPath;' "$PI_PROBE_DIR/cbmem.mjs" ||
2362+
! grep -Fxq "const fs = require('node:fs');" "$PI_PROBE_DIR/cli"; then
2363+
echo "FAIL 8al-node: generated lifecycle probe is not portable across Node launch environments"
2364+
exit 1
2365+
fi
2366+
cat >"$PI_PROBE_DIR/probe.mjs" <<'PIPROBE'
2367+
import extension from './cbmem.mjs';
2368+
2369+
const tools = [];
2370+
extension({ registerTool: (definition) => tools.push(definition) });
2371+
const byName = (name) => tools.find((tool) => tool.name === name);
2372+
const args = { padding: 'x'.repeat(16 * 1024 * 1024) };
2373+
2374+
const successful = await byName('get_graph_schema').execute('probe-ok', args);
2375+
if (successful?.content?.[0]?.text !== 'schema-ok') {
2376+
throw new Error('valid child JSON was not authoritative over stdin EPIPE');
2377+
}
2378+
2379+
let transportError = '';
2380+
try {
2381+
await byName('search_graph').execute('probe-error', args);
2382+
} catch (error) {
2383+
transportError = String(error?.message ?? error);
2384+
}
2385+
if (!/(EPIPE|broken pipe|write)/i.test(transportError)) {
2386+
throw new Error(`missing surfaced stdin transport error: ${transportError || '<none>'}`);
2387+
}
2388+
PIPROBE
2389+
if ! (cd "$PI_PROBE_DIR" && "$PI_NODE" probe.mjs); then
2390+
echo "FAIL 8al-node: generated Pi extension did not contain early-exit stdin errors"
2391+
exit 1
2392+
fi
2393+
echo "OK 8al-node: generated Pi extension contains EPIPE and keeps valid JSON authoritative"
2394+
else
2395+
echo "SKIP 8al-node: Node unavailable in this C-only smoke venue (whitelisted above)"
2396+
fi
23152397

23162398
# 8am: Warp receives the documented shared skill; MCP remains user/UI-managed.
23172399
WARP_SKILL="$FAKE_HOME/.agents/skills/codebase-memory/SKILL.md"

src/cli/client_adapter.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,14 +168,16 @@ char *cbm_client_adapter_pi(const char *binary_path) {
168168
sb_append(
169169
&sb, "async function call(tool, args, signal) {\n"
170170
" return new Promise((resolve) => {\n"
171-
" const child = spawn(BIN, ['cli', '--json', tool, JSON.stringify(args ?? {})], {\n"
172-
" stdio: ['ignore', 'pipe', 'pipe'],\n"
171+
" const child = spawn(BIN, ['cli', '--json', tool], {\n"
172+
" stdio: ['pipe', 'pipe', 'pipe'],\n"
173173
" env: { ...process.env, CBM_LOG_LEVEL: 'error' },\n"
174174
" });\n"
175175
" let out = '';\n"
176+
" let stdinError;\n"
176177
" const onAbort = () => { if (!child.killed) child.kill(); };\n"
177178
" signal?.addEventListener('abort', onAbort, { once: true });\n"
178179
" child.stdout.on('data', (d) => (out += d.toString()));\n"
180+
" child.stdin.on('error', (e) => { stdinError = e; });\n"
179181
" child.on('error', (e) => {\n"
180182
" signal?.removeEventListener('abort', onAbort);\n"
181183
" resolve({ error: String(e && e.message ? e.message : e) });\n"
@@ -186,8 +188,12 @@ char *cbm_client_adapter_pi(const char *binary_path) {
186188
" for (let i = lines.length - 1; i >= 0; i--) {\n"
187189
" try { return resolve(JSON.parse(lines[i])); } catch { /* keep scanning */ }\n"
188190
" }\n"
191+
" if (stdinError) {\n"
192+
" return resolve({ error: String(stdinError.message || stdinError) });\n"
193+
" }\n"
189194
" resolve({ error: 'no JSON response from codebase-memory-mcp' });\n"
190195
" });\n"
196+
" child.stdin.end(JSON.stringify(args ?? {}));\n"
191197
" });\n"
192198
"}\n\n");
193199

src/cli/client_adapter.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,9 @@
3030
/* Emit a pi extension registering every tool in the MCP registry.
3131
*
3232
* pi has no MCP client, so this bridge is the only route to the graph for it.
33-
* Each registered tool shells out to `<binary> cli <tool> '<json-args>'`, which
34-
* is an existing public entry point rather than a private one.
33+
* Each registered tool shells out to `<binary> cli --json <tool>` and writes
34+
* its JSON arguments to stdin, using an existing public entry point without
35+
* the deprecated positional-JSON transport.
3536
*
3637
* `binary_path` is embedded as a JS string literal and is escaped; a Windows
3738
* path such as C:\Users\x\bin\cbm.exe must not corrupt the module or, worse,

tests/test_agent_clients.c

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1139,6 +1139,44 @@ TEST(client_adapter_pi_emits_parameters_and_execute) {
11391139
PASS();
11401140
}
11411141

1142+
/* #1834: raw JSON in argv selects the deprecated compatibility path. The Pi
1143+
* bridge must keep arguments out of process listings and feed the existing
1144+
* stdin transport only after all child handlers are attached. */
1145+
TEST(client_adapter_pi_sends_arguments_over_stdin_issue1834) {
1146+
char *js = cbm_client_adapter_pi("/usr/local/bin/codebase-memory-mcp");
1147+
ASSERT_NOT_NULL(js);
1148+
1149+
const char *spawn = strstr(js, "spawn(BIN, ['cli', '--json', tool], {");
1150+
const char *stdio = strstr(js, "stdio: ['pipe', 'pipe', 'pipe']");
1151+
const char *stdout_handler = strstr(js, "child.stdout.on('data'");
1152+
const char *stdin_error_handler = strstr(js, "child.stdin.on('error'");
1153+
const char *error_handler = strstr(js, "child.on('error'");
1154+
const char *close_handler = strstr(js, "child.on('close'");
1155+
const char *stdin_end = strstr(js, "child.stdin.end(JSON.stringify(args ?? {}));");
1156+
1157+
ASSERT_NOT_NULL(spawn);
1158+
ASSERT_NOT_NULL(stdio);
1159+
ASSERT_NOT_NULL(stdout_handler);
1160+
ASSERT_NOT_NULL(stdin_error_handler);
1161+
ASSERT_NOT_NULL(error_handler);
1162+
ASSERT_NOT_NULL(close_handler);
1163+
ASSERT_NOT_NULL(stdin_end);
1164+
ASSERT(stdout_handler < stdin_end);
1165+
ASSERT(stdin_error_handler < stdin_end);
1166+
ASSERT(error_handler < stdin_end);
1167+
ASSERT(close_handler < stdin_end);
1168+
ASSERT_NOT_NULL(strstr(js, "let stdinError;"));
1169+
ASSERT_NOT_NULL(strstr(js, "stdinError = e;"));
1170+
ASSERT_NOT_NULL(strstr(js, "if (stdinError)"));
1171+
1172+
/* These are the exact deprecated transport shapes being retired. */
1173+
ASSERT_NULL(strstr(js, "tool, JSON.stringify(args ?? {})]"));
1174+
ASSERT_NULL(strstr(js, "stdio: ['ignore', 'pipe', 'pipe']"));
1175+
1176+
free(js);
1177+
PASS();
1178+
}
1179+
11421180
/* Result wrap + abort: Pi's TUI reads result.content; a spawn/parse failure
11431181
* must throw rather than return a result without content; AbortSignal must
11441182
* kill the cli child. String-shape only; a live Pi process is not gated. */
@@ -1248,6 +1286,7 @@ SUITE(agent_clients) {
12481286
RUN_TEST(client_adapter_pi_registers_every_registry_tool);
12491287
RUN_TEST(client_adapter_pi_default_exports_its_factory_issue1550);
12501288
RUN_TEST(client_adapter_pi_emits_parameters_and_execute);
1289+
RUN_TEST(client_adapter_pi_sends_arguments_over_stdin_issue1834);
12511290
RUN_TEST(client_adapter_pi_wraps_result_and_honors_abort);
12521291
RUN_TEST(client_adapter_escapes_windows_paths_and_quotes);
12531292
RUN_TEST(client_adapter_opencode_sends_the_required_hook_event);

0 commit comments

Comments
 (0)