Skip to content

Support comma-separated string for allowed-tools in skills#1984

Merged
rumpl merged 1 commit intodocker:mainfrom
dgageot:skill-format
Mar 8, 2026
Merged

Support comma-separated string for allowed-tools in skills#1984
rumpl merged 1 commit intodocker:mainfrom
dgageot:skill-format

Conversation

@dgageot
Copy link
Member

@dgageot dgageot commented Mar 8, 2026

The allowed-tools field in skill frontmatter now accepts both a YAML list and a comma-separated string (e.g. "allowed-tools: Read, Grep"). This is done via a custom stringOrList type with UnmarshalYAML that first tries list parsing, then falls back to splitting a scalar string by commas.

Assisted-By: cagent

The allowed-tools field in skill frontmatter now accepts both a YAML list
and a comma-separated string (e.g. "allowed-tools: Read, Grep"). This is
done via a custom stringOrList type with UnmarshalYAML that first tries
list parsing, then falls back to splitting a scalar string by commas.

Assisted-By: cagent
@dgageot dgageot requested a review from a team as a code owner March 8, 2026 17:44
Copy link
Member

@maxcleme maxcleme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥

Copy link

@docker-agent docker-agent bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Assessment: 🟢 APPROVE

The implementation is solid overall. The custom stringOrList type correctly handles both YAML list and comma-separated string formats through a fallback UnmarshalYAML strategy. Test coverage is comprehensive with good edge cases.

One minor usability issue was identified regarding error messages when invalid YAML is provided.

Findings

1 low-severity issue found

@rumpl rumpl merged commit 1f2980b into docker:main Mar 8, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants