Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 103 additions & 38 deletions src/commands/list/collect/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ pub(crate) use execution::ExpectedResults;
use execution::{work_items_for_branch, work_items_for_worktree};
use results::drain_results;
use types::DrainOutcome;
use types::{TaskError, TaskResult};
use types::{MissingResult, TaskError, TaskResult};

struct TableRenderPlan {
progressive_table: Option<ProgressiveTable>,
Expand Down Expand Up @@ -366,6 +366,12 @@ pub trait PickerProgressHandler: Send + Sync {
/// skeleton state use the skeleton renderer. The handler writes every
/// slot — slot writes are idempotent.
fn on_reveal(&self, rendered: Vec<String>);

/// Stash a pre-formatted warning line. Skim owns the terminal while
/// collect runs on the picker's bg thread, so eprintln from collect
/// would corrupt the rendered frame. The picker drains stashed lines
/// after `Skim::run_with` returns, when stderr is safe again.
fn stash_warning(&self, line: String);
}

/// Controls how show flags (branches/remotes/full) are determined in [`collect`].
Expand Down Expand Up @@ -452,6 +458,38 @@ fn format_stall_footer(
)
}

/// Build the drain-timeout diagnostic shown when the default 120s
/// `DRAIN_TIMEOUT` is hit. Returns the pre-formatted warning text — the
/// caller wraps it in `warning_message` and routes through the picker stash
/// or stderr. Pure so tests can exercise it without spinning up a 120s drain.
fn format_drain_timeout_diag(
received_count: usize,
items_with_missing: &[MissingResult],
) -> String {
let mut diag = format!(
"Listing worktrees timed out after {}s ({received_count} results received)",
results::DRAIN_TIMEOUT.as_secs()
);

if !items_with_missing.is_empty() {
diag.push_str("; blocked tasks:");
let missing_lines: Vec<String> = items_with_missing
.iter()
.take(5)
.map(|result| {
let missing_names: Vec<&str> =
result.missing_kinds.iter().map(|k| k.into()).collect();
cformat!("<bold>{}</>: {}", result.name, missing_names.join(", "))
})
.collect();
diag.push_str(&format!(
"\n{}",
format_with_gutter(&missing_lines.join("\n"), None)
));
}
diag
}

/// Collect worktree data with rendering driven by `render_target`.
///
/// - [`RenderTarget::Table { progressive: true }`]: renders a skeleton
Expand Down Expand Up @@ -625,6 +663,19 @@ pub fn collect(
}
};

// The picker (`wt switch`) drives a skim TUI that owns the terminal while
// collect runs on a background thread. Any stderr write from collect
// would overlay the picker's rendered frame and corrupt skim's clear
// math, so warnings go through the handler's stash instead — picker
// drains and emits them after `Skim::run_with` returns.
let emit_warning = |line: String| {
if let Some(h) = progressive_handler.as_ref() {
h.stash_warning(line);
} else {
eprintln!("{line}");
}
};

// Opportunistic stale-default-branch check: `default_branch` above is
// the persisted value, now trusted without validation on the hot path.
// Cross-check against the enumerated branch set and surface a warning
Expand Down Expand Up @@ -662,17 +713,17 @@ pub fn collect(
};

