mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-11-04 10:48:20 +00:00
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 <shane.mccarron@corvexconnect.com>
This commit is contained in:
1 parent
a3831c9795
commit
5a666f2fa3
2 files changed
+149
-4
No files matched your search
+66
-4
@@ -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;
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in new issue
Block a user