feat(lifecycle): add on_tool_authorize hook for per-tool authorization (#2868)#2
feat(lifecycle): add on_tool_authorize hook for per-tool authorization (#2868)#2adityasingh2400 wants to merge 2 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughAdded an async lifecycle hook Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ToolExecution
participant RunHooks
participant AgentHooks
participant Tool
Client->>ToolExecution: request tool execution
ToolExecution->>RunHooks: on_tool_authorize(context, agent, tool)
alt run denied
RunHooks-->>ToolExecution: False
ToolExecution-->>Client: "Tool call denied: authorization hook returned False."
else run allowed
RunHooks-->>ToolExecution: True
ToolExecution->>AgentHooks: on_tool_authorize(context, agent, tool)
alt agent denied
AgentHooks-->>ToolExecution: False
ToolExecution-->>Client: "Tool call denied: authorization hook returned False."
else agent allowed
AgentHooks-->>ToolExecution: True
ToolExecution->>RunHooks: on_tool_start(...)
RunHooks-->>ToolExecution: started
ToolExecution->>Tool: invoke(...)
Tool-->>ToolExecution: result
ToolExecution->>RunHooks: on_tool_end(...)
RunHooks-->>ToolExecution: ended
ToolExecution-->>Client: tool result
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/test_tool_authorize_hook.py`:
- Around line 97-123: The test never wires my_tool_impl to func_tool nor asserts
on invoked, so it doesn't prove the tool was skipped; update the setup to create
func_tool using the actual implementation (e.g., attach my_tool_impl to the
function tool created by get_function_tool or use a builder that accepts the
implementation), ensure Runner uses that func_tool, and add an assertion like
assert invoked == [] after Runner.run to verify the implementation was not
called; reference my_tool_impl, invoked, func_tool, get_function_tool,
Runner.run, and hooks to locate and modify the test.
- Around line 126-150: The test test_deny_hook_sends_denial_message_to_model
should explicitly assert that the denial payload (DENIAL_MSG) was forwarded to
the model when OutputCapturingHooks.on_tool_authorize returns False; update the
assertions after Runner.run to inspect result.raw_responses (or the model's
captured inputs) and assert that the second turn's input includes a tool
output/text equal to DENIAL_MSG, in addition to keeping the final_output ==
"done".
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0792dcde-b55c-4824-a61f-fbe9eb2a0c71
📒 Files selected for processing (3)
src/agents/lifecycle.pysrc/agents/run_internal/tool_execution.pytests/test_tool_authorize_hook.py
- Wire my_tool_impl directly into FunctionTool so test_deny_hook_skips_tool_execution actually verifies tool impl was not called; add assert invoked == [] - Add explicit DENIAL_MSG assertion in test_deny_hook_sends_denial_message_to_model by checking new_items for ToolCallOutputItem with denial string - Add type annotation for invoked: list[bool] - Add missing docstrings to all hook classes and methods
openai#2868) Adds on_tool_authorize to both RunHooksBase and AgentHooksBase. The hook fires before each tool execution and can return False to block the call. When denied: - The tool function is never invoked - on_tool_start and on_tool_end are skipped for that call - The model receives 'Tool call denied: authorization hook returned False.' as the tool output so it can react gracefully Both run-level and agent-level hooks are checked; either can deny the call. The default implementation returns True (allow all), so this is fully backwards-compatible. Closes openai#2868
- Wire my_tool_impl directly into FunctionTool so test_deny_hook_skips_tool_execution actually verifies tool impl was not called; add assert invoked == [] - Add explicit DENIAL_MSG assertion in test_deny_hook_sends_denial_message_to_model by checking new_items for ToolCallOutputItem with denial string - Add type annotation for invoked: list[bool] - Add missing docstrings to all hook classes and methods
529993b to
8479139
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/agents/run_internal/tool_execution.py (1)
1605-1605: Extract denial text into a shared constant.Line 1605 hardcodes a user-visible message. Consider centralizing it as a module-level constant to avoid drift across execution logic/tests/docs.
♻️ Proposed refactor
@@ REDACTED_TOOL_ERROR_MESSAGE = "Tool execution failed. Error details are redacted." +TOOL_AUTHORIZATION_DENIED_MESSAGE = "Tool call denied: authorization hook returned False." @@ - if not run_authorized or not agent_authorized: - return "Tool call denied: authorization hook returned False." + if not run_authorized or not agent_authorized: + return TOOL_AUTHORIZATION_DENIED_MESSAGE🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/agents/run_internal/tool_execution.py` at line 1605, Introduce a module-level constant (e.g., TOOL_CALL_DENIED_MSG = "Tool call denied: authorization hook returned False.") at the top of src/agents/run_internal/tool_execution.py and replace the hardcoded return string inside the function that currently returns "Tool call denied: authorization hook returned False." with that constant; update any other occurrences to use the same constant and run tests to ensure no string-drift issues in execution logic or tests/docs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/agents/run_internal/tool_execution.py`:
- Line 1605: Introduce a module-level constant (e.g., TOOL_CALL_DENIED_MSG =
"Tool call denied: authorization hook returned False.") at the top of
src/agents/run_internal/tool_execution.py and replace the hardcoded return
string inside the function that currently returns "Tool call denied:
authorization hook returned False." with that constant; update any other
occurrences to use the same constant and run tests to ensure no string-drift
issues in execution logic or tests/docs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f15acc6e-ca20-4cdf-9a93-5f40a1505e5e
📒 Files selected for processing (3)
src/agents/lifecycle.pysrc/agents/run_internal/tool_execution.pytests/test_tool_authorize_hook.py
✅ Files skipped from review due to trivial changes (1)
- tests/test_tool_authorize_hook.py
Summary
There's currently no way to programmatically block tool execution at the hook layer — you can observe calls via
on_tool_start, but you can't prevent them. This PR addson_tool_authorizeto bothRunHooksBaseandAgentHooksBaseso callers can intercept tool calls before they happen.Closes openai#2868
What changed
on_tool_authorize(context, agent, tool) -> booltoRunHooksBaseandAgentHooksBaseinlifecycle.py_execute_single_tool_bodyinsidetool_execution.py, between input guardrails andon_tool_startTrue— fully backwards compatibleBehavior when denied
on_tool_startandon_tool_endare skipped for that call"Tool call denied: authorization hook returned False."as the tool resultUsage example
Tests
Added
tests/test_tool_authorize_hook.pycovering:on_tool_start/on_tool_endskippedSummary by CodeRabbit
New Features
Tests