fix(mcp): align hidden tool payload

Share MCP tool-visibility predicates between tools/list and _hidden_tools so configured advanced tools are reported as already visible instead of still hidden. Build the _hidden_tools payload with yyjson and keep the progressive reveal/list_changed behavior intact.\n\nAdd a focused configured-tool regression and validate with tool_consolidation, mcp, production build, source-safety, diff check, and bounded MCP protocol replay. Signed-off-by: Andrew Hundt <ATHundt@gmail.com>
2026-10-30 07:23:47 +08:00 · 2026-06-30 08:38:11 -04:00
parent 47ad730a23
commit e63b05a076
2 changed files with 191 additions and 30 deletions
@@ -669,6 +669,13 @@ static void emit_tool(yyjson_mut_doc *doc, yyjson_mut_val *tools, const tool_def
    yyjson_mut_arr_add_val(tools, tool);
 }

+static bool is_streamlined_default_tool(const char *name) {
+    return name && (strcmp(name, "search_graph") == 0 ||
+                    strcmp(name, "query_graph") == 0 ||
+                    strcmp(name, "search_code") == 0 ||
+                    strcmp(name, "trace_path") == 0);
+}
+
 /* cbm_mcp_tools_list() defined after struct cbm_mcp_server (needs full type) */

 /* Supported protocol versions, newest first. The server picks the newest
@@ -963,6 +970,40 @@ struct cbm_mcp_server {
    int64_t active_request_id;       /* JSON-RPC id of the in-progress tool call */
 };

