Release v0.74.0

CHANGELOG.md

@@ -1,13 +1,37 @@
 # Changelog
 
-## Unreleased
+## 0.74.0
 
 ### Improved
 
-- **`wt config approvals add --yes` records approvals without a terminal**: the command refused every non-interactive run, so a container that had just cloned a project could only pre-approve it by hand-writing `approvals.toml` from `wt config approvals list --format=json`. `--yes` now lists what it trusts and writes it. A save it cannot make fails the command rather than warning and exiting 0, since the record is all `add` produces — a change that applies to the interactive run too.
+- **A JSON `true` or `null` nested in a `vars` value renders as `True` or `None`**: minijinja 2.22 adopted Jinja2's spelling, so `{{ vars.<key>.<field> }}` into an object set by `wt config state vars set` prints those words. A var whose whole value is `true` stays a string. (Breaking: the old spellings were `true` and `none`.) ([#3795](https://github.com/max-sixty/worktrunk/pull/3795))
+
+- **`wt config approvals add --yes` records approvals without a terminal**: the command refused every non-interactive run, so a container or CI job could not pre-approve a project it had just cloned. `--yes` now lists what it trusts and writes it, and a save it cannot make fails the command rather than warning and exiting 0. ([#3819](https://github.com/max-sixty/worktrunk/pull/3819))
 
 - **`remote_repo` names the repository as the remote spells it**: `repo` is the directory on disk, so a renamed clone reports the new name. `{{ remote_repo }}` takes it from the primary remote's URL, available everywhere `owner` is and unset when no remote parses. ([#3745](https://github.com/max-sixty/worktrunk/pull/3745), thanks @canac)
 
+### Fixed
+
+- **`wt remove --force` deleted a live worktree moved onto another's registered path**: the ownership gate asked which *repository* the occupant belonged to, which a sibling worktree of this repo satisfies, so the directory went to trash with its uncommitted work. The directory and the registration must now name each other, as `git worktree remove` requires. Also reaches `wt merge`, `wt step prune`, and picker removal, and refuses a bare repository's path that `--force` previously deleted. ([#3808](https://github.com/max-sixty/worktrunk/pull/3808))
+
+- **`wt step copy-ignored` skips a source file that vanishes mid-copy**: the walk collects leaves, then copies them in parallel, so a file removed in between — a concurrent build rewriting `target/`, say — aborted the batch, under `--force` after deleting the destination. The source is stat'd before anything is removed now. ([#3744](https://github.com/max-sixty/worktrunk/pull/3744), fixes [#3743](https://github.com/max-sixty/worktrunk/issues/3743), thanks @dataders for reporting and fixing)
+
+- **`wt step promote` refuses to delete a source it did not fully copy**: across filesystems the move copies then deletes, and an entry with no copy — a socket or FIFO, or a file a concurrent build rewrote mid-walk — was deleted anyway. The copy now reports what it skipped, and a non-zero count fails the move. ([#3820](https://github.com/max-sixty/worktrunk/pull/3820))
+
+- **An `ssh://` remote's host comes from the authority, not a later path segment**: the search for the userinfo `@` covered the whole remainder, so `ssh://git@attacker.example/owner/repo@github.com/org/repo.git` resolved to `github.com` and borrowed that repository's approvals while git dialed the attacker. Every URL form now splits the authority first, so `@` in a namespace parses instead of being rejected. ([#3813](https://github.com/max-sixty/worktrunk/pull/3813))
+
+- **`WORKTRUNK_*` env vars and `--config-set` outrank a `[projects."…"]` entry**: layer and specificity resolved separately, so a project entry answered for the global key whichever layer set it — `WORKTRUNK_WORKTREE_PATH` could not override a project's `worktree-path`. Both invocation layers now apply at either scope, and the help text gained a precedence section. ([#3790](https://github.com/max-sixty/worktrunk/pull/3790), fixes [#3788](https://github.com/max-sixty/worktrunk/issues/3788), thanks @jeremy0dell for reporting)
+
+- **Shell completions register under the `--cmd` name**: `wt config shell init zsh --cmd wot` renamed the wrapper, but clap emitted the registration under `wt`. Bash and zsh completed nothing and regenerated the script on every TAB, PowerShell registered the wrong command, and zsh's `compdef` handed worktrunk's completer to the other `wt`. ([#3817](https://github.com/max-sixty/worktrunk/pull/3817), fixes [#3816](https://github.com/max-sixty/worktrunk/issues/3816), thanks @sbarre for reporting)
+
+### Documentation
+
+- **The picker's `Alt-x` never forces**: the keybinding table read `Remove selected worktree/branch`, which sent a reader looking for a force-remove that isn't there. ([#3811](https://github.com/max-sixty/worktrunk/pull/3811), raised in [#3809](https://github.com/max-sixty/worktrunk/issues/3809), thanks @josh-burton)
+
+### Internal
+
+- **Library API rework** (Breaking library API): `GitError::WorktreePathNotOurs` gained an `occupant_registered_at` field, and `WorkingTree::ensure_belongs_to_repo` is now `ensure_holds_this_worktree`. ([#3808](https://github.com/max-sixty/worktrunk/pull/3808))
+
 ## 0.73.0
 
 ### Fixed

Cargo.toml

@@ -49,7 +49,7 @@ filterset = "test(/readme_sync/)"
 
 [package]
 name = "worktrunk"
-version = "0.73.0"
+version = "0.74.0"
 edition = "2024"
 rust-version = "1.96"
 build = "build.rs"

src/commands/step/copy_ignored.rs

@@ -242,10 +242,12 @@ pub fn step_copy_ignored(
         let dest_entry = dest_path.join(relative);
 
         if *is_dir {
-            copy_dir_recursive(src_entry, &dest_entry, Some(&dest_path), force, &progress)
-                .with_context(|| {
-                    format!("copying directory {}", format_path_for_display(relative))
-                })?;
+            // A pure copy deletes no source, so the skip count has nothing to guard.
+            let _skipped =
+                copy_dir_recursive(src_entry, &dest_entry, Some(&dest_path), force, &progress)
+                    .with_context(|| {
+                        format!("copying directory {}", format_path_for_display(relative))
+                    })?;
         } else {
             if let Some(parent) = dest_entry.parent() {
                 fs::create_dir_all(parent).with_context(|| {

src/commands/step/promote.rs

@@ -4,11 +4,12 @@ use std::fs;
 use std::io::ErrorKind;
 use std::path::{Path, PathBuf};
 
-use anyhow::Context;
+use anyhow::{Context, bail};
 use color_print::cformat;
 use path_slash::PathExt as _;
 use worktrunk::copy::{copy_dir_recursive, copy_leaf};
 use worktrunk::git::Repository;
+use worktrunk::path::format_path_for_display;
 use worktrunk::progress::Progress;
 use worktrunk::styling::{eprintln, hint_message, info_message, success_message, warning_message};
 
@@ -34,12 +35,27 @@ fn move_entry(src: &Path, dest: &Path, is_dir: bool) -> anyhow::Result<()> {
 }
 
 /// Copy then delete — fallback when `rename` fails with EXDEV (cross-device).
+///
+/// Refuses to delete when the copy skipped an entry: the source still holds
+/// content the destination never received, so the delete would destroy it.
 fn copy_and_remove(src: &Path, dest: &Path, is_dir: bool) -> anyhow::Result<()> {
     if is_dir {
-        copy_dir_recursive(src, dest, None, true, &Progress::disabled())?;
+        let skipped = copy_dir_recursive(src, dest, None, true, &Progress::disabled())?;
+        if skipped > 0 {
+            let entry_word = if skipped == 1 { "entry" } else { "entries" };
+            bail!(
+                "{skipped} {entry_word} under {} could not be copied, so the source is left in place; run with -vv to list them",
+                format_path_for_display(src)
+            );
+        }
         fs::remove_dir_all(src).context(format!("removing source directory {}", src.display()))?;
     } else {
-        copy_leaf(src, dest, None, true)?;
+        if copy_leaf(src, dest, None, true)?.is_none() {
+            bail!(
+                "{} could not be copied, so the source is left in place; run with -vv for the reason",
+                format_path_for_display(src)
+            );
+        }
 
         fs::remove_file(src).context(format!("removing source file {}", src.display()))?;
     }
@@ -509,4 +525,55 @@ mod tests {
         );
         assert_eq!(fs::read_to_string(dest.join("root.txt")).unwrap(), "root");
     }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_copy_and_remove_keeps_a_directory_holding_an_uncopied_entry() {
+        // The move copies then deletes, so an entry the copy skipped would be
+        // destroyed rather than moved. A socket is the deterministic stand-in:
+        // it has no copy, exactly as a source that vanished mid-walk has none.
+        let tmp = tempfile::tempdir().unwrap();
+        let src = tmp.path().join("srcdir");
+        let dest = tmp.path().join("destdir");
+        fs::create_dir_all(&src).unwrap();
+        fs::write(src.join("regular.txt"), "content").unwrap();
+        let _listener = std::os::unix::net::UnixListener::bind(src.join("sock")).unwrap();
+
+        let err = copy_and_remove(&src, &dest, true).unwrap_err();
+
+        assert!(
+            err.to_string().contains("1 entry under")
+                && err.to_string().contains("could not be copied"),
+            "unexpected error: {err}"
+        );
+        assert!(src.join("sock").exists());
+        assert_eq!(
+            fs::read_to_string(src.join("regular.txt")).unwrap(),
+            "content"
+        );
+        // The partial copy is left behind: `check_leftover_staging` is what
+        // stops a later promote from walking past it.
+        assert_eq!(
+            fs::read_to_string(dest.join("regular.txt")).unwrap(),
+            "content"
+        );
+    }
+
+    #[test]
+    fn test_copy_and_remove_keeps_a_file_that_was_not_copied() {
+        // The leaf branch reads `copy_leaf`'s `Ok(None)` the same way. A source
+        // that is already gone is the deterministic case; `fs::remove_file`
+        // must not be reached, since it would delete a file recreated since.
+        let tmp = tempfile::tempdir().unwrap();
+        let src = tmp.path().join("gone.txt");
+        let dest = tmp.path().join("dest.txt");
+
+        let err = copy_and_remove(&src, &dest, false).unwrap_err();
+
+        assert!(
+            err.to_string().contains("could not be copied"),
+            "unexpected error: {err}"
+        );
+        assert!(!dest.exists());
+    }
 }

src/copy.rs

@@ -22,6 +22,7 @@ use std::fs;
 use std::io::ErrorKind;
 use std::path::{Path, PathBuf};
 use std::sync::LazyLock;
+use std::sync::atomic::{AtomicUsize, Ordering};
 
 use anyhow::Context;
 use rayon::prelude::*;
@@ -187,7 +188,7 @@ struct CopyLeaf {
 ///
 /// Walks the tree iteratively (no recursion), then copies all files and
 /// symlinks in parallel on a dedicated 4-thread pool. Non-regular files
-/// (sockets, FIFOs) are silently skipped. Existing entries at the destination
+/// (sockets, FIFOs) are skipped. Existing entries at the destination
 /// are skipped for idempotent usage.
 ///
 /// When `force` is true, existing files and symlinks at the destination are
@@ -201,6 +202,13 @@ struct CopyLeaf {
 /// of the source's. Every such skip logs at debug (`wt -vv`). The tree's own
 /// root is the exception: the caller named it, so its absence is an error.
 ///
+/// Returns how many of the entries the walk collected were not copied. A pure
+/// copy ignores it; a caller that deletes the source afterwards must refuse on a
+/// non-zero count, since the source still holds content the destination never
+/// received. Zero does not prove the destination holds everything the source
+/// does — a file created after `read_dir` snapshots its directory is never seen,
+/// and copy-then-delete cannot close that without a rename.
+///
 /// Each copied leaf is recorded on `progress` —
 /// skipped entries are not counted — so a `progress` dedicated to this call
 /// reports `(files_copied, bytes_copied)` in `totals()` afterwards; a shared
@@ -209,14 +217,16 @@ struct CopyLeaf {
 /// When `root` is `Some`, refuses destination directory ancestry that resolves
 /// outside `root`. Leaves inherit the guarantee because `entry.file_name()` is
 /// a single basename and cannot escape the validated parent directory.
+#[must_use = "a caller that deletes the source must refuse on a non-zero skip count"]
 pub fn copy_dir_recursive(
     src: &Path,
     dest: &Path,
     root: Option<&Path>,
     force: bool,
     progress: &Progress,
-) -> anyhow::Result<()> {
+) -> anyhow::Result<usize> {
     // Phase 1: Walk directories iteratively, creating dest dirs and collecting leaves.
+    let mut skipped = 0usize;
     let mut leaves = Vec::new();
     // The bool marks the tree's own root: the caller named it, so its absence
     // is a real error, while a subdirectory that disappears mid-walk is the
@@ -237,6 +247,7 @@ pub fn copy_dir_recursive(
             Ok(entries) => entries,
             Err(e) if e.kind() == ErrorKind::NotFound && !is_root => {
                 tracing::debug!(path = %src_dir.display(), "skipping vanished directory: {}", src_dir.display());
+                skipped += 1;
                 continue;
             }
             Err(e) => {
@@ -256,6 +267,7 @@ pub fn copy_dir_recursive(
                 // The entry was listed but is gone already — same race.
                 Err(e) if e.kind() == ErrorKind::NotFound => {
                     tracing::debug!(path = %entry.path().display(), "skipping vanished entry: {}", entry.path().display());
+                    skipped += 1;
                     continue;
                 }
                 Err(e) => {
@@ -274,22 +286,30 @@ pub fn copy_dir_recursive(
                     dest: dest_path,
                 });
             } else {
+                // Sockets and FIFOs have no copy, so the destination never
+                // receives them — a skip like any other.
                 tracing::debug!(path = %src_path.display(), "skipping non-regular file: {}", src_path.display());
+                skipped += 1;
             }
         }
     }
 
     // Phase 2: Copy all leaves in parallel.
+    let skipped_leaves = AtomicUsize::new(0);
     COPY_POOL.install(|| {
         leaves
             .par_iter()
             .try_for_each(|leaf| -> anyhow::Result<()> {
-                if let Some(bytes) = copy_leaf(&leaf.src, &leaf.dest, None, force)? {
-                    progress.record(bytes);
+                match copy_leaf(&leaf.src, &leaf.dest, None, force)? {
+                    Some(bytes) => progress.record(bytes),
+                    None => {
+                        skipped_leaves.fetch_add(1, Ordering::Relaxed);
+                    }
                 }
                 Ok(())
             })
     })?;
+    skipped += skipped_leaves.into_inner();
 
     // Phase 3: Preserve source directory permissions AFTER copying contents.
     // Must be done after copying — if the source lacks write permission (e.g., 0o555),
@@ -303,6 +323,7 @@ pub fn copy_dir_recursive(
             // permissions rather than failing a batch that otherwise succeeded.
             Err(e) if e.kind() == ErrorKind::NotFound => {
                 tracing::debug!(path = %src_dir.display(), "skipping permissions for vanished directory: {}", src_dir.display());
+                skipped += 1;
                 continue;
             }
             Err(e) => {
@@ -314,7 +335,7 @@ pub fn copy_dir_recursive(
             .with_context(|| format!("setting permissions on {}", dest_dir.display()))?;
     }
 
-    Ok(())
+    Ok(skipped)
 }
 
 /// Remove a file, ignoring "not found" errors. Reports whether one was removed,
@@ -444,6 +465,64 @@ mod tests {
         assert_eq!(fs::read(&dest).unwrap(), b"source content");
     }
 
+    #[test]
+    fn test_copy_dir_recursive_counts_skipped_existing_destination() {
+        // The skipped count is what tells a caller that deletes the source
+        // whether the destination received everything. An idempotent re-run
+        // skips a leaf that is already there, and says so.
+        let dir = tempfile::tempdir().unwrap();
+        let src = dir.path().join("src");
+        let dest = dir.path().join("dest");
+        fs::create_dir_all(&src).unwrap();
+        fs::create_dir_all(&dest).unwrap();
+        fs::write(src.join("a"), b"a").unwrap();
+        fs::write(src.join("b"), b"b").unwrap();
+        fs::write(dest.join("a"), b"pre-existing").unwrap();
+
+        let skipped = copy_dir_recursive(&src, &dest, None, false, &Progress::disabled()).unwrap();
+
+        assert_eq!(skipped, 1);
+        assert_eq!(fs::read(dest.join("a")).unwrap(), b"pre-existing");
+        assert_eq!(fs::read(dest.join("b")).unwrap(), b"b");
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_copy_dir_recursive_counts_non_regular_file() {
+        // A socket has no copy, so the destination never receives it. Counting
+        // it keeps `copy_and_remove` from deleting a source that still holds
+        // something the destination does not.
+        let dir = tempfile::tempdir().unwrap();
+        let src = dir.path().join("src");
+        let dest = dir.path().join("dest");
+        fs::create_dir_all(&src).unwrap();
+        fs::write(src.join("regular"), b"content").unwrap();
+        let _listener = std::os::unix::net::UnixListener::bind(src.join("sock")).unwrap();
+
+        let skipped = copy_dir_recursive(&src, &dest, None, true, &Progress::disabled()).unwrap();
+
+        assert_eq!(skipped, 1);
+        assert_eq!(fs::read(dest.join("regular")).unwrap(), b"content");
+        assert!(!dest.join("sock").exists());
+    }
+
+    #[test]
+    fn test_copy_dir_recursive_reports_no_skips_for_a_complete_copy() {
+        // The control: a clean copy reports zero, which is what lets a move
+        // treat any non-zero count as a reason to keep the source.
+        let dir = tempfile::tempdir().unwrap();
+        let src = dir.path().join("src");
+        let dest = dir.path().join("dest");
+        fs::create_dir_all(src.join("nested")).unwrap();
+        fs::write(src.join("a"), b"a").unwrap();
+        fs::write(src.join("nested").join("b"), b"b").unwrap();
+
+        let skipped = copy_dir_recursive(&src, &dest, None, true, &Progress::disabled()).unwrap();
+
+        assert_eq!(skipped, 0);
+        assert_eq!(fs::read(dest.join("nested").join("b")).unwrap(), b"b");
+    }
+
     #[test]
     fn test_copy_dir_recursive_missing_source_root_errors() {
         // A subdirectory that vanishes mid-walk is skipped, but the root the