Skip to content

[vibecoded] rollback refs on error after push - #126

Merged
slinder1 merged 1 commit into
users/slinder1/I411874873cbf584f5aac31cb2a08c1197fd885f7from
users/slinder1/I89bc2daaf27b219257606b9ce161df3fb23fb005
Sep 2, 2026
Merged

slinder1 merged 1 commit into
users/slinder1/I411874873cbf584f5aac31cb2a08c1197fd885f7from
users/slinder1/I89bc2daaf27b219257606b9ce161df3fb23fb005

Conversation

@slinder1

@slinder1 slinder1 commented Sep 2, 2026

Copy link
Copy Markdown
Owner

Change-Id: I89bc2daaf27b219257606b9ce161df3fb23fb005

@slinder1
slinder1 changed the base branch from main to users/slinder1/I411874873cbf584f5aac31cb2a08c1197fd885f7 September 2, 2026 18:26
@slinder1

slinder1 commented Sep 2, 2026

Copy link
Copy Markdown
Owner Author
🛠️ Initial changes (click to expand):
diff --git b/src/change.rs a/src/change.rs
@@ -27,7 +27,14 @@ impl AnyChange {
     pub fn diff(&self, remote_refs: &RemoteRefs) -> Result<Diff> {
         Ok(match self {
             AnyChange::LocalChange(local_change) => Diff::InitialDiff(local_change.diff()?),
-            AnyChange::Change(change) => Diff::InterDiff(change.interdiff(remote_refs)?),
+            AnyChange::Change(change)
+                if remote_refs
+                    .get(&change.local_change.remote_branch_ref())
+                    .is_some() =>
+            {
+                Diff::InterDiff(change.interdiff(remote_refs)?)
+            }
+            AnyChange::Change(change) => Diff::InitialDiff(change.local_change.diff()?),
         })
     }
 }
diff --git b/src/cli.rs a/src/cli.rs
@@ -101,6 +101,8 @@ pub enum Command {
     /// existing branch is advanced with a synthetic commit whose first parent is its old tip, so
     /// every update is a fast-forward. Each commit is matched to its existing PR or else a new PR
     /// is created for it. The PRs are stacked such that they reproduce the local branch sequence.
+    /// If a later PR operation fails, Praddle attempts to atomically restore the original remote
+    /// ref tips with force-with-lease before returning the error.
     ///
     /// Note: This command never modifies existing commits, local refs, the index, or the worktree.
     /// Synthetic commits are written without attaching local refs and then pushed to `$remote`.
diff --git b/src/praddle.rs a/src/praddle.rs
@@ -128,7 +128,7 @@ fn push(args: &cli::Push) -> Result<()> {
             })
         })
         .collect::<Result<Vec<_>>>()?;
-    publication
+    let published_refs = publication
         .push()
         .context("could not publish local changes")?;
     let mut changes = any_changes
@@ -204,6 +204,7 @@ fn push(args: &cli::Push) -> Result<()> {
             .collect::<Result<Vec<_>>>()
             .context("could not mark prs as ready")?;
     }
+    published_refs.commit();
     Ok(())
 }
 
diff --git b/src/publication.rs a/src/publication.rs
@@ -77,14 +77,8 @@ fn validate_pr_refs(changes: &[AnyChange], refs: &HashMap<String, Oid>) -> Resul
                     change.pr.head_ref_name
                 );
             }
-            if !refs.contains_key(&head_ref) {
-                bail!(
-                    "remote branch for pr {} does not exist: {head_ref}",
-                    change.pr.number
-                );
-            }
             let old_base_ref = branch_ref(&change.pr.base_ref_name);
