fix: verify failed release rollback head (#10684)

* fix: verify failed release rollback head

* style: format release rollback changes

Author: chubes4

crates/homeboy-release/src/release/checkout_guard.rs

@@ -8,7 +8,9 @@ use homeboy_core::git;
 pub(super) struct CheckoutRestoreEvidence {
     pub(super) original_head: String,
     pub(super) temporary_head: String,
-    pub(super) final_head: String,
+    pub(super) final_head: Option<String>,
+    pub(super) restored: bool,
+    pub(super) error: Option<String>,
 }
 
 #[derive(Debug, Clone)]
@@ -56,31 +58,85 @@ impl ReleaseCheckoutGuard {
     }
 
     pub(super) fn restore_after_failure(&self) -> Result<CheckoutRestoreEvidence> {
+        self.restore_after_failure_with_hook(|| {})
+    }
+
+    fn restore_after_failure_with_hook(
+        &self,
+        after_capture: impl FnOnce(),
+    ) -> Result<CheckoutRestoreEvidence> {
         let temporary_head = git_stdout(&self.path, &["rev-parse", "HEAD"])?;
+        after_capture();
+        let mut errors = Vec::new();
         abort_in_progress_operations(&self.path);
-        run_git_checked(&self.path, &["reset", "--hard"])?;
-        remove_new_untracked(&self.path, &self.original_untracked)?;
+        record_cleanup_error(
+            &mut errors,
+            run_git_checked(&self.path, &["reset", "--hard"]),
+        );
+        record_cleanup_error(
+            &mut errors,
+            remove_new_untracked(&self.path, &self.original_untracked),
+        );
 
-        match &self.original_ref {
-            OriginalRef::Branch(branch) => {
-                run_git_checked(&self.path, &["checkout", "-q", branch])?
-            }
+        let checkout_result = match &self.original_ref {
+            OriginalRef::Branch(branch) => run_git_checked(&self.path, &["checkout", "-q", branch]),
             OriginalRef::Detached => {
-                run_git_checked(&self.path, &["checkout", "-q", &self.original_head])?
+                run_git_checked(&self.path, &["checkout", "-q", &self.original_head])
             }
+        };
+        record_cleanup_error(&mut errors, checkout_result);
+
+        let before_restore = git_stdout(&self.path, &["rev-parse", "HEAD"]);
+        match before_restore {
+            Ok(head) if head == self.original_head => {}
+            Ok(head) if head == temporary_head => record_cleanup_error(
+                &mut errors,
+                run_git_checked(&self.path, &["reset", "--hard", &self.original_head]),
+            ),
+            Ok(head) => errors.push(format!(
+                "checkout HEAD moved concurrently to {head}; expected release commit {temporary_head} or original HEAD {}",
+                self.original_head
+            )),
+            Err(error) => errors.push(format!("failed to inspect HEAD before restore: {error}")),
         }
 
-        run_git_checked(&self.path, &["reset", "--hard", &self.original_head])?;
-        remove_new_untracked(&self.path, &self.original_untracked)?;
-        let final_head = git_stdout(&self.path, &["rev-parse", "HEAD"])?;
+        record_cleanup_error(
+            &mut errors,
+            remove_new_untracked(&self.path, &self.original_untracked),
+        );
+        // This read is deliberately independent of the cleanup commands above.
+        // Rollback evidence must describe the checkout that actually remains.
+        let final_head = match git_stdout(&self.path, &["rev-parse", "HEAD"]) {
+            Ok(head) => Some(head),
+            Err(error) => {
+                errors.push(format!("failed to verify final HEAD: {error}"));
+                None
+            }
+        };
+        let restored = errors.is_empty() && final_head.as_deref() == Some(&self.original_head);
+        if errors.is_empty() && !restored {
+            errors.push(format!(
+                "rollback verification mismatch: expected {}, observed {}",
+                self.original_head,
+                final_head.as_deref().unwrap_or("unavailable")
+            ));
+        }
         Ok(CheckoutRestoreEvidence {
             original_head: self.original_head.clone(),
             temporary_head,
             final_head,
+            restored,
+            error: (!errors.is_empty()).then(|| errors.join("; ")),
         })
     }
 }
 
+fn record_cleanup_error(errors: &mut Vec<String>, result: Result<()>) {
+    if let Err(error) = result {
+        errors.push(error.to_string());
+    }
+}
+
 fn current_ref(path: &str) -> Result<OriginalRef> {
     let output = git_output(path, &["symbolic-ref", "--short", "HEAD"])?;
     if output.status.success() {
@@ -237,7 +293,12 @@ mod tests {
         );
         assert_eq!(rollback.original_head, original_head);
         assert_ne!(rollback.temporary_head, rollback.original_head);
-        assert_eq!(rollback.final_head, rollback.original_head);
+        assert!(rollback.restored);
+        assert_eq!(
+            rollback.final_head.as_deref(),
+            Some(rollback.original_head.as_str())
+        );
+        assert_eq!(rollback.error, None);
         assert_eq!(git_stdout_for_test(dir, &["status", "--porcelain=v1"]), "");
         assert!(!dir.join("generated.txt").exists());
         assert_eq!(
@@ -297,6 +358,46 @@ mod tests {
         assert!(err.message.contains("Uncommitted tracked changes"));
     }
 
+    #[test]
+    fn concurrent_head_movement_is_reported_without_overwriting_it() {
+        let temp = init_repo();
+        let dir = temp.path();
+        let original_head = git_stdout_for_test(dir, &["rev-parse", "HEAD"]);
+        let guard = ReleaseCheckoutGuard::capture(&component(dir))
+            .expect("capture")
+            .expect("git repo");
+
+        std::fs::write(dir.join("file.txt"), "release\n").expect("write release change");
+        run_git(dir, &["add", "."]);
+        run_git(dir, &["commit", "-q", "-m", "release: v1.0.0"]);
+        let release_commit = git_stdout_for_test(dir, &["rev-parse", "HEAD"]);
+
+        let rollback = guard
+            .restore_after_failure_with_hook(|| {
+                std::fs::write(dir.join("file.txt"), "concurrent\n")
+                    .expect("write concurrent change");
+                run_git(dir, &["add", "."]);
+                run_git(dir, &["commit", "-q", "-m", "fix: concurrent movement"]);
+            })
+            .expect("rollback evidence");
+        let concurrent_head = git_stdout_for_test(dir, &["rev-parse", "HEAD"]);
+
+        assert!(!rollback.restored);
+        assert_eq!(rollback.original_head, original_head);
+        assert_eq!(rollback.temporary_head, release_commit);
+        assert_eq!(
+            rollback.final_head.as_deref(),
+            Some(concurrent_head.as_str())
+        );
+        assert!(rollback
+            .error
+            .as_deref()
+            .unwrap()
+            .contains("moved concurrently"));
+        assert_ne!(concurrent_head, original_head);
+        assert_ne!(concurrent_head, release_commit);
+    }
+
     fn git_stdout_for_test(dir: &std::path::Path, args: &[&str]) -> String {
         let output = std::process::Command::new("git")
             .args(args)

crates/homeboy-release/src/release/orchestrator.rs

@@ -129,15 +129,40 @@ fn restore_checkout_after_failed_run(
 
     if let Some(checkout_guard) = checkout_guard {
         let evidence = checkout_guard.restore_after_failure()?;
+        let restored = evidence.restored;
+        let recovery_action =
+            (!restored).then(|| format!("homeboy release {} --apply", run.component_id));
+        let tag_state = if run.result.steps.iter().any(|step| {
+            step.step_type == "git.tag" && matches!(step.status, ReleaseStepStatus::Success)
+        }) {
+            "local_tag_created"
+        } else {
+            "not_created"
+        };
         run.result.rollback = Some(ReleaseRollbackEvidence {
+            status: if restored { "restored" } else { "interrupted" }.to_string(),
             original_head: evidence.original_head,
+            release_commit: evidence.temporary_head.clone(),
             temporary_head: evidence.temporary_head,
             final_head: evidence.final_head,
+            tag_state: tag_state.to_string(),
+            error: evidence.error,
+            recovery_action: recovery_action.clone(),
         });
         if let Some(summary) = &mut run.result.summary {
-            summary.next_actions.push(
-                "Inspect remote branch and tag state before retrying: git ls-remote --heads --tags origin"
-                    .to_string(),
+            if let Some(action) = recovery_action {
+                summary.next_actions.push(action);
+            } else {
+                summary.next_actions.push(
+                    "Inspect remote branch and tag state before retrying: git ls-remote --heads --tags origin"
+                        .to_string(),
+                );
+            }
+        }
+        if !restored {
+            run.result.status = ReleaseStepStatus::Failed;
+            run.result.warnings.push(
+                "Release rollback was interrupted; checkout recovery is still required".to_string(),
             );
         }
     }

crates/homeboy-release/src/release/types.rs

@@ -207,9 +207,19 @@ pub struct ReleaseRunResult {
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct ReleaseRollbackEvidence {
+    /// `restored` means final_head was independently observed at original_head.
+    /// `interrupted` requires operator recovery and must never be presented as
+    /// rollback success.
+    pub status: String,
     pub original_head: String,
     pub temporary_head: String,
-    pub final_head: String,
+    pub release_commit: String,
+    pub final_head: Option<String>,
+    pub tag_state: String,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub error: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub recovery_action: Option<String>,
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize)]

crates/homeboy-release/src/release/workflow.rs

@@ -612,13 +612,20 @@ fn release_summary_from_run(run: &ReleaseRun) -> Vec<String> {
     summary.push(github_release_summary_line(run));
     if let Some(rollback) = &run.result.rollback {
         summary.push(format!(
-            "Rollback evidence: original HEAD {}; temporary HEAD {}; final HEAD {}",
-            rollback.original_head, rollback.temporary_head, rollback.final_head
+            "Rollback evidence ({}): original HEAD {}; release commit {}; final HEAD {}; tag state {}",
+            rollback.status,
+            rollback.original_head,
+            rollback.release_commit,
+            rollback.final_head.as_deref().unwrap_or("unavailable"),
+            rollback.tag_state,
         ));
-        summary.push(
+        if let Some(error) = &rollback.error {
+            summary.push(format!("Rollback verification failed: {error}"));
+        }
+        summary.push(rollback.recovery_action.clone().unwrap_or_else(|| {
             "Inspect remote branch and tag state before retrying: git ls-remote --heads --tags origin"
-                .to_string(),
-        );
+                .to_string()
+        }));
     }
     summary
 }
@@ -648,9 +655,17 @@ fn git_commit_summary_line(run: &ReleaseRun) -> String {
     if step_data_bool(step, "skipped") {
         "No release commit created".to_string()
     } else if let Some(rollback) = &run.result.rollback {
+        if rollback.status != "restored" {
+            return format!(
+                "Release commit recovery interrupted: {} (actual HEAD {})",
+                rollback.release_commit,
+                rollback.final_head.as_deref().unwrap_or("unavailable")
+            );
+        }
         format!(
             "Release commit rolled back: {} (checkout restored to {})",
-            rollback.temporary_head, rollback.final_head
+            rollback.release_commit,
+            rollback.final_head.as_deref().unwrap_or("unavailable")
         )
     } else {
         "Release commit created".to_string()
@@ -1248,9 +1263,14 @@ mod tests {
                 summary: None,
                 phase_timings: None,
                 rollback: Some(ReleaseRollbackEvidence {
+                    status: "restored".to_string(),
                     original_head: "original".to_string(),
                     temporary_head: "release-commit".to_string(),
-                    final_head: "original".to_string(),
+                    release_commit: "release-commit".to_string(),
+                    final_head: Some("original".to_string()),
+                    tag_state: "not_created".to_string(),
+                    error: None,
+                    recovery_action: None,
                 }),
             },
         };
@@ -1262,14 +1282,56 @@ mod tests {
                 .to_string()
         ));
         assert!(summary.iter().any(|line| line.contains(
-            "original HEAD original; temporary HEAD release-commit; final HEAD original"
+            "original HEAD original; release commit release-commit; final HEAD original; tag state not_created"
         )));
         assert!(summary
             .iter()
             .any(|line| line.contains("git ls-remote --heads --tags origin")));
         assert!(!summary.contains(&"Release commit created".to_string()));
     }
 
+    #[test]
+    fn release_summary_never_claims_interrupted_rollback_succeeded() {
+        let run = ReleaseRun {
+            component_id: "demo".to_string(),
+            enabled: true,
+            result: ReleaseRunResult {
+                steps: vec![ReleaseStepResult {
+                    id: "git.commit".to_string(),
+                    step_type: "git.commit".to_string(),
+                    status: ReleaseStepStatus::Success,
+                    ..Default::default()
+                }],
+                status: ReleaseStepStatus::Failed,
+                warnings: vec!["Release rollback was interrupted".to_string()],
+                summary: None,
+                phase_timings: None,
+                rollback: Some(ReleaseRollbackEvidence {
+                    status: "interrupted".to_string(),
+                    original_head: "original".to_string(),
+                    temporary_head: "release-commit".to_string(),
+                    release_commit: "release-commit".to_string(),
+                    final_head: Some("concurrent-head".to_string()),
+                    tag_state: "not_created".to_string(),
+                    error: Some("checkout HEAD moved concurrently".to_string()),
+                    recovery_action: Some("homeboy release demo --apply".to_string()),
+                }),
+            },
+        };
+
+        let summary = release_summary_from_run(&run);
+
+        assert!(summary.contains(
+            &"Release commit recovery interrupted: release-commit (actual HEAD concurrent-head)"
+                .to_string()
+        ));
+        assert!(summary
+            .iter()
+            .any(|line| line.contains("Rollback evidence (interrupted)")));
+        assert!(summary.contains(&"homeboy release demo --apply".to_string()));
+        assert!(!summary.iter().any(|line| line.contains("rolled back")));
+    }
+
     #[test]
     fn release_summary_reports_canonical_published_release_url() {
         let canonical_url =