Merge PR #185: Fix pre-merge-commit hook executable permissions

Author: merge-queue-bot

cmd/mdsmith/mergedriver.go

@@ -491,6 +491,12 @@ func ensurePreMergeCommitHook(repoRoot string, files []string) error {
 	if err := os.WriteFile(hookPath, []byte(content), 0o755); err != nil {
 		return fmt.Errorf("writing %s: %w", hookPath, err)
 	}
+	// Explicitly set execute permissions after writing. WriteFile's perm
+	// argument is masked by umask on creation and ignored when the file
+	// already exists, so a separate Chmod ensures the hook is executable.
+	if err := chmodFunc(hookPath, 0o755); err != nil {
+		return fmt.Errorf("setting permissions on %s: %w", hookPath, err)
+	}
 	return nil
 }
 
@@ -521,6 +527,10 @@ func registerMergeDriver() error {
 // Overridden in tests to exercise the non-temporary-exe branch.
 var executableFunc = os.Executable
 
+// chmodFunc is the function used to set file permissions.
+// Overridden in tests to exercise the Chmod error path.
+var chmodFunc = os.Chmod
+
 // resolveInstalledBinary returns the absolute path to the mdsmith
 // binary to use as the git merge driver. It prefers the current
 // executable when it lives outside the OS temp directory (i.e. it

cmd/mdsmith/mergedriver_test.go

@@ -508,6 +508,31 @@ func TestEnsurePreMergeCommitHook_OverwritesManagedHook(t *testing.T) {
 	assert.Contains(t, string(data), "'PLAN.md'")
 }
 
+func TestEnsurePreMergeCommitHook_SetsExecutableBitOnExistingHook(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("POSIX permission semantics not applicable on Windows")
+	}
+	dir := t.TempDir()
+	hooksDir := filepath.Join(dir, ".git", "hooks")
+	require.NoError(t, os.MkdirAll(hooksDir, 0o755))
+	hookPath := filepath.Join(hooksDir, "pre-merge-commit")
+	// Pre-existing hook with our marker but NO execute permissions.
+	old := "#!/bin/sh\n" + preMergeCommitHookMarker + "\n# old\n"
+	require.NoError(t, os.WriteFile(hookPath, []byte(old), 0o644))
+
+	orig := executableFunc
+	t.Cleanup(func() { executableFunc = orig })
+	executableFunc = func() (string, error) { return "/usr/local/bin/mdsmith", nil }
+
+	require.NoError(t, ensurePreMergeCommitHook(dir, []string{"PLAN.md"}))
+
+	info, err := os.Stat(hookPath)
+	require.NoError(t, err)
+	// Verify execute bit is set despite the file existing without it.
+	assert.NotZero(t, info.Mode()&0o111,
+		"hook must have execute bit set even when overwriting non-executable file")
+}
+
 func TestEnsurePreMergeCommitHook_RefusesUnmanagedHook(t *testing.T) {
 	dir := t.TempDir()
 	hooksDir := filepath.Join(dir, ".git", "hooks")
@@ -646,6 +671,26 @@ func TestEnsurePreMergeCommitHook_WriteFileFails(t *testing.T) {
 	assert.Contains(t, err.Error(), "writing")
 }
 
+func TestEnsurePreMergeCommitHook_ChmodFails(t *testing.T) {
+	dir := t.TempDir()
+	hooksDir := filepath.Join(dir, ".git", "hooks")
+	require.NoError(t, os.MkdirAll(hooksDir, 0o755))
+
+	origExe := executableFunc
+	t.Cleanup(func() { executableFunc = origExe })
+	executableFunc = func() (string, error) { return "/usr/local/bin/mdsmith", nil }
+
+	origChmod := chmodFunc
+	t.Cleanup(func() { chmodFunc = origChmod })
+	chmodFunc = func(string, os.FileMode) error {
+		return os.ErrPermission
+	}
+
+	err := ensurePreMergeCommitHook(dir, []string{"PLAN.md"})
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "setting permissions")
+}
+
 // --- resolveHooksDir ---
 
 func TestResolveHooksDir_NotGitRepo(t *testing.T) {