From 99e6bac5ba6e0b4ef927e92fc2f4bff5395e95bb Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 22:01:36 +0000 Subject: [PATCH 1/2] feat: add Undo commit and Revert commit actions to Commits tab Undo removes the latest commit in a workspace's own lineage (not the working copy, not a commit on the target branch), enforced sequentially from the tip via a validated abandon. Revert creates a new commit that reverses any real commit's changes on top of the workspace's current tip, restricted only from targeting the working copy itself. --- .../specs/undo-revert-commit.spec.tsx | 168 +++++++++++++ src-tauri/src/commands/workspace.rs | 60 +++++ src-tauri/src/core/commits.rs | 83 +++++++ src-tauri/src/jj.rs | 140 ++++++++++- src-tauri/src/lib.rs | 2 + src-tauri/tests/core_commits_test.rs | 166 +++++++++++++ src/components/CommitDiffViewer.tsx | 109 +++++++++ .../CommitDiffViewer.undoRevert.test.tsx | 231 ++++++++++++++++++ src/lib/api-extra.ts | 30 +++ 9 files changed, 988 insertions(+), 1 deletion(-) create mode 100644 scripts/screenshot/specs/undo-revert-commit.spec.tsx create mode 100644 src/components/CommitDiffViewer.undoRevert.test.tsx diff --git a/scripts/screenshot/specs/undo-revert-commit.spec.tsx b/scripts/screenshot/specs/undo-revert-commit.spec.tsx new file mode 100644 index 000000000..32be8a17e --- /dev/null +++ b/scripts/screenshot/specs/undo-revert-commit.spec.tsx @@ -0,0 +1,168 @@ +import * as React from "react"; +import { expect, it, vi } from "vitest"; +import userEvent from "@testing-library/user-event"; +import { ask } from "@tauri-apps/plugin-dialog"; +import { + commitWorkspaceFile, + createTestRepo, + openRepo, +} from "../../../test/utils"; +import { render, screen, waitFor, within } from "../../../test/test-utils"; +import { Dashboard } from "../../../src/components/Dashboard"; +import { getWorkspaces } from "../../../src/lib/api"; +import { captureDocument } from "../capture"; + +const BRANCH_NAME = "feat/undo-revert-demo"; + +function rowFor(commitId: string): HTMLElement { + const el = document.querySelector(`[data-commit-id="${commitId}"]`); + if (!el) throw new Error(`No row found for commit ${commitId}`); + return el as HTMLElement; +} + +// Scenario: the user creates a workspace via the home repo's "Stack" button, +// makes 2 workspace commits, opens the Commits tab, and exercises the new +// "Undo commit" (only valid on the latest commit in the lineage) and +// "Revert commit" (valid on any commit) actions. +it("captures undoing and reverting commits from the Commits tab", async () => { + vi.mocked(ask).mockResolvedValue(true); + + const { repoPath } = createTestRepo(false); + openRepo(repoPath); + + const user = userEvent.setup(); + render(); + + await screen.findByTestId("show-workspace-header"); + await user.click(await screen.findByRole("button", { name: "Stack" })); + + const createDialog = await screen.findByTestId("modal"); + const branchNameInput = within(createDialog).getByLabelText("Branch Name"); + await user.type(branchNameInput, BRANCH_NAME); + await user.click( + within(createDialog).getByRole("button", { name: "Create Workspace" }), + ); + await waitFor(() => { + expect(screen.queryByTestId("modal")).not.toBeInTheDocument(); + }); + + const header = await screen.findByTestId("show-workspace-header"); + await within(header).findByText(BRANCH_NAME); + + const workspace = (await getWorkspaces(repoPath)).find( + (candidate) => candidate.branch_name === BRANCH_NAME, + ); + if (!workspace) { + throw new Error(`Expected ${BRANCH_NAME} workspace to exist`); + } + + await commitWorkspaceFile( + repoPath, + { id: workspace.id, path: workspace.workspace_path }, + "a.txt", + "content a", + "First commit", + ); + await commitWorkspaceFile( + repoPath, + { id: workspace.id, path: workspace.workspace_path }, + "b.txt", + "content b", + "Second commit", + ); + + await user.click(await screen.findByRole("tab", { name: "Commits" })); + await screen.findByText("Second commit"); + await screen.findByText("First commit"); + + // Expand the newest commit — it should offer "Undo commit". + const secondCommitHeader = await screen.findByText("Second commit"); + await user.click(secondCommitHeader); + const secondCommitRowEl = secondCommitHeader.closest( + "[data-commit-id]", + ) as HTMLElement; + const secondCommitId = secondCommitRowEl.getAttribute("data-commit-id")!; + await within(rowFor(secondCommitId)).findByRole("button", { + name: "Undo commit", + }); + + await captureDocument(document, { + name: "undo-revert-commit-01-latest-expanded", + expectations: [ + "The Commits tab is active, showing the expanded 'Second commit' row.", + "The action bar for the expanded row includes an 'Undo commit' button alongside 'Edit description', 'Revert commit', 'Move commit', and 'Delete commit'.", + ], + }); + + // Collapse it, then expand the older commit — no "Undo commit" there, + // but "Revert commit" is still available. + await user.click(secondCommitHeader); + const firstCommitHeader = await screen.findByText("First commit"); + await user.click(firstCommitHeader); + const firstCommitRowEl = firstCommitHeader.closest( + "[data-commit-id]", + ) as HTMLElement; + const firstCommitId = firstCommitRowEl.getAttribute("data-commit-id")!; + await within(rowFor(firstCommitId)).findByRole("button", { + name: "Revert commit", + }); + expect( + within(rowFor(firstCommitId)).queryByRole("button", { + name: "Undo commit", + }), + ).not.toBeInTheDocument(); + + await captureDocument(document, { + name: "undo-revert-commit-02-older-expanded-no-undo", + expectations: [ + "The expanded 'First commit' row's action bar shows 'Revert commit' but does NOT show an 'Undo commit' button.", + ], + }); + + // Revert the older commit. + await user.click( + within(rowFor(firstCommitId)).getByRole("button", { + name: "Revert commit", + }), + ); + await screen.findByText("Commit reverted"); + await screen.findByText(/^Revert "First commit"/); + + await captureDocument(document, { + name: "undo-revert-commit-03-after-revert", + expectations: [ + "A success toast reading 'Commit reverted' is visible.", + "The commit list now shows a new top commit whose title starts with 'Revert \"First commit\"', above 'Second commit' and 'First commit'.", + ], + }); + + // Undo the now-latest commit (the revert commit itself). + const revertCommitHeader = await screen.findByText( + /^Revert "First commit"/, + ); + await user.click(revertCommitHeader); + const revertRowEl = revertCommitHeader.closest( + "[data-commit-id]", + ) as HTMLElement; + const revertCommitId = revertRowEl.getAttribute("data-commit-id")!; + await user.click( + within(rowFor(revertCommitId)).getByRole("button", { + name: "Undo commit", + }), + ); + await screen.findByText("Commit undone"); + + await waitFor(() => { + expect( + screen.queryByText(/^Revert "First commit"/), + ).not.toBeInTheDocument(); + }); + + await captureDocument(document, { + name: "undo-revert-commit-04-after-undo", + expectations: [ + "A success toast reading 'Commit undone' is visible.", + "The revert commit is gone from the list, leaving 'Second commit' and 'First commit'.", + ], + }); +}, 60000); diff --git a/src-tauri/src/commands/workspace.rs b/src-tauri/src/commands/workspace.rs index 3e6e1247e..f85cb8ea0 100644 --- a/src-tauri/src/commands/workspace.rs +++ b/src-tauri/src/commands/workspace.rs @@ -558,6 +558,66 @@ pub async fn abandon_commit( result } +/// Undo the latest commit in a workspace's own lineage (not the working copy, +/// not a commit on the target branch). Must be undone sequentially from the tip. +#[tauri::command] +pub async fn undo_commit( + repo_path: String, + workspace_id: i64, + commit_change_id: String, +) -> Result<(), String> { + let started_at = Instant::now(); + let repo_path_for_task = repo_path.clone(); + let commit_change_id_for_task = commit_change_id.clone(); + let result = tauri::async_runtime::spawn_blocking(move || { + crate::core::undo_commit( + &repo_path_for_task, + workspace_id, + &commit_change_id_for_task, + ) + }) + .await + .map_err(|e| format!("Failed to join undo_commit task: {}", e))?; + log::debug!( + "undo_commit(repo_path={}, workspace_id={}, commit_change_id={}) completed in {:?}", + repo_path, + workspace_id, + commit_change_id, + started_at.elapsed() + ); + result +} + +/// Revert a commit by creating a new commit that reverses its changes on top +/// of the workspace's current tip. Can target any commit except the working copy. +#[tauri::command] +pub async fn revert_commit( + repo_path: String, + workspace_id: i64, + commit_change_id: String, +) -> Result<(), String> { + let started_at = Instant::now(); + let repo_path_for_task = repo_path.clone(); + let commit_change_id_for_task = commit_change_id.clone(); + let result = tauri::async_runtime::spawn_blocking(move || { + crate::core::revert_commit( + &repo_path_for_task, + workspace_id, + &commit_change_id_for_task, + ) + }) + .await + .map_err(|e| format!("Failed to join revert_commit task: {}", e))?; + log::debug!( + "revert_commit(repo_path={}, workspace_id={}, commit_change_id={}) completed in {:?}", + repo_path, + workspace_id, + commit_change_id, + started_at.elapsed() + ); + result +} + #[tauri::command] pub async fn rebase_home_repo_branch( repo_path: String, diff --git a/src-tauri/src/core/commits.rs b/src-tauri/src/core/commits.rs index 18d1cb1f7..58b0bd232 100644 --- a/src-tauri/src/core/commits.rs +++ b/src-tauri/src/core/commits.rs @@ -278,6 +278,89 @@ pub fn undo_repo_operation( .map_err(|e| format!("Failed to undo operation: {}", e)) } +/// Undoes the latest commit in a workspace's own lineage by change-id. +/// +/// Only the workspace's current tip commit can be undone — not the working +/// copy, and not a commit inherited from the target branch. To undo an older +/// commit, undo newer commits first, one at a time. +/// +/// # Arguments +/// * `repo_path` - Path to the repository root +/// * `workspace_id` - ID of the workspace that owns the commit +/// * `commit_change_id` - The short change-id of the commit to undo +/// +/// # Returns +/// `Ok(())` on success, or an error string. +pub fn undo_commit( + repo_path: &str, + workspace_id: i64, + commit_change_id: &str, +) -> Result<(), String> { + let workspace = local_db::get_workspace_by_id(repo_path, workspace_id) + .map_err(|e| format!("Failed to get workspace: {}", e))? + .ok_or_else(|| format!("Workspace not found: {}", workspace_id))?; + + let workspace_dir = Path::new(repo_path) + .join(".treq") + .join("workspaces") + .join(&workspace.workspace_path); + let workspace_dir_str = workspace_dir + .to_str() + .ok_or("Failed to convert workspace path to string")?; + + let default_branch = jj::get_default_branch(repo_path) + .map_err(|e| format!("Failed to resolve default branch: {}", e))?; + let target_branch = workspace + .target_branch + .as_deref() + .unwrap_or(&default_branch); + + jj::jj_undo_commit(workspace_dir_str, target_branch, commit_change_id) + .map_err(|e| format!("Failed to undo commit: {}", e))?; + + jj::update_stale_workspace(workspace_dir_str) + .map_err(|e| format!("Failed to update workspace working copy: {}", e))?; + + Ok(()) +} + +/// Reverts a specific commit by change-id, creating a new commit that reverses +/// its changes on top of the workspace's current tip. Can target any real +/// commit reachable from the workspace except the working-copy commit itself. +/// +/// # Arguments +/// * `repo_path` - Path to the repository root +/// * `workspace_id` - ID of the workspace that owns the commit +/// * `commit_change_id` - The short change-id of the commit to revert +/// +/// # Returns +/// `Ok(())` on success, or an error string. +pub fn revert_commit( + repo_path: &str, + workspace_id: i64, + commit_change_id: &str, +) -> Result<(), String> { + let workspace = local_db::get_workspace_by_id(repo_path, workspace_id) + .map_err(|e| format!("Failed to get workspace: {}", e))? + .ok_or_else(|| format!("Workspace not found: {}", workspace_id))?; + + let workspace_dir = Path::new(repo_path) + .join(".treq") + .join("workspaces") + .join(&workspace.workspace_path); + let workspace_dir_str = workspace_dir + .to_str() + .ok_or("Failed to convert workspace path to string")?; + + jj::jj_revert_commit(workspace_dir_str, commit_change_id) + .map_err(|e| format!("Failed to revert commit: {}", e))?; + + jj::update_stale_workspace(workspace_dir_str) + .map_err(|e| format!("Failed to update workspace working copy: {}", e))?; + + Ok(()) +} + /// Returns the full (multi-line) description of a specific commit. /// /// # Arguments diff --git a/src-tauri/src/jj.rs b/src-tauri/src/jj.rs index 44b33c638..7a3986c04 100644 --- a/src-tauri/src/jj.rs +++ b/src-tauri/src/jj.rs @@ -60,7 +60,7 @@ use jj_lib::git; use jj_lib::gitignore::GitIgnoreFile; use jj_lib::lock::FileLock; use jj_lib::matchers::{Matcher, NothingMatcher, PrefixMatcher}; -use jj_lib::merge::Diff; +use jj_lib::merge::{Diff, Merge}; use jj_lib::merged_tree::MergedTree; use jj_lib::merged_tree_builder::MergedTreeBuilder; use jj_lib::object_id::{HexPrefix, ObjectId}; @@ -2687,6 +2687,144 @@ pub fn jj_undo_operation(workspace_path: &str, operation_id: &str) -> Result Result { + validate_branch_name(target_branch, "target")?; + let loaded = load_workspace_repo(workspace_path)?; + let commit = resolve_commit_by_revision(&loaded, change_id)?; + let tip = resolve_commit_by_revision(&loaded, "@-")?; + if commit.id() != tip.id() { + return Err(JjError::IoError( + "Only the latest commit in the workspace can be undone. Undo commits sequentially, starting from the most recent.".to_string(), + )); + } + + let target_symbol = resolve_target_branch_symbol(&loaded, workspace_path, target_branch)?; + let target_commit = resolve_commit_by_revision(&loaded, &target_symbol)?; + let tip_is_on_target = loaded + .repo + .index() + .is_ancestor(tip.id(), target_commit.id()) + .map_err(|e| JjError::IoError(format!("Failed ancestry check: {}", e)))?; + if tip_is_on_target { + return Err(JjError::IoError( + "Cannot undo a commit that belongs to the target branch".to_string(), + )); + } + + drop(loaded); + jj_abandon(workspace_path, change_id) +} + +/// Create a new commit that reverses the changes of `change_id`, applied on top of +/// the workspace's current tip commit. Can target any real commit reachable from +/// the workspace (including immutable or target-branch commits) except the +/// working-copy commit itself. +pub fn jj_revert_commit(workspace_path: &str, change_id: &str) -> Result { + let loaded = load_workspace_repo(workspace_path)?; + let commit = resolve_commit_by_revision(&loaded, change_id)?; + + let workspace_name = loaded.workspace.workspace_name().to_owned(); + let wc_commit_id = loaded + .repo + .view() + .get_wc_commit_id(&workspace_name) + .cloned() + .ok_or_else(|| JjError::IoError("Workspace has no working-copy commit".to_string()))?; + if commit.id() == &wc_commit_id { + return Err(JjError::IoError("Cannot revert the working copy".to_string())); + } + + let wc_commit = loaded + .repo + .store() + .get_commit(&wc_commit_id) + .map_err(|e| JjError::IoError(format!("Failed to load working-copy commit: {}", e)))?; + let wc_parents = block_on(wc_commit.parents()) + .map_err(|e| JjError::InitFailed(format!("Failed to load working-copy parents: {}", e)))?; + let head_commit = wc_parents.into_iter().next().ok_or_else(|| { + JjError::IoError("Workspace working copy has no parent commit".to_string()) + })?; + + let parents = block_on(commit.parents()) + .map_err(|e| JjError::InitFailed(format!("Failed to load commit parents: {}", e)))?; + let parent_tree = block_on(merge_commit_trees(loaded.repo.as_ref(), &parents)) + .map_err(|e| JjError::InitFailed(format!("Failed to merge parent trees: {}", e)))?; + + // Diff from the commit's own tree back to its parent tree is exactly the + // reverse of the commit; applying it onto the current tip's tree (3-way + // merge, using the commit's own tree as the merge base) is a backout. + let reverted_diff = Diff::new( + (commit.tree(), commit.conflict_label()), + ( + parent_tree, + format!("{} (before revert)", commit.conflict_label()), + ), + ); + let new_tree = block_on(MergedTree::merge(Merge::from_diffs( + (head_commit.tree(), head_commit.conflict_label()), + vec![reverted_diff], + ))) + .map_err(|e| JjError::IoError(format!("Failed to compute reverted tree: {}", e)))?; + + let original_first_line = commit + .description() + .lines() + .next() + .unwrap_or("") + .to_string(); + let description = format!( + "Revert \"{}\"\n\nThis reverts commit {}.", + original_first_line, + commit.id().hex() + ); + + let mut tx = loaded.repo.start_transaction(); + + let new_commit = block_on( + tx.repo_mut() + .new_commit(vec![head_commit.id().clone()], new_tree) + .set_description(description) + .write(), + ) + .map_err(|e| JjError::IoError(format!("Failed to create revert commit: {}", e)))?; + + let rebased_wc = block_on(rewrite::rebase_commit( + tx.repo_mut(), + wc_commit, + vec![new_commit.id().clone()], + )) + .map_err(|e| JjError::IoError(format!("Failed to rebase working copy: {}", e)))?; + block_on(tx.repo_mut().edit(workspace_name, &rebased_wc)) + .map_err(|e| JjError::IoError(format!("Failed to update working-copy pointer: {}", e)))?; + + block_on(tx.repo_mut().rebase_descendants()) + .map_err(|e| JjError::InitFailed(format!("Failed to rebase descendants: {}", e)))?; + block_on(tx.commit("revert commit")) + .map_err(|e| JjError::InitFailed(format!("Failed to revert commit: {}", e)))?; + + // rebase_descendants() may have rewritten WC commits of every workspace; reconcile all. + let repo_path = derive_repo_path_from_workspace(workspace_path) + .unwrap_or_else(|| workspace_path.to_string()); + let _ = reconcile_all_workspaces_after_rewrite(&repo_path, None); + + Ok(String::new()) +} + /// Get the full (multi-line) description of a specific commit. pub fn jj_get_commit_description(workspace_path: &str, change_id: &str) -> Result { let loaded = load_workspace_repo(workspace_path)?; diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 31256e421..2c36597be 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -625,6 +625,8 @@ pub fn run() { commands::move_workspace_changes, commands::move_commit_to_existing_workspace, commands::abandon_commit, + commands::undo_commit, + commands::revert_commit, commands::rename_workspace, commands::list_workspace_statuses, commands::get_workspace_status, diff --git a/src-tauri/tests/core_commits_test.rs b/src-tauri/tests/core_commits_test.rs index 149649d29..7737fff5f 100644 --- a/src-tauri/tests/core_commits_test.rs +++ b/src-tauri/tests/core_commits_test.rs @@ -637,6 +637,172 @@ fn test_undo_repo_operation_rejects_stale_operation() { ); } +#[test] +fn test_undo_commit_only_allows_latest_commit_sequentially() { + let repo = TestRepo::new().expect("Failed to create test repo"); + let default_branch = repo.default_branch(); + + let workspace = treq_lib::core::create_workspace( + &repo.repo_path, + "feat/undo-test", + Some("undo test".to_string()), + None, + None, + None, + None, + ) + .expect("Failed to create workspace"); + + let workspace_path = repo.workspaces_dir().join(&workspace.workspace_path); + let workspace_path_str = workspace_path.to_str().unwrap(); + + TestRepo::write_workspace_file(workspace_path_str, "first.txt", "first") + .expect("Failed to write first.txt"); + treq_lib::core::commit_workspace(&repo.repo_path, workspace.id, "First commit") + .expect("Failed to commit first change"); + + TestRepo::write_workspace_file(workspace_path_str, "second.txt", "second") + .expect("Failed to write second.txt"); + treq_lib::core::commit_workspace(&repo.repo_path, workspace.id, "Second commit") + .expect("Failed to commit second change"); + + let commits_ahead = treq_lib::jj::jj_get_commits_ahead(workspace_path_str, default_branch) + .expect("Failed to get commits ahead"); + assert_eq!(commits_ahead.commits.len(), 2, "Should have 2 commits ahead"); + let newest_change_id = commits_ahead.commits[0].change_id.clone(); + let oldest_change_id = commits_ahead.commits[1].change_id.clone(); + + // Cannot undo an older commit while a newer commit still sits on top of it. + let err = treq_lib::core::undo_commit(&repo.repo_path, workspace.id, &oldest_change_id) + .expect_err("Should not allow undoing a non-tip commit"); + assert!( + err.contains("Only the latest commit"), + "unexpected error: {}", + err + ); + + // Undoing the tip commit succeeds. + treq_lib::core::undo_commit(&repo.repo_path, workspace.id, &newest_change_id) + .expect("Failed to undo tip commit"); + + let commits_after = treq_lib::jj::jj_get_commits_ahead(workspace_path_str, default_branch) + .expect("Failed to get commits after undo"); + assert_eq!(commits_after.commits.len(), 1); + assert_eq!(commits_after.commits[0].change_id, oldest_change_id); + assert!(!workspace_path.join("second.txt").exists()); + + // The now-tip commit (previously the older one) can be undone in turn. + treq_lib::core::undo_commit(&repo.repo_path, workspace.id, &oldest_change_id) + .expect("Failed to undo the now-latest commit"); + + let commits_final = treq_lib::jj::jj_get_commits_ahead(workspace_path_str, default_branch) + .expect("Failed to get commits after second undo"); + assert!(commits_final.commits.is_empty()); + assert!(!workspace_path.join("first.txt").exists()); +} + +#[test] +fn test_undo_commit_rejects_commit_on_target_branch() { + let repo = TestRepo::new().expect("Failed to create test repo"); + let default_branch = repo.default_branch(); + + let workspace = treq_lib::core::create_workspace( + &repo.repo_path, + "feat/undo-target-test", + Some("undo target test".to_string()), + None, + None, + None, + None, + ) + .expect("Failed to create workspace"); + + // No new commits made in this workspace — its tip is the target branch tip itself. + let err = treq_lib::core::undo_commit(&repo.repo_path, workspace.id, default_branch) + .expect_err("Should not allow undoing a commit that belongs to the target branch"); + assert!( + err.contains("target branch"), + "unexpected error: {}", + err + ); +} + +#[test] +fn test_revert_commit_creates_backout_commit() { + let repo = TestRepo::new().expect("Failed to create test repo"); + let default_branch = repo.default_branch(); + + let workspace = treq_lib::core::create_workspace( + &repo.repo_path, + "feat/revert-test", + Some("revert test".to_string()), + None, + None, + None, + None, + ) + .expect("Failed to create workspace"); + + let workspace_path = repo.workspaces_dir().join(&workspace.workspace_path); + let workspace_path_str = workspace_path.to_str().unwrap(); + + TestRepo::write_workspace_file(workspace_path_str, "revert-me.txt", "hello") + .expect("Failed to write revert-me.txt"); + treq_lib::core::commit_workspace(&repo.repo_path, workspace.id, "Add revert-me") + .expect("Failed to commit"); + + let commits_ahead = treq_lib::jj::jj_get_commits_ahead(workspace_path_str, default_branch) + .expect("Failed to get commits ahead"); + let change_id = commits_ahead.commits[0].change_id.clone(); + + treq_lib::core::revert_commit(&repo.repo_path, workspace.id, &change_id) + .expect("Failed to revert commit"); + + let commits_after = treq_lib::jj::jj_get_commits_ahead(workspace_path_str, default_branch) + .expect("Failed to get commits after revert"); + assert_eq!( + commits_after.commits.len(), + 2, + "Should have the original commit plus the new revert commit" + ); + assert!( + commits_after.commits[0] + .description + .starts_with("Revert \"Add revert-me\""), + "unexpected description: {}", + commits_after.commits[0].description + ); + + assert!( + !workspace_path.join("revert-me.txt").exists(), + "revert-me.txt should be removed by the revert commit" + ); +} + +#[test] +fn test_revert_commit_rejects_working_copy() { + let repo = TestRepo::new().expect("Failed to create test repo"); + + let workspace = treq_lib::core::create_workspace( + &repo.repo_path, + "feat/revert-wc-test", + Some("revert wc test".to_string()), + None, + None, + None, + None, + ) + .expect("Failed to create workspace"); + + let err = treq_lib::core::revert_commit(&repo.repo_path, workspace.id, "@") + .expect_err("Should not allow reverting the working copy"); + assert!( + err.contains("Cannot revert the working copy"), + "unexpected error: {}", + err + ); +} + #[test] fn test_commit_diff_added_files() { let repo = TestRepo::new().expect("Failed to create test repo"); diff --git a/src/components/CommitDiffViewer.tsx b/src/components/CommitDiffViewer.tsx index 98c6ff31c..096c0d866 100644 --- a/src/components/CommitDiffViewer.tsx +++ b/src/components/CommitDiffViewer.tsx @@ -13,7 +13,9 @@ import { Pencil, Plus, Clock, + RotateCcw, Trash2, + Undo2, } from "lucide-react"; import { Fragment, useEffect, useRef, useState } from "react"; import { @@ -25,8 +27,10 @@ import { type JjLogCommit, type JjRevisionDiff, listCommits, + revertCommit, shiftMutableCommitsToNow, stashCommit, + undoCommit, undoRepoOperation, } from "../lib/api"; import { resolvableConflictedCommits } from "../lib/resolvable-conflicted-commits"; @@ -571,6 +575,79 @@ export const CommitDiffViewer = ({ } }; + const handleUndo = async (commit: JjLogCommit) => { + if (!repoPath || !workspaceId) return; + + const firstLine = commit.description.split("\n")[0] || "(no message)"; + const confirmed = await ask( + `Undo commit ${commit.short_id} — ${firstLine}? Its changes will be returned to your working copy.`, + { title: "Undo Commit", kind: "warning" }, + ); + if (!confirmed) return; + + try { + await undoCommit(repoPath, workspaceId, commit.change_id); + setRemovingCommitIds((prev) => new Set(prev).add(commit.commit_id)); + + window.setTimeout(() => { + setRemovedCommitIds((prev) => new Set(prev).add(commit.commit_id)); + setRemovingCommitIds((prev) => { + const next = new Set(prev); + next.delete(commit.commit_id); + return next; + }); + onCommitAbandoned?.(); + }, REMOVE_ANIMATION_MS); + + addToast({ + title: "Commit undone", + description: `Undid commit ${commit.short_id}`, + type: "success", + }); + } catch (err) { + const errorMsg = err instanceof Error ? err.message : String(err); + addToast({ + title: "Failed to undo commit", + description: errorMsg, + type: "error", + }); + } + }; + + const handleRevert = async (commit: JjLogCommit) => { + if (!repoPath || !workspaceId) return; + + const firstLine = commit.description.split("\n")[0] || "(no message)"; + const confirmed = await ask( + `Revert commit ${commit.short_id} — ${firstLine}? This creates a new commit that reverses its changes.`, + { title: "Revert Commit", kind: "warning" }, + ); + if (!confirmed) return; + + try { + await revertCommit(repoPath, workspaceId, commit.change_id); + void invalidateQueries([ + "commit-diff-viewer-commits", + repoPath, + workspaceId, + ]); + onCommitAbandoned?.(); + + addToast({ + title: "Commit reverted", + description: `Reverted commit ${commit.short_id}`, + type: "success", + }); + } catch (err) { + const errorMsg = err instanceof Error ? err.message : String(err); + addToast({ + title: "Failed to revert commit", + description: errorMsg, + type: "error", + }); + } + }; + // Scroll to commit when scrollToCommitId changes useEffect(() => { if (!scrollToCommitId || loading) return; @@ -884,6 +961,8 @@ export const CommitDiffViewer = ({ onMoveToExisting={handleMoveToExisting} onAbandon={handleAbandon} onStash={handleStashCommit} + onUndo={handleUndo} + onRevert={handleRevert} onEditDescription={handleEditDescription} onEditTimestamp={handleEditTimestamp} onResolveConflict={ @@ -988,6 +1067,7 @@ export const CommitDiffViewer = ({ onMoveToExisting={() => {}} onAbandon={() => {}} onStash={() => {}} + onRevert={handleRevert} onEditDescription={() => {}} onEditTimestamp={() => {}} onCreateAgentWithComment={onCreateAgentWithComment} @@ -1105,6 +1185,10 @@ interface CommitWithDiffProps { onMoveToExisting: (commit: JjLogCommit) => void; onAbandon: (commit: JjLogCommit) => void; onStash: (commit: JjLogCommit) => void; + /** Undo the latest commit in the workspace's own lineage. Only valid when `isFirst`. */ + onUndo?: (commit: JjLogCommit) => void; + /** Revert any commit (except the working copy) by creating a new commit that reverses it. */ + onRevert?: (commit: JjLogCommit) => void; onEditDescription: (commit: JjLogCommit) => void; onEditTimestamp: (commit: JjLogCommit) => void; onResolveConflict?: (commit: JjLogCommit) => void; @@ -1134,6 +1218,8 @@ function CommitWithDiff({ onMoveToExisting, onAbandon, onStash, + onUndo, + onRevert, onEditDescription, onEditTimestamp, onResolveConflict, @@ -1309,6 +1395,29 @@ function CommitWithDiff({ Edit description + {isFirst && onUndo && ( + + )} + {onRevert && ( + + )} {canAction && !commit.is_immutable && (