From 4419b764120e617484f2502eae60574d1f14b1b5 Mon Sep 17 00:00:00 2001 From: Curryraj Date: Fri, 17 Jul 2026 08:59:25 +0800 Subject: [PATCH] fix: catch ImportError in git tools, fall back to CLI when Rust ext missing (#636) get_rust_module() was called outside the try block in GitStatusTool/GitDiffTool/GitLogTool.execute(), so on installs without the compiled openjarvis-rust extension (e.g. plain pip installs, where openjarvis-rust is a uv-only group since #624) the ImportError escaped uncaught instead of degrading. Move the call inside try and fall back to the git CLI via the existing _run_git helper on ImportError, matching the fallback git_log already had. Adds regression tests covering the fallback path for all three tools. Co-authored-by: Elliot Slusky --- src/openjarvis/tools/git_tool.py | 12 ++++++-- tests/tools/test_git_tool.py | 48 ++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/src/openjarvis/tools/git_tool.py b/src/openjarvis/tools/git_tool.py index b838ae78..65995b8d 100644 --- a/src/openjarvis/tools/git_tool.py +++ b/src/openjarvis/tools/git_tool.py @@ -139,8 +139,8 @@ class GitStatusTool(BaseTool): def execute(self, **params: Any) -> ToolResult: repo_path = params.get("repo_path", ".") - _rust = get_rust_module() try: + _rust = get_rust_module() output = _rust.GitStatusTool().execute(repo_path) return ToolResult( tool_name="git_status", @@ -148,6 +148,8 @@ class GitStatusTool(BaseTool): success=True, metadata={"returncode": 0}, ) + except ImportError as exc: + logger.debug("Rust git_status fallback to CLI: %s", exc) except Exception as exc: return ToolResult( tool_name="git_status", @@ -155,6 +157,8 @@ class GitStatusTool(BaseTool): success=False, ) + return _run_git(["git", "status", "--porcelain"], cwd=repo_path) + # --------------------------------------------------------------------------- # GitDiffTool @@ -208,9 +212,9 @@ class GitDiffTool(BaseTool): staged = params.get("staged", False) file_path = params.get("path") - _rust = get_rust_module() if not staged and not file_path: try: + _rust = get_rust_module() output = _rust.GitDiffTool().execute(repo_path) return ToolResult( tool_name="git_diff", @@ -218,6 +222,8 @@ class GitDiffTool(BaseTool): success=True, metadata={"returncode": 0}, ) + except ImportError as exc: + logger.debug("Rust git_diff fallback to CLI: %s", exc) except Exception as exc: return ToolResult( tool_name="git_diff", @@ -371,8 +377,8 @@ class GitLogTool(BaseTool): count = params.get("count", 10) oneline = params.get("oneline", True) - _rust = get_rust_module() try: + _rust = get_rust_module() output = _rust.GitLogTool().execute(repo_path, count) return ToolResult( tool_name="git_log", diff --git a/tests/tools/test_git_tool.py b/tests/tools/test_git_tool.py index e3cc5b80..46897615 100644 --- a/tests/tools/test_git_tool.py +++ b/tests/tools/test_git_tool.py @@ -537,3 +537,51 @@ class TestGitLogTool: fn = tool.to_openai_function() assert fn["type"] == "function" assert fn["function"]["name"] == "git_log" + + +# --------------------------------------------------------------------------- +# CLI fallback when the Rust extension is missing +# --------------------------------------------------------------------------- + + +class TestCliFallbackWhenRustMissing: + """When ``get_rust_module`` raises ImportError (extension not built, + e.g. a plain pip install), the read-only git tools must fall back to + the git CLI instead of letting the ImportError escape ``execute()``.""" + + def _patch_no_rust(self): + return patch( + "openjarvis.tools.git_tool.get_rust_module", + side_effect=ImportError("No module named 'openjarvis_rust'"), + ) + + def test_git_status_falls_back_to_cli(self, tmp_path): + _init_repo(tmp_path) + (tmp_path / "new_file.txt").write_text("hello") + with self._patch_no_rust(): + result = GitStatusTool().execute(repo_path=str(tmp_path)) + assert result.success is True + assert "new_file.txt" in result.content + + def test_git_diff_falls_back_to_cli(self, tmp_path): + _init_repo(tmp_path) + (tmp_path / "README.md").write_text("# Modified\n") + with self._patch_no_rust(): + result = GitDiffTool().execute(repo_path=str(tmp_path)) + assert result.success is True + assert "README.md" in result.content + + def test_git_log_falls_back_to_cli(self, tmp_path): + _init_repo(tmp_path) + with self._patch_no_rust(): + result = GitLogTool().execute(repo_path=str(tmp_path)) + assert result.success is True + assert "Initial commit" in result.content + + def test_fallback_failure_is_a_tool_result_not_an_exception(self, tmp_path): + # Even when the fallback itself fails (not a git repo), the tool + # must return a failed ToolResult rather than raising. + with self._patch_no_rust(): + result = GitStatusTool().execute(repo_path=str(tmp_path)) + assert result.success is False + assert "not a git repository" in result.content