From d0a37dfd1f81451ab04ecca413858680a43148fc Mon Sep 17 00:00:00 2001 From: Daniels-Main Date: Sun, 6 Sep 2026 15:44:09 +0200 Subject: [PATCH 1/5] feat: add validated patch mailbox and bundle interchange --- README.md | 4 + ROADMAP.md | 13 +- TASKS.md | 5 +- crates/strand-core/src/history.rs | 2 + crates/strand-core/src/interchange.rs | 942 ++++++++++++++++++++++++++ crates/strand-core/src/lib.rs | 1 + crates/strand-core/src/repo.rs | 6 +- crates/strand-core/src/watch.rs | 2 + crates/strand-tauri/src/commands.rs | 35 + crates/strand-tauri/src/main.rs | 7 + docs/learnings.md | 12 + ui/src/App.tsx | 13 +- ui/src/lib/interchange.test.ts | 25 + ui/src/lib/interchange.ts | 5 + ui/src/lib/menu.ts | 2 + ui/src/lib/tauri.ts | 8 + ui/src/lib/types.ts | 2 +- ui/src/styles/features.css | 7 + ui/src/views/InterchangeDialog.tsx | 139 ++++ website/docs/everyday-git.md | 32 + 20 files changed, 1254 insertions(+), 8 deletions(-) create mode 100644 crates/strand-core/src/interchange.rs create mode 100644 ui/src/lib/interchange.test.ts create mode 100644 ui/src/lib/interchange.ts create mode 100644 ui/src/views/InterchangeDialog.tsx diff --git a/README.md b/README.md index 40ac8df7..0f917c69 100644 --- a/README.md +++ b/README.md @@ -231,6 +231,10 @@ the resolved app appearance automatically. merge, and a fully keyboard-operable interactive rebase (reorder, reword, edit/pause-to-amend, squash, fixup, drop, and merge preservation) with pause/conflict Continue / Abort. +- **Patch interchange** — preview affected paths and validate patch imports + into the working tree, index, or both; import mailboxes with original authors + and Continue / Skip / Abort recovery; verify bundle refs and prerequisites, + import into a new branch, and export full or incremental bundles. - **Commit graph** — SVG lanes with branch/tag chips, revealable inline stash nodes with non-mutating diff inspection, a resizable commit detail panel with lazy GPG/SSH/X.509 verification, diff --git a/ROADMAP.md b/ROADMAP.md index f048b0bf..8a1ae0a2 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2163,7 +2163,9 @@ and Store certification remain external gates. retention policy, and backend justify adding it - Guided Git bisect - Sparse checkout (cone mode first) -- Patch import/mailbox and Git bundle workflows +- ☑ Patch import/mailbox and Git bundle workflows (F07: `InterchangeDialog`, + validation, author-preserving mailbox recovery and new-branch bundle import; + native desktop flows verified) - Expanded submodule lifecycle (add/remove/deinit/sync/URL/nested status) - Repository/ref/file custom actions with safe argv templates - **CLI companion binary (`strand`)** — `strand ` opens the repo @@ -2810,6 +2812,15 @@ GitHub/Azure review, Workbench and performance work retain their own status. --- +**Patch interchange shipped (2026-09-06):** Added affected-path previews, +explicit index/worktree targets, validated import, original-author mailboxes +with Continue/Skip/Abort, and bundle verification/import/export with +prerequisite/ref summaries. Native fixtures cover stale previews, all patch +targets, mailbox authors/recovery and missing bundle prerequisites. Rust +checks, five fixtures and three frontend IPC tests pass. Native WebView2 +verified palette/focus, worktree/index targets, mailbox authors and conflict +continuation, and bundle verification/import/incremental export. + ## Cross-cutting tracks (run in parallel with all milestones) **Performance audit kick (2026-09-06):** Rechecked `main` at `8e83c8c` on diff --git a/TASKS.md b/TASKS.md index 4c7910bf..bcd6304f 100644 --- a/TASKS.md +++ b/TASKS.md @@ -106,9 +106,12 @@ Detailed comparison and sequencing: [`docs/git-client-1.0-audit.md`](./docs/git- - ☐ **F05 / P1 — Submodule lifecycle.** Extend existing open/status/init/update with add/remove/deinit/sync/URL/nested inspection; verify dirty-state handling, `.gitmodules` and index changes, plus cancellable network operations. -- ☐ **F07 / P2 — Patch/mailbox/bundle import and interchange.** Build on exact +- ☑ **F07 / P2 — Patch/mailbox/bundle import and interchange.** Build on exact patch export and hunk apply with preview/validation, explicit targets, mailbox continue/skip/abort and bundle prerequisites/ref summaries. + Implemented (`interchange.rs`, `InterchangeDialog`, Repository menu/palette); + five native fixtures and three IPC tests pass. Native WebView2 verified patch + targets, authored mailbox/conflict continuation and bundle import/export. - ☐ **F08 / P2 — Sparse checkout.** Cone-directory inspect/change/disable and compatibility fixtures for excluded paths, dirty trees and sparse indexes. - ☐ **F09 / P2 — Advanced clone options.** Branch, depth/single-branch, diff --git a/crates/strand-core/src/history.rs b/crates/strand-core/src/history.rs index 7b3a02ed..3ba99c14 100644 --- a/crates/strand-core/src/history.rs +++ b/crates/strand-core/src/history.rs @@ -370,6 +370,7 @@ impl Repo { .operation_in_progress() .ok_or_else(|| Error::Other("no operation in progress to continue".into()))?; let cmd = match op.as_str() { + "mailbox" => "am", "rebase" => "rebase", "cherry-pick" => "cherry-pick", "revert" => "revert", @@ -428,6 +429,7 @@ impl Repo { .operation_in_progress() .ok_or_else(|| Error::Other("no operation in progress to abort".into()))?; let cmd = match op.as_str() { + "mailbox" => "am", "rebase" => "rebase", "cherry-pick" => "cherry-pick", "revert" => "revert", diff --git a/crates/strand-core/src/interchange.rs b/crates/strand-core/src/interchange.rs new file mode 100644 index 00000000..a5ef4194 --- /dev/null +++ b/crates/strand-core/src/interchange.rs @@ -0,0 +1,942 @@ +//! Explicit, previewed patch/mailbox/bundle interchange. Reads stay off snapshots. + +use crate::{Error, Repo, Result}; +use serde::{Deserialize, Serialize}; +use std::{ + collections::BTreeSet, + fs, + io::{Read, Write}, + path::{Path, PathBuf}, + process::{Output, Stdio}, +}; + +const MAX_PATCH: u64 = 32 * 1024 * 1024; + +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum PatchTarget { + Worktree, + Index, + Both, + Mailbox, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct PatchPreview { + pub token: String, + pub paths: Vec, + pub messages: Vec, + pub valid: bool, + pub validation: String, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct MailboxState { + pub token: String, + pub current: String, + pub total: String, + pub author: String, + pub conflicts: bool, +} + +#[derive(Debug, Clone, Copy, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum MailboxAction { + Continue, + Skip, + Abort, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct InterchangeOutcome { + pub success: bool, + pub paused: bool, + pub output: String, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct BundleRef { + pub oid: String, + pub name: String, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct BundlePreview { + pub token: String, + pub refs: Vec, + pub prerequisites: Vec, + pub valid: bool, + pub validation: String, +} + +// Private temporary directory, independent of filenames from imported content. +pub(crate) struct InterchangeScratch(pub PathBuf); +impl InterchangeScratch { + pub(crate) fn new() -> Result { + use std::sync::atomic::{AtomicU64, Ordering}; + static NEXT: AtomicU64 = AtomicU64::new(0); + for _ in 0..100 { + let p = std::env::temp_dir().join(format!( + "strand-interchange-{}-{}", + std::process::id(), + NEXT.fetch_add(1, Ordering::Relaxed) + )); + match fs::create_dir(&p) { + Ok(()) => return Ok(Self(p)), + Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => continue, + Err(e) => return Err(e.into()), + } + } + Err(Error::Other( + "cannot allocate interchange scratch directory".into(), + )) + } +} +impl Drop for InterchangeScratch { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.0); + } +} + +fn git(cwd: &Path, args: &[&str], stdin: Option<&Path>, index: Option<&Path>) -> Result { + let mut cmd = crate::git_command(); + cmd.current_dir(cwd) + .args(crate::GIT_SAFE_CONFIG) + .args(args) + .env("GIT_EDITOR", "true") + .env("GIT_TERMINAL_PROMPT", "0") + .stdin(Stdio::null()); + if let Some(p) = stdin { + cmd.stdin(fs::File::open(p)?); + } + if let Some(p) = index { + cmd.env("GIT_INDEX_FILE", p); + } + let mut child = cmd.stdout(Stdio::piped()).stderr(Stdio::piped()).spawn()?; + fn drain(mut pipe: impl Read) -> std::io::Result> { + let mut captured = Vec::new(); + let mut buf = [0u8; 8192]; + loop { + let n = pipe.read(&mut buf)?; + if n == 0 { + break; + } + let keep = n.min((1024 * 1024usize).saturating_sub(captured.len())); + captured.extend_from_slice(&buf[..keep]); + } + Ok(captured) + } + let stdout = child.stdout.take().expect("piped stdout"); + let stderr = child.stderr.take().expect("piped stderr"); + let reader = std::thread::spawn(move || drain(stdout)); + let stderr = drain(stderr)?; + let status = child.wait()?; + let stdout = reader + .join() + .map_err(|_| Error::Other("Git output reader failed".into()))??; + Ok(Output { + status, + stdout, + stderr, + }) +} + +fn diagnostic(out: &Output) -> String { + let mut bytes = out + .stdout + .iter() + .chain(out.stderr.iter()) + .take(64 * 1024) + .copied() + .collect::>(); + if out.stdout.len() + out.stderr.len() > bytes.len() { + bytes.extend_from_slice(b"\n[output truncated]"); + } + String::from_utf8_lossy(&bytes).trim().to_owned() +} +fn checked(out: Output) -> Result { + if out.status.success() { + Ok(out) + } else { + Err(Error::Other(diagnostic(&out))) + } +} +fn utf8_path(p: &Path) -> Result<&str> { + p.to_str() + .ok_or_else(|| Error::Other("path is not UTF-8".into())) +} +fn digest(bytes: &[u8]) -> Result { + Ok(git2::Oid::hash_object(git2::ObjectType::Blob, bytes)?.to_string()) +} +fn bounded_read(p: &Path, limit: u64) -> Result> { + let mut data = Vec::new(); + fs::File::open(p)?.take(limit + 1).read_to_end(&mut data)?; + if data.len() as u64 > limit { + return Err(Error::Other(format!( + "{} exceeds the {} MiB import limit", + p.display(), + limit / 1024 / 1024 + ))); + } + Ok(data) +} + +/// Concurrency stamp, not an authentication hash. Streams large bundle files. +fn file_stamp(p: &Path) -> Result { + use std::hash::Hasher; + let mut hash = std::collections::hash_map::DefaultHasher::new(); + let mut file = fs::File::open(p)?; + let mut buf = [0u8; 65536]; + loop { + let n = file.read(&mut buf)?; + if n == 0 { + break; + } + hash.write(&buf[..n]); + } + Ok(format!("{:016x}", hash.finish())) +} + +impl Repo { + // Includes actual file bytes, because equal status rows do not mean equal content. + fn import_stamp(&self, paths: &[String], extra: &[u8]) -> Result { + let mut stamp = extra.to_vec(); + for name in [ + "HEAD", + "index", + "rebase-apply/next", + "rebase-apply/last", + "rebase-apply/info", + "rebase-apply/patch", + ] { + let p = self.git_dir().join(name); + if p.is_file() { + stamp.extend_from_slice(file_stamp(&p)?.as_bytes()); + } + } + stamp.extend_from_slice( + format!("{:?}", self.git2()?.head().ok().and_then(|h| h.target())).as_bytes(), + ); + for p in paths { + self.check_import_path(p)?; + stamp.extend_from_slice(p.as_bytes()); + let full = self.path.join(p); + if full.is_file() { + stamp.extend_from_slice(file_stamp(&full)?.as_bytes()); + } + } + digest(&stamp) + } + + /// Reject nonportable traversal, Git administrative paths, and symlink ancestors, + /// including dangling links and not-yet-created nested directories. + fn check_import_path(&self, path: &str) -> Result<()> { + if path.is_empty() || path.contains(['\\', ':', '\0']) || path.starts_with('/') { + return Err(Error::Other(format!("unsafe patch path: {path}"))); + } + let mut full = self.path.clone(); + for part in path.split('/') { + if part.is_empty() + || part == "." + || part == ".." + || part.eq_ignore_ascii_case(".git") + || part + .trim_end_matches([' ', '.']) + .eq_ignore_ascii_case(".git") + { + return Err(Error::Other(format!("unsafe patch path: {path}"))); + } + full.push(part); + match fs::symlink_metadata(&full) { + Ok(m) if m.file_type().is_symlink() => { + return Err(Error::Other(format!( + "patch path traverses a symlink: {path}" + ))) + } + Ok(_) => { + if !full.canonicalize()?.starts_with(self.path.canonicalize()?) { + return Err(Error::Other(format!( + "patch path escapes repository: {path}" + ))); + } + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => return Err(e.into()), + } + } + Ok(()) + } + + fn patch_paths(&self, bytes: &[u8]) -> Result> { + let diff = git2::Diff::from_buffer(bytes)?; + let mut paths = BTreeSet::new(); + for delta in diff.deltas() { + for file in [delta.old_file(), delta.new_file()] { + // Applying links can change containment of a later patch in a series. + if file.mode() == git2::FileMode::Link { + return Err(Error::Other( + "importing symlink patches is not supported; inspect and apply with Git" + .into(), + )); + } + if let Some(p) = file.path() { + let p = utf8_path(p)?; + self.check_import_path(p)?; + paths.insert(p.to_owned()); + } + } + } + if paths.is_empty() { + return Err(Error::Other("patch contains no affected paths".into())); + } + Ok(paths.into_iter().collect()) + } + + pub fn preview_patch_import(&self, source: &Path, target: PatchTarget) -> Result { + if !source.is_absolute() { + return Err(Error::Other("patch source must be an absolute path".into())); + } + let bytes = bounded_read(source, MAX_PATCH)?; + self.preview_patch_bytes(&bytes, target) + } + + fn preview_patch_bytes(&self, bytes: &[u8], target: PatchTarget) -> Result { + let scratch = InterchangeScratch::new()?; + let input = scratch.0.join("input"); + fs::write(&input, bytes)?; + let mut paths = BTreeSet::new(); + let mut messages = Vec::new(); + let mut valid = true; + let mut validation = String::new(); + if target == PatchTarget::Mailbox { + let maildir = scratch.0.join("mail"); + fs::create_dir(&maildir)?; + let outdir = format!("-o{}", utf8_path(&maildir)?); + let split = checked(git( + &self.path, + &["mailsplit", "-b", &outdir, "--", utf8_path(&input)?], + None, + None, + )?)?; + let count: usize = String::from_utf8_lossy(&split.stdout) + .trim() + .parse() + .map_err(|_| Error::Other("invalid mailbox count".into()))?; + if count == 0 || count > 1000 { + return Err(Error::Other("mailbox must contain 1–1000 patches".into())); + } + let index = scratch.0.join("index"); + checked(git(&self.path, &["read-tree", "HEAD"], None, Some(&index))?)?; + for n in 1..=count { + let mail = maildir.join(format!("{n:04}")); + let patch = scratch.0.join("patch"); + let message = scratch.0.join("message"); + let info = checked(git( + &self.path, + &["mailinfo", utf8_path(&message)?, utf8_path(&patch)?], + Some(&mail), + None, + )?)?; + messages.push(diagnostic(&info)); + paths.extend(self.patch_paths(&bounded_read(&patch, MAX_PATCH)?)?); + if valid { + let out = git( + &self.path, + &["apply", "--cached", "--", utf8_path(&patch)?], + None, + Some(&index), + )?; + if !out.status.success() { + valid = false; + validation = format!("Patch {n}: {}", diagnostic(&out)); + } + } + } + } else { + paths.extend(self.patch_paths(bytes)?); + let mut args = vec!["apply", "--check"]; + match target { + PatchTarget::Index => args.push("--cached"), + PatchTarget::Both => args.push("--index"), + _ => {} + } + args.extend(["--", utf8_path(&input)?]); + let out = git(&self.path, &args, None, None)?; + valid = out.status.success(); + validation = diagnostic(&out); + } + let paths: Vec<_> = paths.into_iter().collect(); + let token = + self.import_stamp(&paths, format!("{target:?}:{}", digest(bytes)?).as_bytes())?; + Ok(PatchPreview { + token, + paths, + messages, + valid, + validation, + }) + } + + pub fn import_patch( + &self, + source: &Path, + target: PatchTarget, + token: &str, + ) -> Result { + if !source.is_absolute() { + return Err(Error::Other("patch source must be an absolute path".into())); + } + if let Some(op) = self.operation_in_progress() { + return Err(Error::Other(format!( + "finish or abort {op} before importing" + ))); + } + let bytes = bounded_read(source, MAX_PATCH)?; + let preview = self.preview_patch_bytes(&bytes, target)?; + if preview.token != token { + return Err(Error::Other( + "patch or repository changed; preview again".into(), + )); + } + if !preview.valid && target != PatchTarget::Mailbox { + return Err(Error::Other(preview.validation)); + } + let scratch = InterchangeScratch::new()?; + let input = scratch.0.join("input"); + fs::write(&input, bytes)?; + let mut args = match target { + PatchTarget::Mailbox => { + let out = checked(git( + &self.path, + &["status", "--porcelain", "--untracked-files=normal"], + None, + None, + )?)?; + if !out.stdout.is_empty() { + return Err(Error::Other("mailbox import requires a clean index and working tree; commit or stash changes first".into())); + } + vec!["am", "--3way"] + } + PatchTarget::Index => vec!["apply", "--cached"], + PatchTarget::Both => vec!["apply", "--index"], + PatchTarget::Worktree => vec!["apply"], + }; + args.extend(["--", utf8_path(&input)?]); + let out = git(&self.path, &args, None, None)?; + Ok(InterchangeOutcome { + success: out.status.success(), + paused: self.operation_in_progress().as_deref() == Some("mailbox"), + output: diagnostic(&out), + }) + } + + pub fn mailbox_state(&self) -> Result> { + if self.operation_in_progress().as_deref() != Some("mailbox") { + return Ok(None); + } + let dir = self.git_dir().join("rebase-apply"); + let read = |n: &str| -> Result { + Ok( + String::from_utf8_lossy(&bounded_read(&dir.join(n), MAX_PATCH)?) + .trim() + .to_owned(), + ) + }; + let mut index = self.git2()?.index()?; + index.read(true)?; + Ok(Some(MailboxState { + token: self.import_stamp(&[], &[])?, + current: read("next")?, + total: read("last")?, + author: read("info").unwrap_or_default(), + conflicts: index.has_conflicts(), + })) + } + + pub fn mailbox_action(&self, action: MailboxAction, token: &str) -> Result { + let state = self + .mailbox_state()? + .ok_or_else(|| Error::Other("no mailbox operation in progress".into()))?; + if state.token != token { + return Err(Error::Other( + "mailbox state changed; refresh before continuing".into(), + )); + } + if matches!(action, MailboxAction::Continue) && state.conflicts { + return Err(Error::Other( + "resolve and stage every conflict first".into(), + )); + } + let arg = match action { + MailboxAction::Continue => "--continue", + MailboxAction::Skip => "--skip", + MailboxAction::Abort => "--abort", + }; + let out = git(&self.path, &["am", arg], None, None)?; + Ok(InterchangeOutcome { + success: out.status.success(), + paused: self.operation_in_progress().as_deref() == Some("mailbox"), + output: diagnostic(&out), + }) + } + + pub fn preview_bundle(&self, source: &Path) -> Result { + if !source.is_absolute() { + return Err(Error::Other( + "bundle source must be an absolute path".into(), + )); + } + // Header is bounded even though pack data can be arbitrarily large. + let mut header = Vec::new(); + use std::io::BufRead; + let mut input = std::io::BufReader::new(fs::File::open(source)?); + loop { + let mut line = Vec::new(); + input + .by_ref() + .take(1024 * 1024 + 1) + .read_until(b'\n', &mut line)?; + if line.is_empty() { + return Err(Error::Other("incomplete bundle header".into())); + } + if header.len() + line.len() > 1024 * 1024 { + return Err(Error::Other("bundle header exceeds 1 MiB".into())); + } + header.extend_from_slice(&line); + if line == b"\n" { + break; + } + } + let text = String::from_utf8_lossy(&header); + let prerequisites = text + .lines() + .filter_map(|l| l.strip_prefix('-').map(str::to_owned)) + .collect(); + let heads = checked(git( + &self.path, + &["bundle", "list-heads", utf8_path(source)?], + None, + None, + )?)?; + let refs = String::from_utf8_lossy(&heads.stdout) + .lines() + .filter_map(|l| { + l.split_once(' ').map(|(oid, name)| BundleRef { + oid: oid.into(), + name: name.into(), + }) + }) + .collect(); + let out = git( + &self.path, + &["bundle", "verify", utf8_path(source)?], + None, + None, + )?; + Ok(BundlePreview { + token: file_stamp(source)?, + refs, + prerequisites, + valid: out.status.success(), + validation: diagnostic(&out), + }) + } + + /// Fetch exactly one reviewed bundle ref into a new local branch. Existing + /// refs/HEAD are never overwritten and Git owns object/prerequisite validation. + pub fn import_bundle( + &self, + source: &Path, + token: &str, + source_ref: &str, + branch: &str, + ) -> Result { + let scratch = InterchangeScratch::new()?; + let copy = scratch.0.join("input.bundle"); + fs::copy(source, ©)?; + let preview = self.preview_bundle(©)?; + if preview.token != token { + return Err(Error::Other("bundle changed; verify again".into())); + } + if !preview.valid { + return Err(Error::Other(preview.validation)); + } + let selected = preview + .refs + .iter() + .find(|r| r.name == source_ref) + .ok_or_else(|| Error::Other("choose an advertised bundle ref".into()))?; + let dest = format!("refs/heads/{branch}"); + if branch.starts_with('-') || !git2::Reference::is_valid_name(&dest) { + return Err(Error::Other("invalid destination branch".into())); + } + // Import objects without publishing a ref, then use a compare-and-swap + // create. A concurrent external branch creation must never be overwritten. + checked(git( + &self.path, + &["bundle", "unbundle", utf8_path(©)?], + None, + None, + )?)?; + let commit = self + .git2()? + .find_object(git2::Oid::from_str(&selected.oid)?, None)? + .peel_to_commit()? + .id(); + self.git2()? + .reference(&dest, commit, false, "Strand bundle import")?; + Ok(InterchangeOutcome { + success: true, + paused: false, + output: format!("Imported {} at {} into {dest}", selected.name, selected.oid), + }) + } + + /// Export a named ref, optionally excluding a prerequisite revision. Verify + /// the exported tip before publishing, so concurrent ref updates fail closed. + pub fn export_bundle( + &self, + destination: &Path, + refname: &str, + prerequisite: Option<&str>, + ) -> Result { + if !destination.is_absolute() || destination.exists() { + return Err(Error::Other( + "choose a new absolute bundle destination; existing files are never overwritten" + .into(), + )); + } + let parent = destination + .parent() + .ok_or_else(|| Error::Other("destination has no parent".into()))? + .canonicalize()?; + if parent.starts_with(self.git_dir().canonicalize()?) + || parent.starts_with(self.gix.common_dir().canonicalize()?) + { + return Err(Error::Other( + "bundle destination cannot be inside Git administrative directories".into(), + )); + } + let reference = self.git2()?.find_reference(refname)?; + let tip = reference.peel_to_commit()?.id().to_string(); + let base = prerequisite + .map(|r| { + self.git2()? + .revparse_single(r)? + .peel_to_commit() + .map(|c| format!("^{}", c.id())) + .map_err(Error::from) + }) + .transpose()?; + // git bundle requires a named positive ref. Verify it again after creation + // and fail without publishing the artifact if it moved during export. + let scratch = InterchangeScratch::new()?; + let output = scratch.0.join("export.bundle"); + let mut args = vec!["bundle", "create", utf8_path(&output)?, refname]; + if let Some(b) = base.as_deref() { + args.push(b); + } + checked(git(&self.path, &args, None, None)?)?; + let preview = self.preview_bundle(&output)?; + if !preview + .refs + .iter() + .any(|r| r.name == refname && r.oid == tip) + { + return Err(Error::Other("export ref changed; retry".into())); + } + let mut dest = fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(destination)?; + if let Err(e) = + std::io::copy(&mut fs::File::open(output)?, &mut dest).and_then(|_| dest.flush()) + { + drop(dest); + let _ = fs::remove_file(destination); + return Err(e.into()); + } + Ok(preview) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn fixture() -> (InterchangeScratch, Repo) { + let scratch = InterchangeScratch::new().unwrap(); + let repo = git2::Repository::init(&scratch.0).unwrap(); + let mut config = repo.config().unwrap(); + config.set_str("user.name", "Committer").unwrap(); + config + .set_str("user.email", "committer@example.test") + .unwrap(); + config.set_bool("commit.gpgsign", false).unwrap(); + config.set_bool("core.autocrlf", false).unwrap(); + config.set_str("core.hooksPath", "/dev/null").unwrap(); + fs::write(scratch.0.join("file.txt"), "one\n").unwrap(); + checked(git(&scratch.0, &["add", "."], None, None).unwrap()).unwrap(); + checked(git(&scratch.0, &["commit", "-m", "base"], None, None).unwrap()).unwrap(); + (scratch, Repo::discover(repo.workdir().unwrap()).unwrap()) + } + fn patch() -> &'static str { + "diff --git a/file.txt b/file.txt\n--- a/file.txt\n+++ b/file.txt\n@@ -1 +1 @@\n-one\n+two\n" + } + fn input(s: &InterchangeScratch, data: &str) -> PathBuf { + let p = s.0.join("input.patch"); + fs::write(&p, data).unwrap(); + p + } + + #[test] + fn patch_targets_validation_and_stale_preview() { + for target in [PatchTarget::Worktree, PatchTarget::Index, PatchTarget::Both] { + let (_s, repo) = fixture(); + let input_dir = InterchangeScratch::new().unwrap(); + let p = input(&input_dir, patch()); + let preview = repo.preview_patch_import(&p, target).unwrap(); + assert!(preview.valid, "{}", preview.validation); + assert_eq!(preview.paths, ["file.txt"]); + assert!( + repo.import_patch(&p, target, &preview.token) + .unwrap() + .success + ); + let wt = fs::read_to_string(repo.path.join("file.txt")).unwrap(); + assert_eq!( + wt, + if target == PatchTarget::Index { + "one\n" + } else { + "two\n" + } + ); + let staged = + checked(git(&repo.path, &["show", ":file.txt"], None, None).unwrap()).unwrap(); + assert_eq!( + staged.stdout, + if target == PatchTarget::Worktree { + b"one\n" + } else { + b"two\n" + } + ); + assert!(repo.import_patch(&p, target, &preview.token).is_err()); + } + let (_s, repo) = fixture(); + let s = InterchangeScratch::new().unwrap(); + let p = input(&s, patch()); + let preview = repo + .preview_patch_import(&p, PatchTarget::Worktree) + .unwrap(); + fs::write(repo.path.join("file.txt"), "external edit\n").unwrap(); + assert!(repo + .import_patch(&p, PatchTarget::Worktree, &preview.token) + .is_err()); + assert!( + !repo + .preview_patch_import(&p, PatchTarget::Worktree) + .unwrap() + .valid + ); + } + + #[test] + fn reject_path_traversal_admin_and_nested_symlinks() { + let (_s, repo) = fixture(); + for path in [ + "../outside", + "/absolute", + ".git/config", + "C:/outside", + "dir/../../out", + "dir\\..\\out", + ] { + assert!(repo.check_import_path(path).is_err(), "{path}"); + } + assert!(repo.check_import_path("new/nested/file").is_ok()); + let s = InterchangeScratch::new().unwrap(); + let p = input(&s, &patch().replace("file.txt", "../outside")); + assert!(repo + .preview_patch_import(&p, PatchTarget::Worktree) + .is_err()); + #[cfg(unix)] + { + std::os::unix::fs::symlink(&s.0, repo.path.join("link")).unwrap(); + assert!(repo.check_import_path("link/new/file").is_err()); + } + } + + fn mailbox(repo: &Repo, dir: &InterchangeScratch) -> PathBuf { + fs::write(repo.path.join("file.txt"), "two\n").unwrap(); + checked( + git( + &repo.path, + &[ + "commit", + "-am", + "authored change", + "--author=Original Author ", + ], + None, + None, + ) + .unwrap(), + ) + .unwrap(); + let output = checked( + git( + &repo.path, + &["format-patch", "--stdout", "-1", "HEAD"], + None, + None, + ) + .unwrap(), + ) + .unwrap(); + checked(git(&repo.path, &["reset", "--hard", "HEAD~1"], None, None).unwrap()).unwrap(); + let p = dir.0.join("mailbox"); + fs::write(&p, output.stdout).unwrap(); + p + } + + #[test] + fn mailbox_preserves_authors_and_recovers_continue_skip_abort() { + for action in [ + MailboxAction::Continue, + MailboxAction::Skip, + MailboxAction::Abort, + ] { + let (_s, repo) = fixture(); + let dir = InterchangeScratch::new().unwrap(); + let p = mailbox(&repo, &dir); + fs::write(repo.path.join("file.txt"), "diverged\n").unwrap(); + checked(git(&repo.path, &["commit", "-am", "divergence"], None, None).unwrap()) + .unwrap(); + let original = repo.git2().unwrap().head().unwrap().target().unwrap(); + let preview = repo.preview_patch_import(&p, PatchTarget::Mailbox).unwrap(); + assert!(preview.messages[0].contains("Original Author")); + let outcome = repo + .import_patch(&p, PatchTarget::Mailbox, &preview.token) + .unwrap(); + assert!(outcome.paused, "{}", outcome.output); + assert_eq!( + Repo::discover(&repo.path) + .unwrap() + .meta() + .unwrap() + .operation + .as_deref(), + Some("mailbox") + ); + let before = repo.mailbox_state().unwrap().unwrap(); + assert!(before.author.contains("Original Author")); + if matches!(action, MailboxAction::Continue) { + fs::write(repo.path.join("file.txt"), "resolved\n").unwrap(); + checked(git(&repo.path, &["add", "file.txt"], None, None).unwrap()).unwrap(); + assert!(repo.mailbox_action(action, &before.token).is_err()); + } + let state = repo.mailbox_state().unwrap().unwrap(); + let outcome = repo.mailbox_action(action, &state.token).unwrap(); + assert!(outcome.success && !outcome.paused, "{}", outcome.output); + let fresh = git2::Repository::open(&repo.path).unwrap(); + let head = fresh.head().unwrap().peel_to_commit().unwrap(); + if matches!(action, MailboxAction::Continue) { + assert_eq!(head.author().email(), Some("author@example.test")); + } else { + assert_eq!(head.id(), original); + } + } + } + + #[test] + fn clean_mailbox_series_previews_every_author_and_applies_in_order() { + let (_s, repo) = fixture(); + let dir = InterchangeScratch::new().unwrap(); + for (text, author) in [ + ("two\n", "First "), + ("three\n", "Second "), + ] { + fs::write(repo.path.join("file.txt"), text).unwrap(); + checked( + git( + &repo.path, + &["commit", "-am", "series", &format!("--author={author}")], + None, + None, + ) + .unwrap(), + ) + .unwrap(); + } + let out = checked( + git( + &repo.path, + &["format-patch", "--stdout", "-2", "HEAD"], + None, + None, + ) + .unwrap(), + ) + .unwrap(); + checked(git(&repo.path, &["reset", "--hard", "HEAD~2"], None, None).unwrap()).unwrap(); + let p = dir.0.join("series"); + fs::write(&p, out.stdout).unwrap(); + let preview = repo.preview_patch_import(&p, PatchTarget::Mailbox).unwrap(); + assert!(preview.valid, "{}", preview.validation); + assert_eq!(preview.messages.len(), 2); + assert!(preview.messages[0].contains("first@example.test")); + assert!(preview.messages[1].contains("second@example.test")); + assert!( + repo.import_patch(&p, PatchTarget::Mailbox, &preview.token) + .unwrap() + .success + ); + assert_eq!( + fs::read_to_string(repo.path.join("file.txt")).unwrap(), + "three\n" + ); + assert!(repo.mailbox_state().unwrap().is_none()); + } + + #[test] + fn bundles_verify_prerequisites_and_never_overwrite_refs() { + let (_s, repo) = fixture(); + let dir = InterchangeScratch::new().unwrap(); + let full = dir.0.join("full.bundle"); + let refname = repo + .git2() + .unwrap() + .head() + .unwrap() + .name() + .unwrap() + .to_owned(); + let full_preview = repo.export_bundle(&full, &refname, None).unwrap(); + assert!(full_preview.valid && full_preview.prerequisites.is_empty()); + let (_r, receiver) = fixture(); + let outcome = receiver + .import_bundle(&full, &full_preview.token, &refname, "imported") + .unwrap(); + assert!(outcome.success); + assert!(receiver + .import_bundle(&full, &full_preview.token, &refname, "imported") + .is_err()); + assert!(repo.export_bundle(&full, &refname, None).is_err()); + fs::write(repo.path.join("file.txt"), "next\n").unwrap(); + checked(git(&repo.path, &["commit", "-am", "next"], None, None).unwrap()).unwrap(); + let incremental = dir.0.join("incremental.bundle"); + let preview = repo + .export_bundle(&incremental, &refname, Some("HEAD~1")) + .unwrap(); + assert_eq!(preview.prerequisites.len(), 1); + let empty = InterchangeScratch::new().unwrap(); + git2::Repository::init(&empty.0).unwrap(); + let empty_repo = Repo::discover(&empty.0).unwrap(); + assert!(!empty_repo.preview_bundle(&incremental).unwrap().valid); + assert!(empty_repo + .import_bundle(&incremental, &preview.token, &refname, "missing") + .is_err()); + } +} diff --git a/crates/strand-core/src/lib.rs b/crates/strand-core/src/lib.rs index e2dc385a..9d3e3b7e 100644 --- a/crates/strand-core/src/lib.rs +++ b/crates/strand-core/src/lib.rs @@ -21,6 +21,7 @@ pub mod log; pub mod diff; pub mod stage; pub mod apply; +pub mod interchange; pub mod commit; pub mod commit_metadata; pub mod network; diff --git a/crates/strand-core/src/repo.rs b/crates/strand-core/src/repo.rs index 19085402..2441693a 100644 --- a/crates/strand-core/src/repo.rs +++ b/crates/strand-core/src/repo.rs @@ -111,14 +111,16 @@ impl Repo { /// Which multi-step history op (if any) is paused mid-flight, detected from /// the on-disk markers git leaves in `.git/`. Returns one of `"rebase"`, - /// `"cherry-pick"`, `"revert"`, `"merge"`, or `None`. Order matters: a + /// `"cherry-pick"`, `"revert"`, `"merge"`, `"mailbox"`, or `None`. Order matters: a /// rebase can leave a `MERGE_HEAD` while resolving, so rebase is checked /// first. Used by [`meta`](Repo::meta) (UI banner) and /// [`abort_operation`](crate::repo::Repo::abort_operation). pub(crate) fn operation_in_progress(&self) -> Option { let git_dir = self.gix.git_dir(); let has = |name: &str| git_dir.join(name).exists(); - if has("rebase-merge") || has("rebase-apply") { + if has("rebase-apply/applying") { + Some("mailbox".into()) + } else if has("rebase-merge") || has("rebase-apply") { Some("rebase".into()) } else if has("CHERRY_PICK_HEAD") { Some("cherry-pick".into()) diff --git a/crates/strand-core/src/watch.rs b/crates/strand-core/src/watch.rs index e29ed408..c2fce910 100644 --- a/crates/strand-core/src/watch.rs +++ b/crates/strand-core/src/watch.rs @@ -176,6 +176,8 @@ mod tests { "/repo/.git/refs/heads/main", "/repo/.git/MERGE_HEAD", "/repo/.git/rebase-merge/done", + "/repo/.git/rebase-apply/applying", + "/repo/.git/rebase-apply/next", ] { assert!(relevant_path(&PathBuf::from(p), &git_dir()), "{p} should refresh"); } diff --git a/crates/strand-tauri/src/commands.rs b/crates/strand-tauri/src/commands.rs index e0ec2642..fe6b2dbc 100644 --- a/crates/strand-tauri/src/commands.rs +++ b/crates/strand-tauri/src/commands.rs @@ -69,6 +69,41 @@ impl From for CmdError { pub(crate) type CmdResult = std::result::Result; +#[tauri::command] +pub async fn repo_patch_preview(path: String, source: String, target: strand_core::interchange::PatchTarget) -> CmdResult { + run_blocking("preview patch", move || Repo::discover(path)?.preview_patch_import(Path::new(&source), target).map_err(Into::into)).await +} + +#[tauri::command] +pub async fn repo_patch_import(path: String, source: String, target: strand_core::interchange::PatchTarget, token: String) -> CmdResult { + run_blocking("import patch", move || Repo::discover(path)?.import_patch(Path::new(&source), target, &token).map_err(Into::into)).await +} + +#[tauri::command] +pub async fn repo_mailbox_state(path: String) -> CmdResult> { + run_blocking("mailbox state", move || Repo::discover(path)?.mailbox_state().map_err(Into::into)).await +} + +#[tauri::command] +pub async fn repo_mailbox_action(path: String, action: strand_core::interchange::MailboxAction, token: String) -> CmdResult { + run_blocking("mailbox action", move || Repo::discover(path)?.mailbox_action(action, &token).map_err(Into::into)).await +} + +#[tauri::command] +pub async fn repo_bundle_preview(path: String, source: String) -> CmdResult { + run_blocking("verify bundle", move || Repo::discover(path)?.preview_bundle(Path::new(&source)).map_err(Into::into)).await +} + +#[tauri::command] +pub async fn repo_bundle_import(path: String, source: String, token: String, source_ref: String, branch: String) -> CmdResult { + run_blocking("import bundle", move || Repo::discover(path)?.import_bundle(Path::new(&source), &token, &source_ref, &branch).map_err(Into::into)).await +} + +#[tauri::command] +pub async fn repo_bundle_export(path: String, destination: String, refname: String, prerequisite: Option) -> CmdResult { + run_blocking("export bundle", move || Repo::discover(path)?.export_bundle(Path::new(&destination), &refname, prerequisite.as_deref()).map_err(Into::into)).await +} + #[tauri::command(async)] pub fn repo_terminal_create( path: String, diff --git a/crates/strand-tauri/src/main.rs b/crates/strand-tauri/src/main.rs index 0b3fad89..e2c7ede2 100644 --- a/crates/strand-tauri/src/main.rs +++ b/crates/strand-tauri/src/main.rs @@ -274,6 +274,13 @@ fn main() { commands::repo_remote_set_urls, commands::repo_remote_set_default, commands::repo_maintenance, + commands::repo_patch_preview, + commands::repo_patch_import, + commands::repo_mailbox_state, + commands::repo_mailbox_action, + commands::repo_bundle_preview, + commands::repo_bundle_import, + commands::repo_bundle_export, commands::repo_tag_create, commands::repo_tag_delete, commands::repo_tag_push, diff --git a/docs/learnings.md b/docs/learnings.md index e50db001..e1ba3fc5 100644 --- a/docs/learnings.md +++ b/docs/learnings.md @@ -1,5 +1,17 @@ # Learnings +## Interchange state comes from Git, not a saved UI session (2026-09-06) + +`rebase-apply/applying` identifies `git am`; `rebase-apply` alone can mean a +rebase. Test this before the generic rebase check so Continue/Abort dispatch +to the right porcelain. Mailbox previews parse every message with Git's +mailsplit/mailinfo, preserving authors and checking old/new paths. Imported +paths reject administrative entries and symlink traversal, including missing +descendants; never enable `--unsafe-paths`. Preview stamps include file bytes, +index and HEAD because status-row equality does not prove unchanged content. +Bundle imports publish only a new local branch after verification/unbundle, +using non-forcing ref creation to reject concurrent external branch creation. + Things we've learned while building Strand that aren't otherwise obvious from the PRD / ROADMAP / TASKS files. Append here when you discover something that future work (yours or another agent's) needs to respect. diff --git a/ui/src/App.tsx b/ui/src/App.tsx index ea335854..06fe2309 100644 --- a/ui/src/App.tsx +++ b/ui/src/App.tsx @@ -119,6 +119,7 @@ const SettingsDialog = lazy(() => import('./views/SettingsDialog').then((m) => ( const BranchCleanupDialog = lazy(() => import('./views/BranchCleanupDialog').then((m) => ({ default: m.BranchCleanupDialog }))); const RebaseEditor = lazy(() => import('./views/RebaseEditor').then((m) => ({ default: m.RebaseEditor }))); const MaintenanceDialog = lazy(() => import('./views/MaintenanceDialog').then((m) => ({ default: m.MaintenanceDialog }))); +const InterchangeDialog = lazy(() => import('./views/InterchangeDialog').then((m) => ({ default: m.InterchangeDialog }))); const WorkspaceManagerDialog = lazy(() => import('./views/WorkspaceManagerDialog').then((m) => ({ default: m.WorkspaceManagerDialog }))); const PullRequests = lazy(() => import('./views/PullRequests').then((m) => ({ default: m.PullRequests }))); @@ -351,6 +352,7 @@ export function App() { // null = closed; otherwise which remote-management flavour (add/rename/url). const [remoteDialog, setRemoteDialog] = useState(null); const [maintenanceOpen, setMaintenanceOpen] = useState(false); + const [interchangePath, setInterchangePath] = useState(null); const [fileEntryDialog, setFileEntryDialog] = useState<{ dir: string; directory: boolean } | null>(null); // null = closed; otherwise the branch to rename. const [renameBranchDialog, setRenameBranchDialog] = useState<{ name: string } | null>(null); @@ -1161,6 +1163,7 @@ export function App() { push: () => { void onPush(); }, openInEditor, openInTerminal, + openInterchange: () => { const path = useRepo.getState().activePath; if (path) setInterchangePath(path); }, }; const hasRepo = Boolean(meta); useEffect(() => { @@ -1875,6 +1878,7 @@ export function App() { ] : []), { id: 'remote-add', label: 'Add remote…', group: 'Actions', keywords: 'remote origin upstream url add', run: () => setRemoteDialog({ kind: 'add' }) }, + { id: 'git-interchange', label: 'Patches, mailboxes & bundles…', group: 'Actions', keywords: 'import export apply index working tree am continue skip abort author bundle verify prerequisites', run: () => { setPaletteOpen(false); setInterchangePath(meta.path); } }, { id: 'repository-maintenance', label: 'Repository maintenance…', group: 'Actions', keywords: 'git gc fsck integrity optimize activity log command output', run: () => { setPaletteOpen(false); setMaintenanceOpen(true); @@ -2251,7 +2255,7 @@ export function App() { ) : view === 'work' ? ( workbenchComposed ? (
- + { if (meta) setInterchangePath(meta.path); }} /> )} - + { if (meta) setInterchangePath(meta.path); }} /> {mainSurfaceId && ( setMaintenanceOpen(false)} onToast={showToast} /> )} + {interchangePath && setInterchangePath(null)} />} {fileEntryDialog && meta && ( , string> = { + mailbox: 'Mailbox in progress', rebase: 'Rebase in progress', 'cherry-pick': 'Cherry-pick in progress', revert: 'Revert in progress', @@ -2657,7 +2663,7 @@ const OP_LABEL: Record, string> = { * conflict remains. The op clears `operation` on the next refresh, which hides * the banner. */ -function OpBanner({ onToast }: { onToast: (msg: string, kind?: 'success' | 'error') => void }) { +function OpBanner({ onToast, onOpenMailbox }: { onToast: (msg: string, kind?: 'success' | 'error') => void; onOpenMailbox: () => void }) { const operation = useRepo((s) => s.meta?.operation ?? null); const status = useRepo((s) => s.status); const abortOperation = useRepo((s) => s.abortOperation); @@ -2667,6 +2673,7 @@ function OpBanner({ onToast }: { onToast: (msg: string, kind?: 'success' | 'erro const hasConflicts = useMemo(() => status.some((s) => s.kind === 'CONFLICTED'), [status]); if (!operation) return null; + if (operation === 'mailbox') return
Mailbox in progressResolve and stage conflicts, then continue the mailbox.
; const onAbort = async () => { if (busy) return; diff --git a/ui/src/lib/interchange.test.ts b/ui/src/lib/interchange.test.ts new file mode 100644 index 00000000..a3a11001 --- /dev/null +++ b/ui/src/lib/interchange.test.ts @@ -0,0 +1,25 @@ +import { beforeEach, expect, it, vi } from 'vitest'; +const invoke = vi.hoisted(() => vi.fn()); +vi.mock('@tauri-apps/api/core', () => ({ invoke, Channel: class {} })); +import { tauri } from './tauri'; + +beforeEach(() => invoke.mockReset()); + +it('carries the exact reviewed patch target and token over IPC without rewriting paths', async () => { + const preview = { token: 'bytes-and-index', paths: ['space name.txt'], valid: true, messages: [], validation: '' }; + invoke.mockResolvedValueOnce(preview).mockResolvedValueOnce({ success: true, paused: false, output: '' }); + const result = await tauri.repoPatchPreview('C:/repo with spaces', 'C:/patch files/a.patch', 'index'); + await tauri.repoPatchImport('C:/repo with spaces', 'C:/patch files/a.patch', 'index', result.token); + expect(invoke).toHaveBeenLastCalledWith('repo_patch_import', { path: 'C:/repo with spaces', source: 'C:/patch files/a.patch', target: 'index', token: 'bytes-and-index' }); +}); + +it('keeps mailbox recovery distinct from rebase and propagates a stale-state rejection', async () => { + invoke.mockRejectedValueOnce({ message: 'mailbox state changed; refresh before continuing' }); + await expect(tauri.repoMailboxAction('repo', 'skip', 'reviewed')).rejects.toMatchObject({ message: expect.stringContaining('changed') }); + expect(invoke).toHaveBeenCalledWith('repo_mailbox_action', { path: 'repo', action: 'skip', token: 'reviewed' }); +}); + +it('imports the chosen advertised bundle ref into an explicit new branch', async () => { + await tauri.repoBundleImport('repo', '/tmp/a.bundle', 'file stamp', 'refs/tags/v1', 'import/v1'); + expect(invoke).toHaveBeenCalledWith('repo_bundle_import', { path: 'repo', source: '/tmp/a.bundle', token: 'file stamp', sourceRef: 'refs/tags/v1', branch: 'import/v1' }); +}); diff --git a/ui/src/lib/interchange.ts b/ui/src/lib/interchange.ts new file mode 100644 index 00000000..509680a6 --- /dev/null +++ b/ui/src/lib/interchange.ts @@ -0,0 +1,5 @@ +export type PatchTarget = 'worktree' | 'index' | 'both' | 'mailbox'; +export interface PatchPreview { token: string; paths: string[]; messages: string[]; valid: boolean; validation: string } +export interface MailboxState { token: string; current: string; total: string; author: string; conflicts: boolean } +export interface InterchangeOutcome { success: boolean; paused: boolean; output: string } +export interface BundlePreview { token: string; refs: Array<{ oid: string; name: string }>; prerequisites: string[]; valid: boolean; validation: string } diff --git a/ui/src/lib/menu.ts b/ui/src/lib/menu.ts index a1b70fe5..15a53dd1 100644 --- a/ui/src/lib/menu.ts +++ b/ui/src/lib/menu.ts @@ -48,6 +48,7 @@ export interface MenuHandlers { push(): void; openInEditor(): void; openInTerminal(): void; + openInterchange(): void; } let preemptsKeydown = false; @@ -195,6 +196,7 @@ export async function installAppMenu( const repoMenu = await Submenu.new({ text: 'Repository', items: [ + await item({ id: 'git-interchange', text: 'Patches, Mailboxes & Bundles…', enabled: hasRepo, action: () => handlers().openInterchange() }), await item({ id: 'sync', text: 'Sync (Fetch + Pull + Push)', diff --git a/ui/src/lib/tauri.ts b/ui/src/lib/tauri.ts index e7356543..65484f69 100644 --- a/ui/src/lib/tauri.ts +++ b/ui/src/lib/tauri.ts @@ -1,4 +1,5 @@ import { Channel, invoke } from '@tauri-apps/api/core'; +import type { PatchTarget, PatchPreview, MailboxState, InterchangeOutcome, BundlePreview } from './interchange'; import type { AiProvider, @@ -115,6 +116,13 @@ export function errMessage(e: unknown): string { * frontend never calls `invoke` with a string literal. */ export const tauri = { + repoPatchPreview: (path: string, source: string, target: PatchTarget) => invoke('repo_patch_preview', { path, source, target }), + repoPatchImport: (path: string, source: string, target: PatchTarget, token: string) => invoke('repo_patch_import', { path, source, target, token }), + repoMailboxState: (path: string) => invoke('repo_mailbox_state', { path }), + repoMailboxAction: (path: string, action: 'continue' | 'skip' | 'abort', token: string) => invoke('repo_mailbox_action', { path, action, token }), + repoBundlePreview: (path: string, source: string) => invoke('repo_bundle_preview', { path, source }), + repoBundleImport: (path: string, source: string, token: string, sourceRef: string, branch: string) => invoke('repo_bundle_import', { path, source, token, sourceRef, branch }), + repoBundleExport: (path: string, destination: string, refname: string, prerequisite: string | null) => invoke('repo_bundle_export', { path, destination, refname, prerequisite }), microsoftStoreUpdateAvailable: () => invoke('microsoft_store_update_available'), microsoftStoreOpenProduct: () => diff --git a/ui/src/lib/types.ts b/ui/src/lib/types.ts index c27be6a0..0f1b75f7 100644 --- a/ui/src/lib/types.ts +++ b/ui/src/lib/types.ts @@ -17,7 +17,7 @@ export interface RepoMeta { * Multi-step history op paused mid-flight, or `null` in a normal state. * Drives the in-progress banner + Abort affordance. */ - operation: 'rebase' | 'cherry-pick' | 'revert' | 'merge' | null; + operation: 'rebase' | 'cherry-pick' | 'revert' | 'merge' | 'mailbox' | null; /** * The shared git dir (`commondir`), identical for every worktree of the same * repository. The tab strip groups worktree tabs on this value. diff --git a/ui/src/styles/features.css b/ui/src/styles/features.css index 03aa8eaa..8bcf7f44 100644 --- a/ui/src/styles/features.css +++ b/ui/src/styles/features.css @@ -9542,3 +9542,10 @@ select.clone-input { .plugin-heroi-select-thinking, .plugin-heroi-select-permission { display: none; } } +/* Repository tools mount only while their dialog is open. */ +.git-tool-body { overflow-y: auto; min-height: 0; max-height: 70vh; } +.git-tool-body > .btn { align-self: flex-start; } +.git-tool-body p { overflow-wrap: anywhere; } +.git-tool-review { border: 1px solid var(--border); border-radius: var(--r-sm); padding: 12px; display: flex; flex-direction: column; gap: 10px; } +.git-tool-review pre, .git-tool-output { white-space: pre-wrap; overflow-wrap: anywhere; max-height: 220px; overflow-y: auto; font: 11px var(--font-mono); } +.git-tool-actions { display: flex; flex-wrap: wrap; gap: 8px; } diff --git a/ui/src/views/InterchangeDialog.tsx b/ui/src/views/InterchangeDialog.tsx new file mode 100644 index 00000000..3a59a0ca --- /dev/null +++ b/ui/src/views/InterchangeDialog.tsx @@ -0,0 +1,139 @@ +import { useCallback, useEffect, useRef, useState } from 'react'; +import { listen } from '@tauri-apps/api/event'; +import { open, save } from '@tauri-apps/plugin-dialog'; +import { Dialog } from '../components/Dialog'; +import { Select } from '../components/Select'; +import { errMessage, tauri } from '../lib/tauri'; +import type { BundlePreview, InterchangeOutcome, MailboxState, PatchPreview, PatchTarget } from '../lib/interchange'; +import { useRepo } from '../stores/repo'; + +export function InterchangeDialog({ path, onClose }: { path: string; onClose: () => void }) { + const [mode, setMode] = useState<'patch' | 'bundle' | 'export'>('patch'); + const [source, setSource] = useState(''); + const [target, setTarget] = useState('worktree'); + const [patch, setPatch] = useState(null); + const [bundle, setBundle] = useState(null); + const [mailbox, setMailbox] = useState(null); + const [sourceRef, setSourceRef] = useState(''); + const [branch, setBranch] = useState(''); + const [exportRef, setExportRef] = useState('refs/heads/main'); + const [base, setBase] = useState(''); + const [busy, setBusy] = useState(false); + const [confirm, setConfirm] = useState<'skip' | 'abort' | null>(null); + const [output, setOutput] = useState(''); + const [error, setError] = useState(''); + const first = useRef(null); + const generation = useRef(0); + const stateGeneration = useRef(0); + const mounted = useRef(true); + const invalidate = useCallback(() => { generation.current++; setPatch(null); setBundle(null); setConfirm(null); }, []); + const refresh = useCallback(async () => { + const seq = ++stateGeneration.current; + try { + const next = await tauri.repoMailboxState(path); + if (mounted.current && stateGeneration.current === seq) setMailbox(next); + } catch (e) { if (mounted.current && stateGeneration.current === seq) { setMailbox(null); setError(errMessage(e)); } } + }, [path]); + useEffect(() => { + mounted.current = true; + void refresh(); + const onChange = () => { invalidate(); void refresh(); }; + const unlisten = listen('repo://changed', (event) => { if (event.payload === path) onChange(); }); + window.addEventListener('focus', onChange); + const focus = requestAnimationFrame(() => first.current?.focus()); + return () => { mounted.current = false; generation.current++; stateGeneration.current++; cancelAnimationFrame(focus); window.removeEventListener('focus', onChange); void unlisten.then((fn) => fn()); }; + }, [path, refresh, invalidate]); + + async function run(work: () => Promise, mutation = false) { + if (busy) return; + setBusy(true); setError(''); + try { await work(); } + catch (e) { if (mounted.current) setError(errMessage(e)); } + finally { + if (mutation) { + invalidate(); + await refresh(); + const repo = useRepo.getState(); + if (repo.activePath === path) await Promise.all([repo.refreshLocalChanges(), repo.refreshLog()]); + } + if (mounted.current) { setBusy(false); requestAnimationFrame(() => first.current?.focus()); } + } + } + function outcome(result: InterchangeOutcome) { + setOutput(result.output || (result.success ? 'Completed.' : 'Git stopped without output.')); + if (!result.success && !result.paused) setError(result.output || 'Git operation failed.'); + } + async function preview() { + const seq = ++generation.current; + if (mode === 'patch') { + const next = await tauri.repoPatchPreview(path, source, target); + if (mounted.current && seq === generation.current) setPatch(next); + else if (mounted.current) setError('Repository changed during validation. Preview again.'); + } else { + const next = await tauri.repoBundlePreview(path, source); + if (mounted.current && seq === generation.current) { setBundle(next); setSourceRef(next.refs[0]?.name ?? ''); } + else if (mounted.current) setError('Repository changed during verification. Verify again.'); + } + } + async function browse() { + const chosen = mode === 'export' ? await save({ title: 'Export Git bundle', filters: [{ name: 'Git bundle', extensions: ['bundle'] }] }) : await open({ title: 'Import patch, mailbox or bundle', multiple: false, directory: false }); + if (typeof chosen === 'string') { setSource(chosen); invalidate(); } + } + + return {busy ? 'Running Git…' : ''}}> +
+

Repository: {path}

+ {mailbox &&
+ Mailbox paused · patch {mailbox.current} of {mailbox.total} + 
{mailbox.author || 'Author metadata is not available yet.'}
+

Resolve and stage conflicts in Local Changes, then return here to continue. Skip discards this patch’s changes; Abort restores the checkout before this mailbox and discards its applied changes.

+
+ + + {(['skip', 'abort'] as const).map((action) => )} +
+
} + + + + {mode === 'patch' && <> + + + {patch &&
+ {patch.valid ? 'Validation passed' : target === 'mailbox' ? 'Direct application failed — Git will try a three-way merge' : 'Validation failed'} + 
{patch.validation}
+
Affected paths ({patch.paths.length})
{patch.paths.join('\n')}
+ {patch.messages.length > 0 &&
Original authors and messages ({patch.messages.length})
{patch.messages.join('\n\n')}
} + +
} + } + {mode === 'bundle' && <> + + {bundle &&
+ {bundle.valid ? 'Bundle verified' : 'Bundle cannot be imported here'}
{bundle.validation}
+

Prerequisites

{bundle.prerequisites.join('\n') || 'None — self-contained bundle'}
+ + +

Imports objects and creates this branch. The current checkout stays in place. Existing branches cannot be overwritten.

+ +
} + } + {mode === 'export' && <> + + +

Recipients need the excluded history. The new file contains the selected ref and reachable history; existing files are never overwritten.

+ + } + {error &&
{error}
} + {output && 
{output}
} +
+ ; +} diff --git a/website/docs/everyday-git.md b/website/docs/everyday-git.md index cf6223f9..78960ca2 100644 --- a/website/docs/everyday-git.md +++ b/website/docs/everyday-git.md @@ -221,6 +221,38 @@ The merge editor is a full-screen three-way view: Once every conflicted file is resolved, the banner's Continue button resumes the operation. +## Patches, mailboxes and bundles + +Open **Repository → Patches, Mailboxes & Bundles…**, or search the same name +in Quick Launch. Tab moves between controls; native selectors use arrow keys; +Enter or Space activates buttons and Escape closes the dialog while idle. + +For a patch, choose the file and an explicit target: **Working tree only** +(unstaged), **Index only** (staged, without changing files), or **Index and +working tree**. **Preview and validate** lists every affected old/new path +and checks application before enabling Apply. A changed input or repository +requires another preview. Paths outside the repository, Git administrative +paths and symlink patches are rejected. Patch and mailbox input is limited +to 32 MiB; mailbox series are limited to 1,000 patches. + +Choose **Mailbox** for format-patch output. The preview includes authors, +dates and subjects, and validates the series against a temporary index. +Starting requires a clean repository and creates commits with the original +author metadata. If direct application fails, Git tries a three-way merge. +A paused mailbox is detected from Git’s own state, including one started in +a terminal. Resolve and stage conflicts in Local Changes, reopen the dialog, +then **Continue mailbox**. **Skip patch** discards the current patch changes; +**Abort mailbox** restores the checkout before the mailbox. Both require a +second click after explaining the discarded changes. Git errors remain visible. + +For a bundle, **Verify bundle** shows advertised refs, prerequisite commits +and Git’s validation output. Choose one advertised ref and a **new local +branch** to import; existing branches and the current checkout are preserved. +An incremental bundle cannot be imported until this repository has its +prerequisites. **Export bundle** takes a full ref (such as `refs/heads/main`) +and an optional prerequisite revision to exclude. It writes a new destination +file and reports the exported refs/prerequisites; it never overwrites a file. + ## Where Strand uses your system git Strand reads repositories with its own fast engine, but the operations where your environment matters shell out to the real `git` binary and therefore honor your global and per-repo configuration: From 31f5ff12826bba144813bd1ff826e5560555fb5b Mon Sep 17 00:00:00 2001 From: Daniels-Main Date: Sun, 6 Sep 2026 16:41:11 +0200 Subject: [PATCH 2/5] feat: guide manual bisect with external state and safe reset --- README.md | 3 + ROADMAP.md | 11 +- TASKS.md | 6 +- crates/strand-core/src/bisect.rs | 421 ++++++++++++++++++++++++++++ crates/strand-core/src/history.rs | 2 +- crates/strand-core/src/lib.rs | 1 + crates/strand-core/src/repo.rs | 2 + crates/strand-core/src/watch.rs | 7 + crates/strand-tauri/src/commands.rs | 15 + crates/strand-tauri/src/main.rs | 3 + docs/learnings.md | 13 + ui/src/App.tsx | 15 +- ui/src/lib/bisect.test.ts | 12 + ui/src/lib/bisect.ts | 16 ++ ui/src/lib/menu.ts | 2 + ui/src/lib/tauri.ts | 4 + ui/src/lib/types.ts | 2 +- ui/src/views/BisectDialog.tsx | 83 ++++++ website/docs/commits-and-history.md | 24 ++ 19 files changed, 633 insertions(+), 9 deletions(-) create mode 100644 crates/strand-core/src/bisect.rs create mode 100644 ui/src/lib/bisect.test.ts create mode 100644 ui/src/lib/bisect.ts create mode 100644 ui/src/views/BisectDialog.tsx diff --git a/README.md b/README.md index 0f917c69..88710ae1 100644 --- a/README.md +++ b/README.md @@ -235,6 +235,9 @@ the resolved app appearance automatically. into the working tree, index, or both; import mailboxes with original authors and Continue / Skip / Abort recovery; verify bundle refs and prerequisites, import into a new branch, and export full or incremental bundles. +- **Guided bisect** — start from known good/bad revisions, test and mark + good/bad/skip, inspect remaining candidates and the culprit, resume external + sessions, and reset to the original checkout while protecting test edits. - **Commit graph** — SVG lanes with branch/tag chips, revealable inline stash nodes with non-mutating diff inspection, a resizable commit detail panel with lazy GPG/SSH/X.509 verification, diff --git a/ROADMAP.md b/ROADMAP.md index 8a1ae0a2..4225e665 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2161,7 +2161,8 @@ and Store certification remain external gates. GitHub Releases channel) - Opt-in product telemetry only if a concrete post-1.0 decision, disclosure, retention policy, and backend justify adding it -- Guided Git bisect +- ☑ Guided Git bisect (F10: manual good/bad/skip, progress/culprit, + external-session resume, clean-tree reset and reviewed original target) - Sparse checkout (cone mode first) - ☑ Patch import/mailbox and Git bundle workflows (F07: `InterchangeDialog`, validation, author-preserving mailbox recovery and new-branch bundle import; @@ -2821,6 +2822,14 @@ checks, five fixtures and three frontend IPC tests pass. Native WebView2 verified palette/focus, worktree/index targets, mailbox authors and conflict continuation, and bundle verification/import/incremental export. +**Guided bisect shipped (2026-09-06):** The Repository menu, palette and +operation banner open a manual bisect dialog backed by worktree-local Git +state. It shows remaining candidates, final/ambiguous outcomes, custom +external terms and no-checkout sessions. Stale ratings and dirty checkout/ +reset transitions are refused. Three native fixtures, two frontend tests, +Rust checks/typecheck and native WebView2 rating/resume/reset/keyboard flows +pass. Automated test-command execution remains a separate later slice. + ## Cross-cutting tracks (run in parallel with all milestones) **Performance audit kick (2026-09-06):** Rechecked `main` at `8e83c8c` on diff --git a/TASKS.md b/TASKS.md index bcd6304f..90a15612 100644 --- a/TASKS.md +++ b/TASKS.md @@ -117,9 +117,11 @@ Detailed comparison and sequencing: [`docs/git-client-1.0-audit.md`](./docs/git- - ☐ **F09 / P2 — Advanced clone options.** Branch, depth/single-branch, partial-clone filter and recursive-submodule options; deepen/unshallow, progress/cancellation, and safe argument construction. -- ☐ **F10 / P2 — Guided bisect.** Good/bad/skip, operation progress, external +- ☑ **F10 / P2 — Guided bisect.** Good/bad/skip, operation progress, external session resume and safe reset to the original checkout; defer test-command - execution until the manual workflow is complete. + execution (`bisect.rs`, `BisectDialog`, Repository menu/palette and banner; + three native fixtures, two frontend tests and native WebView2 keyboard, + external-resume, dirty-reset and original-target checks). - ☐ **F14 / P2 — Publish a new hosted repository.** Provider/account/visibility selection, concrete destination review, remote configuration and explicit initial push, with recovery from partial failure. diff --git a/crates/strand-core/src/bisect.rs b/crates/strand-core/src/bisect.rs new file mode 100644 index 00000000..87eba81e --- /dev/null +++ b/crates/strand-core/src/bisect.rs @@ -0,0 +1,421 @@ +//! Manual bisect, driven by Git's worktree-local state (including external runs). +use crate::{Error, Repo, Result}; +use serde::{Deserialize, Serialize}; +use std::{fs, io::Read, path::Path, process::Stdio}; + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct BisectState { + pub active: bool, + pub token: String, + pub original: String, + pub original_tip: String, + pub current: String, + pub subject: String, + pub expected: String, + pub good_term: String, + pub bad_term: String, + pub remaining: usize, + pub remaining_truncated: bool, + pub range_error: String, + pub culprit: Option, + pub ambiguous: bool, + pub no_checkout: bool, + pub clean: bool, + pub log: String, +} + +#[derive(Debug, Clone, Copy, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum BisectAction { + Good, + Bad, + Skip, + Reset, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct BisectOutcome { + pub success: bool, + pub output: String, + pub state: BisectState, +} + +fn git(cwd: &Path, args: &[&str]) -> Result { + Ok(crate::git_command() + .current_dir(cwd) + .args(crate::GIT_SAFE_CONFIG) + .args(args) + .env("LC_ALL", "C") + .env("GIT_EDITOR", "true") + .env("GIT_TERMINAL_PROMPT", "0") + .stdin(Stdio::null()) + .output()?) +} +fn text(out: &std::process::Output) -> String { + String::from_utf8_lossy( + &out.stdout + .iter() + .chain(&out.stderr) + .take(65536) + .copied() + .collect::>(), + ) + .trim() + .to_owned() +} +fn checked(cwd: &Path, args: &[&str]) -> Result { + let out = git(cwd, args)?; + if !out.status.success() { + return Err(Error::Other(text(&out))); + } + Ok(String::from_utf8_lossy(&out.stdout).trim().to_owned()) +} +fn state_file(dir: &Path, name: &str) -> Result { + let file = match fs::File::open(dir.join(name)) { + Ok(f) => f, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(String::new()), + Err(e) => return Err(e.into()), + }; + let mut text = String::new(); + file.take(2 * 1024 * 1024 + 1).read_to_string(&mut text)?; + if text.len() > 2 * 1024 * 1024 { + return Err(Error::Other("bisect state is too large to inspect".into())); + } + Ok(text.trim().into()) +} + +impl Repo { + pub fn bisect_state(&self) -> Result { + let dir = self.git_dir(); + let active = dir.join("BISECT_START").exists(); + let original = state_file(dir, "BISECT_START")?; + let log = state_file(dir, "BISECT_LOG")?; + let expected = state_file(dir, "BISECT_EXPECTED_REV")?; + let terms = state_file(dir, "BISECT_TERMS")?; + let mut terms = terms.lines(); + let bad_term = terms.next().unwrap_or("bad").to_string(); + let good_term = terms.next().unwrap_or("good").to_string(); + let no_checkout = dir.join("BISECT_HEAD").exists(); + let fresh = self.git2_owned()?; + let original_tip = fresh + .find_reference(&format!("refs/heads/{original}")) + .and_then(|r| r.peel_to_commit()) + .or_else(|_| fresh.revparse_single(&original)?.peel_to_commit()) + .map(|c| c.id().to_string()) + .unwrap_or_default(); + let revision = if no_checkout { "BISECT_HEAD" } else { "HEAD" }; + let commit = fresh.revparse_single(revision)?.peel_to_commit()?; + let current = commit.id().to_string(); + let subject = commit.summary().unwrap_or("").to_owned(); + let status = checked( + &self.path, + &["status", "--porcelain", "--untracked-files=normal"], + )?; + let clean = status.is_empty(); + let candidates = if active { + checked( + &self.path, + &[ + "bisect", + "visualize", + "--format=%H", + "--no-patch", + "--max-count=10001", + ], + ) + } else { + Ok(String::new()) + }; + let (candidates, range_error) = match candidates { + Ok(value) => (value, String::new()), + Err(e) => (String::new(), e.to_string()), + }; + let remaining = candidates + .lines() + .filter(|l| l.len() == 40 || l.len() == 64) + .count(); + let culprit = log.lines().rev().find_map(|line| { + let prefix = format!("# first {bad_term} commit: ["); + line.strip_prefix(&prefix) + .and_then(|s| s.split_once(']')) + .map(|(oid, _)| oid.to_owned()) + }); + let ambiguous = + active && culprit.is_none() && log.contains("# only skipped commits left to test"); + let refs = checked( + &self.path, + &[ + "for-each-ref", + "--format=%(refname) %(objectname)", + "refs/bisect/", + ], + )?; + let stamp = format!("{active}:{original}:{original_tip}:{log}:{expected}:{current}:{refs}:{status}:{bad_term}:{good_term}:{}", state_file(dir, "HEAD")?); + let token = git2::Oid::hash_object(git2::ObjectType::Blob, stamp.as_bytes())?.to_string(); + Ok(BisectState { + active, + token, + original, + original_tip, + current, + subject, + expected, + good_term, + bad_term, + remaining: remaining.min(10000), + remaining_truncated: remaining > 10000, + range_error, + culprit, + ambiguous, + no_checkout, + clean, + log, + }) + } + + pub fn bisect_start(&self, good: &str, bad: &str, token: &str) -> Result { + let before = self.bisect_state()?; + if before.token != token { + return Err(Error::Other( + "repository changed; refresh the bisect review".into(), + )); + } + if before.active || self.operation_in_progress().is_some() { + return Err(Error::Other( + "finish the current Git operation before starting bisect".into(), + )); + } + if !before.clean { + return Err(Error::Other( + "commit or stash working-tree/index changes before bisect".into(), + )); + } + let fresh = self.git2_owned()?; + let good = fresh.revparse_single(good)?.peel_to_commit()?.id(); + let bad = fresh.revparse_single(bad)?.peel_to_commit()?.id(); + if good == bad || !fresh.graph_descendant_of(bad, good)? { + return Err(Error::Other( + "the good revision must be an earlier ancestor of the bad revision".into(), + )); + } + let out = git( + &self.path, + &["bisect", "start", &bad.to_string(), &good.to_string(), "--"], + )?; + Ok(BisectOutcome { + success: out.status.success(), + output: text(&out), + state: self.bisect_state()?, + }) + } + + pub fn bisect_action(&self, action: BisectAction, token: &str) -> Result { + let before = self.bisect_state()?; + if !before.active { + return Err(Error::Other("no bisect session is active".into())); + } + if before.token != token { + return Err(Error::Other( + "bisect changed externally; refresh before rating a revision".into(), + )); + } + if !before.clean { + return Err(Error::Other( + "commit or stash test edits before bisect changes the checkout".into(), + )); + } + if self + .operation_in_progress() + .is_some_and(|op| op != "bisect") + { + return Err(Error::Other( + "finish the other Git operation before continuing bisect".into(), + )); + } + let out = if matches!(action, BisectAction::Reset) { + // Never use checkout/reset --force. Git also protects the original + // branch if another worktree has checked it out while we were testing. + git(&self.path, &["bisect", "reset"])? + } else { + if before.culprit.is_some() || before.ambiguous { + return Err(Error::Other( + "bisect has finished; review the result and reset".into(), + )); + } + if !before.expected.is_empty() && before.current != before.expected { + return Err(Error::Other("HEAD no longer matches Git's bisect selection; restore that checkout before rating".into())); + } + let term = match action { + BisectAction::Good => &before.good_term, + BisectAction::Bad => &before.bad_term, + BisectAction::Skip => "skip", + BisectAction::Reset => unreachable!(), + }; + if term.starts_with('-') + || !term + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_') + { + return Err(Error::Other("unsupported external bisect term".into())); + } + git(&self.path, &["bisect", term, &before.current])? + }; + Ok(BisectOutcome { + success: out.status.success(), + output: text(&out), + state: self.bisect_state()?, + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::interchange::InterchangeScratch; + fn fixture() -> (InterchangeScratch, Repo, Vec) { + let scratch = InterchangeScratch::new().unwrap(); + let repo = git2::Repository::init(&scratch.0).unwrap(); + let mut cfg = repo.config().unwrap(); + cfg.set_str("user.name", "Bisect Tester").unwrap(); + cfg.set_str("user.email", "bisect@example.test").unwrap(); + cfg.set_bool("commit.gpgsign", false).unwrap(); + cfg.set_str("core.hooksPath", "/dev/null").unwrap(); + let mut commits = vec![]; + for n in 0..8 { + fs::write(scratch.0.join("number"), format!("{n}\n")).unwrap(); + checked(&scratch.0, &["add", "."]).unwrap(); + checked(&scratch.0, &["commit", "-m", &format!("step {n}")]).unwrap(); + commits.push(checked(&scratch.0, &["rev-parse", "HEAD"]).unwrap()); + } + let handle = Repo::discover(&scratch.0).unwrap(); + (scratch, handle, commits) + } + #[test] + fn finds_culprit_and_restores_original_branch() { + let (_s, repo, commits) = fixture(); + let original = checked(&repo.path, &["symbolic-ref", "HEAD"]).unwrap(); + let mut state = repo + .bisect_start( + &commits[0], + &commits[7], + &repo.bisect_state().unwrap().token, + ) + .unwrap() + .state; + assert!(state.active && state.remaining > 0); + for _ in 0..10 { + if state.culprit.is_some() { + break; + } + let n = commits.iter().position(|c| c == &state.current).unwrap(); + state = repo + .bisect_action( + if n >= 4 { + BisectAction::Bad + } else { + BisectAction::Good + }, + &state.token, + ) + .unwrap() + .state; + } + assert_eq!(state.culprit, Some(commits[4].clone()), "{}", state.log); + assert!( + !repo + .bisect_action(BisectAction::Reset, &state.token) + .unwrap() + .state + .active + ); + assert_eq!( + checked(&repo.path, &["symbolic-ref", "HEAD"]).unwrap(), + original + ); + assert_eq!( + checked(&repo.path, &["rev-parse", "HEAD"]).unwrap(), + commits[7] + ); + } + #[test] + fn external_session_stale_ratings_dirty_reset_and_skips() { + let (_s, repo, commits) = fixture(); + checked( + &repo.path, + &["bisect", "start", &commits[7], &commits[0], "--"], + ) + .unwrap(); + let state = repo.bisect_state().unwrap(); + assert!(state.active); + checked(&repo.path, &["bisect", "good"]).unwrap(); + assert!(repo.bisect_action(BisectAction::Bad, &state.token).is_err()); + let fresh = repo.bisect_state().unwrap(); + fs::write(repo.path.join("number"), "test edits\n").unwrap(); + assert!(repo + .bisect_action(BisectAction::Reset, &fresh.token) + .is_err()); + assert_eq!( + fs::read_to_string(repo.path.join("number")).unwrap(), + "test edits\n" + ); + checked(&repo.path, &["restore", "number"]).unwrap(); + let mut state = repo.bisect_state().unwrap(); + for _ in 0..10 { + if state.ambiguous || state.culprit.is_some() { + break; + } + state = repo + .bisect_action(BisectAction::Skip, &state.token) + .unwrap() + .state; + } + assert!(state.ambiguous, "{}", state.log); + assert!( + repo.bisect_action(BisectAction::Reset, &state.token) + .unwrap() + .success + ); + } + #[test] + fn linked_worktree_and_external_no_checkout_terms() { + let (scratch, repo, commits) = fixture(); + let link = scratch.0.join("linked"); + checked( + &repo.path, + &["worktree", "add", "-b", "linked", link.to_str().unwrap()], + ) + .unwrap(); + let linked = Repo::discover(&link).unwrap(); + checked( + &link, + &[ + "bisect", + "start", + "--no-checkout", + "--term-good=old", + "--term-bad=new", + &commits[7], + &commits[0], + "--", + ], + ) + .unwrap(); + let state = linked.bisect_state().unwrap(); + assert!(state.active && state.no_checkout); + assert!(!repo.bisect_state().unwrap().active); + assert_eq!(state.good_term, "old"); + assert!( + linked + .bisect_action(BisectAction::Good, &state.token) + .unwrap() + .success + ); + let state = linked.bisect_state().unwrap(); + assert!( + linked + .bisect_action(BisectAction::Reset, &state.token) + .unwrap() + .success + ); + } +} diff --git a/crates/strand-core/src/history.rs b/crates/strand-core/src/history.rs index 3ba99c14..fbfb8785 100644 --- a/crates/strand-core/src/history.rs +++ b/crates/strand-core/src/history.rs @@ -398,7 +398,7 @@ impl Repo { /// suppression). Same pause-aware mapping. fn run_sequencer_env(&self, args: &[&str], envs: &[(&str, &str)]) -> Result { match run_git_env(&self.path, args, envs) { - Ok(_) => Ok(self.operation_in_progress().is_some()), + Ok(_) => Ok(self.operation_in_progress().is_some_and(|op| op != "bisect")), Err(e) => { // A conflict is the expected paused outcome. Git can also // leave CHERRY_PICK_HEAD/REVERT_HEAD behind after a *real* diff --git a/crates/strand-core/src/lib.rs b/crates/strand-core/src/lib.rs index 9d3e3b7e..21d978e5 100644 --- a/crates/strand-core/src/lib.rs +++ b/crates/strand-core/src/lib.rs @@ -22,6 +22,7 @@ pub mod diff; pub mod stage; pub mod apply; pub mod interchange; +pub mod bisect; pub mod commit; pub mod commit_metadata; pub mod network; diff --git a/crates/strand-core/src/repo.rs b/crates/strand-core/src/repo.rs index 2441693a..432734f6 100644 --- a/crates/strand-core/src/repo.rs +++ b/crates/strand-core/src/repo.rs @@ -128,6 +128,8 @@ impl Repo { Some("revert".into()) } else if has("MERGE_HEAD") { Some("merge".into()) + } else if has("BISECT_START") { + Some("bisect".into()) } else { None } diff --git a/crates/strand-core/src/watch.rs b/crates/strand-core/src/watch.rs index c2fce910..d297dd36 100644 --- a/crates/strand-core/src/watch.rs +++ b/crates/strand-core/src/watch.rs @@ -148,6 +148,11 @@ fn relevant_path(path: &Path, git_dir: &Path) -> bool { | "rebase-apply" | "info" | "config" + | "BISECT_START" + | "BISECT_LOG" + | "BISECT_TERMS" + | "BISECT_HEAD" + | "BISECT_EXPECTED_REV" ) } @@ -178,6 +183,8 @@ mod tests { "/repo/.git/rebase-merge/done", "/repo/.git/rebase-apply/applying", "/repo/.git/rebase-apply/next", + "/repo/.git/BISECT_START", + "/repo/.git/BISECT_LOG", ] { assert!(relevant_path(&PathBuf::from(p), &git_dir()), "{p} should refresh"); } diff --git a/crates/strand-tauri/src/commands.rs b/crates/strand-tauri/src/commands.rs index fe6b2dbc..8a32905f 100644 --- a/crates/strand-tauri/src/commands.rs +++ b/crates/strand-tauri/src/commands.rs @@ -69,6 +69,21 @@ impl From for CmdError { pub(crate) type CmdResult = std::result::Result; +#[tauri::command] +pub async fn repo_bisect_state(path: String) -> CmdResult { + run_blocking("bisect state", move || Repo::discover(path)?.bisect_state().map_err(Into::into)).await +} + +#[tauri::command] +pub async fn repo_bisect_start(path: String, good: String, bad: String, token: String) -> CmdResult { + run_blocking("start bisect", move || Repo::discover(path)?.bisect_start(&good, &bad, &token).map_err(Into::into)).await +} + +#[tauri::command] +pub async fn repo_bisect_action(path: String, action: strand_core::bisect::BisectAction, token: String) -> CmdResult { + run_blocking("bisect action", move || Repo::discover(path)?.bisect_action(action, &token).map_err(Into::into)).await +} + #[tauri::command] pub async fn repo_patch_preview(path: String, source: String, target: strand_core::interchange::PatchTarget) -> CmdResult { run_blocking("preview patch", move || Repo::discover(path)?.preview_patch_import(Path::new(&source), target).map_err(Into::into)).await diff --git a/crates/strand-tauri/src/main.rs b/crates/strand-tauri/src/main.rs index e2c7ede2..16a7ffc2 100644 --- a/crates/strand-tauri/src/main.rs +++ b/crates/strand-tauri/src/main.rs @@ -274,6 +274,9 @@ fn main() { commands::repo_remote_set_urls, commands::repo_remote_set_default, commands::repo_maintenance, + commands::repo_bisect_state, + commands::repo_bisect_start, + commands::repo_bisect_action, commands::repo_patch_preview, commands::repo_patch_import, commands::repo_mailbox_state, diff --git a/docs/learnings.md b/docs/learnings.md index e1ba3fc5..6cde2818 100644 --- a/docs/learnings.md +++ b/docs/learnings.md @@ -1,5 +1,18 @@ # Learnings +## Bisect ratings belong to the expected revision (2026-09-06) + +Read `BISECT_*` and refs from Git for every dialog refresh/action; these are +worktree-local and may be driven by another client. Map custom terms and +`BISECT_HEAD` for no-checkout sessions, distinguish skipped ambiguity from a +culprit, and reject a rating when HEAD differs from `BISECT_EXPECTED_REV`. +Require a clean tree/index before checkout transitions and reset; test edits +must not be discarded. Review the original ref's current target again before +reset. A bisect marker remaining after a successful merge/rebase does not mean +that sequencer is still paused. Dialogs that remain open after a busy action +must restore focus once controls are enabled again; disabling the focused +button can move focus out of the modal even with a correct Tab trap. + ## Interchange state comes from Git, not a saved UI session (2026-09-06) `rebase-apply/applying` identifies `git am`; `rebase-apply` alone can mean a diff --git a/ui/src/App.tsx b/ui/src/App.tsx index 06fe2309..9976abc7 100644 --- a/ui/src/App.tsx +++ b/ui/src/App.tsx @@ -120,6 +120,7 @@ const BranchCleanupDialog = lazy(() => import('./views/BranchCleanupDialog').the const RebaseEditor = lazy(() => import('./views/RebaseEditor').then((m) => ({ default: m.RebaseEditor }))); const MaintenanceDialog = lazy(() => import('./views/MaintenanceDialog').then((m) => ({ default: m.MaintenanceDialog }))); const InterchangeDialog = lazy(() => import('./views/InterchangeDialog').then((m) => ({ default: m.InterchangeDialog }))); +const BisectDialog = lazy(() => import('./views/BisectDialog').then((m) => ({ default: m.BisectDialog }))); const WorkspaceManagerDialog = lazy(() => import('./views/WorkspaceManagerDialog').then((m) => ({ default: m.WorkspaceManagerDialog }))); const PullRequests = lazy(() => import('./views/PullRequests').then((m) => ({ default: m.PullRequests }))); @@ -353,6 +354,7 @@ export function App() { const [remoteDialog, setRemoteDialog] = useState(null); const [maintenanceOpen, setMaintenanceOpen] = useState(false); const [interchangePath, setInterchangePath] = useState(null); + const [bisectPath, setBisectPath] = useState(null); const [fileEntryDialog, setFileEntryDialog] = useState<{ dir: string; directory: boolean } | null>(null); // null = closed; otherwise the branch to rename. const [renameBranchDialog, setRenameBranchDialog] = useState<{ name: string } | null>(null); @@ -1164,6 +1166,7 @@ export function App() { openInEditor, openInTerminal, openInterchange: () => { const path = useRepo.getState().activePath; if (path) setInterchangePath(path); }, + openBisect: () => { const path = useRepo.getState().activePath; if (path) setBisectPath(path); }, }; const hasRepo = Boolean(meta); useEffect(() => { @@ -1879,6 +1882,7 @@ export function App() { : []), { id: 'remote-add', label: 'Add remote…', group: 'Actions', keywords: 'remote origin upstream url add', run: () => setRemoteDialog({ kind: 'add' }) }, { id: 'git-interchange', label: 'Patches, mailboxes & bundles…', group: 'Actions', keywords: 'import export apply index working tree am continue skip abort author bundle verify prerequisites', run: () => { setPaletteOpen(false); setInterchangePath(meta.path); } }, + { id: 'git-bisect', label: 'Guided bisect…', group: 'Actions', keywords: 'good bad skip regression culprit test resume reset', run: () => { setPaletteOpen(false); setBisectPath(meta.path); } }, { id: 'repository-maintenance', label: 'Repository maintenance…', group: 'Actions', keywords: 'git gc fsck integrity optimize activity log command output', run: () => { setPaletteOpen(false); setMaintenanceOpen(true); @@ -1976,7 +1980,7 @@ export function App() { { id: 'toggle-sidebar', label: sidebarCollapsed ? 'Show sidebar' : 'Hide sidebar', group: 'Actions', shortcut: keyHint('toggle-sidebar'), keywords: 'sidebar collapse expand hide show panel', run: toggleSidebar }, ); // Surface "Abort" in the palette only while an op is actually paused. - if (meta?.operation) { + if (meta?.operation && meta.operation !== 'bisect' && meta.operation !== 'mailbox') { base.push({ id: 'abort-op', label: `Abort ${meta.operation}`, @@ -2255,7 +2259,7 @@ export function App() { ) : view === 'work' ? ( workbenchComposed ? (
- { if (meta) setInterchangePath(meta.path); }} /> + { if (meta) setInterchangePath(meta.path); }} onOpenBisect={() => { if (meta) setBisectPath(meta.path); }} /> )} - { if (meta) setInterchangePath(meta.path); }} /> + { if (meta) setInterchangePath(meta.path); }} onOpenBisect={() => { if (meta) setBisectPath(meta.path); }} /> {mainSurfaceId && ( setMaintenanceOpen(false)} onToast={showToast} /> )} {interchangePath && setInterchangePath(null)} />} + {bisectPath && setBisectPath(null)} />} {fileEntryDialog && meta && ( , string> = { mailbox: 'Mailbox in progress', + bisect: 'Bisect in progress', rebase: 'Rebase in progress', 'cherry-pick': 'Cherry-pick in progress', revert: 'Revert in progress', @@ -2663,7 +2669,7 @@ const OP_LABEL: Record, string> = { * conflict remains. The op clears `operation` on the next refresh, which hides * the banner. */ -function OpBanner({ onToast, onOpenMailbox }: { onToast: (msg: string, kind?: 'success' | 'error') => void; onOpenMailbox: () => void }) { +function OpBanner({ onToast, onOpenMailbox, onOpenBisect }: { onToast: (msg: string, kind?: 'success' | 'error') => void; onOpenMailbox: () => void; onOpenBisect: () => void }) { const operation = useRepo((s) => s.meta?.operation ?? null); const status = useRepo((s) => s.status); const abortOperation = useRepo((s) => s.abortOperation); @@ -2673,6 +2679,7 @@ function OpBanner({ onToast, onOpenMailbox }: { onToast: (msg: string, kind?: 's const hasConflicts = useMemo(() => status.some((s) => s.kind === 'CONFLICTED'), [status]); if (!operation) return null; + if (operation === 'bisect') return
Bisect in progressTest the selected revision, then rate it.
; if (operation === 'mailbox') return
Mailbox in progressResolve and stage conflicts, then continue the mailbox.
; const onAbort = async () => { diff --git a/ui/src/lib/bisect.test.ts b/ui/src/lib/bisect.test.ts new file mode 100644 index 00000000..ad761e5c --- /dev/null +++ b/ui/src/lib/bisect.test.ts @@ -0,0 +1,12 @@ +import { expect, it } from 'vitest'; +import { bisectRatingBlock, type BisectState } from './bisect'; +const state: BisectState = { active: true, token: 'a', original: 'main', original_tip: 'tip', current: 'candidate', subject: 'Change', expected: 'candidate', good_term: 'good', bad_term: 'bad', remaining: 8, remaining_truncated: false, range_error: '', culprit: null, ambiguous: false, no_checkout: false, clean: true, log: '' }; +it('allows the selected clean revision but blocks an external checkout', () => { + expect(bisectRatingBlock(state)).toBeNull(); + expect(bisectRatingBlock({ ...state, current: 'different' })).toContain('differs'); +}); +it('preserves test edits and blocks rating a completed or ambiguous result', () => { + expect(bisectRatingBlock({ ...state, clean: false })).toContain('stash'); + expect(bisectRatingBlock({ ...state, culprit: 'found' })).toContain('reset'); + expect(bisectRatingBlock({ ...state, ambiguous: true })).toContain('reset'); +}); diff --git a/ui/src/lib/bisect.ts b/ui/src/lib/bisect.ts new file mode 100644 index 00000000..4da76f01 --- /dev/null +++ b/ui/src/lib/bisect.ts @@ -0,0 +1,16 @@ +export type BisectAction = 'good' | 'bad' | 'skip' | 'reset'; +export interface BisectState { + active: boolean; token: string; original: string; original_tip: string; current: string; subject: string; + expected: string; good_term: string; bad_term: string; remaining: number; + remaining_truncated: boolean; range_error: string; culprit: string | null; ambiguous: boolean; + no_checkout: boolean; clean: boolean; log: string; +} +export interface BisectOutcome { success: boolean; output: string; state: BisectState } + +export function bisectRatingBlock(state: BisectState): string | null { + if (!state.active) return 'Start a bisect session first.'; + if (!state.clean) return 'Commit or stash test edits before changing the checkout.'; + if (state.culprit || state.ambiguous) return 'Review the result, then reset to your original checkout.'; + if (state.expected && state.current !== state.expected) return 'The checkout differs from Git’s selected test revision. Restore the expected revision before rating.'; + return null; +} diff --git a/ui/src/lib/menu.ts b/ui/src/lib/menu.ts index 15a53dd1..ac4da9ef 100644 --- a/ui/src/lib/menu.ts +++ b/ui/src/lib/menu.ts @@ -49,6 +49,7 @@ export interface MenuHandlers { openInEditor(): void; openInTerminal(): void; openInterchange(): void; + openBisect(): void; } let preemptsKeydown = false; @@ -197,6 +198,7 @@ export async function installAppMenu( text: 'Repository', items: [ await item({ id: 'git-interchange', text: 'Patches, Mailboxes & Bundles…', enabled: hasRepo, action: () => handlers().openInterchange() }), + await item({ id: 'git-bisect', text: 'Guided Bisect…', enabled: hasRepo, action: () => handlers().openBisect() }), await item({ id: 'sync', text: 'Sync (Fetch + Pull + Push)', diff --git a/ui/src/lib/tauri.ts b/ui/src/lib/tauri.ts index 65484f69..634b3175 100644 --- a/ui/src/lib/tauri.ts +++ b/ui/src/lib/tauri.ts @@ -1,4 +1,5 @@ import { Channel, invoke } from '@tauri-apps/api/core'; +import type { BisectAction, BisectState, BisectOutcome } from './bisect'; import type { PatchTarget, PatchPreview, MailboxState, InterchangeOutcome, BundlePreview } from './interchange'; import type { @@ -116,6 +117,9 @@ export function errMessage(e: unknown): string { * frontend never calls `invoke` with a string literal. */ export const tauri = { + repoBisectState: (path: string) => invoke('repo_bisect_state', { path }), + repoBisectStart: (path: string, good: string, bad: string, token: string) => invoke('repo_bisect_start', { path, good, bad, token }), + repoBisectAction: (path: string, action: BisectAction, token: string) => invoke('repo_bisect_action', { path, action, token }), repoPatchPreview: (path: string, source: string, target: PatchTarget) => invoke('repo_patch_preview', { path, source, target }), repoPatchImport: (path: string, source: string, target: PatchTarget, token: string) => invoke('repo_patch_import', { path, source, target, token }), repoMailboxState: (path: string) => invoke('repo_mailbox_state', { path }), diff --git a/ui/src/lib/types.ts b/ui/src/lib/types.ts index 0f1b75f7..2cdd7041 100644 --- a/ui/src/lib/types.ts +++ b/ui/src/lib/types.ts @@ -17,7 +17,7 @@ export interface RepoMeta { * Multi-step history op paused mid-flight, or `null` in a normal state. * Drives the in-progress banner + Abort affordance. */ - operation: 'rebase' | 'cherry-pick' | 'revert' | 'merge' | 'mailbox' | null; + operation: 'rebase' | 'cherry-pick' | 'revert' | 'merge' | 'mailbox' | 'bisect' | null; /** * The shared git dir (`commondir`), identical for every worktree of the same * repository. The tab strip groups worktree tabs on this value. diff --git a/ui/src/views/BisectDialog.tsx b/ui/src/views/BisectDialog.tsx new file mode 100644 index 00000000..fc58e823 --- /dev/null +++ b/ui/src/views/BisectDialog.tsx @@ -0,0 +1,83 @@ +import { useCallback, useEffect, useRef, useState } from 'react'; +import { listen } from '@tauri-apps/api/event'; +import { Dialog } from '../components/Dialog'; +import { bisectRatingBlock, type BisectAction, type BisectOutcome, type BisectState } from '../lib/bisect'; +import { errMessage, tauri } from '../lib/tauri'; +import { useRepo } from '../stores/repo'; + +export function BisectDialog({ path, onClose }: { path: string; onClose: () => void }) { + const [state, setState] = useState(null); + const [good, setGood] = useState(''); + const [bad, setBad] = useState('HEAD'); + const [busy, setBusy] = useState(false); + const [reset, setReset] = useState(false); + const [error, setError] = useState(''); + const [output, setOutput] = useState(''); + const first = useRef(null); + const generation = useRef(0); + const mounted = useRef(true); + const refresh = useCallback(async () => { + const seq = ++generation.current; + try { const next = await tauri.repoBisectState(path); if (mounted.current && seq === generation.current) { setState(next); setReset(false); } } + catch (e) { if (mounted.current && seq === generation.current) { setState(null); setError(errMessage(e)); } } + }, [path]); + useEffect(() => { + mounted.current = true; + void refresh(); + const changed = () => void refresh(); + const unlisten = listen('repo://changed', (event) => { if (event.payload === path) changed(); }); + window.addEventListener('focus', changed); + const focus = requestAnimationFrame(() => first.current?.focus()); + return () => { mounted.current = false; generation.current++; cancelAnimationFrame(focus); window.removeEventListener('focus', changed); void unlisten.then((fn) => fn()); }; + }, [path, refresh]); + async function run(work: () => Promise) { + if (busy || !state) return; + setBusy(true); setError(''); setReset(false); generation.current++; + try { + const result = await work(); + if (mounted.current) { setState(result.state); setOutput(result.output); if (!result.success) setError(result.output); } + } catch (e) { if (mounted.current) setError(errMessage(e)); } + finally { + await refresh(); + const repo = useRepo.getState(); + if (repo.activePath === path) await Promise.all([repo.refreshLocalChanges(), repo.refreshLog()]); + if (mounted.current) { setBusy(false); requestAnimationFrame(() => first.current?.focus()); } + } + } + const blocked = state?.active ? bisectRatingBlock(state) : null; + const rate = (action: BisectAction) => { if (state) void run(() => tauri.repoBisectAction(path, action, state.token)); }; + return }> +
+

Find the commit that introduced a problem. Test each selected revision yourself, then mark the result. Repository: {path}

+ {busy &&

Git is updating the bisect session…

} + {!state && !error &&

Reading Git state…

} + {state && !state.active && <> +

Current checkout: {state.current} · {state.subject}

+ + +

Git will check out test revisions with detached HEAD. Reset returns to the original branch or detached commit. Start and checkout transitions require a clean working tree and index.

+ {!state.clean &&

Commit or stash changes before starting.

} + + } + {state?.active &&
+ {state.culprit ? 'First bad commit found' : state.ambiguous ? 'Result is ambiguous — skipped commits remain' : 'Test this revision'} + {state.culprit || state.current}

{state.subject}

+ {state.range_error ?

Search range is not available yet: {state.range_error}

:

{state.remaining}{state.remaining_truncated ? '+' : ''} candidate commits remain{state.remaining > 1 && !state.culprit && !state.ambiguous ? ` · about ${Math.ceil(Math.log2(state.remaining))} more tests for a linear history without skips` : ''}.

} + {state.no_checkout &&

This external session uses no-checkout mode. Test BISECT_HEAD; the working files remain at HEAD.

} + {blocked &&

{blocked}

} + {state.expected && state.expected !== state.current &&

Expected revision: {state.expected}

} +
+ + + +
+

Original checkout: {state.original} at {state.original_tip || 'missing ref'}. Reset ends this session and returns there. Test edits must be committed or stashed first.

+ +
Git bisect log
{state.log}
+
} + {error &&
{error}
} + {output && 
{output}
} +
+ ; +} diff --git a/website/docs/commits-and-history.md b/website/docs/commits-and-history.md index f2c19731..bf9b747f 100644 --- a/website/docs/commits-and-history.md +++ b/website/docs/commits-and-history.md @@ -1,5 +1,29 @@ # Commits & History +## Guided bisect + +Open **Repository → Guided Bisect…**, or search **Guided bisect** in Quick +Launch. Enter a known good ancestor and a known bad revision, then Start. +Git checks out a candidate with detached HEAD. Run your tests yourself and +choose **Mark good**, **Mark bad**, or **Skip — cannot test**. The dialog shows +the selected commit, remaining candidate count, and an approximate number of +tests for linear history without skips. Git’s full bisect log is inspectable. + +Once Git identifies the first bad commit, Strand shows its SHA. Skipping too +many candidates can leave an ambiguous result; Strand reports that separately. +**Reset bisect…** reviews the original branch/commit and needs a second click +to return there. Start, ratings and reset refuse dirty working trees or indexes +so test edits must first be committed or stashed. An external checkout that +differs from Git’s expected revision cannot be rated accidentally. + +Closing the dialog keeps the session. Reopen it or use the bisect banner to +resume, including sessions started in a terminal. Custom good/bad terms and +external `--no-checkout` sessions are recognized; in no-checkout mode you test +`BISECT_HEAD`, while working files stay at HEAD. **Refresh from Git** reloads +state explicitly; watcher events and focus refresh it while the dialog is open. +Tab/Shift+Tab move between controls, Enter/Space activate buttons, and Escape +closes while idle. Automated test-command execution is not included. + Strand gives you three lenses on history: the All Commits graph for reachable history, the Reflog for everywhere `HEAD` has been (including orphaned commits), and a per-file view with follow-renames history, revision compare, and blame. ## All Commits (`Mod+3`) From 4d390ce67191fc5aa145db781bd650b45872f872 Mon Sep 17 00:00:00 2001 From: Daniels-Main Date: Sun, 6 Sep 2026 17:00:48 +0200 Subject: [PATCH 3/5] feat: manage Git notes replacements and reviewed tag edits --- README.md | 3 + ROADMAP.md | 2 + TASKS.md | 5 +- crates/strand-core/src/advanced_refs.rs | 667 ++++++++++++++++++++++++ crates/strand-core/src/lib.rs | 1 + crates/strand-tauri/src/commands.rs | 110 ++++ crates/strand-tauri/src/main.rs | 8 + docs/learnings.md | 10 + ui/src/App.tsx | 9 + ui/src/components/Sidebar.tsx | 5 +- ui/src/lib/advancedRefs.ts | 7 + ui/src/lib/menu.ts | 2 + ui/src/lib/tauri.ts | 9 + ui/src/views/AdvancedRefsDialog.tsx | 119 +++++ website/docs/everyday-git.md | 26 + 15 files changed, 980 insertions(+), 3 deletions(-) create mode 100644 crates/strand-core/src/advanced_refs.rs create mode 100644 ui/src/lib/advancedRefs.ts create mode 100644 ui/src/views/AdvancedRefsDialog.tsx diff --git a/README.md b/README.md index 88710ae1..d6bff91c 100644 --- a/README.md +++ b/README.md @@ -235,6 +235,9 @@ the resolved app appearance automatically. into the working tree, index, or both; import mailboxes with original authors and Continue / Skip / Abort recovery; verify bundle refs and prerequisites, import into a new branch, and export full or incremental bundles. +- **Advanced Git refs** — inspect and edit Git notes and replacement refs; + retarget or re-annotate existing unsigned tags with old/new targets and + optional remote publication checks. External edits require a fresh review. - **Guided bisect** — start from known good/bad revisions, test and mark good/bad/skip, inspect remaining candidates and the culprit, resume external sessions, and reset to the original checkout while protecting test edits. diff --git a/ROADMAP.md b/ROADMAP.md index 4225e665..401954b7 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2163,6 +2163,8 @@ and Store certification remain external gates. retention policy, and backend justify adding it - ☑ Guided Git bisect (F10: manual good/bad/skip, progress/culprit, external-session resume, clean-tree reset and reviewed original target) +- ☑ Advanced refs (F18: Git notes, replace refs, explicit tag retarget/re-annotation + with stale-write guards and remote publication checks; native desktop verified) - Sparse checkout (cone mode first) - ☑ Patch import/mailbox and Git bundle workflows (F07: `InterchangeDialog`, validation, author-preserving mailbox recovery and new-branch bundle import; diff --git a/TASKS.md b/TASKS.md index 90a15612..252b6d38 100644 --- a/TASKS.md +++ b/TASKS.md @@ -128,9 +128,10 @@ Detailed comparison and sequencing: [`docs/git-client-1.0-audit.md`](./docs/git- - ☐ **F15 / P2 — User-defined repository/ref/file actions.** Safe executable/ argv templates, exact context, palette/menu discovery, preview, bounded output and cancellation; editor/terminal templates and internal registries already exist. -- ☐ **F18 / P3 — Advanced refs.** Git notes/replace-ref management and explicit +- ☑ **F18 / P3 — Advanced refs.** Git notes/replace-ref management and explicit tag retarget/re-annotation with current/new target review. Signed tags are F03; - existing local Review notes are separate from Git notes. + existing local Review notes are separate from Git notes. (`AdvancedRefsDialog`, + locked notes updates, replace refs and compare-and-swap tag edits; native verified.) - ☐ **F19 / P3 — Git-flow orchestration.** Opt-in tool/config detection and inspectable start/finish feature/release/hotfix flows with conflict recovery. diff --git a/crates/strand-core/src/advanced_refs.rs b/crates/strand-core/src/advanced_refs.rs new file mode 100644 index 00000000..698884eb --- /dev/null +++ b/crates/strand-core/src/advanced_refs.rs @@ -0,0 +1,667 @@ +//! Lazy Git notes/replacements and explicit, compare-and-swap tag editing. +use crate::{Error, Repo, Result}; +use serde::{Deserialize, Serialize}; + +const LIMIT: usize = 2000; + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ObjectSummary { + pub oid: String, + pub kind: String, + pub subject: String, +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::interchange::InterchangeScratch; + fn git(path: &std::path::Path, args: &[&str]) -> String { + let out = crate::git_command() + .current_dir(path) + .args(crate::GIT_SAFE_CONFIG) + .args(args) + .output() + .unwrap(); + assert!( + out.status.success(), + "{:?}: {}", + args, + String::from_utf8_lossy(&out.stderr) + ); + String::from_utf8_lossy(&out.stdout).trim().into() + } + fn fixture() -> (InterchangeScratch, Repo, Vec) { + let scratch = InterchangeScratch::new().unwrap(); + let raw = git2::Repository::init(&scratch.0).unwrap(); + let mut config = raw.config().unwrap(); + config.set_str("user.name", "Refs Tester").unwrap(); + config.set_str("user.email", "refs@example.test").unwrap(); + config.set_bool("commit.gpgsign", false).unwrap(); + config.set_bool("tag.gpgsign", false).unwrap(); + config.set_str("core.hooksPath", "/dev/null").unwrap(); + let mut ids = vec![]; + for i in 0..3 { + std::fs::write(scratch.0.join("file"), format!("{i}\n")).unwrap(); + git(&scratch.0, &["add", "."]); + git(&scratch.0, &["commit", "-m", &format!("step {i}")]); + ids.push(git(&scratch.0, &["rev-parse", "HEAD"])); + } + let repo = Repo::discover(&scratch.0).unwrap(); + (scratch, repo, ids) + } + #[test] + fn notes_preserve_namespaces_external_edits_and_worktree_sharing() { + let (scratch, repo, ids) = fixture(); + let ns = "refs/notes/commits"; + repo.write_git_note(ns, &ids[0], None, Some("first note")) + .unwrap(); + let note = repo.git_note(ns, &ids[0]).unwrap(); + assert_eq!(note.message.as_deref(), Some("first note")); + git( + &repo.path, + &["notes", "add", "-m", "external note", &ids[1]], + ); + assert!(repo + .write_git_note(ns, &ids[0], note.ref_tip.as_deref(), Some("stale")) + .is_err()); + assert_eq!( + repo.git_note(ns, &ids[1]).unwrap().message.as_deref(), + Some("external note\n") + ); + let fresh = repo.git_note(ns, &ids[0]).unwrap(); + repo.write_git_note(ns, &ids[0], fresh.ref_tip.as_deref(), Some("edited")) + .unwrap(); + repo.write_git_note("refs/notes/other", &ids[0], None, Some("separate")) + .unwrap(); + let link = scratch.0.join("linked"); + git( + &repo.path, + &["worktree", "add", "-b", "linked", link.to_str().unwrap()], + ); + let linked = Repo::discover(&link).unwrap(); + let fresh = linked.git_note(ns, &ids[0]).unwrap(); + assert_eq!(fresh.message.as_deref(), Some("edited")); + linked + .write_git_note(ns, &ids[0], fresh.ref_tip.as_deref(), None) + .unwrap(); + assert!(repo.git_note(ns, &ids[0]).unwrap().message.is_none()); + assert_eq!(repo.advanced_refs(ns).unwrap().notes.len(), 1); + assert_eq!( + repo.git_note("refs/notes/other", &ids[0]) + .unwrap() + .message + .as_deref(), + Some("separate") + ); + assert!(git(&repo.path, &["for-each-ref", "refs/strand/notes-"]).is_empty()); + } + #[test] + fn replacements_reject_cycles_type_mismatch_and_stale_writes() { + let (_s, repo, ids) = fixture(); + repo.write_replacement(&ids[0], Some(&ids[1]), None) + .unwrap(); + assert_eq!( + repo.review_replacement(&ids[0], &ids[2]) + .unwrap() + .original + .oid, + ids[0] + ); + assert!(repo.review_replacement(&ids[1], &ids[0]).is_err()); + assert!(repo.review_replacement(&ids[0], "HEAD:file").is_err()); + assert!(repo + .write_replacement(&ids[0], Some(&ids[2]), None) + .is_err()); + repo.write_replacement(&ids[0], Some(&ids[2]), Some(&ids[1])) + .unwrap(); + assert_eq!( + git( + &repo.path, + &["--no-replace-objects", "show", "-s", "--format=%s", &ids[0]] + ), + "step 0" + ); + assert!(repo + .write_replacement(&ids[0], None, Some(&ids[1])) + .is_err()); + repo.write_replacement(&ids[0], None, Some(&ids[2])) + .unwrap(); + assert!(repo + .advanced_refs("refs/notes/commits") + .unwrap() + .replacements + .is_empty()); + } + #[test] + fn tag_edit_preserves_kind_annotation_and_detects_publication_and_staleness() { + let (scratch, repo, ids) = fixture(); + git(&repo.path, &["tag", "light", &ids[0]]); + repo.edit_tag("light", &ids[1], &ids[0], TagEditKind::Retarget, None) + .unwrap(); + assert_eq!( + git(&repo.path, &["cat-file", "-t", "refs/tags/light"]), + "commit" + ); + assert!(repo + .edit_tag("light", &ids[2], &ids[0], TagEditKind::Retarget, None) + .is_err()); + git( + &repo.path, + &["tag", "-a", "annotated", "-m", "preserve me", &ids[0]], + ); + let review = repo.review_tag_edit("annotated", &ids[1]).unwrap(); + assert_eq!(review.changed_files, 1); + repo.edit_tag( + "annotated", + &ids[1], + &review.ref_oid, + TagEditKind::Retarget, + None, + ) + .unwrap(); + let review = repo.review_tag_edit("annotated", &ids[1]).unwrap(); + assert_eq!(review.annotation.as_deref(), Some("preserve me\n")); + assert!(repo + .edit_tag( + "annotated", + &ids[2], + &review.ref_oid, + TagEditKind::Reannotate, + Some("new") + ) + .is_err()); + repo.edit_tag( + "annotated", + &ids[1], + &review.ref_oid, + TagEditKind::Reannotate, + Some("new annotation"), + ) + .unwrap(); + let bare = scratch.0.join("remote.git"); + git2::Repository::init_bare(&bare).unwrap(); + git( + &repo.path, + &["remote", "add", "origin", bare.to_str().unwrap()], + ); + assert!(repo + .published_tag("origin", "annotated") + .unwrap() + .oid + .is_none()); + git(&repo.path, &["push", "origin", "refs/tags/annotated"]); + let review = repo.review_tag_edit("annotated", &ids[2]).unwrap(); + assert_eq!( + repo.published_tag("origin", "annotated") + .unwrap() + .oid + .as_deref(), + Some(review.ref_oid.as_str()) + ); + repo.edit_tag( + "annotated", + &ids[2], + &review.ref_oid, + TagEditKind::Retarget, + None, + ) + .unwrap(); + assert_ne!( + repo.published_tag("origin", "annotated").unwrap().oid, + Some(repo.review_tag_edit("annotated", &ids[2]).unwrap().ref_oid) + ); + let raw = repo.git2_owned().unwrap(); + raw.tag( + "signed", + &raw.find_object(git2::Oid::from_str(&ids[0]).unwrap(), None) + .unwrap(), + &raw.signature().unwrap(), + "signed\n-----BEGIN SSH SIGNATURE-----\nfixture", + false, + ) + .unwrap(); + let signed = repo.review_tag_edit("signed", &ids[1]).unwrap(); + assert!(signed.signed); + assert!(repo + .edit_tag( + "signed", + &ids[1], + &signed.ref_oid, + TagEditKind::Retarget, + None + ) + .is_err()); + } +} +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct NoteEntry { + pub object: String, + pub note: String, +} +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ReplaceEntry { + pub original: String, + pub replacement: String, +} +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct AdvancedRefs { + pub notes_refs: Vec, + pub notes_tip: Option, + pub notes: Vec, + pub notes_truncated: bool, + pub replacements: Vec, + pub replacements_truncated: bool, +} +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct GitNote { + pub target: ObjectSummary, + pub ref_tip: Option, + pub message: Option, +} +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ReplaceReview { + pub original: ObjectSummary, + pub replacement: ObjectSummary, + pub previous: Option, +} +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum TagEditKind { + Retarget, + Reannotate, +} +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct TagEditReview { + pub name: String, + pub ref_oid: String, + pub current: ObjectSummary, + pub proposed: ObjectSummary, + pub annotation: Option, + pub signed: bool, + pub changed_files: usize, + pub remotes: Vec, +} +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct PublishedTag { + pub remote: String, + pub oid: Option, +} + +fn notes_name(name: &str) -> Result<()> { + if !name.starts_with("refs/notes/") || !git2::Reference::is_valid_name(name) { + return Err(Error::Other( + "use a full notes ref such as refs/notes/commits".into(), + )); + } + Ok(()) +} +fn ref_tip(repo: &git2::Repository, name: &str) -> Result> { + match repo.find_reference(name) { + Ok(r) => r + .target() + .map(|o| Some(o.to_string())) + .ok_or_else(|| Error::Other("symbolic refs cannot be edited here".into())), + Err(e) if e.code() == git2::ErrorCode::NotFound => Ok(None), + Err(e) => Err(e.into()), + } +} +fn summary(object: &git2::Object<'_>) -> ObjectSummary { + ObjectSummary { + oid: object.id().to_string(), + kind: object.kind().map(|k| k.str()).unwrap_or("unknown").into(), + subject: object + .as_commit() + .and_then(|c| c.summary()) + .or_else(|| object.as_tag().and_then(|t| t.name())) + .unwrap_or("") + .into(), + } +} +fn expect_tip(repo: &git2::Repository, name: &str, expected: Option<&str>) -> Result<()> { + if ref_tip(repo, name)?.as_deref() != expected { + return Err(Error::Other( + "reference changed externally; inspect it again before editing".into(), + )); + } + Ok(()) +} + +impl Repo { + pub fn advanced_refs(&self, notes_ref: &str) -> Result { + notes_name(notes_ref)?; + let repo = self.git2_owned()?; + let mut notes_refs = vec![]; + for reference in repo.references_glob("refs/notes/*")? { + if let Some(name) = reference?.name() { + notes_refs.push(name.to_owned()); + } + if notes_refs.len() >= LIMIT { + break; + } + } + let notes_tip = ref_tip(&repo, notes_ref)?; + let mut notes = vec![]; + if notes_tip.is_some() { + for entry in repo.notes(Some(notes_ref))?.take(LIMIT + 1) { + let (note, object) = entry?; + notes.push(NoteEntry { + object: object.to_string(), + note: note.to_string(), + }); + } + } + let notes_truncated = notes.len() > LIMIT; + notes.truncate(LIMIT); + let mut replacements = vec![]; + for reference in repo.references_glob("refs/replace/*")?.take(LIMIT + 1) { + let reference = reference?; + if let (Some(original), Some(target)) = ( + reference + .name() + .and_then(|n| n.strip_prefix("refs/replace/")), + reference.target(), + ) { + replacements.push(ReplaceEntry { + original: original.to_owned(), + replacement: target.to_string(), + }); + } + } + let replacements_truncated = replacements.len() > LIMIT; + replacements.truncate(LIMIT); + Ok(AdvancedRefs { + notes_refs, + notes_tip, + notes, + notes_truncated, + replacements, + replacements_truncated, + }) + } + + pub fn git_note(&self, notes_ref: &str, revision: &str) -> Result { + notes_name(notes_ref)?; + let repo = self.git2_owned()?; + let target = repo.revparse_single(revision)?; + let tip = ref_tip(&repo, notes_ref)?; + let message = match repo.find_note(Some(notes_ref), target.id()) { + Ok(note) => { + if note.message_bytes().len() > 1024 * 1024 { + return Err(Error::Other("note exceeds the 1 MiB editing limit".into())); + } + Some( + note.message() + .ok_or_else(|| { + Error::Other("this note is not UTF-8 and cannot be edited here".into()) + })? + .to_owned(), + ) + } + Err(e) if e.code() == git2::ErrorCode::NotFound => None, + Err(e) => return Err(e.into()), + }; + Ok(GitNote { + target: summary(&target), + ref_tip: tip, + message, + }) + } + + pub fn write_git_note( + &self, + notes_ref: &str, + object: &str, + expected: Option<&str>, + message: Option<&str>, + ) -> Result<()> { + notes_name(notes_ref)?; + if message.is_some_and(|m| m.trim().is_empty() || m.len() > 1024 * 1024) { + return Err(Error::Other( + "notes must contain text and be at most 1 MiB; use Remove to delete a note".into(), + )); + } + let repo = self.git2_owned()?; + let object = git2::Oid::from_str(object)?; + repo.find_object(object, None)?; + let mut transaction = repo.transaction()?; + transaction.lock_ref(notes_ref)?; + expect_tip(&repo, notes_ref, expected)?; + // libgit2's notes writer publishes a ref. Write through a private temporary + // ref, then publish its commit to the locked real namespace. External notes + // updates cannot be lost between reading the tree and replacing its tip. + use std::sync::atomic::{AtomicU64, Ordering}; + static NEXT: AtomicU64 = AtomicU64::new(0); + let temporary = format!( + "refs/strand/notes-{}-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_err(|e| Error::Other(e.to_string()))? + .as_nanos(), + NEXT.fetch_add(1, Ordering::Relaxed) + ); + if ref_tip(&repo, &temporary)?.is_some() { + return Err(Error::Other("temporary notes ref already exists".into())); + } + let signature = repo.signature()?; + if let Some(tip) = expected { + repo.reference( + &temporary, + git2::Oid::from_str(tip)?, + false, + "Strand notes preparation", + )?; + } + let result = (|| -> Result<()> { + match message { + Some(text) => { + repo.note(&signature, &signature, Some(&temporary), object, text, true)?; + } + None => repo.note_delete(object, Some(&temporary), &signature, &signature)?, + } + let tip = repo + .find_reference(&temporary)? + .target() + .ok_or_else(|| Error::Other("notes writer returned a symbolic ref".into()))?; + transaction.set_target(notes_ref, tip, Some(&signature), "Strand edit Git note")?; + transaction.commit()?; + Ok(()) + })(); + if let Ok(mut reference) = repo.find_reference(&temporary) { + let _ = reference.delete(); + } + result + } + + pub fn review_replacement(&self, original: &str, replacement: &str) -> Result { + let repo = self.git2_owned()?; + let old = repo.revparse_single(original)?; + let new = repo.revparse_single(replacement)?; + if old.kind() != new.kind() { + return Err(Error::Other( + "replacement objects must have the same Git object type".into(), + )); + } + let mut next = new.id(); + let mut visited = std::collections::HashSet::new(); + loop { + if next == old.id() || !visited.insert(next) { + return Err(Error::Other("replacement would create a cycle".into())); + } + let Some(target) = ref_tip(&repo, &format!("refs/replace/{next}"))? else { + break; + }; + next = git2::Oid::from_str(&target)?; + if visited.len() >= 5 { + return Err(Error::Other( + "replacement chain exceeds Git's supported depth".into(), + )); + } + } + Ok(ReplaceReview { + original: summary(&old), + replacement: summary(&new), + previous: ref_tip(&repo, &format!("refs/replace/{}", old.id()))?, + }) + } + + pub fn write_replacement( + &self, + original: &str, + replacement: Option<&str>, + expected: Option<&str>, + ) -> Result<()> { + let original = git2::Oid::from_str(original)?.to_string(); + let repo = self.git2_owned()?; + let refname = format!("refs/replace/{original}"); + let mut transaction = repo.transaction()?; + transaction.lock_ref(&refname)?; + expect_tip(&repo, &refname, expected)?; + if let Some(replacement) = replacement { + let review = self.review_replacement(&original, replacement)?; + let target = git2::Oid::from_str(&review.replacement.oid)?; + transaction.set_target(&refname, target, None, "Strand edit replacement")?; + } else { + if expected.is_none() { + return Err(Error::Other("no replacement exists for this object".into())); + } + transaction.remove(&refname)?; + } + transaction.commit()?; + Ok(()) + } + + pub fn review_tag_edit(&self, name: &str, target: &str) -> Result { + let repo = self.git2_owned()?; + let refname = format!("refs/tags/{name}"); + if !git2::Reference::is_valid_name(&refname) { + return Err(Error::Other("invalid tag name".into())); + } + let ref_oid = + ref_tip(&repo, &refname)?.ok_or_else(|| Error::Other("tag no longer exists".into()))?; + let raw = repo.find_object(git2::Oid::from_str(&ref_oid)?, None)?; + let current = raw.peel_to_commit()?; + let proposed = repo.revparse_single(target)?.peel_to_commit()?; + let annotation = if let Some(tag) = raw.as_tag() { + let message = tag + .message() + .ok_or_else(|| Error::Other("tag annotation is not UTF-8".into()))?; + if message.len() > 1024 * 1024 { + return Err(Error::Other("annotation exceeds 1 MiB".into())); + } + Some(message.to_owned()) + } else { + None + }; + let signed = annotation.as_deref().is_some_and(|m| { + [ + "-----BEGIN PGP SIGNATURE-----", + "-----BEGIN SSH SIGNATURE-----", + "-----BEGIN SIGNED MESSAGE-----", + ] + .iter() + .any(|marker| m.contains(marker)) + }); + let changed_files = repo + .diff_tree_to_tree(Some(¤t.tree()?), Some(&proposed.tree()?), None)? + .deltas() + .len(); + let remotes = repo + .remotes()? + .iter() + .flatten() + .map(str::to_owned) + .collect(); + Ok(TagEditReview { + name: name.into(), + ref_oid, + current: summary(current.as_object()), + proposed: summary(proposed.as_object()), + annotation, + signed, + changed_files, + remotes, + }) + } + + pub fn edit_tag( + &self, + name: &str, + target: &str, + expected: &str, + kind: TagEditKind, + message: Option<&str>, + ) -> Result<()> { + let review = self.review_tag_edit(name, target)?; + if review.ref_oid != expected { + return Err(Error::Other( + "tag changed externally; review the targets again".into(), + )); + } + if review.signed { + return Err(Error::Other( + "editing this signed tag requires a new signature; use the signed-tag workflow" + .into(), + )); + } + if kind == TagEditKind::Reannotate && review.current.oid != review.proposed.oid { + return Err(Error::Other( + "re-annotation must keep the current target; use Retarget for a different commit" + .into(), + )); + } + let repo = self.git2_owned()?; + let object = repo.find_object(git2::Oid::from_str(&review.proposed.oid)?, None)?; + let annotation = if kind == TagEditKind::Reannotate { + let text = message + .filter(|m| !m.trim().is_empty()) + .ok_or_else(|| Error::Other("an annotation is required".into()))?; + if text.len() > 1024 * 1024 { + return Err(Error::Other("annotation exceeds 1 MiB".into())); + } + Some(text) + } else { + review.annotation.as_deref() + }; + let oid = if let Some(annotation) = annotation { + repo.tag_annotation_create(name, &object, &repo.signature()?, annotation)? + } else { + object.id() + }; + repo.reference_matching( + &format!("refs/tags/{name}"), + oid, + true, + git2::Oid::from_str(expected)?, + "Strand edit tag", + )?; + Ok(()) + } + + pub fn published_tag(&self, remote: &str, name: &str) -> Result { + let repo = self.git2_owned()?; + repo.find_remote(remote)?; + let refname = format!("refs/tags/{name}"); + if remote.starts_with('-') || !git2::Reference::is_valid_name(&refname) { + return Err(Error::Other("invalid remote or tag name".into())); + } + let result = crate::network::run_git_streaming_transcript( + &self.path, + &["ls-remote", "--refs", "--tags", "--", remote, &refname], + |_| {}, + None, + )?; + if !result.success { + return Err(Error::Other(result.output)); + } + let oid = result.output.lines().find_map(|line| { + let mut fields = line.split_whitespace(); + let oid = fields.next()?; + (fields.next()? == refname).then(|| oid.to_owned()) + }); + Ok(PublishedTag { + remote: remote.into(), + oid, + }) + } +} diff --git a/crates/strand-core/src/lib.rs b/crates/strand-core/src/lib.rs index 21d978e5..6386a79b 100644 --- a/crates/strand-core/src/lib.rs +++ b/crates/strand-core/src/lib.rs @@ -22,6 +22,7 @@ pub mod diff; pub mod stage; pub mod apply; pub mod interchange; +pub mod advanced_refs; pub mod bisect; pub mod commit; pub mod commit_metadata; diff --git a/crates/strand-tauri/src/commands.rs b/crates/strand-tauri/src/commands.rs index 8a32905f..f22bc9db 100644 --- a/crates/strand-tauri/src/commands.rs +++ b/crates/strand-tauri/src/commands.rs @@ -69,6 +69,116 @@ impl From for CmdError { pub(crate) type CmdResult = std::result::Result; +#[tauri::command] +pub async fn repo_advanced_refs( + path: String, + notes_ref: String, +) -> CmdResult { + run_blocking("inspect advanced refs", move || { + Repo::discover(path)? + .advanced_refs(¬es_ref) + .map_err(Into::into) + }) + .await +} +#[tauri::command] +pub async fn repo_git_note( + path: String, + notes_ref: String, + revision: String, +) -> CmdResult { + run_blocking("read Git note", move || { + Repo::discover(path)? + .git_note(¬es_ref, &revision) + .map_err(Into::into) + }) + .await +} +#[tauri::command] +pub async fn repo_git_note_write( + path: String, + notes_ref: String, + object: String, + expected: Option, + message: Option, +) -> CmdResult<()> { + run_blocking("write Git note", move || { + Repo::discover(path)? + .write_git_note(¬es_ref, &object, expected.as_deref(), message.as_deref()) + .map_err(Into::into) + }) + .await +} +#[tauri::command] +pub async fn repo_replace_review( + path: String, + original: String, + replacement: String, +) -> CmdResult { + run_blocking("review replacement", move || { + Repo::discover(path)? + .review_replacement(&original, &replacement) + .map_err(Into::into) + }) + .await +} +#[tauri::command] +pub async fn repo_replace_write( + path: String, + original: String, + replacement: Option, + expected: Option, +) -> CmdResult<()> { + run_blocking("write replacement", move || { + Repo::discover(path)? + .write_replacement(&original, replacement.as_deref(), expected.as_deref()) + .map_err(Into::into) + }) + .await +} +#[tauri::command] +pub async fn repo_tag_edit_review( + path: String, + name: String, + target: String, +) -> CmdResult { + run_blocking("review tag edit", move || { + Repo::discover(path)? + .review_tag_edit(&name, &target) + .map_err(Into::into) + }) + .await +} +#[tauri::command] +pub async fn repo_tag_edit( + path: String, + name: String, + target: String, + expected: String, + kind: strand_core::advanced_refs::TagEditKind, + message: Option, +) -> CmdResult<()> { + run_blocking("edit tag", move || { + Repo::discover(path)? + .edit_tag(&name, &target, &expected, kind, message.as_deref()) + .map_err(Into::into) + }) + .await +} +#[tauri::command] +pub async fn repo_tag_published( + path: String, + remote: String, + name: String, +) -> CmdResult { + run_blocking("check published tag", move || { + Repo::discover(path)? + .published_tag(&remote, &name) + .map_err(Into::into) + }) + .await +} + #[tauri::command] pub async fn repo_bisect_state(path: String) -> CmdResult { run_blocking("bisect state", move || Repo::discover(path)?.bisect_state().map_err(Into::into)).await diff --git a/crates/strand-tauri/src/main.rs b/crates/strand-tauri/src/main.rs index 16a7ffc2..78047276 100644 --- a/crates/strand-tauri/src/main.rs +++ b/crates/strand-tauri/src/main.rs @@ -274,6 +274,14 @@ fn main() { commands::repo_remote_set_urls, commands::repo_remote_set_default, commands::repo_maintenance, + commands::repo_advanced_refs, + commands::repo_git_note, + commands::repo_git_note_write, + commands::repo_replace_review, + commands::repo_replace_write, + commands::repo_tag_edit_review, + commands::repo_tag_edit, + commands::repo_tag_published, commands::repo_bisect_state, commands::repo_bisect_start, commands::repo_bisect_action, diff --git a/docs/learnings.md b/docs/learnings.md index 6cde2818..4615f98f 100644 --- a/docs/learnings.md +++ b/docs/learnings.md @@ -2519,3 +2519,13 @@ Pierre reads `navigator.userAgent` during module evaluation; Node 22's built-in `navigator` hid a failure on CI's Node 20. Stub browser globals and restore them after the test, while retaining real integration assertions. Reproduce this class of failure locally with `--no-experimental-global-navigator`. + + +### Advanced refs preserve reviewed identities (2026-09-06) + +Git notes must use a locked namespace tip and publish the prepared notes tree +atomically; a stale note editor must retain its draft when another worktree +changes that namespace. Replacement inspection uses raw object IDs because +libgit2/gix readers do not apply Git's replace refs. Tag retargeting and +re-annotation are separate operations with compare-and-swap of the raw tag ref, +not its peeled commit. Never drop an existing tag signature during an edit. diff --git a/ui/src/App.tsx b/ui/src/App.tsx index 9976abc7..00bdecd9 100644 --- a/ui/src/App.tsx +++ b/ui/src/App.tsx @@ -120,6 +120,7 @@ const BranchCleanupDialog = lazy(() => import('./views/BranchCleanupDialog').the const RebaseEditor = lazy(() => import('./views/RebaseEditor').then((m) => ({ default: m.RebaseEditor }))); const MaintenanceDialog = lazy(() => import('./views/MaintenanceDialog').then((m) => ({ default: m.MaintenanceDialog }))); const InterchangeDialog = lazy(() => import('./views/InterchangeDialog').then((m) => ({ default: m.InterchangeDialog }))); +const AdvancedRefsDialog = lazy(() => import('./views/AdvancedRefsDialog').then((m) => ({ default: m.AdvancedRefsDialog }))); const BisectDialog = lazy(() => import('./views/BisectDialog').then((m) => ({ default: m.BisectDialog }))); const WorkspaceManagerDialog = lazy(() => import('./views/WorkspaceManagerDialog').then((m) => ({ default: m.WorkspaceManagerDialog }))); const PullRequests = lazy(() => import('./views/PullRequests').then((m) => ({ default: m.PullRequests }))); @@ -354,6 +355,7 @@ export function App() { const [remoteDialog, setRemoteDialog] = useState(null); const [maintenanceOpen, setMaintenanceOpen] = useState(false); const [interchangePath, setInterchangePath] = useState(null); + const [advancedRefs, setAdvancedRefs] = useState<{ path: string; mode: 'notes' | 'replace' | 'retarget' | 'reannotate'; tag?: string } | null>(null); const [bisectPath, setBisectPath] = useState(null); const [fileEntryDialog, setFileEntryDialog] = useState<{ dir: string; directory: boolean } | null>(null); // null = closed; otherwise the branch to rename. @@ -1166,6 +1168,7 @@ export function App() { openInEditor, openInTerminal, openInterchange: () => { const path = useRepo.getState().activePath; if (path) setInterchangePath(path); }, + openAdvancedRefs: () => { const path = useRepo.getState().activePath; if (path) setAdvancedRefs({ path, mode: 'notes' }); }, openBisect: () => { const path = useRepo.getState().activePath; if (path) setBisectPath(path); }, }; const hasRepo = Boolean(meta); @@ -1882,6 +1885,10 @@ export function App() { : []), { id: 'remote-add', label: 'Add remote…', group: 'Actions', keywords: 'remote origin upstream url add', run: () => setRemoteDialog({ kind: 'add' }) }, { id: 'git-interchange', label: 'Patches, mailboxes & bundles…', group: 'Actions', keywords: 'import export apply index working tree am continue skip abort author bundle verify prerequisites', run: () => { setPaletteOpen(false); setInterchangePath(meta.path); } }, + { id: 'git-notes', label: 'Git notes…', group: 'Actions', keywords: 'advanced refs objects notes replacements tag annotation', run: () => { setPaletteOpen(false); setAdvancedRefs({ path: meta.path, mode: 'notes' }); } }, + { id: 'git-replace', label: 'Replace refs…', group: 'Actions', keywords: 'advanced refs objects notes replacements tag annotation', run: () => { setPaletteOpen(false); setAdvancedRefs({ path: meta.path, mode: 'replace' }); } }, + { id: 'git-retarget', label: 'Retarget tag…', group: 'Actions', keywords: 'advanced refs objects notes replacements tag annotation', run: () => { setPaletteOpen(false); setAdvancedRefs({ path: meta.path, mode: 'retarget' }); } }, + { id: 'git-reannotate', label: 'Re-annotate tag…', group: 'Actions', keywords: 'advanced refs objects notes replacements tag annotation', run: () => { setPaletteOpen(false); setAdvancedRefs({ path: meta.path, mode: 'reannotate' }); } }, { id: 'git-bisect', label: 'Guided bisect…', group: 'Actions', keywords: 'good bad skip regression culprit test resume reset', run: () => { setPaletteOpen(false); setBisectPath(meta.path); } }, { id: 'repository-maintenance', label: 'Repository maintenance…', group: 'Actions', keywords: 'git gc fsck integrity optimize activity log command output', run: () => { setPaletteOpen(false); @@ -2219,6 +2226,7 @@ export function App() { onOpenRecent={openByPath} onCreateStash={() => setStashDialog({ snapshot: true, keepIndex: false })} onCreateTag={() => setTagDialog({ target: null, label: 'HEAD' })} + onEditTag={(tag, mode) => { if (meta) setAdvancedRefs({ path: meta.path, mode, tag }); }} onCreateBranch={(start, label) => setBranchDialog({ start, label })} onBranchFromStash={(index) => setBranchDialog({ start: `stash@{${index}}`, @@ -2395,6 +2403,7 @@ export function App() { setMaintenanceOpen(false)} onToast={showToast} /> )} {interchangePath && setInterchangePath(null)} />} + {advancedRefs && setAdvancedRefs(null)} />} {bisectPath && setBisectPath(null)} />} {fileEntryDialog && meta && ( diff --git a/ui/src/components/Sidebar.tsx b/ui/src/components/Sidebar.tsx index a1b1c73f..ddb576db 100644 --- a/ui/src/components/Sidebar.tsx +++ b/ui/src/components/Sidebar.tsx @@ -87,6 +87,7 @@ interface SidebarProps { onCreateStash: () => void; /** Open the New-tag dialog targeting HEAD. */ onCreateTag: () => void; + onEditTag: (name: string, kind: 'retarget' | 'reannotate') => void; /** Open the New-branch dialog from `start` (`null` ⇒ HEAD); `label` is the * human name shown in the blurb. */ onCreateBranch: (start: string | null, label: string) => void; @@ -176,7 +177,7 @@ function sortTree(node: TreeNode, leafCmp: (a: T, b: T) => number): void { // ─── component ────────────────────────────────────────────────────────── -export function Sidebar({ onOpenWorkbench, onOpenWorkSurface, onOpenRepo, onOpenRecent, onCreateStash, onCreateTag, onCreateBranch, onBranchFromStash, onCreateWorktree, onMerge, onInteractiveRebase, onManageRemote, onRenameBranch, onManageBranchNetwork, onPull, onPush, onForcePush, onFetchBranch, onPullBranch, onOpenFileInEditor, onCreateFileEntry, onToast }: SidebarProps) { +export function Sidebar({ onOpenWorkbench, onOpenWorkSurface, onOpenRepo, onOpenRecent, onCreateStash, onCreateTag, onEditTag, onCreateBranch, onBranchFromStash, onCreateWorktree, onMerge, onInteractiveRebase, onManageRemote, onRenameBranch, onManageBranchNetwork, onPull, onPush, onForcePush, onFetchBranch, onPullBranch, onOpenFileInEditor, onCreateFileEntry, onToast }: SidebarProps) { const view = useRepo((s) => s.view); const setView = useRepo((s) => s.setView); const selectFile = useRepo((s) => s.selectFile); @@ -745,6 +746,8 @@ export function Sidebar({ onOpenWorkbench, onOpenWorkSurface, onOpenRepo, onOpen const tagMenu = (tg: Tag): MenuItem[] => { const items: MenuItem[] = [ + { label: 'Retarget tag…', onSelect: () => onEditTag(tg.name, 'retarget') }, + { label: 'Re-annotate tag…', onSelect: () => onEditTag(tg.name, 'reannotate') }, { label: 'Checkout', icon: 'branch', onSelect: () => void runBranchOp(() => checkoutCommit(tg.target)) }, { label: 'New branch from here…', icon: 'plus', onSelect: () => onCreateBranch(tg.full_name, tg.name) }, { label: 'New worktree from here…', icon: 'worktree', onSelect: () => onCreateWorktree({ ref: tg.full_name, label: tg.name }) }, diff --git a/ui/src/lib/advancedRefs.ts b/ui/src/lib/advancedRefs.ts new file mode 100644 index 00000000..0304f8ef --- /dev/null +++ b/ui/src/lib/advancedRefs.ts @@ -0,0 +1,7 @@ +export interface ObjectSummary { oid: string; kind: string; subject: string } +export interface AdvancedRefs { notes_refs: string[]; notes_tip: string | null; notes: Array<{ object: string; note: string }>; notes_truncated: boolean; replacements: Array<{ original: string; replacement: string }>; replacements_truncated: boolean } +export interface GitNote { target: ObjectSummary; ref_tip: string | null; message: string | null } +export interface ReplaceReview { original: ObjectSummary; replacement: ObjectSummary; previous: string | null } +export type TagEditKind = 'retarget' | 'reannotate'; +export interface TagEditReview { name: string; ref_oid: string; current: ObjectSummary; proposed: ObjectSummary; annotation: string | null; signed: boolean; changed_files: number; remotes: string[] } +export interface PublishedTag { remote: string; oid: string | null } diff --git a/ui/src/lib/menu.ts b/ui/src/lib/menu.ts index ac4da9ef..27564166 100644 --- a/ui/src/lib/menu.ts +++ b/ui/src/lib/menu.ts @@ -50,6 +50,7 @@ export interface MenuHandlers { openInTerminal(): void; openInterchange(): void; openBisect(): void; + openAdvancedRefs(): void; } let preemptsKeydown = false; @@ -198,6 +199,7 @@ export async function installAppMenu( text: 'Repository', items: [ await item({ id: 'git-interchange', text: 'Patches, Mailboxes & Bundles…', enabled: hasRepo, action: () => handlers().openInterchange() }), + await item({ id: 'git-advanced-refs', text: 'Git Notes, Replacements & Tag Editing…', enabled: hasRepo, action: () => handlers().openAdvancedRefs() }), await item({ id: 'git-bisect', text: 'Guided Bisect…', enabled: hasRepo, action: () => handlers().openBisect() }), await item({ id: 'sync', diff --git a/ui/src/lib/tauri.ts b/ui/src/lib/tauri.ts index 634b3175..99497f40 100644 --- a/ui/src/lib/tauri.ts +++ b/ui/src/lib/tauri.ts @@ -1,4 +1,5 @@ import { Channel, invoke } from '@tauri-apps/api/core'; +import type { AdvancedRefs, GitNote, ReplaceReview, TagEditReview, TagEditKind, PublishedTag } from './advancedRefs'; import type { BisectAction, BisectState, BisectOutcome } from './bisect'; import type { PatchTarget, PatchPreview, MailboxState, InterchangeOutcome, BundlePreview } from './interchange'; @@ -117,6 +118,14 @@ export function errMessage(e: unknown): string { * frontend never calls `invoke` with a string literal. */ export const tauri = { + repoAdvancedRefs: (path: string, notesRef: string) => invoke('repo_advanced_refs', { path, notesRef }), + repoGitNote: (path: string, notesRef: string, revision: string) => invoke('repo_git_note', { path, notesRef, revision }), + repoGitNoteWrite: (path: string, notesRef: string, object: string, expected: string | null, message: string | null) => invoke('repo_git_note_write', { path, notesRef, object, expected, message }), + repoReplaceReview: (path: string, original: string, replacement: string) => invoke('repo_replace_review', { path, original, replacement }), + repoReplaceWrite: (path: string, original: string, replacement: string | null, expected: string | null) => invoke('repo_replace_write', { path, original, replacement, expected }), + repoTagEditReview: (path: string, name: string, target: string) => invoke('repo_tag_edit_review', { path, name, target }), + repoTagEdit: (path: string, name: string, target: string, expected: string, kind: TagEditKind, message: string | null) => invoke('repo_tag_edit', { path, name, target, expected, kind, message }), + repoTagPublished: (path: string, remote: string, name: string) => invoke('repo_tag_published', { path, remote, name }), repoBisectState: (path: string) => invoke('repo_bisect_state', { path }), repoBisectStart: (path: string, good: string, bad: string, token: string) => invoke('repo_bisect_start', { path, good, bad, token }), repoBisectAction: (path: string, action: BisectAction, token: string) => invoke('repo_bisect_action', { path, action, token }), diff --git a/ui/src/views/AdvancedRefsDialog.tsx b/ui/src/views/AdvancedRefsDialog.tsx new file mode 100644 index 00000000..f8a0fae2 --- /dev/null +++ b/ui/src/views/AdvancedRefsDialog.tsx @@ -0,0 +1,119 @@ +import { useCallback, useEffect, useRef, useState } from 'react'; +import { listen } from '@tauri-apps/api/event'; +import { Dialog } from '../components/Dialog'; +import { Select } from '../components/Select'; +import { errMessage, tauri } from '../lib/tauri'; +import type { AdvancedRefs, GitNote, ReplaceReview, TagEditKind, TagEditReview } from '../lib/advancedRefs'; +import { useRepo } from '../stores/repo'; + +export function AdvancedRefsDialog({ path, initialMode = 'notes', initialTag = '', onClose }: { path: string; initialMode?: 'notes' | 'replace' | TagEditKind; initialTag?: string; onClose: () => void }) { + const [mode, setMode] = useState(initialMode); + const [notesRef, setNotesRef] = useState('refs/notes/commits'); + const [data, setData] = useState(null); + const [revision, setRevision] = useState('HEAD'); + const [note, setNote] = useState(null); + const [message, setMessage] = useState(''); + const [original, setOriginal] = useState(''); + const [replacement, setReplacement] = useState(''); + const [replaceReview, setReplaceReview] = useState(null); + const [tag, setTag] = useState(initialTag); + const [target, setTarget] = useState('HEAD'); + const [annotation, setAnnotation] = useState(''); + const [tagReview, setTagReview] = useState(null); + const [remote, setRemote] = useState(''); + const [published, setPublished] = useState('Publication has not been checked.'); + const [acknowledged, setAcknowledged] = useState(false); + const [confirm, setConfirm] = useState(''); + const [busy, setBusy] = useState(false); + const [error, setError] = useState(''); + const [output, setOutput] = useState(''); + const first = useRef(null); + const mounted = useRef(true); + const reads = useRef(0); + const refresh = useCallback(async () => { + const seq = ++reads.current; + try { const next = await tauri.repoAdvancedRefs(path, notesRef); if (mounted.current && seq === reads.current) setData(next); } + catch (e) { if (mounted.current && seq === reads.current) { setData(null); setError(errMessage(e)); } } + }, [path, notesRef]); + useEffect(() => { + mounted.current = true; + void refresh(); + const changed = () => { setConfirm(''); void refresh(); }; + // Any tab in this repository family can change the shared refs. Reads remain + // confined to this open dialog; no advanced-ref work rides the repo snapshot. + const unlisten = listen('repo://changed', changed); + window.addEventListener('focus', changed); + return () => { mounted.current = false; reads.current++; window.removeEventListener('focus', changed); void unlisten.then((fn) => fn()); }; + }, [refresh]); + useEffect(() => { const focus = requestAnimationFrame(() => first.current?.focus()); return () => cancelAnimationFrame(focus); }, []); + async function run(work: () => Promise, mutation = false) { + if (busy) return; + setBusy(true); setError(''); + try { await work(); if (mutation) { setOutput('Local Git reference updated.'); setConfirm(''); } } + catch (e) { if (mounted.current) setError(errMessage(e)); } + finally { + if (mutation) { await refresh(); const repo = useRepo.getState(); if (repo.activePath === path) await Promise.all([repo.refreshLocalChanges(), repo.refreshLog()]); } + if (mounted.current) { setBusy(false); requestAnimationFrame(() => first.current?.focus()); } + } + } + async function inspectNote(object = revision) { const read = await tauri.repoGitNote(path, notesRef, object); setNote(read); setMessage(read.message ?? ''); setRevision(object); setConfirm(''); } + async function inspectTag() { + const review = await tauri.repoTagEditReview(path, tag, mode === 'reannotate' ? `refs/tags/${tag}` : target); + setTagReview(review); setAnnotation(review.annotation ?? ''); setRemote(review.remotes[0] ?? ''); setAcknowledged(false); setPublished('Publication has not been checked.'); setConfirm(''); + } + function clearTag() { setTagReview(null); setAcknowledged(false); setConfirm(''); } + const tags = useRepo((s) => s.refs.tags); + return {busy ? 'Working with Git…' : ''}}> +
+

Repository: {path}. These are Git objects and refs, separate from Strand’s local Review notes.

+ + {mode === 'notes' && <> + + + + + {note &&
+ {note.target.oid}

{note.target.kind} · {note.target.subject}

+