From 9ed876d7e958d63bab80eaaa2e94785042f33681 Mon Sep 17 00:00:00 2001 From: Ian Pascoe Date: Tue, 3 Mar 2026 06:59:31 -0500 Subject: [PATCH] fix(cli): harden non-install output mode rendering --- ...tput-mode-hardening-implementation-plan.md | 140 ++++++++++ crates/crosspack-cli/src/bundle_flows.rs | 14 +- crates/crosspack-cli/src/command_flows.rs | 240 +++++++++++------- crates/crosspack-cli/src/dispatch.rs | 113 ++++++++- crates/crosspack-cli/src/main.rs | 14 +- crates/crosspack-cli/src/render.rs | 14 +- crates/crosspack-cli/src/tests.rs | 170 ++++++++++++- 7 files changed, 597 insertions(+), 108 deletions(-) create mode 100644 .opencode/plans/2026-03-03-cli-output-mode-hardening-implementation-plan.md diff --git a/.opencode/plans/2026-03-03-cli-output-mode-hardening-implementation-plan.md b/.opencode/plans/2026-03-03-cli-output-mode-hardening-implementation-plan.md new file mode 100644 index 0000000..f668fb9 --- /dev/null +++ b/.opencode/plans/2026-03-03-cli-output-mode-hardening-implementation-plan.md @@ -0,0 +1,140 @@ +# CLI Output Mode Hardening Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. + +**Goal:** Make all non-install command output mode-safe so TTY sessions show rich output only and non-TTY sessions show plain deterministic output only. + +**Architecture:** Centralize output mode guarantees in renderer and command flow boundaries. Keep plain deterministic contracts as the source of truth, and only apply rich decoration when `OutputStyle::Rich` is active. Refactor progress-backed commands to avoid mixed/interleaved output by printing status lines through renderer-aware helpers while progress is active. + +**Tech Stack:** Rust, clap, indicatif, cargo test + +--- + +### Task 1: Harden renderer mode boundaries and progress behavior + +**Files:** +- Modify: `crates/crosspack-cli/src/render.rs` +- Test: `crates/crosspack-cli/src/tests.rs` + +**Step 1: Write failing tests for renderer contracts** + +Add tests that fail against current behavior: +- rich progress completion line is not emitted when `total == 0` +- renderer status printing while progress is active does not produce mixed raw-line output behavior +- rich renderer never emits plain-only badges/lines in paths intended to be structured + +**Step 2: Run targeted tests to confirm failures** + +Run: `rustup run stable cargo test -p crosspack-cli renderer -- --nocapture` + +Expected: new tests fail before implementation. + +**Step 3: Implement renderer hardening** + +In `render.rs`: +- Add progress-safe status/line emission APIs for use while a `TerminalProgress` is active. +- Ensure rich progress completion output is suppressed for zero-work totals. +- Keep plain mode behavior unchanged. + +**Step 4: Run renderer-focused tests** + +Run: `rustup run stable cargo test -p crosspack-cli renderer` + +Expected: renderer tests pass. + +### Task 2: Refactor progress-backed command flows to avoid mixed output + +**Files:** +- Modify: `crates/crosspack-cli/src/command_flows.rs` +- Test: `crates/crosspack-cli/src/tests.rs` + +**Step 1: Write failing tests for progress-backed command output** + +Add tests for: +- `run_update_command` zero-source rich mode produces no fake progress completion line +- `run_update_command` still emits deterministic `update summary: ...` line +- rich mode command output for `update`, `upgrade`, `uninstall`, and `self-update` stays renderer-mediated (no mixed plain line artifacts) + +**Step 2: Run targeted tests to confirm failures** + +Run: `rustup run stable cargo test -p crosspack-cli update_output` + +Expected: new tests fail before implementation. + +**Step 3: Implement command flow output refactor** + +In `command_flows.rs`: +- Replace raw `println!` calls inside active progress loops with renderer/progress-safe printing. +- For zero-work paths, skip progress rendering and print only stable status/summary lines. +- Preserve plain-mode deterministic content exactly (especially `update summary: updated= up-to-date= failed=`). + +**Step 4: Run command-flow-focused tests** + +Run: `rustup run stable cargo test -p crosspack-cli update_output` + +Expected: new tests pass. + +### Task 3: Enforce strict TTY split across dispatch and bundle command surfaces + +**Files:** +- Modify: `crates/crosspack-cli/src/dispatch.rs` +- Modify: `crates/crosspack-cli/src/bundle_flows.rs` +- Modify: `crates/crosspack-cli/src/main.rs` +- Test: `crates/crosspack-cli/src/tests.rs` + +**Step 1: Write failing tests for strict mode split** + +Add tests that verify: +- when style resolves to `Rich`, user-facing status lines are rich-only +- when style resolves to `Plain`, output remains plain-only without rich adornments +- bundle status output follows style split while raw payload output (bundle document stdout, completion script content) remains undecorated bytes + +**Step 2: Run targeted tests to confirm failures** + +Run: `rustup run stable cargo test -p crosspack-cli output_style` + +Expected: new tests fail before implementation. + +**Step 3: Implement strict mode split refactor** + +In `dispatch.rs`, `bundle_flows.rs`, and helper functions in `main.rs`: +- Route human-readable status output through style-aware render helpers. +- Ensure no command emits both rich and plain formatting in the same execution mode. +- Keep machine/script payload output raw and deterministic. + +**Step 4: Run targeted output style tests** + +Run: `rustup run stable cargo test -p crosspack-cli output_style` + +Expected: tests pass. + +### Task 4: Full verification and cleanup + +**Files:** +- Modify: `crates/crosspack-cli/src/tests.rs` (if minor cleanup needed) + +**Step 1: Run formatter and lints** + +Run: `rustup run stable cargo fmt --all` + +Run: `rustup run stable cargo clippy -p crosspack-cli --all-targets -- -D warnings` + +Expected: no lint errors. + +**Step 2: Run full crate tests** + +Run: `rustup run stable cargo test -p crosspack-cli` + +Expected: all tests pass. + +**Step 3: Manual mode verification for update output** + +Run (non-TTY): +- `cargo run -q -p crosspack-cli --bin crosspack -- update` + +Run (TTY simulation): +- `script -q -c "cargo run -q -p crosspack-cli --bin crosspack -- update" /dev/null` + +Expected: +- non-TTY shows plain lines only +- TTY shows rich presentation only, with no fake zero-work progress completion line. diff --git a/crates/crosspack-cli/src/bundle_flows.rs b/crates/crosspack-cli/src/bundle_flows.rs index 7cbb855..96bf57c 100644 --- a/crates/crosspack-cli/src/bundle_flows.rs +++ b/crates/crosspack-cli/src/bundle_flows.rs @@ -59,6 +59,10 @@ fn default_bundle_file_path() -> PathBuf { PathBuf::from(DEFAULT_BUNDLE_FILE) } +fn format_bundle_export_status_line(style: OutputStyle, path: &Path) -> String { + render_status_line(style, "ok", &format!("bundle exported: {}", path.display())) +} + fn run_bundle_command( layout: &PrefixLayout, registry_root: Option<&Path>, @@ -108,7 +112,10 @@ fn run_bundle_export_command(layout: &PrefixLayout, output: Option<&Path>) -> Re } fs::write(path, rendered) .with_context(|| format!("failed writing bundle file: {}", path.display()))?; - println!("bundle exported: {}", path.display()); + println!( + "{}", + format_bundle_export_status_line(current_output_style(), path) + ); } None => { print!("{rendered}"); @@ -268,7 +275,10 @@ fn run_bundle_apply_command( })?; if let Err(err) = sync_completion_assets_best_effort(layout, "bundle-apply") { - eprintln!("{err}"); + eprintln!( + "{}", + render_status_line(output_style, "warn", &err.to_string()) + ); } Ok(()) diff --git a/crates/crosspack-cli/src/command_flows.rs b/crates/crosspack-cli/src/command_flows.rs index d79a59d..58a614c 100644 --- a/crates/crosspack-cli/src/command_flows.rs +++ b/crates/crosspack-cli/src/command_flows.rs @@ -626,6 +626,43 @@ fn collect_cache_files_recursive( Ok(()) } +fn should_render_progress(total_steps: u64) -> bool { + total_steps > 0 +} + +fn set_progress(progress: &mut Option, current: u64) { + if let Some(active_progress) = progress.as_mut() { + active_progress.set(current); + } +} + +fn print_status_with_progress( + renderer: TerminalRenderer, + progress: Option<&TerminalProgress>, + status: &str, + message: &str, +) { + if let Some(active_progress) = progress { + active_progress.print_status(status, message); + } else { + renderer.print_status(status, message); + } +} + +fn print_line_with_progress(progress: Option<&TerminalProgress>, line: &str) { + if let Some(active_progress) = progress { + active_progress.print_line(line); + } else { + println!("{line}"); + } +} + +fn finish_progress(progress: Option) { + if let Some(active_progress) = progress { + active_progress.finish_success(); + } +} + struct UpgradeCommandOptions<'a> { dry_run: bool, explain: bool, @@ -803,7 +840,8 @@ fn run_upgrade_command( enforce_no_downgrades(&receipts, &resolved, "upgrade")?; let total_packages = resolved.len() as u64; let mut completed_packages = 0_u64; - let mut progress = renderer.start_progress("upgrade", total_packages); + let mut progress = should_render_progress(total_packages) + .then(|| renderer.start_progress("upgrade", total_packages)); append_transaction_journal_entry( layout, @@ -818,7 +856,7 @@ fn run_upgrade_command( journal_seq += 1; for package in &resolved { - progress.set(completed_packages); + set_progress(&mut progress, completed_packages); if let Some(old) = receipts.iter().find(|r| r.name == package.manifest.name) { let old_version = Version::parse(&old.version).with_context(|| { format!( @@ -827,7 +865,9 @@ fn run_upgrade_command( ) })?; if package.manifest.version <= old_version { - renderer.print_status( + print_status_with_progress( + renderer, + progress.as_ref(), "step", &format!( "{} is up-to-date ({})", @@ -835,7 +875,7 @@ fn run_upgrade_command( ), ); completed_packages += 1; - progress.set(completed_packages); + set_progress(&mut progress, completed_packages); continue; } } @@ -890,30 +930,26 @@ fn run_upgrade_command( Some(&mut source_build_journal), )?; if let Some(old) = receipts.iter().find(|r| r.name == package.manifest.name) { - println!( - "{}", - render_status_line( - output_style, - "ok", - &format!( - "upgraded {} from {} to {}", - package.manifest.name, old.version, package.manifest.version - ) - ) + print_status_with_progress( + renderer, + progress.as_ref(), + "ok", + &format!( + "upgraded {} from {} to {}", + package.manifest.name, old.version, package.manifest.version + ), ); } - println!( - "{}", - render_status_line( - output_style, - "step", - &format!("receipt: {}", outcome.receipt_path.display()) - ) + print_status_with_progress( + renderer, + progress.as_ref(), + "step", + &format!("receipt: {}", outcome.receipt_path.display()), ); completed_packages += 1; - progress.set(completed_packages); + set_progress(&mut progress, completed_packages); } - progress.finish_success(); + finish_progress(progress); } None => { let plans = build_upgrade_plans(&receipts); @@ -979,13 +1015,14 @@ fn run_upgrade_command( .map(std::vec::Vec::len) .sum::() as u64; let mut completed_packages = 0_u64; - let mut progress = renderer.start_progress("upgrade", total_packages); + let mut progress = should_render_progress(total_packages) + .then(|| renderer.start_progress("upgrade", total_packages)); for (resolved, plan) in grouped_resolved.iter().zip(plans.iter()) { let planned_dependency_overrides = build_planned_dependency_overrides(resolved); for package in resolved { - progress.set(completed_packages); + set_progress(&mut progress, completed_packages); if let Some(old) = receipts.iter().find(|r| r.name == package.manifest.name) { let old_version = Version::parse(&old.version).with_context(|| { @@ -995,19 +1032,17 @@ fn run_upgrade_command( ) })?; if package.manifest.version <= old_version { - println!( - "{}", - render_status_line( - output_style, - "step", - &format!( - "{} is up-to-date ({})", - package.manifest.name, old.version - ) - ) + print_status_with_progress( + renderer, + progress.as_ref(), + "step", + &format!( + "{} is up-to-date ({})", + package.manifest.name, old.version + ), ); completed_packages += 1; - progress.set(completed_packages); + set_progress(&mut progress, completed_packages); continue; } } @@ -1066,45 +1101,37 @@ fn run_upgrade_command( )?; if let Some(old) = receipts.iter().find(|r| r.name == package.manifest.name) { - println!( - "{}", - render_status_line( - output_style, - "ok", - &format!( - "upgraded {} from {} to {}", - package.manifest.name, - old.version, - package.manifest.version - ) - ) + print_status_with_progress( + renderer, + progress.as_ref(), + "ok", + &format!( + "upgraded {} from {} to {}", + package.manifest.name, old.version, package.manifest.version + ), ); } else { - println!( - "{}", - render_status_line( - output_style, - "ok", - &format!( - "installed dependency {} {}", - package.manifest.name, package.manifest.version - ) - ) + print_status_with_progress( + renderer, + progress.as_ref(), + "ok", + &format!( + "installed dependency {} {}", + package.manifest.name, package.manifest.version + ), ); } - println!( - "{}", - render_status_line( - output_style, - "step", - &format!("receipt: {}", outcome.receipt_path.display()) - ) + print_status_with_progress( + renderer, + progress.as_ref(), + "step", + &format!("receipt: {}", outcome.receipt_path.display()), ); completed_packages += 1; - progress.set(completed_packages); + set_progress(&mut progress, completed_packages); } } - progress.finish_success(); + finish_progress(progress); } } @@ -2246,14 +2273,19 @@ fn run_uninstall_command(layout: &PrefixLayout, name: String) -> Result<()> { } else { "ok" }; - let total_steps = (1 + result.pruned_dependencies.len()) as u64; - let mut progress = renderer.start_progress("uninstall", total_steps); - progress.set(0); + let total_steps = if matches!(result.status, UninstallStatus::BlockedByDependents) { + 0 + } else { + (1 + result.pruned_dependencies.len()) as u64 + }; + let mut progress = should_render_progress(total_steps) + .then(|| renderer.start_progress("uninstall", total_steps)); + set_progress(&mut progress, 0); for line in format_uninstall_messages(&result) { - renderer.print_status(status, &line); + print_status_with_progress(renderer, progress.as_ref(), status, &line); } - progress.set(total_steps); - progress.finish_success(); + set_progress(&mut progress, total_steps); + finish_progress(progress); Ok(()) })?; @@ -2270,24 +2302,44 @@ fn run_update_command(store: &RegistrySourceStore, registry: &[String]) -> Resul let output_style = renderer.style(); let results = store.update_sources(registry)?; let report = build_update_report(&results); + let update_output = plan_update_output(&report, output_style); + let UpdateOutputPlan { + lines, + render_progress, + summary_line, + } = update_output; renderer.print_section("Registry update"); - let total_sources = report.lines.len() as u64; + let total_sources = lines.len() as u64; let mut processed_sources = 0_u64; - let mut progress = renderer.start_progress("update", total_sources); - for line in format_update_output_lines(&report, output_style) { - progress.set(processed_sources); - println!("{line}"); + let mut progress = render_progress.then(|| renderer.start_progress("update", total_sources)); + for line in lines { + set_progress(&mut progress, processed_sources); + print_line_with_progress(progress.as_ref(), &line); processed_sources += 1; - progress.set(processed_sources); + set_progress(&mut progress, processed_sources); } - progress.finish_success(); - println!( - "{}", - format_update_summary_line(report.updated, report.up_to_date, report.failed) - ); + finish_progress(progress); + println!("{summary_line}"); ensure_update_succeeded(report.failed) } +struct UpdateOutputPlan { + lines: Vec, + render_progress: bool, + summary_line: String, +} + +fn plan_update_output(report: &UpdateReport, style: OutputStyle) -> UpdateOutputPlan { + let lines = format_update_output_lines(report, style); + let render_progress = should_render_progress(lines.len() as u64); + let summary_line = format_update_summary_line(report.updated, report.up_to_date, report.failed); + UpdateOutputPlan { + lines, + render_progress, + summary_line, + } +} + fn run_self_update_command( layout: &PrefixLayout, registry_root: Option<&Path>, @@ -2313,20 +2365,28 @@ fn run_self_update_command( completed_steps = 1; } - let mut progress = renderer.start_progress("self-update", total_steps); - progress.set(completed_steps); + let mut progress = should_render_progress(total_steps) + .then(|| renderer.start_progress("self-update", total_steps)); + set_progress(&mut progress, completed_steps); let args = build_self_update_install_args(registry_root, dry_run, force_redownload, escalation); - renderer.print_status("step", "self-update: installing latest crosspack"); + print_status_with_progress( + renderer, + progress.as_ref(), + "step", + "self-update: installing latest crosspack", + ); let result = run_current_exe_command(&args, "self-update install"); match result { Ok(()) => { - progress.set(total_steps); - progress.finish_success(); + set_progress(&mut progress, total_steps); + finish_progress(progress); Ok(()) } Err(err) => { - progress.finish_abandon(); + if let Some(active_progress) = progress { + active_progress.finish_abandon(); + } Err(err) } } diff --git a/crates/crosspack-cli/src/dispatch.rs b/crates/crosspack-cli/src/dispatch.rs index c4d564e..e30fe26 100644 --- a/crates/crosspack-cli/src/dispatch.rs +++ b/crates/crosspack-cli/src/dispatch.rs @@ -1,3 +1,60 @@ +fn format_pin_status_lines( + style: OutputStyle, + name: &str, + requirement: &VersionReq, + pin_path: &Path, +) -> Vec { + render_status_lines( + style, + vec![ + ("ok", format!("pinned {name} to {requirement}")), + ("step", format!("pin: {}", pin_path.display())), + ], + ) +} + +fn format_registry_add_status_lines( + style: OutputStyle, + name: &str, + kind: &str, + priority: u32, + fingerprint: &str, +) -> Vec { + render_status_lines( + style, + format_registry_add_lines(name, kind, priority, fingerprint) + .into_iter() + .enumerate() + .map(|(index, line)| (if index == 0 { "ok" } else { "step" }, line)), + ) +} + +fn format_registry_remove_status_lines( + style: OutputStyle, + name: &str, + purge_cache: bool, +) -> Vec { + render_status_lines( + style, + format_registry_remove_lines(name, purge_cache) + .into_iter() + .enumerate() + .map(|(index, line)| (if index == 0 { "ok" } else { "step" }, line)), + ) +} + +fn format_registry_list_status_lines( + style: OutputStyle, + sources: Vec, +) -> Vec { + render_status_lines( + style, + format_registry_list_lines(sources) + .into_iter() + .map(|line| ("step", line)), + ) +} + fn run_cli(cli: Cli) -> Result<()> { match cli.command { Commands::Search { query } => { @@ -5,8 +62,18 @@ fn run_cli(cli: Cli) -> Result<()> { let layout = PrefixLayout::new(prefix); let backend = select_metadata_backend(cli.registry_root.as_deref(), &layout)?; let results = run_search_command(&backend, &query)?; - for line in format_search_results(&results, &query) { - println!("{line}"); + let lines = format_search_results(&results, &query); + if results.is_empty() { + for line in render_status_lines( + current_output_style(), + lines.into_iter().map(|line| ("warn", line)), + ) { + println!("{line}"); + } + } else { + for line in lines { + println!("{line}"); + } } } Commands::Info { name } => { @@ -16,7 +83,14 @@ fn run_cli(cli: Cli) -> Result<()> { let versions = backend.package_versions(&name)?; if versions.is_empty() { - println!("No package found: {name}"); + println!( + "{}", + render_status_line( + current_output_style(), + "warn", + &format!("No package found: {name}") + ) + ); } else { for line in format_info_lines(&name, &versions) { println!("{line}"); @@ -181,7 +255,10 @@ fn run_cli(cli: Cli) -> Result<()> { Ok(()) })?; if let Err(err) = sync_completion_assets_best_effort(&layout, "install") { - eprintln!("{err}"); + eprintln!( + "{}", + render_status_line(output_style, "warn", &err.to_string()) + ); } } Commands::Upgrade { @@ -233,7 +310,10 @@ fn run_cli(cli: Cli) -> Result<()> { let layout = PrefixLayout::new(prefix); let receipts = read_install_receipts(&layout)?; if receipts.is_empty() { - println!("No installed packages"); + println!( + "{}", + render_status_line(current_output_style(), "step", "No installed packages") + ); } else { for receipt in receipts { println!("{} {}", receipt.name, receipt.version); @@ -246,8 +326,11 @@ fn run_cli(cli: Cli) -> Result<()> { let layout = PrefixLayout::new(prefix); layout.ensure_base_dirs()?; let pin_path = write_pin(&layout, &name, &requirement.to_string())?; - println!("pinned {name} to {requirement}"); - println!("pin: {}", pin_path.display()); + for line in + format_pin_status_lines(current_output_style(), &name, &requirement, &pin_path) + { + println!("{line}"); + } } Commands::Outdated => { let prefix = default_user_prefix()?; @@ -289,6 +372,7 @@ fn run_cli(cli: Cli) -> Result<()> { let layout = PrefixLayout::new(prefix); let source_state_root = registry_state_root(&layout); let store = RegistrySourceStore::new(&source_state_root); + let output_style = current_output_style(); match command { RegistryCommands::Add { @@ -300,8 +384,13 @@ fn run_cli(cli: Cli) -> Result<()> { } => { let source_kind: RegistrySourceKind = kind.into(); let kind_label = format_registry_kind(source_kind.clone()); - let output_lines = - format_registry_add_lines(&name, kind_label, priority, &fingerprint); + let output_lines = format_registry_add_status_lines( + output_style, + &name, + kind_label, + priority, + &fingerprint, + ); store.add_source(RegistrySourceRecord { name, kind: source_kind, @@ -317,13 +406,15 @@ fn run_cli(cli: Cli) -> Result<()> { } RegistryCommands::List => { let sources = store.list_sources_with_snapshot_state()?; - for line in format_registry_list_lines(sources) { + for line in format_registry_list_status_lines(output_style, sources) { println!("{line}"); } } RegistryCommands::Remove { name, purge_cache } => { store.remove_source_with_cache_purge(&name, purge_cache)?; - for line in format_registry_remove_lines(&name, purge_cache) { + for line in + format_registry_remove_status_lines(output_style, &name, purge_cache) + { println!("{line}"); } } diff --git a/crates/crosspack-cli/src/main.rs b/crates/crosspack-cli/src/main.rs index 61fd448..ad6485f 100644 --- a/crates/crosspack-cli/src/main.rs +++ b/crates/crosspack-cli/src/main.rs @@ -393,8 +393,8 @@ fn build_artifact_install_options<'a>( } } -fn resolve_output_style(stdout_is_tty: bool, _stderr_is_tty: bool) -> OutputStyle { - if stdout_is_tty { +fn resolve_output_style(stdout_is_tty: bool, stderr_is_tty: bool) -> OutputStyle { + if stdout_is_tty && stderr_is_tty { OutputStyle::Rich } else { OutputStyle::Plain @@ -417,6 +417,16 @@ fn render_status_line(style: OutputStyle, status: &str, message: &str) -> String } } +fn render_status_lines( + style: OutputStyle, + entries: impl IntoIterator, +) -> Vec { + entries + .into_iter() + .map(|(status, message)| render_status_line(style, status, &message)) + .collect() +} + fn render_update_line(style: OutputStyle, line: &str) -> String { if line.contains(": failed") { return render_status_line(style, "error", line); diff --git a/crates/crosspack-cli/src/render.rs b/crates/crosspack-cli/src/render.rs index 844265a..b8b2aa6 100644 --- a/crates/crosspack-cli/src/render.rs +++ b/crates/crosspack-cli/src/render.rs @@ -90,6 +90,18 @@ impl TerminalRenderer { } impl TerminalProgress { + fn print_status(&self, status: &str, message: &str) { + self.print_line(&render_status_line(self.style, status, message)); + } + + fn print_line(&self, line: &str) { + if let Some(progress_bar) = &self.progress_bar { + progress_bar.println(line); + } else { + println!("{line}"); + } + } + fn set(&mut self, current: u64) { self.current = current.min(self.total); @@ -184,7 +196,7 @@ fn render_progress_line( total: u64, elapsed: Option, ) -> Option { - if style == OutputStyle::Plain { + if style == OutputStyle::Plain || total == 0 { return None; } diff --git a/crates/crosspack-cli/src/tests.rs b/crates/crosspack-cli/src/tests.rs index b1f547a..4a67ebf 100644 --- a/crates/crosspack-cli/src/tests.rs +++ b/crates/crosspack-cli/src/tests.rs @@ -6519,8 +6519,8 @@ sha256 = "abc" } #[test] - fn resolve_output_style_auto_uses_rich_when_stdout_is_tty_and_stderr_is_not() { - assert_eq!(resolve_output_style(true, false), OutputStyle::Rich); + fn resolve_output_style_auto_uses_plain_when_stdout_is_tty_and_stderr_is_not() { + assert_eq!(resolve_output_style(true, false), OutputStyle::Plain); } #[test] @@ -7059,6 +7059,115 @@ sha256 = "abc" ); } + #[test] + fn dispatch_output_style_formats_pin_status_lines_with_strict_split() { + let requirement = VersionReq::parse("^14").expect("pin requirement should parse"); + let pin_path = Path::new("/tmp/crosspack/state/pins/ripgrep.pin"); + + let plain = format_pin_status_lines(OutputStyle::Plain, "ripgrep", &requirement, pin_path); + assert_eq!( + plain, + vec![ + "pinned ripgrep to ^14".to_string(), + format!("pin: {}", pin_path.display()), + ] + ); + + let rich = format_pin_status_lines(OutputStyle::Rich, "ripgrep", &requirement, pin_path); + assert_eq!(rich[0], "[OK] pinned ripgrep to ^14"); + assert_eq!(rich[1], format!("[..] pin: {}", pin_path.display())); + } + + #[test] + fn dispatch_output_style_formats_registry_add_lines_with_strict_split() { + let plain = format_registry_add_status_lines( + OutputStyle::Plain, + "core", + "git", + 100, + "0123456789abcdef0123456789abcdef", + ); + assert_eq!(plain[0], "added registry core"); + assert_eq!(plain[1], "kind: git"); + assert_eq!(plain[2], "priority: 100"); + assert_eq!(plain[3], "fingerprint: 0123456789abcdef..."); + + let rich = format_registry_add_status_lines( + OutputStyle::Rich, + "core", + "git", + 100, + "0123456789abcdef0123456789abcdef", + ); + assert_eq!(rich[0], "[OK] added registry core"); + assert_eq!(rich[1], "[..] kind: git"); + assert_eq!(rich[2], "[..] priority: 100"); + assert_eq!(rich[3], "[..] fingerprint: 0123456789abcdef..."); + } + + #[test] + fn bundle_output_style_formats_export_status_line_with_strict_split() { + let bundle_path = Path::new("/tmp/crosspack.bundle.toml"); + + let plain = format_bundle_export_status_line(OutputStyle::Plain, bundle_path); + assert_eq!(plain, format!("bundle exported: {}", bundle_path.display())); + + let rich = format_bundle_export_status_line(OutputStyle::Rich, bundle_path); + assert_eq!( + rich, + format!("[OK] bundle exported: {}", bundle_path.display()) + ); + } + + #[test] + fn bundle_output_style_keeps_bundle_document_payload_undecorated() { + let bundle = BundleDocument { + format: BUNDLE_FORMAT_MARKER.to_string(), + version: BUNDLE_FORMAT_VERSION, + roots: vec![BundleRoot { + name: "ripgrep".to_string(), + target: None, + requirement: Some("^14".to_string()), + }], + snapshot_context: None, + }; + + let rendered = render_bundle_document(&bundle).expect("bundle document should render"); + assert!( + rendered.contains("format = \"crosspack.bundle\""), + "bundle payload should remain raw document bytes" + ); + assert!( + !rendered.contains("[OK]") + && !rendered.contains("[..]") + && !rendered.contains("[WARN]") + && !rendered.contains("[ERR]"), + "bundle payload must remain undecorated" + ); + } + + #[test] + fn renderer_progress_line_rich_suppresses_zero_total_completion_line() { + let line = render_progress_line( + OutputStyle::Rich, + "update", + 0, + 0, + Some(std::time::Duration::from_millis(250)), + ); + + assert!( + line.is_none(), + "rich progress completion line must be suppressed when total is zero" + ); + } + + #[test] + fn renderer_terminal_progress_exposes_progress_safe_status_and_line_emitters() { + let _print_status: fn(&TerminalProgress, &str, &str) = TerminalProgress::print_status; + let _print_line: fn(&TerminalProgress, &str) = TerminalProgress::print_line; + } + #[test] fn render_rich_install_detail_row_is_structured_and_badge_free() { let line = render_rich_install_detail_row("step", "archive", "tar.zst"); @@ -7074,6 +7183,54 @@ sha256 = "abc" ); } + #[test] + fn update_output_progress_guard_skips_zero_work_totals() { + assert!(!should_render_progress(0)); + assert!(should_render_progress(1)); + } + + #[test] + fn plan_update_output_disables_progress_for_zero_line_report_and_keeps_summary_contract() { + let report = empty_update_report(); + + let plan = plan_update_output(&report, OutputStyle::Plain); + + assert!(plan.lines.is_empty()); + assert!(!plan.render_progress); + assert_eq!( + plan.summary_line, + "update summary: updated=0 up-to-date=0 failed=0" + ); + } + + #[test] + fn plan_update_output_enables_progress_for_non_empty_report() { + let report = sample_update_report(); + + let plan = plan_update_output(&report, OutputStyle::Plain); + + assert!(plan.render_progress); + } + + #[test] + fn plan_update_output_matches_plain_contract_and_decorates_rich_lines() { + let report = sample_update_report(); + + let plain_plan = plan_update_output(&report, OutputStyle::Plain); + assert_eq!(plain_plan.lines, report.lines); + + let rich_plan = plan_update_output(&report, OutputStyle::Rich); + assert_eq!(rich_plan.lines[0], "[OK] core: updated (snapshot=git:abc)"); + assert_eq!( + rich_plan.lines[1], + "[..] mirror: up-to-date (snapshot=git:abc)" + ); + assert_eq!( + rich_plan.lines[2], + "[ERR] edge: failed (reason=source-sync-failed)" + ); + } + #[test] fn format_update_output_lines_plain_preserves_contract_lines() { let report = sample_update_report(); @@ -8210,6 +8367,15 @@ sha256 = "abc" } } + fn empty_update_report() -> super::UpdateReport { + super::UpdateReport { + lines: Vec::new(), + updated: 0, + up_to_date: 0, + failed: 0, + } + } + static TEST_LAYOUT_COUNTER: AtomicU64 = AtomicU64::new(0); #[cfg(unix)]