feat: Add GovernanceDecision and GovernanceOutcome contract types#6030
feat: Add GovernanceDecision and GovernanceOutcome contract types#6030nagasatish007 wants to merge 9 commits into
Conversation
This module defines the GovernanceDecision and GovernanceOutcome TypedDicts for vendor-neutral governance hooks in CrewAI. It specifies the structure and fields for pre-execution and post-execution records used in governance processes.
This file contains contract tests for GovernanceDecision and GovernanceOutcome, validating decision routes, JSON serialization, and outcome references.
|
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)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds vendor-neutral governance TypedDict contracts: pre-execution GovernanceDecision and post-execution GovernanceOutcome, exposes them from the package, and adds comprehensive pytest fixtures/tests that validate fields and JSON round-trip preservation of extensions. ChangesGovernance Decision and Outcome Contracts
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
lib/crewai/src/crewai/governance/__init__.py (1)
1-2: ⚡ Quick winExport types from
__init__.pyfor better ergonomics.The governance module's
__init__.pyis empty, requiring users to import from the full submodule path. Exporting the public types would make imports more convenient and discoverable.📦 Proposed export pattern
+""" +CrewAI governance contracts. +""" + +from crewai.governance.governance_decision import ( + GovernanceDecision, + GovernanceOutcome, +) + +__all__ = ["GovernanceDecision", "GovernanceOutcome"]This allows cleaner imports:
from crewai.governance import GovernanceDecision, GovernanceOutcomeinstead of:
from crewai.governance.governance_decision import GovernanceDecision, GovernanceOutcome🤖 Prompt for 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. In `@lib/crewai/src/crewai/governance/__init__.py` around lines 1 - 2, Add public exports to the governance package by importing and re-exporting the key types in the package __init__.py: import GovernanceDecision and GovernanceOutcome (and any other public classes) from their defining module (e.g., governance_decision) and add them to the package's __all__ so users can do "from crewai.governance import GovernanceDecision, GovernanceOutcome"; ensure you reference the exact class names GovernanceDecision and GovernanceOutcome and any other public symbols you want exposed.test_governance_decision_contract.py (1)
98-103: ⚡ Quick winConsider adding error outcome fixture for completeness.
The current outcome fixture only tests
outcome="executed". SinceGovernanceOutcomedefineserror_typeanderror_messagefields and includes"error"as a valid outcome, adding a fixture that demonstrates the error case would strengthen contract coverage.🧪 Proposed error outcome fixture
FIXTURE_OUTCOME_ERROR: GovernanceOutcome = { "decision_id": "d-002", # Link to FIXTURE_DENY or any decision "outcome": "error", "error_type": "PermissionError", "error_message": "Insufficient permissions to execute tool", "completed_at": "2026-06-03T14:01:05Z", }Then add to
test_all_fixtures_json_serializable:fixtures: list[dict[str, Any]] = [ FIXTURE_ALLOW, FIXTURE_DENY, FIXTURE_REQUIRE_APPROVAL, FIXTURE_ALLOW_WITH_EXTENSION, FIXTURE_REVISE, FIXTURE_OUTCOME, + FIXTURE_OUTCOME_ERROR, FIXTURE_UNKNOWN_EXTENSION, ]🤖 Prompt for 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. In `@test_governance_decision_contract.py` around lines 98 - 103, Add an error-case fixture for GovernanceOutcome named FIXTURE_OUTCOME_ERROR that sets "outcome" to "error" and includes "error_type" and "error_message" plus "decision_id" and "completed_at" to mirror FIXTURE_OUTCOME structure; then include FIXTURE_OUTCOME_ERROR in the test_all_fixtures_json_serializable assertions so the error-path (outcome="error") is covered by serialization tests (reference symbols: FIXTURE_OUTCOME, FIXTURE_OUTCOME_ERROR, GovernanceOutcome, test_all_fixtures_json_serializable).
🤖 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.
Nitpick comments:
In `@lib/crewai/src/crewai/governance/__init__.py`:
- Around line 1-2: Add public exports to the governance package by importing and
re-exporting the key types in the package __init__.py: import GovernanceDecision
and GovernanceOutcome (and any other public classes) from their defining module
(e.g., governance_decision) and add them to the package's __all__ so users can
do "from crewai.governance import GovernanceDecision, GovernanceOutcome"; ensure
you reference the exact class names GovernanceDecision and GovernanceOutcome and
any other public symbols you want exposed.
In `@test_governance_decision_contract.py`:
- Around line 98-103: Add an error-case fixture for GovernanceOutcome named
FIXTURE_OUTCOME_ERROR that sets "outcome" to "error" and includes "error_type"
and "error_message" plus "decision_id" and "completed_at" to mirror
FIXTURE_OUTCOME structure; then include FIXTURE_OUTCOME_ERROR in the
test_all_fixtures_json_serializable assertions so the error-path
(outcome="error") is covered by serialization tests (reference symbols:
FIXTURE_OUTCOME, FIXTURE_OUTCOME_ERROR, GovernanceOutcome,
test_all_fixtures_json_serializable).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 00f632c3-6eb2-4dd0-89e0-6bb603ca4224
📒 Files selected for processing (3)
governance_decision.pylib/crewai/src/crewai/governance/__init__.pytest_governance_decision_contract.py
|
Thanks, Naga. Directionally this is the right contract shape. One mechanical thing I would fix before merge: I would also move the contract test into CrewAI's test tree, e.g. Field names and boundary look right to me: Optional but useful: add one |
This file contains contract tests for GovernanceDecision and GovernanceOutcome, validating decision routes, JSON serialization, and outcome references.
This module defines the GovernanceDecision and GovernanceOutcome types for vendor-neutral governance hooks in CrewAI, including their fields and documentation.
Added test for error outcomes to validate error_type and error_message fields.
|
@rpelevin the changes have been done. |
|
Thanks, Naga. I went through the latest commits. This addresses the core things I called out: the contract now lives under crewai.governance, the types are exported from the package, GovernanceDecision and GovernanceOutcome stay separate with decision_id linking them, and the extension round-trip plus error-outcome coverage are now in the tests. From my side the field names and vendor-neutral boundary look right. I would avoid adding more scope here and let the CrewAI maintainers decide the final test layout and merge mechanics. Great work getting this tightened up. |
2 participants
Summary
Adds a vendor-neutral
GovernanceDecisionTypedDict contract that crew-level governance hooks (before_tool_call/after_tool_callfrom #5890) can optionally return. Also addsGovernanceOutcomeas the linked post-execution record.This gives the hook one return shape that works for blocking, approval, resume, and audit — without coupling CrewAI to any vendor's evidence format.
Closes #5888 (governance decision contract portion)
Design Principles
extensions: dict[str, Any]is never validated by CrewAI. Vendors attach their evidence (signed receipts, Merkle proofs, etc.) under their own namespace keyGovernanceDecisionis pre-execution,GovernanceOutcomeis post-execution, linked viadecision_idFiles
lib/crewai/src/crewai/governance/governance_decision.pyGovernanceDecision+GovernanceOutcomeTypedDictslib/crewai/tests/governance/test_governance_decision_contract.pyContract Fields (GovernanceDecision)
Test Fixtures
allowdenyrequire_approvalexpires_atreviserevalidate_ifallowextensions["teec"]allowexecuteddecision_idExtension Round-Trip Test
The key test: an unknown vendor extension (
extensions["custom_vendor"]) with nested dicts, arrays, and unicode passes through JSON serialize → deserialize without any data loss or validation failure. This proves CrewAI doesn't filter, validate, or modify vendor evidence.Backward Compatibility
before_tool_call/after_tool_callbehaviorGovernanceDecisionfor richer audit/resume semanticsNoneorTrue, behavior is identical to todayCo-authored with
Co-authored-by: rpelevin rpelevin@users.noreply.github.com
References
before_tool_call/after_tool_callhooks (this PR extends)Summary by CodeRabbit
New Features
Tests