From 5a666f2fa3431a3389027a3428e37816f7208f77 Mon Sep 17 00:00:00 2001 From: Shane McCarron Date: Thu, 29 Oct 2026 11:46:23 -0500 Subject: [PATCH] fix(mcp): match search_code file_pattern with a directory against the repo-relative path search_code matched file_pattern against the file basename only, so any pattern containing a directory (src/cli/*.c, apps/web/src/**/*.ts) silently returned 0 results, while search_graph's file_pattern matches the full repo-relative path. A file_pattern containing '/' is now matched in C against the repo-relative path of every indexed file, on all platforms, with search_graph-equivalent semantics: '*' and '**' match across '/', '**/' also matches zero directories, '?' matches one character, and a pattern without wildcards is a substring match. Patterns without '/' keep the existing basename behaviour. On Windows such patterns are scanned in process from the already-filtered list instead of the PowerShell -like post-filter. A search with a '/' pattern that returns nothing now carries a hint explaining whole-path matching and pointing at path_filter, and the schema describes both parameters. Refs #2836 Signed-off-by: Shane McCarron --- src/mcp/mcp.c | 70 +++++++++++++++++++++++++++++++++++++--- tests/test_mcp.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 4 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 1fc5f4588..e80cc2377 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -729,7 +729,9 @@ static const tool_def_t TOOLS[] = { {"search_code", "Graph-ranked text search: compact symbols, full bounded source, or file paths.", "{\"type\":\"object\",\"properties\":{\"pattern\":{\"type\":\"string\"},\"project\":{\"type\":" - "\"string\"},\"file_pattern\":{\"type\":\"string\"},\"path_filter\":{\"type\":\"string\"}," + "\"string\"},\"file_pattern\":{\"type\":\"string\",\"description\":\"Glob on file name; " + "with '/', on repo-relative path (** = any dirs).\"}," + "\"path_filter\":{\"type\":\"string\",\"description\":\"Regex on repo-relative path.\"}," "\"mode\":{\"type\":\"string\"," "\"enum\":[\"compact\",\"full\",\"files\"],\"default\":\"compact\"}," "\"context\":{\"type\":\"integer\",\"default\":0,\"minimum\":0,\"maximum\":200}," @@ -14716,6 +14718,7 @@ typedef struct { uint64_t enrich_ms; uint64_t elapsed_ms; bool include_phase_timings; + bool hint_path_filter; /* a search with a '/' file_pattern returned no matches */ } search_metrics_t; #ifdef CBM_ENABLE_TEST_SEAMS @@ -14810,6 +14813,47 @@ static int search_result_cmp(const void *a, const void *b) { return 0; } +#define SEARCH_CODE_PATH_HINT \ + "no results; a file_pattern with '/' must match the whole repo-relative path (e.g. " \ + "src/**/*.c); to scope by folder you can also use path_filter (regex on the " \ + "repo-relative path, e.g. ^src/cli/)" + +/* A file_pattern containing '/' is matched against the whole repo-relative path, like + * search_graph's file_pattern: '*' and '**' match any run of characters including '/', a + * '**' followed by '/' also matches zero directories, '?' matches one character, and a + * pattern without wildcards is a substring match. Portable (no fnmatch on Windows) and + * linear: every '*' is equivalent, so one backtrack point suffices. */ +static bool search_code_path_glob_match(const char *pat, const char *path) { + if (!strpbrk(pat, "*?")) { + return strstr(path, pat) != NULL; + } + const char *star_pat = NULL; + const char *star_path = NULL; + for (;;) { + if (*pat == '*') { + const char *run = pat; + while (*pat == '*') { + pat++; + } + if (pat - run > SKIP_ONE && *pat == '/') { + pat++; + } + star_pat = pat; + star_path = path; + } else if (*path && (*pat == '?' || *pat == *path)) { + pat++; + path++; + } else if (!*pat && !*path) { + return true; + } else if (star_pat && *star_path) { + pat = star_pat; + path = ++star_path; + } else { + return false; + } + } +} + /* Moving an arbitrary file_pattern ahead of Select-String is not generally results-preserving: * the current Windows path applies PowerShell -like to the full MatchInfo.Path, while POSIX * delegates glob semantics to grep --include. Restrict the Windows optimization to plain suffix @@ -15432,6 +15476,9 @@ static char *assemble_search_output_toon(search_result_t *sr, int sr_count, grep "(not as alternation). Pass regex=true for 'foo|bar' to mean " "'foo OR bar'."); } + if (metrics->hint_path_filter) { + cbm_tree_scalar_str(&sb, "hint", SEARCH_CODE_PATH_HINT); + } if (metrics->elapsed_ms >= SEARCH_SLOW_MS) { cbm_tree_scalar_str(&sb, "warning_slow", "search was slow; narrow file_pattern/path_filter or use a more " @@ -15671,6 +15718,9 @@ static char *assemble_search_output( if (yyjson_mut_arr_size(warnings) > 0) { yyjson_mut_obj_add_val(doc, root_obj, "warnings", warnings); } + if (metrics->hint_path_filter) { + yyjson_mut_obj_add_str(doc, root_obj, "hint", SEARCH_CODE_PATH_HINT); + } char *json = yy_doc_to_str(doc); if (json) { @@ -16448,6 +16498,17 @@ static bool write_scoped_filelist(cbm_mcp_server_t *srv, const char *project, co continue; } } + /* A file_pattern with '/' is matched here on every platform; the basename + * filters below (and the Windows prefilter, which rejects '/') skip it. */ + bool path_glob = file_pattern && strchr(file_pattern, '/'); + if (path_glob) { +#ifdef _WIN32 + cbm_normalize_path_sep(indexed_files[fi]); +#endif + if (!search_code_path_glob_match(file_pattern, indexed_files[fi])) { + continue; + } + } #ifndef _WIN32 /* GNU grep's --include is unavailable in BusyBox grep. Filter the * canonical list before xargs instead, preserving grep's basename @@ -16455,7 +16516,7 @@ static bool write_scoped_filelist(cbm_mcp_server_t *srv, const char *project, co * on GNU userland. Windows keeps its PowerShell -like filter. */ const char *basename = strrchr(indexed_files[fi], '/'); basename = basename ? basename + SKIP_ONE : indexed_files[fi]; - if (file_pattern && fnmatch(file_pattern, basename, 0) != 0) { + if (file_pattern && !path_glob && fnmatch(file_pattern, basename, 0) != 0) { continue; } #endif @@ -17473,8 +17534,8 @@ static char *handle_search_code(cbm_mcp_server_t *srv, const char *args) { bool scan_in_process = scoped && !srv->search_scan_command_override; #ifdef _WIN32 scan_in_process = - scan_in_process && - (!file_pattern || cbm_search_code_file_pattern_can_prefilter(file_pattern)); + scan_in_process && (!file_pattern || strchr(file_pattern, '/') || + cbm_search_code_file_pattern_can_prefilter(file_pattern)); #endif mcp_scan_cause_t scan_cause; if (scan_in_process) { @@ -17646,6 +17707,7 @@ static char *handle_search_code(cbm_mcp_server_t *srv, const char *args) { directory_output = directory_limit; } bool warn_literal_pipe = pat_has_pipe && !use_regex; + metrics.hint_path_filter = file_pattern && strchr(file_pattern, '/') && gm_count == 0; /* Every render below re-checks each result's file against the root; one * containment for the request keeps that to one resolution per file. */ root_containment_t request_containment; diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 671cef371..e02cd8605 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -11670,6 +11670,88 @@ TEST(search_code_scoped_file_pattern_is_busybox_portable) { #endif } +/* issue #2836: a file_pattern with '/' matched only the basename, so every + * directory-scoped glob silently returned 0 results. */ +TEST(search_code_file_pattern_matches_repo_relative_path_issue2836) { + char tmp[CBM_SZ_512]; + snprintf(tmp, sizeof(tmp), "%s/cbm-search-fp-path-XXXXXX", cbm_tmpdir()); + ASSERT_NOT_NULL(cbm_mkdtemp(tmp)); + char src_dir[CBM_SZ_1K]; + char cli_dir[CBM_SZ_1K]; + char main_path[CBM_SZ_1K]; + char watcher_path[CBM_SZ_1K]; + snprintf(src_dir, sizeof(src_dir), "%s/src", tmp); + snprintf(cli_dir, sizeof(cli_dir), "%s/src/cli", tmp); + snprintf(main_path, sizeof(main_path), "%s/src/main.c", tmp); + snprintf(watcher_path, sizeof(watcher_path), "%s/src/cli/watcher.c", tmp); + ASSERT_EQ(cbm_mkdir(src_dir), 0); + ASSERT_EQ(cbm_mkdir(cli_dir), 0); + ASSERT_EQ(th_write_file(main_path, "int FP_PATH_NEEDLE = 1;\n"), 0); + ASSERT_EQ(th_write_file(watcher_path, "int FP_PATH_NEEDLE = 2;\n"), 0); + + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + cbm_store_t *store = cbm_mcp_server_store(srv); + ASSERT_NOT_NULL(store); + const char *project = "search-fp-path"; + cbm_mcp_server_set_project(srv, project); + ASSERT_EQ(cbm_store_upsert_project(store, project, tmp), CBM_STORE_OK); + cbm_node_t main_file = {.project = project, + .label = "File", + .name = "main.c", + .qualified_name = "search-fp-path.src.main.c", + .file_path = "src/main.c"}; + cbm_node_t watcher_file = {.project = project, + .label = "File", + .name = "watcher.c", + .qualified_name = "search-fp-path.src.cli.watcher.c", + .file_path = "src/cli/watcher.c"}; + ASSERT_GT(cbm_store_upsert_node(store, &main_file), 0); + ASSERT_GT(cbm_store_upsert_node(store, &watcher_file), 0); + + static const struct { + const char *file_pattern; + const char *expect; + bool hint; + } cases[] = { + {"src/cli/*.c", "\"total_grep_matches\":1", false}, + {"src/**/*.c", "\"total_grep_matches\":2", false}, /* ** spans zero dirs */ + {"**/cli/*.c", "\"total_grep_matches\":1", false}, + {"src/cli/", "\"total_grep_matches\":1", false}, /* no wildcard: substring */ + {"watcher.c", "\"total_grep_matches\":1", false}, /* basename unchanged */ + {"*.c", "\"total_grep_matches\":2", false}, + {"lib/*.c", "\"total_grep_matches\":0", true}, + }; + bool ok = true; + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + char args[CBM_SZ_512]; + snprintf(args, sizeof(args), + "{\"pattern\":\"FP_PATH_NEEDLE\",\"project\":\"search-fp-path\"," + "\"file_pattern\":\"%s\",\"format\":\"json\"}", + cases[i].file_pattern); + char *response = cbm_mcp_handle_tool(srv, "search_code", args); + char *inner = extract_text_content(response); + bool case_ok = inner && strstr(inner, cases[i].expect) && + (strstr(inner, "use path_filter") != NULL) == cases[i].hint; + if (!case_ok) { + fprintf(stderr, "file_pattern %s: %s\n", cases[i].file_pattern, + inner ? inner : "(null)"); + ok = false; + } + free(inner); + free(response); + } + + cbm_mcp_server_free(srv); + cbm_unlink(watcher_path); + cbm_unlink(main_path); + cbm_rmdir(cli_dir); + cbm_rmdir(src_dir); + cbm_rmdir(tmp); + ASSERT_TRUE(ok); + PASS(); +} + TEST(search_code_recursive_fallback_propagates_discovery_failures) { #ifdef _WIN32 SKIP_PLATFORM("POSIX find/sort/xargs fallback contract"); @@ -26322,6 +26404,7 @@ SUITE(mcp) { RUN_TEST(search_code_fails_closed_when_complete_scan_is_impossible); RUN_TEST(search_code_scoped_scan_uses_canonical_file_nodes); RUN_TEST(search_code_scoped_file_pattern_is_busybox_portable); + RUN_TEST(search_code_file_pattern_matches_repo_relative_path_issue2836); RUN_TEST(search_code_recursive_fallback_propagates_discovery_failures); RUN_TEST(search_code_default_budget_limits_raw_rows_before_graph_results); RUN_TEST(search_code_raw_and_directory_remainders_are_independently_pageable);