Skip to content

Commit 47b778f

Browse files
committed
fix(mcp): make every tool reply client-usable — structuredContent, pipelining, config get (#1522)
Three regressions shipped in 0.10.0 share one failure shape: an empty result with a success status, indistinguishable from "nothing found" for the LLM clients that are cbm's primary consumers. 1) structuredContent {} on the whole tree-format surface (#1522 bug 1). #1488 replaced the duplicated payload with an EMPTY structuredContent object while every tool still declared a blanket permissive outputSchema. Spec-honoring clients (Claude Code among them) treat structuredContent as THE result when a schema is declared, so search_graph, trace_path, query_graph, get_architecture, search_code, and detect_changes all rendered as literally "{}" on their DEFAULT format, on every platform. The corrected contract: no tool declares an outputSchema (tool output is format-parameter-polymorphic — no static schema is truthful), JSON-object payloads keep their parsed structuredContent, errors keep structuredContent.error, and text-shaped payloads carry NO structuredContent key at all — which also preserves #1375's no-duplication win. 2) Frontend queue overflow killed the session (found by the #1522 sweep). Any 7+ requests pipelined in one stdin burst — an agent issuing parallel tool calls does exactly this — overflowed the 8-frame frontend queue, which failed the whole session: rc=1 with ZERO bytes of output, every buffered response lost. A full queue is now backpressure: the stdin reader blocks until the worker drains (bounded by the same stop/fail flags every teardown path already sets); only a single frame larger than the entire 12 MiB byte budget — which could never be admitted — remains a hard failure. 3) config get printed "" with exit 0 for every unset and every unknown key (#1522 bug 2). list printed stored-or-DEFAULT while get printed stored-or-EMPTY, and no subcommand validated key names, so a typo was indistinguishable from a correctly-read setting. One config-key table now drives help, list, get, set, and reset: get prints the stored value or the key's real default (the same fallback the runtime readers use), and unknown keys error with exit 1 on get, set, and reset alike. Tests — each RED on the pre-fix tree and RED again on revert: * test_mcp.c: text results carry no structuredContent key; tools/list declares no outputSchema; the tool-table guard now binds all three branches (absent / parsed-object / error) for every registered tool. * test_daemon_frontend.c: the over-capacity contract flips from "session fails" to "backpressure without loss" — the held first request plus all 32 over-capacity frames are answered and the run closes cleanly. * test_cli.c: the config command contract — defaults, round-trip, reset, and unknown-key rejection on all three subcommands. * smoke-test.sh: Phase 3z rewritten to the corrected structuredContent contract, new Phase 3z1 (default-format replies usable in schema-honoring clients, no outputSchema advertised), 3z2 (24 pipelined calls all answered), 3z3 (config defaults + unknown-key rejection) — all asserted against the SHIPPED artifact, where #1488's smoke phase previously pinned the empty-object behavior as correct. Verified end-to-end on the locally built production binary: all 15 tools declare no schema; text tools return ABSENT structuredContent, object tools populated, error envelopes intact; 7/24/64-deep pipelined bursts all answered with rc=0; config get prints real defaults and exits 1 on unknown keys. hook_augment (structuredContent.projects) and index_resilience (structuredContent.status) consume object payloads and are unaffected. Fixes #1522. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
1 parent 61b3b1b commit 47b778f

7 files changed

Lines changed: 444 additions & 121 deletions

File tree

‎scripts/smoke-test.sh‎

Lines changed: 117 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ if [ "$TOTAL" -lt 1 ]; then
392392
fi
393393

394394
echo ""
395-
echo "=== Phase 3z: no tool duplicates its payload into structuredContent (#1375) ==="
395+
echo "=== Phase 3z: structuredContent carries structure or is absent — never {} and never a copy (#1375, #1522) ==="
396396
# Asserts on the SHIPPED artifact what the unit suite asserts from source: a
397397
# non-JSON payload must travel ONCE. It used to appear twice — content[0].text
398398
# plus an identical structuredContent.text — costing 2.05x the bytes on a large
@@ -421,21 +421,32 @@ if d.get("isError"):
421421
print("skip"); raise SystemExit
422422
content = d.get("content") or []
423423
text = content[0].get("text", "") if content else ""
424-
sc = d.get("structuredContent")
425-
if not isinstance(sc, dict):
426-
print("no-structured"); raise SystemExit
424+
sc = d.get("structuredContent", "ABSENT")
427425
try:
428426
payload_is_object = isinstance(json.loads(text), dict)
429427
except Exception:
430428
payload_is_object = False
431429
if payload_is_object:
432-
print("skip"); raise SystemExit
433-
print("dup" if sc.get("text") == text and text else "ok")
430+
# Object payloads must carry the parsed, NON-EMPTY object.
431+
print("ok" if isinstance(sc, dict) and sc else "object-lost"); raise SystemExit
432+
# Text payloads: no structuredContent key at all. {} rendered as the whole
433+
# result in outputSchema-honoring clients (#1522); {"text": ...} duplicated
434+
# the wire (#1375).
435+
if sc == "ABSENT":
436+
print("ok")
437+
elif isinstance(sc, dict) and not sc:
438+
print("empty-lie")
439+
elif isinstance(sc, dict) and sc.get("text") == text and text:
440+
print("dup")
441+
else:
442+
print("unexpected")
434443
')
435444
case "$VERDICT" in
436445
dup) echo "FAIL: $(echo "$TOOL_ARGS" | cut -d" " -f1) repeats its payload in structuredContent (#1375)"; DUP_TOOLS=$((DUP_TOOLS+1)) ;;
446+
empty-lie) echo "FAIL: $(echo "$TOOL_ARGS" | cut -d" " -f1) ships structuredContent {} beside a non-empty payload (#1522)"; DUP_TOOLS=$((DUP_TOOLS+1)) ;;
447+
object-lost) echo "FAIL: $(echo "$TOOL_ARGS" | cut -d" " -f1) dropped the parsed object from structuredContent"; DUP_TOOLS=$((DUP_TOOLS+1)) ;;
448+
unexpected) echo "FAIL: $(echo "$TOOL_ARGS" | cut -d" " -f1) has an unexpected structuredContent shape"; DUP_TOOLS=$((DUP_TOOLS+1)) ;;
437449
ok) DUP_CHECKED=$((DUP_CHECKED+1)) ;;
438-
no-structured) echo "FAIL: $(echo "$TOOL_ARGS" | cut -d" " -f1) has no structuredContent object (outputSchema requires one)"; DUP_TOOLS=$((DUP_TOOLS+1)) ;;
439450
esac
440451
done
441452
if [ "$DUP_TOOLS" -ne 0 ]; then
@@ -448,6 +459,105 @@ if [ "$DUP_CHECKED" -eq 0 ]; then
448459
fi
449460
echo "OK: $DUP_CHECKED tool(s) deliver their payload exactly once"
450461

