perf: make the discovery scan read the filesystem once, not once per tool by AakashVelusamy · Pull Request #234 · websentry-ai/coding-discovery-tool

AakashVelusamy · 2026-08-03T21:37:11Z

Discovery scans spend almost all of their time walking the user's filesystem. This PR makes that walk dramatically cheaper in several stacked layers, with zero change to what the scan discovers — verified byte-identical output on a live home directory.

Layer 1 — walk the tree once for all tools

The scan looks for many tool marker directories (.cursor, .claude, .roo, .windsurf, …) under the same tree. Each tool used to run its own full recursive walk to find its one directory name, so a scan re-read the identical directories a dozen-plus times — O(tools × filesystem). Now each subtree is walked once, recorded as a memoized basename → [dirs] index, and every tool does a lookup. Applied to both the rules and the MCP scans.

Fixture walk	directory reads
rules alone — before	12,519
rules alone — after	1,579 (7.9×)
rules + MCP together — before	18,773
rules + MCP — after	3,157 (~6×)

The twelve independent walks (8 rules + 4 MCP) become two shared traversals.

Layer 2 — read entries with `os.scandir`

The walk called Path.iterdir() then is_dir()/is_symlink() per entry — two extra stat/lstat syscalls each. os.scandir returns the type the OS already reported with each directory entry, so those checks cost nothing. (Path.iterdir is built on scandir, so iteration order is unchanged.)

Layer 3 — faster per-entry skip guards

should_skip_path / should_skip_system_path run on every entry; rewritten from Python generator loops to C-level str.startswith(tuple) / set.isdisjoint (8.3× / 6.0× / 2.0× on the predicates).

Measured end-to-end (Linux VM, real home)

	directory reads	scan phase
Baseline (staging)	213,112	23.4 s
This branch	176,946	17.3 s (−26%)

Why the output is identical, not just similar

The single-pass index replicates the old walk exactly: same OS-specific skip predicate, same depth cap (relative to root_path), symlinked dirs recorded but never descended into, and the old "found the tool dir — don't recurse into it" prune reproduced by outermost_only. MCP dirs are still matched case-insensitively. Dispatch stays depth-first, so output is unchanged, not merely equivalent after sorting. Only hidden directories are indexed (every tool marker is hidden), bounding cache memory to ~3% of directories.

Verification

Adversarial dispatch parity — dispatched directories are byte-identical to staging for rules and MCP, over a fixture built to stress the exact semantics (same-name nesting, cross-name nesting, node_modules/.git skips, symlinks, beyond-max-depth), in both the macOS per-top-level-dir and Linux from-home styles.
Real-home end-to-end — the full generate_report output is byte-identical to staging on a live home (0 diff).
Tests — 14 index unit tests plus the full suite (1,685 passed; the only failures are pre-existing environment detections, identical on staging).

Scope left for follow-ups

claude_code MCP keeps its own walker (it prunes an extra plugins path); the per-tool Windows walkers and the few direct rglob callers (.github, .gemini, GEMINI.md) are unchanged.
On macOS the rules and MCP indexes are still two separate traversals of the same tree (their Linux prunes differ); merging them to one needs OS-aware keying.
The macOS project scan still sweeps from /. Scoping it to user homes would help further but is not byte-identical (it would drop discovery of tool dirs outside any home), so it is deliberately excluded here.

Note

Medium Risk
Touches privileged multi-user filesystem discovery with symlink/containment guards; behavior is contract-tested for parity but regressions would change what configs are found.

Overview
Replaces per-tool recursive filesystem walks with a memoized shared index (project_dir_index.py) so each subtree is traversed once and tools look up hidden marker dirs (.cursor, .claude, etc.) by basename. macOS/Linux rules (walk_for_tool_directories) and generic MCP (walk_for_mcp_configs_generic) now route through dispatch_matches, with OS-specific skip_id caches, direct-walk fallback on index failure, and markers_all_hidden so non-hidden markers do not silently miss.

The index walk uses os.scandir, records only hidden dirs, applies depth/skip pruning, outermost-only nesting (same as “don’t recurse into a matched tool dir”), and dispatch-time checks (symlink/junction, path still under scan root). Hot skip predicates are micro-optimized (set.isdisjoint, startswith(tuple)); SKIP_SYSTEM_DIRS is a frozenset so import-time prefix tuples stay in sync.

Adds contract tests for index parity, security hardening, concurrency, and skip-guard behavior.

^{Reviewed by Cursor Bugbot for commit cd02f4b. Bugbot is set up for automated code reviews on this repo. Configure here.}

Greptile Summary

The PR replaces repeated per-tool filesystem traversals with a memoized shared directory index while preserving platform-specific pruning and extraction behavior.

Adds a single-pass, thread-safe hidden-directory index with direct-walk fallback and dispatch-time containment checks.
Routes Linux/macOS rules discovery and generic MCP discovery through the shared index.
Optimizes skip predicates and adds parity, fallback, concurrency, path-containment, and platform-specific tests.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the prior Windows collection concern is resolved by skipping the macOS/Linux path-predicate test classes on non-POSIX platforms.

Important Files Changed

Filename	Overview
scripts/coding_discovery_tools/project_dir_index.py	Introduces the shared filesystem index, fallback traversal, match pruning, cache synchronization, and dispatch validation.
scripts/coding_discovery_tools/linux_extraction_helpers.py	Routes Linux rules discovery through the shared index while retaining Linux-specific skip semantics.
scripts/coding_discovery_tools/macos_extraction_helpers.py	Routes macOS rules discovery through the shared index and optimizes hot-path skip predicates.
scripts/coding_discovery_tools/mcp_extraction_helpers.py	Migrates generic MCP directory discovery to the shared index with caller-selectable prune-policy keys.
tests/test_project_dir_index.py	Covers index parity, fallback behavior, concurrency, depth limits, non-hidden markers, symlink handling, and containment.
tests/test_skip_path_guards.py	Verifies optimized skip semantics and now scopes platform-specific path assertions to POSIX, resolving the previous Windows-CI issue.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[Rules and MCP extractors] --> B[dispatch_matches]
    B --> C{Hidden marker?}
    C -->|Yes| D[Memoized subtree index]
    C -->|No| E[Direct filesystem walk]
    D --> F[Filter and retain outermost matches]
    D -->|Index failure| E
    E --> G[Validate directory and containment]
    F --> G
    G --> H[Tool-specific extraction callback]

_{Reviews (16): Last reviewed commit: "test: scope the filesystem-root containm..." | Re-trigger Greptile}

Context used (3)

Knowledge Base — Linux Extractors
Knowledge Base — macOS extractors
Knowledge Base — MCP Server Discovery and Scanning

