From 53f302f125adf89d5c4cb133737dbcafeb033dcb Mon Sep 17 00:00:00 2001 From: Kirchlive Date: Mon, 22 Jun 2026 19:30:16 +0200 Subject: [PATCH 1/3] fix(get_architecture): treat "overview" as alias for "all" `aspects=["overview"]` silently returned only `{total_nodes, total_edges}` because "overview" was never registered in either guard: - `want_aspect` (src/store/store.c) skipped all DB sub-queries - `aspect_wanted` (src/mcp/mcp.c) skipped all JSON serialization The JSON schema (mcp.c) accepts `aspects` as free-form strings without an enum, so "overview" was silently accepted and produced an empty result instead of a validation error. This 2-line patch treats "overview" as a synonym for "all" in both guards, restoring useful output for the most natural exploratory aspect name. Follow-up (not in this PR): add an enum constraint to the `aspects` JSON schema so unknown aspect strings raise a validation error instead of returning empty. --- src/mcp/mcp.c | 2 +- src/store/store.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 8102b1e7..6ee8394b 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -1841,7 +1841,7 @@ static bool aspect_wanted(yyjson_doc *aspects_doc, yyjson_val *aspects_arr, cons yyjson_val *val; while ((val = yyjson_arr_iter_next(&iter)) != NULL) { const char *s = yyjson_get_str(val); - if (s && (strcmp(s, "all") == 0 || strcmp(s, name) == 0)) { + if (s && (strcmp(s, "all") == 0 || strcmp(s, "overview") == 0 || strcmp(s, name) == 0)) { return true; } } diff --git a/src/store/store.c b/src/store/store.c index c237332e..283146fb 100644 --- a/src/store/store.c +++ b/src/store/store.c @@ -4936,7 +4936,7 @@ static bool want_aspect(const char **aspects, int aspect_count, const char *name return true; } for (int i = 0; i < aspect_count; i++) { - if (strcmp(aspects[i], "all") == 0) { + if (strcmp(aspects[i], "all") == 0 || strcmp(aspects[i], "overview") == 0) { return true; } if (strcmp(aspects[i], name) == 0) { From aa4195eea4b4621c838df169d41dcd5119ed1f79 Mon Sep 17 00:00:00 2001 From: Kirchlive Date: Mon, 22 Jun 2026 21:02:00 +0200 Subject: [PATCH 2/3] refactor(get_architecture): make "overview" a compact subset + add aspects schema enum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Following review feedback on the initial alias approach: making "overview" identical to "all" still produces the same ~30k-char overflow payload (file_tree alone is ~275 entries). A useful "overview" needs to be a true compact subset. Changes: 1. "overview" is now a compact subset = every aspect EXCEPT file_tree. A small static helper `aspect_in_overview(name)` (in both store.c and mcp.c) gates which aspects "overview" expands to. Replacing it with a per-aspect block-list (e.g. also dropping clusters) is trivial later. 2. The aspects parameter now has a JSON Schema enum, so unknown aspect strings raise a validation error instead of being silently accepted (the previous footgun that masked the alias-not-registered bug for so long). Behavior: aspects=["all"] → unchanged, includes file_tree aspects=["overview"] → all aspects EXCEPT file_tree, stays under cap aspects=["bogus"] → validation error (was: silent empty response) aspects omitted → unchanged (= all) --- src/mcp/mcp.c | 23 ++++++++++++++++++----- src/store/store.c | 16 ++++++++++------ 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 6ee8394b..35a05be1 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -391,7 +391,11 @@ static const tool_def_t TOOLS[] = { "representative top_nodes, and the packages/edge_types that bind it) — use these to grasp " "the real architectural seams, which often cut across the folder layout.", "{\"type\":\"object\",\"properties\":{\"project\":{\"type\":\"string\"},\"aspects\":{\"type\":" - "\"array\",\"items\":{\"type\":\"string\"}}},\"required\":[\"project\"]}"}, + "\"array\",\"items\":{\"type\":\"string\",\"enum\":[\"all\",\"overview\",\"structure\"," + "\"dependencies\",\"routes\",\"languages\",\"packages\",\"entry_points\",\"hotspots\"," + "\"boundaries\",\"layers\",\"file_tree\",\"clusters\"]},\"description\":\"Aspects to " + "include. 'all' = everything; 'overview' = compact summary (all except file_tree); " + "omit = all.\"}},\"required\":[\"project\"]}"}, {"search_code", "Graph-augmented code search. Finds text patterns via grep, then enriches results with " @@ -1831,7 +1835,15 @@ static char *handle_delete_project(cbm_mcp_server_t *srv, const char *args) { return result; } -/* Check if an aspect is requested (NULL aspects = all, or array contains "all" or the name). */ +/* "overview" = compact architecture summary: every aspect EXCEPT the large + per-file listing (file_tree), which alone dominates the payload (~275 entries) + and pushes the response past MAX_MCP_OUTPUT_TOKENS. */ +static bool aspect_in_overview(const char *name) { + return strcmp(name, "file_tree") != 0; +} + +/* Check if an aspect is requested. NULL aspects = all. Array can contain "all" + * (everything), "overview" (everything except file_tree), or the aspect name. */ static bool aspect_wanted(yyjson_doc *aspects_doc, yyjson_val *aspects_arr, const char *name) { if (!aspects_arr) { return true; /* no filter = all */ @@ -1841,9 +1853,10 @@ static bool aspect_wanted(yyjson_doc *aspects_doc, yyjson_val *aspects_arr, cons yyjson_val *val; while ((val = yyjson_arr_iter_next(&iter)) != NULL) { const char *s = yyjson_get_str(val); - if (s && (strcmp(s, "all") == 0 || strcmp(s, "overview") == 0 || strcmp(s, name) == 0)) { - return true; - } + if (!s) continue; + if (strcmp(s, "all") == 0) return true; + if (strcmp(s, "overview") == 0 && aspect_in_overview(name)) return true; + if (strcmp(s, name) == 0) return true; } (void)aspects_doc; return false; diff --git a/src/store/store.c b/src/store/store.c index 283146fb..55bdf445 100644 --- a/src/store/store.c +++ b/src/store/store.c @@ -4931,17 +4931,21 @@ static int arch_clusters(cbm_store_t *s, const char *project, cbm_architecture_i /* ── GetArchitecture dispatch ──────────────────────────────────── */ +/* "overview" = compact architecture summary: every aspect EXCEPT the large + per-file listing (file_tree), which alone dominates the payload (~275 entries) + and pushes the response past MAX_MCP_OUTPUT_TOKENS. */ +static bool aspect_in_overview(const char *name) { + return strcmp(name, "file_tree") != 0; +} + static bool want_aspect(const char **aspects, int aspect_count, const char *name) { if (!aspects || aspect_count == 0) { return true; } for (int i = 0; i < aspect_count; i++) { - if (strcmp(aspects[i], "all") == 0 || strcmp(aspects[i], "overview") == 0) { - return true; - } - if (strcmp(aspects[i], name) == 0) { - return true; - } + if (strcmp(aspects[i], "all") == 0) return true; + if (strcmp(aspects[i], "overview") == 0 && aspect_in_overview(name)) return true; + if (strcmp(aspects[i], name) == 0) return true; } return false; } From 6fd7fb782b86264084c0391e1facf1174c278fef Mon Sep 17 00:00:00 2001 From: Kirchlive Date: Mon, 22 Jun 2026 21:11:33 +0200 Subject: [PATCH 3/3] feat(get_architecture): server-side aspect validation (defense in depth) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The JSON-Schema enum added in the previous commit is advisory: it only fires when the MCP client validates client-side before sending. Many clients (including Claude Code) do not, so an unknown aspect like ["bogus"] still produced a silent-empty response — exactly the footgun that hid the original alias-not-registered bug. This commit adds the missing authoritative layer: server-side validation in handle_get_architecture(). A single SSOT array VALID_ASPECTS[] backs both the enum (advisory, client-side) and aspect_is_valid() (authoritative, server-side). Any future aspect addition stays in sync by editing one array. On an unknown aspect, the server returns isError:true with a model-friendly message listing the valid values, so an agent can self-correct without re-prompting: Unknown aspect 'bogus'. Valid: all, overview, structure, dependencies, routes, languages, packages, entry_points, hotspots, boundaries, layers, file_tree, clusters. Defense in depth — clients that DO validate still get a clean client-side error; clients that don't are now caught by the server. Cleanup: properly frees project and aspects_doc on the early-error path. --- src/mcp/mcp.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 35a05be1..2eb4f64f 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -1835,6 +1835,27 @@ static char *handle_delete_project(cbm_mcp_server_t *srv, const char *args) { return result; } +/* Canonical list of valid aspect tokens for get_architecture. + * SSOT consumed by both the JSON-Schema enum (advisory, client-side) + * and the server-side validation (authoritative). Update both together + * if the aspect set changes. */ +static const char *VALID_ASPECTS[] = { + "all", "overview", + "structure", "dependencies", "routes", + "languages", "packages", "entry_points", + "hotspots", "boundaries", "layers", + "file_tree", "clusters", + NULL +}; + +static bool aspect_is_valid(const char *name) { + if (!name) return false; + for (int i = 0; VALID_ASPECTS[i]; i++) { + if (strcmp(name, VALID_ASPECTS[i]) == 0) return true; + } + return false; +} + /* "overview" = compact architecture summary: every aspect EXCEPT the large per-file listing (file_tree), which alone dominates the payload (~275 entries) and pushes the response past MAX_MCP_OUTPUT_TOKENS. */ @@ -1929,6 +1950,25 @@ static char *handle_get_architecture(cbm_mcp_server_t *srv, const char *args) { } } + /* Server-side validation: reject unknown aspect tokens with a model-friendly + * error listing the valid values. The JSON Schema enum on this tool is + * advisory — not every MCP client validates client-side (Claude Code does + * not), so silent-empty was the failure mode before this check. */ + for (int i = 0; i < aspects_strs_count; i++) { + if (!aspect_is_valid(aspects_strs[i])) { + char msg[512]; + snprintf(msg, sizeof(msg), + "Unknown aspect '%s'. Valid: all, overview, structure, " + "dependencies, routes, languages, packages, entry_points, " + "hotspots, boundaries, layers, file_tree, clusters.", + aspects_strs[i]); + char *err = cbm_mcp_text_result(msg, true); + free(project); + if (aspects_doc) yyjson_doc_free(aspects_doc); + return err; + } + } + cbm_schema_info_t schema = {0}; /* Counts-only: this handler renders label/type counts but never property * keys, and full key discovery json_each-scans every row (seconds-to-