5b5f7b6 · crush · git.iko.soy

git.iko.soy/crush

fix(config): sort SetConfigFields keys, remove redundant MkdirAll, rename test

- Sort keys in SetConfigFields before applying sjson.Set for deterministic output regardless of map iteration order. - Remove duplicate os.MkdirAll from atomicWrite (already done in lockConfig). - Rename concurrent test to _concurrentInProcess to accurately reflect that it only exercises the in-process mutex, not the cross-process flock.

commit5b5f7b687d324b8835e90d064e6e16970ebc92ea authorKieran Klukas <kieran@dunkirk.sh> dateFri 29 May 2026, 14:36 (5 months ago) parentc83418d

2 files changed+16−8

compare with main

unifiedsplit

internal/config/store.go+11−5 internal/config/store_test.go+5−3

internal/config/store.go+11−5

@@ -172,9 +172,6 @@ func (s *ConfigStore) atomicWrite(scope Scope, fn func(current []byte) ([]byte,

172172 return err

173173 }

174174

175− if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {

176− return fmt.Errorf("create config directory: %w", err)

177− }

178175 return atomicWriteFile(path, newData, 0o600)

179176}

180177

@@ -221,11 +218,20 @@ func (s *ConfigStore) SetConfigField(scope Scope, key string, value any) error {

221218// The write is protected by an in-process mutex and a cross-process flock

222219// to prevent races between concurrent writers in different processes.

223220func (s *ConfigStore) SetConfigFields(scope Scope, kv map[string]any) error {

221+ // Sort keys for deterministic output regardless of map iteration

222+ // order. This also ensures consistent results when callers pass

223+ // overlapping JSONPath keys (e.g. "a" and "a.b").

224+ keys := make([]string, 0, len(kv))

225+ for k := range kv {

226+ keys = append(keys, k)

227+ }

228+ slices.Sort(keys)

229+

224230 err := s.atomicWrite(scope, func(data []byte) ([]byte, error) {

225231 v := string(data)

226− for key, value := range kv {

232+ for _, key := range keys {

227233 var sErr error

228− v, sErr = sjson.Set(v, key, value)

234+ v, sErr = sjson.Set(v, key, kv[key])

229235 if sErr != nil {

230236 return nil, fmt.Errorf("failed to set config field %s: %w", key, sErr)

231237 }

internal/config/store_test.go+5−3

@@ -732,9 +732,11 @@ func TestRefreshOAuthToken_UsesDiskTokenWhenDifferent(t *testing.T) {

732732 require.Equal(t, "refresh-abc", updatedConfig.OAuthToken.RefreshToken)

733733}

734734

735−// TestConfigStore_SetConfigFields_concurrent verifies that concurrent writes do

736−// not lose data when protected by the in-process mutex and cross-process flock.

737−func TestConfigStore_SetConfigFields_concurrent(t *testing.T) {

735+// TestConfigStore_SetConfigFields_concurrentInProcess verifies that

736+// concurrent in-process writes do not lose data when serialized by the

737+// s.mu mutex. This does not exercise the cross-process flock; testing

738+// that would require spawning a separate OS process.

739+func TestConfigStore_SetConfigFields_concurrentInProcess(t *testing.T) {

738740 t.Parallel()

739741

740742 dir := t.TempDir()