Skip to content

Commit fe55b86

Browse files
DeusDataDeusData
authored
Merge pull request #2749 from tahodev/argv-grep-find-2634
refactor(search): run POSIX unscoped scan as argv stages, no shell (#2634)
2 parents 86ad094 + b9ba6c0 commit fe55b86

3 files changed

Lines changed: 336 additions & 5 deletions

File tree

‎scripts/memory-core-baseline.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ src/git/git_context.c 18
5151
src/main.c 32
5252
src/mcp/compact_out.c 39
5353
src/mcp/index_supervisor.c 8
54-
src/mcp/mcp.c 769
54+
src/mcp/mcp.c 765
5555
src/pipeline/artifact.c 38
5656
src/pipeline/fqn.c 23
5757
src/pipeline/lsp_resolve.h 4

‎src/mcp/mcp.c‎

Lines changed: 197 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17617,6 +17617,28 @@ static mcp_scan_cause_t mcp_run_shell_command_cancellable_bounded(
1761717617
uint64_t deadline_ms, bool deadline_enabled, bool deadline_latched, bool exit_one_is_no_match,
1761817618
bool git_child, cbm_proc_result_t *result_out);
1761917619

17620+
#ifndef _WIN32
17621+
/* POSIX-only: supervised run of one argv stage (no shell). Same latch and
17622+
* containment contract as the shell runner; the measured file's size plus
17623+
* output_base counts toward output_limit so chained stages share one budget.
17624+
* tolerate_exit_one maps a plain exit-1 (grep's no-match) to SUCCESS. */
17625+
static mcp_scan_cause_t mcp_run_argv_stage_cancellable_bounded(
17626+
cbm_mcp_server_t *srv, const char *bin, const char *const *argv,
17627+
const char *const *env_overrides, const char *stdout_capture, uint64_t output_base,
17628+
size_t output_limit, uint64_t deadline_ms, bool deadline_enabled, bool tolerate_exit_one,
17629+
bool *cancellation_latched, bool *deadline_latched, bool *output_limit_latched,
17630+
cbm_proc_result_t *result_out);
17631+
17632+
/* POSIX-only: the unscoped search_code scan as find/sort argv stages feeding the
17633+
* in-process scanner. */
17634+
static mcp_scan_cause_t mcp_run_search_scan_argv(cbm_mcp_server_t *srv, bool use_regex,
17635+
const char *file_pattern, const char *pattern,
17636+
const char *filelist, const char *root_path,
17637+
char output_path[CBM_SZ_2K], size_t output_limit,
17638+
uint64_t deadline_ms, bool deadline_latched,
17639+
cbm_proc_result_t *result_out);
17640+
#endif
17641+
1762017642
static char *search_code_timeout_result(void) {
1762117643
static const char message[] = "search_code scan exceeded its execution deadline";
1762217644
static const char fallback[] =
@@ -18016,10 +18038,22 @@ static char *handle_search_code(cbm_mcp_server_t *srv, const char *args) {
1801618038
scan_cause = MCP_SCAN_COMMAND_FAILURE;
1801718039
}
1801818040
} else {
18019-
scan_cause = mcp_run_shell_command_cancellable_bounded(
18020-
srv, scan_command, output_path, scan_output_limit, scan_deadline_ms, true,
18021-
scan_deadline_latched, /*exit_one_is_no_match=*/false, /*git_child=*/false,
18022-
&scan_result);
18041+
#ifndef _WIN32
18042+
if (!srv->search_scan_command_override) {
18043+
/* POSIX unscoped scans run as find/sort/grep argv stages: no
18044+
* shell parses search scopes or file patterns anymore (#2634).
18045+
* The command override keeps the shell runner for tests. */
18046+
scan_cause = mcp_run_search_scan_argv(
18047+
srv, use_regex, file_pattern, pattern, filelist, root_path, output_path,
18048+
scan_output_limit, scan_deadline_ms, scan_deadline_latched, &scan_result);
18049+
} else
18050+
#endif
18051+
{
18052+
scan_cause = mcp_run_shell_command_cancellable_bounded(
18053+
srv, scan_command, output_path, scan_output_limit, scan_deadline_ms, true,
18054+
scan_deadline_latched, /*exit_one_is_no_match=*/false, /*git_child=*/false,
18055+
&scan_result);
18056+
}
1802318057
}
1802418058
/* Both POSIX commands wrap grep and map its no-match status to 0, so any
1802518059
* non-zero exit (a failed find/sort, an unreadable operand, a broken
@@ -18539,6 +18573,165 @@ static int mcp_run_shell_command_cancellable(cbm_mcp_server_t *srv, const char *
1853918573
return cause == MCP_SCAN_COMMAND_FAILURE || cause == MCP_SCAN_SUPERVISION_FAILURE ? -1 : 0;
1854018574
}
1854118575

18576+
#ifndef _WIN32
18577+
/* Latch poll shared by argv stages: request-cancellation, wall-clock deadline,
18578+
* and the cumulative measured-file output budget. Mirrors the latch updates of
18579+
* the shell runner so both paths fail the same way. */
18580+
static void mcp_argv_stage_poll_latches(cbm_mcp_server_t *srv, const char *measured_path,
18581+
uint64_t output_base, size_t output_limit,
18582+
uint64_t deadline_ms, bool deadline_enabled,
18583+
bool *cancellation_latched, bool *deadline_latched,
18584+
bool *output_limit_latched) {
18585+
*cancellation_latched = *cancellation_latched || mcp_request_cancelled(srv);
18586+
*deadline_latched = *deadline_latched || (deadline_enabled && cbm_now_ms() >= deadline_ms);
18587+
if (!*output_limit_latched && output_limit > 0 && measured_path && measured_path[0]) {
18588+
int64_t measured_size = cbm_file_size(measured_path);
18589+
*output_limit_latched =
18590+
measured_size > 0 && output_base + (uint64_t)measured_size > output_limit;
18591+
}
18592+
}
18593+
18594+
static mcp_scan_cause_t mcp_run_argv_stage_cancellable_bounded(
18595+
cbm_mcp_server_t *srv, const char *bin, const char *const *argv,
18596+
const char *const *env_overrides, const char *stdout_capture, uint64_t output_base,
18597+
size_t output_limit, uint64_t deadline_ms, bool deadline_enabled, bool tolerate_exit_one,
18598+
bool *cancellation_latched, bool *deadline_latched, bool *output_limit_latched,
18599+
cbm_proc_result_t *result_out) {
18600+
memset(result_out, 0, sizeof(*result_out));
18601+
mcp_argv_stage_poll_latches(srv, stdout_capture, output_base, output_limit, deadline_ms,
18602+
deadline_enabled, cancellation_latched, deadline_latched,
18603+
output_limit_latched);
18604+
if (*cancellation_latched || *deadline_latched || *output_limit_latched) {
18605+
result_out->tree_quiesced = true; /* no child was spawned */
18606+
if (*cancellation_latched) {
18607+
return MCP_SCAN_CANCELLED;
18608+
}
18609+
return *deadline_latched ? MCP_SCAN_DEADLINE : MCP_SCAN_OUTPUT_LIMIT;
18610+
}
18611+
cbm_proc_opts_t options = {
18612+
.bin = bin,
18613+
.argv = argv,
18614+
.env_overrides = env_overrides,
18615+
.log_file = NULL,
18616+
.quiet_timeout_ms = 0,
18617+
.cancel_grace_ms = CBM_SUBPROCESS_DEFAULT_CANCEL_GRACE_MS,
18618+
.delete_log_on_exit = false,
18619+
.strip_git_repo_env = false,
18620+
.stdout_file = stdout_capture,
18621+
};
18622+
cbm_subprocess_t *process = NULL;
18623+
if (cbm_subprocess_spawn(&options, &process) != 0) {
18624+
result_out->tree_quiesced = true;
18625+
return MCP_SCAN_COMMAND_FAILURE;
18626+
}
18627+
cbm_proc_poll_t state;
18628+
for (;;) {
18629+
mcp_argv_stage_poll_latches(srv, stdout_capture, output_base, output_limit, deadline_ms,
18630+
deadline_enabled, cancellation_latched, deadline_latched,
18631+
output_limit_latched);
18632+
if (*cancellation_latched || *deadline_latched || *output_limit_latched) {
18633+
(void)cbm_subprocess_request_cancel(process);
18634+
}
18635+
state = cbm_subprocess_poll(process, result_out);
18636+
if (state != CBM_PROC_POLL_RUNNING) {
18637+
break;
18638+
}
18639+
cbm_usleep(10000);
18640+
}
18641+
bool contained = state == CBM_PROC_POLL_TERMINAL && result_out->tree_quiesced &&
18642+
!result_out->supervision_failed;
18643+
cbm_subprocess_destroy(process);
18644+
mcp_argv_stage_poll_latches(srv, stdout_capture, output_base, output_limit, deadline_ms,
18645+
deadline_enabled, cancellation_latched, deadline_latched,
18646+
output_limit_latched);
18647+
if (!contained) {
18648+
return MCP_SCAN_SUPERVISION_FAILURE;
18649+
}
18650+
if (*cancellation_latched) {
18651+
return MCP_SCAN_CANCELLED;
18652+
}
18653+
if (*deadline_latched) {
18654+
return MCP_SCAN_DEADLINE;
18655+
}
18656+
if (*output_limit_latched) {
18657+
return MCP_SCAN_OUTPUT_LIMIT;
18658+
}
18659+
if (tolerate_exit_one && result_out->outcome == CBM_PROC_EXIT_NONZERO &&
18660+
result_out->exit_code == 1) {
18661+
return MCP_SCAN_SUCCESS; /* ordinary grep no-match */
18662+
}
18663+
return result_out->outcome == CBM_PROC_CLEAN ? MCP_SCAN_SUCCESS
18664+
: MCP_SCAN_CONTAINED_COMMAND_FAILURE;
18665+
}
18666+
18667+
/* The unscoped (unindexed or not-fully-indexed root) search_code scan with no
18668+
* shell anywhere: find writes the NUL-delimited candidate list and LC_ALL=C
18669+
* sort orders it in place, both as argv stages, then the list feeds the
18670+
* in-process scanner (search_scan_in_process). Search scopes and file patterns
18671+
* reach find as plain arguments instead of shell-string interpolations (#2634).
18672+
* find/sort failures fail closed, and an empty list is a successful empty
18673+
* scan. */
18674+
static mcp_scan_cause_t mcp_run_search_scan_argv(cbm_mcp_server_t *srv, bool use_regex,
18675+
const char *file_pattern, const char *pattern,
18676+
const char *filelist, const char *root_path,
18677+
char output_path[CBM_SZ_2K], size_t output_limit,
18678+
uint64_t deadline_ms, bool deadline_latched,
18679+
cbm_proc_result_t *result_out) {
18680+
bool cancellation_latched = false;
18681+
bool output_limit_latched = false;
18682+
18683+
const char *find_argv[9];
18684+
int fa = 0;
18685+
find_argv[fa++] = "find";
18686+
find_argv[fa++] = root_path;
18687+
find_argv[fa++] = "-type";
18688+
find_argv[fa++] = "f";
18689+
if (file_pattern) {
18690+
find_argv[fa++] = "-name";
18691+
find_argv[fa++] = file_pattern;
18692+
}
18693+
find_argv[fa++] = "-print0";
18694+
find_argv[fa] = NULL;
18695+
mcp_scan_cause_t cause = mcp_run_argv_stage_cancellable_bounded(
18696+
srv, "find", find_argv, NULL, filelist, 0, 0, deadline_ms, true, false,
18697+
&cancellation_latched, &deadline_latched, &output_limit_latched, result_out);
18698+
if (cause != MCP_SCAN_SUCCESS) {
18699+
return cause;
18700+
}
18701+
18702+
const char *sort_argv[] = {"sort", "-z", "-o", filelist, filelist, NULL};
18703+
const char *sort_env[] = {"LC_ALL=C", NULL};
18704+
cause = mcp_run_argv_stage_cancellable_bounded(
18705+
srv, "sort", sort_argv, sort_env, NULL, 0, 0, deadline_ms, true, false,
18706+
&cancellation_latched, &deadline_latched, &output_limit_latched, result_out);
18707+
if (cause != MCP_SCAN_SUCCESS) {
18708+
return cause;
18709+
}
18710+
18711+
if (!mcp_command_output_path(output_path)) {
18712+
return MCP_SCAN_COMMAND_FAILURE;
18713+
}
18714+
FILE *out = cbm_fopen(output_path, "wb");
18715+
if (!out) {
18716+
(void)cbm_unlink(output_path);
18717+
output_path[0] = '\0';
18718+
return MCP_SCAN_COMMAND_FAILURE;
18719+
}
18720+
/* The sorted, NUL-separated file list feeds the same in-process scanner an
18721+
* indexed project uses, so matching has one implementation. */
18722+
mcp_scan_cause_t scan_cause =
18723+
search_scan_in_process(srv, filelist, pattern, use_regex, out, output_limit, deadline_ms);
18724+
if (fclose(out) != 0 && scan_cause == MCP_SCAN_SUCCESS) {
18725+
scan_cause = MCP_SCAN_COMMAND_FAILURE;
18726+
}
18727+
if (scan_cause != MCP_SCAN_SUCCESS) {
18728+
(void)cbm_unlink(output_path);
18729+
output_path[0] = '\0';
18730+
}
18731+
return scan_cause;
18732+
}
18733+
#endif
18734+
1854218735
/* Does `node`'s line range overlap any recorded hunk for `file`? Used to scope
1854318736
* seed detection to the actually-changed lines rather than the whole file.
1854418737
* Non-static (declared in mcp_internal.h) so tests can exercise the overlap

‎tests/test_mcp.c‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14482,6 +14482,141 @@ TEST(search_code_output_limit_fails_closed_and_cleans_scan) {
1448214482
PASS();
1448314483
}
1448414484

14485+
/* #2634: the POSIX unscoped scan runs find/sort/grep as argv stages with no
14486+
* shell, so file names carrying shell metacharacters are plain operands, not
14487+
* command text. Pin that such files are discovered and scanned. */
14488+
TEST(search_code_unscoped_argv_scans_metachar_filenames_issue2634) {
14489+
#ifdef _WIN32
14490+
SKIP_PLATFORM("argv scan stages are POSIX-only");
14491+
#else
14492+
char tmp[CBM_SZ_512];
14493+
snprintf(tmp, sizeof(tmp), "%s/cbm-search-argv-meta-XXXXXX", cbm_tmpdir());
14494+
ASSERT_NOT_NULL(cbm_mkdtemp(tmp));
14495+
14496+
char hostile[CBM_SZ_1K];
14497+
snprintf(hostile, sizeof(hostile), "%s/we'ird ;na`me$.go", tmp);
14498+
ASSERT_EQ(th_write_file(hostile,
14499+
"package main\n\nfunc HandleHostile2634() error {\n\treturn nil\n}\n"),
14500+
0);
14501+
14502+
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
14503+
ASSERT_NOT_NULL(srv);
14504+
cbm_store_t *store = cbm_mcp_server_store(srv);
14505+
ASSERT_NOT_NULL(store);
14506+
const char *project = "search-argv-metachar";
14507+
cbm_mcp_server_set_project(srv, project);
14508+
ASSERT_EQ(cbm_store_upsert_project(store, project, tmp), CBM_STORE_OK);
14509+
14510+
char *resp = cbm_mcp_handle_tool(srv, "search_code",
14511+
"{\"pattern\":\"HandleHostile2634\","
14512+
"\"project\":\"search-argv-metachar\",\"format\":\"json\"}");
14513+
ASSERT_NOT_NULL(resp);
14514+
ASSERT_TRUE(strstr(resp, "\"isError\":true") == NULL);
14515+
char *inner = extract_text_content(resp);
14516+
ASSERT_NOT_NULL(inner);
14517+
ASSERT_NOT_NULL(strstr(inner, "we'ird ;na`me$.go"));
14518+
ASSERT_NOT_NULL(strstr(inner, "\"total_grep_matches\":1"));
14519+
free(inner);
14520+
free(resp);
14521+
cbm_mcp_server_free(srv);
14522+
ASSERT_EQ(th_rmtree(tmp), 0);
14523+
PASS();
14524+
#endif
14525+
}
14526+
14527+
/* #2634: grep's exit 1 (no match in a chunk) is an ordinary empty result, not
14528+
* a scan failure — the same contract the retired xargs wrapper encoded. */
14529+
TEST(search_code_unscoped_argv_no_match_is_success_issue2634) {
14530+
#ifdef _WIN32
14531+
SKIP_PLATFORM("argv scan stages are POSIX-only");
14532+
#else
14533+
char tmp[CBM_SZ_512];
14534+
snprintf(tmp, sizeof(tmp), "%s/cbm-search-argv-nomatch-XXXXXX", cbm_tmpdir());
14535+
ASSERT_NOT_NULL(cbm_mkdtemp(tmp));
14536+
14537+
char src_path[CBM_SZ_1K];
14538+
snprintf(src_path, sizeof(src_path), "%s/handler.go", tmp);
14539+
ASSERT_EQ(th_write_file(src_path,
14540+
"package main\n\nfunc HandlePresent2634() error {\n\treturn nil\n}\n"),
14541+
0);
14542+
14543+
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
14544+
ASSERT_NOT_NULL(srv);
14545+
cbm_store_t *store = cbm_mcp_server_store(srv);
14546+
ASSERT_NOT_NULL(store);
14547+
const char *project = "search-argv-nomatch";
14548+
cbm_mcp_server_set_project(srv, project);
14549+
ASSERT_EQ(cbm_store_upsert_project(store, project, tmp), CBM_STORE_OK);
14550+
14551+
char *resp = cbm_mcp_handle_tool(srv, "search_code",
14552+
"{\"pattern\":\"NoSuchSymbol2634\","
14553+
"\"project\":\"search-argv-nomatch\",\"format\":\"json\"}");
14554+
ASSERT_NOT_NULL(resp);
14555+
ASSERT_TRUE(strstr(resp, "\"isError\":true") == NULL);
14556+
char *inner = extract_text_content(resp);
14557+
ASSERT_NOT_NULL(inner);
14558+
ASSERT_NOT_NULL(strstr(inner, "\"total_grep_matches\":0"));
14559+
ASSERT_NOT_NULL(strstr(inner, "\"total_relation\":\"eq\""));
14560+
free(inner);
14561+
free(resp);
14562+
cbm_mcp_server_free(srv);
14563+
ASSERT_EQ(th_rmtree(tmp), 0);
14564+
PASS();
14565+
#endif
14566+
}
14567+
14568+
/* #2634: the unscoped file_pattern reaches find as a -name operand, so a
14569+
* non-matching extension is excluded from the scan without any shell glob. */
14570+
TEST(search_code_unscoped_argv_honours_file_pattern_issue2634) {
14571+
#ifdef _WIN32
14572+
SKIP_PLATFORM("argv scan stages are POSIX-only");
14573+
#else
14574+
char tmp[CBM_SZ_512];
14575+
snprintf(tmp, sizeof(tmp), "%s/cbm-search-argv-glob-XXXXXX", cbm_tmpdir());
14576+
ASSERT_NOT_NULL(cbm_mkdtemp(tmp));
14577+
14578+
char py[CBM_SZ_1K];
14579+
snprintf(py, sizeof(py), "%s/handler.py", tmp);
14580+
ASSERT_EQ(th_write_file(py, "def handle_python2634():\n return 1\n"), 0);
14581+
14582+
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
14583+
ASSERT_NOT_NULL(srv);
14584+
cbm_store_t *store = cbm_mcp_server_store(srv);
14585+
ASSERT_NOT_NULL(store);
14586+
const char *project = "search-argv-glob";
14587+
cbm_mcp_server_set_project(srv, project);
14588+
ASSERT_EQ(cbm_store_upsert_project(store, project, tmp), CBM_STORE_OK);
14589+
14590+
char *resp = cbm_mcp_handle_tool(srv, "search_code",
14591+
"{\"pattern\":\"handle_python2634\","
14592+
"\"project\":\"search-argv-glob\","
14593+
"\"file_pattern\":\"*.go\",\"format\":\"json\"}");
14594+
ASSERT_NOT_NULL(resp);
14595+
ASSERT_TRUE(strstr(resp, "\"isError\":true") == NULL);
14596+
char *inner = extract_text_content(resp);
14597+
ASSERT_NOT_NULL(inner);
14598+
ASSERT_TRUE(strstr(inner, "handler.py") == NULL);
14599+
ASSERT_NOT_NULL(strstr(inner, "\"total_grep_matches\":0"));
14600+
free(inner);
14601+
free(resp);
14602+
14603+
resp = cbm_mcp_handle_tool(srv, "search_code",
14604+
"{\"pattern\":\"handle_python2634\","
14605+
"\"project\":\"search-argv-glob\",\"format\":\"json\"}");
14606+
ASSERT_NOT_NULL(resp);
14607+
ASSERT_TRUE(strstr(resp, "\"isError\":true") == NULL);
14608+
inner = extract_text_content(resp);
14609+
ASSERT_NOT_NULL(inner);
14610+
ASSERT_NOT_NULL(strstr(inner, "handler.py"));
14611+
ASSERT_NOT_NULL(strstr(inner, "\"total_grep_matches\":1"));
14612+
free(inner);
14613+
free(resp);
14614+
cbm_mcp_server_free(srv);
14615+
ASSERT_EQ(th_rmtree(tmp), 0);
14616+
PASS();
14617+
#endif
14618+
}
14619+
1448514620
TEST(search_code_scan_deadline_fails_closed_and_resets) {
1448614621
mcp_search_cache_t cache;
1448714622
ASSERT_TRUE(mcp_search_cache_open(&cache, "cbm-search-deadline"));
@@ -28175,6 +28310,9 @@ SUITE(mcp) {
2817528310
RUN_TEST(search_code_file_pattern_prefilter_boundaries);
2817628311
RUN_TEST(search_code_windows_scope_prefilter_removes_pipeline_filter);
2817728312
RUN_TEST(search_code_cancel_cleans_supervised_scan);
28313+
RUN_TEST(search_code_unscoped_argv_scans_metachar_filenames_issue2634);
28314+
RUN_TEST(search_code_unscoped_argv_no_match_is_success_issue2634);
28315+
RUN_TEST(search_code_unscoped_argv_honours_file_pattern_issue2634);
2817828316
RUN_TEST(server_handle_logs_timeout_without_query);
2817928317
RUN_TEST(search_code_output_limit_fails_closed_and_cleans_scan);
2818028318
RUN_TEST(search_code_scan_deadline_fails_closed_and_resets);

0 commit comments

Comments
 (0)