diff --git a/apps/staged/src-tauri/src/lib.rs b/apps/staged/src-tauri/src/lib.rs index f1e8af1e8..769bec219 100644 --- a/apps/staged/src-tauri/src/lib.rs +++ b/apps/staged/src-tauri/src/lib.rs @@ -2478,6 +2478,7 @@ pub fn run() { prs::commands::pull_or_queue_branch, prs::commands::rebase_branch, prs::commands::squash_commits, + prs::commands::revert_commit, prs::commands::clear_branch_pr_status, pr_detection::recover_branch_pr, pr_detection::detect_branch_pr, diff --git a/apps/staged/src-tauri/src/prs/commands.rs b/apps/staged/src-tauri/src/prs/commands.rs index 0edee85a7..71dea5d25 100644 --- a/apps/staged/src-tauri/src/prs/commands.rs +++ b/apps/staged/src-tauri/src/prs/commands.rs @@ -25,6 +25,7 @@ use super::pull::pull_or_queue_branch_for_branch; use super::push::{ has_unpushed_commits_impl, start_or_queue_push_pipeline_for_branch, PushVariant, }; +use super::revert::revert_commit_for_branch; fn get_store(store: &tauri::State<'_, Mutex>>>) -> Result, String> { store @@ -297,3 +298,24 @@ pub async fn squash_commits( ) .await } + +/// Revert a completed commit by creating a new signed-off revert commit. +#[tauri::command(rename_all = "camelCase")] +pub async fn revert_commit( + store: tauri::State<'_, Mutex>>>, + registry: tauri::State<'_, Arc>, + app_handle: tauri::AppHandle, + branch_id: String, + commit_sha: String, + provider: Option, +) -> Result { + let store = get_store(&store)?; + revert_commit_for_branch( + store, + Arc::clone(®istry), + app_handle, + branch_id, + commit_sha, + provider, + ) +} diff --git a/apps/staged/src-tauri/src/prs/commit.rs b/apps/staged/src-tauri/src/prs/commit.rs index b566019bc..204fbe274 100644 --- a/apps/staged/src-tauri/src/prs/commit.rs +++ b/apps/staged/src-tauri/src/prs/commit.rs @@ -191,9 +191,32 @@ pub(super) fn insert_running_commit_pipeline_session( if let Some(target) = rebase_target { pipeline = pipeline.with_rebase_target(target); } + insert_running_pending_commit_session( + store, + registry, + ctx, + pipeline, + prompt.to_string(), + provider, + false, + ) +} +/// The rows every commit-producing run-now pipeline inserts: a `running` +/// session on the branch, under a liveness hold taken before the write, and +/// the pending commit it will fill in. Same locking contract as +/// [`insert_running_commit_pipeline_session`]. +pub(super) fn insert_running_pending_commit_session( + store: &Arc, + registry: &Arc, + ctx: &BranchPipelineContext, + pipeline: PipelineExecution, + prompt: String, + provider: Option<&str>, + capture_pre_head: bool, +) -> Result { let mut session = - store::Session::new_running(prompt, &ctx.working_dir).with_branch(&ctx.branch.id); + store::Session::new_running(&prompt, &ctx.working_dir).with_branch(&ctx.branch.id); if let Some(p) = provider { session = session.with_provider(p); } @@ -208,6 +231,7 @@ pub(super) fn insert_running_commit_pipeline_session( session_id: session.id, pipeline, prompt, + capture_pre_head, _liveness: liveness, }) } @@ -580,6 +604,7 @@ pub(crate) async fn start_queued_commit_pipeline_for_branch( pipeline, working_dir: ctx.working_dir, pre_head_sha: None, + capture_pre_head: false, provider: effective_provider, workspace_name: ctx.workspace_name, remote_working_dir: ctx.remote_working_dir, diff --git a/apps/staged/src-tauri/src/prs/context.rs b/apps/staged/src-tauri/src/prs/context.rs index 2cbd9f096..540c86e26 100644 --- a/apps/staged/src-tauri/src/prs/context.rs +++ b/apps/staged/src-tauri/src/prs/context.rs @@ -34,7 +34,8 @@ pub(crate) struct BranchPipelineContext { /// Resolve branch, project, and working directory for a pipeline command. /// -/// All pipeline commands (create_pr, push_branch, rebase_branch, squash_commits) +/// All pipeline commands (create_pr, push_branch, rebase_branch, squash_commits, +/// revert_commit) /// share the same setup: look up the branch and project, resolve the working /// directory (local worktree vs remote clone), and compute the remote working /// directory for workspace-based branches. diff --git a/apps/staged/src-tauri/src/prs/git_pipeline.rs b/apps/staged/src-tauri/src/prs/git_pipeline.rs index d0ed842d6..6f4cbf06a 100644 --- a/apps/staged/src-tauri/src/prs/git_pipeline.rs +++ b/apps/staged/src-tauri/src/prs/git_pipeline.rs @@ -159,6 +159,7 @@ pub(crate) async fn start_queued_git_pipeline_for_branch( pipeline, working_dir: ctx.working_dir, pre_head_sha: None, + capture_pre_head: false, provider: effective_provider, workspace_name: ctx.workspace_name, remote_working_dir: ctx.remote_working_dir, diff --git a/apps/staged/src-tauri/src/prs/mod.rs b/apps/staged/src-tauri/src/prs/mod.rs index a84cc8eea..ead49054f 100644 --- a/apps/staged/src-tauri/src/prs/mod.rs +++ b/apps/staged/src-tauri/src/prs/mod.rs @@ -18,6 +18,8 @@ //! and the force push "Rebase and force push" queues behind its rebase. //! - [`push`] is the push pipeline and its variants ([`PushVariant`]). //! - [`pull`] fast-forwards the branch to origin, now or queued. +//! - [`revert`] reverts a completed commit with a new signed-off commit, run +//! now and refused while the branch is busy. //! - [`git_pipeline`] rebuilds a queued push or pull when it is dequeued. //! - [`deferred`] is "Push after current work": a push armed on the branch //! row that fires once the branch and its project are idle. @@ -37,6 +39,7 @@ mod pipeline; mod pr_status; mod pull; mod push; +mod revert; #[cfg(test)] mod tests; @@ -62,3 +65,4 @@ pub(crate) use push::build_push_pipeline_steps; pub(crate) use push::{ has_unpushed_commits_impl, start_or_queue_push_pipeline_for_branch, PushVariant, }; +pub(crate) use revert::revert_commit_for_branch; diff --git a/apps/staged/src-tauri/src/prs/pipeline.rs b/apps/staged/src-tauri/src/prs/pipeline.rs index ec9942fe3..9d9cdea42 100644 --- a/apps/staged/src-tauri/src/prs/pipeline.rs +++ b/apps/staged/src-tauri/src/prs/pipeline.rs @@ -106,6 +106,7 @@ pub(super) fn start_pipeline_for_branch( pipeline, working_dir: ctx.working_dir, pre_head_sha: None, + capture_pre_head: false, provider, workspace_name: ctx.workspace_name, remote_working_dir: ctx.remote_working_dir, @@ -185,6 +186,12 @@ pub(crate) enum BranchWorkInFlight { /// What the branch is busy with, if anything. /// +/// A queued row that no longer resolves to anything (`BranchSessionLink::Unlinked`: +/// its commit, note or review was deleted out from under it) does not count, +/// the same as for a session start. The drain steps over it, so a row queued +/// behind it alone would wait for a kick that never comes, and a revert, which +/// refuses rather than queues, would stay refused on the branch for good. +/// /// Callers must hold the branch launch lock: the answer is only meaningful for /// as long as no session can start or be enqueued underneath them. pub(crate) fn branch_work_in_flight( @@ -197,11 +204,7 @@ pub(crate) fn branch_work_in_flight( { return Ok(BranchWorkInFlight::Running); } - if !store - .get_queued_sessions_for_branch(branch_id) - .map_err(|e| e.to_string())? - .is_empty() - { + if crate::session_commands::has_queued_branch_session(store, branch_id)? { return Ok(BranchWorkInFlight::Queued); } Ok(BranchWorkInFlight::None) @@ -228,7 +231,11 @@ pub(super) fn branch_has_work_in_flight( pub(super) struct RunningPipelineSession { pub(super) session_id: String, pub(super) pipeline: PipelineExecution, - pub(super) prompt: &'static str, + pub(super) prompt: String, + /// Passed on as [`session_runner::PipelineConfig::capture_pre_head`]: set + /// for a revert, whose pipeline has no kind that would make the runner + /// read HEAD at start the way it does for a rebase. + pub(super) capture_pre_head: bool, pub(super) _liveness: session_runner::LivenessHold, } @@ -262,16 +269,18 @@ pub(super) fn launch_running_pipeline_session( session_id, pipeline, prompt, + capture_pre_head, _liveness: liveness, } = running; let started = session_runner::start_pipeline_session( session_runner::PipelineConfig { session_id: session_id.clone(), - prompt: prompt.to_string(), + prompt, steps, pipeline, working_dir: ctx.working_dir, pre_head_sha: None, + capture_pre_head, provider, workspace_name: ctx.workspace_name, remote_working_dir: ctx.remote_working_dir, diff --git a/apps/staged/src-tauri/src/prs/push.rs b/apps/staged/src-tauri/src/prs/push.rs index e71cb9b66..809a2d021 100644 --- a/apps/staged/src-tauri/src/prs/push.rs +++ b/apps/staged/src-tauri/src/prs/push.rs @@ -152,7 +152,8 @@ pub(super) fn insert_running_push_pipeline_session( Ok(RunningPipelineSession { session_id: session.id, pipeline, - prompt, + prompt: prompt.to_string(), + capture_pre_head: false, _liveness: liveness, }) } diff --git a/apps/staged/src-tauri/src/prs/revert.rs b/apps/staged/src-tauri/src/prs/revert.rs new file mode 100644 index 000000000..679672b6b --- /dev/null +++ b/apps/staged/src-tauri/src/prs/revert.rs @@ -0,0 +1,156 @@ +//! Revert: a run-now pipeline that creates a signed-off revert commit for a +//! completed commit on the branch. +//! +//! Unlike rebase and squash it never queues: it is refused while the branch +//! has work in flight, and runs `git revert --signoff --no-edit ` with an +//! AI handoff only if git stops on a conflict. A pending-commit stub is linked +//! to the session so completion can record the revert commit's SHA. + +use std::sync::Arc; + +use crate::session_runner; +use crate::store::{FailureStrategy, PipelineExecution, PipelineStep, Store}; + +use super::commit::insert_running_pending_commit_session; +use super::context::{resolve_branch_pipeline_context, BranchPipelineContext}; +use super::pipeline::{ + branch_has_work_in_flight, launch_running_pipeline_session, RunningPipelineSession, +}; + +pub(super) fn validate_commit_sha(commit_sha: &str) -> Result { + let sha = commit_sha.trim(); + if (7..=64).contains(&sha.len()) && sha.chars().all(|c| c.is_ascii_hexdigit()) { + Ok(sha.to_string()) + } else { + Err("Commit SHA must be 7-64 hexadecimal characters".to_string()) + } +} + +fn revert_handoff_prompt(commit_sha: &str) -> String { + format!( + r#" +`git revert --signoff --no-edit {commit_sha}` failed. + +Inspect the git state and command output, then complete the revert safely. + +Requirements: +- Resolve conflicts if present, preserving unrelated local changes. +- If git is in a revert sequence and the resolution is correct, run `git revert --continue`. +- Use the user's global git identity for author and committer. +- Ensure the resulting revert commit is signed off. +- Do not push the branch. +- If the revert should not continue, explain why and leave the repository in a clear state. + +Context from prior steps: + +{{step_outputs}} +"# + ) +} + +pub(super) fn build_revert_pipeline_steps(commit_sha: &str) -> Result, String> { + let commit_sha = validate_commit_sha(commit_sha)?; + Ok(vec![PipelineStep::Command { + label: "Revert commit".to_string(), + command: format!("git revert --signoff --no-edit {commit_sha}"), + on_failure: FailureStrategy::HandoffToAi { + prompt_template: revert_handoff_prompt(&commit_sha), + }, + }]) +} + +pub(super) const REVERT_BRANCH_BUSY: &str = + "Revert is unavailable while this branch has queued or running work"; +pub(super) const REVERT_WORKTREE_BUSY: &str = + "Revert is unavailable while this branch is being reset or its changes discarded"; + +/// Insert the rows for a revert that runs now, or refuse it on a busy branch. +/// +/// A revert is never queued: a queued row would have to carry the target SHA +/// for the drain to rebuild its step from. So where a rebase or squash would +/// queue behind work in flight, this errors — under the branch launch lock, +/// with the insert in the same critical section, so two rapid clicks (or a +/// click racing a session start) cannot both observe an idle branch. This +/// stays synchronous on purpose: the lock must not be held across an await. +/// +/// A reset-to-origin or discard has no session row for that check to find, +/// so the marker they set is read here as well, the way the forced start reads +/// it: a revert started under one would race its `git reset --hard`, and if it +/// lost on `index.lock` the AI handoff would be asked to finish reverting a +/// commit the reset may have just removed from the branch. +/// +/// HEAD is not read here. The runner reads it before the revert step (see +/// [`session_runner::PipelineConfig::capture_pre_head`]), by which point this +/// row has made the branch busy, so nothing that goes through the branch queue +/// can move HEAD in between. Reset-to-origin, discard and delete-commit run +/// outside the queue: the marker above only stops a revert starting under a +/// reset or discard, only the frontend's busy-branch gates keep those two from +/// starting beside a running revert, and nothing gates a delete-commit then. +pub(super) fn insert_running_revert_pipeline_session_if_idle( + store: &Arc, + registry: &Arc, + ctx: &BranchPipelineContext, + steps: &[PipelineStep], + commit_sha: &str, + provider: Option<&str>, +) -> Result { + let launch_lock = crate::session_commands::branch_session_launch_lock_for(&ctx.branch.id); + let _guard = launch_lock.lock().unwrap(); + if crate::session_commands::branch_worktree_operation_in_flight(&ctx.branch.id) { + return Err(REVERT_WORKTREE_BUSY.to_string()); + } + if branch_has_work_in_flight(store, &ctx.branch.id)? { + return Err(REVERT_BRANCH_BUSY.to_string()); + } + + let prompt = format!("Revert commit {}", &commit_sha[..7.min(commit_sha.len())]); + insert_running_pending_commit_session( + store, + registry, + ctx, + PipelineExecution::from_steps(steps), + prompt, + provider, + true, + ) +} + +/// Revert a completed commit by creating a new signed-off revert commit. +/// +/// Runs `git revert --signoff --no-edit ` directly. If git needs conflict +/// resolution, the pipeline hands off to AI with the pending commit row still +/// linked so completion can record the final HEAD. +/// +/// Shared by the Tauri command and the web dispatch arm so both gates behave +/// identically. +pub(crate) fn revert_commit_for_branch( + store: Arc, + registry: Arc, + app_handle: tauri::AppHandle, + branch_id: String, + commit_sha: String, + provider: Option, +) -> Result { + let commit_sha = validate_commit_sha(&commit_sha)?; + let steps = build_revert_pipeline_steps(&commit_sha)?; + let ctx = resolve_branch_pipeline_context(&store, &branch_id)?; + + let running = insert_running_revert_pipeline_session_if_idle( + &store, + ®istry, + &ctx, + &steps, + &commit_sha, + provider.as_deref(), + )?; + launch_running_pipeline_session( + ctx, + running, + steps, + "commit", + provider, + store, + &app_handle, + ®istry, + ) +} diff --git a/apps/staged/src-tauri/src/prs/tests.rs b/apps/staged/src-tauri/src/prs/tests.rs index 3fb235dfb..a897a373d 100644 --- a/apps/staged/src-tauri/src/prs/tests.rs +++ b/apps/staged/src-tauri/src/prs/tests.rs @@ -17,8 +17,8 @@ use super::deferred::{ }; use super::git_pipeline::rebuild_queued_git_pipeline; use super::pipeline::{ - pipeline_prompt, BranchPipelineResponse, FORCE_PUSH_PROMPT, NO_VERIFY_FORCE_PUSH_PROMPT, - NO_VERIFY_PUSH_PROMPT, PULL_PROMPT, PUSH_PROMPT, + branch_has_work_in_flight, pipeline_prompt, BranchPipelineResponse, FORCE_PUSH_PROMPT, + NO_VERIFY_FORCE_PUSH_PROMPT, NO_VERIFY_PUSH_PROMPT, PULL_PROMPT, PUSH_PROMPT, }; use super::pr_status::{collect_branch_refresh_results, BranchRefresh}; use super::pull::{ @@ -29,6 +29,10 @@ use super::push::{ build_push_pipeline_steps, insert_running_push_pipeline_session, queue_push_pipeline_if_branch_busy, queue_push_pipeline_locked, PushVariant, }; +use super::revert::{ + build_revert_pipeline_steps, insert_running_revert_pipeline_session_if_idle, + REVERT_BRANCH_BUSY, REVERT_WORKTREE_BUSY, +}; use crate::git; use crate::session_commands::BranchSessionLaunchStatus; use crate::session_runner; @@ -43,6 +47,7 @@ mod git_pipeline; mod pr_status; mod pull; mod push; +mod revert; fn command_at(steps: &[PipelineStep], index: usize) -> (&str, &str, &FailureStrategy) { match &steps[index] { diff --git a/apps/staged/src-tauri/src/prs/tests/revert.rs b/apps/staged/src-tauri/src/prs/tests/revert.rs new file mode 100644 index 000000000..93c3338f8 --- /dev/null +++ b/apps/staged/src-tauri/src/prs/tests/revert.rs @@ -0,0 +1,165 @@ +//! Revert: the pipeline step, its handoff prompt and SHA validation. + +use super::*; + +#[test] +fn revert_pipeline_uses_signoff_and_hands_off_to_ai() { + let steps = build_revert_pipeline_steps("abc1234").unwrap(); + + assert_eq!(steps.len(), 1); + let (label, command, on_failure) = command_at(&steps, 0); + assert_eq!(label, "Revert commit"); + assert_eq!(command, "git revert --signoff --no-edit abc1234"); + + let FailureStrategy::HandoffToAi { prompt_template } = on_failure else { + panic!("expected revert failure to hand off to AI"); + }; + assert!(prompt_template.contains("git revert --continue")); + assert!(prompt_template.contains("preserving unrelated local changes")); + assert!(prompt_template.contains("Do not push the branch")); + assert!(prompt_template.contains("{step_outputs}")); +} + +#[test] +fn revert_pipeline_rejects_unsafe_commit_sha() { + assert!(build_revert_pipeline_steps("abc1234").is_ok()); + assert!(build_revert_pipeline_steps("abcdef").is_err()); + assert!(build_revert_pipeline_steps("abc1234; rm -rf /").is_err()); + assert!(build_revert_pipeline_steps("not-a-sha").is_err()); +} + +#[test] +fn inserting_a_running_revert_is_what_makes_the_branch_busy() { + let (store, branch) = setup_branch_store(); + let registry = test_registry(); + let ctx = pipeline_context(&branch); + let steps = build_revert_pipeline_steps("abc1234def").unwrap(); + + let running = insert_running_revert_pipeline_session_if_idle( + &store, + ®istry, + &ctx, + &steps, + "abc1234def", + Some("claude"), + ) + .unwrap(); + + // The runner, not the command, reads the pre-revert HEAD. + assert!(running.capture_pre_head); + assert_eq!(running.prompt, "Revert commit abc1234"); + assert!(registry.is_live(&running.session_id)); + let session = store.get_session(&running.session_id).unwrap().unwrap(); + assert_eq!(session.status, store::SessionStatus::Running); + assert_eq!(session.branch_id.as_deref(), Some(branch.id.as_str())); + assert_eq!(session.provider.as_deref(), Some("claude")); + assert_eq!(session.pipeline.unwrap().kind, None); + let pending = store + .get_commit_by_session(&running.session_id) + .unwrap() + .unwrap(); + assert_eq!(pending.sha, None); + + // A second revert racing the first is refused rather than run, and + // the queueable actions queue behind it. + let err = insert_running_revert_pipeline_session_if_idle( + &store, + ®istry, + &ctx, + &steps, + "abc1234def", + None, + ) + .err() + .unwrap(); + assert_eq!(err, REVERT_BRANCH_BUSY); + assert!( + queue_commit_pipeline_locked(&store, &branch.id, &PipelineKind::Rebase, None, None) + .unwrap() + .is_some() + ); + assert!(queue_push(&store, &branch.id, PUSH).is_some()); +} + +#[test] +fn revert_on_a_busy_branch_inserts_nothing() { + let (store, branch) = setup_branch_store(); + start_running_note_session(&store, &branch.id); + let steps = build_revert_pipeline_steps("abc1234").unwrap(); + + let err = insert_running_revert_pipeline_session_if_idle( + &store, + &test_registry(), + &pipeline_context(&branch), + &steps, + "abc1234", + None, + ) + .err() + .unwrap(); + + assert_eq!(err, REVERT_BRANCH_BUSY); + let running = store.get_running_sessions_for_branch(&branch.id).unwrap(); + assert_eq!(running.len(), 1, "only the note session is running"); + assert!(store + .list_commits_for_branch(&branch.id) + .unwrap() + .is_empty()); +} + +/// Reset-to-origin and discard leave no session row, so the marker they +/// set is what refuses a revert started while one is moving HEAD. +#[test] +fn revert_during_a_worktree_operation_inserts_nothing() { + let (store, branch) = setup_branch_store(); + let registry = test_registry(); + let ctx = pipeline_context(&branch); + let steps = build_revert_pipeline_steps("abc1234").unwrap(); + let revert = || { + insert_running_revert_pipeline_session_if_idle( + &store, ®istry, &ctx, &steps, "abc1234", None, + ) + }; + + let operation = crate::session_commands::BranchWorktreeOperationGuard::acquire(&branch.id); + assert_eq!(revert().err().unwrap(), REVERT_WORKTREE_BUSY); + assert!(store + .get_running_sessions_for_branch(&branch.id) + .unwrap() + .is_empty()); + assert!(store + .list_commits_for_branch(&branch.id) + .unwrap() + .is_empty()); + + drop(operation); + assert!(revert().is_ok()); +} + +/// A queued row whose artifact was deleted is stepped over by the drain, +/// so it must not count as work in flight: a revert would be refused on +/// the branch for good, and a rebase queued behind it alone would never +/// be drained. +#[test] +fn an_unlinked_queued_row_leaves_the_branch_idle() { + let (store, branch) = setup_branch_store(); + let orphan = store::Session::new_queued("deleted note").with_branch(&branch.id); + store.create_session(&orphan).unwrap(); + + assert!(!branch_has_work_in_flight(&store, &branch.id).unwrap()); + assert_eq!( + queue_commit_pipeline_locked(&store, &branch.id, &PipelineKind::Rebase, None, None) + .unwrap(), + None + ); + let steps = build_revert_pipeline_steps("abc1234").unwrap(); + assert!(insert_running_revert_pipeline_session_if_idle( + &store, + &test_registry(), + &pipeline_context(&branch), + &steps, + "abc1234", + None, + ) + .is_ok()); +} diff --git a/apps/staged/src-tauri/src/session_commands.rs b/apps/staged/src-tauri/src/session_commands.rs index 20f4937cb..cf6168b3e 100644 --- a/apps/staged/src-tauri/src/session_commands.rs +++ b/apps/staged/src-tauri/src/session_commands.rs @@ -4427,7 +4427,7 @@ pub(crate) fn branch_worktree_operation_in_flight(branch_id: &str) -> bool { .contains_key(branch_id) } -fn has_queued_branch_session(store: &Store, branch_id: &str) -> Result { +pub(crate) fn has_queued_branch_session(store: &Store, branch_id: &str) -> Result { let queued = store .get_queued_sessions_for_branch(branch_id) .map_err(|e| e.to_string())?; diff --git a/apps/staged/src-tauri/src/session_runner.rs b/apps/staged/src-tauri/src/session_runner.rs index 0980f060e..42df0cf1b 100644 --- a/apps/staged/src-tauri/src/session_runner.rs +++ b/apps/staged/src-tauri/src/session_runner.rs @@ -2974,6 +2974,21 @@ pub struct PipelineConfig { /// HEAD before the deterministic pipeline began. Used by rebase pipelines /// when they hand off to AI after conflicts. pub pre_head_sha: Option, + /// Read `pre_head_sha` before the first step, the way a rebase always + /// does, for a pipeline whose kind doesn't say so (a revert). Its + /// completion is judged by whether HEAD moved, so the read has to come + /// after its row already makes the branch busy, and through the same + /// runner as the read it is compared against. + /// + /// Setting it also promises that the pipeline hands off to AI only from a + /// failed step, which leaves HEAD where it was, and that a completed run + /// moved HEAD — both true of a revert's one `git revert` step, which exits + /// non-zero only before it commits. So if that read fails, the HEAD read + /// at handoff stands in for the pre-pipeline HEAD (see + /// `pre_head_for_pipeline_handoff`), and on completion HEAD is claimed + /// without a comparison (see `finalize_head_moving_commit_pipeline_without_ai`), + /// rather than the pending row losing its commit either way. + pub capture_pre_head: bool, /// ACP provider ID (e.g. "goose", "claude"). pub provider: Option, /// Workspace name for remote branches. @@ -3455,18 +3470,34 @@ fn aborted_pipeline_error( } fn pre_head_for_pipeline_handoff(config: &PipelineConfig) -> Option { - match config.pipeline.kind.as_ref() { - Some(PipelineKind::Rebase) => config.pre_head_sha.clone(), - Some(PipelineKind::Squash) => match current_pipeline_head(config) { - Ok(head) => Some(head), - Err(e) => { - log::warn!("Failed to capture squash handoff HEAD: {e}"); - None - } - }, - // A push never rewrites local history and a pull never hands off, so - // neither has a pre-pipeline HEAD to compare against. - Some(PipelineKind::Push | PipelineKind::Pull) | None => None, + if config.pre_head_sha.is_some() { + return config.pre_head_sha.clone(); + } + + let read_head_at_handoff = match config.pipeline.kind.as_ref() { + Some(PipelineKind::Squash) => true, + // A pipeline that asked for its HEAD up front (a revert) only gets here + // if that read failed. It hands off only from a failed step, which + // left HEAD where it started (see `PipelineConfig::capture_pre_head`), + // so HEAD now is the HEAD that read would have returned. + None => config.capture_pre_head, + // A rebase only gets here when its up-front read failed, and it must + // not read HEAD now: it may be detached on a partially applied commit, + // and nothing may claim that SHA. A push never rewrites local history + // and a pull never hands off, so neither has a pre-pipeline HEAD to + // compare against. + Some(PipelineKind::Rebase | PipelineKind::Push | PipelineKind::Pull) => false, + }; + if !read_head_at_handoff { + return None; + } + + match current_pipeline_head(config) { + Ok(head) => Some(head), + Err(e) => { + log::warn!("Failed to capture pipeline handoff HEAD: {e}"); + None + } } } @@ -3550,36 +3581,37 @@ fn reassociate_rebased_commits( } fn resolve_pipeline_artifacts_without_ai(config: &PipelineConfig, store: &Store, completed: bool) { - match config.pipeline.kind.as_ref() { - Some(PipelineKind::Rebase) if completed => { - finalize_rebase_pipeline_without_ai(config, store); - } - Some(PipelineKind::Rebase | PipelineKind::Squash) => { - if let Err(e) = store.delete_pending_commit_for_session(&config.session_id) { - log::warn!( - "Failed to resolve pending commit for terminal pipeline session {}: {e}", - config.session_id - ); - } - } - // Push and pull pipelines create no artifact, so there is nothing to - // resolve. - Some(PipelineKind::Push | PipelineKind::Pull) | None => {} + // A run that didn't complete, or completed with no pre-pipeline HEAD to + // tell a no-op from a commit by, has nothing to claim. Its pending row + // goes; a row that already landed stays, since the delete only matches + // `sha IS NULL`. + if completed && (config.pre_head_sha.is_some() || config.capture_pre_head) { + finalize_head_moving_commit_pipeline_without_ai(config, store); + } else if let Err(e) = store.delete_pending_commit_for_session(&config.session_id) { + log::warn!( + "Failed to resolve pending commit for terminal pipeline session {}: {e}", + config.session_id + ); } } -fn finalize_rebase_pipeline_without_ai(config: &PipelineConfig, store: &Store) { - let Some(pre_head_sha) = config.pre_head_sha.as_deref() else { - let _ = store.delete_pending_commit_for_session(&config.session_id); - return; - }; +fn finalize_head_moving_commit_pipeline_without_ai(config: &PipelineConfig, store: &Store) { + // A pipeline with no pre-pipeline HEAD only gets here if it asked for one + // up front: that promises a completed run moved HEAD (see + // `PipelineConfig::capture_pre_head`), so HEAD is its commit even when the + // read it would be compared against failed. + let pre_head_sha = config.pre_head_sha.as_deref(); + debug_assert!( + pre_head_sha.is_some() || config.capture_pre_head, + "no pre-pipeline HEAD to tell a no-op from a commit by" + ); let commit = match store.get_commit_by_session(&config.session_id) { Ok(Some(commit)) => commit, Ok(None) => return, Err(e) => { log::warn!( - "Failed to load pending commit for rebase pipeline session {}: {e}", + "Failed to load pending commit for pipeline session {}: {e}", config.session_id ); return; @@ -3587,57 +3619,62 @@ fn finalize_rebase_pipeline_without_ai(config: &PipelineConfig, store: &Store) { }; // One runner for both reads below, so the HEAD this claims and the commits - // reassociation lists can't come from different checkouts. + // a rebase's reassociation lists can't come from different checkouts. let git = pipeline_git_runner(config); let current_head = match crate::branches::head_sha(&git) { Ok(head) => head, Err(e) => { log::error!( - "Failed to get HEAD after rebase pipeline session {}: {e}", + "Failed to get HEAD after pipeline session {}: {e}", config.session_id ); return; } }; - if current_head == pre_head_sha { + if pre_head_sha == Some(current_head.as_str()) { if let Err(e) = store.delete_pending_commit_for_session(&config.session_id) { log::warn!( - "Failed to remove no-op rebase pending commit for session {}: {e}", + "Failed to remove no-op pending commit for session {}: {e}", config.session_id ); } return; } - // The rebase rewrote every SHA on the branch, orphaning the metadata rows + // A rebase rewrote every SHA on the branch, orphaning the metadata rows // that pointed at the old ones. Reattach them *before* claiming the new // HEAD: once the pre-existing head row owns it, `complete_pending_commit_sha` // takes its "target SHA already owned" branch and drops this pipeline's // pending row — so the top commit keeps its authoring session instead of // the mechanical "Rebase branch" one. A head commit authored outside // Staged has no prior row, so the rebase session keeps it, as before. + // Any other HEAD-moving pipeline (a revert) only adds a commit on top, + // which orphans nothing, so it skips the history walk — and on a remote + // branch the workspace round trip — the way the AI handoff path does. // // This path runs when the pipeline drove the rebase to completion itself, // and its own HEAD-moved check above already established there's something // to claim, so the returned outcome isn't a gate here: a `MidRebase` answer // (an unattached HEAD, or a branch that wouldn't resolve) just means the // rows stay orphaned while the claim below proceeds as it always has. - let _ = reassociate_rebased_commits(store, &commit.branch_id, &git); + if config.pipeline.kind == Some(PipelineKind::Rebase) { + let _ = reassociate_rebased_commits(store, &commit.branch_id, &git); + } match store.complete_pending_commit_sha(&commit.id, &commit.branch_id, ¤t_head) { Ok(true) => log::info!( - "Rebase pipeline session {} updated pending commit to {}", + "Pipeline session {} updated pending commit to {}", config.session_id, ¤t_head[..7.min(current_head.len())] ), Ok(false) => log::info!( - "Rebase pipeline session {} resolved duplicate commit SHA {}", + "Pipeline session {} resolved duplicate commit SHA {}", config.session_id, ¤t_head[..7.min(current_head.len())] ), Err(e) => log::error!( - "Failed to complete rebase pending commit for session {}: {e}", + "Failed to complete pending commit for session {}: {e}", config.session_id ), } @@ -3705,6 +3742,23 @@ pub(crate) fn drain_queued_after_terminal_state( }); } +/// Capture HEAD before the first step for rebase pipelines and any other that +/// asks (see [`PipelineConfig::capture_pre_head`]). This is deferred from +/// session creation so the (potentially slow) remote HEAD lookup doesn't block +/// session visibility in the UI. +fn capture_pipeline_pre_head(config: &mut PipelineConfig) { + if config.pre_head_sha.is_some() + || !(config.capture_pre_head + || config.pipeline.kind.as_ref() == Some(&PipelineKind::Rebase)) + { + return; + } + match current_pipeline_head(config) { + Ok(head) => config.pre_head_sha = Some(head), + Err(e) => log::warn!("Failed to capture pre-pipeline HEAD: {e}"), + } +} + /// Execute pipeline steps sequentially, emitting events as each step progresses. async fn run_pipeline( config: &mut PipelineConfig, @@ -3712,16 +3766,7 @@ async fn run_pipeline( app_handle: &AppHandle, cancel_token: &CancellationToken, ) -> PipelineOutcome { - // Capture HEAD before the first step for rebase pipelines. This is deferred - // from session creation so the (potentially slow) remote HEAD lookup doesn't - // block session visibility in the UI. - if config.pre_head_sha.is_none() && config.pipeline.kind.as_ref() == Some(&PipelineKind::Rebase) - { - match current_pipeline_head(config) { - Ok(head) => config.pre_head_sha = Some(head), - Err(e) => log::warn!("Failed to capture pre-rebase HEAD: {e}"), - } - } + capture_pipeline_pre_head(config); // Use the pipeline execution state that was already persisted with the // session, rather than reconstructing from step definitions. This keeps @@ -5802,7 +5847,7 @@ fn detect_session_commit( Ok(current_head) if current_head != pre_sha => { // A rebase pipeline that handed off to AI (conflicts, a // failed fetch) lands here instead of - // `finalize_rebase_pipeline_without_ai`. If its turn ended + // `finalize_head_moving_commit_pipeline_without_ai`. If its turn ended // with the rebase still stopped on a conflict, HEAD is // detached on a partially applied commit — a SHA `git // rebase --abort` erases — so nothing may claim it: not @@ -5822,7 +5867,7 @@ fn detect_session_commit( // before it repoints anything, so it reports which way it // went. That also keeps the load-bearing ordering — reattach // the orphaned rows before the pending row below claims the - // new HEAD, see `finalize_rebase_pipeline_without_ai` for + // new HEAD, see `finalize_head_moving_commit_pipeline_without_ai` for // why — true by construction. let rebase_pipeline = session_is_rebase_pipeline(store, session_id); let mid_rebase = rebase_pipeline @@ -7975,6 +8020,7 @@ mod tests { pipeline, working_dir: PathBuf::from("/tmp"), pre_head_sha: None, + capture_pre_head: false, provider: None, workspace_name: None, remote_working_dir: None, @@ -9270,13 +9316,28 @@ mod tests { repo: &std::path::Path, pre_head: &str, ) -> PipelineConfig { + head_moving_pipeline_config(session_id, repo, pre_head, Some(PipelineKind::Rebase)) + } + + fn head_moving_pipeline_config( + session_id: &str, + repo: &std::path::Path, + pre_head: &str, + kind: Option, + ) -> PipelineConfig { + let mut pipeline = PipelineExecution::from_steps(&[]); + if let Some(kind) = kind { + pipeline = pipeline.with_kind(kind); + } + PipelineConfig { session_id: session_id.to_string(), - prompt: "Rebase branch".to_string(), + prompt: "Commit-producing pipeline".to_string(), steps: vec![], - pipeline: PipelineExecution::from_steps(&[]).with_kind(PipelineKind::Rebase), + pipeline, working_dir: repo.to_path_buf(), pre_head_sha: Some(pre_head.to_string()), + capture_pre_head: false, provider: None, workspace_name: None, remote_working_dir: None, @@ -9305,7 +9366,7 @@ mod tests { run_git(&repo, &["commit", "-m", "second"]); let new_head = run_git(&repo, &["rev-parse", "HEAD"]); - finalize_rebase_pipeline_without_ai( + finalize_head_moving_commit_pipeline_without_ai( &rebase_pipeline_config(&session.id, &repo, &pre_head), &store, ); @@ -9339,6 +9400,7 @@ mod tests { pipeline, working_dir: PathBuf::from("/tmp"), pre_head_sha: None, + capture_pre_head: false, provider: None, workspace_name: None, remote_working_dir: None, @@ -9418,7 +9480,7 @@ mod tests { let commit = crate::store::Commit::new_pending(&branch.id).with_session(&session.id); store.create_commit(&commit).unwrap(); - finalize_rebase_pipeline_without_ai( + finalize_head_moving_commit_pipeline_without_ai( &rebase_pipeline_config(&session.id, &repo, &pre_head), &store, ); @@ -9541,7 +9603,7 @@ mod tests { fn rebase_pipeline_completion_reassociates_pre_existing_commits() { let fixture = rebased_branch(); - finalize_rebase_pipeline_without_ai( + finalize_head_moving_commit_pipeline_without_ai( &rebase_pipeline_config( &fixture.rebase_session_id, fixture.repo.path(), @@ -9821,6 +9883,212 @@ mod tests { ); } + #[test] + fn head_moving_pipeline_completion_updates_pending_commit_without_kind() { + let repo = make_git_repo("generic-head-moving-updates-pending"); + let pre_head = run_git(&repo, &["rev-parse", "HEAD"]); + let store = Store::in_memory().unwrap(); + let project = crate::store::Project::new("test-owner/test-repo"); + store.create_project(&project).unwrap(); + let branch = crate::store::Branch::new(&project.id, "feature", "main"); + store.create_branch(&branch).unwrap(); + let mut session = crate::store::Session::new_running("Revert commit", &repo); + session.pipeline = Some(PipelineExecution::from_steps(&[])); + store.create_session(&session).unwrap(); + let commit = crate::store::Commit::new_pending(&branch.id).with_session(&session.id); + store.create_commit(&commit).unwrap(); + + std::fs::write(repo.join("file.txt"), "two\n").unwrap(); + run_git(&repo, &["add", "file.txt"]); + run_git(&repo, &["commit", "-m", "second"]); + let new_head = run_git(&repo, &["rev-parse", "HEAD"]); + + finalize_head_moving_commit_pipeline_without_ai( + &head_moving_pipeline_config(&session.id, &repo, &pre_head, None), + &store, + ); + + let updated = store.get_commit(&commit.id).unwrap().unwrap(); + assert_eq!(updated.sha.as_deref(), Some(new_head.as_str())); + let _ = std::fs::remove_dir_all(repo); + } + + #[test] + fn pipeline_handoff_uses_explicit_pre_head_without_kind() { + let repo = make_git_repo("handoff-explicit-pre-head"); + let pre_head = run_git(&repo, &["rev-parse", "HEAD"]); + let config = head_moving_pipeline_config("session-1", &repo, &pre_head, None); + + assert_eq!(pre_head_for_pipeline_handoff(&config), Some(pre_head)); + let _ = std::fs::remove_dir_all(repo); + } + + /// A revert whose up-front HEAD read failed reads HEAD at handoff instead + /// (the failed step left it alone), so the AI turn's commit detection + /// still has a HEAD to compare against. A kind-less pipeline that never + /// asked reads nothing, and neither does a rebase, whose HEAD at handoff + /// may be detached on a partially applied commit. + #[test] + fn pipeline_handoff_reads_head_when_the_up_front_read_failed() { + let repo = make_git_repo("handoff-reads-head-after-failed-capture"); + let head = run_git(&repo, &["rev-parse", "HEAD"]); + + let mut revert = head_moving_pipeline_config("session-1", &repo, &head, None); + revert.pre_head_sha = None; + revert.capture_pre_head = true; + assert_eq!(pre_head_for_pipeline_handoff(&revert), Some(head.clone())); + + let mut not_asked = head_moving_pipeline_config("session-2", &repo, &head, None); + not_asked.pre_head_sha = None; + assert_eq!(pre_head_for_pipeline_handoff(¬_asked), None); + + let mut rebase = rebase_pipeline_config("session-3", &repo, &head); + rebase.pre_head_sha = None; + assert_eq!(pre_head_for_pipeline_handoff(&rebase), None); + let _ = std::fs::remove_dir_all(repo); + } + + /// A completed revert always committed, so when its up-front HEAD read + /// failed the no-AI resolution claims HEAD for the pending row instead of + /// deleting it — deleting it would unlink the session, and a "Reverting…" + /// stub keyed on that session would never be pruned. A kind-less pipeline + /// that never asked for a HEAD still drops its pending row. + #[test] + fn completed_revert_claims_head_when_the_up_front_read_failed() { + let repo = make_git_repo("revert-claims-head-after-failed-capture"); + let store = Store::in_memory().unwrap(); + let project = crate::store::Project::new("test-owner/test-repo"); + store.create_project(&project).unwrap(); + let branch = crate::store::Branch::new(&project.id, "feature", "main"); + store.create_branch(&branch).unwrap(); + let pending_session = |prompt: &str| { + let mut session = crate::store::Session::new_running(prompt, &repo); + session.pipeline = Some(PipelineExecution::from_steps(&[])); + store.create_session(&session).unwrap(); + let commit = crate::store::Commit::new_pending(&branch.id).with_session(&session.id); + store.create_commit(&commit).unwrap(); + (session.id, commit.id) + }; + let (revert_session, revert_commit) = pending_session("Revert commit abc1234"); + let (other_session, other_commit) = pending_session("Kind-less pipeline"); + + std::fs::write(repo.join("file.txt"), "two\n").unwrap(); + run_git(&repo, &["add", "file.txt"]); + run_git(&repo, &["commit", "-m", "Revert \"first\""]); + let new_head = run_git(&repo, &["rev-parse", "HEAD"]); + + let mut revert = head_moving_pipeline_config(&revert_session, &repo, "unused", None); + revert.pre_head_sha = None; + revert.capture_pre_head = true; + resolve_pipeline_artifacts_without_ai(&revert, &store, true); + let claimed = store.get_commit(&revert_commit).unwrap().unwrap(); + assert_eq!(claimed.sha.as_deref(), Some(new_head.as_str())); + + let mut not_asked = head_moving_pipeline_config(&other_session, &repo, "unused", None); + not_asked.pre_head_sha = None; + resolve_pipeline_artifacts_without_ai(¬_asked, &store, true); + assert!(store.get_commit(&other_commit).unwrap().is_none()); + let _ = std::fs::remove_dir_all(repo); + } + + /// A run that didn't complete drops its pending row, but a row that + /// already landed keeps its SHA, and a session with no row is a no-op. + #[test] + fn unfinished_pipeline_drops_only_an_unlanded_commit_row() { + let repo = make_git_repo("unfinished-pipeline-drops-pending"); + let head = run_git(&repo, &["rev-parse", "HEAD"]); + let store = Store::in_memory().unwrap(); + let project = crate::store::Project::new("test-owner/test-repo"); + store.create_project(&project).unwrap(); + let branch = crate::store::Branch::new(&project.id, "feature", "main"); + store.create_branch(&branch).unwrap(); + let session_with = |commit: Option| { + let mut session = crate::store::Session::new_running("Revert commit", &repo); + session.pipeline = Some(PipelineExecution::from_steps(&[])); + store.create_session(&session).unwrap(); + let commit_id = commit.map(|commit| { + let commit = commit.with_session(&session.id); + store.create_commit(&commit).unwrap(); + commit.id + }); + (session.id, commit_id) + }; + let (pending_session, pending_commit) = + session_with(Some(crate::store::Commit::new_pending(&branch.id))); + let (landed_session, landed_commit) = + session_with(Some(crate::store::Commit::new_with_sha(&branch.id, &head))); + let (bare_session, _) = session_with(None); + + for session_id in [&pending_session, &landed_session, &bare_session] { + let config = head_moving_pipeline_config(session_id, &repo, &head, None); + resolve_pipeline_artifacts_without_ai(&config, &store, false); + } + + let pending = store.get_commit(&pending_commit.unwrap()).unwrap(); + assert!(pending.is_none()); + let landed = store.get_commit(&landed_commit.unwrap()).unwrap().unwrap(); + assert_eq!(landed.sha.as_deref(), Some(head.as_str())); + let _ = std::fs::remove_dir_all(repo); + } + + /// Only a rebase orphans the branch's other commit rows. A kind-less + /// HEAD-moving pipeline (a revert) claims its own commit and leaves every + /// other row and review where it was — even rows a rebase's completion + /// would have moved, which is what this rebased fixture holds. + #[test] + fn kind_less_pipeline_completion_skips_rebase_reassociation() { + let fixture = rebased_branch(); + + finalize_head_moving_commit_pipeline_without_ai( + &head_moving_pipeline_config( + &fixture.rebase_session_id, + fixture.repo.path(), + &fixture.old_head, + None, + ), + &fixture.store, + ); + + let store = &fixture.store; + let head = store.get_commit(&fixture.authored[1].1).unwrap().unwrap(); + assert_eq!(head.sha.as_deref(), Some(fixture.old_head.as_str())); + let review = store.get_review(&fixture.review_id).unwrap().unwrap(); + assert_eq!(review.commit_sha, fixture.old_head); + let pending = store.get_commit(&fixture.pending_id).unwrap().unwrap(); + assert_eq!(pending.sha.as_deref(), Some(fixture.new_head.as_str())); + } + + /// A revert has no kind, so it is the flag that makes the runner read HEAD + /// at start; without it a kind-less pipeline reads nothing, and a HEAD the + /// caller already supplied is never re-read. + #[test] + fn pipeline_start_captures_pre_head_only_when_asked() { + let repo = make_git_repo("capture-pre-head-when-asked"); + let head = run_git(&repo, &["rev-parse", "HEAD"]); + + let mut asked = head_moving_pipeline_config("session-1", &repo, &head, None); + asked.pre_head_sha = None; + asked.capture_pre_head = true; + capture_pipeline_pre_head(&mut asked); + assert_eq!(asked.pre_head_sha.as_deref(), Some(head.as_str())); + + let mut not_asked = head_moving_pipeline_config("session-2", &repo, &head, None); + not_asked.pre_head_sha = None; + capture_pipeline_pre_head(&mut not_asked); + assert_eq!(not_asked.pre_head_sha, None); + + let mut rebase = rebase_pipeline_config("session-3", &repo, &head); + rebase.pre_head_sha = None; + capture_pipeline_pre_head(&mut rebase); + assert_eq!(rebase.pre_head_sha.as_deref(), Some(head.as_str())); + + let mut supplied = head_moving_pipeline_config("session-4", &repo, "supplied", None); + supplied.capture_pre_head = true; + capture_pipeline_pre_head(&mut supplied); + assert_eq!(supplied.pre_head_sha.as_deref(), Some("supplied")); + let _ = std::fs::remove_dir_all(repo); + } + // ── find_closing_fence ────────────────────────────────────────────── #[test] diff --git a/apps/staged/src-tauri/src/web_server.rs b/apps/staged/src-tauri/src/web_server.rs index 61b2afd83..8067a7022 100644 --- a/apps/staged/src-tauri/src/web_server.rs +++ b/apps/staged/src-tauri/src/web_server.rs @@ -4133,6 +4133,21 @@ pub(crate) async fn dispatch( .await?; Ok(serde_json::to_value(response).unwrap()) } + "revert_commit" => { + let store = get_store(store_mutex)?; + let branch_id: String = arg(&args, "branchId")?; + let commit_sha: String = arg(&args, "commitSha")?; + let provider: Option = opt_arg(&args, "provider")?; + let response = crate::prs::revert_commit_for_branch( + store, + Arc::clone(session_registry), + app_handle.clone(), + branch_id, + commit_sha, + provider, + )?; + Ok(serde_json::to_value(response).unwrap()) + } "get_pr_url" => { let store = get_store(store_mutex)?; let branch_id: String = arg(&args, "branchId")?; @@ -5081,6 +5096,7 @@ mod tests { "update_branch_workspace_status(", "update_workspace_status_by_workspace_name(", "_pipeline_for_branch(", + "revert_commit_for_branch(", "pull_or_queue_branch_for_branch(", "start_queued_session_now_for_branch(", "resume_session_for_store(", diff --git a/apps/staged/src/lib/commands.test.ts b/apps/staged/src/lib/commands.test.ts index 705bab6c6..665d5263e 100644 --- a/apps/staged/src/lib/commands.test.ts +++ b/apps/staged/src/lib/commands.test.ts @@ -262,7 +262,7 @@ describe('browser-native command wrappers', () => { }); }); - it('surfaces the queued-vs-running status returned by rebase and squash', async () => { + it('surfaces the queued-vs-running status returned by rebase and squash, and the revert session', async () => { const invokeCommand = vi .fn() .mockResolvedValueOnce({ sessionId: 'session-1', sessionStatus: 'queued' }) @@ -271,13 +271,14 @@ describe('browser-native command wrappers', () => { sessionId: 'session-3', sessionStatus: 'running', followUpSessionId: 'session-4', - }); + }) + .mockResolvedValueOnce('session-5'); vi.doMock('./transport', () => ({ invokeCommand, isTauri: true, })); - const { rebaseBranch, squashCommits } = await import('./commands'); + const { rebaseBranch, squashCommits, revertCommit } = await import('./commands'); await expect(rebaseBranch('branch-1', 'codex', 'origin')).resolves.toEqual({ sessionId: 'session-1', @@ -292,6 +293,7 @@ describe('browser-native command wrappers', () => { sessionStatus: 'running', followUpSessionId: 'session-4', }); + await expect(revertCommit('branch-1', 'abc1234')).resolves.toBe('session-5'); expect(invokeCommand.mock.calls).toEqual([ [ @@ -303,6 +305,7 @@ describe('browser-native command wrappers', () => { 'rebase_branch', { branchId: 'branch-1', provider: 'codex', target: null, thenForcePush: true }, ], + ['revert_commit', { branchId: 'branch-1', commitSha: 'abc1234', provider: null }], ]); }); diff --git a/apps/staged/src/lib/commands.ts b/apps/staged/src/lib/commands.ts index b48a3faf2..017369649 100644 --- a/apps/staged/src/lib/commands.ts +++ b/apps/staged/src/lib/commands.ts @@ -2179,6 +2179,21 @@ export function squashCommits( }); } +/** Revert a completed commit by creating a signed-off revert commit. + * Runs git revert directly and lets AI resolve conflicts if needed. + * Returns the session ID so the frontend can track progress. */ +export function revertCommit( + branchId: string, + commitSha: string, + provider?: string +): Promise { + return invokeCommand('revert_commit', { + branchId, + commitSha, + provider: provider ?? null, + }); +} + // ============================================================================= // Doctor (Health Check) // ============================================================================= diff --git a/apps/staged/src/lib/features/branches/BranchCard.svelte b/apps/staged/src/lib/features/branches/BranchCard.svelte index 8dc43b94e..4ce11f737 100644 --- a/apps/staged/src/lib/features/branches/BranchCard.svelte +++ b/apps/staged/src/lib/features/branches/BranchCard.svelte @@ -484,6 +484,30 @@ } } + async function handleRevertCommit(commitSha: string) { + // Unlike rebase/squash, gated on a busy branch: the backend never queues a + // revert (a queued row would have to persist the target SHA), it refuses one. + if (immediateGitActionDisabledReason) return; + commandPipelinePending = true; + const agents = isRemote ? REMOTE_AGENTS : agentState.providers; + const provider = getPreferredAgent(agents) ?? undefined; + const pendingKey = `pipeline-revert-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; + try { + const sessionId = await commands.revertCommit(branch.id, commitSha, provider); + addPendingSession(branch.id, { + key: pendingKey, + type: 'pending-commit', + title: 'Reverting…', + sessionId, + }); + await loadTimeline(); + } catch (e) { + notifyError('Revert failed', e); + } finally { + commandPipelinePending = false; + } + } + // Compute suggested prefill prompts from the latest visible timeline entry. // Only used when the user hasn't typed a draft yet. let suggestedPrefill = $derived.by(() => { @@ -822,12 +846,34 @@ */ let rebaseAlreadyInFlight = $derived(rebaseInFlight(timeline?.commits)); /** - * Gate for reset to origin, which still executes immediately: it is validated - * against a point-in-time preview of what would be discarded, so a busy branch - * has to keep blocking it. + * Gate for the git actions that execute immediately instead of queueing, so a + * busy branch has to keep blocking them: reset to origin, which is validated + * against a point-in-time preview of what would be discarded, and Revert, which + * the backend refuses rather than queues (a queued row would have to persist the + * target SHA). + * + * A reset, discard or commit delete in flight blocks them too: none has a + * session row, and each moves HEAD or the tree under anything that starts + * beside them. The backend checks the same thing for a revert from another + * client during a reset or discard, but not during a delete. Reset to origin + * shows its own "Resetting..." ahead of this reason, so it never reads as + * blocked by itself. A pending-commit delete also counts, which over-blocks + * only briefly. + * + * `branchSessionBusy` misses branch work with no timeline artifact, such as a + * create-PR session; a Revert clicked then gets the backend's refusal as a toast. */ let immediateGitActionDisabledReason = $derived( - branchCommandDisabledReason ?? (branchSessionBusy ? 'Session in progress' : null) + branchCommandDisabledReason ?? + (resettingToOrigin + ? 'Resetting to origin' + : discardingWorktreeChanges + ? 'Discarding changes' + : deletingCommitKeys.size > 0 + ? 'Deleting commit' + : branchSessionBusy + ? 'Session in progress' + : null) ); // ========================================================================= @@ -2275,6 +2321,8 @@ onReviewClick={handleReviewClick} onDeleteCommit={handleDeleteCommit} onDeletePendingCommit={handleDeletePendingCommit} + onRevertCommit={handleRevertCommit} + revertCommitDisabledReason={immediateGitActionDisabledReason ?? undefined} onDeleteNote={handleDeleteNote} onSetNoteRead={(noteId, read) => void setNoteReadState({ kind: 'branch', noteId }, read)} diff --git a/apps/staged/src/lib/features/timeline/BranchTimeline.svelte b/apps/staged/src/lib/features/timeline/BranchTimeline.svelte index cf5e11e30..608f2bfd7 100644 --- a/apps/staged/src/lib/features/timeline/BranchTimeline.svelte +++ b/apps/staged/src/lib/features/timeline/BranchTimeline.svelte @@ -105,6 +105,8 @@ onImageClick?: (imageId: string) => void; onDeleteCommit?: (sha: string, sessionId?: string, opts?: { altKey: boolean }) => void; onDeletePendingCommit?: (commitId: string, sessionId?: string) => void; + onRevertCommit?: (sha: string) => void; + revertCommitDisabledReason?: string; onDeleteNote?: (noteId: string, sessionId?: string, opts?: { altKey: boolean }) => void; /** Mark a note read or unread from its context menu. */ onSetNoteRead?: (noteId: string, read: boolean) => void; @@ -208,6 +210,8 @@ onImageClick, onDeleteCommit, onDeletePendingCommit, + onRevertCommit, + revertCommitDisabledReason, onDeleteNote, onSetNoteRead, onDeleteReview, @@ -1280,7 +1284,12 @@

No commits or notes yet

{:else} - +
{#each normalItems as item, index (item.key)}
diff --git a/apps/staged/src/lib/features/timeline/TimelineContextMenu.svelte b/apps/staged/src/lib/features/timeline/TimelineContextMenu.svelte index 458c9f3ba..392c268ae 100644 --- a/apps/staged/src/lib/features/timeline/TimelineContextMenu.svelte +++ b/apps/staged/src/lib/features/timeline/TimelineContextMenu.svelte @@ -5,6 +5,7 @@ import MailOpen from '@lucide/svelte/icons/mail-open'; import MessageSquarePlus from '@lucide/svelte/icons/message-square-plus'; import Play from '@lucide/svelte/icons/play'; + import RotateCcw from '@lucide/svelte/icons/rotate-ccw'; import Trash2 from '@lucide/svelte/icons/trash-2'; import * as ContextMenu from '$lib/components/ui/context-menu'; @@ -24,10 +25,18 @@ interface Props { actions?: TimelineContextMenuAction[]; onNewSessionReferring?: (hashtagRef: string) => void; + onRevertCommit?: (commitSha: string) => void; + revertCommitDisabledReason?: string; children?: Snippet; } - let { actions = [], onNewSessionReferring, children }: Props = $props(); + let { + actions = [], + onNewSessionReferring, + onRevertCommit, + revertCommitDisabledReason, + children, + }: Props = $props(); let open = $state(false); let activeActionKey = $state(null); @@ -100,6 +109,13 @@ } } + function handleRevertCommit() { + const sha = activeAction?.commitSha; + if (sha) { + onRevertCommit?.(sha); + } + } + function handleCopySha() { const sha = activeAction?.commitSha; if (sha) { @@ -164,6 +180,19 @@ Start now {/if} + {#if activeAction.commitSha && onRevertCommit} + + + + + Revert + {#if revertCommitDisabledReason} + {revertCommitDisabledReason} + {/if} + + + {/if} {#if activeAction.commitSha} Copy SHA