Skip to content

Commit 506151f

Browse files
fix(mcp): propagate cancellation to Windows code search
Signed-off-by: Ertan <ertan.kucukoglu@gmail.com>
1 parent 93e9308 commit 506151f

3 files changed

Lines changed: 240 additions & 4 deletions

File tree

src/mcp/mcp.c

Lines changed: 105 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ enum {
123123
* growing getline buffers without bound through ignored extension headers. */
124124
#define MCP_MAX_MESSAGE_SIZE ((size_t)10U * 1024U * 1024U)
125125
#define MCP_MAX_HEADER_SIZE ((size_t)8U * 1024U)
126+
#define MCP_SEARCH_OUTPUT_MAX ((size_t)64U * 1024U * 1024U)
126127

127128
/* ── Helpers ────────────────────────────────────────────────────── */
128129

@@ -1569,6 +1570,7 @@ struct cbm_mcp_server {
15691570
void *quarantine_test_context;
15701571
cbm_mcp_command_test_hook_fn command_test_hook;
15711572
void *command_test_context;
1573+
size_t search_output_limit_override;
15721574
cbm_thread_t autoindex_tid;
15731575
bool autoindex_active; /* true if auto-index thread was started */
15741576

@@ -1867,6 +1869,12 @@ void cbm_mcp_server_set_command_test_hook(cbm_mcp_server_t *srv, cbm_mcp_command
18671869
srv->command_test_context = context;
18681870
}
18691871

1872+
void cbm_mcp_server_set_search_output_limit_for_test(cbm_mcp_server_t *srv, size_t limit) {
1873+
if (srv) {
1874+
srv->search_output_limit_override = limit;
1875+
}
1876+
}
1877+
18701878
/* ── Cache dir + project DB path helpers ───────────────────────── */
18711879

18721880
/* Returns the cache directory. Writes to buf, returns buf for convenience. */
@@ -9686,6 +9694,32 @@ static bool compile_path_filter(const char *filter, cbm_regex_t *re) {
96869694
return cbm_regcomp(re, filter, CBM_REG_EXTENDED | CBM_REG_NOSUB) == CBM_REG_OK;
96879695
}
96889696

9697+
static int mcp_run_shell_command_cancellable_bounded(cbm_mcp_server_t *srv, const char *command,
9698+
char output_path[CBM_SZ_2K],
9699+
size_t output_limit,
9700+
bool *output_limit_exceeded,
9701+
cbm_proc_result_t *result_out);
9702+
9703+
#ifdef _WIN32
9704+
static char *search_code_scan_error(search_scratch_t *scratch, const char *output_path,
9705+
bool has_path_filter, cbm_regex_t *path_regex, char *root_path,
9706+
char *pattern, char *project, char *file_pattern,
9707+
const char *message) {
9708+
if (output_path && output_path[0]) {
9709+
(void)cbm_unlink(output_path);
9710+
}
9711+
search_scratch_close(scratch);
9712+
if (has_path_filter) {
9713+
cbm_regfree(path_regex);
9714+
}
9715+
free(root_path);
9716+
free(pattern);
9717+
free(project);
9718+
free(file_pattern);
9719+
return cbm_mcp_text_result(message, true);
9720+
}
9721+
#endif
9722+
96899723
static char *handle_search_code(cbm_mcp_server_t *srv, const char *args) {
96909724
char *pattern = cbm_mcp_get_string_arg(args, "pattern");
96919725
char *project = get_project_arg(args);
@@ -9869,6 +9903,44 @@ static char *handle_search_code(cbm_mcp_server_t *srv, const char *args) {
98699903
cbm_search_code_build_grep_cmd(cmd, sizeof(cmd), use_regex, scoped, file_pattern, tmpfile,
98709904
filelist, root_path);
98719905

9906+
#ifdef _WIN32
9907+
char output_path[CBM_SZ_2K] = {0};
9908+
cbm_proc_result_t scan_result = {0};
9909+
bool scan_output_exceeded = false;
9910+
size_t scan_output_limit = srv->search_output_limit_override
9911+
? srv->search_output_limit_override
9912+
: MCP_SEARCH_OUTPUT_MAX;
9913+
int scan_run = mcp_run_shell_command_cancellable_bounded(
9914+
srv, cmd, output_path, scan_output_limit, &scan_output_exceeded, &scan_result);
9915+
if (scan_output_exceeded) {
9916+
char message[CBM_SZ_128];
9917+
snprintf(message, sizeof(message),
9918+
"search failed: output exceeded the %zu-byte safety limit", scan_output_limit);
9919+
return search_code_scan_error(&scratch, output_path, has_path_filter, &path_regex,
9920+
root_path, pattern, project, file_pattern, message);
9921+
}
9922+
bool scan_cancelled = scan_result.cancellation_requested || mcp_request_cancelled(srv);
9923+
if (scan_cancelled) {
9924+
return search_code_scan_error(&scratch, output_path, has_path_filter, &path_regex,
9925+
root_path, pattern, project, file_pattern,
9926+
"search_code cancelled for this request");
9927+
}
9928+
if (scan_run != 0) {
9929+
return search_code_scan_error(
9930+
&scratch, output_path, has_path_filter, &path_regex, root_path, pattern, project,
9931+
file_pattern, "search failed: the contained command could not complete");
9932+
}
9933+
FILE *fp = cbm_fopen(output_path, "rb");
9934+
if (!fp) {
9935+
return search_code_scan_error(&scratch, output_path, has_path_filter, &path_regex,
9936+
root_path, pattern, project, file_pattern,
9937+
"search failed: contained output could not be read");
9938+
}
9939+
gm = collect_grep_matches(fp, root_path, strlen(root_path), has_path_filter, &path_regex,
9940+
grep_limit, &gm_count);
9941+
(void)fclose(fp);
9942+
(void)cbm_unlink(output_path);
9943+
#else
98729944
FILE *fp = cbm_popen(cmd, "r");
98739945
if (!fp) {
98749946
search_scratch_close(&scratch);
@@ -9885,6 +9957,7 @@ static char *handle_search_code(cbm_mcp_server_t *srv, const char *args) {
98859957
gm = collect_grep_matches(fp, root_path, strlen(root_path), has_path_filter, &path_regex,
98869958
grep_limit, &gm_count);
98879959
cbm_pclose(fp);
9960+
#endif
98889961
/* Both scratch files and the private directory go here — unlike the old
98899962
* code, the file list is removed even when the scan was not scoped. */
98909963
search_scratch_close(&scratch);
@@ -10045,10 +10118,16 @@ static bool mcp_resolve_windows_cmd(char out[CBM_SZ_4K]) {
1004510118
}
1004610119
#endif
1004710120

10048-
static int mcp_run_shell_command_cancellable(cbm_mcp_server_t *srv, const char *command,
10049-
char output_path[CBM_SZ_2K],
10050-
cbm_proc_result_t *result_out) {
10051-
if (!srv || !command || !output_path || !result_out || !mcp_command_output_path(output_path)) {
10121+
static int mcp_run_shell_command_cancellable_bounded(cbm_mcp_server_t *srv, const char *command,
10122+
char output_path[CBM_SZ_2K],
10123+
size_t output_limit,
10124+
bool *output_limit_exceeded,
10125+
cbm_proc_result_t *result_out) {
10126+
if (output_limit_exceeded) {
10127+
*output_limit_exceeded = false;
10128+
}
10129+
if (!srv || !command || !output_path || !result_out ||
10130+
(output_limit > 0 && !output_limit_exceeded) || !mcp_command_output_path(output_path)) {
1005210131
return -1;
1005310132
}
1005410133
/* Internal test seam: rejecting after output allocation exercises the same
@@ -10087,10 +10166,18 @@ static int mcp_run_shell_command_cancellable(cbm_mcp_server_t *srv, const char *
1008710166
}
1008810167

1008910168
cbm_proc_poll_t state;
10169+
bool limit_exceeded = false;
1009010170
for (;;) {
1009110171
if (mcp_request_cancelled(srv)) {
1009210172
(void)cbm_subprocess_request_cancel(process);
1009310173
}
10174+
if (!limit_exceeded && output_limit > 0) {
10175+
int64_t output_size = cbm_file_size(output_path);
10176+
if (output_size > 0 && (uint64_t)output_size > output_limit) {
10177+
limit_exceeded = true;
10178+
(void)cbm_subprocess_request_cancel(process);
10179+
}
10180+
}
1009410181
state = cbm_subprocess_poll(process, result_out);
1009510182
if (state != CBM_PROC_POLL_RUNNING) {
1009610183
break;
@@ -10100,9 +10187,23 @@ static int mcp_run_shell_command_cancellable(cbm_mcp_server_t *srv, const char *
1010010187
bool contained = state == CBM_PROC_POLL_TERMINAL && result_out->tree_quiesced &&
1010110188
!result_out->supervision_failed;
1010210189
cbm_subprocess_destroy(process);
10190+
if (!limit_exceeded && output_limit > 0) {
10191+
int64_t final_size = cbm_file_size(output_path);
10192+
limit_exceeded = final_size > 0 && (uint64_t)final_size > output_limit;
10193+
}
10194+
if (output_limit_exceeded) {
10195+
*output_limit_exceeded = limit_exceeded;
10196+
}
1010310197
return contained ? 0 : -1;
1010410198
}
1010510199

10200+
static int mcp_run_shell_command_cancellable(cbm_mcp_server_t *srv, const char *command,
10201+
char output_path[CBM_SZ_2K],
10202+
cbm_proc_result_t *result_out) {
10203+
return mcp_run_shell_command_cancellable_bounded(srv, command, output_path, 0, NULL,
10204+
result_out);
10205+
}
10206+
1010610207
/* Does `node`'s line range overlap any recorded hunk for `file`? Used to scope
1010710208
* seed detection to the actually-changed lines rather than the whole file.
1010810209
* Non-static (declared in mcp_internal.h) so tests can exercise the overlap

src/mcp/mcp_internal.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ void cbm_mcp_server_set_quarantine_test_hook(cbm_mcp_server_t *srv,
1414
cbm_mcp_quarantine_test_hook_fn hook, void *context);
1515
void cbm_mcp_server_set_command_test_hook(cbm_mcp_server_t *srv, cbm_mcp_command_test_hook_fn hook,
1616
void *context);
17+
void cbm_mcp_server_set_search_output_limit_for_test(cbm_mcp_server_t *srv, size_t limit);
1718

1819
/* Release only the constructor-created pristine in-memory store. Public
1920
* cbm_mcp_server_new(NULL) semantics remain unchanged; daemon sessions use

tests/test_mcp.c

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,21 @@ typedef struct {
162162
int merge_base_calls;
163163
} mcp_command_hook_probe_t;
164164

165+
#ifdef _WIN32
166+
typedef struct {
167+
cbm_mcp_server_t *server;
168+
bool cancel_on_call;
169+
bool cancel_accepted;
170+
int calls;
171+
char command[CBM_SZ_4K];
172+
} mcp_search_command_probe_t;
173+
174+
typedef struct {
175+
char path[512];
176+
char *saved_cache;
177+
} mcp_search_cache_t;
178+
#endif
179+
165180
static bool mcp_quarantine_hook_probe(void *context, const char *step) {
166181
mcp_quarantine_hook_probe_t *probe = context;
167182
if (!probe || !step) {
@@ -187,6 +202,45 @@ static bool mcp_command_hook_probe(void *context, const char *command) {
187202
return true;
188203
}
189204

205+
#ifdef _WIN32
206+
static bool mcp_search_command_hook_probe(void *context, const char *command) {
207+
mcp_search_command_probe_t *probe = context;
208+
if (!probe || !command) {
209+
return false;
210+
}
211+
probe->calls++;
212+
snprintf(probe->command, sizeof(probe->command), "%s", command);
213+
if (probe->cancel_on_call && probe->server) {
214+
probe->cancel_accepted = cbm_mcp_server_cancel_active(probe->server);
215+
}
216+
return true;
217+
}
218+
219+
static bool mcp_search_cache_open(mcp_search_cache_t *cache, const char *prefix) {
220+
memset(cache, 0, sizeof(*cache));
221+
snprintf(cache->path, sizeof(cache->path), "%s/%s-XXXXXX", cbm_tmpdir(), prefix);
222+
if (!cbm_mkdtemp(cache->path)) {
223+
return false;
224+
}
225+
const char *saved_cache = getenv("CBM_CACHE_DIR");
226+
cache->saved_cache = saved_cache ? strdup(saved_cache) : NULL;
227+
if ((saved_cache && !cache->saved_cache) || cbm_setenv("CBM_CACHE_DIR", cache->path, 1) != 0) {
228+
free(cache->saved_cache);
229+
cache->saved_cache = NULL;
230+
(void)th_rmtree(cache->path);
231+
return false;
232+
}
233+
return true;
234+
}
235+
236+
static bool mcp_search_cache_close(mcp_search_cache_t *cache) {
237+
restore_cache_dir(cache->saved_cache);
238+
free(cache->saved_cache);
239+
cache->saved_cache = NULL;
240+
return th_rmtree(cache->path) == 0;
241+
}
242+
#endif
243+
190244
typedef struct {
191245
const char *name;
192246
char *value;
@@ -4543,6 +4597,84 @@ TEST(search_code_windows_prefilter_precedes_content_scan) {
45434597
#endif
45444598
}
45454599

4600+
TEST(search_code_windows_cancel_cleans_supervised_scan) {
4601+
#ifdef _WIN32
4602+
mcp_search_cache_t cache;
4603+
ASSERT_TRUE(mcp_search_cache_open(&cache, "cbm-search-cancel"));
4604+
4605+
char tmp[512], src_path[768], vendor_path[768];
4606+
cbm_mcp_server_t *srv = setup_prefilter_server(tmp, sizeof(tmp), src_path, sizeof(src_path),
4607+
vendor_path, sizeof(vendor_path));
4608+
ASSERT_NOT_NULL(srv);
4609+
mcp_search_command_probe_t probe = {
4610+
.server = srv,
4611+
.cancel_on_call = true,
4612+
};
4613+
cbm_mcp_server_set_command_test_hook(srv, mcp_search_command_hook_probe, &probe);
4614+
4615+
char *response =
4616+
cbm_mcp_handle_tool(srv, "search_code",
4617+
"{\"pattern\":\"HandleRequest\",\"project\":\"prefilter-search\","
4618+
"\"file_pattern\":\"*.go\"}");
4619+
ASSERT_NOT_NULL(response);
4620+
ASSERT_TRUE(probe.cancel_accepted);
4621+
ASSERT_NOT_NULL(strstr(response, "cancelled"));
4622+
ASSERT_NOT_NULL(strstr(response, "\"isError\":true"));
4623+
4624+
char logs[640];
4625+
snprintf(logs, sizeof(logs), "%s/logs", cache.path);
4626+
ASSERT_EQ(mcp_count_directory_entries_with_prefix(logs, ".mcp-command-"), 0);
4627+
4628+
free(response);
4629+
cbm_mcp_server_free(srv);
4630+
cleanup_prefilter_dir(tmp, src_path, vendor_path);
4631+
ASSERT_TRUE(mcp_search_cache_close(&cache));
4632+
PASS();
4633+
#else
4634+
SKIP_PLATFORM("supervised Select-String cancellation runs on Windows");
4635+
#endif
4636+
}
4637+
4638+
TEST(search_code_windows_output_limit_fails_closed_and_cleans_scan) {
4639+
#ifdef _WIN32
4640+
mcp_search_cache_t cache;
4641+
ASSERT_TRUE(mcp_search_cache_open(&cache, "cbm-search-limit"));
4642+
4643+
char tmp[512], src_path[768], vendor_path[768];
4644+
cbm_mcp_server_t *srv = setup_prefilter_server(tmp, sizeof(tmp), src_path, sizeof(src_path),
4645+
vendor_path, sizeof(vendor_path));
4646+
ASSERT_NOT_NULL(srv);
4647+
cbm_mcp_server_set_search_output_limit_for_test(srv, 512);
4648+
4649+
FILE *source = cbm_fopen(src_path, "ab");
4650+
ASSERT_NOT_NULL(source);
4651+
for (int i = 0; i < 256; i++) {
4652+
ASSERT_GT(fprintf(source, "func HandleRequest%d() error { return nil }\n", i), 0);
4653+
}
4654+
ASSERT_EQ(fclose(source), 0);
4655+
4656+
char *response =
4657+
cbm_mcp_handle_tool(srv, "search_code",
4658+
"{\"pattern\":\"HandleRequest\",\"project\":\"prefilter-search\","
4659+
"\"file_pattern\":\"*.go\"}");
4660+
ASSERT_NOT_NULL(response);
4661+
ASSERT_NOT_NULL(strstr(response, "output exceeded"));
4662+
ASSERT_NOT_NULL(strstr(response, "\"isError\":true"));
4663+
4664+
char logs[640];
4665+
snprintf(logs, sizeof(logs), "%s/logs", cache.path);
4666+
ASSERT_EQ(mcp_count_directory_entries_with_prefix(logs, ".mcp-command-"), 0);
4667+
4668+
free(response);
4669+
cbm_mcp_server_free(srv);
4670+
cleanup_prefilter_dir(tmp, src_path, vendor_path);
4671+
ASSERT_TRUE(mcp_search_cache_close(&cache));
4672+
PASS();
4673+
#else
4674+
SKIP_PLATFORM("supervised Select-String output limit runs on Windows");
4675+
#endif
4676+
}
4677+
45464678
/* issue #283: search_code with regex=true and a syntactically invalid pattern
45474679
* must return an explicit error, not an empty result indistinguishable from a
45484680
* legitimate no-match. */
@@ -10806,6 +10938,8 @@ SUITE(mcp) {
1080610938
RUN_TEST(search_code_path_filter_matches_nothing);
1080710939
RUN_TEST(search_code_file_pattern_prefilter_boundaries);
1080810940
RUN_TEST(search_code_windows_prefilter_precedes_content_scan);
10941+
RUN_TEST(search_code_windows_cancel_cleans_supervised_scan);
10942+
RUN_TEST(search_code_windows_output_limit_fails_closed_and_cleans_scan);
1080910943
RUN_TEST(search_code_invalid_regex_errors_issue283);
1081010944
RUN_TEST(search_code_literal_pipe_warns_issue282);
1081110945
RUN_TEST(search_code_reports_phase_timings_only_in_debug_mode);

0 commit comments

Comments
 (0)