diff --git a/scripts/run-test-wave.py b/scripts/run-test-wave.py index 99f61bfd4..d24f80815 100755 --- a/scripts/run-test-wave.py +++ b/scripts/run-test-wave.py @@ -17,6 +17,7 @@ import re import signal import subprocess import sys +import tempfile import time from dataclasses import dataclass @@ -105,6 +106,31 @@ def append_log(path: pathlib.Path, message: str) -> None: stream.write("\n") +def publish_barrier_file(path: pathlib.Path, text: str) -> None: + """Publish a barrier file so a poller sees either no file or its content. + + Path.write_text creates and truncates before it writes, so a reader that + polls for existence and then parses the content can observe the zero-byte + window in between. Write next to the destination and rename into place. + """ + with tempfile.NamedTemporaryFile( + mode="w", + encoding="utf-8", + newline="\n", + dir=path.parent, + prefix=f".{path.name}.", + delete=False, + ) as temporary: + temporary.write(text) + temporary.flush() + temporary_path = pathlib.Path(temporary.name) + try: + os.replace(temporary_path, path) + except BaseException: + temporary_path.unlink(missing_ok=True) + raise + + def start_suite( suite: str, runner_command: list[str], @@ -288,12 +314,12 @@ def wait_for_test_pre_terminate_barrier( ready = barrier_dir / f"{active.name}.ready" leader_exited = barrier_dir / f"{active.name}.leader-exited" release = barrier_dir / f"{active.name}.release" - ready.write_text(f"{active.process.pid}\n", encoding="utf-8") + publish_barrier_file(ready, f"{active.process.pid}\n") deadline = time.monotonic() + 10 while not release.exists(): returncode = active.process.poll() if returncode is not None and not leader_exited.exists(): - leader_exited.write_text(f"{returncode}\n", encoding="utf-8") + publish_barrier_file(leader_exited, f"{returncode}\n") if time.monotonic() >= deadline: raise RuntimeError( f"test pre-terminate barrier for {active.name!r} was not released" @@ -312,7 +338,7 @@ def wait_for_test_post_exit_barrier( return ready = barrier_dir / f"{suite}.ready" release = barrier_dir / f"{suite}.release" - ready.write_text("child exited; result intentionally not recorded\n", encoding="utf-8") + publish_barrier_file(ready, "child exited; result intentionally not recorded\n") deadline = time.monotonic() + 10 while not release.exists(): if time.monotonic() >= deadline: diff --git a/src/discover/discover.c b/src/discover/discover.c index b89db8a3f..e568efcf2 100644 --- a/src/discover/discover.c +++ b/src/discover/discover.c @@ -758,14 +758,18 @@ static int wide_stat(const char *path, struct stat *st) { /* Stat a path, skipping symlinks (POSIX) and junctions / reparse points * (Windows). Returns 0 on success, -1 to skip. Skipping reparse points keeps * discovery from walking through a junction that points outside the project - * root, mirroring the POSIX S_ISLNK skip. */ -static int safe_stat(const char *abs_path, struct stat *st) { + * root, mirroring the POSIX S_ISLNK skip. *is_symlink reports whether the + * skip (if any) was specifically the symlink/reparse-point check, as + * opposed to some other stat failure (permissions, a race with a delete). */ +static int safe_stat(const char *abs_path, struct stat *st, bool *is_symlink) { + *is_symlink = false; #ifdef _WIN32 wchar_t *wpath = cbm_path_to_wide(abs_path); if (wpath) { DWORD attr = GetFileAttributesW(wpath); free(wpath); if (attr != INVALID_FILE_ATTRIBUTES && (attr & FILE_ATTRIBUTE_REPARSE_POINT)) { + *is_symlink = true; return CBM_NOT_FOUND; } } @@ -775,6 +779,7 @@ static int safe_stat(const char *abs_path, struct stat *st) { return CBM_NOT_FOUND; } if (S_ISLNK(st->st_mode)) { + *is_symlink = true; return CBM_NOT_FOUND; } return 0; @@ -891,9 +896,15 @@ static void walk_dir_process_entry(cbm_dirent_t *entry, const walk_frame_t *fram } struct stat st; - if (safe_stat(abs_path, &st) != 0) { + bool is_symlink = false; + if (safe_stat(abs_path, &st, &is_symlink) != 0) { if (out->count_only) { out->failed = true; + } else if (is_symlink) { + /* Deliberately not indexed (#963): record so callers can + * surface it, matching the directory-exclusion and file + * skip_reason paths a few lines below. */ + file_list_add_ignored(out, rel_path, "symlink"); } return; } diff --git a/tests/test_discover.c b/tests/test_discover.c index 7e0007b11..9227d37ca 100644 --- a/tests/test_discover.c +++ b/tests/test_discover.c @@ -909,6 +909,46 @@ TEST(discover_symlink_skipped) { #endif } +/* #1815: a symlink skip must be as visible as the other two skip paths in + * the same walk (gitignore/cbmignore/skip-list files, excluded dirs): a + * subtree dropped because it's a symlink must not look identical to a + * complete index. */ +TEST(discover_symlink_skip_is_reported) { +#ifdef _WIN32 + SKIP_PLATFORM("Windows: symlinks need admin / symlink() unavailable"); +#else + char *base = th_mktempdir("cbm_disc_sym_rep"); + ASSERT(base != NULL); + + th_write_file(TH_PATH(base, "real.go"), "package main\n"); + char real_path[512], link_path[512]; + snprintf(real_path, sizeof(real_path), "%s/real.go", base); + snprintf(link_path, sizeof(link_path), "%s/link.go", base); + symlink(real_path, link_path); + + cbm_discover_opts_t opts = {0}; + cbm_file_info_t *files = NULL; + int count = 0; + cbm_ignored_file_t *ignored = NULL; + int ignored_count = 0; + int ignored_total = 0; + + int rc = cbm_discover_ex2(base, &opts, &files, &count, NULL, NULL, &ignored, &ignored_count, + &ignored_total); + ASSERT_EQ(rc, 0); + ASSERT_EQ(count, 1); /* real.go only, same as discover_symlink_skipped */ + ASSERT_EQ(ignored_total, 1); + ASSERT_EQ(ignored_count, 1); + ASSERT_STR_EQ(ignored[0].rel_path, "link.go"); + ASSERT_STR_EQ(ignored[0].reason, "symlink"); + + cbm_discover_free(files, count); + cbm_discover_free_ignored(ignored, ignored_count); + th_cleanup(base); + PASS(); +#endif +} + TEST(discover_new_ignore_patterns) { char *base = th_mktempdir("cbm_disc_newign"); ASSERT(base != NULL); @@ -1785,6 +1825,7 @@ SUITE(discover) { RUN_TEST(discover_cbmignore); RUN_TEST(discover_cbmignore_stacks); RUN_TEST(discover_symlink_skipped); + RUN_TEST(discover_symlink_skip_is_reported); RUN_TEST(discover_new_ignore_patterns); RUN_TEST(discover_generic_dirs_full_mode); RUN_TEST(discover_generic_dirs_fast_mode); diff --git a/tests/test_mcp.c b/tests/test_mcp.c index ef7823a12..7cd9a279c 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -10237,6 +10237,73 @@ TEST(search_code_path_filter_prefilter_keeps_matches) { PASS(); } +/* #2011: a grep record longer than the read buffer used to be split across two + * fgets calls, and the continuation was parsed as a fresh file:line:content + * record — inventing a file path out of matched content and a line number of 0. + * A single >2 KiB line containing colons reproduces it: the repository holds + * two matches, and the split used to report three. */ +TEST(search_code_long_line_does_not_invent_matches) { + char tmp[512]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_srch_longline_XXXXXX"); + ASSERT_TRUE(cbm_mkdtemp(tmp) != NULL); + + char big_path[768], normal_path[768]; + snprintf(big_path, sizeof(big_path), "%s/big.js", tmp); + snprintf(normal_path, sizeof(normal_path), "%s/normal.js", tmp); + + /* One line well over the 2 KiB read buffer, full of colons, with the + * needle at the very end so the match lands past the split point. */ + FILE *fp = fopen(big_path, "w"); + ASSERT_NOT_NULL(fp); + fprintf(fp, "var CFG=({"); + for (int i = 0; i < 260; i++) { + fprintf(fp, "k%d:\"v%d\",", i, i); + } + fprintf(fp, "NEEDLEmarker:1});\n"); + fclose(fp); + + fp = fopen(normal_path, "w"); + ASSERT_NOT_NULL(fp); + fprintf(fp, "const NEEDLEmarker = 42;\n"); + fclose(fp); + + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + const char *proj = "longline-search"; + cbm_mcp_server_set_project(srv, proj); + cbm_store_upsert_project(cbm_mcp_server_store(srv), proj, tmp); + + char *resp = cbm_mcp_server_handle( + srv, "{\"jsonrpc\":\"2.0\",\"id\":96,\"method\":\"tools/call\"," + "\"params\":{\"name\":\"search_code\"," + "\"arguments\":{\"pattern\":\"NEEDLEmarker\",\"project\":\"longline-search\"}}}"); + ASSERT_NOT_NULL(resp); + char *inner = extract_text_content(resp); + ASSERT_NOT_NULL(inner); + + /* Exactly the two real matches — the split must not produce a third. */ + int grep_matches = -1; + const char *g = strstr(inner, "\"total_grep_matches\":"); + if (g) { + sscanf(g, "\"total_grep_matches\":%d", &grep_matches); + } else if ((g = strstr(inner, "total_grep_matches: ")) != NULL) { + sscanf(g, "total_grep_matches: %d", &grep_matches); + } + ASSERT_EQ(grep_matches, 2); + + /* Every reported line number belongs to a real line: the fabricated row + * carried line 0, which no grep -n record can produce. */ + ASSERT_TRUE(strstr(inner, "\"line\":0") == NULL); + + free(inner); + free(resp); + cbm_mcp_server_free(srv); + unlink(big_path); + unlink(normal_path); + rmdir(tmp); + PASS(); +} + /* PR #756 (distilled): path_filter matching ZERO indexed files. With the * prefilter the scoped filelist has 0 records, and handle_search_code now * skips the grep subprocess entirely (xargs on an empty filelist is @@ -20044,6 +20111,7 @@ SUITE(mcp) { RUN_TEST(search_code_scoped_path_with_cjk_root_issue903); #endif RUN_TEST(search_code_path_filter_prefilter_keeps_matches); + RUN_TEST(search_code_long_line_does_not_invent_matches); RUN_TEST(search_code_path_filter_matches_nothing); RUN_TEST(search_code_file_pattern_prefilter_boundaries); RUN_TEST(search_code_windows_scope_prefilter_removes_pipeline_filter); diff --git a/tests/test_parallel_harness_contract.sh b/tests/test_parallel_harness_contract.sh index 17dff489d..31854b995 100755 --- a/tests/test_parallel_harness_contract.sh +++ b/tests/test_parallel_harness_contract.sh @@ -45,6 +45,23 @@ if [[ "$probe_sites" == *kill_grace* ]]; then exit 1 fi +# Barrier files must be published atomically. Path.write_text creates and +# truncates before it writes, so a poller that saw `.ready` appear and +# then parsed the leader pid could read the zero-byte window and fail on +# int("") -- a scheduler-side race surfacing as a harness flake. Asserted +# structurally: no barrier file is written in place, and the scheduler renames +# a same-directory temp file onto the destination instead. +barrier_writes=$(grep -nE '^[[:space:]]*(ready|leader_exited)\.write_text\(' "$scheduler" || true) +if [ -n "$barrier_writes" ]; then + echo "FAIL: scheduler barrier files are written in place (non-atomic):" >&2 + echo "$barrier_writes" >&2 + exit 1 +fi +if ! grep -Fq 'os.replace(' "$scheduler"; then + echo "FAIL: scheduler does not rename barrier files into place" >&2 + exit 1 +fi + python3 - "$scheduler" <<'PROBE' from __future__ import annotations