From 5df9173288ada5ca21aceeeb1369c0d07b8df42d Mon Sep 17 00:00:00 2001 From: naman616 Date: Mon, 29 Jun 2026 22:44:28 +0530 Subject: [PATCH] fix(search): use xargs -0 for null-delimited filelist to handle spaces in path (#670) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit write_scoped_filelist wrote newline-delimited absolute paths; BSD xargs split each entry on whitespace, so a space in root_path (e.g. /Users/John Smith/...) caused grep to receive broken path fragments, silently producing zero matches. Fix: write null-delimited paths (fputc '\0') and invoke xargs -0 so each path is passed as a single argument regardless of embedded spaces or metacharacters. Also add -H to the non-scoped grep -rn fallback so BSD grep always emits the filename prefix that collect_grep_matches requires. Regression test: search_code_space_in_path_issue670 creates a project rooted at a directory containing a space, inserts an indexed node, and asserts that search_code returns a match — reproducing the exact reported failure scenario. Co-Authored-By: Claude Sonnet 4.6 Signed-off-by: naman616 --- src/mcp/mcp.c | 18 +++++++----- tests/test_mcp.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 8 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 5235aaab5..eb55d79b8 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -3663,18 +3663,19 @@ static void build_grep_cmd(char *cmd, size_t cmd_sz, bool use_regex, bool scoped const char *flag = use_regex ? "-E" : "-F"; if (scoped) { if (file_pattern) { - snprintf(cmd, cmd_sz, "xargs grep -Hn %s --include='%s' -f '%s' < '%s' 2>/dev/null", + snprintf(cmd, cmd_sz, "xargs -0 grep -Hn %s --include='%s' -f '%s' < '%s' 2>/dev/null", flag, file_pattern, tmpfile, filelist); } else { - snprintf(cmd, cmd_sz, "xargs grep -Hn %s -f '%s' < '%s' 2>/dev/null", flag, tmpfile, + snprintf(cmd, cmd_sz, "xargs -0 grep -Hn %s -f '%s' < '%s' 2>/dev/null", flag, tmpfile, filelist); } } else { if (file_pattern) { - snprintf(cmd, cmd_sz, "grep -rn %s --include='%s' -f '%s' '%s' 2>/dev/null", flag, + snprintf(cmd, cmd_sz, "grep -rHn %s --include='%s' -f '%s' '%s' 2>/dev/null", flag, file_pattern, tmpfile, root_path); } else { - snprintf(cmd, cmd_sz, "grep -rn %s -f '%s' '%s' 2>/dev/null", flag, tmpfile, root_path); + snprintf(cmd, cmd_sz, "grep -rHn %s -f '%s' '%s' 2>/dev/null", flag, tmpfile, + root_path); } } #endif @@ -4082,10 +4083,11 @@ static bool write_scoped_filelist(cbm_mcp_server_t *srv, const char *project, co bool ok = false; if (fl) { for (int fi = 0; fi < indexed_count; fi++) { - /* Use forward slashes so xargs doesn't interpret Windows - * backslashes as escape sequences (e.g. \n becomes newline). - * Binary mode to prevent CRLF (xargs would see trailing \r). */ - (void)fprintf(fl, "%s/%s\n", root_path, indexed_files[fi]); + /* Null-delimited so xargs -0 passes each path as a single + * argument regardless of spaces, backslashes, or other shell + * metacharacters in the path. Binary mode prevents CRLF. */ + (void)fprintf(fl, "%s/%s", root_path, indexed_files[fi]); + (void)fputc('\0', fl); } (void)fclose(fl); ok = true; diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 3bfb45c8f..30c32fc1d 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -1182,6 +1182,81 @@ TEST(search_code_ampersand_accepted_issue272) { PASS(); } +/* issue #670: search_code returned zero matches on macOS arm64 when the + * project root path contained a space. write_scoped_filelist wrote + * newline-delimited paths to the filelist; BSD xargs split each entry on + * whitespace, turning "Test Project/main.go" into two broken tokens that + * grep could not open. Fix: null-delimited filelist + xargs -0. */ +TEST(search_code_space_in_path_issue670) { + /* mkdtemp template must not contain a space */ + char tmp[512]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_670_XXXXXX"); + if (!cbm_mkdtemp(tmp)) { + FAIL("cbm_mkdtemp failed"); + } + + /* Project root contains a space — reproduces the reporter's environment */ + char proj_dir[512]; + snprintf(proj_dir, sizeof(proj_dir), "%s/Test Project", tmp); + cbm_mkdir(proj_dir); + + char src_path[512]; + snprintf(src_path, sizeof(src_path), "%s/main.go", proj_dir); + FILE *fp = fopen(src_path, "w"); + if (!fp) { + rmdir(proj_dir); + rmdir(tmp); + FAIL("fopen failed"); + } + fprintf(fp, "package main\n\nfunc HandleRequest() error {\n\treturn nil\n}\n"); + fclose(fp); + + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + if (!srv) { + unlink(src_path); + rmdir(proj_dir); + rmdir(tmp); + FAIL("cbm_mcp_server_new failed"); + } + cbm_store_t *st = cbm_mcp_server_store(srv); + const char *proj_name = "test-project"; + cbm_mcp_server_set_project(srv, proj_name); + cbm_store_upsert_project(st, proj_name, proj_dir); + + /* Insert a node so write_scoped_filelist finds indexed_count > 0 and + * returns true, forcing the scoped xargs path — the path the bug affected. */ + cbm_node_t n = {0}; + n.project = proj_name; + n.label = "Function"; + n.name = "HandleRequest"; + n.qualified_name = "test.HandleRequest"; + n.file_path = "main.go"; + n.start_line = 3; + n.end_line = 5; + cbm_store_upsert_node(st, &n); + + char req[512]; + snprintf(req, sizeof(req), + "{\"jsonrpc\":\"2.0\",\"id\":670,\"method\":\"tools/call\"," + "\"params\":{\"name\":\"search_code\"," + "\"arguments\":{\"pattern\":\"HandleRequest\"," + "\"project\":\"test-project\"}}}"); + + char *resp = cbm_mcp_server_handle(srv, req); + ASSERT_NOT_NULL(resp); + /* Before the fix xargs split "Test Project" on the space, passing broken + * path fragments to grep → zero stdout → zero matches → no symbol in resp. */ + ASSERT_TRUE(strstr(resp, "HandleRequest") != NULL); + ASSERT_TRUE(strstr(resp, "\"isError\":true") == NULL); + free(resp); + + cbm_mcp_server_free(srv); + unlink(src_path); + rmdir(proj_dir); + rmdir(tmp); + PASS(); +} + TEST(tool_detect_changes_no_project) { cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); @@ -2501,6 +2576,7 @@ SUITE(mcp) { RUN_TEST(search_code_invalid_regex_errors_issue283); RUN_TEST(search_code_literal_pipe_warns_issue282); RUN_TEST(search_code_ampersand_accepted_issue272); + RUN_TEST(search_code_space_in_path_issue670); RUN_TEST(tool_detect_changes_no_project); RUN_TEST(tool_manage_adr_no_project); RUN_TEST(tool_manage_adr_get_with_existing_adr);