From 329c47954f10d4541fe1c074361c5868fe224e51 Mon Sep 17 00:00:00 2001 From: bethz Date: Fri, 10 Jul 2026 19:05:22 +0800 Subject: [PATCH 1/3] fix: stop CPU/memory blowup on non-git projects (#713, #937, #841) Two independent resource-exhaustion bugs hit non-git folders: 1. Memory (#713): the auto-index file-count guard ran `git ls-files | wc -l`, which returns 0 in a non-git folder, so the auto_index_limit cap could never fire and auto-index would eat tens of GB on large code folders. The pipe was also broken on Windows (cmd.exe passes single quotes literally and has no `wc`). Fix: count `git ls-files` lines in C (portable), and fall back to a new bounded native walk - cbm_discover_count_files() - when the git count is 0 (non-git roots). The walk honors the same skip dirs, suffix filters, and language gate as discovery and stops at the limit, so huge trees cost O(limit) work. 2. CPU (#937/#841): the watcher treated "working tree is dirty" as "changed", so a tree that merely STAYED dirty re-indexed on every poll cycle (5-60s), forever. Worse, `git rev-parse` walks up the tree, so a plain non-git folder nested under an unrelated ancestor repo was misclassified as git - and, being entirely untracked, it looked permanently dirty: a guaranteed infinite re-index loop. Fix: (a) treat a folder with no .git of its own and zero tracked files as non-git (skip polling); an empty repo root keeps its .git, a monorepo sub-package keeps its tracked files, so both stay watched. (b) Replace the boolean dirty check with a status signature - an FNV-1a hash of `git status --porcelain --untracked-files=all -- .` output plus the mtime/size of every listed file. A poll triggers a re-index only when the signature MOVES, so persistent dirt no longer loops while repeated edits to the same file are still caught. Status is scoped to the watched subtree so a monorepo package does not churn on sibling changes. The signature is committed only after a successful re-index, so failed runs retry on the next poll. Tests: watcher/discover/gitignore/git_context (173) and mcp (145) suites pass under ASan+UBSan. New regression tests: watcher_nested_non_git_skips, discover_count_files_bounded; the old watcher_continued_dirty test asserted the buggy every-poll re-index and now asserts the fixed behavior. E2E-verified with the production binary: a 15-file non-git folder with auto_index_limit=10 now logs autoindex.skip reason=too_many_files files=11 (early exit at limit+1); with limit=100 auto-index proceeds normally. Co-Authored-By: Claude Fable 5 Signed-off-by: bethz --- scripts/security-allowlist.txt | 3 +- src/discover/discover.c | 87 +++++++++++++ src/discover/discover.h | 8 ++ src/mcp/mcp.c | 49 ++++++-- src/watcher/watcher.c | 215 +++++++++++++++++++++++++++------ tests/test_discover.c | 32 +++++ tests/test_watcher.c | 79 +++++++++++- 7 files changed, 419 insertions(+), 54 deletions(-) diff --git a/scripts/security-allowlist.txt b/scripts/security-allowlist.txt index ed329619b..258bbc6df 100644 --- a/scripts/security-allowlist.txt +++ b/scripts/security-allowlist.txt @@ -19,8 +19,9 @@ src/cli/cli.c:popen:sha256 checksum computation via shasum # ── Watcher: git status polling (repo paths validated via cbm_validate_shell_arg) ── src/watcher/watcher.c:system:git repo detection (is_git_repo) src/watcher/watcher.c:cbm_popen:git HEAD hash (git_head) -src/watcher/watcher.c:cbm_popen:git working tree status (git_is_dirty) +src/watcher/watcher.c:cbm_popen:git working tree status signature (git_status_signature) src/watcher/watcher.c:cbm_popen:git file count (git_file_count) +src/watcher/watcher.c:cbm_popen:git toplevel resolution (git_show_toplevel) src/watcher/watcher.c:popen:via cbm_popen wrapper calls # ── Git context: git metadata resolution (repo paths validated via cbm_validate_shell_arg) ── diff --git a/src/discover/discover.c b/src/discover/discover.c index 785640713..db1a2d165 100644 --- a/src/discover/discover.c +++ b/src/discover/discover.c @@ -1080,6 +1080,93 @@ void cbm_discover_free_excluded(char **excluded, int count) { free(excluded); } +/* ── Bounded file counting (#713) ───────────────────────────────── + * Count the indexable files under repo_path without building a result list, + * stopping as soon as the count exceeds `limit`. Serves as the auto-index + * OOM guard for non-git roots, where `git ls-files` cannot provide a count. + * Honors the same hardcoded skip dirs, suffix filters, and language gate as + * full discovery (mode FULL) but deliberately NOT gitignore: over-counting + * only makes the guard more conservative, and this path must stay cheap. */ +int cbm_discover_count_files(const char *repo_path, int limit) { + if (!repo_path || !repo_path[0] || limit < 0) { + return 0; + } + + struct stat st; + if (wide_stat(repo_path, &st) != 0 || !S_ISDIR(st.st_mode)) { + return 0; + } + + /* Dynamic stack of absolute directory paths (heap-owned). */ + int cap = CBM_SZ_64; + char **stack = malloc((size_t)cap * sizeof(char *)); + if (!stack) { + return 0; + } + stack[0] = strdup(repo_path); + if (!stack[0]) { + free(stack); + return 0; + } + int top = 1; + + int count = 0; + while (top > 0 && count <= limit) { + char *dir = stack[--top]; + cbm_dir_t *d = cbm_opendir(dir); + if (!d) { + free(dir); + continue; + } + + cbm_dirent_t *entry; + while (count <= limit && (entry = cbm_readdir(d)) != NULL) { + if (entry->is_dir) { + if (cbm_should_skip_dir(entry->name, CBM_MODE_FULL)) { + continue; + } + char sub[CBM_SZ_4K]; + snprintf(sub, sizeof(sub), "%s/%s", dir, entry->name); + /* safe_stat skips symlinks / junctions so the walk cannot + * cycle or escape the root — same policy as discovery. */ + struct stat sst; + if (safe_stat(sub, &sst) != 0 || !S_ISDIR(sst.st_mode)) { + continue; + } + if (top >= cap) { + int new_cap = cap * PAIR_LEN; + char **grown = realloc(stack, (size_t)new_cap * sizeof(char *)); + if (!grown) { + continue; /* OOM: skip subtree — count stays a lower bound */ + } + stack = grown; + cap = new_cap; + } + char *copy = strdup(sub); + if (copy) { + stack[top++] = copy; + } + } else { + if (cbm_has_ignored_suffix(entry->name, CBM_MODE_FULL)) { + continue; + } + if (cbm_language_for_filename(entry->name) == CBM_LANG_COUNT) { + continue; + } + count++; + } + } + cbm_closedir(d); + free(dir); + } + + for (int i = 0; i < top; i++) { + free(stack[i]); + } + free(stack); + return count; +} + void cbm_discover_free_ignored(cbm_ignored_file_t *ignored, int count) { if (!ignored) { return; diff --git a/src/discover/discover.h b/src/discover/discover.h index fd2ad35d7..1d476f2cb 100644 --- a/src/discover/discover.h +++ b/src/discover/discover.h @@ -157,6 +157,14 @@ int cbm_discover_ex2(const char *repo_path, const cbm_discover_opts_t *opts, cbm cbm_ignored_file_t **ignored_out, int *ignored_count_out, int *ignored_total_out); +/* Count indexable files under repo_path with an early exit: stops walking as + * soon as the count exceeds `limit` and returns limit + 1 (#713). Honors the + * hardcoded skip dirs, suffix filters, and language gate (mode FULL), but not + * gitignore — over-counting only makes a size guard more conservative. Used + * as the auto-index OOM guard for roots where `git ls-files` has no answer + * (non-git folders). Returns 0 on a missing/invalid root. */ +int cbm_discover_count_files(const char *repo_path, int limit); + /* Free an array of file info results. NULL-safe. */ void cbm_discover_free(cbm_file_info_t *files, int count); diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index a0d8e2a91..fc16372f4 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -44,6 +44,7 @@ enum { #include "pipeline/pipeline.h" #include "pipeline/pass_cross_repo.h" #include "git/git_context.h" +#include "discover/discover.h" #include "cli/cli.h" #include "watcher/watcher.h" #include "foundation/mem.h" @@ -6280,26 +6281,52 @@ static void maybe_auto_index(cbm_mcp_server_t *srv) { return; } - /* Quick file count check to avoid OOM on massive repos */ + /* File-count guard against OOM on massive roots (#713). For git repos, + * count tracked files via `git ls-files`, tallying newlines in C — the + * old `| wc -l` pipe used single quotes (passed through literally by + * cmd.exe) and `wc` (absent on Windows), so it silently returned nothing + * there. For non-git roots — where ls-files reports 0 files and the old + * guard could NEVER fire, letting auto-index eat tens of GB on large + * code folders — fall back to a bounded native walk that honors the same + * skip dirs and language gate as discovery and stops at the limit. */ if (!cbm_validate_shell_arg(srv->session_root)) { cbm_log_warn("autoindex.skip", "reason", "path contains shell metacharacters"); return; } +#ifdef _WIN32 + const char *ai_null_dev = "NUL"; +#else + const char *ai_null_dev = "/dev/null"; +#endif + int count = 0; char cmd[CBM_SZ_1K]; - snprintf(cmd, sizeof(cmd), "git -C '%s' ls-files 2>/dev/null | wc -l", srv->session_root); + snprintf(cmd, sizeof(cmd), "git -C \"%s\" ls-files 2>%s", srv->session_root, ai_null_dev); FILE *fp = cbm_popen(cmd, "r"); if (fp) { - char line[CBM_SZ_64]; - if (fgets(line, sizeof(line), fp)) { - int count = (int)strtol(line, NULL, CBM_DECIMAL_BASE); - if (count > file_limit) { - cbm_log_warn("autoindex.skip", "reason", "too_many_files", "files", line, "limit", - CBM_CONFIG_AUTO_INDEX_LIMIT); - cbm_pclose(fp); - return; + char buf[CBM_SZ_1K]; + size_t n; + while ((n = fread(buf, 1, sizeof(buf), fp)) > 0) { + for (size_t i = 0; i < n; i++) { + if (buf[i] == '\n') { + count++; + } } } - cbm_pclose(fp); + if (cbm_pclose(fp) != 0) { + count = 0; /* not a git repo (or git failed) → use the walk below */ + } + } + if (count == 0) { + count = cbm_discover_count_files(srv->session_root, file_limit); + } + if (count > file_limit) { + char count_str[CBM_SZ_32]; + char limit_str[CBM_SZ_32]; + snprintf(count_str, sizeof(count_str), "%d", count); + snprintf(limit_str, sizeof(limit_str), "%d", file_limit); + cbm_log_warn("autoindex.skip", "reason", "too_many_files", "files", count_str, "limit", + limit_str); + return; } /* Launch auto-index in background */ diff --git a/src/watcher/watcher.c b/src/watcher/watcher.c index 396964208..9284f14a1 100644 --- a/src/watcher/watcher.c +++ b/src/watcher/watcher.c @@ -3,13 +3,25 @@ * * Strategy: git status + HEAD tracking (the most reliable approach). * For non-git projects, the watcher skips polling (no fsnotify/dirmtime yet). - * + * A directory is treated as git only when it has its own .git entry or at + * least one tracked file: `git rev-parse` alone walks UP the tree, so a + * plain non-git folder nested under an unrelated ancestor repo used to be + * misclassified as git — and, being entirely untracked, looked permanently + * dirty (#937/#841). * * Per-project state tracks: * - Last git HEAD hash (detects commits, checkout, pull) + * - Last working-tree status signature (detects content changes) * - Last poll time + adaptive interval * - Whether the project is a git repo * + * Change detection is signature-based, not dirty-based: a poll triggers a + * re-index only when the status signature (porcelain output + mtime/size of + * each listed file) CHANGES, so a tree that merely STAYS dirty no longer + * re-indexes on every poll cycle (#937 write amplification). Status is + * scoped to the watched subtree (`-- .`) so a monorepo sub-package does not + * re-index on sibling churn. + * * Adaptive interval: 5s base + 1s per 500 files, capped at 60s. * Matches the Go watcher's `pollInterval()` logic. */ @@ -38,7 +50,10 @@ typedef struct { char *project_name; char *root_path; + char *worktree_root; /* git toplevel (status paths are relative to it) */ char last_head[CBM_SZ_64]; /* git HEAD hash */ + uint64_t last_status_sig; /* working-tree status signature (0 = clean) */ + uint64_t pending_status_sig; /* signature computed by the latest check */ bool is_git; /* false → skip polling */ bool baseline_done; /* true after first poll */ int missing_root_count; /* consecutive polls where root was missing (ENOENT/ENOTDIR) */ @@ -150,64 +165,159 @@ static int git_head(const char *root_path, char *out, size_t out_size) { return CBM_NOT_FOUND; } -/* Returns true if working tree has changes (modified, untracked, etc.). - * Also checks submodules via `git submodule foreach` to detect uncommitted - * changes inside submodules that `git status` alone would not report. */ -static bool git_is_dirty(const char *root_path) { +/* Resolve the repo toplevel for root_path (git prints '/'-separated absolute + * paths on every platform). Returns a heap copy or NULL. */ +static char *git_show_toplevel(const char *root_path) { char cmd[CBM_SZ_1K]; - snprintf(cmd, sizeof(cmd), - "git --no-optional-locks -C \"%s\" status --porcelain " - "--untracked-files=normal 2>%s", - root_path, WATCHER_NULDEV); + snprintf(cmd, sizeof(cmd), "git -C \"%s\" rev-parse --show-toplevel 2>%s", root_path, + WATCHER_NULDEV); FILE *fp = cbm_popen(cmd, "r"); if (!fp) { - return false; + return NULL; } - - char line[CBM_SZ_256]; - bool dirty = false; + char line[CBM_SZ_1K]; + char *out = NULL; if (fgets(line, sizeof(line), fp)) { size_t len = strlen(line); while (len > 0 && (line[len - SKIP_ONE] == '\n' || line[len - SKIP_ONE] == '\r')) { line[--len] = '\0'; } if (len > 0) { - dirty = true; + out = strdup(line); } } - cbm_pclose(fp); + if (cbm_pclose(fp) != 0) { + free(out); + return NULL; + } + return out; +} + +/* True when root_path has its OWN .git entry (directory, or the gitlink file + * of a linked worktree) — i.e. root_path is itself a repo root. */ +static bool has_dot_git(const char *root_path) { + char path[CBM_SZ_1K]; + snprintf(path, sizeof(path), "%s/.git", root_path); + struct stat st; + return stat(path, &st) == 0; +} - if (dirty) { - return true; +/* ── Working-tree status signature ────────────────────────────────── + * FNV-1a hash over `git status --porcelain` output scoped to the watched + * subtree, mixing in the mtime + size of every listed path. Two properties + * the old boolean "is dirty?" check lacked (#937): + * - a tree that merely STAYS dirty hashes to the same value, so persistent + * untracked/modified files no longer re-index on every poll; + * - a repeated edit to an already-dirty file changes its mtime, so the + * signature still moves and the change is picked up. + * Returns 0 for a clean tree; dirty trees always return nonzero. */ + +#define FNV_OFFSET 1469598103934665603ULL +#define FNV_PRIME 1099511628211ULL + +static void sig_mix(uint64_t *h, const void *data, size_t len) { + const unsigned char *p = data; + for (size_t i = 0; i < len; i++) { + *h = (*h ^ p[i]) * FNV_PRIME; } +} -#if !defined(_WIN32) - /* Check submodules: uncommitted changes inside a submodule are invisible - * to the parent's git status. Use `git submodule foreach` as a portable - * fallback (Apple Git lacks --recurse-submodules). POSIX-only: foreach takes - * an inner shell command that cmd.exe cannot pass intact; the parent-repo - * status check above already covers the common (non-submodule) case. */ +/* Extract the pathname from one porcelain-v1 status line ("XY path", + * "?? path", "R old -> new"). Returns NULL when the line is too short. */ +static const char *status_line_path(const char *line) { + if (strlen(line) < 4) { /* "XY " + at least one path char */ + return NULL; + } + const char *path = line + 3; + const char *arrow = strstr(path, " -> "); + if (arrow) { + path = arrow + 4; /* rename: stat the new name */ + } + return path; +} + +static uint64_t git_status_signature(const char *root_path, const char *worktree_root) { + char cmd[CBM_SZ_1K]; + /* `-- .` scopes status to the watched subtree so a monorepo sub-package + * does not churn on sibling changes. --untracked-files=all lists files + * inside untracked directories individually so their mtimes are seen. */ snprintf(cmd, sizeof(cmd), - "git --no-optional-locks -C '%s' submodule foreach --quiet --recursive " - "'git status --porcelain --untracked-files=normal 2>/dev/null' " - "2>/dev/null", - root_path); - fp = cbm_popen(cmd, "r"); + "git --no-optional-locks -C \"%s\" status --porcelain " + "--untracked-files=all -- . 2>%s", + root_path, WATCHER_NULDEV); + FILE *fp = cbm_popen(cmd, "r"); if (!fp) { - return false; + return 0; } - if (fgets(line, sizeof(line), fp)) { + + uint64_t h = FNV_OFFSET; + bool any = false; + char line[CBM_SZ_1K]; + while (fgets(line, sizeof(line), fp)) { size_t len = strlen(line); while (len > 0 && (line[len - SKIP_ONE] == '\n' || line[len - SKIP_ONE] == '\r')) { line[--len] = '\0'; } - if (len > 0) { - dirty = true; + if (len == 0) { + continue; + } + any = true; + sig_mix(&h, line, len); + + /* Mix in mtime + size so edits to an already-listed file move the + * signature. Porcelain paths are relative to the repo toplevel. + * Quoted paths (core.quotepath escapes) and stat failures simply + * skip this step — the line hash above still covers add/remove. */ + const char *rel = status_line_path(line); + if (rel && rel[0] != '"' && worktree_root && worktree_root[0]) { + char abs[CBM_SZ_2K]; + int n = snprintf(abs, sizeof(abs), "%s/%s", worktree_root, rel); + struct stat st; + if (n > 0 && n < (int)sizeof(abs) && stat(abs, &st) == 0) { + int64_t mtime = (int64_t)st.st_mtime; + int64_t size = (int64_t)st.st_size; + sig_mix(&h, &mtime, sizeof(mtime)); + sig_mix(&h, &size, sizeof(size)); + } } } cbm_pclose(fp); + +#if !defined(_WIN32) + /* Submodules: uncommitted changes inside a submodule are invisible to + * the parent's git status. Hash the foreach output too (POSIX-only: + * foreach takes an inner shell command that cmd.exe cannot pass intact). */ + snprintf(cmd, sizeof(cmd), + "git --no-optional-locks -C '%s' submodule foreach --quiet --recursive " + "'git status --porcelain --untracked-files=normal 2>/dev/null; echo \"$sm_path\"' " + "2>/dev/null", + root_path); + fp = cbm_popen(cmd, "r"); + if (fp) { + bool sub_any = false; + while (fgets(line, sizeof(line), fp)) { + size_t len = strlen(line); + while (len > 0 && (line[len - SKIP_ONE] == '\n' || line[len - SKIP_ONE] == '\r')) { + line[--len] = '\0'; + } + /* Porcelain lines start with a 2-char code; the bare $sm_path + * separator lines only matter once real status lines exist. */ + if (len > 2 && (line[2] == ' ' || strstr(line, " -> "))) { + sub_any = true; + } + if (len > 0) { + sig_mix(&h, line, len); + } + } + cbm_pclose(fp); + any = any || sub_any; + } #endif - return dirty; + + if (!any) { + return 0; + } + return h ? h : 1; /* dirty must never collide with the clean value 0 */ } /* Count tracked files via git ls-files */ @@ -254,6 +364,7 @@ static void state_free(project_state_t *s) { } free(s->project_name); free(s->root_path); + free(s->worktree_root); free(s); } @@ -496,9 +607,29 @@ static void init_baseline(project_state_t *s) { s->baseline_done = true; if (s->is_git) { - git_head(s->root_path, s->last_head, sizeof(s->last_head)); s->file_count = git_file_count(s->root_path); + /* `git rev-parse` walks UP the tree, so a plain non-git folder nested + * under an unrelated ancestor repo answers "git" too. Such a folder + * has no .git of its own and zero tracked files — every file in it is + * untracked in the ancestor, which made the old dirty check fire on + * EVERY poll and re-index forever (#937/#841). Treat it as non-git. + * A freshly-initialized empty repo root keeps its own .git and stays + * watched; a monorepo sub-package keeps its tracked files ditto. */ + if (s->file_count == 0 && !has_dot_git(s->root_path)) { + s->is_git = false; + } + } + + if (s->is_git) { + git_head(s->root_path, s->last_head, sizeof(s->last_head)); s->interval_ms = cbm_watcher_poll_interval_ms(s->file_count); + free(s->worktree_root); + s->worktree_root = git_show_toplevel(s->root_path); + /* Deliberately NOT snapshotting the status signature here: it stays + * 0 ("clean"), so a tree already dirty at registration re-indexes + * ONCE on the first poll — edits racing the initial index are never + * missed — and then settles (the signature is committed after that + * re-index, so persistent dirt does not loop, #937). */ cbm_log_info("watcher.baseline", "project", s->project_name, "strategy", "git", "files", s->file_count > 0 ? "yes" : "0"); } else { @@ -515,18 +646,23 @@ static bool check_changes(project_state_t *s) { } /* Check HEAD movement */ + bool head_moved = false; char head[CBM_SZ_64] = {0}; if (git_head(s->root_path, head, sizeof(head)) == 0) { if (s->last_head[0] != '\0' && strcmp(head, s->last_head) != 0) { /* HEAD moved — commit, checkout, pull */ - strncpy(s->last_head, head, sizeof(s->last_head) - 1); - return true; + head_moved = true; } strncpy(s->last_head, head, sizeof(s->last_head) - 1); } - /* Check working tree */ - return git_is_dirty(s->root_path); + /* Check working tree: trigger only when the status SIGNATURE moves, not + * whenever the tree is merely dirty — a persistently dirty tree used to + * re-index on every single poll (#937). The signature is committed to + * last_status_sig only after a successful re-index, so a failed run is + * retried on the next poll. */ + s->pending_status_sig = git_status_signature(s->root_path, s->worktree_root); + return head_moved || s->pending_status_sig != s->last_status_sig; } /* Context for poll_once foreach callback */ @@ -639,6 +775,9 @@ static void poll_project(const char *key, void *val, void *ud) { ctx->reindexed++; /* Update HEAD after successful reindex */ git_head(s->root_path, s->last_head, sizeof(s->last_head)); + /* Commit the PRE-index signature: files that changed while the + * pipeline ran hash differently on the next poll and re-trigger. */ + s->last_status_sig = s->pending_status_sig; /* Refresh file count for interval */ s->file_count = git_file_count(s->root_path); s->interval_ms = cbm_watcher_poll_interval_ms(s->file_count); diff --git a/tests/test_discover.c b/tests/test_discover.c index 1663b7f38..8b9a543f9 100644 --- a/tests/test_discover.c +++ b/tests/test_discover.c @@ -327,6 +327,37 @@ TEST(discover_simple) { PASS(); } +TEST(discover_count_files_bounded) { + /* #713: the auto-index guard for non-git roots — counts indexable files + * natively (git ls-files answers 0 there) and stops early at the limit. */ + char *base = th_mktempdir("cbm_disc_count"); + ASSERT(base != NULL); + + th_write_file(TH_PATH(base, "a.go"), "package main\n"); + th_write_file(TH_PATH(base, "sub/b.py"), "print(1)\n"); + th_write_file(TH_PATH(base, "sub/c.py"), "print(2)\n"); + th_write_file(TH_PATH(base, "icon.png"), "binary\n"); /* suffix-skipped */ + th_write_file(TH_PATH(base, "notes.xyzzy"), "no language\n"); /* no grammar */ + th_write_file(TH_PATH(base, "node_modules/d.js"), "x\n"); /* skip dir */ + + /* Full count: only the 3 indexable sources. */ + ASSERT_EQ(cbm_discover_count_files(base, 100), 3); + + /* Early exit: limit 1 → walk stops at 2 (limit + 1), not the true total. */ + ASSERT_EQ(cbm_discover_count_files(base, 1), 2); + + /* Exceeds ⇔ returned value > limit; equal-to-limit does not exceed. */ + ASSERT_EQ(cbm_discover_count_files(base, 3), 3); + + /* Edge cases. */ + ASSERT_EQ(cbm_discover_count_files(NULL, 100), 0); + ASSERT_EQ(cbm_discover_count_files(TH_PATH(base, "missing"), 100), 0); + ASSERT_EQ(cbm_discover_count_files(base, -1), 0); + + th_cleanup(base); + PASS(); +} + TEST(discover_skips_git_dir) { char *base = th_mktempdir("cbm_disc_git"); ASSERT(base != NULL); @@ -1314,6 +1345,7 @@ SUITE(discover) { /* Integration tests (cross-platform) */ RUN_TEST(discover_simple); + RUN_TEST(discover_count_files_bounded); RUN_TEST(discover_skips_git_dir); RUN_TEST(discover_with_gitignore); RUN_TEST(discover_with_global_xdg_ignore); diff --git a/tests/test_watcher.c b/tests/test_watcher.c index ffa817495..71ffb939c 100644 --- a/tests/test_watcher.c +++ b/tests/test_watcher.c @@ -764,6 +764,60 @@ TEST(watcher_non_git_skips) { PASS(); } +TEST(watcher_nested_non_git_skips) { + /* A non-git folder nested under an unrelated ancestor git repo must be + * treated as non-git (#937/#841): `git rev-parse` answers from the + * ancestor, and since every file in the folder is untracked there, the + * old code saw a permanently-dirty tree and re-indexed on EVERY poll. */ + char tmpdir[256]; + snprintf(tmpdir, sizeof(tmpdir), "/tmp/cbm_watcher_nest_XXXXXX"); + if (!cbm_mkdtemp(tmpdir)) + FAIL("cbm_mkdtemp failed"); + + /* Ancestor is a real git repo... */ + if (wt_git(tmpdir, "init -q") != 0) { + th_rmtree(tmpdir); + FAIL("git init failed"); + } + { + char p[300]; + th_write_file(wt_path(p, sizeof(p), tmpdir, "tracked.txt"), "hello\n"); + } + wt_git(tmpdir, "add tracked.txt"); + wt_git(tmpdir, "commit -q -m init"); + + /* ...but the WATCHED folder inside it has no .git and no tracked files. */ + char subdir[512]; + snprintf(subdir, sizeof(subdir), "%s/myproject", tmpdir); + { + char _p[1024]; + snprintf(_p, sizeof(_p), "%s/main.py", subdir); + th_write_file(_p, "def main(): pass\n"); + } + + cbm_store_t *store = cbm_store_open_memory(); + cbm_watcher_t *w = cbm_watcher_new(store, index_callback, NULL); + cbm_watcher_watch(w, "nested-nongit", subdir); + index_call_count = 0; + + /* Baseline — must classify as non-git despite the git ancestor */ + cbm_watcher_poll_once(w); + ASSERT_EQ(index_call_count, 0); + + /* Untracked-in-ancestor files exist the whole time; polls must stay + * quiet instead of re-indexing every cycle. */ + for (int i = 0; i < 3; i++) { + cbm_watcher_touch(w, "nested-nongit"); + cbm_watcher_poll_once(w); + } + ASSERT_EQ(index_call_count, 0); + + cbm_watcher_free(w); + cbm_store_close(store); + th_rmtree(tmpdir); + PASS(); +} + /* ══════════════════════════════════════════════════════════════════ * ADAPTIVE INTERVAL BEHAVIOR * ══════════════════════════════════════════════════════════════════ */ @@ -880,7 +934,7 @@ TEST(watcher_git_removed_no_crash) { th_rmtree(_p); } - /* Poll — should not crash, git_head() and git_is_dirty() fail gracefully */ + /* Poll — should not crash, git_head() and git_status_signature() fail gracefully */ cbm_watcher_touch(w, "rmgit-repo"); cbm_watcher_poll_once(w); /* No assertion on index_call_count — behavior is implementation-defined. @@ -893,8 +947,9 @@ TEST(watcher_git_removed_no_crash) { } TEST(watcher_continued_dirty) { - /* If working tree stays dirty, each poll should re-trigger reindex. - * Port of repeated git sentinel detection behavior. */ + /* A tree that merely STAYS dirty must NOT re-trigger on every poll — + * that was #937 (continuous re-index churn / disk write amplification). + * Only a signature movement (new edit, add/remove, HEAD move) triggers. */ char tmpdir[256]; snprintf(tmpdir, sizeof(tmpdir), "/tmp/cbm_watcher_cont_XXXXXX"); if (!cbm_mkdtemp(tmpdir)) @@ -932,7 +987,22 @@ TEST(watcher_continued_dirty) { cbm_watcher_poll_once(w); ASSERT_EQ(index_call_count, 1); - /* Still dirty — should detect again */ + /* Still dirty but UNCHANGED — must NOT re-trigger (#937: the old + * boolean dirty check re-indexed on every poll, forever). */ + cbm_watcher_touch(w, "cont-repo"); + cbm_watcher_poll_once(w); + ASSERT_EQ(index_call_count, 1); + cbm_watcher_touch(w, "cont-repo"); + cbm_watcher_poll_once(w); + ASSERT_EQ(index_call_count, 1); + + /* A FURTHER edit to the already-dirty file moves the signature + * (size/mtime are mixed in) → detected again. */ + { + char _p[1024]; + snprintf(_p, sizeof(_p), "%s/file.txt", tmpdir); + th_append_file(_p, "dirtier\n"); + } cbm_watcher_touch(w, "cont-repo"); cbm_watcher_poll_once(w); ASSERT_EQ(index_call_count, 2); @@ -1918,6 +1988,7 @@ SUITE(watcher) { /* Non-git project */ RUN_TEST(watcher_non_git_skips); + RUN_TEST(watcher_nested_non_git_skips); /* Adaptive interval behavior */ RUN_TEST(watcher_interval_blocks_repoll); From 0a7d8a5d24477f55f69c92594f603da40e8b4b5b Mon Sep 17 00:00:00 2001 From: bethz Date: Fri, 10 Jul 2026 19:50:41 +0800 Subject: [PATCH 2/3] style: realign project_state_t trailing comments to clang-format The new pending_status_sig member is longer than the previous longest declaration, so clang-format's trailing-comment alignment moves every comment in the struct block two columns right. Formatted with clang-format 20 using the repo .clang-format; fixes the lint / lint CI failure on PR #1001. Co-Authored-By: Claude Fable 5 Signed-off-by: bethz --- src/watcher/watcher.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/watcher/watcher.c b/src/watcher/watcher.c index 9284f14a1..7bd7929dc 100644 --- a/src/watcher/watcher.c +++ b/src/watcher/watcher.c @@ -50,17 +50,17 @@ typedef struct { char *project_name; char *root_path; - char *worktree_root; /* git toplevel (status paths are relative to it) */ - char last_head[CBM_SZ_64]; /* git HEAD hash */ - uint64_t last_status_sig; /* working-tree status signature (0 = clean) */ + char *worktree_root; /* git toplevel (status paths are relative to it) */ + char last_head[CBM_SZ_64]; /* git HEAD hash */ + uint64_t last_status_sig; /* working-tree status signature (0 = clean) */ uint64_t pending_status_sig; /* signature computed by the latest check */ - bool is_git; /* false → skip polling */ - bool baseline_done; /* true after first poll */ - int missing_root_count; /* consecutive polls where root was missing (ENOENT/ENOTDIR) */ - uint64_t first_missing_ms; /* cbm_now_ms() of the streak's first miss (0 = no streak) */ - int file_count; /* approximate, for interval calc */ - int interval_ms; /* adaptive poll interval */ - int64_t next_poll_ns; /* next poll time (monotonic ns) */ + bool is_git; /* false → skip polling */ + bool baseline_done; /* true after first poll */ + int missing_root_count; /* consecutive polls where root was missing (ENOENT/ENOTDIR) */ + uint64_t first_missing_ms; /* cbm_now_ms() of the streak's first miss (0 = no streak) */ + int file_count; /* approximate, for interval calc */ + int interval_ms; /* adaptive poll interval */ + int64_t next_poll_ns; /* next poll time (monotonic ns) */ } project_state_t; /* ── Watcher struct ─────────────────────────────────────────────── */ From 18c5d21ae46727c2a3a11751facf6c5ed2ebc62f Mon Sep 17 00:00:00 2001 From: bethz Date: Fri, 10 Jul 2026 21:40:05 +0800 Subject: [PATCH 3/3] fix(watcher): derive worktree root via --show-cdup, not --show-toplevel The status signature mixes each listed file's mtime/size via stat(worktree_root + "/" + rel). worktree_root came from `git rev-parse --show-toplevel`, but MSYS/Cygwin git prints POSIX-mapped absolute paths (/tmp/...) that a native binary's stat() cannot resolve, so on the CLANG64 CI runner every stat failed silently and the mtime/size mix-in was lost. A second edit to an already-dirty file then left the signature unchanged and no re-index fired - watcher_continued_dirty failed at test_watcher.c:1008 (expected index_call_count 2, got 1) on windows-latest/CLANG64. Use `git rev-parse --show-cdup` instead: it prints the RELATIVE walk-up from root_path to the toplevel (empty when root_path is the toplevel), which we join onto root_path so the result always keeps the caller's path form regardless of which git flavor answered. Verified the output contract with git on Windows: toplevel prints an empty line, a subdir prints "../../" (trailing slash trimmed before the join). Co-Authored-By: Claude Fable 5 Signed-off-by: bethz --- src/watcher/watcher.c | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/src/watcher/watcher.c b/src/watcher/watcher.c index 7bd7929dc..83c264a91 100644 --- a/src/watcher/watcher.c +++ b/src/watcher/watcher.c @@ -165,32 +165,42 @@ static int git_head(const char *root_path, char *out, size_t out_size) { return CBM_NOT_FOUND; } -/* Resolve the repo toplevel for root_path (git prints '/'-separated absolute - * paths on every platform). Returns a heap copy or NULL. */ +/* Resolve the repo toplevel for root_path. Uses --show-cdup — the RELATIVE + * walk-up from root_path to the toplevel (empty when root_path IS the + * toplevel) — joined onto root_path, so the result keeps the caller's path + * form. --show-toplevel is unusable here: MSYS/Cygwin git prints POSIX-mapped + * absolute paths (/tmp/...) that a native stat() cannot resolve, so + * git_status_signature silently lost its mtime/size mix-in on Windows. + * Returns a heap copy or NULL. */ static char *git_show_toplevel(const char *root_path) { char cmd[CBM_SZ_1K]; - snprintf(cmd, sizeof(cmd), "git -C \"%s\" rev-parse --show-toplevel 2>%s", root_path, + snprintf(cmd, sizeof(cmd), "git -C \"%s\" rev-parse --show-cdup 2>%s", root_path, WATCHER_NULDEV); FILE *fp = cbm_popen(cmd, "r"); if (!fp) { return NULL; } char line[CBM_SZ_1K]; - char *out = NULL; + line[0] = '\0'; if (fgets(line, sizeof(line), fp)) { size_t len = strlen(line); - while (len > 0 && (line[len - SKIP_ONE] == '\n' || line[len - SKIP_ONE] == '\r')) { + while (len > 0 && (line[len - SKIP_ONE] == '\n' || line[len - SKIP_ONE] == '\r' || + line[len - SKIP_ONE] == '/')) { line[--len] = '\0'; } - if (len > 0) { - out = strdup(line); - } } if (cbm_pclose(fp) != 0) { - free(out); return NULL; } - return out; + if (line[0] == '\0') { + return strdup(root_path); /* root_path is the toplevel itself */ + } + char joined[CBM_SZ_2K]; + int n = snprintf(joined, sizeof(joined), "%s/%s", root_path, line); + if (n <= 0 || n >= (int)sizeof(joined)) { + return NULL; + } + return strdup(joined); } /* True when root_path has its OWN .git entry (directory, or the gitlink file