diff --git a/internal/config/store.go b/internal/config/store.go index 259c495e81437dca17ee991eb91ad0a988b99785..9d9cef7d1b368251b03a495d5f0a3c8e44b17b5a 100644 --- a/internal/config/store.go +++ b/internal/config/store.go @@ -172,9 +172,6 @@ func (s *ConfigStore) atomicWrite(scope Scope, fn func(current []byte) ([]byte, return err } - if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { - return fmt.Errorf("create config directory: %w", err) - } return atomicWriteFile(path, newData, 0o600) } @@ -221,11 +218,20 @@ func (s *ConfigStore) SetConfigField(scope Scope, key string, value any) error { // The write is protected by an in-process mutex and a cross-process flock // to prevent races between concurrent writers in different processes. func (s *ConfigStore) SetConfigFields(scope Scope, kv map[string]any) error { + // Sort keys for deterministic output regardless of map iteration + // order. This also ensures consistent results when callers pass + // overlapping JSONPath keys (e.g. "a" and "a.b"). + keys := make([]string, 0, len(kv)) + for k := range kv { + keys = append(keys, k) + } + slices.Sort(keys) + err := s.atomicWrite(scope, func(data []byte) ([]byte, error) { v := string(data) - for key, value := range kv { + for _, key := range keys { var sErr error - v, sErr = sjson.Set(v, key, value) + v, sErr = sjson.Set(v, key, kv[key]) if sErr != nil { return nil, fmt.Errorf("failed to set config field %s: %w", key, sErr) } diff --git a/internal/config/store_test.go b/internal/config/store_test.go index 601d3a8487bc339345b347e7d5276d57bf67666d..5094f668896723d7eddf3907e400f1250b8f46ec 100644 --- a/internal/config/store_test.go +++ b/internal/config/store_test.go @@ -732,9 +732,11 @@ func TestRefreshOAuthToken_UsesDiskTokenWhenDifferent(t *testing.T) { require.Equal(t, "refresh-abc", updatedConfig.OAuthToken.RefreshToken) } -// TestConfigStore_SetConfigFields_concurrent verifies that concurrent writes do -// not lose data when protected by the in-process mutex and cross-process flock. -func TestConfigStore_SetConfigFields_concurrent(t *testing.T) { +// TestConfigStore_SetConfigFields_concurrentInProcess verifies that +// concurrent in-process writes do not lose data when serialized by the +// s.mu mutex. This does not exercise the cross-process flock; testing +// that would require spawning a separate OS process. +func TestConfigStore_SetConfigFields_concurrentInProcess(t *testing.T) { t.Parallel() dir := t.TempDir()