feat: add pre-tool GuardrailProvider adapter#6432
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR adds guardrail request and decision types, a before-tool-call hook that evaluates a provider response, blocks denied tool calls, and handles provider errors. It also re-exports the new hook API and extends tests for registration, detached input, and failure handling. ChangesGuardrail hook API
Sequence Diagram(s)sequenceDiagram
participant ToolCallHookContext
participant enable_guardrail
participant GuardrailProvider
participant logger
ToolCallHookContext->>enable_guardrail: before-tool-call context
enable_guardrail->>enable_guardrail: deepcopy tool_input into GuardrailRequest
enable_guardrail->>GuardrailProvider: evaluate(request)
GuardrailProvider-->>enable_guardrail: GuardrailDecision or error
enable_guardrail->>logger: log exception or invalid result
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 review |
✅ Action performedReview finished.
|
|
Relationship to #6165 ( The two changes operate at different layers:
The first #6432 contract is intentionally binary: If #6165 lands first, this adapter can be rebased onto @maxpetrusenkoagent your view on this seam would be useful before either PR is finalized. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Good first boundary. I would keep this PR exactly this narrow. For this layer, the useful contract is:
I would avoid calling this a full authority receipt yet. This is the adapter boundary. The next layer can bind decision receipts, continuation or resume state, mode-compatible evidence, and executor-side verification, but those should stay separate from the first adapter PR. For merge, I would want the tests to prove:
That keeps the claim crisp: minimal provider boundary is in place; authority receipts and mode-compatible transition proof remain the next layer. |
|
Exact-head validation completed for Results:
The first test attempt had one matrix failure that triggered fail-fast cancellation of several jobs. Re-running only failed/cancelled jobs on the unchanged SHA completed successfully, so no code change was required. Fork validation run: https://github.com/safal207/crewAI-upstream/actions/runs/28592141628 Upstream workflows on this exact SHA still show |
|
@rpelevin thank you — agreed on keeping this as the adapter boundary rather than describing it as an authority receipt. I checked the current tests against your merge list. The existing suite already covered deep detachment, invalid result handling, non-boolean verdicts, fail-closed provider errors, and original tool name versus sanitized alias. The one missing explicit assertion was registry removal, so I added a focused regression proving that the hook returned by I also clarified the synchronous timing contract in the PR description: providers that perform I/O must enforce their own finite timeout and return promptly. The adapter does not claim safe preemption of arbitrary synchronous Python code. Current exact head: On that head, Lint, Type Checks (Python 3.10–3.13), CodeQL, PR Title, PR Size, and CodeRabbit are green. The full fork test matrix is running on the same SHA; completed shards are green so far. Receipts, continuation/resume, mode-compatible evidence, and executor-side verification remain explicitly outside this PR. |
|
This is a clean, minimal boundary — exactly narrow enough to compose with an external provider without the adapter needing to know anything about who's on the other end. Built a real one against it to make sure the shape actually works end-to-end, not just in theory: class InvinoveritasGuardrailProvider:
"""Backed by invinoveritas's /review — an independent, third-party pre-action
verdict (approve / approve_with_concerns / reject) with a signed, recomputable proof."""
def evaluate(self, request: GuardrailRequest) -> GuardrailDecision:
artifact = f"Tool call: {request.tool_name} Input: {dict(request.tool_input)}" \
f" Agent role: {request.agent_role}"
try:
resp = requests.post(ENDPOINT, headers={"Authorization": f"Bearer {api_key}"},
json={"artifact_type": "agent_output", "artifact": artifact},
timeout=8)
except requests.exceptions.RequestException:
return GuardrailDecision(allow=True, reason="unreachable — failing open") # never block on OUR availability
verdict = resp.json().get("verdict") if resp.status_code == 200 else None
if verdict == "reject":
return GuardrailDecision(allow=False, reason=resp.json()["summary"])
return GuardrailDecision(allow=True, reason=resp.json().get("summary") if verdict else None)Ran it live against a deliberately reckless synthetic tool call ( And confirmed the fail-open path independently (mocked a One open question for the design: |
|
@babyblueviper1 thank you — this is exactly the kind of independent integration proof this boundary was meant to enable. The live On the open question: I would keep
One important current limitation: So the intended sequence is:
Also, the provider-level Current exact head |
|
That resolves it cleanly, thanks — the split makes sense: this PR is the detached request/binary boundary, Good catch on the propagation gap too — worth being explicit about for anyone adapting this provider: right now Will hold here for the |
Summary
Add the minimal provider-agnostic
GuardrailProviderboundary proposed in #4877 on top of CrewAI's existing global before-tool-call hook registry.This first PR is intentionally binary and narrow: external providers receive a detached request and return allow/deny. Async suspension, continuation, decision receipts, and executor-side verification remain outside this change.
Changes
GuardrailRequestandGuardrailDecisionrecords;GuardrailProviderprotocol with onlyevaluate();enable_guardrail()as a thin adapter overregister_before_tool_call_hook();crewai.hooks;Minimal request boundary
GuardrailRequestcontains only:It deliberately does not duplicate task, crew, policy-engine identity, continuation, or governance-record fields.
Safety behavior
fail_closed=Falseis an explicit availability-over-enforcement opt-out;GuardrailProvider.evaluate()is synchronous; providers that perform I/O must enforce their own finite timeout and return promptly. The adapter does not claim it can safely preempt arbitrary synchronous Python code.Compatibility
unregister_before_tool_call_hook().Relationship to #6165
#6165 defines a richer runtime hook-result type (
PROCEED,NEEDS_REVIEW,SILENCE) and propagates it through execution paths. This PR defines the external provider boundary only.On current
main,GuardrailDecision(allow=True)maps toNoneandallow=Falsemaps toFalse. If #6165 lands first, this adapter can be rebased ontoToolCallDecisionwithout duplicating its execution-path changes.Out of scope
suspend/resolve()/ resume semantics;GovernanceDecision/GovernanceOutcomeduplication from feat: Add GovernanceDecision and GovernanceOutcome contract types #6030;Related to #4877.