refactor: Enhance _run_git_command method for improved subprocess handling and error reporting

cogs/ai_code_agent_cog.py

@@ -196,30 +196,72 @@ class AICodeAgentCog(commands.Cog):
         else:
             await ctx.send("AICodeAgent: No conversation history found for you to clear.")
 
-    async def _run_git_command(self, command: str) -> Tuple[bool, str]:
-        """Runs a Git command and returns (success_status, output_string)."""
-        try:
-            process = await asyncio.create_subprocess_shell(
-                command,
-                stdout=subprocess.PIPE,
-                stderr=subprocess.PIPE
-            )
-            stdout, stderr = await process.communicate()
-            
-            output = ""
-            if stdout:
-                output += stdout.decode(errors='replace')
-            if stderr:
-                output += stderr.decode(errors='replace')
-            
-            if process.returncode == 0:
-                return True, output.strip()
-            else:
-                error_message = f"Git command failed with exit code {process.returncode}:\n{output.strip()}"
-                print(f"AICodeAgentCog: {error_message}")
-                return False, error_message
-        except Exception as e:
-            error_message = f"Exception running Git command '{command}': {e}"
+    async def _run_git_command(self, command_str: str) -> Tuple[bool, str]:
+        """
+        Runs a Git command using subprocess.Popen in a thread and returns (success_status, output_string).
+        """
+        # For Git commands, we generally want them to run in the bot's current working directory,
+        # which should be the root of the Git repository.
+        cwd = os.getcwd()
+        env = os.environ.copy()
+
+        print(f"AICodeAgentCog: Executing Git command: '{command_str}' in CWD: '{cwd}'")
+
+        def run_sync_subprocess():
+            try:
+                # For git commands, shell=False is safer if command_str is split into a list.
+                # If command_str is a single string and might contain shell features (though unlikely for our git use),
+                # shell=True would be needed, but then command_str must be trustworthy.
+                # Given our specific git commands, splitting them is safer.
+                
+                # Simplified: if it's a simple git command, can pass as string with shell=True,
+                # but better to split for shell=False.
+                # For now, let's assume simple commands or trust shell=True for git.
+                # However, the example used shell=True. Let's try that first for consistency with the hint.
+                
+                proc = subprocess.Popen(
+                    command_str, # Command as a string
+                    shell=True,  # Execute through the shell
+                    cwd=cwd,
+                    env=env,
+                    stdout=subprocess.PIPE,
+                    stderr=subprocess.PIPE,
+                    text=True, # Decodes stdout/stderr as text
+                    errors='replace' # Handles decoding errors
+                )
+                stdout, stderr = proc.communicate(timeout=60) # 60-second timeout for git commands
+                return (stdout, stderr, proc.returncode, False)
+            except subprocess.TimeoutExpired:
+                proc.kill()
+                stdout, stderr = proc.communicate()
+                return (stdout, stderr, -1, True) # -1 for timeout-specific return code
+            except FileNotFoundError as fnf_err: # Specifically catch if 'git' command itself is not found
+                 print(f"AICodeAgentCog: FileNotFoundError for command '{command_str}': {fnf_err}. Is Git installed and in PATH?")
+                 return ("", f"FileNotFoundError: {fnf_err}. Ensure Git is installed and in PATH.", -2, False)
+            except Exception as e:
+                print(f"AICodeAgentCog: Exception in run_sync_subprocess for '{command_str}': {type(e).__name__} - {e}")
+                return ("", str(e), -3, False) # -3 for other exceptions
+
+        stdout_str, stderr_str, returncode, timed_out = await asyncio.to_thread(run_sync_subprocess)
+        
+        full_output = ""
+        if timed_out:
+            full_output += "Command timed out after 60 seconds.\n"
+        if stdout_str:
+            full_output += f"Stdout:\n{stdout_str.strip()}\n"
+        if stderr_str:
+            full_output += f"Stderr:\n{stderr_str.strip()}\n"
+        
+        if returncode == 0:
+            # For commands like `git rev-parse --abbrev-ref HEAD`, stdout is the primary result.
+            # If stdout is empty but no error, return it as is.
+            # If full_output is just "Stdout:\n\n", it means empty stdout.
+            # We want the actual stdout for rev-parse, not the "Stdout:" prefix.
+            if command_str == "git rev-parse --abbrev-ref HEAD" and stdout_str:
+                 return True, stdout_str.strip() # Return just the branch name
+            return True, full_output.strip() if full_output.strip() else "Command executed successfully with no output."
+        else:
+            error_message = f"Git command failed. Return Code: {returncode}\n{full_output.strip()}"
             print(f"AICodeAgentCog: {error_message}")
             return False, error_message