Config definition maps use inconsistent lock discipline

[michaelw]

Summary

Config definition maps are protected inconsistently today. At least in the roles path, some readers use RoleConfig.mu while writers replace c.Roles.Definitions under Config.mu, and some provider-definition reads are currently unlocked.

That means the code does not have one consistent lock domain for Definitions access.

Minimal repro

I added a focused race-oriented test on the current branch state and ran:

go test -race ./internal/config -run 'TestRoleDefinitionsLockDisciplineRace'

The repro runs two concurrent loops:

• read roles through RoleConfig.ListRoles (uses rc.mu)
• replace c.Roles.Definitions under c.mu

Representative race-detector output:

WARNING: DATA RACE
Read at 0x00c0008fe298 by goroutine 25:
  github.com/thand-io/agent/internal/config.(*RoleConfig).ListRoles()
      /private/tmp/thand-agent-issue3-repros/internal/config/roles.go:186 +0x78
...
Previous write at 0x00c0008fe298 by goroutine 24:
  github.com/thand-io/agent/internal/config.TestRoleDefinitionsLockDisciplineRace.func1()
      /private/tmp/thand-agent-issue3-repros/internal/config/sync_lock_discipline_race_test.go:30 +0x1bc
...
--- FAIL: TestRoleDefinitionsLockDisciplineRace (0.10s)
    testing.go:1712: race detected during execution of test

Analysis

This issue is broader than the sync bug itself. The main problem is inconsistent lock discipline around config Definitions maps:

• roles readers can use rc.mu
• sync/reload writers use c.mu
• some provider-definition reads are not locked at all

Even with stale-snapshot retries in place, this remains a separate correctness and maintainability problem.

Scope

This issue is specifically about unifying lock discipline for config definition maps and auditing the remaining unlocked or differently locked readers/writers.

Please keep separate from:

• the bug where sync results were not applied to live config
• stale-snapshot lost-update handling in the sync path

[michaelw]

Exact temporary race-style repro test source used for the lock-discipline evidence run:

package config

import (
	"context"
	"fmt"
	"sync"
	"testing"

	"github.com/stretchr/testify/require"
	"github.com/thand-io/agent/internal/models"
)

func TestRoleDefinitionsLockDisciplineRace(t *testing.T) {
	config := newSyncTestConfig(t,
		map[string]models.Role{
			"seed": {Name: "seed", Enabled: true},
		},
		nil, nil, "",
	)

	ctx := context.Background()

	var wg sync.WaitGroup
	start := make(chan struct{})

	wg.Go(func() {
		<-start
		for i := range 1000 {
			config.mu.Lock()
			config.Roles.Definitions = map[string]models.Role{
				fmt.Sprintf("role-%d", i): {Name: fmt.Sprintf("role-%d", i), Enabled: true},
			}
			config.mu.Unlock()
		}
	})

	wg.Go(func() {
		<-start
		for range 1000 {
			_, err := config.Roles.ListRoles(ctx, nil)
			require.NoError(t, err)
		}
	})

	close(start)
	wg.Wait()
}