From 58b4834f1a808b8597e789d0d991a33958c2780f Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Sat, 22 Aug 2026 11:58:07 -0700 Subject: [PATCH 1/2] Simplify picker tab and temporary index handling --- src/commands/list/collect/tasks.rs | 14 +-- src/commands/picker/items.rs | 182 ++++++++--------------------- src/commands/step/commit.rs | 54 ++------- src/git/repository/diff.rs | 2 +- src/git/repository/working_tree.rs | 164 ++++++++++++++++++++------ tests/integration_tests/list.rs | 46 ++++++++ tests/integration_tests/merge.rs | 13 ++- 7 files changed, 246 insertions(+), 229 deletions(-) diff --git a/src/commands/list/collect/tasks.rs b/src/commands/list/collect/tasks.rs index 0e192a9e81..e892093ab5 100644 --- a/src/commands/list/collect/tasks.rs +++ b/src/commands/list/collect/tasks.rs @@ -8,7 +8,6 @@ use std::time::Duration; use std::sync::Arc; -use anyhow::Context; use worktrunk::git::{ ErrorExt, IntegrationTargets, LineDiff, RefSnapshot, Repository, select_comparison_base, }; @@ -739,17 +738,8 @@ impl Task for WorkingTreeConflictsTask { // Stage all dirty + untracked entries into a temp index so // `write-tree` produces a tree representing the full working // state. Real index untouched. See `WorkingTree::temp_index`. - let idx = wt.temp_index().map_err(|e| ctx.error(Self::KIND, &e))?; - idx.git(["add", "-A"]) - .run() - .context("Failed to stage working tree changes") - .map_err(|e| ctx.error(Self::KIND, &e))?; - let output = idx - .git(["write-tree"]) - .run() - .context("Failed to write tree from temporary index") - .map_err(|e| ctx.error(Self::KIND, &e))?; - String::from_utf8_lossy(&output.stdout).trim().to_string() + wt.write_worktree_tree() + .map_err(|e| ctx.error(Self::KIND, &e))? } else { wt.run_command(&["write-tree"]) .map(|s| s.trim().to_string()) diff --git a/src/commands/picker/items.rs b/src/commands/picker/items.rs index 04d818d844..40947b3d82 100644 --- a/src/commands/picker/items.rs +++ b/src/commands/picker/items.rs @@ -742,46 +742,6 @@ impl LocalContent { } } -/// The local-checkout-backed preview tabs — present for a row with a local -/// worktree, absent for a listed PR/MR row (`--prs`), which has no local copy. -/// Grouping them names *why* they're empty on a `--prs` row (no checkout), so -/// the difference reads as a data fact rather than a row-type policy. -#[derive(Debug, Clone, Copy, Default)] -struct LocalTabs { - unified_diff: bool, - working_tree: bool, - branch_diff: bool, - upstream: bool, - summary: bool, -} - -impl LocalTabs { - /// A local row. The subsidiary diff tabs (`working_tree`, `branch_diff`, - /// `upstream`) follow the live [`LocalContent`] signals — available while - /// loading, dim once their diff is known empty; `unified_diff` dims only - /// when both component signals are empty because their net changes can - /// cancel. `upstream` also requires the - /// synchronous `has_upstream` floor (no tracking ref → dim with no loading - /// window). `summary` requires the process-wide `[commit.generation]` flag - /// *and* something to summarize: its pane is generated from the combined diff - /// (`crate::summary::compute_combined_diff` = commits ahead of the comparison - /// base + tracked working-tree changes), so it uses `branch_diff` and the - /// tracked-only `summary_working_tree` signal — dimming in concert with its - /// actual input rather than the untracked-inclusive working pane. - fn worktree(content: LocalContent, has_upstream: bool, summaries_enabled: bool) -> Self { - let working_tree = content.working_tree.unwrap_or(true); - let summary_working_tree = content.summary_working_tree.unwrap_or(true); - let branch_diff = content.branch_diff.unwrap_or(true); - Self { - unified_diff: working_tree || branch_diff, - working_tree, - branch_diff, - upstream: has_upstream && content.upstream_diverged.unwrap_or(true), - summary: summaries_enabled && (summary_working_tree || branch_diff), - } - } -} - /// Which preview tabs have renderable content for the selected row. /// /// Empty tabs are de-emphasized in the bar (number dimmed). Skim computes a @@ -789,9 +749,9 @@ impl LocalTabs { /// `loading_placeholder`). Two genuine axes drive availability, and `--prs` /// touches neither — it only decides whether a PR row is *listed* at all: /// -/// - The local-checkout tabs ([`LocalTabs`]): the three subsidiary diff tabs -/// (`working_tree` / `branch_diff` / `upstream`) follow the row's live -/// [`LocalContent`] exactly; `unified_diff` uses their conservative union, +/// - The local-checkout tabs: the three subsidiary diff tabs (`working_tree` / +/// `branch_diff` / `upstream`) follow the row's live [`LocalContent`] +/// exactly; `unified_diff` uses the first two's conservative union, /// since opposing working and committed changes can cancel only after its /// net diff is computed. Tabs stay available while loading and dim once /// known empty, the same shape as the `pr` tab. `upstream` also @@ -800,8 +760,8 @@ impl LocalTabs { /// `summary` requires the process-wide `[commit.generation]` flag *and* a /// non-empty combined diff — its pane's content source — so it uses the /// `branch_diff` plus tracked-working signals and dims alongside the "no -/// changes to summarize" pane, not only when summaries are disabled. A `--prs` row has -/// no local checkout, so all five local tabs are empty. +/// changes to summarize" pane, not only when summaries are disabled. A +/// `--prs` row has no local checkout, so all five local tabs are empty. /// - The PR-backed tabs: `pr` and `comments` are available together, gated by /// `has_pr`. On a worktree row that's the live status slot (primed from the CI /// cache, then refreshed by the `CiStatus` task — see @@ -816,49 +776,33 @@ impl LocalTabs { /// `--prs` row). #[derive(Debug, Clone, Copy)] pub(super) struct TabAvailability { - unified_diff: bool, working_tree: bool, - log: bool, branch_diff: bool, upstream: bool, summary: bool, pr: bool, - comments: bool, } impl TabAvailability { - /// Build from the two genuine axes: which local-checkout tabs the row has, - /// and whether it has a PR. Centralizing the mapping pins the invariants the - /// task depends on — `log` is always present, and the PR-backed tabs (`pr`, - /// `comments`) both follow `has_pr` — so they can't drift between a worktree - /// row and a listed-PR row. - fn from_axes(local: LocalTabs, has_pr: bool) -> Self { - Self { - unified_diff: local.unified_diff, - working_tree: local.working_tree, - log: true, - branch_diff: local.branch_diff, - upstream: local.upstream, - summary: local.summary, - pr: has_pr, - comments: has_pr, - } - } - - /// A worktree-backed row: the local-checkout tabs follow [`LocalTabs`] - /// (diff tabs gated by the live [`LocalContent`] + the `has_upstream` floor); - /// the PR-backed tabs follow the live PR status (see - /// [`pr_tab_available`]). + /// A worktree-backed row: diff tabs follow the live [`LocalContent`] plus + /// the `has_upstream` floor; PR-backed tabs follow the live PR status (see + /// [`pr_tab_available`]). Derived tabs are handled by [`Self::contains`]. pub(super) fn worktree( content: LocalContent, has_upstream: bool, summaries_enabled: bool, has_pr: bool, ) -> Self { - Self::from_axes( - LocalTabs::worktree(content, has_upstream, summaries_enabled), - has_pr, - ) + let working_tree = content.working_tree.unwrap_or(true); + let summary_working_tree = content.summary_working_tree.unwrap_or(true); + let branch_diff = content.branch_diff.unwrap_or(true); + Self { + working_tree, + branch_diff, + upstream: has_upstream && content.upstream_diverged.unwrap_or(true), + summary: summaries_enabled && (summary_working_tree || branch_diff), + pr: has_pr, + } } /// A listed PR/MR row (`--prs`): no local checkout, so the local-checkout @@ -867,21 +811,26 @@ impl TabAvailability { /// `prs::compute_pr_log`). The differences from a worktree row are only the /// genuine data ones — no local checkout, and a PR that's always present. pub(super) fn listed_pr() -> Self { - Self::from_axes(LocalTabs::default(), true) + Self { + working_tree: false, + branch_diff: false, + upstream: false, + summary: false, + pr: true, + } } /// Whether `mode` has content for this row. Empty tabs remain directly /// selectable with Alt-N, but sequential cycling skips them. fn contains(self, mode: PreviewMode) -> bool { match mode { - PreviewMode::UnifiedDiff => self.unified_diff, + PreviewMode::UnifiedDiff => self.working_tree || self.branch_diff, PreviewMode::WorkingTree => self.working_tree, PreviewMode::BranchDiff => self.branch_diff, - PreviewMode::Log => self.log, + PreviewMode::Log => true, PreviewMode::UpstreamDiff => self.upstream, PreviewMode::Summary => self.summary, - PreviewMode::Pr => self.pr, - PreviewMode::Comments => self.comments, + PreviewMode::Pr | PreviewMode::Comments => self.pr, } } @@ -943,55 +892,21 @@ pub(super) fn render_preview_tabs( let reset = Reset; let tabs = [ - Tab { - number: 1, - label: "diff", - is_active: mode == PreviewMode::UnifiedDiff, - has_content: avail.unified_diff, - }, - Tab { - number: 2, - label: "working", - is_active: mode == PreviewMode::WorkingTree, - has_content: avail.working_tree, - }, - Tab { - number: 3, - label: "committed", - is_active: mode == PreviewMode::BranchDiff, - has_content: avail.branch_diff, - }, - Tab { - number: 4, - label: "log", - is_active: mode == PreviewMode::Log, - has_content: avail.log, - }, - Tab { - number: 5, - label: "remote⇅", - is_active: mode == PreviewMode::UpstreamDiff, - has_content: avail.upstream, - }, - Tab { - number: 6, - label: "summary", - is_active: mode == PreviewMode::Summary, - has_content: avail.summary, - }, - Tab { - number: 7, - label: "pr", - is_active: mode == PreviewMode::Pr, - has_content: avail.pr, - }, - Tab { - number: 8, - label: "comments", - is_active: mode == PreviewMode::Comments, - has_content: avail.comments, - }, - ]; + (1, "diff", PreviewMode::UnifiedDiff), + (2, "working", PreviewMode::WorkingTree), + (3, "committed", PreviewMode::BranchDiff), + (4, "log", PreviewMode::Log), + (5, "remote⇅", PreviewMode::UpstreamDiff), + (6, "summary", PreviewMode::Summary), + (7, "pr", PreviewMode::Pr), + (8, "comments", PreviewMode::Comments), + ] + .map(|(number, label, tab_mode)| Tab { + number, + label, + is_active: mode == tab_mode, + has_content: avail.contains(tab_mode), + }); // Prefer the full bar; fall back to compact only when it would overflow the // pane (`visual_width` measures the styled string with ANSI stripped). @@ -2203,10 +2118,10 @@ mod tests { fn diff_tabs_dim_only_once_known_empty() { let has = |avail: TabAvailability| { ( - avail.unified_diff, - avail.working_tree, - avail.branch_diff, - avail.upstream, + avail.contains(PreviewMode::UnifiedDiff), + avail.contains(PreviewMode::WorkingTree), + avail.contains(PreviewMode::BranchDiff), + avail.contains(PreviewMode::UpstreamDiff), ) }; @@ -2252,7 +2167,8 @@ mod tests { #[test] fn summary_tab_tracks_combined_diff_content() { let summary = |content, summaries_enabled| { - TabAvailability::worktree(content, true, summaries_enabled, false).summary + TabAvailability::worktree(content, true, summaries_enabled, false) + .contains(PreviewMode::Summary) }; // Summaries disabled dims the tab regardless of content. diff --git a/src/commands/step/commit.rs b/src/commands/step/commit.rs index 4389015c89..02af8223d7 100644 --- a/src/commands/step/commit.rs +++ b/src/commands/step/commit.rs @@ -1,11 +1,8 @@ //! `wt step commit` — commit working tree changes. -use std::path::Path; - use anyhow::Context; use worktrunk::HookType; use worktrunk::config::UserConfig; -use worktrunk::git::{Repository, TempIndex}; use worktrunk::styling::println; use super::super::command_approval::{approve_or_skip, resolve_template_for_preview}; @@ -82,16 +79,20 @@ fn preview_commit(stage: Option, dry_run: bool, yes: bool) -> anyhow: // For --dry-run, stage to a copy of the index so the preview reflects what a real // run would send. --show-prompt skips this — it's the cheap "what's already staged" // path. StageMode::None has nothing to stage, so we use the existing index as-is. + let stage_mode = stage.unwrap_or(env.resolved().commit.stage()); let temp_index = if dry_run { - stage - .unwrap_or(env.resolved().commit.stage()) + stage_mode .add_args() - .map(|args| stage_to_temp_index(&env.repo, args)) + .map(|_| { + let temp = env.repo.current_worktree().temp_index()?; + temp.stage(stage_mode)?; + Ok::<_, anyhow::Error>(temp) + }) .transpose()? } else { None }; - let index_override = temp_index.as_ref().map(|temp| Path::new(temp.path())); + let index_override = temp_index.as_ref().map(|temp| temp.path()); let ctx = env.context(yes); let project_append = resolve_template_for_preview(&ctx, &commit_config, dry_run)?; @@ -109,42 +110,3 @@ fn preview_commit(stage: Option, dry_run: bool, yes: bool) -> anyhow: )?; print_dry_run(&prompt, &commit_config, &message) } - -/// Stage into the worktree's shared temporary-index abstraction. -/// -/// Returning [`TempIndex`] keeps the copied index alive while the caller builds -/// the prompt, then removes it on drop without touching the user's real index. -fn stage_to_temp_index(repo: &Repository, add_args: &[&str]) -> anyhow::Result { - let temp = repo.current_worktree().temp_index()?; - let output = temp - .git(add_args.iter().copied()) - .run() - .context("Failed to stage changes into temp index")?; - if !output.status.success() { - return Err( - worktrunk::git::CommandError::from_failed_output("git", add_args, &output).into(), - ); - } - Ok(temp) -} - -#[cfg(test)] -mod tests { - use worktrunk::git::{CommandError, Repository}; - use worktrunk::testing::TestRepo; - - /// A failing `git add` into the temp index (here: an invalid pathspec) - /// must surface as a typed `CommandError`. - #[test] - fn stage_to_temp_index_failure_is_command_error() { - let test = TestRepo::with_initial_commit(); - let repo = Repository::at(test.root_path()).unwrap(); - - let err = match super::stage_to_temp_index(&repo, &["add", "--", ":(bad"]) { - Ok(_) => panic!("invalid pathspec should fail"), - Err(err) => err, - }; - let cmd_err = CommandError::find_in(&err).expect("error should carry a CommandError"); - assert_eq!(cmd_err.command_string(), "git add -- :(bad"); - } -} diff --git a/src/git/repository/diff.rs b/src/git/repository/diff.rs index 7d14316abb..7c7e0f6ee6 100644 --- a/src/git/repository/diff.rs +++ b/src/git/repository/diff.rs @@ -61,7 +61,7 @@ impl<'repo> PreparedDiff<'repo> { .current_dir(path) .context(path_to_logging_context(path)), ), - DiffSource::TempIndex(index) => index.git(args.iter().cloned()), + DiffSource::TempIndex(index) => index.command(args.iter().cloned()), } } diff --git a/src/git/repository/working_tree.rs b/src/git/repository/working_tree.rs index beeda5d107..4fef6dd9fc 100644 --- a/src/git/repository/working_tree.rs +++ b/src/git/repository/working_tree.rs @@ -791,14 +791,7 @@ impl<'a> WorkingTree<'a> { "--pathspec-from-file=-", "--pathspec-file-nul", ]; - let add_output = idx - .git(add_args) - .stdin_bytes(output.stdout) - .run() - .context("Failed to stage untracked files")?; - if !add_output.status.success() { - return Err(CommandError::from_failed_output("git", &add_args, &add_output).into()); - } + idx.run_command_with_input(add_args, output.stdout)?; let mut args = vec![ "diff".to_string(), @@ -808,16 +801,10 @@ impl<'a> WorkingTree<'a> { "--".to_string(), ]; args.extend(paths); - let output = idx - .git(&args) - .run() - .context("Failed to compute untracked diff stats")?; - if !output.status.success() { - return Err(CommandError::from_failed_output("git", &args, &output).into()); - } + let output = idx.run_command(&args)?; let mut stats = LineDiff::default(); - for line in String::from_utf8_lossy(&output.stdout).lines() { + for line in output.lines() { if let Some((added, deleted)) = parse_numstat_line(line) { stats.added += added; stats.deleted += deleted; @@ -859,9 +846,6 @@ impl<'a> WorkingTree<'a> { // Windows: deleting a still-open file leaves the name pending. std::fs::remove_file(&temp).context("Failed to clear temporary index")?; } - // Validate UTF-8 once so `TempIndex::path` is infallible. - temp.to_str() - .context("Temporary index path is not valid UTF-8")?; Ok(TempIndex { temp, @@ -873,6 +857,18 @@ impl<'a> WorkingTree<'a> { }) } + /// Write a tree containing the index plus every working-tree change. + /// + /// A temporary index keeps the user's staging state unchanged. `--sparse` + /// is intentional here: this is a complete worktree snapshot for conflict + /// detection, so untracked files outside a sparse-checkout definition must + /// participate too. + pub fn write_worktree_tree(&self) -> anyhow::Result { + let index = self.temp_index()?; + index.stage_worktree_snapshot()?; + index.write_tree() + } + /// Determine whether there are staged changes in the index. /// /// Returns `Ok(true)` when staged changes are present, `Ok(false)` otherwise. @@ -986,9 +982,49 @@ pub struct TempIndex { } impl TempIndex { - /// UTF-8 path to the temp index file. Validated at construction. - pub fn path(&self) -> &str { - self.temp.to_str().expect("validated in temp_index()") + /// Path to the temporary index file. + pub fn path(&self) -> &Path { + &self.temp + } + + /// Stage the paths selected by `mode` into this temporary index. + /// + /// Uses the same `git add` mode as [`WorkingTree::stage`]. All-files mode + /// only adds a pathspec that excludes Worktrunk's own temporary indexes if + /// the system temp directory sits inside the worktree. + /// + /// [`StageMode`]: crate::config::StageMode + pub fn stage(&self, mode: crate::config::StageMode) -> anyhow::Result<()> { + let Some(add_args) = mode.add_args() else { + return Ok(()); + }; + let mut args: Vec = add_args.iter().map(|arg| (*arg).to_string()).collect(); + if mode == crate::config::StageMode::All { + args.extend(["--".to_string(), ".".to_string()]); + self.append_temp_index_exclusion(&mut args)?; + } + self.run_command(args)?; + Ok(()) + } + + /// Stage a complete working-tree snapshot, crossing sparse boundaries. + fn stage_worktree_snapshot(&self) -> anyhow::Result<()> { + let mut args = vec![ + "add".to_string(), + "-A".to_string(), + "--sparse".to_string(), + "--".to_string(), + ".".to_string(), + ]; + self.append_temp_index_exclusion(&mut args)?; + self.run_command(args)?; + Ok(()) + } + + /// Write the temporary index as a tree and return its object id. + fn write_tree(&self) -> anyhow::Result { + self.run_command(["write-tree"]) + .map(|output| output.trim().to_string()) } /// Register untracked files in the temporary index without adding their @@ -1010,6 +1046,12 @@ impl TempIndex { "--".to_string(), ".".to_string(), ]; + self.append_temp_index_exclusion(&mut args)?; + self.run_command(args)?; + Ok(()) + } + + fn append_temp_index_exclusion(&self, args: &mut Vec) -> anyhow::Result<()> { let temp_dir = canonicalize_with_parents( self.temp .parent() @@ -1023,14 +1065,33 @@ impl TempIndex { ":(top,exclude,glob){relative}{separator}{TEMP_INDEX_PREFIX}*" )); } - let output = self - .git(&args) + Ok(()) + } + + fn run_command( + &self, + args: impl IntoIterator>, + ) -> anyhow::Result { + self.run_command_with_input(args, Vec::new()) + } + + fn run_command_with_input( + &self, + args: impl IntoIterator>, + stdin: Vec, + ) -> anyhow::Result { + let args: Vec = args.into_iter().map(Into::into).collect(); + let mut command = self.command(args.iter().cloned()); + if !stdin.is_empty() { + command = command.stdin_bytes(stdin); + } + let output = command .run() - .context("Failed to register untracked files")?; + .with_context(|| format!("Failed to execute: git {}", args.join(" ")))?; if !output.status.success() { return Err(CommandError::from_failed_output("git", &args, &output).into()); } - Ok(()) + Ok(String::from_utf8_lossy(&output.stdout).into_owned()) } /// Build a `git` command pointed at this temp index. @@ -1038,7 +1099,7 @@ impl TempIndex { /// Wires `current_dir` to the worktree root, the worktree's logging /// context, and `GIT_INDEX_FILE`. The caller adds the subcommand and /// chooses `.run()` / `.stream()`. - pub fn git(&self, args: I) -> Cmd + pub(super) fn command(&self, args: I) -> Cmd where I: IntoIterator, S: Into, @@ -1301,8 +1362,7 @@ mod tests { ); } - #[test] - fn untracked_diff_stats_crosses_sparse_checkout_boundary() { + fn sparse_checkout_with_untracked_file() -> TestRepo { let test = TestRepo::with_initial_commit(); std::fs::create_dir_all(test.root_path().join("visible")).unwrap(); std::fs::create_dir_all(test.root_path().join("hidden")).unwrap(); @@ -1316,12 +1376,51 @@ mod tests { std::fs::create_dir_all(test.root_path().join("hidden")).unwrap(); std::fs::write(test.root_path().join("hidden/loose.txt"), "loose\n").unwrap(); + test + } + + #[test] + fn untracked_diff_stats_crosses_sparse_checkout_boundary() { + let test = sparse_checkout_with_untracked_file(); + let repo = Repository::at(test.root_path()).unwrap(); let stats = repo.current_worktree().untracked_diff_stats().unwrap(); assert_eq!(stats.added, 1); assert_eq!(stats.deleted, 0); } + #[test] + fn temp_index_stage_all_preserves_sparse_checkout_boundary() { + let test = sparse_checkout_with_untracked_file(); + + let repo = Repository::at(test.root_path()).unwrap(); + let idx = repo.current_worktree().temp_index().unwrap(); + let err = idx + .stage(crate::config::StageMode::All) + .expect_err("temporary staging must mirror real git add -A"); + let cmd_err = crate::git::CommandError::find_in(&err) + .expect("sparse-boundary refusal should remain a CommandError"); + assert_eq!(cmd_err.command_string(), "git add -A -- ."); + } + + #[test] + fn write_worktree_tree_crosses_sparse_checkout_boundary() { + let test = sparse_checkout_with_untracked_file(); + + let repo = Repository::at(test.root_path()).unwrap(); + let tree = repo.current_worktree().write_worktree_tree().unwrap(); + let files = test.git_output(&["ls-tree", "-r", "--name-only", &tree]); + assert_eq!( + files.lines().collect::>(), + [ + "file.txt", + "hidden/loose.txt", + "hidden/tracked.txt", + "visible/tracked.txt", + ] + ); + } + #[test] fn untracked_diff_stats_unborn_head_is_command_error() { // With an unborn HEAD the untracked files stage fine into the temp @@ -1365,11 +1464,8 @@ mod tests { // (b) git add -A against the resulting temp index produces a tree // containing the working-tree files. - idx.git(["add", "-A"]).run().unwrap(); - let write_tree = idx.git(["write-tree"]).run().unwrap(); - let tree_sha = String::from_utf8_lossy(&write_tree.stdout) - .trim() - .to_string(); + idx.stage(crate::config::StageMode::All).unwrap(); + let tree_sha = idx.write_tree().unwrap(); let ls_tree = Cmd::new("git") .args(["ls-tree", "-r", "--name-only", &tree_sha]) .current_dir(test.root_path()) diff --git a/tests/integration_tests/list.rs b/tests/integration_tests/list.rs index b12bfbf391..57565d359d 100644 --- a/tests/integration_tests/list.rs +++ b/tests/integration_tests/list.rs @@ -3369,6 +3369,52 @@ fn test_list_working_tree_conflicts(mut repo: TestRepo) { }); } +/// A dirty worktree may contain untracked files outside its sparse-checkout +/// definition. The temporary-index snapshot must still include the visible +/// tracked change and produce the working-tree conflict result. +#[rstest] +fn test_list_working_tree_conflicts_cross_sparse_checkout_boundary(mut repo: TestRepo) { + repo.write_test_config("[list]\njson-schema = 2\n"); + + std::fs::create_dir_all(repo.root_path().join("visible")).unwrap(); + std::fs::create_dir_all(repo.root_path().join("hidden")).unwrap(); + std::fs::write(repo.root_path().join("visible/shared.txt"), "base\n").unwrap(); + std::fs::write(repo.root_path().join("hidden/tracked.txt"), "base\n").unwrap(); + repo.run_git(&["add", "."]); + repo.commit("Add sparse fixture"); + + let feature = repo.add_worktree("feature"); + std::fs::write(repo.root_path().join("visible/shared.txt"), "main\n").unwrap(); + repo.commit("Change shared file on main"); + + repo.run_git_in(&feature, &["sparse-checkout", "init", "--cone"]); + repo.run_git_in(&feature, &["sparse-checkout", "set", "visible"]); + std::fs::write(feature.join("visible/shared.txt"), "feature\n").unwrap(); + std::fs::create_dir_all(feature.join("hidden")).unwrap(); + std::fs::write(feature.join("hidden/loose.txt"), "loose\n").unwrap(); + + let output = repo + .wt_command() + .args(["list", "--format=json"]) + .current_dir(repo.root_path()) + .output() + .unwrap(); + assert!( + output.status.success(), + "wt list should succeed; stderr:\n{}", + String::from_utf8_lossy(&output.stderr) + ); + + let json: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let feature = json["items"] + .as_array() + .unwrap() + .iter() + .find(|item| item["branch"] == "feature") + .expect("feature row"); + assert_eq!(feature["default_branch"]["merge_conflicts"], true); +} + /// /// Even with --full, if the working tree is clean, we skip the working-tree check /// and just use the commit-level conflict detection. diff --git a/tests/integration_tests/merge.rs b/tests/integration_tests/merge.rs index 56789168b0..21ebb4bce7 100644 --- a/tests/integration_tests/merge.rs +++ b/tests/integration_tests/merge.rs @@ -2865,6 +2865,9 @@ command = "cat >/dev/null && echo 'feat: add untracked'" fs::write(repo.test_config_path(), worktrunk_config).unwrap(); let output = make_snapshot_cmd(&repo, "step", &["commit", "--dry-run"], None) + // The worktree-root case exercises the empty relative path in the + // temporary-index exclusion. + .env("TMPDIR", repo.root_path()) .output() .expect("wt step commit --dry-run failed"); let stdout = String::from_utf8(output.stdout).unwrap(); @@ -2873,6 +2876,10 @@ command = "cat >/dev/null && echo 'feat: add untracked'" "--dry-run with default --stage=all should include untracked files in the prompt; \ got stdout:\n{stdout}" ); + assert!( + !stdout.contains("worktrunk-temp-index-"), + "--dry-run must exclude its temporary index artifacts; got stdout:\n{stdout}" + ); // The user's real index must not have absorbed the untracked file. let status = repo @@ -3026,7 +3033,7 @@ command = "cat >/dev/null && echo 'feat: combined feature work'" /// `GIT_INDEX_FILE` pointed at the copy. If that `git add` exits non-zero, the /// error must propagate (not silently feed an empty diff to the LLM). We /// reproduce a non-zero exit by replacing `.git/index` with garbage — the temp -/// copy succeeds, but `git add -A` rejects the corrupt index. +/// copy succeeds, but the all-files `git add` rejects the corrupt index. #[rstest] fn test_step_commit_dry_run_propagates_git_add_failure(repo: TestRepo) { fs::write(repo.root_path().join("new.txt"), "x").expect("Failed to write file"); @@ -3045,8 +3052,8 @@ fn test_step_commit_dry_run_propagates_git_add_failure(repo: TestRepo) { ); let stderr = String::from_utf8_lossy(&output.stderr); assert!( - stderr.contains("git add -A failed"), - "expected bail! to surface 'git add -A failed'; got stderr:\n{stderr}" + stderr.contains("git add -A -- . failed"), + "expected the failing git add command; got stderr:\n{stderr}" ); } From 07c2088abcb6edecf2474481d9d7085a9e3815ae Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Sat, 22 Aug 2026 12:16:15 -0700 Subject: [PATCH 2/2] Clarify temporary staging semantics --- src/commands/step/commit.rs | 5 ++--- src/git/repository/working_tree.rs | 7 ++++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/commands/step/commit.rs b/src/commands/step/commit.rs index 02af8223d7..d7987e2c5d 100644 --- a/src/commands/step/commit.rs +++ b/src/commands/step/commit.rs @@ -81,9 +81,8 @@ fn preview_commit(stage: Option, dry_run: bool, yes: bool) -> anyhow: // path. StageMode::None has nothing to stage, so we use the existing index as-is. let stage_mode = stage.unwrap_or(env.resolved().commit.stage()); let temp_index = if dry_run { - stage_mode - .add_args() - .map(|_| { + (stage_mode != StageMode::None) + .then(|| { let temp = env.repo.current_worktree().temp_index()?; temp.stage(stage_mode)?; Ok::<_, anyhow::Error>(temp) diff --git a/src/git/repository/working_tree.rs b/src/git/repository/working_tree.rs index 4fef6dd9fc..8d5ac370b6 100644 --- a/src/git/repository/working_tree.rs +++ b/src/git/repository/working_tree.rs @@ -989,9 +989,10 @@ impl TempIndex { /// Stage the paths selected by `mode` into this temporary index. /// - /// Uses the same `git add` mode as [`WorkingTree::stage`]. All-files mode - /// only adds a pathspec that excludes Worktrunk's own temporary indexes if - /// the system temp directory sits inside the worktree. + /// Uses the same `git add` mode as [`WorkingTree::stage`], minus its + /// unmerged-paths gate. All-files mode also scopes the add to the worktree + /// root (`-- .`) and, when the system temp directory sits inside the + /// worktree, excludes Worktrunk's own temporary indexes. /// /// [`StageMode`]: crate::config::StageMode pub fn stage(&self, mode: crate::config::StageMode) -> anyhow::Result<()> {