fix(agent): preserve durable LSP setting on refresh rollback

commit: dc29cb41b198649f6abec0ce49eaf242396f2f4c
author: brice · 2mo ago · 2026-08-05 07:41 UTC
parent: 4091863
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

2 files · +65 -3

M packages/agent/modes/interactive.go +13 -3


@@ -4573,15 +4573,25 @@ func (i *Interactive) applySettingToggle(key string, value bool) {
4573	4573		i.cfg.LSPEnabled = &val
4574	4574		if i.cfg.RefreshTools != nil {
4575	4575		if err := i.cfg.RefreshTools(); err != nil {
	4576	+	errMsg := "settings: refresh tools: " + err.Error()
4576	4577		rollbackErr := error(nil)
4577	4578		if hasStore {
4578	4579		rollbackErr = store.SetLSPEnabled(previous)
4579	4580		}
4580		-	previousVal := previous
4581		-	i.cfg.LSPEnabled = &previousVal
4582		-	errMsg := "settings: refresh tools: " + err.Error()
4583	4581		if rollbackErr != nil {
	4582	+	// The first write succeeded, so keep the in-memory setting at
	4583	+	// the value that remains durable instead of claiming that the
	4584	+	// rollback took effect.
4584	4585		errMsg += "; rollback persistence: " + rollbackErr.Error()
	4586	+	if reconcileErr := i.cfg.RefreshTools(); reconcileErr != nil {
	4587	+	errMsg += "; durable-state refresh: " + reconcileErr.Error()
	4588	+	}
	4589	+	} else {
	4590	+	previousVal := previous
	4591	+	i.cfg.LSPEnabled = &previousVal
	4592	+	if refreshErr := i.cfg.RefreshTools(); refreshErr != nil {
	4593	+	errMsg += "; rollback refresh: " + refreshErr.Error()
	4594	+	}
4585	4595		}
4586	4596		i.mu.Lock()
4587	4597		i.statusOK = ""

 		i.cfg.LSPEnabled = &val
 		if i.cfg.RefreshTools != nil {
 			if err := i.cfg.RefreshTools(); err != nil {
+				errMsg := "settings: refresh tools: " + err.Error()
 				rollbackErr := error(nil)
 				if hasStore {
 					rollbackErr = store.SetLSPEnabled(previous)
 				}
-				previousVal := previous
-				i.cfg.LSPEnabled = &previousVal
-				errMsg := "settings: refresh tools: " + err.Error()
 				if rollbackErr != nil {
+					// The first write succeeded, so keep the in-memory setting at
+					// the value that remains durable instead of claiming that the
+					// rollback took effect.
 					errMsg += "; rollback persistence: " + rollbackErr.Error()
+					if reconcileErr := i.cfg.RefreshTools(); reconcileErr != nil {
+						errMsg += "; durable-state refresh: " + reconcileErr.Error()
+					}
+				} else {
+					previousVal := previous
+					i.cfg.LSPEnabled = &previousVal
+					if refreshErr := i.cfg.RefreshTools(); refreshErr != nil {
+						errMsg += "; rollback refresh: " + refreshErr.Error()
+					}
 				}
 				i.mu.Lock()
 				i.statusOK = ""

M packages/agent/modes/lsp_setting_test.go +52 -0


@@ -2,6 +2,7 @@ package modes
2	2
3	3		import (
4	4		"errors"
	5	+	"strings"
5	6		"testing"
6	7		)
7	8

@@ -17,6 +18,25 @@ func (failingLSPSettingsStore) SetSubagentLSPEnabled(bool) error {
17	18		return errors.New("persist subagent lsp setting")
18	19		}
19	20
	21	+	type rollbackFailingLSPSettingsStore struct {
	22	+	SettingsStore
	23	+	calls int
	24	+	values []bool
	25	+	}
	26	+
	27	+	func (s *rollbackFailingLSPSettingsStore) SetLSPEnabled(enabled bool) error {
	28	+	s.calls++
	29	+	if s.calls == 2 {
	30	+	return errors.New("rollback disk full")
	31	+	}
	32	+	s.values = append(s.values, enabled)
	33	+	return nil
	34	+	}
	35	+
	36	+	func (s *rollbackFailingLSPSettingsStore) SetSubagentLSPEnabled(bool) error {
	37	+	return nil
	38	+	}
	39	+
20	40		func TestLSPSettingToggleKeepsMemoryOnPersistenceFailure(t *testing.T) {
21	41		mainEnabled := false
22	42		subagentEnabled := false

@@ -35,3 +55,35 @@ func TestLSPSettingToggleKeepsMemoryOnPersistenceFailure(t *testing.T) {
35	55		t.Fatal("sub-agent LSP changed after persistence failure")
36	56		}
37	57		}
	58	+
	59	+	func TestLSPSettingToggleKeepsDurableStateWhenRollbackPersistenceFails(t *testing.T) {
	60	+	mainEnabled := true
	61	+	store := &rollbackFailingLSPSettingsStore{}
	62	+	refreshCalls := 0
	63	+	interactive := NewInteractive(InteractiveConfig{
	64	+	LSPEnabled: &mainEnabled,
	65	+	SettingsStore: store,
	66	+	RefreshTools: func() error {
	67	+	refreshCalls++
	68	+	if refreshCalls == 1 {
	69	+	return errors.New("refresh failed")
	70	+	}
	71	+	return nil
	72	+	},
	73	+	})
	74	+	interactive.rend = nil
	75	+
	76	+	interactive.applySettingToggle("lsp_enabled", false)
	77	+	if *interactive.cfg.LSPEnabled {
	78	+	t.Fatal("in-memory LSP setting reverted despite rollback persistence failure")
	79	+	}
	80	+	if len(store.values) != 1 \|\| store.values[0] {
	81	+	t.Fatalf("durable writes = %v, want only the successful disable", store.values)
	82	+	}
	83	+	if refreshCalls != 2 {
	84	+	t.Fatalf("refresh calls = %d, want failed refresh plus durable-state reconciliation", refreshCalls)
	85	+	}
	86	+	if !strings.Contains(interactive.statusErr, "rollback disk full") {
	87	+	t.Fatalf("status error = %q, want rollback failure", interactive.statusErr)
	88	+	}
	89	+	}


@@ -2,6 +2,7 @@ package modes
2		2
3	import (	3		import (
4	"errors"	4		"errors"
		5	+	"strings"
5	"testing"	6		"testing"
6	)	7		)
7		8

@@ -17,6 +18,25 @@ func (failingLSPSettingsStore) SetSubagentLSPEnabled(bool) error {
17	return errors.New("persist subagent lsp setting")	18		return errors.New("persist subagent lsp setting")
18	}	19		}
19		20
		21	+	type rollbackFailingLSPSettingsStore struct {
		22	+	SettingsStore
		23	+	calls int
		24	+	values []bool
		25	+	}
		26	+
		27	+	func (s *rollbackFailingLSPSettingsStore) SetLSPEnabled(enabled bool) error {
		28	+	s.calls++
		29	+	if s.calls == 2 {
		30	+	return errors.New("rollback disk full")
		31	+	}
		32	+	s.values = append(s.values, enabled)
		33	+	return nil
		34	+	}
		35	+
		36	+	func (s *rollbackFailingLSPSettingsStore) SetSubagentLSPEnabled(bool) error {
		37	+	return nil
		38	+	}
		39	+
20	func TestLSPSettingToggleKeepsMemoryOnPersistenceFailure(t *testing.T) {	40		func TestLSPSettingToggleKeepsMemoryOnPersistenceFailure(t *testing.T) {
21	mainEnabled := false	41		mainEnabled := false
22	subagentEnabled := false	42		subagentEnabled := false

@@ -35,3 +55,35 @@ func TestLSPSettingToggleKeepsMemoryOnPersistenceFailure(t *testing.T) {
35	t.Fatal("sub-agent LSP changed after persistence failure")	55		t.Fatal("sub-agent LSP changed after persistence failure")
36	}	56		}
37	}	57		}
		58	+
		59	+	func TestLSPSettingToggleKeepsDurableStateWhenRollbackPersistenceFails(t *testing.T) {
		60	+	mainEnabled := true
		61	+	store := &rollbackFailingLSPSettingsStore{}
		62	+	refreshCalls := 0
		63	+	interactive := NewInteractive(InteractiveConfig{
		64	+	LSPEnabled: &mainEnabled,
		65	+	SettingsStore: store,
		66	+	RefreshTools: func() error {
		67	+	refreshCalls++
		68	+	if refreshCalls == 1 {
		69	+	return errors.New("refresh failed")
		70	+	}
		71	+	return nil
		72	+	},
		73	+	})
		74	+	interactive.rend = nil
		75	+
		76	+	interactive.applySettingToggle("lsp_enabled", false)
		77	+	if *interactive.cfg.LSPEnabled {
		78	+	t.Fatal("in-memory LSP setting reverted despite rollback persistence failure")
		79	+	}
		80	+	if len(store.values) != 1 \|\| store.values[0] {
		81	+	t.Fatalf("durable writes = %v, want only the successful disable", store.values)
		82	+	}
		83	+	if refreshCalls != 2 {
		84	+	t.Fatalf("refresh calls = %d, want failed refresh plus durable-state reconciliation", refreshCalls)
		85	+	}
		86	+	if !strings.Contains(interactive.statusErr, "rollback disk full") {
		87	+	t.Fatalf("status error = %q, want rollback failure", interactive.statusErr)
		88	+	}
		89	+	}