fix(daemon): atomically publish theme config
- commit
- efa8333e58032e70ef408aceae189706cd0e0816
- author
- brice · · 2026-07-17 08:47 UTC
- parent
- 8f22d3a
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +117 -10
M internal/usecase/daemon/config.go +24 -4
| @@ -16,6 +16,26 @@ import ( | |||
| 16 | 16 | ||
| 17 | 17 | var commandCodePattern = regexp.MustCompile(`^[A-Z0-9]{2,3}$`) | |
| 18 | 18 | ||
| 19 | + | // themeConfigSnapshot is published as one immutable value. Its zero value is | |
| 20 | + | // the default theme configuration: automatic mode with palette inheritance on. | |
| 21 | + | type themeConfigSnapshot struct { | |
| 22 | + | mode domain.ThemeMode | |
| 23 | + | paletteOff bool | |
| 24 | + | } | |
| 25 | + | ||
| 26 | + | func (d *Daemon) storeThemeConfig(cfg domain.Config) { | |
| 27 | + | snapshot := themeConfigSnapshot{mode: cfg.Theme, paletteOff: !cfg.ThemePalette} | |
| 28 | + | d.themeConfig.Store(&snapshot) | |
| 29 | + | } | |
| 30 | + | ||
| 31 | + | func (d *Daemon) currentThemeConfig() themeConfigSnapshot { | |
| 32 | + | snapshot := d.themeConfig.Load() | |
| 33 | + | if snapshot == nil { | |
| 34 | + | return themeConfigSnapshot{} | |
| 35 | + | } | |
| 36 | + | return *snapshot | |
| 37 | + | } | |
| 38 | + | ||
| 19 | 39 | // ApplyConfig validates and atomically swaps daemon runtime configuration. | |
| 20 | 40 | func (d *Daemon) ApplyConfig(cfg domain.Config) { | |
| 21 | 41 | bindings, warnings := keys.BuildBindingEntries(cfg.BindingEntries) | |
| @@ -35,8 +55,7 @@ func (d *Daemon) ApplyConfig(cfg domain.Config) { | |||
| 35 | 55 | d.paletteConfig.Store(&palette) | |
| 36 | 56 | tabs := cfg.Tabs | |
| 37 | 57 | d.tabsConfig.Store(&tabs) | |
| 38 | - | d.themeMode.Store(uint32(cfg.Theme)) | |
| 39 | - | d.themePaletteOff.Store(!cfg.ThemePalette) | |
| 58 | + | d.storeThemeConfig(cfg) | |
| 40 | 59 | if d.barScripts != nil { | |
| 41 | 60 | d.barScripts.mu.Lock() | |
| 42 | 61 | d.barScripts.initLocked() | |
| @@ -241,13 +260,14 @@ func (d *Daemon) commandByEffectiveCode(code string) (command.Command, bool) { | |||
| 241 | 260 | } | |
| 242 | 261 | ||
| 243 | 262 | func (d *Daemon) effectiveTheme(clientTheme theme.Theme) theme.Theme { | |
| 244 | - | switch domain.ThemeMode(d.themeMode.Load()) { | |
| 263 | + | config := d.currentThemeConfig() | |
| 264 | + | switch config.mode { | |
| 245 | 265 | case domain.ThemeDark: | |
| 246 | 266 | return theme.BuiltinDark | |
| 247 | 267 | case domain.ThemeLight: | |
| 248 | 268 | return theme.BuiltinLight | |
| 249 | 269 | default: | |
| 250 | - | clientTheme.UsePalette = !d.themePaletteOff.Load() | |
| 270 | + | clientTheme.UsePalette = !config.paletteOff | |
| 251 | 271 | return clientTheme | |
| 252 | 272 | } | |
| 253 | 273 | } | |
| @@ -16,6 +16,26 @@ import ( | |||||
| 16 | 16 | ||||
| 17 | var commandCodePattern = regexp.MustCompile(`^[A-Z0-9]{2,3}$`) | 17 | var commandCodePattern = regexp.MustCompile(`^[A-Z0-9]{2,3}$`) | ||
| 18 | 18 | ||||
| 19 | + | // themeConfigSnapshot is published as one immutable value. Its zero value is | |||
| 20 | + | // the default theme configuration: automatic mode with palette inheritance on. | |||
| 21 | + | type themeConfigSnapshot struct { | |||
| 22 | + | mode domain.ThemeMode | |||
| 23 | + | paletteOff bool | |||
| 24 | + | } | |||
| 25 | + | ||||
| 26 | + | func (d *Daemon) storeThemeConfig(cfg domain.Config) { | |||
| 27 | + | snapshot := themeConfigSnapshot{mode: cfg.Theme, paletteOff: !cfg.ThemePalette} | |||
| 28 | + | d.themeConfig.Store(&snapshot) | |||
| 29 | + | } | |||
| 30 | + | ||||
| 31 | + | func (d *Daemon) currentThemeConfig() themeConfigSnapshot { | |||
| 32 | + | snapshot := d.themeConfig.Load() | |||
| 33 | + | if snapshot == nil { | |||
| 34 | + | return themeConfigSnapshot{} | |||
| 35 | + | } | |||
| 36 | + | return *snapshot | |||
| 37 | + | } | |||
| 38 | + | ||||
| 19 | // ApplyConfig validates and atomically swaps daemon runtime configuration. | 39 | // ApplyConfig validates and atomically swaps daemon runtime configuration. | ||
| 20 | func (d *Daemon) ApplyConfig(cfg domain.Config) { | 40 | func (d *Daemon) ApplyConfig(cfg domain.Config) { | ||
| 21 | bindings, warnings := keys.BuildBindingEntries(cfg.BindingEntries) | 41 | bindings, warnings := keys.BuildBindingEntries(cfg.BindingEntries) | ||
| @@ -35,8 +55,7 @@ func (d *Daemon) ApplyConfig(cfg domain.Config) { | |||||
| 35 | d.paletteConfig.Store(&palette) | 55 | d.paletteConfig.Store(&palette) | ||
| 36 | tabs := cfg.Tabs | 56 | tabs := cfg.Tabs | ||
| 37 | d.tabsConfig.Store(&tabs) | 57 | d.tabsConfig.Store(&tabs) | ||
| 38 | - | d.themeMode.Store(uint32(cfg.Theme)) | 58 | + | d.storeThemeConfig(cfg) |
| 39 | - | d.themePaletteOff.Store(!cfg.ThemePalette) | |||
| 40 | if d.barScripts != nil { | 59 | if d.barScripts != nil { | ||
| 41 | d.barScripts.mu.Lock() | 60 | d.barScripts.mu.Lock() | ||
| 42 | d.barScripts.initLocked() | 61 | d.barScripts.initLocked() | ||
| @@ -241,13 +260,14 @@ func (d *Daemon) commandByEffectiveCode(code string) (command.Command, bool) { | |||||
| 241 | } | 260 | } | ||
| 242 | 261 | ||||
| 243 | func (d *Daemon) effectiveTheme(clientTheme theme.Theme) theme.Theme { | 262 | func (d *Daemon) effectiveTheme(clientTheme theme.Theme) theme.Theme { | ||
| 244 | - | switch domain.ThemeMode(d.themeMode.Load()) { | 263 | + | config := d.currentThemeConfig() |
| 264 | + | switch config.mode { | |||
| 245 | case domain.ThemeDark: | 265 | case domain.ThemeDark: | ||
| 246 | return theme.BuiltinDark | 266 | return theme.BuiltinDark | ||
| 247 | case domain.ThemeLight: | 267 | case domain.ThemeLight: | ||
| 248 | return theme.BuiltinLight | 268 | return theme.BuiltinLight | ||
| 249 | default: | 269 | default: | ||
| 250 | - | clientTheme.UsePalette = !d.themePaletteOff.Load() | 270 | + | clientTheme.UsePalette = !config.paletteOff |
| 251 | return clientTheme | 271 | return clientTheme | ||
| 252 | } | 272 | } | ||
| 253 | } | 273 | } | ||
M internal/usecase/daemon/config_test.go +90 -0
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package daemon | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "sync" | |
| 4 | 5 | "testing" | |
| 5 | 6 | ||
| 6 | 7 | "github.com/stretchr/testify/require" | |
| @@ -122,6 +123,14 @@ func TestEffectiveThemePaletteGate(t *testing.T) { | |||
| 122 | 123 | }, | |
| 123 | 124 | want: themeui.BuiltinDark, | |
| 124 | 125 | }, | |
| 126 | + | { | |
| 127 | + | name: "forced dark ignores disabled palette inheritance", | |
| 128 | + | config: domain.Config{ | |
| 129 | + | Theme: domain.ThemeDark, | |
| 130 | + | ThemePalette: false, | |
| 131 | + | }, | |
| 132 | + | want: themeui.BuiltinDark, | |
| 133 | + | }, | |
| 125 | 134 | { | |
| 126 | 135 | name: "forced light uses palette free builtin", | |
| 127 | 136 | config: domain.Config{ | |
| @@ -130,6 +139,14 @@ func TestEffectiveThemePaletteGate(t *testing.T) { | |||
| 130 | 139 | }, | |
| 131 | 140 | want: themeui.BuiltinLight, | |
| 132 | 141 | }, | |
| 142 | + | { | |
| 143 | + | name: "forced light ignores disabled palette inheritance", | |
| 144 | + | config: domain.Config{ | |
| 145 | + | Theme: domain.ThemeLight, | |
| 146 | + | ThemePalette: false, | |
| 147 | + | }, | |
| 148 | + | want: themeui.BuiltinLight, | |
| 149 | + | }, | |
| 133 | 150 | } | |
| 134 | 151 | ||
| 135 | 152 | for _, tt := range tests { | |
| @@ -145,6 +162,79 @@ func TestEffectiveThemePaletteGate(t *testing.T) { | |||
| 145 | 162 | } | |
| 146 | 163 | } | |
| 147 | 164 | ||
| 165 | + | func TestThemeConfigSnapshotIsAtomic(t *testing.T) { | |
| 166 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 167 | + | ||
| 168 | + | // A nil atomic pointer is the zero/default snapshot: auto theme with | |
| 169 | + | // terminal palette inheritance enabled. | |
| 170 | + | require.Equal(t, themeConfigSnapshot{}, d.currentThemeConfig()) | |
| 171 | + | ||
| 172 | + | for _, tt := range []struct { | |
| 173 | + | name string | |
| 174 | + | mode domain.ThemeMode | |
| 175 | + | gate bool | |
| 176 | + | }{ | |
| 177 | + | {name: "auto palette enabled", mode: domain.ThemeAuto, gate: true}, | |
| 178 | + | {name: "auto palette disabled", mode: domain.ThemeAuto, gate: false}, | |
| 179 | + | {name: "dark palette enabled", mode: domain.ThemeDark, gate: true}, | |
| 180 | + | {name: "dark palette disabled", mode: domain.ThemeDark, gate: false}, | |
| 181 | + | {name: "light palette enabled", mode: domain.ThemeLight, gate: true}, | |
| 182 | + | {name: "light palette disabled", mode: domain.ThemeLight, gate: false}, | |
| 183 | + | } { | |
| 184 | + | t.Run(tt.name, func(t *testing.T) { | |
| 185 | + | d.storeThemeConfig(domain.Config{Theme: tt.mode, ThemePalette: tt.gate}) | |
| 186 | + | require.Equal(t, themeConfigSnapshot{mode: tt.mode, paletteOff: !tt.gate}, d.currentThemeConfig()) | |
| 187 | + | }) | |
| 188 | + | } | |
| 189 | + | ||
| 190 | + | // Publish complementary configurations concurrently. Every load must be a | |
| 191 | + | // complete published snapshot, never mode from one update and gate from | |
| 192 | + | // another. Coordination establishes concurrent readers/writers without | |
| 193 | + | // timing sleeps. | |
| 194 | + | first := domain.Config{Theme: domain.ThemeAuto, ThemePalette: false} | |
| 195 | + | second := domain.Config{Theme: domain.ThemeDark, ThemePalette: true} | |
| 196 | + | allowed := map[themeConfigSnapshot]struct{}{ | |
| 197 | + | {mode: domain.ThemeAuto, paletteOff: true}: {}, | |
| 198 | + | {mode: domain.ThemeDark, paletteOff: false}: {}, | |
| 199 | + | } | |
| 200 | + | ready := make(chan struct{}) | |
| 201 | + | readerStarted := make(chan struct{}) | |
| 202 | + | invalid := make(chan themeConfigSnapshot, 1) | |
| 203 | + | var wg sync.WaitGroup | |
| 204 | + | wg.Add(2) | |
| 205 | + | go func() { | |
| 206 | + | defer wg.Done() | |
| 207 | + | d.storeThemeConfig(first) | |
| 208 | + | close(ready) | |
| 209 | + | <-readerStarted | |
| 210 | + | for range 10_000 { | |
| 211 | + | d.storeThemeConfig(second) | |
| 212 | + | d.storeThemeConfig(first) | |
| 213 | + | } | |
| 214 | + | }() | |
| 215 | + | go func() { | |
| 216 | + | defer wg.Done() | |
| 217 | + | <-ready | |
| 218 | + | close(readerStarted) | |
| 219 | + | for range 20_000 { | |
| 220 | + | snapshot := d.currentThemeConfig() | |
| 221 | + | if _, ok := allowed[snapshot]; !ok { | |
| 222 | + | select { | |
| 223 | + | case invalid <- snapshot: | |
| 224 | + | default: | |
| 225 | + | } | |
| 226 | + | return | |
| 227 | + | } | |
| 228 | + | } | |
| 229 | + | }() | |
| 230 | + | wg.Wait() | |
| 231 | + | select { | |
| 232 | + | case snapshot := <-invalid: | |
| 233 | + | t.Fatalf("observed mixed theme configuration: %#v", snapshot) | |
| 234 | + | default: | |
| 235 | + | } | |
| 236 | + | } | |
| 237 | + | ||
| 148 | 238 | func TestApplyConfigPublishesCopyConfig(t *testing.T) { | |
| 149 | 239 | tests := []struct { | |
| 150 | 240 | name string | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package daemon | 1 | package daemon | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "sync" | |||
| 4 | "testing" | 5 | "testing" | ||
| 5 | 6 | ||||
| 6 | "github.com/stretchr/testify/require" | 7 | "github.com/stretchr/testify/require" | ||
| @@ -122,6 +123,14 @@ func TestEffectiveThemePaletteGate(t *testing.T) { | |||||
| 122 | }, | 123 | }, | ||
| 123 | want: themeui.BuiltinDark, | 124 | want: themeui.BuiltinDark, | ||
| 124 | }, | 125 | }, | ||
| 126 | + | { | |||
| 127 | + | name: "forced dark ignores disabled palette inheritance", | |||
| 128 | + | config: domain.Config{ | |||
| 129 | + | Theme: domain.ThemeDark, | |||
| 130 | + | ThemePalette: false, | |||
| 131 | + | }, | |||
| 132 | + | want: themeui.BuiltinDark, | |||
| 133 | + | }, | |||
| 125 | { | 134 | { | ||
| 126 | name: "forced light uses palette free builtin", | 135 | name: "forced light uses palette free builtin", | ||
| 127 | config: domain.Config{ | 136 | config: domain.Config{ | ||
| @@ -130,6 +139,14 @@ func TestEffectiveThemePaletteGate(t *testing.T) { | |||||
| 130 | }, | 139 | }, | ||
| 131 | want: themeui.BuiltinLight, | 140 | want: themeui.BuiltinLight, | ||
| 132 | }, | 141 | }, | ||
| 142 | + | { | |||
| 143 | + | name: "forced light ignores disabled palette inheritance", | |||
| 144 | + | config: domain.Config{ | |||
| 145 | + | Theme: domain.ThemeLight, | |||
| 146 | + | ThemePalette: false, | |||
| 147 | + | }, | |||
| 148 | + | want: themeui.BuiltinLight, | |||
| 149 | + | }, | |||
| 133 | } | 150 | } | ||
| 134 | 151 | ||||
| 135 | for _, tt := range tests { | 152 | for _, tt := range tests { | ||
| @@ -145,6 +162,79 @@ func TestEffectiveThemePaletteGate(t *testing.T) { | |||||
| 145 | } | 162 | } | ||
| 146 | } | 163 | } | ||
| 147 | 164 | ||||
| 165 | + | func TestThemeConfigSnapshotIsAtomic(t *testing.T) { | |||
| 166 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 167 | + | ||||
| 168 | + | // A nil atomic pointer is the zero/default snapshot: auto theme with | |||
| 169 | + | // terminal palette inheritance enabled. | |||
| 170 | + | require.Equal(t, themeConfigSnapshot{}, d.currentThemeConfig()) | |||
| 171 | + | ||||
| 172 | + | for _, tt := range []struct { | |||
| 173 | + | name string | |||
| 174 | + | mode domain.ThemeMode | |||
| 175 | + | gate bool | |||
| 176 | + | }{ | |||
| 177 | + | {name: "auto palette enabled", mode: domain.ThemeAuto, gate: true}, | |||
| 178 | + | {name: "auto palette disabled", mode: domain.ThemeAuto, gate: false}, | |||
| 179 | + | {name: "dark palette enabled", mode: domain.ThemeDark, gate: true}, | |||
| 180 | + | {name: "dark palette disabled", mode: domain.ThemeDark, gate: false}, | |||
| 181 | + | {name: "light palette enabled", mode: domain.ThemeLight, gate: true}, | |||
| 182 | + | {name: "light palette disabled", mode: domain.ThemeLight, gate: false}, | |||
| 183 | + | } { | |||
| 184 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 185 | + | d.storeThemeConfig(domain.Config{Theme: tt.mode, ThemePalette: tt.gate}) | |||
| 186 | + | require.Equal(t, themeConfigSnapshot{mode: tt.mode, paletteOff: !tt.gate}, d.currentThemeConfig()) | |||
| 187 | + | }) | |||
| 188 | + | } | |||
| 189 | + | ||||
| 190 | + | // Publish complementary configurations concurrently. Every load must be a | |||
| 191 | + | // complete published snapshot, never mode from one update and gate from | |||
| 192 | + | // another. Coordination establishes concurrent readers/writers without | |||
| 193 | + | // timing sleeps. | |||
| 194 | + | first := domain.Config{Theme: domain.ThemeAuto, ThemePalette: false} | |||
| 195 | + | second := domain.Config{Theme: domain.ThemeDark, ThemePalette: true} | |||
| 196 | + | allowed := map[themeConfigSnapshot]struct{}{ | |||
| 197 | + | {mode: domain.ThemeAuto, paletteOff: true}: {}, | |||
| 198 | + | {mode: domain.ThemeDark, paletteOff: false}: {}, | |||
| 199 | + | } | |||
| 200 | + | ready := make(chan struct{}) | |||
| 201 | + | readerStarted := make(chan struct{}) | |||
| 202 | + | invalid := make(chan themeConfigSnapshot, 1) | |||
| 203 | + | var wg sync.WaitGroup | |||
| 204 | + | wg.Add(2) | |||
| 205 | + | go func() { | |||
| 206 | + | defer wg.Done() | |||
| 207 | + | d.storeThemeConfig(first) | |||
| 208 | + | close(ready) | |||
| 209 | + | <-readerStarted | |||
| 210 | + | for range 10_000 { | |||
| 211 | + | d.storeThemeConfig(second) | |||
| 212 | + | d.storeThemeConfig(first) | |||
| 213 | + | } | |||
| 214 | + | }() | |||
| 215 | + | go func() { | |||
| 216 | + | defer wg.Done() | |||
| 217 | + | <-ready | |||
| 218 | + | close(readerStarted) | |||
| 219 | + | for range 20_000 { | |||
| 220 | + | snapshot := d.currentThemeConfig() | |||
| 221 | + | if _, ok := allowed[snapshot]; !ok { | |||
| 222 | + | select { | |||
| 223 | + | case invalid <- snapshot: | |||
| 224 | + | default: | |||
| 225 | + | } | |||
| 226 | + | return | |||
| 227 | + | } | |||
| 228 | + | } | |||
| 229 | + | }() | |||
| 230 | + | wg.Wait() | |||
| 231 | + | select { | |||
| 232 | + | case snapshot := <-invalid: | |||
| 233 | + | t.Fatalf("observed mixed theme configuration: %#v", snapshot) | |||
| 234 | + | default: | |||
| 235 | + | } | |||
| 236 | + | } | |||
| 237 | + | ||||
| 148 | func TestApplyConfigPublishesCopyConfig(t *testing.T) { | 238 | func TestApplyConfigPublishesCopyConfig(t *testing.T) { | ||
| 149 | tests := []struct { | 239 | tests := []struct { | ||
| 150 | name string | 240 | name string | ||
M internal/usecase/daemon/daemon.go +3 -6
| @@ -144,12 +144,9 @@ type Daemon struct { | |||
| 144 | 144 | copyConfig atomic.Pointer[domain.CopyConfig] | |
| 145 | 145 | paletteConfig atomic.Pointer[domain.PaletteConfig] | |
| 146 | 146 | tabsConfig atomic.Pointer[domain.TabsConfig] | |
| 147 | - | themeMode atomic.Uint32 | |
| 148 | - | // themePaletteOff is deliberately inverted so its zero value preserves the | |
| 149 | - | // default-on terminal palette inheritance before configuration is loaded. | |
| 150 | - | themePaletteOff atomic.Bool | |
| 151 | - | barScripts *barScriptState | |
| 152 | - | resumeParkGrace time.Duration | |
| 147 | + | themeConfig atomic.Pointer[themeConfigSnapshot] | |
| 148 | + | barScripts *barScriptState | |
| 149 | + | resumeParkGrace time.Duration | |
| 153 | 150 | // tempDir overrides os.TempDir() for clipboard-image-transfer writes | |
| 154 | 151 | // (see clipboard.go); empty means use os.TempDir(). | |
| 155 | 152 | tempDir string | |
| @@ -144,12 +144,9 @@ type Daemon struct { | |||||
| 144 | copyConfig atomic.Pointer[domain.CopyConfig] | 144 | copyConfig atomic.Pointer[domain.CopyConfig] | ||
| 145 | paletteConfig atomic.Pointer[domain.PaletteConfig] | 145 | paletteConfig atomic.Pointer[domain.PaletteConfig] | ||
| 146 | tabsConfig atomic.Pointer[domain.TabsConfig] | 146 | tabsConfig atomic.Pointer[domain.TabsConfig] | ||
| 147 | - | themeMode atomic.Uint32 | 147 | + | themeConfig atomic.Pointer[themeConfigSnapshot] |
| 148 | - | // themePaletteOff is deliberately inverted so its zero value preserves the | 148 | + | barScripts *barScriptState |
| 149 | - | // default-on terminal palette inheritance before configuration is loaded. | 149 | + | resumeParkGrace time.Duration |
| 150 | - | themePaletteOff atomic.Bool | |||
| 151 | - | barScripts *barScriptState | |||
| 152 | - | resumeParkGrace time.Duration | |||
| 153 | // tempDir overrides os.TempDir() for clipboard-image-transfer writes | 150 | // tempDir overrides os.TempDir() for clipboard-image-transfer writes | ||
| 154 | // (see clipboard.go); empty means use os.TempDir(). | 151 | // (see clipboard.go); empty means use os.TempDir(). | ||
| 155 | tempDir string | 152 | tempDir string | ||