fix(config,server): reject zero-agent configs & guard GET /api/agents (#3588)#3589
Draft
aheritier wants to merge 3 commits into
Draft
fix(config,server): reject zero-agent configs & guard GET /api/agents (#3588)#3589aheritier wants to merge 3 commits into
aheritier wants to merge 3 commits into
Conversation
agent-schema.json added agents.minProperties:1 (rejecting an explicit
agents: {}) but never required the agents property itself, so a config
that omits agents entirely (e.g. just version: "12") was still schema
valid — contradicting validateConfig's "at least one agent must be
configured" rule and the docs.
Add root-level required: [agents] alongside the existing
minProperties: 1, so both the omitted-agents and empty-map shapes are
schema-invalid.
Pin the invariant with direct schema-negative tests
(TestJsonSchemaRejectsZeroAgents) asserting the schema rejects both
shapes; previously neither was covered, so removing either constraint
silently passed. Also tighten doc_yaml_test.go's looksLikeFullConfig
heuristic to require an "agents" key before schema-validating a doc
snippet as a full config, since many docs show partial models/
permissions/providers fragments without agents that are no longer
schema-valid as standalone full configs.
Refs #3588
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3588 — a config with no agents caused two problems:
GET /api/agentspanicked on every request. The handler evaluatedc.Agents.First().Descriptionbefore thelen==0guard that was meant toprotect it, and
latest.Agents.First()panics on an empty slice. With noecho Recover middleware, each request died with an empty reply and
stack-trace log spam.
validateConfignever required at least one agent, so agent-less configsloaded successfully in the first place.
This is finding B1 from the internal codebase audit.
What changed
getAgentsnow checkslen(cfg.Agents) == 0before anyFirst()access; the per-sourcesummarization is extracted into a small, directly testable
agentsAPIEntryhelper. The endpoint can no longer panic.validateConfignow rejects a config with noagents first, before providers/models/toolsets, with a clear, actionable
error: "at least one agent must be configured (add an entry under
'agents')". Validation runs after migration to
latest, so the ruleapplies to every supported config version (v0–v12).
agent-schema.jsonnow marksagentsasrequiredat the root and keepsminProperties: 1, so both an omittedagentskey and an explicitagents: {}are schema-invalid.agent is required; the schema and runtime now match.
Notes
latest.Agents.First()keeps its panic (frozenvNpackages untouched, perconfig-versioning discipline). Its production callers are all guarded, and
configs can no longer load without agents, so the optional
(AgentConfig, bool)signature change was skipped as pure churn.Testing
pkg/server/agents_test.go—agentsAPIEntrywith an empty config isNotPanics.pkg/server/server_test.go+testdata/no_agents.yaml— end-to-end handlertest: an agent-less source returns a clean 200 listing only the valid
agent, not a panic.
pkg/config/validation_test.go— agent-less config is rejected with theexpected message.
pkg/config/schema_test.go—TestJsonSchemaRejectsZeroAgentspins boththe
requiredandminPropertiesconstraints (each fails if its constraintis removed).
task dev(lint + 19 custom cops + tests + build) green;TestJsonSchemaWorksForExamples/TestSchemaMatchesGoTypesstill pass.Behavior change
Configs with no agents previously loaded (and then broke
GET /api/agents);they are now rejected at load time. Worth a release note.
Opened as a draft pending final maintainer review.