-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Draft pull: feature request_human_approval [work in progress] #6875
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
1184567
c6de690
a0ca65d
ccc873c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -181,6 +181,10 @@ def _serialize_result_schema( | |
| default=False, | ||
| description="Flag to check if the tool should be the final agent answer.", | ||
| ) | ||
| requires_human_approval: bool = Field( | ||
| default=False, | ||
| description="Flag to check if tool execution requires explicit human approval before running.", | ||
| ) | ||
|
Comment on lines
+185
to
+188
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win Preserve
🤖 Prompt for AI Agents |
||
| max_usage_count: int | None = Field( | ||
| default=None, | ||
| description="Maximum number of times this tool can be used. None means unlimited usage.", | ||
|
|
@@ -416,6 +420,7 @@ def to_structured_tool(self) -> CrewStructuredTool: | |
| current_usage_count=self.current_usage_count, | ||
| cache_function=self.cache_function, | ||
| tool_failure_policy=self.tool_failure_policy, | ||
| requires_human_approval=self.requires_human_approval, | ||
| ) | ||
| structured_tool._original_tool = self | ||
| return structured_tool | ||
|
|
@@ -686,6 +691,7 @@ def tool( | |
| result_schema: type[BaseModel] | None = ..., | ||
| result_as_answer: bool = ..., | ||
| max_usage_count: int | None = ..., | ||
| requires_human_approval: bool = ..., | ||
| ) -> Callable[[Callable[P2, R2]], Tool[P2, R2]]: ... | ||
|
|
||
|
|
||
|
|
@@ -695,6 +701,7 @@ def tool( | |
| result_schema: type[BaseModel] | None = ..., | ||
| result_as_answer: bool = ..., | ||
| max_usage_count: int | None = ..., | ||
| requires_human_approval: bool = ..., | ||
| ) -> Callable[[Callable[P2, R2]], Tool[P2, R2]]: ... | ||
|
|
||
|
|
||
|
|
@@ -703,6 +710,7 @@ def tool( | |
| result_schema: type[BaseModel] | None = None, | ||
| result_as_answer: bool = False, | ||
| max_usage_count: int | None = None, | ||
| requires_human_approval: bool = False, | ||
| ) -> Tool[P2, R2] | Callable[[Callable[P2, R2]], Tool[P2, R2]]: | ||
| """Decorator to create a Tool from a function. | ||
|
|
||
|
|
@@ -769,6 +777,7 @@ def _make_tool(f: Callable[P2, R2]) -> Tool[P2, R2]: | |
| result_as_answer=result_as_answer, | ||
| max_usage_count=max_usage_count, | ||
| current_usage_count=0, | ||
| requires_human_approval=requires_human_approval, | ||
| ) | ||
|
|
||
| return _make_tool | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -792,3 +792,99 @@ def test_prompt_rendering_still_uses_composite(self): | |
| assert "Tool Name: get_temperature" in rendered | ||
| assert "Tool Arguments:" in rendered | ||
| assert f"Tool Description: {self.AUTHORED}" in rendered | ||
| def test_tool_requires_human_approval_type_validation(): | ||
| """Verify that requires_human_approval strictly enforces boolean types | ||
| and handles valid/invalid type conversions correctly.""" | ||
|
|
||
| class StringFlagTool(BaseTool): | ||
| name: str = "Validation Tool" | ||
| description: str = "Test tool" | ||
| requires_human_approval: bool = True | ||
|
|
||
| def _run(self, val: str) -> str: | ||
| return val | ||
|
|
||
| tool_inst = StringFlagTool() | ||
| assert tool_inst.requires_human_approval is True | ||
| assert isinstance(tool_inst.requires_human_approval, bool) | ||
|
|
||
|
|
||
| # 2. CRITICAL: Dynamic Mutability | ||
| def test_tool_requires_human_approval_dynamic_toggle(): | ||
| """Verify that human approval can be dynamically toggled at runtime | ||
| (e.g., toggled on/off based on context or user permissions).""" | ||
|
|
||
| @tool("Dynamic Approval Tool") | ||
| def action_tool(data: str) -> str: | ||
| """Dynamic tool.""" | ||
| return data | ||
|
|
||
| # Starts as default False | ||
| assert action_tool.requires_human_approval is False | ||
|
|
||
| # Enable dynamically | ||
| action_tool.requires_human_approval = True | ||
| assert action_tool.requires_human_approval is True | ||
|
|
||
| # Disable dynamically | ||
| action_tool.requires_human_approval = False | ||
| assert action_tool.requires_human_approval is False | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Cover the configured decorator option. This test changes the field after decoration. It does not test propagation from Add a case that supplies the decorator option and asserts As per coding guidelines, write unit tests for new functionality that focus on behavior. 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
|
|
||
|
|
||
| # 3. CRITICAL: Schema & Serialization | ||
| def test_tool_pydantic_schema_and_serialization(): | ||
| """Verify that requiring human approval doesn't corrupt Pydantic serialization | ||
| or the schema exported to LLMs.""" | ||
|
|
||
| class CriticalDataTool(BaseTool): | ||
| name: str = "Critical Data Tool" | ||
| description: str = "Processes sensitive payload" | ||
| requires_human_approval: bool = True | ||
|
|
||
| def _run(self, payload: str) -> str: | ||
| return payload | ||
|
|
||
| t = CriticalDataTool() | ||
| serialized = t.model_dump() | ||
|
|
||
| # Ensure field is present in model dump for Agent inspectability | ||
| assert "requires_human_approval" in serialized | ||
| assert serialized["requires_human_approval"] is True | ||
|
|
||
|
|
||
| # 4. CRITICAL: Decorator Metadata Preservation | ||
| def test_tool_decorator_preserves_approval_state_on_copy_or_args(): | ||
| """Verify that the @tool decorator preserves requires_human_approval state | ||
| even when additional tool attributes or docstrings are modified.""" | ||
|
|
||
| @tool("Sensitivty Test Tool") | ||
| def sensitive_fn(val: int) -> int: | ||
| """Processes integer val.""" | ||
| return val * 2 | ||
|
|
||
| sensitive_fn.requires_human_approval = True | ||
|
|
||
| # Check function introspection attributes remain intact | ||
| assert sensitive_fn.name == "Sensitivty Test Tool" | ||
| assert sensitive_fn.requires_human_approval is True | ||
| assert callable(sensitive_fn._run) | ||
|
|
||
|
|
||
| # 5. CRITICAL: Tool Execution Behavior Contract | ||
| def test_tool_execution_preserves_approval_flag_during_run(): | ||
| """Verify that calling the tool's execution method directly does not reset | ||
| or mutate the requires_human_approval flag.""" | ||
|
|
||
| @tool("Execution Test Tool") | ||
| def execute_tool(data: str) -> str: | ||
| """Executes data operation.""" | ||
| return f"Executed {data}" | ||
|
|
||
| execute_tool.requires_human_approval = True | ||
|
|
||
| # Run the tool | ||
| result = execute_tool.run(data="test_input") | ||
|
|
||
| # Assert execution result and flag immutability after run | ||
| assert "Executed test_input" in result | ||
| assert execute_tool.requires_human_approval is True | ||
Uh oh!
There was an error while loading. Please reload this page.