From d31ce220de71e8229b946ec2007ef868b8b29901 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 6 May 2026 17:08:43 +1000 Subject: [PATCH 1/6] feat: add commit revert action Signed-off-by: Matt Toohey --- apps/staged/src-tauri/src/lib.rs | 1 + apps/staged/src-tauri/src/prs/commands.rs | 23 +++ apps/staged/src-tauri/src/prs/context.rs | 3 +- apps/staged/src-tauri/src/prs/mod.rs | 4 + apps/staged/src-tauri/src/prs/revert.rs | 175 ++++++++++++++++++ apps/staged/src-tauri/src/prs/tests.rs | 2 + apps/staged/src-tauri/src/prs/tests/revert.rs | 29 +++ apps/staged/src-tauri/src/session_runner.rs | 130 +++++++++---- apps/staged/src-tauri/src/web_server.rs | 16 ++ apps/staged/src/lib/commands.ts | 15 ++ .../lib/features/branches/BranchCard.svelte | 24 +++ .../features/timeline/BranchTimeline.svelte | 11 +- .../timeline/TimelineContextMenu.svelte | 23 ++- 13 files changed, 422 insertions(+), 34 deletions(-) create mode 100644 apps/staged/src-tauri/src/prs/revert.rs create mode 100644 apps/staged/src-tauri/src/prs/tests/revert.rs 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..7ed70c7a3 100644 --- a/apps/staged/src-tauri/src/prs/commands.rs +++ b/apps/staged/src-tauri/src/prs/commands.rs @@ -22,6 +22,7 @@ use super::pr_status::{ PR_REFRESH_CONCURRENCY, }; use super::pull::pull_or_queue_branch_for_branch; +use super::revert::revert_commit_for_branch; use super::push::{ has_unpushed_commits_impl, start_or_queue_push_pipeline_for_branch, PushVariant, }; @@ -297,3 +298,25 @@ 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, + ) + .await +} 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/mod.rs b/apps/staged/src-tauri/src/prs/mod.rs index a84cc8eea..3cb37a038 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; @@ -57,6 +60,7 @@ pub(crate) use pr_status::{ PrStatusEvent, PR_REFRESH_CONCURRENCY, }; pub(crate) use pull::pull_or_queue_branch_for_branch; +pub(crate) use revert::revert_commit_for_branch; #[cfg(test)] pub(crate) use push::build_push_pipeline_steps; pub(crate) use push::{ 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..fd6d1835e --- /dev/null +++ b/apps/staged/src-tauri/src/prs/revert.rs @@ -0,0 +1,175 @@ +//! 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::{self, FailureStrategy, PipelineExecution, PipelineStep, Store}; + +use super::context::{resolve_branch_pipeline_context, BranchPipelineContext}; + +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), + }, + }]) +} + +/// HEAD of the checkout the pipeline will run in, read off a blocking thread +/// because on a remote branch it is a `ws exec` round trip. +async fn current_head_for_pipeline_context(ctx: &BranchPipelineContext) -> Result { + let working_dir = ctx.working_dir.clone(); + let workspace_name = ctx.workspace_name.clone(); + let remote_dir = ctx + .remote_working_dir + .as_deref() + .and_then(|dir| dir.to_str()) + .map(str::to_string); + tauri::async_runtime::spawn_blocking(move || { + let git = crate::branches::git_runner(&working_dir, workspace_name.as_deref(), remote_dir); + crate::branches::head_sha(&git) + }) + .await + .map_err(|e| format!("HEAD lookup task failed: {e}"))? + .map_err(|e| format!("Failed to get HEAD SHA: {e}")) +} + +async fn start_running_revert_pipeline_for_branch( + ctx: BranchPipelineContext, + commit_sha: String, + provider: Option, + store: Arc, + app_handle: &tauri::AppHandle, + registry: &Arc, +) -> Result { + let steps = build_revert_pipeline_steps(&commit_sha)?; + let prompt = format!("Revert commit {}", &commit_sha[..7.min(commit_sha.len())]); + let pre_head_sha = Some(current_head_for_pipeline_context(&ctx).await?); + let pipeline = PipelineExecution::from_steps(&steps); + + let mut session = store::Session::new_running(&prompt, &ctx.working_dir); + if let Some(ref p) = provider { + session = session.with_provider(p); + } + session.pipeline = Some(pipeline.clone()); + store.create_session(&session).map_err(|e| e.to_string())?; + + let branch_id = ctx.branch.id.clone(); + let project_id = ctx.branch.project_id.clone(); + + let commit = store::Commit::new_pending(&branch_id).with_session(&session.id); + store.create_commit(&commit).map_err(|e| e.to_string())?; + + session_runner::emit_session_running( + app_handle, + &session.id, + &branch_id, + &project_id, + "commit", + ); + + session_runner::start_pipeline_session( + session_runner::PipelineConfig { + session_id: session.id.clone(), + prompt, + steps, + pipeline, + working_dir: ctx.working_dir, + pre_head_sha, + provider, + workspace_name: ctx.workspace_name, + remote_working_dir: ctx.remote_working_dir, + branch_id: Some(branch_id), + project_id: Some(project_id), + }, + store, + app_handle.clone(), + Arc::clone(registry), + )?; + + Ok(session.id) +} + +/// 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) async 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 ctx = resolve_branch_pipeline_context(&store, &branch_id)?; + + let branch_has_running_session = store + .has_running_session_for_branch(&branch_id) + .map_err(|e| e.to_string())?; + let branch_has_queued_session = !store + .get_queued_sessions_for_branch(&branch_id) + .map_err(|e| e.to_string())? + .is_empty(); + + if branch_has_running_session || branch_has_queued_session { + return Err( + "Revert is unavailable while this branch has queued or running work".to_string(), + ); + } + + start_running_revert_pipeline_for_branch( + ctx, + commit_sha, + provider, + store, + &app_handle, + ®istry, + ) + .await +} diff --git a/apps/staged/src-tauri/src/prs/tests.rs b/apps/staged/src-tauri/src/prs/tests.rs index 3fb235dfb..99d6881d0 100644 --- a/apps/staged/src-tauri/src/prs/tests.rs +++ b/apps/staged/src-tauri/src/prs/tests.rs @@ -25,6 +25,7 @@ use super::pull::{ build_pull_pipeline_steps, claim_or_queue_pull_for_branch, finish_immediate_pull_session, pull_or_queue_branch_with, PullDisposition, }; +use super::revert::build_revert_pipeline_steps; use super::push::{ build_push_pipeline_steps, insert_running_push_pipeline_session, queue_push_pipeline_if_branch_busy, queue_push_pipeline_locked, PushVariant, @@ -43,6 +44,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..052df5a61 --- /dev/null +++ b/apps/staged/src-tauri/src/prs/tests/revert.rs @@ -0,0 +1,29 @@ +//! 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()); +} diff --git a/apps/staged/src-tauri/src/session_runner.rs b/apps/staged/src-tauri/src/session_runner.rs index 0980f060e..47d8acdb5 100644 --- a/apps/staged/src-tauri/src/session_runner.rs +++ b/apps/staged/src-tauri/src/session_runner.rs @@ -3455,8 +3455,11 @@ fn aborted_pipeline_error( } fn pre_head_for_pipeline_handoff(config: &PipelineConfig) -> Option { + if config.pre_head_sha.is_some() { + return config.pre_head_sha.clone(); + } + 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) => { @@ -3464,9 +3467,11 @@ fn pre_head_for_pipeline_handoff(config: &PipelineConfig) -> Option { 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, + // A rebase always carries its pre-pipeline HEAD explicitly, so the early + // return above already answered for it. 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) | None => None, } } @@ -3550,25 +3555,34 @@ 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); + let linked_commit = match store.get_commit_by_session(&config.session_id) { + Ok(Some(commit)) => commit, + Ok(None) => return, + Err(e) => { + log::warn!( + "Failed to load linked commit for terminal pipeline session {}: {e}", + config.session_id + ); + return; } - 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 - ); - } + }; + + if completed && config.pre_head_sha.is_some() { + finalize_head_moving_commit_pipeline_without_ai(config, store); + return; + } + + if linked_commit.sha.is_none() { + 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 => {} } } -fn finalize_rebase_pipeline_without_ai(config: &PipelineConfig, store: &Store) { +fn finalize_head_moving_commit_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; @@ -3579,7 +3593,7 @@ fn finalize_rebase_pipeline_without_ai(config: &PipelineConfig, store: &Store) { 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; @@ -3593,7 +3607,7 @@ fn finalize_rebase_pipeline_without_ai(config: &PipelineConfig, store: &Store) { 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; @@ -3603,7 +3617,7 @@ fn finalize_rebase_pipeline_without_ai(config: &PipelineConfig, store: &Store) { if current_head == pre_head_sha { 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 ); } @@ -3627,17 +3641,17 @@ fn finalize_rebase_pipeline_without_ai(config: &PipelineConfig, store: &Store) { 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 ), } @@ -5802,7 +5816,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 +5836,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 @@ -9270,11 +9284,25 @@ 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()), provider: None, @@ -9305,7 +9333,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, ); @@ -9418,7 +9446,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 +9569,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 +9849,46 @@ 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); + } + // ── 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..1371ec330 100644 --- a/apps/staged/src-tauri/src/web_server.rs +++ b/apps/staged/src-tauri/src/web_server.rs @@ -4133,6 +4133,22 @@ 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, + ) + .await?; + Ok(serde_json::to_value(response).unwrap()) + } "get_pr_url" => { let store = get_store(store_mutex)?; let branch_id: String = arg(&args, "branchId")?; diff --git a/apps/staged/src/lib/commands.ts b/apps/staged/src/lib/commands.ts index b48a3faf2..2533b5966 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 invoke('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..15b6c6430 100644 --- a/apps/staged/src/lib/features/branches/BranchCard.svelte +++ b/apps/staged/src/lib/features/branches/BranchCard.svelte @@ -435,6 +435,13 @@ projectSessions: projectSessionsInFlight, }); let workInFlight = $derived(branchSessionsInFlight > 0 || projectSessionsInFlight > 0); + let revertCommitDisabledReason = $derived( + commandPipelinePending + ? 'Starting command...' + : branchSessionBusy + ? 'Wait for the current branch work to finish' + : undefined + ); /** Timeline label the backend gives a queued rebase/squash pipeline session. */ const branchCommandLabels = { rebase: 'Rebase branch', squash: 'Squash commits' } as const; @@ -484,6 +491,21 @@ } } + async function handleRevertCommit(commitSha: string) { + if (commandPipelinePending || branchSessionBusy) return; + commandPipelinePending = true; + const agents = isRemote ? REMOTE_AGENTS : agentState.providers; + const provider = getPreferredAgent(agents) ?? undefined; + try { + await commands.revertCommit(branch.id, commitSha, provider); + 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(() => { @@ -2275,6 +2297,8 @@ onReviewClick={handleReviewClick} onDeleteCommit={handleDeleteCommit} onDeletePendingCommit={handleDeletePendingCommit} + onRevertCommit={handleRevertCommit} + {revertCommitDisabledReason} 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..92a7cc340 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,11 @@ Start now {/if} + {#if activeAction.commitSha && onRevertCommit} + + Revert + + {/if} {#if activeAction.commitSha} Copy SHA From aaa148345b2eda1f5d480601d0a8c693a80da034 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 4 Nov 2026 13:05:04 +1100 Subject: [PATCH 2/6] fix(staged): launch revert pipelines like other run-now pipelines After rebasing the revert-commit action onto main, its session row no longer matched what the backend queue controller (#993) expects of a `running` row: - Busy checks (`has_running_session_for_branch`, `get_*_sessions_for_branch`) now filter on `sessions.branch_id`, and the revert row was created without one, so a running revert left its branch looking idle to the queue. - The stranded-session sweep requires every `running` row to carry a registry entry from before its insert until the runner registers it, and a failed start must be recorded on the row and kick the queue. Create the revert row with `with_branch`, take the liveness hold before the insert, and hand it to the shared `launch_running_pipeline_session` instead of starting the runner inline. `RunningPipelineSession` grows a `pre_head_sha` (None for rebase/squash/push, unchanged) and an owned prompt so the revert can carry its captured HEAD and SHA-bearing label through the launcher. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- apps/staged/src-tauri/src/prs/commands.rs | 2 +- apps/staged/src-tauri/src/prs/commit.rs | 3 +- apps/staged/src-tauri/src/prs/mod.rs | 2 +- apps/staged/src-tauri/src/prs/pipeline.rs | 11 ++++-- apps/staged/src-tauri/src/prs/push.rs | 3 +- apps/staged/src-tauri/src/prs/revert.rs | 48 +++++++---------------- apps/staged/src-tauri/src/prs/tests.rs | 2 +- 7 files changed, 30 insertions(+), 41 deletions(-) diff --git a/apps/staged/src-tauri/src/prs/commands.rs b/apps/staged/src-tauri/src/prs/commands.rs index 7ed70c7a3..9f00d6b7c 100644 --- a/apps/staged/src-tauri/src/prs/commands.rs +++ b/apps/staged/src-tauri/src/prs/commands.rs @@ -22,10 +22,10 @@ use super::pr_status::{ PR_REFRESH_CONCURRENCY, }; use super::pull::pull_or_queue_branch_for_branch; -use super::revert::revert_commit_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 diff --git a/apps/staged/src-tauri/src/prs/commit.rs b/apps/staged/src-tauri/src/prs/commit.rs index b566019bc..c108fbd2c 100644 --- a/apps/staged/src-tauri/src/prs/commit.rs +++ b/apps/staged/src-tauri/src/prs/commit.rs @@ -207,7 +207,8 @@ pub(super) fn insert_running_commit_pipeline_session( Ok(RunningPipelineSession { session_id: session.id, pipeline, - prompt, + prompt: prompt.to_string(), + pre_head_sha: None, _liveness: liveness, }) } diff --git a/apps/staged/src-tauri/src/prs/mod.rs b/apps/staged/src-tauri/src/prs/mod.rs index 3cb37a038..ead49054f 100644 --- a/apps/staged/src-tauri/src/prs/mod.rs +++ b/apps/staged/src-tauri/src/prs/mod.rs @@ -60,9 +60,9 @@ pub(crate) use pr_status::{ PrStatusEvent, PR_REFRESH_CONCURRENCY, }; pub(crate) use pull::pull_or_queue_branch_for_branch; -pub(crate) use revert::revert_commit_for_branch; #[cfg(test)] 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..58e329471 100644 --- a/apps/staged/src-tauri/src/prs/pipeline.rs +++ b/apps/staged/src-tauri/src/prs/pipeline.rs @@ -228,7 +228,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, + /// HEAD captured before the insert, for a pipeline whose completion is + /// judged against it up front (a revert). A rebase leaves this `None` and + /// the runner captures it at start. + pub(super) pre_head_sha: Option, pub(super) _liveness: session_runner::LivenessHold, } @@ -262,16 +266,17 @@ pub(super) fn launch_running_pipeline_session( session_id, pipeline, prompt, + pre_head_sha, _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, + pre_head_sha, 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..9128f955a 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(), + pre_head_sha: None, _liveness: liveness, }) } diff --git a/apps/staged/src-tauri/src/prs/revert.rs b/apps/staged/src-tauri/src/prs/revert.rs index fd6d1835e..0cd871fde 100644 --- a/apps/staged/src-tauri/src/prs/revert.rs +++ b/apps/staged/src-tauri/src/prs/revert.rs @@ -12,6 +12,7 @@ use crate::session_runner; use crate::store::{self, FailureStrategy, PipelineExecution, PipelineStep, Store}; use super::context::{resolve_branch_pipeline_context, BranchPipelineContext}; +use super::pipeline::{launch_running_pipeline_session, RunningPipelineSession}; pub(super) fn validate_commit_sha(commit_sha: &str) -> Result { let sha = commit_sha.trim(); @@ -87,47 +88,28 @@ async fn start_running_revert_pipeline_for_branch( let pre_head_sha = Some(current_head_for_pipeline_context(&ctx).await?); let pipeline = PipelineExecution::from_steps(&steps); - let mut session = store::Session::new_running(&prompt, &ctx.working_dir); + let mut session = + store::Session::new_running(&prompt, &ctx.working_dir).with_branch(&ctx.branch.id); if let Some(ref p) = provider { session = session.with_provider(p); } session.pipeline = Some(pipeline.clone()); + let liveness = registry.hold(&session.id); store.create_session(&session).map_err(|e| e.to_string())?; - let branch_id = ctx.branch.id.clone(); - let project_id = ctx.branch.project_id.clone(); - - let commit = store::Commit::new_pending(&branch_id).with_session(&session.id); + let commit = store::Commit::new_pending(&ctx.branch.id).with_session(&session.id); store.create_commit(&commit).map_err(|e| e.to_string())?; - session_runner::emit_session_running( - app_handle, - &session.id, - &branch_id, - &project_id, - "commit", - ); - - session_runner::start_pipeline_session( - session_runner::PipelineConfig { - session_id: session.id.clone(), - prompt, - steps, - pipeline, - working_dir: ctx.working_dir, - pre_head_sha, - provider, - workspace_name: ctx.workspace_name, - remote_working_dir: ctx.remote_working_dir, - branch_id: Some(branch_id), - project_id: Some(project_id), - }, - store, - app_handle.clone(), - Arc::clone(registry), - )?; - - Ok(session.id) + let running = RunningPipelineSession { + session_id: session.id, + pipeline, + prompt, + pre_head_sha, + _liveness: liveness, + }; + launch_running_pipeline_session( + ctx, running, steps, "commit", provider, store, app_handle, registry, + ) } /// Revert a completed commit by creating a new signed-off revert commit. diff --git a/apps/staged/src-tauri/src/prs/tests.rs b/apps/staged/src-tauri/src/prs/tests.rs index 99d6881d0..0ac8da524 100644 --- a/apps/staged/src-tauri/src/prs/tests.rs +++ b/apps/staged/src-tauri/src/prs/tests.rs @@ -25,11 +25,11 @@ use super::pull::{ build_pull_pipeline_steps, claim_or_queue_pull_for_branch, finish_immediate_pull_session, pull_or_queue_branch_with, PullDisposition, }; -use super::revert::build_revert_pipeline_steps; 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; use crate::git; use crate::session_commands::BranchSessionLaunchStatus; use crate::session_runner; From 8aa39f66dec1e8d5863ab9f385a078885a91f2ac Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 4 Nov 2026 13:14:28 +1100 Subject: [PATCH 3/6] fix(staged): make commit revert race-safe and finish its frontend wiring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the review of the revert-commit action: - Busy check under the launch lock. `revert_commit_for_branch` checked for in-flight work without the branch launch lock, then awaited a HEAD lookup before inserting its row, so a rebase, squash or chat session could start in that window and run alongside the revert. The check and the session/pending-commit insert now share one locked section (`insert_running_revert_pipeline_session_if_idle`). The same row insert is shared with rebase/squash through `insert_running_pending_commit_session`. - HEAD is read by the runner, not the command. `PipelineConfig` gains `capture_pre_head`, which makes `run_pipeline` read pre-HEAD before the first step for a kind-less revert, the way it already does for a rebase. The read happens after the row makes the branch busy and goes through `pipeline_git_runner`, so it respects `remote_working_dir` on a multi-repo workspace and matches the post-HEAD read it is compared against. The bare `ws_exec` helper is gone. Keeping the read off the command also means a Stop during it hits a registered runner, and the command answers without waiting on a workspace round trip. - `revertCommit` used an undefined `invoke`; it now uses `invokeCommand` like `rebaseBranch`/`squashCommits`, which also routes web mode to the `revert_commit` dispatch arm. - BranchCard adds a "Reverting…" pending-session stub keyed on the returned session id, matching rebase/squash. The busy gate stays, since the backend refuses rather than queues a revert (queueing would need a persisted target SHA, which is a data-model change). It now reuses `immediateGitActionDisabledReason`, so a detached HEAD or wrong checkout also blocks a revert. - The timeline context menu renders the disabled reason inline under "Revert". A disabled item takes no pointer events, so a title tooltip would never show. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- apps/staged/src-tauri/src/prs/commands.rs | 1 - apps/staged/src-tauri/src/prs/commit.rs | 30 ++++- apps/staged/src-tauri/src/prs/git_pipeline.rs | 1 + apps/staged/src-tauri/src/prs/pipeline.rs | 14 +- apps/staged/src-tauri/src/prs/push.rs | 2 +- apps/staged/src-tauri/src/prs/revert.rs | 120 ++++++++---------- apps/staged/src-tauri/src/prs/tests.rs | 4 +- apps/staged/src-tauri/src/prs/tests/revert.rs | 79 ++++++++++++ apps/staged/src-tauri/src/session_runner.rs | 68 ++++++++-- apps/staged/src-tauri/src/web_server.rs | 3 +- apps/staged/src/lib/commands.ts | 2 +- .../lib/features/branches/BranchCard.svelte | 22 ++-- .../timeline/TimelineContextMenu.svelte | 10 +- 13 files changed, 252 insertions(+), 104 deletions(-) diff --git a/apps/staged/src-tauri/src/prs/commands.rs b/apps/staged/src-tauri/src/prs/commands.rs index 9f00d6b7c..71dea5d25 100644 --- a/apps/staged/src-tauri/src/prs/commands.rs +++ b/apps/staged/src-tauri/src/prs/commands.rs @@ -318,5 +318,4 @@ pub async fn revert_commit( commit_sha, provider, ) - .await } diff --git a/apps/staged/src-tauri/src/prs/commit.rs b/apps/staged/src-tauri/src/prs/commit.rs index c108fbd2c..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); } @@ -207,8 +230,8 @@ pub(super) fn insert_running_commit_pipeline_session( Ok(RunningPipelineSession { session_id: session.id, pipeline, - prompt: prompt.to_string(), - pre_head_sha: None, + prompt, + capture_pre_head, _liveness: liveness, }) } @@ -581,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/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/pipeline.rs b/apps/staged/src-tauri/src/prs/pipeline.rs index 58e329471..14fc3aa12 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, @@ -229,10 +230,10 @@ pub(super) struct RunningPipelineSession { pub(super) session_id: String, pub(super) pipeline: PipelineExecution, pub(super) prompt: String, - /// HEAD captured before the insert, for a pipeline whose completion is - /// judged against it up front (a revert). A rebase leaves this `None` and - /// the runner captures it at start. - pub(super) pre_head_sha: Option, + /// 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, } @@ -266,7 +267,7 @@ pub(super) fn launch_running_pipeline_session( session_id, pipeline, prompt, - pre_head_sha, + capture_pre_head, _liveness: liveness, } = running; let started = session_runner::start_pipeline_session( @@ -276,7 +277,8 @@ pub(super) fn launch_running_pipeline_session( steps, pipeline, working_dir: ctx.working_dir, - pre_head_sha, + 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 9128f955a..809a2d021 100644 --- a/apps/staged/src-tauri/src/prs/push.rs +++ b/apps/staged/src-tauri/src/prs/push.rs @@ -153,7 +153,7 @@ pub(super) fn insert_running_push_pipeline_session( session_id: session.id, pipeline, prompt: prompt.to_string(), - pre_head_sha: None, + 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 index 0cd871fde..a99b0b9ed 100644 --- a/apps/staged/src-tauri/src/prs/revert.rs +++ b/apps/staged/src-tauri/src/prs/revert.rs @@ -9,10 +9,13 @@ use std::sync::Arc; use crate::session_runner; -use crate::store::{self, FailureStrategy, PipelineExecution, PipelineStep, Store}; +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::{launch_running_pipeline_session, RunningPipelineSession}; +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(); @@ -56,59 +59,44 @@ pub(super) fn build_revert_pipeline_steps(commit_sha: &str) -> Result Result { - let working_dir = ctx.working_dir.clone(); - let workspace_name = ctx.workspace_name.clone(); - let remote_dir = ctx - .remote_working_dir - .as_deref() - .and_then(|dir| dir.to_str()) - .map(str::to_string); - tauri::async_runtime::spawn_blocking(move || { - let git = crate::branches::git_runner(&working_dir, workspace_name.as_deref(), remote_dir); - crate::branches::head_sha(&git) - }) - .await - .map_err(|e| format!("HEAD lookup task failed: {e}"))? - .map_err(|e| format!("Failed to get HEAD SHA: {e}")) -} +pub(super) const REVERT_BRANCH_BUSY: &str = + "Revert is unavailable while this branch has queued or running work"; -async fn start_running_revert_pipeline_for_branch( - ctx: BranchPipelineContext, - commit_sha: String, - provider: Option, - store: Arc, - app_handle: &tauri::AppHandle, +/// 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. +/// +/// 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 else can move HEAD in between. +pub(super) fn insert_running_revert_pipeline_session_if_idle( + store: &Arc, registry: &Arc, -) -> Result { - let steps = build_revert_pipeline_steps(&commit_sha)?; - let prompt = format!("Revert commit {}", &commit_sha[..7.min(commit_sha.len())]); - let pre_head_sha = Some(current_head_for_pipeline_context(&ctx).await?); - let pipeline = PipelineExecution::from_steps(&steps); - - let mut session = - store::Session::new_running(&prompt, &ctx.working_dir).with_branch(&ctx.branch.id); - if let Some(ref p) = provider { - session = session.with_provider(p); + 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 branch_has_work_in_flight(store, &ctx.branch.id)? { + return Err(REVERT_BRANCH_BUSY.to_string()); } - session.pipeline = Some(pipeline.clone()); - let liveness = registry.hold(&session.id); - store.create_session(&session).map_err(|e| e.to_string())?; - - let commit = store::Commit::new_pending(&ctx.branch.id).with_session(&session.id); - store.create_commit(&commit).map_err(|e| e.to_string())?; - let running = RunningPipelineSession { - session_id: session.id, - pipeline, + 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, - pre_head_sha, - _liveness: liveness, - }; - launch_running_pipeline_session( - ctx, running, steps, "commit", provider, store, app_handle, registry, + provider, + true, ) } @@ -120,7 +108,7 @@ async fn start_running_revert_pipeline_for_branch( /// /// Shared by the Tauri command and the web dispatch arm so both gates behave /// identically. -pub(crate) async fn revert_commit_for_branch( +pub(crate) fn revert_commit_for_branch( store: Arc, registry: Arc, app_handle: tauri::AppHandle, @@ -129,29 +117,25 @@ pub(crate) async fn revert_commit_for_branch( 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 branch_has_running_session = store - .has_running_session_for_branch(&branch_id) - .map_err(|e| e.to_string())?; - let branch_has_queued_session = !store - .get_queued_sessions_for_branch(&branch_id) - .map_err(|e| e.to_string())? - .is_empty(); - - if branch_has_running_session || branch_has_queued_session { - return Err( - "Revert is unavailable while this branch has queued or running work".to_string(), - ); - } - - start_running_revert_pipeline_for_branch( + let running = insert_running_revert_pipeline_session_if_idle( + &store, + ®istry, + &ctx, + &steps, + &commit_sha, + provider.as_deref(), + )?; + launch_running_pipeline_session( ctx, - commit_sha, + running, + steps, + "commit", provider, store, &app_handle, ®istry, ) - .await } diff --git a/apps/staged/src-tauri/src/prs/tests.rs b/apps/staged/src-tauri/src/prs/tests.rs index 0ac8da524..c9d494fa5 100644 --- a/apps/staged/src-tauri/src/prs/tests.rs +++ b/apps/staged/src-tauri/src/prs/tests.rs @@ -29,7 +29,9 @@ 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; +use super::revert::{ + build_revert_pipeline_steps, insert_running_revert_pipeline_session_if_idle, REVERT_BRANCH_BUSY, +}; use crate::git; use crate::session_commands::BranchSessionLaunchStatus; use crate::session_runner; diff --git a/apps/staged/src-tauri/src/prs/tests/revert.rs b/apps/staged/src-tauri/src/prs/tests/revert.rs index 052df5a61..1ef6e45d6 100644 --- a/apps/staged/src-tauri/src/prs/tests/revert.rs +++ b/apps/staged/src-tauri/src/prs/tests/revert.rs @@ -27,3 +27,82 @@ fn revert_pipeline_rejects_unsafe_commit_sha() { 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()); +} diff --git a/apps/staged/src-tauri/src/session_runner.rs b/apps/staged/src-tauri/src/session_runner.rs index 47d8acdb5..fd4a54631 100644 --- a/apps/staged/src-tauri/src/session_runner.rs +++ b/apps/staged/src-tauri/src/session_runner.rs @@ -2974,6 +2974,12 @@ 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. + pub capture_pre_head: bool, /// ACP provider ID (e.g. "goose", "claude"). pub provider: Option, /// Workspace name for remote branches. @@ -3719,6 +3725,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, @@ -3726,16 +3749,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 @@ -7989,6 +8003,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, @@ -9305,6 +9320,7 @@ mod tests { 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, @@ -9367,6 +9383,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, @@ -9889,6 +9906,37 @@ mod tests { let _ = std::fs::remove_dir_all(repo); } + /// 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 1371ec330..69677d4c3 100644 --- a/apps/staged/src-tauri/src/web_server.rs +++ b/apps/staged/src-tauri/src/web_server.rs @@ -4145,8 +4145,7 @@ pub(crate) async fn dispatch( branch_id, commit_sha, provider, - ) - .await?; + )?; Ok(serde_json::to_value(response).unwrap()) } "get_pr_url" => { diff --git a/apps/staged/src/lib/commands.ts b/apps/staged/src/lib/commands.ts index 2533b5966..017369649 100644 --- a/apps/staged/src/lib/commands.ts +++ b/apps/staged/src/lib/commands.ts @@ -2187,7 +2187,7 @@ export function revertCommit( commitSha: string, provider?: string ): Promise { - return invoke('revert_commit', { + return invokeCommand('revert_commit', { branchId, commitSha, provider: provider ?? null, diff --git a/apps/staged/src/lib/features/branches/BranchCard.svelte b/apps/staged/src/lib/features/branches/BranchCard.svelte index 15b6c6430..08d7b8dc5 100644 --- a/apps/staged/src/lib/features/branches/BranchCard.svelte +++ b/apps/staged/src/lib/features/branches/BranchCard.svelte @@ -435,13 +435,6 @@ projectSessions: projectSessionsInFlight, }); let workInFlight = $derived(branchSessionsInFlight > 0 || projectSessionsInFlight > 0); - let revertCommitDisabledReason = $derived( - commandPipelinePending - ? 'Starting command...' - : branchSessionBusy - ? 'Wait for the current branch work to finish' - : undefined - ); /** Timeline label the backend gives a queued rebase/squash pipeline session. */ const branchCommandLabels = { rebase: 'Rebase branch', squash: 'Squash commits' } as const; @@ -492,12 +485,21 @@ } async function handleRevertCommit(commitSha: string) { - if (commandPipelinePending || branchSessionBusy) return; + // 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 { - await commands.revertCommit(branch.id, commitSha, provider); + 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); @@ -2298,7 +2300,7 @@ onDeleteCommit={handleDeleteCommit} onDeletePendingCommit={handleDeletePendingCommit} onRevertCommit={handleRevertCommit} - {revertCommitDisabledReason} + revertCommitDisabledReason={immediateGitActionDisabledReason ?? undefined} onDeleteNote={handleDeleteNote} onSetNoteRead={(noteId, read) => void setNoteReadState({ kind: 'branch', noteId }, read)} diff --git a/apps/staged/src/lib/features/timeline/TimelineContextMenu.svelte b/apps/staged/src/lib/features/timeline/TimelineContextMenu.svelte index 92a7cc340..392c268ae 100644 --- a/apps/staged/src/lib/features/timeline/TimelineContextMenu.svelte +++ b/apps/staged/src/lib/features/timeline/TimelineContextMenu.svelte @@ -181,8 +181,16 @@ {/if} {#if activeAction.commitSha && onRevertCommit} + - Revert + + + Revert + {#if revertCommitDisabledReason} + {revertCommitDisabledReason} + {/if} + {/if} {#if activeAction.commitSha} From 659bb840408440b4ecae986640f854cc18b7402d Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 4 Nov 2026 13:29:26 +1100 Subject: [PATCH 4/6] fix(staged): close revert-commit gaps around resets, orphans and HEAD reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the follow-up review of the revert-commit action: - Refuse a revert during a reset-to-origin or discard. Neither has a session row, so the busy check could not see them, and a revert started under one raced its `git reset --hard`. The revert gate now reads `branch_worktree_operation_in_flight` under the launch lock, as the forced start does, and refuses with its own message. - Stop orphaned queued rows from keeping a branch busy. `branch_has_work_in_flight` now reuses `session_commands::has_queued_branch_session`, which skips `Unlinked` rows the drain steps over. Before, one such row blocked Revert on the branch for good, and a rebase, squash, push or pull queued behind it alone was never drained. - Keep the revert commit linked when the runner's up-front pre-HEAD read fails. `capture_pre_head` now also promises that the pipeline hands off only from a failed step (HEAD unchanged) and that a completed run moved HEAD, which holds for `git revert`. So the handoff reads HEAD as the pre-pipeline HEAD, and the no-AI completion claims HEAD for the pending row instead of deleting it. Deleting it left a "Reverting…" stub that was never pruned. - Run orphan reassociation on the no-AI path only for a rebase again, matching the AI path. A revert adds a commit on top and orphans nothing, so it no longer pays the history walk or the remote round trip. - `immediateGitActionDisabledReason` now covers Revert as well as reset to origin, and blocks while a reset or discard is in flight. Reset to origin still shows its own "Resetting..." first, so it never reads as blocked by itself. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- apps/staged/src-tauri/src/prs/pipeline.rs | 12 +- apps/staged/src-tauri/src/prs/revert.rs | 11 ++ apps/staged/src-tauri/src/prs/tests.rs | 7 +- apps/staged/src-tauri/src/prs/tests/revert.rs | 57 +++++++ apps/staged/src-tauri/src/session_commands.rs | 2 +- apps/staged/src-tauri/src/session_runner.rs | 156 ++++++++++++++++-- .../lib/features/branches/BranchCard.svelte | 26 ++- 7 files changed, 242 insertions(+), 29 deletions(-) diff --git a/apps/staged/src-tauri/src/prs/pipeline.rs b/apps/staged/src-tauri/src/prs/pipeline.rs index 14fc3aa12..9d9cdea42 100644 --- a/apps/staged/src-tauri/src/prs/pipeline.rs +++ b/apps/staged/src-tauri/src/prs/pipeline.rs @@ -186,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( @@ -198,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) diff --git a/apps/staged/src-tauri/src/prs/revert.rs b/apps/staged/src-tauri/src/prs/revert.rs index a99b0b9ed..b13f6d76c 100644 --- a/apps/staged/src-tauri/src/prs/revert.rs +++ b/apps/staged/src-tauri/src/prs/revert.rs @@ -61,6 +61,8 @@ pub(super) fn build_revert_pipeline_steps(commit_sha: &str) -> Result 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()); } diff --git a/apps/staged/src-tauri/src/prs/tests.rs b/apps/staged/src-tauri/src/prs/tests.rs index c9d494fa5..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::{ @@ -30,7 +30,8 @@ use super::push::{ 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, + 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; diff --git a/apps/staged/src-tauri/src/prs/tests/revert.rs b/apps/staged/src-tauri/src/prs/tests/revert.rs index 1ef6e45d6..93c3338f8 100644 --- a/apps/staged/src-tauri/src/prs/tests/revert.rs +++ b/apps/staged/src-tauri/src/prs/tests/revert.rs @@ -106,3 +106,60 @@ fn revert_on_a_busy_branch_inserts_nothing() { .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 fd4a54631..b95072628 100644 --- a/apps/staged/src-tauri/src/session_runner.rs +++ b/apps/staged/src-tauri/src/session_runner.rs @@ -2979,6 +2979,15 @@ pub struct PipelineConfig { /// 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, @@ -3465,19 +3474,29 @@ fn pre_head_for_pipeline_handoff(config: &PipelineConfig) -> Option { return config.pre_head_sha.clone(); } - match config.pipeline.kind.as_ref() { - Some(PipelineKind::Squash) => match current_pipeline_head(config) { - Ok(head) => Some(head), - Err(e) => { - log::warn!("Failed to capture squash handoff HEAD: {e}"); - None - } - }, + 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 always carries its pre-pipeline HEAD explicitly, so the early // return above already answered for it. 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) | None => None, + 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 + } } } @@ -3573,7 +3592,7 @@ fn resolve_pipeline_artifacts_without_ai(config: &PipelineConfig, store: &Store, } }; - if completed && config.pre_head_sha.is_some() { + if completed && (config.pre_head_sha.is_some() || config.capture_pre_head) { finalize_head_moving_commit_pipeline_without_ai(config, store); return; } @@ -3589,10 +3608,15 @@ fn resolve_pipeline_artifacts_without_ai(config: &PipelineConfig, store: &Store, } fn finalize_head_moving_commit_pipeline_without_ai(config: &PipelineConfig, store: &Store) { - let Some(pre_head_sha) = config.pre_head_sha.as_deref() else { + // With no pre-pipeline HEAD there is nothing to tell a no-op from a commit + // by, unless the pipeline 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(); + if pre_head_sha.is_none() && !config.capture_pre_head { let _ = store.delete_pending_commit_for_session(&config.session_id); return; - }; + } let commit = match store.get_commit_by_session(&config.session_id) { Ok(Some(commit)) => commit, @@ -3607,7 +3631,7 @@ fn finalize_head_moving_commit_pipeline_without_ai(config: &PipelineConfig, stor }; // 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, @@ -3620,7 +3644,7 @@ fn finalize_head_moving_commit_pipeline_without_ai(config: &PipelineConfig, stor } }; - 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 pending commit for session {}: {e}", @@ -3630,20 +3654,25 @@ fn finalize_head_moving_commit_pipeline_without_ai(config: &PipelineConfig, stor 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!( @@ -9906,6 +9935,101 @@ mod tests { 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); + } + + /// 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. diff --git a/apps/staged/src/lib/features/branches/BranchCard.svelte b/apps/staged/src/lib/features/branches/BranchCard.svelte index 08d7b8dc5..201432d81 100644 --- a/apps/staged/src/lib/features/branches/BranchCard.svelte +++ b/apps/staged/src/lib/features/branches/BranchCard.svelte @@ -846,12 +846,30 @@ */ 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 or discard in flight blocks them too: neither has a session row, and + * both move HEAD or the tree under anything that starts beside them. The + * backend checks the same thing for a revert from another client. Reset to + * origin shows its own "Resetting..." ahead of this reason, so it never reads + * as blocked by itself. + * + * `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' + : branchSessionBusy + ? 'Session in progress' + : null) ); // ========================================================================= From 226913c245ade15631824425caaa85e035b2fadc Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 4 Nov 2026 13:37:19 +1100 Subject: [PATCH 5/6] fix(staged): block Revert while a commit delete is in flight Address the follow-up review of the revert-commit action: - Delete commit is a third HEAD-mover with no session row, and it takes neither the worktree marker nor the launch lock. If a revert's commit landed after a delete had read HEAD, the delete's `git reset --hard` to the parent wiped the revert along with the deleted commit, and the no-AI completion claimed the wrong commit for the revert row. `immediateGitActionDisabledReason` now refuses with "Deleting commit" while `deletingCommitKeys` is non-empty, alongside reset and discard. The reset-to-origin row reads the same reason, which is correct: a reset racing a delete hits the same hazard. A pending-commit delete also counts, which over-blocks only briefly. - Correct the `insert_running_revert_pipeline_session_if_idle` doc. A busy row only keeps queue-routed work from moving HEAD before the runner reads it. Reset-to-origin, discard and delete-commit bypass the queue. The marker only stops a revert starting under a reset or discard, only frontend gates keep those two from starting beside a running revert, and nothing gates a delete-commit then. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- apps/staged/src-tauri/src/prs/revert.rs | 6 +++++- .../lib/features/branches/BranchCard.svelte | 20 +++++++++++-------- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/apps/staged/src-tauri/src/prs/revert.rs b/apps/staged/src-tauri/src/prs/revert.rs index b13f6d76c..679672b6b 100644 --- a/apps/staged/src-tauri/src/prs/revert.rs +++ b/apps/staged/src-tauri/src/prs/revert.rs @@ -81,7 +81,11 @@ pub(super) const REVERT_WORKTREE_BUSY: &str = /// /// 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 else can move HEAD in between. +/// 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, diff --git a/apps/staged/src/lib/features/branches/BranchCard.svelte b/apps/staged/src/lib/features/branches/BranchCard.svelte index 201432d81..4ce11f737 100644 --- a/apps/staged/src/lib/features/branches/BranchCard.svelte +++ b/apps/staged/src/lib/features/branches/BranchCard.svelte @@ -852,11 +852,13 @@ * the backend refuses rather than queues (a queued row would have to persist the * target SHA). * - * A reset or discard in flight blocks them too: neither has a session row, and - * both move HEAD or the tree under anything that starts beside them. The - * backend checks the same thing for a revert from another client. Reset to - * origin shows its own "Resetting..." ahead of this reason, so it never reads - * as blocked by itself. + * 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. @@ -867,9 +869,11 @@ ? 'Resetting to origin' : discardingWorktreeChanges ? 'Discarding changes' - : branchSessionBusy - ? 'Session in progress' - : null) + : deletingCommitKeys.size > 0 + ? 'Deleting commit' + : branchSessionBusy + ? 'Session in progress' + : null) ); // ========================================================================= From 4e70866d133f260a5bc3af8c1f5a21e843f5f1ed Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Wed, 4 Nov 2026 13:47:34 +1100 Subject: [PATCH 6/6] refactor(staged): simplify no-AI pipeline resolution and tighten revert guards Apply the review suggestions on the revert-commit action: - `resolve_pipeline_artifacts_without_ai` no longer loads the linked commit up front. A completed run with a pre-pipeline HEAD (or one that asked for it) finalizes; anything else deletes its pending row. The lookup was redundant: `delete_pending_commit_for_session` already matches only `sha IS NULL`, and finalize reloads the row and handles a missing one. Finalize's no-HEAD guard became unreachable from its only caller, so it is now a `debug_assert!`. No behaviour change; a new test pins that an unfinished run drops a pending row, keeps a landed one and is a no-op without a row. - Correct the rebase arm comment in `pre_head_for_pipeline_handoff`. A rebase only reaches it when its up-front HEAD read failed, and it must not read HEAD then, since it may be detached mid-rebase. - Add `revert_commit_for_branch(` to the web guard's `WRITE_MARKERS`, so `web_arms_that_write_running_rows_are_detached` covers the revert arm if it ever gains an await. - Cover `revertCommit` in the invoke-shape test (`revert_commit` with `branchId`, `commitSha`, `provider: null`), since the wrapper once shipped calling an undefined `invoke`. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Matt Toohey --- apps/staged/src-tauri/src/session_runner.rs | 96 +++++++++++++-------- apps/staged/src-tauri/src/web_server.rs | 1 + apps/staged/src/lib/commands.test.ts | 9 +- 3 files changed, 69 insertions(+), 37 deletions(-) diff --git a/apps/staged/src-tauri/src/session_runner.rs b/apps/staged/src-tauri/src/session_runner.rs index b95072628..42df0cf1b 100644 --- a/apps/staged/src-tauri/src/session_runner.rs +++ b/apps/staged/src-tauri/src/session_runner.rs @@ -3481,10 +3481,11 @@ fn pre_head_for_pipeline_handoff(config: &PipelineConfig) -> Option { // 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 always carries its pre-pipeline HEAD explicitly, so the early - // return above already answered for it. A push never rewrites local - // history and a pull never hands off, so neither has a pre-pipeline HEAD - // to compare against. + // 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 { @@ -3580,43 +3581,30 @@ fn reassociate_rebased_commits( } fn resolve_pipeline_artifacts_without_ai(config: &PipelineConfig, store: &Store, completed: bool) { - let linked_commit = match store.get_commit_by_session(&config.session_id) { - Ok(Some(commit)) => commit, - Ok(None) => return, - Err(e) => { - log::warn!( - "Failed to load linked commit for terminal pipeline session {}: {e}", - config.session_id - ); - return; - } - }; - + // 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); - return; - } - - if linked_commit.sha.is_none() { - 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 - ); - } + } 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_head_moving_commit_pipeline_without_ai(config: &PipelineConfig, store: &Store) { - // With no pre-pipeline HEAD there is nothing to tell a no-op from a commit - // by, unless the pipeline 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. + // 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(); - if pre_head_sha.is_none() && !config.capture_pre_head { - let _ = store.delete_pending_commit_for_session(&config.session_id); - return; - } + 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, @@ -10003,6 +9991,46 @@ mod tests { 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 diff --git a/apps/staged/src-tauri/src/web_server.rs b/apps/staged/src-tauri/src/web_server.rs index 69677d4c3..8067a7022 100644 --- a/apps/staged/src-tauri/src/web_server.rs +++ b/apps/staged/src-tauri/src/web_server.rs @@ -5096,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 }], ]); });