Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/acp.md
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,11 @@ negotiate RPC protocol v2 to preserve structured text/image tool results.
call stops only that caller's wait. Until the retirement ends, turns already
waiting for the room and turns that arrive during it are cancelled rather than
starting a new process. Once cleanup succeeds, a later turn can start normally.
- **Activity.** With `Emit.TOOL_CALLS`, overlapping native tools show the most
recently started still-open call; only the last completion returns to reasoning.
Manual permission waits are reported regardless of narration and remain active
until every resolver for that turn completes, fails or is cancelled. Dialogs
run independently, and rooms and turns do not share permission-wait ownership.
- **Permissions.** `approval_mode="yolo"` bypasses native approval checks without
changing Band platform permissions. Otherwise `resolve_permission` receives
`OmpPermissionRequest(room_id, session_id, title, options)` and returns an
Expand Down
35 changes: 26 additions & 9 deletions src/band/adapters/omp.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@
OmpUIRequest,
OmpUIResponse,
)
from band.runtime.activity import set_permission_wait
from band.runtime.custom_tools import (
CustomToolDef,
custom_tools_to_schemas,
Expand All @@ -115,6 +116,7 @@
TURN_COMPLETION_GUIDANCE,
render_system_prompt,
)
from band.runtime.toolactivity import end_tool_activity, report_tool_activity
from band.runtime.tools import (
BAND_MCP_SERVER_NAME,
BandTool,
Expand Down Expand Up @@ -348,6 +350,7 @@ class OmpTurn:
calls: dict[str, tuple[str, dict[str, JsonValue]]] = field(default_factory=dict)
closing_text: list[str] = field(default_factory=list)
closed: bool = False
pending_permission_waits: int = 0


@dataclass
Expand Down Expand Up @@ -899,7 +902,7 @@ async def on_ui(request: OmpUIRequest) -> OmpUIResponse:
if request.method == OmpUIMethod.ASK:
return await self._ask_room(request, msg, tools, room_id)
return await self._permission(
request, room_id, room.session_id, tools
request, room_id, room.session_id, turn
)

await room.runtime.prompt(
Expand Down Expand Up @@ -1164,22 +1167,29 @@ async def _permission(
request: OmpUIRequest,
room_id: str,
session_id: str,
tools: AgentToolsProtocol,
turn: OmpTurn,
) -> OmpUIResponse:
if request.method != OmpUIMethod.SELECT:
logger.warning("OMP cancelled unsupported UI method %s", request.method)
return OmpUIResponse(cancelled=True)
tools = turn.tools
options = tuple(request.options or ())
choice = None
if self._resolve_permission is not None:
choice = await self._resolve_permission(
OmpPermissionRequest(
room_id=room_id,
session_id=session_id,
title=request.title or "",
options=options,
turn.pending_permission_waits += 1
try:
await set_permission_wait(tools, True)
choice = await self._resolve_permission(
OmpPermissionRequest(
room_id=room_id,
session_id=session_id,
title=request.title or "",
options=options,
)
)
)
finally:
turn.pending_permission_waits -= 1
await set_permission_wait(tools, bool(turn.pending_permission_waits))
elif options == (OMP_FORM_APPROVE, OMP_FORM_DENY):
choice = OMP_FORM_APPROVE
response = (
Expand Down Expand Up @@ -1248,6 +1258,7 @@ async def _emit_event(self, event: OmpEvent, turn: OmpTurn) -> None:
)
turn.calls[event.toolCallId] = (name, arguments)
if Emit.TOOL_CALLS in self.features.emit:
await report_tool_activity(turn.tools, name)
await send_event_safe(
turn.tools,
self._encode_tool_call(name, arguments, event.toolCallId),
Expand All @@ -1258,6 +1269,12 @@ async def _emit_event(self, event: OmpEvent, turn: OmpTurn) -> None:
if call is None:
return
name, _arguments = call
if Emit.TOOL_CALLS in self.features.emit:
if turn.calls:
remaining_name = next(reversed(turn.calls.values()))[0]
await report_tool_activity(turn.tools, remaining_name)
else:
await end_tool_activity(turn.tools)
if not self.config.inject_band_tools and not event.isError:
turn.tools.turn.record(turn_effect(name))
if Emit.TOOL_CALLS in self.features.emit:
Expand Down
61 changes: 60 additions & 1 deletion tests/adapters/omp/kit.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@
OMP_SESSION_DIRECTORY,
OmpAdapter,
OmpAdapterConfig,
OmpPermissionRequest,
)
from band.converters.omp import OmpSessionState
from band.core.delivery import DeliveryFailedError
from band.core.model_catalog import ModelSelection
from band.core.types import MessageType
from band.core.types import Emit, MessageType
from band.integrations.omp import OMP_FORM_APPROVE, OMP_FORM_DENY
from band.integrations.ompprocess import OmpMcpServer, OmpMcpTransport
from band.integrations.omprpc import OmpProcessError, OmpRpcClient, OmpRpcError
Expand Down Expand Up @@ -1157,3 +1158,61 @@ async def ask_turn(
async def ask_frames(adapter: OmpAdapter) -> list[dict[str, JsonValue]]:
checkpoint = await adapter._rooms["room-123"].runtime.request("checkpoint")
return cast(list[dict[str, JsonValue]], checkpoint["frames"])


EMIT_ON: tuple[Emit, ...] = (Emit.TOOL_CALLS,)
EMIT_OFF: tuple[Emit, ...] = ()


def make_adapter(
tmp_path: Path,
emit: tuple[Emit, ...],
resolve_permission=None, # type: ignore[no-untyped-def]
) -> OmpAdapter:
return OmpAdapter(
OmpAdapterConfig(cwd=str(tmp_path)),
emit=emit,
resolve_permission=resolve_permission,
)


class GatedPermissionResolver:
"""Hold each manual choice until its test supplies a result."""

def __init__(self, *titles: str) -> None:
self.entered = {title: asyncio.Event() for title in titles}
self.responses: dict[str, asyncio.Future[str | None]] = {
title: asyncio.get_running_loop().create_future() for title in titles
}

async def __call__(self, request: OmpPermissionRequest) -> str | None:
self.entered[request.title].set()
return await self.responses[request.title]


def permission_request(request_id: str) -> OmpUIRequest:
return OmpUIRequest(
id=request_id,
method="select",
title=request_id,
options=["Approve", "Deny"],
)


def tool_start(call_id: str, name: str, **args: object) -> OmpEvent:
return OmpEvent(
type="tool_execution_start",
toolCallId=call_id,
toolName=name,
args=args,
)


def tool_end(call_id: str, name: str, *, is_error: bool = False) -> OmpEvent:
return OmpEvent(
type="tool_execution_end",
toolCallId=call_id,
toolName=name,
result=OmpToolResult(content=[OmpContent(type="text", text="private-result")]),
isError=is_error,
)
199 changes: 199 additions & 0 deletions tests/adapters/omp/test_permission_activity.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
"""Native OMP permission waits are reported as activity whatever the narration choice."""

from __future__ import annotations

import asyncio
from pathlib import Path

import pytest

from band.adapters.omp import (
OmpAdapter,
OmpAdapterConfig,
OmpPermissionRequest,
OmpTurn,
)
from band.core.types import Emit
from band.integrations.ompwire import OmpUIRequest
from band.testing.activity import ActivityRecorder
from band.testing.fake_tools import FakeAgentTools
from tests.adapters.omp.kit import (
EMIT_OFF,
EMIT_ON,
GatedPermissionResolver,
make_adapter,
permission_request,
)


def waiting(recorder: ActivityRecorder) -> bool:
"""Whether the last manual-wait observation left the turn waiting."""
waits = [event["waiting"] for event in recorder.events if "waiting" in event]
return bool(waits) and waits[-1]


class TestOmpPermissionActivity:
@pytest.mark.parametrize(
"emit", [EMIT_ON, EMIT_OFF], ids=["narration-on", "narration-off"]
)
async def test_wait_is_reported_whatever_the_narration_choice(
self, tmp_path: Path, emit: tuple[Emit, ...]
) -> None:
recorder = ActivityRecorder()
tools = recorder.observing(FakeAgentTools())
turn = OmpTurn(tools=tools)
release = asyncio.Event()
entered = asyncio.Event()
waiting_during: list[bool] = []

async def resolve(request: OmpPermissionRequest) -> str | None:
waiting_during.append(waiting(recorder))
entered.set()
await release.wait()
return "Approve" if request.options == ("Approve", "Deny") else None

adapter = OmpAdapter(
OmpAdapterConfig(cwd=str(tmp_path)),
emit=emit,
resolve_permission=resolve,
)
request = OmpUIRequest(
id="ui-1",
method="select",
title='Allow tool: Bash\nArguments: {"command": "private-command"}',
options=["Approve", "Deny"],
)
running = asyncio.create_task(
adapter._permission(request, "room", "sess", turn)
)
try:
await entered.wait()
assert waiting(recorder)
release.set()
response = await running
finally:
running.cancel()
await asyncio.gather(running, return_exceptions=True)

assert response.value == "Approve"
assert waiting_during == [True]
assert not waiting(recorder)
assert recorder.events == [{"waiting": True}, {"waiting": False}]
assert not tools.events_sent

@pytest.mark.parametrize("outcome", ["complete", "cancel", "error"])
async def test_concurrent_permissions_keep_waiting_until_the_last_resolver_finishes(
self, tmp_path: Path, outcome: str
) -> None:
recorder = ActivityRecorder()
turn = OmpTurn(tools=recorder.observing(FakeAgentTools()))
resolver = GatedPermissionResolver("first", "second")
adapter = make_adapter(tmp_path, EMIT_OFF, resolve_permission=resolver)
first = asyncio.create_task(
adapter._permission(permission_request("first"), "room", "sess", turn)
)
second = asyncio.create_task(
adapter._permission(permission_request("second"), "room", "sess", turn)
)
try:
await resolver.entered["first"].wait()
await resolver.entered["second"].wait()

match outcome:
case "complete":
resolver.responses["first"].set_result("Approve")
await first
case "cancel":
first.cancel()
with pytest.raises(asyncio.CancelledError):
await first
case "error":
resolver.responses["first"].set_exception(
RuntimeError("permission resolver failed")
)
with pytest.raises(
RuntimeError, match="permission resolver failed"
):
await first

assert waiting(recorder)
assert not second.done()
resolver.responses["second"].set_result("Approve")
await second
assert not waiting(recorder)
finally:
first.cancel()
second.cancel()
await asyncio.gather(first, second, return_exceptions=True)

@pytest.mark.parametrize(
"second_room",
["other-room", "room"],
ids=["independent-room", "replacement-turn"],
)
async def test_permission_waits_belong_to_the_turn_that_started_them(
self, tmp_path: Path, second_room: str
) -> None:
first_recorder, second_recorder = ActivityRecorder(), ActivityRecorder()
first_turn = OmpTurn(tools=first_recorder.observing(FakeAgentTools()))
second_turn = OmpTurn(tools=second_recorder.observing(FakeAgentTools()))
resolver = GatedPermissionResolver("first", "second")
adapter = make_adapter(tmp_path, EMIT_OFF, resolve_permission=resolver)
first = asyncio.create_task(
adapter._permission(permission_request("first"), "room", "sess", first_turn)
)
second = asyncio.create_task(
adapter._permission(
permission_request("second"), second_room, "sess", second_turn
)
)
try:
await resolver.entered["first"].wait()
await resolver.entered["second"].wait()
resolver.responses["first"].set_result("Approve")
await first

assert not waiting(first_recorder)
assert waiting(second_recorder)
resolver.responses["second"].set_result("Approve")
await second
assert not waiting(second_recorder)
finally:
first.cancel()
second.cancel()
await asyncio.gather(first, second, return_exceptions=True)

async def test_auto_approved_selection_reports_no_wait(
self, tmp_path: Path
) -> None:
adapter = make_adapter(tmp_path, EMIT_OFF)
recorder = ActivityRecorder()
tools = recorder.observing(FakeAgentTools())
turn = OmpTurn(tools=tools)
request = OmpUIRequest(
id="ui-1",
method="select",
title="Allow tool: Bash",
options=["Approve", "Deny"],
)

response = await adapter._permission(request, "room", "sess", turn)

assert response.value == "Approve"
assert recorder.events == []

async def test_non_select_ui_requests_report_no_wait(self, tmp_path: Path) -> None:
adapter = make_adapter(tmp_path, EMIT_OFF)
recorder = ActivityRecorder()
tools = recorder.observing(FakeAgentTools())
turn = OmpTurn(tools=tools)

response = await adapter._permission(
OmpUIRequest(id="ui-1", method="confirm", title="private-question"),
"room",
"sess",
turn,
)

assert response.cancelled
assert recorder.events == []
Loading
Loading