fix: make text-mode progress output usable in CI

Cargo.toml

@@ -30,7 +30,7 @@ arc-swap           = "1"
 chrono             = "0.4"
 clap               = { version = "4", features = ["derive"] }
 clap_usage         = "2"
-clx                = "2.0"
+clx                = "2.0.1"
 color-eyre         = "0.6"
 console            = "0.16"
 dashmap            = "6.1.0"

docs/environment_variables.md

@@ -239,7 +239,7 @@ Example output shape:
 Type: `bool`
 Default: `false`
 
-Controls whether per-step output summaries are printed in plain text mode. By default, summaries are only shown when hk is rendering progress bars (non-text mode). Set this to `true` to force summaries to appear in text mode.
+Controls whether per-step output summaries are printed in plain text mode. In text mode, hk only emits summaries for **failed** steps by default (so CI logs always include the full diagnostic for a failure). Successful steps stream their output during execution, so a trailing summary would just duplicate it. Set this to `true` to force summaries to appear for every step in text mode.
 
 Example:
 

src/hook.rs

@@ -153,6 +153,10 @@ pub struct HookContext {
     skipped_steps: std::sync::Mutex<IndexMap<String, SkipReason>>,
     /// Aggregated output per step name (in insertion order)
     pub output_by_step: std::sync::Mutex<IndexMap<String, (OutputSummary, String)>>,
+    /// Names of steps that failed during this run. Tracked here because
+    /// `step_contexts` are shift_remove'd as soon as each step finishes, so
+    /// status info is not available to the end-of-run summary.
+    pub failed_steps: std::sync::Mutex<HashSet<String>>,
     /// Collected fix suggestions to display at end of run
     pub fix_suggestions: std::sync::Mutex<Vec<String>>,
     pub should_stage: bool,
@@ -203,6 +207,7 @@ impl HookContext {
             skip_steps,
             skipped_steps: StdMutex::new(IndexMap::new()),
             output_by_step: StdMutex::new(IndexMap::new()),
+            failed_steps: StdMutex::new(HashSet::new()),
             fix_suggestions: StdMutex::new(Vec::new()),
             should_stage,
             initial_untracked,
@@ -292,6 +297,13 @@ impl HookContext {
             .or_insert_with(|| (mode, text.to_string()));
     }
 
+    pub fn mark_step_failed(&self, step_name: &str) {
+        self.failed_steps
+            .lock()
+            .unwrap()
+            .insert(step_name.to_string());
+    }
+
     pub fn add_fix_suggestion(&self, suggestion: String) {
         self.fix_suggestions.lock().unwrap().push(suggestion);
     }
@@ -1015,10 +1027,24 @@ impl Hook {
         // Clear progress bars before displaying summary
         clx::progress::stop();
 
-        // Display aggregated output from steps, once per step
-        if clx::progress::output() != ProgressOutput::Text || *env::HK_SUMMARY_TEXT {
+        // Display aggregated output from steps, once per step.
+        //
+        // In UI mode we always emit summaries because the progress display
+        // hides streamed output behind the spinner. In Text mode, successful
+        // steps already streamed their full output, so we suppress those
+        // summaries by default — but failed steps still get a summary so the
+        // user can see the diagnostic in full (text-mode streaming truncates
+        // each line via the `message` prop). `HK_SUMMARY_TEXT=1` forces all
+        // summaries to print in text mode.
+        let in_text_mode = clx::progress::output() == ProgressOutput::Text;
+        let force_summary = *env::HK_SUMMARY_TEXT;
+        let failed_steps = hook_ctx.failed_steps.lock().unwrap().clone();
+        if !in_text_mode || force_summary || !failed_steps.is_empty() {
             let outputs = hook_ctx.output_by_step.lock().unwrap().clone();
             for (step_name, (mode, output)) in outputs.into_iter() {
+                if in_text_mode && !force_summary && !failed_steps.contains(&step_name) {
+                    continue;
+                }
                 let trimmed = output.trim_end();
                 if trimmed.is_empty() {
                     continue;

src/step/output.rs

@@ -46,6 +46,10 @@ impl Step {
             return;
         }
 
+        if is_failure {
+            ctx.hook_ctx.mark_step_failed(&self.name);
+        }
+
         // On failure, use combined output so diagnostic messages are never
         // lost regardless of which stream the tool writes to — but keep
         // the configured label so tests/users see the expected header.

src/step/runner.rs

@@ -27,6 +27,24 @@ use super::shell::ShellType;
 use super::types::{Pattern, RunType, Script, Step};
 use crate::error::Error;
 
+/// Cap on the per-job progress message in printable characters. The
+/// rendered run command can be a multi-line shell script or contain
+/// inline-generated args; text mode prints every prop update verbatim,
+/// so an unbounded message floods CI logs. 2048 is generous for any
+/// realistic diagnostic and bounds the pathological case.
+const MAX_PROGRESS_MESSAGE_CHARS: usize = 2048;
+
+/// Truncate a progress message to `max_chars` printable characters with
+/// a trailing `…`. ANSI-aware via `console::truncate_str` so escape
+/// sequences don't get split mid-cluster or counted toward the budget.
+/// Returns the input unchanged if it fits.
+fn truncate_progress_message(s: &str, max_chars: usize) -> String {
+    if console::measure_text_width(s) <= max_chars {
+        return s.to_string();
+    }
+    console::truncate_str(s, max_chars, "…").into_owned()
+}
+
 impl Step {
     /// Execute a single job.
     ///
@@ -128,17 +146,34 @@ impl Step {
         if let Some(prefix) = &self.prefix {
             run = format!("{prefix} {run}");
         }
+        // Render twice: once with the full file list for execution, and
+        // once with the display context — which truncates `files` /
+        // `workspace_files` to `first_file …` when there are multiple
+        // files. A 98-file step would otherwise emit ~4KB of paths in the
+        // progress message; this keeps the command shape and one
+        // concrete example path visible without unbounded expansion.
+        let run_for_display =
+            tera::render(&run, &tctx.for_display()).unwrap_or_else(|_| run.clone());
         let run = tera::render(&run, &tctx)
             .wrap_err_with(|| format!("{self}: failed to render command template"))?;
         let pattern_display = match &self.glob {
             Some(Pattern::Globs(g)) => g.join(" "),
             Some(Pattern::Regex { pattern, .. }) => format!("regex: {}", pattern),
             None => String::new(),
         };
-        job.progress.as_ref().unwrap().prop(
-            "message",
-            &format!("{} – {} – {}", file_msg(&job.files), pattern_display, run),
+        // Cap the full progress message: a `check =` value can be a
+        // multi-line shell script or an inline-generated command, and
+        // text mode prints every render verbatim. 2KB is generous for
+        // any realistic diagnostic and bounds the pathological case
+        // (e.g. a 200-line embedded script dumped on every prop update).
+        let raw_message = format!(
+            "{} – {} – {}",
+            file_msg(&job.files),
+            pattern_display,
+            run_for_display
         );
+        let message = truncate_progress_message(&raw_message, MAX_PROGRESS_MESSAGE_CHARS);
+        job.progress.as_ref().unwrap().prop("message", &message);
         job.progress.as_ref().unwrap().update();
         if log::log_enabled!(log::Level::Trace) {
             for file in &job.files {
@@ -403,6 +438,34 @@ impl Step {
 mod tests {
     use super::*;
 
+    #[test]
+    fn truncate_progress_message_passes_short_input() {
+        assert_eq!(truncate_progress_message("hello", 100), "hello");
+    }
+
+    #[test]
+    fn truncate_progress_message_caps_long_input() {
+        let s = "a".repeat(5000);
+        let out = truncate_progress_message(&s, 2048);
+        assert_eq!(console::measure_text_width(&out), 2048);
+        assert!(out.ends_with('…'));
+    }
+
+    #[test]
+    fn truncate_progress_message_preserves_ansi_escapes() {
+        // Color escape + long plain run; truncation must be width-aware
+        // so the ANSI bytes don't count toward the budget.
+        let s = format!("\x1b[31m{}\x1b[0m", "a".repeat(100));
+        let out = truncate_progress_message(&s, 50);
+        assert_eq!(console::measure_text_width(&out), 50);
+    }
+
+    #[test]
+    fn truncate_progress_message_at_exact_length_unchanged() {
+        let s = "x".repeat(2048);
+        assert_eq!(truncate_progress_message(&s, 2048), s);
+    }
+
     #[test]
     fn test_has_command_for_empty_command() {
         // Mirror the nix_fmt.pkl pattern: windows is empty, other has the real command.

src/step_job.rs

@@ -94,8 +94,13 @@ impl StepJob {
             .body(
                 "{{spinner()}} {% if ensembler_cmd %}{{ensembler_cmd | flex}}{% if ensembler_stdout %}\n{{ensembler_stdout | flex}}{% endif %}{% else %}{{message | flex}}{% endif %}"
             )
+            // Text mode (CI / piped stderr) keeps the full message — the
+            // log viewer handles wrapping, and a 60-char truncate just hides
+            // the diagnostic detail callers actually need to debug a
+            // failure. The UI-mode `body` above still uses `flex` because
+            // the in-place renderer needs bounded line widths.
             .body_text(Some(
-                "{% if ensembler_stdout %}  {{name}} – {{ensembler_stdout | truncate_text}}{% elif message %}{{spinner()}} {{name}} – {{message | truncate_text}}{% endif %}".to_string(),
+                "{% if ensembler_stdout %}  {{name}} – {{ensembler_stdout}}{% elif message %}{{spinner()}} {{name}} – {{message}}{% endif %}".to_string(),
             ))
             .status(ProgressStatus::Hide)
             .on_done(ProgressJobDoneBehavior::Hide)

src/tera.rs

@@ -91,4 +91,138 @@ impl Context {
         self.insert("workspace_files", &files);
         self
     }
+
+    /// Returns a clone of this context where `files` and `workspace_files`
+    /// are truncated to "first_file …" when there is more than one file.
+    /// Used to render the human-readable progress message — keeps the
+    /// rendered command compact for steps matching hundreds of files
+    /// while still showing one concrete path.
+    pub fn for_display(&self) -> Self {
+        let mut ctx = self.clone();
+        if let Some(truncated) = truncate_quoted_list(self.ctx.get("files")) {
+            ctx.insert("files", &truncated);
+        }
+        if let Some(truncated) = truncate_quoted_list(self.ctx.get("workspace_files")) {
+            ctx.insert("workspace_files", &truncated);
+        }
+        ctx
+    }
+}
+
+/// Truncate a space-separated quoted-token list to "first …" when it
+/// contains more than one token. Returns `None` if the value is missing,
+/// not a string, or has 0–1 tokens (no truncation needed).
+fn truncate_quoted_list(value: Option<&tera::Value>) -> Option<String> {
+    let s = value.and_then(|v| v.as_str())?;
+    let mut tokens = split_quoted_tokens(s);
+    let first = tokens.next()?;
+    tokens.next()?;
+    Some(format!("{first} …"))
+}
+
+/// Split a `with_files`-style joined string back into its quoted tokens.
+/// Tokens are space-separated; quoted tokens preserve embedded spaces.
+/// Honors `\\` escapes inside quoted runs because that's what `ShellType::quote`
+/// emits on Unix shells.
+fn split_quoted_tokens(s: &str) -> impl Iterator<Item = &str> {
+    let bytes = s.as_bytes();
+    let mut start = 0usize;
+    std::iter::from_fn(move || {
+        while start < bytes.len() && bytes[start] == b' ' {
+            start += 1;
+        }
+        if start >= bytes.len() {
+            return None;
+        }
+        let token_start = start;
+        let mut quote: Option<u8> = None;
+        let mut i = start;
+        while i < bytes.len() {
+            let b = bytes[i];
+            match quote {
+                Some(q) => {
+                    if b == b'\\' && i + 1 < bytes.len() {
+                        i += 2;
+                        continue;
+                    }
+                    if b == q {
+                        quote = None;
+                    }
+                }
+                None => {
+                    if b == b' ' {
+                        break;
+                    }
+                    if b == b'\'' || b == b'"' {
+                        quote = Some(b);
+                    }
+                }
+            }
+            i += 1;
+        }
+        let token = &s[token_start..i];
+        start = i;
+        Some(token)
+    })
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use serde_json::json;
+
+    fn split(s: &str) -> Vec<&str> {
+        split_quoted_tokens(s).collect()
+    }
+
+    #[test]
+    fn split_quoted_tokens_handles_unquoted() {
+        assert_eq!(split("a b c"), vec!["a", "b", "c"]);
+    }
+
+    #[test]
+    fn split_quoted_tokens_preserves_single_quoted_spaces() {
+        assert_eq!(
+            split("'has space.txt' other.txt"),
+            vec!["'has space.txt'", "other.txt"]
+        );
+    }
+
+    #[test]
+    fn split_quoted_tokens_handles_escaped_quote_in_double() {
+        assert_eq!(split(r#""a\"b" c"#), vec![r#""a\"b""#, "c"]);
+    }
+
+    #[test]
+    fn truncate_quoted_list_returns_none_for_single_token() {
+        let v = json!("only.txt");
+        assert_eq!(truncate_quoted_list(Some(&v)), None);
+    }
+
+    #[test]
+    fn truncate_quoted_list_truncates_multiple_tokens() {
+        let v = json!("first.txt second.txt third.txt");
+        assert_eq!(
+            truncate_quoted_list(Some(&v)),
+            Some("first.txt …".to_string())
+        );
+    }
+
+    #[test]
+    fn truncate_quoted_list_preserves_quoted_first_token() {
+        let v = json!("'a b.txt' other.txt");
+        assert_eq!(
+            truncate_quoted_list(Some(&v)),
+            Some("'a b.txt' …".to_string())
+        );
+    }
+
+    #[test]
+    fn truncate_quoted_list_returns_none_for_empty_or_missing() {
+        assert_eq!(truncate_quoted_list(None), None);
+        let v = json!("");
+        assert_eq!(truncate_quoted_list(Some(&v)), None);
+        let v = json!("   ");
+        assert_eq!(truncate_quoted_list(Some(&v)), None);
+    }
 }

test/git.bats

@@ -104,6 +104,11 @@ hooks {
 }
 EOF
 
+    # The progress message renders the command with `files` truncated to
+    # "first_file …" when there's more than one file — keeps CI log lines
+    # bounded for steps matching hundreds of files while still showing one
+    # concrete example. The full list still surfaces via streamed stdout.
+
     # Test files between base and feature
     run hk fix -v --from-ref=$BASE_COMMIT --to-ref=$FEATURE_COMMIT
     assert_success
@@ -113,7 +118,7 @@ EOF
     # Test files between base and merge commit
     run hk fix --from-ref=$BASE_COMMIT --to-ref=$MERGE_COMMIT
     assert_success
-    assert_output --partial "print-files – 2 files –  – echo 'feature.txt main.txt'"
+    assert_output --partial "print-files – 2 files –  – echo 'feature.txt …'"
     assert_output --partial "print-files – feature.txt main.txt"
 
     # Test files between feature and merge commit