diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6cd87369b..dec1a0527 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -50,7 +50,7 @@ Don't invent new output formats. Don't add RTK-specific headers or markers in th If a filter fails, fall back to raw output. RTK should never prevent a command from executing or producing output. Better to pass through unfiltered than to error out. Same for hooks: exit 0 on all error paths so the agent's command runs unmodified. -Every filter needs a fallback path. Every hook must handle malformed input gracefully. +Every filter needs a fallback path. Every hook must handle malformed input gracefully. Truncation follows the same rule: capping output at N items is only acceptable if accompanied by a hint that lets the agent recover the hidden data. ### Zero Overhead @@ -262,6 +262,7 @@ cargo fmt --all --check && cargo clippy --all-targets && cargo test - [ ] Unit tests added/updated for changed code - [ ] Snapshot tests reviewed (`cargo insta review`) - [ ] Token savings >=60% verified +- [ ] Any truncated list has a recovery hint (`force_tee_tail_hint` or `force_tee_hint`) and uses a `CAP_*` from `src/core/truncate.rs` - [ ] Edge cases covered - [ ] `cargo fmt --all --check && cargo clippy --all-targets && cargo test` passes - [ ] Manual test: run `rtk ` and inspect output diff --git a/README.md b/README.md index a0db81d25..f8d65efe5 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,7 @@

CI Release - License: MIT + License: Apache 2.0 Discord Homebrew

