Fix: user-scope Windows hook group duplication (#784)

JSON-quoted absolute paths doubled backslashes, so merge failed to recognize the group it had just written and appended another copy on every update.

Prepared with AI assistance.

Co-authored-by: Cursor <cursoragent@cursor.com>

crates/context/src/hook_markers.rs

@@ -30,9 +30,30 @@ static LAUNCHER_HOOK_MARKER: Lazy<Regex> = Lazy::new(|| {
     Regex::new(r#"skills/impeccable/scripts/impeccable(?:\.cmd|\.exe)?["']?\s+hook(?:-before-edit|-probe|-after-edit|-stop)?(?:\s|$|["'&|;)])"#).unwrap()
 });
 
+/// User-scope Windows commands embed a JSON-quoted path whose backslashes are
+/// doubled in the command string (#784). #604's single `\`→`/` replace is not
+/// enough: `\\` becomes `//`, which breaks `skills/impeccable` matching.
+fn normalize_hook_separators(command: &str) -> String {
+    let mut out = String::with_capacity(command.len());
+    let mut last_was_sep = false;
+    for ch in command.chars() {
+        if ch == '\\' || ch == '/' {
+            if !last_was_sep {
+                out.push('/');
+                last_was_sep = true;
+            }
+        } else {
+            out.push(ch);
+            last_was_sep = false;
+        }
+    }
+    out
+}
+
 /// True when `command` invokes an Impeccable hook in either generation's spelling.
 pub fn is_impeccable_hook_command(command: &str) -> bool {
-    LEGACY_HOOK_SCRIPT_MARKERS.iter().any(|m| command.contains(m)) || LAUNCHER_HOOK_MARKER.is_match(command)
+    let command = normalize_hook_separators(command);
+    LEGACY_HOOK_SCRIPT_MARKERS.iter().any(|m| command.contains(m)) || LAUNCHER_HOOK_MARKER.is_match(&command)
 }
 
 /// True when `command` invokes an Impeccable hook in the launcher generation
@@ -40,7 +61,7 @@ pub fn is_impeccable_hook_command(command: &str) -> bool {
 /// one: install/update use this to decide which manifests still need
 /// migrating to the launcher form.
 pub fn is_launcher_hook_command(command: &str) -> bool {
-    LAUNCHER_HOOK_MARKER.is_match(command)
+    LAUNCHER_HOOK_MARKER.is_match(&normalize_hook_separators(command))
 }
 
 /// The launcher-era markers `context` and `doctor` treat as the design hook
@@ -54,9 +75,10 @@ static LAUNCHER_DESIGN_HOOK: Lazy<Regex> = Lazy::new(|| {
 /// `hook-before-edit`) in either spelling; the JS `context.mjs` scan and
 /// `staleness-deep` HOOK_SCRIPT_MARKERS both meant exactly these two.
 pub fn is_design_hook_command(command: &str) -> bool {
+    let command = normalize_hook_separators(command);
     command.contains("skills/impeccable/scripts/hook.mjs")
         || command.contains("skills/impeccable/scripts/hook-before-edit.mjs")
-        || LAUNCHER_DESIGN_HOOK.is_match(command)
+        || LAUNCHER_DESIGN_HOOK.is_match(&command)
 }
 
 /// True when `command` runs the design hook (`hook` / `hook-before-edit`) in
@@ -66,7 +88,7 @@ pub fn is_design_hook_command(command: &str) -> bool {
 /// so the hook is dead and `MANUAL_DETECTOR_REQUIRED` must fire until an
 /// install/update repairs it.
 pub fn is_launcher_design_hook_command(command: &str) -> bool {
-    LAUNCHER_DESIGN_HOOK.is_match(command)
+    LAUNCHER_DESIGN_HOOK.is_match(&normalize_hook_separators(command))
 }
 
 /// The shell token that names the hook program inside `command`, for
@@ -74,7 +96,11 @@ pub fn is_launcher_design_hook_command(command: &str) -> bool {
 /// path in the binary form. `None` when the command carries no marker or
 /// the token cannot be isolated (a `'\''` escape sequence, for instance).
 pub fn hook_program_token(command: &str) -> Option<String> {
-    if !is_design_hook_command(command) {
+    if command.contains("'\\''") {
+        return None;
+    }
+    let command = normalize_hook_separators(command);
+    if !is_design_hook_command(&command) {
         return None;
     }
     static QUOTED: Lazy<Regex> = Lazy::new(|| {
@@ -86,16 +112,13 @@ pub fn hook_program_token(command: &str) -> Option<String> {
     static BARE: Lazy<Regex> = Lazy::new(|| {
         Regex::new(r#"([^\s"'|&;()]*skills/impeccable/scripts/(?:hook(?:-before-edit)?\.mjs|impeccable(?:\.cmd|\.exe)?))"#).unwrap()
     });
-    if let Some(m) = QUOTED.captures(command) {
+    if let Some(m) = QUOTED.captures(&command) {
         return Some(m[1].to_string());
     }
-    if command.contains("'\\''") {
-        return None;
-    }
-    if let Some(m) = SINGLE.captures(command) {
+    if let Some(m) = SINGLE.captures(&command) {
         return Some(m[1].to_string());
     }
-    BARE.captures(command).map(|m| m[1].to_string())
+    BARE.captures(&command).map(|m| m[1].to_string())
 }
 
 #[cfg(test)]
@@ -183,4 +206,26 @@ mod tests {
         assert_eq!(hook_program_token("'/x/it'\\''s/.claude/skills/impeccable/scripts/impeccable' hook"), None);
         assert_eq!(hook_program_token("echo hi"), None);
     }
+
+    #[test]
+    fn recognizes_json_escaped_windows_launcher_path() {
+        let launcher = r"C:\Users\alice\.claude\skills\impeccable\scripts\impeccable";
+        let quoted = serde_json::to_string(launcher).unwrap();
+        let cmd = format!("[ ! -f {quoted} ] || {quoted} hook");
+        assert!(is_impeccable_hook_command(&cmd), "{cmd}");
+        assert!(is_launcher_hook_command(&cmd), "{cmd}");
+        assert!(is_design_hook_command(&cmd), "{cmd}");
+        assert_eq!(
+            hook_program_token(&cmd).as_deref(),
+            Some("C:/Users/alice/.claude/skills/impeccable/scripts/impeccable")
+        );
+    }
+
+    #[test]
+    fn recognizes_single_backslash_windows_path() {
+        let cmd = r#"[ ! -f "C:\Users\alice\.claude\skills\impeccable\scripts\impeccable" ] || "C:\Users\alice\.claude\skills\impeccable\scripts\impeccable" hook"#;
+        assert!(is_impeccable_hook_command(cmd), "{cmd}");
+        assert!(is_launcher_hook_command(cmd), "{cmd}");
+        assert!(is_design_hook_command(cmd), "{cmd}");
+    }
 }

crates/skills/src/hook_manifest.rs

@@ -219,13 +219,10 @@ fn rewrite_value(value: &Value, provider: &str, quoted: &QuotedPath, win32: bool
     }
 }
 
-/// JS: valueHasImpeccableHookMarker(value). Command separators are
-/// normalized to `/` first so a legacy Windows-path guard is still
-/// recognized as ours and replaced instead of duplicated (upstream
-/// 665c51b9, #604).
+/// JS: valueHasImpeccableHookMarker(value).
 pub fn value_has_impeccable_hook_marker(value: &Value) -> bool {
     match value {
-        Value::String(s) => is_impeccable_hook_command(&s.replace('\\', "/")),
+        Value::String(s) => is_impeccable_hook_command(s),
         Value::Array(a) => a.iter().any(value_has_impeccable_hook_marker),
         Value::Object(o) => o.values().any(value_has_impeccable_hook_marker),
         _ => false,
@@ -233,12 +230,9 @@ pub fn value_has_impeccable_hook_marker(value: &Value) -> bool {
 }
 
 /// True when `value` names an Impeccable hook in the launcher generation.
-/// Separators are normalized to `/` first, matching
-/// `value_has_impeccable_hook_marker`, so a legacy Windows-path launcher
-/// command is still recognized.
 pub fn value_has_launcher_hook_marker(value: &Value) -> bool {
     match value {
-        Value::String(s) => is_launcher_hook_command(&s.replace('\\', "/")),
+        Value::String(s) => is_launcher_hook_command(s),
         Value::Array(a) => a.iter().any(value_has_launcher_hook_marker),
         Value::Object(o) => o.values().any(value_has_launcher_hook_marker),
         _ => false,

crates/skills/tests/hook_manifest_tests.rs

@@ -357,3 +357,57 @@ fn hook_artifacts_map_providers_to_manifest_files() {
     assert_eq!(c[0].dest, jsp::join(&["/p", ".codex", "hooks.json"]));
     assert!(c[0].shared_dest.is_none());
 }
+
+fn windows_user_scope_hook_command() -> String {
+    let launcher = r"C:\Users\alice\.claude\skills\impeccable\scripts\impeccable";
+    let q = json_string(launcher);
+    format!("[ ! -f {q} ] || {q} hook")
+}
+
+#[test]
+fn merge_json_escaped_windows_launcher_is_idempotent() {
+    let cmd = windows_user_scope_hook_command();
+    let hook_entry = |cmd: String| {
+        json!({ "matcher": "Edit", "hooks": [{ "type": "command", "command": cmd }] })
+    };
+    let stop_entry = |cmd: String| {
+        json!({ "hooks": [{ "type": "command", "command": cmd, "timeout": 30 }] })
+    };
+    let existing = json!({
+        "hooks": {
+            "PostToolUse": [hook_entry(cmd.clone())],
+            "Stop": [stop_entry(cmd.clone())]
+        }
+    });
+    let fresh = json!({
+        "description": "fresh",
+        "hooks": {
+            "PostToolUse": [hook_entry(cmd.clone())],
+            "Stop": [stop_entry(cmd.clone())]
+        }
+    });
+    let merged = merge_hook_manifests(&existing, &fresh);
+    assert_eq!(merged["hooks"]["PostToolUse"].as_array().unwrap().len(), 1);
+    assert_eq!(merged["hooks"]["Stop"].as_array().unwrap().len(), 1);
+    let merged2 = merge_hook_manifests(&merged, &fresh);
+    assert_eq!(merged2["hooks"]["PostToolUse"].as_array().unwrap().len(), 1);
+    assert_eq!(merged2["hooks"]["Stop"].as_array().unwrap().len(), 1);
+}
+
+#[test]
+fn merge_heals_triplicated_stop_groups() {
+    let cmd = windows_user_scope_hook_command();
+    let stop_entry = json!({ "hooks": [{ "type": "command", "command": cmd.clone(), "timeout": 30 }] });
+    let existing = json!({
+        "hooks": {
+            "Stop": [stop_entry.clone(), stop_entry.clone(), stop_entry]
+        }
+    });
+    let fresh = json!({
+        "hooks": {
+            "Stop": [json!({ "hooks": [{ "type": "command", "command": cmd, "timeout": 30 }] })]
+        }
+    });
+    let merged = merge_hook_manifests(&existing, &fresh);
+    assert_eq!(merged["hooks"]["Stop"].as_array().unwrap().len(), 1);
+}