Fix Codex hook reinstall verification

.github/workflows/ci.yml

@@ -72,7 +72,7 @@ jobs:
       # These packages have no cgo dependency, so no ALSA headers are needed.
       - name: Enforce coverage floor
         run: |
-          THRESHOLD=87.0
+          THRESHOLD=89.0
           status=0
           for pkg in ./internal/hooks ./internal/install; do
             line=$(go test "$pkg" -count=1 -cover)

internal/audio/backend_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"io"
+	"runtime"
 	"strings"
 	"testing"
 	"time"
@@ -83,6 +84,7 @@ func TestSystemCommandBackend_Interface(t *testing.T) {
 }
 
 func TestSystemCommandBackend_Play(t *testing.T) {
+	successCommand := successfulNoopCommand()
 	tests := []struct {
 		name    string
 		command string
@@ -91,15 +93,15 @@ func TestSystemCommandBackend_Play(t *testing.T) {
 	}{
 		{
 			name:    "play file source with nonexistent file",
-			command: "echo", // Use echo instead of paplay to avoid system dependencies
+			command: successCommand,
 			source:  NewFileSource("/test/sound.wav"),
-			wantErr: false, // echo will succeed even with nonexistent args
+			wantErr: false,
 		},
 		{
 			name:    "play reader source",
-			command: "echo", // Use echo instead of paplay
+			command: successCommand,
 			source:  NewReaderSource(io.NopCloser(strings.NewReader("test")), "wav"),
-			wantErr: false, // echo will succeed
+			wantErr: false,
 		},
 		{
 			name:    "invalid command",
@@ -127,6 +129,13 @@ func TestSystemCommandBackend_Play(t *testing.T) {
 	}
 }
 
+func successfulNoopCommand() string {
+	if runtime.GOOS == "windows" {
+		return "cmd.exe"
+	}
+	return "true"
+}
+
 func TestSystemCommandBackend_Lifecycle(t *testing.T) {
 	backend := NewSystemCommandBackend("paplay")
 

internal/audio/platform_test.go

@@ -1,6 +1,7 @@
 package audio
 
 import (
+	"runtime"
 	"testing"
 )
 
@@ -13,19 +14,20 @@ func TestPlatformDetectionInterface(t *testing.T) {
 }
 
 func TestCommandExists(t *testing.T) {
+	firstExisting, secondExisting := existingCommandFixtures()
 	tests := []struct {
 		name     string
 		command  string
 		expected bool
 	}{
 		{
-			name:     "existing command - echo",
-			command:  "echo",
+			name:     "existing command - primary",
+			command:  firstExisting,
 			expected: true,
 		},
 		{
-			name:     "existing command - ls",
-			command:  "ls",
+			name:     "existing command - secondary",
+			command:  secondExisting,
 			expected: true,
 		},
 		{
@@ -190,15 +192,13 @@ func TestGetPreferredSystemCommand(t *testing.T) {
 // TestRealSystemIntegration tests against the real system (these may vary by environment)
 func TestRealSystemIntegration(t *testing.T) {
 	t.Run("real command detection", func(t *testing.T) {
-		// Test some commands that should exist on most systems
-		echoExists := CommandExists("echo")
-		if !echoExists {
-			t.Error("echo command should exist on most systems")
+		firstExisting, secondExisting := existingCommandFixtures()
+		if !CommandExists(firstExisting) {
+			t.Errorf("%s command should exist on this system", firstExisting)
 		}
 
-		lsExists := CommandExists("ls")
-		if !lsExists {
-			t.Error("ls command should exist on most Unix-like systems")
+		if !CommandExists(secondExisting) {
+			t.Errorf("%s command should exist on this system", secondExisting)
 		}
 
 		fakeExists := CommandExists("definitely-does-not-exist-12345")
@@ -222,3 +222,10 @@ func TestRealSystemIntegration(t *testing.T) {
 		}
 	})
 }
+
+func existingCommandFixtures() (string, string) {
+	if runtime.GOOS == "windows" {
+		return "cmd.exe", "where.exe"
+	}
+	return "sh", "true"
+}

internal/audio/system_command_backend_test.go

@@ -143,8 +143,8 @@ func TestBuildPlayerArgv_AplayFullVolumeNoWarn(t *testing.T) {
 }
 
 // TestBuildPlayerArgv_UnknownCommandFallsBack confirms the default branch
-// passes only filePath. The existing TestSystemCommandBackend_Play uses
-// command="echo" -- this guards that test from regressing.
+// passes only filePath. TestSystemCommandBackend_Play uses a harmless
+// platform command, but echo still exercises the default argv branch here.
 func TestBuildPlayerArgv_UnknownCommandFallsBack(t *testing.T) {
 	scb := NewSystemCommandBackend("echo")
 	argv := scb.buildPlayerArgv("/tmp/x.wav", 0.5)

internal/install/coverage_test.go

@@ -140,6 +140,33 @@ func TestWriteSettingsFileRoundTrip(t *testing.T) {
 	}
 }
 
+func TestWriteSettingsFileMarshalError(t *testing.T) {
+	fsys := afero.NewMemMapFs()
+	settings := &SettingsMap{"bad": make(chan int)}
+	if err := WriteSettingsFile(fsys, "/settings.json", settings); err == nil {
+		t.Error("expected marshal error for unsupported settings value")
+	}
+}
+
+func TestWriteSettingsFilePreservesExistingPermissions(t *testing.T) {
+	fsys := afero.NewMemMapFs()
+	path := "/settings.json"
+	if err := afero.WriteFile(fsys, path, []byte(`{"old":true}`), 0600); err != nil {
+		t.Fatal(err)
+	}
+	settings := &SettingsMap{"new": true}
+	if err := WriteSettingsFile(fsys, path, settings); err != nil {
+		t.Fatal(err)
+	}
+	info, err := fsys.Stat(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got := info.Mode() & os.ModePerm; got != 0600 {
+		t.Errorf("mode = %v, want 0600", got)
+	}
+}
+
 func TestMergeHookValuesStringFormatExisting(t *testing.T) {
 	// Existing hook in legacy string format (non-claudio) must be preserved
 	// and merged into array form alongside the claudio command.
@@ -178,6 +205,143 @@ func TestMergeHookValuesStringFormatExisting(t *testing.T) {
 	}
 }
 
+func TestMergeHooksRefreshesClaudioWithoutDroppingExistingHooks(t *testing.T) {
+	existing := &SettingsMap{
+		"hooks": map[string]interface{}{
+			"PreToolUse": []interface{}{
+				map[string]interface{}{
+					"matcher": ".*",
+					"hooks": []interface{}{
+						map[string]interface{}{"command": "/usr/bin/logger"},
+					},
+				},
+				map[string]interface{}{
+					"matcher": "*",
+					"hooks": []interface{}{
+						map[string]interface{}{"command": "/old/claudio"},
+					},
+				},
+			},
+		},
+	}
+	claudioHooks, err := GenerateClaudioHooksForAgent("/new/claudio", AgentCodex)
+	if err != nil {
+		t.Fatal(err)
+	}
+	merged, err := MergeHooksIntoSettings(existing, claudioHooks)
+	if err != nil {
+		t.Fatal(err)
+	}
+	hooksSection := (*merged)["hooks"].(map[string]interface{})
+	arr := hooksSection["PreToolUse"].([]interface{})
+
+	foundLogger := false
+	foundOldClaudio := false
+	foundNewClaudio := false
+	for _, e := range arr {
+		cfg := e.(map[string]interface{})
+		for _, h := range cfg["hooks"].([]interface{}) {
+			switch h.(map[string]interface{})["command"] {
+			case "/usr/bin/logger":
+				foundLogger = true
+			case "/old/claudio":
+				foundOldClaudio = true
+			case "/new/claudio":
+				foundNewClaudio = true
+			}
+		}
+	}
+	if !foundLogger {
+		t.Error("existing non-claudio hook was dropped")
+	}
+	if foundOldClaudio {
+		t.Error("old claudio hook was not refreshed")
+	}
+	if !foundNewClaudio {
+		t.Error("new claudio hook missing after refresh")
+	}
+}
+
+func TestMergeHookValuesPreservesNonClaudioEntriesWhileRefreshingClaudio(t *testing.T) {
+	entries := []interface{}{
+		"raw-entry",
+		map[string]interface{}{"matcher": "*"},
+		map[string]interface{}{
+			"matcher": "mixed",
+			"hooks": []interface{}{
+				"raw-hook",
+				map[string]interface{}{"command": 42},
+				map[string]interface{}{"command": "/old/claudio"},
+				map[string]interface{}{"command": "/usr/bin/logger"},
+			},
+		},
+		map[string]interface{}{
+			"matcher": "claudio-only",
+			"hooks": []interface{}{
+				map[string]interface{}{"command": "/old/claudio"},
+			},
+		},
+	}
+	claudioValue := []interface{}{
+		map[string]interface{}{
+			"matcher": "*",
+			"hooks": []interface{}{
+				map[string]interface{}{"command": "/new/claudio"},
+			},
+		},
+	}
+
+	filtered := mergeHookValues(entries, claudioValue).([]interface{})
+	if len(filtered) != 4 {
+		t.Fatalf("merged entry count = %d, want 4: %#v", len(filtered), filtered)
+	}
+
+	foundLogger := false
+	foundOldClaudio := false
+	foundNewClaudio := false
+	foundRawHook := false
+	foundNumericCommand := false
+	for _, entry := range filtered {
+		cfg, ok := entry.(map[string]interface{})
+		if !ok {
+			continue
+		}
+		hooksList, ok := cfg["hooks"].([]interface{})
+		if !ok {
+			continue
+		}
+		for _, hook := range hooksList {
+			if hook == "raw-hook" {
+				foundRawHook = true
+				continue
+			}
+			cmd, ok := hook.(map[string]interface{})
+			if !ok {
+				continue
+			}
+			switch cmd["command"] {
+			case 42:
+				foundNumericCommand = true
+			case "/old/claudio":
+				foundOldClaudio = true
+			case "/new/claudio":
+				foundNewClaudio = true
+			case "/usr/bin/logger":
+				foundLogger = true
+			}
+		}
+	}
+	if !foundLogger || !foundRawHook || !foundNumericCommand {
+		t.Errorf("non-claudio content not preserved: logger=%v raw=%v numeric=%v", foundLogger, foundRawHook, foundNumericCommand)
+	}
+	if foundOldClaudio {
+		t.Error("old claudio command was not removed")
+	}
+	if !foundNewClaudio {
+		t.Error("new claudio command was not appended")
+	}
+}
+
 func TestIsClaudioHookFormats(t *testing.T) {
 	if !IsClaudioHook("/usr/local/bin/claudio") {
 		t.Error("expected string claudio command recognized")
@@ -200,6 +364,55 @@ func TestIsClaudioHookFormats(t *testing.T) {
 	}
 }
 
+func TestIsClaudioHookFindsClaudioInMergedHookArrays(t *testing.T) {
+	cases := []struct {
+		name string
+		arr  []interface{}
+	}{
+		{
+			name: "claudio after existing hook",
+			arr: []interface{}{
+				map[string]interface{}{
+					"matcher": ".*",
+					"hooks": []interface{}{
+						map[string]interface{}{"command": "/usr/bin/logger"},
+					},
+				},
+				map[string]interface{}{
+					"matcher": "*",
+					"hooks": []interface{}{
+						map[string]interface{}{"command": "/usr/local/bin/claudio"},
+					},
+				},
+			},
+		},
+		{
+			name: "claudio before existing hook",
+			arr: []interface{}{
+				map[string]interface{}{
+					"matcher": "*",
+					"hooks": []interface{}{
+						map[string]interface{}{"command": `C:\tools\claudio.exe`},
+					},
+				},
+				map[string]interface{}{
+					"matcher": ".*",
+					"hooks": []interface{}{
+						map[string]interface{}{"command": "/usr/bin/logger"},
+					},
+				},
+			},
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if !IsClaudioHook(tc.arr) {
+				t.Error("expected merged hook array to be recognized when any entry is claudio")
+			}
+		})
+	}
+}
+
 func TestMergeHooksMarshalErrorPropagates(t *testing.T) {
 	// A channel value cannot be JSON-marshaled, forcing deepCopySettings to error.
 	bad := &SettingsMap{"x": make(chan int)}