fix(core): error handl, RAW_CAP, recursion guard, atomic writes

aeppling · aeppling · commit 0d07f04d03dd · 2026-04-05T14:09:13.000+02:00
diff --git a/src/core/stream.rs b/src/core/stream.rs
@@ -152,7 +152,7 @@ pub fn run_streaming(
         match stdout_mode {
             FilterMode::Passthrough => {
                 for line in BufReader::new(stdout).lines().map_while(Result::ok) {
-                    if raw_stdout.len() < RAW_CAP {
+                    if raw_stdout.len() + line.len() + 1 <= RAW_CAP {
                         raw_stdout.push_str(&line);
                         raw_stdout.push('\n');
                     }
@@ -166,7 +166,7 @@ pub fn run_streaming(
             }
             FilterMode::Streaming(mut filter) => {
                 for line in BufReader::new(stdout).lines().map_while(Result::ok) {
-                    if raw_stdout.len() < RAW_CAP {
+                    if raw_stdout.len() + line.len() + 1 <= RAW_CAP {
                         raw_stdout.push_str(&line);
                         raw_stdout.push('\n');
                     }
@@ -189,22 +189,21 @@ pub fn run_streaming(
             }
             FilterMode::Buffered(filter_fn) => {
                 for line in BufReader::new(stdout).lines().map_while(Result::ok) {
-                    if raw_stdout.len() < RAW_CAP {
+                    if raw_stdout.len() + line.len() + 1 <= RAW_CAP {
                         raw_stdout.push_str(&line);
                         raw_stdout.push('\n');
                     }
                 }
-                let result = filter_fn(&raw_stdout);
-                filtered = result.clone();
-                match write!(out, "{}", result) {
+                filtered = filter_fn(&raw_stdout);
+                match write!(out, "{}", filtered) {
                     Err(e) if e.kind() == io::ErrorKind::BrokenPipe => {}
                     Err(e) => return Err(e.into()),
                     Ok(_) => {}
                 }
             }
             FilterMode::CaptureOnly => {
                 for line in BufReader::new(stdout).lines().map_while(Result::ok) {
-                    if raw_stdout.len() < RAW_CAP {
+                    if raw_stdout.len() + line.len() + 1 <= RAW_CAP {
                         raw_stdout.push_str(&line);
                         raw_stdout.push('\n');
                     }
@@ -214,7 +213,10 @@ pub fn run_streaming(
         }
     }
 
-    let raw_stderr = stderr_thread.join().unwrap_or_else(|_| String::new());
+    let raw_stderr = stderr_thread.join().unwrap_or_else(|e| {
+        eprintln!("[rtk] warning: stderr reader thread panicked: {:?}", e);
+        String::new()
+    });
     if let Some(t) = stdin_thread {
         t.join().ok();
     }
diff --git a/src/discover/registry.rs b/src/discover/registry.rs
@@ -485,24 +485,28 @@ fn rewrite_line_range(cmd: &str) -> Option<String> {
 /// but don't change which command runs. Strip before routing, re-prepend after.
 const SHELL_PREFIX_BUILTINS: &[&str] = &["noglob", "command", "builtin", "exec", "nocorrect"];
 
-/// Rewrite a single (non-compound) command segment.
-/// Returns `Some(rewritten)` if matched (including already-RTK pass-through).
-/// Returns `None` if no match (caller uses original segment).
+const MAX_PREFIX_DEPTH: usize = 10;
+
 fn rewrite_segment(seg: &str, excluded: &[String]) -> Option<String> {
+    rewrite_segment_inner(seg, excluded, 0)
+}
+
+fn rewrite_segment_inner(seg: &str, excluded: &[String], depth: usize) -> Option<String> {
     let trimmed = seg.trim();
     if trimmed.is_empty() {
         return None;
     }
 
-    // Peel shell prefix builtins (noglob, command, builtin, exec, nocorrect)
-    // before routing, re-prepend after.
+    if depth >= MAX_PREFIX_DEPTH {
+        return None;
+    }
+
     for &prefix in SHELL_PREFIX_BUILTINS {
         if let Some(rest) = strip_word_prefix(trimmed, prefix) {
             if rest.is_empty() {
-                return None; // bare "noglob" etc. — nothing to rewrite
+                return None;
             }
-            // Recursively rewrite the inner command
-            return match rewrite_segment(rest, excluded) {
+            return match rewrite_segment_inner(rest, excluded, depth + 1) {
                 Some(rewritten) => Some(format!("{} {}", prefix, rewritten)),
                 None => None,
             };
diff --git a/src/hooks/init.rs b/src/hooks/init.rs
@@ -302,15 +302,15 @@ fn write_if_changed(path: &Path, content: &str, name: &str, verbose: u8) -> Resu
             }
             Ok(false)
         } else {
-            fs::write(path, content)
+            atomic_write(path, content)
                 .with_context(|| format!("Failed to write {}: {}", name, path.display()))?;
             if verbose > 0 {
                 eprintln!("Updated {}: {}", name, path.display());
             }
             Ok(true)
         }
     } else {
-        fs::write(path, content)
+        atomic_write(path, content)
             .with_context(|| format!("Failed to write {}: {}", name, path.display()))?;
         if verbose > 0 {
             eprintln!("Created {}: {}", name, path.display());
@@ -682,8 +682,7 @@ fn patch_settings_json_command(
         }
     }
 
-    // Deep-merge hook
-    insert_hook_entry(&mut root, hook_command);
+    insert_hook_entry(&mut root, hook_command)?;
 
     // Backup original
     if settings_path.exists() {
@@ -748,38 +747,35 @@ fn clean_double_blanks(content: &str) -> String {
 
 /// Deep-merge RTK hook entry into settings.json
 /// Creates hooks.PreToolUse structure if missing, preserves existing hooks
-fn insert_hook_entry(root: &mut serde_json::Value, hook_command: &str) {
-    // Ensure root is an object
+fn insert_hook_entry(root: &mut serde_json::Value, hook_command: &str) -> Result<()> {
     let root_obj = match root.as_object_mut() {
         Some(obj) => obj,
         None => {
             *root = serde_json::json!({});
-            root.as_object_mut()
-                .expect("Just created object, must succeed")
+            root.as_object_mut().expect("just-created json object")
         }
     };
 
-    // Use entry() API for idiomatic insertion
     let hooks = root_obj
         .entry("hooks")
         .or_insert_with(|| serde_json::json!({}))
         .as_object_mut()
-        .expect("hooks must be an object");
+        .context("hooks value is not an object")?;
 
     let pre_tool_use = hooks
         .entry(PRE_TOOL_USE_KEY)
         .or_insert_with(|| serde_json::json!([]))
         .as_array_mut()
-        .expect("PreToolUse must be an array");
+        .context("PreToolUse value is not an array")?;
 
-    // Append RTK hook entry
     pre_tool_use.push(serde_json::json!({
         "matcher": "Bash",
         "hooks": [{
             "type": "command",
             "command": hook_command
         }]
     }));
+    Ok(())
 }
 
 /// Check if RTK hook is already present in settings.json
@@ -1622,8 +1618,7 @@ fn patch_cursor_hooks_json(path: &Path, verbose: u8) -> Result<bool> {
         return Ok(false);
     }
 
-    // Insert the RTK preToolUse entry
-    insert_cursor_hook_entry(&mut root);
+    insert_cursor_hook_entry(&mut root)?;
 
     // Backup if exists
     if path.exists() {
@@ -1664,35 +1659,34 @@ fn cursor_hook_already_present(root: &serde_json::Value) -> bool {
 }
 
 /// Insert RTK preToolUse entry into Cursor hooks.json
-fn insert_cursor_hook_entry(root: &mut serde_json::Value) {
+fn insert_cursor_hook_entry(root: &mut serde_json::Value) -> Result<()> {
     let root_obj = match root.as_object_mut() {
         Some(obj) => obj,
         None => {
             *root = serde_json::json!({ "version": 1 });
-            root.as_object_mut()
-                .expect("Just created object, must succeed")
+            root.as_object_mut().expect("just-created json object")
         }
     };
 
-    // Ensure version key
     root_obj.entry("version").or_insert(serde_json::json!(1));
 
     let hooks = root_obj
         .entry("hooks")
         .or_insert_with(|| serde_json::json!({}))
         .as_object_mut()
-        .expect("hooks must be an object");
+        .context("hooks value is not an object")?;
 
     let pre_tool_use = hooks
         .entry("preToolUse")
         .or_insert_with(|| serde_json::json!([]))
         .as_array_mut()
-        .expect("preToolUse must be an array");
+        .context("preToolUse value is not an array")?;
 
     pre_tool_use.push(serde_json::json!({
         "command": CURSOR_HOOK_COMMAND,
         "matcher": "Shell"
     }));
+    Ok(())
 }
 
 /// Remove Cursor RTK artifacts: hook script + hooks.json entry
@@ -2745,7 +2739,7 @@ More notes
         let mut json_content = serde_json::json!({});
         let hook_command = "/Users/test/.claude/hooks/rtk-rewrite.sh";
 
-        insert_hook_entry(&mut json_content, hook_command);
+        insert_hook_entry(&mut json_content, hook_command).unwrap();
 
         // Should create full structure
         assert!(json_content.get("hooks").is_some());
@@ -2777,7 +2771,7 @@ More notes
         });
 
         let hook_command = "/Users/test/.claude/hooks/rtk-rewrite.sh";
-        insert_hook_entry(&mut json_content, hook_command);
+        insert_hook_entry(&mut json_content, hook_command).unwrap();
 
         let pre_tool_use = json_content["hooks"]["PreToolUse"].as_array().unwrap();
         assert_eq!(pre_tool_use.len(), 2); // Should have both hooks
@@ -2800,7 +2794,7 @@ More notes
         });
 
         let hook_command = "/Users/test/.claude/hooks/rtk-rewrite.sh";
-        insert_hook_entry(&mut json_content, hook_command);
+        insert_hook_entry(&mut json_content, hook_command).unwrap();
 
         // Should preserve all other keys
         assert_eq!(json_content["env"]["PATH"], "/custom/path");
@@ -3003,7 +2997,7 @@ More notes
     #[test]
     fn test_insert_cursor_hook_entry_empty() {
         let mut json_content = serde_json::json!({ "version": 1 });
-        insert_cursor_hook_entry(&mut json_content);
+        insert_cursor_hook_entry(&mut json_content).unwrap();
 
         let hooks = json_content["hooks"]["preToolUse"].as_array().unwrap();
         assert_eq!(hooks.len(), 1);
@@ -3027,7 +3021,7 @@ More notes
             }
         });
 
-        insert_cursor_hook_entry(&mut json_content);
+        insert_cursor_hook_entry(&mut json_content).unwrap();
 
         let pre_tool_use = json_content["hooks"]["preToolUse"].as_array().unwrap();
         assert_eq!(pre_tool_use.len(), 2);
diff --git a/src/hooks/permissions.rs b/src/hooks/permissions.rs
@@ -98,6 +98,10 @@ fn load_permission_rules() -> (Vec<String>, Vec<String>, Vec<String>) {
             continue;
         };
         let Ok(json) = serde_json::from_str::<Value>(&content) else {
+            eprintln!(
+                "[rtk] warning: failed to parse permissions from {}",
+                path.display()
+            );
             continue;
         };
         let Some(permissions) = json.get("permissions") else {

Original file line number	Diff line number	Diff line change
`@@ -152,7 +152,7 @@ pub fn run_streaming(`
`152`	`152`	`match stdout_mode {`
`153`	`153`	`FilterMode::Passthrough => {`
`154`	`154`	`for line in BufReader::new(stdout).lines().map_while(Result::ok) {`
`155`		`- if raw_stdout.len() < RAW_CAP {`
	`155`	`+ if raw_stdout.len() + line.len() + 1 <= RAW_CAP {`
`156`	`156`	`raw_stdout.push_str(&line);`
`157`	`157`	`raw_stdout.push('\n');`
`158`	`158`	`}`
`@@ -166,7 +166,7 @@ pub fn run_streaming(`
`166`	`166`	`}`
`167`	`167`	`FilterMode::Streaming(mut filter) => {`
`168`	`168`	`for line in BufReader::new(stdout).lines().map_while(Result::ok) {`
`169`		`- if raw_stdout.len() < RAW_CAP {`
	`169`	`+ if raw_stdout.len() + line.len() + 1 <= RAW_CAP {`
`170`	`170`	`raw_stdout.push_str(&line);`
`171`	`171`	`raw_stdout.push('\n');`
`172`	`172`	`}`
`@@ -189,22 +189,21 @@ pub fn run_streaming(`
`189`	`189`	`}`
`190`	`190`	`FilterMode::Buffered(filter_fn) => {`
`191`	`191`	`for line in BufReader::new(stdout).lines().map_while(Result::ok) {`
`192`		`- if raw_stdout.len() < RAW_CAP {`
	`192`	`+ if raw_stdout.len() + line.len() + 1 <= RAW_CAP {`
`193`	`193`	`raw_stdout.push_str(&line);`
`194`	`194`	`raw_stdout.push('\n');`
`195`	`195`	`}`
`196`	`196`	`}`
`197`		`- let result = filter_fn(&raw_stdout);`
`198`		`- filtered = result.clone();`
`199`		`- match write!(out, "{}", result) {`
	`197`	`+ filtered = filter_fn(&raw_stdout);`
	`198`	`+ match write!(out, "{}", filtered) {`
`200`	`199`	`Err(e) if e.kind() == io::ErrorKind::BrokenPipe => {}`
`201`	`200`	`Err(e) => return Err(e.into()),`
`202`	`201`	`Ok(_) => {}`
`203`	`202`	`}`
`204`	`203`	`}`
`205`	`204`	`FilterMode::CaptureOnly => {`
`206`	`205`	`for line in BufReader::new(stdout).lines().map_while(Result::ok) {`
`207`		`- if raw_stdout.len() < RAW_CAP {`
	`206`	`+ if raw_stdout.len() + line.len() + 1 <= RAW_CAP {`
`208`	`207`	`raw_stdout.push_str(&line);`
`209`	`208`	`raw_stdout.push('\n');`
`210`	`209`	`}`
`@@ -214,7 +213,10 @@ pub fn run_streaming(`
`214`	`213`	`}`
`215`	`214`	`}`
`216`	`215`
`217`		`- let raw_stderr = stderr_thread.join().unwrap_or_else(\|_\| String::new());`
	`216`	`+ let raw_stderr = stderr_thread.join().unwrap_or_else(\|e\| {`
	`217`	`+ eprintln!("[rtk] warning: stderr reader thread panicked: {:?}", e);`
	`218`	`+ String::new()`
	`219`	`+ });`
`218`	`220`	`if let Some(t) = stdin_thread {`
`219`	`221`	`t.join().ok();`
`220`	`222`	`}`