Repository navigation
feat(guardrails): attach guardrails to agent tool calls via control plane - #1793
Open
madhumitha-chandrasekaran-1 wants to merge 5 commits into
Conversation
…lane Lane connections only ever see what feeds a pipeline's final response, so guardrails could never check what an agent does with a tool (Slack, Gmail, GitHub, DB-as-tool, etc.) since tool calls happen entirely inside the agent's own turn. A guard node can now attach via `control` to an agent node (classType: "guard"), and every tool call gets checked before it runs (args) and after it returns (result) using the same policy_mode semantics as the questions/answers lanes. Closes the general case from rocketride-org#1792 for any lane-connected agent holding tools directly; the CrewAI hierarchical-subagent case is a narrower follow-up.
madhumitha-chandrasekaran-1
requested review from
Rod-Christensen,
jmaionchi and
stepmikhaylov
as code owners
August 3, 2026 20:51
github-actions
Bot
added
docs
Contributor
🤖 Internal: Discord sync markerAuto-managed by the Discord notification workflow. Stores the linked Discord message ID and forum thread ID. Do not edit or delete. |
Contributor
📝 WalkthroughWalkthroughThe PR adds a public control-plane guard check contract. Agent tools and memory operations evaluate connected guardrails before crossing the control-plane boundary. The example and documentation show the new wiring. ChangesGuardrails control plane
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Agent
participant Tools
participant Guardrails
participant Tool
Agent->>Tools: Request tool invocation
Tools->>Guardrails: Check tool arguments
Guardrails-->>Tools: Return pass or block verdict
Tools->>Tool: Invoke when arguments pass
Tool-->>Tools: Return tool output
Tools->>Guardrails: Check tool output
Guardrails-->>Tools: Return pass or block verdict
Tools-->>Agent: Return output or raise ValueError
Possibly related PRs
Suggested reviewers: 🚥 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 |
![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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nodes/src/nodes/guardrails/IInstance.py`:
- Around line 176-183: Update the violation loop in check() to call warning()
only when result['action'] is 'block' or 'warn', matching writeQuestions() and
writeAnswers(). Preserve result assignment and return behavior, and keep action
'log' silent.
In `@packages/ai/src/ai/common/agent/_internal/host.py`:
- Around line 177-183: Update the guard evaluation in _run_guard so a missing,
malformed, or unrecognized param.result/action fails closed instead of allowing
the tool call to proceed. Validate the result after invoking each guard node,
and preserve the existing violation reporting for an explicit 'block' action
while treating any action other than an accepted pass value as a guard failure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 36732e5c-7dc1-473b-a069-346c75defa78
📒 Files selected for processing (9)
examples/slack-agent.pipenodes/src/nodes/guardrails/IInstance.pynodes/src/nodes/guardrails/README.mdnodes/test/guardrails/test_all.pypackages/ai/src/ai/common/agent/_internal/host.pypackages/ai/tests/ai/common/agent/_internal/__init__.pypackages/ai/tests/ai/common/agent/_internal/test_host_guard.pypackages/server/engine-lib/rocketlib-python/lib/rocketlib/__init__.pypackages/server/engine-lib/rocketlib-python/lib/rocketlib/types.py
…ard result Address coderabbit review on rocketride-org#1793: - check() was warning() on every violation regardless of policy_mode, logging even in `log` mode, unlike writeQuestions/writeAnswers and contrary to the documented behavior. - _run_guard() treated a missing/malformed `action` from a guard node as an implicit pass. Since this is a security control, a broken or misconfigured guard should fail closed instead of silently disabling enforcement.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ai/src/ai/common/agent/_internal/host.py`:
- Around line 184-192: Update the guard-result handling around the action
validation to first require that result is a mapping, raising ValueError for any
other type, and validate that violations is an iterable of mappings with usable
rule values before constructing the blocked-tool message. Ensure all malformed
results, including non-mapping param.result values, consistently raise
ValueError rather than AttributeError or iteration errors, and add coverage for
a non-mapping result.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4b543f1e-419e-48e9-8877-3b3253812f0e
📒 Files selected for processing (4)
nodes/src/nodes/guardrails/IInstance.pynodes/test/guardrails/test_all.pypackages/ai/src/ai/common/agent/_internal/host.pypackages/ai/tests/ai/common/agent/_internal/test_host_guard.py
Address follow-up coderabbit review on rocketride-org#1793: getattr(param, 'result', None) or {} only replaces falsy non-dicts, so a non-empty list or string from a malformed guard node passed through and crashed result.get() with AttributeError instead of the intended fail-closed ValueError. Now validates result is a dict (falls through to the "no recognized action" branch otherwise) and defends the violations-formatting path against a non-list violations value or non-dict violation items.
Memory.put/get/list bypassed the guard entirely -- a separate host channel from Tools, discovered and invoked independently, so an agent could write PII/injected content into persistent memory or read back poisoned stored content with a guard fully configured and attached, and it would never see either. Memory is architecturally the same kind of control-plane side effect this PR already covers for tool calls (the issue itself calls out db-as-tool writes as in-scope), so it gets the same treatment: - Memory.put is guarded pre-write (mode='output') on the value being stored. - Memory.get/list are guarded post-read (mode='input') on the value/keys read back -- the indirect-prompt-injection case, but via memory another run wrote instead of a tool response. - Memory.clear is left unguarded -- deletion carries no content to check. Extracted the shared block/warn/log + fail-closed logic out of Tools._run_guard into a module-level _check_guard() used by both Tools and Memory, rather than duplicating it.
…e lookup /simplify pass on PR rocketride-org#1793: - _guard_text() reimplemented what ai.common.utils.safe_str already does (explicitly documented as "the single home of safe_str... no duplicate copy anywhere else"). Drop it and call safe_str() directly -- also fixes a latent bug where the except-fallback's own str(value) call wasn't itself guarded. - Tools and Memory each called getControllerNodeIds('guard') independently even though both are constructed together from AgentHostServices.__init__. Resolve guard_nodes once there and pass it to both constructors.
Contributor
Author
|
@coderabbitai resume |
Contributor
✅ Action performedReview finished.
|
Contributor
✅ Action performedReviews resumed. |
4 of 7 tasks
Collaborator
|
The conflict here comes from your own merged #1861.
|
This branch has not been deployed
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
3 participants
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
Guardrails could only ever check one place in a pipeline: whatever feeds the final response node, via lane connections (
questions/answers/documents). Every tool node (tool_slack,tool_gmail,tool_github, DB-as-tool,tool_filesystem, etc.) and persistent-memory read/write runs entirely on the control plane inside an agent's own turn, so guardrails never saw either — no matter how many tools an agent held, or how the pipeline was wired.This adds a second attachment mode: a
guardnode can now attach viacontroldirectly to an agent node (classType: "guard"), and every tool call and memory read/write the agent makes gets checked automatically:mode='output') — the agent-generated content is checked the same way an answer would be (PII, content-safety, hallucination).mode='input') — checked the same way a question would be (prompt-injection defense against a poisoned webhook/API/DB response, or content another run stored earlier — the standard indirect-prompt-injection-into-tool-use case).policy_mode: blockraises before the call executes or before its result reaches the agent; the framework driver surfaces this as a normal tool/memory error (not a pipeline crash).warn/logbehave the same as on the existing lanes —logstays silent, matchingwriteQuestions/writeAnswers. A guard node that returns anything other than a recognizedpass/warn/log/blockaction (missing, malformed, wrong type) is treated as a block — fails closed instead of silently disabling the check. One guard node covers every tool and memory channel the agent holds — no per-tool wiring needed.Memory.clearis left unguarded since deletion carries no content to evaluate.Closes the general case in #1792 for any lane-connected agent holding tools/memory directly (covers all framework drivers — LangChain, deepagents, llamaindex, plain CrewAI — since they all route through the same
AgentBase.call_toolchoke point). The CrewAI hierarchical-subagent case (agent_crewai_subagent, itself control-plane-only) needs a follow-up fix to the pipeline-graph visited-propagation walk inpipeline_config.cppand is intentionally out of scope here.Changes
rocketlib/types.py,rocketlib/__init__.py— newIInvokeGuard.Checkop, following the existingIInvokeTool/IInvokeMemorypattern.nodes/src/nodes/guardrails/IInstance.py— newcheck()control-plane handler. Reports the verdict onparam.resultrather than enforcing it directly, since only the caller knows whether this is a pre- or post-call check. Stays silent forlogmode, matchingwriteQuestions/writeAnswers.packages/ai/src/ai/common/agent/_internal/host.py— bothAgentHostServices.ToolsandAgentHostServices.Memorydiscover attached guard nodes (getControllerNodeIds('guard')) and run them around everyinvoke()/put/get/list. Shared block/warn/log + fail-closed logic lives in one module-level_check_guard()rather than duplicated per channel. Validates the guard's result is actually a mapping before reading it, so a malformed result (list/string/None) fails closed instead of crashing withAttributeError.examples/slack-agent.pipe— the pipeline that originally surfaced this gap, now with a guard wired to the agent.nodes/src/nodes/guardrails/README.md— new section documenting the control-plane attachment mode, for both tools and memory.invoke_functionstubbed), added coverage forcheck()(pass/block/log-silent/warn-logs/no-engine/empty-text), and a newtest_host_guard.pycovering tool block-on-args, block-on-result-after-ran, pass-through, no-guard-attached, malformed/non-mapping guard results, and the equivalent memory put/get/list/clear cases.Test plan
pytest nodes/test/guardrails/test_all.py packages/ai/tests/ai/common/agent/_internal/test_host_guard.py packages/ai/tests/ai/common/test_config_shapes.py --noconftest— 133 passed (stub-based, no compiled engine required;--noconftestmatches the documented pattern already used bytest_config_shapes.pyfor engine-free files)ruff check/ruff format --checkclean on all touched files./builder build server/./builder test) — needs Xcode Command Line Tools on the dev machine this was built on; the C++ side (pipeline_config.cppcontrol validation,getControllerNodeIds) is unmodified and reuses the exact dispatch path the existingtool/llm/memorycontrol edges already use in production.Closes #1792
Summary by CodeRabbit