test(persona): cover output-style rollback when retired style removal fails

internal/cli/persona_output_style_rollback_test.go

@@ -0,0 +1,181 @@
+package cli
+
+import (
+	"bytes"
+	"errors"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/gentleman-programming/gentle-ai/v4/internal/agents/claude"
+	"github.com/gentleman-programming/gentle-ai/v4/internal/components/persona"
+	"github.com/gentleman-programming/gentle-ai/v4/internal/model"
+	"github.com/gentleman-programming/gentle-ai/v4/internal/system"
+)
+
+var errInjectedRetiredStyleRemoval = errors.New("injected retired output style removal failure")
+
+// outputStyleTransitionFixture is a Claude home with the Gentleman persona
+// applied, plus a user-owned output style and an unrelated settings key.
+type outputStyleTransitionFixture struct {
+	home      string
+	gentleman string
+	neutral   string
+	settings  string
+	before    map[string][]byte
+}
+
+func newOutputStyleTransitionFixture(t *testing.T, home string) outputStyleTransitionFixture {
+	t.Helper()
+	claudeDir := filepath.Join(home, ".claude")
+	f := outputStyleTransitionFixture{
+		home:      home,
+		gentleman: filepath.Join(claudeDir, "output-styles", "gentleman.md"),
+		neutral:   filepath.Join(claudeDir, "output-styles", "neutral.md"),
+		settings:  filepath.Join(claudeDir, "settings.json"),
+	}
+	userStyle := filepath.Join(claudeDir, "output-styles", "user-owned.md")
+	mustWriteFile(t, f.settings, []byte(`{"permissions":{"allow":["Read"]}}`))
+	mustWriteFile(t, userStyle, []byte("user-owned style\n"))
+
+	if _, err := persona.Inject(home, claude.NewAdapter(), model.PersonaGentleman); err != nil {
+		t.Fatalf("Inject(gentleman) error = %v", err)
+	}
+
+	f.before = map[string][]byte{}
+	for _, path := range []string{f.gentleman, f.settings, userStyle, filepath.Join(claudeDir, "CLAUDE.md")} {
+		content, err := os.ReadFile(path)
+		if err != nil {
+			t.Fatalf("precondition: read %s: %v", path, err)
+		}
+		f.before[path] = content
+	}
+	if !strings.Contains(string(f.before[f.settings]), `"outputStyle": "Gentleman"`) {
+		t.Fatalf("precondition: settings do not select Gentleman: %s", f.before[f.settings])
+	}
+	if _, err := os.Stat(f.neutral); !os.IsNotExist(err) {
+		t.Fatalf("precondition: neutral output style should be absent, stat err=%v", err)
+	}
+	return f
+}
+
+// failRetiredGentlemanRemoval makes the real persona injection fail when it
+// removes the retired gentleman.md, after the Neutral style and settings have
+// been published. reached records that the removal was attempted in that state.
+// Every other removal goes through os.Remove.
+func (f outputStyleTransitionFixture) failRetiredGentlemanRemoval(t *testing.T, reached *bool) {
+	t.Helper()
+	persona.SetRemoveForTest(t, func(path string) error {
+		if path != f.gentleman {
+			return os.Remove(path)
+		}
+		settings, err := os.ReadFile(f.settings)
+		if err != nil {
+			return err
+		}
+		if _, err := os.Stat(f.neutral); err == nil && strings.Contains(string(settings), `"outputStyle": "Neutral"`) {
+			*reached = true
+		}
+		return errInjectedRetiredStyleRemoval
+	})
+}
+
+func (f outputStyleTransitionFixture) assertRestored(t *testing.T) {
+	t.Helper()
+	for path, want := range f.before {
+		got, err := os.ReadFile(path)
+		if err != nil {
+			t.Fatalf("after rollback: read %s: %v", path, err)
+		}
+		if !bytes.Equal(got, want) {
+			t.Fatalf("after rollback %s = %q, want exact before-image %q", path, got, want)
+		}
+	}
+	if _, err := os.Stat(f.neutral); !os.IsNotExist(err) {
+		t.Fatalf("after rollback a partially published neutral output style remains, stat err=%v", err)
+	}
+}
+
+func TestInstallRollsBackOutputStyleSwitchWhenRetiredStyleRemovalFails(t *testing.T) {
+	home := installTestHome(t)
+	f := newOutputStyleTransitionFixture(t, home)
+
+	reached := false
+	f.failRetiredGentlemanRemoval(t, &reached)
+
+	_, err := RunInstall([]string{"--agent", "claude-code", "--component", "persona", "--persona", "neutral"}, system.DetectionResult{})
+	if !errors.Is(err, errInjectedRetiredStyleRemoval) {
+		t.Fatalf("RunInstall() error = %v, want injected retired style removal failure", err)
+	}
+	if !reached {
+		t.Fatal("injection never published the Neutral style and settings before failing")
+	}
+	f.assertRestored(t)
+}
+
+func TestSyncRollsBackOutputStyleSwitchWhenRetiredStyleRemovalFails(t *testing.T) {
+	f := newOutputStyleTransitionFixture(t, t.TempDir())
+	selection := model.Selection{
+		Agents:     []model.AgentID{model.AgentClaudeCode},
+		Components: []model.ComponentID{model.ComponentPersona},
+		Persona:    model.PersonaNeutral,
+	}
+
+	reached := false
+	f.failRetiredGentlemanRemoval(t, &reached)
+
+	if _, err := RunSyncWithSelection(f.home, selection); !errors.Is(err, errInjectedRetiredStyleRemoval) {
+		t.Fatalf("RunSyncWithSelection() error = %v, want injected retired style removal failure", err)
+	}
+	if !reached {
+		t.Fatal("injection never published the Neutral style and settings before failing")
+	}
+	f.assertRestored(t)
+
+	// Once removal succeeds, the switch completes and a rerun is a no-op.
+	persona.SetRemoveForTest(t, os.Remove)
+	if _, err := RunSyncWithSelection(f.home, selection); err != nil {
+		t.Fatalf("recovery RunSyncWithSelection() error = %v", err)
+	}
+	if _, err := os.Stat(f.gentleman); !os.IsNotExist(err) {
+		t.Fatalf("recovery sync left retired gentleman output style, stat err=%v", err)
+	}
+	if _, err := os.Stat(f.neutral); err != nil {
+		t.Fatalf("recovery sync did not publish neutral output style: %v", err)
+	}
+	settled := map[string][]byte{}
+	for path := range f.before {
+		if path == f.gentleman {
+			continue
+		}
+		settled[path] = nil
+	}
+	settled[f.neutral] = nil
+	for path := range settled {
+		content, err := os.ReadFile(path)
+		if err != nil {
+			t.Fatalf("after recovery: read %s: %v", path, err)
+		}
+		settled[path] = content
+	}
+	second, err := RunSyncWithSelection(f.home, selection)
+	if err != nil {
+		t.Fatalf("second RunSyncWithSelection() error = %v", err)
+	}
+	if second.FilesChanged != 0 || !second.NoOp {
+		t.Fatalf("second sync files changed = %d, no-op = %t; want idempotent no-op", second.FilesChanged, second.NoOp)
+	}
+	if _, err := os.Stat(f.gentleman); !os.IsNotExist(err) {
+		t.Fatalf("second sync restored retired gentleman output style, stat err=%v", err)
+	}
+	for path, want := range settled {
+		got, err := os.ReadFile(path)
+		if err != nil {
+			t.Fatalf("after second sync: read %s: %v", path, err)
+		}
+		if !bytes.Equal(got, want) {
+			t.Fatalf("second sync changed %s = %q, want %q", path, got, want)
+		}
+	}
+}

internal/components/persona/inject.go

@@ -997,11 +997,27 @@ func legacyVSCodePersonaPaths(homeDir string) []string {
 	}
 }
 
+var osRemove = os.Remove
+
+// SetRemoveForTest replaces the file removal used by persona injection for the
+// duration of a test and restores the original after the test completes.
+// Exported so that external test packages (e.g. internal/cli) can fail a real
+// removal inside the install and sync pipelines.
+func SetRemoveForTest(t interface {
+	Helper()
+	Cleanup(func())
+}, remove func(string) error) {
+	t.Helper()
+	orig := osRemove
+	osRemove = remove
+	t.Cleanup(func() { osRemove = orig })
+}
+
 // removeFileAtomic removes path if it exists. Returns true when the file was
 // present and successfully deleted, false when it did not exist. Any other
 // OS-level error is returned as-is.
 func removeFileAtomic(path string) (bool, error) {
-	err := os.Remove(path)
+	err := osRemove(path)
 	if err == nil {
 		return true, nil
 	}

internal/components/persona/inject_test.go

@@ -2,6 +2,7 @@ package persona
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -604,6 +605,42 @@ func TestInjectClaudeNeutralWritesNeutralOutputStyleAndSettings(t *testing.T) {
 	}
 }
 
+// TestInjectClaudeNeutralReturnsRetiredStyleRemovalFailure pins the contract
+// the install/sync rollback boundary depends on: a failed removal of the
+// retired output style is returned to the caller instead of being swallowed.
+func TestInjectClaudeNeutralReturnsRetiredStyleRemovalFailure(t *testing.T) {
+	home := t.TempDir()
+	stylesDir := filepath.Join(home, ".claude", "output-styles")
+	if err := os.MkdirAll(stylesDir, 0o755); err != nil {
+		t.Fatalf("MkdirAll() error = %v", err)
+	}
+	gentlemanPath := filepath.Join(stylesDir, "gentleman.md")
+	if err := os.WriteFile(gentlemanPath, []byte("gentleman style"), 0o644); err != nil {
+		t.Fatalf("WriteFile(gentleman) error = %v", err)
+	}
+
+	injected := errors.New("injected remove failure")
+	originalRemove := osRemove
+	t.Cleanup(func() { osRemove = originalRemove })
+	osRemove = func(path string) error {
+		if path == gentlemanPath {
+			return injected
+		}
+		return originalRemove(path)
+	}
+
+	_, err := Inject(home, claudeAdapter(), model.PersonaNeutral)
+	if !errors.Is(err, injected) {
+		t.Fatalf("Inject() error = %v, want wrapped injected remove failure", err)
+	}
+	if !strings.Contains(err.Error(), "remove retired output style") {
+		t.Fatalf("Inject() error = %q, want retired output style context", err)
+	}
+	if _, statErr := os.Stat(gentlemanPath); statErr != nil {
+		t.Fatalf("retired gentleman output style should remain after failed removal, stat err=%v", statErr)
+	}
+}
+
 func TestInjectCustomClaudeDoesNothing(t *testing.T) {
 	home := t.TempDir()