@@ -483,7 +483,7 @@ Join the community on [Discord](https://discord.gg/RySmvNF5kF). ## License -MIT License - see [LICENSE](LICENSE) for details. +Apache License 2.0 - see [LICENSE](LICENSE) for details. ## Disclaimer diff --git a/install.sh b/install.sh index 32a7b1306..ab6afb18f 100644 --- a/install.sh +++ b/install.sh @@ -98,6 +98,13 @@ install() { error "Failed to download binary" fi + # Verify archive contents before extraction (CWE-22 path traversal). + # Reject any entry with an absolute path or a ".." component. + info "Verifying archive..." + if tar -tzf "$ARCHIVE" | grep -qE '^/|(^|/)\.\.(/|$)'; then + error "Archive contains unsafe paths (absolute or directory traversal) — refusing to extract" + fi + info "Extracting..." tar -xzf "$ARCHIVE" -C "$TEMP_DIR" diff --git a/scripts/test-install.sh b/scripts/test-install.sh new file mode 100755 index 000000000..cc863d25e --- /dev/null +++ b/scripts/test-install.sh @@ -0,0 +1,98 @@ +#!/usr/bin/env sh +# Tests for install.sh path traversal check (issue #1250, CWE-22). +# +# Verifies: +# 1. Safe archives (single binary, "./prefix", subdirs) are accepted. +# 2. Archives with absolute paths are rejected pre-extraction. +# 3. Archives with ".." components are rejected pre-extraction. +# 4. The check is still present in install.sh (regression guard). + +set -eu + +REPO_ROOT=$(cd "$(dirname "$0")/.." && pwd) +INSTALL_SH="$REPO_ROOT/install.sh" + +if [ ! -f "$INSTALL_SH" ]; then + echo "FAIL: install.sh not found at $INSTALL_SH" + exit 1 +fi + +if ! command -v python3 >/dev/null 2>&1; then + echo "SKIP: python3 not available — crafted tarball tests require python3" + exit 0 +fi + +TMPDIR=$(mktemp -d) +trap 'rm -rf "$TMPDIR"' EXIT + +# The check replicated from install.sh (keep in sync with install.sh). +# Returns 0 when archive is safe, 1 when unsafe. +check_archive() { + if tar -tzf "$1" | grep -qE '^/|(^|/)\.\.(/|$)'; then + return 1 + fi + return 0 +} + +# --- Build safe archive using standard tar --- +mkdir -p "$TMPDIR/safe_src" +printf '#!/bin/sh\necho rtk\n' > "$TMPDIR/safe_src/rtk" +(cd "$TMPDIR/safe_src" && tar -czf "$TMPDIR/safe.tgz" rtk) + +# --- Build crafted malicious archives with python --- +python3 - "$TMPDIR" <<'PY' +import sys, tarfile, io + +base = sys.argv[1] + + +def make(name, entry): + with tarfile.open(f"{base}/{name}", "w:gz") as t: + info = tarfile.TarInfo(name=entry) + data = b"pwned" + info.size = len(data) + t.addfile(info, io.BytesIO(data)) + + +make("traversal.tgz", "../etc/evil") +make("absolute.tgz", "/tmp/evil_abs") +make("middle.tgz", "rtk/../../../etc/evil") +make("end_dotdot.tgz", "rtk/..") +PY + +FAIL=0 +pass() { printf ' PASS: %s\n' "$1"; } +fail() { printf ' FAIL: %s\n' "$1"; FAIL=1; } + +echo "==> Functional checks" + +if check_archive "$TMPDIR/safe.tgz"; then + pass "safe archive accepted" +else + fail "safe archive rejected (false positive)" +fi + +for bad in traversal absolute middle end_dotdot; do + if check_archive "$TMPDIR/$bad.tgz"; then + fail "$bad archive accepted (should be rejected)" + else + pass "$bad archive rejected" + fi +done + +echo "==> Regression guard" + +if grep -qF 'tar -tzf' "$INSTALL_SH" && grep -qF '\.\.' "$INSTALL_SH"; then + pass "install.sh still contains the path-traversal check" +else + fail "install.sh is missing the path-traversal check — was it removed?" +fi + +echo "" +if [ "$FAIL" -eq 0 ]; then + echo "All install.sh path traversal tests passed" + exit 0 +else + echo "Some tests failed" + exit 1 +fi diff --git a/src/cmds/README.md b/src/cmds/README.md index 010e9495a..ad3a6ab77 100644 --- a/src/cmds/README.md +++ b/src/cmds/README.md @@ -252,6 +252,21 @@ When filtering fails, fall back to raw output and warn on stderr. Never block th Modules that parse structured output (JSON, NDJSON, state machines) must call `tee::tee_and_hint()` so users can recover full output on failure. +### Internal Truncation Recovery + +When a filter caps a list at N items (e.g. `take(20)`), the remaining items must be accessible via a tee hint. **Never show `"… +N more"` without a recovery path** — the agent has no way to retrieve the hidden content. + +**Choosing the right hint:** + +| Content type | Function | Condition | +|---|---|---| +| Flat list — one item = one line in the tee | `force_tee_tail_hint(content, slug, MAX + 1)` | PR lists, error lines, file paths — anything where each item is a single-line string | +| Multi-line blocks | `force_tee_hint(content, slug)` | Test failures, build error blocks — items that span multiple lines so a line offset is meaningless | + +**Cap values come from `src/core/truncate.rs`.** Pick the `CAP_*` matching your data class (`CAP_ERRORS`, `CAP_WARNINGS`, `CAP_LIST`, `CAP_INVENTORY`) and bind it to a local `const MAX_XXX: usize = CAP_Y;`. Derive `take(MAX_XXX)`, `> MAX_XXX`, and the offset `MAX_XXX + 1` from the local. These CAPs will later become the configuration surface for per-filter cap tuning (user-overridable via config) — keep all truncation values routed through them so that hook lands as a single switch rather than a codebase-wide hunt. A filter that genuinely needs to deviate uses **`truncate::reduced(CAP_Y, n)`** (e.g. `reduced(CAP_WARNINGS, 5)`) so it still tracks the global when reconfigured — never a bare literal, never `cap - n` (underflows once caps are runtime-configurable), and never `*`/`/` (those scale unboundedly). `reduced` falls back to the full cap if the reduction would empty the list. Each deviation needs a one-line comment stating why; if there's no real reason, just use the plain CAP. See `src/core/README.md` ("Truncation Caps") for the full rationale. + +**The tee content must match what `tail` produces.** For `force_tee_tail_hint`, build the tee from the same formatted values shown in the output — not raw/intermediate data. If the filter reformats items before displaying them, pre-build a `Vec` of formatted lines and use it for both the display loop and the tee. + ### Stderr Handling Modules must capture stderr and include it in the raw string passed to `timer.track()`, so token savings reflect total output. @@ -278,6 +293,7 @@ Adding a new filter or command requires changes in multiple places. For TOML-vs- - Use `RunOptions::default()` when filtering combined text output - Add `.tee("label")` when the filter parses structured output (enables raw output recovery on failure) - **Exit codes**: handled automatically by `run_filtered()` — just return its result + - **Truncation**: if the filter caps any list at N items, emit `force_tee_tail_hint` (flat lists) or `force_tee_hint` (multi-line blocks) so the agent can recover hidden items — see [Internal Truncation Recovery](#internal-truncation-recovery). Use a named constant for the cap; derive the offset from it (`MAX_XXX + 1`) 2. **Register module**: - Ecosystem `mod.rs` files use `automod::dir!()` — any `.rs` file in the directory becomes a public module automatically. No manual `pub mod` needed, but be aware: WIP or helper files will also be exposed. Only commit command-ready modules. - Add variant to `Commands` enum in `main.rs` with `#[arg(trailing_var_arg = true, allow_hyphen_values = true)]` diff --git a/src/cmds/cloud/aws_cmd.rs b/src/cmds/cloud/aws_cmd.rs index 2657218b8..64c18cf7c 100644 --- a/src/cmds/cloud/aws_cmd.rs +++ b/src/cmds/cloud/aws_cmd.rs @@ -5,6 +5,7 @@ use crate::core::tee::force_tee_hint; use crate::core::tracking; +use crate::core::truncate::{CAP_INVENTORY, CAP_LIST}; use crate::core::utils::{ exit_code_from_output, exit_code_from_status, human_bytes, join_with_overflow, resolved_command, shorten_arn, truncate_iso_date, @@ -15,7 +16,7 @@ use lazy_static::lazy_static; use regex::Regex; use serde_json::Value; -const MAX_ITEMS: usize = 20; +const MAX_ITEMS: usize = CAP_LIST; const JSON_COMPRESS_DEPTH: usize = 4; /// Result of a filter function: filtered text + whether items were truncated. @@ -494,7 +495,7 @@ fn filter_s3_ls(output: &str) -> FilterResult { if total > limit { let text = format!( - "{}\n... +{} more items", + "{}\n… +{} more items", lines[..limit].join("\n"), total - limit ); @@ -553,7 +554,7 @@ fn filter_ec2_instances(json_str: &str) -> Option { } if truncated { - result.push_str(&format!(" ... +{} more\n", total - MAX_ITEMS)); + result.push_str(&format!(" … +{} more\n", total - MAX_ITEMS)); } let text = result.trim_end().to_string(); @@ -700,7 +701,7 @@ fn filter_cfn_describe_stacks(json_str: &str) -> Option { // --- P0 filters: CloudWatch Logs, CloudFormation Events, Lambda --- -const MAX_LOG_EVENTS: usize = 50; +const MAX_LOG_EVENTS: usize = CAP_INVENTORY; /// Convert days since Unix epoch to (year, month, day). Civil calendar, UTC. fn days_to_ymd(days: i64) -> (i64, i64, i64) { @@ -759,7 +760,7 @@ fn filter_logs_events(json_str: &str) -> Option { } if truncated { - lines.push(format!("... +{} more events", total - MAX_LOG_EVENTS)); + lines.push(format!("… +{} more events", total - MAX_LOG_EVENTS)); } let text = lines.join("\n"); @@ -1132,7 +1133,7 @@ fn filter_dynamodb_items(json_str: &str) -> Option { } if truncated { - lines.push(format!("... +{} more items", total - MAX_ITEMS)); + lines.push(format!("… +{} more items", total - MAX_ITEMS)); } let text = lines.join("\n"); @@ -1426,7 +1427,7 @@ fn filter_logs_query_results(json_str: &str) -> Option { } if truncated { - lines.push(format!("... +{} more rows", total - MAX_ITEMS)); + lines.push(format!("… +{} more rows", total - MAX_ITEMS)); } let text = lines.join("\n"); @@ -1616,7 +1617,7 @@ mod tests { } let input = lines.join("\n"); let result = filter_s3_ls(&input); - assert!(result.text.contains("... +20 more items")); + assert!(result.text.contains("… +20 more items")); assert!(result.truncated); } @@ -1852,7 +1853,7 @@ mod tests { } let json = format!(r#"{{"DBInstances": [{}]}}"#, dbs.join(",")); let result = filter_rds_instances(&json).unwrap(); - assert!(result.text.contains("... +5 more instances")); + assert!(result.text.contains("… +5 more instances")); assert!(result.truncated); } @@ -1893,7 +1894,7 @@ mod tests { } let json = format!(r#"{{"events": [{}]}}"#, events.join(",")); let result = filter_logs_events(&json).unwrap(); - assert!(result.text.contains("... +10 more events")); + assert!(result.text.contains("… +10 more events")); assert!(result.truncated); } diff --git a/src/cmds/cloud/container.rs b/src/cmds/cloud/container.rs index 3931ebf6e..2738cf29e 100644 --- a/src/cmds/cloud/container.rs +++ b/src/cmds/cloud/container.rs @@ -3,6 +3,7 @@ use crate::core::runner::{self, RunOptions}; use crate::core::stream::exec_capture; use crate::core::tracking; +use crate::core::truncate::{CAP_INVENTORY, CAP_LIST, CAP_WARNINGS}; use crate::core::utils::resolved_command; use anyhow::{Context, Result}; use serde_json::Value; @@ -12,6 +13,7 @@ use std::process::Command; #[derive(Debug, Clone, Copy)] pub enum ContainerCmd { DockerPs, + DockerPsAll, DockerImages, DockerLogs, KubectlPods, @@ -22,6 +24,7 @@ pub enum ContainerCmd { pub fn run(cmd: ContainerCmd, args: &[String], verbose: u8) -> Result { match cmd { ContainerCmd::DockerPs => docker_ps(verbose), + ContainerCmd::DockerPsAll => docker_ps_all(verbose), ContainerCmd::DockerImages => docker_images(verbose), ContainerCmd::DockerLogs => docker_logs(args, verbose), ContainerCmd::KubectlPods => kubectl_pods(args, verbose), @@ -81,40 +84,136 @@ fn docker_ps(_verbose: u8) -> Result { return Ok(0); } - let count = stdout.lines().count(); - rtk.push_str(&format!("[docker] {} containers:\n", count)); + const MAX_CONTAINERS: usize = CAP_LIST; + let lines: Vec = stdout + .lines() + .filter(|l| !l.trim().is_empty()) + .filter_map(|line| format_container_line(line, true)) + .collect(); + + rtk.push_str(&format!("[docker] {} containers:\n", lines.len())); + for entry in lines.iter().take(MAX_CONTAINERS) { + rtk.push_str(entry); + } + if lines.len() > MAX_CONTAINERS { + rtk.push_str(&format!(" … +{} more\n", lines.len() - MAX_CONTAINERS)); + let full: String = lines.concat(); + if let Some(hint) = crate::core::tee::force_tee_hint(&full, "docker-ps") { + rtk.push_str(&format!("{}\n", hint)); + } + } + + print!("{}", rtk); + timer.track("docker ps", "rtk docker ps", &raw, &rtk); + Ok(0) +} - for line in stdout.lines().take(15) { +fn docker_ps_all(_verbose: u8) -> Result { + let timer = tracking::TimedExecution::start(); + + let raw = exec_capture(resolved_command("docker").args(["ps", "-a"])) + .map(|r| r.stdout) + .unwrap_or_default(); + + let result = exec_capture(resolved_command("docker").args([ + "ps", + "-a", + "--format", + "{{.State}}\t{{.ID}}\t{{.Names}}\t{{.Status}}\t{{.Image}}\t{{.Ports}}", + ])) + .context("Failed to run docker ps -a")?; + + if !result.success() { + eprint!("{}", result.stderr); + timer.track("docker ps -a", "rtk docker ps -a", &raw, &raw); + return Ok(result.exit_code); + } + + let mut running_lines: Vec = Vec::new(); + let mut stopped_lines: Vec = Vec::new(); + for line in result.stdout.lines().filter(|l| !l.trim().is_empty()) { let parts: Vec<&str> = line.split('\t').collect(); - if parts.len() >= 4 { - let id = &parts[0][..12.min(parts[0].len())]; - let name = parts[1]; - let short_image = parts - .get(3) - .unwrap_or(&"") - .split('/') - .next_back() - .unwrap_or(""); - let ports = compact_ports(parts.get(4).unwrap_or(&"")); - if ports == "-" { - rtk.push_str(&format!(" {} {} ({})\n", id, name, short_image)); + let state = parts.first().copied().unwrap_or(""); + let is_running = matches!(state, "running" | "restarting"); + if let Some(entry) = format_container_line_from_parts(&parts[1..], is_running) { + if is_running { + running_lines.push(entry); } else { - rtk.push_str(&format!( - " {} {} ({}) [{}]\n", - id, name, short_image, ports - )); + stopped_lines.push(entry); } } } - if count > 15 { - rtk.push_str(&format!(" ... +{} more", count - 15)); + + const MAX_CONTAINERS: usize = 20; + let truncated = running_lines.len() > MAX_CONTAINERS || stopped_lines.len() > MAX_CONTAINERS; + + let mut rtk = String::new(); + rtk.push_str(&format!("[docker] {} running:\n", running_lines.len())); + for l in running_lines.iter().take(MAX_CONTAINERS) { + rtk.push_str(l); + } + if running_lines.len() > MAX_CONTAINERS { + rtk.push_str(&format!( + " … +{} more\n", + running_lines.len() - MAX_CONTAINERS + )); + } + if !stopped_lines.is_empty() { + rtk.push_str(&format!( + "[docker] {} stopped/exited:\n", + stopped_lines.len() + )); + for l in stopped_lines.iter().take(MAX_CONTAINERS) { + rtk.push_str(l); + } + if stopped_lines.len() > MAX_CONTAINERS { + rtk.push_str(&format!( + " … +{} more\n", + stopped_lines.len() - MAX_CONTAINERS + )); + } + } + if truncated { + let full: String = running_lines.iter().chain(stopped_lines.iter()).cloned().collect(); + if let Some(hint) = crate::core::tee::force_tee_hint(&full, "docker-ps-a") { + rtk.push_str(&format!("{}\n", hint)); + } } print!("{}", rtk); - timer.track("docker ps", "rtk docker ps", &raw, &rtk); + timer.track("docker ps -a", "rtk docker ps -a", &raw, &rtk); Ok(0) } +fn format_container_line(line: &str, with_ports: bool) -> Option { + let parts: Vec<&str> = line.split('\t').collect(); + format_container_line_from_parts(&parts, with_ports) +} + +fn format_container_line_from_parts(parts: &[&str], with_ports: bool) -> Option { + if parts.len() < 4 { + return None; + } + let id = &parts[0][..12.min(parts[0].len())]; + let name = parts[1]; + let status = parts[2].trim(); + let short_image = parts[3].split('/').next_back().unwrap_or(""); + let port_suffix = if with_ports { + let ports = compact_ports(parts.get(4).unwrap_or(&"")); + if ports == "-" { + String::new() + } else { + format!(" [{}]", ports) + } + } else { + String::new() + }; + Some(format!( + " {} {} ({}) {}{}\n", + id, name, short_image, status, port_suffix + )) +} + fn docker_images(_verbose: u8) -> Result { let timer = tracking::TimedExecution::start(); @@ -173,21 +272,31 @@ fn docker_images(_verbose: u8) -> Result { total_display )); - for line in lines.iter().take(15) { - let parts: Vec<&str> = line.split('\t').collect(); - if !parts.is_empty() { - let image = parts[0]; - let size = parts.get(1).unwrap_or(&""); - let short = if image.len() > 40 { - format!("...{}", &image[image.len() - 37..]) - } else { - image.to_string() - }; - rtk.push_str(&format!(" {} [{}]\n", short, size)); - } + // a full image list is an inventory query, like pip list. + const MAX_IMAGES: usize = CAP_INVENTORY; + let image_lines: Vec = lines + .iter() + .map(|line| { + let parts: Vec<&str> = line.split('\t').collect(); + let image = parts.first().copied().unwrap_or(""); + let size = parts.get(1).copied().unwrap_or(""); + format!(" {} [{}]\n", image, size) + }) + .collect(); + + let mut full_rtk = rtk.clone(); + for l in &image_lines { + full_rtk.push_str(l); + } + + for l in image_lines.iter().take(MAX_IMAGES) { + rtk.push_str(l); } - if lines.len() > 15 { - rtk.push_str(&format!(" ... +{} more", lines.len() - 15)); + if image_lines.len() > MAX_IMAGES { + rtk.push_str(&format!(" … +{} more\n", image_lines.len() - MAX_IMAGES)); + if let Some(hint) = crate::core::tee::force_tee_tail_hint(&full_rtk, "docker-images", MAX_IMAGES + 2) { + rtk.push_str(&format!("{}\n", hint)); + } } print!("{}", rtk); @@ -289,12 +398,19 @@ fn format_kubectl_pods(json: &Value) -> String { let mut out = format!("{} pods: {}\n", pods.len(), parts.join(", ")); if !issues.is_empty() { + const MAX_PODS_ISSUES: usize = CAP_WARNINGS; out.push_str("[warn] Issues:\n"); - for issue in issues.iter().take(10) { + for issue in issues.iter().take(MAX_PODS_ISSUES) { out.push_str(&format!(" {}\n", issue)); } - if issues.len() > 10 { - out.push_str(&format!(" ... +{} more", issues.len() - 10)); + if issues.len() > MAX_PODS_ISSUES { + out.push_str(&format!(" … +{} more", issues.len() - MAX_PODS_ISSUES)); + let all_issues = issues.join("\n"); + if let Some(hint) = + crate::core::tee::force_tee_tail_hint(&all_issues, "kubectl-pods", MAX_PODS_ISSUES + 1) + { + out.push_str(&format!(" {}", hint)); + } } } out @@ -315,39 +431,48 @@ fn format_kubectl_services(json: &Value) -> String { }; let mut out = format!("{} services:\n", services.len()); - for svc in services.iter().take(15) { - let ns = svc["metadata"]["namespace"].as_str().unwrap_or("-"); - let name = svc["metadata"]["name"].as_str().unwrap_or("-"); - let svc_type = svc["spec"]["type"].as_str().unwrap_or("-"); - let ports: Vec = svc["spec"]["ports"] - .as_array() - .map(|arr| { - arr.iter() - .map(|p| { - let port = p["port"].as_i64().unwrap_or(0); - let target = p["targetPort"] - .as_i64() - .or_else(|| p["targetPort"].as_str().and_then(|s| s.parse().ok())) - .unwrap_or(port); - if port == target { - format!("{}", port) - } else { - format!("{}→{}", port, target) - } - }) - .collect() - }) - .unwrap_or_default(); - out.push_str(&format!( - " {}/{} {} [{}]\n", - ns, - name, - svc_type, - ports.join(",") - )); - } - if services.len() > 15 { - out.push_str(&format!(" ... +{} more", services.len() - 15)); + let all_lines: Vec = services + .iter() + .map(|svc| { + let ns = svc["metadata"]["namespace"].as_str().unwrap_or("-"); + let name = svc["metadata"]["name"].as_str().unwrap_or("-"); + let svc_type = svc["spec"]["type"].as_str().unwrap_or("-"); + let ports: Vec = svc["spec"]["ports"] + .as_array() + .map(|arr| { + arr.iter() + .map(|p| { + let port = p["port"].as_i64().unwrap_or(0); + let target = p["targetPort"] + .as_i64() + .or_else(|| p["targetPort"].as_str().and_then(|s| s.parse().ok())) + .unwrap_or(port); + if port == target { + format!("{}", port) + } else { + format!("{}→{}", port, target) + } + }) + .collect() + }) + .unwrap_or_default(); + format!(" {}/{} {} [{}]", ns, name, svc_type, ports.join(",")) + }) + .collect(); + + const MAX_KUBECTL_SERVICES: usize = CAP_LIST; + for line in all_lines.iter().take(MAX_KUBECTL_SERVICES) { + out.push_str(&format!("{}\n", line)); + } + if all_lines.len() > MAX_KUBECTL_SERVICES { + out.push_str(&format!(" … +{} more", all_lines.len() - MAX_KUBECTL_SERVICES)); + let all_text = all_lines.join("\n"); + if let Some(hint) = + crate::core::tee::force_tee_tail_hint(&all_text, "kubectl-services", MAX_KUBECTL_SERVICES + 1) + { + out.push_str(&format!(" {}", hint)); + } + out.push('\n'); } out } @@ -385,6 +510,7 @@ fn kubectl_logs(args: &[String], _verbose: u8) -> Result { /// Expects tab-separated lines: Name\tImage\tStatus\tPorts /// (no header row — `--format` output is headerless) pub fn format_compose_ps(raw: &str) -> String { + const MAX_COMPOSE_SERVICES: usize = CAP_LIST; let lines: Vec<&str> = raw.lines().filter(|l| !l.trim().is_empty()).collect(); if lines.is_empty() { @@ -393,16 +519,19 @@ pub fn format_compose_ps(raw: &str) -> String { let mut result = format!("[compose] {} services:\n", lines.len()); - for line in lines.iter().take(20) { - let parts: Vec<&str> = line.split('\t').collect(); - if parts.len() >= 4 { + // Pre-build all formatted lines so the tee file matches what the agent sees. + let all_formatted: Vec = lines + .iter() + .filter_map(|line| { + let parts: Vec<&str> = line.split('\t').collect(); + if parts.len() < 4 { + return None; + } let name = parts[0]; let image = parts[1]; let status = parts[2]; let ports = parts[3]; - let short_image = image.split('/').next_back().unwrap_or(image); - let port_str = if ports.trim().is_empty() { String::new() } else { @@ -413,15 +542,20 @@ pub fn format_compose_ps(raw: &str) -> String { format!(" [{}]", compact) } }; + Some(format!(" {} ({}) {}{}", name, short_image, status, port_str)) + }) + .collect(); - result.push_str(&format!( - " {} ({}) {}{}\n", - name, short_image, status, port_str - )); - } + for line in all_formatted.iter().take(MAX_COMPOSE_SERVICES) { + result.push_str(line); + result.push('\n'); } - if lines.len() > 20 { - result.push_str(&format!(" ... +{} more\n", lines.len() - 20)); + if all_formatted.len() > MAX_COMPOSE_SERVICES { + result.push_str(&format!(" … +{} more\n", all_formatted.len() - MAX_COMPOSE_SERVICES)); + let all_text = all_formatted.join("\n"); + if let Some(hint) = crate::core::tee::force_tee_tail_hint(&all_text, "compose-ps", MAX_COMPOSE_SERVICES + 1) { + result.push_str(&format!(" {}\n", hint)); + } } result.trim_end().to_string() @@ -511,7 +645,7 @@ fn compact_ports(ports: &str) -> String { port_nums.join(", ") } else { format!( - "{}, ... +{}", + "{}, … +{}", port_nums[..2].join(", "), port_nums.len() - 2 ) @@ -522,12 +656,15 @@ pub fn run_docker_passthrough(args: &[OsString], verbose: u8) -> Result { crate::core::runner::run_passthrough("docker", args, verbose) } -/// Run `docker compose ps` with compact output -pub fn run_compose_ps(verbose: u8) -> Result { +/// Run `docker compose ps` (or `docker compose ps -a`) with compact output +pub fn run_compose_ps(all: bool, verbose: u8) -> Result { let timer = tracking::TimedExecution::start(); - // Raw output for token tracking - let raw_result = exec_capture(resolved_command("docker").args(["compose", "ps"])) + let mut raw_args: Vec<&str> = vec!["compose", "ps"]; + if all { + raw_args.push("-a"); + } + let raw_result = exec_capture(resolved_command("docker").args(&raw_args)) .context("Failed to run docker compose ps")?; if !raw_result.success() { @@ -536,14 +673,13 @@ pub fn run_compose_ps(verbose: u8) -> Result { } let raw = raw_result.stdout; - // Structured output for parsing (same pattern as docker_ps) - let result = exec_capture(resolved_command("docker").args([ - "compose", - "ps", - "--format", - "{{.Name}}\t{{.Image}}\t{{.Status}}\t{{.Ports}}", - ])) - .context("Failed to run docker compose ps --format")?; + let mut format_args: Vec<&str> = vec!["compose", "ps"]; + if all { + format_args.push("-a"); + } + format_args.extend(["--format", "{{.Name}}\t{{.Image}}\t{{.Status}}\t{{.Ports}}"]); + let result = exec_capture(resolved_command("docker").args(&format_args)) + .context("Failed to run docker compose ps --format")?; if !result.success() { eprintln!("{}", result.stderr); @@ -557,13 +693,16 @@ pub fn run_compose_ps(verbose: u8) -> Result { let rtk = format_compose_ps(&structured); println!("{}", rtk); - timer.track("docker compose ps", "rtk docker compose ps", &raw, &rtk); + let label = if all { "docker compose ps -a" } else { "docker compose ps" }; + let rtk_label = if all { "rtk docker compose ps -a" } else { "rtk docker compose ps" }; + timer.track(label, rtk_label, &raw, &rtk); Ok(0) } -pub fn run_compose_logs(service: Option<&str>, verbose: u8) -> Result { +pub fn run_compose_logs(service: Option<&str>, tail: u32, verbose: u8) -> Result { let mut cmd = resolved_command("docker"); - cmd.args(["compose", "logs", "--tail", "100"]); + let tail_str = tail.to_string(); + cmd.args(["compose", "logs", "--tail", &tail_str]); if let Some(svc) = service { cmd.arg(svc); } @@ -611,6 +750,45 @@ pub fn run_compose_passthrough(args: &[OsString], verbose: u8) -> Result { crate::core::runner::run_passthrough("docker", &combined, verbose) } +pub fn run_kubectl_get(args: &[String], verbose: u8) -> Result { + match kubectl_get_target(args) { + Some(("pods", rest)) => run(ContainerCmd::KubectlPods, rest, verbose), + Some(("services", rest)) => run(ContainerCmd::KubectlServices, rest, verbose), + _ => run_kubectl_get_passthrough(args, verbose), + } +} + +fn kubectl_get_target(args: &[String]) -> Option<(&'static str, &[String])> { + let resource = args.first()?.as_str(); + let rest = &args[1..]; + if kubectl_get_requests_raw_output(rest) { + return None; + } + + match resource { + "po" | "pod" | "pods" => Some(("pods", rest)), + "svc" | "service" | "services" => Some(("services", rest)), + _ => None, + } +} + +fn kubectl_get_requests_raw_output(args: &[String]) -> bool { + args.iter().any(|arg| { + matches!( + arg.as_str(), + "-o" | "--output" | "-w" | "--watch" | "--show-labels" | "--show-kind" + ) || arg.starts_with("-o") + || arg.starts_with("--output=") + }) +} + +fn run_kubectl_get_passthrough(args: &[String], verbose: u8) -> Result { + let passthrough_args: Vec = std::iter::once(OsString::from("get")) + .chain(args.iter().map(|arg| OsString::from(arg.as_str()))) + .collect(); + run_kubectl_passthrough(&passthrough_args, verbose) +} + pub fn run_kubectl_passthrough(args: &[OsString], verbose: u8) -> Result { crate::core::runner::run_passthrough("kubectl", args, verbose) } @@ -749,6 +927,56 @@ api-1 | Connected to database"; #[test] fn test_compact_ports_many() { let result = compact_ports("0.0.0.0:80->80/tcp, 0.0.0.0:443->443/tcp, 0.0.0.0:8080->8080/tcp, 0.0.0.0:9090->9090/tcp"); - assert!(result.contains("..."), "should truncate for >3 ports"); + assert!(result.contains("…"), "should truncate for >3 ports"); + } + + #[test] + fn test_kubectl_get_target_pods_aliases() { + for resource in ["po", "pod", "pods"] { + let args = vec![resource.to_string(), "-n".to_string(), "default".to_string()]; + + assert_eq!( + kubectl_get_target(&args), + Some(("pods", &args[1..])), + "failed for {resource}" + ); + } + } + + #[test] + fn test_kubectl_get_target_services_aliases() { + for resource in ["svc", "service", "services"] { + let args = vec![resource.to_string(), "-A".to_string()]; + + assert_eq!( + kubectl_get_target(&args), + Some(("services", &args[1..])), + "failed for {resource}" + ); + } + } + + #[test] + fn test_kubectl_get_target_unsupported_resource() { + let args = vec!["deployments".to_string()]; + + assert_eq!(kubectl_get_target(&args), None); + } + + #[test] + fn test_kubectl_get_target_respects_output_flags() { + for output_flag in ["-o", "-owide", "--output", "--output=json"] { + let args = vec![ + "pods".to_string(), + output_flag.to_string(), + "wide".to_string(), + ]; + + assert_eq!( + kubectl_get_target(&args), + None, + "should pass through {output_flag}" + ); + } } } diff --git a/src/cmds/cloud/psql_cmd.rs b/src/cmds/cloud/psql_cmd.rs index 4b4e5f2c2..c250538e1 100644 --- a/src/cmds/cloud/psql_cmd.rs +++ b/src/cmds/cloud/psql_cmd.rs @@ -4,13 +4,14 @@ //! and produces compact tab-separated or key=value output. use crate::core::runner::{self, RunOptions}; +use crate::core::truncate::CAP_LIST; use crate::core::utils::resolved_command; use anyhow::Result; use lazy_static::lazy_static; use regex::Regex; -const MAX_TABLE_ROWS: usize = 30; -const MAX_EXPANDED_RECORDS: usize = 20; +const MAX_TABLE_ROWS: usize = CAP_LIST; +const MAX_EXPANDED_RECORDS: usize = CAP_LIST; lazy_static! { static ref EXPANDED_RECORD: Regex = Regex::new(r"-\[ RECORD \d+ \]-").unwrap(); @@ -250,10 +251,10 @@ mod tests { let input = lines.join("\n"); let result = filter_table(&input); - assert!(result.contains("... +10 more rows")); - // Header + 30 data rows + overflow line + assert!(result.contains("... +20 more rows")); + // Header + MAX_TABLE_ROWS data rows + overflow line let result_lines: Vec<&str> = result.lines().collect(); - assert_eq!(result_lines.len(), 32); // 1 header + 30 data + 1 overflow + assert_eq!(result_lines.len(), MAX_TABLE_ROWS + 2); // 1 header + data + 1 overflow } #[test] diff --git a/src/cmds/dotnet/dotnet_cmd.rs b/src/cmds/dotnet/dotnet_cmd.rs index 4c307ff50..d16f8bc6f 100644 --- a/src/cmds/dotnet/dotnet_cmd.rs +++ b/src/cmds/dotnet/dotnet_cmd.rs @@ -3,6 +3,7 @@ use crate::binlog; use crate::core::stream::exec_capture; use crate::core::tracking; +use crate::core::truncate::{CAP_ERRORS, CAP_LIST, CAP_WARNINGS}; use crate::core::utils::{resolved_command, truncate}; use crate::dotnet_format_report; use crate::dotnet_trx; @@ -369,7 +370,13 @@ fn format_dotnet_format_output( let mut output = format!("Format: {} files need formatting", changed_count); output.push_str("\n---------------------------------------"); - for (index, file) in summary.files_with_changes.iter().take(20).enumerate() { + const MAX_FORMAT_FILES: usize = CAP_LIST; + for (index, file) in summary + .files_with_changes + .iter() + .take(MAX_FORMAT_FILES) + .enumerate() + { let first_change = &file.changes[0]; let rule = if first_change.diagnostic_id.is_empty() { first_change.format_description.as_str() @@ -386,8 +393,21 @@ fn format_dotnet_format_output( )); } - if changed_count > 20 { - output.push_str(&format!("\n... +{} more files", changed_count - 20)); + if changed_count > MAX_FORMAT_FILES { + output.push_str(&format!("\n… +{} more files", changed_count - MAX_FORMAT_FILES)); + let all_files = summary + .files_with_changes + .iter() + .map(|f| f.path.as_str()) + .collect::>() + .join("\n"); + if let Some(hint) = crate::core::tee::force_tee_tail_hint( + &all_files, + "dotnet-format-files", + MAX_FORMAT_FILES + 1, + ) { + output.push_str(&format!(" {}", hint)); + } } output.push_str(&format!( @@ -979,31 +999,60 @@ fn format_build_output(summary: &binlog::BuildSummary, _binlog_path: &Path) -> S let status_icon = if summary.succeeded { "ok" } else { "fail" }; let duration = summary.duration_text.as_deref().unwrap_or("unknown"); + const MAX_BUILD_ERRORS: usize = CAP_ERRORS; + const MAX_BUILD_WARNINGS: usize = CAP_WARNINGS; + let mut errors = String::new(); if !summary.errors.is_empty() { errors.push_str("Errors:\n"); - for issue in summary.errors.iter().take(20) { + for issue in summary.errors.iter().take(MAX_BUILD_ERRORS) { errors.push_str(&format!("{}\n", format_issue(issue, "error"))); } - if summary.errors.len() > 20 { + if summary.errors.len() > MAX_BUILD_ERRORS { errors.push_str(&format!( - " ... +{} more errors\n", - summary.errors.len() - 20 + " … +{} more errors\n", + summary.errors.len() - MAX_BUILD_ERRORS )); + let all_errors = summary + .errors + .iter() + .map(|e| format_issue(e, "error")) + .collect::>() + .join("\n"); + if let Some(hint) = crate::core::tee::force_tee_tail_hint( + &all_errors, + "dotnet-build-errors", + MAX_BUILD_ERRORS + 1, + ) { + errors.push_str(&format!(" {}\n", hint)); + } } } let mut warnings = String::new(); if !summary.warnings.is_empty() { warnings.push_str("Warnings:\n"); - for issue in summary.warnings.iter().take(10) { + for issue in summary.warnings.iter().take(MAX_BUILD_WARNINGS) { warnings.push_str(&format!("{}\n", format_issue(issue, "warning"))); } - if summary.warnings.len() > 10 { + if summary.warnings.len() > MAX_BUILD_WARNINGS { warnings.push_str(&format!( - " ... +{} more warnings\n", - summary.warnings.len() - 10 + " … +{} more warnings\n", + summary.warnings.len() - MAX_BUILD_WARNINGS )); + let all_warnings = summary + .warnings + .iter() + .map(|w| format_issue(w, "warning")) + .collect::>() + .join("\n"); + if let Some(hint) = crate::core::tee::force_tee_tail_hint( + &all_warnings, + "dotnet-build-warnings", + MAX_BUILD_WARNINGS + 1, + ) { + warnings.push_str(&format!(" {}\n", hint)); + } } } @@ -1077,43 +1126,95 @@ fn format_test_output( ) }; + const MAX_DOTNET_FAILURES: usize = CAP_WARNINGS; let mut failed_tests_section = String::new(); if has_failures && !summary.failed_tests.is_empty() { failed_tests_section.push_str("Failed Tests:\n"); - for failed in summary.failed_tests.iter().take(15) { + for failed in summary.failed_tests.iter().take(MAX_DOTNET_FAILURES) { failed_tests_section.push_str(&format!(" {}\n", failed.name)); for detail in &failed.details { failed_tests_section.push_str(&format!(" {}\n", truncate(detail, 320))); } failed_tests_section.push('\n'); } - if summary.failed_tests.len() > 15 { + if summary.failed_tests.len() > MAX_DOTNET_FAILURES { failed_tests_section.push_str(&format!( - "... +{} more failed tests\n", - summary.failed_tests.len() - 15 + "… +{} more failed tests\n", + summary.failed_tests.len() - MAX_DOTNET_FAILURES )); + let all_failed = summary + .failed_tests + .iter() + .skip(MAX_DOTNET_FAILURES) + .map(|t| { + let mut s = t.name.clone(); + for detail in &t.details { + s.push_str(&format!("\n {}", truncate(detail, 320))); + } + s + }) + .collect::>() + .join("\n\n"); + if let Some(hint) = + crate::core::tee::force_tee_hint(&all_failed, "dotnet-test-failures") + { + failed_tests_section.push_str(&format!(" {}\n", hint)); + } } } + const MAX_TEST_ERRORS: usize = CAP_WARNINGS; + const MAX_TEST_WARNINGS: usize = CAP_WARNINGS; + let mut errors_section = String::new(); if !errors.is_empty() { errors_section.push_str("Errors:\n"); - for issue in errors.iter().take(10) { + for issue in errors.iter().take(MAX_TEST_ERRORS) { errors_section.push_str(&format!("{}\n", format_issue(issue, "error"))); } - if errors.len() > 10 { - errors_section.push_str(&format!(" ... +{} more errors\n", errors.len() - 10)); + if errors.len() > MAX_TEST_ERRORS { + errors_section.push_str(&format!( + " … +{} more errors\n", + errors.len() - MAX_TEST_ERRORS + )); + let all_errors = errors + .iter() + .map(|e| format_issue(e, "error")) + .collect::>() + .join("\n"); + if let Some(hint) = crate::core::tee::force_tee_tail_hint( + &all_errors, + "dotnet-test-errors", + MAX_TEST_ERRORS + 1, + ) { + errors_section.push_str(&format!(" {}\n", hint)); + } } } let mut warnings_section = String::new(); if !warnings.is_empty() { warnings_section.push_str("Warnings:\n"); - for issue in warnings.iter().take(10) { + for issue in warnings.iter().take(MAX_TEST_WARNINGS) { warnings_section.push_str(&format!("{}\n", format_issue(issue, "warning"))); } - if warnings.len() > 10 { - warnings_section.push_str(&format!(" ... +{} more warnings\n", warnings.len() - 10)); + if warnings.len() > MAX_TEST_WARNINGS { + warnings_section.push_str(&format!( + " … +{} more warnings\n", + warnings.len() - MAX_TEST_WARNINGS + )); + let all_warnings = warnings + .iter() + .map(|w| format_issue(w, "warning")) + .collect::>() + .join("\n"); + if let Some(hint) = crate::core::tee::force_tee_tail_hint( + &all_warnings, + "dotnet-test-warnings", + MAX_TEST_WARNINGS + 1, + ) { + warnings_section.push_str(&format!(" {}\n", hint)); + } } } @@ -1155,25 +1256,58 @@ fn format_restore_output( let status_icon = if has_errors { "fail" } else { "ok" }; let duration = summary.duration_text.as_deref().unwrap_or("unknown"); + const MAX_FORMAT_ERRORS: usize = CAP_ERRORS; + const MAX_FORMAT_WARNINGS: usize = CAP_WARNINGS; + let mut errors_section = String::new(); if !errors.is_empty() { errors_section.push_str("Errors:\n"); - for issue in errors.iter().take(20) { + for issue in errors.iter().take(MAX_FORMAT_ERRORS) { errors_section.push_str(&format!("{}\n", format_issue(issue, "error"))); } - if errors.len() > 20 { - errors_section.push_str(&format!(" ... +{} more errors\n", errors.len() - 20)); + if errors.len() > MAX_FORMAT_ERRORS { + errors_section.push_str(&format!( + " … +{} more errors\n", + errors.len() - MAX_FORMAT_ERRORS + )); + let all_errors = errors + .iter() + .map(|e| format_issue(e, "error")) + .collect::>() + .join("\n"); + if let Some(hint) = crate::core::tee::force_tee_tail_hint( + &all_errors, + "dotnet-format-errors", + MAX_FORMAT_ERRORS + 1, + ) { + errors_section.push_str(&format!(" {}\n", hint)); + } } } let mut warnings_section = String::new(); if !warnings.is_empty() { warnings_section.push_str("Warnings:\n"); - for issue in warnings.iter().take(10) { + for issue in warnings.iter().take(MAX_FORMAT_WARNINGS) { warnings_section.push_str(&format!("{}\n", format_issue(issue, "warning"))); } - if warnings.len() > 10 { - warnings_section.push_str(&format!(" ... +{} more warnings\n", warnings.len() - 10)); + if warnings.len() > MAX_FORMAT_WARNINGS { + warnings_section.push_str(&format!( + " … +{} more warnings\n", + warnings.len() - MAX_FORMAT_WARNINGS + )); + let all_warnings = warnings + .iter() + .map(|w| format_issue(w, "warning")) + .collect::>() + .join("\n"); + if let Some(hint) = crate::core::tee::force_tee_tail_hint( + &all_warnings, + "dotnet-format-warnings", + MAX_FORMAT_WARNINGS + 1, + ) { + warnings_section.push_str(&format!(" {}\n", hint)); + } } } diff --git a/src/cmds/git/README.md b/src/cmds/git/README.md index 09febe2d4..93807be3b 100644 --- a/src/cmds/git/README.md +++ b/src/cmds/git/README.md @@ -5,7 +5,7 @@ ## Specifics - **git.rs** uses `trailing_var_arg = true` + `allow_hyphen_values = true` so native git flags (`--oneline`, `--cached`, etc.) pass through correctly -- Auto-detects `--merges` flag to avoid conflicting with `--no-merges` injection +- Default `git status` uses `--porcelain -b` so the compact output never exceeds raw `git status` (an untracked directory collapses to a single line, matching git's default); branch/short-only flags reuse the compact path, other explicit args still pass through unchanged - Global git options (`-C`, `--git-dir`, `--work-tree`, `--no-pager`) are prepended before the subcommand - Exit code propagation is critical for CI/CD pipelines - **glab_cmd.rs** declares `-R`/`--repo` and `-g`/`--group` at the clap level; they are **appended** to the glab args (not prepended) so subcommand dispatch stays intact diff --git a/src/cmds/git/gh_cmd.rs b/src/cmds/git/gh_cmd.rs index 3535fd4ec..c22c16b40 100644 --- a/src/cmds/git/gh_cmd.rs +++ b/src/cmds/git/gh_cmd.rs @@ -4,6 +4,7 @@ //! Focuses on extracting essential information from JSON outputs. use crate::core::runner::{self, RunOptions}; +use crate::core::truncate::CAP_LIST; use crate::core::utils::{ok_confirmation, resolved_command, truncate}; use crate::git; use anyhow::Result; @@ -249,25 +250,27 @@ fn format_pr_list(json: &Value, ultra_compact: bool) -> String { } else { "Pull Requests\n" }); - for pr in prs.iter().take(20) { - let number = pr["number"].as_i64().unwrap_or(0); - let title = pr["title"].as_str().unwrap_or("???"); - let state = pr["state"].as_str().unwrap_or("???"); - let author = pr["author"]["login"].as_str().unwrap_or("???"); - let icon = state_icon(state, ultra_compact); - out.push_str(&format!( - " {} #{} {} ({})\n", - icon, - number, - truncate(title, 60), - author - )); - } - if prs.len() > 20 { - out.push_str(&format!( - " ... {} more (use gh pr list for all)\n", - prs.len() - 20 - )); + let all_lines: Vec = prs + .iter() + .map(|pr| { + let number = pr["number"].as_i64().unwrap_or(0); + let title = pr["title"].as_str().unwrap_or("???"); + let state = pr["state"].as_str().unwrap_or("???"); + let author = pr["author"]["login"].as_str().unwrap_or("???"); + let icon = state_icon(state, ultra_compact); + format!(" {} #{} {} ({})", icon, number, truncate(title, 60), author) + }) + .collect(); + const MAX_LIST: usize = CAP_LIST; + for line in all_lines.iter().take(MAX_LIST) { + out.push_str(&format!("{}\n", line)); + } + if all_lines.len() > MAX_LIST { + out.push_str(&format!(" … +{} more\n", all_lines.len() - MAX_LIST)); + let all_text = all_lines.join("\n"); + if let Some(hint) = crate::core::tee::force_tee_tail_hint(&all_text, "gh-prs", MAX_LIST + 1) { + out.push_str(&format!(" {}\n", hint)); + } } out } @@ -589,25 +592,32 @@ fn format_issue_list(json: &Value, ultra_compact: bool) -> String { } let mut out = String::new(); out.push_str("Issues\n"); - for issue in issues.iter().take(20) { - let number = issue["number"].as_i64().unwrap_or(0); - let title = issue["title"].as_str().unwrap_or("???"); - let state = issue["state"].as_str().unwrap_or("???"); - let icon = if ultra_compact { - if state == "OPEN" { - "O" + let all_lines: Vec = issues + .iter() + .map(|issue| { + let number = issue["number"].as_i64().unwrap_or(0); + let title = issue["title"].as_str().unwrap_or("???"); + let state = issue["state"].as_str().unwrap_or("???"); + let icon = if ultra_compact { + if state == "OPEN" { "O" } else { "C" } + } else if state == "OPEN" { + "[open]" } else { - "C" - } - } else if state == "OPEN" { - "[open]" - } else { - "[closed]" - }; - out.push_str(&format!(" {} #{} {}\n", icon, number, truncate(title, 60))); - } - if issues.len() > 20 { - out.push_str(&format!(" ... {} more\n", issues.len() - 20)); + "[closed]" + }; + format!(" {} #{} {}", icon, number, truncate(title, 60)) + }) + .collect(); + const MAX_LIST: usize = CAP_LIST; + for line in all_lines.iter().take(MAX_LIST) { + out.push_str(&format!("{}\n", line)); + } + if all_lines.len() > MAX_LIST { + out.push_str(&format!(" … +{} more\n", all_lines.len() - MAX_LIST)); + let all_text = all_lines.join("\n"); + if let Some(hint) = crate::core::tee::force_tee_tail_hint(&all_text, "gh-issues", MAX_LIST + 1) { + out.push_str(&format!(" {}\n", hint)); + } } out } diff --git a/src/cmds/git/git.rs b/src/cmds/git/git.rs index 35a56da52..b67120e3e 100644 --- a/src/cmds/git/git.rs +++ b/src/cmds/git/git.rs @@ -1,8 +1,10 @@ //! Filters git output — log, status, diff, and more — keeping just the essential info. -use crate::core::config; -use crate::core::stream::{exec_capture, CaptureResult}; +use crate::core::stream::{ + self, exec_capture, CaptureResult, FilterMode, LineHandler, LineStreamFilter, StdinMode, +}; use crate::core::tracking; +use crate::core::truncate::CAP_WARNINGS; use crate::core::utils::{exit_code_from_output, exit_code_from_status, resolved_command}; use anyhow::{Context, Result}; use std::ffi::OsString; @@ -46,6 +48,35 @@ fn git_cmd_c_locale(global_args: &[String]) -> Command { cmd } +fn uses_compact_status_path(args: &[String]) -> bool { + if args.is_empty() { + return true; + } + + let mut saw_branch = false; + for arg in args { + match arg.as_str() { + "-b" | "--branch" => saw_branch = true, + "-sb" | "-bs" => return true, + "-s" | "--short" => {} + _ => return false, + } + } + + saw_branch +} + +fn build_status_command(args: &[String], global_args: &[String]) -> Command { + let mut cmd = git_cmd(global_args); + cmd.arg("status"); + if uses_compact_status_path(args) { + cmd.args(["--porcelain", "-b"]); + } else { + cmd.args(args); + } + cmd +} + pub fn run( cmd: GitCommand, args: &[String], @@ -649,116 +680,44 @@ fn truncate_line(line: &str, width: usize) -> String { } pub(crate) fn format_status_output(porcelain: &str) -> String { - let lines: Vec<&str> = porcelain.lines().collect(); + format_status_inner(porcelain, None) +} + +pub(crate) fn format_status_output_detached(porcelain: &str, detached_ref: &str) -> String { + format_status_inner(porcelain, Some(detached_ref)) +} + +fn format_status_inner(porcelain: &str, detached: Option<&str>) -> String { + let lines: Vec<&str> = porcelain + .lines() + .filter(|line| !line.trim().is_empty()) + .collect(); if lines.is_empty() { return "Clean working tree".to_string(); } - let mut output = String::new(); + let mut output = Vec::new(); - // Parse branch info if let Some(branch_line) = lines.first() { if branch_line.starts_with("##") { let branch = branch_line.trim_start_matches("## "); - output.push_str(&format!("* {}\n", branch)); + let display = detached.unwrap_or(branch); + output.push(format!("* {}", display)); + } else { + output.push((*branch_line).to_string()); } } - // Count changes by type - let mut staged = 0; - let mut modified = 0; - let mut untracked = 0; - let mut conflicts = 0; - - let mut staged_files = Vec::new(); - let mut modified_files = Vec::new(); - let mut untracked_files = Vec::new(); - for line in lines.iter().skip(1) { - if line.len() < 3 { - continue; - } - let status = line.get(0..2).unwrap_or(" "); - let file = line.get(3..).unwrap_or(""); - - match status.chars().next().unwrap_or(' ') { - 'M' | 'A' | 'D' | 'R' | 'C' => { - staged += 1; - staged_files.push(file); - } - 'U' => conflicts += 1, - _ => {} - } - - match status.chars().nth(1).unwrap_or(' ') { - 'M' | 'D' => { - modified += 1; - modified_files.push(file); - } - _ => {} - } - - if status == "??" { - untracked += 1; - untracked_files.push(file); - } + output.push((*line).to_string()); } - // Build summary - let limits = config::limits(); - let max_files = limits.status_max_files; - let max_untracked = limits.status_max_untracked; - - if staged > 0 { - output.push_str(&format!("+ Staged: {} files\n", staged)); - for f in staged_files.iter().take(max_files) { - output.push_str(&format!(" {}\n", f)); - } - if staged_files.len() > max_files { - output.push_str(&format!( - " ... +{} more\n", - staged_files.len() - max_files - )); - } + if lines.len() == 1 && lines[0].starts_with("##") { + output.push("clean — nothing to commit".to_string()); } - if modified > 0 { - output.push_str(&format!("~ Modified: {} files\n", modified)); - for f in modified_files.iter().take(max_files) { - output.push_str(&format!(" {}\n", f)); - } - if modified_files.len() > max_files { - output.push_str(&format!( - " ... +{} more\n", - modified_files.len() - max_files - )); - } - } - - if untracked > 0 { - output.push_str(&format!("? Untracked: {} files\n", untracked)); - for f in untracked_files.iter().take(max_untracked) { - output.push_str(&format!(" {}\n", f)); - } - if untracked_files.len() > max_untracked { - output.push_str(&format!( - " ... +{} more\n", - untracked_files.len() - max_untracked - )); - } - } - - if conflicts > 0 { - output.push_str(&format!("conflicts: {} files\n", conflicts)); - } - - // When working tree is clean (only branch line, no changes) - if staged == 0 && modified == 0 && untracked == 0 && conflicts == 0 { - output.push_str("clean — nothing to commit\n"); - } - - output.trim_end().to_string() + output.join("\n") } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -859,6 +818,20 @@ fn extract_state_header(raw: &str) -> Option { None } +/// Extract the explicit "HEAD detached at/from " line from plain +/// `git status` output. +/// +/// Porcelain `-b` collapses a detached HEAD to the opaque `## HEAD (no branch)`, +/// which an agent (or a distracted human) can misread as a branch literally +/// named `HEAD`. The plain-status output keeps the explicit SHA/ref, so we +/// surface that instead. Returns `None` when HEAD is on a branch. +fn extract_detached_head(raw: &str) -> Option { + raw.lines() + .map(str::trim) + .find(|l| l.starts_with("HEAD detached ")) + .map(str::to_string) +} + /// Minimal filtering for git status with user-provided args fn filter_status_with_args(output: &str) -> String { let mut result = Vec::new(); @@ -899,10 +872,10 @@ fn filter_status_with_args(output: &str) -> String { fn run_status(args: &[String], verbose: u8, global_args: &[String]) -> Result { let timer = tracking::TimedExecution::start(); - // If user provided flags, apply minimal filtering - if !args.is_empty() { - let mut cmd = git_cmd(global_args); - cmd.arg("status").args(args); + // Keep a narrow compact path for no-arg status and branch/short-only flags. + // More complex explicit args still use the existing minimal-filter path. + if !uses_compact_status_path(args) { + let mut cmd = build_status_command(args, global_args); let result = exec_capture(&mut cmd).context("Failed to run git status")?; if !result.success() { @@ -936,26 +909,37 @@ fn run_status(args: &[String], verbose: u8, global_args: &[String]) -> Result format_status_output_detached(&result.stdout, &detached_ref), + None => format_status_output(&result.stdout), + }; // Surface in-progress state (rebase/merge/cherry-pick/bisect/am) from the // plain-status output we already captured for tracking. Porcelain omits it @@ -967,8 +951,18 @@ fn run_status(args: &[String], verbose: u8, global_args: &[String]) -> Result Result stat_cmd.args(["diff", "--cached", "--stat", "--shortstat"]); let stat_result = exec_capture(&mut stat_cmd).context("Failed to check staged files")?; + // Mirror git's own behaviour: a no-op `git add` is silent. Emitting a + // generic "ok" here is misleading — an agent can't tell "staged N files" + // from "staged nothing" when both print "ok". let compact = if stat_result.stdout.trim().is_empty() { - "ok (nothing to add)".to_string() + String::new() } else { // Parse "1 file changed, 5 insertions(+)" format let short = stat_result.stdout.lines().last().unwrap_or("").trim(); @@ -1014,7 +1011,9 @@ fn run_add(args: &[String], verbose: u8, global_args: &[String]) -> Result } }; - println!("{}", compact); + if !compact.is_empty() { + println!("{}", compact); + } timer.track( &format!("git add {}", args.join(" ")), @@ -1106,6 +1105,62 @@ fn run_commit(args: &[String], verbose: u8, global_args: &[String]) -> Result, +} + +impl LineHandler for GitPushLineHandler { + fn should_skip(&mut self, line: &str) -> bool { + if line.is_empty() { + return true; + } + let trimmed = line.trim_start(); + GIT_PUSH_NOISE_PREFIXES + .iter() + .any(|p| trimmed.starts_with(p)) + } + + fn observe_line(&mut self, line: &str) { + if line.contains("Everything up-to-date") { + self.up_to_date = true; + } + if self.pushed_ref.is_none() { + if let Some(idx) = line.find(" -> ") { + let after = &line[idx + 4..]; + if let Some(dest) = after.split_whitespace().next() { + self.pushed_ref = Some(dest.to_string()); + } + } + } + } + + fn format_summary(&self, exit_code: i32, _raw: &str) -> Option { + if exit_code != 0 { + return None; + } + let summary = if self.up_to_date { + "ok (up-to-date)".to_string() + } else if let Some(dest) = &self.pushed_ref { + format!("ok {}", dest) + } else { + "ok".to_string() + }; + Some(format!("{}\n", summary)) + } +} + fn run_push(args: &[String], verbose: u8, global_args: &[String]) -> Result { let timer = tracking::TimedExecution::start(); @@ -1119,56 +1174,23 @@ fn run_push(args: &[String], verbose: u8, global_args: &[String]) -> Result cmd.arg(arg); } - let output = cmd - .stdin(Stdio::inherit()) - .output() - .context("Failed to run git push")?; - - let stderr = String::from_utf8_lossy(&output.stderr); - let stdout = String::from_utf8_lossy(&output.stdout); - let raw = format!("{}{}", stdout, stderr); - - if output.status.success() { - let compact = if stderr.contains("Everything up-to-date") { - "ok (up-to-date)".to_string() - } else { - let mut push_info = String::new(); - for line in stderr.lines() { - if line.contains("->") { - let parts: Vec<&str> = line.split_whitespace().collect(); - if parts.len() >= 3 { - push_info = format!("ok {}", parts[parts.len() - 1]); - break; - } - } - } - if !push_info.is_empty() { - push_info - } else { - "ok".to_string() - } - }; - - println!("{}", compact); + let cmd_label = format!("git push {}", args.join(" ")); + let filter = LineStreamFilter::new(GitPushLineHandler::default()); + let result = stream::run_streaming( + &mut cmd, + StdinMode::Inherit, + FilterMode::Streaming(Box::new(filter)), + ) + .context("Failed to run git push")?; - timer.track( - &format!("git push {}", args.join(" ")), - &format!("rtk git push {}", args.join(" ")), - &raw, - &compact, - ); - } else { - eprintln!("FAILED: git push"); - if !stderr.trim().is_empty() { - eprintln!("{}", stderr); - } - if !stdout.trim().is_empty() { - eprintln!("{}", stdout); - } - return Ok(exit_code_from_output(&output, "git push")); - } + timer.track( + &cmd_label, + &format!("rtk {}", cmd_label), + &result.raw, + &result.filtered, + ); - Ok(0) + Ok(result.exit_code) } fn run_pull(args: &[String], verbose: u8, global_args: &[String]) -> Result { @@ -1450,12 +1472,16 @@ fn filter_branch_output(output: &str) -> String { .filter(|r| *r != ¤t && !local.contains(r)) .collect(); if !remote_only.is_empty() { + const MAX_REMOTE_BRANCHES: usize = CAP_WARNINGS; result.push(format!(" remote-only ({}):", remote_only.len())); - for b in remote_only.iter().take(10) { + for b in remote_only.iter().take(MAX_REMOTE_BRANCHES) { result.push(format!(" {}", b)); } - if remote_only.len() > 10 { - result.push(format!(" ... +{} more", remote_only.len() - 10)); + if remote_only.len() > MAX_REMOTE_BRANCHES { + result.push(format!( + " ... +{} more", + remote_only.len() - MAX_REMOTE_BRANCHES + )); } } } @@ -1512,9 +1538,12 @@ fn run_fetch(args: &[String], verbose: u8, global_args: &[String]) -> Result, result: &CaptureResult) -> String { match subcommand { None | Some("push") | Some("save") => { - // Create operations check for "no local changes" - if result.stdout.contains("No local changes") { - "ok (nothing to stash)".to_string() + // A successful stash collapses to "ok stashed" (the WIP ref/sha git + // prints isn't needed to `git stash pop`). But a no-op must NOT look + // like success — pass git's "No local changes to save" through so the + // agent can tell nothing was stashed. + if result.combined().contains("No local changes") { + "No local changes to save".to_string() } else { "ok stashed".to_string() } @@ -1869,6 +1898,42 @@ mod tests { assert!(envs.contains(&("LC_ALL".to_string(), "C".to_string()))); } + #[test] + fn test_build_status_command_default_compact() { + let cmd = build_status_command(&[], &[]); + let args: Vec<_> = cmd.get_args().collect(); + assert_eq!(args, vec!["status", "--porcelain", "-b"]); + } + + #[test] + fn test_uses_compact_status_path_for_branch_and_short_flags() { + assert!(uses_compact_status_path(&["-b".to_string()])); + assert!(uses_compact_status_path(&["--branch".to_string()])); + assert!(uses_compact_status_path(&["-sb".to_string()])); + assert!(uses_compact_status_path(&["-s".to_string(), "-b".to_string()])); + assert!(uses_compact_status_path(&["--short".to_string(), "--branch".to_string()])); + assert!(!uses_compact_status_path(&["-s".to_string()])); + assert!(!uses_compact_status_path(&["--short".to_string()])); + assert!(!uses_compact_status_path(&["--porcelain".to_string()])); + assert!(!uses_compact_status_path(&["-uno".to_string()])); + } + + #[test] + fn test_build_status_command_with_user_args_passthrough() { + let args = vec!["--short".to_string(), "--branch".to_string()]; + let cmd = build_status_command(&args, &[]); + let cmd_args: Vec<_> = cmd.get_args().collect(); + assert_eq!(cmd_args, vec!["status", "--porcelain", "-b"]); + } + + #[test] + fn test_build_status_command_with_incompatible_user_args_passthrough() { + let args = vec!["--porcelain".to_string(), "-uno".to_string()]; + let cmd = build_status_command(&args, &[]); + let cmd_args: Vec<_> = cmd.get_args().collect(); + assert_eq!(cmd_args, vec!["status", "--porcelain", "-uno"]); + } + #[test] fn test_compact_diff() { let diff = r#"diff --git a/foo.rs b/foo.rs @@ -2150,9 +2215,9 @@ mod tests { #[test] fn test_format_status_output_clean() { - let porcelain = ""; + let porcelain = "## main...origin/main\n"; let result = format_status_output(porcelain); - assert_eq!(result, "Clean working tree"); + assert_eq!(result, "* main...origin/main\nclean — nothing to commit"); } #[test] @@ -2224,27 +2289,17 @@ mod tests { } #[test] - fn test_format_status_output_modified_files() { - let porcelain = "## main...origin/main\n M src/main.rs\n M src/lib.rs\n"; + fn test_format_status_output_preserves_nested_untracked_paths() { + let porcelain = "## main\n?? tmp/c.txt\n?? tmp/nested/d.txt\n"; let result = format_status_output(porcelain); - assert!(result.contains("* main...origin/main")); - assert!(result.contains("~ Modified: 2 files")); - assert!(result.contains("src/main.rs")); - assert!(result.contains("src/lib.rs")); - assert!(!result.contains("Staged")); - assert!(!result.contains("Untracked")); - } - - #[test] - fn test_format_status_output_untracked_files() { - let porcelain = "## feature/new\n?? temp.txt\n?? debug.log\n?? test.sh\n"; - let result = format_status_output(porcelain); - assert!(result.contains("* feature/new")); - assert!(result.contains("? Untracked: 3 files")); - assert!(result.contains("temp.txt")); - assert!(result.contains("debug.log")); - assert!(result.contains("test.sh")); - assert!(!result.contains("Modified")); + assert!(result.contains("* main")); + assert!(result.contains("?? tmp/c.txt")); + assert!(result.contains("?? tmp/nested/d.txt")); + assert!( + result.lines().all(|line| line != "?? tmp/"), + "Nested untracked files must not collapse back to a directory marker:\n{}", + result + ); } #[test] @@ -2257,59 +2312,24 @@ A added.rs "#; let result = format_status_output(porcelain); assert!(result.contains("* main")); - assert!(result.contains("+ Staged: 2 files")); - assert!(result.contains("staged.rs")); - assert!(result.contains("added.rs")); - assert!(result.contains("~ Modified: 1 files")); - assert!(result.contains("modified.rs")); - assert!(result.contains("? Untracked: 1 files")); - assert!(result.contains("untracked.txt")); - } - - #[test] - fn test_format_status_output_truncation() { - // Test that >15 staged files show "... +N more" - let mut porcelain = String::from("## main\n"); - for i in 1..=20 { - porcelain.push_str(&format!("M file{}.rs\n", i)); - } - let result = format_status_output(&porcelain); - assert!(result.contains("+ Staged: 20 files")); - assert!(result.contains("file1.rs")); - assert!(result.contains("file15.rs")); - assert!(result.contains("... +5 more")); - assert!(!result.contains("file16.rs")); - assert!(!result.contains("file20.rs")); - } - - #[test] - fn test_format_status_modified_truncation() { - // Test that >15 modified files show "... +N more" - let mut porcelain = String::from("## main\n"); - for i in 1..=20 { - porcelain.push_str(&format!(" M file{}.rs\n", i)); - } - let result = format_status_output(&porcelain); - assert!(result.contains("~ Modified: 20 files")); - assert!(result.contains("file1.rs")); - assert!(result.contains("file15.rs")); - assert!(result.contains("... +5 more")); - assert!(!result.contains("file16.rs")); + assert!(result.contains("M staged.rs")); + assert!(result.contains(" M modified.rs")); + assert!(result.contains("A added.rs")); + assert!(result.contains("?? untracked.txt")); + assert!(!result.contains("Staged")); + assert!(!result.contains("Modified")); + assert!(!result.contains("Untracked")); } #[test] - fn test_format_status_untracked_truncation() { - // Test that >10 untracked files show "... +N more" - let mut porcelain = String::from("## main\n"); - for i in 1..=15 { - porcelain.push_str(&format!("?? file{}.rs\n", i)); - } - let result = format_status_output(&porcelain); - assert!(result.contains("? Untracked: 15 files")); - assert!(result.contains("file1.rs")); - assert!(result.contains("file10.rs")); - assert!(result.contains("... +5 more")); - assert!(!result.contains("file11.rs")); + fn test_format_status_output_preserves_rename_and_conflict_lines() { + let porcelain = "## main\nR old.rs -> new.rs\nUU conflict.rs\nMM mixed.rs\n"; + let result = format_status_output(porcelain); + assert!(result.contains("* main")); + assert!(result.contains("R old.rs -> new.rs")); + assert!(result.contains("UU conflict.rs")); + assert!(result.contains("MM mixed.rs")); + assert!(!result.contains("conflicts:")); } #[test] @@ -2719,22 +2739,25 @@ no changes added to commit (use "git add" and/or "git commit -a") // --- truncation accuracy --- #[test] - fn test_format_status_overflow_count_exact() { - // 25 staged files, default status_max_files = 15 - // Should show 15, overflow = 25 - 15 = 10, report "+10 more" + fn test_format_status_output_shows_every_file_when_many_are_dirty() { let mut porcelain = String::from("## main...origin/main\n"); for i in 0..25 { porcelain.push_str(&format!("M staged_file_{}.rs\n", i)); } let result = format_status_output(&porcelain); assert!( - result.contains("+10 more"), - "Expected '+10 more' for 25 staged files (max_files=15), got:\n{}", + result.contains("staged_file_24.rs"), + "Expected the last staged file to remain visible, got:\n{}", + result + ); + assert!( + result.lines().count() == 26, + "Expected branch + all 25 staged files, got:\n{}", result ); assert!( - result.contains("Staged: 25 files"), - "Expected 'Staged: 25 files', got:\n{}", + !result.contains("... +"), + "Status output must not hide dirty paths behind overflow markers:\n{}", result ); } @@ -2777,6 +2800,35 @@ no changes added to commit (use "git add" and/or "git commit -a") ); } + #[test] + fn test_extract_detached_head_returns_line() { + let raw = "HEAD detached at abc1234\nnothing to commit, working tree clean\n"; + assert_eq!( + extract_detached_head(raw), + Some("HEAD detached at abc1234".to_string()) + ); + } + + #[test] + fn test_extract_detached_head_on_branch_is_none() { + let raw = "On branch main\nnothing to commit, working tree clean\n"; + assert!(extract_detached_head(raw).is_none()); + } + + #[test] + fn test_format_status_output_detached_head() { + let porcelain = "## HEAD (no branch)\n M src/main.rs\n"; + let result = format_status_output_detached(porcelain, "HEAD detached at abc1234"); + assert!( + result.contains("HEAD detached at abc1234"), + "should use explicit detached ref, got: {result}" + ); + assert!( + !result.contains("HEAD (no branch)"), + "should not show opaque porcelain string, got: {result}" + ); + } + #[test] fn test_filter_log_output_body_omission_indicator() { // Commit with 6 meaningful body lines: only 3 shown, must signal "+3 lines omitted" @@ -2795,4 +2847,123 @@ no changes added to commit (use "git add" and/or "git commit -a") result ); } + + fn run_push_filter(input: &str, exit_code: i32) -> String { + use crate::core::stream::StreamFilter; + let mut f = LineStreamFilter::new(GitPushLineHandler::default()); + let mut out = String::new(); + for line in input.lines() { + if let Some(s) = f.feed_line(line) { + out.push_str(&s); + } + } + out.push_str(&f.flush()); + if let Some(s) = f.on_exit(exit_code, input) { + out.push_str(&s); + } + out + } + + #[test] + fn test_push_filter_drops_progress_phases() { + let input = "\ +Enumerating objects: 5, done. +Counting objects: 100% (5/5), done. +Delta compression using up to 8 threads +Compressing objects: 100% (3/3), done. +Writing objects: 100% (3/3), 312 bytes | 312.00 KiB/s, done. +Total 3 (delta 2), reused 0 (delta 0) +To https://github.com/foo/bar.git + abc1234..def5678 master -> master +"; + let result = run_push_filter(input, 0); + for prefix in GIT_PUSH_NOISE_PREFIXES { + assert!( + !result.contains(prefix), + "noise prefix '{}' leaked through, got: {}", + prefix, + result + ); + } + assert!(result.contains("To https://github.com/foo/bar.git")); + assert!(result.contains("master -> master")); + assert!(result.ends_with("ok master\n"), "got: {}", result); + } + + #[test] + fn test_push_filter_up_to_date_summary() { + let input = "Everything up-to-date\n"; + let result = run_push_filter(input, 0); + assert!(result.contains("Everything up-to-date")); + assert!(result.ends_with("ok (up-to-date)\n"), "got: {}", result); + } + + #[test] + fn test_push_filter_passes_remote_messages_through() { + let input = "\ +remote: Resolving deltas: 100% (2/2), completed with 2 local objects. +remote: GitHub found 1 vulnerability on foo/bar's default branch (1 moderate). +To https://github.com/foo/bar.git + abc1234..def5678 feature -> feature +"; + let result = run_push_filter(input, 0); + assert!(result.contains("remote: Resolving deltas")); + assert!(result.contains("remote: GitHub found 1 vulnerability")); + assert!(result.ends_with("ok feature\n"), "got: {}", result); + } + + #[test] + fn test_push_filter_no_summary_on_failure() { + let input = "\ +To https://github.com/foo/bar.git + ! [rejected] master -> master (non-fast-forward) +error: failed to push some refs to 'https://github.com/foo/bar.git' +"; + let result = run_push_filter(input, 1); + assert!(result.contains("[rejected]")); + assert!(result.contains("error: failed to push")); + assert!( + !result.contains("ok "), + "summary leaked on failure, got: {}", + result + ); + } + + #[test] + fn test_push_filter_first_ref_wins_for_summary() { + let input = "\ +To https://github.com/foo/bar.git + abc1234..def5678 feat/a -> feat/a + 1111111..2222222 feat/b -> feat/b +"; + let result = run_push_filter(input, 0); + assert!(result.ends_with("ok feat/a\n"), "got: {}", result); + } + + #[test] + fn test_push_filter_token_savings_on_verbose_output() { + let input = "\ +Enumerating objects: 142, done. +Counting objects: 100% (142/142), done. +Delta compression using up to 8 threads +Compressing objects: 100% (88/88), done. +Writing objects: 100% (104/104), 28.50 KiB | 14.25 MiB/s, done. +Total 104 (delta 64), reused 0 (delta 0), pack-reused 0 +remote: Resolving deltas: 100% (64/64), completed with 24 local objects. +To https://github.com/foo/bar.git + abc1234..def5678 master -> master +"; + let result = run_push_filter(input, 0); + let count_tokens = |s: &str| s.split_whitespace().count(); + let input_tokens = count_tokens(input); + let output_tokens = count_tokens(&result); + let savings = 100.0 - (output_tokens as f64 / input_tokens as f64 * 100.0); + assert!( + savings >= 60.0, + "expected >=60% savings, got {:.1}% (in={}, out={})", + savings, + input_tokens, + output_tokens + ); + } } diff --git a/src/cmds/git/glab_cmd.rs b/src/cmds/git/glab_cmd.rs index 139ed5898..a0282a221 100644 --- a/src/cmds/git/glab_cmd.rs +++ b/src/cmds/git/glab_cmd.rs @@ -12,6 +12,7 @@ use super::git; use crate::core::runner::{self, RunOptions}; +use crate::core::truncate::{CAP_LIST, CAP_WARNINGS}; use crate::core::utils::{ok_confirmation, resolved_command, strip_ansi, truncate}; use anyhow::Result; use lazy_static::lazy_static; @@ -304,27 +305,27 @@ fn format_mr_list(json: &Value, ultra_compact: bool) -> String { "Merge Requests\n" }); - for mr in mrs.iter().take(20) { - let iid = mr["iid"].as_i64().unwrap_or(0); - let title = mr["title"].as_str().unwrap_or("???"); - let state = mr["state"].as_str().unwrap_or("???"); - let author = mr["author"]["username"].as_str().unwrap_or("???"); - - let icon = state_icon(state, ultra_compact); - filtered.push_str(&format!( - " {} !{} {} ({})\n", - icon, - iid, - truncate(title, 60), - author - )); - } - - if mrs.len() > 20 { - filtered.push_str(&format!( - " ... {} more (use glab mr list for all)\n", - mrs.len() - 20 - )); + let all_lines: Vec = mrs + .iter() + .map(|mr| { + let iid = mr["iid"].as_i64().unwrap_or(0); + let title = mr["title"].as_str().unwrap_or("???"); + let state = mr["state"].as_str().unwrap_or("???"); + let author = mr["author"]["username"].as_str().unwrap_or("???"); + let icon = state_icon(state, ultra_compact); + format!(" {} !{} {} ({})", icon, iid, truncate(title, 60), author) + }) + .collect(); + const MAX_LIST: usize = CAP_LIST; + for line in all_lines.iter().take(MAX_LIST) { + filtered.push_str(&format!("{}\n", line)); + } + if all_lines.len() > MAX_LIST { + filtered.push_str(&format!(" … +{} more\n", all_lines.len() - MAX_LIST)); + let all_text = all_lines.join("\n"); + if let Some(hint) = crate::core::tee::force_tee_tail_hint(&all_text, "glab-mrs", MAX_LIST + 1) { + filtered.push_str(&format!(" {}\n", hint)); + } } filtered @@ -524,27 +525,32 @@ fn format_issue_list(json: &Value, ultra_compact: bool) -> String { let mut filtered = String::new(); filtered.push_str("Issues\n"); - for issue in issues.iter().take(20) { - let iid = issue["iid"].as_i64().unwrap_or(0); - let title = issue["title"].as_str().unwrap_or("???"); - let state = issue["state"].as_str().unwrap_or("???"); - - let icon = if ultra_compact { - if state == "opened" { - "O" + let all_lines: Vec = issues + .iter() + .map(|issue| { + let iid = issue["iid"].as_i64().unwrap_or(0); + let title = issue["title"].as_str().unwrap_or("???"); + let state = issue["state"].as_str().unwrap_or("???"); + let icon = if ultra_compact { + if state == "opened" { "O" } else { "C" } + } else if state == "opened" { + "[open]" } else { - "C" - } - } else if state == "opened" { - "[open]" - } else { - "[closed]" - }; - filtered.push_str(&format!(" {} #{} {}\n", icon, iid, truncate(title, 60))); - } - - if issues.len() > 20 { - filtered.push_str(&format!(" ... {} more\n", issues.len() - 20)); + "[closed]" + }; + format!(" {} #{} {}", icon, iid, truncate(title, 60)) + }) + .collect(); + const MAX_LIST: usize = CAP_LIST; + for line in all_lines.iter().take(MAX_LIST) { + filtered.push_str(&format!("{}\n", line)); + } + if all_lines.len() > MAX_LIST { + filtered.push_str(&format!(" … +{} more\n", all_lines.len() - MAX_LIST)); + let all_text = all_lines.join("\n"); + if let Some(hint) = crate::core::tee::force_tee_tail_hint(&all_text, "glab-issues", MAX_LIST + 1) { + filtered.push_str(&format!(" {}\n", hint)); + } } filtered @@ -646,13 +652,28 @@ fn format_ci_list(json: &Value, ultra_compact: bool) -> String { let mut filtered = String::new(); filtered.push_str("Pipelines\n"); - for pipeline in pipelines.iter().take(10) { - let id = pipeline["id"].as_i64().unwrap_or(0); - let status = pipeline["status"].as_str().unwrap_or("???"); - let ref_name = pipeline["ref"].as_str().unwrap_or("???"); - - let icon = pipeline_icon(status, ultra_compact); - filtered.push_str(&format!(" {} #{} {} ({})\n", icon, id, status, ref_name)); + let all_lines: Vec = pipelines + .iter() + .map(|pipeline| { + let id = pipeline["id"].as_i64().unwrap_or(0); + let status = pipeline["status"].as_str().unwrap_or("???"); + let ref_name = pipeline["ref"].as_str().unwrap_or("???"); + let icon = pipeline_icon(status, ultra_compact); + format!(" {} #{} {} ({})", icon, id, status, ref_name) + }) + .collect(); + const MAX_CI_LIST: usize = CAP_WARNINGS; + for line in all_lines.iter().take(MAX_CI_LIST) { + filtered.push_str(&format!("{}\n", line)); + } + if all_lines.len() > MAX_CI_LIST { + filtered.push_str(&format!(" … +{} more\n", all_lines.len() - MAX_CI_LIST)); + let all_text = all_lines.join("\n"); + if let Some(hint) = + crate::core::tee::force_tee_tail_hint(&all_text, "glab-pipelines", MAX_CI_LIST + 1) + { + filtered.push_str(&format!(" {}\n", hint)); + } } filtered } diff --git a/src/cmds/git/gt_cmd.rs b/src/cmds/git/gt_cmd.rs index bfa06fd2c..1fb3ee711 100644 --- a/src/cmds/git/gt_cmd.rs +++ b/src/cmds/git/gt_cmd.rs @@ -2,6 +2,7 @@ use crate::core::stream::exec_capture; use crate::core::tracking; +use crate::core::truncate::{reduced, CAP_LIST}; use crate::core::utils::{ok_confirmation, resolved_command, strip_ansi, truncate}; use anyhow::{Context, Result}; use lazy_static::lazy_static; @@ -168,7 +169,8 @@ fn passthrough_gt(subcommand: &str, args: &[String], verbose: u8) -> Result crate::core::runner::run_passthrough("gt", &os_args, verbose) } -const MAX_LOG_ENTRIES: usize = 15; +// gt log entries are multi-line — trim the list cap to keep token savings above 60%. +const MAX_LOG_ENTRIES: usize = reduced(CAP_LIST, 5); fn filter_gt_log_entries(input: &str) -> String { let trimmed = input.trim(); diff --git a/src/cmds/go/go_cmd.rs b/src/cmds/go/go_cmd.rs index 965a7571b..e962e3e1c 100644 --- a/src/cmds/go/go_cmd.rs +++ b/src/cmds/go/go_cmd.rs @@ -2,6 +2,7 @@ use crate::core::runner; use crate::core::tracking; +use crate::core::truncate::CAP_ERRORS; use crate::core::utils::{exit_code_from_output, resolved_command, truncate}; use crate::golangci_cmd; use anyhow::{Context, Result}; @@ -587,12 +588,17 @@ pub(crate) fn filter_go_build(output: &str) -> String { result.push_str(&format!("Go build: {} errors\n", errors.len())); result.push_str("═══════════════════════════════════════\n"); - for (i, error) in errors.iter().take(20).enumerate() { + const MAX_GO_BUILD_ERRORS: usize = CAP_ERRORS; + for (i, error) in errors.iter().take(MAX_GO_BUILD_ERRORS).enumerate() { result.push_str(&format!("{}. {}\n", i + 1, truncate(error, 120))); } - if errors.len() > 20 { - result.push_str(&format!("\n... +{} more errors\n", errors.len() - 20)); + if errors.len() > MAX_GO_BUILD_ERRORS { + result.push_str(&format!("\n… +{} more errors\n", errors.len() - MAX_GO_BUILD_ERRORS)); + let all_errors = errors.join("\n"); + if let Some(hint) = crate::core::tee::force_tee_tail_hint(&all_errors, "go-build", MAX_GO_BUILD_ERRORS + 1) { + result.push_str(&format!(" {}\n", hint)); + } } result.trim().to_string() @@ -674,12 +680,17 @@ fn filter_go_vet(output: &str) -> String { result.push_str(&format!("Go vet: {} issues\n", issues.len())); result.push_str("═══════════════════════════════════════\n"); - for (i, issue) in issues.iter().take(20).enumerate() { + const MAX_GO_VET_ISSUES: usize = CAP_ERRORS; + for (i, issue) in issues.iter().take(MAX_GO_VET_ISSUES).enumerate() { result.push_str(&format!("{}. {}\n", i + 1, truncate(issue, 120))); } - if issues.len() > 20 { - result.push_str(&format!("\n... +{} more issues\n", issues.len() - 20)); + if issues.len() > MAX_GO_VET_ISSUES { + result.push_str(&format!("\n… +{} more issues\n", issues.len() - MAX_GO_VET_ISSUES)); + let all_issues = issues.join("\n"); + if let Some(hint) = crate::core::tee::force_tee_tail_hint(&all_issues, "go-vet", MAX_GO_VET_ISSUES + 1) { + result.push_str(&format!(" {}\n", hint)); + } } result.trim().to_string() diff --git a/src/cmds/go/golangci_cmd.rs b/src/cmds/go/golangci_cmd.rs index 384ff1855..8c9ef8427 100644 --- a/src/cmds/go/golangci_cmd.rs +++ b/src/cmds/go/golangci_cmd.rs @@ -3,6 +3,7 @@ use crate::core::config; use crate::core::runner; use crate::core::stream::exec_capture; +use crate::core::truncate::CAP_WARNINGS; use crate::core::utils::{resolved_command, truncate}; use anyhow::Result; use serde::Deserialize; @@ -324,8 +325,9 @@ pub(crate) fn filter_golangci_json(output: &str, version: u32) -> String { } // Show top files + const MAX_GOLANGCI_FILES: usize = CAP_WARNINGS; result.push_str("Top files:\n"); - for (file, count) in file_counts.iter().take(10) { + for (file, count) in file_counts.iter().take(MAX_GOLANGCI_FILES) { let short_path = compact_path(file); result.push_str(&format!(" {} ({} issues)\n", short_path, count)); @@ -360,8 +362,11 @@ pub(crate) fn filter_golangci_json(output: &str, version: u32) -> String { } } - if file_counts.len() > 10 { - result.push_str(&format!("\n... +{} more files\n", file_counts.len() - 10)); + if file_counts.len() > MAX_GOLANGCI_FILES { + result.push_str(&format!( + "\n... +{} more files\n", + file_counts.len() - MAX_GOLANGCI_FILES + )); } result.trim().to_string() diff --git a/src/cmds/js/lint_cmd.rs b/src/cmds/js/lint_cmd.rs index cfb5bea86..13fa95464 100644 --- a/src/cmds/js/lint_cmd.rs +++ b/src/cmds/js/lint_cmd.rs @@ -3,6 +3,7 @@ use crate::core::config; use crate::core::stream::exec_capture; use crate::core::tracking; +use crate::core::truncate::{CAP_ERRORS, CAP_WARNINGS}; use crate::core::utils::{package_manager_exec, resolved_command, truncate}; use crate::mypy_cmd; use crate::ruff_cmd; @@ -281,30 +282,38 @@ fn filter_eslint_json(output: &str) -> String { result.push('\n'); } - // Show top files with most issues + // Show top files with most issues, plus the top rules in each + const MAX_FILES: usize = CAP_WARNINGS; result.push_str("Top files:\n"); - for (file_result, count) in by_file.iter().take(10) { + for (file_result, count) in by_file.iter().take(MAX_FILES) { let short_path = compact_path(&file_result.file_path); result.push_str(&format!(" {} ({} issues)\n", short_path, count)); - // Show top 3 rules in this file let mut file_rules: HashMap = HashMap::new(); for msg in &file_result.messages { if let Some(rule) = &msg.rule_id { *file_rules.entry(rule.clone()).or_insert(0) += 1; } } - let mut file_rule_counts: Vec<_> = file_rules.iter().collect(); file_rule_counts.sort_by(|a, b| b.1.cmp(a.1)); - for (rule, count) in file_rule_counts.iter().take(3) { result.push_str(&format!(" {} ({})\n", rule, count)); } } - if by_file.len() > 10 { - result.push_str(&format!("\n... +{} more files\n", by_file.len() - 10)); + if by_file.len() > MAX_FILES { + result.push_str(&format!("\n… +{} more files\n", by_file.len() - MAX_FILES)); + let all_file_lines = by_file + .iter() + .map(|(r, count)| format!("{} ({} issues)", compact_path(&r.file_path), count)) + .collect::>() + .join("\n"); + if let Some(hint) = + crate::core::tee::force_tee_tail_hint(&all_file_lines, "eslint-files", MAX_FILES + 1) + { + result.push_str(&format!(" {}\n", hint)); + } } result.trim().to_string() @@ -400,8 +409,9 @@ fn filter_pylint_json(output: &str) -> String { } // Show top files + const MAX_FILES: usize = CAP_WARNINGS; result.push_str("Top files:\n"); - for (file, count) in file_counts.iter().take(10) { + for (file, count) in file_counts.iter().take(MAX_FILES) { let short_path = compact_path(file); result.push_str(&format!(" {} ({} issues)\n", short_path, count)); @@ -420,8 +430,18 @@ fn filter_pylint_json(output: &str) -> String { } } - if file_counts.len() > 10 { - result.push_str(&format!("\n... +{} more files\n", file_counts.len() - 10)); + if file_counts.len() > MAX_FILES { + result.push_str(&format!("\n… +{} more files\n", file_counts.len() - MAX_FILES)); + let all_file_lines = file_counts + .iter() + .map(|(file, count)| format!("{} ({} issues)", compact_path(file), count)) + .collect::>() + .join("\n"); + if let Some(hint) = + crate::core::tee::force_tee_tail_hint(&all_file_lines, "pylint-files", MAX_FILES + 1) + { + result.push_str(&format!(" {}\n", hint)); + } } result.trim().to_string() @@ -453,12 +473,19 @@ fn filter_generic_lint(output: &str) -> String { result.push_str(&format!("Lint: {} errors, {} warnings\n", errors, warnings)); result.push_str("═══════════════════════════════════════\n"); - for issue in issues.iter().take(20) { + const MAX_ISSUES: usize = CAP_ERRORS; + for issue in issues.iter().take(MAX_ISSUES) { result.push_str(&format!("{}\n", truncate(issue, 100))); } - if issues.len() > 20 { - result.push_str(&format!("\n... +{} more issues\n", issues.len() - 20)); + if issues.len() > MAX_ISSUES { + result.push_str(&format!("\n… +{} more issues\n", issues.len() - MAX_ISSUES)); + let all_issues = issues.join("\n"); + if let Some(hint) = + crate::core::tee::force_tee_tail_hint(&all_issues, "lint-issues", MAX_ISSUES + 1) + { + result.push_str(&format!(" {}\n", hint)); + } } result.trim().to_string() diff --git a/src/cmds/js/next_cmd.rs b/src/cmds/js/next_cmd.rs index ab28f5b8b..1f15a2af0 100644 --- a/src/cmds/js/next_cmd.rs +++ b/src/cmds/js/next_cmd.rs @@ -1,6 +1,7 @@ //! Filters Next.js build output down to route metrics and bundle sizes. use crate::core::runner; +use crate::core::truncate::CAP_WARNINGS; use crate::core::utils::{resolved_command, strip_ansi, tool_exists, truncate}; use anyhow::Result; use regex::Regex; @@ -131,7 +132,8 @@ fn filter_next_build(output: &str) -> String { // Sort by size (descending) and show top 10 bundles.sort_by(|a, b| b.1.partial_cmp(&a.1).unwrap_or(std::cmp::Ordering::Equal)); - for (route, size, pct_change) in bundles.iter().take(10) { + const MAX_BUNDLES: usize = CAP_WARNINGS; + for (route, size, pct_change) in bundles.iter().take(MAX_BUNDLES) { let warning_marker = if let Some(pct) = pct_change { if *pct > 10.0 { format!(" [warn] (+{:.0}%)", pct) @@ -150,8 +152,11 @@ fn filter_next_build(output: &str) -> String { )); } - if bundles.len() > 10 { - result.push_str(&format!("\n ... +{} more routes\n", bundles.len() - 10)); + if bundles.len() > MAX_BUNDLES { + result.push_str(&format!( + "\n ... +{} more routes\n", + bundles.len() - MAX_BUNDLES + )); } result.push('\n'); diff --git a/src/cmds/js/pnpm_cmd.rs b/src/cmds/js/pnpm_cmd.rs index 53661faaf..6dd53f08c 100644 --- a/src/cmds/js/pnpm_cmd.rs +++ b/src/cmds/js/pnpm_cmd.rs @@ -2,6 +2,7 @@ use crate::core::stream::exec_capture; use crate::core::tracking; +use crate::core::truncate::CAP_LIST; use crate::core::utils::resolved_command; use anyhow::{Context, Result}; use serde::Deserialize; @@ -13,6 +14,8 @@ use crate::parser::{ DependencyState, FormatMode, OutputParser, ParseResult, TokenFormatter, }; +const MAX_LISTING: usize = CAP_LIST; + /// pnpm list JSON output structure #[derive(Debug, Deserialize)] struct PnpmListOutput { @@ -125,20 +128,32 @@ fn collect_dependencies( fn extract_list_text(output: &str) -> Option { let mut dependencies = Vec::new(); let mut count = 0; + let mut is_dev = false; for line in output.lines() { + let trimmed = line.trim(); + + if trimmed == "devDependencies:" { + is_dev = true; + continue; + } + if trimmed == "dependencies:" { + is_dev = false; + continue; + } + // Skip box-drawing and metadata if line.contains('│') || line.contains('├') || line.contains('└') || line.contains("Legend:") - || line.trim().is_empty() + || trimmed.is_empty() { continue; } // Parse lines like: "package@1.2.3" - let parts: Vec<&str> = line.split_whitespace().collect(); + let parts: Vec<&str> = trimmed.split_whitespace().collect(); if !parts.is_empty() { let pkg_str = parts[0]; if let Some(at_pos) = pkg_str.rfind('@') { @@ -150,7 +165,7 @@ fn extract_list_text(output: &str) -> Option { current_version: version.to_string(), latest_version: None, wanted_version: None, - dev_dependency: false, + dev_dependency: is_dev, }); count += 1; } @@ -270,6 +285,67 @@ fn extract_outdated_text(output: &str) -> Option { } } +/// Format a dependency listing with grouped [prod]/[dev] sections. +/// `cap = true` for plain `pnpm list` (both categories present, may truncate). +/// `cap = false` for `pnpm list --prod` / `pnpm list --dev` (hint targets, +/// must show every package so the LLM can find what was hidden by the cap). +fn format_dependency_listing(state: &DependencyState, cap: bool) -> String { + let prod: Vec<_> = state.dependencies.iter().filter(|d| !d.dev_dependency).collect(); + let dev: Vec<_> = state.dependencies.iter().filter(|d| d.dev_dependency).collect(); + let total = state.total_packages.max(state.dependencies.len()); + + let mut lines = vec![format!( + "{} packages ({} prod / {} dev)", + total, + prod.len(), + dev.len() + )]; + + if !prod.is_empty() { + lines.push("[prod]".to_string()); + let shown = if cap { prod.len().min(MAX_LISTING) } else { prod.len() }; + for dep in prod.iter().take(shown) { + lines.push(format!(" {} {}", dep.name, dep.current_version)); + } + if cap && prod.len() > MAX_LISTING { + lines.push(format!(" … +{} more", prod.len() - MAX_LISTING)); + let all_prod = prod + .iter() + .map(|dep| format!(" {} {}", dep.name, dep.current_version)) + .collect::>() + .join("\n"); + if let Some(hint) = + crate::core::tee::force_tee_tail_hint(&all_prod, "pnpm-prod", MAX_LISTING + 1) + { + lines.push(format!(" {}", hint)); + } + } + } + + if !dev.is_empty() { + lines.push("[dev]".to_string()); + let shown = if cap { dev.len().min(MAX_LISTING) } else { dev.len() }; + for dep in dev.iter().take(shown) { + lines.push(format!(" {} {}", dep.name, dep.current_version)); + } + if cap && dev.len() > MAX_LISTING { + lines.push(format!(" … +{} more", dev.len() - MAX_LISTING)); + let all_dev = dev + .iter() + .map(|dep| format!(" {} {}", dep.name, dep.current_version)) + .collect::>() + .join("\n"); + if let Some(hint) = + crate::core::tee::force_tee_tail_hint(&all_dev, "pnpm-dev", MAX_LISTING + 1) + { + lines.push(format!(" {}", hint)); + } + } + } + + lines.join("\n") +} + #[derive(Debug, Clone)] pub enum PnpmCommand { List { depth: usize }, @@ -304,22 +380,24 @@ fn run_list(depth: usize, args: &[String], verbose: u8) -> Result { return Ok(result.exit_code); } - // Parse output using PnpmListParser + let is_filtered = args + .iter() + .any(|a| matches!(a.as_str(), "--prod" | "-P" | "--dev" | "-D")); + let parse_result = PnpmListParser::parse(&result.stdout); - let mode = FormatMode::from_verbosity(verbose); let filtered = match parse_result { ParseResult::Full(data) => { if verbose > 0 { eprintln!("pnpm list (Tier 1: Full JSON parse)"); } - data.format(mode) + format_dependency_listing(&data, !is_filtered) } ParseResult::Degraded(data, warnings) => { if verbose > 0 { emit_degradation_warning("pnpm list", &warnings.join(", ")); } - data.format(mode) + format_dependency_listing(&data, !is_filtered) } ParseResult::Passthrough(raw) => { emit_passthrough_warning("pnpm list", "All parsing tiers failed"); @@ -513,4 +591,82 @@ mod tests { let _args: Vec = vec![OsString::from("help")]; // Compile-time verification that the function exists with correct signature } + + fn make_state(prod: &[&str], dev: &[&str]) -> DependencyState { + let mut deps = Vec::new(); + for name in prod { + deps.push(Dependency { + name: name.to_string(), + current_version: "1.0.0".to_string(), + latest_version: None, + wanted_version: None, + dev_dependency: false, + }); + } + for name in dev { + deps.push(Dependency { + name: name.to_string(), + current_version: "1.0.0".to_string(), + latest_version: None, + wanted_version: None, + dev_dependency: true, + }); + } + DependencyState { + total_packages: deps.len(), + outdated_count: 0, + dependencies: deps, + } + } + + #[test] + fn test_format_listing_grouped_sections() { + let state = make_state(&["react", "typescript"], &["eslint", "vitest"]); + let out = format_dependency_listing(&state, true); + assert!(out.contains("[prod]"), "prod section missing"); + assert!(out.contains("[dev]"), "dev section missing"); + assert!(out.contains("react"), "prod package missing"); + assert!(out.contains("eslint"), "dev package missing"); + assert!(!out.contains("(dev)"), "per-line (dev) marker should be gone"); + } + + #[test] + fn test_format_listing_cap_shows_hint_with_offset() { + let prod: Vec<&str> = (0..60).map(|_| "pkg").collect(); + let state = make_state(&prod, &["eslint"]); + let out = format_dependency_listing(&state, true); + let prod_count = 60usize; + assert!( + out.contains(&format!("… +{} more", prod_count - MAX_LISTING)), + "truncation count missing: got\n{out}" + ); + } + + #[test] + fn test_format_listing_no_cap_when_prod_only() { + let prod: Vec<&str> = (0..60).map(|_| "pkg").collect(); + let state = make_state(&prod, &[]); + let out = format_dependency_listing(&state, false); + assert!(!out.contains("… +"), "should not truncate when cap=false"); + assert!(!out.contains("[dev]"), "no dev section for prod-only state"); + } + + #[test] + fn test_format_listing_no_cap_when_dev_only() { + let dev: Vec<&str> = (0..60).map(|_| "pkg").collect(); + let state = make_state(&[], &dev); + let out = format_dependency_listing(&state, false); + assert!(!out.contains("… +"), "should not truncate when cap=false"); + assert!(!out.contains("[prod]"), "no prod section for dev-only state"); + } + + #[test] + fn test_extract_list_text_tracks_dev_section() { + let input = "dependencies:\nreact@18.0.0\ndevDependencies:\neslint@8.0.0\n"; + let state = extract_list_text(input).expect("should parse"); + let react = state.dependencies.iter().find(|d| d.name == "react").unwrap(); + let eslint = state.dependencies.iter().find(|d| d.name == "eslint").unwrap(); + assert!(!react.dev_dependency, "react should be prod"); + assert!(eslint.dev_dependency, "eslint should be dev"); + } } diff --git a/src/cmds/js/prettier_cmd.rs b/src/cmds/js/prettier_cmd.rs index 5a6b6c6bc..685b2f51d 100644 --- a/src/cmds/js/prettier_cmd.rs +++ b/src/cmds/js/prettier_cmd.rs @@ -1,6 +1,7 @@ //! Filters Prettier output to show only files that need formatting. use crate::core::runner::{self, RunOptions}; +use crate::core::truncate::CAP_WARNINGS; use crate::core::utils::package_manager_exec; use anyhow::Result; @@ -95,14 +96,15 @@ pub fn filter_prettier_output(output: &str) -> String { )); result.push_str("═══════════════════════════════════════\n"); - for (i, file) in files_to_format.iter().take(10).enumerate() { + const MAX_PRETTIER_FILES: usize = CAP_WARNINGS; + for (i, file) in files_to_format.iter().take(MAX_PRETTIER_FILES).enumerate() { result.push_str(&format!("{}. {}\n", i + 1, file)); } - if files_to_format.len() > 10 { + if files_to_format.len() > MAX_PRETTIER_FILES { result.push_str(&format!( "\n... +{} more files\n", - files_to_format.len() - 10 + files_to_format.len() - MAX_PRETTIER_FILES )); } diff --git a/src/cmds/jvm/gradlew_cmd.rs b/src/cmds/jvm/gradlew_cmd.rs index ad2a2987e..68d03acfe 100644 --- a/src/cmds/jvm/gradlew_cmd.rs +++ b/src/cmds/jvm/gradlew_cmd.rs @@ -1,5 +1,6 @@ use crate::core::runner::{self, RunOptions}; use crate::core::stream::StreamFilter; +use crate::core::truncate::CAP_LIST; use crate::core::utils::resolved_command; use anyhow::Result; use lazy_static::lazy_static; @@ -507,13 +508,17 @@ fn filter_dependencies(output: &str) -> String { configs.len() ); + const MAX_GRADLE_DEPS: usize = CAP_LIST; for (config, deps) in &configs { result.push_str(&format!("\n{} ({}):\n", config, deps.len())); - for dep in deps.iter().take(20) { + for dep in deps.iter().take(MAX_GRADLE_DEPS) { result.push_str(&format!(" {}\n", dep)); } - if deps.len() > 20 { - result.push_str(&format!(" ... +{} more\n", deps.len() - 20)); + if deps.len() > MAX_GRADLE_DEPS { + result.push_str(&format!( + " ... +{} more\n", + deps.len() - MAX_GRADLE_DEPS + )); } } diff --git a/src/cmds/python/pip_cmd.rs b/src/cmds/python/pip_cmd.rs index ddb56c47e..037dd58a4 100644 --- a/src/cmds/python/pip_cmd.rs +++ b/src/cmds/python/pip_cmd.rs @@ -2,6 +2,7 @@ use crate::core::stream::exec_capture; use crate::core::tracking; +use crate::core::truncate::{CAP_INVENTORY, CAP_LIST}; use crate::core::utils::{resolved_command, tool_exists}; use anyhow::{Context, Result}; use serde::Deserialize; @@ -17,12 +18,16 @@ struct Package { pub fn run(args: &[String], verbose: u8) -> Result { let timer = tracking::TimedExecution::start(); - // Auto-detect uv vs pip - let use_uv = tool_exists("uv"); + // The user ran `pip` — run `pip` so RTK stays transparent and reports the + // *same* environment the bare command would. Only fall back to `uv pip` when + // `pip` genuinely isn't on PATH (uv-only environments). Auto-substituting + // `uv pip` unconditionally made `pip list` show uv's discovered env instead + // of the active one — often just the 2-package base interpreter. + let use_uv = !tool_exists("pip") && tool_exists("uv"); let base_cmd = if use_uv { "uv" } else { "pip" }; if verbose > 0 && use_uv { - eprintln!("Using uv (pip-compatible)"); + eprintln!("pip not found — falling back to `uv pip`"); } // Detect subcommand @@ -162,16 +167,21 @@ fn filter_pip_list(output: &str) -> String { let mut letters: Vec<_> = by_letter.keys().collect(); letters.sort(); + // `pip list` is an inventory query — dependency audits need every package + // visible. The compression here is structural (drop the alignment padding, + // group by initial); the per-group cap is just a safety bound for + // pathological environments, not a normal-case truncation. + const MAX_PER_LETTER: usize = CAP_INVENTORY; for letter in letters { let pkgs = by_letter.get(letter).unwrap(); result.push_str(&format!("\n[{}]\n", letter.to_uppercase())); - for pkg in pkgs.iter().take(10) { + for pkg in pkgs.iter().take(MAX_PER_LETTER) { result.push_str(&format!(" {} ({})\n", pkg.name, pkg.version)); } - if pkgs.len() > 10 { - result.push_str(&format!(" ... +{} more\n", pkgs.len() - 10)); + if pkgs.len() > MAX_PER_LETTER { + result.push_str(&format!(" ... +{} more\n", pkgs.len() - MAX_PER_LETTER)); } } @@ -195,7 +205,8 @@ fn filter_pip_outdated(output: &str) -> String { result.push_str(&format!("pip outdated: {} packages\n", packages.len())); result.push_str("═══════════════════════════════════════\n"); - for (i, pkg) in packages.iter().take(20).enumerate() { + const MAX_PIP_PACKAGES: usize = CAP_LIST; + for (i, pkg) in packages.iter().take(MAX_PIP_PACKAGES).enumerate() { let latest = pkg.latest_version.as_deref().unwrap_or("unknown"); result.push_str(&format!( "{}. {} ({} → {})\n", @@ -206,8 +217,11 @@ fn filter_pip_outdated(output: &str) -> String { )); } - if packages.len() > 20 { - result.push_str(&format!("\n... +{} more packages\n", packages.len() - 20)); + if packages.len() > MAX_PIP_PACKAGES { + result.push_str(&format!( + "\n... +{} more packages\n", + packages.len() - MAX_PIP_PACKAGES + )); } result.push_str("\n[hint] Run `pip install --upgrade ` to update\n"); diff --git a/src/cmds/python/pytest_cmd.rs b/src/cmds/python/pytest_cmd.rs index 0b91d7daf..2febb2ea1 100644 --- a/src/cmds/python/pytest_cmd.rs +++ b/src/cmds/python/pytest_cmd.rs @@ -1,9 +1,13 @@ //! Filters pytest output to show only failures and the summary line. use crate::core::runner; +use crate::core::truncate::CAP_WARNINGS; use crate::core::utils::{resolved_command, tool_exists, truncate}; use anyhow::Result; +const MAX_XFAIL: usize = CAP_WARNINGS; +const MAX_PYTEST_FAILURES: usize = CAP_WARNINGS; + #[derive(Debug, PartialEq)] enum ParseState { Header, @@ -23,6 +27,8 @@ pub fn run(args: &[String], verbose: u8) -> Result { let has_tb_flag = args.iter().any(|a| a.starts_with("--tb")); let has_quiet_flag = args.iter().any(|a| a == "-q" || a == "--quiet"); + // Only treat a short `-r…` as pytest's report flag (not `--randomly-seed` etc.) + let has_report_flag = args.iter().any(|a| a.starts_with("-r") && !a.starts_with("--")); if !has_tb_flag { cmd.arg("--tb=short"); @@ -30,6 +36,12 @@ pub fn run(args: &[String], verbose: u8) -> Result { if !has_quiet_flag { cmd.arg("-q"); } + // Surface xfailed/xpassed (and their reasons) in the short summary section + // so the compact output can report expected failures and — crucially — + // unexpected passes (XPASS), which signal a behavior change. + if !has_report_flag { + cmd.arg("-rxX"); + } for arg in args { cmd.arg(arg); @@ -53,6 +65,7 @@ pub(crate) fn filter_pytest_output(output: &str) -> String { let mut test_files: Vec = Vec::new(); let mut failures: Vec = Vec::new(); let mut current_failure: Vec = Vec::new(); + let mut xfail_lines: Vec = Vec::new(); let mut summary_line = String::new(); for line in output.lines() { @@ -127,6 +140,8 @@ pub(crate) fn filter_pytest_output(output: &str) -> String { // FAILED test lines if trimmed.starts_with("FAILED") || trimmed.starts_with("ERROR") { failures.push(trimmed.to_string()); + } else if trimmed.starts_with("XFAIL") || trimmed.starts_with("XPASS") { + xfail_lines.push(trimmed.to_string()); } } } @@ -138,19 +153,41 @@ pub(crate) fn filter_pytest_output(output: &str) -> String { } // Build compact output - build_pytest_summary(&summary_line, &test_files, &failures) + build_pytest_summary(&summary_line, &test_files, &failures, &xfail_lines) } -fn build_pytest_summary(summary: &str, _test_files: &[String], failures: &[String]) -> String { - // Parse summary line - let (passed, failed, skipped) = parse_summary_line(summary); +#[derive(Default)] +struct PytestCounts { + passed: usize, + failed: usize, + skipped: usize, + xfailed: usize, + xpassed: usize, +} - if failed == 0 && passed > 0 { - return format!("Pytest: {} passed", passed); +fn build_pytest_summary( + summary: &str, + _test_files: &[String], + failures: &[String], + xfail_lines: &[String], +) -> String { + let counts = parse_summary_line(summary); + let PytestCounts { + passed, + failed, + skipped, + xfailed, + xpassed, + } = counts; + + if passed == 0 && failed == 0 && skipped == 0 && xfailed == 0 && xpassed == 0 { + return "Pytest: No tests collected".to_string(); } - if passed == 0 && failed == 0 && skipped == 0 { - return "Pytest: No tests collected".to_string(); + let extras_present = skipped > 0 || xfailed > 0 || xpassed > 0 || !xfail_lines.is_empty(); + + if failed == 0 && passed > 0 && !extras_present { + return format!("Pytest: {} passed", passed); } let mut result = String::new(); @@ -158,9 +195,31 @@ fn build_pytest_summary(summary: &str, _test_files: &[String], failures: &[Strin if skipped > 0 { result.push_str(&format!(", {} skipped", skipped)); } + if xfailed > 0 { + result.push_str(&format!(", {} xfailed", xfailed)); + } + if xpassed > 0 { + result.push_str(&format!(", {} xpassed", xpassed)); + } result.push('\n'); result.push_str("═══════════════════════════════════════\n"); + // Surface xfail/xpass entries (with their reasons) — XPASS in particular + // signals that something expected-to-fail now passes. + if !xfail_lines.is_empty() { + result.push_str("\nExpected-failure outcomes:\n"); + for line in xfail_lines.iter().take(MAX_XFAIL) { + result.push_str(&format!(" {}\n", truncate(line, 120))); + } + if xfail_lines.len() > MAX_XFAIL { + result.push_str(&format!(" … +{} more\n", xfail_lines.len() - MAX_XFAIL)); + let all_xfail = xfail_lines.join("\n"); + if let Some(hint) = crate::core::tee::force_tee_tail_hint(&all_xfail, "pytest-xfail", MAX_XFAIL + 1) { + result.push_str(&format!(" {}\n", hint)); + } + } + } + if failures.is_empty() { return result.trim().to_string(); } @@ -168,7 +227,7 @@ fn build_pytest_summary(summary: &str, _test_files: &[String], failures: &[Strin // Show failures (limit to key information) result.push_str("\nFailures:\n"); - for (i, failure) in failures.iter().take(5).enumerate() { + for (i, failure) in failures.iter().take(MAX_PYTEST_FAILURES).enumerate() { // Extract test name and key error info let lines: Vec<&str> = failure.lines().collect(); @@ -213,43 +272,49 @@ fn build_pytest_summary(summary: &str, _test_files: &[String], failures: &[Strin } } - if failures.len() > 5 { - result.push_str(&format!("\n... +{} more failures\n", failures.len() - 5)); + if failures.len() > MAX_PYTEST_FAILURES { + result.push_str(&format!( + "\n… +{} more failures\n", + failures.len() - MAX_PYTEST_FAILURES + )); + let all_failures = failures.join("\n\n"); + if let Some(hint) = crate::core::tee::force_tee_hint(&all_failures, "pytest-failures") { + result.push_str(&format!(" {}\n", hint)); + } } result.trim().to_string() } -fn parse_summary_line(summary: &str) -> (usize, usize, usize) { - let mut passed = 0; - let mut failed = 0; - let mut skipped = 0; - - // Parse lines like "=== 4 passed, 1 failed in 0.50s ===" - let parts: Vec<&str> = summary.split(',').collect(); +fn parse_summary_line(summary: &str) -> PytestCounts { + let mut counts = PytestCounts::default(); - for part in parts { + // Parse lines like "=== 4 passed, 1 failed, 2 xfailed, 1 xpassed in 0.50s ===" + for part in summary.split(',') { let words: Vec<&str> = part.split_whitespace().collect(); for (i, word) in words.iter().enumerate() { - if i > 0 { - if word.contains("passed") { - if let Ok(n) = words[i - 1].parse::() { - passed = n; - } - } else if word.contains("failed") { - if let Ok(n) = words[i - 1].parse::() { - failed = n; - } - } else if word.contains("skipped") { - if let Ok(n) = words[i - 1].parse::() { - skipped = n; - } - } + if i == 0 { + continue; + } + let Ok(n) = words[i - 1].parse::() else { + continue; + }; + // Order matters: "xpassed"/"xfailed" contain "passed"/"failed". + if word.contains("xpassed") { + counts.xpassed = n; + } else if word.contains("xfailed") { + counts.xfailed = n; + } else if word.contains("passed") { + counts.passed = n; + } else if word.contains("failed") { + counts.failed = n; + } else if word.contains("skipped") { + counts.skipped = n; } } } - (passed, failed, skipped) + counts } #[cfg(test)] @@ -337,15 +402,72 @@ collected 0 items #[test] fn test_parse_summary_line() { - assert_eq!(parse_summary_line("=== 5 passed in 0.50s ==="), (5, 0, 0)); + let c = parse_summary_line("=== 5 passed in 0.50s ==="); + assert_eq!((c.passed, c.failed, c.skipped), (5, 0, 0)); + + let c = parse_summary_line("=== 4 passed, 1 failed in 0.50s ==="); + assert_eq!((c.passed, c.failed, c.skipped), (4, 1, 0)); + + let c = parse_summary_line("=== 3 passed, 1 failed, 2 skipped in 1.0s ==="); + assert_eq!((c.passed, c.failed, c.skipped), (3, 1, 2)); + + let c = parse_summary_line("=== 2 passed, 1 failed, 2 xfailed, 1 xpassed in 1.0s ==="); assert_eq!( - parse_summary_line("=== 4 passed, 1 failed in 0.50s ==="), - (4, 1, 0) + (c.passed, c.failed, c.xfailed, c.xpassed), + (2, 1, 2, 1) ); - assert_eq!( - parse_summary_line("=== 3 passed, 1 failed, 2 skipped in 1.0s ==="), - (3, 1, 2) + } + + #[test] + fn test_filter_pytest_xfail_caps_and_tee_hint() { + let mut lines = String::from("=== test session starts ===\ncollected 30 items\n\n"); + lines.push_str("test_x.py "); + for _ in 0..15 { + lines.push('x'); + } + lines.push_str("\n\n=== short test summary info ===\n"); + for i in 0..15 { + lines.push_str(&format!( + "XFAIL test_x.py::test_case_{i} - known issue #{i}\n" + )); + } + lines.push_str("=== 0 passed, 15 xfailed in 0.05s ===\n"); + + let result = filter_pytest_output(&lines); + let xfail_in_section = result + .split("Expected-failure outcomes:") + .nth(1) + .unwrap_or(""); + let listed = xfail_in_section + .lines() + .filter(|l| l.trim().starts_with("XFAIL")) + .count(); + assert!( + listed <= 10, + "MAX_XFAIL cap not enforced: listed {listed}" ); + assert!(result.contains("… +5 more"), "missing '+N more': {result}"); + } + + #[test] + fn test_filter_pytest_xfail_xpass() { + let output = r#"=== test session starts === +collected 5 items + +test_math.py ..xxX [100%] + +=== short test summary info === +XFAIL test_math.py::test_division_by_zero - known bug in division +XFAIL test_math.py::test_float_precision - float precision issue — bug #42 +XPASS test_math.py::test_unexpected_pass - this should fail but currently passes +=== 2 passed, 2 xfailed, 1 xpassed in 0.05s ==="#; + + let result = filter_pytest_output(output); + assert!(result.contains("xfailed"), "got: {result}"); + assert!(result.contains("xpassed"), "got: {result}"); + assert!(result.contains("XPASS"), "got: {result}"); + assert!(result.contains("float precision"), "got: {result}"); + assert!(result.contains("test_division_by_zero"), "got: {result}"); } #[test] diff --git a/src/cmds/python/ruff_cmd.rs b/src/cmds/python/ruff_cmd.rs index b50409629..3f240df13 100644 --- a/src/cmds/python/ruff_cmd.rs +++ b/src/cmds/python/ruff_cmd.rs @@ -2,6 +2,7 @@ use crate::core::config; use crate::core::runner; +use crate::core::truncate::CAP_WARNINGS; use crate::core::utils::{resolved_command, truncate}; use anyhow::Result; use serde::Deserialize; @@ -9,9 +10,7 @@ use std::collections::HashMap; #[derive(Debug, Deserialize)] struct RuffLocation { - #[allow(dead_code)] row: usize, - #[allow(dead_code)] column: usize, } @@ -24,9 +23,7 @@ struct RuffFix { #[derive(Debug, Deserialize)] struct RuffDiagnostic { code: String, - #[allow(dead_code)] message: String, - #[allow(dead_code)] location: RuffLocation, #[allow(dead_code)] end_location: Option, @@ -153,9 +150,11 @@ pub fn filter_ruff_check_json(output: &str) -> String { let mut rule_counts: Vec<_> = by_rule.iter().collect(); rule_counts.sort_by(|a, b| b.1.cmp(a.1)); + const MAX_RUFF_RULES: usize = CAP_WARNINGS; + const MAX_RUFF_FILES: usize = CAP_WARNINGS; if !rule_counts.is_empty() { result.push_str("Top rules:\n"); - for (rule, count) in rule_counts.iter().take(10) { + for (rule, count) in rule_counts.iter().take(MAX_RUFF_RULES) { result.push_str(&format!(" {} ({}x)\n", rule, count)); } result.push('\n'); @@ -163,7 +162,7 @@ pub fn filter_ruff_check_json(output: &str) -> String { // Show top files result.push_str("Top files:\n"); - for (file, count) in file_counts.iter().take(10) { + for (file, count) in file_counts.iter().take(MAX_RUFF_FILES) { let short_path = compact_path(file); result.push_str(&format!(" {} ({} issues)\n", short_path, count)); @@ -181,8 +180,43 @@ pub fn filter_ruff_check_json(output: &str) -> String { } } - if file_counts.len() > 10 { - result.push_str(&format!("\n... +{} more files\n", file_counts.len() - 10)); + if file_counts.len() > MAX_RUFF_FILES { + result.push_str(&format!( + "\n... +{} more files\n", + file_counts.len() - MAX_RUFF_FILES + )); + } + + const MAX_VIOLATIONS: usize = 50; + let violation_lines: Vec = diagnostics + .iter() + .map(|diag| { + format!( + " {}:{}:{} {} {}\n", + compact_path(&diag.filename), + diag.location.row, + diag.location.column, + diag.code, + truncate(diag.message.trim(), 100), + ) + }) + .collect(); + + result.push_str("\nViolations:\n"); + for line in violation_lines.iter().take(MAX_VIOLATIONS) { + result.push_str(line); + } + if violation_lines.len() > MAX_VIOLATIONS { + result.push_str(&format!( + " … +{} more\n", + violation_lines.len() - MAX_VIOLATIONS + )); + let full: String = violation_lines.concat(); + if let Some(hint) = + crate::core::tee::force_tee_tail_hint(&full, "ruff-check", MAX_VIOLATIONS + 1) + { + result.push_str(&format!(" {}\n", hint)); + } } if fixable_count > 0 { @@ -256,14 +290,19 @@ pub fn filter_ruff_format(output: &str) -> String { )); result.push_str("═══════════════════════════════════════\n"); - for (i, file) in files_to_format.iter().take(10).enumerate() { + const MAX_RUFF_FORMAT_FILES: usize = CAP_WARNINGS; + for (i, file) in files_to_format + .iter() + .take(MAX_RUFF_FORMAT_FILES) + .enumerate() + { result.push_str(&format!("{}. {}\n", i + 1, compact_path(file))); } - if files_to_format.len() > 10 { + if files_to_format.len() > MAX_RUFF_FORMAT_FILES { result.push_str(&format!( "\n... +{} more files\n", - files_to_format.len() - 10 + files_to_format.len() - MAX_RUFF_FORMAT_FILES )); } @@ -346,6 +385,8 @@ mod tests { assert!(result.contains("E501")); assert!(result.contains("main.py")); assert!(result.contains("utils.py")); + assert!(result.contains("Violations:"), "Violations section missing"); + assert!(result.contains("1:8"), "line:col location missing"); } #[test] @@ -368,6 +409,39 @@ Would reformat: tests/test_utils.py assert!(result.contains("3 files already formatted")); } + #[test] + fn test_filter_ruff_check_caps_violations_and_emits_hint() { + // Mirror ruff's pretty-printed JSON shape so the input-vs-output + // comparison reflects what a real `ruff check --output-format=json` emits. + let mut diags = Vec::new(); + for i in 0..200 { + diags.push(format!( + " {{\n \"code\": \"F401\",\n \"message\": \"`module_{i}` imported but unused\",\n \"location\": {{\"row\": {i}, \"column\": 4}},\n \"end_location\": {{\"row\": {i}, \"column\": 20}},\n \"filename\": \"/Users/dev/project/src/feature_{i}.py\",\n \"fix\": null\n }}" + )); + } + let json = format!("[\n{}\n]", diags.join(",\n")); + let result = filter_ruff_check_json(&json); + + let in_section = result.split("Violations:").nth(1).unwrap_or(""); + let listed = in_section + .lines() + .filter(|l| l.trim().starts_with("src/")) + .count(); + assert!(listed <= 50, "violations cap not enforced: got {listed}"); + assert!( + result.contains("… +150 more"), + "missing '+N more' indicator" + ); + + let raw_tokens = json.split_whitespace().count(); + let out_tokens = result.split_whitespace().count(); + let savings = 100.0 - (out_tokens as f64 / raw_tokens as f64) * 100.0; + assert!( + savings >= 60.0, + "token savings dropped below 60%: {savings:.1}%" + ); + } + #[test] fn test_compact_path() { assert_eq!( diff --git a/src/cmds/ruby/rake_cmd.rs b/src/cmds/ruby/rake_cmd.rs index 76f64db75..4336b3846 100644 --- a/src/cmds/ruby/rake_cmd.rs +++ b/src/cmds/ruby/rake_cmd.rs @@ -5,9 +5,12 @@ //! Uses `ruby_exec("rake")` to auto-detect `bundle exec`. use crate::core::runner; +use crate::core::truncate::CAP_WARNINGS; use crate::core::utils::{ruby_exec, strip_ansi}; use anyhow::Result; +const MAX_RAKE_FAILURES: usize = CAP_WARNINGS; + /// Decide whether to use `rake test` or `rails test` based on args. /// /// `rake test` only supports a single file via `TEST=path` and ignores positional @@ -198,7 +201,7 @@ fn build_minitest_summary(summary: &str, failures: &[String]) -> String { result.push('\n'); - for (i, failure) in failures.iter().take(10).enumerate() { + for (i, failure) in failures.iter().take(MAX_RAKE_FAILURES).enumerate() { let lines: Vec<&str> = failure.lines().collect(); // First line is like " 1) Failure:" or " 1) Error:" if let Some(header) = lines.first() { @@ -214,13 +217,16 @@ fn build_minitest_summary(summary: &str, failures: &[String]) -> String { )); } } - if i < failures.len().min(10) - 1 { + if i < failures.len().min(MAX_RAKE_FAILURES) - 1 { result.push('\n'); } } - if failures.len() > 10 { - result.push_str(&format!("\n... +{} more failures\n", failures.len() - 10)); + if failures.len() > MAX_RAKE_FAILURES { + result.push_str(&format!( + "\n... +{} more failures\n", + failures.len() - MAX_RAKE_FAILURES + )); } result.trim().to_string() diff --git a/src/cmds/ruby/rspec_cmd.rs b/src/cmds/ruby/rspec_cmd.rs index cd909b301..80fbe1c10 100644 --- a/src/cmds/ruby/rspec_cmd.rs +++ b/src/cmds/ruby/rspec_cmd.rs @@ -6,12 +6,16 @@ //! fails to parse. use crate::core::runner; +use crate::core::truncate::{reduced, CAP_WARNINGS}; use crate::core::utils::{fallback_tail, ruby_exec, truncate}; use anyhow::Result; use lazy_static::lazy_static; use regex::Regex; use serde::Deserialize; +// rspec failures carry full backtraces — show fewer than a generic warning list. +const MAX_RSPEC_FAILURES: usize = reduced(CAP_WARNINGS, 5); + // ── Noise-stripping regex patterns ────────────────────────────────────────── lazy_static! { @@ -224,7 +228,7 @@ fn build_rspec_summary(rspec: &RspecOutput) -> String { result.push_str("\nFailures:\n"); - for (i, example) in failures.iter().take(5).enumerate() { + for (i, example) in failures.iter().take(MAX_RSPEC_FAILURES).enumerate() { result.push_str(&format!( "{}. ❌ {}\n {}:{}\n", i + 1, @@ -251,13 +255,16 @@ fn build_rspec_summary(rspec: &RspecOutput) -> String { } } - if i < failures.len().min(5) - 1 { + if i < failures.len().min(MAX_RSPEC_FAILURES) - 1 { result.push('\n'); } } - if failures.len() > 5 { - result.push_str(&format!("\n... +{} more failures\n", failures.len() - 5)); + if failures.len() > MAX_RSPEC_FAILURES { + result.push_str(&format!( + "\n... +{} more failures\n", + failures.len() - MAX_RSPEC_FAILURES + )); } result.trim().to_string() @@ -347,14 +354,17 @@ fn filter_rspec_text(output: &str) -> String { } let mut result = format!("RSpec: {}\n", summary_line); result.push_str("═══════════════════════════════════════\n\n"); - for (i, failure) in failures.iter().take(5).enumerate() { + for (i, failure) in failures.iter().take(MAX_RSPEC_FAILURES).enumerate() { result.push_str(&format!("{}. ❌ {}\n", i + 1, failure)); - if i < failures.len().min(5) - 1 { + if i < failures.len().min(MAX_RSPEC_FAILURES) - 1 { result.push('\n'); } } - if failures.len() > 5 { - result.push_str(&format!("\n... +{} more failures\n", failures.len() - 5)); + if failures.len() > MAX_RSPEC_FAILURES { + result.push_str(&format!( + "\n... +{} more failures\n", + failures.len() - MAX_RSPEC_FAILURES + )); } return result.trim().to_string(); } diff --git a/src/cmds/ruby/rubocop_cmd.rs b/src/cmds/ruby/rubocop_cmd.rs index 5ad541ca7..bed3153f7 100644 --- a/src/cmds/ruby/rubocop_cmd.rs +++ b/src/cmds/ruby/rubocop_cmd.rs @@ -184,7 +184,7 @@ fn filter_rubocop_json(output: &str) -> String { } if sorted_offenses.len() > max_offenses_per_file { result.push_str(&format!( - " ... +{} more\n", + " … +{} more\n", sorted_offenses.len() - max_offenses_per_file )); } @@ -192,9 +192,19 @@ fn filter_rubocop_json(output: &str) -> String { if files_with_offenses.len() > max_files { result.push_str(&format!( - "\n... +{} more files\n", + "\n… +{} more files\n", files_with_offenses.len() - max_files )); + let all_files = files_with_offenses + .iter() + .map(|f| compact_ruby_path(&f.path)) + .collect::>() + .join("\n"); + if let Some(hint) = + crate::core::tee::force_tee_tail_hint(&all_files, "rubocop-files", max_files + 1) + { + result.push_str(&format!(" {}\n", hint)); + } } if correctable_count > 0 { @@ -530,7 +540,7 @@ mod tests { let result = filter_rubocop_json(json); assert!(result.contains(":5 Cop/E"), "should show 5th offense"); assert!(!result.contains(":6 Cop/F"), "should not show 6th inline"); - assert!(result.contains("+2 more"), "should show overflow"); + assert!(result.contains("… +2 more"), "should show overflow"); } #[test] @@ -620,7 +630,7 @@ mod tests { ); let result = filter_rubocop_json(&json); assert!( - result.contains("+2 more files"), + result.contains("… +2 more files"), "should show +2 more files overflow: {}", result ); diff --git a/src/cmds/rust/cargo_cmd.rs b/src/cmds/rust/cargo_cmd.rs index 04f3f6324..5b8152ce0 100644 --- a/src/cmds/rust/cargo_cmd.rs +++ b/src/cmds/rust/cargo_cmd.rs @@ -2,6 +2,7 @@ use crate::core::runner; use crate::core::stream::{BlockHandler, BlockStreamFilter, StreamFilter}; +use crate::core::truncate::{CAP_ERRORS, CAP_LIST, CAP_WARNINGS}; use crate::core::utils::{resolved_command, truncate}; use anyhow::Result; use std::cmp::Ordering; @@ -527,7 +528,8 @@ fn filter_cargo_install(output: &str) -> String { } result.push_str("═══════════════════════════════════════\n"); - for (i, err) in errors.iter().enumerate().take(15) { + const MAX_INSTALL_ERRORS: usize = CAP_ERRORS; + for (i, err) in errors.iter().enumerate().take(MAX_INSTALL_ERRORS) { result.push_str(err); result.push('\n'); if i < errors.len() - 1 { @@ -535,8 +537,16 @@ fn filter_cargo_install(output: &str) -> String { } } - if errors.len() > 15 { - result.push_str(&format!("\n... +{} more issues\n", errors.len() - 15)); + if errors.len() > MAX_INSTALL_ERRORS { + result.push_str(&format!( + "\n… +{} more issues\n", + errors.len() - MAX_INSTALL_ERRORS + )); + let all_errors = errors.join("\n\n"); + if let Some(hint) = crate::core::tee::force_tee_hint(&all_errors, "cargo-build-errors") + { + result.push_str(&format!(" {}\n", hint)); + } } return result.trim().to_string(); @@ -827,15 +837,27 @@ fn filter_cargo_build(output: &str) -> String { "cargo build: {} errors, {} warnings ({} crates)\n═══════════════════════════════════════\n", handler.error_count, handler.warnings, handler.compiled ); - for (i, blk) in blocks.iter().enumerate().take(15) { + const MAX_CHECK_BLOCKS: usize = CAP_ERRORS; + for (i, blk) in blocks.iter().enumerate().take(MAX_CHECK_BLOCKS) { result.push_str(&blk.join("\n")); result.push('\n'); if i < blocks.len() - 1 { result.push('\n'); } } - if blocks.len() > 15 { - result.push_str(&format!("\n... +{} more issues\n", blocks.len() - 15)); + if blocks.len() > MAX_CHECK_BLOCKS { + result.push_str(&format!( + "\n… +{} more issues\n", + blocks.len() - MAX_CHECK_BLOCKS + )); + let all_blocks: String = blocks + .iter() + .map(|b| b.join("\n")) + .collect::>() + .join("\n\n"); + if let Some(hint) = crate::core::tee::force_tee_hint(&all_blocks, "cargo-check-issues") { + result.push_str(&format!(" {}\n", hint)); + } } result.trim().to_string() } @@ -1028,11 +1050,18 @@ pub(crate) fn filter_cargo_test(output: &str) -> String { if !failures.is_empty() { result.push_str(&format!("FAILURES ({}):\n", failures.len())); result.push_str("═══════════════════════════════════════\n"); - for (i, failure) in failures.iter().enumerate().take(10) { + const MAX_FAILURES: usize = CAP_WARNINGS; + for (i, failure) in failures.iter().enumerate().take(MAX_FAILURES) { result.push_str(&format!("{}. {}\n", i + 1, truncate(failure, 200))); } - if failures.len() > 10 { - result.push_str(&format!("\n... +{} more failures\n", failures.len() - 10)); + if failures.len() > MAX_FAILURES { + result.push_str(&format!("\n… +{} more failures\n", failures.len() - MAX_FAILURES)); + let all_failures = failures.join("\n\n"); + if let Some(hint) = + crate::core::tee::force_tee_hint(&all_failures, "cargo-test-failures") + { + result.push_str(&format!(" {}\n", hint)); + } } result.push('\n'); } @@ -1181,15 +1210,29 @@ fn filter_cargo_clippy(output: &str) -> String { // Show full error blocks so developers can see what needs fixing if !error_blocks.is_empty() { + const MAX_CLIPPY_ERRORS: usize = CAP_WARNINGS; result.push_str("\nErrors:\n"); - for block in error_blocks.iter().take(10) { + for block in error_blocks.iter().take(MAX_CLIPPY_ERRORS) { for block_line in block { result.push_str(&format!(" {}\n", truncate(block_line, 160))); } result.push('\n'); } - if error_blocks.len() > 10 { - result.push_str(&format!(" ... +{} more errors\n", error_blocks.len() - 10)); + if error_blocks.len() > MAX_CLIPPY_ERRORS { + result.push_str(&format!( + " … +{} more errors\n", + error_blocks.len() - MAX_CLIPPY_ERRORS + )); + let all_blocks: String = error_blocks + .iter() + .map(|b| b.join("\n")) + .collect::>() + .join("\n\n"); + if let Some(hint) = + crate::core::tee::force_tee_hint(&all_blocks, "cargo-clippy-errors") + { + result.push_str(&format!(" {}\n", hint)); + } } } @@ -1197,18 +1240,29 @@ fn filter_cargo_clippy(output: &str) -> String { let mut rule_counts: Vec<_> = by_rule.iter().collect(); rule_counts.sort_by_key(|b| std::cmp::Reverse(b.1.len())); - for (rule, locations) in rule_counts.iter().take(15) { + const MAX_RULES: usize = CAP_LIST; + for (rule, locations) in rule_counts.iter().take(MAX_RULES) { result.push_str(&format!(" {} ({}x)\n", rule, locations.len())); for loc in locations.iter().take(3) { result.push_str(&format!(" {}\n", loc)); } if locations.len() > 3 { - result.push_str(&format!(" ... +{} more\n", locations.len() - 3)); + result.push_str(&format!(" … +{} more\n", locations.len() - 3)); } } - if by_rule.len() > 15 { - result.push_str(&format!("\n... +{} more rules\n", by_rule.len() - 15)); + if by_rule.len() > MAX_RULES { + result.push_str(&format!("\n… +{} more rules\n", by_rule.len() - MAX_RULES)); + let all_rules = rule_counts + .iter() + .map(|(rule, locs)| format!("{} ({}x)", rule, locs.len())) + .collect::>() + .join("\n"); + if let Some(hint) = + crate::core::tee::force_tee_tail_hint(&all_rules, "cargo-clippy-rules", MAX_RULES + 1) + { + result.push_str(&format!(" {}\n", hint)); + } } result.trim().to_string() diff --git a/src/cmds/rust/runner.rs b/src/cmds/rust/runner.rs index 476f90671..8a13c1ce1 100644 --- a/src/cmds/rust/runner.rs +++ b/src/cmds/rust/runner.rs @@ -1,11 +1,15 @@ //! Runs arbitrary commands and captures only stderr or test failures. use crate::core::stream::StreamFilter; +use crate::core::truncate::{CAP_LIST, CAP_WARNINGS}; use anyhow::Result; use lazy_static::lazy_static; use regex::Regex; use std::process::Command; +const MAX_RUNNER_FAILURES: usize = CAP_WARNINGS; +const MAX_RUNNER_LINES: usize = CAP_LIST; + lazy_static! { static ref ERROR_PATTERNS: Vec = vec![ // Generic errors @@ -236,17 +240,23 @@ fn extract_test_summary(output: &str, command: &str) -> String { if !failures.is_empty() { output.push_str("[FAIL] FAILURES:\n"); - for f in failures.iter().take(10) { + for f in failures.iter().take(MAX_RUNNER_FAILURES) { output.push_str(&format!(" {}\n", f)); } - if failures.len() > 10 { - output.push_str(&format!(" ... +{} more failures\n", failures.len() - 10)); + if failures.len() > MAX_RUNNER_FAILURES { + output.push_str(&format!( + " ... +{} more failures\n", + failures.len() - MAX_RUNNER_FAILURES + )); } - for f in failure_lines.iter().take(20) { + for f in failure_lines.iter().take(MAX_RUNNER_LINES) { output.push_str(&format!(" {}\n", f.trim())); } - if failure_lines.len() > 20 { - output.push_str(&format!(" ... +{} more\n", failure_lines.len() - 20)); + if failure_lines.len() > MAX_RUNNER_LINES { + output.push_str(&format!( + " ... +{} more\n", + failure_lines.len() - MAX_RUNNER_LINES + )); } output.push('\n'); } diff --git a/src/cmds/system/constants.rs b/src/cmds/system/constants.rs index 2454f5299..4b39d54fe 100644 --- a/src/cmds/system/constants.rs +++ b/src/cmds/system/constants.rs @@ -14,8 +14,7 @@ pub const NOISE_DIRS: &[&str] = &[ ".tox", ".venv", "venv", - "env", - ".env", + "env", // Python legacy virtualenv dir — noise. .env (dotenv) is intentionally NOT here: agents must see it. "coverage", ".nyc_output", ".DS_Store", diff --git a/src/cmds/system/deps.rs b/src/cmds/system/deps.rs index 0342b2bef..3cba21139 100644 --- a/src/cmds/system/deps.rs +++ b/src/cmds/system/deps.rs @@ -1,11 +1,16 @@ //! Summarizes project dependencies from lock files and manifests. use crate::core::tracking; +use crate::core::truncate::{reduced, CAP_WARNINGS}; use anyhow::Result; use regex::Regex; use std::fs; use std::path::Path; +const MAX_DEPS: usize = CAP_WARNINGS; +// dev deps are secondary to prod — show fewer. +const MAX_DEV_DEPS: usize = reduced(CAP_WARNINGS, 5); + /// Summarize project dependencies pub fn run(path: &Path, verbose: u8) -> Result<()> { let timer = tracking::TimedExecution::start(); @@ -107,20 +112,20 @@ fn summarize_cargo_str(path: &Path) -> Result { if !deps.is_empty() { out.push_str(&format!(" Dependencies ({}):\n", deps.len())); - for d in deps.iter().take(10) { + for d in deps.iter().take(MAX_DEPS) { out.push_str(&format!(" {}\n", d)); } - if deps.len() > 10 { - out.push_str(&format!(" ... +{} more\n", deps.len() - 10)); + if deps.len() > MAX_DEPS { + out.push_str(&format!(" ... +{} more\n", deps.len() - MAX_DEPS)); } } if !dev_deps.is_empty() { out.push_str(&format!(" Dev ({}):\n", dev_deps.len())); - for d in dev_deps.iter().take(5) { + for d in dev_deps.iter().take(MAX_DEV_DEPS) { out.push_str(&format!(" {}\n", d)); } - if dev_deps.len() > 5 { - out.push_str(&format!(" ... +{} more\n", dev_deps.len() - 5)); + if dev_deps.len() > MAX_DEV_DEPS { + out.push_str(&format!(" ... +{} more\n", dev_deps.len() - MAX_DEV_DEPS)); } } Ok(out) @@ -138,8 +143,8 @@ fn summarize_package_json_str(path: &Path) -> Result { if let Some(deps) = json.get("dependencies").and_then(|v| v.as_object()) { out.push_str(&format!(" Dependencies ({}):\n", deps.len())); for (i, (name, version)) in deps.iter().enumerate() { - if i >= 10 { - out.push_str(&format!(" ... +{} more\n", deps.len() - 10)); + if i >= MAX_DEPS { + out.push_str(&format!(" ... +{} more\n", deps.len() - MAX_DEPS)); break; } out.push_str(&format!( @@ -152,8 +157,8 @@ fn summarize_package_json_str(path: &Path) -> Result { if let Some(dev_deps) = json.get("devDependencies").and_then(|v| v.as_object()) { out.push_str(&format!(" Dev Dependencies ({}):\n", dev_deps.len())); for (i, (name, _)) in dev_deps.iter().enumerate() { - if i >= 5 { - out.push_str(&format!(" ... +{} more\n", dev_deps.len() - 5)); + if i >= MAX_DEV_DEPS { + out.push_str(&format!(" ... +{} more\n", dev_deps.len() - MAX_DEV_DEPS)); break; } out.push_str(&format!(" {}\n", name)); @@ -181,11 +186,11 @@ fn summarize_requirements_str(path: &Path) -> Result { } out.push_str(&format!(" Packages ({}):\n", deps.len())); - for d in deps.iter().take(15) { + for d in deps.iter().take(MAX_DEPS) { out.push_str(&format!(" {}\n", d)); } - if deps.len() > 15 { - out.push_str(&format!(" ... +{} more\n", deps.len() - 15)); + if deps.len() > MAX_DEPS { + out.push_str(&format!(" ... +{} more\n", deps.len() - MAX_DEPS)); } Ok(out) } @@ -216,11 +221,11 @@ fn summarize_pyproject_str(path: &Path) -> Result { if !deps.is_empty() { out.push_str(&format!(" Dependencies ({}):\n", deps.len())); - for d in deps.iter().take(10) { + for d in deps.iter().take(MAX_DEPS) { out.push_str(&format!(" {}\n", d)); } - if deps.len() > 10 { - out.push_str(&format!(" ... +{} more\n", deps.len() - 10)); + if deps.len() > MAX_DEPS { + out.push_str(&format!(" ... +{} more\n", deps.len() - MAX_DEPS)); } } Ok(out) @@ -259,11 +264,11 @@ fn summarize_gomod_str(path: &Path) -> Result { } if !deps.is_empty() { out.push_str(&format!(" Dependencies ({}):\n", deps.len())); - for d in deps.iter().take(10) { + for d in deps.iter().take(MAX_DEPS) { out.push_str(&format!(" {}\n", d)); } - if deps.len() > 10 { - out.push_str(&format!(" ... +{} more\n", deps.len() - 10)); + if deps.len() > MAX_DEPS { + out.push_str(&format!(" ... +{} more\n", deps.len() - MAX_DEPS)); } } Ok(out) diff --git a/src/cmds/system/env_cmd.rs b/src/cmds/system/env_cmd.rs index 72b20166a..f131b2634 100644 --- a/src/cmds/system/env_cmd.rs +++ b/src/cmds/system/env_cmd.rs @@ -1,6 +1,7 @@ //! Filters environment variables, hiding secrets and noise. use crate::core::tracking; +use crate::core::truncate::{CAP_LIST, CAP_WARNINGS}; use anyhow::Result; use std::collections::HashSet; use std::env; @@ -71,11 +72,12 @@ pub fn run(filter: Option<&str>, show_all: bool, verbose: u8) -> Result<()> { // Split PATH for readability let paths: Vec<&str> = v.split(':').collect(); println!(" PATH ({} entries):", paths.len()); - for p in paths.iter().take(5) { + const MAX_PATH_ENTRIES: usize = CAP_WARNINGS; + for p in paths.iter().take(MAX_PATH_ENTRIES) { println!(" {}", p); } - if paths.len() > 5 { - println!(" ... +{} more", paths.len() - 5); + if paths.len() > MAX_PATH_ENTRIES { + println!(" ... +{} more", paths.len() - MAX_PATH_ENTRIES); } } else { println!(" {}={}", k, v); @@ -105,12 +107,13 @@ pub fn run(filter: Option<&str>, show_all: bool, verbose: u8) -> Result<()> { } if !other_vars.is_empty() { + const MAX_OTHER_VARS: usize = CAP_LIST; println!("\nOther:"); - for (k, v) in other_vars.iter().take(20) { + for (k, v) in other_vars.iter().take(MAX_OTHER_VARS) { println!(" {}={}", k, v); } - if other_vars.len() > 20 { - println!(" ... +{} more", other_vars.len() - 20); + if other_vars.len() > MAX_OTHER_VARS { + println!(" ... +{} more", other_vars.len() - MAX_OTHER_VARS); } } diff --git a/src/cmds/system/format_cmd.rs b/src/cmds/system/format_cmd.rs index 6e37c1596..c71dd1072 100644 --- a/src/cmds/system/format_cmd.rs +++ b/src/cmds/system/format_cmd.rs @@ -2,6 +2,7 @@ use crate::core::stream::exec_capture; use crate::core::tracking; +use crate::core::truncate::CAP_WARNINGS; use crate::core::utils::{package_manager_exec, resolved_command}; use crate::prettier_cmd; use crate::ruff_cmd; @@ -237,14 +238,15 @@ fn filter_black_output(output: &str) -> String { result.push_str("═══════════════════════════════════════\n"); if !files_to_format.is_empty() { - for (i, file) in files_to_format.iter().take(10).enumerate() { + const MAX_FORMAT_FILES: usize = CAP_WARNINGS; + for (i, file) in files_to_format.iter().take(MAX_FORMAT_FILES).enumerate() { result.push_str(&format!("{}. {}\n", i + 1, compact_path(file))); } - if files_to_format.len() > 10 { + if files_to_format.len() > MAX_FORMAT_FILES { result.push_str(&format!( "\n... +{} more files\n", - files_to_format.len() - 10 + files_to_format.len() - MAX_FORMAT_FILES )); } } diff --git a/src/cmds/system/log_cmd.rs b/src/cmds/system/log_cmd.rs index fd9942d0b..7c765f5e6 100644 --- a/src/cmds/system/log_cmd.rs +++ b/src/cmds/system/log_cmd.rs @@ -1,6 +1,7 @@ //! Deduplicates repeated log lines and shows counts instead. use crate::core::tracking; +use crate::core::truncate::{reduced, CAP_WARNINGS}; use anyhow::Result; use lazy_static::lazy_static; use regex::Regex; @@ -81,17 +82,24 @@ fn analyze_logs(content: &str) -> String { let normalized = normalize_log_line(line, &TIMESTAMP_RE, &UUID_RE, &HEX_RE, &NUM_RE, &PATH_RE); - // Categorize + // Categorize. The error bucket also covers severity labels above ERROR + // (CRITICAL, FATAL, ALERT, EMERGENCY, SEVERE, PANIC) — these are the most + // important lines in a log and were previously dropped as noise when they + // didn't literally contain "error". if line_lower.contains("error") || line_lower.contains("fatal") || line_lower.contains("panic") + || line_lower.contains("critical") + || line_lower.contains("alert") + || line_lower.contains("emerg") + || line_lower.contains("severe") { let count = error_counts.entry(normalized.clone()).or_insert(0); if *count == 0 { unique_errors.push(line.to_string()); } *count += 1; - } else if line_lower.contains("warn") { + } else if line_lower.contains("warn") || line_lower.contains("notice") { let count = warn_counts.entry(normalized.clone()).or_insert(0); if *count == 0 { unique_warnings.push(line.to_string()); @@ -129,7 +137,8 @@ fn analyze_logs(content: &str) -> String { let mut error_list: Vec<_> = error_counts.iter().collect(); error_list.sort_by(|a, b| b.1.cmp(a.1)); - for (normalized, count) in error_list.iter().take(10) { + const MAX_LOG_ERRORS: usize = CAP_WARNINGS; + for (normalized, count) in error_list.iter().take(MAX_LOG_ERRORS) { // Find original message let original = unique_errors .iter() @@ -154,10 +163,10 @@ fn analyze_logs(content: &str) -> String { } } - if error_list.len() > 10 { + if error_list.len() > MAX_LOG_ERRORS { result.push(format!( " ... +{} more unique errors", - error_list.len() - 10 + error_list.len() - MAX_LOG_ERRORS )); } result.push(String::new()); @@ -170,7 +179,9 @@ fn analyze_logs(content: &str) -> String { let mut warn_list: Vec<_> = warn_counts.iter().collect(); warn_list.sort_by(|a, b| b.1.cmp(a.1)); - for (normalized, count) in warn_list.iter().take(5) { + // warnings are lower severity than errors — show fewer. + const MAX_LOG_WARNS: usize = reduced(CAP_WARNINGS, 5); + for (normalized, count) in warn_list.iter().take(MAX_LOG_WARNS) { let original = unique_warnings .iter() .find(|w| { @@ -194,10 +205,10 @@ fn analyze_logs(content: &str) -> String { } } - if warn_list.len() > 5 { + if warn_list.len() > MAX_LOG_WARNS { result.push(format!( " ... +{} more unique warnings", - warn_list.len() - 5 + warn_list.len() - MAX_LOG_WARNS )); } } @@ -239,6 +250,18 @@ mod tests { assert!(result.contains("ERRORS")); } + #[test] + fn test_analyze_logs_extended_severity_keywords() { + let logs = "2024-01-01 10:00:00 CRITICAL: disk full\n\ + 2024-01-01 10:00:01 ALERT: memory pressure\n\ + 2024-01-01 10:00:02 emerg: system shutdown imminent\n\ + 2024-01-01 10:00:03 SEVERE: data corruption detected\n\ + 2024-01-01 10:00:04 notice: config reloaded\n"; + let result = analyze_logs(logs); + assert!(result.contains("ERRORS"), "critical/alert/emerg/severe should count as errors"); + assert!(result.contains("WARNINGS"), "notice should count as warning"); + } + #[test] fn test_analyze_logs_multibyte() { let logs = format!( diff --git a/src/cmds/system/ls.rs b/src/cmds/system/ls.rs index 537e8272f..b96f84876 100644 --- a/src/cmds/system/ls.rs +++ b/src/cmds/system/ls.rs @@ -2,6 +2,7 @@ use super::constants::NOISE_DIRS; use crate::core::runner::{self, RunOptions}; +use crate::core::truncate::{reduced, CAP_WARNINGS}; use crate::core::utils::resolved_command; use anyhow::Result; use lazy_static::lazy_static; @@ -252,17 +253,19 @@ fn compact_ls(raw: &str, show_all: bool) -> (String, String, usize) { // Summary line (separate so caller can suppress when piped) let mut summary = format!("\nSummary: {} files, {} dirs", files.len(), dirs.len()); if !by_ext.is_empty() { + // inline single-line summary — fewer entries to avoid wrapping. + const MAX_EXT_SUMMARY: usize = reduced(CAP_WARNINGS, 5); let mut ext_counts: Vec<_> = by_ext.iter().collect(); ext_counts.sort_by(|a, b| b.1.cmp(a.1)); let ext_parts: Vec = ext_counts .iter() - .take(5) + .take(MAX_EXT_SUMMARY) .map(|(ext, count)| format!("{} {}", count, ext)) .collect(); summary.push_str(" ("); summary.push_str(&ext_parts.join(", ")); - if ext_counts.len() > 5 { - summary.push_str(&format!(", +{} more", ext_counts.len() - 5)); + if ext_counts.len() > MAX_EXT_SUMMARY { + summary.push_str(&format!(", +{} more", ext_counts.len() - MAX_EXT_SUMMARY)); } summary.push(')'); } diff --git a/src/cmds/system/pipe_cmd.rs b/src/cmds/system/pipe_cmd.rs index fe569a597..6dcc4cdb6 100644 --- a/src/cmds/system/pipe_cmd.rs +++ b/src/cmds/system/pipe_cmd.rs @@ -2,6 +2,11 @@ use anyhow::Result; use std::io::Read; use crate::core::stream::RAW_CAP; +use crate::core::truncate::{CAP_LIST, CAP_WARNINGS}; + +const MAX_PIPE_MATCHES: usize = CAP_WARNINGS; +const MAX_PIPE_FILES: usize = CAP_WARNINGS; +const MAX_PIPE_DIRS: usize = CAP_LIST; pub fn resolve_filter(name: &str) -> Option String> { match name { @@ -15,7 +20,8 @@ pub fn resolve_filter(name: &str) -> Option String> { "find" | "fd" => Some(find_wrapper), "git-log" => Some(git_log_wrapper), "git-diff" => Some(git_diff_wrapper), - "git-status" => Some(crate::cmds::git::git::format_status_output), + "git-status" => Some(git_status_wrapper), + "log" => Some(crate::cmds::system::log_cmd::run_stdin_str), "mypy" => Some(crate::cmds::python::mypy_cmd::filter_mypy_output), "ruff-check" => Some(crate::cmds::python::ruff_cmd::filter_ruff_check_json), "ruff-format" => Some(crate::cmds::python::ruff_cmd::filter_ruff_format), @@ -28,6 +34,10 @@ fn go_test_wrapper(input: &str) -> String { crate::cmds::go::go_cmd::filter_go_test_json(input) } +fn git_status_wrapper(input: &str) -> String { + crate::cmds::git::git::format_status_output(input) +} + fn git_log_wrapper(input: &str) -> String { crate::cmds::git::git::filter_log_output(input, 50, false, false) } @@ -73,11 +83,11 @@ fn grep_wrapper(input: &str) -> String { for (file, matches) in files { out.push_str(&format!("[file] {} ({}):\n", file, matches.len())); - for (line_num, content) in matches.iter().take(10) { + for (line_num, content) in matches.iter().take(MAX_PIPE_MATCHES) { out.push_str(&format!(" {:>4}: {}\n", line_num, content.trim())); } - if matches.len() > 10 { - out.push_str(&format!(" +{}\n", matches.len() - 10)); + if matches.len() > MAX_PIPE_MATCHES { + out.push_str(&format!(" +{}\n", matches.len() - MAX_PIPE_MATCHES)); } out.push('\n'); } @@ -112,18 +122,18 @@ fn find_wrapper(input: &str) -> String { let mut dirs: Vec<_> = by_dir.iter().collect(); dirs.sort_by_key(|(d, _)| *d); - for (dir, files) in dirs.iter().take(20) { + for (dir, files) in dirs.iter().take(MAX_PIPE_DIRS) { out.push_str(&format!("{}/ ({})\n", dir, files.len())); - for f in files.iter().take(10) { + for f in files.iter().take(MAX_PIPE_FILES) { out.push_str(&format!(" {}\n", f)); } - if files.len() > 10 { - out.push_str(&format!(" +{}\n", files.len() - 10)); + if files.len() > MAX_PIPE_FILES { + out.push_str(&format!(" +{}\n", files.len() - MAX_PIPE_FILES)); } } - if dirs.len() > 20 { - out.push_str(&format!("\n+{} more dirs\n", dirs.len() - 20)); + if dirs.len() > MAX_PIPE_DIRS { + out.push_str(&format!("\n+{} more dirs\n", dirs.len() - MAX_PIPE_DIRS)); } out @@ -220,7 +230,7 @@ pub fn run(filter_name: Option<&str>, passthrough: bool) -> Result<()> { anyhow::anyhow!( "Unknown filter '{}'. Available: cargo-test, pytest, go-test, go-build, \ tsc, vitest, grep, rg, find, fd, git-log, git-diff, git-status, \ - mypy, ruff-check, ruff-format, prettier", + log, mypy, ruff-check, ruff-format, prettier", name ) })?, @@ -300,6 +310,11 @@ mod tests { assert!(resolve_filter("git-status").is_some()); } + #[test] + fn test_resolve_filter_log() { + assert!(resolve_filter("log").is_some()); + } + #[test] fn test_resolve_filter_unknown_returns_none() { assert!(resolve_filter("nonexistent-filter").is_none()); diff --git a/src/cmds/system/summary.rs b/src/cmds/system/summary.rs index c6e2ec051..31de537ea 100644 --- a/src/cmds/system/summary.rs +++ b/src/cmds/system/summary.rs @@ -2,11 +2,15 @@ use crate::core::stream::exec_capture; use crate::core::tracking; +use crate::core::truncate::CAP_WARNINGS; use crate::core::utils::truncate; use anyhow::{Context, Result}; use regex::Regex; use std::process::Command; +const MAX_SUMMARY_LIST: usize = CAP_WARNINGS; +const MAX_SUMMARY_KEYS: usize = CAP_WARNINGS; + /// Run a command and provide a heuristic summary pub fn run(command: &str, verbose: u8) -> Result { let timer = tracking::TimedExecution::start(); @@ -229,11 +233,11 @@ fn summarize_list(output: &str, result: &mut Vec) { let lines: Vec<&str> = output.lines().filter(|l| !l.trim().is_empty()).collect(); result.push(format!("List ({} items):", lines.len())); - for line in lines.iter().take(10) { + for line in lines.iter().take(MAX_SUMMARY_LIST) { result.push(format!(" • {}", truncate(line, 70))); } - if lines.len() > 10 { - result.push(format!(" ... +{} more", lines.len() - 10)); + if lines.len() > MAX_SUMMARY_LIST { + result.push(format!(" ... +{} more", lines.len() - MAX_SUMMARY_LIST)); } } @@ -248,11 +252,11 @@ fn summarize_json(output: &str, result: &mut Vec) { } serde_json::Value::Object(obj) => { result.push(format!(" Object with {} keys:", obj.len())); - for key in obj.keys().take(10) { + for key in obj.keys().take(MAX_SUMMARY_KEYS) { result.push(format!(" • {}", key)); } - if obj.len() > 10 { - result.push(format!(" ... +{} more keys", obj.len() - 10)); + if obj.len() > MAX_SUMMARY_KEYS { + result.push(format!(" ... +{} more keys", obj.len() - MAX_SUMMARY_KEYS)); } } _ => { diff --git a/src/cmds/system/wc_cmd.rs b/src/cmds/system/wc_cmd.rs index dcfe98b39..78c392441 100644 --- a/src/cmds/system/wc_cmd.rs +++ b/src/cmds/system/wc_cmd.rs @@ -21,12 +21,22 @@ pub fn run(args: &[String], verbose: u8) -> Result { } let mode = detect_mode(args); + + // No file operands → wc reads from stdin. Forward rtk's stdin to the child + // so `cat file | rtk wc` counts the piped data instead of reporting zero. + let reads_stdin = !args.iter().any(|a| !a.starts_with('-')); + let opts = if reads_stdin { + RunOptions::stdout_only().inherit_stdin() + } else { + RunOptions::stdout_only() + }; + runner::run_filtered( cmd, "wc", &args.join(" "), |stdout| filter_wc_output(stdout, &mode), - RunOptions::stdout_only(), + opts, ) } diff --git a/src/core/README.md b/src/core/README.md index ca031a15a..5d6f8e5ca 100644 --- a/src/core/README.md +++ b/src/core/README.md @@ -121,5 +121,15 @@ Consumers that parse structured output (JSON, NDJSON, state machines) should cal For truncation recovery on **success** (e.g., list truncated at 20 items), use `tee::force_tee_hint()` which bypasses the tee mode check and writes regardless of exit code. This ensures LLMs always have a `[full output: ...]` recovery path instead of burning tokens working around missing data. +When the truncated output is a **flat list** and the hidden items start at a predictable line, prefer `tee::force_tee_tail_hint(content, slug, offset)`. It writes the same tee file but emits a directly runnable hint — `[see remaining: tail -n +{offset} ~/path]` — so the agent jumps to exactly the first hidden item without scanning the whole file. The offset is `header_lines + MAX_CAP + 1`. Use `force_tee_hint` instead when the output has multiple sections (e.g. running + stopped containers) and no single offset cleanly covers the gap. + +### Truncation Caps (`truncate`) + +`src/core/truncate.rs` defines four global cap policies — `CAP_ERRORS`, `CAP_WARNINGS`, `CAP_LIST`, `CAP_INVENTORY` — for the data classes RTK filters truncate. Each filter binds the right CAP to a local `const MAX_*` so the cap is one named jump away from the call site. These CAPs are the staging point for filter-level cap configuration (planned, not yet implemented): once the config surface lands, overriding `CAP_LIST` in `~/.config/rtk/config.toml` will tune every list filter in one place instead of editing 20+ files. + +**Config policy.** Configured values are accepted as-is, including `0`, which means "summary only" — the filter still prints the count and the `[full output: …]` recovery hint, just no individual items. Caps are never refused and rtk never aborts on them, in keeping with the never-block-the-user fallback philosophy. + +**Deviating from a cap.** A filter whose items are unusually verbose (multi-line entries, backtraces) may show fewer than its class cap. Use `truncate::reduced(cap, by)` rather than a bare `cap - by`: `reduced` returns `cap - by`, except when the reduction would empty the list (`by >= cap`), in which case it drops the deviation and uses the full `cap`. This guarantees a deviation can never hide every item, and — crucially — stays a `usize`-underflow-safe `const fn` once caps become runtime-configurable (a bare `CAP_WARNINGS - 5` would panic or wrap to "no truncation" if a user set `CAP_WARNINGS` below `5`). Never deviate with a bare literal or with `*`/`/` (those scale unboundedly). Each deviation needs a one-line comment stating why. + ## Adding New Functionality Place new infrastructure code here if it meets **all** of these criteria: (1) it has no dependencies on command modules or hooks, (2) it is used by two or more other modules, and (3) it provides a general-purpose utility rather than command-specific logic. Follow the existing pattern of lazy-initialized resources (`lazy_static!` for regex, on-demand config loading) to preserve the <10ms startup target. Add `#[cfg(test)] mod tests` with unit tests in the same file. diff --git a/src/core/mod.rs b/src/core/mod.rs index 01317e942..fe017d910 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -11,4 +11,5 @@ pub mod telemetry; pub mod telemetry_cmd; pub mod toml_filter; pub mod tracking; +pub mod truncate; pub mod utils; diff --git a/src/core/runner.rs b/src/core/runner.rs index f127a6081..17f0997da 100644 --- a/src/core/runner.rs +++ b/src/core/runner.rs @@ -20,6 +20,10 @@ pub struct RunOptions<'a> { pub filter_stdout_only: bool, pub skip_filter_on_failure: bool, pub no_trailing_newline: bool, + /// Forward rtk's own stdin to the child process. Needed for commands that + /// can read from a pipe (e.g. `cat file | rtk wc`); without it the child + /// gets an empty stdin and reports zero. + pub inherit_stdin: bool, } impl<'a> RunOptions<'a> { @@ -51,6 +55,11 @@ impl<'a> RunOptions<'a> { self.no_trailing_newline = true; self } + + pub fn inherit_stdin(mut self) -> Self { + self.inherit_stdin = true; + self + } } pub enum RunMode<'a> { @@ -71,7 +80,12 @@ pub fn run( match mode { RunMode::Filtered(filter_fn) => { - let result = stream::run_streaming(&mut cmd, StdinMode::Null, FilterMode::CaptureOnly) + let stdin_mode = if opts.inherit_stdin { + StdinMode::Inherit + } else { + StdinMode::Null + }; + let result = stream::run_streaming(&mut cmd, stdin_mode, FilterMode::CaptureOnly) .with_context(|| format!("Failed to run {}", tool_name))?; let exit_code = result.exit_code; diff --git a/src/core/stream.rs b/src/core/stream.rs index f651e9dde..02a6cfc79 100644 --- a/src/core/stream.rs +++ b/src/core/stream.rs @@ -85,6 +85,49 @@ impl StreamFilter for BlockStreamFilter { } } +/// Counterpart to [`BlockHandler`] for line-oriented streams. +/// +/// Default behaviour is KEEP — every line is emitted unchanged. Implementors +/// opt in to dropping noise via [`LineHandler::should_skip`] and may capture +/// state for the final summary via [`LineHandler::observe_line`]. +pub trait LineHandler { + fn should_skip(&mut self, _line: &str) -> bool { + false + } + + fn observe_line(&mut self, _line: &str) {} + + fn format_summary(&self, exit_code: i32, raw: &str) -> Option; +} + +pub struct LineStreamFilter { + handler: H, +} + +impl LineStreamFilter { + pub fn new(handler: H) -> Self { + Self { handler } + } +} + +impl StreamFilter for LineStreamFilter { + fn feed_line(&mut self, line: &str) -> Option { + if self.handler.should_skip(line) { + return None; + } + self.handler.observe_line(line); + Some(format!("{}\n", line)) + } + + fn flush(&mut self) -> String { + String::new() + } + + fn on_exit(&mut self, exit_code: i32, raw: &str) -> Option { + self.handler.format_summary(exit_code, raw) + } +} + #[cfg(test)] // available for command modules; currently used in tests only pub struct RegexBlockFilter { start_re: Regex, @@ -981,4 +1024,97 @@ pub(crate) mod tests { "raw_stderr should capture all stderr lines" ); } + + struct CountingLineHandler { + observed: Vec, + skip_prefixes: Vec, + summary_tag: &'static str, + } + + impl LineHandler for CountingLineHandler { + fn should_skip(&mut self, line: &str) -> bool { + self.skip_prefixes.iter().any(|p| line.starts_with(p)) + } + + fn observe_line(&mut self, line: &str) { + self.observed.push(line.to_string()); + } + + fn format_summary(&self, exit_code: i32, _raw: &str) -> Option { + Some(format!( + "{}: {} kept, exit={}\n", + self.summary_tag, + self.observed.len(), + exit_code + )) + } + } + + fn run_line_filter(filter: &mut dyn StreamFilter, input: &str, exit_code: i32) -> String { + let mut out = String::new(); + for line in input.lines() { + if let Some(s) = filter.feed_line(line) { + out.push_str(&s); + } + } + out.push_str(&filter.flush()); + if let Some(post) = filter.on_exit(exit_code, input) { + out.push_str(&post); + } + out + } + + #[test] + fn test_line_filter_defaults_keep_all() { + struct DefaultHandler; + impl LineHandler for DefaultHandler { + fn format_summary(&self, _: i32, _: &str) -> Option { + None + } + } + let mut f = LineStreamFilter::new(DefaultHandler); + let result = run_line_filter(&mut f, "a\nb\nc\n", 0); + assert_eq!(result, "a\nb\nc\n"); + } + + #[test] + fn test_line_filter_skip_drops_matching_lines() { + let handler = CountingLineHandler { + observed: Vec::new(), + skip_prefixes: vec!["NOISE:".to_string()], + summary_tag: "demo", + }; + let mut f = LineStreamFilter::new(handler); + let input = "NOISE: progress 10%\nkeep me\nNOISE: progress 90%\nalso keep\n"; + let result = run_line_filter(&mut f, input, 0); + assert!(!result.contains("NOISE:"), "got: {}", result); + assert!(result.contains("keep me\n")); + assert!(result.contains("also keep\n")); + assert!(result.contains("demo: 2 kept, exit=0\n")); + } + + #[test] + fn test_line_filter_summary_propagates_exit_code() { + let handler = CountingLineHandler { + observed: Vec::new(), + skip_prefixes: Vec::new(), + summary_tag: "demo", + }; + let mut f = LineStreamFilter::new(handler); + let result = run_line_filter(&mut f, "one\n", 42); + assert!(result.contains("exit=42"), "got: {}", result); + } + + #[test] + fn test_line_filter_observe_only_called_for_kept_lines() { + let handler = CountingLineHandler { + observed: Vec::new(), + skip_prefixes: vec!["DROP".to_string()], + summary_tag: "demo", + }; + let mut f = LineStreamFilter::new(handler); + let result = run_line_filter(&mut f, "DROP a\nDROP b\nkeep\n", 0); + // Only "keep" was observed, so summary says "1 kept" + assert!(result.contains("demo: 1 kept"), "got: {}", result); + } } diff --git a/src/core/tee.rs b/src/core/tee.rs index c67ea8349..0b01bcae3 100644 --- a/src/core/tee.rs +++ b/src/core/tee.rs @@ -168,19 +168,17 @@ pub fn tee_raw(raw: &str, command_slug: &str, exit_code: i32) -> Option ) } -/// Format the hint line with ~ shorthand for home directory. -fn format_hint(path: &std::path::Path) -> String { - let display = if let Some(home) = dirs::home_dir() { +fn display_path(path: &std::path::Path) -> String { + if let Some(home) = dirs::home_dir() { if let Ok(relative) = path.strip_prefix(&home) { - format!("~/{}", relative.display()) - } else { - path.display().to_string() + return format!("~/{}", relative.display()); } - } else { - path.display().to_string() - }; + } + path.display().to_string() +} - format!("[full output: {}]", display) +fn format_hint(path: &std::path::Path) -> String { + format!("[full output: {}]", display_path(path)) } /// Convenience: tee + format hint in one call. @@ -190,26 +188,17 @@ pub fn tee_and_hint(raw: &str, command_slug: &str, exit_code: i32) -> Option= MIN_TEE_SIZE and tee is enabled. -/// Returns hint string if file was written, None if skipped/disabled. -/// -/// Used by AWS filters when FilterResult.truncated = true, ensuring -/// the LLM has access to full untruncated output via the hint path. -pub fn force_tee_hint(raw: &str, command_slug: &str) -> Option { - // Check RTK_TEE=0 env override (disable) +fn force_tee_path(content: &str, command_slug: &str) -> Option { if std::env::var("RTK_TEE").ok().as_deref() == Some("0") { return None; } - // Skip if output too small - if raw.len() < MIN_TEE_SIZE { + if content.is_empty() { return None; } let config = Config::load().ok()?; - // Respect enabled flag but ignore mode (force tee) if !config.tee.enabled { return None; } @@ -217,17 +206,35 @@ pub fn force_tee_hint(raw: &str, command_slug: &str) -> Option { let tee_dir = get_tee_dir(&config)?; let tee_dir = std::fs::create_dir_all(&tee_dir).ok().and(Some(tee_dir))?; - let path = write_tee_file( - raw, + write_tee_file( + content, command_slug, &tee_dir, config.tee.max_file_size, config.tee.max_files, - )?; + ) +} +/// Returns `[full output: ~/path]`, or None if tee is disabled/skipped. +pub fn force_tee_hint(raw: &str, command_slug: &str) -> Option { + let path = force_tee_path(raw, command_slug)?; Some(format_hint(&path)) } +/// Returns `[see remaining: tail -n +{line_offset} ~/path]`, or None if tee is disabled/skipped. +pub fn force_tee_tail_hint( + content: &str, + command_slug: &str, + line_offset: usize, +) -> Option { + let path = force_tee_path(content, command_slug)?; + Some(format!( + "[see remaining: tail -n +{} {}]", + line_offset, + display_path(&path) + )) +} + /// TeeMode controls when tee writes files. #[derive(Debug, Clone, serde::Serialize, serde::Deserialize, PartialEq, Default)] #[serde(rename_all = "lowercase")] @@ -487,11 +494,9 @@ directory = "/tmp/rtk-tee" } #[test] - fn test_force_tee_hint_skip_small_output() { - // force_tee_hint should respect MIN_TEE_SIZE - let small_output = "short error"; - let hint = force_tee_hint(small_output, "test_cmd"); - assert!(hint.is_none(), "Should skip output < MIN_TEE_SIZE"); + fn test_force_tee_hint_skip_empty() { + let hint = force_tee_hint("", "test_cmd"); + assert!(hint.is_none(), "Should skip empty content"); } #[test] @@ -503,4 +508,20 @@ directory = "/tmp/rtk-tee" std::env::remove_var("RTK_TEE"); assert!(hint.is_none(), "Should respect RTK_TEE=0"); } + + #[test] + fn test_force_tee_tail_hint_skip_empty() { + let hint = force_tee_tail_hint("", "test_cmd", 22); + assert!(hint.is_none(), "Should skip empty content"); + } + + #[test] + fn test_force_tee_tail_hint_format() { + let path = std::path::PathBuf::from("/tmp/rtk/tee/123_docker_images.log"); + let display = display_path(&path); + let hint = format!("[see remaining: tail -n +{} {}]", 22, display); + assert!(hint.starts_with("[see remaining: tail -n +22 ")); + assert!(hint.ends_with(']')); + assert!(hint.contains("123_docker_images.log")); + } } diff --git a/src/core/truncate.rs b/src/core/truncate.rs new file mode 100644 index 000000000..c1776cdcb --- /dev/null +++ b/src/core/truncate.rs @@ -0,0 +1,64 @@ +//! Global truncation caps shared by every filter. See `src/core/README.md` +//! ("Truncation Caps") for the cap classes, config policy, and deviation rules. + +/// Errors: most actionable, shown the most. +pub const CAP_ERRORS: usize = 20; +/// Warnings and test failures: lower signal density than errors. +pub const CAP_WARNINGS: usize = 10; +/// Flat lists (PRs, services, packages): one line per item. +pub const CAP_LIST: usize = 20; +/// Inventories (`pip list`, `docker images`): exhaustive lookups. +pub const CAP_INVENTORY: usize = 50; + +/// A cap reduced for a verbose data class. Falls back to `cap` when `by >= cap` +/// so a deviation can never empty the list; `0` stays `0`. `const fn`, underflow-safe. +pub const fn reduced(cap: usize, by: usize) -> usize { + if by < cap { + cap - by + } else { + cap + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn reduced_preserves_current_values() { + assert_eq!(reduced(CAP_WARNINGS, 5), 5); + assert_eq!(reduced(CAP_LIST, 5), 15); + } + + #[test] + fn reduced_falls_back_to_cap_when_offset_too_large() { + assert_eq!(reduced(4, 5), 4); + assert_eq!(reduced(5, 5), 5); + } + + #[test] + fn reduced_honors_zero_cap() { + assert_eq!(reduced(0, 5), 0); + } + + // Sweep every plausible (cap, by) a future config could produce and assert + // the invariants that make caps safe: the result never wraps past `cap`, and + // the offset never empties a non-zero cap. `usize::MAX` covers a wraparound bug. + #[test] + fn reduced_is_underflow_safe_across_all_inputs() { + for cap in 0..=64usize { + for by in [0usize, 1, 5, 10, 64, usize::MAX] { + let r = reduced(cap, by); + assert!(r <= cap, "reduced({cap}, {by}) = {r} exceeds cap (wrapped)"); + if cap == 0 { + assert_eq!(r, 0, "zero cap must stay zero"); + } else { + assert!(r >= 1, "reduced({cap}, {by}) = {r} emptied a non-zero cap"); + } + if by < cap { + assert_eq!(r, cap - by, "exact deviation must be preserved"); + } + } + } + } +} diff --git a/src/core/utils.rs b/src/core/utils.rs index a73eba9bc..a3bc84fe0 100644 --- a/src/core/utils.rs +++ b/src/core/utils.rs @@ -147,7 +147,7 @@ pub fn format_cpt(cpt: f64) -> String { pub fn join_with_overflow(items: &[String], total: usize, max: usize, label: &str) -> String { let mut out = items.join("\n"); if total > max { - out.push_str(&format!("\n... +{} more {}", total - max, label)); + out.push_str(&format!("\n… +{} more {}", total - max, label)); } out } diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 21da7c748..9af21dc57 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -1482,72 +1482,22 @@ fn run_claude_md_mode(global: bool, install_opencode: bool, ctx: InitContext) -> eprintln!("Writing rtk instructions to: {}", path.display()); } - if path.exists() { - let existing = fs::read_to_string(&path)?; - // upsert_rtk_block handles all 4 cases: add, update, unchanged, malformed - let (new_content, action) = upsert_rtk_block(&existing, RTK_INSTRUCTIONS); - - match action { - RtkBlockUpsert::Added => { - if dry_run { - println!("[dry-run] would add rtk instructions to {}", path.display()); - } else { - fs::write(&path, new_content)?; - println!("[ok] Added rtk instructions to existing {}", path.display()); - } - } - RtkBlockUpsert::Updated => { - if dry_run { - println!( - "[dry-run] would update rtk instructions in {}", - path.display() - ); - } else { - fs::write(&path, new_content)?; - println!("[ok] Updated rtk instructions in {}", path.display()); - } - } - RtkBlockUpsert::Unchanged => { - if !dry_run { - println!( - "[ok] {} already contains up-to-date rtk instructions", - path.display() - ); - } - return Ok(()); - } - RtkBlockUpsert::Malformed => { - eprintln!( - "[warn] Warning: Found '{}' without closing marker in {}", - RTK_BLOCK_START, - path.display() - ); + let recovery_cmd = if global { + "rtk init -g --claude-md" + } else { + "rtk init --claude-md" + }; - if let Some((line_num, _)) = existing - .lines() - .enumerate() - .find(|(_, line)| line.contains(RTK_BLOCK_START)) - { - eprintln!(" Location: line {}", line_num + 1); - } + let action = write_rtk_block( + &path, + RTK_INSTRUCTIONS, + "rtk instructions", + recovery_cmd, + ctx, + )?; - eprintln!(" Action: Manually remove the incomplete block, then re-run:"); - if global { - eprintln!(" rtk init -g --claude-md"); - } else { - eprintln!(" rtk init --claude-md"); - } - return Ok(()); - } - } - } else if dry_run { - println!( - "[dry-run] would create {} with rtk instructions", - path.display() - ); - } else { - fs::write(&path, RTK_INSTRUCTIONS)?; - println!("[ok] Created {} with rtk instructions", path.display()); + if matches!(action, RtkBlockUpsert::Unchanged) { + return Ok(()); } if global { @@ -2416,6 +2366,85 @@ fn upsert_rtk_block(content: &str, block: &str) -> (String, RtkBlockUpsert) { } } +/// Idempotently write an RTK-owned marker block into `path`, preserving user content. +/// +/// Reads the file (if any), passes it through [`upsert_rtk_block`], and writes the +/// result back via [`atomic_write`]. Refuses to modify files containing an opening +/// marker without a matching closing marker (bails with a diagnostic and the exact +/// `recovery_cmd` to re-run after manual cleanup). +/// +/// Returns the [`RtkBlockUpsert`] action so callers can branch on whether anything +/// was actually changed (e.g., to skip post-install steps on `Unchanged`). +/// +/// `label` is shown in user-facing messages (e.g., `"rtk instructions"`, +/// `"Copilot instructions"`). +fn write_rtk_block( + path: &Path, + block: &str, + label: &str, + recovery_cmd: &str, + ctx: InitContext, +) -> Result { + let InitContext { dry_run, .. } = ctx; + + let existing = if path.exists() { + fs::read_to_string(path).with_context(|| format!("Failed to read {}", path.display()))? + } else { + String::new() + }; + + let (new_content, action) = upsert_rtk_block(&existing, block); + + match action { + RtkBlockUpsert::Added => { + if dry_run { + println!("[dry-run] would add {} to {}", label, path.display()); + } else { + atomic_write(path, &new_content) + .with_context(|| format!("Failed to write {}", path.display()))?; + println!("[ok] Added {} to {}", label, path.display()); + } + } + RtkBlockUpsert::Updated => { + if dry_run { + println!("[dry-run] would update {} in {}", label, path.display()); + } else { + atomic_write(path, &new_content) + .with_context(|| format!("Failed to write {}", path.display()))?; + println!("[ok] Updated {} in {}", label, path.display()); + } + } + RtkBlockUpsert::Unchanged => { + if !dry_run { + println!("[ok] {} already up to date in {}", label, path.display()); + } + } + RtkBlockUpsert::Malformed => { + eprintln!( + "[warn] Found '{}' without closing marker in {}", + RTK_BLOCK_START, + path.display() + ); + if let Some((line_num, _)) = existing + .lines() + .enumerate() + .find(|(_, line)| line.contains(RTK_BLOCK_START)) + { + eprintln!(" Location: line {}", line_num + 1); + } + eprintln!(" Action: Manually remove the incomplete block, then re-run:"); + eprintln!(" {recovery_cmd}"); + anyhow::bail!( + "Refusing to modify malformed {} at {}", + label, + path.display() + ); + } + } + + Ok(action) +} + /// Patch CLAUDE.md: add @RTK.md, migrate if old block exists fn patch_claude_md(path: &Path, ctx: InitContext) -> Result { let InitContext { verbose, dry_run } = ctx; @@ -3682,7 +3711,8 @@ const COPILOT_HOOK_JSON: &str = r#"{ } "#; -const COPILOT_INSTRUCTIONS: &str = r#"# RTK — Token-Optimized CLI +const COPILOT_INSTRUCTIONS: &str = r#" +# RTK — Token-Optimized CLI **rtk** is a CLI proxy that filters and compresses command outputs, saving 60-90% tokens. @@ -3707,32 +3737,46 @@ rtk gain --history # Per-command savings history rtk discover # Find missed rtk opportunities rtk proxy # Run raw (no filtering) but track usage ``` + "#; -/// Entry point for `rtk init --copilot` +/// Entry point for `rtk init --copilot`. +/// +/// Installs in the current working directory's `.github/` subdirectory. pub fn run_copilot(ctx: InitContext) -> Result<()> { + run_copilot_at(Path::new("."), ctx) +} + +/// Same as [`run_copilot`] but operates relative to an explicit base path. +/// +/// Used by tests to avoid mutating process-global `cwd` (which is racy under +/// `cargo test`'s default parallel execution). +fn run_copilot_at(base: &Path, ctx: InitContext) -> Result<()> { let InitContext { dry_run, .. } = ctx; - // Install in current project's .github/ directory - let github_dir = Path::new(".github"); + let github_dir = base.join(".github"); let hooks_dir = github_dir.join("hooks"); if !dry_run { - fs::create_dir_all(&hooks_dir).context("Failed to create .github/hooks/ directory")?; + fs::create_dir_all(&hooks_dir) + .with_context(|| format!("Failed to create {} directory", hooks_dir.display()))?; } - // 1. Write hook config - let hook_path = hooks_dir.join("rtk-rewrite.json"); - write_if_changed(&hook_path, COPILOT_HOOK_JSON, "Copilot hook config", ctx)?; - - // 2. Write instructions + // 1. Upsert RTK marker block in copilot-instructions.md (preserves user content). + // Done BEFORE writing the hook config so a malformed file aborts the install + // without leaving a stale hook on disk. let instructions_path = github_dir.join("copilot-instructions.md"); - write_if_changed( + write_rtk_block( &instructions_path, COPILOT_INSTRUCTIONS, "Copilot instructions", + "rtk init --copilot", ctx, )?; + // 2. Write hook config (only reached if the upsert above succeeded). + let hook_path = hooks_dir.join("rtk-rewrite.json"); + write_if_changed(&hook_path, COPILOT_HOOK_JSON, "Copilot hook config", ctx)?; + if dry_run { print_dry_run_footer(); } else { @@ -5634,4 +5678,208 @@ mod tests { "RTK end marker must be removed" ); } + + #[test] + fn test_copilot_init_preserves_existing_instructions() { + let temp = TempDir::new().unwrap(); + let github_dir = temp.path().join(".github"); + fs::create_dir_all(&github_dir).unwrap(); + + let instructions_path = github_dir.join("copilot-instructions.md"); + let user_content = "# My Copilot Instructions\n\n\ + Always respond in Spanish.\n\ + Never suggest npm; prefer pnpm.\n"; + fs::write(&instructions_path, user_content).unwrap(); + + run_copilot_at(temp.path(), InitContext::default()).unwrap(); + + let final_content = fs::read_to_string(&instructions_path).unwrap(); + + assert!( + final_content.contains("Always respond in Spanish."), + "User custom rule was destroyed. Got: {final_content}" + ); + assert!( + final_content.contains("Never suggest npm; prefer pnpm."), + "User custom rule was destroyed. Got: {final_content}" + ); + assert!( + final_content.contains(RTK_BLOCK_START), + "RTK block start marker missing" + ); + assert!( + final_content.contains(RTK_BLOCK_END), + "RTK block end marker missing" + ); + } + + #[test] + fn test_copilot_init_idempotent_repeats() { + let temp = TempDir::new().unwrap(); + let github_dir = temp.path().join(".github"); + fs::create_dir_all(&github_dir).unwrap(); + + run_copilot_at(temp.path(), InitContext::default()).unwrap(); + let after_first = fs::read_to_string(github_dir.join("copilot-instructions.md")).unwrap(); + + run_copilot_at(temp.path(), InitContext::default()).unwrap(); + let after_second = fs::read_to_string(github_dir.join("copilot-instructions.md")).unwrap(); + + assert_eq!( + after_first, after_second, + "Second init must be a no-op (idempotent)" + ); + + let count_start = after_first.matches(RTK_BLOCK_START).count(); + let count_end = after_first.matches(RTK_BLOCK_END).count(); + assert_eq!( + count_start, 1, + "RTK_BLOCK_START must appear once, got {count_start}" + ); + assert_eq!( + count_end, 1, + "RTK_BLOCK_END must appear once, got {count_end}" + ); + } + + #[test] + fn test_copilot_init_updates_stale_block() { + let temp = TempDir::new().unwrap(); + let github_dir = temp.path().join(".github"); + fs::create_dir_all(&github_dir).unwrap(); + + let instructions_path = github_dir.join("copilot-instructions.md"); + let stale = format!( + "# Project rules\n\nUse rg.\n\n{}\n# OLD RTK CONTENT\nrtk foo\n{}\n", + RTK_BLOCK_START, RTK_BLOCK_END + ); + fs::write(&instructions_path, &stale).unwrap(); + + run_copilot_at(temp.path(), InitContext::default()).unwrap(); + + let updated = fs::read_to_string(&instructions_path).unwrap(); + + assert!( + updated.contains("Use rg."), + "User content outside the block must be preserved" + ); + assert!( + !updated.contains("# OLD RTK CONTENT"), + "Stale RTK block content must be removed" + ); + assert!( + updated.contains("rtk cargo test"), + "Fresh COPILOT_INSTRUCTIONS content must be present" + ); + } + + #[test] + fn test_copilot_init_dry_run_no_write() { + let temp = TempDir::new().unwrap(); + let instructions_path = temp.path().join(".github").join("copilot-instructions.md"); + assert!(!instructions_path.exists()); + + let ctx = InitContext { + dry_run: true, + ..InitContext::default() + }; + run_copilot_at(temp.path(), ctx).unwrap(); + + assert!( + !instructions_path.exists(), + "Dry-run must not create copilot-instructions.md" + ); + } + + #[test] + fn test_copilot_init_fresh_install_creates_file() { + let temp = TempDir::new().unwrap(); + let instructions_path = temp.path().join(".github").join("copilot-instructions.md"); + assert!(!instructions_path.exists()); + + run_copilot_at(temp.path(), InitContext::default()).unwrap(); + + assert!( + instructions_path.exists(), + "Fresh install must create copilot-instructions.md" + ); + let content = fs::read_to_string(&instructions_path).unwrap(); + assert!(content.contains(RTK_BLOCK_START)); + assert!(content.contains(RTK_BLOCK_END)); + assert!(content.contains("rtk cargo test")); + } + + #[test] + fn test_copilot_init_refuses_malformed_block() { + let temp = TempDir::new().unwrap(); + let github_dir = temp.path().join(".github"); + fs::create_dir_all(&github_dir).unwrap(); + + let instructions_path = github_dir.join("copilot-instructions.md"); + let malformed = format!("# My rules\n\n{}\nincomplete RTK block\n", RTK_BLOCK_START); + fs::write(&instructions_path, &malformed).unwrap(); + + let result = run_copilot_at(temp.path(), InitContext::default()); + + assert!( + result.is_err(), + "Malformed file must cause an error, not silent rewrite" + ); + + let after = fs::read_to_string(&instructions_path).unwrap(); + assert_eq!(after, malformed, "File must not be modified when malformed"); + } + + #[test] + fn test_copilot_init_malformed_leaves_no_hook_on_disk() { + // Regression: a malformed copilot-instructions.md aborted the install + // mid-way, but the hook config had already been written. The upsert + // now runs first, so the hook config must not appear when the upsert + // bails. + let temp = TempDir::new().unwrap(); + let github_dir = temp.path().join(".github"); + fs::create_dir_all(&github_dir).unwrap(); + + let instructions_path = github_dir.join("copilot-instructions.md"); + let malformed = format!("# My rules\n\n{}\nincomplete RTK block\n", RTK_BLOCK_START); + fs::write(&instructions_path, &malformed).unwrap(); + + let hook_path = github_dir.join("hooks").join("rtk-rewrite.json"); + + let result = run_copilot_at(temp.path(), InitContext::default()); + + assert!(result.is_err(), "Malformed file must cause a hard error"); + assert!( + !hook_path.exists(), + "Hook config must not be written when the upsert aborts: {}", + hook_path.display() + ); + } + + #[test] + fn test_claude_md_mode_refuses_malformed_block() { + // Mirrors `test_copilot_init_refuses_malformed_block`: a malformed + // CLAUDE.md previously emitted a warning and exited 0, silently + // skipping the OpenCode plugin step. The shared `write_rtk_block` + // dispatcher now bails for both paths. + let tmp = TempDir::new().unwrap(); + with_claude_dir_override(&tmp, |claude_dir| { + let claude_md = claude_dir.join(CLAUDE_MD); + let malformed = format!( + "# Existing notes\n\n{}\nincomplete RTK block\n", + RTK_BLOCK_START + ); + fs::write(&claude_md, &malformed).unwrap(); + + let result = run_claude_md_mode(true, false, InitContext::default()); + + assert!( + result.is_err(), + "Malformed CLAUDE.md must cause a hard error, not silent skip" + ); + + let after = fs::read_to_string(&claude_md).unwrap(); + assert_eq!(after, malformed, "File must not be modified when malformed"); + }); + } } diff --git a/src/main.rs b/src/main.rs index d6d0aa4f3..c1a897190 100644 --- a/src/main.rs +++ b/src/main.rs @@ -903,7 +903,10 @@ enum PnpmCommands { #[derive(Debug, Subcommand)] enum DockerCommands { /// List running containers - Ps, + Ps { + #[arg(short = 'a', long)] + all: bool, + }, /// List images Images, /// Show container logs (deduplicated) @@ -921,11 +924,17 @@ enum DockerCommands { #[derive(Debug, Subcommand)] enum ComposeCommands { /// List compose services (compact) - Ps, + Ps { + #[arg(short = 'a', long)] + all: bool, + }, /// Show compose logs (deduplicated) Logs { /// Optional service name service: Option, + /// Number of log lines to fetch + #[arg(long, default_value_t = 100)] + tail: u32, }, /// Build compose services (summary) Build { @@ -939,6 +948,12 @@ enum ComposeCommands { #[derive(Debug, Subcommand)] enum KubectlCommands { + /// Get Kubernetes resources (compact for pods/services) + Get { + /// kubectl get arguments + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + args: Vec, + }, /// List pods Pods { #[arg(short, long)] @@ -1696,8 +1711,13 @@ fn run_cli() -> Result { }, Commands::Docker { command } => match command { - DockerCommands::Ps => { - container::run(container::ContainerCmd::DockerPs, &[], cli.verbose)? + DockerCommands::Ps { all } => { + let cmd = if all { + container::ContainerCmd::DockerPsAll + } else { + container::ContainerCmd::DockerPs + }; + container::run(cmd, &[], cli.verbose)? } DockerCommands::Images => { container::run(container::ContainerCmd::DockerImages, &[], cli.verbose)? @@ -1706,9 +1726,9 @@ fn run_cli() -> Result { container::run(container::ContainerCmd::DockerLogs, &[c], cli.verbose)? } DockerCommands::Compose { command: compose } => match compose { - ComposeCommands::Ps => container::run_compose_ps(cli.verbose)?, - ComposeCommands::Logs { service } => { - container::run_compose_logs(service.as_deref(), cli.verbose)? + ComposeCommands::Ps { all } => container::run_compose_ps(all, cli.verbose)?, + ComposeCommands::Logs { service, tail } => { + container::run_compose_logs(service.as_deref(), tail, cli.verbose)? } ComposeCommands::Build { service } => { container::run_compose_build(service.as_deref(), cli.verbose)? @@ -1721,6 +1741,7 @@ fn run_cli() -> Result { }, Commands::Kubectl { command } => match command { + KubectlCommands::Get { args } => container::run_kubectl_get(&args, cli.verbose)?, KubectlCommands::Pods { namespace, all } => { let mut args: Vec = Vec::new(); if all { @@ -2637,6 +2658,18 @@ mod tests { } } + #[test] + fn test_try_parse_kubectl_get_alias() { + let cli = Cli::try_parse_from(["rtk", "kubectl", "get", "pods", "-n", "default"]).unwrap(); + + match cli.command { + Commands::Kubectl { + command: KubectlCommands::Get { args }, + } => assert_eq!(args, vec!["pods", "-n", "default"]), + _ => panic!("Expected Kubectl Get command"), + } + } + #[test] fn test_try_parse_init_agent_hermes_uninstall() { let cli = Cli::try_parse_from(["rtk", "init", "--agent", "hermes", "--uninstall"]).unwrap(); diff --git a/src/parser/formatter.rs b/src/parser/formatter.rs index 6fc9022ef..613be8f9a 100644 --- a/src/parser/formatter.rs +++ b/src/parser/formatter.rs @@ -1,5 +1,8 @@ /// Token-efficient formatting trait for canonical types use super::types::*; +use crate::core::truncate::CAP_INVENTORY; + +const MAX_DEPS_LISTING: usize = CAP_INVENTORY; /// Output formatting modes #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -45,7 +48,13 @@ pub trait TokenFormatter { impl TokenFormatter for TestResult { fn format_compact(&self) -> String { - let mut lines = vec![format!("PASS ({}) FAIL ({})", self.passed, self.failed)]; + // Always surface skipped/pending tests — hiding them lets coverage gaps + // (test.skip / it.skip / xfail) accumulate invisibly. + let mut summary = format!("PASS ({}) FAIL ({})", self.passed, self.failed); + if self.skipped > 0 { + summary.push_str(&format!(" skipped ({})", self.skipped)); + } + let mut lines = vec![summary]; if !self.failures.is_empty() { lines.push(String::new()); @@ -112,6 +121,29 @@ impl TokenFormatter for TestResult { impl TokenFormatter for DependencyState { fn format_compact(&self) -> String { + // A plain package listing (`pnpm list` / `npm ls`) carries no upgrade + // info — every dep has `latest_version == None`. Reporting "All packages + // up-to-date" there is a false positive that hides the entire list, so + // we render the actual packages instead. + let is_listing = self.outdated_count == 0 + && !self.dependencies.is_empty() + && self.dependencies.iter().all(|d| d.latest_version.is_none()); + if is_listing { + let total = self.total_packages.max(self.dependencies.len()); + let mut lines = vec![format!("{} packages", total)]; + for dep in self.dependencies.iter().take(MAX_DEPS_LISTING) { + let dev = if dep.dev_dependency { " (dev)" } else { "" }; + lines.push(format!(" {} {}{}", dep.name, dep.current_version, dev)); + } + if self.dependencies.len() > MAX_DEPS_LISTING { + lines.push(format!( + " ... +{} more", + self.dependencies.len() - MAX_DEPS_LISTING + )); + } + return lines.join("\n"); + } + if self.outdated_count == 0 { return "All packages up-to-date".to_string(); } @@ -198,6 +230,35 @@ mod tests { } } + fn make_dep(name: &str, version: &str, latest: Option<&str>) -> Dependency { + Dependency { + name: name.to_string(), + current_version: version.to_string(), + latest_version: latest.map(str::to_string), + wanted_version: None, + dev_dependency: false, + } + } + + #[test] + fn test_dependency_state_plain_listing_shows_packages() { + let state = DependencyState { + total_packages: 2, + outdated_count: 0, + dependencies: vec![ + make_dep("react", "18.0.0", None), + make_dep("typescript", "5.0.0", None), + ], + }; + let out = state.format_compact(); + assert!(out.contains("react"), "package name missing"); + assert!(out.contains("typescript"), "package name missing"); + assert!( + !out.contains("up-to-date"), + "false positive: plain listing should not say up-to-date" + ); + } + // RED: format_compact must show the full error message, not just 2 lines. // Playwright errors contain the expected/received diff and call log starting // at line 3+. Truncating to 2 lines leaves the agent with no debug info.