Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 66 additions & 4 deletions src/mcp/mcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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},"
Expand Down Expand Up @@ -14714,6 +14716,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
Expand Down Expand Up @@ -14808,6 +14811,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
Expand Down Expand Up @@ -15430,6 +15474,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 "
Expand Down Expand Up @@ -15669,6 +15716,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) {
Expand Down Expand Up @@ -16446,14 +16496,25 @@ 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
* glob semantics without making the shipped static binary depend
* 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
Expand Down Expand Up @@ -17471,8 +17532,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) {
Expand Down Expand Up @@ -17644,6 +17705,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;
Expand Down
83 changes: 83 additions & 0 deletions tests/test_mcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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);
Expand Down
Loading