462+
echo "=== Phase 3z1: default-format MCP replies are usable in schema-honoring clients (#1522) ==="
463+
# The exact end-to-end failure shape of #1522: a spec-compliant MCP client
464+
# (Claude Code) reads structuredContent as THE result when a tool declares an
465+
# outputSchema. Drive the REAL stdio server the way such a client does and
466+
# assert a default-format search_graph reply cannot render as "{}": either
467+
# structuredContent is absent (client falls back to content text) or it is a
468+
# non-empty object. Also assert no tool declares an outputSchema anymore.
469+
MCP_1522=$(python3 - "$BINARY" "$PROJECT" <<'PYMCP'
470+
import json, subprocess, sys
471+
BIN, PROJECT = sys.argv[1], sys.argv[2]
472+
def rpc(i, m, p): return json.dumps({"jsonrpc": "2.0", "id": i, "method": m, "params": p})
473+
reqs = "\n".join([
474+
rpc(1, "initialize", {"protocolVersion": "2025-06-18", "capabilities": {},
475+
"clientInfo": {"name": "smoke-1522", "version": "0"}}),
476+
json.dumps({"jsonrpc": "2.0", "method": "notifications/initialized"}),
477+
rpc(2, "tools/list", {}),
478+
rpc(3, "tools/call", {"name": "search_graph", "arguments":
479+
{"project": PROJECT, "name_pattern": "compute"}}),
480+
]) + "\n"
481+
out = subprocess.run([BIN], input=reqs, capture_output=True, text=True, timeout=180).stdout
482+
verdict = []
483+
for line in out.splitlines():
484+
try:
485+
d = json.loads(line)
486+
except Exception:
487+
continue
488+
if d.get("id") == 2:
489+
schemas = [t["name"] for t in d.get("result", {}).get("tools", []) if "outputSchema" in t]
490+
if schemas:
491+
verdict.append("FAIL: outputSchema declared by: " + ",".join(schemas))
492+
if d.get("id") == 3:
493+
r = d.get("result", {})
494+
text = (r.get("content") or [{}])[0].get("text", "")
495+
sc = r.get("structuredContent", "ABSENT")
496+
if not text:
497+
verdict.append("FAIL: search_graph content text is empty")
498+
if isinstance(sc, dict) and not sc:
499+
verdict.append("FAIL: search_graph ships structuredContent {} (#1522)")
500+
if not verdict:
501+
verdict.append("OK")
502+
print("; ".join(verdict))
503+
PYMCP
504+
)
505+
case "$MCP_1522" in
506+
OK) echo "OK: default-format MCP reply is client-usable, no outputSchema declared" ;;
507+
*) echo "$MCP_1522"; exit 1 ;;
508+
esac
509+
510+
echo "=== Phase 3z2: pipelined requests beyond queue capacity all get answers ==="
511+
# Any 7+ requests written in one stdin burst used to kill the server with
512+
# rc=1 and ZERO bytes of output — the frontend queue (capacity 8 frames,
513+
# initialize + notification included) failed the whole session at overflow
514+
# instead of backpressuring the reader. Agent clients issuing parallel tool
515+
# calls pipeline exactly like this.
516+
PIPELINE=$(python3 - "$BINARY" <<'PYPIPE'
517+
import json, subprocess, sys
518+
BIN = sys.argv[1]
519+
N = 24
520+
def rpc(i, m, p): return json.dumps({"jsonrpc": "2.0", "id": i, "method": m, "params": p})
521+
lines = [rpc(1, "initialize", {"protocolVersion": "2025-06-18", "capabilities": {},
522+
"clientInfo": {"name": "smoke-pipe", "version": "0"}}),
523+
json.dumps({"jsonrpc": "2.0", "method": "notifications/initialized"})]
524+
for i in range(N):
525+
lines.append(rpc(100 + i, "tools/call", {"name": "list_projects", "arguments": {}}))
526+
r = subprocess.run([BIN], input="\n".join(lines) + "\n",
527+
capture_output=True, text=True, timeout=300)
528+
answered = set()
529+
for line in r.stdout.splitlines():
530+
try:
531+
d = json.loads(line)
532+
except Exception:
533+
continue
534+
if isinstance(d.get("id"), int) and d["id"] >= 100:
535+
answered.add(d["id"])
536+
if r.returncode == 0 and len(answered) == N:
537+
print("OK")
538+
else:
539+
print(f"FAIL: rc={r.returncode} answered={len(answered)}/{N} stdout_bytes={len(r.stdout)}")
540+
PYPIPE
541+
)
542+
case "$PIPELINE" in
543+
OK) echo "OK: 24 pipelined tool calls all answered, clean exit" ;;
544+
*) echo "$PIPELINE"; exit 1 ;;
545+
esac
546+
547+
echo "=== Phase 3z3: config get prints real defaults and rejects unknown keys (#1522) ==="
548+
CFG_HOME=$(mktemp -d)
549+
CFG_WATCH=$(CBM_CACHE_DIR="$CFG_HOME" "$BINARY" config get auto_watch 2>/dev/null || true)
550+
if [ "$CFG_WATCH" != "true" ] && [ "$CFG_WATCH" != "false" ]; then
551+
echo "FAIL: config get auto_watch printed '$CFG_WATCH' (expected the stored value or the default 'true')"
552+
rm -rf "$CFG_HOME"; exit 1
553+
fi
554+
if CBM_CACHE_DIR="$CFG_HOME" "$BINARY" config get totally_bogus_key >/dev/null 2>&1; then
555+
echo "FAIL: config get of an unknown key exited 0"
556+
rm -rf "$CFG_HOME"; exit 1
557+
fi
558+
rm -rf "$CFG_HOME"
559+
echo "OK: config get returns defaults and errors on unknown keys"
560+
451561
echo "OK: search_graph found $TOTAL result(s) for 'compute'"
452562

