feat(config): support top-level shared skills and commands#2971
Conversation
Add top-level `commands` and `skills` sections holding reusable, named groups that agents opt into by name via `use_commands` / `use_skills`. Groups are merged into each agent during config resolution; an agent's inline entries win on name conflicts. Bumps the config schema to v11 and updates the JSON schema, HCL converter, example, and docs. Closes #2967
aheritier
added
area/config
| @@ -0,0 +1,253 @@ | |||
| package v10 | |||
There was a problem hiding this comment.
All those changes should go in the latest package, no need to create a new version
|
❌ PR Review Failed — The review agent encountered an error and could not complete the review. View logs. |
Drop the v11 schema bump and the frozen v10 package. The top-level commands/skills support now lives entirely in the latest package and the schema stays at version 10.
Adds `use_commands` and `use_skills` to the agent properties reference and full schema YAML example in docs/configuration/agents/index.md. These agent-level fields were introduced by #2971 but were missing from the agent reference doc. Also fixes a broken example link in docs/configuration/overview/index.md that pointed to the wrong repo (cagent instead of docker-agent). Source PR: #2971
|
/review |
docker-agent
left a comment
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The implementation is well-structured. Merge precedence (inline wins over shared), nil-map guards, skill deduplication, and validation ordering are all correct. No high or medium severity bugs were found.
Two minor observations are left as inline comments.
| // SkillsConfig. Every top-level group is validated up front (parity with | ||
| // resolveMCPDefinitions), then each referenced group is merged into the agent. | ||
| func resolveSkillDefinitions(cfg *latest.Config) error { | ||
| for name := range cfg.Skills { |
There was a problem hiding this comment.
[LOW] Non-deterministic error message when multiple skill groups are invalid
for name := range cfg.Skills iterates the map in random order. If two or more top-level skill groups are simultaneously invalid, the error returned on each run may name a different group, producing unstable output that can make error messages hard to reproduce and could cause flaky tests if any future test ever checks which group's error surfaces.
Suggested fix: collect all group names into a sorted slice before iterating, so validation always reports errors in a deterministic order (consistent with how other validation loops in the codebase use slices.Sorted / sort.Strings).
| // Commands map. Commands defined inline on the agent take precedence over a | ||
| // group's command of the same name, mirroring the override semantics used for | ||
| // MCP definitions (see applyMCPDefaults). | ||
| func resolveCommandDefinitions(cfg *latest.Config) error { |
There was a problem hiding this comment.
[LOW] Top-level commands groups are not validated proactively
resolveSkillDefinitions validates every top-level skill group (even unreferenced ones) before merging. resolveCommandDefinitions has no equivalent loop — top-level command groups are only inspected when an agent explicitly references them via use_commands. A misconfigured but unreferenced command group will never be caught.
While current CommandConfig fields are validated at YAML parse time, the asymmetry will become a silent correctness gap if richer semantic validation is added to commands in the future.
Suggested fix: add an up-front validation loop over cfg.Commands (mirroring the cfg.Skills loop) or add a comment explaining the intentional difference.
4 participants
What
Implements top-level, shareable
skillsandcommands, so the same command set or skill configuration can be reused across multiple agents in a single YAML instead of being duplicated.How
commandsandskillsmaps of named, reusable groups.use_commands/use_skillsfields.teamloaderand the runtime are unchanged. Agent inline entries take precedence on name conflicts; skill sources/includes are unioned and inline skills deduped by name.pkg/config/v10).agent-schema.json, the HCL block rules, addedexamples/shared-commands-skills.yaml, and documented the sections indocs/configuration/overview.Testing
pkg/configcover merge semantics, inline override, missing-group errors, and top-level group validation.cagent debug config examples/shared-commands-skills.yaml.Closes #2967