fix(feedback): clone existing batch branch directly to prevent non-fast-forward push failure

server/src/routes/feedback/proposed_edits/mod.rs

@@ -59,12 +59,17 @@ impl EditRequest {
     async fn apply_changes_and_generate_description(
         &self,
         branch_name: &str,
+        branch_is_new: bool,
     ) -> anyhow::Result<String> {
         let Some(pat) = crate::external::github::GitHub::github_token() else {
             anyhow::bail!("Failed to get GitHub token");
         };
         let url = format!("https://{pat}@github.com/TUM-Dev/NavigaTUM");
-        let repo = TempRepo::clone_and_checkout(&url, branch_name).await?;
+        let repo = if branch_is_new {
+            TempRepo::clone_and_checkout_new_branch(&url, branch_name).await?
+        } else {
+            TempRepo::clone_and_checkout_existing_branch(&url, branch_name).await?
+        };
         let desc = repo.apply_and_gen_description(self, branch_name);
         repo.commit(&desc.title).await?;
         repo.push().await?;
@@ -193,9 +198,10 @@ pub async fn propose_edits(
         }
         None => (branch_name, None),
     };
+    let branch_is_new = pr_number_opt.is_none();
 
     match req_data
-        .apply_changes_and_generate_description(&branch_to_use)
+        .apply_changes_and_generate_description(&branch_to_use, branch_is_new)
         .await
     {
         Ok(description) => {

server/src/routes/feedback/proposed_edits/tmp_repo.rs

@@ -11,11 +11,18 @@ pub struct TempRepo {
     branch_name: String,
 }
 impl TempRepo {
-    #[tracing::instrument]
-    pub async fn clone_and_checkout(url: &str, branch_name: &str) -> anyhow::Result<Self> {
+    /// Clone the repository from `main` and create `branch_name` as a new local branch.
+    ///
+    /// Use this when no batch PR exists yet and a fresh branch needs to be pushed for the first
+    /// time.
+    #[tracing::instrument(skip(url))]
+    pub async fn clone_and_checkout_new_branch(
+        url: &str,
+        branch_name: &str,
+    ) -> anyhow::Result<Self> {
         let dir = tempfile::tempdir()?;
 
-        info!(url, target_dir= ?dir,"Cloning repository");
+        info!(target_dir = ?dir, "Cloning repository (new branch)");
         let out = Command::new("git")
             .current_dir(&dir)
             .arg("clone")
@@ -26,10 +33,10 @@ impl TempRepo {
             .await?;
         debug!(output=?out,"git clone output");
         if out.status.code() != Some(0) {
-            anyhow::bail!("git status failed with output: {out:?}");
+            anyhow::bail!("git clone failed with output: {out:?}");
         }
 
-        // checkout + create branch
+        // Create a new local branch from main.
         let out = Command::new("git")
             .current_dir(&dir)
             .arg("checkout")
@@ -44,7 +51,40 @@ impl TempRepo {
                 dir,
                 branch_name: branch_name.to_string(),
             }),
-            _ => anyhow::bail!("git commit failed with output: {out:?}"),
+            _ => anyhow::bail!("git checkout failed with output: {out:?}"),
+        }
+    }
+
+    /// Clone the repository by checking out an already-existing remote branch `branch_name`.
+    ///
+    /// Use this when adding an edit to an existing batch PR.  Cloning `main` and branching from
+    /// it would create a diverging history and cause the subsequent push to be rejected as
+    /// non-fast-forward.
+    #[tracing::instrument(skip(url))]
+    pub async fn clone_and_checkout_existing_branch(
+        url: &str,
+        branch_name: &str,
+    ) -> anyhow::Result<Self> {
+        let dir = tempfile::tempdir()?;
+
+        info!(target_dir = ?dir, branch_name, "Cloning repository (existing branch)");
+        let out = Command::new("git")
+            .current_dir(&dir)
+            .arg("clone")
+            .arg("--depth=1")
+            .arg("--branch")
+            .arg(branch_name)
+            .arg(url)
+            .arg(dir.path())
+            .output()
+            .await?;
+        debug!(output=?out,"git clone output");
+        match out.status.code() {
+            Some(0) => Ok(Self {
+                dir,
+                branch_name: branch_name.to_string(),
+            }),
+            _ => anyhow::bail!("git clone failed with output: {out:?}"),
         }
     }
 
@@ -153,7 +193,7 @@ mod tests {
     const GIT_URL: &str = "https://github.com/CommanderStorm/dotfiles.git";
     #[tokio::test]
     async fn test_new() {
-        let temp_repo = TempRepo::clone_and_checkout(GIT_URL, "branch_does_not_exist")
+        let temp_repo = TempRepo::clone_and_checkout_new_branch(GIT_URL, "branch_does_not_exist")
             .await
             .unwrap();
         assert!(temp_repo.dir.path().exists());
@@ -163,7 +203,7 @@ mod tests {
 
     #[tokio::test]
     async fn test_checkout_and_commit() {
-        let temp_repo = TempRepo::clone_and_checkout(GIT_URL, "branch_does_not_exist")
+        let temp_repo = TempRepo::clone_and_checkout_new_branch(GIT_URL, "branch_does_not_exist")
             .await
             .unwrap();
         // test the branch was created