From f0ab045cad8f48e40dcbf5998edb5cf3f46490eb Mon Sep 17 00:00:00 2001 From: Taco Engineer <268588431+TacoengineerIT@users.noreply.github.com> Date: Thu, 7 May 2026 09:41:58 +0200 Subject: [PATCH] fix(tools): correct Python-fallback bugs in calculator, file tools, and git tool (#236) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - calculator: add `ln` alias for `math.log`, replace `^` with `**` before AST parsing so caret-as-power works, convert SyntaxError → ValueError for consistent error handling, and return `math.inf` on division by zero (matching the documented meval behaviour) instead of raising an exception - file_read / file_write: replace the hardcoded `str(path).startswith(str(d) + "/") guard with `Path.is_relative_to()` so allowed-directory checks work on Windows (and any OS whose separator is not `/`) - git_tool: catch `NotADirectoryError` raised by `subprocess.run` on Windows when `cwd` points to a non-existent path, returning a proper error result Co-authored-by: Claude Sonnet 4.6 Co-authored-by: Robby Manihani --- src/openjarvis/tools/calculator.py | 15 ++++++++++++--- src/openjarvis/tools/file_read.py | 3 +-- src/openjarvis/tools/file_write.py | 3 +-- src/openjarvis/tools/git_tool.py | 6 ++++++ 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/openjarvis/tools/calculator.py b/src/openjarvis/tools/calculator.py index a3e02803..47ce773f 100644 --- a/src/openjarvis/tools/calculator.py +++ b/src/openjarvis/tools/calculator.py @@ -36,6 +36,7 @@ _MATH_FUNCS = { "max": max, "sqrt": math.sqrt, "log": math.log, + "ln": math.log, # alias: ln(x) == log(x) "log10": math.log10, "log2": math.log2, "sin": math.sin, @@ -84,7 +85,7 @@ def _safe_eval_node(node: ast.AST) -> Any: val = _MATH_FUNCS[name] if isinstance(val, (int, float)): return val - raise ValueError(f"Unknown variable: {name}") + raise ValueError(f"unknown variable: {name}") raise ValueError(f"Unsupported expression type: {type(node).__name__}") @@ -98,8 +99,16 @@ def safe_eval(expression: str) -> float: except ImportError: import ast as _ast - tree = _ast.parse(expression, mode="eval") - return float(_safe_eval_node(tree.body)) + # Support ^ as the power operator (common math/calculator notation). + expression = expression.replace("^", "**") + try: + tree = _ast.parse(expression, mode="eval") + except SyntaxError as exc: + raise ValueError(f"Syntax error in expression: {exc}") from exc + try: + return float(_safe_eval_node(tree.body)) + except ZeroDivisionError: + return math.inf @ToolRegistry.register("calculator") diff --git a/src/openjarvis/tools/file_read.py b/src/openjarvis/tools/file_read.py index c96195d6..59f6315a 100644 --- a/src/openjarvis/tools/file_read.py +++ b/src/openjarvis/tools/file_read.py @@ -53,8 +53,7 @@ class FileReadTool(BaseTool): return True resolved = path.resolve() return any( - resolved == d or str(resolved).startswith(str(d) + "/") - for d in self._allowed_dirs + resolved == d or resolved.is_relative_to(d) for d in self._allowed_dirs ) def execute(self, **params: Any) -> ToolResult: diff --git a/src/openjarvis/tools/file_write.py b/src/openjarvis/tools/file_write.py index dfd789fc..7369c344 100644 --- a/src/openjarvis/tools/file_write.py +++ b/src/openjarvis/tools/file_write.py @@ -69,8 +69,7 @@ class FileWriteTool(BaseTool): return True resolved = path.resolve() return any( - resolved == d or str(resolved).startswith(str(d) + "/") - for d in self._allowed_dirs + resolved == d or resolved.is_relative_to(d) for d in self._allowed_dirs ) def execute(self, **params: Any) -> ToolResult: diff --git a/src/openjarvis/tools/git_tool.py b/src/openjarvis/tools/git_tool.py index b660f974..b838ae78 100644 --- a/src/openjarvis/tools/git_tool.py +++ b/src/openjarvis/tools/git_tool.py @@ -76,6 +76,12 @@ def _run_git( content="git binary not found.", success=False, ) + except NotADirectoryError as exc: + return ToolResult( + tool_name=tool_name, + content=f"Invalid repository path: {exc}", + success=False, + ) except subprocess.TimeoutExpired: return ToolResult( tool_name=tool_name,