Skip to content
Merged
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
3 changes: 2 additions & 1 deletion src/acp/interfaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
WriteTextFileRequest,
WriteTextFileResponse,
)
from .terminal import TerminalHandle
from .utils import param_model

__all__ = ["Agent", "Client"]
Expand Down Expand Up @@ -111,7 +112,7 @@ async def create_terminal(
env: list[EnvVariable] | None = None,
output_byte_limit: int | None = None,
**kwargs: Any,
) -> CreateTerminalResponse: ...
) -> CreateTerminalResponse | TerminalHandle: ...
Copy link

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The union return type CreateTerminalResponse | TerminalHandle creates ambiguity for API consumers. Callers cannot determine at compile-time which type will be returned, requiring runtime type checking. Consider documenting which implementations return which type, or ideally standardizing all implementations to return the same type (TerminalHandle) for consistency with the runtime behavior shown in agent/connection.py.

Suggested change
) -> CreateTerminalResponse | TerminalHandle: ...
) -> TerminalHandle: ...

Copilot uses AI. Check for mistakes.

@param_model(TerminalOutputRequest)
async def terminal_output(self, session_id: str, terminal_id: str, **kwargs: Any) -> TerminalOutputResponse: ...
Expand Down
4 changes: 4 additions & 0 deletions src/acp/terminal.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ def __init__(self, terminal_id: str, session_id: str, conn: Connection) -> None:
self._session_id = session_id
self._conn = conn

@property
def terminal_id(self) -> str:
return self.id

async def current_output(self) -> TerminalOutputResponse:
response = await self._conn.send_request(
CLIENT_METHODS["terminal_output"],
Expand Down
8 changes: 7 additions & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
McpServerStdio,
PermissionOption,
ResourceContentBlock,
SessionInfoUpdate,
SseMcpServer,
TextContentBlock,
ToolCallProgress,
Expand Down Expand Up @@ -129,6 +130,10 @@ def __init__(self) -> None:
self.notifications: list[SessionNotification] = []
self.ext_calls: list[tuple[str, dict]] = []
self.ext_notes: list[tuple[str, dict]] = []
self._agent_conn = None

def on_connect(self, conn) -> None:
self._agent_conn = conn

def queue_permission_cancelled(self) -> None:
self.permission_outcomes.append(RequestPermissionResponse(outcome=DeniedOutcome(outcome="cancelled")))
Expand Down Expand Up @@ -167,7 +172,8 @@ async def session_update(
| ToolCallProgress
| AgentPlanUpdate
| AvailableCommandsUpdate
| CurrentModeUpdate,
| CurrentModeUpdate
| SessionInfoUpdate,
**kwargs: Any,
) -> None:
self.notifications.append(SessionNotification(session_id=session_id, update=update, field_meta=kwargs or None))
Expand Down
53 changes: 53 additions & 0 deletions tests/test_rpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
Agent,
AuthenticateResponse,
Client,
CreateTerminalResponse,
InitializeResponse,
LoadSessionResponse,
NewSessionResponse,
Expand All @@ -24,13 +25,15 @@
update_agent_message_text,
update_tool_call,
)
from acp.core import AgentSideConnection, ClientSideConnection
from acp.schema import (
AgentMessageChunk,
AllowedOutcome,
AudioContentBlock,
ClientCapabilities,
DeniedOutcome,
EmbeddedResourceContentBlock,
EnvVariable,
HttpMcpServer,
ImageContentBlock,
Implementation,
Expand Down Expand Up @@ -130,6 +133,56 @@ async def test_session_notifications_flow(connect, client):
assert client.notifications[0].session_id == "sess"


@pytest.mark.asyncio
async def test_on_connect_create_terminal_handle(server):
class _TerminalAgent(Agent):
__test__ = False

def __init__(self) -> None:
self._conn: Client | None = None
self.handle_id: str | None = None

def on_connect(self, conn: Client) -> None:
self._conn = conn

async def prompt(
self,
prompt: list[TextContentBlock],
session_id: str,
**kwargs: Any,
) -> PromptResponse:
assert self._conn is not None
handle = await self._conn.create_terminal(command="echo", session_id=session_id)
self.handle_id = handle.terminal_id
return PromptResponse(stop_reason="end_turn")

class _TerminalClient(TestClient):
__test__ = False

async def create_terminal(
self,
command: str,
session_id: str,
args: list[str] | None = None,
cwd: str | None = None,
env: list[EnvVariable] | None = None,
output_byte_limit: int | None = None,
**kwargs: Any,
) -> CreateTerminalResponse:
return CreateTerminalResponse(terminal_id="term-123")

agent = _TerminalAgent()
client = _TerminalClient()
agent_conn = AgentSideConnection(agent, server.server_writer, server.server_reader, listening=True)
client_conn = ClientSideConnection(client, server.client_writer, server.client_reader)

await client_conn.prompt(session_id="sess", prompt=[TextContentBlock(type="text", text="start")])
assert agent.handle_id == "term-123"

await client_conn.close()
await agent_conn.close()


@pytest.mark.asyncio
async def test_concurrent_reads(connect, client):
for i in range(5):
Expand Down