diff --git a/docs/security.md b/docs/security.md index 55e56120..5bc1ca45 100644 --- a/docs/security.md +++ b/docs/security.md @@ -60,6 +60,8 @@ inspection. |----------|---------|-------------| | `SWML_BASIC_AUTH_USER` | `signalwire` | Basic auth username | | `SWML_BASIC_AUTH_PASSWORD` | *auto-generated* | Basic auth password (32-char token if not set) | +| `SIGNALWIRE_SIGNING_KEY` | *unset* | Signing Key for **inbound** webhook signature validation. See [Webhook Signature Validation](#webhook-signature-validation). | +| `SIGNALWIRE_SWAIG_SECRET` | *random per process* | Secret used to sign this agent's **outbound** SWAIG function tokens. See [SWAIG Function Token Signing](#swaig-function-token-signing). | ### Security Headers and Policies @@ -380,3 +382,48 @@ Before deploying to production: - [ ] SSL certificate expiration monitoring in place - [ ] `signing_key` (or `SIGNALWIRE_SIGNING_KEY` env) configured on every AgentBase - [ ] Regular security updates applied + +## SWAIG Function Token Signing + +Each SWAIG function call an agent hands to the AI carries a short-lived token +that the agent signs and later verifies itself, so a function URL cannot be +replayed or called out of context. + +By default that secret is **generated randomly per process**. Tokens therefore +stop verifying whenever the agent restarts, and every replica of the same agent +signs with a different key. + +The failure is easy to miss and points away from its cause. A call placed +before a restart keeps running; its next tool call arrives carrying a token the +new process cannot verify; and the caller is told: + +``` +the security token for this function is invalid or expired +``` + +which reads as though the tool failed, rather than as though it was never +allowed to run. Nothing errors server-side and nothing logs a mismatch. + +Set the secret explicitly whenever an agent restarts while calls are live — +which includes every rolling deploy — and always when more than one replica +serves the same agent: + + +```python +from signalwire import AgentBase + +agent = AgentBase( + name="my-agent", + swaig_secret="a-long-random-string", # or set SIGNALWIRE_SWAIG_SECRET +) +agent.serve() +``` + +Resolution order is the constructor argument, then `SIGNALWIRE_SWAIG_SECRET`, +then a fresh random secret. + +Treat it like any other signing secret: keep it out of source control, and use +the same value across every replica of one agent. It is unrelated to +`signing_key` / `SIGNALWIRE_SIGNING_KEY`, which validates **inbound** webhooks +and is issued by SignalWire; this one is the agent's own and never leaves the +process. diff --git a/signalwire/signalwire/ai_chat/__init__.py b/signalwire/signalwire/ai_chat/__init__.py index ad0671a1..250a855c 100644 --- a/signalwire/signalwire/ai_chat/__init__.py +++ b/signalwire/signalwire/ai_chat/__init__.py @@ -11,6 +11,7 @@ """ from .gateway import ChatGateway, GatewayRejection +from .handoff import HandoffRouter, NonceEntry from .client import ( AIChatClient, AIChatError, @@ -35,6 +36,8 @@ "ConversationInfo", "ConversationNotFoundError", "GatewayRejection", + "HandoffRouter", + "NonceEntry", "RateLimitError", "SummaryError", ] diff --git a/signalwire/signalwire/ai_chat/client.py b/signalwire/signalwire/ai_chat/client.py index 7f3259a3..018b3079 100644 --- a/signalwire/signalwire/ai_chat/client.py +++ b/signalwire/signalwire/ai_chat/client.py @@ -38,6 +38,7 @@ """ import os +import re from collections.abc import AsyncIterator from contextlib import asynccontextmanager from dataclasses import dataclass, field @@ -45,8 +46,11 @@ import aiohttp +from signalwire.core.logging_config import get_logger from signalwire.rest._base import _user_agent +logger = get_logger("ai_chat.client") + DEFAULT_PATH = "/api/ai/chat" # The service streams keepalive whitespace ahead of slow responses (every @@ -169,6 +173,45 @@ class ChatLog: # ── Client ─────────────────────────────────────────────────────────── +# Characters the chat service keeps in a conversation id. Anything else is +# stripped on arrival, silently and without error. +_ID_SAFE = re.compile(r"[^a-zA-Z0-9_\-.:]") + + +def _warn_if_id_will_be_altered(conversation_id: str) -> None: + """Warn when the service will not store the id it is being given. + + The service sanitizes conversation ids and drops disallowed characters + without reporting it, so a caller that composes ids -- ``root~2`` for a + second leg of ``root``, say -- gets back ``root2``, which is a DIFFERENT, + valid-looking id. Everything filed under the original is then unreachable, + with no error at any layer to indicate the id changed. + + Two of the three characters worth avoiding come from this SDK itself: + ``_`` and ``-`` occur inside ``secrets.token_urlsafe`` output, which is + what ``ChatGateway.mint_handle`` uses to generate ids, so a suffix built + from either cannot be told apart from the id it was appended to. ``:`` is + the gateway's own handle delimiter. That leaves ``.`` as the safe + separator for composing ids. + """ + if not isinstance(conversation_id, str) or not conversation_id: + return + cleaned = _ID_SAFE.sub("", conversation_id) + if cleaned != conversation_id: + removed = "".join(sorted({c for c in conversation_id if _ID_SAFE.match(c)})) + logger.warning( + "conversation_id_will_be_sanitized", + requested=conversation_id, + stored_as=cleaned, + removed_characters=removed, + message=( + "[signalwire] the chat service will store this conversation " + "under a different id; anything filed under the requested id " + "will not be found. Use '.' to compose ids." + ), + ) + + class AIChatClient: """Async client for the SignalWire AI Chat service.""" @@ -340,6 +383,7 @@ async def create_conversation( reinit: bool = False, ) -> ConversationInfo: """Create a conversation (or reinitialize an existing one).""" + _warn_if_id_will_be_altered(conversation_id) params: dict[str, Any] = {"id": conversation_id, "config_url": config_url} if user_message: params["user_message"] = user_message diff --git a/signalwire/signalwire/ai_chat/handoff.py b/signalwire/signalwire/ai_chat/handoff.py new file mode 100644 index 00000000..e79b93a0 --- /dev/null +++ b/signalwire/signalwire/ai_chat/handoff.py @@ -0,0 +1,402 @@ +#!/usr/bin/env python3 +""" +Copyright (c) 2026 SignalWire + +This file is part of the SignalWire SDK. + +Licensed under the MIT License. +See LICENSE file in the project root for full license information. + +Moving one conversation between voice and text. + +:class:`~signalwire.ai_chat.gateway.ChatGateway` lets a browser hold a text +conversation. This module is the other half of what a browser client needs: the +three routes it calls to move that conversation to a phone call and back, and +to type into a live call. + +Why this is in the SDK rather than in each application: the browser side is +already shipped. The SignalWire address widget hardcodes ``{gateway-url}/handoff``, +``{gateway-url}/escalate`` and ``{gateway-url}/say`` against the same URL that +points at a ``ChatGateway``, and sends ``handoff_nonce`` and ``chat_handle`` as +user variables. Without these routes a gateway answers the widget's JSON-RPC +and 404s everything else -- so the SDK would be shipping a gateway its own +widget considers incomplete, with the missing half specified nowhere. + +MECHANISM VS POLICY +------------------- +This class owns the wire contract only: the routes, the nonce, the ordering +guarantee, and the spend guards. It owns nothing about what a conversation +*is*. Where a leg's transcript gets written, what a resumed greeting says, how +much history to carry -- all of that is the application's, injected as +callbacks. + +THE NONCE +--------- +A browser cannot be trusted to name a call. ``ai_message`` takes a ``call_id``, +and a page-supplied one would let anyone who learned or guessed an id inject +speech into a stranger's live call. So the browser proves which call it is on +instead: the application puts a random ``handoff_nonce`` in the user variables +of one dial, registers it here against that call's ids, and the browser +presents it later. The nonce appears nowhere else, so knowing it is proof of +having placed the call. + +Redemption for a handle is single use. Typing is not -- it is repeatable for +the life of the call, bounded by ``max_messages_per_call``. + +An unknown nonce is answered exactly like an expired one, so this cannot be +used to probe whether a given call is live. + +THE ORDERING GUARANTEE +---------------------- +A medium never starts until the one it replaces has finished and its record is +durable. ``/handoff`` ends the call and waits for the application to confirm +capture before minting a handle; ``/escalate`` ends the chat leg and waits +before returning. Skipping the wait means the new medium's config fetch races a +record that is still seconds away, and it opens knowing nothing -- which is +what polling and retries elsewhere end up papering over. + +The wait is event-driven. An earlier polling implementation deadlocked: a +synchronous sleep inside an async route blocked the event loop, and therefore +blocked the very webhook it was waiting for, which then arrived milliseconds +after the wait timed out -- every time. + +DEPLOYMENT +---------- +The nonce registry lives in this process, like ``ChatGateway``'s rate-limit +counters. A redemption must reach the replica that served the dial. Run one +replica, use sticky routing, or supply a shared ``registry``. +""" + +# NOTE: deliberately no `from __future__ import annotations` -- FastAPI resolves +# route annotations against module globals, and `Request` is imported inside +# router() below. Stringified annotations would make it unresolvable, and +# FastAPI would silently treat `request` as a query parameter (422 on every +# call). gateway.py avoids the future import for the same reason. +import asyncio +import time +from collections.abc import Awaitable, Callable +from dataclasses import dataclass, field +from typing import TYPE_CHECKING, Any + +from signalwire.core.logging_config import get_logger + +if TYPE_CHECKING: # pragma: no cover + from fastapi import APIRouter + + from signalwire.ai_chat.gateway import ChatGateway + +logger = get_logger("ai_chat.handoff") + +__all__ = ["HandoffRouter", "NonceEntry"] + +DEFAULT_NONCE_TTL = 3600 +DEFAULT_MAX_MESSAGES_PER_CALL = 200 +DEFAULT_CAPTURE_TIMEOUT = 8.0 + + +@dataclass +class NonceEntry: + """What a nonce is a capability for.""" + + conversation_id: str + call_id: str | None = None + issued_at: float = field(default_factory=time.monotonic) + messages: int = 0 + + +# Application-supplied policy. +# +# capture_leg(conversation_id, medium) -> awaitable/bool +# End the leg and write its record. Return True once the record is durable. +# Called before the replacement medium is allowed to exist. +# end_call(call_id) -> awaitable/None +# Hang the call up server-side so its teardown hooks fire immediately. +# send_message(call_id, text) -> awaitable/bool +# Inject typed text into the live call as if the caller had spoken it. +CaptureLeg = Callable[[str, str], "Awaitable[bool] | bool"] +EndCall = Callable[[str], "Awaitable[None] | None"] +SendMessage = Callable[[str, str], "Awaitable[bool] | bool"] + + +async def _maybe_await(value: Any) -> Any: + """Allow every injected callback to be sync or async.""" + if asyncio.iscoroutine(value) or isinstance(value, asyncio.Future): + return await value + return value + + +class HandoffRouter: + """The three routes a browser client needs beside a :class:`ChatGateway`. + + Args: + gateway: The gateway that owns the conversations. Used to mint handles + and to check origins, so both halves of the URL enforce the same + origin policy. + capture_leg: Called as ``capture_leg(conversation_id, medium)`` to end + a leg and write its record. Must return truthy only once that + record is durable. May be sync or async. When omitted, no wait + happens and the ordering guarantee is not provided. + end_call: Called as ``end_call(call_id)`` to hang up server-side. + send_message: Called as ``send_message(call_id, text)`` for ``/say``. + Omit to leave typing disabled (the route then answers 404). + next_conversation_id: Called as ``next_conversation_id(conversation_id)`` + to produce the id for the NEW leg. Defaults to appending ``.N``. + A fresh id is required because an ended conversation cannot be + reopened; the separator must be ``.`` -- see + ``_warn_if_id_will_be_altered`` in ``ai_chat.client``. + nonce_ttl: Seconds a nonce stays redeemable. + max_messages_per_call: Ceiling on typed messages for one call. Each is + a billable turn, so this is a spend guard as much as an abuse one. + capture_timeout: Seconds to wait for ``capture_leg``. A ceiling, not a + budget -- capture is normally sub-second. + registry: Optional shared mapping for the nonce table. Supply one + backed by shared storage to run more than one replica. + """ + + def __init__( + self, + *, + gateway: "ChatGateway", + capture_leg: CaptureLeg | None = None, + end_call: EndCall | None = None, + send_message: SendMessage | None = None, + next_conversation_id: Callable[[str], str] | None = None, + nonce_ttl: int = DEFAULT_NONCE_TTL, + max_messages_per_call: int = DEFAULT_MAX_MESSAGES_PER_CALL, + capture_timeout: float = DEFAULT_CAPTURE_TIMEOUT, + registry: dict[str, NonceEntry] | None = None, + ) -> None: + self.gateway = gateway + self.capture_leg = capture_leg + self.end_call = end_call + self.send_message = send_message + self.next_conversation_id = next_conversation_id or self._default_next_id + self.nonce_ttl = nonce_ttl + self.max_messages_per_call = max_messages_per_call + self.capture_timeout = capture_timeout + self._nonces: dict[str, NonceEntry] = registry if registry is not None else {} + + # -- nonce lifecycle --------------------------------------------------- + + @staticmethod + def _default_next_id(conversation_id: str) -> str: + """``root`` -> ``root.1``; ``root.2`` -> ``root.3``. + + ``.`` specifically: the chat service strips ``~`` silently, ``_`` and + ``-`` already occur inside generated ids so a suffix built from either + cannot be distinguished from the id it was appended to, and ``:`` is + the gateway's handle delimiter. + """ + root, _, tail = conversation_id.rpartition(".") + if root and tail.isdigit(): + return f"{root}.{int(tail) + 1}" + return f"{conversation_id}.1" + + def register( + self, nonce: str, *, conversation_id: str, call_id: str | None = None + ) -> None: + """Record what a nonce is a capability for. + + Call this from the dynamic-config callback of the dial that carried the + nonce, reading ``call_id`` from the request the platform sent -- never + from anything the browser supplied. + """ + if not nonce or not isinstance(nonce, str): + return + self._prune() + self._nonces[nonce] = NonceEntry( + conversation_id=conversation_id, call_id=call_id + ) + logger.info( + "handoff_nonce_registered", + conversation_id=conversation_id, + call_id=call_id, + ) + + def _prune(self) -> None: + cutoff = time.monotonic() - self.nonce_ttl + for nonce in [n for n, e in self._nonces.items() if e.issued_at < cutoff]: + self._nonces.pop(nonce, None) + + def _lookup(self, nonce: Any) -> NonceEntry | None: + if not nonce or not isinstance(nonce, str): + return None + self._prune() + return self._nonces.get(nonce) + + # -- operations -------------------------------------------------------- + + async def _capture(self, conversation_id: str, medium: str) -> bool: + """Await the application's capture, bounded. Never raises.""" + if self.capture_leg is None: + return False + try: + return bool( + await asyncio.wait_for( + _maybe_await(self.capture_leg(conversation_id, medium)), + timeout=self.capture_timeout, + ) + ) + except TimeoutError: + logger.warning( + "handoff_capture_timeout", + conversation_id=conversation_id, + medium=medium, + note="starting the next medium without this leg's record", + ) + except Exception as exc: + logger.error( + "handoff_capture_failed", + conversation_id=conversation_id, + error=str(exc), + ) + return False + + async def redeem(self, nonce: str) -> str | None: + """Exchange a nonce for a chat handle. Single use. + + Ends the call, waits for its record, and only then mints a handle for a + new leg of the same conversation. + + Returns: + The signed handle, or None for an unknown, expired or already + redeemed nonce -- deliberately indistinguishable from each other. + """ + entry = self._lookup(nonce) + if entry is None: + return None + # Consumed even if what follows fails: a nonce is one attempt. + self._nonces.pop(nonce, None) + + if entry.call_id and self.end_call is not None: + try: + await _maybe_await(self.end_call(entry.call_id)) + except Exception as exc: + logger.warning("handoff_end_call_failed", error=str(exc)) + + await self._capture(entry.conversation_id, "voice") + + try: + handle: str = self.gateway.mint_handle( + self.next_conversation_id(entry.conversation_id) + ) + except Exception as exc: + logger.error("handoff_mint_failed", error=str(exc)) + return None + + logger.info("handoff_redeemed", conversation_id=entry.conversation_id) + return handle + + async def escalate(self, handle: str) -> bool: + """End a chat leg and wait for its record, before a call is placed. + + The browser calls this and waits, so a voice leg started immediately + afterwards is guaranteed to find the text leg already recorded. + """ + try: + conversation_id = self.gateway.read_handle(handle) + except Exception: + return False + await self._capture(conversation_id, "chat") + logger.info("handoff_escalated", conversation_id=conversation_id) + return True + + async def say(self, nonce: str, text: str) -> bool: + """Deliver typed text into the live call the nonce names. + + Does NOT consume the nonce -- typing is repeatable for the life of the + call. Addressed by nonce rather than by any browser-supplied call id, + and no other request field is forwarded: ``global_data`` in particular + is trusted agent state that step logic branches on, and letting a page + write it would be a far larger hole than injecting text. + """ + if self.send_message is None: + return False + cleaned = (text or "").strip() + if not cleaned: + return False + entry = self._lookup(nonce) + if entry is None or not entry.call_id: + return False + if entry.messages >= self.max_messages_per_call: + logger.warning("handoff_say_cap_reached", call_id=entry.call_id) + return False + try: + await _maybe_await(self.send_message(entry.call_id, cleaned)) + except Exception as exc: + logger.error("handoff_say_failed", error=str(exc)) + return False + entry.messages += 1 + return True + + # -- transport --------------------------------------------------------- + + def router(self) -> "APIRouter": + """Build the router. Mount at the SAME prefix as the gateway's. + + The browser derives all three paths from one configured URL, so they + must be siblings of the gateway's JSON-RPC endpoint:: + + agent.mount(gateway.router(), prefix="/chat") + agent.mount(handoff.router(), prefix="/chat") + """ + from fastapi import APIRouter, Request + from fastapi.responses import JSONResponse + + router = APIRouter() + + def _forbidden_origin(request: Request) -> JSONResponse | None: + try: + self.gateway.check_origin(request.headers.get("origin")) + except Exception: + return JSONResponse({"error": "origin not allowed"}, status_code=403) + return None + + async def _body(request: Request) -> dict[str, Any]: + try: + data = await request.json() + except Exception: + return {} + return data if isinstance(data, dict) else {} + + @router.post("/handoff") + async def _handoff(request: Request) -> JSONResponse: + denied = _forbidden_origin(request) + if denied: + return denied + nonce = (await _body(request)).get("nonce") + if not isinstance(nonce, str): + return JSONResponse({"error": "not found"}, status_code=404) + handle = await self.redeem(nonce) + if not handle: + # Same answer for unknown, expired and already-redeemed. + return JSONResponse({"error": "not found"}, status_code=404) + return JSONResponse({"handle": handle}) + + @router.post("/escalate") + async def _escalate(request: Request) -> JSONResponse: + denied = _forbidden_origin(request) + if denied: + return denied + handle = (await _body(request)).get("handle") + if not handle or not isinstance(handle, str): + return JSONResponse({"error": "bad request"}, status_code=400) + if not await self.escalate(handle): + return JSONResponse({"error": "not found"}, status_code=404) + return JSONResponse({"ok": True}) + + @router.post("/say") + async def _say(request: Request) -> JSONResponse: + denied = _forbidden_origin(request) + if denied: + return denied + data = await _body(request) + nonce = data.get("nonce") + text = data.get("text", "") + if not isinstance(nonce, str) or not isinstance(text, str): + return JSONResponse({"error": "not found"}, status_code=404) + if not await self.say(nonce, text): + return JSONResponse({"error": "not found"}, status_code=404) + return JSONResponse({"ok": True}) + + return router diff --git a/signalwire/signalwire/core/agent_base.py b/signalwire/signalwire/core/agent_base.py index fcbe7b2c..e2e0b055 100644 --- a/signalwire/signalwire/core/agent_base.py +++ b/signalwire/signalwire/core/agent_base.py @@ -61,6 +61,7 @@ from signalwire.core.security.session_manager import SessionManager from signalwire.core.swml_service import SWMLService +from signalwire.core.function_result import FunctionResult from signalwire.pom.pom import PromptObjectModel from signalwire.core.skill_manager import SkillManager from signalwire.core.logging_config import get_logger, get_execution_mode @@ -143,6 +144,7 @@ def __init__( schema_validation: bool = True, signing_key: str | None = None, trust_proxy_for_signature: bool = False, + swaig_secret: str | None = None, ): """ Initialize a new agent @@ -174,6 +176,16 @@ def __init__( enforced on POST /, /swaig, /post_prompt — unsigned or invalidly-signed requests get a 403. Falls back to the SIGNALWIRE_SIGNING_KEY env var if not passed. + swaig_secret: Optional secret used to sign this agent's per-call + SWAIG function tokens. Falls back to the + SIGNALWIRE_SWAIG_SECRET env var. When neither is set a + random secret is generated per process, so tokens + issued before a restart stop verifying afterwards and + callers still on those calls see "the security token + for this function is invalid or expired" on their next + tool call. Set it in production, and whenever more + than one replica serves the same agent. Distinct from + `signing_key`, which validates inbound webhooks. trust_proxy_for_signature: If True, honor X-Forwarded-Proto / X-Forwarded-Host when reconstructing the URL during signature validation. Default False — proxy headers @@ -243,8 +255,30 @@ def __init__( # Initialize tool registry (separate from SWMLService verb registry) - # Initialize session manager - self._session_manager = SessionManager(token_expiry_secs=token_expiry_secs) + # Initialize session manager. + # + # The secret signs the per-call SWAIG function tokens this agent mints + # and later verifies itself. SessionManager generates a random one when + # none is passed, which means a restart invalidates every token already + # issued to calls still in progress: the next tool call the caller + # triggers comes back "the security token for this function is invalid + # or expired", which reads to them like the tool failed rather than + # like it was never allowed to run. Restarting an agent mid-call is + # ordinary in development and unavoidable in a rolling deploy, so the + # secret is settable and, once set, survives both. + # + # Also required for horizontal scaling: two replicas with different + # random secrets cannot verify each other's tokens, so a tool call that + # lands on the wrong instance fails the same way. + # + # NOT `signing_key` (below). That is a SignalWire-issued credential used + # to verify INBOUND webhooks are genuinely from SignalWire. This one is + # ours, outbound, and never leaves the process. + self._swaig_secret = swaig_secret or os.environ.get("SIGNALWIRE_SWAIG_SECRET") + self._session_manager = SessionManager( + token_expiry_secs=token_expiry_secs, + secret_key=self._swaig_secret, + ) # Webhook signature validation (porting-sdk/webhooks.md). # Resolution order: explicit constructor arg → SIGNALWIRE_SIGNING_KEY env. @@ -520,6 +554,101 @@ def on_summary( # Default implementation does nothing pass + def on_call_end( + self, handler: Callable[[list[dict[str, Any]], dict[str, Any]], None] + ) -> Callable[[list[dict[str, Any]], dict[str, Any]], None]: + """ + Register a handler that runs when the call ends, with the transcript. + + Usable as a decorator or called directly. Handlers run in registration + order and receive: + + call_log (list[dict]): the conversation as the platform recorded + it, already resolved from whichever field carried it. + raw_data (dict): the complete SWAIG request, including + `global_data` and `call_id`. + + This wraps the platform's reserved `hangup_hook` function. That name is + internal: it fires on hangup and is never offered to the model as + something it could choose, so it cannot be called early or skipped. + + Registering a handler also turns on `swaig_post_conversation`, and that + coupling is the reason this method exists rather than leaving callers + to define the hook themselves. `call_log` is a CONDITIONAL field on a + SWAIG request: without that parameter the hook still fires, still + returns 200, and carries no transcript at all -- which is + indistinguishable from the hook never having been registered. Nothing + errors, nothing logs, and the handler simply receives an empty list + forever. If the parameter has been explicitly set to False, it is left + alone and a warning is emitted, because silently overriding an explicit + choice would be the same class of surprise in the other direction. + + The return value is ignored -- the call is over and there is nobody to + speak to. Exceptions are caught and logged rather than raised, since a + failing teardown handler must not turn into a failed hangup. + + Args: + handler: Callable taking (call_log, raw_data). + + Returns: + The handler, so this can be used as a decorator. + + Example: + @agent.on_call_end + def archive(call_log, raw_data): + conversation_id = raw_data.get("global_data", {}).get("conversation_id") + store(conversation_id, call_log) + """ + handlers = self.__dict__.get("_call_end_handlers") + if handlers is None: + # Rebind rather than mutate -- see the ephemeral-copy contract on + # _create_ephemeral_copy; this list is not in the copied set. + self._call_end_handlers = [handler] + self._ensure_call_end_hook() + else: + self._call_end_handlers = [*handlers, handler] + return handler + + def _ensure_call_end_hook(self) -> None: + """Register the reserved hangup_hook once, and enable its payload.""" + if self._params.get("swaig_post_conversation") is False: + self.log.warning( + "call_end_handler_without_conversation", + message=( + "[signalwire] on_call_end handlers are registered but " + "swaig_post_conversation is explicitly False -- they will " + "receive an empty call_log" + ), + ) + elif "swaig_post_conversation" not in self._params: + self._params["swaig_post_conversation"] = True + + def _hangup_handler(args: Any, raw_data: Any) -> FunctionResult: + raw = raw_data or {} + # Both spellings are seen in the wild depending on engine. + call_log = raw.get("call_log") or raw.get("raw_call_log") or [] + for callback in self.__dict__.get("_call_end_handlers") or []: + # The try/except is inside the loop deliberately: it isolates + # each handler so one failure cannot stop the others from + # running. This path executes once per call, so the overhead + # PERF203 warns about is irrelevant next to that guarantee. + try: + callback(call_log, raw) + except Exception as exc: # noqa: PERF203 + self.log.error( + "call_end_handler_failed", + error=str(exc), + handler=getattr(callback, "__name__", repr(callback)), + ) + return FunctionResult("") + + self.define_tool( + name="hangup_hook", + description="Internal: fires when the call ends.", + parameters={}, + handler=_hangup_handler, + ) + def on_debug_event(self, handler: Callable[..., Any]) -> Callable[..., Any]: """ Register a handler for debug webhook events. @@ -1556,6 +1685,44 @@ def _create_ephemeral_copy(self) -> "AgentBase": configuration for SWML generation. Used when dynamic configuration callbacks need to modify the agent without affecting the persistent state. + THE CALL-SCOPED MUTATION CONTRACT + --------------------------------- + Every attribute is first copied by *reference* from the master agent. + Only the attributes listed below are then replaced with independent + copies, and those -- and only those -- are safe to mutate from a + per-request callback: + + _params, _hints, _languages, _multilingual, _pronounce, + _global_data, _function_includes, _routing_callbacks, + _pre_answer_verbs, _answer_config, _post_answer_verbs, + _post_ai_verbs, _prompt_llm_params, _post_prompt_llm_params, + _internal_fillers, _swaig_query_params, _native_functions, + native_functions, pom, _contexts_builder, _contexts_defined, + skill_manager, _debug_events_enabled, _debug_events_level, + _debug_event_handler, + _prompt_manager (fresh instance; its _sections, _prompt_text, + _post_prompt_text and _contexts are copied), + _tool_registry (fresh instance; its _swaig_functions and + _tool_instances are copied) + + Anything NOT in that list is the master's own object, shared with + every other request in flight. The distinction that matters is between + rebinding and mutating: + + agent.my_thing = {...} # safe: rebinds on the copy only + agent.my_thing["k"] = v # UNSAFE if my_thing is not listed + # above -- writes into the master, + # and therefore into other callers + + Getting this wrong does not raise. It produces one visitor's data + appearing in another visitor's prompt, under concurrency, which is why + the safe set is enumerated here rather than left to be inferred from + the code below. + + Callers who need per-request state of their own should keep it in + `_global_data` (copied) or rebind a fresh object onto the ephemeral + agent, never mutate a shared one in place. + Returns: A lightweight copy of the agent suitable for ephemeral modifications """ diff --git a/signalwire/signalwire/core/capabilities.py b/signalwire/signalwire/core/capabilities.py new file mode 100644 index 00000000..f1aad99c --- /dev/null +++ b/signalwire/signalwire/core/capabilities.py @@ -0,0 +1,127 @@ +#!/usr/bin/env python3 +""" +Copyright (c) 2026 SignalWire + +This file is part of the SignalWire SDK. + +Licensed under the MIT License. +See LICENSE file in the project root for full license information. + +Reading what a client says it can do. + +A browser client -- the SignalWire address widget, or anything speaking the +same convention -- declares its rendering capabilities in the user variables it +sends at dial time:: + + { + "vars": { + "userVariables": { + "capabilities": { + "display_content": true, + "transcript": true, + "chat_handoff": false, + ... + }, + "metadata": {"page": {...}, "client": {...}, "widget": {...}} + } + } + } + +Both ends of that wire are SignalWire's, which is the only reason it belongs in +the SDK: no single application can standardize a convention between two +products it does not own. + +**These are declarations of what the client can RENDER, not grants of +authority.** Treat them as hints for deciding what to offer -- whether to push +code to a screen, whether to advertise a text-handoff tool -- never as +permission to do anything privileged. A caller controls its own user variables. + +**Absence means no.** Every function here resolves errors and missing data to +"not declared", because offering a caller something they cannot reach is worse +than never mentioning it: a PSTN caller has no browser, and an agent that +offers to "put that on your screen" to someone on a phone has simply lied. + +Deliberately NOT provided: + +* An enum of known capability names. The producer side evolves by adding + booleans, and an SDK release per new capability would invert that -- a client + must be able to declare something this SDK has never heard of and have an + application act on it today. +* Any wiring of capabilities to tools. Deciding what a capability *implies* + behaviourally is application policy, and applications write it in a few lines. +""" + +from __future__ import annotations + +from typing import Any + +__all__ = [ + "declared_capabilities", + "has_capability", + "user_variables", +] + + +def user_variables(body_params: Any) -> dict[str, Any]: + """Return the user variables from a SWML request body. + + They are nested two levels down (``vars.userVariables``), which is easy to + get subtly wrong and easy to get wrong silently -- a missing level yields + an empty dict and every downstream check quietly reports "not declared". + + Args: + body_params: The SWML request body. + + Returns: + The user variables, or ``{}``. + """ + try: + variables = (body_params or {}).get("vars", {}).get("userVariables", {}) + except (AttributeError, TypeError): + return {} + return variables if isinstance(variables, dict) else {} + + +def declared_capabilities(body_params: Any) -> frozenset[str]: + """Return the capability names the client declared as truthy. + + Accepts either a full SWML request body or an already-extracted user + variables dict, so it is usable from a dynamic-config callback and from a + SWAIG handler without the caller having to remember which one it holds. + + Args: + body_params: SWML request body, or a user variables dict. + + Returns: + Names whose declared value is truthy. Empty when nothing was declared, + the payload was malformed, or the client is not a browser at all. + + Example: + caps = declared_capabilities(body_params) + if "display_content" in caps: + agent.prompt_add_section("Screen", body=...) + """ + variables = user_variables(body_params) + if not variables and isinstance(body_params, dict): + # Already-extracted user variables were passed directly. + variables = body_params + + capabilities = variables.get("capabilities") + if not isinstance(capabilities, dict): + return frozenset() + return frozenset( + name for name, value in capabilities.items() if value and isinstance(name, str) + ) + + +def has_capability(body_params: Any, name: str) -> bool: + """Whether the client declared ``name``. + + Args: + body_params: SWML request body, or a user variables dict. + name: Capability name, e.g. ``"display_content"``. + + Returns: + True only when explicitly declared truthy. + """ + return name in declared_capabilities(body_params) diff --git a/signalwire/signalwire/core/mixins/web_mixin.py b/signalwire/signalwire/core/mixins/web_mixin.py index e635c978..3a12a5e0 100644 --- a/signalwire/signalwire/core/mixins/web_mixin.py +++ b/signalwire/signalwire/core/mixins/web_mixin.py @@ -68,9 +68,62 @@ class WebMixin(_HostTyped): # type: ignore[misc] # _HostTyped is object at run # has-type ordering gap. Runtime is unaffected (no assignment here). _app: Any | None _proxy_url_base: str | None - _dynamic_config_callback: ( - Callable[[dict[str, Any], dict[str, Any], dict[str, Any], Any], None] | None - ) + + # Per-request configuration callbacks, run in registration order. + # + # Stored as a list but read through the `_dynamic_config_callback` + # property below, so every existing call site -- the truthiness checks and + # the four-argument invocation -- works unchanged whether one callback is + # registered or five. + # + # Populated at setup time on the master agent, never mutated per request. + # That matters: an ephemeral copy shares this list by reference, so a + # per-request append would leak into every other in-flight caller. + _per_call_configs: list[ + Callable[[dict[str, Any], dict[str, Any], dict[str, Any], Any], None] + ] + + @property + def _dynamic_config_callback( + self, + ) -> Callable[[dict[str, Any], dict[str, Any], dict[str, Any], Any], None] | None: + """The registered per-call configuration, as one callable or None. + + Composed on read rather than on write so that registration order is + the run order and a later registration is always picked up. + """ + callbacks: list[ + Callable[[dict[str, Any], dict[str, Any], dict[str, Any], Any], None] + ] = self.__dict__.get("_per_call_configs") or [] + if not callbacks: + return None + if len(callbacks) == 1: + return callbacks[0] + + def _run_all( + query_params: dict[str, Any], + body_params: dict[str, Any], + headers: dict[str, Any], + agent: Any, + ) -> None: + for callback in callbacks: + callback(query_params, body_params, headers, agent) + + return _run_all + + @_dynamic_config_callback.setter + def _dynamic_config_callback( + self, + callback: ( + Callable[[dict[str, Any], dict[str, Any], dict[str, Any], Any], None] | None + ), + ) -> None: + """Assigning replaces the whole chain; None clears it. + + Preserves the historical meaning of a direct assignment, including + `self._dynamic_config_callback = None` during agent construction. + """ + self._per_call_configs = [] if callback is None else [callback] def get_app(self) -> FastAPI: """ @@ -173,6 +226,91 @@ async def handle_all_routes(request: Request, full_path: str) -> Response: return self._app + def mount( + self, + app_or_router: Any, + *, + prefix: str = "", + name: str | None = None, + ) -> "AgentBase": + """ + Mount an extra router or ASGI app alongside this agent's own routes. + + Use this instead of reaching for ``get_app()`` and mounting by hand. + Three separate details have to be right for an added route to work, all + of which fail silently: + + 1. ``serve()`` only builds an app when ``self._app`` is None, so + anything mounted after ``serve()`` starts is lost. Calling this + method materialises the app first, so later ``serve()`` reuses it. + + 2. ``get_app()`` registers a ``/{full_path:path}`` catch-all, and + FastAPI matches routes in registration order -- so ANY route added + afterwards is shadowed by it and never runs. This moves the + catch-all back to the end. + + 3. ``get_app()`` and ``serve()`` build different apps, and only + ``serve()``'s catch-all routes the agent's own bare route (no + trailing slash) to the SWML handler. ``get_app()``'s answers 204. + Since mounting forces the ``get_app()`` path, the bare route is + re-registered here -- without it the SWML endpoint the platform + actually fetches returns 204 while the trailing-slash form keeps + working, which looks like a platform problem rather than a routing + one. + + Args: + app_or_router: A FastAPI ``APIRouter`` (included at ``prefix``) or + any other ASGI app such as ``StaticFiles`` (mounted at + ``prefix``). + prefix: Path prefix. No trailing slash. + name: Optional mount name, only used for ASGI apps. + + Returns: + Self for method chaining + + Example: + agent.mount(gateway.router(), prefix="/chat") + agent.mount(StaticFiles(directory="web", html=True), prefix="/demo") + """ + from fastapi import APIRouter + + app = self.get_app() + clean_prefix = prefix.rstrip("/") + + if isinstance(app_or_router, APIRouter): + app.include_router(app_or_router, prefix=clean_prefix) + else: + app.mount(clean_prefix or "/", app_or_router, name=name) + + self._restore_route_precedence(app) + self.log.info("agent_route_mounted", prefix=clean_prefix or "/") + return self + + def _restore_route_precedence(self, app: FastAPI) -> None: + """Re-register the bare agent route and push catch-alls to the end. + + Idempotent: safe to call after every mount. + """ + catch_all_path = "/{full_path:path}" + routes = app.router.routes + + # (3) The bare route, e.g. "/sigmond" with no trailing slash. + if self.route and not any( + getattr(r, "path", None) == self.route for r in routes + ): + + @app.get(self.route) + @app.post(self.route) + async def _swml_bare_route(request: Request) -> Response: + return _as_response(await self._handle_root_request(request)) + + # (2) Anything registered before the catch-all wins; move it last so + # every route added by mount() -- and the bare route just added -- is + # reachable. + for route in [r for r in routes if getattr(r, "path", None) == catch_all_path]: + routes.remove(route) + routes.append(route) + def as_router(self) -> "HostAppRouter": """ Get a router to embed this agent's routes in a host web app. @@ -1346,10 +1484,59 @@ def my_config(query_params, body_params, headers, agent): agent.set_global_data({"tier": query_params.get('tier', 'standard')}) my_agent.set_dynamic_config_callback(my_config) + + Note: + This REPLACES any previously registered per-call configuration. + Two calls to this method mean the first one never runs, silently: + the agent still renders valid SWML and every tool still works, so + whatever the discarded callback configured -- a voice, a language + set, a hint list -- is simply absent, with nothing to indicate it + was dropped. Use `add_per_call_config` when composing, and reserve + this method for the single-callback case it was written for. """ self._dynamic_config_callback = callback return self + def add_per_call_config( + self, + callback: Callable[[dict[str, Any], dict[str, Any], dict[str, Any], Any], None], + ) -> "AgentBase": + """ + Register a per-request configuration callback, keeping any already set + + Same signature and contract as `set_dynamic_config_callback`, except + that callbacks accumulate instead of overwriting. They run in + registration order against the same ephemeral agent, so a later one + sees what an earlier one configured and can build on or override it. + + This is the composable form, and the one to prefer. A base class and a + subclass, or an agent and a mixin, can each register what they own + without either needing to know the other exists -- which with + `set_dynamic_config_callback` requires them to find and manually chain + each other's callbacks, and silently drops one when they don't. + + Args: + callback: Callable taking (query_params, body_params, headers, agent). + `agent` is the EPHEMERAL per-request copy; configure that, + never `self`, or the configuration leaks across callers. + + Returns: + Self for method chaining + + Example: + agent.add_per_call_config(configure_voice) + agent.add_per_call_config(configure_page_context) + # both run, in that order, on every request + """ + callbacks = self.__dict__.get("_per_call_configs") + if callbacks is None: + # Rebind rather than mutate: an ephemeral copy shares the master's + # list by reference, so appending in place would write into it. + self._per_call_configs = [callback] + else: + self._per_call_configs = [*callbacks, callback] + return self + def manual_set_proxy_url(self, proxy_url: str) -> "AgentBase": """ Manually set the proxy URL base for webhook callbacks diff --git a/signalwire/signalwire/core/post_prompt.py b/signalwire/signalwire/core/post_prompt.py new file mode 100644 index 00000000..d0ae16c0 --- /dev/null +++ b/signalwire/signalwire/core/post_prompt.py @@ -0,0 +1,262 @@ +#!/usr/bin/env python3 +""" +Copyright (c) 2026 SignalWire + +This file is part of the SignalWire SDK. + +Licensed under the MIT License. +See LICENSE file in the project root for full license information. + +Post-prompt normalization. + +One conversation can run over voice and over text chat, and both ends produce +"the post-prompt" -- but they do not produce the same shape, and the +differences are not documented anywhere a caller would find them. This module +absorbs that divergence so an application sees one artifact regardless of which +engine finished the conversation. + +Known divergences between the two engines: + +=================== ========================= ============================ +field voice chat +=================== ========================= ============================ +``app_name`` ``"swml app"`` ``"ai_chat"`` +``conversation_id`` absent present at top level +full log ``raw_call_log`` ``raw_messages`` +summary arrives as ``summarize_conversation`` a bare ``role: assistant`` + tool call turn inside ``call_log`` +``post_prompt_data`` parsed object ``{"raw": "```json ...```"}`` +=================== ========================= ============================ + +``conversation_type`` is a reliable top-level discriminator on both. + +There is also a *third* ``post_prompt_data`` shape seen from the voice engine: +``{"parsed": [ {...} ], "raw": "..."}`` -- the object wrapped in a list under +``parsed``. It survives structurally and misses every field lookup, so a caller +that does not handle it silently gets nothing while appearing to work. + +What this module does NOT do is decide what a summary should contain. The +schema is the application's -- it is whatever its post-prompt text asked the +model to produce -- so parsing here is deliberately schema-agnostic and returns +the dict as found. +""" + +from __future__ import annotations + +import json +import re +from dataclasses import dataclass, field +from typing import Any + +__all__ = [ + "DIALOGUE_ROLES", + "NormalizedPostPrompt", + "dialogue_turns", + "normalize_post_prompt", + "parse_post_prompt_data", + "strip_json_fence", +] + +# Roles that are actual dialogue. Everything else in a call log is machinery: +# ``system`` is the prompt, ``system-log`` is lifecycle and step tracing, +# ``tool`` is function output, and ``assistant-manual`` is filler speech +# ("let me look that up") that was spoken but carries nothing worth replaying. +DIALOGUE_ROLES: tuple[str, ...] = ("user", "assistant") + +_FENCE_OPEN = re.compile(r"^```[a-zA-Z]*\s*") +_FENCE_CLOSE = re.compile(r"\s*```$") + + +@dataclass(frozen=True) +class NormalizedPostPrompt: + """One finished conversation leg, in a shape that does not vary by engine. + + Attributes: + medium: ``conversation_type`` as reported, e.g. ``"voice"`` or + ``"chat"``. Empty string when the engine did not say. + conversation_id: Present on chat, absent on voice. ``None`` when the + engine did not supply one -- callers that need a stable key should + fall back to their own (``global_data``, ``call_id``) rather than + treating this as authoritative. + summary: The parsed ``post_prompt_data``, whatever keys the + application's post-prompt asked for. ``{}`` when there was none or + it could not be parsed at all. A model that answered in prose + instead of JSON yields ``{"summary": ""}`` -- a usable + paragraph is better than a discarded one. + dialogue: ``user``/``assistant`` turns only, with tool calls and the + chat engine's summary echo removed. + call_id: The platform call id, when present. + raw: The complete request body, untouched. + """ + + medium: str = "" + conversation_id: str | None = None + summary: dict[str, Any] = field(default_factory=dict) + dialogue: list[dict[str, str]] = field(default_factory=list) + call_id: str | None = None + raw: dict[str, Any] = field(default_factory=dict) + + +def strip_json_fence(text: str) -> str: + """Unwrap ```` ```json ... ``` ```` fencing. + + The chat engine hands the model's answer back verbatim, fence and all, + where the voice engine parses it first. + """ + stripped = (text or "").strip() + if stripped.startswith("```"): + stripped = _FENCE_OPEN.sub("", stripped) + stripped = _FENCE_CLOSE.sub("", stripped) + return stripped.strip() + + +def _unwrap_parsed(data: dict[str, Any]) -> dict[str, Any] | None: + """Pull the object out of a ``{"parsed": [...]}`` wrapper, if present. + + Checked before the generic sweep, which would otherwise happily return + ``{"parsed": [...]}`` -- structurally fine, semantically empty, and every + subsequent field lookup misses without anything indicating why. + """ + parsed = data.get("parsed") + if isinstance(parsed, dict): + return parsed + if isinstance(parsed, list): + for item in parsed: + if isinstance(item, dict) and item: + return item + return None + + +def parse_post_prompt_data(data: Any) -> dict[str, Any]: + """Return ``post_prompt_data`` as a plain dict, whichever shape it arrived in. + + Never raises. The conversation that produced this is already over and there + is nobody to show an error to, so a malformed summary degrades rather than + failing the request that delivered it. + + Args: + data: The ``post_prompt_data`` value from a post-prompt body. + + Returns: + The summary object, or ``{}`` when there is nothing usable. + """ + if not isinstance(data, dict): + return {} + + unwrapped = _unwrap_parsed(data) + if unwrapped: + return unwrapped + + # Flat shape: real keys already present (anything but raw/parsed). + flat = {k: v for k, v in data.items() if k not in ("raw", "parsed")} + if flat: + return flat + + raw = data.get("raw") + if not isinstance(raw, str) or not raw.strip(): + return {} + unfenced = strip_json_fence(raw) + try: + loaded = json.loads(unfenced) + except (ValueError, TypeError): + # Prose instead of JSON. Still a summary. + return {"summary": unfenced} + return loaded if isinstance(loaded, dict) else {"summary": str(loaded)} + + +def dialogue_turns( + call_log: Any, + *, + roles: tuple[str, ...] = DIALOGUE_ROLES, + drop_echo: str | None = None, +) -> list[dict[str, str]]: + """Extract the real dialogue from a call log. + + Drops everything that is machinery rather than speech: non-dialogue roles, + entries carrying ``tool_calls``, and empty content. + + ``drop_echo`` exists for one specific engine behaviour. The chat engine + appends its own post-prompt output to ``call_log`` as a bare + ``role: assistant`` entry with no ``tool_calls`` -- by role alone it is + indistinguishable from real assistant speech. Replayed into another medium, + the agent appears to narrate a summary of itself in the third person. It is + identifiable only by content, being byte-identical to + ``post_prompt_data.raw``, which is what this parameter compares against. + The voice engine delivers the same artifact as a ``summarize_conversation`` + tool call, which the ``tool_calls`` check already removes. + + Args: + call_log: The log, as ``call_log`` / ``raw_call_log`` / ``raw_messages``. + roles: Roles to keep. + drop_echo: Exact content to treat as the summary echo and drop. + + Returns: + ``[{"role": ..., "content": ...}, ...]`` in order. + """ + # Guarded rather than relying on `call_log or []`: a non-iterable value + # (an int, say, from a malformed body) is truthy and would raise on + # iteration. Nothing in this module may raise -- the conversation that + # produced the input is already over. + if not isinstance(call_log, (list, tuple)): + return [] + + out: list[dict[str, str]] = [] + echo = (drop_echo or "").strip() + for entry in call_log: + if not isinstance(entry, dict): + continue + if entry.get("role") not in roles: + continue + if entry.get("tool_calls"): + continue + content = entry.get("content") + if not isinstance(content, str) or not content.strip(): + continue + if echo and content.strip() == echo: + continue + out.append({"role": entry["role"], "content": content}) + return out + + +def normalize_post_prompt(body: Any) -> NormalizedPostPrompt: + """Normalize a post-prompt body from either engine. + + Args: + body: The complete post-prompt request body. + + Returns: + A :class:`NormalizedPostPrompt`. Never raises; a body this function + cannot make sense of yields one with empty fields. + + Example: + leg = normalize_post_prompt(raw_body) + if leg.dialogue: + store(leg.conversation_id, leg.medium, leg.summary, leg.dialogue) + """ + if not isinstance(body, dict): + return NormalizedPostPrompt() + + summary = parse_post_prompt_data(body.get("post_prompt_data")) + + # The echo is compared against the RAW string the engine returned, not the + # parsed summary -- the assistant turn carries the fence too. + raw_summary = "" + ppd = body.get("post_prompt_data") + if isinstance(ppd, dict) and isinstance(ppd.get("raw"), str): + raw_summary = ppd["raw"] + + log = ( + body.get("call_log") + or body.get("raw_call_log") + or body.get("raw_messages") + or [] + ) + + return NormalizedPostPrompt( + medium=str(body.get("conversation_type") or ""), + conversation_id=body.get("conversation_id") or None, + summary=summary, + dialogue=dialogue_turns(log, drop_echo=raw_summary or None), + call_id=body.get("call_id") or None, + raw=body, + ) diff --git a/signalwire/signalwire/core/swml_service.py b/signalwire/signalwire/core/swml_service.py index 82999ccf..6ca732e3 100644 --- a/signalwire/signalwire/core/swml_service.py +++ b/signalwire/signalwire/core/swml_service.py @@ -71,6 +71,31 @@ MAX_REQUEST_BODY_SIZE = 10 * 1024 * 1024 +class _NullLog: + """No-op stand-in used only when ``log`` cannot be resolved. + + ``__getattr__`` logs, and it can run before ``__init__`` assigns + ``self.log`` (line ~136) or after ``__init__`` raised. Falling back to + this keeps a diagnostic log line from turning a missing attribute into a + second, unrelated failure. + """ + + def debug(self, *args: Any, **kwargs: Any) -> None: + return None + + +_NULL_LOG = _NullLog() + +# Attributes ``SWMLService.__getattr__`` needs in order to run. None can be a +# SWML verb, and resolving any of them *through* ``__getattr__`` would re-enter +# it -- so they are rejected before the body touches anything. Without this, +# a partially constructed instance (``self.log`` is assigned in ``__init__`` +# well before ``self.schema_utils``) turns any attribute miss into unbounded +# recursion whose traceback names the dependency rather than the access that +# triggered it. +_GETATTR_INTERNALS = frozenset({"log", "schema_utils", "_verb_methods_cache"}) + + def _as_response(result: "Response | dict[str, Any]") -> "Response": """Coerce a handler result into a Response for FastAPI route handlers. @@ -323,26 +348,68 @@ def __getattr__(self, name: str) -> Any: Raises: AttributeError: If name is not a valid SWML verb """ - self.log.debug("getattr_called", attribute=name) + # Re-entry guard. This method's own dependencies must never be + # resolved through it. + # + # __getattr__ runs on every failed attribute lookup, and the body + # below reaches for `self.log` and `self.schema_utils`. If either is + # itself unset -- on a partially constructed instance, during + # __init__ before they are assigned, or after __init__ raised -- + # resolving it re-enters __getattr__, which reaches for it again, and + # the recursion ends only when the stack does. The traceback then + # names the *dependency*, not the attribute access that started it, + # so the symptom points away from the cause. + # + # Dunders are rejected here too: copy, pickle and inspect probe for + # them constantly, none can be a SWML verb, and each one previously + # cost a schema lookup and a log line. + if name.startswith("__") or name in _GETATTR_INTERNALS: + raise AttributeError( + f"'{type(self).__name__}' object has no attribute '{name}'" + ) + + # object.__getattribute__ does not fall back to __getattr__, so this + # cannot recurse even if `log` is genuinely absent. Every log call in + # this method's own body goes through `_log` for that reason; the + # nested verb methods below may use `self.log` freely, since they run + # long after construction. + try: + _log = object.__getattribute__(self, "log") + except AttributeError: + _log = _NULL_LOG + + _log.debug("getattr_called", attribute=name) # Simple version to match our test script - # First check if this is a valid SWML verb - if not self.schema_utils: + # First check if this is a valid SWML verb. + # + # Resolved defensively for the same reason as `log`: on a partially + # constructed instance `schema_utils` is absent, and reaching for it + # normally would surface an AttributeError naming *it* rather than + # the attribute the caller actually asked for -- pointing the reader + # at the wrong name. Absent schema means "not a verb", which is the + # answer the existing branch below already gives. + try: + _schema_utils = object.__getattribute__(self, "schema_utils") + except AttributeError: + _schema_utils = None + + if not _schema_utils: msg = f"'{self.__class__.__name__}' object has no attribute '{name}' (no schema available)" - self.log.debug("getattr_no_schema", attribute=name) + _log.debug("getattr_no_schema", attribute=name) raise AttributeError(msg) - verb_names = self.schema_utils.get_all_verb_names() + verb_names = _schema_utils.get_all_verb_names() if name in verb_names: - self.log.debug("getattr_valid_verb", verb=name) + _log.debug("getattr_valid_verb", verb=name) # Check if we already have this method in the cache if not hasattr(self, "_verb_methods_cache"): self._verb_methods_cache = {} if name in self._verb_methods_cache: - self.log.debug("getattr_cached_method", verb=name) + _log.debug("getattr_cached_method", verb=name) return types.MethodType(self._verb_methods_cache[name], self) # Handle sleep verb specially since it takes an integer directly @@ -371,7 +438,7 @@ def sleep_method( raise TypeError("sleep() missing required argument: 'duration'") # Cache the method for future use - self.log.debug("caching_sleep_method", verb=name) + _log.debug("caching_sleep_method", verb=name) self._verb_methods_cache[name] = sleep_method # Return the bound method @@ -398,7 +465,7 @@ def verb_method(self_instance: "SWMLService", **kwargs: Any) -> bool: verb_method.__doc__ = f"Add the {name} verb to the document." # Cache the method for future use - self.log.debug("caching_verb_method", verb=name) + _log.debug("caching_verb_method", verb=name) self._verb_methods_cache[name] = verb_method # Return the bound method @@ -406,7 +473,7 @@ def verb_method(self_instance: "SWMLService", **kwargs: Any) -> bool: # Not a valid verb msg = f"'{self.__class__.__name__}' object has no attribute '{name}'" - self.log.debug("getattr_invalid_attribute", attribute=name, error=msg) + _log.debug("getattr_invalid_attribute", attribute=name, error=msg) raise AttributeError(msg) @property diff --git a/tests/unit/ai_chat/test_handoff.py b/tests/unit/ai_chat/test_handoff.py new file mode 100644 index 00000000..b33711ae --- /dev/null +++ b/tests/unit/ai_chat/test_handoff.py @@ -0,0 +1,296 @@ +"""HandoffRouter: moving one conversation between voice and text. + +The browser side of this contract is already shipped -- the address widget +hardcodes ``/handoff``, ``/escalate`` and ``/say`` against its gateway URL -- +so these tests pin the server half against that fixed shape. + +Three properties matter more than the happy path: + +* **The nonce is proof of having placed a call.** It is never a call id, and an + unknown nonce is answered exactly like an expired one so the route cannot be + used to probe whether a given call is live. +* **Ordering.** A medium never starts until the one it replaces has finished + and been recorded, or the new medium's config fetch races a record that is + still seconds away and it opens knowing nothing. +* **Typing is repeatable but bounded.** Each injected message is a billable + turn, so the cap is a spend guard as much as an abuse guard. +""" + +import asyncio +from typing import Any + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from signalwire.ai_chat import AIChatClient, ChatGateway, HandoffRouter +from signalwire.ai_chat.client import _warn_if_id_will_be_altered + +SECRET = "s" * 32 + + +def _recording_sender(events: list[Any]) -> Any: + """A send_message that records and always succeeds.""" + + def _send(call_id: str, text: str) -> bool: + events.append(("say", text)) + return True + + return _send + + +@pytest.fixture +def gateway() -> ChatGateway: + client = AIChatClient( + project="p", + token="t", # noqa: S106 - test fixture, not a credential + url="https://service.example.invalid/aichat", + ) + return ChatGateway( + config_url="https://agent.example.com/swml", + key="pk_test", + secret=SECRET, + client=client, + ) + + +@pytest.fixture +def events() -> list[tuple[Any, ...]]: + return [] + + +@pytest.fixture +def handoff(gateway: ChatGateway, events: list[Any]) -> HandoffRouter: + async def capture(conversation_id: str, medium: str) -> bool: + await asyncio.sleep(0) # a real await, not a poll + events.append(("capture", conversation_id, medium)) + return True + + def end_call(call_id: str) -> None: + events.append(("end_call", call_id)) + + def send_message(call_id: str, text: str) -> bool: + events.append(("say", call_id, text)) + return True + + return HandoffRouter( + gateway=gateway, + capture_leg=capture, + end_call=end_call, + send_message=send_message, + ) + + +@pytest.fixture +def client(handoff: HandoffRouter) -> TestClient: + app = FastAPI() + app.include_router(handoff.router(), prefix="/chat") + return TestClient(app) + + +class TestHandoffRedemption: + def test_returns_a_handle_the_gateway_can_read( + self, handoff: HandoffRouter, client: TestClient, gateway: ChatGateway + ) -> None: + handoff.register("n1", conversation_id="conv-root", call_id="call-9") + response = client.post("/chat/handoff", json={"nonce": "n1"}) + assert response.status_code == 200 + assert gateway.read_handle(response.json()["handle"]) + + def test_call_ends_before_the_leg_is_captured( + self, handoff: HandoffRouter, client: TestClient, events: list[Any] + ) -> None: + """Ending first is what makes the record exist to be captured.""" + handoff.register("n1", conversation_id="conv-root", call_id="call-9") + client.post("/chat/handoff", json={"nonce": "n1"}) + assert events == [ + ("end_call", "call-9"), + ("capture", "conv-root", "voice"), + ] + + def test_new_leg_gets_a_fresh_dotted_id( + self, handoff: HandoffRouter, client: TestClient, gateway: ChatGateway + ) -> None: + """An ended conversation cannot be reopened, so the handle must name a + new leg -- and '.' is the only separator the service preserves.""" + handoff.register("n1", conversation_id="conv-root", call_id="call-9") + response = client.post("/chat/handoff", json={"nonce": "n1"}) + assert gateway.read_handle(response.json()["handle"]) == "conv-root.1" + + def test_leg_ids_increment(self, handoff: HandoffRouter) -> None: + assert handoff.next_conversation_id("root.2") == "root.3" + assert handoff.next_conversation_id("root") == "root.1" + + def test_a_nonce_is_single_use( + self, handoff: HandoffRouter, client: TestClient + ) -> None: + handoff.register("n1", conversation_id="conv-root", call_id="call-9") + assert client.post("/chat/handoff", json={"nonce": "n1"}).status_code == 200 + assert client.post("/chat/handoff", json={"nonce": "n1"}).status_code == 404 + + def test_unknown_and_spent_nonces_are_indistinguishable( + self, handoff: HandoffRouter, client: TestClient + ) -> None: + """Otherwise this route reports whether a given call is live.""" + handoff.register("n1", conversation_id="conv-root", call_id="call-9") + client.post("/chat/handoff", json={"nonce": "n1"}) + spent = client.post("/chat/handoff", json={"nonce": "n1"}) + unknown = client.post("/chat/handoff", json={"nonce": "never-existed"}) + assert spent.status_code == unknown.status_code == 404 + assert spent.json() == unknown.json() + + def test_expired_nonces_are_not_redeemable( + self, gateway: ChatGateway, client: TestClient + ) -> None: + expired = HandoffRouter(gateway=gateway, nonce_ttl=-1) + expired.register("n1", conversation_id="conv-root", call_id="call-9") + assert expired._lookup("n1") is None + + def test_missing_nonce_is_rejected(self, client: TestClient) -> None: + assert client.post("/chat/handoff", json={}).status_code == 404 + + +class TestEscalate: + def test_captures_the_chat_leg_before_returning( + self, client: TestClient, gateway: ChatGateway, events: list[Any] + ) -> None: + """The browser blocks on this, which is what makes the following dial + safe.""" + handle = gateway.mint_handle("conv-root.5") + assert client.post("/chat/escalate", json={"handle": handle}).status_code == 200 + assert events == [("capture", "conv-root.5", "chat")] + + def test_a_forged_handle_is_refused(self, client: TestClient) -> None: + assert ( + client.post("/chat/escalate", json={"handle": "forged"}).status_code == 404 + ) + + def test_a_missing_handle_is_a_bad_request(self, client: TestClient) -> None: + assert client.post("/chat/escalate", json={}).status_code == 400 + + +class TestSay: + def test_delivers_trimmed_text_to_the_call_the_nonce_names( + self, handoff: HandoffRouter, client: TestClient, events: list[Any] + ) -> None: + handoff.register("n2", conversation_id="conv-root", call_id="call-9") + assert ( + client.post( + "/chat/say", json={"nonce": "n2", "text": " hello "} + ).status_code + == 200 + ) + assert events == [("say", "call-9", "hello")] + + def test_is_repeatable(self, handoff: HandoffRouter, client: TestClient) -> None: + """Unlike redemption -- typing lasts the life of the call.""" + handoff.register("n2", conversation_id="conv-root", call_id="call-9") + for _ in range(3): + assert ( + client.post("/chat/say", json={"nonce": "n2", "text": "x"}).status_code + == 200 + ) + + def test_is_capped_per_call(self, gateway: ChatGateway, events: list[Any]) -> None: + """Every injection is a billable turn.""" + router = HandoffRouter( + gateway=gateway, + send_message=_recording_sender(events), + max_messages_per_call=2, + ) + router.register("n", conversation_id="c", call_id="call-1") + assert asyncio.run(router.say("n", "one")) + assert asyncio.run(router.say("n", "two")) + assert not asyncio.run(router.say("n", "three")) + + def test_empty_text_is_refused( + self, handoff: HandoffRouter, client: TestClient + ) -> None: + handoff.register("n2", conversation_id="conv-root", call_id="call-9") + assert ( + client.post("/chat/say", json={"nonce": "n2", "text": " "}).status_code + == 404 + ) + + def test_an_unknown_nonce_cannot_inject(self, client: TestClient) -> None: + """The whole point: a browser cannot name someone else's call.""" + assert ( + client.post( + "/chat/say", json={"nonce": "guessed", "text": "hello"} + ).status_code + == 404 + ) + + def test_disabled_when_no_sender_is_configured(self, gateway: ChatGateway) -> None: + router = HandoffRouter(gateway=gateway) + router.register("n", conversation_id="c", call_id="call-1") + assert not asyncio.run(router.say("n", "hello")) + + +class TestCaptureFailures: + def test_a_capture_timeout_does_not_block_the_switch( + self, gateway: ChatGateway + ) -> None: + """Thin context beats refusing a switch the visitor asked for.""" + + async def never_finishes(conversation_id: str, medium: str) -> bool: + await asyncio.sleep(10) + return True + + router = HandoffRouter( + gateway=gateway, capture_leg=never_finishes, capture_timeout=0.05 + ) + router.register("n", conversation_id="c", call_id="call-1") + handle = asyncio.run(router.redeem("n")) + assert handle is not None + assert gateway.read_handle(handle) == "c.1" + + def test_a_raising_capture_does_not_block_the_switch( + self, gateway: ChatGateway + ) -> None: + def boom(conversation_id: str, medium: str) -> bool: + raise RuntimeError("storage down") + + router = HandoffRouter(gateway=gateway, capture_leg=boom) + router.register("n", conversation_id="c", call_id="call-1") + handle = asyncio.run(router.redeem("n")) + assert handle is not None + assert gateway.read_handle(handle) == "c.1" + + +class TestConversationIdSanitization: + """The service strips disallowed characters silently, so an id composed + with the wrong separator is stored under a different, valid-looking id and + everything filed under the original becomes unreachable. + """ + + # structlog renders to stdout rather than through the stdlib handlers + # `caplog` installs, so the warning is asserted via captured output. + + @pytest.mark.parametrize("safe", ["conv-abc", "root.2", "a_b-c.d:e"]) + def test_safe_ids_are_quiet(self, safe: str, capsys: Any) -> None: + _warn_if_id_will_be_altered(safe) + assert "conversation_id_will_be_sanitized" not in capsys.readouterr().out + + @pytest.mark.parametrize( + ("unsafe", "stored_as"), + [("root~2", "root2"), ("conv id", "convid"), ("x!", "x")], + ) + def test_unsafe_ids_warn_with_what_will_actually_be_stored( + self, unsafe: str, stored_as: str, capsys: Any + ) -> None: + _warn_if_id_will_be_altered(unsafe) + out = capsys.readouterr().out + assert "conversation_id_will_be_sanitized" in out + # The warning must name the id the service will really use -- that is + # the fact the caller needs, and the one nothing else reports. + assert stored_as in out + + @pytest.mark.parametrize("junk", [None, "", 123, []]) + def test_junk_is_ignored_rather_than_warned_about( + self, junk: Any, capsys: Any + ) -> None: + """Paired with the warning case above: this asserts the warning is + absent, so it can fail, rather than merely asserting no exception.""" + _warn_if_id_will_be_altered(junk) + assert "conversation_id_will_be_sanitized" not in capsys.readouterr().out diff --git a/tests/unit/core/test_agent_consolidation.py b/tests/unit/core/test_agent_consolidation.py new file mode 100644 index 00000000..690beb11 --- /dev/null +++ b/tests/unit/core/test_agent_consolidation.py @@ -0,0 +1,233 @@ +"""Agent lifecycle surfaces: signing secret, per-call config, call-end, mounting. + +Four defects that all failed the same way -- silently, with a symptom pointing +somewhere other than the cause. Each test here reproduces the original failure +so a regression is caught rather than rediscovered in production. +""" + +from typing import Any + +import pytest +from fastapi import APIRouter +from fastapi.testclient import TestClient + +from signalwire import AgentBase + + +def agent(**kwargs: Any) -> AgentBase: + return AgentBase(name="t", route="/myagent", schema_validation=False, **kwargs) + + +# ── swaig_secret ───────────────────────────────────────────────────── + + +class TestSwaigSecret: + """SessionManager generated a random secret per process, so tokens issued + before a restart stopped verifying after it -- and the caller saw "the + security token for this function is invalid or expired", which reads like + the tool failed rather than like it was never allowed to run. + """ + + def test_without_a_secret_two_instances_cannot_verify_each_other(self) -> None: + a, b = agent(), agent() + token = a._session_manager.create_tool_token("search", "call-1") + assert not b._session_manager.validate_tool_token("search", token, "call-1") + + def test_a_shared_secret_survives_the_restart(self) -> None: + a = agent(swaig_secret="shared") # noqa: S106 - test fixture + b = agent(swaig_secret="shared") # noqa: S106 - test fixture + token = a._session_manager.create_tool_token("search", "call-1") + assert b._session_manager.validate_tool_token("search", token, "call-1") + + def test_env_var_is_honoured(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("SIGNALWIRE_SWAIG_SECRET", "from-env") + a, b = agent(), agent() + token = a._session_manager.create_tool_token("search", "call-1") + assert b._session_manager.validate_tool_token("search", token, "call-1") + + def test_explicit_argument_beats_the_env( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("SIGNALWIRE_SWAIG_SECRET", "from-env") + assert ( + agent(swaig_secret="explicit")._swaig_secret # noqa: S106 - test fixture + == "explicit" # noqa: S105 - test fixture + ) + + +# ── per-call config ────────────────────────────────────────────────── + + +class TestPerCallConfig: + """`set_dynamic_config_callback` holds ONE callback. A second call + discarded the first with no error: SWML still rendered, every tool still + worked, and whatever the dropped callback configured was simply absent. + """ + + def test_added_callbacks_all_run_in_registration_order(self) -> None: + seen: list[str] = [] + a = agent() + a.add_per_call_config(lambda q, b, h, ag: seen.append("first")) + a.add_per_call_config(lambda q, b, h, ag: seen.append("second")) + configure = a._dynamic_config_callback + assert configure is not None + configure({}, {}, {}, a) + assert seen == ["first", "second"] + + def test_set_still_replaces(self) -> None: + """Documented behaviour of the original method is preserved.""" + seen: list[str] = [] + a = agent() + a.set_dynamic_config_callback(lambda q, b, h, ag: seen.append("one")) + a.set_dynamic_config_callback(lambda q, b, h, ag: seen.append("two")) + configure = a._dynamic_config_callback + assert configure is not None + configure({}, {}, {}, a) + assert seen == ["two"] + + def test_add_composes_with_a_previously_set_callback(self) -> None: + seen: list[str] = [] + a = agent() + a.set_dynamic_config_callback(lambda q, b, h, ag: seen.append("set")) + a.add_per_call_config(lambda q, b, h, ag: seen.append("added")) + configure = a._dynamic_config_callback + assert configure is not None + configure({}, {}, {}, a) + assert seen == ["set", "added"] + + def test_none_clears_and_reads_falsy(self) -> None: + """Construction does `self._dynamic_config_callback = None`, and call + sites branch on truthiness.""" + a = agent() + a.add_per_call_config(lambda q, b, h, ag: None) + a._dynamic_config_callback = None + assert a._dynamic_config_callback is None + assert not a._dynamic_config_callback + + def test_a_fresh_agent_has_no_callback(self) -> None: + assert agent()._dynamic_config_callback is None + + +# ── on_call_end ────────────────────────────────────────────────────── + + +class TestOnCallEnd: + """`call_log` is a CONDITIONAL field on a SWAIG request. Without + `swaig_post_conversation` the hangup hook fires, returns 200, and carries + no transcript -- indistinguishable from the hook never running. + """ + + @staticmethod + def _fire(a: AgentBase, payload: dict[str, Any]) -> None: + hook = a._tool_registry._swaig_functions["hangup_hook"] + handler = getattr(hook, "handler", None) + assert handler is not None + handler({}, payload) + + def test_registering_enables_the_payload_parameter(self) -> None: + a = agent() + assert a._params.get("swaig_post_conversation") is None + a.on_call_end(lambda call_log, raw: None) + assert a._params["swaig_post_conversation"] is True + + def test_registering_defines_the_reserved_hook(self) -> None: + a = agent() + a.on_call_end(lambda call_log, raw: None) + assert "hangup_hook" in a._tool_registry._swaig_functions + + def test_handlers_receive_the_log_and_run_in_order(self) -> None: + seen: list[Any] = [] + a = agent() + a.on_call_end(lambda call_log, raw: seen.append(("one", len(call_log)))) + a.on_call_end(lambda call_log, raw: seen.append(("two", raw.get("call_id")))) + self._fire(a, {"call_log": [{"role": "user"}], "call_id": "c-1"}) + assert seen == [("one", 1), ("two", "c-1")] + + def test_raw_call_log_is_accepted_too(self) -> None: + seen: list[int] = [] + a = agent() + a.on_call_end(lambda call_log, raw: seen.append(len(call_log))) + self._fire(a, {"raw_call_log": [{"role": "user"}, {"role": "assistant"}]}) + assert seen == [2] + + def test_one_failing_handler_does_not_stop_the_others(self) -> None: + """A failing teardown handler must not turn into a failed hangup.""" + seen: list[str] = [] + + def boom(call_log: Any, raw: Any) -> None: + raise RuntimeError("boom") + + a = agent() + a.on_call_end(boom) + a.on_call_end(lambda call_log, raw: seen.append("still ran")) + self._fire(a, {"call_log": []}) + assert seen == ["still ran"] + + def test_an_explicit_false_is_not_overridden(self) -> None: + a = agent() + a.set_params({"swaig_post_conversation": False}) + a.on_call_end(lambda call_log, raw: None) + assert a._params["swaig_post_conversation"] is False + + def test_usable_as_a_decorator(self) -> None: + a = agent() + + @a.on_call_end + def handler(call_log: Any, raw: Any) -> None: + return None + + assert callable(handler) + assert "hangup_hook" in a._tool_registry._swaig_functions + + +# ── mount ──────────────────────────────────────────────────────────── + + +class TestMount: + """`get_app()` registers a `/{full_path:path}` catch-all and FastAPI matches + in registration order, so anything mounted afterwards was unreachable. And + `get_app()`'s catch-all answers the agent's own bare route with 204 where + `serve()`'s routes it to the SWML handler -- so mounting anything at all + silently killed the endpoint the platform actually fetches. + """ + + @staticmethod + def _router(path: str) -> APIRouter: + router = APIRouter() + + @router.post(path) + async def _handler() -> dict[str, bool]: + return {"ok": True} + + return router + + def test_mounted_route_is_reachable(self) -> None: + a = agent() + a.mount(self._router("/handoff"), prefix="/myagent/chat") + response = TestClient(a.get_app()).post("/myagent/chat/handoff") + assert response.status_code == 200 + assert response.json() == {"ok": True} + + def test_bare_agent_route_is_not_swallowed(self) -> None: + """204 here means the SWML endpoint is dead. 401 means it is alive and + merely demanding auth, which is correct.""" + a = agent() + a.mount(self._router("/x"), prefix="/myagent/chat") + assert TestClient(a.get_app()).post("/myagent").status_code != 204 + + def test_several_mounts_all_stay_reachable(self) -> None: + a = agent() + a.mount(self._router("/one"), prefix="/myagent/a") + a.mount(self._router("/two"), prefix="/myagent/b") + client = TestClient(a.get_app()) + assert client.post("/myagent/a/one").status_code == 200 + assert client.post("/myagent/b/two").status_code == 200 + + def test_health_endpoints_survive(self) -> None: + a = agent() + a.mount(self._router("/x"), prefix="/myagent/chat") + assert TestClient(a.get_app()).get("/health").status_code == 200 + + def test_mount_returns_self_for_chaining(self) -> None: + a = agent() + assert a.mount(self._router("/x"), prefix="/myagent/c") is a diff --git a/tests/unit/core/test_capabilities.py b/tests/unit/core/test_capabilities.py new file mode 100644 index 00000000..64335686 --- /dev/null +++ b/tests/unit/core/test_capabilities.py @@ -0,0 +1,102 @@ +"""Reading what a client declares it can render. + +Both ends of this convention are SignalWire's -- the address widget writes it, +the SDK reads it -- which is the only reason it belongs here rather than in +each application. + +The rule these tests exist to pin is that **absence means no**. Every path +resolves malformed or missing data to "not declared", because offering a caller +something they cannot reach is worse than never mentioning it: a PSTN caller +has no browser, and an agent that offers to put something on their screen has +simply lied to them. +""" + +from typing import Any + +import pytest + +from signalwire.core.capabilities import ( + declared_capabilities, + has_capability, + user_variables, +) + +BODY: dict[str, Any] = { + "vars": { + "userVariables": { + "capabilities": { + "display_content": True, + "transcript": True, + "chat_handoff": False, + }, + "metadata": {"widget": {"opened_at": "2026-01-01T00:00:00Z"}}, + } + } +} + + +class TestUserVariables: + def test_extracts_from_the_nested_shape(self) -> None: + assert "capabilities" in user_variables(BODY) + + @pytest.mark.parametrize( + "junk", + [ + None, + {}, + "nonsense", + 42, + {"vars": None}, + {"vars": {}}, + {"vars": {"userVariables": None}}, + {"vars": {"userVariables": "not a dict"}}, + ], + ) + def test_missing_levels_yield_an_empty_dict(self, junk: Any) -> None: + assert user_variables(junk) == {} + + +class TestDeclaredCapabilities: + def test_only_truthy_names_are_returned(self) -> None: + assert declared_capabilities(BODY) == frozenset( + {"display_content", "transcript"} + ) + + def test_false_is_not_a_declaration(self) -> None: + assert "chat_handoff" not in declared_capabilities(BODY) + + def test_accepts_already_extracted_user_variables(self) -> None: + """Callers hold one or the other depending on where they are.""" + assert declared_capabilities({"capabilities": {"a": True}}) == frozenset({"a"}) + + def test_a_name_this_sdk_has_never_heard_of_still_passes_through(self) -> None: + """The producer evolves by adding booleans; an SDK release per + capability would invert that.""" + assert has_capability({"capabilities": {"future_thing": True}}, "future_thing") + + @pytest.mark.parametrize( + "junk", + [ + None, + {}, + "nonsense", + 42, + {"vars": {"userVariables": {"capabilities": "not a dict"}}}, + {"vars": {"userVariables": {"capabilities": None}}}, + {"vars": {"userVariables": {}}}, + ], + ) + def test_absence_and_malformation_both_mean_no(self, junk: Any) -> None: + assert declared_capabilities(junk) == frozenset() + assert not has_capability(junk, "display_content") + + +class TestHasCapability: + def test_declared(self) -> None: + assert has_capability(BODY, "display_content") + + def test_declared_false(self) -> None: + assert not has_capability(BODY, "chat_handoff") + + def test_never_mentioned(self) -> None: + assert not has_capability(BODY, "telepathy") diff --git a/tests/unit/core/test_post_prompt_normalize.py b/tests/unit/core/test_post_prompt_normalize.py new file mode 100644 index 00000000..6df6b83b --- /dev/null +++ b/tests/unit/core/test_post_prompt_normalize.py @@ -0,0 +1,178 @@ +"""Post-prompt normalization across the voice and chat engines. + +The two engines emit the same artifact in different shapes, and the differences +are not discoverable from either end: a caller who handles one is silently +wrong about the other. ``post_prompt_data`` alone arrives three ways, and one +of them -- the object wrapped in a list under ``parsed`` -- survives every +structural check while missing every field lookup. + +The chat engine additionally appends its own summary to ``call_log`` as a bare +``role: assistant`` turn. By role alone it is indistinguishable from real +speech; replayed into another medium the agent narrates a summary of itself in +the third person. Only a byte comparison against ``post_prompt_data.raw`` +identifies it. +""" + +from typing import Any, ClassVar + +import pytest + +from signalwire.core.post_prompt import ( + dialogue_turns, + normalize_post_prompt, + parse_post_prompt_data, + strip_json_fence, +) + +FENCED = '```json\n{"summary": "s", "already_answered": ["pricing"]}\n```' + + +class TestParseShapes: + def test_flat_keys_from_the_voice_engine(self) -> None: + assert parse_post_prompt_data({"summary": "s", "user_goal": "g"}) == { + "summary": "s", + "user_goal": "g", + } + + def test_fenced_raw_from_the_chat_engine(self) -> None: + assert parse_post_prompt_data({"raw": FENCED}) == { + "summary": "s", + "already_answered": ["pricing"], + } + + def test_object_wrapped_in_a_list_under_parsed(self) -> None: + """The shape that structurally survives and semantically vanishes.""" + assert parse_post_prompt_data( + {"parsed": [{"summary": "s3"}], "raw": "..."} + ) == {"summary": "s3"} + + def test_parsed_wrapper_wins_over_the_generic_sweep(self) -> None: + """Without the unwrap this returns {"parsed": [...]} -- fine to look at, + useless to read.""" + result = parse_post_prompt_data({"parsed": [{"summary": "s"}]}) + assert "parsed" not in result + + def test_parsed_as_a_bare_dict(self) -> None: + assert parse_post_prompt_data({"parsed": {"summary": "s"}}) == {"summary": "s"} + + def test_prose_instead_of_json_is_kept(self) -> None: + """A usable paragraph beats a discarded one.""" + assert parse_post_prompt_data({"raw": "They asked about pricing."}) == { + "summary": "They asked about pricing." + } + + def test_json_that_is_not_an_object(self) -> None: + assert parse_post_prompt_data({"raw": '"just a string"'}) == { + "summary": "just a string" + } + + @pytest.mark.parametrize( + "junk", [None, {}, "text", 42, [], {"raw": ""}, {"raw": " "}, {"raw": None}] + ) + def test_junk_degrades_rather_than_raising(self, junk: Any) -> None: + """The conversation is already over; there is nobody to show an error to.""" + assert parse_post_prompt_data(junk) == {} + + +class TestStripFence: + @pytest.mark.parametrize( + ("raw", "expected"), + [ + ('```json\n{"a":1}\n```', '{"a":1}'), + ("```\nplain\n```", "plain"), + ("no fence at all", "no fence at all"), + ("", ""), + ], + ) + def test_unwraps(self, raw: str, expected: str) -> None: + assert strip_json_fence(raw) == expected + + +class TestDialogueTurns: + # Deliberately heterogeneous: the last entry is not a dict at all, + # which is exactly the malformed input this must survive. + LOG: ClassVar[list[Any]] = [ + {"role": "user", "content": "hi"}, + {"role": "assistant", "content": "hello"}, + {"role": "system", "content": "the prompt"}, + {"role": "system-log", "content": "step trace"}, + {"role": "tool", "content": "tool output"}, + {"role": "assistant", "content": "", "tool_calls": [{"id": 1}]}, + {"role": "assistant-manual", "content": "let me look that up"}, + {"role": "assistant", "content": " "}, + "not even a dict", + ] + + def test_keeps_only_real_dialogue(self) -> None: + assert dialogue_turns(self.LOG) == [ + {"role": "user", "content": "hi"}, + {"role": "assistant", "content": "hello"}, + ] + + def test_drops_the_chat_summary_echo(self) -> None: + log = [*self.LOG, {"role": "assistant", "content": FENCED}] + assert {"role": "assistant", "content": FENCED} not in dialogue_turns( + log, drop_echo=FENCED + ) + + def test_keeps_the_echo_when_not_asked_to_drop_it(self) -> None: + """The voice engine delivers it as a tool call instead, so there is + nothing to drop and a blanket rule would eat real speech.""" + log = [*self.LOG, {"role": "assistant", "content": FENCED}] + assert len(dialogue_turns(log)) == 3 + + @pytest.mark.parametrize("junk", [None, [], "nonsense", 42]) + def test_junk_logs_yield_nothing(self, junk: Any) -> None: + assert dialogue_turns(junk) == [] + + +class TestNormalize: + def test_voice_body(self) -> None: + result = normalize_post_prompt( + { + "conversation_type": "voice", + "call_id": "c-1", + "post_prompt_data": {"parsed": [{"summary": "v"}]}, + "raw_call_log": [{"role": "user", "content": "hi"}], + } + ) + assert result.medium == "voice" + assert result.conversation_id is None # voice does not send one + assert result.summary == {"summary": "v"} + assert result.call_id == "c-1" + assert len(result.dialogue) == 1 + + def test_chat_body(self) -> None: + result = normalize_post_prompt( + { + "conversation_type": "chat", + "conversation_id": "conv-9", + "post_prompt_data": {"raw": FENCED}, + "raw_messages": [ + {"role": "user", "content": "hi"}, + {"role": "assistant", "content": FENCED}, + ], + } + ) + assert result.medium == "chat" + assert result.conversation_id == "conv-9" + assert result.summary["already_answered"] == ["pricing"] + # The echo is gone; only the real turn survives. + assert result.dialogue == [{"role": "user", "content": "hi"}] + + def test_call_log_key_is_also_accepted(self) -> None: + result = normalize_post_prompt( + {"call_log": [{"role": "user", "content": "hi"}]} + ) + assert len(result.dialogue) == 1 + + @pytest.mark.parametrize("junk", [None, "text", 42, []]) + def test_junk_body_yields_empty_fields(self, junk: Any) -> None: + result = normalize_post_prompt(junk) + assert result.medium == "" + assert result.summary == {} + assert result.dialogue == [] + + def test_raw_is_preserved(self) -> None: + body = {"conversation_type": "voice", "extra": "kept"} + assert normalize_post_prompt(body).raw is body diff --git a/tests/unit/core/test_swml_service_getattr.py b/tests/unit/core/test_swml_service_getattr.py new file mode 100644 index 00000000..520c6114 --- /dev/null +++ b/tests/unit/core/test_swml_service_getattr.py @@ -0,0 +1,71 @@ +"""SWMLService.__getattr__: attribute misses must not recurse. + +``__getattr__`` runs on every failed attribute lookup and its body reaches for +``self.log`` and ``self.schema_utils``. Both are assigned during ``__init__``, +and ``log`` well before ``schema_utils`` -- so on a partially constructed +instance, resolving either one re-entered ``__getattr__``, which reached for it +again, until the stack ran out. + +Two things made that expensive to diagnose rather than merely broken: the +traceback named the *dependency* rather than the attribute access that started +it, and an application cannot fix it without overriding ``__getattr__`` on the +base class, which is forking the SDK. + +These tests pin the guard and, just as importantly, pin that the guard did not +turn every legitimate verb lookup into an AttributeError. +""" + +import pytest + +from signalwire.core.swml_service import SWMLService + + +def _unconstructed() -> SWMLService: + """An instance whose __init__ never ran -- neither log nor schema_utils.""" + return SWMLService.__new__(SWMLService) + + +class TestNoRecursion: + def test_missing_attribute_raises_rather_than_recursing(self) -> None: + with pytest.raises(AttributeError): + _ = _unconstructed().some_typo_attribute + + def test_error_names_the_attribute_actually_requested(self) -> None: + """Not the dependency that happened to be missing underneath it.""" + with pytest.raises(AttributeError) as exc: + _ = _unconstructed().some_typo_attribute + assert "some_typo_attribute" in str(exc.value) + assert "schema_utils" not in str(exc.value) + + @pytest.mark.parametrize("name", ["log", "schema_utils", "_verb_methods_cache"]) + def test_own_dependencies_short_circuit(self, name: str) -> None: + """The three names __getattr__ needs can never be resolved through it.""" + with pytest.raises(AttributeError): + getattr(_unconstructed(), name) + + @pytest.mark.parametrize("name", ["__deepcopy__", "__copy__", "__wrapped__"]) + def test_dunder_probes_are_rejected_cheaply(self, name: str) -> None: + """copy/pickle/inspect probe these constantly; none can be a SWML verb. + + Only dunders that `object` does not itself provide are listed -- + `__getstate__` exists on every object from Python 3.11, so it resolves + before `__getattr__` is ever consulted. + """ + with pytest.raises(AttributeError): + getattr(_unconstructed(), name) + + def test_hasattr_is_false_rather_than_exploding(self) -> None: + assert not hasattr(_unconstructed(), "definitely_not_here") + + +class TestVerbLookupStillWorks: + """The guard must not break the feature __getattr__ exists for.""" + + def test_valid_verb_resolves_to_a_callable(self) -> None: + service = SWMLService(name="t", route="/t", schema_validation=False) + assert callable(service.answer) + + def test_invalid_attribute_on_a_live_instance_still_raises(self) -> None: + service = SWMLService(name="t", route="/t", schema_validation=False) + with pytest.raises(AttributeError): + _ = service.not_a_verb_at_all