Fix PR review payload: Use full clone and merge-base for accurate diffs (Vibe Kanban) (#1583)

* The code compiles successfully. Let me provide a summary of the changes made:

## Summary of Changes

All changes have been implemented successfully. Here's what was modified:

### 1. `crates/remote/src/github_app/service.rs`
- **Removed `--depth 1`** from `git clone` command (line 306-307) to do a full clone instead of shallow
- **Removed `--depth 1`** from `git fetch` command (line 338-340)
- **Added `get_merge_base()` function** (lines 392-415) - calculates the merge-base between HEAD and the base branch

### 2. `crates/remote/src/github_app/pr_review.rs`
- **Changed `base_sha` to `base_ref`** in `PrReviewParams` struct (line 29) - now stores the branch name instead of SHA
- **Added merge-base calculation** after cloning (lines 110-113) - computes the correct base commit for diffing
- **Updated `start_request`** to use the calculated `base_commit` instead of `params.base_sha` (line 162)

### 3. `crates/remote/src/routes/github_app.rs`
- **Extract `base_ref`** from webhook payload (lines 883-886) instead of `base_sha`
- **Updated `PrReviewParams`** construction to use `base_ref` (line 915)
- **Updated debug endpoint** to use `pr_details.base.ref_name` (line 1025)

### 4. `crates/remote/src/github_app/service.rs` (PrRef struct)
- **Added `ref_name` field** to `PrRef` struct (lines 78-80) to support the debug endpoint

The payload.tar.gz will now include the full git history needed for the review worker to compute diffs properly.

* Cleanup script changes for task attempt 3548bc82-973b-4e1b-a95a-37bb692ead71

crates/remote/src/github_app/pr_review.rs

@@ -26,7 +26,7 @@ pub struct PrReviewParams {
     pub pr_title: String,
     pub pr_body: String,
     pub head_sha: String,
-    pub base_sha: String,
+    pub base_ref: String, // Branch name like "main" - used to calculate merge-base
 }
 
 #[derive(Debug, Error)]
@@ -107,18 +107,23 @@ impl PrReviewService {
 
         debug!(review_id = %review_id, "Repository cloned");
 
-        // 2. Create tarball
+        // 2. Calculate merge-base for accurate diff computation
+        let base_commit =
+            GitHubAppService::get_merge_base(temp_dir.path(), &params.base_ref).await?;
+        debug!(review_id = %review_id, base_commit = %base_commit, "Merge-base calculated");
+
+        // 3. Create tarball
         let tarball =
             create_tarball(temp_dir.path()).map_err(|e| PrReviewError::Archive(e.to_string()))?;
 
         let tarball_size_mb = tarball.len() as f64 / 1_048_576.0;
         debug!(review_id = %review_id, size_mb = tarball_size_mb, "Tarball created");
 
-        // 3. Upload to R2
+        // 4. Upload to R2
         let r2_path = self.r2.upload_bytes(review_id, tarball).await?;
         debug!(review_id = %review_id, r2_path = %r2_path, "Uploaded to R2");
 
-        // 4. Create review record in database
+        // 5. Create review record in database
         let gh_pr_url = format!(
             "https://github.com/{}/{}/pull/{}",
             params.owner, params.repo, params.pr_number
@@ -139,7 +144,7 @@ impl PrReviewService {
 
         debug!(review_id = %review_id, "Review record created");
 
-        // 5. Start the review worker
+        // 6. Start the review worker
         let codebase_url = format!(
             "{}/reviews/{}/payload.tar.gz",
             self.r2_public_url(),
@@ -154,7 +159,7 @@ impl PrReviewService {
             "org": params.owner,
             "repo": params.repo,
             "codebaseUrl": codebase_url,
-            "baseCommit": params.base_sha,
+            "baseCommit": base_commit,
             "callbackUrl": callback_url,
         });
 

crates/remote/src/github_app/service.rs

@@ -76,6 +76,8 @@ pub struct PrDetails {
 #[derive(Debug, Clone, Deserialize)]
 pub struct PrRef {
     pub sha: String,
+    #[serde(rename = "ref")]
+    pub ref_name: String,
 }
 
 /// Service for interacting with the GitHub App API
@@ -294,6 +296,7 @@ impl GitHubAppService {
         debug!(owner, repo, head_sha, "Cloning repository");
 
         // Clone the repository with security flags to prevent code execution from untrusted repos
+        // Note: We do a full clone (not shallow) to ensure git history is available for merge-base calculation
         let output = Command::new("git")
             .args([
                 "-c",
@@ -303,8 +306,6 @@ impl GitHubAppService {
                 "-c",
                 "core.symlinks=false",
                 "clone",
-                "--depth",
-                "1",
                 &clone_url,
                 ".",
             ])
@@ -331,14 +332,12 @@ impl GitHubAppService {
             )));
         }
 
-        // Fetch the specific commit (in case it's not in shallow clone)
+        // Fetch the specific commit (in case it's not in the default branch)
         let output = Command::new("git")
             .args([
                 "-c",
                 "core.hooksPath=/dev/null",
                 "fetch",
-                "--depth",
-                "1",
                 "origin",
                 head_sha,
             ])
@@ -392,6 +391,31 @@ impl GitHubAppService {
         Ok(temp_dir)
     }
 
+    /// Calculate the merge-base between the current HEAD and the base branch.
+    /// This gives the correct base commit for computing diffs, even if the base branch has moved.
+    pub async fn get_merge_base(
+        repo_dir: &std::path::Path,
+        base_ref: &str,
+    ) -> Result<String, GitHubAppError> {
+        let output = Command::new("git")
+            .args(["merge-base", &format!("origin/{}", base_ref), "HEAD"])
+            .env("GIT_CONFIG_GLOBAL", "/dev/null")
+            .env("GIT_CONFIG_SYSTEM", "/dev/null")
+            .current_dir(repo_dir)
+            .output()
+            .await
+            .map_err(|e| GitHubAppError::GitOperation(format!("merge-base failed: {e}")))?;
+
+        if !output.status.success() {
+            let stderr = String::from_utf8_lossy(&output.stderr);
+            return Err(GitHubAppError::GitOperation(format!(
+                "merge-base failed: {stderr}"
+            )));
+        }
+
+        Ok(String::from_utf8_lossy(&output.stdout).trim().to_string())
+    }
+
     /// Get details about a pull request
     pub async fn get_pr_details(
         &self,

crates/remote/src/routes/github_app.rs

@@ -880,9 +880,9 @@ async fn handle_pull_request_event(
         .as_str()
         .unwrap_or("")
         .to_string();
-    let base_sha = payload["pull_request"]["base"]["sha"]
+    let base_ref = payload["pull_request"]["base"]["ref"]
         .as_str()
-        .unwrap_or("")
+        .unwrap_or("main")
         .to_string();
 
     // Spawn async task to process PR review
@@ -912,7 +912,7 @@ async fn handle_pull_request_event(
             pr_title,
             pr_body,
             head_sha,
-            base_sha,
+            base_ref,
         };
 
         if let Err(e) = service.process_pr_review(&pool, params).await {
@@ -1022,7 +1022,7 @@ pub async fn trigger_pr_review(
         pr_title: pr_details.title,
         pr_body: pr_details.body.unwrap_or_default(),
         head_sha: pr_details.head.sha,
-        base_sha: pr_details.base.sha,
+        base_ref: pr_details.base.ref_name,
     };
 
     let review_id = service