diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 57dba0f42..16ecfa716 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -1,6 +1,7 @@ version: 2 updates: - package-ecosystem: "cargo" + target-branch: "develop" directory: "/" schedule: interval: "weekly" @@ -9,9 +10,10 @@ updates: open-pull-requests-limit: 5 - package-ecosystem: "github-actions" + target-branch: "develop" directory: "/" schedule: interval: "weekly" labels: - "dependencies" - - "ci" + - "area:ci" diff --git a/.github/workflows/next-release.yml b/.github/workflows/next-release.yml new file mode 100644 index 000000000..e7535f864 --- /dev/null +++ b/.github/workflows/next-release.yml @@ -0,0 +1,126 @@ +name: Update Next Release PR + +on: + pull_request: + types: [closed] + branches: [develop] + +permissions: + contents: read + pull-requests: write + +jobs: + update-next-release: + if: github.event.pull_request.merged == true + runs-on: ubuntu-latest + steps: + - name: Update Next Release PR + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + PR_TITLE: ${{ github.event.pull_request.title }} + PR_URL: ${{ github.event.pull_request.html_url }} + PR_BODY: ${{ github.event.pull_request.body }} + REPO: ${{ github.repository }} + ALLOWED_REPOS: "rtk-ai/rtk" + run: | + set -euo pipefail + + URL_PATTERN="" + for repo in $ALLOWED_REPOS; do + URL_PATTERN="${URL_PATTERN}|https://github\\.com/${repo}/issues/[0-9]+" + done + URL_PATTERN="${URL_PATTERN#|}" + + if printf '%s' "$PR_TITLE" | grep -qiE '^feat'; then + SECTION="Feats" + elif printf '%s' "$PR_TITLE" | grep -qiE '^fix'; then + SECTION="Fix" + else + SECTION="Other" + fi + + ISSUE_REFS="" + if [ -n "$PR_BODY" ]; then + ISSUE_REFS=$(echo "$PR_BODY" \ + | grep -oiE "(closes|fixes|resolves):?\s+#[0-9]+|${URL_PATTERN}" \ + | grep -oE '#[0-9]+|issues/[0-9]+' \ + | sed 's|issues/|#|' \ + | sort -u \ + || true) + fi + + ENTRY="- ${PR_TITLE} [#${PR_NUMBER}](${PR_URL})" + if [ -n "$ISSUE_REFS" ]; then + CLOSES_PARTS="" + while IFS= read -r ref; do + [ -z "$ref" ] && continue + NUM="${ref#\#}" + ISSUE_URL="https://github.com/${REPO}/issues/${NUM}" + if [ -n "$CLOSES_PARTS" ]; then + CLOSES_PARTS="${CLOSES_PARTS}, [${ref}](${ISSUE_URL}) (to verify)" + else + CLOSES_PARTS="Closes [${ref}](${ISSUE_URL}) (to verify)" + fi + done <<< "$ISSUE_REFS" + ENTRY="${ENTRY} — ${CLOSES_PARTS}" + fi + + NEXT_PR=$(gh pr list \ + --repo "$REPO" \ + --label next-release \ + --base master \ + --head develop \ + --state open \ + --json number,body \ + --jq '.[0] // empty') + + NEXT_PR_NUMBER="" + if [ -n "$NEXT_PR" ]; then + NEXT_PR_NUMBER=$(echo "$NEXT_PR" | jq -r '.number') + fi + + if [ -z "$NEXT_PR_NUMBER" ]; then + TEMPLATE="### Feats + + ### Fix + + ### Other" + + PR_CREATE_URL=$(gh pr create \ + --repo "$REPO" \ + --base master \ + --head develop \ + --title "Next Release" \ + --label next-release \ + --body "$TEMPLATE") + + NEXT_PR_NUMBER=$(echo "$PR_CREATE_URL" | grep -oE '/pull/[0-9]+' | grep -oE '[0-9]+') + CURRENT_BODY="$TEMPLATE" + else + CURRENT_BODY=$(echo "$NEXT_PR" | jq -r '.body') + fi + + SECTION_HEADER="### ${SECTION}" + export ENTRY + if echo "$CURRENT_BODY" | grep -qF "$SECTION_HEADER"; then + UPDATED_BODY=$(echo "$CURRENT_BODY" | awk -v section="$SECTION_HEADER" ' + $0 == section { + print + print ENVIRON["ENTRY"] + next + } + { print } + ') + else + UPDATED_BODY="${CURRENT_BODY} + + ${SECTION_HEADER} + ${ENTRY}" + fi + + gh pr edit "$NEXT_PR_NUMBER" \ + --repo "$REPO" \ + --body "$UPDATED_BODY" + + echo "Updated Next Release PR #${NEXT_PR_NUMBER} — added entry to ### ${SECTION}" diff --git a/CHANGELOG.md b/CHANGELOG.md index 08cae60ae..9f207b940 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -431,6 +431,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * **cargo clippy:** include actionable error details in compact output instead of summary-only counts ([#602](https://github.com/rtk-ai/rtk/issues/602)) * **curl:** skip JSON schema replacement when schema is larger than original payload ([#297](https://github.com/rtk-ai/rtk/issues/297)) +* **init:** `rtk init -g --uninstall` now removes `` block from CLAUDE.md ([#384](https://github.com/rtk-ai/rtk/issues/384)) * **toml-dsl:** fix regex overmatch on `tofu-plan/init/validate/fmt` and `mix-format/compile` — add `(\s|$)` word boundary to prevent matching subcommands (e.g. `tofu planet`, `mix formats`) ([#349](https://github.com/rtk-ai/rtk/issues/349)) * **toml-dsl:** remove 3 dead built-in filters (`docker-inspect`, `docker-compose-ps`, `pnpm-build`) — Clap routes these commands before `run_fallback`, so the TOML filters never fire ([#351](https://github.com/rtk-ai/rtk/issues/351)) * **toml-dsl:** `uv-sync` — remove `Resolved` short-circuit; it fires before the package list is printed, hiding installed packages ([#386](https://github.com/rtk-ai/rtk/issues/386)) @@ -485,6 +486,7 @@ breakage, but future rule additions won't take effect until they migrate. * +48 regression tests covering all command categories: aws, psql, Python, Go, JS/TS, compound operators, sudo/env prefixes, registry invariants (607 total, was 559) +* +5 tests for uninstall `--claude-md` artifact cleanup (614 total) ## [0.24.0](https://github.com/rtk-ai/rtk/compare/v0.23.0...v0.24.0) (2026-03-04) diff --git a/Cargo.lock b/Cargo.lock index 7ad9dc981..b7796e6d8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -971,9 +971,9 @@ dependencies = [ [[package]] name = "rustls-webpki" -version = "0.103.9" +version = "0.103.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d7df23109aa6c1567d1c575b9952556388da57401e4ace1d15f79eedad0d8f53" +checksum = "61c429a8649f110dddef65e2a5ad240f747e85f7758a6bccc7e5777bd33f756e" dependencies = [ "ring", "rustls-pki-types", diff --git a/INSTALL.md b/INSTALL.md index f439d4e74..fb231500d 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -296,9 +296,9 @@ rtk git push # → "ok ✓ main" ### Pnpm (fork only) ```bash -rtk pnpm list # Dependency tree (-70% tokens) -rtk pnpm outdated # Available updates (-80-90%) -rtk pnpm install pkg # Silent installation +rtk pnpm list # Dependency tree (-70% tokens) +rtk pnpm outdated # Available updates (-80-90%) +rtk pnpm install # Silent installation ``` ### Tests diff --git a/docs/usage/FEATURES.md b/docs/usage/FEATURES.md index bf3412b82..ed5ee0bd7 100644 --- a/docs/usage/FEATURES.md +++ b/docs/usage/FEATURES.md @@ -215,11 +215,11 @@ rtk grep [chemin] [options] |--------|-------|--------|-------------| | `--max-len` | `-l` | 80 | Longueur maximale de ligne | | `--max` | `-m` | 50 | Nombre maximum de resultats | -| `--context-only` | `-c` | non | Afficher uniquement le contexte du match | +| `--context-only` | | non | Afficher uniquement le contexte du match (pas de raccourci, `-c` est reserve a `grep --count`) | | `--file-type` | `-t` | tous | Filtrer par type (ts, py, rust, etc.) | | `--line-numbers` | `-n` | oui | Numeros de ligne (toujours actif) | -Les arguments supplementaires sont transmis a `rg` (ripgrep). +Les arguments supplementaires sont transmis a `rg` (ripgrep). Les flags qui changent le format de sortie (`-c`, `-l`, `-L`, `-o`, `-Z`) passent directement a `rg`/`grep` sans filtrage RTK. **Economies :** ~80% @@ -800,7 +800,7 @@ Detecte automatiquement : prettier, black, ruff format, rustfmt. Applique un fil |----------|-------------|-----------| | `rtk pnpm list [-d N]` | Arbre de dependances compact | ~70% | | `rtk pnpm outdated` | Paquets obsoletes : `pkg: old -> new` | ~80% | -| `rtk pnpm install [pkgs...]` | Filtre les barres de progression | ~60% | +| `rtk pnpm install` | Filtre les barres de progression | ~60% | | `rtk pnpm build` | Delegue au filtre Next.js | ~87% | | `rtk pnpm typecheck` | Delegue au filtre tsc | ~83% | diff --git a/src/cmds/cloud/curl_cmd.rs b/src/cmds/cloud/curl_cmd.rs index a24506d8c..ed5b04a86 100644 --- a/src/cmds/cloud/curl_cmd.rs +++ b/src/cmds/cloud/curl_cmd.rs @@ -1,14 +1,20 @@ -//! Runs curl and applies a simple truncation with tee hint if the output is too long. +//! Runs curl and condenses long output for human consumption. +//! +//! For pipes / redirects (non-TTY) and JSON bodies the full response is passed +//! through unchanged — truncating mid-stream would break downstream parsers. +//! The condensed-form-with-tee-hint path is reserved for non-JSON bodies on +//! a real terminal where a human reads the output and the tee file gives the +//! LLM a way to recover the raw response. use crate::core::tee::force_tee_hint; use crate::core::tracking; use crate::core::{stream::exec_capture, utils::resolved_command}; use anyhow::{Context, Result}; +use std::borrow::Cow; +use std::io::IsTerminal; const MAX_RESPONSE_SIZE: usize = 500; -/// Not using run_filtered: on failure, curl can return HTML error pages (404, 500) -/// that the JSON schema filter would mangle. The early exit skips filtering entirely. pub fn run(args: &[String], verbose: u8) -> Result { let timer = tracking::TimedExecution::start(); let mut cmd = resolved_command("curl"); @@ -24,7 +30,8 @@ pub fn run(args: &[String], verbose: u8) -> Result { let result = exec_capture(&mut cmd).context("Failed to run curl")?; - // Early exit: don't feed HTTP error bodies (HTML 404 etc.) through JSON schema filter + // Skip filtering on failure: curl can return HTML error bodies that would + // be misleading to summarize, and we want the real exit code surfaced. if !result.success() { let msg = if result.stderr.trim().is_empty() { result.stdout.trim().to_string() @@ -35,12 +42,13 @@ pub fn run(args: &[String], verbose: u8) -> Result { return Ok(result.exit_code); } - let raw = result.stdout.clone(); + let exit_code = result.exit_code; + let raw = result.stdout; + let is_tty = std::io::stdout().is_terminal(); + let filtered = filter_curl_output(&raw, is_tty); - let result = filter_curl_output(&result.stdout); - - println!("{}", result.content); - if let Some(hint) = &result.tee_hint { + println!("{}", filtered.content); + if let Some(hint) = &filtered.tee_hint { println!("{}", hint); } @@ -48,34 +56,65 @@ pub fn run(args: &[String], verbose: u8) -> Result { &format!("curl {}", args.join(" ")), &format!("rtk curl {}", args.join(" ")), &raw, - &result.content, + &filtered.content, ); - Ok(0) + Ok(exit_code) } -fn filter_curl_output(raw: &str) -> FilterResult { +fn filter_curl_output(raw: &str, is_tty: bool) -> FilterResult<'_> { let trimmed = raw.trim(); - let tee_hint = force_tee_hint(raw, "curl"); - - // If the output is too long and we have a tee hint, truncate the output. - let content = if trimmed.len() >= MAX_RESPONSE_SIZE && tee_hint.is_some() { - let mut end = MAX_RESPONSE_SIZE; - // Ensure we don't cut in the middle of a UTF-8 character. - // .len() counts bytes, not chars. - while !trimmed.is_char_boundary(end) { - end -= 1; - } - format!("{}... ({} bytes total)", &trimmed[..end], trimmed.len()) - } else { - trimmed.to_string() + + // Heuristic: looks like a top-level JSON document. Numbers / booleans / null + // are always under MAX_RESPONSE_SIZE so they don't need detection here. + let looks_like_json = (trimmed.starts_with('{') && trimmed.ends_with('}')) + || (trimmed.starts_with('[') && trimmed.ends_with(']')) + || (trimmed.starts_with('"') && trimmed.ends_with('"') && trimmed.len() >= 2); + + // Pass through unchanged when: + // - body looks like JSON (mid-stream truncation produces invalid JSON, #1536) + // - stdout is not a terminal (pipes / redirects need the full body, #1282) + // - body fits under the truncation threshold + // + // Critically, do NOT call `force_tee_hint` on this path — it has a side effect + // (writes the raw body to a tee log file) and we don't need a recovery file + // when the consumer already receives the full body. + if !is_tty || looks_like_json || trimmed.len() < MAX_RESPONSE_SIZE { + return FilterResult { + content: Cow::Borrowed(trimmed), + tee_hint: None, + }; + } + + // We're about to truncate for a human reader. Write a tee file so they (or + // the LLM in their stead) can recover the full body from the printed hint. + let Some(hint) = force_tee_hint(raw, "curl") else { + // Tee disabled (RTK_TEE=0 or below MIN_TEE_SIZE): we have nowhere to + // point a recovery hint to, so pass through rather than emit an + // unrecoverable truncation marker. + return FilterResult { + content: Cow::Borrowed(trimmed), + tee_hint: None, + }; }; - FilterResult { content, tee_hint } + let mut end = MAX_RESPONSE_SIZE; + // Don't cut in the middle of a UTF-8 character — .len() counts bytes. + while !trimmed.is_char_boundary(end) { + end -= 1; + } + FilterResult { + content: Cow::Owned(format!( + "{}... ({} bytes total)", + &trimmed[..end], + trimmed.len() + )), + tee_hint: Some(hint), + } } -struct FilterResult { - content: String, +struct FilterResult<'a> { + content: Cow<'a, str>, tee_hint: Option, } @@ -86,32 +125,33 @@ mod tests { #[test] fn test_filter_curl_json_small_no_tee_hint() { let output = r#"{"r2Ready":true,"status":"ok"}"#; - let result = filter_curl_output(output); - assert_eq!(result.content, output); + let result = filter_curl_output(output, true); + assert_eq!(&*result.content, output); assert!(result.tee_hint.is_none()); } #[test] fn test_filter_curl_non_json() { let output = "Hello, World!\nThis is plain text."; - let result = filter_curl_output(output); - assert_eq!(result.content, output); + let result = filter_curl_output(output, true); + assert_eq!(&*result.content, output); } #[test] fn test_filter_curl_long_output_truncated() { let long: String = "x".repeat(1000); - let result = filter_curl_output(&long); + let result = filter_curl_output(&long, true); assert!(result.content.starts_with('x')); assert!(result.content.contains("bytes total")); assert!(result.content.contains("1000")); assert!(result.content.len() < 600); + assert!(result.tee_hint.is_some(), "TTY truncation must emit a hint"); } #[test] fn test_filter_curl_multibyte_boundary() { let content = "a".repeat(499) + "é"; - let result = filter_curl_output(&content); + let result = filter_curl_output(&content, true); assert!(result.content.contains("bytes total")); assert!(result.content.len() < 600); } @@ -119,7 +159,84 @@ mod tests { #[test] fn test_filter_curl_exact_500_bytes() { let content = "a".repeat(500); - let result = filter_curl_output(&content); + let result = filter_curl_output(&content, true); assert!(result.content.contains("bytes total")); } + + // --- #1536: large JSON must remain parseable for downstream tools --- + + #[test] + fn test_filter_curl_large_json_object_passthrough() { + let payload = "x".repeat(600); + let json = format!(r#"{{"data":"{}"}}"#, payload); + let result = filter_curl_output(&json, true); + assert!(!result.content.contains("bytes total")); + assert!(result.content.starts_with('{')); + assert!(result.content.ends_with('}')); + assert!(result.tee_hint.is_none()); + } + + #[test] + fn test_filter_curl_large_json_array_passthrough() { + let body = (0..50) + .map(|i| format!(r#"{{"id":{},"name":"item-{:04}"}}"#, i, i)) + .collect::>() + .join(","); + let json = format!("[{}]", body); + assert!( + json.len() >= MAX_RESPONSE_SIZE, + "fixture must exceed cap, got {}", + json.len() + ); + let result = filter_curl_output(&json, true); + assert!(!result.content.contains("bytes total")); + assert!(result.content.starts_with('[')); + assert!(result.content.ends_with(']')); + } + + #[test] + fn test_filter_curl_large_json_bare_string_passthrough() { + // Bare top-level JSON string — e.g. an /api/token endpoint returning "". + let token = "z".repeat(800); + let json = format!(r#""{}""#, token); + let result = filter_curl_output(&json, true); + assert!(!result.content.contains("bytes total")); + assert!(result.content.starts_with('"')); + assert!(result.content.ends_with('"')); + } + + // --- #1282: pipes / redirects (non-TTY) must receive full body --- + + #[test] + fn test_filter_curl_pipe_no_truncation_for_non_json() { + let long: String = "x".repeat(1000); + let result = filter_curl_output(&long, false); + assert!(!result.content.contains("bytes total")); + assert_eq!(result.content.len(), 1000); + assert!(result.tee_hint.is_none()); + } + + #[test] + fn test_filter_curl_pipe_no_truncation_for_json() { + let payload = "y".repeat(600); + let json = format!(r#"{{"data":"{}"}}"#, payload); + let result = filter_curl_output(&json, false); + assert!(!result.content.contains("bytes total")); + assert!(result.content.ends_with('}')); + assert!(result.tee_hint.is_none()); + } + + // --- Cow optimization: passthrough must not allocate --- + + #[test] + fn test_filter_curl_passthrough_is_borrowed() { + // Passthrough paths return Cow::Borrowed to avoid copying multi-MB bodies. + let pipe_payload = "x".repeat(2000); + let pipe_result = filter_curl_output(&pipe_payload, false); + assert!(matches!(pipe_result.content, Cow::Borrowed(_))); + + let json_payload = format!(r#"[{}]"#, "1,".repeat(300)); + let json_result = filter_curl_output(&json_payload, true); + assert!(matches!(json_result.content, Cow::Borrowed(_))); + } } diff --git a/src/cmds/dotnet/dotnet_trx.rs b/src/cmds/dotnet/dotnet_trx.rs index ed3726453..42275e727 100644 --- a/src/cmds/dotnet/dotnet_trx.rs +++ b/src/cmds/dotnet/dotnet_trx.rs @@ -565,9 +565,12 @@ mod tests { let trx_old = r#" "#; std::fs::write(trx_dir.join("old.trx"), trx_old).expect("write old trx"); - std::thread::sleep(Duration::from_millis(5)); - let since = SystemTime::now(); - std::thread::sleep(Duration::from_millis(5)); + + std::thread::sleep(Duration::from_millis(10)); + + let since = SystemTime::now() + .checked_sub(Duration::from_millis(10)) + .expect("threshold overflow"); let trx_new = r#" "#; diff --git a/src/cmds/git/git.rs b/src/cmds/git/git.rs index f32c825b4..35a56da52 100644 --- a/src/cmds/git/git.rs +++ b/src/cmds/git/git.rs @@ -1,13 +1,13 @@ //! Filters git output — log, status, diff, and more — keeping just the essential info. use crate::core::config; -use crate::core::stream::exec_capture; +use crate::core::stream::{exec_capture, CaptureResult}; use crate::core::tracking; use crate::core::utils::{exit_code_from_output, exit_code_from_status, resolved_command}; -use std::process::Stdio; use anyhow::{Context, Result}; use std::ffi::OsString; use std::process::Command; +use std::process::Stdio; #[derive(Debug, Clone)] pub enum GitCommand { @@ -35,6 +35,17 @@ fn git_cmd(global_args: &[String]) -> Command { cmd } +/// Create a git Command for internal parsing that must be locale-stable. +/// +/// We only use this for non-user-facing parses where RTK depends on git's +/// English status phrases. User-visible passthrough output keeps the user's +/// locale. +fn git_cmd_c_locale(global_args: &[String]) -> Command { + let mut cmd = git_cmd(global_args); + cmd.env("LC_ALL", "C"); + cmd +} + pub fn run( cmd: GitCommand, args: &[String], @@ -750,6 +761,104 @@ pub(crate) fn format_status_output(porcelain: &str) -> String { output.trim_end().to_string() } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum GitStatusState { + Rebase, + MergeConflicts, + MergeReadyToCommit, + CherryPick, + Revert, + Bisect, + Am, + SparseCheckout, +} + +impl GitStatusState { + fn summary(self) -> &'static str { + match self { + Self::Rebase => "rebase in progress", + Self::MergeConflicts => "merge in progress. unresolved conflicts", + Self::MergeReadyToCommit => "merge in progress. no conflicts", + Self::CherryPick => "cherry-pick in progress", + Self::Revert => "revert in progress", + Self::Bisect => "bisect in progress", + Self::Am => "am session in progress", + Self::SparseCheckout => "sparse checkout enabled", + } + } +} + +const REBASE_INDICATORS: &[&str] = &[ + "rebase in progress", + "You are currently rebasing", + "You are currently editing", + "You are currently splitting", + "Last command done", + "Next command to do", + "No commands remaining", +]; + +fn detect_status_state(line: &str) -> Option { + if line.contains("All conflicts fixed but you are still merging") { + Some(GitStatusState::MergeReadyToCommit) + } else if line.contains("You have unmerged paths") { + Some(GitStatusState::MergeConflicts) + } else if line.contains("You are currently cherry-picking") { + Some(GitStatusState::CherryPick) + } else if line.contains("You are currently reverting") { + Some(GitStatusState::Revert) + } else if line.contains("You are currently bisecting") { + Some(GitStatusState::Bisect) + } else if line.contains("You are in the middle of an am session") { + Some(GitStatusState::Am) + } else if line.contains("You are in a sparse checkout") { + Some(GitStatusState::SparseCheckout) + } else if REBASE_INDICATORS.iter().any(|i| line.contains(i)) { + Some(GitStatusState::Rebase) + } else { + None + } +} + +/// Extract a compact in-progress state summary from plain `git status` output. +/// +/// Compact mode runs `git status --porcelain -b`, which omits the state header +/// git prints for rebase / merge / cherry-pick / revert / bisect / am / sparse +/// checkout. Hiding that block is a correctness bug — e.g. during an interactive +/// rebase edit, the user sees a "clean" status and misses "You are currently +/// editing a commit while rebasing ...". +/// +/// This helper walks the plain-status output we already capture for tracking +/// and emits a compact, RTK-style summary rather than dumping git's full prose. +/// Returns `None` when no state is in progress. +fn extract_state_header(raw: &str) -> Option { + // Headers of the file-change blocks — everything relevant to state appears + // above these in git's output, so they double as a terminator. + const STOPPERS: &[&str] = &[ + "Changes to be committed:", + "Changes not staged for commit:", + "Untracked files:", + "Unmerged paths:", + "no changes added to commit", + "nothing to commit", + "nothing added to commit", + ]; + + for line in raw.lines() { + let stripped = line.trim(); + + if STOPPERS.iter().any(|s| stripped.starts_with(s)) { + break; + } + + if let Some(state) = detect_status_state(stripped) { + return Some(state.summary().to_string()); + } + } + + None +} + /// Minimal filtering for git status with user-provided args fn filter_status_with_args(output: &str) -> String { let mut result = Vec::new(); @@ -829,7 +938,7 @@ fn run_status(args: &[String], verbose: u8, global_args: &[String]) -> Result Result format!("{}\n{}", state, formatted), + None => formatted, + }; + + println!("{}", final_output); // Track for statistics - timer.track("git status", "rtk git status", &raw_output, &formatted); + timer.track("git status", "rtk git status", &raw_output, &final_output); Ok(0) } @@ -883,8 +1000,7 @@ fn run_add(args: &[String], verbose: u8, global_args: &[String]) -> Result // Count what was added let mut stat_cmd = git_cmd(global_args); stat_cmd.args(["diff", "--cached", "--stat", "--shortstat"]); - let stat_result = - exec_capture(&mut stat_cmd).context("Failed to check staged files")?; + let stat_result = exec_capture(&mut stat_cmd).context("Failed to check staged files")?; let compact = if stat_result.stdout.trim().is_empty() { "ok (nothing to add)".to_string() @@ -1003,7 +1119,10 @@ 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 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); @@ -1224,11 +1343,7 @@ fn run_branch(args: &[String], verbose: u8, global_args: &[String]) -> Result Result" format +fn format_stash_message(subcommand: Option<&str>, 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() + } else { + "ok stashed".to_string() + } + } + Some(sub) => format!("ok stash {}", sub), + } +} + fn run_stash( subcommand: Option<&str>, args: &[String], @@ -1407,8 +1539,7 @@ fn run_stash( Some("list") => { let mut cmd = git_cmd(global_args); cmd.args(["stash", "list"]); - let result = - exec_capture(&mut cmd).context("Failed to run git stash list")?; + let result = exec_capture(&mut cmd).context("Failed to run git stash list")?; if result.stdout.trim().is_empty() { let msg = "No stashes"; @@ -1432,8 +1563,7 @@ fn run_stash( for arg in args { cmd.arg(arg); } - let result = - exec_capture(&mut cmd).context("Failed to run git stash show")?; + let result = exec_capture(&mut cmd).context("Failed to run git stash show")?; let filtered = if result.stdout.trim().is_empty() { let msg = "Empty stash"; @@ -1452,7 +1582,8 @@ fn run_stash( &filtered, ); } - Some("pop") | Some("apply") | Some("drop") | Some("push") => { + Some("apply") | Some("branch") | Some("clear") | Some("create") | Some("drop") + | Some("export") | Some("import") | Some("pop") | Some("store") => { let sub = subcommand.unwrap(); let mut cmd = git_cmd(global_args); cmd.args(["stash", sub]); @@ -1463,7 +1594,7 @@ fn run_stash( let combined = result.combined(); let msg = if result.success() { - let msg = format!("ok stash {}", sub); + let msg = format_stash_message(subcommand, &result); println!("{}", msg); msg } else { @@ -1485,10 +1616,19 @@ fn run_stash( return Ok(result.exit_code); } } - Some(sub) => { - // Unrecognized subcommand: passthrough to git stash [args] + // Default: "git stash [push] [--] [...]" or "git stash save []" + Some(_) | None => { + let (sub, arg) = match subcommand { + Some("save") => ("save", None), + Some("push") => ("push", None), + Some(s) => ("push", Some(s)), + None => ("push", None), + }; let mut cmd = git_cmd(global_args); cmd.args(["stash", sub]); + if let Some(arg) = arg { + cmd.arg(arg); + } for arg in args { cmd.arg(arg); } @@ -1496,7 +1636,7 @@ fn run_stash( let combined = result.combined(); let msg = if result.success() { - let msg = format!("ok stash {}", sub); + let msg = format_stash_message(subcommand, &result); println!("{}", msg); msg } else { @@ -1514,40 +1654,6 @@ fn run_stash( &msg, ); - if !result.success() { - return Ok(result.exit_code); - } - } - None => { - // Default: git stash (push) - let mut cmd = git_cmd(global_args); - cmd.arg("stash"); - for arg in args { - cmd.arg(arg); - } - let result = exec_capture(&mut cmd).context("Failed to run git stash")?; - let combined = result.combined(); - - let msg = if result.success() { - if result.stdout.contains("No local changes") { - let msg = "ok (nothing to stash)"; - println!("{}", msg); - msg.to_string() - } else { - let msg = "ok stashed"; - println!("{}", msg); - msg.to_string() - } - } else { - eprintln!("FAILED: git stash"); - if !result.stderr.trim().is_empty() { - eprintln!("{}", result.stderr); - } - combined.clone() - }; - - timer.track("git stash", "rtk git stash", &combined, &msg); - if !result.success() { return Ok(result.exit_code); } @@ -1599,11 +1705,7 @@ fn run_worktree(args: &[String], verbose: u8, global_args: &[String]) -> Result< let result = exec_capture(&mut cmd).context("Failed to run git worktree")?; let combined = result.combined(); - let msg = if result.success() { - "ok" - } else { - &combined - }; + let msg = if result.success() { "ok" } else { &combined }; timer.track( &format!("git worktree {}", args.join(" ")), @@ -1627,8 +1729,7 @@ fn run_worktree(args: &[String], verbose: u8, global_args: &[String]) -> Result< // Default: list mode let mut cmd = git_cmd(global_args); cmd.args(["worktree", "list"]); - let result = - exec_capture(&mut cmd).context("Failed to run git worktree list")?; + let result = exec_capture(&mut cmd).context("Failed to run git worktree list")?; let filtered = filter_worktree_list(&result.stdout); println!("{}", filtered); @@ -1753,6 +1854,21 @@ mod tests { assert_eq!(args, vec!["--no-pager", "--bare"]); } + #[test] + fn test_git_cmd_c_locale_sets_stable_env() { + let cmd = git_cmd_c_locale(&[]); + let envs: Vec<_> = cmd + .get_envs() + .map(|(key, value)| { + ( + key.to_string_lossy().to_string(), + value.expect("env value").to_string_lossy().to_string(), + ) + }) + .collect(); + assert!(envs.contains(&("LC_ALL".to_string(), "C".to_string()))); + } + #[test] fn test_compact_diff() { let diff = r#"diff --git a/foo.rs b/foo.rs @@ -1866,7 +1982,11 @@ mod tests { let normalized = normalize_diff_args_impl(&args, exists_mock(&["src/foo.rs"])); assert_eq!( normalized, - vec!["HEAD".to_string(), "--".to_string(), "src/foo.rs".to_string()] + vec![ + "HEAD".to_string(), + "--".to_string(), + "src/foo.rs".to_string() + ] ); } @@ -1877,7 +1997,11 @@ mod tests { let normalized = normalize_diff_args_impl(&args, exists_mock(&["src/foo.rs"])); assert_eq!( normalized, - vec!["--cached".to_string(), "--".to_string(), "src/foo.rs".to_string()] + vec![ + "--cached".to_string(), + "--".to_string(), + "src/foo.rs".to_string() + ] ); } @@ -1893,10 +2017,7 @@ mod tests { fn test_normalize_diff_args_dotfile_is_path() { let args = vec![".gitignore".to_string()]; let normalized = normalize_diff_args_impl(&args, exists_mock(&[".gitignore"])); - assert_eq!( - normalized, - vec!["--".to_string(), ".gitignore".to_string()] - ); + assert_eq!(normalized, vec!["--".to_string(), ".gitignore".to_string()]); } /// A bare ref (HEAD) that doesn't exist as a file → no injection. @@ -1979,7 +2100,11 @@ mod tests { let result = filter_branch_output(output); assert!(result.contains("* main")); assert!(result.contains("develop")); - assert!(result.contains("feature-x"), "origin branch shown: {}", result); + assert!( + result.contains("feature-x"), + "origin branch shown: {}", + result + ); assert!( result.contains("release-v3"), "upstream branch shown: {}", @@ -2030,6 +2155,74 @@ mod tests { assert_eq!(result, "Clean working tree"); } + #[test] + fn test_extract_state_header_clean_returns_none() { + let raw = "On branch main\nYour branch is up to date with 'origin/main'.\n\nnothing to commit, working tree clean\n"; + assert_eq!(extract_state_header(raw), None); + } + + #[test] + fn test_extract_state_header_no_state_with_changes_returns_none() { + let raw = "On branch main\nChanges not staged for commit:\n (use \"git add ...\" to update what will be committed)\n\tmodified: src/main.rs\n\nno changes added to commit\n"; + assert_eq!(extract_state_header(raw), None); + } + + #[test] + fn test_extract_state_header_editing_while_rebasing() { + let raw = "On branch feature\n\ninteractive rebase in progress; onto abc1234\nLast command done (1 command done):\n edit abc123 some message\nNo commands remaining.\nYou are currently editing a commit while rebasing branch 'feature' on 'abc1234'.\n (use \"git commit --amend\" to amend the current commit)\n (use \"git rebase --continue\" once you are satisfied with your changes)\n\nnothing to commit, working tree clean\n"; + let out = extract_state_header(raw).expect("state expected"); + assert_eq!(out, "rebase in progress"); + } + + #[test] + fn test_extract_state_header_merge_unresolved() { + let raw = "On branch main\nYou have unmerged paths.\n (fix conflicts and run \"git commit\")\n (use \"git merge --abort\" to abort the merge)\n\nUnmerged paths:\n\tboth modified: src/main.rs\n"; + let out = extract_state_header(raw).expect("state expected"); + assert_eq!(out, "merge in progress. unresolved conflicts"); + } + + #[test] + fn test_extract_state_header_cherry_pick() { + let raw = "On branch main\n\nYou are currently cherry-picking commit abc1234.\n (fix conflicts and run \"git cherry-pick --continue\")\n (use \"git cherry-pick --abort\" to cancel the cherry-pick operation)\n\nnothing to commit, working tree clean\n"; + let out = extract_state_header(raw).expect("state expected"); + assert_eq!(out, "cherry-pick in progress"); + } + + #[test] + fn test_extract_state_header_bisect() { + let raw = "On branch main\n\nYou are currently bisecting, started from branch 'main'.\n (use \"git bisect reset\" to get back to the original branch)\n\nnothing to commit, working tree clean\n"; + let out = extract_state_header(raw).expect("state expected"); + assert_eq!(out, "bisect in progress"); + } + + #[test] + fn test_extract_state_header_revert() { + let raw = "On branch main\n\nYou are currently reverting commit abc1234.\n (fix conflicts and run \"git revert --continue\")\n (use \"git revert --abort\" to cancel the revert operation)\n\nnothing to commit, working tree clean\n"; + let out = extract_state_header(raw).expect("state expected"); + assert_eq!(out, "revert in progress"); + } + + #[test] + fn test_extract_state_header_merge_in_middle() { + let raw = "On branch main\n\nAll conflicts fixed but you are still merging.\n (use \"git commit\" to conclude merge)\n\nChanges to be committed:\n\tmodified: src/main.rs\n"; + let out = extract_state_header(raw).expect("state expected"); + assert_eq!(out, "merge in progress. no conflicts"); + } + + #[test] + fn test_extract_state_header_am_session() { + let raw = "On branch main\n\nYou are in the middle of an am session.\n (use \"git am --continue\" to continue)\n (use \"git am --abort\" to restore the original branch)\n\nnothing to commit, working tree clean\n"; + let out = extract_state_header(raw).expect("state expected"); + assert_eq!(out, "am session in progress"); + } + + #[test] + fn test_extract_state_header_sparse_checkout() { + let raw = "On branch main\n\nYou are in a sparse checkout with 17% of tracked files present.\n\nnothing to commit, working tree clean\n"; + let out = extract_state_header(raw).expect("state expected"); + assert_eq!(out, "sparse checkout enabled"); + } + #[test] fn test_format_status_output_modified_files() { let porcelain = "## main...origin/main\n M src/main.rs\n M src/lib.rs\n"; diff --git a/src/cmds/js/pnpm_cmd.rs b/src/cmds/js/pnpm_cmd.rs index 924a9a54b..1048060f4 100644 --- a/src/cmds/js/pnpm_cmd.rs +++ b/src/cmds/js/pnpm_cmd.rs @@ -270,34 +270,18 @@ fn extract_outdated_text(output: &str) -> Option { } } -/// Validates npm package name according to official rules -fn is_valid_package_name(name: &str) -> bool { - if name.is_empty() || name.len() > 214 { - return false; - } - - // No path traversal - if name.contains("..") { - return false; - } - - // Only safe characters - name.chars() - .all(|c| c.is_alphanumeric() || matches!(c, '@' | '/' | '-' | '_' | '.')) -} - #[derive(Debug, Clone)] pub enum PnpmCommand { List { depth: usize }, Outdated, - Install { packages: Vec }, + Install, } pub fn run(cmd: PnpmCommand, args: &[String], verbose: u8) -> Result { match cmd { PnpmCommand::List { depth } => run_list(depth, args, verbose), PnpmCommand::Outdated => run_outdated(args, verbose), - PnpmCommand::Install { packages } => run_install(&packages, args, verbose), + PnpmCommand::Install => run_install(args, verbose), } } @@ -404,26 +388,12 @@ fn run_outdated(args: &[String], verbose: u8) -> Result { Ok(0) } -fn run_install(packages: &[String], args: &[String], verbose: u8) -> Result { +fn run_install(args: &[String], verbose: u8) -> Result { let timer = tracking::TimedExecution::start(); - // Validate package names to prevent command injection - for pkg in packages { - if !is_valid_package_name(pkg) { - anyhow::bail!( - "Invalid package name: '{}' (contains unsafe characters)", - pkg - ); - } - } - let mut cmd = resolved_command("pnpm"); cmd.arg("install"); - for pkg in packages { - cmd.arg(pkg); - } - for arg in args { cmd.arg(arg); } @@ -445,8 +415,8 @@ fn run_install(packages: &[String], args: &[String], verbose: u8) -> Result println!("{}", filtered); timer.track( - &format!("pnpm install {}", packages.join(" ")), - &format!("rtk pnpm install {}", packages.join(" ")), + &format!("pnpm install"), + &format!("rtk pnpm install"), &combined, &filtered, ); @@ -542,14 +512,6 @@ mod tests { assert_eq!(data.dependencies[0].name, "express"); } - #[test] - fn test_package_name_validation() { - assert!(is_valid_package_name("lodash")); - assert!(is_valid_package_name("@clerk/express")); - assert!(!is_valid_package_name("../../../etc/passwd")); - assert!(!is_valid_package_name("lodash; rm -rf /")); - } - #[test] fn test_run_passthrough_accepts_args() { // Test that run_passthrough compiles and has correct signature diff --git a/src/cmds/system/README.md b/src/cmds/system/README.md index 1fdf13971..55de28912 100644 --- a/src/cmds/system/README.md +++ b/src/cmds/system/README.md @@ -5,7 +5,7 @@ ## Specifics - `read.rs` uses `core/filter` for language-aware code stripping (FilterLevel: none/minimal/aggressive) -- `grep_cmd.rs` reads `core/config` for `limits.grep_max_results` and `limits.grep_max_per_file` +- `grep_cmd.rs` reads `core/config` for `limits.grep_max_results` and `limits.grep_max_per_file`. Format-altering flags (`-c`, `-l`, `-L`, `-o`, `-Z`) bypass RTK filtering and run raw. - `local_llm.rs` (`rtk smart`) uses `core/filter` for heuristic file summarization - `format_cmd.rs` is a cross-ecosystem dispatcher: auto-detects and routes to `prettier_cmd` or `ruff_cmd` (black is handled inline, not as a separate module) diff --git a/src/cmds/system/grep_cmd.rs b/src/cmds/system/grep_cmd.rs index 83b1886db..6a33cf3a4 100644 --- a/src/cmds/system/grep_cmd.rs +++ b/src/cmds/system/grep_cmd.rs @@ -50,20 +50,39 @@ pub fn run( let result = exec_capture(&mut rg_cmd) .or_else(|_| { let mut grep_cmd = resolved_command("grep"); - grep_cmd.args(["-rn", pattern, path]); + //When we fall back to grep,include all args, not just -rn. + grep_cmd.args(["-rn", pattern, path]).args(extra_args); exec_capture(&mut grep_cmd) }) .context("grep/rg failed")?; + // Passthrough output flags that produce output that is already small. + if has_format_flag(extra_args) { + print!("{}", result.stdout); + if !result.stderr.is_empty() { + eprint!("{}", result.stderr.trim()); + } + + let args_display = if extra_args.is_empty() { + format!("'{}' {}", pattern, path) + } else { + format!("{} '{}' {}", extra_args.join(" "), pattern, path) + }; + + timer.track_passthrough( + &format!("grep {}", args_display), + &format!("rtk grep {} (passthrough)", args_display), + ); + return Ok(result.exit_code); + } + let exit_code = result.exit_code; let raw_output = result.stdout.clone(); if result.stdout.trim().is_empty() { // Show stderr for errors (bad regex, missing file, etc.) - if exit_code == 2 { - if !result.stderr.trim().is_empty() { - eprintln!("{}", result.stderr.trim()); - } + if exit_code == 2 && !result.stderr.trim().is_empty() { + eprintln!("{}", result.stderr.trim()); } let msg = format!("0 matches for '{}'", pattern); println!("{}", msg); @@ -147,6 +166,23 @@ pub fn run( Ok(exit_code) } +fn has_format_flag(extra_args: &[String]) -> bool { + extra_args.iter().any(|arg| { + matches!( + arg.as_str(), + "-c" | "--count" + | "-l" + | "--files-with-matches" + | "-L" + | "--files-without-match" + | "-o" + | "--only-matching" + | "-Z" + | "--null" + ) + }) +} + fn clean_line(line: &str, max_len: usize, context_re: Option<&Regex>, pattern: &str) -> String { let trimmed = line.trim(); @@ -295,6 +331,48 @@ mod tests { ); } + // --- format flag detection --- + + #[test] + fn test_format_flag_detects_count() { + assert!(has_format_flag(&["-c".to_string()])); + assert!(has_format_flag(&["--count".to_string()])); + } + + #[test] + fn test_format_flag_detects_files_with_matches() { + assert!(has_format_flag(&["-l".to_string()])); + assert!(has_format_flag(&["--files-with-matches".to_string()])); + } + + #[test] + fn test_format_flag_detects_files_without_match() { + assert!(has_format_flag(&["-L".to_string()])); + assert!(has_format_flag(&["--files-without-match".to_string()])); + } + + #[test] + fn test_format_flag_detects_only_matching() { + assert!(has_format_flag(&["-o".to_string()])); + assert!(has_format_flag(&["--only-matching".to_string()])); + } + + #[test] + fn test_format_flag_detects_null() { + assert!(has_format_flag(&["-Z".to_string()])); + assert!(has_format_flag(&["--null".to_string()])); + } + + #[test] + fn test_format_flag_ignores_normal_flags() { + assert!(!has_format_flag(&[ + "-i".to_string(), + "-w".to_string(), + "-A".to_string(), + "3".to_string(), + ])); + } + // Verify line numbers are always enabled in rg invocation (grep_cmd.rs:24). // The -n/--line-numbers clap flag in main.rs is a no-op accepted for compat. #[test] diff --git a/src/cmds/system/json_cmd.rs b/src/cmds/system/json_cmd.rs index 176b6e568..148221444 100644 --- a/src/cmds/system/json_cmd.rs +++ b/src/cmds/system/json_cmd.rs @@ -106,7 +106,8 @@ fn compact_json(value: &Value, depth: usize, max_depth: usize) -> String { Value::Number(n) => format!("{}{}", indent, n), Value::String(s) => { if s.len() > 80 { - format!("{}\"{}...\"", indent, &s[..77]) + let end = s.floor_char_boundary(77); + format!("{}\"{}...\"", indent, &s[..end]) } else { format!("{}\"{}\"", indent, s) } @@ -328,4 +329,36 @@ mod tests { assert!(schema.contains("items")); assert!(schema.contains("(3)")); } + + fn assert_value_truncated(payload: &str) { + let json = format!(r#"{{"key": "{}"}}"#, payload); + let output = filter_json_compact(&json, 5) + .expect("filter_json_compact must not error on valid JSON"); + + assert!(output.contains("key")); + assert!( + output.contains("..."), + "long string should be truncated, got: {output}" + ); + + let value = output + .split('"') + .nth(1) + .expect("output should contain a quoted string value"); + assert!( + value.len() <= 80, + "truncated value is {} bytes: {value}", + value.len() + ); + } + + #[test] + fn test_compact_truncates_pure_multibyte_string() { + assert_value_truncated(&"日本語テスト".repeat(85)); + } + + #[test] + fn test_compact_truncates_mixed_ascii_multibyte_string() { + assert_value_truncated(&("a".repeat(76) + &"日本語".repeat(5))); + } } diff --git a/src/cmds/system/ls.rs b/src/cmds/system/ls.rs index ac40ae328..537e8272f 100644 --- a/src/cmds/system/ls.rs +++ b/src/cmds/system/ls.rs @@ -35,6 +35,7 @@ pub fn run(args: &[String], verbose: u8) -> Result { .collect(); let mut cmd = resolved_command("ls"); + cmd.env("LC_ALL", "C"); cmd.arg("-la"); for flag in &flags { if flag.starts_with("--") { @@ -72,7 +73,16 @@ pub fn run(args: &[String], verbose: u8) -> Result { "ls", &format!("-la {}", target_display), |raw| { - let (entries, summary) = compact_ls(raw, show_all); + let (entries, summary, parsed_count) = compact_ls(raw, show_all); + + // If no lines were parsed (e.g., unrecognized locale), fall back to raw output. + // This is safer than returning "(empty)" for a non-empty directory. + let has_real_content = raw + .lines() + .any(|l| !l.starts_with("total ") && !l.is_empty() && !is_dotdir(l)); + if parsed_count == 0 && has_real_content { + return raw.to_string(); + } // Only show summary in interactive mode (not when piped) let is_tty = std::io::stdout().is_terminal(); @@ -121,6 +131,11 @@ fn human_size(bytes: u64) -> String { /// filename (everything after the date). This handles owner/group names that /// contain spaces, which break the old fixed-column approach. fn parse_ls_line(line: &str) -> Option<(char, u64, String)> { + // Skip . and .. entries before date parsing (works for non-English locales too) + if is_dotdir(line) { + return None; + } + let date_match = LS_DATE_RE.find(line)?; let name = line[date_match.end()..].to_string(); @@ -147,30 +162,45 @@ fn parse_ls_line(line: &str) -> Option<(char, u64, String)> { Some((file_type, size, name)) } +/// Returns true if the line represents a . or .. directory entry. +/// +/// POSIX.1-2017 (IEEE Std 1003.1) specifies that each directory contains +/// entries for "." (the directory itself) and ".." (its parent). These entries +/// always appear in `ls -la` output and are skipped during parsing since they +/// carry no meaningful content for token reduction. +fn is_dotdir(line: &str) -> bool { + line.trim().ends_with('.') || line.trim().ends_with("..") +} + /// Parse ls -la output into compact format: /// name/ (dirs) /// name size (files) -/// Returns (entries, summary) so caller can suppress summary when piped. -fn compact_ls(raw: &str, show_all: bool) -> (String, String) { +/// Returns (entries, summary, parsed_count) so caller can suppress summary when piped. +/// parsed_count tracks how many non-header lines were successfully parsed. +/// If parsed_count == 0 but raw had content, caller should fall back to raw output. +fn compact_ls(raw: &str, show_all: bool) -> (String, String, usize) { use std::collections::HashMap; let mut dirs: Vec = Vec::new(); let mut files: Vec<(String, String)> = Vec::new(); // (name, size) let mut by_ext: HashMap = HashMap::new(); + let mut lines_seen: usize = 0; + let mut parsed_count: usize = 0; + let mut dotdirs: usize = 0; for line in raw.lines() { if line.starts_with("total ") || line.is_empty() { continue; } + lines_seen += 1; let Some((file_type, size, name)) = parse_ls_line(line) else { + if is_dotdir(line) { + dotdirs += 1; + } continue; }; - - // Skip . and .. - if name == "." || name == ".." { - continue; - } + parsed_count += 1; // Filter noise dirs unless -a if !show_all && NOISE_DIRS.iter().any(|noise| name == *noise) { @@ -179,7 +209,8 @@ fn compact_ls(raw: &str, show_all: bool) -> (String, String) { if file_type == 'd' { dirs.push(name); - } else if file_type == '-' || file_type == 'l' { + } else { + // Regular files, symlinks, character/block devices, pipes, sockets let ext = if let Some(pos) = name.rfind('.') { name[pos..].to_string() } else { @@ -191,7 +222,15 @@ fn compact_ls(raw: &str, show_all: bool) -> (String, String) { } if dirs.is_empty() && files.is_empty() { - return ("(empty)\n".to_string(), String::new()); + if lines_seen > 0 && parsed_count == 0 { + if dotdirs == lines_seen { + // Only . and .. entries (empty directory) + return ("(empty)\n".to_string(), String::new(), 0); + } + // Real content that couldn't be parsed (e.g., non-English locale) + return (String::new(), String::new(), 0); + } + return ("(empty)\n".to_string(), String::new(), 0); } let mut entries = String::new(); @@ -229,7 +268,7 @@ fn compact_ls(raw: &str, show_all: bool) -> (String, String) { } summary.push('\n'); - (entries, summary) + (entries, summary, parsed_count) } #[cfg(test)] @@ -244,7 +283,7 @@ mod tests { drwxr-xr-x 2 user staff 64 Jan 1 12:00 src\n\ -rw-r--r-- 1 user staff 1234 Jan 1 12:00 Cargo.toml\n\ -rw-r--r-- 1 user staff 5678 Jan 1 12:00 README.md\n"; - let (entries, _summary) = compact_ls(input, false); + let (entries, _summary, _) = compact_ls(input, false); assert!(entries.contains("src/")); assert!(entries.contains("Cargo.toml")); assert!(entries.contains("README.md")); @@ -265,7 +304,7 @@ mod tests { drwxr-xr-x 2 user staff 64 Jan 1 12:00 target\n\ drwxr-xr-x 2 user staff 64 Jan 1 12:00 src\n\ -rw-r--r-- 1 user staff 100 Jan 1 12:00 main.rs\n"; - let (entries, _summary) = compact_ls(input, false); + let (entries, _summary, _) = compact_ls(input, false); assert!(!entries.contains("node_modules")); assert!(!entries.contains(".git")); assert!(!entries.contains("target")); @@ -278,7 +317,7 @@ mod tests { let input = "total 8\n\ drwxr-xr-x 2 user staff 64 Jan 1 12:00 .git\n\ drwxr-xr-x 2 user staff 64 Jan 1 12:00 src\n"; - let (entries, _summary) = compact_ls(input, true); + let (entries, _summary, _) = compact_ls(input, true); assert!(entries.contains(".git/")); assert!(entries.contains("src/")); } @@ -286,7 +325,29 @@ mod tests { #[test] fn test_compact_empty() { let input = "total 0\n"; - let (entries, summary) = compact_ls(input, false); + let (entries, summary, _) = compact_ls(input, false); + assert_eq!(entries, "(empty)\n"); + assert!(summary.is_empty()); + } + + #[test] + fn test_compact_empty_chinese_locale() { + let input = "total 8\n\ + drwxr-xr-x 2 user user 4096 1月 1 12:00 .\n\ + drwxr-xr-x 16 user user 20480 1月 1 12:00 ..\n"; + let (entries, summary, parsed_count) = compact_ls(input, false); + assert_eq!(parsed_count, 0); + assert_eq!(entries, "(empty)\n"); + assert!(summary.is_empty()); + } + + #[test] + fn test_compact_empty_english_locale() { + let input = "total 0\n\ + drwxr-xr-x 2 lumin wheel 64 Apr 23 00:37 .\n\ + drwxr-xr-x 16 root wheel 164576 Apr 23 00:37 ..\n"; + let (entries, summary, parsed_count) = compact_ls(input, false); + assert_eq!(parsed_count, 0); assert_eq!(entries, "(empty)\n"); assert!(summary.is_empty()); } @@ -298,7 +359,7 @@ mod tests { -rw-r--r-- 1 user staff 1234 Jan 1 12:00 main.rs\n\ -rw-r--r-- 1 user staff 5678 Jan 1 12:00 lib.rs\n\ -rw-r--r-- 1 user staff 100 Jan 1 12:00 Cargo.toml\n"; - let (_entries, summary) = compact_ls(input, false); + let (_entries, summary, _) = compact_ls(input, false); assert!(summary.contains("Summary: 3 files, 1 dirs")); assert!(summary.contains(".rs")); assert!(summary.contains(".toml")); @@ -318,7 +379,7 @@ mod tests { fn test_compact_handles_filenames_with_spaces() { let input = "total 8\n\ -rw-r--r-- 1 user staff 1234 Jan 1 12:00 my file.txt\n"; - let (entries, _summary) = compact_ls(input, false); + let (entries, _summary, _) = compact_ls(input, false); assert!(entries.contains("my file.txt")); } @@ -326,7 +387,7 @@ mod tests { fn test_compact_symlinks() { let input = "total 8\n\ lrwxr-xr-x 1 user staff 10 Jan 1 12:00 link -> target\n"; - let (entries, _summary) = compact_ls(input, false); + let (entries, _summary, _) = compact_ls(input, false); assert!(entries.contains("link -> target")); } @@ -336,7 +397,7 @@ mod tests { let input = "total 48\n\ drwxr-xr-x 2 user staff 64 Jan 1 12:00 src\n\ -rw-r--r-- 1 user staff 1234 Jan 1 12:00 main.rs\n"; - let (entries, summary) = compact_ls(input, false); + let (entries, summary, _) = compact_ls(input, false); assert!( !entries.contains("Summary:"), "entries must not contain summary" @@ -355,7 +416,7 @@ mod tests { drwxr-xr-x 2 user staff 64 Jan 1 12:00 src\n\ -rw-r--r-- 1 user staff 1234 Jan 1 12:00 main.rs\n\ -rw-r--r-- 1 user staff 5678 Jan 1 12:00 lib.rs\n"; - let (entries, _summary) = compact_ls(input, false); + let (entries, _summary, _) = compact_ls(input, false); let line_count = entries.lines().count(); assert_eq!( line_count, 3, @@ -370,7 +431,7 @@ mod tests { let input = "total 8\n\ -rw-r--r-- 1 fjeanne utilisa. du domaine 0 Mar 31 16:18 empty.txt\n\ -rw-r--r-- 1 fjeanne utilisa. du domaine 1234 Mar 31 16:18 data.json\n"; - let (entries, _summary) = compact_ls(input, false); + let (entries, _summary, _) = compact_ls(input, false); assert!( entries.contains("empty.txt"), "should contain 'empty.txt', got: {entries}" @@ -398,23 +459,18 @@ mod tests { // Some systems show year instead of time for old files let input = "total 8\n\ -rw-r--r-- 1 user staff 5678 Dec 25 2024 archive.tar\n"; - let (entries, _summary) = compact_ls(input, false); + let (entries, _summary, _) = compact_ls(input, false); assert!( entries.contains("archive.tar"), "should contain filename, got: {entries}" ); - assert!( - entries.contains("5.5K"), - "should show 5.5K, got: {entries}" - ); + assert!(entries.contains("5.5K"), "should show 5.5K, got: {entries}"); } #[test] fn test_parse_ls_line_basic() { - let (ft, size, name) = parse_ls_line( - "-rw-r--r-- 1 user staff 1234 Jan 1 12:00 file.txt", - ) - .unwrap(); + let (ft, size, name) = + parse_ls_line("-rw-r--r-- 1 user staff 1234 Jan 1 12:00 file.txt").unwrap(); assert_eq!(ft, '-'); assert_eq!(size, 1234); assert_eq!(name, "file.txt"); @@ -422,10 +478,9 @@ mod tests { #[test] fn test_parse_ls_line_multiline_group() { - let (ft, size, name) = parse_ls_line( - "-rw-r--r-- 1 fjeanne utilisa. du domaine 0 Mar 31 16:18 empty.txt", - ) - .unwrap(); + let (ft, size, name) = + parse_ls_line("-rw-r--r-- 1 fjeanne utilisa. du domaine 0 Mar 31 16:18 empty.txt") + .unwrap(); assert_eq!(ft, '-'); assert_eq!(size, 0); assert_eq!(name, "empty.txt"); @@ -433,10 +488,9 @@ mod tests { #[test] fn test_parse_ls_line_dir_with_space_in_group() { - let (ft, size, name) = parse_ls_line( - "drwxr-xr-x 2 fjeanne utilisa. du domaine 64 Mar 31 16:18 my dir", - ) - .unwrap(); + let (ft, size, name) = + parse_ls_line("drwxr-xr-x 2 fjeanne utilisa. du domaine 64 Mar 31 16:18 my dir") + .unwrap(); assert_eq!(ft, 'd'); assert_eq!(size, 64); assert_eq!(name, "my dir"); @@ -444,15 +498,47 @@ mod tests { #[test] fn test_parse_ls_line_symlink() { - let (ft, size, name) = parse_ls_line( - "lrwxr-xr-x 1 user staff 10 Jan 1 12:00 link -> target", - ) - .unwrap(); + let (ft, size, name) = + parse_ls_line("lrwxr-xr-x 1 user staff 10 Jan 1 12:00 link -> target").unwrap(); assert_eq!(ft, 'l'); assert_eq!(size, 10); assert_eq!(name, "link -> target"); } + #[test] + fn test_compact_device_files() { + // Regression test for #844: `rtk ls /dev/ttyACM*` returned "(empty)" + // because character devices (type 'c') were not handled by compact_ls. + let input = "crw-rw---- 1 root dialout 166, 0 Apr 22 09:46 /dev/ttyACM0\n"; + let (entries, _summary, _parsed) = compact_ls(input, false); + assert!( + entries.contains("/dev/ttyACM0"), + "should contain device file, got: {entries}" + ); + assert!(!entries.contains("(empty)"), "should not be empty"); + } + + #[test] + fn test_compact_device_files_macos_hex_size() { + // macOS shows device major/minor as hex (e.g. 0x2000000) + let input = "crw-rw-rw- 1 root wheel 0x2000000 Mar 31 19:25 /dev/tty\n"; + let (entries, _summary, _parsed) = compact_ls(input, false); + assert!( + entries.contains("/dev/tty"), + "should contain device file, got: {entries}" + ); + } + + #[test] + fn test_compact_block_device() { + let input = "brw-rw---- 1 root disk 8, 0 Apr 22 09:46 /dev/sda\n"; + let (entries, _summary, _parsed) = compact_ls(input, false); + assert!( + entries.contains("/dev/sda"), + "should contain block device, got: {entries}" + ); + } + #[test] fn test_parse_ls_line_returns_none_for_total() { assert!(parse_ls_line("total 48").is_none()); @@ -460,12 +546,21 @@ mod tests { #[test] fn test_parse_ls_line_year_format() { - let (ft, size, name) = parse_ls_line( - "-rw-r--r-- 1 user staff 5678 Dec 25 2024 old.tar.gz", - ) - .unwrap(); + let (ft, size, name) = + parse_ls_line("-rw-r--r-- 1 user staff 5678 Dec 25 2024 old.tar.gz").unwrap(); assert_eq!(ft, '-'); assert_eq!(size, 5678); assert_eq!(name, "old.tar.gz"); } + + #[test] + fn test_compact_chinese_locale_fallback() { + let input = "total 8\n\ + drwxr-xr-x 2 user staff 64 1月 1 12:00 src\n\ + -rw-r--r-- 1 user staff 1234 1月 1 12:00 main.rs\n"; + let (entries, summary, parsed_count) = compact_ls(input, false); + assert_eq!(parsed_count, 0); + assert!(entries.is_empty()); + assert!(summary.is_empty()); + } } diff --git a/src/core/stream.rs b/src/core/stream.rs index 0ef451468..f651e9dde 100644 --- a/src/core/stream.rs +++ b/src/core/stream.rs @@ -1,9 +1,11 @@ use anyhow::{Context, Result}; -use regex::Regex; use std::io::{self, BufRead, BufReader, BufWriter, Write}; use std::process::{Command, Stdio}; use std::sync::mpsc; +#[cfg(test)] +use regex::Regex; + pub trait StreamFilter { fn feed_line(&mut self, line: &str) -> Option; fn flush(&mut self) -> String; @@ -83,7 +85,7 @@ impl StreamFilter for BlockStreamFilter { } } -#[allow(dead_code)] // available for command modules; currently used in tests only +#[cfg(test)] // available for command modules; currently used in tests only pub struct RegexBlockFilter { start_re: Regex, skip_prefixes: Vec, @@ -91,6 +93,7 @@ pub struct RegexBlockFilter { block_count: usize, } +#[cfg(test)] impl RegexBlockFilter { pub fn new(tool_name: &str, start_pattern: &str) -> Self { Self { @@ -103,13 +106,11 @@ impl RegexBlockFilter { } } - #[allow(dead_code)] pub fn skip_prefix(mut self, prefix: &str) -> Self { self.skip_prefixes.push(prefix.to_string()); self } - #[allow(dead_code)] pub fn skip_prefixes(mut self, prefixes: &[&str]) -> Self { self.skip_prefixes .extend(prefixes.iter().map(|s| s.to_string())); @@ -117,6 +118,7 @@ impl RegexBlockFilter { } } +#[cfg(test)] impl BlockHandler for RegexBlockFilter { fn should_skip(&mut self, line: &str) -> bool { self.skip_prefixes.iter().any(|p| line.starts_with(p)) @@ -152,30 +154,9 @@ pub trait StdinFilter: Send { fn flush(&mut self) -> String; } -#[allow(dead_code)] // test utility: wraps closures as StreamFilter -pub struct LineFilter Option> { - f: F, -} - -#[allow(dead_code)] -impl Option> LineFilter { - pub fn new(f: F) -> Self { - Self { f } - } -} - -impl Option> StreamFilter for LineFilter { - fn feed_line(&mut self, line: &str) -> Option { - (self.f)(line) - } - - fn flush(&mut self) -> String { - String::new() - } -} - pub enum FilterMode<'a> { Streaming(Box), + #[allow(dead_code)] Buffered(Box String + 'a>), CaptureOnly, Passthrough, @@ -197,7 +178,7 @@ pub struct StreamResult { } impl StreamResult { - #[allow(dead_code)] + #[cfg(test)] pub fn success(&self) -> bool { self.exit_code == 0 } @@ -344,7 +325,7 @@ pub fn run_streaming( }; if is_stderr { if !capped_err { - if raw_stderr.len() + line.len() + 1 <= RAW_CAP { + if raw_stderr.len() + line.len() < RAW_CAP { raw_stderr.push_str(&line); raw_stderr.push('\n'); } else { @@ -353,7 +334,7 @@ pub fn run_streaming( } } } else if !capped_out { - if raw_stdout.len() + line.len() + 1 <= RAW_CAP { + if raw_stdout.len() + line.len() < RAW_CAP { raw_stdout.push_str(&line); raw_stdout.push('\n'); } else { @@ -394,7 +375,7 @@ pub fn run_streaming( let mut raw_err = String::new(); let mut capped = false; for line in BufReader::new(stderr).lines().map_while(Result::ok) { - if raw_err.len() + line.len() + 1 <= RAW_CAP { + if raw_err.len() + line.len() < RAW_CAP { raw_err.push_str(&line); raw_err.push('\n'); } else if !capped { @@ -413,7 +394,7 @@ pub fn run_streaming( FilterMode::Streaming(_) => unreachable!("handled by is_streaming branch"), FilterMode::Buffered(filter_fn) => { for line in BufReader::new(stdout).lines().map_while(Result::ok) { - if raw_stdout.len() + line.len() + 1 <= RAW_CAP { + if raw_stdout.len() + line.len() < RAW_CAP { raw_stdout.push_str(&line); raw_stdout.push('\n'); } else if !capped_out { @@ -438,7 +419,7 @@ pub fn run_streaming( } FilterMode::CaptureOnly => { for line in BufReader::new(stdout).lines().map_while(Result::ok) { - if raw_stdout.len() + line.len() + 1 <= RAW_CAP { + if raw_stdout.len() + line.len() < RAW_CAP { raw_stdout.push_str(&line); raw_stdout.push('\n'); } else if !capped_out { @@ -522,6 +503,26 @@ pub(crate) mod tests { use super::*; use std::process::Command; + struct LineFilter Option> { + f: F, + } + + impl Option> LineFilter { + pub fn new(f: F) -> Self { + Self { f } + } + } + + impl Option> StreamFilter for LineFilter { + fn feed_line(&mut self, line: &str) -> Option { + (self.f)(line) + } + + fn flush(&mut self) -> String { + String::new() + } + } + #[test] fn test_exit_code_zero() { let status = Command::new("true").status().unwrap(); diff --git a/src/core/tracking.rs b/src/core/tracking.rs index 7bb3e575f..359e4f688 100644 --- a/src/core/tracking.rs +++ b/src/core/tracking.rs @@ -335,6 +335,7 @@ impl Tracker { Ok(tracker) } + #[cfg(test)] fn init_schema(&self) -> Result<()> { self.conn.execute( "CREATE TABLE IF NOT EXISTS commands ( diff --git a/src/discover/lexer.rs b/src/discover/lexer.rs index 8a126530a..5fb366f3b 100644 --- a/src/discover/lexer.rs +++ b/src/discover/lexer.rs @@ -306,6 +306,7 @@ pub fn split_on_operators(cmd: &str, stop_at_pipe: bool) -> Vec<&str> { results } +#[cfg(test)] pub fn strip_quotes(s: &str) -> String { let chars: Vec = s.chars().collect(); if chars.len() >= 2 diff --git a/src/discover/registry.rs b/src/discover/registry.rs index e40ff08b1..3921089ca 100644 --- a/src/discover/registry.rs +++ b/src/discover/registry.rs @@ -63,12 +63,16 @@ lazy_static! { // --git-dir , --work-tree , and flag-only options (#163) static ref GIT_GLOBAL_OPT: Regex = Regex::new(r"^(?:(?:-C\s+\S+|-c\s+\S+|--git-dir(?:=\S+|\s+\S+)|--work-tree(?:=\S+|\s+\S+)|--no-pager|--no-optional-locks|--bare|--literal-pathspecs)\s+)+").unwrap(); - static ref HEAD_N: Regex = Regex::new(r"^head\s+-(\d+)\s+(.+)$").unwrap(); - static ref HEAD_LINES: Regex = Regex::new(r"^head\s+--lines=(\d+)\s+(.+)$").unwrap(); - static ref TAIL_N: Regex = Regex::new(r"^tail\s+-(\d+)\s+(.+)$").unwrap(); - static ref TAIL_N_SPACE: Regex = Regex::new(r"^tail\s+-n\s+(\d+)\s+(.+)$").unwrap(); - static ref TAIL_LINES_EQ: Regex = Regex::new(r"^tail\s+--lines=(\d+)\s+(.+)$").unwrap(); - static ref TAIL_LINES_SPACE: Regex = Regex::new(r"^tail\s+--lines\s+(\d+)\s+(.+)$").unwrap(); + // Issue #1362: each capture expects a SINGLE file argument (`\S+$`). Multi-file + // invocations like `head -3 a b c` fail to match so the segment is passed through + // to the native `head`/`tail` binary — which already handles multi-file with + // `==> name <==` banners that `rtk read --max-lines` cannot reproduce. + static ref HEAD_N: Regex = Regex::new(r"^head\s+-(\d+)\s+(\S+)$").unwrap(); + static ref HEAD_LINES: Regex = Regex::new(r"^head\s+--lines=(\d+)\s+(\S+)$").unwrap(); + static ref TAIL_N: Regex = Regex::new(r"^tail\s+-(\d+)\s+(\S+)$").unwrap(); + static ref TAIL_N_SPACE: Regex = Regex::new(r"^tail\s+-n\s+(\d+)\s+(\S+)$").unwrap(); + static ref TAIL_LINES_EQ: Regex = Regex::new(r"^tail\s+--lines=(\d+)\s+(\S+)$").unwrap(); + static ref TAIL_LINES_SPACE: Regex = Regex::new(r"^tail\s+--lines\s+(\d+)\s+(\S+)$").unwrap(); } const GOLANGCI_GLOBAL_OPT_WITH_VALUE: &[&str] = &[ @@ -660,10 +664,8 @@ fn rewrite_segment_inner(seg: &str, excluded: &[ExcludePattern], depth: usize) - if rest.is_empty() { return None; } - return match rewrite_segment_inner(rest, excluded, depth + 1) { - Some(rewritten) => Some(format!("{} {}", prefix, rewritten)), - None => None, - }; + return rewrite_segment_inner(rest, excluded, depth + 1) + .map(|rewritten| format!("{} {}", prefix, rewritten)); } } @@ -1714,6 +1716,48 @@ mod tests { assert_eq!(rewrite_command("tail src/main.rs", &[]), None); } + // --- Issue #1362: head/tail with multiple files falls back to native command --- + // + // `rtk read --max-lines N` only accepts a single positional file path in + // a shape that maps cleanly to `head -N`. Rewriting `head -N a b c` to + // `rtk read a b c --max-lines N` previously produced a command where `rtk read` + // would concatenate the files without the `==> name <==` banners that native + // `head` emits, so the fix is to skip the rewrite and let the shell run the + // real `head`/`tail` binary. + + #[test] + fn test_rewrite_head_numeric_flag_multi_file_skipped() { + assert_eq!(rewrite_command("head -3 /tmp/a /tmp/b /tmp/c", &[]), None); + } + + #[test] + fn test_rewrite_head_lines_long_flag_multi_file_skipped() { + assert_eq!( + rewrite_command("head --lines=50 src/main.rs src/lib.rs", &[]), + None + ); + } + + #[test] + fn test_rewrite_tail_numeric_flag_multi_file_skipped() { + assert_eq!(rewrite_command("tail -20 a.log b.log", &[]), None); + } + + #[test] + fn test_rewrite_tail_n_space_flag_multi_file_skipped() { + assert_eq!(rewrite_command("tail -n 12 a.log b.log c.log", &[]), None); + } + + #[test] + fn test_rewrite_tail_lines_eq_multi_file_skipped() { + assert_eq!(rewrite_command("tail --lines=7 a.log b.log", &[]), None); + } + + #[test] + fn test_rewrite_tail_lines_space_multi_file_skipped() { + assert_eq!(rewrite_command("tail --lines 7 a.log b.log", &[]), None); + } + // --- New registry entries --- #[test] diff --git a/src/discover/report.rs b/src/discover/report.rs index 128ecb45e..2af129a5c 100644 --- a/src/discover/report.rs +++ b/src/discover/report.rs @@ -1,6 +1,6 @@ //! Data types for reporting which commands RTK can and cannot optimize. -use crate::hooks::constants::{HOOKS_SUBDIR, REWRITE_HOOK_FILE}; +use crate::hooks::constants::{CURSOR_DIR, HOOKS_SUBDIR, REWRITE_HOOK_FILE}; use serde::Serialize; /// RTK support status for a command. @@ -171,7 +171,7 @@ pub fn format_text(report: &DiscoverReport, limit: usize, verbose: bool) -> Stri // Cursor note: check if Cursor hooks are installed if let Some(home) = dirs::home_dir() { let cursor_hook = home - .join(".cursor") + .join(CURSOR_DIR) .join(HOOKS_SUBDIR) .join(REWRITE_HOOK_FILE); if cursor_hook.exists() { diff --git a/src/hooks/constants.rs b/src/hooks/constants.rs index fcacdb3e5..1d9f33ccd 100644 --- a/src/hooks/constants.rs +++ b/src/hooks/constants.rs @@ -13,7 +13,11 @@ pub const CLAUDE_HOOK_COMMAND: &str = "rtk hook claude"; /// Native Rust hook command for Cursor (replaces rtk-rewrite.sh). pub const CURSOR_HOOK_COMMAND: &str = "rtk hook cursor"; -pub const OPENCODE_PLUGIN_PATH: &str = ".config/opencode/plugins/rtk.ts"; +pub const CONFIG_DIR: &str = ".config"; +pub const OPENCODE_SUBDIR: &str = "opencode"; +pub const PLUGIN_SUBDIR: &str = "plugins"; +pub const OPENCODE_PLUGIN_FILE: &str = "rtk.ts"; + pub const CURSOR_DIR: &str = ".cursor"; pub const CODEX_DIR: &str = ".codex"; pub const GEMINI_DIR: &str = ".gemini"; diff --git a/src/hooks/hook_check.rs b/src/hooks/hook_check.rs index 12f49755e..ce288ba99 100644 --- a/src/hooks/hook_check.rs +++ b/src/hooks/hook_check.rs @@ -4,8 +4,6 @@ use super::constants::{ CLAUDE_DIR, CLAUDE_HOOK_COMMAND, HOOKS_SUBDIR, PRE_TOOL_USE_KEY, REWRITE_HOOK_FILE, SETTINGS_JSON, }; -#[cfg(test)] -use super::constants::{CODEX_DIR, CURSOR_DIR, GEMINI_DIR, GEMINI_HOOK_FILE, OPENCODE_PLUGIN_PATH}; use crate::core::constants::RTK_DATA_DIR; use std::path::PathBuf; @@ -135,21 +133,6 @@ pub fn parse_hook_version(content: &str) -> u8 { 0 // No version tag = version 0 (outdated) } -#[cfg(test)] -fn other_integration_installed(home: &std::path::Path) -> bool { - let paths = [ - home.join(OPENCODE_PLUGIN_PATH), - home.join(CURSOR_DIR) - .join(HOOKS_SUBDIR) - .join(REWRITE_HOOK_FILE), - home.join(CODEX_DIR).join("AGENTS.md"), - home.join(GEMINI_DIR) - .join(HOOKS_SUBDIR) - .join(GEMINI_HOOK_FILE), - ]; - paths.iter().any(|p| p.exists()) -} - fn hook_installed_path() -> Option { let home = dirs::home_dir()?; let path = home @@ -171,6 +154,27 @@ fn warn_marker_path() -> Option { #[cfg(test)] mod tests { use super::*; + use crate::hooks::constants::{ + CODEX_DIR, CONFIG_DIR, CURSOR_DIR, GEMINI_DIR, GEMINI_HOOK_FILE, OPENCODE_PLUGIN_FILE, + OPENCODE_SUBDIR, PLUGIN_SUBDIR, + }; + + fn other_integration_installed(home: &std::path::Path) -> bool { + let paths = [ + home.join(CONFIG_DIR) + .join(OPENCODE_SUBDIR) + .join(PLUGIN_SUBDIR) + .join(OPENCODE_PLUGIN_FILE), + home.join(CURSOR_DIR) + .join(HOOKS_SUBDIR) + .join(REWRITE_HOOK_FILE), + home.join(CODEX_DIR).join("AGENTS.md"), + home.join(GEMINI_DIR) + .join(HOOKS_SUBDIR) + .join(GEMINI_HOOK_FILE), + ]; + paths.iter().any(|p| p.exists()) + } #[test] fn test_parse_hook_version_present() { @@ -215,7 +219,12 @@ mod tests { #[test] fn test_other_integration_opencode() { let tmp = tempfile::tempdir().expect("tempdir"); - let path = tmp.path().join(OPENCODE_PLUGIN_PATH); + let path = tmp + .path() + .join(CONFIG_DIR) + .join(OPENCODE_SUBDIR) + .join(PLUGIN_SUBDIR) + .join(OPENCODE_PLUGIN_FILE); std::fs::create_dir_all(path.parent().unwrap()).unwrap(); std::fs::write(&path, b"plugin").unwrap(); assert!(other_integration_installed(tmp.path())); diff --git a/src/hooks/init.rs b/src/hooks/init.rs index c83185b90..c6bd05c2b 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -6,6 +6,10 @@ use std::io::Write; use std::path::{Path, PathBuf}; use tempfile::NamedTempFile; +use crate::hooks::constants::{ + CONFIG_DIR, CURSOR_DIR, GEMINI_DIR, OPENCODE_PLUGIN_FILE, OPENCODE_SUBDIR, PLUGIN_SUBDIR, +}; + use super::constants::{ BEFORE_TOOL_KEY, CLAUDE_DIR, CLAUDE_HOOK_COMMAND, CODEX_DIR, CURSOR_HOOK_COMMAND, GEMINI_HOOK_FILE, HOOKS_JSON, HOOKS_SUBDIR, PRE_TOOL_USE_KEY, REWRITE_HOOK_FILE, SETTINGS_JSON, @@ -56,6 +60,9 @@ const AGENTS_MD: &str = "AGENTS.md"; const RTK_MD_REF: &str = "@RTK.md"; const GEMINI_MD: &str = "GEMINI.md"; +const RTK_BLOCK_START: &str = ""; + /// Control flow for settings.json patching #[derive(Debug, Clone, Copy, PartialEq)] pub enum PatchMode { @@ -157,7 +164,7 @@ rtk prisma # Prisma without ASCII art (88%) ```bash rtk ls # Tree format, compact (65%) rtk read # Code reading with filtering (60%) -rtk grep # Search grouped by file (75%) +rtk grep # Search grouped by file (75%). Format flags (-c, -l, -L, -o, -Z) run raw. rtk find # Find grouped by directory (70%) ``` @@ -608,21 +615,48 @@ pub fn uninstall(global: bool, gemini: bool, codex: bool, cursor: bool, verbose: let content = fs::read_to_string(&claude_md_path) .with_context(|| format!("Failed to read CLAUDE.md: {}", claude_md_path.display()))?; - if content.contains(RTK_MD_REF) { - let new_content = content + let mut claude_md_changed = false; + let mut working_content = content.clone(); + + if working_content.contains(RTK_MD_REF) { + let new_content = working_content .lines() .filter(|line| !line.trim().starts_with(RTK_MD_REF)) .collect::>() .join("\n"); - // Clean up double blanks - let cleaned = clean_double_blanks(&new_content); - - fs::write(&claude_md_path, cleaned).with_context(|| { - format!("Failed to write CLAUDE.md: {}", claude_md_path.display()) - })?; + working_content = clean_double_blanks(&new_content); + claude_md_changed = true; removed.push("CLAUDE.md: removed @RTK.md reference".to_string()); } + + if working_content.contains(RTK_BLOCK_START) { + let (cleaned, did_remove) = remove_rtk_block(&working_content); + if did_remove { + working_content = cleaned; + claude_md_changed = true; + removed.push("CLAUDE.md: removed rtk-instructions block".to_string()); + } + } + + if claude_md_changed { + let trimmed = working_content.trim(); + if trimmed.is_empty() { + // nosemgrep: filesystem-deletion + fs::remove_file(&claude_md_path).with_context(|| { + format!( + "Failed to remove empty CLAUDE.md: {}", + claude_md_path.display() + ) + })?; + removed.retain(|r| !r.starts_with("CLAUDE.md:")); + removed.push("CLAUDE.md: removed (was empty after cleanup)".to_string()); + } else { + fs::write(&claude_md_path, &working_content).with_context(|| { + format!("Failed to write CLAUDE.md: {}", claude_md_path.display()) + })?; + } + } } // 4. Remove hook entry from settings.json @@ -643,6 +677,10 @@ pub fn uninstall(global: bool, gemini: bool, codex: bool, cursor: bool, verbose: // Report results if removed.is_empty() { println!("RTK was not installed (nothing to remove)"); + println!(" Checked: {}", hook_path.display()); + println!(" Checked: {}", claude_dir.join(RTK_MD).display()); + println!(" Checked: {}", claude_md_path.display()); + println!(" Checked: {}", claude_dir.join(SETTINGS_JSON).display()); } else { println!("RTK uninstalled:"); for item in removed { @@ -691,6 +729,29 @@ fn uninstall_codex_at(codex_dir: &Path, verbose: u8) -> Result> { } let agents_md_path = codex_dir.join(AGENTS_MD); + if agents_md_path.exists() { + let content = fs::read_to_string(&agents_md_path) + .with_context(|| format!("Failed to read AGENTS.md: {}", agents_md_path.display()))?; + + let mut working_content = content.clone(); + let mut agents_changed = false; + + if working_content.contains(RTK_BLOCK_START) { + let (cleaned, did_remove) = remove_rtk_block(&working_content); + if did_remove { + working_content = cleaned; + agents_changed = true; + removed.push("AGENTS.md: removed rtk-instructions block".to_string()); + } + } + + if agents_changed { + atomic_write(&agents_md_path, &working_content).with_context(|| { + format!("Failed to write AGENTS.md: {}", agents_md_path.display()) + })?; + } + } + if remove_rtk_reference_from_agents( &agents_md_path, &[RTK_MD_REF, absolute_rtk_md_ref.as_str()], @@ -986,7 +1047,7 @@ fn migrate_old_hook_script(verbose: u8) { let _ = std::fs::remove_file(&hash_file); } // Remove Cursor legacy hook - let cursor_hook = home.join(".cursor").join("hooks").join(REWRITE_HOOK_FILE); + let cursor_hook = home.join(CURSOR_DIR).join("hooks").join(REWRITE_HOOK_FILE); if cursor_hook.exists() { let _ = std::fs::remove_file(&cursor_hook); } @@ -1213,14 +1274,15 @@ fn run_claude_md_mode(global: bool, verbose: u8, install_opencode: bool) -> Resu } RtkBlockUpsert::Malformed => { eprintln!( - "[warn] Warning: Found '"; + let start_marker = RTK_BLOCK_START; + let end_marker = RTK_BLOCK_END; if let Some(start) = content.find(start_marker) { if let Some(relative_end) = content[start..].find(end_marker) { @@ -1545,7 +1607,7 @@ fn patch_claude_md(path: &Path, verbose: u8) -> Result { let mut migrated = false; // Check for old block and migrate - if content.contains(""), - ) { - let end_pos = end + "".len(); + if let (Some(start), Some(end)) = (content.find(RTK_BLOCK_START), content.find(RTK_BLOCK_END)) { + let end_pos = end + RTK_BLOCK_END.len(); let before = content[..start].trim_end(); let after = content[end_pos..].trim_start(); let result = if after.is_empty() { - before.to_string() + format!("{}\n", before) } else { format!("{}\n\n{}", before, after) }; (result, true) // migrated - } else if content.contains(" -OLD RTK STUFF - - -More content"#; + let input = format!( + "# My Config\n\n{} v2 -->\nOLD RTK STUFF\n{}\n\nMore content", + RTK_BLOCK_START, RTK_BLOCK_END + ); - let (result, migrated) = remove_rtk_block(input); + let (result, migrated) = remove_rtk_block(&input); assert!(migrated); assert!(!result.contains("OLD RTK STUFF")); assert!(result.contains("# My Config")); @@ -2759,8 +2825,8 @@ More content"#; #[test] fn test_migration_warns_on_missing_end_marker() { - let input = "\nOLD STUFF\nNo end marker"; - let (result, migrated) = remove_rtk_block(input); + let input = format!("{} v2 -->\nOLD STUFF\nNo end marker", RTK_BLOCK_START); + let (result, migrated) = remove_rtk_block(&input); assert!(!migrated); assert_eq!(result, input); } @@ -2780,9 +2846,9 @@ More content"#; #[test] fn test_claude_md_mode_creates_full_injection() { // Just verify RTK_INSTRUCTIONS constant has the right content - assert!(RTK_INSTRUCTIONS.contains("")); + assert!(RTK_INSTRUCTIONS.contains(RTK_BLOCK_END)); assert!(RTK_INSTRUCTIONS.len() > 4000); } @@ -2794,21 +2860,17 @@ More content"#; let (content, action) = upsert_rtk_block(input, RTK_INSTRUCTIONS); assert_eq!(action, RtkBlockUpsert::Added); assert!(content.contains("# Team instructions")); - assert!(content.contains(" -OLD RTK CONTENT - - -More notes -"#; + let input = format!( + "# Team instructions\n\n{} v1 -->\nOLD RTK CONTENT\n{}\n\nMore notes\n", + RTK_BLOCK_START, RTK_BLOCK_END + ); - let (content, action) = upsert_rtk_block(input, RTK_INSTRUCTIONS); + let (content, action) = upsert_rtk_block(&input, RTK_INSTRUCTIONS); assert_eq!(action, RtkBlockUpsert::Updated); assert!(!content.contains("OLD RTK CONTENT")); assert!(content.contains("rtk cargo test")); // from current RTK_INSTRUCTIONS @@ -2829,8 +2891,8 @@ More notes #[test] fn test_upsert_rtk_block_detects_malformed_block() { - let input = "\npartial"; - let (content, action) = upsert_rtk_block(input, RTK_INSTRUCTIONS); + let input = format!("{} v2 -->\npartial", RTK_BLOCK_START); + let (content, action) = upsert_rtk_block(&input, RTK_INSTRUCTIONS); assert_eq!(action, RtkBlockUpsert::Malformed); assert_eq!(content, input); } @@ -2975,7 +3037,10 @@ More notes let agents_md = temp.path().join("AGENTS.md"); fs::write( &agents_md, - "# Team rules\n\n\nold\n\n", + format!( + "# Team rules\n\n{} v2 -->\nold\n{}\n", + RTK_BLOCK_START, RTK_BLOCK_END + ), ) .unwrap(); @@ -3060,6 +3125,32 @@ More notes assert!(content.contains("# Team rules")); } + #[test] + fn test_uninstall_codex_at_removes_rtk_instructions_block() { + let temp = TempDir::new().unwrap(); + let codex_dir = temp.path(); + let agents_md = codex_dir.join("AGENTS.md"); + let rtk_md = codex_dir.join("RTK.md"); + + fs::write( + &agents_md, + format!( + "# Team rules\n\n{} v2 -->\nOLD RTK STUFF\n{}\n\nMore content", + RTK_BLOCK_START, RTK_BLOCK_END + ), + ) + .unwrap(); + fs::write(&rtk_md, "codex config").unwrap(); + + let removed = uninstall_codex_at(codex_dir, 0).unwrap(); + + let content = fs::read_to_string(&agents_md).unwrap(); + assert!(!content.contains("OLD RTK STUFF")); + assert!(content.contains("# Team rules")); + assert!(content.contains("More content")); + assert!(removed.iter().any(|r| r.contains("rtk-instructions block"))); + } + #[test] fn test_local_init_unchanged() { // Local init should use claude-md mode @@ -3069,7 +3160,7 @@ More notes fs::write(&claude_md, RTK_INSTRUCTIONS).unwrap(); let content = fs::read_to_string(&claude_md).unwrap(); - assert!(content.contains("