-            if !refs.contains_key(&old_base_ref) {
+            if refs.contains_key(&head_ref) && !refs.contains_key(&old_base_ref) {
                 bail!(
                     "base branch for pr {} does not exist: {old_base_ref}",
                     change.pr.number
@@ -146,29 +140,98 @@ impl PublicationPlan {
         &self.bases
     }
 
-    pub fn push(&self) -> Result<()> {
-        let refspecs: Vec<String> = self
+    pub fn push(&self) -> Result<PublishedRefs> {
+        let updates: Vec<&BranchUpdate> = self
             .updates
             .iter()
             .filter(|update| update.old_oid != Some(update.new_oid))
-            .map(|update| format!("{}:{}", update.new_oid, update.reference))
             .collect();
-        if refspecs.is_empty() {
-            return Ok(());
+        if updates.is_empty() {
+            return Ok(PublishedRefs { rollback: None });
         }
 
+        let remote_url = env::get().remote_url()?;
         let mut command = Command::new("git");
-        command.args(["push", "--atomic", &env::get().remote_url()?]);
-        command.args(refspecs);
+        command.args(["push", "--atomic", &remote_url]);
+        command.args(
+            updates
+                .iter()
+                .map(|update| format!("{}:{}", update.new_oid, update.reference)),
+        );
         if env::get().dry_run() {
             print_cmd_and_files(&command, std::iter::empty())?;
+            return Ok(PublishedRefs { rollback: None });
         } else {
             exec(&mut command)?;
         }
+        Ok(PublishedRefs {
+            rollback: Some(Rollback {
+                remote_url,
+                refs: updates
+                    .into_iter()
+                    .map(|update| RefRestore {
+                        reference: update.reference.clone(),
+                        old_oid: update.old_oid,
+                        new_oid: update.new_oid,
+                    })
+                    .collect(),
+            }),
+        })
+    }
+}
+
+#[must_use = "published refs are rolled back unless committed"]
+pub struct PublishedRefs {
+    rollback: Option<Rollback>,
+}
+
+impl PublishedRefs {
+    pub fn commit(mut self) {
+        self.rollback = None;
+    }
+}
+
+impl Drop for PublishedRefs {
+    fn drop(&mut self) {
+        let Some(rollback) = self.rollback.take() else {
+            return;
+        };
+        if let Err(error) = rollback.restore() {
+            eprintln!("warning: could not restore published refs: {error:#}");
+        }
+    }
+}
+
+struct Rollback {
+    remote_url: String,
+    refs: Vec<RefRestore>,
+}
+
+impl Rollback {
+    fn restore(self) -> Result<()> {
+        let mut command = Command::new("git");
+        command.args(["push", "--atomic", &self.remote_url]);
+        for restore in &self.refs {
+            command.arg(format!(
+                "--force-with-lease={}:{}",
+                restore.reference, restore.new_oid
+            ));
+        }
+        command.args(self.refs.iter().map(|restore| match restore.old_oid {
+            Some(old_oid) => format!("{old_oid}:{}", restore.reference),
+            None => format!(":{}", restore.reference),
+        }));
+        exec(&mut command).context("rollback push failed")?;
         Ok(())
     }
 }
 
+struct RefRestore {
+    reference: String,
+    old_oid: Option<Oid>,
+    new_oid: Oid,
+}
+
 fn desired_bases(changes: &[AnyChange]) -> Vec<String> {
     changes
         .iter()
diff --git b/test-server/src/lib.rs a/test-server/src/lib.rs
@@ -487,19 +487,24 @@ zero=0000000000000000000000000000000000000000
 while read old new ref
 do
     [ "$old" = "$zero" ] && continue
-    if [ "$new" = "$zero" ]; then
-        echo "deletion rejected: $ref" >&2
-        exit 1
-    fi
+    [ "$new" = "$zero" ] && continue
     case "$ref" in
         refs/heads/*)
             git merge-base --is-ancestor "$old" "$new"
             status=$?
+            if [ "$status" -eq 0 ]; then
+                continue
+            elif [ "$status" -ne 1 ]; then
+                echo "could not verify update: $ref" >&2
+                exit "$status"
+            fi
+            git merge-base --is-ancestor "$new" "$old"
+            status=$?
             if [ "$status" -eq 1 ]; then
-                echo "non-fast-forward update rejected: $ref" >&2
+                echo "divergent update rejected: $ref" >&2
                 exit 1
             elif [ "$status" -ne 0 ]; then
-                echo "could not verify update: $ref" >&2
+                echo "could not verify inverse update: $ref" >&2
                 exit "$status"
             fi
             ;;
diff --git b/test-server/tests/remote_graph.rs a/test-server/tests/remote_graph.rs
@@ -33,7 +33,7 @@ async fn inspects_the_bare_remote_graph() {
 }
 
 #[tokio::test(flavor = "multi_thread")]
-async fn rejects_non_fast_forward_updates_and_deletions() {
+async fn rejects_divergent_updates_but_allows_inverse_fast_forwards() {
     let harness = TestHarness::start("alice", "widgets").await.unwrap();
     harness.write("feature", "initial\n").unwrap();
     harness.git(["add", "feature"]).unwrap();
@@ -59,14 +59,22 @@ async fn rejects_non_fast_forward_updates_and_deletions() {
         harness.remote_ref_oid("refs/heads/change").unwrap(),
         Some(change)
     );
-    assert!(
-        harness
-            .git(["push", "origin", ":refs/heads/change"])
-            .is_err()
+
+    harness
+        .git(["push", "--force", "origin", "main:refs/heads/change"])
+        .unwrap();
+    assert_eq!(
+        harness.remote_ref_oid("refs/heads/change").unwrap(),
+        harness.remote_ref_oid("refs/heads/main").unwrap()
     );
 
     harness
         .git(["push", "origin", "HEAD:refs/heads/new-branch"])
         .unwrap();
-    assert!(harness.remote_ref_exists("refs/heads/new-branch").unwrap());
+    assert!(
+        harness
+            .git(["push", "origin", ":refs/heads/new-branch"])
+            .is_ok()
+    );
+    assert!(!harness.remote_ref_exists("refs/heads/new-branch").unwrap());
 }
diff --git b/tests/push.rs a/tests/push.rs
@@ -1,4 +1,5 @@
 use praddle_test_server::TestHarness;
+use std::process::Output;
 
 const INITIAL_COMMENT_PREFIX: &str =
     "<details>\n<summary>🛠️ Initial changes (click to expand):</summary>\n\n```diff\n";
@@ -16,7 +17,7 @@ fn interdiff_comment(path: &str, before: &str, after: &str) -> String {
     )
 }
 
-fn push(harness: &TestHarness) {
+fn push_output(harness: &TestHarness) -> Output {
     let mut command = harness.command(env!("CARGO_BIN_EXE_praddle"));
     command.args([
         "--remote=origin",
@@ -25,7 +26,11 @@ fn push(harness: &TestHarness) {
         "--serial",
         "push",
     ]);
-    let output = command.output().unwrap();
+    command.output().unwrap()
+}
+
+fn push(harness: &TestHarness) {
+    let output = push_output(harness);
     assert!(
         output.status.success(),
         "praddle failed\nstdout:\n{}\nstderr:\n{}",
@@ -293,6 +298,81 @@ async fn leaves_an_unchanged_stack_alone() {
 }
 
 #[tokio::test(flavor = "multi_thread")]
+async fn restores_remote_refs_when_pr_reconciliation_fails() {
+    let harness = TestHarness::start("alice", "widgets").await.unwrap();
+    harness.write("change", "before\n").unwrap();
+    harness.git(["add", "change"]).unwrap();
+    harness
+        .git(["commit", "-m", "Change", "-m", "Change-Id: I0001"])
+        .unwrap();
+    push(&harness);
+    let old_head = harness
+        .remote_ref_oid("refs/heads/users/alice/I0001")
+        .unwrap()
+        .unwrap();
+
+    harness
+        .git(["push", "origin", "refs/heads/main:refs/heads/auxiliary"])
+        .unwrap();
+    let mut edit = harness.command("gh");
+    edit.args([
+        "pr",
+        "edit",
+        "1",
+        "--repo=https://github.com/alice/widgets",
+        "--base=auxiliary",
+    ]);
+    assert!(edit.output().unwrap().status.success());
+
+    harness.write("change", "after\n").unwrap();
+    harness.git(["add", "change"]).unwrap();
+    harness
+        .git([
+            "commit",
+            "--amend",
+            "-m",
+            "Change",
+            "-m",
+            "Change-Id: I0001",
+        ])
+        .unwrap();
+
+    let failed = push_output(&harness);
+    assert!(!failed.status.success());
+    assert_eq!(
+        harness
+            .remote_ref_oid("refs/heads/users/alice/I0001")
+            .unwrap(),
+        Some(old_head.clone())
+    );
+
+    let mut unstack = harness.command("gh");
+    unstack.args([
+        "api",
+        "--method=POST",
+        "repos/alice/widgets/stacks/1/unstack",
+    ]);
+    assert!(unstack.output().unwrap().status.success());
+    push(&harness);
+
+    let new_head = harness
+        .remote_ref_oid("refs/heads/users/alice/I0001")
+        .unwrap()
+        .unwrap();
+    assert!(harness.is_ancestor(&old_head, &new_head).unwrap());
+    let snapshot = harness.snapshot();
+    assert_eq!(snapshot.pull_requests[0].base_ref_name, "main");
+    assert_eq!(
+        snapshot.pull_requests[0].comments,
+        [
+            initial_comment("change", "before"),
+            interdiff_comment("change", "before", "after"),
+        ]
+    );
+    assert_eq!(snapshot.stacks, [(2, vec![1])].into());
+}
+
+#[tokio::test(flavor = "multi_thread")]
 async fn updates_a_change_when_it_is_edited_and_pushed_again() {
     let harness = TestHarness::start("alice", "widgets").await.unwrap();
     harness.write("change", "before\n").unwrap();

@slinder1
slinder1 marked this pull request as ready for review September 2, 2026 18:26
@slinder1
slinder1 merged commit 93d7686 into main Sep 2, 2026
1 check passed
@slinder1
slinder1 deleted the users/slinder1/I89bc2daaf27b219257606b9ce161df3fb23fb005 branch September 2, 2026 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant