Skip to content

Commit 26a0ba0

Browse files
committed
fix(contrib): reject explicit empty permission options
1 parent 9d07d78 commit 26a0ba0

3 files changed

Lines changed: 61 additions & 3 deletions

File tree

‎docs/contrib.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ The helpers under `acp.contrib` package recurring patterns we saw in integration
2424

2525
- `ToolCallTracker.start()/progress()/append_stream_text()` emits canonical `ToolCallStart` / `ToolCallProgress` updates and keeps an in-memory view via `view()` or `tool_call_model()`.
2626
- `PermissionBroker.request_for()` wraps `requestPermission` RPCs. It reuses tracker state (or a provided `ToolCall`), lets you append extra content, and defaults to Approve / Approve for session / Reject options.
27+
- Omit `options` or pass `None` to use broker defaults. An explicit empty option list raises `MissingPermissionOptionsError` before the permission request is sent; the same applies to empty `default_options` when no per-request options are supplied.
2728
- `default_permission_options()` exposes that canonical option triple if you need to customise it.
2829

2930
> Tip: Keep one tracker near the agent event loop. Emit notifications through it and share the tracker with `PermissionBroker` so permission prompts always match the latest tool call state.

‎src/acp/contrib/permissions.py‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ def __init__(
5656
self._requester = requester
5757
self._tracker = tracker
5858
self._default_options = tuple(
59-
option.model_copy(deep=True) for option in (default_options or default_permission_options())
59+
option.model_copy(deep=True)
60+
for option in (default_permission_options() if default_options is None else default_options)
6061
)
6162

6263
async def request_for(
@@ -84,7 +85,9 @@ async def request_for(
8485
existing.append(ContentToolCallContent(content=TextContentBlock(text=description)))
8586
tool_call.content = existing
8687

87-
option_set = tuple(option.model_copy(deep=True) for option in (options or self._default_options))
88+
option_set = tuple(
89+
option.model_copy(deep=True) for option in (self._default_options if options is None else options)
90+
)
8891
if not option_set:
8992
raise MissingPermissionOptionsError()
9093

‎tests/contrib/test_contrib_permissions.py‎

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import pytest
44

5-
from acp.contrib.permissions import PermissionBroker, default_permission_options
5+
from acp.contrib.permissions import MissingPermissionOptionsError, PermissionBroker, default_permission_options
66
from acp.contrib.tool_calls import ToolCallTracker
77
from acp.schema import (
88
AllowedOutcome,
@@ -58,6 +58,60 @@ async def requester(request: RequestPermissionRequest):
5858
assert recorded == ["allow"]
5959

6060

61+
@pytest.mark.asyncio
62+
async def test_permission_broker_none_uses_standard_options():
63+
tracker = ToolCallTracker(id_factory=lambda: "standard")
64+
tracker.start("external", title="Standard options")
65+
recorded: list[list[str]] = []
66+
67+
async def requester(request: RequestPermissionRequest):
68+
recorded.append([option.option_id for option in request.options])
69+
return RequestPermissionResponse(outcome=AllowedOutcome(option_id="approve", outcome="selected"))
70+
71+
broker = PermissionBroker("session", requester, tracker=tracker, default_options=None)
72+
await broker.request_for("external", options=None)
73+
assert recorded == [["approve", "approve_for_session", "reject"]]
74+
75+
76+
@pytest.mark.asyncio
77+
async def test_permission_broker_custom_default_and_override():
78+
tracker = ToolCallTracker(id_factory=lambda: "reject-only")
79+
tracker.start("external", title="Reject only")
80+
reject = PermissionOption(option_id="reject", name="Reject", kind="reject_once")
81+
allow = PermissionOption(option_id="allow", name="Allow", kind="allow_once")
82+
recorded: list[list[str]] = []
83+
84+
async def requester(request: RequestPermissionRequest):
85+
recorded.append([option.option_id for option in request.options])
86+
return RequestPermissionResponse(
87+
outcome=AllowedOutcome(option_id=request.options[0].option_id, outcome="selected")
88+
)
89+
90+
broker = PermissionBroker("session", requester, tracker=tracker, default_options=[reject])
91+
await broker.request_for("external")
92+
await broker.request_for("external", options=[allow])
93+
assert recorded == [["reject"], ["allow"]]
94+
95+
96+
@pytest.mark.asyncio
97+
@pytest.mark.parametrize(("default_options", "options"), [(None, []), ([], None)])
98+
async def test_permission_broker_rejects_empty_option_sources(default_options, options):
99+
tracker = ToolCallTracker(id_factory=lambda: "empty")
100+
tracker.start("external", title="No options")
101+
recorded: list[RequestPermissionRequest] = []
102+
103+
async def requester(request: RequestPermissionRequest):
104+
recorded.append(request)
105+
return RequestPermissionResponse(
106+
outcome=AllowedOutcome(option_id=request.options[0].option_id, outcome="selected")
107+
)
108+
109+
broker = PermissionBroker("session", requester, tracker=tracker, default_options=default_options)
110+
with pytest.raises(MissingPermissionOptionsError, match="requires at least one permission option"):
111+
await broker.request_for("external", options=options)
112+
assert recorded == []
113+
114+
61115
def test_default_permission_options_shape():
62116
options = default_permission_options()
63117
assert len(options) == 3

0 commit comments

Comments
 (0)