A profile of a real scan (8 tools on a Linux VM, ~17.8s) showed the single biggest cost wasn't the filesystem at all — it was two tiny predicates that run on every entry of every walk, each burning CPU in a Python generator loop: - `should_skip_system_path` — 210k calls, 4.7s, driven by a 4.6M-iteration generator that re-checked every system dir against every path. - `should_skip_path` — same shape over `path.parts`. They now do the same work in C. `str.startswith(tuple)` scans all prefixes in one call, and `set.isdisjoint(path.parts)` replaces the per-part `any(...)` loop. | predicate | before | after | speedup | |------------------------------|--------|-------|---------| | linux `should_skip_system_path` | 381ms | 46ms | **8.3x** | | macOS `should_skip_system_path` | 243ms | 41ms | **6.0x** | | `should_skip_path` | 54ms | 26ms | **2.0x** | (200k calls each, same machine) Behavior is identical — this cannot change what the scan discovers. A parity check over 6080 adversarial paths (exact system dirs, prefixed, lookalikes such as `/usrlocal` vs `/usr`, skip-dir substrings) found zero differences from the old implementations, and `tests/test_skip_path_guards.py` pins the contract: component- boundary semantics on Linux, raw-prefix semantics on macOS, whole-component matches for `SKIP_DIRS`. Only the two shared helper files change; the ~40 per-tool walkers are untouched, so every OS benefits without touching extractor logic. This is the safe, isolated win from the WEB-4701 scan audit; collapsing the ~40 redundant tree walks into one shared traversal remains the larger, separately-scoped follow-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

vigneshsubbiah16

✅ Security consensus: no issues found. (reviewers: Cursor, Claude, Semgrep, Gitleaks)

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head 4a2919db · 2026-08-03T21:41Z

The two system-path predicates only run on macOS/Linux, and the tests use POSIX absolute paths (/System, /proc) that pathlib resolves differently on Windows, so they failed on the Windows CI matrix. Guarded both classes with skipUnless(posix). The should_skip_path (SKIP_DIRS-in-parts) tests stay cross-platform -- component matching works identically on Windows.

AakashVelusamy · 2026-08-03T21:43:25Z

Fixed in `e588781` — the sole finding from both Greptile and Cursor was the same one: the new system-path tests used POSIX absolute paths (/System, /proc) that pathlib renders with backslashes on Windows, so the positive assertions failed on the windows-latest matrix.

Those two predicates (should_skip_system_path) only ever run on macOS/Linux, so I marked both classes @unittest.skipUnless(os.name == "posix"). The should_skip_path tests stay cross-platform — component matching against SKIP_DIRS works identically on Windows.

No production code changed for this; the perf change itself was clean (security consensus came back with no issues). CI is re-running on e588781.

vigneshsubbiah16

🛡️ Automated Security Review (consensus)

0 findings — 0 high-confidence, 0 to triage. Reviewers: Lead, Claude, Semgrep, Gitleaks.

✅ Security consensus: no issues found. (reviewers: Lead, Claude, Semgrep, Gitleaks)

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head e5887817 · 2026-08-03T21:52Z

The discovery scan looks for many different tool marker directories (.cursor, .claude, .roo, .windsurf, and so on) under the same home tree. Until now every tool ran its OWN full recursive walk of that tree to find its one directory name, so a single scan re-read the identical directories a dozen-plus times — the cost grew with the number of tools times the size of the filesystem. This adds a single-pass directory index: each subtree is walked once, recorded as a basename -> [directories] map, and memoized. Every tool then does a dict lookup instead of a fresh walk, so the scan reads the tree a single time. On a 2,490-entry fixture, the eight rules extractors together: | | directory reads (iterdir) | walk time | |---|---|---| | before | 12,519 | 396 ms | | after | 1,579 | 50 ms | That is 7.9x fewer filesystem reads for the rules walk, stacking on top of the per-entry skip-guard speedup already in this branch. Behavior is identical, not merely equivalent. The traversal replicates the old walk's filters verbatim (OS-specific skip predicate, the same depth cap measured against root_path, symlinked dirs recorded but never descended into) and dispatches to the same directories in the same depth-first order. The old walk's "found the tool dir, don't recurse into it" prune is reproduced by outermost_only, which drops any match nested under another match of the same basename. Verified byte-identical dispatch on an adversarial fixture (same-name nesting, cross-name nesting, skip dirs, symlinks, beyond-max-depth) for both the macOS per-top-level-dir and Linux from-home invocation styles. Scope: the shared rules walker on macOS and Linux. The MCP walker, the few direct rglob callers (.github/.gemini/GEMINI.md), and the per-tool Windows walkers still walk independently and are the next candidates for the same index. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

vigneshsubbiah16

✅ Security consensus: no issues found. (reviewers: Cursor, Claude, Semgrep, Gitleaks)

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head e4380aee · 2026-08-03T23:20Z

AakashVelusamy · 2026-08-04T01:58:26Z

Pushed a second, bigger perf layer on top of the skip-guard speedup: the scan now walks the filesystem once for all tools instead of once per tool (e4380ae).

Every rules extractor used to run its own full recursive walk to find its one marker dir. They now share a single memoized basename → [dirs] pass. On a 2,490-entry fixture the eight rules extractors together drop from 12,519 → 1,579 iterdir calls (7.9×).

Behavior is identical, not just equivalent — same filters, same depth cap, same symlink handling, same depth-first dispatch order, and the old "don't recurse into a matched dir" prune is reproduced by outermost_only. Evidence:

Check	Result
Adversarial dispatch parity (same/cross-name nesting, skips, symlinks, depth)	byte-identical, both invocation styles
Real-home `generate_report` end-to-end	0-diff vs staging (churn between two staging runs: 856 lines)
New unit tests	11 green
Full suite	1,671 passed (10 failures are pre-existing env detections, identical on staging)

Scope is the shared rules walker (macOS + Linux). MCP / direct-rglob / Windows walkers are the documented next phase on the same index.

Trims speculative complexity from the single-pass index while keeping behavior byte-identical (same dispatch, same 7.9x fewer reads, all parity checks green): - Drop the threading lock and double-checked build. The macOS/Linux tool loop is sequential — there is no concurrent access to guard, so the lock only added ceremony. - Reduce outermost_only to a single ancestor-check pass. The index records a directory before its descendants, so the sort/dedup/drop-set bookkeeping was unnecessary to keep only the shallowest match per path. - Flatten the walk to guard clauses and read each directory's entries once. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

AakashVelusamy · 2026-08-04T02:14:20Z

Simplified the index in a84a088 — dropped a speculative threading lock (the macOS/Linux tool loop is sequential, nothing to guard) and reduced outermost_only to a single ancestor-check pass (the index records a dir before its descendants, so the sort/dedup bookkeeping was unnecessary). Behavior unchanged: same byte-identical dispatch, same 7.9× fewer reads, walker parity + 11 unit tests + 579 walker-exercising tests all green.

vigneshsubbiah16

🛡️ Automated Security Review (consensus)

1 finding — 0 high-confidence, 1 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.

🟡 TRIAGE

