fix(acp): close the permission bubble per the decision actually taken
The closer in await_permission() only recognised an allow when the response outcome was the SDK AllowedOutcome class, while the edit-approval requester duck-types (outcome == "selected"). A client answering with a plain selected outcome therefore had its edit applied but the edit-approval-N bubble closed "failed". Duck-type the closer on the wire discriminator so its terminal status always matches the decision. Live-pass side-effect on this PR.
This commit is contained in:
@@ -103,9 +103,12 @@ def await_permission(
|
||||
if send_update is not None:
|
||||
import acp as _acp
|
||||
|
||||
# Duck-typed like the callers' own allow checks (``outcome == "selected"`` is the wire
|
||||
# discriminator), so the bubble's terminal status always matches the decision taken.
|
||||
outcome = getattr(response, "outcome", None)
|
||||
allowed = isinstance(outcome, AllowedOutcome) and any(
|
||||
option.option_id == outcome.option_id and option.kind.startswith("allow") for option in options
|
||||
allowed = getattr(outcome, "outcome", None) == "selected" and any(
|
||||
option.option_id == getattr(outcome, "option_id", None) and option.kind.startswith("allow")
|
||||
for option in options
|
||||
)
|
||||
send_update(_acp.update_tool_call(tool_call.tool_call_id, status="completed" if allowed else "failed"))
|
||||
return response, timed_out
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
import asyncio
|
||||
import inspect
|
||||
from concurrent.futures import Future
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
@@ -240,3 +241,24 @@ class TestPermissionRequestToolCallReachesATerminalStatus:
|
||||
make_acp_edit_approval_requester, DeniedOutcome(outcome="cancelled"), lambda cb: cb(proposal),
|
||||
)
|
||||
assert [(u.tool_call_id, u.status) for u in sent] == [(requested.tool_call_id, "failed")]
|
||||
|
||||
def test_allowed_edit_approval_request_is_closed_as_completed_once(self):
|
||||
"""Live regression: a client answering with a plain ``selected`` outcome (not the SDK
|
||||
``AllowedOutcome`` class) had the edit applied but the bubble closed ``failed``."""
|
||||
from acp_adapter.edit_approval import EditProposal, make_acp_edit_approval_requester
|
||||
|
||||
proposal = EditProposal(tool_name="write_file", path="/tmp/x", old_text="", new_text="y", arguments={})
|
||||
decisions = []
|
||||
response = SimpleNamespace(outcome=SimpleNamespace(outcome="selected", option_id="allow_once"))
|
||||
request_permission = AsyncMock(name="request_permission")
|
||||
future = MagicMock(spec=Future)
|
||||
future.result.return_value = response
|
||||
sent = []
|
||||
with patch("agent.async_utils.asyncio.run_coroutine_threadsafe", return_value=future):
|
||||
requester = make_acp_edit_approval_requester(
|
||||
request_permission, MagicMock(spec=asyncio.AbstractEventLoop), "s1", send_update=sent.append,
|
||||
)
|
||||
decisions.append(requester(proposal))
|
||||
requested = request_permission.call_args.kwargs["tool_call"]
|
||||
assert decisions == [True]
|
||||
assert [(u.tool_call_id, u.status) for u in sent] == [(requested.tool_call_id, "completed")]
|
||||
|
||||
Reference in New Issue
Block a user