Fix unhandled GitRepositoryError on GET /api/git/changes

openhands-agent-server/openhands/agent_server/git_router.py

@@ -5,10 +5,10 @@
 import logging
 from pathlib import Path
 
-from fastapi import APIRouter, Query
+from fastapi import APIRouter, HTTPException, Query
 
 from openhands.agent_server.server_details_router import update_last_execution_time
-from openhands.sdk.git.exceptions import GitRepositoryError
+from openhands.sdk.git.exceptions import GitError, GitRepositoryError
 from openhands.sdk.git.git_changes import get_git_changes
 from openhands.sdk.git.git_diff import get_git_diff
 from openhands.sdk.git.models import GitChange, GitDiff
@@ -62,7 +62,14 @@ async def git_changes_query(
     ref: str | None = Query(None, description=_REF_QUERY_DESCRIPTION),
 ) -> list[GitChange]:
     """Get git changes using query parameter (preferred method)."""
-    return await _get_git_changes(path, ref)
+    try:
+        return await _get_git_changes(path, ref)
+    except GitError as e:
+        # GitRepositoryError is already handled in the helper (returns []).
+        # Any remaining GitError subclass (e.g. GitCommandError) surfaces as
+        # 400 so the client can show an actionable error instead of an
+        # opaque 500.
+        raise HTTPException(status_code=400, detail=str(e))
 
 
 @git_router.get("/diff")
@@ -71,4 +78,11 @@ async def git_diff_query(
     ref: str | None = Query(None, description=_REF_QUERY_DESCRIPTION),
 ) -> GitDiff:
     """Get git diff using query parameter (preferred method)."""
-    return await _get_git_diff(path, ref)
+    try:
+        return await _get_git_diff(path, ref)
+    except GitError as e:
+        # GitRepositoryError is already handled in the helper (returns an
+        # empty diff). Any remaining GitError subclass (e.g. GitCommandError,
+        # GitPathError) surfaces as 400 so the client can show an actionable
+        # error instead of an opaque 500.
+        raise HTTPException(status_code=400, detail=str(e))

openhands-sdk/openhands/sdk/git/git_changes.py

@@ -9,7 +9,7 @@
 import os
 from pathlib import Path
 
-from openhands.sdk.git.exceptions import GitCommandError
+from openhands.sdk.git.exceptions import GitCommandError, GitError
 from openhands.sdk.git.models import GitChange, GitChangeStatus
 from openhands.sdk.git.utils import (
     get_valid_ref,
@@ -226,7 +226,13 @@ def get_git_changes(cwd: str | Path, ref: str | None = None) -> list[GitChange]:
 
     # Add changes from git directories
     for git_dir in git_dirs:
-        git_dir_changes = get_changes_in_repo(str(Path(cwd, git_dir)), ref=ref)
+        try:
+            git_dir_changes = get_changes_in_repo(str(Path(cwd, git_dir)), ref=ref)
+        except GitError:
+            logger.warning(
+                f"Skipping nested git directory {git_dir}: not a valid repository"
+            )
+            continue
         for change in git_dir_changes:
             # Create a new GitChange with the updated path
             updated_change = GitChange(

tests/agent_server/test_git_router.py

@@ -8,7 +8,7 @@
 
 from openhands.agent_server.api import create_app
 from openhands.agent_server.config import Config
-from openhands.sdk.git.exceptions import GitRepositoryError
+from openhands.sdk.git.exceptions import GitCommandError, GitRepositoryError
 from openhands.sdk.git.models import GitChange, GitChangeStatus, GitDiff
 
 
@@ -72,12 +72,28 @@ async def test_git_changes_query_param_with_exception(client):
     with patch("openhands.agent_server.git_router.get_git_changes") as mock_git_changes:
         mock_git_changes.side_effect = RuntimeError("unexpected failure")
 
-        test_path = "nonexistent/repo"
-        response = client.get("/api/git/changes", params={"path": test_path})
+        response = client.get("/api/git/changes", params={"path": "nonexistent/repo"})
 
         assert response.status_code == 500
 
 
+@pytest.mark.asyncio
+async def test_git_changes_query_param_with_command_error(client):
+    """Test git changes returns 400 for GitCommandError."""
+    with patch("openhands.agent_server.git_router.get_git_changes") as mock_git_changes:
+        mock_git_changes.side_effect = GitCommandError(
+            message="git diff failed",
+            command=["git", "diff"],
+            exit_code=128,
+            stderr="fatal: bad revision",
+        )
+
+        response = client.get("/api/git/changes", params={"path": "broken/repo"})
+
+        assert response.status_code == 400
+        assert "git diff failed" in response.json()["detail"]
+
+
 @pytest.mark.asyncio
 async def test_git_changes_returns_empty_list_when_path_is_not_git_repo(client):
     """Non-repo workspaces should yield 200 + [] instead of 500.
@@ -192,15 +208,20 @@ async def test_git_diff_query_param_with_none_values(client):
 
 
 @pytest.mark.asyncio
-async def test_git_diff_query_param_with_exception(client):
-    """Test git diff endpoint with query parameter when git operation fails."""
+async def test_git_diff_query_param_with_command_error(client):
+    """Test git diff returns 400 for GitCommandError."""
     with patch("openhands.agent_server.git_router.get_git_diff") as mock_git_diff:
-        mock_git_diff.side_effect = Exception("Git diff failed")
+        mock_git_diff.side_effect = GitCommandError(
+            message="git diff failed",
+            command=["git", "diff"],
+            exit_code=128,
+            stderr="fatal: bad revision",
+        )
 
-        test_path = "nonexistent/file.py"
-        response = client.get("/api/git/diff", params={"path": test_path})
+        response = client.get("/api/git/diff", params={"path": "broken/file.py"})
 
-        assert response.status_code == 500
+        assert response.status_code == 400
+        assert "git diff failed" in response.json()["detail"]
 
 
 @pytest.mark.asyncio

tests/sdk/git/test_git_changes.py

@@ -340,6 +340,55 @@ def test_git_changes_with_gitignore():
         assert "__pycache__/module.pyc" not in paths
 
 
+def test_get_git_changes_skips_vanished_nested_repo():
+    """Test that get_git_changes skips nested repos that vanish (TOCTOU).
+
+    Simulates a directory disappearing between glob scan and
+    validate_git_repository by patching get_changes_in_repo to raise
+    GitRepositoryError for one nested directory.
+    """
+    from unittest.mock import patch
+
+    from openhands.sdk.git.exceptions import GitRepositoryError
+
+    with tempfile.TemporaryDirectory() as temp_dir:
+        setup_git_repo(temp_dir)
+
+        # Create a file in the main repo
+        (Path(temp_dir) / "main.txt").write_text("main repo file")
+
+        # Create a valid nested repo
+        nested = Path(temp_dir) / "goodrepo"
+        nested.mkdir()
+        setup_git_repo(str(nested))
+        (nested / "nested.txt").write_text("nested file")
+
+        # Create a second nested repo that will "vanish"
+        vanished = Path(temp_dir) / "vanished"
+        vanished.mkdir()
+        (vanished / ".git").mkdir()  # just enough for glob to find it
+
+        # Patch get_changes_in_repo to raise for the vanished directory
+        original_fn = get_changes_in_repo
+
+        def patched_get_changes(repo_dir, ref=None):
+            if str(Path(repo_dir).resolve()) == str(vanished.resolve()):
+                raise GitRepositoryError(f"Directory does not exist: {repo_dir}")
+            return original_fn(repo_dir, ref=ref)
+
+        with patch(
+            "openhands.sdk.git.git_changes.get_changes_in_repo",
+            side_effect=patched_get_changes,
+        ):
+            changes = get_git_changes(temp_dir)
+
+        paths = {str(c.path) for c in changes}
+        assert "main.txt" in paths
+        assert "goodrepo/nested.txt" in paths
+        # vanished repo should be skipped, not crash
+        assert all("vanished/" not in p for p in paths)
+
+
 def test_git_changes_with_binary_files():
     """Test git changes detection with binary files."""
     with tempfile.TemporaryDirectory() as temp_dir: