compact: extract shared helpers, deduplicate hook scanning, toggle, and path collision logic

- Extract str_field and parse_json_or_string to types.rs (removed 3 duplicates)
- Merge near-identical hooks + _agentswitch_disabled blocks in scan_hook_entries
- Extract apply_json_toggle helper in toggler.rs
- Extract unique_path helper in chat.rs (4 collision loops → 1)
- All 15 tests pass, zero behavioral change

Author: RoyCoding8

src/chat.rs

@@ -1,4 +1,4 @@
-use crate::types::ProviderId;
+use crate::types::{str_field, ProviderId};
 use anyhow::{anyhow, Result};
 use serde::{Deserialize, Serialize};
 use serde_json::Value;
@@ -275,16 +275,33 @@ pub fn import_archive(path: &Path, project_dir: Option<&Path>) -> Result<PathBuf
         archive.source_provider.id(),
         archive.title
     ));
-    let mut target = dir.join(format!("{base}.{ARCHIVE_EXT}"));
-    let mut n = 2usize;
-    while target.exists() {
-        target = dir.join(format!("{base}-{n}.{ARCHIVE_EXT}"));
-        n += 1;
-    }
+    let target = unique_path(&dir, &base, ARCHIVE_EXT);
     fs::write(&target, serde_json::to_string_pretty(&archive)?)?;
     Ok(target)
 }
 
+fn unique_path(dir: &Path, stem: &str, ext: &str) -> PathBuf {
+    let first = if ext.is_empty() {
+        dir.join(stem)
+    } else {
+        dir.join(format!("{stem}.{ext}"))
+    };
+    if !first.exists() {
+        return first;
+    }
+    for n in 2.. {
+        let candidate = if ext.is_empty() {
+            dir.join(format!("{stem}-{n}"))
+        } else {
+            dir.join(format!("{stem}-{n}.{ext}"))
+        };
+        if !candidate.exists() {
+            return candidate;
+        }
+    }
+    unreachable!()
+}
+
 pub fn import_zip(path: &Path, project_dir: Option<&Path>) -> Result<BatchReport> {
     let file = File::open(path)?;
     let mut zip = ZipArchive::new(file)?;
@@ -318,12 +335,7 @@ pub fn import_zip(path: &Path, project_dir: Option<&Path>) -> Result<BatchReport
             archive.source_provider.id(),
             archive.title
         ));
-        let mut target = dir.join(format!("{base}.{ARCHIVE_EXT}"));
-        let mut n = 2usize;
-        while target.exists() {
-            target = dir.join(format!("{base}-{n}.{ARCHIVE_EXT}"));
-            n += 1;
-        }
+        let target = unique_path(&dir, &base, ARCHIVE_EXT);
         fs::write(&target, serde_json::to_string_pretty(&archive)?)?;
         report.ok += 1;
     }
@@ -351,12 +363,7 @@ pub fn soft_delete(session: &ChatSession, workspace: &Path) -> Result<()> {
         .extension()
         .and_then(|e| e.to_str())
         .unwrap_or("chat");
-    let mut target = trash_dir.join(format!("{stem}.{ext}"));
-    let mut n = 2usize;
-    while target.exists() {
-        target = trash_dir.join(format!("{stem}-{n}.{ext}"));
-        n += 1;
-    }
+    let target = unique_path(&trash_dir, &stem, ext);
     move_path(source, &target)?;
     let manifest = DeleteManifest {
         schema_version: ARCHIVE_VERSION,
@@ -390,12 +397,7 @@ fn soft_delete_kiro_session(session: &ChatSession) -> Result<()> {
     let trash_dir = trash_dir().join(session.provider.id());
     fs::create_dir_all(&trash_dir)?;
     let stem = safe_file_stem(&format!("{}-{}", session.id, session.title));
-    let mut target = trash_dir.join(&stem);
-    let mut n = 2usize;
-    while target.exists() {
-        target = trash_dir.join(format!("{stem}-{n}"));
-        n += 1;
-    }
+    let target = unique_path(&trash_dir, &stem, "");
     fs::create_dir_all(&target)?;
     let base = meta_path.with_extension("");
     for ext in ["json", "jsonl", "lock"] {
@@ -1177,11 +1179,6 @@ fn project_label_from_path(provider: ChatProvider, _root: &Path, path: &Path) ->
     }
 }
 
-fn str_field<'a>(value: &'a Value, keys: &[&str]) -> Option<&'a str> {
-    keys.iter()
-        .find_map(|key| value.get(*key).and_then(Value::as_str))
-}
-
 fn first_string(value: Option<&Value>) -> Option<&str> {
     value
         .and_then(Value::as_array)

src/diagnostics.rs

@@ -1,4 +1,7 @@
-use crate::{scanner, types::*};
+use crate::{
+    scanner,
+    types::{parse_json_or_string as parse_detail, str_field, *},
+};
 use serde_json::Value;
 use std::{
     collections::BTreeMap,
@@ -219,10 +222,6 @@ fn item_key(item: &ConfigItem) -> String {
     item.name.clone()
 }
 
-fn parse_detail(raw: &str) -> Value {
-    serde_json::from_str(raw).unwrap_or_else(|_| Value::String(raw.into()))
-}
-
 fn fingerprint(value: &Value) -> String {
     serde_json::to_string(&canonical(value, None, true)).unwrap_or_else(|_| value.to_string())
 }
@@ -340,11 +339,6 @@ fn hook_handler(value: &Value) -> Option<&str> {
         .or_else(|| str_field(value, &["name", "command", "cmd"]))
 }
 
-fn str_field<'a>(value: &'a Value, keys: &[&str]) -> Option<&'a str> {
-    keys.iter()
-        .find_map(|k| value.get(*k).and_then(|v| v.as_str()))
-}
-
 fn object_len(value: &Value, key: &str) -> Option<usize> {
     value.get(key).and_then(|v| v.as_object()).map(|o| o.len())
 }

src/hook_diag.rs

@@ -1,4 +1,7 @@
-use crate::{scanner, types::*};
+use crate::{
+    scanner,
+    types::{str_field, *},
+};
 use serde_json::Value;
 use std::{
     collections::HashMap,
@@ -103,7 +106,7 @@ pub fn build(provider: ProviderId, workspace: &Path) -> Vec<HookRow> {
 }
 
 fn row(item: ConfigItem, scope: Scope) -> HookRow {
-    let value = parse(item.detail.as_deref().unwrap_or(""));
+    let value = parse_json_or_string(item.detail.as_deref().unwrap_or(""));
     let loc = item.hook_loc.clone().unwrap_or(HookLoc {
         event: "hook".into(),
         index: 0,
@@ -259,12 +262,8 @@ fn timeout(value: &Value) -> Option<u64> {
         .or_else(|| num_field(value, &["timeout", "timeout_ms", "timeoutMs"]))
 }
 
-fn parse(raw: &str) -> Value {
-    serde_json::from_str(raw).unwrap_or_else(|_| Value::String(raw.into()))
-}
-fn str_field<'a>(v: &'a Value, keys: &[&str]) -> Option<&'a str> {
-    keys.iter().find_map(|k| v.get(*k).and_then(|v| v.as_str()))
-}
+use crate::types::parse_json_or_string;
+
 fn num_field(v: &Value, keys: &[&str]) -> Option<u64> {
     keys.iter().find_map(|k| v.get(*k).and_then(|v| v.as_u64()))
 }

src/scanner.rs

@@ -302,29 +302,16 @@ fn scan_toml_hooks(path: &Path, provider: ProviderId) -> Vec<ConfigItem> {
     out
 }
 
-fn scan_hook_entries(
+fn collect_hook_items(
+    entries_iter: impl Iterator<Item = (String, serde_json::Value)>,
     path: &Path,
     provider: ProviderId,
     disabled_names: &[String],
+    event_prefix: &str,
+    force_disabled: bool,
 ) -> Vec<ConfigItem> {
     let mut out = vec![];
-    let text = match std::fs::read_to_string(path) {
-        Ok(t) => t,
-        _ => return out,
-    };
-    let doc: serde_json::Value = match serde_json::from_str(&text) {
-        Ok(d) => d,
-        _ => return out,
-    };
-    let hooks_obj = match doc.get("hooks").and_then(|v| v.as_object()) {
-        Some(o) => o,
-        _ => return out,
-    };
-
-    for (event, entries) in hooks_obj {
-        if event == "disabled" || event.starts_with("_agentswitch") {
-            continue;
-        }
+    for (event, entries) in entries_iter {
         let arr = match entries.as_array() {
             Some(a) => a,
             _ => continue,
@@ -340,11 +327,12 @@ fn scan_hook_entries(
             let display = hook_name
                 .clone()
                 .unwrap_or_else(|| format!("{}: {}", event, matcher));
-            let is_disabled = hook_name
-                .as_ref()
-                .is_some_and(|n| disabled_names.contains(n));
+            let is_disabled = force_disabled
+                || hook_name
+                    .as_ref()
+                    .is_some_and(|n| disabled_names.contains(n));
             let loc = HookLoc {
-                event: event.clone(),
+                event: format!("{}{}", event_prefix, event),
                 index: i,
                 hook_name: hook_name.unwrap_or_else(|| matcher.to_string()),
             };
@@ -358,36 +346,47 @@ fn scan_hook_entries(
             out.push(item);
         }
     }
+    out
+}
+
+fn scan_hook_entries(
+    path: &Path,
+    provider: ProviderId,
+    disabled_names: &[String],
+) -> Vec<ConfigItem> {
+    let mut out = vec![];
+    let text = match std::fs::read_to_string(path) {
+        Ok(t) => t,
+        _ => return out,
+    };
+    let doc: serde_json::Value = match serde_json::from_str(&text) {
+        Ok(d) => d,
+        _ => return out,
+    };
+    if let Some(hooks_obj) = doc.get("hooks").and_then(|v| v.as_object()) {
+        let filtered = hooks_obj
+            .iter()
+            .filter(|(event, _)| event.as_str() != "disabled" && !event.starts_with("_agentswitch"))
+            .map(|(e, v)| (e.clone(), v.clone()));
+        out.extend(collect_hook_items(
+            filtered,
+            path,
+            provider,
+            disabled_names,
+            "",
+            false,
+        ));
+    }
     if let Some(stashed) = doc.get("_agentswitch_disabled").and_then(|v| v.as_object()) {
-        for (event, entries) in stashed {
-            let arr = match entries.as_array() {
-                Some(a) => a,
-                _ => continue,
-            };
-            for (i, entry) in arr.iter().enumerate() {
-                let matcher = entry.get("matcher").and_then(|v| v.as_str()).unwrap_or("*");
-                let hook_name = entry
-                    .get("hooks")
-                    .and_then(|h| h.as_array())
-                    .and_then(|a| a.first())
-                    .and_then(|h| h.get("name").and_then(|n| n.as_str()))
-                    .map(String::from);
-                let display = hook_name
-                    .clone()
-                    .unwrap_or_else(|| format!("{}: {}", event, matcher));
-                let loc = HookLoc {
-                    event: format!("_stashed_{}", event),
-                    index: i,
-                    hook_name: hook_name.unwrap_or_else(|| matcher.to_string()),
-                };
-                let mut item = ConfigItem::new(display, ItemKind::Hook, path.to_owned(), provider);
-                item.hook_loc = Some(loc);
-                item.state = ItemState::Disabled;
-                item.editable = false;
-                item.detail = Some(json_detail(entry));
-                out.push(item);
-            }
-        }
+        let mapped = stashed.iter().map(|(e, v)| (e.clone(), v.clone()));
+        out.extend(collect_hook_items(
+            mapped,
+            path,
+            provider,
+            &[],
+            "_stashed_",
+            true,
+        ));
     }
     out
 }

src/toggler.rs

@@ -160,31 +160,41 @@ fn toggle_toml_mcp(item: &mut ConfigItem) -> Result<()> {
     Ok(())
 }
 
+/// Set enable/disable markers on a JSON object. For "mcpServers" uses
+/// a "disabled" bool; for "mcp"/"agent" uses an "enabled" bool.
+fn apply_json_toggle(
+    o: &mut serde_json::Map<String, serde_json::Value>,
+    section: &str,
+    enable: bool,
+) {
+    if section == "mcpServers" {
+        if enable {
+            o.remove("disabled");
+        } else {
+            o.insert("disabled".into(), serde_json::Value::Bool(true));
+        }
+    } else {
+        o.insert("enabled".into(), serde_json::Value::Bool(enable));
+    }
+}
+
 fn toggle_json_item(item: &mut ConfigItem) -> Result<()> {
     backup(&item.path)?;
     let mut doc: serde_json::Value = serde_json::from_str(&std::fs::read_to_string(&item.path)?)?;
+    let enable = !item.state.is_enabled();
 
     let candidates = ["mcpServers", "mcp", "agent"];
     let mut found = false;
     for key in candidates {
         if let Some(obj) = doc.get_mut(key).and_then(|v| v.as_object_mut()) {
             if let Some(val) = obj.get_mut(&item.name) {
                 if let Some(o) = val.as_object_mut() {
-                    if item.state.is_enabled() {
-                        if key == "mcp" || key == "agent" {
-                            o.insert("enabled".to_string(), serde_json::Value::Bool(false));
-                        } else {
-                            o.insert("disabled".to_string(), serde_json::Value::Bool(true));
-                        }
-                        item.state = ItemState::Disabled;
+                    apply_json_toggle(o, key, enable);
+                    item.state = if enable {
+                        ItemState::Enabled
                     } else {
-                        if key == "mcp" || key == "agent" {
-                            o.insert("enabled".to_string(), serde_json::Value::Bool(true));
-                        } else {
-                            o.remove("disabled");
-                        }
-                        item.state = ItemState::Enabled;
-                    }
+                        ItemState::Disabled
+                    };
                     found = true;
                     break;
                 }
@@ -195,21 +205,12 @@ fn toggle_json_item(item: &mut ConfigItem) -> Result<()> {
         if let Some(obj) = doc.get_mut(&disabled_key).and_then(|v| v.as_object_mut()) {
             if let Some(mut val) = obj.remove(&item.name) {
                 if let Some(o) = val.as_object_mut() {
-                    if item.state.is_enabled() {
-                        if key == "mcp" || key == "agent" {
-                            o.insert("enabled".to_string(), serde_json::Value::Bool(false));
-                        } else {
-                            o.insert("disabled".to_string(), serde_json::Value::Bool(true));
-                        }
-                        item.state = ItemState::Disabled;
+                    apply_json_toggle(o, key, enable);
+                    item.state = if enable {
+                        ItemState::Enabled
                     } else {
-                        if key == "mcp" || key == "agent" {
-                            o.insert("enabled".to_string(), serde_json::Value::Bool(true));
-                        } else {
-                            o.remove("disabled");
-                        }
-                        item.state = ItemState::Enabled;
-                    }
+                        ItemState::Disabled
+                    };
                 }
                 let main_obj = doc
                     .as_object_mut()

src/types.rs

@@ -176,3 +176,14 @@ pub enum FilterKind {
     All,
     Specific(ItemKind),
 }
+
+/// Look up the first matching key in a JSON value and return it as &str.
+pub fn str_field<'a>(value: &'a serde_json::Value, keys: &[&str]) -> Option<&'a str> {
+    keys.iter()
+        .find_map(|key| value.get(*key).and_then(|v| v.as_str()))
+}
+
+/// Parse a string as JSON, falling back to a JSON string literal.
+pub fn parse_json_or_string(raw: &str) -> serde_json::Value {
+    serde_json::from_str(raw).unwrap_or_else(|_| serde_json::Value::String(raw.into()))
+}