From 9d15b502ad4810fd7d46b1ba348e3b1c1f56d600 Mon Sep 17 00:00:00 2001 From: Owen Atamari Date: Fri, 23 Oct 2026 17:21:01 -0500 Subject: [PATCH 1/4] test(persona): cover output-style rollback when retired style removal fails Add an os.Remove seam to the persona package and failure-injection tests proving that a failed removal of the retired Gentleman output style is returned to the caller, and that install and sync restore CLAUDE.md, both managed output styles, settings.json and user-owned styles to their exact before-images. After recovery the switch completes and a rerun is a no-op. Refs #3163 --- .../cli/persona_output_style_rollback_test.go | 168 ++++++++++++++++++ internal/components/persona/inject.go | 4 +- internal/components/persona/inject_test.go | 37 ++++ 3 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 internal/cli/persona_output_style_rollback_test.go diff --git a/internal/cli/persona_output_style_rollback_test.go b/internal/cli/persona_output_style_rollback_test.go new file mode 100644 index 0000000000..d4c9ed5775 --- /dev/null +++ b/internal/cli/persona_output_style_rollback_test.go @@ -0,0 +1,168 @@ +package cli + +import ( + "bytes" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/gentleman-programming/gentle-ai/v4/internal/agents" + "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" +) + +type personaInjectFunc func(string, agents.Adapter, model.PersonaID, string) (persona.InjectionResult, error) + +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 runs the real persona injection, then reproduces +// the observable state of a failed retired-style removal: the selected Neutral +// style and settings are already published, gentleman.md is still present, and +// the injection reports an error. +func (f outputStyleTransitionFixture) failRetiredGentlemanRemoval(t *testing.T, inject personaInjectFunc, reached *bool) personaInjectFunc { + t.Helper() + return func(home string, adapter agents.Adapter, id model.PersonaID, selectedSettingsPath string) (persona.InjectionResult, error) { + if adapter.Agent() != model.AgentClaudeCode { + return inject(home, adapter, id, selectedSettingsPath) + } + if _, err := inject(home, adapter, id, selectedSettingsPath); err != nil { + return persona.InjectionResult{}, err + } + settings, err := os.ReadFile(f.settings) + if err != nil { + return persona.InjectionResult{}, err + } + if _, err := os.Stat(f.neutral); err == nil && strings.Contains(string(settings), `"outputStyle": "Neutral"`) { + *reached = true + } + if err := os.WriteFile(f.gentleman, f.before[f.gentleman], 0o644); err != nil { + return persona.InjectionResult{}, err + } + return persona.InjectionResult{}, fmt.Errorf("remove retired output style: %w", 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 + original := injectInstallPersona + injectInstallPersona = f.failRetiredGentlemanRemoval(t, original, &reached) + t.Cleanup(func() { injectInstallPersona = original }) + + _, 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 + original := injectSyncPersona + injectSyncPersona = f.failRetiredGentlemanRemoval(t, original, &reached) + t.Cleanup(func() { injectSyncPersona = original }) + + 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. + injectSyncPersona = original + 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) + } + 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) + } +} diff --git a/internal/components/persona/inject.go b/internal/components/persona/inject.go index 5b4ac49587..1f92d836e4 100644 --- a/internal/components/persona/inject.go +++ b/internal/components/persona/inject.go @@ -997,11 +997,13 @@ func legacyVSCodePersonaPaths(homeDir string) []string { } } +var osRemove = os.Remove + // 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 } diff --git a/internal/components/persona/inject_test.go b/internal/components/persona/inject_test.go index 19c651cd21..e8603ec46d 100644 --- a/internal/components/persona/inject_test.go +++ b/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() From e4dddd763374f960f81a213b94803b7521be4618 Mon Sep 17 00:00:00 2001 From: Owen Atamari Date: Sat, 24 Oct 2026 15:00:11 -0500 Subject: [PATCH 2/4] test(persona): assert files stay settled after the idempotent sync --- .../cli/persona_output_style_rollback_test.go | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/internal/cli/persona_output_style_rollback_test.go b/internal/cli/persona_output_style_rollback_test.go index d4c9ed5775..1cfde0751f 100644 --- a/internal/cli/persona_output_style_rollback_test.go +++ b/internal/cli/persona_output_style_rollback_test.go @@ -158,6 +158,21 @@ func TestSyncRollsBackOutputStyleSwitchWhenRetiredStyleRemovalFails(t *testing.T 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) @@ -165,4 +180,16 @@ func TestSyncRollsBackOutputStyleSwitchWhenRetiredStyleRemovalFails(t *testing.T 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) + } + } } From c8a2247373388c03e93ba8eda2b9a40e3742f535 Mon Sep 17 00:00:00 2001 From: Owen Atamari Date: Mon, 26 Oct 2026 15:15:05 -0500 Subject: [PATCH 3/4] test(persona): fail the real retired style removal in CLI rollback tests --- .../cli/persona_output_style_rollback_test.go | 42 +++++++------------ internal/components/persona/inject.go | 14 +++++++ 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/internal/cli/persona_output_style_rollback_test.go b/internal/cli/persona_output_style_rollback_test.go index 1cfde0751f..cbdab30411 100644 --- a/internal/cli/persona_output_style_rollback_test.go +++ b/internal/cli/persona_output_style_rollback_test.go @@ -3,21 +3,17 @@ package cli import ( "bytes" "errors" - "fmt" "os" "path/filepath" "strings" "testing" - "github.com/gentleman-programming/gentle-ai/v4/internal/agents" "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" ) -type personaInjectFunc func(string, agents.Adapter, model.PersonaID, string) (persona.InjectionResult, error) - var errInjectedRetiredStyleRemoval = errors.New("injected retired output style removal failure") // outputStyleTransitionFixture is a Claude home with the Gentleman persona @@ -64,31 +60,25 @@ func newOutputStyleTransitionFixture(t *testing.T, home string) outputStyleTrans return f } -// failRetiredGentlemanRemoval runs the real persona injection, then reproduces -// the observable state of a failed retired-style removal: the selected Neutral -// style and settings are already published, gentleman.md is still present, and -// the injection reports an error. -func (f outputStyleTransitionFixture) failRetiredGentlemanRemoval(t *testing.T, inject personaInjectFunc, reached *bool) personaInjectFunc { +// 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() - return func(home string, adapter agents.Adapter, id model.PersonaID, selectedSettingsPath string) (persona.InjectionResult, error) { - if adapter.Agent() != model.AgentClaudeCode { - return inject(home, adapter, id, selectedSettingsPath) - } - if _, err := inject(home, adapter, id, selectedSettingsPath); err != nil { - return persona.InjectionResult{}, err + 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 persona.InjectionResult{}, err + return err } if _, err := os.Stat(f.neutral); err == nil && strings.Contains(string(settings), `"outputStyle": "Neutral"`) { *reached = true } - if err := os.WriteFile(f.gentleman, f.before[f.gentleman], 0o644); err != nil { - return persona.InjectionResult{}, err - } - return persona.InjectionResult{}, fmt.Errorf("remove retired output style: %w", errInjectedRetiredStyleRemoval) - } + return errInjectedRetiredStyleRemoval + }) } func (f outputStyleTransitionFixture) assertRestored(t *testing.T) { @@ -112,9 +102,7 @@ func TestInstallRollsBackOutputStyleSwitchWhenRetiredStyleRemovalFails(t *testin f := newOutputStyleTransitionFixture(t, home) reached := false - original := injectInstallPersona - injectInstallPersona = f.failRetiredGentlemanRemoval(t, original, &reached) - t.Cleanup(func() { injectInstallPersona = original }) + f.failRetiredGentlemanRemoval(t, &reached) _, err := RunInstall([]string{"--agent", "claude-code", "--component", "persona", "--persona", "neutral"}, system.DetectionResult{}) if !errors.Is(err, errInjectedRetiredStyleRemoval) { @@ -135,9 +123,7 @@ func TestSyncRollsBackOutputStyleSwitchWhenRetiredStyleRemovalFails(t *testing.T } reached := false - original := injectSyncPersona - injectSyncPersona = f.failRetiredGentlemanRemoval(t, original, &reached) - t.Cleanup(func() { injectSyncPersona = original }) + 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) @@ -148,7 +134,7 @@ func TestSyncRollsBackOutputStyleSwitchWhenRetiredStyleRemovalFails(t *testing.T f.assertRestored(t) // Once removal succeeds, the switch completes and a rerun is a no-op. - injectSyncPersona = original + persona.SetRemoveForTest(t, os.Remove) if _, err := RunSyncWithSelection(f.home, selection); err != nil { t.Fatalf("recovery RunSyncWithSelection() error = %v", err) } diff --git a/internal/components/persona/inject.go b/internal/components/persona/inject.go index 1f92d836e4..387173430f 100644 --- a/internal/components/persona/inject.go +++ b/internal/components/persona/inject.go @@ -999,6 +999,20 @@ 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. From e6105f7becc074ff0d27a7b54c5185368019ec2c Mon Sep 17 00:00:00 2001 From: Owen Atamari Date: Tue, 27 Oct 2026 13:22:46 -0500 Subject: [PATCH 4/4] test(persona): baseline the removal seam and assert recovery keeps user state SetRemoveForTest only has test callers. The deadcode ratchet analyzes ./cmd/gentle-ai without -test, so test callers never count, even in the same package. It is baselined like engram.SetLookPathForTest and the codex and tui test seams. The persona test now drives the failure through the same helper. The recovery sync now also checks that the user-owned output style and the unrelated settings permissions survive the successful transition. --- .deadcode-baseline.txt | 1 + internal/cli/persona_output_style_rollback_test.go | 13 ++++++++++--- internal/components/persona/inject_test.go | 8 +++----- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/.deadcode-baseline.txt b/.deadcode-baseline.txt index 74abb5b90c..a5e61f7687 100644 --- a/.deadcode-baseline.txt +++ b/.deadcode-baseline.txt @@ -60,6 +60,7 @@ internal/components/gga/config.go PostInstallMessages internal/components/gga/install.go ShouldInstall internal/components/gga/runtime.go RuntimeBinDir internal/components/mcp/inject.go context7Args +internal/components/persona/inject.go SetRemoveForTest internal/components/telemetryruntime/managed.go Reconcile internal/filecoord/lock.go UnsupportedError.Error internal/filecoord/lock.go UnsupportedError.Unwrap diff --git a/internal/cli/persona_output_style_rollback_test.go b/internal/cli/persona_output_style_rollback_test.go index cbdab30411..e4649c8b3c 100644 --- a/internal/cli/persona_output_style_rollback_test.go +++ b/internal/cli/persona_output_style_rollback_test.go @@ -23,6 +23,7 @@ type outputStyleTransitionFixture struct { gentleman string neutral string settings string + userStyle string before map[string][]byte } @@ -34,17 +35,17 @@ func newOutputStyleTransitionFixture(t *testing.T, home string) outputStyleTrans 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"), } - 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")) + mustWriteFile(t, f.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")} { + for _, path := range []string{f.gentleman, f.settings, f.userStyle, filepath.Join(claudeDir, "CLAUDE.md")} { content, err := os.ReadFile(path) if err != nil { t.Fatalf("precondition: read %s: %v", path, err) @@ -144,6 +145,12 @@ func TestSyncRollsBackOutputStyleSwitchWhenRetiredStyleRemovalFails(t *testing.T if _, err := os.Stat(f.neutral); err != nil { t.Fatalf("recovery sync did not publish neutral output style: %v", err) } + if got, err := os.ReadFile(f.userStyle); err != nil || !bytes.Equal(got, f.before[f.userStyle]) { + t.Fatalf("recovery sync changed user-owned output style = %q, err=%v", got, err) + } + if got, err := os.ReadFile(f.settings); err != nil || !strings.Contains(string(got), `"Read"`) { + t.Fatalf("recovery sync dropped unrelated permissions from settings = %s, err=%v", got, err) + } settled := map[string][]byte{} for path := range f.before { if path == f.gentleman { diff --git a/internal/components/persona/inject_test.go b/internal/components/persona/inject_test.go index e8603ec46d..712b238d8c 100644 --- a/internal/components/persona/inject_test.go +++ b/internal/components/persona/inject_test.go @@ -620,14 +620,12 @@ func TestInjectClaudeNeutralReturnsRetiredStyleRemovalFailure(t *testing.T) { } injected := errors.New("injected remove failure") - originalRemove := osRemove - t.Cleanup(func() { osRemove = originalRemove }) - osRemove = func(path string) error { + SetRemoveForTest(t, func(path string) error { if path == gentlemanPath { return injected } - return originalRemove(path) - } + return os.Remove(path) + }) _, err := Inject(home, claudeAdapter(), model.PersonaNeutral) if !errors.Is(err, injected) {