mirror of
https://github.com/open-jarvis/OpenJarvis.git
synced 2026-07-27 21:05:34 +00:00
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 <elliot@slusky.com>
This commit is contained in:
co-authored by
Elliot Slusky
parent
99bbc2054a
commit
4419b76412
@@ -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",
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user