diff --git a/internal/lifecycle/hookinstall.go b/internal/lifecycle/hookinstall.go index 0ee403d..6c6d17f 100644 --- a/internal/lifecycle/hookinstall.go +++ b/internal/lifecycle/hookinstall.go @@ -8,7 +8,9 @@ import ( ) // InstallHooksConfig reads an existing Claude Code settings JSON file (or creates one) -// and merges in the SpiceBox PreToolUse HTTP hook configuration. +// and merges in the SpiceBox PreToolUse HTTP hook configuration. Existing hooks in +// the PreToolUse array are preserved; if a prior SpiceBox entry (matched by hookURL) +// is present, it is replaced in-place to avoid duplicates on repeated installs. func InstallHooksConfig(path string, hookURL string) error { if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { return fmt.Errorf("create directory: %w", err) @@ -29,18 +31,39 @@ func InstallHooksConfig(path string, hookURL string) error { hooks = make(map[string]any) } - hooks["PreToolUse"] = []map[string]any{ - { - "matcher": "*", - "hooks": []map[string]any{ - { - "type": "http", - "url": hookURL, - "timeout": 5000, - }, + spiceboxBundle := map[string]any{ + "matcher": "*", + "hooks": []map[string]any{ + { + "type": "http", + "url": hookURL, + "timeout": 5000, }, }, } + + // Merge: keep existing PreToolUse bundles, drop any prior SpiceBox bundle + // (identified by our hookURL) to prevent duplicates, then append ours last + // so existing guards run first and SpiceBox gets the final say. + var existingPreToolUse []any + switch v := hooks["PreToolUse"].(type) { + case []any: + existingPreToolUse = v + case []map[string]any: + for _, entry := range v { + existingPreToolUse = append(existingPreToolUse, entry) + } + } + + merged := make([]any, 0, len(existingPreToolUse)+1) + for _, entry := range existingPreToolUse { + if bundleContainsHookURL(entry, hookURL) { + continue + } + merged = append(merged, entry) + } + merged = append(merged, spiceboxBundle) + hooks["PreToolUse"] = merged settings["hooks"] = hooks data, err := json.MarshalIndent(settings, "", " ") @@ -51,6 +74,29 @@ func InstallHooksConfig(path string, hookURL string) error { return os.WriteFile(path, data, 0o644) } +// bundleContainsHookURL returns true if the given PreToolUse bundle entry +// contains an HTTP hook matching hookURL. +func bundleContainsHookURL(entry any, hookURL string) bool { + entryMap, ok := entry.(map[string]any) + if !ok { + return false + } + innerHooks, ok := entryMap["hooks"].([]any) + if !ok { + return false + } + for _, h := range innerHooks { + hMap, ok := h.(map[string]any) + if !ok { + continue + } + if hMap["url"] == hookURL { + return true + } + } + return false +} + // UninstallHooksConfig removes the SpiceBox hook entry (identified by hookURL) // from a Claude Code settings JSON file. Other hooks and settings are preserved. func UninstallHooksConfig(path string, hookURL string) error { diff --git a/internal/lifecycle/hookinstall_test.go b/internal/lifecycle/hookinstall_test.go index dfb8f06..cede4bf 100644 --- a/internal/lifecycle/hookinstall_test.go +++ b/internal/lifecycle/hookinstall_test.go @@ -61,16 +61,17 @@ func TestInstallHooksConfig_MergesWithExistingSettings(t *testing.T) { } } -func TestInstallHooksConfig_OverwritesExistingHooks(t *testing.T) { +func TestInstallHooksConfig_PreservesExistingPreToolUseBundles(t *testing.T) { dir := t.TempDir() path := filepath.Join(dir, "settings.json") - // Write existing hooks config with a different URL. + // Write existing config with a Fort-style guard hook. This is the + // regression case: previously InstallHooksConfig would clobber this. existing := map[string]any{ "hooks": map[string]any{ "PreToolUse": []map[string]any{ - {"matcher": "*", "hooks": []map[string]any{ - {"type": "command", "command": "echo old"}, + {"matcher": "Bash", "hooks": []map[string]any{ + {"type": "command", "command": "/path/to/guard.sh", "timeout": 5000}, }}, }, }, @@ -84,6 +85,82 @@ func TestInstallHooksConfig_OverwritesExistingHooks(t *testing.T) { got := readJSON(t, path) assertHookURL(t, got, newURL) + + // Existing guard must still be present. + hooks := got["hooks"].(map[string]any) + preToolUse := hooks["PreToolUse"].([]any) + if len(preToolUse) != 2 { + t.Fatalf("PreToolUse has %d entries, want 2 (existing guard + spicebox)", len(preToolUse)) + } + + // The first entry should be the preserved guard. + guard := preToolUse[0].(map[string]any) + if guard["matcher"] != "Bash" { + t.Errorf("first entry matcher = %v, want %q (existing guard should come first)", guard["matcher"], "Bash") + } + guardHooks := guard["hooks"].([]any) + guardHook := guardHooks[0].(map[string]any) + if guardHook["command"] != "/path/to/guard.sh" { + t.Errorf("existing guard command = %v, want %q", guardHook["command"], "/path/to/guard.sh") + } +} + +func TestInstallHooksConfig_DedupesOnReinstall(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "settings.json") + hookURL := "http://localhost:9090/check" + + // Install twice. + if err := InstallHooksConfig(path, hookURL); err != nil { + t.Fatalf("first InstallHooksConfig: %v", err) + } + if err := InstallHooksConfig(path, hookURL); err != nil { + t.Fatalf("second InstallHooksConfig: %v", err) + } + + got := readJSON(t, path) + hooks := got["hooks"].(map[string]any) + preToolUse := hooks["PreToolUse"].([]any) + if len(preToolUse) != 1 { + t.Errorf("PreToolUse has %d entries after reinstall, want 1 (dedup)", len(preToolUse)) + } +} + +func TestInstallHooksConfig_ReinstallPreservesOtherBundles(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "settings.json") + hookURL := "http://localhost:9090/check" + + // First install with an existing guard. + existing := map[string]any{ + "hooks": map[string]any{ + "PreToolUse": []map[string]any{ + {"matcher": "Bash", "hooks": []map[string]any{ + {"type": "command", "command": "/path/to/guard.sh"}, + }}, + }, + }, + } + writeJSON(t, path, existing) + if err := InstallHooksConfig(path, hookURL); err != nil { + t.Fatalf("first InstallHooksConfig: %v", err) + } + + // Reinstall (simulates SpiceBox restart). + if err := InstallHooksConfig(path, hookURL); err != nil { + t.Fatalf("second InstallHooksConfig: %v", err) + } + + got := readJSON(t, path) + hooks := got["hooks"].(map[string]any) + preToolUse := hooks["PreToolUse"].([]any) + if len(preToolUse) != 2 { + t.Fatalf("PreToolUse has %d entries after reinstall, want 2 (guard + single spicebox)", len(preToolUse)) + } + guard := preToolUse[0].(map[string]any) + if guard["matcher"] != "Bash" { + t.Errorf("guard lost after reinstall, got first matcher = %v", guard["matcher"]) + } } func TestInstallHooksConfig_InvalidExistingJSON(t *testing.T) { @@ -321,7 +398,9 @@ func writeJSON(t *testing.T, path string, v any) { } // assertHookURL verifies the parsed settings JSON contains a PreToolUse HTTP -// hook pointing at the expected URL. +// hook pointing at the expected URL. The SpiceBox bundle may now be any entry +// in the array (merge semantics), so scan for a matching URL rather than +// assuming index 0. func assertHookURL(t *testing.T, settings map[string]any, wantURL string) { t.Helper() @@ -339,24 +418,31 @@ func assertHookURL(t *testing.T, settings map[string]any, wantURL string) { t.Fatal("PreToolUse is empty") } - entry := preToolUse[0].(map[string]any) - if entry["matcher"] != "*" { - t.Errorf("matcher = %v, want %q", entry["matcher"], "*") - } - - innerHooks := entry["hooks"].([]any) - if len(innerHooks) == 0 { - t.Fatal("inner hooks array is empty") - } - - hook := innerHooks[0].(map[string]any) - if hook["type"] != "http" { - t.Errorf("hook type = %v, want %q", hook["type"], "http") - } - if hook["url"] != wantURL { - t.Errorf("hook url = %v, want %q", hook["url"], wantURL) - } - if hook["timeout"] != float64(5000) { - t.Errorf("hook timeout = %v, want %v", hook["timeout"], 5000) - } + for _, entry := range preToolUse { + entryMap, ok := entry.(map[string]any) + if !ok { + continue + } + if entryMap["matcher"] != "*" { + continue + } + innerHooks, ok := entryMap["hooks"].([]any) + if !ok || len(innerHooks) == 0 { + continue + } + for _, h := range innerHooks { + hook, ok := h.(map[string]any) + if !ok { + continue + } + if hook["type"] == "http" && hook["url"] == wantURL { + if hook["timeout"] != float64(5000) { + t.Errorf("hook timeout = %v, want %v", hook["timeout"], 5000) + } + return // found — all good + } + } + } + + t.Fatalf("no HTTP hook with url=%q found in PreToolUse: %v", wantURL, preToolUse) }