Unbounded process-lifetime index cache on large subtrees — scripts/coding_discovery_tools/project_dir_index.py:34
Impact: _INDEX_CACHE retains every directory path discovered per (skip_id, root_path, current_dir) for the full scan process; a root-run sweep across many homes (or one pathologically large tree) can spike RSS and OOM the discovery worker, cutting coverage for remaining users.
Fix: Bound or evict cache entries (e.g., LRU keyed by subtree, or clear_cache() after each home/root completes); alternatively cap stored paths per index and fall back to a fresh walk.
Flagged by: Cursor Security Reviewer

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head a84a088b · 2026-08-04T02:23Z

The single-pass index kept a process-lifetime map of every directory it saw, which a root-run sweep across many homes could grow large enough to pressure a discovery worker's memory (raised in security review). Every tool-marker directory the scan looks for is hidden (.cursor, .claude, .roo, …), so the index now records only hidden dirs. The traversal is unchanged — it still descends into non-hidden dirs to reach hidden ones nested inside — so lookups, dispatch, and the 7.9x read reduction are all identical; only what the cache holds shrinks. On the parity fixture that is 62 paths instead of 1,743 (~3% of directories). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

AakashVelusamy · 2026-08-04T02:38:46Z

Addressed the security-review finding in baf2fab.

🟡 Unbounded process-lifetime index cache — fixed. Every tool-marker directory the scan looks for is hidden (.cursor, .claude, .roo, …), so the index now records only hidden directories. The traversal is unchanged (it still descends into non-hidden dirs to reach hidden ones nested inside), so lookups, dispatch, and the 7.9× read reduction are all identical — only what the cache holds shrinks.

	paths held in cache	dispatch	iterdir calls
before fix	every directory (1,743 on the fixture)	identical	1,579
after fix	hidden dirs only (62, ~3%)	identical	1,579

Verified: walker dispatch still byte-identical old-vs-new on the adversarial fixture (macOS + Linux), 12 index unit tests green, traversal cost unchanged.

vigneshsubbiah16

🛡️ Automated Security Review (consensus)

2 findings — 0 high-confidence, 2 to triage. Reviewers: Claude, Cursor, Semgrep, Gitleaks.

🟡 Transient `iterdir` failure is memoized for all tools

scripts/coding_discovery_tools/project_dir_index.py:55

Impact: A one-off PermissionError/OSError on current_dir.iterdir() returns a partial (possibly empty) index that is cached in _INDEX_CACHE for the process lifetime, so every later tool reuses the truncated result with no error surfaced — widening silent under-reporting vs. the old per-tool walks.
Fix: Track traversal failure in _collect and skip _INDEX_CACHE writes (or invalidate/re-walk on next call); at minimum logger.warning the unreadable path.
Flagged by: Claude

🟡 Broad per-entry exception guard removed from shared walk (macOS regression)

scripts/coding_discovery_tools/project_dir_index.py:71

Impact: _collect only catches (PermissionError, OSError, ValueError) per entry; the replaced macOS walker also caught Exception (e.g. from is_home_dotdir_descendant / should_skip on a bad name), so one unexpected entry error can abort the shared index build and stop discovery for the whole subtree.
Fix: Restore the old per-item except Exception: logger.debug(...); continue in the item loop (and consider the outer walk guard) to match prior macOS resilience.
Flagged by: Claude

Previously acknowledged (not re-flagged)

Unbounded process-lifetime index cache (project_dir_index.py) — Maintainer fixed in baf2fab: index now records only hidden (dot-prefixed) directories, bounding retained paths (~3% on fixture) while keeping traversal and dispatch byte-identical.
POSIX path tests break Windows CI (tests/test_skip_path_guards.py) — Maintainer fixed in e588781: gated macOS/Linux system-path test classes with @unittest.skipUnless(os.name == "posix") since those predicates only run on POSIX hosts.

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head baf2fab1 · 2026-08-04T02:44Z

The new "non-hidden dirs are descended but not stored" test asserted a match with endswith("components/.cursor"), which fails on Windows where paths use backslash separators. Compare Path.parts instead so it holds on every OS. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

vigneshsubbiah16

🛡️ Automated Security Review (consensus)

1 finding — 0 high-confidence, 1 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.

Narrowed exception handling in shared index walk 🟡 TRIAGE

scripts/coding_discovery_tools/project_dir_index.py:58-75

Impact: _collect only catches (PermissionError, OSError, ValueError) per entry; the old macOS walk_for_tool_directories also caught broad Exception and continued, so an unexpected error from a skip predicate (e.g. is_home_dotdir_descendant) or another per-entry failure can abort the whole subtree index before it is cached, yielding silently incomplete discovery for every tool sharing that walk.
Fix: Restore a catch-all in the per-item loop (except Exception: logger.debug(...); continue) and wrap the _collect call in get_subtree_index the same way; optionally persist a partial index and surface incomplete-scan status in the report.
Reviewers: Claude, Lead (macOS regression; Linux walker was already narrow)

Previously acknowledged (not re-flagged)

Unbounded process-lifetime _INDEX_CACHE — Addressed in baf2fab: index now records only hidden (dot-prefixed) directories; maintainer verified ~97% smaller cache with byte-identical dispatch.
Hidden-directory-only index recording — Accepted design: maintainer confirmed every tool marker dir (.cursor, .claude, .roo, …) is hidden, so non-dot tool_dir_name lookups are intentionally out of scope.

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head a05f9df5 · 2026-08-04T02:55Z

Two robustness gaps versus the per-tool walks the index replaced, both from security review: - A one-off failure to list a subtree root was cached for the whole scan, so every later tool inherited the empty result. The index now caches only when the root was readable; an unreadable root returns empty but is re-attempted next time, as the old per-tool walks re-listed each pass. - The old macOS walker caught any per-entry exception and continued (e.g. a bad name tripping a predicate). The index had narrowed that to specific errors, letting one odd entry abort the whole shared build. Restored the broad per-entry guard with a debug log. Traversal, dispatch, and the 7.9x read reduction are unchanged — verified byte-identical walker dispatch on the fixture (macOS + Linux). Adds two tests: an unreadable root is not cached (a retry sees later state) and one blowing-up entry does not abort sibling subtrees. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

AakashVelusamy · 2026-08-04T02:59:07Z

Both triage findings addressed in 6496810 — both were real resilience gaps versus the per-tool walks the index replaced.

🟡 Transient iterdir failure memoized for all tools — fixed. The index now caches a subtree only when its root was readable. An unreadable root returns empty and is not cached, so a later tool re-attempts it — matching the old per-tool walks that re-listed every pass. A deep, persistent permission error still caches (same outcome as before, no perf regression); only the "root couldn't be listed" case is left to retry, with a warning logged.

🟡 Broad per-entry exception guard removed — fixed. Restored the old macOS walker's except Exception: logger.debug(...); continue around each entry, so one odd name that trips should_skip / is_home_dotdir_descendant can't abort the whole shared build.

	before	after
root unreadable	cached empty, reused by all tools	not cached, re-attempted (warn logged)
unexpected entry error	aborts subtree build	skipped, siblings still indexed
dispatch / iterdir count	—	byte-identical / 1,579 (unchanged)

Two tests added (unreadable root not cached → retry sees later state; a blowing-up entry doesn't abort siblings). 14 index unit tests green, walker dispatch still byte-identical on the fixture (macOS + Linux).

vigneshsubbiah16

🛡️ Automated Security Review (consensus)

0 findings — 0 high-confidence, 0 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.

✅ Security consensus: no issues found. (reviewers: Cursor, Claude, Semgrep, Gitleaks)

Previously acknowledged (not re-flagged)

Unbounded process-lifetime index cache — maintainer fixed in baf2fab: index now stores only dot-prefixed (hidden) directories (~3% of paths on fixture); traversal and dispatch unchanged.
Transient iterdir failure memoized for all tools — maintainer fixed in 6496810: unreadable subtree roots return empty and are not cached, so later tools re-attempt (matches old per-tool walks).
Broad per-entry exception guard removed — maintainer fixed in 6496810: per-entry except Exception restored so one bad entry cannot abort the shared subtree build.
POSIX path tests break Windows CI — maintainer fixed in e588781: @unittest.skipUnless(os.name == "posix") on macOS/Linux system-path test classes (test-only; predicates never run on Windows).

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head 64968103 · 2026-08-04T03:00Z

The shared walk called Path.iterdir() and then Path.is_dir() / Path.is_symlink() on every entry — two extra stat/lstat syscalls per item. os.scandir returns the type the OS already reported with each directory entry, so is_dir()/is_symlink() are answered without touching the disk again. Byte-identical: Path.iterdir is itself built on os.scandir, so iteration order — and therefore dispatch order — is unchanged, and is_dir() still follows symlinks as Path.is_dir() did. Verified the dispatched directories are identical to staging on the adversarial fixture (macOS + Linux); 14 index unit tests and 787 walker-exercising tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

The MCP project scan re-walked the whole tree once per tool (cursor, windsurf, roo, kilocode), exactly like the rules scan used to. It now goes through the same single-pass directory index, so those tools share one traversal. Combined rules + MCP over the fixture drops from 18,773 to 3,157 directory reads (~6x) — the twelve independent walks become two shared ones (one rules index, one MCP index; they stay separate because the Linux rules prune omits the hidden-home term). Byte-identical: MCP dispatch is matched case-insensitively as before and outermost_only reproduces the old "found it — don't recurse" prune. Verified the dispatched MCP directories are identical to staging on the adversarial fixture (macOS + Linux), rules dispatch is unchanged, and the full generate_report output is identical to staging on a live home (0 diff). claude_code MCP keeps its own walker (it prunes an extra plugins path); the Windows MCP path is untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

AakashVelusamy · 2026-08-04T07:41:32Z

Pushed two more byte-identical layers (31f0ea7, 671918a):

os.scandir instead of iterdir + stat — is_dir/is_symlink now come from the directory entry the OS already returned, removing two syscalls per item. Order (and dispatch) is unchanged since Path.iterdir is itself built on scandir.

MCP configs go through the same single walk — cursor/windsurf/roo/kilocode used to re-walk the tree once each, just like rules did. Combined rules + MCP over the fixture drops from 18,773 → 3,157 directory reads (12 walks → 2).

Byte-identical, verified the same way as before:

Check	Result
MCP dispatch vs staging (adversarial fixture, macOS + Linux)	identical
Rules dispatch vs staging	still identical
Full `generate_report` on a live home	0 diff vs staging
Suite	1,685 passed (10 failures are pre-existing env detections, identical on staging)

Not included — stopping the macOS / sweep: it would drop discovery of tool dirs outside any home, so it can't be byte-identical. Flagged in the description rather than bundled.

vigneshsubbiah16

✅ Security consensus: no issues found. (reviewers: Cursor, Claude, Semgrep, Gitleaks)

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head 671918aa · 2026-08-04T07:46Z

Addresses the shared index's one worse-than-before failure shape: because every tool reads from it, a fault there could break discovery for all of them at once, whereas the old per-tool walks failed in isolation. All three walkers now dispatch through a fail-safe helper: it looks matches up in the shared index on the fast path, but if anything in the index raises it degrades to an independent, stateless per-tool walk for that one call. So a bug or fault in the shared index costs at most one tool a fall back to the old (correct, slower) behavior — never a scan-wide blackout. Extraction errors are handled by the match callback and never trigger the fallback, so there is no double dispatch. Both paths dispatch the same directories, so output is unchanged. Verified: rules and MCP dispatch stay byte-identical to staging (macOS + Linux); two new tests prove the fallback walk is identical to the index and that discovery survives a raising index. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

anonpran · 2026-08-05T12:51:02Z

+    treated as an index failure (so it never triggers a re-walk / double dispatch).
+    Both paths dispatch the same directories, so output is unchanged whether or not
+    the fallback fires.
+    """


anonpran · 2026-08-05T12:51:08Z

+same-basename de-nesting (``outermost_only``), the depth cap, symlink handling,
+skip-predicate pruning, and per-key memoization.
+"""
+


…-4701-faster-skip-guards # Conflicts: # scripts/coding_discovery_tools/mcp_extraction_helpers.py

vigneshsubbiah16

✅ Security consensus: no issues found. (reviewers: Cursor, Claude, Semgrep, Gitleaks)

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head ff492826 · 2026-08-05T12:55Z

Per review: shorten the module/function docstrings and skip-id comments to the load-bearing points (invariants, why the fallback exists) and drop the step-by-step working narration. Code unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

AakashVelusamy · 2026-08-05T13:00:27Z

@anonpran done in 428a6d1 — trimmed the docstrings/comments across all five spots (project_dir_index.py module + functions, the MCP walker + skip-id, the macOS/Linux walkers, and the test header). Kept only the load-bearing notes (the invariants and why the fallback exists) and dropped the working narration. Comment-only change: net −99 lines, code untouched, 16 index tests still green.

vigneshsubbiah16

🛡️ Automated Security Review (consensus)

5 findings — 1 high-confidence, 4 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.

Medium

🔴 Indexed paths dispatched without re-validation (widened TOCTOU)

Location: scripts/coding_discovery_tools/project_dir_index.py:145
Impact: A user who can mutate their tree during a root-run scan can swap an indexed tool dir (e.g. ~/proj/.cursor) for a symlink after indexing; dispatch later follows it and the privileged scanner may read/report config contents outside the intended boundary.
Fix: Re-validate each target at dispatch (os.open(..., O_RDONLY|O_NOFOLLOW|O_DIRECTORY) or at minimum is_dir() and not is_symlink()) before on_match, and read configs through that descriptor.
Reviewers: Claude

🟡 MCP index cache ignores per-caller `should_skip_func`

Location: scripts/coding_discovery_tools/mcp_extraction_helpers.py:789
Impact: dispatch_matches always keys the shared cache as mcp_project even though should_skip_func is caller-supplied; the first MCP walker to index a subtree pins its prune policy for all later tools, so stricter callers can still have excluded directories dispatched and scanned.
Fix: Include the predicate identity in the cache key (e.g. caller-supplied skip id, or should_skip_func.__qualname__ / id(should_skip)) and assert one id per distinct prune policy.
Reviewers: Claude

🟡 Unsynchronized global `_INDEX_CACHE` under parallel MCP walks

Location: scripts/coding_discovery_tools/project_dir_index.py:23
Impact: The module-global cache has no locking; Windows MCP discovery fans out via ThreadPoolExecutor, so concurrent get_subtree_index calls can race on read/modify of the same dict key, risking corrupted indexes or inconsistent dispatch.
Fix: Guard cache access with a threading.Lock, or avoid sharing mutable cache state across worker threads (per-thread / per-pool indexes).
Reviewers: Cursor

🟡 `outermost_only` may mis-prune cross-basename MCP matches

Location: scripts/coding_discovery_tools/project_dir_index.py:136
Impact: dispatch_matches flattens matches bucket-by-bucket before outermost_only; with case-insensitive MCP matching, a nested differently-cased child can appear before its ancestor, so nested tool dirs the old walk would prune may still be dispatched and scanned.
Fix: Flatten matches in depth-first discovery order (or sort by path depth/length) before outermost_only, and add a case-variant nesting regression test.
Reviewers: Cursor

Low

🟡 System-skip prefixes frozen at import time

Location: scripts/coding_discovery_tools/macos_extraction_helpers.py:99, scripts/coding_discovery_tools/linux_extraction_helpers.py:56
Impact: _SKIP_SYSTEM_PREFIXES / _LINUX_SKIP_SYSTEM_PREFIXES are built once at import; any runtime mutation of the underlying skip lists (tests, future config-driven hardening) is silently ignored and system paths may be walked/reported.
Fix: Derive prefixes inside the predicate (with a small invalidating cache) or freeze the source constants and test that derived tuples stay in sync.
Reviewers: Claude

Previously acknowledged (not re-flagged)

Unbounded process-lifetime index cache — fixed in baf2fab: index now stores only hidden directories (~3% of dirs), bounding memory while keeping dispatch byte-identical.
Transient unreadable-root memoization / per-entry exception abort — fixed in 6496810: unreadable roots are not cached (retried per tool); per-entry errors skip the entry instead of aborting the subtree build.
POSIX system-path tests breaking Windows CI — fixed in e588781 with @unittest.skipUnless(os.name == "posix"); predicates are macOS/Linux-only by design.

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head 428a6d1b · 2026-08-05T13:06Z

…-4701-faster-skip-guards

- TOCTOU at dispatch (Medium): the index records a dir, but a user could swap it for a symlink before dispatch. Re-validate each target at dispatch (lstat, must still be a real directory) before reading config through it, and stop recording or descending symlinked dirs in both the index walk and the fallback walk — a privileged scan must not follow a user's symlink into config. - MCP cache key (triage): dispatch keyed the shared cache by a fixed id even though the prune is caller-supplied. Expose skip_id on walk_for_mcp_configs_ generic (default shared) and document the contract; all current MCP callers use the identical per-OS prune (should_skip_path + should_skip_system_path), so they intentionally share it — a different prune must pass a distinct id. - Unsynchronized cache (triage): Windows MCP fans out over a ThreadPoolExecutor. Guard _INDEX_CACHE with a lock; build outside the lock so parallel walks of different subtrees don't serialize, publish via setdefault. - outermost_only ordering (triage): case-insensitive matches bucket by exact basename, so a differently-cased nested child could flatten ahead of its ancestor. Sort matches shallowest-first before pruning. - Skip prefixes frozen at import (low): freeze SKIP_SYSTEM_DIRS (now a frozenset) so the import-time derived prefix tuple can't drift; test it stays in sync. Also merged staging (2 behind). New regression tests for each finding. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

AakashVelusamy · 2026-08-12T23:18:43Z

All five findings addressed in af94db8 (and merged staging, which was 2 behind).

🔴 Indexed paths dispatched without re-validation (TOCTOU) — `project_dir_index.py`

The index records a real dir, but a user could swap it for a symlink before dispatch. Now:

Re-validate at dispatch: each target is lstat'd right before on_match and must still be a real directory (not a symlink) — narrowing the window back to the direct walk's.
Never follow a symlinked config dir in the first place: both the index walk (_collect) and the fallback walk (_walk_direct) stop recording and descending symlinked dirs. A privileged scan shouldn't follow a user's symlink into config. (Deliberate, security-motivated change from the prior symlink-dispatch behavior; test updated.)

🟡 MCP index cache ignores per-caller `should_skip_func` — `mcp_extraction_helpers.py`

skip_id is now a parameter of walk_for_mcp_configs_generic (default shared), and the contract is documented. Worth noting: every current MCP caller uses the identical per-OS prune (should_skip_path + should_skip_system_path, both stable module functions), so sharing one id is correct and is what preserves the single-walk perf win — id()/__qualname__ keying would fragment the cache (each extractor builds a fresh closure) and regress the very thing this PR fixes. A future caller with a genuinely different prune now passes a distinct skip_id.

🟡 Unsynchronized global `_INDEX_CACHE` under parallel MCP walks — `project_dir_index.py`

Guarded with a threading.Lock. The build runs outside the lock (so parallel walks of different subtrees don't serialize) and publishes via setdefault; a duplicate concurrent build of the same key is wasted but harmless.

🟡 `outermost_only` may mis-prune cross-basename matches — `project_dir_index.py`

Matches are now sorted shallowest-first (len(path.parts)) before outermost_only, so an ancestor always precedes its descendants regardless of which case-variant bucket flattened first.

🟡 System-skip prefixes frozen at import (Low) — `constants.py`

SKIP_SYSTEM_DIRS is now a frozenset (immutable source), so the import-time derived prefix tuple can't silently drift; a test asserts they stay in sync.

Tests: new regressions for each (TestReviewHardening in test_project_dir_index.py) — dispatch symlink-swap + vanished-dir refusal, cross-basename prune, concurrent-cache safety, MCP skip_id forwarding, and the frozen-source sync. The byte-identical index-vs-fallback test still passes. Full local suite green except pre-existing environmental cases (this dev box has junie/Copilot binaries installed, so their "not-detected-when-absent" tests trip — unrelated to this diff, pass on CI).

cursor · 2026-08-12T23:20:48Z

+                continue
+        except OSError:
+            continue
+        on_match(target)


Dispatch order is no longer DFS

Low Severity

Matches are sorted by len(p.parts) before on_match, so extraction runs shallowest-first instead of filesystem DFS order. MCP projects lists and first-seen keys in projects_by_root can therefore differ from the old walk even when the same directories are found.

^{Reviewed by Cursor Bugbot for commit af94db8. Configure here.}

vigneshsubbiah16

🛡️ Automated Security Review (consensus)

2 findings — 0 high-confidence, 2 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.

🟡 TRIAGE — Non-hidden tool markers silently skipped by index path

scripts/coding_discovery_tools/project_dir_index.py:56

Impact: _collect only stores hidden dirs; dispatch_matches looks up the index only, so a non-hidden tool_dir_name yields zero matches with no warning while _walk_direct would still find it — a future marker like AGENTS/ could disappear from reports with all tests still green.

Fix: Enforce the hidden-dir contract at the API boundary (warn/error and route to _walk_direct when the marker is not hidden), or index on “hidden or requested basename.”

Flagged by: Claude

🟡 TRIAGE — Windows MCP may skip NTFS directory junctions

scripts/coding_discovery_tools/project_dir_index.py:54

Impact: DirEntry.is_symlink() treats NTFS junctions as symlinks; the shared index and fallback walk refuse to record or descend them, so project MCP configs under junction-linked trees may be missed compared with the prior Windows walk (PR scope says Windows walkers were unchanged).

Fix: On Windows, distinguish junctions from symlinks (e.g. reuse constants.is_symlink_or_junction only where junction descent is intended) so junction trees are still traversed while real symlinks stay blocked.

Flagged by: Cursor

Previously acknowledged (not re-flagged)

Unbounded process-lifetime index cache — Fixed in baf2fab by indexing hidden dirs only (~3% of dirs); maintainer verified dispatch parity unchanged.
Unsynchronized _INDEX_CACHE under parallel MCP walks — Fixed in af94db8 with threading.Lock and build-outside-lock / setdefault publish.
Dispatch-time TOCTOU (match dir swapped to symlink) — Fixed in af94db8 via os.lstat re-validation before on_match; residual ancestor-symlink window noted as same class, not re-opened.
MCP index cache keyed only by skip_id — Accepted contract in af94db8; all current MCP callers share the same prune; distinct skip_id is the escape hatch.
Deep (non-root) read failures memoized for all tools — Kept as-is in 6496810 (same outcome as old per-tool walks; only unreadable roots are retried).
outermost_only mis-prune across case-variant buckets — Fixed in af94db8 with shallowest-first sort before pruning.
SKIP_SYSTEM_DIRS import-time prefix drift — Fixed in af94db8 by freezing the source set and adding a sync test.
POSIX system-path tests breaking Windows CI — Fixed in e588781 with @unittest.skipUnless(os.name == "posix"); predicates are macOS/Linux-only by design.

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head af94db8a · 2026-08-12T23:25Z

…d index Two follow-ups from the security review: - Windows junctions: DirEntry.is_symlink() reports an NTFS junction as a symlink, so refusing every symlink in the walk also dropped junction-linked config trees the old Windows walk still read. Move the symlink defense to dispatch only: the walk records links/junctions again (never descends them), and dispatch keeps its lstat re-validation — a symlink is S_IFLNK so it is dropped, a junction is S_IFDIR so it passes, exactly like a real directory. This also restores the walk to byte-identical recording. - Non-hidden markers: the index stores only hidden dirs, so a non-hidden marker would match nothing while the direct walk would still find it. Add markers_all_hidden (default True) to dispatch_matches; the MCP walker sets it from whether the marker starts with a dot, routing a non-hidden marker to the direct walk instead of silently missing it. New regressions: junction/real-dir dispatch parity, symlinked-dir recorded but not descended, non-hidden marker routed and found end to end. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

AakashVelusamy · 2026-08-12T23:39:37Z

Both triage findings addressed in ab24644.

🟡 Non-hidden tool markers silently skipped by index path

Real gap: the index stores only hidden dirs, so a non-hidden marker (a future AGENTS/) would match nothing on the index path while the direct walk would still find it. dispatch_matches now takes markers_all_hidden (default True); the MCP walker sets it from tool_dir_name.startswith("."), so a non-hidden marker routes straight to _walk_direct instead of quietly returning zero. Covered by two tests — the walker flags the non-hidden marker, and such a marker is dispatched end to end.

🟡 Windows MCP may skip NTFS directory junctions

Correct — DirEntry.is_symlink() reports a junction as a symlink, so my earlier "skip every symlink in the walk" over-reached and dropped junction-linked config trees the old Windows walk still read. Fixed by moving the symlink defense to dispatch only:

Entry at dispatch	`lstat` type	Old walk	This PR
Real directory	`S_IFDIR`	read	read
NTFS junction	`S_IFDIR` + reparse tag	read	read
Symlink (incl. swapped-in)	`S_IFLNK`	read	dropped
Vanished	—	—	dropped

The walk records links/junctions again (and, as before, never descends into them, so a planted junction can't redirect the scan into another tree). The TOCTOU guard is unchanged — _is_dispatchable re-validates with a single lstat right before on_match, and S_ISDIR naturally passes junctions while dropping symlinks. This also restores the walk to byte-identical recording.

The 5 findings from the prior round remain resolved; full local suite green apart from the pre-existing environmental cases (this box has junie/Copilot binaries installed).

vigneshsubbiah16

🛡️ Automated Security Review (consensus)

2 findings — 1 high-confidence, 1 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.

🔴 HIGH — Ancestor symlink swap bypasses scan-root containment

scripts/coding_discovery_tools/project_dir_index.py:187

Impact: dispatch_matches only lstats the leaf before on_match; if a user replaces an indexed ancestor (e.g. ~/proj) with a symlink after indexing, the cached path can resolve outside the scanned subtree and a privileged multi-user scan may read/report configs from another user's tree.

Fix: Re-check containment at dispatch (e.g. os.path.realpath(target) must stay under os.path.realpath(current_dir) / root_path), or open the leaf with O_NOFOLLOW | O_DIRECTORY and read config via dir_fd= so the path cannot shift between validation and use.

Flagged by: Claude, Cursor (security), Lead

🟡 TRIAGE — Rules walkers omit `markers_all_hidden` fallback

scripts/coding_discovery_tools/macos_extraction_helpers.py:610, scripts/coding_discovery_tools/linux_extraction_helpers.py:170

Impact: The shared index stores only hidden dirs; MCP discovery already routes non-hidden markers to _walk_direct, but macOS/Linux rules walkers always use the index path — a future non-hidden marker (e.g. AGENTS/) would silently discover nothing with no log.

Fix: Pass markers_all_hidden=tool_dir_name.startswith(".") in both rules walkers, mirroring mcp_extraction_helpers.py:797.

Flagged by: Claude, Lead

Previously acknowledged (not re-flagged)

Unbounded process-lifetime index cache — Maintainer bounded cache to hidden dirs only (~3% of paths); traversal and dispatch unchanged (af94db8).
Unsynchronized _INDEX_CACHE under parallel MCP walks — threading.Lock added; build runs outside lock (af94db8).
Leaf directory swapped to symlink at dispatch (TOCTOU) — _is_dispatchable() re-lstats the leaf and drops symlinks/vanished dirs before on_match (af94db8).
outermost_only mis-prune across case-variant buckets — Matches sorted shallowest-first before pruning (af94db8).
Windows MCP skipping NTFS directory junctions — Symlink defense moved to dispatch-only; junctions pass S_ISDIR lstat, walk no longer over-skips them (ab24644).
MCP non-hidden marker silently skipped — markers_all_hidden routes non-hidden markers to direct walk (ab24644).
MCP shared skip_id across callers — Accepted: all current MCP callers share identical per-OS prune; distinct skip_id required only for a genuinely different prune (af94db8).
SKIP_SYSTEM_DIRS import-time prefix drift — Source frozen to frozenset with sync test (af94db8).
POSIX system-path tests breaking Windows CI — Test-only; classes gated with @unittest.skipUnless(os.name == "posix") (e588781).

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head ab24644c · 2026-08-12T23:45Z

…ules markers Two more from the security review: - Ancestor symlink swap (high): dispatch only lstat'd the leaf, so a user who replaced an indexed ancestor (e.g. ~/proj) with a symlink after indexing could make the cached leaf resolve outside the scanned tree — a privileged multi-user scan could then read another user's configs. Both the index dispatch and the direct-walk dispatch now realpath the target and require it to stay under realpath(root_path) before on_match runs. - Rules walkers: the macOS and Linux rules walkers always used the index path, which stores only hidden dirs, so a future non-hidden marker would silently discover nothing. They now pass markers_all_hidden the same way the MCP walker does, routing a non-hidden marker to the direct walk. New tests: an ancestor swapped to an escaping symlink is dropped at dispatch, and both rules walkers flag a non-hidden marker to the direct walk. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

AakashVelusamy · 2026-08-13T05:04:49Z

Both addressed in 61a0852.

🔴 HIGH — Ancestor symlink swap bypasses scan-root containment

Correct — the leaf lstat only re-checks the leaf, so a swapped ancestor could still resolve the cached path outside the scanned tree. Both dispatch sites (the index path and the direct-walk path) now realpath the target and require it to stay under realpath(root_path) before on_match runs:

After indexing, at dispatch	Resolves to	Result
Ancestor is a real dir	inside scan root	dispatched
Ancestor swapped to symlink → another user's tree	outside scan root	dropped
Leaf swapped to symlink	`S_IFLNK`	dropped (leaf check)

The direct walk was already contained in practice (it never descends a link/junction, so it can't walk into a swapped ancestor), but it now carries the same explicit check so both paths behave identically. Regression test swaps an indexed ancestor for a symlink pointing outside the root and asserts the leaf is dropped — note the leaf there is a real dir reached through the symlink, so only the containment check stops it.

🟡 TRIAGE — Rules walkers omit `markers_all_hidden` fallback

Fixed — walk_for_tool_directories in both macos_extraction_helpers.py and linux_extraction_helpers.py now passes markers_all_hidden=tool_dir_name.startswith("."), mirroring the MCP walker, so a future non-hidden marker routes to _walk_direct instead of silently matching nothing. A test drives both walkers with a hidden and a non-hidden marker and asserts the flag flips.

The prior findings remain resolved; full local suite green apart from the pre-existing environmental cases (this box has junie/Copilot binaries installed).

vigneshsubbiah16

🛡️ Automated Security Review (consensus)

2 findings — 0 high-confidence, 2 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.

🟡 TRIAGE — Containment check rejects all paths when scan root is filesystem root

scripts/coding_discovery_tools/project_dir_index.py:53
Impact: _within_scan_root uses real.startswith(root_real + os.sep); when root_path resolves to / (macOS project sweep from /, per PR description), the prefix becomes // and no descendant satisfies either branch — every dispatch is dropped with no error, causing silent total discovery loss on that path.
Fix: Normalize before compare (base = root_real.rstrip(os.sep) + os.sep, or Path.is_relative_to / os.path.commonpath); apply os.path.normcase on Windows; add a regression test with root_path="/".
Flagged by: Claude, Lead

🟡 TRIAGE — Uncaught `scandir` iterator errors abort per-tool discovery

scripts/coding_discovery_tools/project_dir_index.py:126
Impact: _walk_direct (and the direct-route branches in dispatch_matches) lack the outer except (PermissionError, OSError) the replaced walkers had around the full iteration loop, so a mid-walk iterator failure (removed dir, NFS/permission fault) can abort that tool's entire extraction instead of skipping the subtree.
Fix: Wrap the for entry in scan: loop in _walk_direct (and mirror in _collect if desired) with except (PermissionError, OSError): return/continue, matching the old walker resilience.
Flagged by: Claude

Previously acknowledged (not re-flagged)

Residual TOCTOU between dispatch validation and on_match config read — maintainer: intentional; narrowed to the same window as the prior direct walk, not fully closed.
NTFS directory junctions dispatched (not treated as symlinks) — maintainer: accepted by design to preserve byte-identical Windows behavior; symlinks still dropped at dispatch.
Deep/persistent permission errors remain cached in the index — maintainer: known/accepted risk; matches prior per-tool walk semantics; only unreadable roots are left uncached for retry.
Windows CI POSIX path test failures — maintainer: false positive; predicates are POSIX-only; tests guarded with @unittest.skipUnless(os.name == "posix").
Unbounded process-lifetime index cache — maintainer: fixed; index now stores hidden dirs only (~3% of entries).
Unsynchronized _INDEX_CACHE under parallel MCP walks — maintainer: fixed with threading.Lock + build-outside/publish pattern.
Indexed paths dispatched without re-validation (TOCTOU) / ancestor symlink escape — maintainer: fixed with dispatch-time lstat + realpath containment checks (61a0852).
Non-hidden tool markers silently skipped on index path — maintainer: fixed via markers_all_hidden routing to _walk_direct.
outermost_only cross-basename / dispatch-order pruning — maintainer: fixed with shallowest-first sort before prune.

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head 61a08529 · 2026-08-13T05:14Z

… iterator faults Two follow-ups from the security review: - Containment at "/": the scan-root check built its prefix as root_real + os.sep, so a root that resolves to "/" (the macOS sweep from filesystem root) became "//" and no descendant matched — silently dropping every dispatch on that path. Strip a trailing separator before re-appending one, and normcase both sides so Windows case/separator folding is handled too. - Iterator resilience: the old per-tool walkers wrapped the whole scandir loop, but the new walks only guarded per-entry. An OSError raised while advancing the iterator (a dir removed mid-walk, an NFS/permission fault) could abort that tool's extraction instead of skipping the subtree. Wrap the loop in _walk_direct and _collect so an iterator fault is logged and the walk moves on. New tests: containment holds for a "/" root and rejects a sibling tree, and a walk whose iterator faults mid-scan still dispatches what it saw without propagating. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

AakashVelusamy · 2026-08-13T05:48:39Z

Both addressed in 07374e5.

🟡 Containment check rejects all paths when scan root is filesystem root

Real bug — the macOS sweep runs from /, and root_real + os.sep made the prefix //, so _within_scan_root dropped every descendant with no error. Fixed by stripping a trailing separator before re-appending one, plus os.path.normcase on both sides for Windows case/separator folding:

Root resolves to	Target	Before	Now
`/`	`/Users/x/.cursor`	dropped (`//` prefix)	dispatched
`/home/alice`	`/home/alice/proj/.cursor`	dispatched	dispatched
`/home/alice`	`/nope/x` (escape)	dropped	dropped

Regression test asserts a /-root contains its descendants and still rejects a sibling tree.

🟡 Uncaught `scandir` iterator errors abort per-tool discovery

Correct — the per-entry try didn't cover an error raised while advancing the iterator (dir removed mid-walk, NFS/permission fault), which the old per-tool walkers guarded around the whole loop. Wrapped the for entry in scan loop in both _walk_direct and _collect with except (PermissionError, OSError), so a mid-walk fault is logged and the subtree is skipped rather than aborting the tool. Test drives a scan whose iterator raises after one entry and asserts the entry seen before the fault is still dispatched and nothing propagates.

Prior findings remain resolved; full local suite green apart from the pre-existing environmental cases (this box has junie/Copilot binaries installed).

vigneshsubbiah16

🛡️ Automated Security Review (consensus)

2 findings — 2 high-confidence, 0 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.

🔴 HIGH — Dispatch containment bound to `root_path`, not indexed subtree

scripts/coding_discovery_tools/project_dir_index.py:217 (also project_dir_index.py:155 in _walk_direct)

Impact: Re-validation uses realpath(root_path); when the macOS project sweep uses filesystem root (/), every resolved path passes containment, so an ancestor swapped to a symlink between index and dispatch can still route reads into another user's tree (leaf stays a real dir/junction).
Fix: Resolve containment against the subtree actually indexed — realpath(current_dir) in dispatch_matches, and the active recursion directory in _walk_direct — not the depth-cap root_path.
Flagged by: Claude, Lead

🔴 HIGH — Walk may descend NTFS directory junctions on Windows

scripts/coding_discovery_tools/project_dir_index.py:93 (also project_dir_index.py:168 in _walk_direct)

Impact: Recursion is gated only on DirEntry.is_symlink(); NTFS junctions often report as directories, not symlinks, so a user-planted junction can redirect a privileged walk into another user's tree before dispatch checks run.
Fix: Use the existing is_symlink_or_junction() helper (or equivalent reparse-tag check) for the no-descent guard in _collect and _walk_direct; keep junction allowlisting at dispatch via _is_dispatchable.
Flagged by: Cursor, Lead

Previously acknowledged (not re-flagged)

Post-dispatch TOCTOU window — Maintainer accepted the residual race where an ancestor can be re-swapped after realpath containment but before on_match reads config; treated as inherent and narrowed to the same window as the legacy direct walk.

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head 07374e56 · 2026-08-13T05:55Z

cursor

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 07374e5. Configure here.}

cursor · 2026-08-13T05:58:19Z

+        logger.warning("subtree root unreadable, not caching: %s", current_dir)
+        return index
+    with _INDEX_LOCK:
+        return _INDEX_CACHE.setdefault(key, index)


Partial index cached after mid-walk fault

Medium Severity

If scandir opens successfully but the iterator raises mid-walk, _collect still returns success and get_subtree_index memoizes the incomplete basename → dirs map. Later tools reuse that truncated tree instead of re-listing, so transient mid-walk faults permanently hide directories the old per-tool walks would rediscover.

Additional Locations (1)

scripts/coding_discovery_tools/project_dir_index.py#L122-L128

^{Reviewed by Cursor Bugbot for commit 07374e5. Configure here.}

The "/" filesystem-root sweep is a macOS/POSIX scenario, and the test hard-codes POSIX paths; Windows resolves "/" to a drive root, so the assertions don't hold there. Skip it on non-POSIX. The containment code itself is cross-platform (normcase folds Windows case/separators); only this scenario is POSIX-specific. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

vigneshsubbiah16

🛡️ Automated Security Review (consensus)

0 findings — 0 high-confidence, 0 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.

✅ Security consensus: no issues found. (reviewers: Cursor, Claude, Semgrep, Gitleaks)

Previously acknowledged (not re-flagged)

Unbounded process-lifetime index cache — bounded to hidden marker dirs only; maintainer verified identical dispatch with ~97% fewer cached paths (baf2fab).
Unsynchronized _INDEX_CACHE under parallel Windows MCP walks — guarded with threading.Lock; build stays outside the lock (af94db8).
TOCTOU: indexed dir swapped to symlink before dispatch — _is_dispatchable() re-lstats at dispatch; symlinks dropped, real dirs/junctions kept (af94db8).
TOCTOU: ancestor symlink swap escapes scan root — _within_scan_root() re-checks realpath(target) against realpath(root_path) before on_match (61a0852).
Windows NTFS junction vs symlink semantics — intentional dispatch-time guard: junctions pass S_ISDIR, symlinks are refused; walk records but does not follow symlinks (ab24644).
outermost_only mis-prune across case-variant buckets — shallowest-first sort before pruning (af94db8).
Non-hidden tool markers silently missed on index path — markers_all_hidden routes to _walk_direct in MCP and rules walkers (ab24644, 61a0852).
_within_scan_root broken for filesystem-root scans (/ → //) — trailing-separator normalization + normcase (07374e5).
Mid-walk scandir iterator faults abort discovery — iterator loop wrapped in OSError/PermissionError handler in _collect and _walk_direct (07374e5).
MCP skip_id cache keying / frozen SKIP_SYSTEM_DIRS / POSIX-only system-path tests — documented skip_id contract, frozenset source, @unittest.skipUnless(os.name == "posix") on platform-specific tests (af94db8, e588781).

🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head cd02f4bc · 2026-08-13T06:03Z

AakashVelusamy requested a review from a team August 3, 2026 21:37

greptile-apps Bot reviewed Aug 3, 2026

View reviewed changes

Comment thread tests/test_skip_path_guards.py

cursor Bot reviewed Aug 3, 2026

View reviewed changes

Comment thread tests/test_skip_path_guards.py

vigneshsubbiah16 reviewed Aug 3, 2026

View reviewed changes

AakashVelusamy changed the title ~~perf: speed up the discovery scan's hottest path guards~~ perf: make the discovery scan read the filesystem once, not once per tool Aug 4, 2026

cursor Bot reviewed Aug 4, 2026

View reviewed changes