fix(config): decode each load from a transformed copy instead of persistent viper overrides

commit: ba63da0a0d9f1ee852f09275a2cfbaabc2032bd4
author: brice · 8d ago · 2026-10-14 08:32 UTC
parent: 9a83276
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

5 files · +157 -50

M internal/infrastructure/config/browsing_contexts_test.go +17 -14


@@ -273,20 +273,21 @@ func TestLegacyPopupsFullRoundTrip(t *testing.T) {
273	273		m.viper.Set("workspace.popups.oauth_auto_close", false)
274	274
275	275		// Transform should map popups → browsing_contexts.
276		-	m.transformLegacyConfig()
	276	+	settings, err := m.transformedSettings()
	277	+	require.NoError(t, err)
277	278
278	279		// popups key should be gone from viper after transform.
279		-	assert.False(t, m.viper.IsSet("workspace.popups.behavior"),
	280	+	assert.False(t, settings.IsSet("workspace.popups.behavior"),
280	281		"workspace.popups.behavior should not exist after transform")
281	282
282	283		// browsing_contexts should now hold the values.
283		-	assert.Equal(t, "stacked", m.viper.GetString("workspace.browsing_contexts.behavior"))
284		-	assert.Equal(t, "left", m.viper.GetString("workspace.browsing_contexts.placement"))
285		-	assert.False(t, m.viper.GetBool("workspace.browsing_contexts.oauth_auto_close"))
	284	+	assert.Equal(t, "stacked", settings.GetString("workspace.browsing_contexts.behavior"))
	285	+	assert.Equal(t, "left", settings.GetString("workspace.browsing_contexts.placement"))
	286	+	assert.False(t, settings.GetBool("workspace.browsing_contexts.oauth_auto_close"))
286	287
287	288		// Unmarshal should succeed.
288	289		cfg := &Config{}
289		-	err := m.viper.Unmarshal(cfg)
	290	+	err = settings.Unmarshal(cfg)
290	291		require.NoError(t, err)
291	292
292	293		// BrowsingContexts should have the legacy values.

@@ -319,14 +320,15 @@ oauth_auto_close = false
319	320		m.setDefaults()
320	321		require.NoError(t, m.viper.ReadInConfig())
321	322
322		-	m.transformLegacyConfig()
	323	+	settings, err := m.transformedSettings()
	324	+	require.NoError(t, err)
323	325
324		-	assert.Equal(t, "stacked", m.viper.GetString("workspace.browsing_contexts.behavior"))
325		-	assert.Equal(t, "left", m.viper.GetString("workspace.browsing_contexts.placement"))
326		-	assert.False(t, m.viper.GetBool("workspace.browsing_contexts.oauth_auto_close"))
	326	+	assert.Equal(t, "stacked", settings.GetString("workspace.browsing_contexts.behavior"))
	327	+	assert.Equal(t, "left", settings.GetString("workspace.browsing_contexts.placement"))
	328	+	assert.False(t, settings.GetBool("workspace.browsing_contexts.oauth_auto_close"))
327	329
328	330		cfg := &Config{}
329		-	require.NoError(t, m.viper.Unmarshal(cfg))
	331	+	require.NoError(t, settings.Unmarshal(cfg))
330	332		normalizeConfig(cfg)
331	333
332	334		assert.Equal(t, PopupBehaviorStacked, cfg.Workspace.BrowsingContexts.Behavior)

@@ -354,10 +356,11 @@ placement = "left"
354	356		m.setDefaults()
355	357		require.NoError(t, m.viper.ReadInConfig())
356	358
357		-	m.transformLegacyConfig()
	359	+	settings, err := m.transformedSettings()
	360	+	require.NoError(t, err)
358	361
359		-	assert.Equal(t, "tabbed", m.viper.GetString("workspace.browsing_contexts.behavior"))
360		-	assert.Equal(t, "left", m.viper.GetString("workspace.browsing_contexts.placement"))
	362	+	assert.Equal(t, "tabbed", settings.GetString("workspace.browsing_contexts.behavior"))
	363	+	assert.Equal(t, "left", settings.GetString("workspace.browsing_contexts.placement"))
361	364		}
362	365
363	366		func TestReload_ReappliesLegacyPopupsCompatibility(t *testing.T) {

 	m.viper.Set("workspace.popups.oauth_auto_close", false)
 	// Transform should map popups → browsing_contexts.
-	m.transformLegacyConfig()
+	settings, err := m.transformedSettings()
+	require.NoError(t, err)
 	// popups key should be gone from viper after transform.
-	assert.False(t, m.viper.IsSet("workspace.popups.behavior"),
+	assert.False(t, settings.IsSet("workspace.popups.behavior"),
 		"workspace.popups.behavior should not exist after transform")
 	// browsing_contexts should now hold the values.
-	assert.Equal(t, "stacked", m.viper.GetString("workspace.browsing_contexts.behavior"))
+	assert.Equal(t, "stacked", settings.GetString("workspace.browsing_contexts.behavior"))
-	assert.Equal(t, "left", m.viper.GetString("workspace.browsing_contexts.placement"))
+	assert.Equal(t, "left", settings.GetString("workspace.browsing_contexts.placement"))
-	assert.False(t, m.viper.GetBool("workspace.browsing_contexts.oauth_auto_close"))
+	assert.False(t, settings.GetBool("workspace.browsing_contexts.oauth_auto_close"))
 	// Unmarshal should succeed.
 	cfg := &Config{}
-	err := m.viper.Unmarshal(cfg)
+	err = settings.Unmarshal(cfg)
 	require.NoError(t, err)
 	// BrowsingContexts should have the legacy values.
 	m.setDefaults()
 	require.NoError(t, m.viper.ReadInConfig())
-	m.transformLegacyConfig()
+	settings, err := m.transformedSettings()
+	require.NoError(t, err)
-	assert.Equal(t, "stacked", m.viper.GetString("workspace.browsing_contexts.behavior"))
+	assert.Equal(t, "stacked", settings.GetString("workspace.browsing_contexts.behavior"))
-	assert.Equal(t, "left", m.viper.GetString("workspace.browsing_contexts.placement"))
+	assert.Equal(t, "left", settings.GetString("workspace.browsing_contexts.placement"))
-	assert.False(t, m.viper.GetBool("workspace.browsing_contexts.oauth_auto_close"))
+	assert.False(t, settings.GetBool("workspace.browsing_contexts.oauth_auto_close"))
 	cfg := &Config{}
-	require.NoError(t, m.viper.Unmarshal(cfg))
+	require.NoError(t, settings.Unmarshal(cfg))
 	normalizeConfig(cfg)
 	assert.Equal(t, PopupBehaviorStacked, cfg.Workspace.BrowsingContexts.Behavior)
 	m.setDefaults()
 	require.NoError(t, m.viper.ReadInConfig())
-	m.transformLegacyConfig()
+	settings, err := m.transformedSettings()
+	require.NoError(t, err)
-	assert.Equal(t, "tabbed", m.viper.GetString("workspace.browsing_contexts.behavior"))
+	assert.Equal(t, "tabbed", settings.GetString("workspace.browsing_contexts.behavior"))
-	assert.Equal(t, "left", m.viper.GetString("workspace.browsing_contexts.placement"))
+	assert.Equal(t, "left", settings.GetString("workspace.browsing_contexts.placement"))
 }
 func TestReload_ReappliesLegacyPopupsCompatibility(t *testing.T) {

M internal/infrastructure/config/loader.go +22 -10


@@ -145,15 +145,19 @@ func (m *Manager) readConfigFile() error {
145	145		}
146	146		}
147	147
148		-	// Transform legacy action bindings and engine config before unmarshaling.
149		-	m.transformLegacyConfig()
150		-
151	148		return nil
152	149		}
153	150
154		-	// transformLegacyConfig converts old-format action bindings and engine config to the new format.
155		-	// This is called after reading config but before unmarshaling.
156		-	func (m *Manager) transformLegacyConfig() {
	151	+	// transformedSettings returns a throwaway Viper holding the effective settings
	152	+	// (config file, defaults and environment as resolved by the long-lived Viper)
	153	+	// after the legacy transforms: old-format action bindings, legacy engine config
	154	+	// and legacy popups.
	155	+	//
	156	+	// The transformed values are deliberately NOT written back to m.viper with
	157	+	// Set(). Set() registers top-priority overrides that ReadInConfig never
	158	+	// clears, so a later reload would keep returning the values of the first read
	159	+	// instead of the file contents.
	160	+	func (m Manager) transformedSettings() (viper.Viper, error) {
157	161		legacyPopupsInConfig := m.viper.InConfig("workspace.popups")
158	162
159	163		rawConfig := m.viper.AllSettings()

@@ -175,10 +179,11 @@ func (m *Manager) transformLegacyConfig() {
175	179		transformer.TransformLegacyPopupsToBrowsingContexts(rawConfig)
176	180		}
177	181
178		-	// Apply transformed config back to viper
179		-	for key, value := range rawConfig {
180		-	m.viper.Set(key, value)
	182	+	settings := viper.New()
	183	+	if err := settings.MergeConfigMap(rawConfig); err != nil {
	184	+	return nil, fmt.Errorf("failed to merge transformed config: %w", err)
181	185		}
	186	+	return settings, nil
182	187		}
183	188
184	189		func (m *Manager) configureAutomaticEnv() {

@@ -348,9 +353,16 @@ func (m *Manager) hasEngineSectionInConfig() bool {
348	353		return m.viper.InConfig("engine.type")
349	354		}
350	355
	356	+	// unmarshalConfig decodes the current file, defaults and environment, with the
	357	+	// legacy transforms applied, into a fresh Config. It is used by both the initial
	358	+	// load and every reload, so neither depends on state left by a previous read.
351	359		func (m Manager) unmarshalConfig() (Config, error) {
	360	+	settings, err := m.transformedSettings()
	361	+	if err != nil {
	362	+	return nil, err
	363	+	}
352	364		config := &Config{}
353		-	if err := m.viper.Unmarshal(config); err != nil {
	365	+	if err := settings.Unmarshal(config); err != nil {
354	366		configFile := m.viper.ConfigFileUsed()
355	367		return nil, fmt.Errorf(
356	368		"failed to parse config file at %s: %w\nCheck for syntax errors, invalid values, or type mismatches",

 		}
 	}
-	// Transform legacy action bindings and engine config before unmarshaling.
-	m.transformLegacyConfig()
 	return nil
 }
-// transformLegacyConfig converts old-format action bindings and engine config to the new format.
+// transformedSettings returns a throwaway Viper holding the effective settings
-// This is called after reading config but before unmarshaling.
+// (config file, defaults and environment as resolved by the long-lived Viper)
-func (m *Manager) transformLegacyConfig() {
+// after the legacy transforms: old-format action bindings, legacy engine config
+// and legacy popups.
+//
+// The transformed values are deliberately NOT written back to m.viper with
+// Set(). Set() registers top-priority overrides that ReadInConfig never
+// clears, so a later reload would keep returning the values of the first read
+// instead of the file contents.
+func (m *Manager) transformedSettings() (*viper.Viper, error) {
 	legacyPopupsInConfig := m.viper.InConfig("workspace.popups")
 	rawConfig := m.viper.AllSettings()
 		transformer.TransformLegacyPopupsToBrowsingContexts(rawConfig)
 	}
-	// Apply transformed config back to viper
+	settings := viper.New()
-	for key, value := range rawConfig {
+	if err := settings.MergeConfigMap(rawConfig); err != nil {
-		m.viper.Set(key, value)
+		return nil, fmt.Errorf("failed to merge transformed config: %w", err)
 	}
+	return settings, nil
 }
 func (m *Manager) configureAutomaticEnv() {
 	return m.viper.InConfig("engine.type")
 }
+// unmarshalConfig decodes the current file, defaults and environment, with the
+// legacy transforms applied, into a fresh Config. It is used by both the initial
+// load and every reload, so neither depends on state left by a previous read.
 func (m *Manager) unmarshalConfig() (*Config, error) {
+	settings, err := m.transformedSettings()
+	if err != nil {
+		return nil, err
+	}
 	config := &Config{}
-	if err := m.viper.Unmarshal(config); err != nil {
+	if err := settings.Unmarshal(config); err != nil {
 		configFile := m.viper.ConfigFileUsed()
 		return nil, fmt.Errorf(
 			"failed to parse config file at %s: %w\nCheck for syntax errors, invalid values, or type mismatches",

M internal/infrastructure/config/loader_legacy_test.go +18 -14


@@ -149,11 +149,12 @@ func TestTransformLegacyConfig_DefaultAdaptiveDoesNotBlockLegacyCEFMigration(t *
149	149		m.setDefaults()
150	150		require.NoError(t, m.viper.ReadInConfig())
151	151
152		-	m.transformLegacyConfig()
	152	+	settings, err := m.transformedSettings()
	153	+	require.NoError(t, err)
153	154
154		-	require.True(t, m.viper.GetBool("engine.cef.adaptive_windowless_frame_rate"))
155		-	require.Equal(t, 0, m.viper.GetInt("engine.cef.windowless_frame_rate"))
156		-	require.Equal(t, defaultCEFWindowlessFrameRateMax, m.viper.GetInt("engine.cef.windowless_frame_rate_max"))
	155	+	require.True(t, settings.GetBool("engine.cef.adaptive_windowless_frame_rate"))
	156	+	require.Equal(t, 0, settings.GetInt("engine.cef.windowless_frame_rate"))
	157	+	require.Equal(t, defaultCEFWindowlessFrameRateMax, settings.GetInt("engine.cef.windowless_frame_rate_max"))
157	158		}
158	159
159	160		func TestTransformLegacyConfig_ExplicitAdaptiveEnvOverrideBlocksLegacyCEFMigration(t *testing.T) {

@@ -169,10 +170,11 @@ func TestTransformLegacyConfig_ExplicitAdaptiveEnvOverrideBlocksLegacyCEFMigrati
169	170		m.setDefaults()
170	171		require.NoError(t, m.viper.ReadInConfig())
171	172
172		-	m.transformLegacyConfig()
	173	+	settings, err := m.transformedSettings()
	174	+	require.NoError(t, err)
173	175
174		-	require.False(t, m.viper.GetBool("engine.cef.adaptive_windowless_frame_rate"))
175		-	require.Equal(t, 60, m.viper.GetInt("engine.cef.windowless_frame_rate"))
	176	+	require.False(t, settings.GetBool("engine.cef.adaptive_windowless_frame_rate"))
	177	+	require.Equal(t, 60, settings.GetInt("engine.cef.windowless_frame_rate"))
176	178		}
177	179
178	180		func TestTransformLegacyConfig_UnconfiguredManagerIgnoresAmbientAdaptiveEnv(t *testing.T) {

@@ -187,10 +189,11 @@ func TestTransformLegacyConfig_UnconfiguredManagerIgnoresAmbientAdaptiveEnv(t *t
187	189		m.setDefaults()
188	190		require.NoError(t, m.viper.ReadInConfig())
189	191
190		-	m.transformLegacyConfig()
	192	+	settings, err := m.transformedSettings()
	193	+	require.NoError(t, err)
191	194
192		-	require.True(t, m.viper.GetBool("engine.cef.adaptive_windowless_frame_rate"))
193		-	require.Equal(t, 0, m.viper.GetInt("engine.cef.windowless_frame_rate"))
	195	+	require.True(t, settings.GetBool("engine.cef.adaptive_windowless_frame_rate"))
	196	+	require.Equal(t, 0, settings.GetInt("engine.cef.windowless_frame_rate"))
194	197		}
195	198
196	199		func TestTransformLegacyConfig_ExplicitAdaptiveEnvTrueMigratesLegacyCEFDefault(t *testing.T) {

@@ -206,9 +209,10 @@ func TestTransformLegacyConfig_ExplicitAdaptiveEnvTrueMigratesLegacyCEFDefault(t
206	209		m.setDefaults()
207	210		require.NoError(t, m.viper.ReadInConfig())
208	211
209		-	m.transformLegacyConfig()
	212	+	settings, err := m.transformedSettings()
	213	+	require.NoError(t, err)
210	214
211		-	require.True(t, m.viper.GetBool("engine.cef.adaptive_windowless_frame_rate"))
212		-	require.Equal(t, 0, m.viper.GetInt("engine.cef.windowless_frame_rate"))
213		-	require.Equal(t, defaultCEFWindowlessFrameRateMax, m.viper.GetInt("engine.cef.windowless_frame_rate_max"))
	215	+	require.True(t, settings.GetBool("engine.cef.adaptive_windowless_frame_rate"))
	216	+	require.Equal(t, 0, settings.GetInt("engine.cef.windowless_frame_rate"))
	217	+	require.Equal(t, defaultCEFWindowlessFrameRateMax, settings.GetInt("engine.cef.windowless_frame_rate_max"))
214	218		}


@@ -149,11 +149,12 @@ func TestTransformLegacyConfig_DefaultAdaptiveDoesNotBlockLegacyCEFMigration(t *
149		m.setDefaults()	149		m.setDefaults()
150		require.NoError(t, m.viper.ReadInConfig())	150		require.NoError(t, m.viper.ReadInConfig())
151			151
152	-	m.transformLegacyConfig()	152	+	settings, err := m.transformedSettings()
			153	+	require.NoError(t, err)
153			154
154	-	require.True(t, m.viper.GetBool("engine.cef.adaptive_windowless_frame_rate"))	155	+	require.True(t, settings.GetBool("engine.cef.adaptive_windowless_frame_rate"))
155	-	require.Equal(t, 0, m.viper.GetInt("engine.cef.windowless_frame_rate"))	156	+	require.Equal(t, 0, settings.GetInt("engine.cef.windowless_frame_rate"))
156	-	require.Equal(t, defaultCEFWindowlessFrameRateMax, m.viper.GetInt("engine.cef.windowless_frame_rate_max"))	157	+	require.Equal(t, defaultCEFWindowlessFrameRateMax, settings.GetInt("engine.cef.windowless_frame_rate_max"))
157		}	158		}
158			159
159		func TestTransformLegacyConfig_ExplicitAdaptiveEnvOverrideBlocksLegacyCEFMigration(t *testing.T) {	160		func TestTransformLegacyConfig_ExplicitAdaptiveEnvOverrideBlocksLegacyCEFMigration(t *testing.T) {

@@ -169,10 +170,11 @@ func TestTransformLegacyConfig_ExplicitAdaptiveEnvOverrideBlocksLegacyCEFMigrati
169		m.setDefaults()	170		m.setDefaults()
170		require.NoError(t, m.viper.ReadInConfig())	171		require.NoError(t, m.viper.ReadInConfig())
171			172
172	-	m.transformLegacyConfig()	173	+	settings, err := m.transformedSettings()
			174	+	require.NoError(t, err)
173			175
174	-	require.False(t, m.viper.GetBool("engine.cef.adaptive_windowless_frame_rate"))	176	+	require.False(t, settings.GetBool("engine.cef.adaptive_windowless_frame_rate"))
175	-	require.Equal(t, 60, m.viper.GetInt("engine.cef.windowless_frame_rate"))	177	+	require.Equal(t, 60, settings.GetInt("engine.cef.windowless_frame_rate"))
176		}	178		}
177			179
178		func TestTransformLegacyConfig_UnconfiguredManagerIgnoresAmbientAdaptiveEnv(t *testing.T) {	180		func TestTransformLegacyConfig_UnconfiguredManagerIgnoresAmbientAdaptiveEnv(t *testing.T) {

@@ -187,10 +189,11 @@ func TestTransformLegacyConfig_UnconfiguredManagerIgnoresAmbientAdaptiveEnv(t *t
187		m.setDefaults()	189		m.setDefaults()
188		require.NoError(t, m.viper.ReadInConfig())	190		require.NoError(t, m.viper.ReadInConfig())
189			191
190	-	m.transformLegacyConfig()	192	+	settings, err := m.transformedSettings()
			193	+	require.NoError(t, err)
191			194
192	-	require.True(t, m.viper.GetBool("engine.cef.adaptive_windowless_frame_rate"))	195	+	require.True(t, settings.GetBool("engine.cef.adaptive_windowless_frame_rate"))
193	-	require.Equal(t, 0, m.viper.GetInt("engine.cef.windowless_frame_rate"))	196	+	require.Equal(t, 0, settings.GetInt("engine.cef.windowless_frame_rate"))
194		}	197		}
195			198
196		func TestTransformLegacyConfig_ExplicitAdaptiveEnvTrueMigratesLegacyCEFDefault(t *testing.T) {	199		func TestTransformLegacyConfig_ExplicitAdaptiveEnvTrueMigratesLegacyCEFDefault(t *testing.T) {

@@ -206,9 +209,10 @@ func TestTransformLegacyConfig_ExplicitAdaptiveEnvTrueMigratesLegacyCEFDefault(t
206		m.setDefaults()	209		m.setDefaults()
207		require.NoError(t, m.viper.ReadInConfig())	210		require.NoError(t, m.viper.ReadInConfig())
208			211
209	-	m.transformLegacyConfig()	212	+	settings, err := m.transformedSettings()
			213	+	require.NoError(t, err)
210			214
211	-	require.True(t, m.viper.GetBool("engine.cef.adaptive_windowless_frame_rate"))	215	+	require.True(t, settings.GetBool("engine.cef.adaptive_windowless_frame_rate"))
212	-	require.Equal(t, 0, m.viper.GetInt("engine.cef.windowless_frame_rate"))	216	+	require.Equal(t, 0, settings.GetInt("engine.cef.windowless_frame_rate"))
213	-	require.Equal(t, defaultCEFWindowlessFrameRateMax, m.viper.GetInt("engine.cef.windowless_frame_rate_max"))	217	+	require.Equal(t, defaultCEFWindowlessFrameRateMax, settings.GetInt("engine.cef.windowless_frame_rate_max"))
214		}	218		}

A internal/infrastructure/config/loader_reload_test.go +96 -0

@@ -0,0 +1,96 @@
	1	+	package config
	2	+
	3	+	import (
	4	+	"os"
	5	+	"strings"
	6	+	"testing"
	7	+
	8	+	"github.com/stretchr/testify/require"
	9	+	)
	10	+
	11	+	// setLegendDelayInFile rewrites the mode_legend_delay_ms line of the manager's
	12	+	// config file, simulating an external edit.
	13	+	func setLegendDelayInFile(t testing.T, mgr Manager, delayMs string) {
	14	+	t.Helper()
	15	+
	16	+	path := mgr.GetConfigFile()
	17	+	require.NotEmpty(t, path)
	18	+	data, err := os.ReadFile(path)
	19	+	require.NoError(t, err)
	20	+
	21	+	lines := strings.Split(string(data), "\n")
	22	+	replaced := false
	23	+	for i, line := range lines {
	24	+	if strings.HasPrefix(strings.TrimSpace(line), "mode_legend_delay_ms") {
	25	+	lines[i] = "mode_legend_delay_ms = " + delayMs
	26	+	replaced = true
	27	+	}
	28	+	}
	29	+	require.True(t, replaced, "mode_legend_delay_ms not found in written config")
	30	+	require.NoError(t, os.WriteFile(path, []byte(strings.Join(lines, "\n")), 0o600))
	31	+	}
	32	+
	33	+	func TestManagerReload_KeepsSavedValue(t *testing.T) {
	34	+	mgr := newTestOmniboxPreferencesGatewayManager(t, t.TempDir())
	35	+	require.Equal(t, 500, mgr.Get().Workspace.Styling.ModeLegendDelayMs)
	36	+
	37	+	require.NoError(t, mgr.Update(func(cfg *Config) { cfg.Workspace.Styling.ModeLegendDelayMs = 700 }))
	38	+	require.Equal(t, 700, mgr.Get().Workspace.Styling.ModeLegendDelayMs)
	39	+
	40	+	mgr.mu.Lock()
	41	+	err := mgr.reload()
	42	+	mgr.mu.Unlock()
	43	+	require.NoError(t, err)
	44	+	require.Equal(t, 700, mgr.Get().Workspace.Styling.ModeLegendDelayMs)
	45	+	}
	46	+
	47	+	func TestManagerReload_PicksUpExternalEditAfterSave(t *testing.T) {
	48	+	mgr := newTestOmniboxPreferencesGatewayManager(t, t.TempDir())
	49	+	require.NoError(t, mgr.Update(func(cfg *Config) { cfg.Workspace.Styling.ModeLegendDelayMs = 700 }))
	50	+
	51	+	setLegendDelayInFile(t, mgr, "900")
	52	+
	53	+	mgr.mu.Lock()
	54	+	err := mgr.reload()
	55	+	mgr.mu.Unlock()
	56	+	require.NoError(t, err)
	57	+	require.Equal(t, 900, mgr.Get().Workspace.Styling.ModeLegendDelayMs)
	58	+	}
	59	+
	60	+	func TestManagerReload_PicksUpExternalEditWithoutPriorSave(t *testing.T) {
	61	+	mgr := newTestOmniboxPreferencesGatewayManager(t, t.TempDir())
	62	+	require.Equal(t, 500, mgr.Get().Workspace.Styling.ModeLegendDelayMs)
	63	+
	64	+	setLegendDelayInFile(t, mgr, "900")
	65	+
	66	+	mgr.mu.Lock()
	67	+	err := mgr.reload()
	68	+	mgr.mu.Unlock()
	69	+	require.NoError(t, err)
	70	+	require.Equal(t, 900, mgr.Get().Workspace.Styling.ModeLegendDelayMs)
	71	+
	72	+	setLegendDelayInFile(t, mgr, "300")
	73	+	mgr.mu.Lock()
	74	+	err = mgr.reload()
	75	+	mgr.mu.Unlock()
	76	+	require.NoError(t, err)
	77	+	require.Equal(t, 300, mgr.Get().Workspace.Styling.ModeLegendDelayMs)
	78	+	}
	79	+
	80	+	func TestManagerUpdate_ConsecutiveUpdatesEachPersist(t *testing.T) {
	81	+	configHome := t.TempDir()
	82	+	mgr := newTestOmniboxPreferencesGatewayManager(t, configHome)
	83	+
	84	+	require.NoError(t, mgr.Update(func(cfg *Config) { cfg.Workspace.Styling.ModeLegendCollapsed = true }))
	85	+	require.NoError(t, mgr.Update(func(cfg *Config) { cfg.Workspace.Styling.ModeLegendDelayMs = 700 }))
	86	+	require.NoError(t, mgr.Update(func(cfg *Config) { cfg.Workspace.Styling.ModeLegendDelayMs = 800 }))
	87	+	require.NoError(t, mgr.Update(func(cfg *Config) { cfg.Workspace.Styling.ModeLegendCollapsed = false }))
	88	+
	89	+	for name, got := range map[string]*Config{
	90	+	"in-memory": mgr.Get(),
	91	+	"on-disk": newTestOmniboxPreferencesGatewayManager(t, configHome).Get(),
	92	+	} {
	93	+	require.False(t, got.Workspace.Styling.ModeLegendCollapsed, name)
	94	+	require.Equal(t, 800, got.Workspace.Styling.ModeLegendDelayMs, name)
	95	+	}
	96	+	}

@@ -0,0 +1,96 @@
	1	+	package config
	2	+
	3	+	import (
	4	+	"os"
	5	+	"strings"
	6	+	"testing"
	7	+
	8	+	"github.com/stretchr/testify/require"
	9	+	)
	10	+
	11	+	// setLegendDelayInFile rewrites the mode_legend_delay_ms line of the manager's
	12	+	// config file, simulating an external edit.
	13	+	func setLegendDelayInFile(t testing.T, mgr Manager, delayMs string) {
	14	+	t.Helper()
	15	+
	16	+	path := mgr.GetConfigFile()
	17	+	require.NotEmpty(t, path)
	18	+	data, err := os.ReadFile(path)
	19	+	require.NoError(t, err)
	20	+
	21	+	lines := strings.Split(string(data), "\n")
	22	+	replaced := false
	23	+	for i, line := range lines {
	24	+	if strings.HasPrefix(strings.TrimSpace(line), "mode_legend_delay_ms") {
	25	+	lines[i] = "mode_legend_delay_ms = " + delayMs
	26	+	replaced = true
	27	+	}
	28	+	}
	29	+	require.True(t, replaced, "mode_legend_delay_ms not found in written config")
	30	+	require.NoError(t, os.WriteFile(path, []byte(strings.Join(lines, "\n")), 0o600))
	31	+	}
	32	+
	33	+	func TestManagerReload_KeepsSavedValue(t *testing.T) {
	34	+	mgr := newTestOmniboxPreferencesGatewayManager(t, t.TempDir())
	35	+	require.Equal(t, 500, mgr.Get().Workspace.Styling.ModeLegendDelayMs)
	36	+
	37	+	require.NoError(t, mgr.Update(func(cfg *Config) { cfg.Workspace.Styling.ModeLegendDelayMs = 700 }))
	38	+	require.Equal(t, 700, mgr.Get().Workspace.Styling.ModeLegendDelayMs)
	39	+
	40	+	mgr.mu.Lock()
	41	+	err := mgr.reload()
	42	+	mgr.mu.Unlock()
	43	+	require.NoError(t, err)
	44	+	require.Equal(t, 700, mgr.Get().Workspace.Styling.ModeLegendDelayMs)
	45	+	}
	46	+
	47	+	func TestManagerReload_PicksUpExternalEditAfterSave(t *testing.T) {
	48	+	mgr := newTestOmniboxPreferencesGatewayManager(t, t.TempDir())
	49	+	require.NoError(t, mgr.Update(func(cfg *Config) { cfg.Workspace.Styling.ModeLegendDelayMs = 700 }))
	50	+
	51	+	setLegendDelayInFile(t, mgr, "900")
	52	+
	53	+	mgr.mu.Lock()
	54	+	err := mgr.reload()
	55	+	mgr.mu.Unlock()
	56	+	require.NoError(t, err)
	57	+	require.Equal(t, 900, mgr.Get().Workspace.Styling.ModeLegendDelayMs)
	58	+	}
	59	+
	60	+	func TestManagerReload_PicksUpExternalEditWithoutPriorSave(t *testing.T) {
	61	+	mgr := newTestOmniboxPreferencesGatewayManager(t, t.TempDir())
	62	+	require.Equal(t, 500, mgr.Get().Workspace.Styling.ModeLegendDelayMs)
	63	+
	64	+	setLegendDelayInFile(t, mgr, "900")
	65	+
	66	+	mgr.mu.Lock()
	67	+	err := mgr.reload()
	68	+	mgr.mu.Unlock()
	69	+	require.NoError(t, err)
	70	+	require.Equal(t, 900, mgr.Get().Workspace.Styling.ModeLegendDelayMs)
	71	+
	72	+	setLegendDelayInFile(t, mgr, "300")
	73	+	mgr.mu.Lock()
	74	+	err = mgr.reload()
	75	+	mgr.mu.Unlock()
	76	+	require.NoError(t, err)
	77	+	require.Equal(t, 300, mgr.Get().Workspace.Styling.ModeLegendDelayMs)
	78	+	}
	79	+
	80	+	func TestManagerUpdate_ConsecutiveUpdatesEachPersist(t *testing.T) {
	81	+	configHome := t.TempDir()
	82	+	mgr := newTestOmniboxPreferencesGatewayManager(t, configHome)
	83	+
	84	+	require.NoError(t, mgr.Update(func(cfg *Config) { cfg.Workspace.Styling.ModeLegendCollapsed = true }))
	85	+	require.NoError(t, mgr.Update(func(cfg *Config) { cfg.Workspace.Styling.ModeLegendDelayMs = 700 }))
	86	+	require.NoError(t, mgr.Update(func(cfg *Config) { cfg.Workspace.Styling.ModeLegendDelayMs = 800 }))
	87	+	require.NoError(t, mgr.Update(func(cfg *Config) { cfg.Workspace.Styling.ModeLegendCollapsed = false }))
	88	+
	89	+	for name, got := range map[string]*Config{
	90	+	"in-memory": mgr.Get(),
	91	+	"on-disk": newTestOmniboxPreferencesGatewayManager(t, configHome).Get(),
	92	+	} {
	93	+	require.False(t, got.Workspace.Styling.ModeLegendCollapsed, name)
	94	+	require.Equal(t, 800, got.Workspace.Styling.ModeLegendDelayMs, name)
	95	+	}
	96	+	}

M internal/infrastructure/config/watcher.go +4 -12


@@ -87,20 +87,12 @@ func (m *Manager) reload() error {
87	87		return err
88	88		}
89	89
90		-	m.transformLegacyConfig()
91		-
92		-	config := &Config{}
93		-	if err := m.viper.Unmarshal(config); err != nil {
	90	+	config, err := m.unmarshalConfig()
	91	+	if err != nil {
94	92		return err
95	93		}
96		-
97		-	// Set database path if not specified
98		-	if config.Database.Path == "" {
99		-	dbPath, err := GetDatabaseFile()
100		-	if err != nil {
101		-	return fmt.Errorf("failed to get database path: %w", err)
102		-	}
103		-	config.Database.Path = dbPath
	94	+	if err := ensureDatabasePath(config); err != nil {
	95	+	return err
104	96		}
105	97
106	98		normalizeConfig(config)


@@ -87,20 +87,12 @@ func (m *Manager) reload() error {
87		return err	87		return err
88		}	88		}
89			89
90	-	m.transformLegacyConfig()	90	+	config, err := m.unmarshalConfig()
91	-		91	+	if err != nil {
92	-	config := &Config{}
93	-	if err := m.viper.Unmarshal(config); err != nil {
94		return err	92		return err
95		}	93		}
96	-		94	+	if err := ensureDatabasePath(config); err != nil {
97	-	// Set database path if not specified	95	+	return err
98	-	if config.Database.Path == "" {
99	-	dbPath, err := GetDatabaseFile()
100	-	if err != nil {
101	-	return fmt.Errorf("failed to get database path: %w", err)
102	-	}
103	-	config.Database.Path = dbPath
104		}	96		}
105			97
106		normalizeConfig(config)	98		normalizeConfig(config)