feat(agent): add primary allowed_tools/denied_tools filtering#2663
Merged
theonlyhennygod merged 2 commits intomainfrom Mar 3, 2026
Merged
feat(agent): add primary allowed_tools/denied_tools filtering#2663theonlyhennygod merged 2 commits intomainfrom
theonlyhennygod merged 2 commits intomainfrom
Conversation
PR intake checks found warnings (non-blocking)Fast safe checks found advisory issues. CI lint/test/build gates still enforce merge quality.
Action items:
Detected Linear keys: none Run logs: https://github.com/zeroclaw-labs/zeroclaw/actions/runs/22646588999 Detected blocking line issues (sample):
Detected advisory line issues (sample):
|
|
Thanks for contributing to ZeroClaw. For faster review, please ensure:
See |
Contributor
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Configuration Schema & Validation src/config/schema.rs |
Add AgentConfig.allowed_tools and AgentConfig.denied_tools (Vec) with Default init and validation enforcing non-empty trimmed entries and allowed chars (alphanumeric, _, -, *). Update unit tests for defaults and validation errors. |
Tool Filtering Logic src/tools/mod.rs |
Add filter_primary_agent_tools() and PrimaryAgentToolFilterReport to apply allowlist then denylist, with wildcard and case-insensitive matching, return unmatched allowed entries and match count. Include tests for wildcard, unmatched entries, and denylist interactions. |
Agent Integration & Prompt Sync src/agent/agent.rs, src/agent/loop_.rs |
Apply filtering during agent construction via filter_primary_agent_tools_or_fail(), error if filtering results in zero executable tools when both lists configured, and prune tool description blocks via retain_visible_tool_descriptions() so prompts match visible tools. Add tests covering filtering behavior and fail-fast case. |
Test Serialization Guard src/lib.rs, src/test_locks.rs, src/plugins/runtime.rs |
Introduce crate-private PLUGIN_RUNTIME_LOCK (parking_lot::Mutex) in src/test_locks.rs and expose test-only module. Acquire the lock in plugin runtime init tests to serialize global plugin runtime mutations. |
Documentation & i18n docs/config-reference.md, docs/i18n/{el,fr,ja,ru,vi,zh-CN}/config-reference.md |
Document allowed_tools and denied_tools under [agent], startup semantics (applied before prompt construction), logging for unknown allowed entries (debug), fail-fast behavior when filtering yields no tools, and provide a TOML example. Translations updated accordingly. |
Misc — Main/Test modules src/main.rs, Cargo.toml |
Add test-only test_locks module declarations; Cargo metadata updated alongside new test module files. |
Sequence Diagram(s)
sequenceDiagram
participant Config as Configuration (schema.rs)
participant Startup as Agent Startup (agent.rs)
participant Tools as Tool Registry (tools/mod.rs)
participant Filter as Filter Logic (loop_.rs)
participant Agent as Primary Agent (Agent)
Config->>Config: Validate `agent.allowed_tools` & `agent.denied_tools`
Config-->>Startup: Validated config
Startup->>Tools: Build full tool registry
Tools-->>Startup: Registry (all tools)
Startup->>Filter: filter_primary_agent_tools_or_fail(registry, allowed, denied)
Filter->>Filter: Apply allowed_tools (if non-empty)
Filter->>Filter: Record unmatched allowed entries
Filter->>Filter: Apply denied_tools
Filter->>Filter: If both lists present && no executable tools -> return error
alt Tools remain
Filter-->>Startup: Filtered registry + report
else No tools remain
Filter-->>Startup: Fail-fast error
end
Startup->>Filter: retain_visible_tool_descriptions(tool_descriptions, filtered_registry)
Filter-->>Startup: Pruned descriptions
Startup->>Agent: Construct Agent with filtered tools and descriptions
Agent-->>Startup: Agent ready (system prompt built from filtered tools)
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
- feat(gateway): add per-endpoint rate limiting and webhook idempotency #188 — Modifies tool initialization paths (
src/agent/loop_.rs,src/tools/mod.rs), intersecting with the registry/filtering and runtime-aware tool handling introduced here.
Suggested labels
size: L, risk: medium, tool, tests
Suggested reviewers
- chumyin
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description check | The PR description covers Summary, Validation, and references the linked issue #2651, but is missing required template sections including Label Snapshot, Change Metadata, Linked Issue details, Security Impact, Privacy/Data Hygiene, Compatibility/Migration, i18n Follow-Through, Human Verification, Side Effects, Rollback Plan, and Risks. |
Complete the PR description by filling in all required template sections: Label Snapshot (risk, size, scope labels), Change Metadata (type, primary scope), full Linked Issue section with Linear keys, Security/Privacy Impact assessments, Compatibility details, detailed i18n checklist completion, Human Verification scenarios, Side Effects analysis, and Rollback Plan. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title 'feat(agent): add primary allowed_tools/denied_tools filtering' directly and accurately describes the main change: adding allowed_tools/denied_tools filtering to the primary agent configuration. |
| Linked Issues check | ✅ Passed | The PR implements all core requirements from #2651: allowed_tools/denied_tools in AgentConfig, startup-time filtering with debug logging for unmatched entries, fail-fast when combined filters remove all tools, comprehensive documentation, i18n updates for all locales, and tests covering filtering/validation/integration scenarios. |
| Out of Scope Changes check | ✅ Passed | All changes directly address #2651 requirements: configuration schema, tool filtering logic, documentation/i18n, tests, and a shared test-lock module for serializing plugin-runtime tests. No unrelated modifications detected. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 87.80% which is sufficient. The required threshold is 80.00%. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
- 📝 Generate docstrings (stacked PR)
- 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
issue-2651-agent-allowed-denied-tools
Comment @coderabbitai help to get the list of available commands and usage tips.
github-actions
Bot
added
docs
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/config/schema.rs (1)
1047-1054: Document default and rollback semantics inline for the new public config keys.The new
[agent]keys are user-facing contract fields; add explicit default + compatibility/rollback notes in these rustdoc comments for consistency with other config keys.Suggested doc patch
/// Optional allowlist for primary-agent tool visibility. /// When non-empty, only listed tools are exposed to the primary agent. + /// Default: `[]` (all registered tools remain visible). + /// Compatibility/rollback: omit/remove this key to keep legacy behavior. #[serde(default)] pub allowed_tools: Vec<String>, /// Optional denylist for primary-agent tool visibility. /// Applied after `allowed_tools`. + /// Default: `[]` (no explicit denies). + /// Compatibility/rollback: omit/remove this key to disable deny overrides. #[serde(default)] pub denied_tools: Vec<String>,As per coding guidelines
src/config/**/*.rs: "Treat config keys as public contract: document defaults, compatibility impact, and migration/rollback path".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/config/schema.rs` around lines 1047 - 1054, Update the rustdoc for the public struct fields allowed_tools and denied_tools to clearly state their default values (both default to empty lists via #[serde(default]) and therefore impose no restrictions when empty), describe the semantics/order (allowed_tools is applied first to restrict to only listed tools, then denied_tools removes specific tools from that result), and add a brief compatibility/rollback note telling operators that adding these keys is safe (old configs behave the same because empty defaults are used), and how to roll back (remove or set keys to empty to restore prior behavior); place these notes inline as comments above the allowed_tools and denied_tools fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/i18n/fr/config-reference.md`:
- Around line 25-26: Update the French bullet for `allowed_tools` to improve
phrasing and typography: change "non vide: seuls les outils listés sont
exposés." to use proper French spacing and a smoother wording such as "non
vide : seuls les outils listés sont exposés." and ensure the following
`denied_tools` line remains "retrait supplémentaire appliqué après
`allowed_tools`." so both bullets read fluently and consistently
(`allowed_tools` and `denied_tools`).
---
Nitpick comments:
In `@src/config/schema.rs`:
- Around line 1047-1054: Update the rustdoc for the public struct fields
allowed_tools and denied_tools to clearly state their default values (both
default to empty lists via #[serde(default]) and therefore impose no
restrictions when empty), describe the semantics/order (allowed_tools is applied
first to restrict to only listed tools, then denied_tools removes specific tools
from that result), and add a brief compatibility/rollback note telling operators
that adding these keys is safe (old configs behave the same because empty
defaults are used), and how to roll back (remove or set keys to empty to restore
prior behavior); place these notes inline as comments above the allowed_tools
and denied_tools fields.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
docs/config-reference.mddocs/i18n/el/config-reference.mddocs/i18n/fr/config-reference.mddocs/i18n/ja/config-reference.mddocs/i18n/ru/config-reference.mddocs/i18n/vi/config-reference.mddocs/i18n/zh-CN/config-reference.mdsrc/agent/agent.rssrc/agent/loop_.rssrc/config/schema.rssrc/lib.rssrc/plugins/runtime.rssrc/test_locks.rssrc/tools/mod.rs
Comment on lines
+25
to
+26
| - `allowed_tools` non vide: seuls les outils listés sont exposés. | ||
| - `denied_tools`: retrait supplémentaire appliqué après `allowed_tools`. |
Contributor
There was a problem hiding this comment.
Polish the French phrasing/typography in this bullet.
Line 25 reads slightly awkwardly; a small rewrite improves fluency and apostrophe style.
✍️ Suggested wording tweak
- - `allowed_tools` non vide: seuls les outils listés sont exposés.
+ - `allowed_tools` n’est pas vide : seuls les outils listés sont exposés.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - `allowed_tools` non vide: seuls les outils listés sont exposés. | |
| - `denied_tools`: retrait supplémentaire appliqué après `allowed_tools`. | |
| - `allowed_tools` n'est pas vide : seuls les outils listés sont exposés. | |
| - `denied_tools`: retrait supplémentaire appliqué après `allowed_tools`. |
🧰 Tools
🪛 LanguageTool
[typographical] ~26-~26: Caractère d’apostrophe incorrect.
Context: ...: retrait supplémentaire appliqué après allowed_tools. - Les entrées inconnues dans `allowed_...
(APOS_INCORRECT)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/i18n/fr/config-reference.md` around lines 25 - 26, Update the French
bullet for `allowed_tools` to improve phrasing and typography: change "non vide:
seuls les outils listés sont exposés." to use proper French spacing and a
smoother wording such as "non vide : seuls les outils listés sont exposés." and
ensure the following `denied_tools` line remains "retrait supplémentaire
appliqué après `allowed_tools`." so both bullets read fluently and consistently
(`allowed_tools` and `denied_tools`).
This was referenced Mar 4, 2026
2 tasks
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
1 participant
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
[agent].allowed_toolsand[agent].denied_toolstoAgentConfigsecurity.roles[].allowed_tools/denied_toolstools::filter_primary_agent_tools)Agent::from_configagent::loop_::runandprocess_message_with_session)allowed_toolsentriesallowed_tools + denied_toolsremove all matched allowlisted toolsdocs/config-reference.mddocs/i18n/{zh-CN,ja,ru,fr,vi,el}/config-reference.mdValidation
cargo fmt --allcargo test --lib filter_primary_agent_tools_ -- --nocapturecargo test --lib from_config_ -- --nocapturecargo test --lib agent_validation_rejects_ -- --nocaptureCloses #2651
Summary by CodeRabbit
New Features
Documentation