453563
# 3b: trace_path — verify compute has callers

‎src/cli/cli.c‎

Lines changed: 61 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6510,6 +6510,43 @@ int cbm_config_delete(cbm_config_t *cfg, const char *key) {
65106510

65116511
/* ── Config CLI subcommand ────────────────────────────────────── */
65126512

6513+
/* THE config-key table. list, get, help, and key validation all read this one
6514+
* definition, so a key cannot exist with different defaults in different
6515+
* subcommands again: `config list` used to print stored-or-DEFAULT while
6516+
* `config get` printed stored-or-EMPTY — every unset key (and every typo'd
6517+
* key, at exit 0) "got" an empty string, indistinguishable from a real value
6518+
* (#1522). The defaults here mirror the runtime readers' fallbacks
6519+
* (cbm_config_get_bool/int call sites); keep them in sync. */
6520+
typedef struct {
6521+
const char *key;
6522+
const char *default_value;
6523+
const char *help;
6524+
} config_key_def_t;
6525+
6526+
static const config_key_def_t CONFIG_KEYS[] = {
6527+
{CBM_CONFIG_AUTO_INDEX, "false", "Enable auto-indexing on MCP session start"},
6528+
{CBM_CONFIG_AUTO_INDEX_LIMIT, "50000", "Max files for auto-indexing new projects"},
6529+
{CBM_CONFIG_AUTO_WATCH, "true", "Register background git watcher on session connect"},
6530+
{CBM_CONFIG_UI_LANG, "auto", "Pin graph UI language: en, zh, or auto"},
6531+
};
6532+
6533+
static const config_key_def_t *config_key_lookup(const char *key) {
6534+
for (size_t i = 0; key && i < sizeof(CONFIG_KEYS) / sizeof(CONFIG_KEYS[0]); i++) {
6535+
if (strcmp(CONFIG_KEYS[i].key, key) == 0) {
6536+
return &CONFIG_KEYS[i];
6537+
}
6538+
}
6539+
return NULL;
6540+
}
6541+
6542+
static void config_print_unknown_key(const char *key) {
6543+
(void)fprintf(stderr, "error: unknown config key: %s\nKnown keys:", key ? key : "");
6544+
for (size_t i = 0; i < sizeof(CONFIG_KEYS) / sizeof(CONFIG_KEYS[0]); i++) {
6545+
(void)fprintf(stderr, " %s", CONFIG_KEYS[i].key);
6546+
}
6547+
(void)fprintf(stderr, "\n");
6548+
}
6549+
65136550
int cbm_cmd_config(int argc, char **argv) {
65146551
/* NULL argv with a nonzero argc previously slipped past this guard (the
65156552
* inner `argv &&` shielded only the help comparison) and dereferenced
@@ -6522,14 +6559,10 @@ int cbm_cmd_config(int argc, char **argv) {
65226559
printf(" set <key> <val> Set a config value\n");
65236560
printf(" reset <key> Reset a key to default\n\n");
65246561
printf("Config keys:\n");
6525-
printf(" %-25s default=%-10s %s\n", CBM_CONFIG_AUTO_INDEX, "false",
6526-
"Enable auto-indexing on MCP session start");
6527-
printf(" %-25s default=%-10s %s\n", CBM_CONFIG_AUTO_INDEX_LIMIT, "50000",
6528-
"Max files for auto-indexing new projects");
6529-
printf(" %-25s default=%-10s %s\n", CBM_CONFIG_AUTO_WATCH, "true",
6530-
"Register background git watcher on session connect");
6531-
printf(" %-25s default=%-10s %s\n", CBM_CONFIG_UI_LANG, "auto",
6532-
"Pin graph UI language: en, zh, or auto");
6562+
for (size_t i = 0; i < sizeof(CONFIG_KEYS) / sizeof(CONFIG_KEYS[0]); i++) {
6563+
printf(" %-25s default=%-10s %s\n", CONFIG_KEYS[i].key, CONFIG_KEYS[i].default_value,
6564+
CONFIG_KEYS[i].help);
6565+
}
65336566
return 0;
65346567
}
65356568

@@ -6551,25 +6584,33 @@ int cbm_cmd_config(int argc, char **argv) {
65516584
int rc = 0;
65526585
if (strcmp(argv[0], "list") == 0 || strcmp(argv[0], "ls") == 0) {
65536586
printf("Configuration:\n");
6554-
printf(" %-25s = %-10s\n", CBM_CONFIG_AUTO_INDEX,
6555-
cbm_config_get(cfg, CBM_CONFIG_AUTO_INDEX, "false"));
6556-
printf(" %-25s = %-10s\n", CBM_CONFIG_AUTO_INDEX_LIMIT,
6557-
cbm_config_get(cfg, CBM_CONFIG_AUTO_INDEX_LIMIT, "50000"));
6558-
printf(" %-25s = %-10s\n", CBM_CONFIG_AUTO_WATCH,
6559-
cbm_config_get(cfg, CBM_CONFIG_AUTO_WATCH, "true"));
6560-
printf(" %-25s = %-10s\n", CBM_CONFIG_UI_LANG,
6561-
cbm_config_get(cfg, CBM_CONFIG_UI_LANG, "auto"));
6587+
for (size_t i = 0; i < sizeof(CONFIG_KEYS) / sizeof(CONFIG_KEYS[0]); i++) {
6588+
printf(" %-25s = %-10s\n", CONFIG_KEYS[i].key,
6589+
cbm_config_get(cfg, CONFIG_KEYS[i].key, CONFIG_KEYS[i].default_value));
6590+
}
65626591
} else if (strcmp(argv[0], "get") == 0) {
65636592
if (argc < MIN_ARGC_GET) {
65646593
(void)fprintf(stderr, "Usage: config get <key>\n");
65656594
rc = CLI_TRUE;
65666595
} else {
6567-
printf("%s\n", cbm_config_get(cfg, argv[CLI_SKIP_ONE], ""));
6596+
const config_key_def_t *def = config_key_lookup(argv[CLI_SKIP_ONE]);
6597+
if (!def) {
6598+
config_print_unknown_key(argv[CLI_SKIP_ONE]);
6599+
rc = CLI_TRUE;
6600+
} else {
6601+
/* Stored value or the key's real default — the same fallback
6602+
* the runtime readers use. `""` here was #1522's bug 2: every
6603+
* unset key read as empty with exit 0. */
6604+
printf("%s\n", cbm_config_get(cfg, def->key, def->default_value));
6605+
}
65686606
}
65696607
} else if (strcmp(argv[0], "set") == 0) {
65706608
if (argc < MIN_ARGC_CMD) {
65716609
(void)fprintf(stderr, "Usage: config set <key> <value>\n");
65726610
rc = CLI_TRUE;
6611+
} else if (!config_key_lookup(argv[CLI_SKIP_ONE])) {
6612+
config_print_unknown_key(argv[CLI_SKIP_ONE]);
6613+
rc = CLI_TRUE;
65736614
} else {
65746615
if (cbm_config_set(cfg, argv[CLI_SKIP_ONE], argv[CLI_PAIR_LEN]) == 0) {
65756616
printf("%s = %s\n", argv[CLI_SKIP_ONE], argv[CLI_PAIR_LEN]);
@@ -6582,6 +6623,9 @@ int cbm_cmd_config(int argc, char **argv) {
65826623
if (argc < MIN_ARGC_GET) {
65836624
(void)fprintf(stderr, "Usage: config reset <key>\n");
65846625
rc = CLI_TRUE;
6626+
} else if (!config_key_lookup(argv[CLI_SKIP_ONE])) {
6627+
config_print_unknown_key(argv[CLI_SKIP_ONE]);
6628+
rc = CLI_TRUE;
65856629
} else {
65866630
cbm_config_delete(cfg, argv[CLI_SKIP_ONE]);
65876631
printf("%s reset to default\n", argv[CLI_SKIP_ONE]);

‎src/daemon/frontend.c‎

Lines changed: 47 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -492,33 +492,56 @@ static bool frontend_enqueue(frontend_state_t *state, char *message, bool conten
492492
return false;
493493
}
494494
}
495-
cbm_mutex_lock(&state->mutex);
496-
bool stopped = state->stopping || state->failed;
497-
bool capacity = state->count < FRONTEND_QUEUE_CAPACITY &&
498-
state->queued_bytes <= FRONTEND_QUEUE_BYTES_MAX &&
499-
length <= FRONTEND_QUEUE_BYTES_MAX - state->queued_bytes;
500-
if (!stopped && capacity) {
501-
size_t tail = (state->head + state->count) % FRONTEND_QUEUE_CAPACITY;
502-
state->queue[tail] = (frontend_item_t){
503-
.message = message,
504-
.length = length,
505-
.content_length_framed = content_length_framed,
506-
.has_id = has_id,
507-
.id = id,
508-
.id_str = id_str,
509-
};
510-
state->count++;
511-
state->queued_bytes += length;
495+
/* A frame that exceeds the whole byte budget can never be admitted no
496+
* matter how long we wait; only that case is a hard failure. */
497+
if (length > FRONTEND_QUEUE_BYTES_MAX) {
498+
cbm_mutex_lock(&state->mutex);
499+
if (!state->stopping && !state->failed) {
500+
state->failed = true;
501+
state->stopping = true;
502+
}
512503
cbm_mutex_unlock(&state->mutex);
513-
return true;
504+
free(id_str);
505+
return false;
514506
}
515-
if (!stopped) {
516-
state->failed = true;
517-
state->stopping = true;
507+
/* A full queue is BACKPRESSURE, not failure. This runs on the stdin reader
508+
* thread, so blocking here stops further reads and lets the kernel pipe
509+
* absorb the client's burst — a pipelining client is a client going fast,
510+
* not a client going wrong. The old code failed the whole session at frame
511+
* capacity+1, killing it with every buffered response unwritten: any 7+
512+
* pipelined requests (e.g. an agent issuing parallel tool calls) died with
513+
* rc=1 and zero output (#1522 follow-up). Waits are bounded only by the
514+
* stop/fail flags, which every teardown path already sets — the same
515+
* condition the polling worker and watchdogs key on. */
516+
for (;;) {
517+
cbm_mutex_lock(&state->mutex);
518+
bool stopped = state->stopping || state->failed;
519+
if (stopped) {
520+
cbm_mutex_unlock(&state->mutex);
521+
free(id_str);
522+
return false;
523+
}
524+
bool capacity = state->count < FRONTEND_QUEUE_CAPACITY &&
525+
state->queued_bytes <= FRONTEND_QUEUE_BYTES_MAX &&
526+
length <= FRONTEND_QUEUE_BYTES_MAX - state->queued_bytes;
527+
if (capacity) {
528+
size_t tail = (state->head + state->count) % FRONTEND_QUEUE_CAPACITY;
529+
state->queue[tail] = (frontend_item_t){
530+
.message = message,
531+
.length = length,
532+
.content_length_framed = content_length_framed,
533+
.has_id = has_id,
534+
.id = id,
535+
.id_str = id_str,
536+
};
537+
state->count++;
538+
state->queued_bytes += length;
539+
cbm_mutex_unlock(&state->mutex);
540+
return true;
541+
}
542+
cbm_mutex_unlock(&state->mutex);
543+
cbm_usleep(FRONTEND_WAIT_US);
518544
}
519-
cbm_mutex_unlock(&state->mutex);
520-
free(id_str);
521-
return false;
522545
}
523546

524547
static bool frontend_stop_begin(frontend_state_t *state) {

0 commit comments

Comments
 (0)