Skip to content

feat: add optional before_tool_call governance hook to OrchestratorAgent#601

Open
foundationx wants to merge 1 commit into
open-jarvis:mainfrom
foundationx:feat/before-tool-call-hook
Open

feat: add optional before_tool_call governance hook to OrchestratorAgent#601
foundationx wants to merge 1 commit into
open-jarvis:mainfrom
foundationx:feat/before-tool-call-hook

Conversation

@foundationx

Copy link
Copy Markdown

Adds an optional before_tool_call: Callable[[str, dict], bool] param to OrchestratorAgent. The hook fires before each tool executes across all three execution paths (sequential function-calling, parallel function-calling, and structured ReAct mode). Returning False injects a denial message instead of running the tool; returning True allows normal execution. Hook defaults to None -- no behaviour change for existing users.

Adds TestOrchestratorGovernanceHook to tests/agents/test_orchestrator.py with 6 tests covering every execution path and both allow/deny outcomes. 35/35 tests pass. Ruff lint clean; ruff format applied to changed files.

What does this PR do?

Adds an optional before_tool_call governance hook to OrchestratorAgent so callers can intercept and approve or deny tool calls before execution. The hook is wired into all three execution paths (sequential, parallel, and structured ReAct). Existing users see no behaviour change.

How was this tested?

Added TestOrchestratorGovernanceHook (6 tests) to tests/agents/test_orchestrator.py covering: no hook set, hook returns True, hook returns False, correct name/args delivered to hook, denial in parallel mode, denial in structured mode. All 35 tests pass.

Checklist

  • Tests pass (uv run pytest tests/ -v)
  • Linter passes (uv run ruff check src/ tests/)
  • Formatter passes (uv run ruff format --check src/ tests/)
  • New/changed public API has docstrings
  • Follows registry pattern (if adding new component)
  • Documentation updated (if applicable)

Adds an optional before_tool_call: Callable[[str, dict], bool] param
to OrchestratorAgent. The hook fires before each tool executes across
all three execution paths (sequential function-calling, parallel
function-calling, and structured ReAct mode). Returning False injects a
denial message instead of running the tool; returning True allows normal
execution. Hook defaults to None -- no behaviour change for existing users.

Adds TestOrchestratorGovernanceHook to tests/agents/test_orchestrator.py
with 6 tests covering every execution path and both allow/deny outcomes.
35/35 tests pass. Ruff lint clean; ruff format applied to changed files.
@ElliotSlusky

Copy link
Copy Markdown
Collaborator

Thanks for the PR. Are you trying to complete #472? This is directionally useful, and I’d like to keep it open, but I don’t think it should merge as-is.

A few things need tightening first:

  • If before_tool_call raises, the agent currently crashes. Since this is a governance hook, please catch hook errors and fail closed with a denied ToolResult.
  • Invalid JSON currently gets passed to the hook as {}, while the executor would reject it. Please preserve enough raw argument context for policy review, or run the hook after the same argument parsing behavior used by the executor.
  • In parallel tool mode, the hook runs from worker threads. Please either document that callbacks must be thread-safe or evaluate the hook before dispatching parallel work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants