From b624443bdad8351292c0659b6f040147e978260e Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Sat, 27 Dec 2025 02:38:57 +0000
Subject: [PATCH] Address code review feedback

- Add sys/wait.h include for WEXITSTATUS macro
- Check config command results before commit
- Escape commit messages to prevent injection
- Fix potential npos overflow in string trimming
- Use std::filesystem::temp_directory_path() for portability
- Fix base branch parameter issue (clone already at base_ref)
- All tests still pass (17/17)

Co-authored-by: johndoe6345789 <224850594+johndoe6345789@users.noreply.github.com>
---
 backend/src/controllers/PRController.cc |  9 +++---
 backend/src/git/git_cli.cpp             | 39 ++++++++++++++++++++++---
 backend/tests/test_git_cli.cpp          |  5 ++--
 3 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/backend/src/controllers/PRController.cc b/backend/src/controllers/PRController.cc
index e7b37c0..0a6642d 100644
--- a/backend/src/controllers/PRController.cc
+++ b/backend/src/controllers/PRController.cc
@@ -195,8 +195,9 @@ void PRController::resolvePR(
             response["note"] = "Git CLI not available - branch creation skipped";
         } else {
             // Clone repository to temporary location
-            std::string temp_dir = "/tmp/wizardmerge_pr_" + std::to_string(pr_number) + "_" + 
-                                  std::to_string(std::time(nullptr));
+            std::filesystem::path temp_base = std::filesystem::temp_directory_path();
+            std::string temp_dir = (temp_base / ("wizardmerge_pr_" + std::to_string(pr_number) + "_" + 
+                                   std::to_string(std::time(nullptr)))).string();
             
             // Build repository URL
             std::string repo_url;
@@ -216,8 +217,8 @@ void PRController::resolvePR(
             if (!clone_result.success) {
                 response["note"] = "Failed to clone repository: " + clone_result.error;
             } else {
-                // Create new branch
-                auto branch_result = create_branch(temp_dir, branch_name, pr.base_ref);
+                // Create new branch (without base_branch parameter since we cloned from base_ref)
+                auto branch_result = create_branch(temp_dir, branch_name);
                 
                 if (!branch_result.success) {
                     response["note"] = "Failed to create branch: " + branch_result.error;
diff --git a/backend/src/git/git_cli.cpp b/backend/src/git/git_cli.cpp
index 99c36a3..d0f7d6f 100644
--- a/backend/src/git/git_cli.cpp
+++ b/backend/src/git/git_cli.cpp
@@ -9,6 +9,7 @@
 #include <sstream>
 #include <iostream>
 #include <filesystem>
+#include <sys/wait.h>
 
 namespace wizardmerge {
 namespace git {
@@ -141,13 +142,36 @@ GitResult commit(
 ) {
     // Set user config if provided
     if (!config.user_name.empty() && !config.user_email.empty()) {
-        execute_command(git_command(repo_path, 
+        auto name_result = execute_command(git_command(repo_path, 
             "config user.name \"" + config.user_name + "\""));
-        execute_command(git_command(repo_path, 
+        if (!name_result.success) {
+            GitResult result;
+            result.success = false;
+            result.error = "Failed to set user.name: " + name_result.error;
+            result.exit_code = name_result.exit_code;
+            return result;
+        }
+        
+        auto email_result = execute_command(git_command(repo_path, 
             "config user.email \"" + config.user_email + "\""));
+        if (!email_result.success) {
+            GitResult result;
+            result.success = false;
+            result.error = "Failed to set user.email: " + email_result.error;
+            result.exit_code = email_result.exit_code;
+            return result;
+        }
     }
     
-    std::string cmd = "commit -m \"" + message + "\"";
+    // Escape commit message for shell
+    std::string escaped_message = message;
+    size_t pos = 0;
+    while ((pos = escaped_message.find("\"", pos)) != std::string::npos) {
+        escaped_message.replace(pos, 1, "\\\"");
+        pos += 2;
+    }
+    
+    std::string cmd = "commit -m \"" + escaped_message + "\"";
     return execute_command(git_command(repo_path, cmd));
 }
 
@@ -190,7 +214,14 @@ std::optional<std::string> get_current_branch(const std::string& repo_path) {
     
     // Trim whitespace
     std::string branch = result.output;
-    branch.erase(branch.find_last_not_of(" \n\r\t") + 1);
+    size_t last_non_ws = branch.find_last_not_of(" \n\r\t");
+    
+    if (last_non_ws == std::string::npos) {
+        // String contains only whitespace
+        return std::nullopt;
+    }
+    
+    branch.erase(last_non_ws + 1);
     
     return branch;
 }
diff --git a/backend/tests/test_git_cli.cpp b/backend/tests/test_git_cli.cpp
index 76a600e..bed9758 100644
--- a/backend/tests/test_git_cli.cpp
+++ b/backend/tests/test_git_cli.cpp
@@ -16,8 +16,9 @@ protected:
     std::string test_dir;
     
     void SetUp() override {
-        // Create temporary test directory
-        test_dir = "/tmp/wizardmerge_git_test_" + std::to_string(time(nullptr));
+        // Create temporary test directory using std::filesystem
+        std::filesystem::path temp_base = std::filesystem::temp_directory_path();
+        test_dir = (temp_base / ("wizardmerge_git_test_" + std::to_string(time(nullptr)))).string();
         fs::create_directories(test_dir);
     }