Skip to content

fix(agents): reject a boolean-like tools scalar instead of turning it into a tool name - #899

Open
L4XB wants to merge 2 commits into
Gentleman-Programming:mainfrom
L4XB:fix/agent-tools-scalar-validation
Open

fix(agents): reject a boolean-like tools scalar instead of turning it into a tool name#899
L4XB wants to merge 2 commits into
Gentleman-Programming:mainfrom
L4XB:fix/agent-tools-scalar-validation

Conversation

@L4XB

@L4XB L4XB commented Sep 11, 2026

Copy link
Copy Markdown

Closes #895.

Problem

parseFrontmatter keeps every scalar as a string and parseTools split it on commas, so an agent file with tools: false (a natural way to write "no tools" in YAML) produced a definition whose tools was ["false"]. That token reached the child's tool allowlist as a nonexistent tool name, silently.

Change

parseTools now 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 existing AgentDefinitionError path, 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.ts covering each rejected spelling (including Off) and the four legitimate forms.

node --experimental-strip-types --test tests/agents-config.test.ts
ℹ tests 10  ℹ pass 10  ℹ fail 0

Summary by CodeRabbit

  • Bug Fixes

    • Agent configurations now reject boolean- or null-like scalar values in the tools setting with a clear validation error recommending tools: [].
    • Valid single-tool, comma-separated, empty-list, and missing-tool configurations continue to parse as expected.
    • Invalid tool settings now produce a consistent configuration error without affecting valid agent definitions.
  • Tests

    • Added coverage for invalid scalar tool values and existing valid parsing scenarios.

… 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
@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 0a91a71a-9e37-4959-bafb-2459f83282a1

📥 Commits

Reviewing files that changed from the base of the PR and between 045c1ee and f0486dc.

📒 Files selected for processing (2)
  • lib/agents-config.ts
  • tests/agents-config.test.ts

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

parseTools rejects boolean-like scalar values. parseAgentDefinition returns an error for those values and reuses the parsed tools result. Tests cover invalid scalars and valid tool formats.

Changes

Agent tool validation

Layer / File(s) Summary
Reject invalid scalar tools
lib/agents-config.ts
parseTools rejects boolean-like and null-like scalar values. parseAgentDefinition returns an AgentDefinitionError for the resulting error.
Reuse parsed tools and validate formats
lib/agents-config.ts, tests/agents-config.test.ts
The agent definition reuses the computed tools value. Tests cover invalid scalar values, valid single tools, CSV values, empty lists, and missing keys.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: alan-thegentleman

Merge Risk: ⚪ Minimal · up to f0486

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)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: rejecting boolean-like scalar values in the tools field instead of treating them as tool names.
Linked Issues check ✅ Passed Issue #895 requires boolean-like or null-like scalar values under tools to avoid becoming tool names. lib/agents-config.ts checks scalar values against true, false, yes, no, on, off, `…
Out of Scope Changes check ✅ Passed The changes are limited to scalar tools validation in lib/agents-config.ts and focused tests in tests/agents-config.test.ts. These changes directly support issue #895. No unrelated behavior is i…
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

The only conflict was tests/agents-config.test.ts, where main and this
branch each append new cells to the same tail. Both kept.
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.

bug(agents): scalar tools: frontmatter value (e.g. tools: false) silently becomes a garbage tool token instead of a validation error

1 participant