fix(agents): reject a boolean-like tools scalar instead of turning it into a tool name - #899
fix(agents): reject a boolean-like tools scalar instead of turning it into a tool name#899L4XB wants to merge 2 commits into
Conversation
… into a tool name The frontmatter parser keeps every scalar as a string and parseTools split it on commas, so `tools: false` (a natural way to write "no tools" in YAML) produced a subagent allowlist containing a tool named "false" with no warning anywhere. Treat true/false/yes/no/on/off/null/none/~ as a definition error through the existing AgentDefinitionError path, with a hint to use `tools: []` or omit the key. Real tool names, csv lists, inline lists, and a missing key parse as before. Closes Gentleman-Programming#895
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthrough
ChangesAgent tool validation
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to Boolean- and null-like tool values are rejected as intended, with no identified merge-blocking risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The only conflict was tests/agents-config.test.ts, where main and this branch each append new cells to the same tail. Both kept.
Closes #895.
Problem
parseFrontmatterkeeps every scalar as a string andparseToolssplit it on commas, so an agent file withtools: false(a natural way to write "no tools" in YAML) produced a definition whosetoolswas["false"]. That token reached the child's tool allowlist as a nonexistent tool name, silently.Change
parseToolsnow recognises the YAML scalars that spell a boolean or null (true,false,yes,no,on,off,null,none,~, case-insensitive) when they arrive as the whole value and reports them through the existingAgentDefinitionErrorpath, the same way unknown thinking levels and modes are rejected:tools "false" is not a tool list; use "tools: []" or omit the key for no tools. A real single tool name, a csv (read, bash), an inline list ([],[read]), a block list, and a missing key parse exactly as before. Items inside an explicit list are not second-guessed.Tests
New case in
tests/agents-config.test.tscovering each rejected spelling (includingOff) and the four legitimate forms.Summary by CodeRabbit
Bug Fixes
tools: [].Tests