patch create tool: final changes:

- add a simple and a complex test case
- convert output to StringToolOutput, much simpler
- wrap everything in try/except, otherwise errors are not displayed

Signed-off-by: Tomas Tomecek <ttomecek@redhat.com>

Author: TomasTomecek

beeai/agents/tests/unit/test_tools.py

@@ -1,4 +1,5 @@
 import datetime
+import subprocess
 from textwrap import dedent
 
 import pytest
@@ -7,6 +8,7 @@
 
 from beeai_framework.middleware.trajectory import GlobalTrajectoryMiddleware
 
+from tools.wicked_git import GitPatchCreationTool, GitPatchCreationToolInput
 from tools.commands import RunShellCommandTool, RunShellCommandToolInput
 from tools.specfile import (
     AddChangelogEntryTool,
@@ -279,3 +281,70 @@ async def test_str_replace(tmp_path):
             """
         )[1:]
     )
+
+@pytest.mark.asyncio
+async def test_git_patch_creation_tool_nonexistent_repo(tmp_path):
+    # This test checks the error message for a non-existent repo path
+    repo_path = tmp_path / "not_a_repo"
+    patch_file_path = tmp_path / "patch.patch"
+    tool = GitPatchCreationTool()
+    output = await tool.run(
+        input=GitPatchCreationToolInput(
+            repository_path=str(repo_path),
+            patch_file_path=str(patch_file_path),
+        )
+    ).middleware(GlobalTrajectoryMiddleware(pretty=True))
+    result = output.result
+    assert "ERROR: Repository path does not exist" in result
+
+@pytest.fixture
+def git_repo(tmp_path):
+    repo_path = tmp_path / "repo"
+    repo_path.mkdir()
+    subprocess.run(["git", "init"], cwd=repo_path, check=True)
+    # Create a file and commit it
+    file_path = repo_path / "file.txt"
+    file_path.write_text("Line 1\n")
+    subprocess.run(["git", "add", "file.txt"], cwd=repo_path, check=True)
+    subprocess.run(["git", "commit", "-m", "Initial commit"], cwd=repo_path, check=True)
+    file_path.write_text("Line1\nLine 2\n")
+    subprocess.run(["git", "add", "file.txt"], cwd=repo_path, check=True)
+    subprocess.run(["git", "commit", "-m", "Initial commit2"], cwd=repo_path, check=True)
+    subprocess.run(["git", "branch", "line-2"], cwd=repo_path, check=True)
+    return repo_path
+
+@pytest.mark.asyncio
+async def test_git_patch_creation_tool_success(git_repo, tmp_path):
+    # Simulate a git-am session by creating a new commit and then using format-patch
+    # Create a new file and stage it
+    subprocess.run(["git", "reset", "--hard", "HEAD~1"], cwd=git_repo, check=True)
+    new_file = git_repo / "file.txt"
+    new_file.write_text("Line 1\nLine 3\n")
+    subprocess.run(["git", "add", "file.txt"], cwd=git_repo, check=True)
+    subprocess.run(["git", "commit", "-m", "Add line 3"], cwd=git_repo, check=True)
+
+    patch_file = tmp_path / "patch.patch"
+    subprocess.run(["git", "format-patch", "-1", "HEAD", "--stdout"], cwd=git_repo, check=True, stdout=patch_file.open("w"))
+
+    subprocess.run(["git", "switch", "line-2"], cwd=git_repo, check=True)
+
+    # Now apply the patch with git am
+    # This will fail with a merge conflict, but we don't care about that
+    subprocess.run(["git", "am", str(patch_file)], cwd=git_repo)
+
+    new_file.write_text("Line 1\nLine 2\nLine 3\n")
+
+    # Now use the tool to create a patch file from the repo
+    tool = GitPatchCreationTool()
+    output_patch = tmp_path / "output.patch"
+    output = await tool.run(
+        input=GitPatchCreationToolInput(
+            repository_path=str(git_repo),
+            patch_file_path=str(output_patch),
+        )
+    ).middleware(GlobalTrajectoryMiddleware(pretty=True))
+    result = output.result
+    assert "Successfully created a patch file" in result
+    assert output_patch.exists()
+    # The patch file should contain the commit message "Add line 3"
+    assert "Add line 3" in output_patch.read_text()

beeai/agents/tools/wicked_git.py

@@ -5,22 +5,12 @@
 
 from beeai_framework.context import RunContext
 from beeai_framework.emitter import Emitter
-from beeai_framework.tools import JSONToolOutput, Tool, ToolRunOptions
+from beeai_framework.tools import StringToolOutput, Tool, ToolRunOptions
 
 
 class GitPatchCreationToolInput(BaseModel):
-    repository_path: Path = Field(description="Absolute path to the git repository")
-    patch_file_path: Path = Field(description="Absolute path where the patch file should be saved")
-
-
-class GitPatchCreationToolResult(BaseModel):
-    success: bool = Field(description="Whether the patch creation was successful")
-    patch_file_path: str = Field(description="Path to the created patch file")
-    error: str | None = Field(description="Error message if patch creation failed", default=None)
-
-
-class GitPatchCreationToolOutput(JSONToolOutput[GitPatchCreationToolResult]):
-    """ Returns a dictionary with success or error and the path to the created patch file. """
+    repository_path: str = Field(description="Absolute path to the git repository")
+    patch_file_path: str = Field(description="Absolute path where the patch file should be saved")
 
 
 async def run_command(cmd: list[str], cwd: Path) -> dict[str, str | int]:
@@ -40,13 +30,12 @@ async def run_command(cmd: list[str], cwd: Path) -> dict[str, str | int]:
         "stderr": stderr.decode() if stderr else None,
     }
 
-class GitPatchCreationTool(Tool[GitPatchCreationToolInput, ToolRunOptions, GitPatchCreationToolOutput]):
+class GitPatchCreationTool(Tool[GitPatchCreationToolInput, ToolRunOptions, StringToolOutput]):
     name = "git_patch_create"
     description = """
-    Creates a patch file from the specified git repository with an active git-am session
-    and after you resolved all merge conflicts. The tool generates a patch file that can be
-    applied later in the RPM build process. Returns a dictionary with success or error and
-    the path to the created patch file.
+    Creates a patch file from the specified git repository with an active git-am session.
+    The tool expects you resolved all conflicts. It generates a patch file that can be
+    applied later in the RPM build process.
     """
     input_schema = GitPatchCreationToolInput
 
@@ -58,117 +47,64 @@ def _create_emitter(self) -> Emitter:
 
     async def _run(
         self, tool_input: GitPatchCreationToolInput, options: ToolRunOptions | None, context: RunContext
-    ) -> GitPatchCreationToolOutput:
-        # Ensure the repository path exists and is a git repository
-        if not tool_input.repository_path.exists():
-            return GitPatchCreationToolOutput(
-                result=GitPatchCreationToolResult(
-                    success=False,
-                    patch_file_path="",
-                    patch_content="",
-                    error=f"Repository path does not exist: {tool_input.repository_path}"
-                )
-            )
-
-        git_dir = tool_input.repository_path / ".git"
-        if not git_dir.exists():
-            return GitPatchCreationToolOutput(
-                result=GitPatchCreationToolResult(
-                    success=False,
-                    patch_file_path="",
-                    patch_content="",
-                    error=f"Not a git repository: {tool_input.repository_path}"
-                )
-            )
-
-        # list all untracked files in the repository
-        cmd = ["git", "ls-files", "--others", "--exclude-standard"]
-        result = await run_command(cmd, cwd=tool_input.repository_path)
-        if result["exit_code"] != 0:
-            return GitPatchCreationToolOutput(
-                result=GitPatchCreationToolResult(
-                    success=False,
-                    patch_file_path="",
-                    patch_content="",
-                    error=f"Git command failed: {result['stderr']}"
-                )
-            )
-        untracked_files = result["stdout"].splitlines()
-        # list staged as well since that's what the agent usually does after it resolves conflicts
-        cmd = ["git", "diff", "--name-only", "--cached"]
-        result = await run_command(cmd, cwd=tool_input.repository_path)
-        if result["exit_code"] != 0:
-            return GitPatchCreationToolOutput(
-                result=GitPatchCreationToolResult(
-                    success=False,
-                    patch_file_path="",
-                    patch_content="",
-                    error=f"Git command failed: {result['stderr']}"
-                )
-            )
-        staged_files = result["stdout"].splitlines()
-        all_files = untracked_files + staged_files
-        # make sure there are no *.rej files in the repository
-        rej_files = [file for file in all_files if file.endswith(".rej")]
-        if rej_files:
-            return GitPatchCreationToolOutput(
-                result=GitPatchCreationToolResult(
-                    success=False,
-                    patch_file_path="",
-                    patch_content="",
-                    error="Merge conflicts detected in the repository: "
-                    f"{tool_input.repository_path}, {rej_files}"
-                )
-            )
-
-        # git-am leaves the repository in a dirty state, so we need to stage everything
-        # I considered to inspect the patch and only stage the files that are changed by the patch,
-        # but the backport process could create new files or change new ones
-        # so let's go the naive route: git add -A
-        cmd = ["git", "add", "-A"]
-        result = await run_command(cmd, cwd=tool_input.repository_path)
-        if result["exit_code"] != 0:
-            return GitPatchCreationToolOutput(
-                result=GitPatchCreationToolResult(
-                    success=False,
-                    patch_file_path="",
-                    patch_content="",
-                    error=f"Git command failed: {result['stderr']}"
-                )
-            )
-        # continue git-am process
-        cmd = ["git", "am", "--continue"]
-        result = await run_command(cmd, cwd=tool_input.repository_path)
-        if result["exit_code"] != 0:
-            return GitPatchCreationToolOutput(
-                result=GitPatchCreationToolResult(
-                    success=False,
-                    patch_file_path="",
-                    patch_content="",
-                    error=f"git-am failed: {result['stderr']}, out={result['stdout']}"
-                )
-            )
-        # good, now we should have the patch committed, so let's get the file
-        cmd = [
-            "git", "format-patch",
-            "--output",
-            str(tool_input.patch_file_path),
-            "HEAD~1..HEAD"
-        ]
-        result = await run_command(cmd, cwd=tool_input.repository_path)
-        if result["exit_code"] != 0:
-            return GitPatchCreationToolOutput(
-                result=GitPatchCreationToolResult(
-                    success=False,
-                    patch_file_path="",
-                    patch_content="",
-                    error=f"git-format-patch failed: {result['stderr']}"
-                )
-            )
-        return GitPatchCreationToolOutput(
-            result=GitPatchCreationToolResult(
-                success=True,
-                patch_file_path=str(tool_input.patch_file_path),
-                error=None
-            )
-        )
+    ) -> StringToolOutput:
+        try:
+            # Ensure the repository path exists and is a git repository
+            tool_input_path = Path(tool_input.repository_path)
+            if not tool_input_path.exists():
+                return StringToolOutput(result=f"ERROR: Repository path does not exist: {tool_input_path}")
+
+            git_dir = tool_input_path / ".git"
+            if not git_dir.exists():
+                return StringToolOutput(result=f"ERROR: Not a git repository: {tool_input_path}")
+
+            # list all untracked files in the repository
+            rej_candidates = []
+            cmd = ["git", "ls-files", "--others", "--exclude-standard"]
+            result = await run_command(cmd, cwd=tool_input_path)
+            if result["exit_code"] != 0:
+                return StringToolOutput(result=f"ERROR: Git command failed: {result['stderr']}")
+            if result["stdout"]:  # none means no untracked files
+                rej_candidates.extend(result["stdout"].splitlines())
+            # list staged as well since that's what the agent usually does after it resolves conflicts
+            cmd = ["git", "diff", "--name-only", "--cached"]
+            result = await run_command(cmd, cwd=tool_input_path)
+            if result["exit_code"] != 0:
+                return StringToolOutput(result=f"ERROR: Git command failed: {result['stderr']}")
+            if result["stdout"]:
+                rej_candidates.extend(result["stdout"].splitlines())
+            if rej_candidates:
+                # make sure there are no *.rej files in the repository
+                rej_files = [file for file in rej_candidates if file.endswith(".rej")]
+                if rej_files:
+                    return StringToolOutput(result=f"ERROR: Merge conflicts detected in the repository: "
+                                            f"{tool_input.repository_path}, {rej_files}")
+
+            # git-am leaves the repository in a dirty state, so we need to stage everything
+            # I considered to inspect the patch and only stage the files that are changed by the patch,
+            # but the backport process could create new files or change new ones
+            # so let's go the naive route: git add -A
+            cmd = ["git", "add", "-A"]
+            result = await run_command(cmd, cwd=tool_input_path)
+            if result["exit_code"] != 0:
+                return StringToolOutput(result=f"ERROR: Git command failed: {result['stderr']}")
+            # continue git-am process
+            cmd = ["git", "am", "--continue"]
+            result = await run_command(cmd, cwd=tool_input_path)
+            if result["exit_code"] != 0:
+                return StringToolOutput(result=f"ERROR: git-am failed: {result['stderr']},"
+                f" out={result['stdout']}")
+            # good, now we should have the patch committed, so let's get the file
+            cmd = [
+                "git", "format-patch",
+                "--output",
+                tool_input.patch_file_path,
+                "HEAD~1..HEAD"
+            ]
+            result = await run_command(cmd, cwd=tool_input_path)
+            if result["exit_code"] != 0:
+                return StringToolOutput(result=f"ERROR: git-format-patch failed: {result['stderr']}")
+            return StringToolOutput(result=f"Successfully created a patch file: {tool_input.patch_file_path}")
+        except Exception as e:
+            # we absolutely need to do this otherwise the error won't appear anywhere
+            return StringToolOutput(result=f"ERROR: {e}")