diff --git a/agent-schema.json b/agent-schema.json index 3c126ebf8b..731c133a5c 100644 --- a/agent-schema.json +++ b/agent-schema.json @@ -48,7 +48,8 @@ }, "agents": { "type": "object", - "description": "Map of agent configurations", + "description": "Map of agent configurations. At least one agent is required.", + "minProperties": 1, "additionalProperties": { "$ref": "#/definitions/AgentConfig" } @@ -109,6 +110,9 @@ } }, "additionalProperties": false, + "required": [ + "agents" + ], "definitions": { "Lifecycle": { "type": "object", diff --git a/docs/configuration/agents/index.md b/docs/configuration/agents/index.md index 998910e22e..631c5d516b 100644 --- a/docs/configuration/agents/index.md +++ b/docs/configuration/agents/index.md @@ -9,6 +9,8 @@ canonical: https://docs.docker.com/ai/docker-agent/configuration/agents/ _Complete reference for defining agents in your YAML configuration._ +A configuration must define at least one agent under `agents`. + ## Full Schema diff --git a/docs/configuration/overview/index.md b/docs/configuration/overview/index.md index 253da9dc97..7380c080c4 100644 --- a/docs/configuration/overview/index.md +++ b/docs/configuration/overview/index.md @@ -34,7 +34,7 @@ models: model: claude-sonnet-4-5 max_tokens: 64000 -# 4. Agents — define AI agents with their behavior +# 4. Agents — define AI agents with their behavior (at least one is required) agents: root: model: claude diff --git a/pkg/config/config.go b/pkg/config/config.go index 2bb42e5e65..d0c464b649 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -228,6 +228,10 @@ func migrateToLatestConfig(c any, raw []byte) (latest.Config, error) { } func validateConfig(cfg *latest.Config) error { + if len(cfg.Agents) == 0 { + return errors.New("at least one agent must be configured (add an entry under 'agents')") + } + if err := validateProviders(cfg); err != nil { return err } diff --git a/pkg/config/doc_yaml_test.go b/pkg/config/doc_yaml_test.go index 5dd682d00c..7882c1c29c 100644 --- a/pkg/config/doc_yaml_test.go +++ b/pkg/config/doc_yaml_test.go @@ -94,13 +94,18 @@ func TestDocYAMLSnippetsAreValid(t *testing.T) { } // looksLikeFullConfig returns true when the parsed YAML value is a map whose -// keys are all recognized top-level config keys. This avoids schema-validating -// partial snippets that would trivially fail required-field checks. +// keys are all recognized top-level config keys AND it declares "agents". +// The schema requires "agents" at the root, so a snippet lacking it is a +// partial doc example (e.g. a models/permissions fragment), not a full +// config, and shouldn't be schema-validated as one. func looksLikeFullConfig(v any) bool { m, ok := v.(map[string]any) if !ok || len(m) == 0 { return false } + if _, hasAgents := m["agents"]; !hasAgents { + return false + } for k := range m { if !topLevelConfigKeys[k] { return false diff --git a/pkg/config/schema_test.go b/pkg/config/schema_test.go index 476d469d04..d663348ee9 100644 --- a/pkg/config/schema_test.go +++ b/pkg/config/schema_test.go @@ -56,6 +56,51 @@ func TestJsonSchemaWorksForExamples(t *testing.T) { } } +// TestJsonSchemaRejectsZeroAgents pins the runtime's "at least one agent" +// invariant (enforced by validateConfig) at the schema level too. Both the +// root-level "required": ["agents"] and the "agents"."minProperties": 1 +// constraints are needed: without either one, one of these shapes would +// pass schema validation despite being rejected at runtime. +func TestJsonSchemaRejectsZeroAgents(t *testing.T) { + t.Parallel() + + schemaBytes, err := os.ReadFile(schemaFile) + require.NoError(t, err) + + schema, err := gojsonschema.NewSchema(gojsonschema.NewBytesLoader(schemaBytes)) + require.NoError(t, err) + + tests := []struct { + name string + yaml string + }{ + { + name: "agents omitted entirely", + yaml: `version: "12" +`, + }, + { + name: "agents is an empty map", + yaml: `version: "12" +agents: {} +`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + var rawJSON any + require.NoError(t, yaml.Unmarshal([]byte(tt.yaml), &rawJSON)) + + result, err := schema.Validate(gojsonschema.NewRawLoader(rawJSON)) + require.NoError(t, err) + assert.False(t, result.Valid(), "expected schema to reject config with %s", tt.name) + }) + } +} + // TestSchemaMatchesGoTypes verifies that every JSON-tagged field in the Go // config structs has a corresponding property in agent-schema.json (and // vice-versa). This prevents the schema from silently drifting out of sync diff --git a/pkg/config/validation_test.go b/pkg/config/validation_test.go index 3f6ebbd38f..6da870ff4e 100644 --- a/pkg/config/validation_test.go +++ b/pkg/config/validation_test.go @@ -133,6 +133,20 @@ agents: assert.Contains(t, err.Error(), "2") } +func TestLoadConfig_RequiresAtLeastOneAgent(t *testing.T) { + t.Parallel() + + cfg := `version: "12" +models: + fast: + provider: openai + model: gpt-4o-mini +` + _, err := Load(t.Context(), NewBytesSource("test", []byte(cfg))) + require.Error(t, err) + assert.Contains(t, err.Error(), "at least one agent must be configured") +} + func TestValidSkillsConfiguration(t *testing.T) { t.Parallel() diff --git a/pkg/server/agents_test.go b/pkg/server/agents_test.go new file mode 100644 index 0000000000..d03d4c5567 --- /dev/null +++ b/pkg/server/agents_test.go @@ -0,0 +1,42 @@ +package server + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/config/latest" +) + +// TestAgentsAPIEntry_ZeroAgents pins the docker/docker-agent#3588 handler +// guard directly: agentsAPIEntry must report ok=false (not panic) for a +// config with no agents, exercising the len(cfg.Agents)==0 check regardless +// of whether validateConfig also rejects such a config at load time +// (defense in depth). +func TestAgentsAPIEntry_ZeroAgents(t *testing.T) { + t.Parallel() + + cfg := &latest.Config{} + + require.NotPanics(t, func() { + _, ok := agentsAPIEntry("empty", cfg) + assert.False(t, ok) + }) +} + +func TestAgentsAPIEntry_SingleAndMultiAgent(t *testing.T) { + t.Parallel() + + single := &latest.Config{Agents: latest.Agents{{Name: "root", Description: "solo"}}} + agent, ok := agentsAPIEntry("single", single) + require.True(t, ok) + assert.False(t, agent.Multi) + assert.Equal(t, "solo", agent.Description) + + multi := &latest.Config{Agents: latest.Agents{{Name: "root", Description: "lead"}, {Name: "helper"}}} + agent, ok = agentsAPIEntry("multi", multi) + require.True(t, ok) + assert.True(t, agent.Multi) + assert.Equal(t, "lead", agent.Description) +} diff --git a/pkg/server/server.go b/pkg/server/server.go index 6f1c7e3b74..39900f52eb 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -21,6 +21,7 @@ import ( "github.com/docker/docker-agent/pkg/api" "github.com/docker/docker-agent/pkg/config" + "github.com/docker/docker-agent/pkg/config/latest" "github.com/docker/docker-agent/pkg/echolog" "github.com/docker/docker-agent/pkg/runtime" "github.com/docker/docker-agent/pkg/session" @@ -157,31 +158,18 @@ func (s *Server) getAgents(c echo.Context) error { for k, agentSource := range s.sm.Sources { slog.Debug("API source", "source", agentSource.Name()) - c, err := config.Load(c.Request().Context(), agentSource) + cfg, err := config.Load(c.Request().Context(), agentSource) if err != nil { slog.Error("Failed to load config from API source", "key", k, "error", err) continue } - desc := c.Agents.First().Description - - switch { - case len(c.Agents) > 1: - agents = append(agents, api.Agent{ - Name: k, - Multi: true, - Description: desc, - }) - case len(c.Agents) == 1: - agents = append(agents, api.Agent{ - Name: k, - Multi: false, - Description: desc, - }) - default: + agent, ok := agentsAPIEntry(k, cfg) + if !ok { slog.Warn("No agents found in config from API source", "key", k) continue } + agents = append(agents, agent) } slices.SortFunc(agents, func(a, b api.Agent) int { @@ -191,6 +179,23 @@ func (s *Server) getAgents(c echo.Context) error { return c.JSON(http.StatusOK, agents) } +// agentsAPIEntry summarizes a loaded config into the api.Agent listing +// entry for /api/agents. The len(cfg.Agents)==0 check MUST run before any +// access to cfg.Agents (e.g. First()), which panics on an empty slice: this +// guards the handler even though validateConfig already rejects agent-less +// configs at load time (defense in depth against a bypass or future +// regression in that check). +func agentsAPIEntry(name string, cfg *latest.Config) (api.Agent, bool) { + if len(cfg.Agents) == 0 { + return api.Agent{}, false + } + return api.Agent{ + Name: name, + Multi: len(cfg.Agents) > 1, + Description: cfg.Agents.First().Description, + }, true +} + func (s *Server) getAgentConfig(c echo.Context) error { agentID := c.Param("id") diff --git a/pkg/server/server_test.go b/pkg/server/server_test.go index 92d9c49afb..88b210eeff 100644 --- a/pkg/server/server_test.go +++ b/pkg/server/server_test.go @@ -58,6 +58,28 @@ func TestServer_EmptyList(t *testing.T) { assert.Equal(t, "[]\n", string(buf)) // We don't want null, but an empty array } +// TestServer_ZeroAgentSource pins the fix for docker/docker-agent#3588: +// a config source with no agents must never make GET /api/agents panic +// (latest.Agents.First() panics on an empty slice). Today validateConfig +// rejects the agent-less config at load time, so the handler's own +// len(cfg.Agents)==0 guard (agentsAPIEntry) never even gets exercised by +// this path — the request still yields a clean, empty listing rather than +// a panic either way. +func TestServer_ZeroAgentSource(t *testing.T) { + t.Parallel() + + ctx := t.Context() + lnPath := startServer(t, ctx, prepareAgentsDir(t, "no_agents.yaml", "pirate.yaml")) + + buf := httpGET(t, ctx, lnPath, "/api/agents") + + var agents []api.Agent + unmarshal(t, buf, &agents) + + require.Len(t, agents, 1) + assert.Contains(t, agents[0].Name, "pirate") +} + func TestServer_ListSessions(t *testing.T) { t.Parallel() diff --git a/pkg/server/testdata/no_agents.yaml b/pkg/server/testdata/no_agents.yaml new file mode 100644 index 0000000000..8c0ddab6be --- /dev/null +++ b/pkg/server/testdata/no_agents.yaml @@ -0,0 +1 @@ +version: "12"