detached 0d2fc9a

fix(config): apply preference updates atomically under the manager lock

Add Manager.Update, which applies a mutation to the latest config and
validates, normalizes and writes it under a single write lock. The mode
legend and omnibox preference gateways use it instead of Get() then Save(),
so a concurrent save or reload can no longer be overwritten by a stale
whole-config snapshot.
commit
0d2fc9afe0d627cbbb239bb9ca2c1598af03a330
author
brice · · 2026-10-14 07:31 UTC
parent
6e02386
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
4 files · +144 -8
Unified
UnifiedSplit
M internal/infrastructure/config/loader.go +26 -0
@@ -517,6 +517,32 @@ func (m *Manager) Save(cfg *Config) error {
517517 m.mu.Lock()
518518 defer m.mu.Unlock()
519519
520+ return m.saveLocked(cfg)
521+}
522+
523+// Update applies mutate to a copy of the latest configuration and persists the
524+// result while holding the manager's write lock. Unlike Get() followed by
525+// Save(), no concurrent reload or save can be overwritten by a stale snapshot.
526+//
527+// mutate runs under the lock: it must not call back into the Manager, and it
528+// should only replace fields of the config it receives (the copy is shallow,
529+// so nested maps and slices must be replaced rather than modified in place).
530+func (m *Manager) Update(mutate func(*Config)) error {
531+ if mutate == nil {
532+ return fmt.Errorf("config mutation is nil")
533+ }
534+
535+ m.mu.Lock()
536+ defer m.mu.Unlock()
537+
538+ updated := *m.config
539+ mutate(&updated)
540+
541+ return m.saveLocked(&updated)
542+}
543+
544+// saveLocked validates, writes and publishes cfg. Must be called with m.mu held for write.
545+func (m *Manager) saveLocked(cfg *Config) error {
520546 if cfg == nil {
521547 return fmt.Errorf("config is nil")
522548 }
@@ -517,6 +517,32 @@ func (m *Manager) Save(cfg *Config) error {
517 m.mu.Lock()517 m.mu.Lock()
518 defer m.mu.Unlock()518 defer m.mu.Unlock()
519 519
520+ return m.saveLocked(cfg)
521+}
522+
523+// Update applies mutate to a copy of the latest configuration and persists the
524+// result while holding the manager's write lock. Unlike Get() followed by
525+// Save(), no concurrent reload or save can be overwritten by a stale snapshot.
526+//
527+// mutate runs under the lock: it must not call back into the Manager, and it
528+// should only replace fields of the config it receives (the copy is shallow,
529+// so nested maps and slices must be replaced rather than modified in place).
530+func (m *Manager) Update(mutate func(*Config)) error {
531+ if mutate == nil {
532+ return fmt.Errorf("config mutation is nil")
533+ }
534+
535+ m.mu.Lock()
536+ defer m.mu.Unlock()
537+
538+ updated := *m.config
539+ mutate(&updated)
540+
541+ return m.saveLocked(&updated)
542+}
543+
544+// saveLocked validates, writes and publishes cfg. Must be called with m.mu held for write.
545+func (m *Manager) saveLocked(cfg *Config) error {
520 if cfg == nil {546 if cfg == nil {
521 return fmt.Errorf("config is nil")547 return fmt.Errorf("config is nil")
522 }548 }
M internal/infrastructure/config/mode_legend_preferences_gateway.go +3 -4
@@ -19,8 +19,7 @@ func (g *ModeLegendPreferencesGateway) SaveModeLegendCollapsed(_ context.Context
1919 return fmt.Errorf("config manager not initialized")
2020 }
2121
22- current := g.mgr.Get()
23- current.Workspace.Styling.ModeLegendCollapsed = collapsed
24-
25- return g.mgr.Save(current)
22+ return g.mgr.Update(func(cfg *Config) {
23+ cfg.Workspace.Styling.ModeLegendCollapsed = collapsed
24+ })
2625 }
@@ -19,8 +19,7 @@ func (g *ModeLegendPreferencesGateway) SaveModeLegendCollapsed(_ context.Context
19 return fmt.Errorf("config manager not initialized")19 return fmt.Errorf("config manager not initialized")
20 }20 }
21 21
22- current := g.mgr.Get()22+ return g.mgr.Update(func(cfg *Config) {
23- current.Workspace.Styling.ModeLegendCollapsed = collapsed23+ cfg.Workspace.Styling.ModeLegendCollapsed = collapsed
24-24+ })
25- return g.mgr.Save(current)
26 }25 }
M internal/infrastructure/config/mode_legend_preferences_gateway_test.go +112 -0
@@ -4,6 +4,8 @@ import (
44 "context"
55 "testing"
66
7+ "github.com/bnema/dumber/internal/domain/entity"
8+
79 "github.com/stretchr/testify/require"
810 )
911
@@ -23,3 +25,113 @@ func TestModeLegendPreferencesGateway_SaveModeLegendCollapsed_Persists(t *testin
2325 func TestModeLegendPreferencesGateway_NilManagerReturnsError(t *testing.T) {
2426 require.Error(t, NewModeLegendPreferencesGateway(nil).SaveModeLegendCollapsed(context.Background(), true))
2527 }
28+
29+// newWatchedTestManager mirrors production, where the manager is in watching mode
30+// so Save() publishes the in-memory config directly instead of re-reading the
31+// file. No fsnotify watcher is started, which keeps the tests deterministic.
32+func newWatchedTestManager(t *testing.T, configHome string) *Manager {
33+ t.Helper()
34+
35+ mgr := newTestOmniboxPreferencesGatewayManager(t, configHome)
36+ mgr.watching = true
37+
38+ return mgr
39+}
40+
41+func TestManagerUpdate_OverlappingUpdatesDoNotLoseWrites(t *testing.T) {
42+ configHome := t.TempDir()
43+ mgr := newWatchedTestManager(t, configHome)
44+ legend := NewModeLegendPreferencesGateway(mgr)
45+ omnibox := NewOmniboxPreferencesGateway(mgr)
46+
47+ otherStarted := make(chan struct{})
48+ otherDone := make(chan error, 1)
49+
50+ // The first update starts a second one while it still holds the lock. The
51+ // second update must wait and then be applied on top of the first result.
52+ err := mgr.Update(func(cfg *Config) {
53+ go func() {
54+ close(otherStarted)
55+ otherDone <- omnibox.SaveOmniboxInitialBehavior(context.Background(), entity.OmniboxInitialBehaviorMostVisited)
56+ }()
57+ <-otherStarted
58+ cfg.Workspace.Styling.ModeLegendCollapsed = true
59+ })
60+ require.NoError(t, err)
61+ require.NoError(t, <-otherDone)
62+
63+ got := mgr.Get()
64+ require.True(t, got.Workspace.Styling.ModeLegendCollapsed)
65+ require.Equal(t, entity.OmniboxInitialBehaviorMostVisited, got.Omnibox.InitialBehavior)
66+
67+ reloaded := newTestOmniboxPreferencesGatewayManager(t, configHome).Get()
68+ require.True(t, reloaded.Workspace.Styling.ModeLegendCollapsed)
69+ require.Equal(t, entity.OmniboxInitialBehaviorMostVisited, reloaded.Omnibox.InitialBehavior)
70+
71+ require.NoError(t, legend.SaveModeLegendCollapsed(context.Background(), false))
72+ got = mgr.Get()
73+ require.False(t, got.Workspace.Styling.ModeLegendCollapsed)
74+ require.Equal(t, entity.OmniboxInitialBehaviorMostVisited, got.Omnibox.InitialBehavior)
75+}
76+
77+func TestManagerUpdate_AppliesToLatestConfigAfterSave(t *testing.T) {
78+ mgr := newWatchedTestManager(t, t.TempDir())
79+
80+ cfg := mgr.Get()
81+ cfg.Omnibox.InitialBehavior = entity.OmniboxInitialBehaviorMostVisited
82+ require.NoError(t, mgr.Save(cfg))
83+
84+ require.NoError(t, NewModeLegendPreferencesGateway(mgr).SaveModeLegendCollapsed(context.Background(), true))
85+
86+ got := mgr.Get()
87+ require.True(t, got.Workspace.Styling.ModeLegendCollapsed)
88+ require.Equal(t, entity.OmniboxInitialBehaviorMostVisited, got.Omnibox.InitialBehavior)
89+}
90+
91+func TestManagerUpdate_RejectsNilAndInvalidMutations(t *testing.T) {
92+ mgr := newWatchedTestManager(t, t.TempDir())
93+ before := mgr.Get().Workspace.Styling.ModeLegendDelayMs
94+
95+ require.Error(t, mgr.Update(nil))
96+ require.Error(t, mgr.Update(func(cfg *Config) { cfg.Workspace.Styling.ModeLegendDelayMs = -1 }))
97+ require.Equal(t, before, mgr.Get().Workspace.Styling.ModeLegendDelayMs)
98+}
99+
100+func TestManagerUpdate_ConcurrentUpdatesAreAllPersisted(t *testing.T) {
101+ configHome := t.TempDir()
102+ mgr := newWatchedTestManager(t, configHome)
103+
104+ mutations := []func(*Config){
105+ func(cfg *Config) { cfg.Workspace.Styling.ModeLegendCollapsed = true },
106+ func(cfg *Config) { cfg.Omnibox.InitialBehavior = entity.OmniboxInitialBehaviorNone },
107+ func(cfg *Config) { cfg.Workspace.Styling.ModeLegendDelayMs = 1234 },
108+ func(cfg *Config) { cfg.Workspace.Styling.TransitionDuration = 321 },
109+ func(cfg *Config) { cfg.Workspace.Styling.ModeBorderWidth = 7 },
110+ func(cfg *Config) { cfg.Workspace.Styling.ModeLegendLinger = false },
111+ }
112+
113+ start := make(chan struct{})
114+ errs := make(chan error, len(mutations))
115+ for _, mutate := range mutations {
116+ go func() {
117+ <-start
118+ errs <- mgr.Update(mutate)
119+ }()
120+ }
121+ close(start)
122+ for range mutations {
123+ require.NoError(t, <-errs)
124+ }
125+
126+ for name, got := range map[string]*Config{
127+ "in-memory": mgr.Get(),
128+ "on-disk": newTestOmniboxPreferencesGatewayManager(t, configHome).Get(),
129+ } {
130+ require.True(t, got.Workspace.Styling.ModeLegendCollapsed, name)
131+ require.Equal(t, entity.OmniboxInitialBehaviorNone, got.Omnibox.InitialBehavior, name)
132+ require.Equal(t, 1234, got.Workspace.Styling.ModeLegendDelayMs, name)
133+ require.Equal(t, 321, got.Workspace.Styling.TransitionDuration, name)
134+ require.Equal(t, 7, got.Workspace.Styling.ModeBorderWidth, name)
135+ require.False(t, got.Workspace.Styling.ModeLegendLinger, name)
136+ }
137+}
@@ -4,6 +4,8 @@ import (
4 "context"4 "context"
5 "testing"5 "testing"
6 6
7+ "github.com/bnema/dumber/internal/domain/entity"
8+
7 "github.com/stretchr/testify/require"9 "github.com/stretchr/testify/require"
8 )10 )
9 11
@@ -23,3 +25,113 @@ func TestModeLegendPreferencesGateway_SaveModeLegendCollapsed_Persists(t *testin
23 func TestModeLegendPreferencesGateway_NilManagerReturnsError(t *testing.T) {25 func TestModeLegendPreferencesGateway_NilManagerReturnsError(t *testing.T) {
24 require.Error(t, NewModeLegendPreferencesGateway(nil).SaveModeLegendCollapsed(context.Background(), true))26 require.Error(t, NewModeLegendPreferencesGateway(nil).SaveModeLegendCollapsed(context.Background(), true))
25 }27 }
28+
29+// newWatchedTestManager mirrors production, where the manager is in watching mode
30+// so Save() publishes the in-memory config directly instead of re-reading the
31+// file. No fsnotify watcher is started, which keeps the tests deterministic.
32+func newWatchedTestManager(t *testing.T, configHome string) *Manager {
33+ t.Helper()
34+
35+ mgr := newTestOmniboxPreferencesGatewayManager(t, configHome)
36+ mgr.watching = true
37+
38+ return mgr
39+}
40+
41+func TestManagerUpdate_OverlappingUpdatesDoNotLoseWrites(t *testing.T) {
42+ configHome := t.TempDir()
43+ mgr := newWatchedTestManager(t, configHome)
44+ legend := NewModeLegendPreferencesGateway(mgr)
45+ omnibox := NewOmniboxPreferencesGateway(mgr)
46+
47+ otherStarted := make(chan struct{})
48+ otherDone := make(chan error, 1)
49+
50+ // The first update starts a second one while it still holds the lock. The
51+ // second update must wait and then be applied on top of the first result.
52+ err := mgr.Update(func(cfg *Config) {
53+ go func() {
54+ close(otherStarted)
55+ otherDone <- omnibox.SaveOmniboxInitialBehavior(context.Background(), entity.OmniboxInitialBehaviorMostVisited)
56+ }()
57+ <-otherStarted
58+ cfg.Workspace.Styling.ModeLegendCollapsed = true
59+ })
60+ require.NoError(t, err)
61+ require.NoError(t, <-otherDone)
62+
63+ got := mgr.Get()
64+ require.True(t, got.Workspace.Styling.ModeLegendCollapsed)
65+ require.Equal(t, entity.OmniboxInitialBehaviorMostVisited, got.Omnibox.InitialBehavior)
66+
67+ reloaded := newTestOmniboxPreferencesGatewayManager(t, configHome).Get()
68+ require.True(t, reloaded.Workspace.Styling.ModeLegendCollapsed)
69+ require.Equal(t, entity.OmniboxInitialBehaviorMostVisited, reloaded.Omnibox.InitialBehavior)
70+
71+ require.NoError(t, legend.SaveModeLegendCollapsed(context.Background(), false))
72+ got = mgr.Get()
73+ require.False(t, got.Workspace.Styling.ModeLegendCollapsed)
74+ require.Equal(t, entity.OmniboxInitialBehaviorMostVisited, got.Omnibox.InitialBehavior)
75+}
76+
77+func TestManagerUpdate_AppliesToLatestConfigAfterSave(t *testing.T) {
78+ mgr := newWatchedTestManager(t, t.TempDir())
79+
80+ cfg := mgr.Get()
81+ cfg.Omnibox.InitialBehavior = entity.OmniboxInitialBehaviorMostVisited
82+ require.NoError(t, mgr.Save(cfg))
83+
84+ require.NoError(t, NewModeLegendPreferencesGateway(mgr).SaveModeLegendCollapsed(context.Background(), true))
85+
86+ got := mgr.Get()
87+ require.True(t, got.Workspace.Styling.ModeLegendCollapsed)
88+ require.Equal(t, entity.OmniboxInitialBehaviorMostVisited, got.Omnibox.InitialBehavior)
89+}
90+
91+func TestManagerUpdate_RejectsNilAndInvalidMutations(t *testing.T) {
92+ mgr := newWatchedTestManager(t, t.TempDir())
93+ before := mgr.Get().Workspace.Styling.ModeLegendDelayMs
94+
95+ require.Error(t, mgr.Update(nil))
96+ require.Error(t, mgr.Update(func(cfg *Config) { cfg.Workspace.Styling.ModeLegendDelayMs = -1 }))
97+ require.Equal(t, before, mgr.Get().Workspace.Styling.ModeLegendDelayMs)
98+}
99+
100+func TestManagerUpdate_ConcurrentUpdatesAreAllPersisted(t *testing.T) {
101+ configHome := t.TempDir()
102+ mgr := newWatchedTestManager(t, configHome)
103+
104+ mutations := []func(*Config){
105+ func(cfg *Config) { cfg.Workspace.Styling.ModeLegendCollapsed = true },
106+ func(cfg *Config) { cfg.Omnibox.InitialBehavior = entity.OmniboxInitialBehaviorNone },
107+ func(cfg *Config) { cfg.Workspace.Styling.ModeLegendDelayMs = 1234 },
108+ func(cfg *Config) { cfg.Workspace.Styling.TransitionDuration = 321 },
109+ func(cfg *Config) { cfg.Workspace.Styling.ModeBorderWidth = 7 },
110+ func(cfg *Config) { cfg.Workspace.Styling.ModeLegendLinger = false },
111+ }
112+
113+ start := make(chan struct{})
114+ errs := make(chan error, len(mutations))
115+ for _, mutate := range mutations {
116+ go func() {
117+ <-start
118+ errs <- mgr.Update(mutate)
119+ }()
120+ }
121+ close(start)
122+ for range mutations {
123+ require.NoError(t, <-errs)
124+ }
125+
126+ for name, got := range map[string]*Config{
127+ "in-memory": mgr.Get(),
128+ "on-disk": newTestOmniboxPreferencesGatewayManager(t, configHome).Get(),
129+ } {
130+ require.True(t, got.Workspace.Styling.ModeLegendCollapsed, name)
131+ require.Equal(t, entity.OmniboxInitialBehaviorNone, got.Omnibox.InitialBehavior, name)
132+ require.Equal(t, 1234, got.Workspace.Styling.ModeLegendDelayMs, name)
133+ require.Equal(t, 321, got.Workspace.Styling.TransitionDuration, name)
134+ require.Equal(t, 7, got.Workspace.Styling.ModeBorderWidth, name)
135+ require.False(t, got.Workspace.Styling.ModeLegendLinger, name)
136+ }
137+}
M internal/infrastructure/config/omnibox_preferences_gateway.go +3 -4
@@ -20,8 +20,7 @@ func (g *OmniboxPreferencesGateway) SaveOmniboxInitialBehavior(_ context.Context
2020 return fmt.Errorf("config manager not initialized")
2121 }
2222
23- current := g.mgr.Get()
24- current.Omnibox.InitialBehavior = behavior
25-
26- return g.mgr.Save(current)
23+ return g.mgr.Update(func(cfg *Config) {
24+ cfg.Omnibox.InitialBehavior = behavior
25+ })
2726 }
@@ -20,8 +20,7 @@ func (g *OmniboxPreferencesGateway) SaveOmniboxInitialBehavior(_ context.Context
20 return fmt.Errorf("config manager not initialized")20 return fmt.Errorf("config manager not initialized")
21 }21 }
22 22
23- current := g.mgr.Get()23+ return g.mgr.Update(func(cfg *Config) {
24- current.Omnibox.InitialBehavior = behavior24+ cfg.Omnibox.InitialBehavior = behavior
25-25+ })
26- return g.mgr.Save(current)
27 }26 }

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 8

Tags 42

↑↓ moveenter openesc close