diff --git a/tests/adapters/nooa/reproduction-evidence.md b/tests/adapters/nooa/reproduction-evidence.md new file mode 100644 index 000000000..ce837f1d8 --- /dev/null +++ b/tests/adapters/nooa/reproduction-evidence.md @@ -0,0 +1,66 @@ +# NOOA reproduction evidence + +Base revision: `69e15a20fcd69da8a1241a004062aa28637bcd41`. +Environment: Python 3.12.12, NOOA 0.0.10, Pydantic 2.14.1, pytest 9.1.1. +The tests run real NOOA generation/execution with scripted model responses. +Band transport is replaced with existing test doubles; no live model or platform +was used. Production code is unchanged. + +## Focused result + +```sh +PYTHONPATH=src python -m pytest tests/adapters/nooa -q -o addopts='' --tb=short +``` + +**48 passed, 6 failed.** The original 38 cases pass. The 16 added cases comprise +10 passing controls/characterizations and six failing regressions for two defects. +The failing cases are deliberately not skipped or marked `xfail`. + +| Test file | Cases | Result and interpretation | +|---|---|---| +| [test_validation_aliases.py](test_validation_aliases.py) | 9 | Five alias variants fail before the handler runs. Plain fields, ordinary aliases, serialization-only aliases, and strict rejection of an invalid integer pass. | +| [test_bootstrap_retry.py](test_bootstrap_retry.py) | 2 | Normal bootstrap passes. After a constructor fails once, the next successful agent loses earlier history because the coordinator already committed bootstrap context. | +| [test_resource_lifecycle.py](test_resource_lifecycle.py) | 3 | Passing characterizations: an owned worker remains running after room removal, session replacement, or adapter shutdown. These document the absent teardown convention, not a promised universal `close()` API. Test teardown stops every worker. | +| [test_thought_cancellation.py](test_thought_cancellation.py) | 2 | Passing characterizations: an unscoped fake thought send stalls until released; real `AgentTools` bound to a terminated scope cancel before REST dispatch. The fake-only stall is not proof of a production shutdown defect. | + +The alias failures share this pytest prefix: + +```text +tests/adapters/nooa/test_validation_aliases.py::TestValidationAliases::test_advertised_field_name_calls_handler_once_with_validated_input +``` + +| Parameter | Observed error | +|---|---| +| `[validation-alias]` | `orderId: Field required`; handler receives zero calls | +| `[alias-choices]` | `orderId: Field required`; handler receives zero calls | +| `[alias-path]` | `order.ids.0: Field required`; handler receives zero calls | +| `[choices-with-path]` | `order.ids.0: Field required`; handler receives zero calls | +| `[distinct-validation-and-serialization]` | `orderId: Field required`; handler receives zero calls | + +The sixth failure is: + +```text +tests/adapters/nooa/test_bootstrap_retry.py::TestBootstrapRetry::test_history_reaches_first_successful_room_agent[constructor-retry] +``` + +It expects the earlier note `The deployment window is named BLUE-PINE.` in the +model prompt. That assertion fails after the constructor retry. This case uses +the real Band coordinator, preprocessor, execution context, and NOOA adapter; +it does not assert live-server automatic redelivery behavior. + +## Other checks and limits + +Checks used Ruff 0.17.0 and Pyrefly 1.3.2 in a minimal NOOA review environment. + +| Check | Outcome | +|---|---| +| Ruff check of the five added Python files | Passed | +| Repository-wide `ruff format --check .` | Passed; 1,400 Python files already formatted | +| Repository-wide `ruff check .` | One finding in unchanged `tests/runtime/test_tools.py:906`: an unused `DTZ001` noqa under Ruff 0.17.0 | +| `pyrefly check` | Not passing: 180 diagnostics, including 157 missing imports; this environment lacks the full development extras. Tests are excluded by the repository's Pyrefly configuration. | +| Unit suite excluding `tests/integration/` and `tests/e2e/` | Stopped during collection with 90 errors in the minimal environment, including missing `mcp`; no full-suite pass is claimed | +| Live integration / E2E | Not run | + +The focused suite's failures reproduce the defects. The broad checks above do not +establish a clean repository-wide validation baseline. The reproduction branch +is intended for review before implementation fixes and is not merge-ready. diff --git a/tests/adapters/nooa/resources.py b/tests/adapters/nooa/resources.py new file mode 100644 index 000000000..9931eda27 --- /dev/null +++ b/tests/adapters/nooa/resources.py @@ -0,0 +1,88 @@ +"""Owned asynchronous workers and gated telemetry for lifecycle probes.""" + +from __future__ import annotations + +import asyncio +from typing import Any + +from nooa.unifiedllm import FakeLLMClient + +from band.adapters.nooa import NooaRoomAgent +from band.core.types import MessageType +from band.testing import FakeAgentTools +from band.testing.nooa import room_agent_class + + +class OwnedWorker: + """A persistent background task with an explicit owner-supplied close method.""" + + def __init__(self) -> None: + self.started = asyncio.Event() + self.closed = asyncio.Event() + self.task = asyncio.create_task(self.run()) + + async def run(self) -> None: + self.started.set() + try: + await asyncio.Event().wait() + finally: + self.closed.set() + + @property + def running(self) -> bool: + return self.started.is_set() and not self.task.done() + + async def close(self) -> None: + self.task.cancel() + await asyncio.gather(self.task, return_exceptions=True) + + +class WorkerOwner: + """Keeps external handles so a missing adapter teardown cannot leak test tasks.""" + + def __init__(self) -> None: + self.workers: list[OwnedWorker] = [] + self.agent_close_calls = 0 + + def agent_class(self) -> type[NooaRoomAgent]: + owner = self + + async def respond(agent: NooaRoomAgent, message: str) -> None: + worker = OwnedWorker() + agent.worker = worker + owner.workers.append(worker) + await worker.started.wait() + await agent.room.band_no_reply() + + async def close(agent: NooaRoomAgent) -> None: + owner.agent_close_calls += 1 + await agent.worker.close() + + return room_agent_class( + FakeLLMClient(), + namespace={"respond": respond, "close": close}, + ) + + async def close(self) -> None: + for worker in self.workers: + await worker.close() + + +class BlockedThoughtTools(FakeAgentTools): + """Fake tools with no execution-scope guard and an event-controlled thought send.""" + + def __init__(self) -> None: + super().__init__() + self.posting = asyncio.Event() + self.release = asyncio.Event() + + async def send_event( + self, + content: str, + message_type: str, + metadata: dict[str, Any] | None = None, + ) -> dict[str, Any]: + if message_type == MessageType.THOUGHT: + self.posting.set() + await self.release.wait() + return await super().send_event(content, message_type, metadata) diff --git a/tests/adapters/nooa/test_bootstrap_retry.py b/tests/adapters/nooa/test_bootstrap_retry.py new file mode 100644 index 000000000..8e94e8b7d --- /dev/null +++ b/tests/adapters/nooa/test_bootstrap_retry.py @@ -0,0 +1,65 @@ +"""A room whose agent could not be constructed still needs its bootstrap history.""" + +from __future__ import annotations + +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from band.adapters.nooa import NooaAdapter, NooaRoomAgent +from band.agent import Agent +from band.runtime.execution import ExecutionContext +from band.testing.nooa import room_agent_class, room_call, scripted_llm +from tests.adapters.nooa.kit import ROOM_ID, prompt_text +from tests.conftest import make_message_event + + +class TestBootstrapRetry: + @pytest.mark.parametrize( + "fail_first", [False, True], ids=["normal-bootstrap", "constructor-retry"] + ) + async def test_history_reaches_first_successful_room_agent( + self, mock_rest_client: MagicMock, fail_first: bool + ) -> None: + attempts = 0 + + def initialize(agent: NooaRoomAgent) -> None: + nonlocal attempts + attempts += 1 + if fail_first and attempts == 1: + raise RuntimeError("Agent dependency temporarily unavailable") + NooaRoomAgent.__init__(agent) + + llm = scripted_llm(room_call("band_no_reply")) + adapter = NooaAdapter( + agent=room_agent_class(llm, namespace={"__init__": initialize}), + emit=(), + ) + await adapter.on_started("Agent", "A test agent") + agent = Agent(runtime=MagicMock(agent_id="agent-123"), adapter=adapter) + ctx = ExecutionContext(ROOM_ID, MagicMock(rest=mock_rest_client), AsyncMock()) + ctx.set_participants([{"id": "user-456", "name": "Alice", "type": "User"}]) + event = make_message_event(room_id=ROOM_ID) + note = "The deployment window is named BLUE-PINE." + ctx.get_context = AsyncMock( + return_value=MagicMock( + messages=[ + { + "id": "earlier-message", + "sender_name": "Alice", + "sender_type": "User", + "content": note, + }, + {"id": event.payload.id, "content": event.payload.content}, + ] + ) + ) + + if fail_first: + with pytest.raises( + RuntimeError, match="dependency temporarily unavailable" + ): + await agent._on_execute(ctx, event) + await agent._on_execute(ctx, event) + + assert note in prompt_text(llm) diff --git a/tests/adapters/nooa/test_resource_lifecycle.py b/tests/adapters/nooa/test_resource_lifecycle.py new file mode 100644 index 000000000..3b1e57de4 --- /dev/null +++ b/tests/adapters/nooa/test_resource_lifecycle.py @@ -0,0 +1,46 @@ +"""Characterize resource ownership until the adapter defines a teardown contract.""" + +from __future__ import annotations + +import pytest + +from band.adapters.nooa import NooaAdapter +from band.testing import FakeAgentTools +from tests.adapters.nooa.kit import ROOM_ID, run_turn +from tests.adapters.nooa.resources import WorkerOwner + + +class TestResourceLifecycle: + @pytest.mark.parametrize( + "boundary", ["room-removal", "session-replacement", "shutdown"] + ) + async def test_owned_worker_still_requires_explicit_owner_teardown( + self, boundary: str + ) -> None: + owner = WorkerOwner() + adapter = NooaAdapter(agent=owner.agent_class(), emit=()) + await adapter.on_started("Agent", "A resource-owning agent") + + try: + await run_turn(adapter, FakeAgentTools()) + worker = owner.workers[0] + + match boundary: + case "room-removal": + await adapter.on_cleanup(ROOM_ID) + await adapter.on_cleanup(ROOM_ID) + case "session-replacement": + await run_turn(adapter, FakeAgentTools(), is_session_bootstrap=True) + assert len(owner.workers) == 2 + case "shutdown": + await adapter.cleanup_all() + await adapter.cleanup_all() + + assert owner.agent_close_calls == 0 + assert worker.running + assert not worker.closed.is_set() + finally: + await owner.close() + + assert worker.closed.is_set() + assert not worker.running diff --git a/tests/adapters/nooa/test_thought_cancellation.py b/tests/adapters/nooa/test_thought_cancellation.py new file mode 100644 index 000000000..802f34fd3 --- /dev/null +++ b/tests/adapters/nooa/test_thought_cancellation.py @@ -0,0 +1,75 @@ +"""Separate unscoped fake-tool cancellation from the production post-scope guard.""" + +from __future__ import annotations + +import asyncio +from unittest.mock import AsyncMock, MagicMock + +import pytest +from nooa.unifiedllm import FakeLLMClient + +from band.adapters.nooa import NooaAdapter +from band.core.types import Emit +from band.runtime.execution import ExecutionContext +from band.runtime.tools.agent import AgentTools +from band.testing.nooa import code_turn, room_agent_class +from tests.adapters.nooa.kit import ROOM_ID, carrying, room_message +from tests.adapters.nooa.resources import BlockedThoughtTools + + +class TestThoughtCancellation: + @pytest.mark.parametrize( + "scoped", [False, True], ids=["unscoped-fake", "real-scoped"] + ) + async def test_cancelled_thought_publication_respects_execution_scope( + self, scoped: bool, mock_rest_client: MagicMock + ) -> None: + entered = asyncio.Event() + llm = FakeLLMClient( + [ + code_turn( + "self.entered.set()\nawait asyncio.Event().wait()", + reasoning="Working on the request.", + ) + ] + ) + adapter = NooaAdapter( + agent=room_agent_class(llm, namespace=carrying("entered", entered)), + emit={Emit.THOUGHTS}, + ) + await adapter.on_started("Agent", "A test agent") + ctx = ExecutionContext(ROOM_ID, MagicMock(rest=mock_rest_client), AsyncMock()) + await ctx._begin_scope() + fake_tools = BlockedThoughtTools() + tools = AgentTools.from_context(ctx) if scoped else fake_tools + task = asyncio.create_task( + adapter.on_message( + room_message(), + tools, + [], + None, + None, + is_session_bootstrap=True, + room_id=ROOM_ID, + ) + ) + + try: + await asyncio.wait_for(entered.wait(), timeout=5) + if scoped: + assert ctx.current_scope is not None + ctx.current_scope.terminate(shutdown=True) + task.cancel() + + if not scoped: + await asyncio.wait_for(fake_tools.posting.wait(), timeout=5) + assert not task.done() + fake_tools.release.set() + + with pytest.raises(asyncio.CancelledError): + await asyncio.wait_for(task, timeout=5) + mock_rest_client.agent_api_events.create_agent_chat_event.assert_not_awaited() + finally: + fake_tools.release.set() + task.cancel() + await asyncio.gather(task, return_exceptions=True) diff --git a/tests/adapters/nooa/test_validation_aliases.py b/tests/adapters/nooa/test_validation_aliases.py new file mode 100644 index 000000000..605e6daea --- /dev/null +++ b/tests/adapters/nooa/test_validation_aliases.py @@ -0,0 +1,117 @@ +"""Advertised Python arguments must reach custom handlers as validated models.""" + +from __future__ import annotations + +from typing import Any + +import pytest +from pydantic import AliasChoices, AliasPath, BaseModel, ConfigDict, Field, create_model +from pydantic.fields import FieldInfo + +from band.runtime.tools import BandTool +from band.testing import FakeAgentTools +from band.testing.nooa import room_call, scripted_llm +from tests.adapters.nooa.kit import prompt_text, run_turn, started_adapter + + +class TestValidationAliases: + @pytest.mark.parametrize( + ("field", "validation_input"), + [ + pytest.param(Field(), {"order_id": "A1"}, id="field-name"), + pytest.param(Field(alias="orderId"), {"orderId": "A1"}, id="alias"), + pytest.param( + Field(serialization_alias="orderId"), + {"order_id": "A1"}, + id="serialization-only", + ), + pytest.param( + Field(validation_alias="orderId"), + {"orderId": "A1"}, + id="validation-alias", + ), + pytest.param( + Field(validation_alias=AliasChoices("orderId", "legacyId")), + {"legacyId": "A1"}, + id="alias-choices", + ), + pytest.param( + Field(validation_alias=AliasPath("order", "ids", 0)), + {"order": {"ids": ["A1"]}}, + id="alias-path", + ), + pytest.param( + Field( + validation_alias=AliasChoices( + AliasPath("order", "ids", 0), "orderId" + ) + ), + {"orderId": "A1"}, + id="choices-with-path", + ), + pytest.param( + Field( + alias="legacyId", + validation_alias="orderId", + serialization_alias="outputId", + ), + {"orderId": "A1"}, + id="distinct-validation-and-serialization", + ), + ], + ) + async def test_advertised_field_name_calls_handler_once_with_validated_input( + self, + tools: FakeAgentTools, + field: FieldInfo, + validation_input: dict[str, Any], + ) -> None: + input_model = create_model( + "LookupInput", + __doc__="Look up an order.", + __config__=ConfigDict(strict=True), + order_id=(str, field), + ) + expected = input_model.model_validate(validation_input) + received: list[BaseModel] = [] + + def lookup(args: BaseModel) -> str: + received.append(args) + return f"ORDER_OK:{args.model_dump()['order_id']}" + + llm = scripted_llm( + f"print({room_call('lookup', {'order_id': 'A1'})})", + room_call(BandTool.NO_REPLY), + ) + adapter = await started_adapter(llm, additional_tools=[(input_model, lookup)]) + + await run_turn(adapter, tools) + + assert received == [expected] + assert "ORDER_OK:A1" in prompt_text(llm) + + async def test_invalid_field_value_does_not_reach_handler( + self, tools: FakeAgentTools + ) -> None: + input_model = create_model( + "LookupInput", + __doc__="Look up an order.", + __config__=ConfigDict(strict=True), + order_id=(int, Field(alias="orderId")), + ) + received: list[BaseModel] = [] + + def lookup(args: BaseModel) -> str: + received.append(args) + return "unexpected" + + llm = scripted_llm( + room_call("lookup", {"order_id": "123"}), + room_call(BandTool.NO_REPLY), + ) + adapter = await started_adapter(llm, additional_tools=[(input_model, lookup)]) + + await run_turn(adapter, tools) + + assert received == [] + assert "Input should be a valid integer" in prompt_text(llm)