if warn_stale_default && let Some(branch) = default_branch.as_deref() {
eprintln!(
"{}",
emit_warning(
warning_message(cformat!(
"Configured default branch <bold>{branch}</> does not exist locally"
))
.to_string(),
);
eprintln!(
"{}",
emit_warning(
hint_message(cformat!(
"To reset, run <underline>wt config state default-branch clear</>"
))
.to_string(),
);
}

Expand Down Expand Up @@ -751,9 +802,8 @@ pub fn collect(
// Surface git's actual stderr (when available via the typed leaf)
// rather than our `CommandError` summary.
let detail = err.display_message();
eprintln!(
"{}",
warning_message(cformat!("Failed to batch-fetch commit details: {detail}"))
emit_warning(
warning_message(cformat!("Failed to batch-fetch commit details: {detail}")).to_string(),
);
std::collections::HashMap::new()
});
Expand Down Expand Up @@ -1375,36 +1425,13 @@ pub fn collect(
items_with_missing,
} = drain_outcome
{
// Warning: what happened + gutter showing which tasks blocked
let mut diag = format!(
"wt list timed out after {}s ({received_count} results received)",
results::DRAIN_TIMEOUT.as_secs()
);

if !items_with_missing.is_empty() {
diag.push_str("; blocked tasks:");
let missing_lines: Vec<String> = items_with_missing
.iter()
.take(5)
.map(|result| {
let missing_names: Vec<&str> =
result.missing_kinds.iter().map(|k| k.into()).collect();
cformat!("<bold>{}</>: {}", result.name, missing_names.join(", "))
})
.collect();
diag.push_str(&format!(
"\n{}",
format_with_gutter(&missing_lines.join("\n"), None)
));
}

eprintln!("{}", warning_message(&diag));

eprintln!(
"{}",
let diag = format_drain_timeout_diag(received_count, &items_with_missing);
emit_warning(warning_message(&diag).to_string());
emit_warning(
hint_message(cformat!(
"A git command likely hung; run <underline>wt list -v</> for details or <underline>wt list -vv</> to create a diagnostic file"
"A git command likely hung; for details, re-run with <underline>-v</>; for a diagnostic file, re-run with <underline>-vv</>"
))
.to_string(),
);
}

Expand Down Expand Up @@ -1478,10 +1505,10 @@ pub fn collect(
}

let warning = warning_parts.join("\n");
eprintln!("{}", warning_message(&warning));
emit_warning(warning_message(&warning).to_string());

// Show issue reporting hint (free function - doesn't collect diagnostic data)
eprintln!("{}", hint_message(crate::diagnostic::issue_hint()));
emit_warning(hint_message(crate::diagnostic::issue_hint()).to_string());
}

// Populate display fields for all items (used by JSON output and statusline)
Expand Down Expand Up @@ -1819,6 +1846,44 @@ mod tests {
);
}

/// Drain timeout diagnostic without per-item breakdown — the early-exit
/// path when no items are blocked.
#[test]
fn test_format_drain_timeout_diag_no_items() {
let rendered = format_drain_timeout_diag(7, &[]);
insta::assert_snapshot!(
strip_ansi(&rendered),
@"Listing worktrees timed out after 120s (7 results received)"
);
}

/// With blocked items, the diagnostic appends a gutter listing each
/// item's missing task kinds. `take(5)` caps the list; cover that here.
#[test]
fn test_format_drain_timeout_diag_with_blocked_items() {
let items = vec![
MissingResult {
item_idx: 0,
name: "feature-a".to_string(),
missing_kinds: vec![TaskKind::CiStatus, TaskKind::BranchDiff],
},
MissingResult {
item_idx: 1,
name: "feature-b".to_string(),
missing_kinds: vec![TaskKind::AheadBehind],
},
];
let rendered = format_drain_timeout_diag(3, &items);
insta::assert_snapshot!(
strip_ansi(&rendered),
@r"
Listing worktrees timed out after 120s (3 results received); blocked tasks:
feature-a: ci-status, branch-diff
feature-b: ahead-behind
"
);
}

/// `render_reveal` picks `format_list_item_line` for rows with data and
/// `render_skeleton_row` for rows without — the choice is load-bearing
/// for the picker's partial-row reveal correctness, since the
Expand Down
45 changes: 44 additions & 1 deletion src/commands/picker/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ use anyhow::Context;
use skim::prelude::*;
use skim::reader::CommandCollector;
use worktrunk::git::{Repository, current_or_recover};
use worktrunk::styling::eprintln;

use super::command_executor::FailureStrategy;
use super::hooks::{HookAnnouncer, execute_hook};
Expand All @@ -130,6 +131,16 @@ use items::{PreviewCache, WorktreeSkimItem};
use preview::{PreviewLayout, PreviewMode, PreviewState};
use preview_orchestrator::PreviewOrchestrator;

/// Drain stashed warnings to stderr. Called after skim has released the
/// terminal (or in the dry-run path after the bg thread joins) — eprintln
/// during the picker would corrupt skim's frame, so collect routes warnings
/// through `PickerProgressHandler::stash_warning` and we emit them here.
fn drain_stashed_warnings(stash: &Mutex<Vec<String>>) {
for line in stash.lock().unwrap().drain(..) {
eprintln!("{line}");
}
}

/// Action selected by the user in the picker.
enum PickerAction {
/// Switch to the selected worktree (Enter key).
Expand Down Expand Up @@ -610,6 +621,11 @@ pub fn handle_picker(

let (tx, rx): (SkimItemSender, SkimItemReceiver) = unbounded();

// Shared between the bg-thread handler (which pushes warnings while
// skim owns the terminal) and the main thread (which drains them after
// `Skim::run_with` returns and stderr is safe again).
let stashed_warnings: Arc<Mutex<Vec<String>>> = Arc::new(Mutex::new(Vec::new()));

let handler: Arc<dyn collect::PickerProgressHandler> =
Arc::new(progressive_handler::PickerHandler {
tx: tx.clone(),
Expand All @@ -620,6 +636,7 @@ pub fn handle_picker(
preview_dims,
llm_command,
summary_hint,
stashed_warnings: Arc::clone(&stashed_warnings),
});

// Spawn collect on a background thread. The handler holds the only
Expand Down Expand Up @@ -661,6 +678,7 @@ pub fn handle_picker(
drop(rx);
let _ = bg_handle.join();
orchestrator.wait_for_idle();
drain_stashed_warnings(&stashed_warnings);
println!("{}", orchestrator.dump_cache_json());
return Ok(());
}
Expand All @@ -675,6 +693,12 @@ pub fn handle_picker(
let output = Skim::run_with(&options, Some(rx));
drop(bg_handle);

// Skim has released the terminal — emit any warnings that collect's bg
// thread stashed during the run. Late warnings (e.g. drain timeouts)
// may still be in flight; we capture whatever has landed by now and let
// the rest fall on the floor with the bg thread.
drain_stashed_warnings(&stashed_warnings);

// Handle selection (signal file cleaned up by PreviewState::Drop)
if let Some(out) = output
&& !out.is_abort
Expand Down Expand Up @@ -821,11 +845,30 @@ fn resolve_identifier(
#[cfg(test)]
pub mod tests {
use super::preview::{PreviewLayout, PreviewMode, PreviewStateData};
use super::{PickerAction, PickerCollector, resolve_identifier};
use super::{PickerAction, PickerCollector, drain_stashed_warnings, resolve_identifier};
use crate::commands::worktree::RemoveResult;
use std::fs;
use std::sync::Mutex;
use worktrunk::git::BranchDeletionMode;

/// Empties the stash and emits each line. Verifies post-skim drain
/// semantics without standing up a real picker.
#[test]
fn drain_stashed_warnings_empties_the_stash() {
let stash = Mutex::new(vec!["one".to_string(), "two".to_string()]);
drain_stashed_warnings(&stash);
assert!(stash.lock().unwrap().is_empty());
}

/// A fresh stash with no warnings is a no-op — exercising the empty path
/// keeps the loop body covered when the picker exits cleanly.
#[test]
fn drain_stashed_warnings_handles_empty_stash() {
let stash: Mutex<Vec<String>> = Mutex::new(Vec::new());
drain_stashed_warnings(&stash);
assert!(stash.lock().unwrap().is_empty());
}

#[test]
fn test_preview_state_data_roundtrip() {
let state_path = PreviewStateData::state_path();
Expand Down
21 changes: 21 additions & 0 deletions src/commands/picker/progressive_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ pub(super) struct PickerHandler {
/// are disabled — gives the Summary tab something useful instead of a
/// perpetual "Generating…" placeholder.
pub(super) summary_hint: Option<String>,
/// Pre-formatted warning lines stashed by `collect::collect` while skim
/// owns the terminal. The picker drains and emits these to stderr after
/// `Skim::run_with` returns. Lines are kept in arrival order.
pub(super) stashed_warnings: Arc<Mutex<Vec<String>>>,
}

impl PickerProgressHandler for PickerHandler {
Expand Down Expand Up @@ -123,6 +127,10 @@ impl PickerProgressHandler for PickerHandler {
*slot.lock().unwrap() = strip_osc8_hyperlinks(&line);
}
}

fn stash_warning(&self, line: String) {
self.stashed_warnings.lock().unwrap().push(line);
}
}

#[cfg(test)]
Expand Down Expand Up @@ -150,6 +158,7 @@ mod tests {
preview_dims: (80, 24),
llm_command: None,
summary_hint: Some("disabled".to_string()),
stashed_warnings: Arc::new(Mutex::new(Vec::new())),
};
(handler, test, rx)
}
Expand Down Expand Up @@ -227,4 +236,16 @@ mod tests {
assert_eq!(shared[1].output().as_ref(), "feat-a");
assert_eq!(shared[2].output().as_ref(), "feat-b");
}

/// `stash_warning` accumulates lines in arrival order so the picker can
/// drain them in one shot after skim releases the terminal.
#[test]
fn stash_warning_preserves_order() {
let (handler, _test, _rx) = make_handler();
handler.stash_warning("first".into());
handler.stash_warning("second".into());
handler.stash_warning("third".into());
let stash = handler.stashed_warnings.lock().unwrap();
assert_eq!(stash.as_slice(), &["first", "second", "third"]);
}
}
Loading
Loading