From fe90edd71f74db0b70a3e64b921d3565158a3ebf Mon Sep 17 00:00:00 2001 From: chengyongru Date: Fri, 15 May 2026 15:54:36 +0800 Subject: [PATCH] refactor(tools): remove GlobTool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GlobTool is redundant — GrepTool already supports glob-based file filtering via its `glob` parameter, making a standalone glob-only tool unnecessary. Removing it simplifies the tool surface and reduces LLM confusion between glob and grep. --- nanobot/agent/runner.py | 2 +- nanobot/agent/tools/search.py | 142 +------------------------------ nanobot/utils/tool_hints.py | 1 - tests/agent/test_subagent.py | 1 - tests/agent/test_tool_hint.py | 4 - tests/test_nanobot_facade.py | 4 +- tests/tools/test_search_tools.py | 73 +--------------- tests/tools/test_tool_loader.py | 2 +- 8 files changed, 9 insertions(+), 220 deletions(-) diff --git a/nanobot/agent/runner.py b/nanobot/agent/runner.py index 37da63872..64709afe2 100644 --- a/nanobot/agent/runner.py +++ b/nanobot/agent/runner.py @@ -47,7 +47,7 @@ _SNIP_SAFETY_BUFFER = 1024 _MICROCOMPACT_KEEP_RECENT = 10 _MICROCOMPACT_MIN_CHARS = 500 _COMPACTABLE_TOOLS = frozenset({ - "read_file", "exec", "grep", "glob", + "read_file", "exec", "grep", "web_search", "web_fetch", "list_dir", }) _BACKFILL_CONTENT = "[Tool result unavailable — call was interrupted or lost]" diff --git a/nanobot/agent/tools/search.py b/nanobot/agent/tools/search.py index fb04a4456..b495a451f 100644 --- a/nanobot/agent/tools/search.py +++ b/nanobot/agent/tools/search.py @@ -1,4 +1,4 @@ -"""Search tools: grep and glob.""" +"""Search tools: grep.""" from __future__ import annotations @@ -108,146 +108,6 @@ class _SearchTool(_FsTool): for filename in sorted(filenames): yield current / filename - def _iter_entries( - self, - root: Path, - *, - include_files: bool, - include_dirs: bool, - ) -> Iterable[Path]: - if root.is_file(): - if include_files: - yield root - return - - for dirpath, dirnames, filenames in os.walk(root): - dirnames[:] = sorted(d for d in dirnames if d not in self._IGNORE_DIRS) - current = Path(dirpath) - if include_dirs: - for dirname in dirnames: - yield current / dirname - if include_files: - for filename in sorted(filenames): - yield current / filename - - -class GlobTool(_SearchTool): - """Find files matching a glob pattern.""" - _scopes = {"core", "subagent"} - - @property - def name(self) -> str: - return "glob" - - @property - def description(self) -> str: - return ( - "Find files matching a glob pattern (e.g. '*.py', 'tests/**/test_*.py'). " - "Results are sorted by modification time (newest first). " - "Skips .git, node_modules, __pycache__, and other noise directories." - ) - - @property - def read_only(self) -> bool: - return True - - @property - def parameters(self) -> dict[str, Any]: - return { - "type": "object", - "properties": { - "pattern": { - "type": "string", - "description": "Glob pattern to match, e.g. '*.py' or 'tests/**/test_*.py'", - "minLength": 1, - }, - "path": { - "type": "string", - "description": "Directory to search from (default '.')", - }, - "max_results": { - "type": "integer", - "description": "Legacy alias for head_limit", - "minimum": 1, - "maximum": 1000, - }, - "head_limit": { - "type": "integer", - "description": "Maximum number of matches to return (default 250)", - "minimum": 0, - "maximum": 1000, - }, - "offset": { - "type": "integer", - "description": "Skip the first N matching entries before returning results", - "minimum": 0, - "maximum": 100000, - }, - "entry_type": { - "type": "string", - "enum": ["files", "dirs", "both"], - "description": "Whether to match files, directories, or both (default files)", - }, - }, - "required": ["pattern"], - } - - async def execute( - self, - pattern: str, - path: str = ".", - max_results: int | None = None, - head_limit: int | None = None, - offset: int = 0, - entry_type: str = "files", - **kwargs: Any, - ) -> str: - try: - root = self._resolve(path or ".") - if not root.exists(): - return f"Error: Path not found: {path}" - if not root.is_dir(): - return f"Error: Not a directory: {path}" - - if head_limit is not None: - limit = None if head_limit == 0 else head_limit - elif max_results is not None: - limit = max_results - else: - limit = _DEFAULT_HEAD_LIMIT - include_files = entry_type in {"files", "both"} - include_dirs = entry_type in {"dirs", "both"} - matches: list[tuple[str, float]] = [] - for entry in self._iter_entries( - root, - include_files=include_files, - include_dirs=include_dirs, - ): - rel_path = entry.relative_to(root).as_posix() - if _match_glob(rel_path, entry.name, pattern): - display = self._display_path(entry, root) - if entry.is_dir(): - display += "/" - try: - mtime = entry.stat().st_mtime - except OSError: - mtime = 0.0 - matches.append((display, mtime)) - - if not matches: - return f"No paths matched pattern '{pattern}' in {path}" - - matches.sort(key=lambda item: (-item[1], item[0])) - ordered = [name for name, _ in matches] - paged, truncated = _paginate(ordered, limit, offset) - result = "\n".join(paged) - if note := _pagination_note(limit, offset, truncated): - result += f"\n\n{note}" - return result - except PermissionError as e: - return f"Error: {e}" - except Exception as e: - return f"Error finding files: {e}" class GrepTool(_SearchTool): diff --git a/nanobot/utils/tool_hints.py b/nanobot/utils/tool_hints.py index 289870665..272a19c9a 100644 --- a/nanobot/utils/tool_hints.py +++ b/nanobot/utils/tool_hints.py @@ -11,7 +11,6 @@ _TOOL_FORMATS: dict[str, tuple[list[str], str, bool, bool]] = { "read_file": (["path", "file_path"], "read {}", True, False), "write_file": (["path", "file_path"], "write {}", True, False), "edit": (["file_path", "path"], "edit {}", True, False), - "glob": (["pattern"], 'glob "{}"', False, False), "grep": (["pattern"], 'grep "{}"', False, False), "exec": (["command"], "$ {}", False, True), "web_search": (["query"], 'search "{}"', False, False), diff --git a/tests/agent/test_subagent.py b/tests/agent/test_subagent.py index ef6940a7c..5bdfc18dd 100644 --- a/tests/agent/test_subagent.py +++ b/tests/agent/test_subagent.py @@ -25,7 +25,6 @@ async def test_subagent_uses_tool_loader(): tools = sm._build_tools() assert tools.has("read_file") assert tools.has("write_file") - assert tools.has("glob") assert not tools.has("message") assert not tools.has("spawn") diff --git a/tests/agent/test_tool_hint.py b/tests/agent/test_tool_hint.py index 174eb208d..6e3bdb03b 100644 --- a/tests/agent/test_tool_hint.py +++ b/tests/agent/test_tool_hint.py @@ -34,10 +34,6 @@ class TestToolHintKnownTools: assert "main.py" in result assert "edit " in result - def test_glob_shows_pattern(self): - result = _hint([_tc("glob", {"pattern": "**/*.py", "path": "src"})]) - assert result == 'glob "**/*.py"' - def test_grep_shows_pattern(self): result = _hint([_tc("grep", {"pattern": "TODO|FIXME", "path": "src"})]) assert result == 'grep "TODO|FIXME"' diff --git a/tests/test_nanobot_facade.py b/tests/test_nanobot_facade.py index 2dfde6c7c..c2ef35f9f 100644 --- a/tests/test_nanobot_facade.py +++ b/tests/test_nanobot_facade.py @@ -190,7 +190,7 @@ async def test_run_populates_tools_used_across_iterations(tmp_path): ctx1 = AgentHookContext(iteration=0, messages=messages) ctx1.tool_calls = [ ToolCallRequest(id="c1", name="read_file", arguments={}), - ToolCallRequest(id="c2", name="glob", arguments={}), + ToolCallRequest(id="c2", name="grep", arguments={}), ] for h in extras: await h.after_iteration(ctx1) @@ -204,7 +204,7 @@ async def test_run_populates_tools_used_across_iterations(tmp_path): bot._loop.process_direct = fake_process_direct result = await bot.run("do stuff") assert result.content == "final" - assert result.tools_used == ["read_file", "glob", "web_fetch"] + assert result.tools_used == ["read_file", "grep", "web_fetch"] @pytest.mark.asyncio diff --git a/tests/tools/test_search_tools.py b/tests/tools/test_search_tools.py index 4230e236d..0d3697044 100644 --- a/tests/tools/test_search_tools.py +++ b/tests/tools/test_search_tools.py @@ -1,4 +1,4 @@ -"""Tests for grep/glob search tools.""" +"""Tests for grep search tools.""" from __future__ import annotations @@ -12,7 +12,7 @@ import pytest from nanobot.agent.loop import AgentLoop from nanobot.agent.subagent import SubagentManager, SubagentStatus -from nanobot.agent.tools.search import GlobTool, GrepTool +from nanobot.agent.tools.search import GrepTool from nanobot.agent.tools.web import WebSearchTool from nanobot.bus.queue import MessageBus from nanobot.config.schema import WebSearchConfig @@ -33,39 +33,6 @@ async def test_web_search_tool_refreshes_dynamic_config_loader(monkeypatch) -> N assert await tool.execute("nanobot") == "duckduckgo:nanobot:3" -@pytest.mark.asyncio -async def test_glob_matches_recursively_and_skips_noise_dirs(tmp_path: Path) -> None: - (tmp_path / "src").mkdir() - (tmp_path / "nested").mkdir() - (tmp_path / "node_modules").mkdir() - (tmp_path / "src" / "app.py").write_text("print('ok')\n", encoding="utf-8") - (tmp_path / "nested" / "util.py").write_text("print('ok')\n", encoding="utf-8") - (tmp_path / "node_modules" / "skip.py").write_text("print('skip')\n", encoding="utf-8") - - tool = GlobTool(workspace=tmp_path, allowed_dir=tmp_path) - result = await tool.execute(pattern="*.py", path=".") - - assert "src/app.py" in result - assert "nested/util.py" in result - assert "node_modules/skip.py" not in result - - -@pytest.mark.asyncio -async def test_glob_can_return_directories_only(tmp_path: Path) -> None: - (tmp_path / "src").mkdir() - (tmp_path / "src" / "api").mkdir(parents=True) - (tmp_path / "src" / "api" / "handlers.py").write_text("ok\n", encoding="utf-8") - - tool = GlobTool(workspace=tmp_path, allowed_dir=tmp_path) - result = await tool.execute( - pattern="api", - path="src", - entry_type="dirs", - ) - - assert result.splitlines() == ["src/api/"] - - @pytest.mark.asyncio async def test_grep_respects_glob_filter_and_context(tmp_path: Path) -> None: (tmp_path / "src").mkdir() @@ -246,33 +213,6 @@ async def test_grep_files_with_matches_mode_respects_max_results(tmp_path: Path) assert "pagination: limit=2, offset=0" in result -@pytest.mark.asyncio -async def test_glob_supports_head_limit_offset_and_recent_first(tmp_path: Path) -> None: - (tmp_path / "src").mkdir() - a = tmp_path / "src" / "a.py" - b = tmp_path / "src" / "b.py" - c = tmp_path / "src" / "c.py" - a.write_text("a\n", encoding="utf-8") - b.write_text("b\n", encoding="utf-8") - c.write_text("c\n", encoding="utf-8") - - os.utime(a, (1, 1)) - os.utime(b, (2, 2)) - os.utime(c, (3, 3)) - - tool = GlobTool(workspace=tmp_path, allowed_dir=tmp_path) - result = await tool.execute( - pattern="*.py", - path="src", - head_limit=1, - offset=1, - ) - - lines = result.splitlines() - assert lines[0] == "src/b.py" - assert "pagination: limit=1, offset=1" in result - - @pytest.mark.asyncio async def test_grep_reports_skipped_binary_and_large_files( tmp_path: Path, @@ -296,16 +236,13 @@ async def test_search_tools_reject_paths_outside_workspace(tmp_path: Path) -> No outside.write_text("secret\n", encoding="utf-8") grep_tool = GrepTool(workspace=tmp_path, allowed_dir=tmp_path) - glob_tool = GlobTool(workspace=tmp_path, allowed_dir=tmp_path) grep_result = await grep_tool.execute(pattern="secret", path=str(outside)) - glob_result = await glob_tool.execute(pattern="*.txt", path=str(outside.parent)) assert grep_result.startswith("Error:") - assert glob_result.startswith("Error:") -def test_agent_loop_registers_grep_and_glob(tmp_path: Path) -> None: +def test_agent_loop_registers_grep(tmp_path: Path) -> None: bus = MessageBus() provider = MagicMock() provider.get_default_model.return_value = "test-model" @@ -313,11 +250,10 @@ def test_agent_loop_registers_grep_and_glob(tmp_path: Path) -> None: loop = AgentLoop(bus=bus, provider=provider, workspace=tmp_path, model="test-model") assert "grep" in loop.tools.tool_names - assert "glob" in loop.tools.tool_names @pytest.mark.asyncio -async def test_subagent_registers_grep_and_glob(tmp_path: Path) -> None: +async def test_subagent_registers_grep(tmp_path: Path) -> None: bus = MessageBus() provider = MagicMock() provider.get_default_model.return_value = "test-model" @@ -345,7 +281,6 @@ async def test_subagent_registers_grep_and_glob(tmp_path: Path) -> None: await mgr._run_subagent("sub-1", "search task", "label", {"channel": "cli", "chat_id": "direct"}, status) assert "grep" in captured["tool_names"] - assert "glob" in captured["tool_names"] def test_subagent_prompt_respects_disabled_skills(tmp_path: Path) -> None: diff --git a/tests/tools/test_tool_loader.py b/tests/tools/test_tool_loader.py index fa33b140b..54b4d92d5 100644 --- a/tests/tools/test_tool_loader.py +++ b/tests/tools/test_tool_loader.py @@ -406,7 +406,7 @@ def test_loader_registers_same_tools_as_old_hardcoded(): expected = { "read_file", "write_file", "edit_file", "list_dir", - "glob", "grep", "notebook_edit", "exec", "web_search", "web_fetch", + "grep", "notebook_edit", "exec", "web_search", "web_fetch", "message", "spawn", "cron", } actual = set(registered)