Skip to content

Commit 15ea9b4

Browse files
authored
Merge pull request #1729 from dgageot/cleanup-config
Cleanup config code
2 parents f8d707e + a6a5fc9 commit 15ea9b4

File tree

15 files changed

+127
-141
lines changed

15 files changed

+127
-141
lines changed

.dockerignore

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,5 @@
77
!./**/*.css
88
!./**/*.go
99
!./**/*.txt
10-
!/pkg/config/default-agent.yaml
11-
!/pkg/config/coder-agent.yaml
10+
!/pkg/config/builtin-agents/*.yaml
1211
!/pkg/tui/styles/themes/*.yaml

cmd/root/run.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"github.com/docker/cagent/pkg/teamloader"
2525
"github.com/docker/cagent/pkg/telemetry"
2626
"github.com/docker/cagent/pkg/tui/styles"
27+
"github.com/docker/cagent/pkg/userconfig"
2728
)
2829

2930
type runExecFlags struct {
@@ -160,7 +161,7 @@ func (f *runExecFlags) runOrExec(ctx context.Context, out *cli.Printer, args []s
160161

161162
// Apply global user settings first (lowest priority)
162163
// User settings only apply if the flag wasn't explicitly set by the user
163-
userSettings := config.GetUserSettings()
164+
userSettings := userconfig.Get()
164165
if userSettings.HideToolResults && !f.hideToolResults {
165166
f.hideToolResults = true
166167
slog.Debug("Applying user settings", "hide_tool_results", true)
@@ -493,7 +494,7 @@ func (f *runExecFlags) handleRunMode(ctx context.Context, rt runtime.Runtime, se
493494
func applyTheme() {
494495
// Resolve theme from user config > built-in default
495496
themeRef := styles.DefaultThemeRef
496-
if userSettings := config.GetUserSettings(); userSettings.Theme != "" {
497+
if userSettings := userconfig.Get(); userSettings.Theme != "" {
497498
themeRef = userSettings.Theme
498499
}
499500

pkg/config/commands_test.go

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
11
package config
22

33
import (
4-
"context"
5-
"os"
64
"testing"
75

86
"github.com/stretchr/testify/require"
97
)
108

119
func TestV2Commands_AllForms(t *testing.T) {
12-
cfg, err := Load(t.Context(), testfileSource("testdata/commands_v2.yaml"))
10+
cfg, err := Load(t.Context(), NewFileSource("testdata/commands_v2.yaml"))
1311
require.NoError(t, err)
1412

1513
// Test simple map format
@@ -40,7 +38,7 @@ func TestV2Commands_AllForms(t *testing.T) {
4038
}
4139

4240
func TestV2Commands_DisplayText(t *testing.T) {
43-
cfg, err := Load(t.Context(), testfileSource("testdata/commands_v2.yaml"))
41+
cfg, err := Load(t.Context(), NewFileSource("testdata/commands_v2.yaml"))
4442
require.NoError(t, err)
4543

4644
// Simple format: DisplayText returns the instruction
@@ -53,7 +51,7 @@ func TestV2Commands_DisplayText(t *testing.T) {
5351
}
5452

5553
func TestMigrate_v1_Commands_AllForms(t *testing.T) {
56-
cfg, err := Load(t.Context(), testfileSource("testdata/commands_v1.yaml"))
54+
cfg, err := Load(t.Context(), NewFileSource("testdata/commands_v1.yaml"))
5755
require.NoError(t, err)
5856

5957
require.Equal(t, "root", cfg.Agents[0].Name)
@@ -71,7 +69,7 @@ func TestMigrate_v1_Commands_AllForms(t *testing.T) {
7169
}
7270

7371
func TestMigrate_v0_Commands_AllForms(t *testing.T) {
74-
cfg, err := Load(t.Context(), testfileSource("testdata/commands_v0.yaml"))
72+
cfg, err := Load(t.Context(), NewFileSource("testdata/commands_v0.yaml"))
7573
require.NoError(t, err)
7674

7775
require.Equal(t, "root", cfg.Agents[0].Name)
@@ -87,9 +85,3 @@ func TestMigrate_v0_Commands_AllForms(t *testing.T) {
8785
require.Equal(t, "yet_another_agent", cfg.Agents[2].Name)
8886
require.Empty(t, cfg.Agents[2].Commands)
8987
}
90-
91-
type testfileSource string
92-
93-
func (s testfileSource) Read(context.Context) ([]byte, error) {
94-
return os.ReadFile(string(s))
95-
}

pkg/config/config.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,7 @@ import (
1616
"github.com/docker/cagent/pkg/environment"
1717
)
1818

19-
type Reader interface {
20-
Read(ctx context.Context) ([]byte, error)
21-
}
22-
23-
func Load(ctx context.Context, source Reader) (*latest.Config, error) {
19+
func Load(ctx context.Context, source Source) (*latest.Config, error) {
2420
data, err := source.Read(ctx)
2521
if err != nil {
2622
return nil, err

pkg/config/config_test.go

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515
func TestAutoRegisterModels(t *testing.T) {
1616
t.Parallel()
1717

18-
cfg, err := Load(t.Context(), testfileSource("testdata/autoregister.yaml"))
18+
cfg, err := Load(t.Context(), NewFileSource("testdata/autoregister.yaml"))
1919
require.NoError(t, err)
2020

2121
assert.Len(t, cfg.Models, 2)
@@ -28,7 +28,7 @@ func TestAutoRegisterModels(t *testing.T) {
2828
func TestAutoRegisterAlloy(t *testing.T) {
2929
t.Parallel()
3030

31-
cfg, err := Load(t.Context(), testfileSource("testdata/autoregister_alloy.yaml"))
31+
cfg, err := Load(t.Context(), NewFileSource("testdata/autoregister_alloy.yaml"))
3232
require.NoError(t, err)
3333

3434
assert.Len(t, cfg.Models, 2)
@@ -41,7 +41,7 @@ func TestAutoRegisterAlloy(t *testing.T) {
4141
func TestAlloyModelComposition(t *testing.T) {
4242
t.Parallel()
4343

44-
cfg, err := Load(t.Context(), testfileSource("testdata/alloy_model_composition.yaml"))
44+
cfg, err := Load(t.Context(), NewFileSource("testdata/alloy_model_composition.yaml"))
4545
require.NoError(t, err)
4646

4747
// The alloy model should be expanded to its constituent models
@@ -57,7 +57,7 @@ func TestAlloyModelComposition(t *testing.T) {
5757
func TestAlloyModelNestedComposition(t *testing.T) {
5858
t.Parallel()
5959

60-
cfg, err := Load(t.Context(), testfileSource("testdata/alloy_model_nested.yaml"))
60+
cfg, err := Load(t.Context(), NewFileSource("testdata/alloy_model_nested.yaml"))
6161
require.NoError(t, err)
6262

6363
// The nested alloy should be fully expanded to all constituent models
@@ -72,7 +72,7 @@ func TestAlloyModelNestedComposition(t *testing.T) {
7272
func TestMigrate_v0_v1_provider(t *testing.T) {
7373
t.Parallel()
7474

75-
cfg, err := Load(t.Context(), testfileSource("testdata/provider_v0.yaml"))
75+
cfg, err := Load(t.Context(), NewFileSource("testdata/provider_v0.yaml"))
7676
require.NoError(t, err)
7777

7878
assert.Equal(t, "openai", cfg.Models["gpt"].Provider)
@@ -81,7 +81,7 @@ func TestMigrate_v0_v1_provider(t *testing.T) {
8181
func TestMigrate_v1_provider(t *testing.T) {
8282
t.Parallel()
8383

84-
cfg, err := Load(t.Context(), testfileSource("testdata/provider_v1.yaml"))
84+
cfg, err := Load(t.Context(), NewFileSource("testdata/provider_v1.yaml"))
8585
require.NoError(t, err)
8686

8787
assert.Equal(t, "openai", cfg.Models["gpt"].Provider)
@@ -90,7 +90,7 @@ func TestMigrate_v1_provider(t *testing.T) {
9090
func TestMigrate_v0_v1_todo(t *testing.T) {
9191
t.Parallel()
9292

93-
cfg, err := Load(t.Context(), testfileSource("testdata/todo_v0.yaml"))
93+
cfg, err := Load(t.Context(), NewFileSource("testdata/todo_v0.yaml"))
9494
require.NoError(t, err)
9595

9696
assert.Len(t, cfg.Agents.First().Toolsets, 2)
@@ -102,7 +102,7 @@ func TestMigrate_v0_v1_todo(t *testing.T) {
102102
func TestMigrate_v1_todo(t *testing.T) {
103103
t.Parallel()
104104

105-
cfg, err := Load(t.Context(), testfileSource("testdata/todo_v1.yaml"))
105+
cfg, err := Load(t.Context(), NewFileSource("testdata/todo_v1.yaml"))
106106
require.NoError(t, err)
107107

108108
assert.Len(t, cfg.Agents.First().Toolsets, 2)
@@ -114,7 +114,7 @@ func TestMigrate_v1_todo(t *testing.T) {
114114
func TestMigrate_v0_v1_shared_todo(t *testing.T) {
115115
t.Parallel()
116116

117-
cfg, err := Load(t.Context(), testfileSource("testdata/shared_todo_v0.yaml"))
117+
cfg, err := Load(t.Context(), NewFileSource("testdata/shared_todo_v0.yaml"))
118118
require.NoError(t, err)
119119

120120
assert.Len(t, cfg.Agents.First().Toolsets, 2)
@@ -126,7 +126,7 @@ func TestMigrate_v0_v1_shared_todo(t *testing.T) {
126126
func TestMigrate_v1_shared_todo(t *testing.T) {
127127
t.Parallel()
128128

129-
cfg, err := Load(t.Context(), testfileSource("testdata/shared_todo_v1.yaml"))
129+
cfg, err := Load(t.Context(), NewFileSource("testdata/shared_todo_v1.yaml"))
130130
require.NoError(t, err)
131131

132132
assert.Len(t, cfg.Agents.First().Toolsets, 2)
@@ -138,7 +138,7 @@ func TestMigrate_v1_shared_todo(t *testing.T) {
138138
func TestMigrate_v0_v1_think(t *testing.T) {
139139
t.Parallel()
140140

141-
cfg, err := Load(t.Context(), testfileSource("testdata/think_v0.yaml"))
141+
cfg, err := Load(t.Context(), NewFileSource("testdata/think_v0.yaml"))
142142
require.NoError(t, err)
143143

144144
assert.Len(t, cfg.Agents.First().Toolsets, 2)
@@ -149,7 +149,7 @@ func TestMigrate_v0_v1_think(t *testing.T) {
149149
func TestMigrate_v1_think(t *testing.T) {
150150
t.Parallel()
151151

152-
cfg, err := Load(t.Context(), testfileSource("testdata/think_v1.yaml"))
152+
cfg, err := Load(t.Context(), NewFileSource("testdata/think_v1.yaml"))
153153
require.NoError(t, err)
154154

155155
assert.Len(t, cfg.Agents.First().Toolsets, 2)
@@ -160,7 +160,7 @@ func TestMigrate_v1_think(t *testing.T) {
160160
func TestMigrate_v0_v1_memory(t *testing.T) {
161161
t.Parallel()
162162

163-
cfg, err := Load(t.Context(), testfileSource("testdata/memory_v0.yaml"))
163+
cfg, err := Load(t.Context(), NewFileSource("testdata/memory_v0.yaml"))
164164
require.NoError(t, err)
165165

166166
assert.Len(t, cfg.Agents.First().Toolsets, 2)
@@ -172,7 +172,7 @@ func TestMigrate_v0_v1_memory(t *testing.T) {
172172
func TestMigrate_v1_memory(t *testing.T) {
173173
t.Parallel()
174174

175-
cfg, err := Load(t.Context(), testfileSource("testdata/memory_v1.yaml"))
175+
cfg, err := Load(t.Context(), NewFileSource("testdata/memory_v1.yaml"))
176176
require.NoError(t, err)
177177

178178
assert.Len(t, cfg.Agents.First().Toolsets, 2)
@@ -184,7 +184,7 @@ func TestMigrate_v1_memory(t *testing.T) {
184184
func TestMigrate_v1(t *testing.T) {
185185
t.Parallel()
186186

187-
_, err := Load(t.Context(), testfileSource("testdata/v1.yaml"))
187+
_, err := Load(t.Context(), NewFileSource("testdata/v1.yaml"))
188188
require.NoError(t, err)
189189
}
190190

@@ -262,7 +262,7 @@ func TestCheckRequiredEnvVars(t *testing.T) {
262262
t.Run(test.yaml, func(t *testing.T) {
263263
t.Parallel()
264264

265-
cfg, err := Load(t.Context(), testfileSource("testdata/env/"+test.yaml))
265+
cfg, err := Load(t.Context(), NewFileSource("testdata/env/"+test.yaml))
266266
require.NoError(t, err)
267267

268268
err = CheckRequiredEnvVars(t.Context(), cfg, "", &noEnvProvider{})
@@ -282,7 +282,7 @@ func TestCheckRequiredEnvVars(t *testing.T) {
282282
func TestCheckRequiredEnvVarsWithModelGateway(t *testing.T) {
283283
t.Parallel()
284284

285-
cfg, err := Load(t.Context(), testfileSource("testdata/env/all.yaml"))
285+
cfg, err := Load(t.Context(), NewFileSource("testdata/env/all.yaml"))
286286
require.NoError(t, err)
287287

288288
err = CheckRequiredEnvVars(t.Context(), cfg, "gateway:8080", &noEnvProvider{})

pkg/config/examples_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func TestParseExamples(t *testing.T) {
4242
t.Run(file, func(t *testing.T) {
4343
t.Parallel()
4444

45-
cfg, err := Load(t.Context(), testfileSource(file))
45+
cfg, err := Load(t.Context(), NewFileSource(file))
4646

4747
require.NoError(t, err)
4848
require.Equal(t, latest.Version, cfg.Version, "Version should be %d in %s", latest.Version, file)

0 commit comments

Comments
 (0)