feat(plugins): pass approve rule keys to approval gate - #59163
Conversation
|
This is good for CLI. Is there a follow up somewhere for same for desktop and gateways? |
|
Good call. I do not see a separate follow-up filed yet. This PR is intentionally keeping the change at the shared plugin/runtime approval boundary: I would keep desktop-specific UX/config parity as a follow-up rather than widen this PR: verify the desktop/gateway approval surfaces display and persist the plugin-provided rule key cleanly, and add coverage around that surface if it has a separate approval path. |
|
Follow-up opened for the Desktop/TUI/gateway parity piece: #59298. It keeps the approval allowlist grain visible across gateway clients by emitting |
67f4a04 to
55bf4ac
Compare
|
Rebased this on current main and re-ran focused plugin/runtime approval verification. Verified locally:
|
|
Merged via #60504 - your commit was cherry-picked onto current main with your authorship preserved, plus a follow-up wiring the gateway notify round-trip into _run_approval_gate so plugin-escalated approvals actually send the interactive prompt on Discord/Telegram/Slack (#59413). Credit also to @kshitijk4poor for the original #58698 design this re-lands. Thanks! |
Summary
Restores the
pre_tool_callapprove escalation lane from #58698 on top of currentmainafter the revert in #59131, and fixes #59067 by preserving plugin-supplied approval rule keys.Plugins can now return an approve directive with an optional
rule_key, for example:resolve_pre_tool_block()passes that validated non-empty string through torequest_tool_approval(). Missing, empty, whitespace-only, or non-stringrule_keyvalues fall back to the priorrule_key=tool_namebehavior.Why
Before this fix, all approve rules for a given tool shared one allowlist key because
resolve_pre_tool_block()hardcodedrule_key=tool_name. An[a]lwaysapproval for onewrite_filerule could therefore auto-approve unrelatedwrite_filerules. This PR lets plugins define per-rule allowlist granularity while keeping backward compatibility.Fixes #59067.
Validation
python3 -m pytest tests/hermes_cli/test_plugins.py -qfailed because the system Python lacks pytest.uv run --extra dev pytest tests/hermes_cli/test_plugins.py -q-> 116 passed.uv run --extra dev pytest tests/tools/test_request_tool_approval.py -q-> 13 passed.uv run --extra dev pytest tests/run_agent/test_tool_call_guardrail_runtime.py -q-> 9 passed.uv run --extra dev pytest tests/run_agent/test_run_agent.py::TestConcurrentToolExecution -q-> 39 passed.uv run --extra dev pytest tests/test_model_tools.py -q-> 31 passed.uv run --extra dev ruff check agent/agent_runtime_helpers.py agent/tool_executor.py hermes_cli/plugins.py model_tools.py tests/hermes_cli/test_plugins.py tests/run_agent/test_run_agent.py tests/run_agent/test_tool_call_guardrail_runtime.py tests/tools/test_request_tool_approval.py tools/approval.py-> passed.