fix(hooks): merge PreToolUse bundles instead of replacing

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 {

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)
 }