feat(acp): enrich permission request cards
This commit is contained in:
@@ -23,11 +23,21 @@ _OPTION_ID_TO_HERMES = {
|
|||||||
"allow_session": "session",
|
"allow_session": "session",
|
||||||
"allow_always": "always",
|
"allow_always": "always",
|
||||||
"deny": "deny",
|
"deny": "deny",
|
||||||
|
"deny_always": "deny",
|
||||||
}
|
}
|
||||||
|
|
||||||
_PERMISSION_REQUEST_IDS = count(1)
|
_PERMISSION_REQUEST_IDS = count(1)
|
||||||
|
|
||||||
|
|
||||||
|
def _permission_option_supports_kind(kind: str) -> bool:
|
||||||
|
"""Return whether the installed ACP SDK accepts a permission option kind."""
|
||||||
|
try:
|
||||||
|
PermissionOption(option_id="__probe__", kind=kind, name="probe")
|
||||||
|
except Exception:
|
||||||
|
return False
|
||||||
|
return True
|
||||||
|
|
||||||
|
|
||||||
def _build_permission_options(*, allow_permanent: bool) -> list[PermissionOption]:
|
def _build_permission_options(*, allow_permanent: bool) -> list[PermissionOption]:
|
||||||
"""Return ACP options that match Hermes approval semantics."""
|
"""Return ACP options that match Hermes approval semantics."""
|
||||||
options = [
|
options = [
|
||||||
@@ -49,6 +59,14 @@ def _build_permission_options(*, allow_permanent: bool) -> list[PermissionOption
|
|||||||
),
|
),
|
||||||
)
|
)
|
||||||
options.append(PermissionOption(option_id="deny", kind="reject_once", name="Deny"))
|
options.append(PermissionOption(option_id="deny", kind="reject_once", name="Deny"))
|
||||||
|
if _permission_option_supports_kind("reject_always"):
|
||||||
|
options.append(
|
||||||
|
PermissionOption(
|
||||||
|
option_id="deny_always",
|
||||||
|
kind="reject_always",
|
||||||
|
name="Deny always",
|
||||||
|
),
|
||||||
|
)
|
||||||
return options
|
return options
|
||||||
|
|
||||||
|
|
||||||
@@ -62,12 +80,14 @@ def _build_permission_tool_call(command: str, description: str):
|
|||||||
import acp as _acp
|
import acp as _acp
|
||||||
|
|
||||||
tool_call_id = f"perm-check-{next(_PERMISSION_REQUEST_IDS)}"
|
tool_call_id = f"perm-check-{next(_PERMISSION_REQUEST_IDS)}"
|
||||||
|
title = f"{description}: {command}" if description else command
|
||||||
|
content_text = f"{description}\n$ {command}" if description else f"$ {command}"
|
||||||
return _acp.update_tool_call(
|
return _acp.update_tool_call(
|
||||||
tool_call_id,
|
tool_call_id,
|
||||||
title=description,
|
title=title,
|
||||||
kind="execute",
|
kind="execute",
|
||||||
status="pending",
|
status="pending",
|
||||||
content=[_acp.tool_content(_acp.text_block(f"$ {command}"))],
|
content=[_acp.tool_content(_acp.text_block(content_text))],
|
||||||
raw_input={"command": command, "description": description},
|
raw_input={"command": command, "description": description},
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
@@ -76,12 +76,22 @@ class TestApprovalBridge:
|
|||||||
assert tool_call.tool_call_id.startswith("perm-check-")
|
assert tool_call.tool_call_id.startswith("perm-check-")
|
||||||
assert tool_call.kind == "execute"
|
assert tool_call.kind == "execute"
|
||||||
assert tool_call.status == "pending"
|
assert tool_call.status == "pending"
|
||||||
assert tool_call.title == "dangerous command"
|
assert "dangerous command" in tool_call.title
|
||||||
|
assert "rm -rf /" in tool_call.title
|
||||||
|
content_text = tool_call.content[0].content.text
|
||||||
|
assert "$ rm -rf /" in content_text
|
||||||
|
assert "dangerous command" in content_text
|
||||||
assert tool_call.raw_input == {
|
assert tool_call.raw_input == {
|
||||||
"command": "rm -rf /",
|
"command": "rm -rf /",
|
||||||
"description": "dangerous command",
|
"description": "dangerous command",
|
||||||
}
|
}
|
||||||
assert option_ids == ["allow_once", "allow_session", "allow_always", "deny"]
|
assert option_ids == [
|
||||||
|
"allow_once",
|
||||||
|
"allow_session",
|
||||||
|
"allow_always",
|
||||||
|
"deny",
|
||||||
|
"deny_always",
|
||||||
|
]
|
||||||
|
|
||||||
def test_tool_call_ids_are_unique(self):
|
def test_tool_call_ids_are_unique(self):
|
||||||
_, first_kwargs, _, _, _ = _invoke_callback(
|
_, first_kwargs, _, _, _ = _invoke_callback(
|
||||||
@@ -103,7 +113,19 @@ class TestApprovalBridge:
|
|||||||
option_ids = [option.option_id for option in kwargs["options"]]
|
option_ids = [option.option_id for option in kwargs["options"]]
|
||||||
|
|
||||||
assert result == "session"
|
assert result == "session"
|
||||||
assert option_ids == ["allow_once", "allow_session", "deny"]
|
assert option_ids == ["allow_once", "allow_session", "deny", "deny_always"]
|
||||||
|
|
||||||
|
def test_reject_always_outcome_denies_without_changing_policy(self):
|
||||||
|
result, kwargs, _, _, _ = _invoke_callback(
|
||||||
|
AllowedOutcome(option_id="deny_always", outcome="selected"),
|
||||||
|
use_prompt_path=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
deny_always = [option for option in kwargs["options"] if option.option_id == "deny_always"]
|
||||||
|
|
||||||
|
assert result == "deny"
|
||||||
|
assert len(deny_always) == 1
|
||||||
|
assert deny_always[0].kind == "reject_always"
|
||||||
|
|
||||||
def test_allow_always_maps_correctly(self):
|
def test_allow_always_maps_correctly(self):
|
||||||
result, _, _, _, _ = _invoke_callback(
|
result, _, _, _, _ = _invoke_callback(
|
||||||
|
|||||||
Reference in New Issue
Block a user