Skip to content

fix(tools): prune _SKIP_DIRS below the glob base, not the whole path - #256

Merged
Zongwei9888 merged 1 commit into
HKUDS:mainfrom
D41910:fix/glob-skip-dirs-relative-to-base
Oct 12, 2026
Merged

Zongwei9888 merged 1 commit into
HKUDS:mainfrom
D41910:fix/glob-skip-dirs-relative-to-base

Conversation

@D41910

@D41910 D41910 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Summary

glob silently reports "No files matched." for a workspace whose path contains a
directory component named build, dist, venv, .venv, node_modules,
__pycache__ or .git — even though the project has matching files. grep, which
shares the same _SKIP_DIRS constant in the same module, returns results for the very
same workspace.

Root cause

GlobTool.execute prunes ignored directories against p.parts of an absolute path:

# core/harness/tools/search.py:196-198 (before)
for p in sorted(base.glob(pattern)):
    if any(part in _SKIP_DIRS for part in p.parts):
        continue

base.glob() yields absolute paths, so p.parts also carries every component above
the search base — e.g. ('/', 'home', 'me', 'build', 'app', 'src', 'm.py'). When any of
those ancestors collides with _SKIP_DIRS, every file matches and the loop skips all of
them. The prune is meant to skip noise directories inside the search tree, not the
workspace's own ancestors.

The sibling in the same module gets this right. GrepTool._walk prunes during the walk,
so only directories below root are ever considered:

# core/harness/tools/search.py:133-139
def _walk(self, root: Path, include: str | None):
    for dirpath, dirnames, filenames in os.walk(root):
        dirnames[:] = [d for d in dirnames if d not in _SKIP_DIRS]

root is never tested against _SKIP_DIRS. That is why the two siblings disagree.

Reproduction (real code, workspace named build)

ws=build          glob -> 'No files matched.'
                  grep -> '.../build/top.py:1: x'
ws=dist           glob -> 'No files matched.'
ws=venv           glob -> 'No files matched.'
ws=node_modules   glob -> 'No files matched.'
ws=normalproj     glob -> 'src/m.py\ntop.py'

Why it matters

The failure is silent, not an error. The agent asks "which Python files exist?", gets
"No files matched.", and concludes the project is empty — a wrong premise that then
drives planning, edits and review. Nothing in the output hints that a directory-name
collision caused it.

Fix

Prune relative to the search base instead of the whole absolute path:

         matches = []
         for p in sorted(base.glob(pattern)):
-            if any(part in _SKIP_DIRS for part in p.parts):
+            # Prune relative to the base only. ``p.parts`` also carries the
+            # workspace's own ancestors, so a workspace rooted under a
+            # directory named e.g. "build" or "venv" skipped every file.
+            rel = os.path.relpath(p, base)
+            if any(part in _SKIP_DIRS for part in Path(rel).parts):
                 continue
             if p.is_file():
                 matches.append(os.path.relpath(p, self._workspace))

Two changed lines; the output path (workspace-relative) is untouched.

Verification

Test effectiveness. Reverting only the source change while keeping the new test:

FAILED tests/test_shell_search_tools.py::test_glob_prunes_skip_dirs_below_the_base_only[build]
FAILED ...[dist]  FAILED ...[venv]  FAILED ...[node_modules]
E  AssertionError: assert ('top.py' in 'No files matched.')
4 failed, 24 passed

Restoring the fix: 28 passed.

Differential fuzzing, 500 randomized workspace trees (ancestor chains mixing
_SKIP_DIRS names with ordinary names, nested ignored dirs, 8 patterns, path
argument sometimes set):

  • 431 identical, 69 changed — 69 expected, 0 unexpected
  • 129 files recovered across the changed cases
  • 0 cases where a genuinely ignored directory leaked into the output

Every changed case has an ignored-name ancestor and a strict superset of the previous
result, i.e. the fix only adds files that were wrongly suppressed; it never removed one.

Existing tests. Related subset (search / file tools / harness / spawn_agent /
code_mode / apply_patch) on unmodified main: 162 passed, 3 skipped. With the fix:
166 passed, 3 skipped — exactly +4, the new parametrised cases.

Full suite: 2266 passed, 11 skipped in 2m25s, no failures.

Scope / no data impact

The change only affects which paths are included in glob output. It touches no
persisted key, file path, cache key or serialization format, so there is no existing-data
migration concern.

Not addressed here

While reproducing this I noticed a separate, pre-existing issue I deliberately left out
of this PR: os.path.relpath(p, self._workspace) relativises against the unresolved
workspace string while base is .resolve()d, so when the workspace path contains a
symlink (macOS /tmp → /private/tmp, /var → /private/var) both tools emit
../../../../private/var/... rather than a workspace-relative path, despite
GlobTool.description promising "workspace-relative". It is independent of the bug
fixed here and changing it alters existing output formatting, so it seems better as its
own PR — happy to open one if you agree.


Assisted by AI (CodeBuddy/Claude). I verified every claim above by running the real
GlobTool/GrepTool code, not a reimplementation. Happy to convert this to an issue
first, or to drop the comment lines if you prefer the change to be purely mechanical.

GlobTool skipped every file when a component of the workspace path
collided with _SKIP_DIRS (build/dist/venv/node_modules/...), because
p.parts carries the workspace's own ancestors and not just the search
tree. It silently returned "No files matched." for a project that has
files, while grep -- sharing the same constant -- returned results.

Prune relative to the search base instead, matching GrepTool._walk.
@Zongwei9888
Zongwei9888 merged commit 9550d53 into HKUDS:main Oct 12, 2026
@Zongwei9888

Copy link
Copy Markdown
Collaborator

Merged in 9550d53. Thank you for the fix and the side-by-side comparison with GrepTool._walk, @D41910. It made this quick to verify.

Checked before merging:

  • Reproduced: with the workspace at .../build/app, glob with **/*.py returned No files matched. on main, while grep found the same files. After the merge it lists them.
  • os.path.relpath(p, base) is safe here. base is resolved, and every path from base.glob() is lexically under it, so the relative path never escapes and never crosses drives. The workspace-relative output is unchanged.
  • The new test fails on main for all four names and still covers pruning of a node_modules inside the workspace.

One consequence, which I think is the intended behavior: an explicit path pointing into an ignored directory (e.g. path="node_modules") now lists its files instead of returning nothing, which is what grep already does.

On the symlink issue you mentioned, I can reproduce it too: a workspace passed as /tmp/... on macOS yields ../../../../private/tmp/.... A separate PR for it would be welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants