diff --git a/.github/scripts/setup-copilot.sh b/.github/scripts/setup-copilot.sh index 95fa4f578..7c9ca02aa 100755 --- a/.github/scripts/setup-copilot.sh +++ b/.github/scripts/setup-copilot.sh @@ -16,7 +16,7 @@ set -euo pipefail # Pinned for the same reason as codex: an unpinned global install lets the CLI # float between runs, so a CLI change lands as an unrelated-looking lane # failure. Bump deliberately. -COPILOT_CLI_VERSION="${COPILOT_CLI_VERSION:-1.0.80}" +COPILOT_CLI_VERSION="${COPILOT_CLI_VERSION:-1.0.89}" npm install -g "@github/copilot@${COPILOT_CLI_VERSION}" copilot --version diff --git a/AGENTS.md b/AGENTS.md index b80ddab9e..f2a355f55 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -223,7 +223,7 @@ agent keys and platform URLs should stay aligned with `.env.test` / - `GOOGLE_API_KEY`: Google API key for Gemini Developer API (for Gemini/Google ADK examples) - `GOOGLE_GENAI_USE_VERTEXAI`: Set to `true` to use Vertex AI instead of Gemini Developer API - `GOOGLE_CLOUD_PROJECT`: Google Cloud project ID (required when using Vertex AI) -- `GITHUB_TOKEN`: A Copilot-entitled GitHub token. The baseline `copilot_sdk` and `copilot_acp` builders use Anthropic BYOK and never read it; the only baseline reader is the single Copilot-hosted auth smoke (`test_copilot_hosted_auth_replies`, skips when unset). Also used by Copilot-hosted examples outside the baseline; optional when a stored `copilot login` is present. +- `GITHUB_TOKEN`: A Copilot-entitled GitHub token. The baseline `copilot_sdk` and `copilot_acp` builders use Anthropic BYOK and never read it; the only baseline readers are the Copilot-hosted smokes (`test_copilot_hosted_auth_replies`, `test_copilot_switches_model_and_effort_in_a_live_room`, and the `hosted-effort` cell of `test_copilot_turn_fails_loudly_on_an_unadvertised_selection`, which skip when unset). Also used by Copilot-hosted examples outside the baseline; optional when a stored `copilot login` is present. - `E2E_TESTS_ENABLED`: Set to `true` to enable E2E tests (default: disabled) - `E2E_LLM_MODEL`: OpenAI model for E2E tests (default: `gpt-5.4-mini`) - `E2E_ANTHROPIC_MODEL`: Anthropic model for E2E tests (default: `claude-haiku-4-5` — the baseline judge uses structured outputs, which older Haiku models do not support) diff --git a/src/band/adapters/copilot_acp.py b/src/band/adapters/copilot_acp.py index 074117bf4..e391ae3a2 100644 --- a/src/band/adapters/copilot_acp.py +++ b/src/band/adapters/copilot_acp.py @@ -31,6 +31,7 @@ from typing_extensions import Unpack +from band.core.model_catalog import ModelSelection from band.core.types import FeatureKwargs from band.integrations.acp.client_adapter import ( DEFAULT_TURN_TIMEOUT_SECONDS, @@ -70,6 +71,12 @@ class CopilotACPAdapterConfig: resolve_session_config: SessionConfigResolver | None = None resolve_permission: PermissionResolver | None = None turn_timeout_s: float = DEFAULT_TURN_TIMEOUT_SECONDS + # Selected from each session's advertised catalog; a value it does not + # offer fails the turn. Only GitHub-hosted sessions advertise these; under + # BYOK the provider env (COPILOT_MODEL) picks the model. Exclusive with + # resolve_session_config. + model: str | None = None + reasoning_effort: str | None = None class CopilotACPAdapter(ACPClientAdapter): @@ -126,6 +133,9 @@ def __init__( "inject_band_tools": config.inject_band_tools, "custom_section": config.custom_section, "resolve_session_config": config.resolve_session_config, + "model_selection": ModelSelection( + model=config.model, reasoning_effort=config.reasoning_effort + ), "resolve_permission": config.resolve_permission, "turn_timeout_s": config.turn_timeout_s, } diff --git a/src/band/agent.py b/src/band/agent.py index 8c732388c..c618556ee 100644 --- a/src/band/agent.py +++ b/src/band/agent.py @@ -13,8 +13,8 @@ from band.core.protocols import FrameworkAdapter, Preprocessor from band.core.simple_adapter import SimpleAdapter from band.preprocessing.default import DefaultPreprocessor -from band.runtime.capabilities import prune_unsupported from band.runtime.platform_runtime import PlatformRuntime +from band.runtime.startup import release_adapter, start_adapter from band.runtime.types import ( AgentConfig, ContactEventConfig, @@ -257,18 +257,11 @@ async def start(self) -> None: # setattr rather than assignment: FrameworkAdapter is a Protocol, so # a duck-typed adapter may not declare the attribute. setattr(self._adapter, "platform", self._runtime.connection) # noqa: B010 - if isinstance(self._adapter, SimpleAdapter): - # A bare FrameworkAdapter has no SUPPORTED_CAPABILITIES, so it - # cannot request a gated capability in the first place and - # takes no part in negotiation. - self._adapter.apply_effective_features( - prune_unsupported( - self._adapter.features, self._runtime.feature_flags - ) - ) - await self._adapter.on_started( - self._runtime.agent_name, - self._runtime.agent_description, + await start_adapter( + self._adapter, + agent_name=self._runtime.agent_name, + agent_description=self._runtime.agent_description, + feature_flags=self._runtime.feature_flags, ) # 3. NOW start message processing (connects WebSocket) @@ -282,10 +275,10 @@ async def start(self) -> None: on_control=getattr(self._adapter, "on_interrupt", None), ) except BaseException: - # on_started may have acquired resources (e.g. a CLI runtime - # subprocess); a failed start must release them — stop() won't - # run for an agent that never started. - await self._cleanup_adapter() + # Adapter startup may have acquired resources (e.g. a CLI + # runtime subprocess); a failed start must release them — + # stop() won't run for an agent that never started. + await release_adapter(self._adapter) raise except BaseException: # Idempotent: a failure inside runtime.start() already released. @@ -323,7 +316,7 @@ async def stop(self, timeout: float | None = None) -> bool: # Always release adapter-wide resources (e.g. a CLI runtime # subprocess, a self-hosted MCP server, an external registration), # even when the runtime fails to stop cleanly. - await self._cleanup_adapter() + await release_adapter(self._adapter) self._started = False _running_agents.discard(self) logger.info( @@ -331,15 +324,6 @@ async def stop(self, timeout: float | None = None) -> bool: ) return graceful - async def _cleanup_adapter(self) -> None: - """Release adapter-wide resources, best-effort.""" - cleanup_all = getattr(self._adapter, "cleanup_all", None) - if cleanup_all is not None: - try: - await cleanup_all() - except Exception: - logger.exception("Adapter cleanup_all failed") - async def run( self, shutdown_timeout: float | None = DEFAULT_SHUTDOWN_TIMEOUT ) -> None: diff --git a/src/band/core/model_catalog.py b/src/band/core/model_catalog.py new file mode 100644 index 000000000..8381b96b7 --- /dev/null +++ b/src/band/core/model_catalog.py @@ -0,0 +1,148 @@ +"""Model and reasoning-effort selection checked against a harness catalog.""" + +from __future__ import annotations + +from enum import StrEnum + +from pydantic import BaseModel, ConfigDict + +from band.core.exceptions import BandConfigError +from band.core.validation import listing + + +class ModelSetting(StrEnum): + """A selectable model setting, valued as its ``ModelSelection`` field. + + Declared in apply order: the model decides which efforts exist. + """ + + MODEL = "model" + REASONING_EFFORT = "reasoning_effort" + + +class ModelSelection(BaseModel): + """The model and reasoning effort an adapter is configured to use. + + ``None`` leaves the choice to the harness. + """ + + model_config = ConfigDict(frozen=True) + + model: str | None = None + reasoning_effort: str | None = None + + def value_of(self, setting: ModelSetting) -> str | None: + return getattr(self, setting) + + @property + def is_empty(self) -> bool: + return self.model is None and self.reasoning_effort is None + + +class ModelChoice(BaseModel): + """One model a harness advertises. + + ``efforts`` is ``None`` when the harness does not report them for this + model (the effort goes unchecked), and ``()`` when the model offers none. + """ + + model_config = ConfigDict(frozen=True) + + id: str + label: str | None = None + efforts: tuple[str, ...] | None = None + default_effort: str | None = None + + +class ModelCatalog(BaseModel): + """The models a harness advertises, and the one it currently uses.""" + + model_config = ConfigDict(frozen=True) + + models: tuple[ModelChoice, ...] + current_model: str | None = None + + @property + def model_ids(self) -> tuple[str, ...]: + return tuple(choice.id for choice in self.models) + + def choice(self, model_id: str) -> ModelChoice | None: + return next((choice for choice in self.models if choice.id == model_id), None) + + +class ModelSelectionError(BandConfigError): + """A configured model or effort is not what the harness advertises.""" + + def __init__( + self, + message: str, + *, + setting: ModelSetting, + value: str, + advertised: tuple[str, ...], + ) -> None: + super().__init__(message) + self.setting = setting + self.value = value + self.advertised = advertised + + +def check_model_selection( + selection: ModelSelection, catalog: ModelCatalog | None +) -> None: + """Raise ``ModelSelectionError`` when ``catalog`` rejects ``selection``. + + A ``None`` catalog means the harness advertises nothing, so nothing is + checked. The effort is checked against the selected model, or the + catalog's current model when none is selected. + """ + if catalog is None: + return + if selection.model is not None: + _check_model(selection.model, catalog) + if selection.reasoning_effort is not None: + _check_effort( + selection.reasoning_effort, + model_id=selection.model or catalog.current_model, + catalog=catalog, + ) + + +def _check_model(model_id: str, catalog: ModelCatalog) -> None: + if catalog.choice(model_id) is not None: + return + raise ModelSelectionError( + f'model "{model_id}" is not advertised; available: ' + f"{listing(catalog.model_ids)}", + setting=ModelSetting.MODEL, + value=model_id, + advertised=catalog.model_ids, + ) + + +def _check_effort(effort: str, *, model_id: str | None, catalog: ModelCatalog) -> None: + choice = catalog.choice(model_id) if model_id is not None else None + if choice is None or choice.efforts is None or effort in choice.efforts: + return + message = ( + f'reasoning effort "{effort}" is not advertised for model "{model_id}"; ' + f"available: {listing(choice.efforts)}" + if choice.efforts + else f'model "{model_id}" offers no reasoning effort' + ) + raise ModelSelectionError( + message, + setting=ModelSetting.REASONING_EFFORT, + value=effort, + advertised=choice.efforts, + ) + + +__all__ = [ + "ModelCatalog", + "ModelChoice", + "ModelSelection", + "ModelSelectionError", + "ModelSetting", + "check_model_selection", +] diff --git a/src/band/core/simple_adapter.py b/src/band/core/simple_adapter.py index 06dd708cf..fc32a5d1e 100644 --- a/src/band/core/simple_adapter.py +++ b/src/band/core/simple_adapter.py @@ -13,6 +13,11 @@ from band.client.rest import AsyncRestClient from band.client.streaming import ControlMode from band.core.exceptions import BandConfigError +from band.core.model_catalog import ( + ModelCatalog, + ModelSelection, + check_model_selection, +) from band.core.protocols import AgentToolsProtocol, HistoryConverter from band.core.types import ( USAGE_EVENT_TYPE, @@ -26,6 +31,7 @@ PlatformMessage, TurnUsage, ) +from band.core.validation import listing from band.logging_config import trace_context_scope logger = logging.getLogger(__name__) @@ -58,7 +64,7 @@ def _normalize_flags( def _describe(values: Iterable[Emit] | Iterable[Capability]) -> str: - return ", ".join(sorted(v.value for v in values)) or "(none)" + return listing(sorted(v.value for v in values)) class SimpleAdapter(ABC, Generic[H]): @@ -170,12 +176,11 @@ def _resolve_features( def apply_effective_features(self, features: AdapterFeatures) -> None: """Adopt ``features`` as the post-negotiation feature set. - Called by ``Agent.start()``/``OneShotInvoker.startup()`` after pruning - capabilities the deployment doesn't serve (see - ``runtime.capabilities.prune_unsupported``). Override if the adapter - caches something derived from ``self.features`` at construction (e.g. - a tool-definition list built in ``__init__``) so that cache rebuilds - too. + Called from ``runtime.startup.start_adapter`` after pruning capabilities + the deployment doesn't serve (see ``runtime.capabilities.prune_unsupported``). + Override if the adapter caches something derived from ``self.features`` at + construction (e.g. a tool-definition list built in ``__init__``) so that + cache rebuilds too. """ self.features = features @@ -299,6 +304,55 @@ async def cleanup_all(self) -> None: release here. """ + @property + def model_selection(self) -> ModelSelection: + """The model and reasoning effort this adapter is configured to use. + + Adapters with typed model settings override this to project them. The + default selects nothing, leaving the choice to the harness. + """ + return ModelSelection() + + async def list_models(self) -> ModelCatalog | None: + """The models this adapter's harness advertises, or ``None``. + + Adapters whose harness reports a catalog override this; ``None`` means + it cannot, so the configured selection goes unchecked. It runs after + ``on_started``, so it may use a client started there. Harnesses whose + catalog belongs to a live session (ACP) keep ``None`` and check the + selection per session instead. + """ + return None + + async def apply_model_selection( + self, selection: ModelSelection, *, room_id: str + ) -> None: + """Switch ``room_id``'s live conversation to ``selection``. + + Adapters whose harness can change model mid-conversation override + this. The default refuses, so a requested switch is never silently + ignored. + """ + del selection, room_id + raise BandConfigError( + f"{type(self).__name__} does not support switching models at runtime" + ) + + async def startup( + self, agent_name: str, agent_description: str, *, features: AdapterFeatures + ) -> None: + """Start for an agent: negotiated ``features``, ``on_started``, then the + model-selection check. + + The runtime calls this once per start; adapters override the hooks it + runs, not this. + """ + self.apply_effective_features(features) + await self.on_started(agent_name, agent_description) + selection = self.model_selection + if not selection.is_empty: + check_model_selection(selection, await self.list_models()) + async def on_started(self, agent_name: str, agent_description: str) -> None: """Override for post-start setup.""" self.agent_name = agent_name diff --git a/src/band/core/validation.py b/src/band/core/validation.py index 06af1169f..3b052a499 100644 --- a/src/band/core/validation.py +++ b/src/band/core/validation.py @@ -1,9 +1,17 @@ """Shared cross-field validation rules used by tool input models and their ``AgentTools``-level backstops (some adapters hand-register tools as plain -functions and never construct/validate the input model).""" +functions and never construct/validate the input model), and the shared +wording of rejection messages.""" from __future__ import annotations +from collections.abc import Iterable + + +def listing(values: Iterable[str]) -> str: + """The accepted values as a rejection message shows them.""" + return ", ".join(values) or "(none)" + def _join_with_or(names: list[str]) -> str: if len(names) == 1: diff --git a/src/band/integrations/acp/client_adapter.py b/src/band/integrations/acp/client_adapter.py index 46c19e69d..3f95c9318 100644 --- a/src/band/integrations/acp/client_adapter.py +++ b/src/band/integrations/acp/client_adapter.py @@ -5,9 +5,17 @@ import asyncio import logging import shutil -from collections.abc import AsyncIterator, Awaitable, Callable, Coroutine, Mapping +from collections.abc import ( + AsyncIterator, + Awaitable, + Callable, + Coroutine, + Mapping, + Sequence, +) from contextlib import asynccontextmanager, suppress from dataclasses import dataclass +from functools import partial from typing import Any, ClassVar, TypeAlias from uuid import uuid4 @@ -18,6 +26,7 @@ HttpMcpServer, NewSessionResponse, PermissionOption, + SetSessionConfigOptionResponse, SseMcpServer, ) from band_sdk_core import AgentFailure @@ -26,6 +35,8 @@ from band.converters.acp_client import ACPClientHistoryConverter from band.converters.helpers import build_replay_messages from band.core.delivery import DeliveryFailedError, reraise_delivery_cause +from band.core.exceptions import BandConfigError +from band.core.model_catalog import ModelSelection from band.core.protocols import ( FAILURE_CODE_TIMEOUT, GENERIC_PROVIDER_FAILURE_MESSAGE, @@ -58,15 +69,22 @@ ACPClientSessionState, BandACPClient, ) +from band.integrations.acp.model_selection import ( + ACPModelOptions, + apply_model_selection, + locate_model_options, +) from band.integrations.acp.room_emitter import RoomTurnEmitter from band.integrations.acp.session_config import ( + CONFIG_FAILURE_PREFIX, RESOLVER_CONFIG_OPTION_ID, ACPConfigError, ACPConfigRequest, + ACPConfigUnreachableError, SessionConfigOption, SessionConfigResolver, + SessionConfigSetter, apply_session_config_selections, - session_config_options, ) from band.integrations.acp.types import ACPToolCall from band.integrations.mcp.backends import ( @@ -153,6 +171,7 @@ class ACPTurnTimeoutError(TimeoutError): NEW_MESSAGE_MARKER_PREFIX = "[New Message" SESSION_CLOSE_TIMEOUT_SECONDS = 5.0 DEFAULT_TURN_TIMEOUT_SECONDS = 300.0 +_EMPTY_MODEL_SELECTION = ModelSelection() def new_message_marker() -> str: @@ -198,6 +217,19 @@ def _resolve_launcher(command: list[str]) -> list[str]: return [resolved, *command[1:]] if resolved else list(command) +def _config_setter(runtime: ACPRuntime) -> SessionConfigSetter: + """``session/set_config_option`` on ``runtime``, in the applier's shape.""" + + async def set_option( + session_id: str, option_id: str, value: str + ) -> SetSessionConfigOptionResponse | None: + return await runtime.set_config_option( + session_id=session_id, config_id=option_id, value=value + ) + + return set_option + + def _to_agent_failure(exc: Exception) -> AgentFailure: """Parse a turn-ending exception into the shared provider-failure shape. @@ -250,12 +282,17 @@ def __init__( client_capabilities: ClientCapabilities | None = None, use_unstable_protocol: bool = False, turn_timeout_s: float = DEFAULT_TURN_TIMEOUT_SECONDS, + model_selection: ModelSelection = _EMPTY_MODEL_SELECTION, **features: Unpack[FeatureKwargs], ) -> None: super().__init__( history_converter=ACPClientHistoryConverter(), **features, ) + if not model_selection.is_empty and resolve_session_config is not None: + raise ValueError( + "set either model_selection or resolve_session_config, not both" + ) if cwd is not None: raise ValueError( "cwd is not supported; use workspace_for_room or the default" @@ -281,6 +318,7 @@ def __init__( self._auth_method = auth_method self._profile = profile self._resolve_session_config = resolve_session_config + self._model_selection = model_selection self._resolve_permission = resolve_permission self._client_capabilities = client_capabilities self._use_unstable_protocol = use_unstable_protocol @@ -292,6 +330,8 @@ def __init__( self._turn_timeout_s = turn_timeout_s self._room_to_session: dict[str, str] = {} + # Outlives the room's sessions; see apply_model_selection. + self._room_selections: dict[str, ModelSelection] = {} self._session_initializers: dict[str, SessionInitializer] = {} self._room_tools: dict[str, AgentToolsProtocol] = {} self._background_tasks: set[asyncio.Task[None]] = set() @@ -308,6 +348,96 @@ def __init__( # and is never stopped -- a real leaked server, not just a failed turn. self._stopped = False + @property + def model_selection(self) -> ModelSelection: + """Applied per session from its live catalog, not checked at start.""" + return self._model_selection + + async def apply_model_selection( + self, selection: ModelSelection, *, room_id: str + ) -> None: + """Switch ``room_id``'s live ACP session, checked against its catalog. + + Raises ``BandConfigError`` (an ``ACPConfigError`` naming what the + session offers, when it rejects ``selection``). Only a switch that + returns counts: the room then keeps what its session runs for the + adapter's lifetime, and every later session for it starts there + until one refuses it. A switch that raises may leave the live session + partly switched, but later sessions start where they would have + before it. Other rooms keep the configured selection. Refused when + ``resolve_session_config`` configures sessions instead. An empty + ``selection`` changes nothing. + """ + if self._resolve_session_config is not None: + raise BandConfigError( + "a runtime model switch cannot be combined with resolve_session_config" + ) + if selection.is_empty: + return + session = await self._live_session(room_id) + if session is None: + raise BandConfigError(f"room {room_id} has no live ACP session to switch") + session_id, runtime = session + async with runtime.config_lock: + try: + await self._apply_model_selection(runtime, session_id, selection) + except ACPConfigError: + # A cleanup while this switch queued or ran fails it however + # the dead runtime answered; the ended session is the cause. + await self._ensure_session_live(room_id, session) + logger.warning( + "Switching room %s to %s failed; its session runs %s", + room_id, + selection, + self._running_selection(runtime, session_id), + ) + raise + await self._ensure_session_live(room_id, session) + self._remember_room_selection(room_id, session_id, runtime) + + async def _ensure_session_live( + self, room_id: str, session: tuple[str, ACPRuntime] + ) -> None: + if await self._live_session(room_id) != session: + raise BandConfigError(f"room {room_id}'s ACP session ended mid-switch") + + def _running_selection( + self, runtime: ACPRuntime, session_id: str + ) -> ModelSelection: + return self.locate_model_options( + runtime.config_options(session_id) + ).current_selection() + + def _remember_room_selection( + self, room_id: str, session_id: str, runtime: ACPRuntime + ) -> None: + running = self._running_selection(runtime, session_id) + self._room_selections[room_id] = running + logger.info( + "ACP session %s for room %s now runs %s", session_id, room_id, running + ) + + def _room_selection(self, room_id: str) -> ModelSelection: + return self._room_selections.get(room_id, self.model_selection) + + async def _live_session(self, room_id: str) -> tuple[str, ACPRuntime] | None: + async with self._session_lock: + session_id = self._room_to_session.get(room_id) + runtime = self._runtimes.get(room_id) + if session_id is None or runtime is None: + return None + return session_id, runtime + + def locate_model_options( + self, options: Sequence[SessionConfigOption] + ) -> ACPModelOptions: + """Find the session's model and effort selects. + + The default reads the spec-reserved categories; agents that publish + them under other ids or categories override this. + """ + return locate_model_options(options) + def apply_effective_features(self, features: AdapterFeatures) -> None: """Rebuild the lazy MCP registration after capability negotiation.""" super().apply_effective_features(features) @@ -427,6 +557,10 @@ async def on_message( history if is_session_bootstrap else None, ) except ACPConfigError as error: + # An unanswered set may have left a dead connection the runtime + # never replaces; only a fresh runtime lets the next turn retry. + if isinstance(error, ACPConfigUnreachableError): + await self.on_cleanup(room_id) await self._report_config_error(tools, error) return runtime.reset_session(session_id) @@ -893,12 +1027,7 @@ async def _restore_session( return None try: - await self._configure_session( - runtime, - room_id, - session_id, - session_config_options(loaded), - ) + await self._configure_session(runtime, room_id, session_id) except BaseException: await self._close_fresh_session(runtime, session_id) raise @@ -911,12 +1040,7 @@ async def _create_session( ) -> str: """Create, configure, and publish a session for one room.""" async with self._fresh_session(runtime, room_id, mcp_servers) as session: - await self._configure_session( - runtime, - room_id, - session.session_id, - session_config_options(session), - ) + await self._configure_session(runtime, room_id, session.session_id) await self._record_session(room_id, session.session_id) logger.info( @@ -1020,19 +1144,65 @@ async def _session_mcp_servers(self, room_id: str) -> list[object]: return mcp_servers async def _configure_session( + self, runtime: ACPRuntime, room_id: str, session_id: str + ) -> None: + """Apply caller-selected values from the session's live ACP catalog.""" + selection = self._room_selection(room_id) + if not selection.is_empty: + await self._apply_room_selection(runtime, room_id, session_id, selection) + elif self._resolve_session_config is not None: + await self._apply_resolved_config( + resolver=self._resolve_session_config, + room_id=room_id, + session_id=session_id, + runtime=runtime, + ) + + async def _apply_room_selection( self, runtime: ACPRuntime, room_id: str, session_id: str, - config_options: tuple[SessionConfigOption, ...] | None, + selection: ModelSelection, ) -> None: - """Apply caller-selected values from the session's live ACP catalog.""" - if self._resolve_session_config is None: - return + try: + await self._apply_model_selection(runtime, session_id, selection) + except ACPConfigUnreachableError: + raise + except ACPConfigError: + # A remembered switch the agent no longer accepts must fail this + # turn only; later sessions go back to the configured selection. + if self._room_selections.pop(room_id, None) is not None: + logger.warning( + "Room %s's switched model was refused; reverting to %s", + room_id, + self.model_selection, + ) + raise + + async def _apply_model_selection( + self, runtime: ACPRuntime, session_id: str, selection: ModelSelection + ) -> None: + await apply_model_selection( + session_id=session_id, + catalog=partial(runtime.config_options, session_id), + selection=selection, + locate=self.locate_model_options, + set_option=_config_setter(runtime), + ) - catalog = tuple(config_options or ()) + async def _apply_resolved_config( + self, + *, + resolver: SessionConfigResolver, + room_id: str, + session_id: str, + runtime: ACPRuntime, + ) -> None: + """Apply what the caller's ``resolve_session_config`` selects.""" + catalog = runtime.config_options(session_id) try: - selections = await self._resolve_session_config( + selections = await resolver( ACPConfigRequest( room_id=room_id, session_id=session_id, @@ -1055,11 +1225,7 @@ async def _configure_session( session_id=session_id, config_options=catalog, selections=selections, - set_option=lambda session_id, option_id, value: runtime.set_config_option( - session_id=session_id, - config_id=option_id, - value=value, - ), + set_option=_config_setter(runtime), ) async def _report_config_error( @@ -1067,11 +1233,18 @@ async def _report_config_error( tools: AgentToolsProtocol, error: ACPConfigError, ) -> None: - logger.warning("ACP session configuration failed: %s", error) + logger.warning( + "%s%s (session %s, option %s, value %s)", + CONFIG_FAILURE_PREFIX, + error, + error.session_id, + error.option_id, + error.selected_value, + ) await tools.send_failure( AgentFailure( _PROVIDER, - f"ACP session configuration failed: {error}", + f"{CONFIG_FAILURE_PREFIX}{error}", "acp_session_config", { "session_id": error.session_id, diff --git a/src/band/integrations/acp/client_runtime.py b/src/band/integrations/acp/client_runtime.py index 8db65feed..219c0ca1d 100644 --- a/src/band/integrations/acp/client_runtime.py +++ b/src/band/integrations/acp/client_runtime.py @@ -5,7 +5,7 @@ import asyncio import json import logging -from collections.abc import AsyncIterator, Awaitable, Callable, Mapping +from collections.abc import AsyncIterator, Awaitable, Callable, Mapping, Sequence from contextlib import AbstractAsyncContextManager, asynccontextmanager from typing import Any, Literal, Protocol, cast @@ -14,6 +14,7 @@ from acp.interfaces import Client from acp.schema import ( ClientCapabilities, + ConfigOptionUpdate, DeclineElicitationResponse, LoadSessionResponse, NewSessionResponse, @@ -24,6 +25,10 @@ ACPClientProfile, NoopACPClientProfile, ) +from band.integrations.acp.session_config import ( + SessionConfigOption, + session_config_options, +) from band.integrations.acp.types import ( ACPToolCall, ACPToolResult, @@ -352,6 +357,9 @@ def __init__( # Never popped in reset_session: replacing a lock a straggler task still # holds would let two tasks into the session's critical section. self._session_locks: dict[str, asyncio.Lock] = {} + # Each session's latest advertised config catalog; like the locks, + # it outlives reset_session, which runs every turn. + self._config_options: dict[str, tuple[SessionConfigOption, ...]] = {} def _session_lock(self, session_id: str) -> asyncio.Lock: return self._session_locks.setdefault(session_id, asyncio.Lock()) @@ -371,6 +379,10 @@ async def session_update( self, session_id: str, update: object, **kwargs: object ) -> None: del kwargs + if isinstance(update, ConfigOptionUpdate): + logger.debug("ACP session %s pushed new config options", session_id) + self.record_config_options(session_id, update.config_options) + return async with self._session_lock(session_id): chunk = self._chunk_from_update(update) if chunk is not None: @@ -699,6 +711,17 @@ def set_permission_handler( else: self._permission_handlers[session_id] = handler + def record_config_options( + self, session_id: str, options: Sequence[SessionConfigOption] + ) -> None: + self._config_options[session_id] = tuple(options) + + def config_options(self, session_id: str) -> tuple[SessionConfigOption, ...]: + return self._config_options.get(session_id, ()) + + def forget_config_options(self, session_id: str) -> None: + self._config_options.pop(session_id, None) + def reset_session(self, session_id: str) -> None: self._session_chunks.pop(session_id, None) self._permission_handlers.pop(session_id, None) @@ -827,6 +850,7 @@ def __init__( self._agent_mcp_transport: MCPTransportKind = "http" self._agent_supports_session_load = False self._agent_supports_session_close = False + self._config_lock = asyncio.Lock() async def start(self, *, respawn: bool = False) -> None: """Spawn or respawn the ACP agent subprocess.""" @@ -906,10 +930,12 @@ async def create_session_response( self, *, cwd: str, mcp_servers: list[object] ) -> NewSessionResponse: conn = await self.ensure_connection(can_respawn=False) - return cast( + response = cast( NewSessionResponse, await conn.new_session(cwd=cwd, mcp_servers=mcp_servers), ) + self._record_config_options(response.session_id, response) + return response async def load_session( self, @@ -974,6 +1000,7 @@ async def load_session_response( error, ) return None + self._record_config_options(session_id, response) return response async def set_config_option( @@ -985,14 +1012,39 @@ async def set_config_option( ) -> SetSessionConfigOptionResponse | None: """Set one advertised select option and return the refreshed catalog.""" conn = await self.ensure_connection(can_respawn=False) - return await conn.set_config_option( + response = await conn.set_config_option( session_id=session_id, config_id=config_id, value=value, ) + self._record_config_options(session_id, response) + return response + + def config_options(self, session_id: str) -> tuple[SessionConfigOption, ...]: + """The session's live catalog: its setup response, then every change.""" + if self._client is None: + return () + return self._client.config_options(session_id) + + @property + def config_lock(self) -> asyncio.Lock: + """Held across a runtime switch. + + Each step is checked against the catalog the previous one returned, so + an interleaved switch would validate against a model no longer current. + Session setup needs no lock: its room is not yet published to switch. + """ + return self._config_lock + + def _record_config_options(self, session_id: str, response: object) -> None: + options = session_config_options(response) + if options is not None and self._client is not None: + self._client.record_config_options(session_id, options) async def close_session(self, session_id: str) -> None: """Close a session when the agent advertised lifecycle support.""" + if self._client is not None: + self._client.forget_config_options(session_id) if not self._agent_supports_session_close: return conn = await self.ensure_connection(can_respawn=False) diff --git a/src/band/integrations/acp/model_selection.py b/src/band/integrations/acp/model_selection.py new file mode 100644 index 000000000..9dbeb1c13 --- /dev/null +++ b/src/band/integrations/acp/model_selection.py @@ -0,0 +1,174 @@ +"""Apply a typed ``ModelSelection`` to an ACP session's live config catalog.""" + +from __future__ import annotations + +from collections.abc import Callable, Sequence +from dataclasses import dataclass + +from acp.schema import SessionConfigOptionSelect, SessionConfigSelectOption + +from band.core.model_catalog import ( + ModelCatalog, + ModelChoice, + ModelSelection, + ModelSelectionError, + ModelSetting, + check_model_selection, +) +from band.core.validation import listing +from band.integrations.acp.session_config import ( + ACPConfigError, + SessionConfigOption, + SessionConfigSetter, + apply_session_config_selections, + flatten_select_options, + select_ids, + select_values, + selects, +) + +# Category ids the ACP spec reserves for these selectors. +MODEL_CATEGORY = "model" +THOUGHT_LEVEL_CATEGORY = "thought_level" + + +@dataclass(frozen=True) +class ACPModelOptions: + """The selects a session uses for its model and reasoning effort.""" + + model: SessionConfigOptionSelect | None + effort: SessionConfigOptionSelect | None + + def option_for(self, setting: ModelSetting) -> SessionConfigOptionSelect | None: + match setting: + case ModelSetting.MODEL: + return self.model + case ModelSetting.REASONING_EFFORT: + return self.effort + + def current_selection(self) -> ModelSelection: + """What the session currently runs; no effort when it offers none.""" + return ModelSelection( + model=self.model.current_value if self.model else None, + reasoning_effort=self.effort.current_value if self.effort else None, + ) + + def model_catalog(self) -> ModelCatalog | None: + """The advertised models; efforts are known only for the current one.""" + if self.model is None: + return None + return ModelCatalog( + models=tuple( + self._choice(entry, current=self.model.current_value) + for entry in flatten_select_options(self.model.options) + ), + current_model=self.model.current_value, + ) + + def _choice(self, entry: SessionConfigSelectOption, *, current: str) -> ModelChoice: + efforts: tuple[str, ...] | None = None + default_effort: str | None = None + if entry.value == current: + efforts = select_values(self.effort) if self.effort else () + default_effort = self.effort.current_value if self.effort else None + return ModelChoice( + id=entry.value, + label=entry.name, + efforts=efforts, + default_effort=default_effort, + ) + + +def locate_model_options(options: Sequence[SessionConfigOption]) -> ACPModelOptions: + """Find the model and effort selects by their spec-reserved categories.""" + return ACPModelOptions( + model=_select_in_category(options, MODEL_CATEGORY), + effort=_select_in_category(options, THOUGHT_LEVEL_CATEGORY), + ) + + +async def apply_model_selection( + *, + session_id: str, + catalog: Callable[[], Sequence[SessionConfigOption]], + selection: ModelSelection, + locate: Callable[[Sequence[SessionConfigOption]], ACPModelOptions], + set_option: SessionConfigSetter, +) -> None: + """Select the model, then the effort, each against the session's catalog + as it stands then. + + Choosing a model can add, remove, or replace the effort select, so the + effort is located and checked only after the model has been applied. + """ + for setting in ModelSetting: + value = selection.value_of(setting) + if value is None: + continue + await _apply_setting( + session_id=session_id, + config_options=tuple(catalog()), + setting=setting, + value=value, + locate=locate, + set_option=set_option, + ) + + +async def _apply_setting( + *, + session_id: str, + config_options: tuple[SessionConfigOption, ...], + setting: ModelSetting, + value: str, + locate: Callable[[Sequence[SessionConfigOption]], ACPModelOptions], + set_option: SessionConfigSetter, +) -> None: + located = locate(config_options) + option = located.option_for(setting) + option_id = option.id if option is not None else setting + try: + check_model_selection( + ModelSelection.model_validate({setting: value}), + located.model_catalog(), + ) + except ModelSelectionError as error: + raise ACPConfigError( + session_id=session_id, + option_id=option_id, + selected_value=value, + message=str(error), + ) from error + if option is None: + raise ACPConfigError( + session_id=session_id, + option_id=option_id, + selected_value=value, + message=( + f"ACP session advertises no {setting} option; available: " + f"{listing(select_ids(config_options))}." + ), + ) + await apply_session_config_selections( + session_id=session_id, + config_options=config_options, + selections={option.id: value}, + set_option=set_option, + ) + + +def _select_in_category( + options: Sequence[SessionConfigOption], category: str +) -> SessionConfigOptionSelect | None: + return next( + (option for option in selects(options) if option.category == category), None + ) + + +__all__ = [ + "MODEL_CATEGORY", + "THOUGHT_LEVEL_CATEGORY", + "ACPModelOptions", + "apply_model_selection", + "locate_model_options", +] diff --git a/src/band/integrations/acp/session_config.py b/src/band/integrations/acp/session_config.py index 01852e051..88e543aae 100644 --- a/src/band/integrations/acp/session_config.py +++ b/src/band/integrations/acp/session_config.py @@ -3,10 +3,11 @@ from __future__ import annotations import asyncio -from collections.abc import Awaitable, Callable, Mapping, Sequence +from collections.abc import Awaitable, Callable, Iterator, Mapping, Sequence from dataclasses import dataclass from typing import TypeAlias +from acp.exceptions import RequestError from acp.schema import ( SessionConfigOptionBoolean, SessionConfigOptionSelect, @@ -14,6 +15,10 @@ SessionConfigSelectOption, SetSessionConfigOptionResponse, ) +from pydantic import ValidationError + +from band.core.exceptions import BandConfigError +from band.core.validation import listing SessionConfigOption: TypeAlias = SessionConfigOptionSelect | SessionConfigOptionBoolean SessionConfigSelections: TypeAlias = Mapping[str, str | None] @@ -26,6 +31,8 @@ SESSION_CONFIG_TIMEOUT_SECONDS = 10.0 RESOLVER_CONFIG_OPTION_ID = "resolver" +# How the room reads a failure to apply a session configuration. +CONFIG_FAILURE_PREFIX = "ACP session configuration failed: " @dataclass(frozen=True) @@ -37,7 +44,8 @@ class ACPConfigRequest: config_options: tuple[SessionConfigOption, ...] -class ACPConfigError(RuntimeError): +# Also a RuntimeError, so existing ``except RuntimeError`` handlers catch it. +class ACPConfigError(BandConfigError, RuntimeError): """A requested ACP session configuration could not be applied.""" def __init__( @@ -54,6 +62,11 @@ def __init__( self.selected_value = selected_value +class ACPConfigUnreachableError(ACPConfigError): + """No reply ever came to a config request (a timeout or a lost + connection), so nothing says the agent refused the value.""" + + def flatten_select_options( options: Sequence[SessionConfigSelectOption | SessionConfigSelectGroup], ) -> tuple[SessionConfigSelectOption, ...]: @@ -67,6 +80,32 @@ def flatten_select_options( return tuple(flattened) +def select_values(option: SessionConfigOptionSelect) -> tuple[str, ...]: + """The values an ACP select option advertises, groups flattened.""" + return tuple(entry.value for entry in flatten_select_options(option.options)) + + +def selects( + options: Sequence[SessionConfigOption], +) -> Iterator[SessionConfigOptionSelect]: + """The select options in an ACP catalog.""" + return ( + option for option in options if isinstance(option, SessionConfigOptionSelect) + ) + + +def find_select( + options: Sequence[SessionConfigOption], option_id: str +) -> SessionConfigOptionSelect | None: + """The select option with ``option_id`` in an ACP catalog, if any.""" + return next((option for option in selects(options) if option.id == option_id), None) + + +def select_ids(options: Sequence[SessionConfigOption]) -> tuple[str, ...]: + """The ids of the select options in an ACP catalog.""" + return tuple(option.id for option in selects(options)) + + def session_config_options(response: object) -> tuple[SessionConfigOption, ...] | None: """Return a response's complete, typed ACP configuration catalog.""" options = getattr(response, "config_options", None) @@ -106,21 +145,21 @@ async def apply_session_config_selections( message=f'ACP config value for option "{option_id}" must be a string.', ) - option = next((entry for entry in catalog if entry.id == option_id), None) - if not isinstance(option, SessionConfigOptionSelect): + option = find_select(catalog, option_id) + if option is None: raise ACPConfigError( session_id=session_id, option_id=option_id, selected_value=selected_value, - message=f'ACP config option "{option_id}" is not available.', + message=( + f'ACP session offers no config option "{option_id}"; ' + f"available: {listing(select_ids(catalog))}." + ), ) - if selected_value == option.current_value: - continue - - available_values = { - entry.value for entry in flatten_select_options(option.options) - } + # Sent even when the catalog already shows it: a set whose reply was + # lost leaves that catalog stale, and only the agent's reply is proof. + available_values = select_values(option) if selected_value not in available_values: raise ACPConfigError( session_id=session_id, @@ -128,7 +167,8 @@ async def apply_session_config_selections( selected_value=selected_value, message=( f'ACP config value "{selected_value}" is not advertised ' - f'for option "{option_id}".' + f'for option "{option_id}"; available: ' + f"{listing(available_values)}." ), ) @@ -140,7 +180,7 @@ async def apply_session_config_selections( except (asyncio.CancelledError, KeyboardInterrupt): raise except TimeoutError as error: - raise ACPConfigError( + raise ACPConfigUnreachableError( session_id=session_id, option_id=option_id, selected_value=selected_value, @@ -150,7 +190,13 @@ async def apply_session_config_selections( ), ) from error except Exception as error: - raise ACPConfigError( + # A JSON-RPC error or a malformed reply is still the agent's answer. + error_type = ( + ACPConfigError + if isinstance(error, (RequestError, ValidationError)) + else ACPConfigUnreachableError + ) + raise error_type( session_id=session_id, option_id=option_id, selected_value=selected_value, @@ -192,8 +238,8 @@ def refreshed_catalog( selected_value=selected_value, message=f'ACP config option "{option_id}" returned a malformed catalog.', ) - selected_option = next((entry for entry in catalog if entry.id == option_id), None) - if not isinstance(selected_option, SessionConfigOptionSelect): + selected_option = find_select(catalog, option_id) + if selected_option is None: raise ACPConfigError( session_id=session_id, option_id=option_id, diff --git a/src/band/runtime/oneshot.py b/src/band/runtime/oneshot.py index 3da775ca9..9f8889535 100644 --- a/src/band/runtime/oneshot.py +++ b/src/band/runtime/oneshot.py @@ -63,7 +63,6 @@ ) from band.logging_config import current_traceparent from band.platform.link import BandLink -from band.runtime.capabilities import prune_unsupported from band.runtime.context_serialization import context_item_to_dict from band.runtime.formatters import ( build_participants_message, @@ -71,6 +70,7 @@ replace_uuid_mentions, ) from band.runtime.participants import participant_snapshot +from band.runtime.startup import start_adapter from band.runtime.tools import AgentTools # BandLink.get_next_message returns this dataclass, not band.core.types' @@ -197,14 +197,12 @@ async def startup(self) -> None: "platform", self._link.to_platform_connection(self._agent_id), ) - if isinstance(self._adapter, SimpleAdapter): - # A bare FrameworkAdapter has no SUPPORTED_CAPABILITIES, so it - # cannot request a gated capability in the first place and takes - # no part in negotiation. - self._adapter.apply_effective_features( - prune_unsupported(self._adapter.features, self._feature_flags) - ) - await self._adapter.on_started(self._agent_name, self._agent_description) + await start_adapter( + self._adapter, + agent_name=self._agent_name, + agent_description=self._agent_description, + feature_flags=self._feature_flags, + ) self._started = True logger.info( "OneShotInvoker ready: agent_id=%s name=%s", diff --git a/src/band/runtime/startup.py b/src/band/runtime/startup.py new file mode 100644 index 000000000..a66185146 --- /dev/null +++ b/src/band/runtime/startup.py @@ -0,0 +1,53 @@ +"""Bring an adapter up for a started agent, the same way from every entry point.""" + +from __future__ import annotations + +import logging + +from band.core.protocols import FrameworkAdapter +from band.core.simple_adapter import SimpleAdapter +from band.runtime.capabilities import prune_unsupported + +logger = logging.getLogger(__name__) + + +async def start_adapter( + adapter: FrameworkAdapter | SimpleAdapter, + *, + agent_name: str, + agent_description: str, + feature_flags: dict[str, bool] | None, +) -> None: + """Start ``adapter`` for an agent whose deployment serves ``feature_flags``. + + A bare ``FrameworkAdapter`` has no ``SUPPORTED_CAPABILITIES`` to negotiate + and no model selection to check, so it only receives ``on_started``. A + failed start releases what the adapter acquired before re-raising. + """ + try: + if isinstance(adapter, SimpleAdapter): + await adapter.startup( + agent_name, + agent_description, + features=prune_unsupported(adapter.features, feature_flags), + ) + else: + await adapter.on_started(agent_name, agent_description) + except BaseException: + await release_adapter(adapter) + raise + + +async def release_adapter(adapter: FrameworkAdapter | SimpleAdapter) -> None: + """Best-effort ``cleanup_all``: a failure is logged, never raised, so it + cannot replace the error that made the caller release.""" + cleanup_all = getattr(adapter, "cleanup_all", None) + if cleanup_all is None: + return + try: + await cleanup_all() + except Exception: + logger.exception("Adapter cleanup_all failed") + + +__all__ = ["release_adapter", "start_adapter"] diff --git a/tests/adapters/test_copilot_acp_adapter.py b/tests/adapters/test_copilot_acp_adapter.py index ec4b00d2a..02de7ca3b 100644 --- a/tests/adapters/test_copilot_acp_adapter.py +++ b/tests/adapters/test_copilot_acp_adapter.py @@ -3,13 +3,18 @@ CopilotACPAdapter is a thin specialization of ACPClientAdapter — its contract is how a CopilotACPAdapterConfig maps onto the base adapter's transport, auth, and system-context wiring. Runtime/on_started behavior is covered by the generic ACP -client suite (tests/integrations/acp/), so these are construction-level tests. +client suite (tests/integrations/acp/); the model-selection tests drive the +adapter against an in-process fake shaped like Copilot's live catalog. """ from __future__ import annotations +import asyncio import logging +from collections.abc import AsyncIterator +from contextlib import asynccontextmanager from pathlib import Path +from typing import Any import pytest from pydantic import BaseModel @@ -19,9 +24,26 @@ CopilotACPAdapter, CopilotACPAdapterConfig, ) +from band.core.exceptions import BandConfigError +from band.core.model_catalog import ModelSelection +from band.integrations.acp import session_config from band.integrations.acp.client_adapter import ACPClientAdapter from band.integrations.acp.client_profiles import NoopACPClientProfile -from band.integrations.acp.session_config import ACPConfigRequest +from band.integrations.acp.client_types import ACPClientSessionState +from band.integrations.acp.session_config import ( + CONFIG_FAILURE_PREFIX, + ACPConfigRequest, +) +from tests.integrations.acp.acp_toolkit.agent import ( + EFFORT_OPTION_ID, + MODEL_OPTION_ID, + FakeACPAgent, +) +from tests.integrations.acp.acp_toolkit.harness import ( + DEFAULT_ROOM, + AcpSession, + started_acp_adapter, +) class TestCopilotACPAdapterConstruction: @@ -157,3 +179,436 @@ def test_tcp_with_auth_warns_before_rejection( ) ) assert any("ignored over TCP" in r.message for r in caplog.records) + + +# Copilot CLI 1.0.89's efforts per model (probed live). +COPILOT_EFFORTS = { + "claude-sonnet-5": ("low", "medium", "high", "xhigh", "max"), + "gpt-5.4": ("none", "low", "medium", "high", "xhigh"), + "claude-haiku-4.5": (), +} + + +def copilot(current: str = "claude-sonnet-5") -> FakeACPAgent: + """A fake ``copilot --acp`` advertising Copilot's model catalog.""" + return ( + FakeACPAgent(supports_session_load=True) + .advertises_models(COPILOT_EFFORTS, current=current) + .knows_session("persisted-session") + .will_say("Configured") + ) + + +@asynccontextmanager +async def copilot_room(agent: FakeACPAgent, **config: Any) -> AsyncIterator[AcpSession]: + adapter = CopilotACPAdapter( + CopilotACPAdapterConfig(inject_band_tools=False, **config) + ) + async with started_acp_adapter(adapter, agent) as session: + yield session + + +async def switch_room(session: AcpSession, **selection: str) -> None: + """Switch the live session of the room ``session.send`` talks to.""" + await session.adapter.apply_model_selection( + ModelSelection(**selection), room_id=DEFAULT_ROOM + ) + + +class TestCopilotACPModelSelection: + @pytest.mark.asyncio + @pytest.mark.parametrize("current", ["claude-sonnet-5", "claude-haiku-4.5"]) + async def test_model_then_effort_are_set_before_the_first_prompt( + self, current: str + ) -> None: + # From Haiku, the effort select only exists once gpt-5.4 is chosen. + agent = copilot(current) + + async with copilot_room( + agent, model="gpt-5.4", reasoning_effort="high" + ) as session: + reply = await session.send("Hello") + + assert reply.texts == ["Configured"] + assert agent.config_selections() == [ + (MODEL_OPTION_ID, "gpt-5.4"), + (EFFORT_OPTION_ID, "high"), + ] + + @pytest.mark.asyncio + async def test_a_restored_session_is_configured_too(self) -> None: + agent = copilot() + + async with copilot_room(agent, model="gpt-5.4") as session: + reply = await session.send( + "Resume", + bootstrap=True, + history=ACPClientSessionState( + room_to_session={"room-1": "persisted-session"} + ), + ) + + assert reply.texts == ["Configured"] + assert agent.config_option_requests == [ + ("persisted-session", MODEL_OPTION_ID, "gpt-5.4") + ] + + @pytest.mark.asyncio + @pytest.mark.parametrize( + ("config", "set_first", "error"), + [ + pytest.param( + {"model": "gpt-9"}, + [], + 'model "gpt-9" is not advertised; available: ' + "claude-sonnet-5, gpt-5.4, claude-haiku-4.5", + id="unknown-model", + ), + pytest.param( + {"model": "claude-haiku-4.5", "reasoning_effort": "high"}, + [(MODEL_OPTION_ID, "claude-haiku-4.5")], + 'model "claude-haiku-4.5" offers no reasoning effort', + id="model-without-efforts", + ), + pytest.param( + {"model": "gpt-5.4", "reasoning_effort": "max"}, + [(MODEL_OPTION_ID, "gpt-5.4")], + 'reasoning effort "max" is not advertised for model "gpt-5.4"; ' + "available: none, low, medium, high, xhigh", + id="effort-the-model-lacks", + ), + ], + ) + async def test_an_unadvertised_selection_fails_the_turn_naming_the_options( + self, + config: dict[str, str], + set_first: list[tuple[str, str]], + error: str, + ) -> None: + agent = copilot() + + async with copilot_room(agent, **config) as session: + reply = await session.send("Hello") + + assert reply.errors == [CONFIG_FAILURE_PREFIX + error] + assert agent.config_selections() == set_first + assert agent.prompt_texts() == [] + + @pytest.mark.asyncio + async def test_a_live_room_switches_against_its_current_catalog(self) -> None: + # Sonnet offers "high"; after switching to Haiku the room's catalog has + # no effort select, so a stale catalog would wrongly accept "high". + agent = copilot() + + async with copilot_room(agent) as session: + await session.send("Hello") + await switch_room(session, model="claude-haiku-4.5") + with pytest.raises(BandConfigError) as rejected: + await switch_room(session, reasoning_effort="high") + + assert str(rejected.value) == ( + 'model "claude-haiku-4.5" offers no reasoning effort' + ) + assert agent.config_selections() == [(MODEL_OPTION_ID, "claude-haiku-4.5")] + + @pytest.mark.asyncio + async def test_a_partly_applied_switch_does_not_carry_to_later_sessions( + self, + ) -> None: + # The model lands before the effort is refused; the switch raised, so + # the room's next session starts where it would have without it. + agent = copilot() + + async with copilot_room(agent, reasoning_effort="high") as session: + await session.send("Hello") + with pytest.raises(BandConfigError): + await switch_room(session, model="gpt-5.4", reasoning_effort="max") + await session.adapter.on_cleanup("room-1") + await session.send("Again") + + assert agent.config_selections("fake-session-2") == [(EFFORT_OPTION_ID, "high")] + + @pytest.mark.asyncio + async def test_an_empty_switch_leaves_the_room_on_its_configured_selection( + self, + ) -> None: + agent = copilot() + + async with copilot_room(agent, model="gpt-5.4") as session: + await session.send("Hello") + await agent.selects_on_its_own( + session.session_id("room-1"), MODEL_OPTION_ID, "claude-haiku-4.5" + ) + await switch_room(session) + await session.adapter.on_cleanup("room-1") + await session.send("Again") + + assert agent.config_selections("fake-session-2") == [ + (MODEL_OPTION_ID, "gpt-5.4") + ] + + @pytest.mark.asyncio + async def test_a_switch_follows_a_model_the_agent_chose_itself(self) -> None: + agent = copilot() + + @agent.on_prompt + async def pick_haiku(fake: FakeACPAgent, session_id: str) -> None: + await fake.selects_on_its_own( + session_id, MODEL_OPTION_ID, "claude-haiku-4.5" + ) + await fake.say(session_id, "Switched") + + async with copilot_room(agent) as session: + await session.send("Use haiku") + with pytest.raises(BandConfigError) as rejected: + await switch_room(session, reasoning_effort="high") + + assert str(rejected.value) == ( + 'model "claude-haiku-4.5" offers no reasoning effort' + ) + + @pytest.mark.asyncio + async def test_concurrent_switches_on_a_room_apply_in_call_order(self) -> None: + # Only gpt-5.4 offers "none", so the second switch is accepted only + # when checked against the catalog the first one left. + agent = copilot() + + async with copilot_room(agent) as session: + await session.send("Hello") + gate = agent.holds_config_replies() + first = asyncio.create_task(switch_room(session, model="gpt-5.4")) + await gate.received.wait() + second = asyncio.create_task(switch_room(session, reasoning_effort="none")) + gate.release.set() + await asyncio.gather(first, second) + + assert agent.config_selections() == [ + (MODEL_OPTION_ID, "gpt-5.4"), + (EFFORT_OPTION_ID, "none"), + ] + + @pytest.mark.asyncio + @pytest.mark.parametrize( + ("history", "next_session"), + [ + pytest.param(None, "fake-session-2", id="recreated"), + pytest.param( + ACPClientSessionState(room_to_session={"room-1": "persisted-session"}), + "persisted-session", + id="restored", + ), + ], + ) + async def test_a_switch_outlives_the_session_it_was_made_on( + self, history: ACPClientSessionState | None, next_session: str + ) -> None: + # The switch changes the model only, so the configured effort stays. + agent = copilot() + + async with copilot_room(agent, reasoning_effort="high") as session: + await session.send("Hello") + await switch_room(session, model="gpt-5.4") + await session.adapter.on_cleanup("room-1") + await session.send("Again", bootstrap=history is not None, history=history) + + assert agent.config_selections(next_session) == [ + (MODEL_OPTION_ID, "gpt-5.4"), + (EFFORT_OPTION_ID, "high"), + ] + + @pytest.mark.asyncio + async def test_a_switch_to_a_model_without_efforts_drops_the_configured_effort( + self, + ) -> None: + # Haiku offers no effort, so the room's next session must not ask for + # the configured "high". + agent = copilot() + + async with copilot_room( + agent, model="gpt-5.4", reasoning_effort="high" + ) as session: + await session.send("Hello") + await switch_room(session, model="claude-haiku-4.5") + await session.adapter.on_cleanup("room-1") + reply = await session.send("Again") + + assert reply.errors == [] + assert agent.config_selections("fake-session-2") == [ + (MODEL_OPTION_ID, "claude-haiku-4.5") + ] + + @pytest.mark.asyncio + async def test_a_remembered_switch_the_agent_refuses_fails_one_turn_only( + self, + ) -> None: + agent = copilot() + + async with copilot_room(agent) as session: + await session.send("Hello") + await switch_room(session, model="gpt-5.4") + await session.adapter.on_cleanup("room-1") + agent.advertises_models( + {"claude-sonnet-5": COPILOT_EFFORTS["claude-sonnet-5"]}, + current="claude-sonnet-5", + ) + refused = await session.send("Again") + recovered = await session.send("And again") + + assert refused.errors == [ + CONFIG_FAILURE_PREFIX + + 'model "gpt-5.4" is not advertised; available: claude-sonnet-5' + ] + assert recovered.texts == ["Configured"] + + @pytest.mark.asyncio + async def test_a_remembered_switch_survives_a_set_the_agent_never_answered( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr(session_config, "SESSION_CONFIG_TIMEOUT_SECONDS", 0.05) + agent = copilot() + + async with copilot_room(agent) as session: + await session.send("Hello") + await switch_room(session, model="gpt-5.4") + await session.adapter.on_cleanup("room-1") + gate = agent.holds_config_replies() + timed_out = await session.send("Again") + gate.release.set() + await session.adapter.on_cleanup("room-1") + await session.send("And again") + + assert "did not respond" in timed_out.errors[0] + assert agent.config_selections("fake-session-3")[0] == ( + MODEL_OPTION_ID, + "gpt-5.4", + ) + + @pytest.mark.asyncio + async def test_a_switch_refused_before_any_change_leaves_the_room_unpinned( + self, + ) -> None: + agent = copilot() + + async with copilot_room(agent) as session: + await session.send("Hello") + with pytest.raises(BandConfigError): + await switch_room(session, model="gpt-9") + await session.adapter.on_cleanup("room-1") + await session.send("Again") + + assert agent.config_selections("fake-session-2") == [] + + @pytest.mark.asyncio + async def test_a_switch_queued_behind_a_room_cleanup_names_the_ended_session( + self, + ) -> None: + agent = copilot() + + async with copilot_room(agent) as session: + await session.send("Hello") + gate = agent.holds_config_replies() + first = asyncio.create_task(switch_room(session, model="gpt-5.4")) + await gate.received.wait() + queued = asyncio.create_task(switch_room(session, model="claude-haiku-4.5")) + await asyncio.sleep(0) # one tick: the queued switch parks on the lock + await session.adapter.on_cleanup("room-1") + results = await asyncio.gather(first, queued, return_exceptions=True) + + assert isinstance(results[1], BandConfigError) + assert str(results[1]) == "room room-1's ACP session ended mid-switch" + + @pytest.mark.asyncio + async def test_a_cleanup_at_any_point_of_a_switch_names_the_ended_session( + self, + ) -> None: + # Sweeps where the cleanup lands: before the model reply, between the + # model and effort steps, and after the switch completes. + outcomes = {await self._switch_cleaned_up_after(ticks) for ticks in range(12)} + + assert outcomes <= {"ok", "room room-1's ACP session ended mid-switch"} + assert "room room-1's ACP session ended mid-switch" in outcomes + + @staticmethod + async def _switch_cleaned_up_after(ticks: int) -> str: + agent = copilot() + async with copilot_room(agent) as session: + await session.send("Hello") + gate = agent.holds_config_replies() + switch = asyncio.create_task( + switch_room(session, model="gpt-5.4", reasoning_effort="high") + ) + await gate.received.wait() + gate.release.set() + for _ in range(ticks): + await asyncio.sleep(0) + await session.adapter.on_cleanup(DEFAULT_ROOM) + (result,) = await asyncio.gather(switch, return_exceptions=True) + return "ok" if result is None else str(result) + + @pytest.mark.asyncio + async def test_a_setup_that_loses_its_connection_retries_on_the_next_turn( + self, + ) -> None: + agent = copilot() + + async with copilot_room(agent) as session: + await session.send("Hello") + await switch_room(session, model="gpt-5.4") + await session.adapter.on_cleanup(DEFAULT_ROOM) + agent.hangs_up_on_next_config_option() + lost = await session.send("Again") + healed = await session.send("And again") + + assert lost.errors == [CONFIG_FAILURE_PREFIX + "Connection closed"] + assert healed.texts == ["Configured"] + assert agent.config_selections("fake-session-3") == [ + (MODEL_OPTION_ID, "gpt-5.4"), + (EFFORT_OPTION_ID, "medium"), + ] + + @pytest.mark.asyncio + async def test_a_switch_leaves_other_rooms_on_the_configured_selection( + self, + ) -> None: + agent = copilot() + + async with copilot_room(agent) as session: + await session.send("Hello") + await switch_room(session, model="gpt-5.4") + await session.send("Hi", room="room-2") + + assert agent.config_selections("fake-session-2") == [] + + @pytest.mark.asyncio + async def test_a_switch_survives_an_adapter_restart(self) -> None: + agent = copilot() + + async with copilot_room(agent) as session: + await session.send("Hello") + await switch_room(session, model="gpt-5.4") + await session.adapter.cleanup_all() + await session.adapter.on_started("Fake Agent", "restarted") + await session.send("Again") + + assert agent.config_selections("fake-session-2") == [ + (MODEL_OPTION_ID, "gpt-5.4"), + (EFFORT_OPTION_ID, "medium"), + ] + + @pytest.mark.asyncio + async def test_switching_a_room_without_a_live_session_is_refused(self) -> None: + async with copilot_room(copilot()) as session: + with pytest.raises(BandConfigError, match="no live ACP session"): + await switch_room(session, model="gpt-5.4") + + def test_typed_selection_and_a_resolver_are_exclusive(self) -> None: + async def resolver(request: ACPConfigRequest) -> dict[str, str]: + del request + return {} + + with pytest.raises(ValueError, match="not both"): + CopilotACPAdapter( + CopilotACPAdapterConfig( + model="gpt-5.4", resolve_session_config=resolver + ) + ) diff --git a/tests/catalogs.py b/tests/catalogs.py new file mode 100644 index 000000000..d25abc4db --- /dev/null +++ b/tests/catalogs.py @@ -0,0 +1,45 @@ +"""An adapter whose harness advertises a model catalog, for start-up checks. + +The catalog is only reachable after ``on_started``, like a client that +``on_started`` launches. +""" + +from __future__ import annotations + +from band.core.model_catalog import ModelCatalog, ModelChoice, ModelSelection +from band.core.simple_adapter import SimpleAdapter + +SONNET_CATALOG = ModelCatalog(models=(ModelChoice(id="sonnet", efforts=("high",)),)) + + +class CatalogAdapter(SimpleAdapter[None]): + """Lists ``SONNET_CATALOG``, but only once ``on_started`` has run.""" + + def __init__( + self, selection: ModelSelection, *, cleanup_fails: bool = False + ) -> None: + super().__init__() + self._selection = selection + self._cleanup_fails = cleanup_fails + self._started_catalog: ModelCatalog | None = None + self.cleaned_up = False + + async def on_started(self, agent_name: str, agent_description: str) -> None: + await super().on_started(agent_name, agent_description) + self._started_catalog = SONNET_CATALOG + + @property + def model_selection(self) -> ModelSelection: + return self._selection + + async def list_models(self) -> ModelCatalog | None: + assert self._started_catalog is not None, "listed before on_started" + return self._started_catalog + + async def on_message(self, *args: object, **kwargs: object) -> None: + del args, kwargs + + async def cleanup_all(self) -> None: + self.cleaned_up = True + if self._cleanup_fails: + raise RuntimeError("cleanup failed") diff --git a/tests/core/test_model_catalog.py b/tests/core/test_model_catalog.py new file mode 100644 index 000000000..a598ddc99 --- /dev/null +++ b/tests/core/test_model_catalog.py @@ -0,0 +1,71 @@ +"""The one rule deciding whether a harness accepts a model/effort selection.""" + +from __future__ import annotations + +import pytest + +from band.core.model_catalog import ( + ModelCatalog, + ModelChoice, + ModelSelection, + ModelSelectionError, + ModelSetting, + check_model_selection, +) + +CATALOG = ModelCatalog( + models=( + ModelChoice(id="sonnet", efforts=("low", "high", "max")), + ModelChoice(id="haiku", efforts=()), + ModelChoice(id="gpt", efforts=None), + ), + current_model="sonnet", +) + + +def rejection(selection: ModelSelection) -> ModelSelectionError: + with pytest.raises(ModelSelectionError) as caught: + check_model_selection(selection, CATALOG) + return caught.value + + +@pytest.mark.parametrize( + "selection", + [ + ModelSelection(model="haiku"), + ModelSelection(model="sonnet", reasoning_effort="max"), + ModelSelection(reasoning_effort="low"), # against the current model + ModelSelection(model="gpt", reasoning_effort="anything"), # not reported + ], +) +def test_advertised_selections_pass(selection: ModelSelection) -> None: + check_model_selection(selection, CATALOG) + + +def test_anything_passes_when_the_harness_advertises_no_catalog() -> None: + check_model_selection(ModelSelection(model="x", reasoning_effort="y"), None) + + +def test_unknown_model_names_what_is_advertised() -> None: + error = rejection(ModelSelection(model="gpt-9")) + + assert (error.setting, error.value) == (ModelSetting.MODEL, "gpt-9") + assert ( + str(error) == 'model "gpt-9" is not advertised; available: sonnet, haiku, gpt' + ) + + +def test_effort_is_checked_against_the_selected_model_not_the_current_one() -> None: + error = rejection(ModelSelection(model="haiku", reasoning_effort="high")) + + assert (error.setting, error.value) == (ModelSetting.REASONING_EFFORT, "high") + assert str(error) == 'model "haiku" offers no reasoning effort' + + +def test_unadvertised_effort_names_the_models_efforts() -> None: + error = rejection(ModelSelection(reasoning_effort="xhigh")) + + assert str(error) == ( + 'reasoning effort "xhigh" is not advertised for model "sonnet"; ' + "available: low, high, max" + ) diff --git a/tests/core/test_simple_adapter.py b/tests/core/test_simple_adapter.py index 82f02267e..0aeacd9d7 100644 --- a/tests/core/test_simple_adapter.py +++ b/tests/core/test_simple_adapter.py @@ -5,6 +5,8 @@ import pytest +from band.core.exceptions import BandConfigError +from band.core.model_catalog import ModelSelection from band.core.protocols import FrameworkAdapter, HistoryConverter from band.core.simple_adapter import SimpleAdapter from band.core.types import AgentInput, HistoryProvider, PlatformMessage @@ -254,6 +256,31 @@ async def test_defaults_empty(self): assert adapter.agent_description == "" +class SelectingAdapter(RecordingAdapter): + """Configured with a model, but its harness lists no catalog.""" + + @property + def model_selection(self) -> ModelSelection: + return ModelSelection(model="any-model", reasoning_effort="any-effort") + + +class TestStartupModelCheck: + async def test_a_harness_without_a_catalog_starts_on_any_selection(self) -> None: + adapter = SelectingAdapter() + + await adapter.startup("Agent", "desc", features=adapter.features) + + assert adapter.agent_name == "Agent" + + +class TestApplyModelSelection: + async def test_an_adapter_that_cannot_switch_models_refuses(self) -> None: + with pytest.raises(BandConfigError, match="does not support switching"): + await RecordingAdapter().apply_model_selection( + ModelSelection(model="sonnet"), room_id="room-1" + ) + + class TestOnCleanup: """Tests for on_cleanup() lifecycle hook.""" diff --git a/tests/e2e/baseline/README.md b/tests/e2e/baseline/README.md index 288b80cb1..36861ec1b 100644 --- a/tests/e2e/baseline/README.md +++ b/tests/e2e/baseline/README.md @@ -573,7 +573,7 @@ the `dev` extra but are split out for isolation. | `core` | `dev` | anthropic, claude_sdk, agno, langgraph, pydantic_ai, copilot_sdk | provider keys (secrets); copilot_sdk self-downloads its CLI runtime and uses Anthropic BYOK without GitHub auth | | `crewai` | `dev-crewai` | crewai, crewai_flow | provider keys; isolated venv (crewai conflicts with `dev`'s deps — `pyproject.toml [tool.uv] conflicts`) | | `google` | `dev` | gemini, google_adk | provider keys; split from `core` so Google free-tier rate-limit flakiness is isolated | -| `backends` | `dev` | codex, opencode, copilot_acp | the CLI/server coding agents in one job: the `codex` CLI + login + a disposable `CODEX_CWD` (+ the codex-acp e2e), a running `opencode serve` (`OPENCODE_BASE_URL`, gating `bash` to `ask` → `E2E_OPENCODE_BASH_ASKS`), and the `copilot` CLI (`Dep.COPILOT_CLI` + `Dep.ANTHROPIC` — Anthropic BYOK; optional `GITHUB_TOKEN` for the single Copilot-hosted auth smoke) | +| `backends` | `dev` | codex, opencode, copilot_acp | the CLI/server coding agents in one job: the `codex` CLI + login + a disposable `CODEX_CWD` (+ the codex-acp e2e), a running `opencode serve` (`OPENCODE_BASE_URL`, gating `bash` to `ask` → `E2E_OPENCODE_BASH_ASKS`), and the `copilot` CLI (`Dep.COPILOT_CLI` + `Dep.ANTHROPIC` — Anthropic BYOK; optional `GITHUB_TOKEN` for the Copilot-hosted smokes) | | `letta` | `dev` | letta | a self-hosted Letta server (docker — `.github/scripts/setup-letta.sh`); the adapter self-hosts its Band MCP server inside pytest (see "Letta lane" below). **Linux-only** (`LINUX_ONLY_LANES`) — no Windows cells | | `parlant` | `dev-parlant` | *(none — parlant is a bespoke smoke, not a registered matrix adapter; pinned via `@lane(Lane.PARLANT)`)* | provider keys; isolated venv (parlant's `griffe`/`griffelib` transitive deps collide with pydantic_ai's — `pyproject.toml [tool.uv] conflicts`); no server setup — the smoke spins up its own in-process Parlant server | diff --git a/tests/e2e/baseline/settings.py b/tests/e2e/baseline/settings.py index 7e71e1cf2..589490f04 100644 --- a/tests/e2e/baseline/settings.py +++ b/tests/e2e/baseline/settings.py @@ -174,15 +174,14 @@ class Backends(BaseSettings): omp_command: str = "" # OMP_COMMAND omp_model: str = "google/gemini-2.5-flash" # OMP_MODEL - # Copilot-hosted auth for the single non-BYOK smoke - # (test_copilot_acp.py::test_copilot_hosted_auth_replies); the BYOK matrix - # cells never read it. The smoke skips when unset. + # Copilot-hosted auth for the non-BYOK smokes in test_copilot_acp.py; the + # BYOK matrix cells never read it. Those smokes skip when unset. github_token: str = "" # GITHUB_TOKEN - # Pins that same smoke's model instead of leaving it to Copilot's `auto` - # picker, which can land on an expensive reasoning-tier model (observed: - # gpt-5.6-terra). gpt-5.6-luna is the cheapest model in the GPT-5.6 family - # while still agentic/tool-calling, so the one billed reply turn stays - # cheap and deterministic across runs. + # Those smokes' typed model (see hermetic_copilot_config) rather than + # Copilot's own pick, which can be an expensive reasoning-tier model + # (observed: gpt-5.6-terra). gpt-5.6-luna is the cheapest model in the + # GPT-5.6 family while still agentic/tool-calling, so their billed turns + # stay cheap and deterministic across runs. copilot_hosted_model: str = "gpt-5.6-luna" # COPILOT_HOSTED_MODEL diff --git a/tests/e2e/baseline/smoke/adapters/test_copilot_acp.py b/tests/e2e/baseline/smoke/adapters/test_copilot_acp.py index 25ec880c2..e1d092cce 100644 --- a/tests/e2e/baseline/smoke/adapters/test_copilot_acp.py +++ b/tests/e2e/baseline/smoke/adapters/test_copilot_acp.py @@ -8,12 +8,15 @@ from __future__ import annotations +from functools import partial from pathlib import Path from typing import Any import pytest -from band.core.types import MessageType +from band.converters.acp_client import ACPClientHistoryConverter +from band.core.model_catalog import ModelSelection +from band.core.types import MessageType, metadata_to_dict from tests.e2e.baseline.agents import Adapter, Lane, lane, with_adapters from tests.e2e.baseline.flaky import flaky_model from tests.e2e.baseline.requires import Dep, requires @@ -24,7 +27,8 @@ unique_marker, ) from tests.e2e.baseline.toolkit.builders import copilot_acp_env, copilot_home_dir -from tests.e2e.baseline.toolkit.capture import CaptureFactory +from tests.e2e.baseline.toolkit.capture import CaptureFactory, ReplyCapture +from tests.e2e.baseline.toolkit.observations.events import Tasks from tests.e2e.baseline.toolkit.provisioning import ( ProvisionedAgent, ResourceManager, @@ -129,8 +133,20 @@ async def test_acp_band_tool_result_is_a_single_clean_payload( band_results.assert_json_output() +def skip_without_github_token(settings: BaselineSettings) -> None: + """Hosted-auth coverage is optional; the BYOK cells are the lane's bar.""" + if not settings.backends.github_token: + pytest.skip("GITHUB_TOKEN unset — the Copilot-hosted smokes need one") + + def hermetic_copilot_config( - settings: BaselineSettings, work_dir: Path, *, hosted: bool = False + settings: BaselineSettings, + work_dir: Path, + *, + hosted: bool = False, + model: str | None = None, + reasoning_effort: str | None = None, + inject_band_tools: bool = True, ) -> Any: """A per-test ``CopilotACPAdapterConfig`` with a fresh cwd + ``COPILOT_HOME``. @@ -139,24 +155,28 @@ def hermetic_copilot_config( ``session/load`` deterministically miss. ``hosted=False`` (default) mirrors the registry builder's Anthropic BYOK env (``copilot_acp_env``); ``hosted=True`` omits it and authenticates with ``github_token`` — the - Copilot-hosted path production users run. The hosted env also pins - ``COPILOT_MODEL`` (``settings.backends.copilot_hosted_model``) so this - smoke's one billed turn uses a cheap, deterministic model instead of - Copilot's ``auto`` picker. + Copilot-hosted path production users run. ``model`` / ``reasoning_effort`` + set the typed selection the adapter applies from each session's + advertised catalog; a hosted ``model`` defaults to + ``settings.backends.copilot_hosted_model``, since the env's + ``COPILOT_MODEL`` does not pick an ACP session's model. + ``inject_band_tools=False`` leaves the agent without Band tools, so the + adapter posts its text as the room reply. """ from band.adapters.copilot_acp import ( # noqa: PLC0415 -- copilot_acp imports the acp (agent-client-protocol) extra at its own top level; not installed in every lane's venv CopilotACPAdapterConfig, ) home = copilot_home_dir(str(work_dir)) - hosted_env = { - "COPILOT_HOME": home, - "COPILOT_MODEL": settings.backends.copilot_hosted_model, - } + if hosted and model is None: + model = settings.backends.copilot_hosted_model kwargs: dict[str, Any] = { "cwd": str(work_dir), "custom_section": "Keep responses short and concise.", - "env": (hosted_env if hosted else copilot_acp_env(settings, home)), + "env": ({"COPILOT_HOME": home} if hosted else copilot_acp_env(settings, home)), + "model": model, + "reasoning_effort": reasoning_effort, + "inject_band_tools": inject_band_tools, } if hosted: kwargs["github_token"] = settings.backends.github_token @@ -165,39 +185,143 @@ def hermetic_copilot_config( return CopilotACPAdapterConfig(**kwargs) +async def reply_with( + capture: ReplyCapture, + *, + marker: str, + user_ops: UserOps, + identity: ProvisionedAgent, + settings: BaselineSettings, +) -> None: + """One room turn whose reply must carry ``marker``.""" + mark = capture.messages.snapshot() + mid = await user_ops.send_message( + capture.room_id, + f"Reply with one short sentence that includes the marker {marker}.", + mention_id=identity.id, + mention_name=identity.name, + ) + replies = await capture.wait_for_reply( + mid, identity.id, since=mark, deadline_s=settings.e2e_timeout + ) + replies.assert_contains_any([marker]) + + @lane(Lane.BACKENDS) # bespoke build exposes no framework; pin scheduling to backends @requires(Dep.COPILOT_CLI) @pytest.mark.timeout(extra=180) # Copilot CLI cold boot + hosted-auth handshake @pytest.mark.asyncio(loop_scope="session") +@pytest.mark.parametrize( + "reasoning_effort", + # A non-default effort copilot_hosted_model advertises (its default: "medium"). + [pytest.param(None, id="default"), pytest.param("high", id="typed-effort")], +) async def test_copilot_hosted_auth_replies( baseline_settings: BaselineSettings, resource_manager: ResourceManager, user_ops: UserOps, reply_capture: CaptureFactory, tmp_path: Any, + reasoning_effort: str | None, ) -> None: """One reply turn on Copilot-hosted auth (GITHUB_TOKEN, no BYOK). The matrix cells run Anthropic BYOK to spare the monthly Copilot-hosted - quota, but the hosted path is the one production users run — this single - cheap turn keeps it proven. Skips (not fails) without a token: hosted - auth is optional extra coverage, the BYOK cells are the lane's bar. + quota, but the hosted path is the one production users run — one cheap + turn per cell keeps it proven. Only hosted sessions advertise model/effort + selects, so the typed-effort cell proves a typed selection is accepted + against Copilot's live catalog. Band tools stay off: with them, a hosted + turn keeps calling band_send_message after its reply and never ends. """ from band.adapters.copilot_acp import ( # noqa: PLC0415 -- copilot_acp imports the acp (agent-client-protocol) extra at its own top level; not installed in every lane's venv CopilotACPAdapter, ) - if not baseline_settings.backends.github_token: - pytest.skip("GITHUB_TOKEN unset — the Copilot-hosted auth smoke needs one") + skip_without_github_token(baseline_settings) - marker = unique_marker("hosted") identity = await resource_manager.provision_agent("copilot-hosted-auth") room_id = await resource_manager.provision_room( title="e2e-copilot-hosted-auth", participants=[identity.id] ) adapter = CopilotACPAdapter( - hermetic_copilot_config(baseline_settings, tmp_path / "hosted", hosted=True) + hermetic_copilot_config( + baseline_settings, + tmp_path / "hosted", + hosted=True, + reasoning_effort=reasoning_effort, + inject_band_tools=False, + ) + ) + async with ( + running_agent(identity, adapter, baseline_settings), + reply_capture(room_id) as capture, + ): + await reply_with( + capture, + marker=unique_marker("hosted"), + user_ops=user_ops, + identity=identity, + settings=baseline_settings, + ) + + +@lane(Lane.BACKENDS) # bespoke build exposes no framework; pin scheduling to backends +@requires(Dep.COPILOT_CLI) +@pytest.mark.timeout(extra=180) # Copilot CLI cold boot +@pytest.mark.asyncio(loop_scope="session") +@pytest.mark.parametrize( + ("hosted", "selection", "failure"), + [ + # Under BYOK the provider env picks the model; no model select exists. + pytest.param( + False, + {"model": "not-a-model"}, + "ACP session advertises no model option; available:", + marks=requires(Dep.ANTHROPIC), + id="byok-model", + ), + # Hosted sessions advertise per-model efforts; the refusal proves the + # selection was checked against Copilot's live catalog. + pytest.param( + True, + {"reasoning_effort": "not-an-effort"}, + 'reasoning effort "not-an-effort" is not advertised for model', + id="hosted-effort", + ), + ], +) +async def test_copilot_turn_fails_loudly_on_an_unadvertised_selection( + baseline_settings: BaselineSettings, + resource_manager: ResourceManager, + user_ops: UserOps, + reply_capture: CaptureFactory, + tmp_path: Any, + hosted: bool, + selection: dict[str, str], + failure: str, +) -> None: + """A typed selection the session does not advertise fails the turn with a + visible error naming what it offers, rather than being ignored.""" + from band.adapters.copilot_acp import ( # noqa: PLC0415 -- copilot_acp imports the acp (agent-client-protocol) extra at its own top level; not installed in every lane's venv + CopilotACPAdapter, + ) + from band.integrations.acp.session_config import ( # noqa: PLC0415 -- same acp extra as above + CONFIG_FAILURE_PREFIX, + ) + + if hosted: + skip_without_github_token(baseline_settings) + + identity = await resource_manager.provision_agent("copilot-unadvertised") + room_id = await resource_manager.provision_room( + title="e2e-copilot-unadvertised", participants=[identity.id] + ) + + adapter = CopilotACPAdapter( + hermetic_copilot_config( + baseline_settings, tmp_path / "copilot", hosted=hosted, **selection + ) ) async with ( running_agent(identity, adapter, baseline_settings), @@ -205,14 +329,127 @@ async def test_copilot_hosted_auth_replies( ): mid = await user_ops.send_message( room_id, - f"Reply with one short sentence that includes the marker {marker}.", + "Reply with one short sentence.", mention_id=identity.id, mention_name=identity.name, ) - replies = await capture.wait_for_reply( - mid, identity.id, deadline_s=baseline_settings.e2e_timeout + await capture.wait_for_processed(mid, identity.id) + errors = await capture.errors(sender_id=identity.id) + + errors.assert_contains_any([CONFIG_FAILURE_PREFIX + failure]) + + +def room_session_id(tasks: Tasks, room_id: str) -> str: + """The ACP session the adapter last reported for ``room_id``, read the way + it reads one back to restore it.""" + reported = ( + ACPClientHistoryConverter() + .convert([{"metadata": metadata_to_dict(task.metadata)} for task in tasks]) + .room_to_session + ) + assert room_id in reported, f"no ACP session reported for room {room_id}" + return reported[room_id] + + +async def copilot_persisted_selection( + config: Any, *, room_id: str, session_id: str +) -> ModelSelection: + """What a fresh ``copilot --acp`` reports ``session_id`` runs after + ``session/load``: Copilot's own record, not the adapter's. It is spawned + by a fresh adapter's own runtime wiring (launcher, env, workspace).""" + from band.adapters.copilot_acp import ( # noqa: PLC0415 -- the acp extra, as above + CopilotACPAdapter, + ) + from band.integrations.acp.model_selection import ( # noqa: PLC0415 -- the acp extra, as above + locate_model_options, + ) + from band.integrations.acp.session_config import ( # noqa: PLC0415 -- the acp extra, as above + session_config_options, + ) + + spawner = CopilotACPAdapter(config) + workspace = spawner._workspace(room_id) + runtime = spawner._build_runtime(workspace) + await runtime.start() + try: + loaded = await runtime.load_session_response( + cwd=workspace, session_id=session_id, mcp_servers=[] + ) + finally: + await runtime.stop() + assert loaded is not None, f"Copilot could not load session {session_id}" + return locate_model_options( + session_config_options(loaded) or () + ).current_selection() + + +# Probed as advertised; an account without them fails the switch naming +# what it offers. +SWITCHED = ModelSelection(model="gpt-5.4", reasoning_effort="low") + + +@lane(Lane.BACKENDS) # bespoke build exposes no framework; pin scheduling to backends +@requires(Dep.COPILOT_CLI) +# Two hosted turns plus a second Copilot CLI boot for the session/load check. +@pytest.mark.timeout(extra=300) +@pytest.mark.asyncio(loop_scope="session") +async def test_copilot_switches_model_and_effort_in_a_live_room( + baseline_settings: BaselineSettings, + resource_manager: ResourceManager, + user_ops: UserOps, + reply_capture: CaptureFactory, + tmp_path: Any, +) -> None: + """A runtime switch changes what Copilot itself runs for the room, and the + room keeps replying on it. + + The check is Copilot's persisted session, read by a fresh process, so a + switch that only updated the adapter's own record, or an effort Copilot + silently ignored, fails. Band tools stay off: with them, a hosted turn + keeps calling band_send_message after its reply and never ends. + """ + from band.adapters.copilot_acp import ( # noqa: PLC0415 -- copilot_acp imports the acp (agent-client-protocol) extra at its own top level; not installed in every lane's venv + CopilotACPAdapter, + ) + + skip_without_github_token(baseline_settings) + + identity = await resource_manager.provision_agent("copilot-live-switch") + room_id = await resource_manager.provision_room( + title="e2e-copilot-live-switch", participants=[identity.id] + ) + config = hermetic_copilot_config( + baseline_settings, + tmp_path / "copilot", + hosted=True, + reasoning_effort="high", + inject_band_tools=False, + ) + # Only a switch away from what the session starts on proves anything. + assert config.model != SWITCHED.model, "COPILOT_HOSTED_MODEL is the switch target" + adapter = CopilotACPAdapter(config) + turn = partial( + reply_with, user_ops=user_ops, identity=identity, settings=baseline_settings + ) + + async with ( + running_agent(identity, adapter, baseline_settings), + reply_capture(room_id) as capture, + ): + await turn(capture, marker=unique_marker("before-switch")) + await adapter.apply_model_selection(SWITCHED, room_id=room_id) + # Also the prompt Copilot persists the switched session on. + await turn(capture, marker=unique_marker("after-switch")) + session_id = room_session_id( + await capture.tasks(sender_id=identity.id), room_id ) - replies.assert_contains_any([marker]) + + assert ( + await copilot_persisted_selection( + config, room_id=room_id, session_id=session_id + ) + == SWITCHED + ) @lane(Lane.BACKENDS) # bespoke build exposes no framework; pin scheduling to backends diff --git a/tests/integrations/acp/acp_toolkit/__init__.py b/tests/integrations/acp/acp_toolkit/__init__.py index c4ef7b275..1ddaaf7cb 100644 --- a/tests/integrations/acp/acp_toolkit/__init__.py +++ b/tests/integrations/acp/acp_toolkit/__init__.py @@ -22,8 +22,13 @@ from __future__ import annotations -from tests.integrations.acp.acp_toolkit.agent import FakeACPAgent, PromptHandler +from tests.integrations.acp.acp_toolkit.agent import ( + FakeACPAgent, + PromptHandler, + select_option, +) from tests.integrations.acp.acp_toolkit.harness import ( + DEFAULT_ROOM, AcpSession, FakeSpawn, Reply, @@ -33,9 +38,11 @@ inject_acp_spawn, live_line, make_acp_connection, + started_acp_adapter, ) __all__ = [ + "DEFAULT_ROOM", "AcpSession", "FakeACPAgent", "FakeSpawn", @@ -47,4 +54,6 @@ "inject_acp_spawn", "live_line", "make_acp_connection", + "select_option", + "started_acp_adapter", ] diff --git a/tests/integrations/acp/acp_toolkit/agent.py b/tests/integrations/acp/acp_toolkit/agent.py index ca554847f..688f5ff5c 100644 --- a/tests/integrations/acp/acp_toolkit/agent.py +++ b/tests/integrations/acp/acp_toolkit/agent.py @@ -2,7 +2,9 @@ from __future__ import annotations -from collections.abc import Awaitable, Callable, Sequence +import asyncio +from collections.abc import Awaitable, Callable, Iterable, Mapping, Sequence +from dataclasses import dataclass, field from typing import Any from acp import RequestError @@ -19,6 +21,7 @@ ) from acp.schema import ( AgentCapabilities, + ConfigOptionUpdate, InitializeResponse, LoadSessionResponse, McpCapabilities, @@ -28,19 +31,36 @@ SessionCapabilities, SessionCloseCapabilities, SessionConfigOptionSelect, + SessionConfigSelectOption, SetSessionConfigOptionResponse, ToolCallUpdate, ) from mcp import ClientSession from mcp.client.streamable_http import streamable_http_client -from band.integrations.acp.session_config import SessionConfigOption +from band.integrations.acp.model_selection import ( + MODEL_CATEGORY, + THOUGHT_LEVEL_CATEGORY, +) +from band.integrations.acp.session_config import SessionConfigOption, find_select PromptHandler = Callable[["FakeACPAgent", str], Awaitable[None]] ConfigOptionHandler = Callable[ ["FakeACPAgent", str, str, str], Awaitable[Sequence[SessionConfigOption]] ] +# The option ids Copilot CLI publishes its model and effort selects under. +MODEL_OPTION_ID = "model" +EFFORT_OPTION_ID = "reasoning_effort" + + +@dataclass +class ReplyGate: + """Holds ``set_config_option`` replies: each is applied, then waits.""" + + received: asyncio.Event = field(default_factory=asyncio.Event) + release: asyncio.Event = field(default_factory=asyncio.Event) + class FakeACPAgent: """A scripted, in-process ACP agent (the peer an ACP client drives). @@ -76,6 +96,10 @@ def __init__( self._custom: PromptHandler | None = None self._config_options = list(config_options) self._config_option_handler: ConfigOptionHandler | None = None + self._reply_gate: ReplyGate | None = None + self._hangs_up_on_config = False + # Closes this agent's end of the transport; the harness binds it. + self.hang_up: Callable[[], None] = lambda: None # Observability for assertions: self.sessions: list[dict[str, Any]] = [] self._mcp_servers_by_session: dict[str, list[Any]] = {} @@ -103,6 +127,75 @@ def on_config_option(self, handler: ConfigOptionHandler) -> ConfigOptionHandler: self._config_option_handler = handler return handler + def hangs_up_on_next_config_option(self) -> None: + """Drop the connection on the next ``set_config_option``, unanswered, + as a crashed agent process does.""" + self._hangs_up_on_config = True + + def holds_config_replies(self) -> ReplyGate: + """Apply each ``set_config_option`` at once but reply only on release.""" + self._reply_gate = ReplyGate() + return self._reply_gate + + def advertises_models( + self, + efforts_by_model: Mapping[str, Sequence[str]], + *, + current: str, + default_effort: str = "medium", + pushes_updates: bool = True, + ) -> FakeACPAgent: + """Advertise a model select whose effort select follows the chosen model. + + Mirrors Copilot CLI 1.0.89 (probed live): selecting a model replaces + the effort select with that model's levels, or drops it for a model + with none; the current effort carries over when the new model offers + it, else falls back to ``default_effort``; an unoffered effort is + silently ignored; and every change is also pushed as a + ``config_option_update``. ``pushes_updates=False`` models a spec agent + whose set replies are the only record of a change. + """ + active = {"model": current, "effort": default_effort} + + def catalog() -> list[SessionConfigOption]: + options: list[SessionConfigOption] = [ + select_option( + MODEL_OPTION_ID, + active["model"], + efforts_by_model, + category=MODEL_CATEGORY, + ) + ] + efforts = efforts_by_model[active["model"]] + if efforts: + if active["effort"] not in efforts: + active["effort"] = default_effort + options.append( + select_option( + EFFORT_OPTION_ID, + active["effort"], + efforts, + category=THOUGHT_LEVEL_CATEGORY, + ) + ) + return options + + @self.on_config_option + async def select( + fake: FakeACPAgent, session_id: str, option_id: str, value: str + ) -> list[SessionConfigOption]: + if option_id == MODEL_OPTION_ID: + active["model"] = value + elif value in efforts_by_model[active["model"]]: + active["effort"] = value + options = catalog() + if pushes_updates: + await fake.push_config_options(session_id, options) + return options + + self._config_options = catalog() + return self + def will_say(self, text: str) -> FakeACPAgent: self._script.append(lambda a, sid: a.say(sid, text)) return self @@ -284,6 +377,24 @@ def _conn_for(self, session_id: str) -> AgentSideConnection: async def emit(self, session_id: str, update: Any) -> None: await self._conn_for(session_id).session_update(session_id, update) + async def selects_on_its_own( + self, session_id: str, option_id: str, value: str + ) -> None: + """Change an option agent-side and push ``config_option_update``, as + Copilot's in-session ``/model`` does.""" + self._config_options = await self._select(session_id, option_id, value) + await self.push_config_options(session_id, self._config_options) + + async def push_config_options( + self, session_id: str, options: Sequence[SessionConfigOption] + ) -> None: + await self.emit( + session_id, + ConfigOptionUpdate( + session_update="config_option_update", config_options=list(options) + ), + ) + async def ask_permission( self, session_id: str, tool_call: Any, options: list[Any] ) -> Any: @@ -389,26 +500,49 @@ async def set_config_option( """Apply one advertised select option and return the full live catalog.""" del kwargs self.config_option_requests.append((session_id, config_id, value)) + if self._hangs_up_on_config: + self._hangs_up_on_config = False + self.hang_up() + await asyncio.Event().wait() + self._config_options = await self._select(session_id, config_id, value) + if self._reply_gate is not None: + self._reply_gate.received.set() + await self._reply_gate.release.wait() + return SetSessionConfigOptionResponse(config_options=self._config_options) + + async def _select( + self, session_id: str, option_id: str, value: str + ) -> list[SessionConfigOption]: if self._config_option_handler is not None: - self._config_options = list( - await self._config_option_handler(self, session_id, config_id, value) + return list( + await self._config_option_handler(self, session_id, option_id, value) ) - return SetSessionConfigOptionResponse(config_options=self._config_options) - - updated: list[SessionConfigOption] = [] - for option in self._config_options: - if option.id == config_id and isinstance(option, SessionConfigOptionSelect): - updated.append(option.model_copy(update={"current_value": value})) - else: - updated.append(option) - self._config_options = updated - return SetSessionConfigOptionResponse(config_options=self._config_options) + return [ + option.model_copy(update={"current_value": value}) + if option.id == option_id and isinstance(option, SessionConfigOptionSelect) + else option + for option in self._config_options + ] + + def current_value(self, option_id: str) -> str | None: + """The value the agent currently has selected for ``option_id``.""" + option = find_select(self._config_options, option_id) + return option.current_value if option is not None else None async def close_session(self, session_id: str, **kwargs: Any) -> None: """Record that the client closed a session before prompting it.""" del kwargs self.closed_sessions.append(session_id) + def config_selections(self, session_id: str | None = None) -> list[tuple[str, str]]: + """Each ``(option, value)`` the client set, in order, on ``session_id`` + or across sessions.""" + return [ + (option, value) + for sid, option, value in self.config_option_requests + if session_id in (None, sid) + ] + def prompt_texts(self) -> list[str]: """Each received prompt's text, one string per prompt, in arrival order.""" return [ @@ -435,3 +569,24 @@ async def prompt( for action in self._script: await action(self, session_id) return PromptResponse(stop_reason="end_turn") + + +def select_option( + option_id: str, + current_value: str, + values: Iterable[str], + *, + category: str | None = None, +) -> SessionConfigOptionSelect: + """A concise ACP select catalog entry, optionally in a spec category.""" + return SessionConfigOptionSelect( + id=option_id, + name=option_id.replace("_", " ").title(), + type="select", + category=category, + current_value=current_value, + options=[ + SessionConfigSelectOption(value=value, name=value.title()) + for value in values + ], + ) diff --git a/tests/integrations/acp/acp_toolkit/harness.py b/tests/integrations/acp/acp_toolkit/harness.py index 27a8cbd9e..6c54e3e1a 100644 --- a/tests/integrations/acp/acp_toolkit/harness.py +++ b/tests/integrations/acp/acp_toolkit/harness.py @@ -24,6 +24,9 @@ from band.testing import FakeAgentTools from tests.integrations.acp.acp_toolkit.agent import FakeACPAgent +# The room ``AcpSession.send`` talks to unless told otherwise. +DEFAULT_ROOM = "room-1" + _SESSION_EVENT_MARKER = "acp_client_session_id" # the adapter's trailing task event @@ -182,6 +185,10 @@ def texts(self) -> list[str]: def thoughts(self) -> list[str]: return [e["content"] for e in self._events_of("thought")] + @property + def errors(self) -> list[str]: + return [e["content"] for e in self._events_of("error")] + @property def tool_calls(self) -> list[dict[str, Any]]: return [ @@ -258,7 +265,7 @@ async def send( self, content: str, *, - room: str = "room-1", + room: str = DEFAULT_ROOM, history: ACPClientSessionState | None = None, bootstrap: bool = False, room_context: list[dict[str, Any]] | None = None, @@ -310,6 +317,18 @@ async def acp_adapter( inject_band_tools=inject_band_tools, **adapter_kwargs, ) + async with started_acp_adapter(adapter, agent) as session: + yield session + + +@asynccontextmanager +async def started_acp_adapter( + adapter: ACPClientAdapter, agent: FakeACPAgent +) -> AsyncIterator[AcpSession]: + """Start any ``ACPClientAdapter`` (or subclass) wired to ``agent`` in process. + + Yields an :class:`AcpSession`; tears the adapter down on exit. + """ pair_in_process(adapter, agent) await adapter.on_started("Fake Agent", "in-process fake") try: @@ -347,6 +366,7 @@ async def _spawn( use_unstable_protocol=True, ) conn = connect_to_agent(client, writer_c, reader_c) + agent.hang_up = writer_a.close try: yield conn, agent_conn finally: diff --git a/tests/integrations/acp/test_client_runtime.py b/tests/integrations/acp/test_client_runtime.py index 8ec9eb972..12fae5db6 100644 --- a/tests/integrations/acp/test_client_runtime.py +++ b/tests/integrations/acp/test_client_runtime.py @@ -10,7 +10,11 @@ import pytest from acp.client.connection import ClientSideConnection from acp.exceptions import RequestError -from acp.schema import ClientCapabilities, DeclineElicitationResponse +from acp.schema import ( + ClientCapabilities, + DeclineElicitationResponse, + NewSessionResponse, +) from band.integrations.acp.client_profiles import ( CursorACPClientProfile, @@ -23,7 +27,9 @@ select_allow_option_id, tcp_spawn_process, ) +from band.integrations.acp.session_config import select_ids from band.integrations.acp.types import ChunkType, CollectedChunk +from tests.integrations.acp.acp_toolkit import FakeSpawn, select_option class TestSelectAllowOptionId: @@ -835,6 +841,32 @@ async def test_create_session_and_prompt_use_active_connection(self) -> None: mock_conn.new_session.assert_awaited_once_with(cwd="/tmp", mcp_servers=[]) mock_conn.prompt.assert_awaited_once() + @pytest.mark.asyncio + @pytest.mark.parametrize("ended_by", ["stop", "close_session"]) + async def test_an_ended_session_keeps_no_catalog(self, ended_by: str) -> None: + # Session ids belong to the agent process; a respawn must not reuse + # what the stopped process advertised. + spawn = FakeSpawn() + spawn.conn.new_session = AsyncMock( + return_value=NewSessionResponse( + session_id="sess-1", + config_options=[select_option("model", "small", ["small", "large"])], + ) + ) + runtime = ACPRuntime(command=["codex"], spawn_process=spawn) + await runtime.start() + await runtime.create_session_response(cwd="/tmp", mcp_servers=[]) + advertised = runtime.config_options("sess-1") + + match ended_by: + case "stop": + await runtime.stop() + case "close_session": + await runtime.close_session("sess-1") + + assert select_ids(advertised) == ("model",) + assert runtime.config_options("sess-1") == () + @pytest.mark.asyncio async def test_close_session_uses_the_active_connection_when_supported( self, diff --git a/tests/integrations/acp/test_session_config.py b/tests/integrations/acp/test_session_config.py index 3aa6f64d3..0c858d79d 100644 --- a/tests/integrations/acp/test_session_config.py +++ b/tests/integrations/acp/test_session_config.py @@ -3,10 +3,12 @@ from __future__ import annotations import asyncio +from collections.abc import Sequence from types import SimpleNamespace from unittest.mock import AsyncMock import pytest +from acp.exceptions import RequestError from acp.schema import ( SessionConfigOptionSelect, SessionConfigSelectGroup, @@ -14,33 +16,29 @@ SetSessionConfigOptionResponse, ) +from band.core.exceptions import BandConfigError +from band.core.model_catalog import ModelCatalog, ModelChoice, ModelSelection +from band.integrations.acp import session_config +from band.integrations.acp.client_adapter import ACPClientAdapter from band.integrations.acp.client_types import ACPClientSessionState +from band.integrations.acp.model_selection import MODEL_CATEGORY, ACPModelOptions from band.integrations.acp.session_config import ( + CONFIG_FAILURE_PREFIX, RESOLVER_CONFIG_OPTION_ID, ACPConfigError, ACPConfigRequest, + ACPConfigUnreachableError, SessionConfigOption, apply_session_config_selections, + find_select, +) +from tests.integrations.acp.acp_toolkit import ( + FakeACPAgent, + Reply, + acp_adapter, + select_option, + started_acp_adapter, ) -from tests.integrations.acp.acp_toolkit import FakeACPAgent, Reply, acp_adapter - - -def select_option( - option_id: str, - current_value: str, - values: list[str], -) -> SessionConfigOptionSelect: - """A concise ACP select catalog entry for one test.""" - return SessionConfigOptionSelect( - id=option_id, - name=option_id.replace("_", " ").title(), - type="select", - current_value=current_value, - options=[ - SessionConfigSelectOption(value=value, name=value.title()) - for value in values - ], - ) def malformed_catalog_response() -> SimpleNamespace: @@ -136,7 +134,7 @@ async def test_fails_when_a_prior_selection_removes_a_later_effort_option( ) ) - with pytest.raises(ACPConfigError, match='"reasoning_effort" is not available'): + with pytest.raises(ACPConfigError) as rejected: await apply_session_config_selections( session_id="session-1", config_options=[model, effort], @@ -144,8 +142,56 @@ async def test_fails_when_a_prior_selection_removes_a_later_effort_option( set_option=set_option, ) + assert str(rejected.value) == ( + 'ACP session offers no config option "reasoning_effort"; available: model.' + ) set_option.assert_awaited_once_with("session-1", "model", "auto") + @pytest.mark.asyncio + @pytest.mark.parametrize( + ("failure", "raised"), + [ + pytest.param(RequestError.invalid_params(), ACPConfigError, id="refused"), + # The acp client validates each reply; a null one fails that. + pytest.param( + lambda *_: SetSessionConfigOptionResponse.model_validate({}), + ACPConfigError, + id="malformed-reply", + ), + pytest.param( + RuntimeError("Connection closed"), + ACPConfigUnreachableError, + id="unanswered", + ), + ], + ) + async def test_a_failed_set_says_whether_the_agent_answered( + self, failure: object, raised: type[ACPConfigError] + ) -> None: + with pytest.raises(ACPConfigError) as rejected: + await apply_session_config_selections( + session_id="session-1", + config_options=[select_option("model", "small", ["small", "large"])], + selections={"model": "large"}, + set_option=AsyncMock(side_effect=failure), + ) + + assert type(rejected.value) is raised + + @pytest.mark.asyncio + async def test_an_option_on_an_empty_catalog_is_refused_naming_none(self) -> None: + with pytest.raises(ACPConfigError) as rejected: + await apply_session_config_selections( + session_id="session-1", + config_options=[], + selections={"model": "auto"}, + set_option=AsyncMock(), + ) + + assert str(rejected.value) == ( + 'ACP session offers no config option "model"; available: (none).' + ) + @pytest.mark.asyncio async def test_rejects_a_malformed_refreshed_catalog(self) -> None: effort = select_option("reasoning_effort", "medium", ["medium", "high"]) @@ -239,6 +285,13 @@ async def resolve_config(request: ACPConfigRequest) -> dict[str, str]: "selected_value": "unsupported", }, ) + assert reply.errors == [ + ( + f"{CONFIG_FAILURE_PREFIX}ACP config value " + '"unsupported" is not advertised for option "reasoning_effort"; ' + "available: medium, high." + ) + ] assert agent.prompt_texts() == [] assert agent.closed_sessions == ["fake-session-1"] @@ -457,3 +510,153 @@ async def resolve_config(request: ACPConfigRequest) -> dict[str, str]: }, ) assert agent.closed_sessions == ["persisted-session"] + + +class ThinkingIdAdapter(ACPClientAdapter): + """An ACP adapter for an agent that publishes uncategorized ``model`` and + ``thinking`` selects, the way OMP does.""" + + def locate_model_options( + self, options: Sequence[SessionConfigOption] + ) -> ACPModelOptions: + return ACPModelOptions( + model=find_select(options, "model"), effort=find_select(options, "thinking") + ) + + +class TestTypedModelSelection: + def test_the_catalog_knows_efforts_only_for_the_current_model(self) -> None: + # ACP advertises efforts for the active model alone: the others are + # unknown (None), which a host must not show as "offers none" (()). + located = ACPModelOptions( + model=select_option("model", "large", ["small", "large"]), + effort=select_option("thinking", "low", ["low", "high"]), + ) + + assert located.model_catalog() == ModelCatalog( + models=( + ModelChoice(id="small", label="Small", efforts=None), + ModelChoice( + id="large", + label="Large", + efforts=("low", "high"), + default_effort="low", + ), + ), + current_model="large", + ) + + @pytest.mark.asyncio + async def test_an_adapter_can_locate_selects_that_carry_no_category( + self, + ) -> None: + agent = FakeACPAgent( + config_options=[ + select_option("model", "small", ["small", "large"]), + select_option("thinking", "low", ["low", "high"]), + ] + ).will_say("Configured") + adapter = ThinkingIdAdapter( + command="fake-agent", + inject_band_tools=False, + model_selection=ModelSelection(model="large", reasoning_effort="high"), + ) + + async with started_acp_adapter(adapter, agent) as session: + reply = await session.send("Hello") + + assert reply.texts == ["Configured"] + assert agent.config_selections() == [("model", "large"), ("thinking", "high")] + + @pytest.mark.asyncio + async def test_a_switch_is_checked_against_the_catalog_a_set_reply_returned( + self, + ) -> None: + # No config_option_update is pushed, so only the model set's reply + # tells the client that "large" brings a "max" effort. + agent = ( + FakeACPAgent() + .advertises_models( + {"small": (), "large": ("medium", "max")}, + current="small", + pushes_updates=False, + ) + .will_say("ok") + ) + + async with acp_adapter(agent) as session: + await session.send("Hello") + for selection in ( + ModelSelection(model="large"), + ModelSelection(reasoning_effort="max"), + ): + await session.adapter.apply_model_selection(selection, room_id="room-1") + + assert agent.current_value("reasoning_effort") == "max" + + @pytest.mark.asyncio + async def test_a_runtime_switch_is_refused_beside_a_resolver(self) -> None: + async def resolve_config(request: ACPConfigRequest) -> None: + del request + + agent = FakeACPAgent( + config_options=[ + select_option( + "model", "small", ["small", "large"], category=MODEL_CATEGORY + ) + ] + ).will_say("ok") + + async with acp_adapter(agent, resolve_session_config=resolve_config) as session: + await session.send("Hello") + with pytest.raises(BandConfigError, match="resolve_session_config"): + await session.adapter.apply_model_selection( + ModelSelection(model="large"), room_id="room-1" + ) + + assert agent.config_selections() == [] + + @pytest.mark.asyncio + async def test_a_switch_back_after_a_timed_out_switch_reaches_the_agent( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + # The agent applies "large" but its reply misses the deadline, so the + # client never sees the session leave "small". + monkeypatch.setattr(session_config, "SESSION_CONFIG_TIMEOUT_SECONDS", 0.05) + agent = FakeACPAgent( + config_options=[ + select_option( + "model", "small", ["small", "large"], category=MODEL_CATEGORY + ) + ] + ).will_say("ok") + + async with acp_adapter(agent) as session: + await session.send("Hello") + gate = agent.holds_config_replies() + with pytest.raises(BandConfigError, match="did not respond"): + await session.adapter.apply_model_selection( + ModelSelection(model="large"), room_id="room-1" + ) + gate.release.set() + await session.adapter.apply_model_selection( + ModelSelection(model="small"), room_id="room-1" + ) + + assert agent.current_value("model") == "small" + + @pytest.mark.asyncio + async def test_a_model_is_refused_when_the_agent_advertises_no_model_option( + self, + ) -> None: + agent = FakeACPAgent().will_say("unreachable") + + async with acp_adapter( + agent, model_selection=ModelSelection(model="large") + ) as session: + reply = await session.send("Hello") + + assert reply.errors == [ + f"{CONFIG_FAILURE_PREFIX}ACP session advertises no model option; available: (none)." + ] + assert agent.prompt_texts() == [] diff --git a/tests/runtime/test_oneshot.py b/tests/runtime/test_oneshot.py index 9dbfa874f..9029966e7 100644 --- a/tests/runtime/test_oneshot.py +++ b/tests/runtime/test_oneshot.py @@ -23,6 +23,7 @@ from pydantic import BaseModel, Field from band.adapters.anthropic import AnthropicAdapter +from band.core.model_catalog import ModelSelection, ModelSelectionError from band.core.simple_adapter import SimpleAdapter from band.core.types import Capability from band.runtime.capabilities import FeatureFlag @@ -34,6 +35,7 @@ _lookup_sender_name, _parse_inserted_at, ) +from tests.catalogs import CatalogAdapter from tests.runtime.conftest import ctx_item, make_link_mock, platform_msg @@ -232,6 +234,15 @@ async def test_keeps_files_capability_when_flag_on(self) -> None: assert Capability.FILES in adapter.features.capabilities + async def test_startup_rejects_a_model_the_harness_does_not_offer(self) -> None: + adapter = CatalogAdapter(ModelSelection(model="gpt-9")) + invoker = OneShotInvoker(link=make_link_mock(), adapter=adapter, agent_id="a") + + with pytest.raises(ModelSelectionError, match='model "gpt-9"'): + await invoker.startup() + + assert adapter.cleaned_up + async def test_startup_is_idempotent(self) -> None: link = make_link_mock() adapter = _make_adapter_mock() diff --git a/tests/test_agent.py b/tests/test_agent.py index 86abd6f68..23e62d78b 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -12,13 +12,15 @@ ParticipantAddedPayload, RoomAddedPayload, ) +from band.core.model_catalog import ModelSelection, ModelSelectionError from band.core.simple_adapter import SimpleAdapter -from band.core.types import AdapterFeatures, AgentInput, Capability +from band.core.types import AgentInput, Capability from band.platform.event import MessageEvent, ParticipantAddedEvent, RoomAddedEvent from band.preprocessing.default import DefaultPreprocessor from band.runtime.capabilities import FeatureFlag from band.runtime.types import AgentConfig, ConversationContext, SessionConfig from band.testing.platform import platform_connection_stub +from tests.catalogs import CatalogAdapter @pytest.fixture @@ -423,19 +425,23 @@ class TestSimpleAdapterIntegration: @pytest.mark.asyncio async def test_works_with_simple_adapter(self, mock_runtime): - """Should work with SimpleAdapter subclass.""" - # Create a mock SimpleAdapter - adapter = MagicMock(spec=SimpleAdapter) - adapter.on_started = AsyncMock() - adapter.on_cleanup = AsyncMock() - adapter.on_event = AsyncMock() - adapter.features = AdapterFeatures() - + """A SimpleAdapter subclass is started with the agent's identity.""" + adapter = MinimalAdapter() agent = Agent(runtime=mock_runtime, adapter=adapter) await agent.start() - adapter.on_started.assert_awaited_once() + assert (adapter.agent_name, adapter.agent_description) == ( + "TestBot", + "A test bot", + ) + + +class MinimalAdapter(SimpleAdapter): + """A SimpleAdapter that keeps every default hook.""" + + async def on_message(self, *args, **kwargs) -> None: + pass class FilesAdapter(SimpleAdapter): @@ -474,6 +480,43 @@ async def test_files_capability_kept_when_flag_on(self, mock_runtime): assert Capability.FILES in adapter.features.capabilities +class TestModelSelectionCheckOnStart: + """Agent.start() refuses a model selection the adapter's catalog lacks.""" + + @pytest.mark.asyncio + async def test_an_advertised_selection_starts(self, mock_runtime): + adapter = CatalogAdapter( + ModelSelection(model="sonnet", reasoning_effort="high") + ) + + await Agent(runtime=mock_runtime, adapter=adapter).start() + + mock_runtime.start.assert_awaited_once() + + @pytest.mark.asyncio + async def test_a_rejected_selection_fails_start_before_connecting( + self, mock_runtime + ): + adapter = CatalogAdapter(ModelSelection(model="gpt-9")) + + with pytest.raises(ModelSelectionError, match='model "gpt-9"'): + await Agent(runtime=mock_runtime, adapter=adapter).start() + + mock_runtime.start.assert_not_awaited() + assert adapter.cleaned_up + + @pytest.mark.asyncio + async def test_a_failing_release_does_not_mask_the_rejection( + self, mock_runtime, caplog: pytest.LogCaptureFixture + ): + adapter = CatalogAdapter(ModelSelection(model="gpt-9"), cleanup_fails=True) + + with pytest.raises(ModelSelectionError, match='model "gpt-9"'): + await Agent(runtime=mock_runtime, adapter=adapter).start() + + assert "Adapter cleanup_all failed" in caplog.text + + class TestDefaultPreprocessorIntegration: """Tests for DefaultPreprocessor behavior."""