+static bool cbm_mcp_tool_mode_is_classic(cbm_mcp_server_t *srv) {
+    /* Env var keeps script/test overrides independent from the persisted config. */
+    char tool_mode_buf[CBM_SZ_64];
+    const char *tool_mode = cbm_safe_getenv("CBM_TOOL_MODE", tool_mode_buf,
+                                            sizeof(tool_mode_buf), NULL);
+    if (tool_mode && tool_mode[0] != '\0') {
+        return strcmp(tool_mode, "classic") == 0;
+    }
+    tool_mode = (srv && srv->config)
+        ? cbm_config_get(srv->config, CBM_CONFIG_TOOL_MODE, "streamlined")
+        : "streamlined";
+    return strcmp(tool_mode, "classic") == 0;
+}
+
+static bool cbm_mcp_tool_config_enabled(cbm_mcp_server_t *srv, const char *tool_name) {
+    if (!srv || !srv->config || !tool_name) {
+        return false;
+    }
+    char key[CBM_SZ_64];
+    int n = snprintf(key, sizeof(key), "tool_%s", tool_name);
+    if (n < 0 || (size_t)n >= sizeof(key)) {
+        return false;
+    }
+    return cbm_config_get_bool(srv->config, key, false);
+}
+
+static bool cbm_mcp_advanced_tool_visible(cbm_mcp_server_t *srv, const char *tool_name) {
+    if (cbm_mcp_tool_mode_is_classic(srv)) {
+        return true;
+    }
+    return (srv && srv->hidden_tools_revealed) ||
+           cbm_mcp_tool_config_enabled(srv, tool_name);
+}
+
 static int cbm_mcp_config_int_clamped(cbm_mcp_server_t *srv, const char *key, int default_val,
                                      int min_val, int max_val) {
    int value = srv && srv->config ? cbm_config_get_int(srv->config, key, default_val) : default_val;
@@ -1070,14 +1111,7 @@ static bool cbm_mcp_run_sync_auto_index(cbm_mcp_server_t *srv, const char *root_
 /* ── Tool list (needs full struct definition above) ──────────── */

 char *cbm_mcp_tools_list(cbm_mcp_server_t *srv) {
-    /* Env var CBM_TOOL_MODE overrides config (for backwards compat without config store) */
-    const char *tool_mode = getenv("CBM_TOOL_MODE");
-    if (!tool_mode || tool_mode[0] == '\0') {
-        tool_mode = (srv && srv->config)
-            ? cbm_config_get(srv->config, CBM_CONFIG_TOOL_MODE, "streamlined")
-            : "streamlined";
-    }
-    bool classic = (strcmp(tool_mode, "classic") == 0);
+    bool classic = cbm_mcp_tool_mode_is_classic(srv);
    bool reveal_hidden = (!classic && srv && srv->hidden_tools_revealed);

    yyjson_mut_doc *doc = yyjson_mut_doc_new(NULL);
@@ -1091,10 +1125,7 @@ char *cbm_mcp_tools_list(cbm_mcp_server_t *srv) {
         * TOOLS[] plus fork-only aliases from STREAMLINED_TOOLS[]. Keeping
         * canonical tools in TOOLS[] prevents schema drift between modes. */
        for (int i = 0; i < TOOL_COUNT; i++) {
-            if (strcmp(TOOLS[i].name, "search_graph") == 0 ||
-                strcmp(TOOLS[i].name, "query_graph") == 0 ||
-                strcmp(TOOLS[i].name, "search_code") == 0 ||
-                strcmp(TOOLS[i].name, "trace_path") == 0) {
+            if (is_streamlined_default_tool(TOOLS[i].name)) {
                emit_tool(doc, tools, &TOOLS[i]);
            }
        }
@@ -1108,16 +1139,10 @@ char *cbm_mcp_tools_list(cbm_mcp_server_t *srv) {
         * trace_path is already listed from TOOLS[] above, so skip it here.
         * (get_code is streamlined-only; get_code_snippet is the TOOLS[] name.) */
        for (int i = 0; i < TOOL_COUNT; i++) {
-            if (strcmp(TOOLS[i].name, "search_graph") == 0 ||
-                strcmp(TOOLS[i].name, "query_graph") == 0 ||
-                strcmp(TOOLS[i].name, "search_code") == 0 ||
-                strcmp(TOOLS[i].name, "trace_path") == 0) {
+            if (is_streamlined_default_tool(TOOLS[i].name)) {
                continue;
            }
-            char key[64];
-            snprintf(key, sizeof(key), "tool_%s", TOOLS[i].name);
-            if (reveal_hidden ||
-                (srv && srv->config && cbm_config_get_bool(srv->config, key, false))) {
+            if (reveal_hidden || cbm_mcp_tool_config_enabled(srv, TOOLS[i].name)) {
                emit_tool(doc, tools, &TOOLS[i]);
            }
        }
@@ -6901,6 +6926,55 @@ static char *handle_index_dependencies(cbm_mcp_server_t *srv, const char *args)

 /* ── Tool dispatch ────────────────────────────────────────────── */

+static char *build_hidden_tools_payload(cbm_mcp_server_t *srv) {
+    yyjson_mut_doc *doc = yyjson_mut_doc_new(NULL);
+    if (!doc) {
+        return heap_strdup("{\"error\":\"failed to build hidden tools payload\"}");
+    }
+
+    yyjson_mut_val *root = yyjson_mut_obj(doc);
+    yyjson_mut_doc_set_root(doc, root);
+
+    yyjson_mut_val *advanced = yyjson_mut_arr(doc);
+    yyjson_mut_val *hidden = yyjson_mut_arr(doc);
+    yyjson_mut_val *visible = yyjson_mut_arr(doc);
+
+    for (int i = 0; i < TOOL_COUNT; i++) {
+        const char *name = TOOLS[i].name;
+        if (is_streamlined_default_tool(name)) {
+            continue;
+        }
+
+        yyjson_mut_arr_add_str(doc, advanced, name);
+        if (cbm_mcp_advanced_tool_visible(srv, name)) {
+            yyjson_mut_arr_add_str(doc, visible, name);
+        } else {
+            yyjson_mut_arr_add_str(doc, hidden, name);
+        }
+    }
+
+    yyjson_mut_obj_add_val(doc, root, "advanced_tools", advanced);
+    yyjson_mut_obj_add_val(doc, root, "hidden_tools", hidden);
+    yyjson_mut_obj_add_val(doc, root, "already_visible_tools", visible);
+    yyjson_mut_obj_add_bool(doc, root, "revealed", true);
+    yyjson_mut_obj_add_str(doc, root, "next_step",
+                           "call tools/list again; hidden tools are now advertised for this MCP server process");
+    yyjson_mut_obj_add_str(doc, root, "enable_all",
+                           "set env CBM_TOOL_MODE=classic or config set tool_mode classic");
+    yyjson_mut_obj_add_str(doc, root, "enable_one",
+                           "config set tool_<name> true (e.g. tool_index_repository true)");
+
+    yyjson_mut_val *resources = yyjson_mut_arr(doc);
+    yyjson_mut_arr_add_str(doc, resources, "codebase://schema");
+    yyjson_mut_arr_add_str(doc, resources, "codebase://architecture");
+    yyjson_mut_arr_add_str(doc, resources, "codebase://status");
+    yyjson_mut_obj_add_val(doc, root, "resources", resources);
+
+    char *out = yy_doc_to_str(doc);
+    yyjson_mut_doc_free(doc);
+    return out ? out : heap_strdup("{\"error\":\"failed to serialize hidden tools payload\"}");
+}
+
 char *cbm_mcp_handle_tool(cbm_mcp_server_t *srv, const char *tool_name, const char *args_json) {
    if (!tool_name) {
        return cbm_mcp_text_result(
@@ -6968,22 +7042,16 @@ char *cbm_mcp_handle_tool(cbm_mcp_server_t *srv, const char *tool_name, const ch
    /* _hidden_tools: informational pseudo-tool for progressive disclosure */
    if (strcmp(tool_name, "_hidden_tools") == 0) {
        bool changed = srv && !srv->hidden_tools_revealed;
+        char *payload = build_hidden_tools_payload(srv);
        if (srv) {
            srv->hidden_tools_revealed = true;
        }
        if (changed) {
            send_notification(srv, "notifications/tools/list_changed");
        }
-        return cbm_mcp_text_result(
-            "{\"hidden_tools\":[\"index_repository\",\"get_code_snippet\","
-            "\"get_graph_schema\",\"get_architecture\",\"list_projects\","
-            "\"delete_project\",\"index_status\",\"detect_changes\","
-            "\"manage_adr\",\"ingest_traces\",\"index_dependencies\"],"
-            "\"revealed\":true,"
-            "\"next_step\":\"call tools/list again; these tools are now advertised for this MCP server process\","
-            "\"enable_all\":\"set env CBM_TOOL_MODE=classic or config set tool_mode classic\","
-            "\"enable_one\":\"config set tool_<name> true (e.g. tool_index_repository true)\","
-            "\"resources\":[\"codebase://schema\",\"codebase://architecture\",\"codebase://status\"]}", false);
+        char *result = cbm_mcp_text_result(payload, false);
+        free(payload);
+        return result;
    }

    char msg[512];
@@ -9,7 +9,10 @@
 */
 #include "../src/foundation/compat.h"
 #include "../src/foundation/compat_fs.h"
+#include "../src/foundation/constants.h"
+#include "test_helpers.h"
 #include "test_framework.h"
+#include <cli/cli.h>
 #include <mcp/mcp.h>
 #include <store/store.h>
 #include <depindex/depindex.h>
@@ -141,6 +144,48 @@ static void restore_tool_mode(char *saved) {
    }
 }

+static char *extract_tool_text(const char *mcp_result) {
+    yyjson_doc *doc = yyjson_read(mcp_result, strlen(mcp_result), 0);
+    if (!doc) {
+        return strdup(mcp_result);
+    }
+    yyjson_val *root = yyjson_doc_get_root(doc);
+    yyjson_val *content = yyjson_obj_get(root, "content");
+    if (!content || !yyjson_is_arr(content)) {
+        yyjson_doc_free(doc);
+        return strdup(mcp_result);
+    }
+    yyjson_val *item = yyjson_arr_get(content, 0);
+    yyjson_val *text = item ? yyjson_obj_get(item, "text") : NULL;
+    const char *str = text && yyjson_is_str(text) ? yyjson_get_str(text) : mcp_result;
+    char *copy = strdup(str);
+    yyjson_doc_free(doc);
+    return copy;
+}
+
+static bool json_array_has_string(const char *json, const char *array_key, const char *value) {
+    yyjson_doc *doc = yyjson_read(json, strlen(json), 0);
+    if (!doc) {
+        return false;
+    }
+    yyjson_val *root = yyjson_doc_get_root(doc);
+    yyjson_val *arr = yyjson_obj_get(root, array_key);
+    bool found = false;
+    if (arr && yyjson_is_arr(arr)) {
+        yyjson_arr_iter it;
+        yyjson_arr_iter_init(arr, &it);
+        yyjson_val *item;
+        while ((item = yyjson_arr_iter_next(&it)) != NULL) {
+            if (yyjson_is_str(item) && strcmp(yyjson_get_str(item), value) == 0) {
+                found = true;
+                break;
+            }
+        }
+    }
+    yyjson_doc_free(doc);
+    return found;
+}
+
 /* ── 1. Tool visibility tests ─────────────────────────────── */

 TEST(streamlined_mode_shows_default_user_tools) {
@@ -276,6 +321,53 @@ TEST(hidden_tools_reveal_discoverable_tools) {
    PASS();
 }

+TEST(hidden_tools_payload_excludes_already_visible_configured_tools) {
+    char *saved_mode = save_tool_mode();
+    setenv("CBM_TOOL_MODE", "streamlined", 1);
+
+    char *tmp = th_mktempdir("cbm_hidden_tools_cfg");
+    ASSERT_NOT_NULL(tmp);
+    char cfg_dir[CBM_SZ_512];
+    int n = snprintf(cfg_dir, sizeof(cfg_dir), "%s", tmp);
+    ASSERT_TRUE(n > 0 && (size_t)n < sizeof(cfg_dir));
+
+    cbm_config_t *cfg = cbm_config_open(cfg_dir);
+    ASSERT_NOT_NULL(cfg);
+    ASSERT_EQ(cbm_config_set(cfg, "tool_index_repository", "true"), 0);
+
+    cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
+    ASSERT_NOT_NULL(srv);
+    cbm_mcp_server_set_config(srv, cfg);
+
+    char *before = cbm_mcp_tools_list(srv);
+    ASSERT_NOT_NULL(before);
+    ASSERT(tool_list_has_exact_name(before, "index_repository"));
+    ASSERT(!tool_list_has_exact_name(before, "get_architecture"));
+    free(before);
+
+    char *hint = cbm_mcp_handle_tool(srv, "_hidden_tools", "{}");
+    ASSERT_NOT_NULL(hint);
+    char *text = extract_tool_text(hint);
+    ASSERT_NOT_NULL(text);
+    ASSERT(json_array_has_string(text, "advanced_tools", "index_repository"));
+    ASSERT(json_array_has_string(text, "already_visible_tools", "index_repository"));
+    ASSERT(!json_array_has_string(text, "hidden_tools", "index_repository"));
+    ASSERT(json_array_has_string(text, "hidden_tools", "get_architecture"));
+    free(text);
+    free(hint);
+
+    char *after = cbm_mcp_tools_list(srv);
+    ASSERT_NOT_NULL(after);
+    ASSERT(tool_list_has_exact_name(after, "get_architecture"));
+    ASSERT_EQ(17, tool_list_exact_count(after));
+    free(after);
+
+    cbm_mcp_server_free(srv);
+    cbm_config_close(cfg);
+    restore_tool_mode(saved_mode);
+    PASS();
+}
+
 TEST(streamlined_reveal_covers_classic_capabilities) {
    char *saved_mode = save_tool_mode();

@@ -2527,6 +2619,7 @@ SUITE(tool_consolidation) {
    RUN_TEST(api_surface_default_streamlined_regression_gate);
    RUN_TEST(api_surface_classic_regression_gate);
    RUN_TEST(hidden_tools_reveal_discoverable_tools);
+    RUN_TEST(hidden_tools_payload_excludes_already_visible_configured_tools);
    RUN_TEST(streamlined_reveal_covers_classic_capabilities);
    RUN_TEST(streamlined_core_parameter_contract);
    RUN_TEST(revealed_trace_path_parameter_contract);