From 56f7c705417ff4dd9bfa05bcf68ce0b8750b508c Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 16 Aug 2026 02:03:28 +0800 Subject: [PATCH] fix(git): make a new op state whether it can lose work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `GitOp::destructive` decides whether an operation gets a confirmation before it runs, and it ended in `_ => return None`. The default of that catch-all is "needs no confirmation", so a verb added later reaches the user ungated and takes their work with it — the one direction a safety gate must not fail in. A new variant does already break the build, but on `label` above, which asks a different question and is answered by adding a string. Nothing then asks about this one, and `destructive_marks_exactly_what_can_lose_work` cannot: it walks two hand-written lists, so an op in neither passes. Spelling the safe ones out puts the compile error at the place the answer belongs. It also says out loud what the `_` was hiding — that `Pull`, `CherryPick`, `Revert` and a lease-less `Push` were each considered and found harmless, rather than never considered at all. No behaviour change: every variant returns exactly what it returned before. Confirmed the guard fires for the case it exists for — a probe variant is now an E0004 at `destructive` as well as at `label`. --- crates/tty7-core/src/core/git/ops.rs | 31 +++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/crates/tty7-core/src/core/git/ops.rs b/crates/tty7-core/src/core/git/ops.rs index b7d6e16a..6df2b34a 100644 --- a/crates/tty7-core/src/core/git/ops.rs +++ b/crates/tty7-core/src/core/git/ops.rs @@ -221,7 +221,36 @@ impl GitOp { force_with_lease: true, .. } => Destructive::RewritesHistory, - _ => return None, + + // Spelled out rather than left to a `_`, because the default of a + // catch-all here is "needs no confirmation": a verb added later + // would reach the user ungated, and the compile error that a new + // variant does produce lands on `label` above, which is a + // different question. Listing them makes the compiler ask this + // one at the place the answer belongs. + // + // None of these can lose committed or staged work. `Reset` here is + // soft or mixed, which move the branch pointer and leave the + // worktree alone; `CherryPick` and `Revert` only ever add; `Stash` + // files changes away where `stash pop` gets them back; a `Push` + // without the lease is refused by the remote rather than + // overwriting it; and git itself refuses a `Checkout` that would + // clobber local edits. + GitOp::Stage { .. } + | GitOp::StageAll + | GitOp::Unstage { .. } + | GitOp::UnstageAll + | GitOp::Commit { .. } + | GitOp::CheckoutBranch { .. } + | GitOp::CheckoutDetached { .. } + | GitOp::CreateBranch { .. } + | GitOp::CherryPick { .. } + | GitOp::Revert { .. } + | GitOp::Reset { .. } + | GitOp::Stash { .. } + | GitOp::Fetch { .. } + | GitOp::Pull { .. } + | GitOp::Push { .. } => return None, }) }